Fix doc_type names and document inheritance with Index

Author: honzakral

Changelog.rst

@@ -6,6 +6,7 @@ Changelog
 6.3.0 (dev)
 -----------
 
+* Fixed logic around defining a different ``doc_type`` name.
 * Added ``retry_on_conflict`` parameter to ``Document.update``.
 
 6.2.1 (2018-07-03)

docs/persistence.rst

@@ -334,6 +334,39 @@ the index, its name, settings and other attributes:
 ``aliases``
   dictionary with any aliases definitions
 
+Document Inheritance
+~~~~~~~~~~~~~~~~~~~~
+
+You can use standard Python inheritance to extend models, this can be useful in
+a few scenarios. For example if you want to have a ``BaseDocument`` defining some common fields that several different ``Document`` classes should share:
+
+.. code:: python
+
+    class User(InnerDoc):
+        username = Text(fields={'keyword': Keyword()})
+        email = Text()
+
+    class BaseDocument(Document):
+        created_by = Object(User)
+        created_date = Date()
+        last_updated = Date()
+
+        def save(**kwargs):
+            if not self.created_date:
+                self.created_date = datetime.now()
+            self.last_updated = datetime.now()
+            return super(BaseDocument, self).save(**kwargs)
+
+    class BlogPost(BaseDocument):
+        class Index:
+            name = 'blog'
+
+Another use case would be using the `join type
+<https://www.elastic.co/guide/en/elasticsearch/reference/current/parent-join.html>`_
+to have multiple different entities in a single index. You can see an `example
+<https://github.com/elastic/elasticsearch-dsl-py/blob/master/examples/parent_child.py>`_
+of this approach.
+
 .. _index:
 
 Index

elasticsearch_dsl/document.py

@@ -10,7 +10,7 @@
 from .search import Search
 from .connections import connections
 from .exceptions import ValidationException, IllegalOperation
-from .index import Index, DEFAULT_INDEX
+from .index import Index
 
 
 class MetaField(object):
@@ -25,24 +25,30 @@ def __new__(cls, name, bases, attrs):
         return super(DocumentMeta, cls).__new__(cls, name, bases, attrs)
 
 class IndexMeta(DocumentMeta):
+    # global flag to guard us from associating an Index with the base Document
+    # class, only user defined subclasses should have an _index attr
+    _document_initialized = False
+
     def __new__(cls, name, bases, attrs):
-        index_opts = attrs.pop('Index', None)
         new_cls = super(IndexMeta, cls).__new__(cls, name, bases, attrs)
-        new_cls._index = cls.construct_index(index_opts, bases)
-        new_cls._index.document(new_cls)
+        if cls._document_initialized:
+            index_opts = attrs.pop('Index', None)
+            new_cls._index = cls.construct_index(index_opts, bases)
+            new_cls._index.document(new_cls)
+        cls._document_initialized = True
         return new_cls
 
     @classmethod
     def construct_index(cls, opts, bases):
         if opts is None:
             for b in bases:
-                if getattr(b, '_index', DEFAULT_INDEX) is not DEFAULT_INDEX:
+                if hasattr(b, '_index'):
                     return b._index
-            return DEFAULT_INDEX
+            # create an all-matching index pattern
+            return Index('*')
 
         i = Index(
             getattr(opts, 'name', '*'),
-            doc_type=getattr(opts, 'doc_type', 'doc'),
             using=getattr(opts, 'using', 'default')
         )
         i.settings(**getattr(opts, 'settings', {}))

elasticsearch_dsl/index.py

@@ -3,6 +3,8 @@
 from .exceptions import IllegalOperation
 from .mapping import Mapping
 
+DEFAULT_DOC_TYPE = 'doc'
+
 class IndexTemplate(object):
     def __init__(self, name, template, index=None, **kwargs):
         if index is None:
@@ -28,7 +30,7 @@ def save(self, using=None):
         es.indices.put_template(name=self._template_name, body=self.to_dict())
 
 class Index(object):
-    def __init__(self, name, doc_type='doc', using='default'):
+    def __init__(self, name, doc_type=DEFAULT_DOC_TYPE, using='default'):
         """
         :arg name: name of the index
         :arg using: connection alias to use, defaults to ``'default'``
@@ -39,7 +41,14 @@ def __init__(self, name, doc_type='doc', using='default'):
         self._settings = {}
         self._aliases = {}
         self._analysis = {}
-        self._mapping = Mapping(doc_type)
+        self._mapping = None
+        if doc_type is not DEFAULT_DOC_TYPE:
+            self._mapping = Mapping(doc_type)
+
+    def get_or_create_mapping(self, doc_type=DEFAULT_DOC_TYPE):
+        if self._mapping is None:
+            self._mapping = Mapping(doc_type)
+        return self._mapping
 
     def as_template(self, template_name, pattern=None):
         # TODO: should we allow pattern to be a top-level arg?
@@ -48,10 +57,10 @@ def as_template(self, template_name, pattern=None):
         return IndexTemplate(template_name, pattern or self._name, index=self)
 
     def resolve_field(self, field_path):
-        return self._mapping.resolve_field(field_path)
+        return self.get_or_create_mapping().resolve_field(field_path)
 
     def load_mappings(self, using=None):
-        self._mapping.update_from_es(self._name, using=using or self._using)
+        self.get_or_create_mapping().update_from_es(self._name, using=using or self._using)
 
     def clone(self, name=None, doc_type=None, using=None):
         """
@@ -68,14 +77,18 @@ def clone(self, name=None, doc_type=None, using=None):
         :arg name: name of the index
         :arg using: connection alias to use, defaults to ``'default'``
         """
+        doc_type = doc_type or (
+            DEFAULT_DOC_TYPE if self._mapping is None else self._mapping.doc_type
+        )
         i = Index(name or self._name,
-                  doc_type=doc_type or self._mapping.doc_type,
+                  doc_type=doc_type,
                   using=using or self._using)
         i._settings = self._settings.copy()
         i._aliases = self._aliases.copy()
         i._analysis = self._analysis.copy()
         i._doc_types = self._doc_types[:]
-        i._mapping = self._mapping._clone()
+        if self._mapping is not None:
+            i._mapping = self._mapping._clone()
         return i
 
     def _get_connection(self, using=None):
@@ -89,11 +102,11 @@ def mapping(self, mapping):
         This means that, when this index is created, it will contain the
         mappings for the document type defined by those mappings.
         """
-        if mapping.doc_type != self._mapping.doc_type:
+        if self._mapping is not None and mapping.doc_type != self._mapping.doc_type:
             raise IllegalOperation(
                 'Index object cannot have multiple types, %s already set, '
                 'trying to assign %s.' % (self._mapping.doc_type, mapping.doc_type))
-        self._mapping.update(mapping)
+        self.get_or_create_mapping(mapping.doc_type).update(mapping)
 
     def document(self, document):
         """
@@ -117,17 +130,14 @@ class Post(Document):
             s = i.search()
         """
         name = document._doc_type.name
-        if name != self._mapping.doc_type:
+        if self._mapping is not None and name != self._mapping.doc_type:
             raise IllegalOperation(
                 'Index object cannot have multiple types, %s already set, '
                 'trying to assign %s.' % (self._mapping.doc_type, name))
         self._doc_types.append(document)
         # TODO: do this at save time to allow Document to be modified after
         # creation?
-        self._mapping.update(document._doc_type.mapping)
-
-        if document._index is DEFAULT_INDEX:
-            document._index = self
+        self.get_or_create_mapping(document._doc_type.name).update(document._doc_type.mapping)
         return document
     doc_type = document
 
@@ -188,9 +198,9 @@ def to_dict(self):
             out['settings'] = self._settings
         if self._aliases:
             out['aliases'] = self._aliases
-        mappings = self._mapping.to_dict()
-        analysis = self._mapping._collect_analysis()
-        if mappings[self._mapping.doc_type]:
+        mappings = self._mapping.to_dict() if self._mapping else None
+        analysis = self._mapping._collect_analysis() if self._mapping else {}
+        if mappings and mappings[self._mapping.doc_type]:
             out['mappings'] = mappings
         if analysis or self._analysis:
             for key in self._analysis:
@@ -548,5 +558,3 @@ def shrink(self, using=None, **kwargs):
         ``Elasticsearch.indices.shrink`` unchanged.
         """
         return self._get_connection(using).indices.shrink(index=self._name, **kwargs)
-
-DEFAULT_INDEX = Index('*')

elasticsearch_dsl/utils.py

@@ -396,16 +396,24 @@ def __getattr__(self, name):
                     return value
             raise
 
+    def __get_field(self, name):
+        try:
+            return self._doc_type.mapping[name]
+        except KeyError:
+            # fallback to fields on the Index
+            if hasattr(self, '_index') and self._index._mapping:
+                try:
+                    return self._index._mapping[name]
+                except KeyError:
+                    pass
+
     def to_dict(self, skip_empty=True):
         out = {}
         for k, v in iteritems(self._d_):
-            try:
-                f = self._doc_type.mapping[k]
-            except KeyError:
-                pass
-            else:
-                if f._coerce:
-                    v = f.serialize(v)
+            # if this is a mapped field,
+            f = self.__get_field(k)
+            if f and f._coerce:
+                v = f.serialize(v)
 
             # if someone assigned AttrList, unwrap it
             if isinstance(v, AttrList):
@@ -420,11 +428,32 @@ def to_dict(self, skip_empty=True):
             out[k] = v
         return out
 
-    def clean_fields(self):
-        errors = {}
+    def __list_fields(self):
+        """
+        Get all the fields defined for our class, if we have an Index, try
+        looking at the index mappings as well, mark the fields from Index as
+        optional.
+        """
         for name in self._doc_type.mapping:
             field = self._doc_type.mapping[name]
+            yield name, field, False
+
+        if hasattr(self, '_index'):
+            if not self._index._mapping:
+                return
+            for name in self._index._mapping:
+                # don't return fields that are in _doc_type
+                if name in self._doc_type.mapping:
+                    continue
+                field = self._index._mapping[name]
+                yield name, field, True
+
+    def clean_fields(self):
+        errors = {}
+        for name, field, optional in self.__list_fields():
             data = self._d_.get(name, None)
+            if data is None and optional:
+                continue
             try:
                 # save the cleaned value
                 data = field.clean(data)

test_elasticsearch_dsl/test_document.py

@@ -72,8 +72,18 @@ def test_document_can_redefine_doc_type():
     class D(document.Document):
         class Meta:
             doc_type = 'not-doc'
-        class Index:
-            doc_type = 'not-doc'
+    assert D._index._mapping.doc_type == 'not-doc'
+
+def test_document_cannot_specify_different_doc_type_if_index_defined():
+    # this will initiate ._index with doc_type = 'doc'
+    class C(document.Document):
+        pass
+
+    with raises(IllegalOperation):
+        class D(C):
+            class Meta:
+                doc_type = 'not-doc'
+
 
 def test_ip_address_serializes_properly():
     host = Host(ip=ipaddress.IPv4Address(u'10.0.0.1'))
@@ -459,9 +469,6 @@ def test_index_inheritance():
     assert issubclass(MyMultiSubDoc, document.Document)
     assert hasattr(MyMultiSubDoc, '_doc_type')
     assert hasattr(MyMultiSubDoc, '_index')
-    # index and using should be
-    assert MyMultiSubDoc._index._name == MySubDoc._index._name
-    assert MyMultiSubDoc._index._using == MySubDoc._index._using
     assert {
         'doc': {
             'properties': {