Repository: Scille/umongo
Pull request #270 (2020-05-06 22:09:20)

Remove allow_inheritance
This feature behaves differently on `Document` and `EmbeddedDocument`.

As I was about to make both consistent, I realized that I didn't really see the point of the feature. It seems to only serve the purpose of triggering an exception if someone tries to inherit a document class that was not meant to. It has no influence on the `cls` field.

Let's assume we know what we're doing and, rather than default to `allow_inheritance=True` for `Document` to match `EmbeddedDocument`, remove the feature.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/30611251/badge)](https://coveralls.io/builds/30611251)

Coverage increased (+0.07%) to 95.671% when pulling **ebb4962e8af44ff17dae9670cc065d05e902dd20 on remove_allow_inheritance** into **8a038c195292d9f64dd52d49e22551cd7fe7a19d on master**.


Patch:
diff --git a/docs/userguide.rst b/docs/userguide.rst
index 1bba8ec..a2790f4 100644
--- a/docs/userguide.rst
+++ b/docs/userguide.rst
@@ -105,7 +105,6 @@ Object orientation means inheritance, of course you can do that
         birthday = fields.DateTimeField()
 
         class Meta:
-            allow_inheritance = True
             abstract = True
 
     @instance.register
@@ -125,9 +124,9 @@ Here we use this to allow ``Animal`` to be inheritable and to make it abstract.
 .. code-block:: python
 
     >>> Animal.opts
-    <DocumentOpts(instance=<umongo.frameworks.PyMongoInstance object at 0x7efe7daa9320>, template=<Document template class '__main__.Animal'>, abstract=True, allow_inheritance=True, collection_name=None, is_child=False, base_schema_cls=<class 'umongo.schema.Schema'>, indexes=[], offspring={<Implementation class '__main__.Duck'>, <Implementation class '__main__.Dog'>})>
+    <DocumentOpts(instance=<umongo.frameworks.PyMongoInstance object at 0x7efe7daa9320>, template=<Document template class '__main__.Animal'>, abstract=True, collection_name=None, is_child=False, base_schema_cls=<class 'umongo.schema.Schema'>, indexes=[], offspring={<Implementation class '__main__.Duck'>, <Implementation class '__main__.Dog'>})>
     >>> Dog.opts
-    <DocumentOpts(instance=<umongo.frameworks.PyMongoInstance object at 0x7efe7daa9320>, template=<Document template class '__main__.Dog'>, abstract=False, allow_inheritance=False, collection_name=dog, is_child=False, base_schema_cls=<class 'umongo.schema.Schema'>, indexes=[], offspring=set())>
+    <DocumentOpts(instance=<umongo.frameworks.PyMongoInstance object at 0x7efe7daa9320>, template=<Document template class '__main__.Dog'>, abstract=False, collection_name=dog, is_child=False, base_schema_cls=<class 'umongo.schema.Schema'>, indexes=[], offspring=set())>
     >>> class NotAllowedSubDog(Dog): pass
     [...]
     DocumentDefinitionError: Document <class '__main__.Dog'> doesn't allow inheritance
@@ -349,8 +348,6 @@ Inheritance inside the same collection is achieve by adding a ``_cls`` field
     >>> @instance.register
     ... class Parent(Document):
     ...     unique_in_parent = fields.IntField(unique=True)
-    ...     class Meta:
-    ...         allow_inheritance = True
     >>> @instance.register
     ... class Child(Parent):
     ...     unique_in_child = fields.StrField(unique=True)
@@ -435,8 +432,6 @@ compounded with the ``_cls``
       >>> @instance.register
       ... class Parent(Document):
       ...     unique_in_parent = fields.IntField(unique=True)
-      ...     class Meta:
-      ...         allow_inheritance = True
       >>> @instance.register
       ... class Child(Parent):
       ...     unique_in_child = fields.StrField(unique=True)
diff --git a/examples/inheritance/app.py b/examples/inheritance/app.py
index 68e7c5e..bdf207a 100644
--- a/examples/inheritance/app.py
+++ b/examples/inheritance/app.py
@@ -14,7 +14,6 @@ class Vehicle(Document):
 
     class Meta:
         collection_name = "vehicle"
-        allow_inheritance = True
 
 
 @instance.register
diff --git a/umongo/builder.py b/umongo/builder.py
index 98bb747..af45bf9 100644
--- a/umongo/builder.py
+++ b/umongo/builder.py
@@ -149,7 +149,6 @@ def _build_document_opts(instance, template, name, nmspc, bases):
     kwargs['instance'] = instance
     kwargs['template'] = template
     kwargs['abstract'] = getattr(meta, 'abstract', False)
-    kwargs['allow_inheritance'] = getattr(meta, 'allow_inheritance', None)
     kwargs['is_child'] = _is_child(bases)
     kwargs['strict'] = getattr(meta, 'strict', True)
 
@@ -158,8 +157,6 @@ def _build_document_opts(instance, template, name, nmspc, bases):
         if not issubclass(base, DocumentImplementation):
             continue
         popts = base.opts
-        if not popts.allow_inheritance:
-            raise DocumentDefinitionError("Document %r doesn't allow inheritance" % base)
         if kwargs['abstract'] and not popts.abstract:
             raise DocumentDefinitionError(
                 "Abstract document should have all it parents abstract")
@@ -186,7 +183,6 @@ def _build_embedded_document_opts(instance, template, name, nmspc, bases):
     kwargs['instance'] = instance
     kwargs['template'] = template
     kwargs['abstract'] = getattr(meta, 'abstract', False)
-    kwargs['allow_inheritance'] = getattr(meta, 'allow_inheritance', True)
     kwargs['is_child'] = _is_child_embedded_document(bases)
     kwargs['strict'] = getattr(meta, 'strict', True)
 
@@ -195,8 +191,6 @@ def _build_embedded_document_opts(instance, template, name, nmspc, bases):
         if not issubclass(base, EmbeddedDocumentImplementation):
             continue
         popts = base.opts
-        if not popts.allow_inheritance:
-            raise DocumentDefinitionError("EmbeddedDocument %r doesn't allow inheritance" % base)
         if kwargs['abstract'] and not popts.abstract:
             raise DocumentDefinitionError(
                 "Abstract embedded document should have all it parents abstract")
diff --git a/umongo/document.py b/umongo/document.py
index f6b87a6..de5c206 100644
--- a/umongo/document.py
+++ b/umongo/document.py
@@ -9,9 +9,7 @@ from marshmallow import (
 
 from .abstract import BaseDataObject
 from .exceptions import (
-    AlreadyCreatedError, NotCreatedError, NoDBDefinedError,
-    AbstractDocumentError, DocumentDefinitionError,
-)
+    AlreadyCreatedError, NotCreatedError, NoDBDefinedError, AbstractDocumentError)
 from .template import Implementation, Template, MetaImplementation
 from .data_objects import Reference
 
@@ -73,7 +71,6 @@ class DocumentOpts:
     instance             no                     Implementation's instance
     abstract             yes                    Document has no collection
                                                 and can only be inherited
-    allow_inheritance    yes                    Allow the document to be subclassed
     collection_name      yes                    Name of the collection to store
                                                 the document into
     is_child             no                     Document inherit of a non-abstract document
@@ -88,7 +85,6 @@ class DocumentOpts:
                 'instance={self.instance}, '
                 'template={self.template}, '
                 'abstract={self.abstract}, '
-                'allow_inheritance={self.allow_inheritance}, '
                 'collection_name={self.collection_name}, '
                 'is_child={self.is_child}, '
                 'strict={self.strict}, '
@@ -97,19 +93,15 @@ class DocumentOpts:
                 .format(ClassName=self.__class__.__name__, self=self))
 
     def __init__(self, instance, template, collection_name=None, abstract=False,
-                 allow_inheritance=None, indexes=None, is_child=True, strict=True,
-                 offspring=None):
+                 indexes=None, is_child=True, strict=True, offspring=None):
         self.instance = instance
         self.template = template
         self.collection_name = collection_name if not abstract else None
         self.abstract = abstract
-        self.allow_inheritance = abstract if allow_inheritance is None else allow_inheritance
         self.indexes = indexes or []
         self.is_child = is_child
         self.strict = strict
         self.offspring = set(offspring) if offspring else set()
-        if self.abstract and not self.allow_inheritance:
-            raise DocumentDefinitionError("Abstract document cannot disable inheritance")
 
 
 class MetaDocumentImplementation(MetaImplementation):
diff --git a/umongo/embedded_document.py b/umongo/embedded_document.py
index 29a794d..dda1478 100644
--- a/umongo/embedded_document.py
+++ b/umongo/embedded_document.py
@@ -3,7 +3,7 @@ import marshmallow as ma
 
 from .document import Implementation, Template
 from .data_objects import BaseDataObject
-from .exceptions import DocumentDefinitionError, AbstractDocumentError
+from .exceptions import AbstractDocumentError
 
 
 __all__ = (
@@ -52,7 +52,6 @@ class EmbeddedDocumentOpts:
     template             no                     Origin template of the embedded document
     instance             no                     Implementation's instance
     abstract             yes                    Embedded document can only be inherited
-    allow_inheritance    yes                    Allow the embedded document to be subclassed
     is_child             no                     Embedded document inherit of a non-abstract
                                                 embedded document
     strict               yes                    Don't accept unknown fields from mongo
@@ -65,23 +64,19 @@ class EmbeddedDocumentOpts:
                 'instance={self.instance}, '
                 'template={self.template}, '
                 'abstract={self.abstract}, '
-                'allow_inheritance={self.allow_inheritance}, '
                 'is_child={self.is_child}, '
                 'strict={self.strict}, '
                 'offspring={self.offspring})>'
                 .format(ClassName=self.__class__.__name__, self=self))
 
-    def __init__(self, instance, template, abstract=False, allow_inheritance=True,
+    def __init__(self, instance, template, abstract=False,
                  is_child=False, strict=True, offspring=None):
         self.instance = instance
         self.template = template
         self.abstract = abstract
-        self.allow_inheritance = allow_inheritance
         self.is_child = is_child
         self.strict = strict
         self.offspring = set(offspring) if offspring else set()
-        if self.abstract and not self.allow_inheritance:
-            raise DocumentDefinitionError("Abstract embedded document cannot disable inheritance")
 
 
 class EmbeddedDocumentImplementation(Implementation, BaseDataObject):


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 516a98c..652dd54 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -29,7 +29,4 @@ def classroom_model(instance):
         birthday = fields.DateTimeField()
         courses = fields.ListField(fields.ReferenceField(Course))
 
-        class Meta:
-            allow_inheritance = True
-
     return namedtuple('Mapping', ('Teacher', 'Course', 'Student'))(Teacher, Course, Student)
diff --git a/tests/frameworks/test_motor_asyncio.py b/tests/frameworks/test_motor_asyncio.py
index e25e40d..70e1021 100644
--- a/tests/frameworks/test_motor_asyncio.py
+++ b/tests/frameworks/test_motor_asyncio.py
@@ -654,9 +654,6 @@ class TestMotorAsyncio(BaseDBTest):
                 not_unique = fields.StrField(unique=False)
                 unique = fields.IntField(unique=True)
 
-                class Meta:
-                    allow_inheritance = True
-
             @instance.register
             class UniqueIndexChildDoc(UniqueIndexParentDoc):
                 child_not_unique = fields.StrField(unique=False)
@@ -718,16 +715,10 @@ class TestMotorAsyncio(BaseDBTest):
             class InheritanceSearchParent(Document):
                 pf = fields.IntField()
 
-                class Meta:
-                    allow_inheritance = True
-
             @instance.register
             class InheritanceSearchChild1(InheritanceSearchParent):
                 c1f = fields.IntField()
 
-                class Meta:
-                    allow_inheritance = True
-
             @instance.register
             class InheritanceSearchChild1Child(InheritanceSearchChild1):
                 sc1f = fields.IntField()
diff --git a/tests/frameworks/test_pymongo.py b/tests/frameworks/test_pymongo.py
index 3751f08..c9598f9 100644
--- a/tests/frameworks/test_pymongo.py
+++ b/tests/frameworks/test_pymongo.py
@@ -503,9 +503,6 @@ class TestPymongo(BaseDBTest):
             not_unique = fields.StrField(unique=False)
             unique = fields.IntField(unique=True)
 
-            class Meta:
-                allow_inheritance = True
-
         @instance.register
         class UniqueIndexChildDoc(UniqueIndexParentDoc):
             child_not_unique = fields.StrField(unique=False)
@@ -563,16 +560,10 @@ class TestPymongo(BaseDBTest):
         class InheritanceSearchParent(Document):
             pf = fields.IntField()
 
-            class Meta:
-                allow_inheritance = True
-
         @instance.register
         class InheritanceSearchChild1(InheritanceSearchParent):
             c1f = fields.IntField()
 
-            class Meta:
-                allow_inheritance = True
-
         @instance.register
         class InheritanceSearchChild1Child(InheritanceSearchChild1):
             sc1f = fields.IntField()
diff --git a/tests/frameworks/test_txmongo.py b/tests/frameworks/test_txmongo.py
index 5b79a76..299f9e4 100644
--- a/tests/frameworks/test_txmongo.py
+++ b/tests/frameworks/test_txmongo.py
@@ -589,9 +589,6 @@ class TestTxMongo(BaseDBTest):
             not_unique = fields.StrField(unique=False)
             unique = fields.IntField(unique=True)
 
-            class Meta:
-                allow_inheritance = True
-
         @instance.register
         class UniqueIndexChildDoc(UniqueIndexParentDoc):
             child_not_unique = fields.StrField(unique=False)
@@ -655,16 +652,10 @@ class TestTxMongo(BaseDBTest):
         class InheritanceSearchParent(Document):
             pf = fields.IntField()
 
-            class Meta:
-                allow_inheritance = True
-
         @instance.register
         class InheritanceSearchChild1(InheritanceSearchParent):
             c1f = fields.IntField()
 
-            class Meta:
-                allow_inheritance = True
-
         @instance.register
         class InheritanceSearchChild1Child(InheritanceSearchChild1):
             sc1f = fields.IntField()
diff --git a/tests/test_document.py b/tests/test_document.py
index 033eb25..c17eb2c 100644
--- a/tests/test_document.py
+++ b/tests/test_document.py
@@ -217,9 +217,7 @@ class TestDocument(BaseTest):
 
         @self.instance.register
         class AutoId(Document):
-
-            class Meta:
-                allow_inheritance = True
+            pass
 
         assert 'id' in AutoId.schema.fields
 
@@ -245,9 +243,6 @@ class TestDocument(BaseTest):
         class CustomId(Document):
             int_id = fields.IntField(attribute='_id')
 
-            class Meta:
-                allow_inheritance = True
-
         assert 'id' not in CustomId.schema.fields
         with pytest.raises(ma.ValidationError):
             CustomId(id=my_id)
@@ -310,8 +305,7 @@ class TestDocument(BaseTest):
         # a template instead of from an implementation
 
         class ParentAsTemplate(Document):
-            class Meta:
-                allow_inheritance = True
+            pass
 
         Parent = self.instance.register(ParentAsTemplate)
 
@@ -324,18 +318,15 @@ class TestDocument(BaseTest):
     def test_grand_child_inheritance(self):
         @self.instance.register
         class GrandParent(Document):
-            class Meta:
-                allow_inheritance = True
+            pass
 
         @self.instance.register
         class Parent(GrandParent):
-            class Meta:
-                allow_inheritance = True
+            pass
 
         @self.instance.register
         class Uncle(GrandParent):
-            class Meta:
-                allow_inheritance = True
+            pass
 
         @self.instance.register
         class Child(Parent):
@@ -462,7 +453,6 @@ class TestConfig(BaseTest):
 
         assert Doc.opts.collection_name == 'doc'
         assert Doc.opts.abstract is False
-        assert Doc.opts.allow_inheritance is False
         assert Doc.opts.instance is self.instance
         assert Doc.opts.is_child is False
         assert Doc.opts.indexes == []
@@ -480,7 +470,6 @@ class TestConfig(BaseTest):
         class DocChild1(AbsDoc):
 
             class Meta:
-                allow_inheritance = True
                 collection_name = 'col1'
 
         @self.instance.register
@@ -495,7 +484,6 @@ class TestConfig(BaseTest):
 
         assert DocChild1.opts.collection_name is 'col1'
         assert DocChild1Child.opts.collection_name is 'col1'
-        assert DocChild1Child.opts.allow_inheritance is False
         assert DocChild2.opts.collection_name == 'col2'
 
     def test_marshmallow_tags_build(self):
@@ -516,9 +504,6 @@ class TestConfig(BaseTest):
         class Animal(Document):
             name = fields.StrField(attribute='_id')  # Overwrite automatic pk
 
-            class Meta:
-                allow_inheritance = True
-
         @self.instance.register
         class Dog(Animal):
             pass
@@ -553,32 +538,9 @@ class TestConfig(BaseTest):
         exc.value.args[0] == {'name': 'Not suitable name for duck !'}
 
     def test_bad_inheritance(self):
-        with pytest.raises(exceptions.DocumentDefinitionError) as exc:
-            @self.instance.register
-            class BadAbstractDoc(Document):
-                class Meta:
-                    allow_inheritance = False
-                    abstract = True
-        assert exc.value.args[0] == "Abstract document cannot disable inheritance"
-
-        @self.instance.register
-        class NotParent(Document):
-            pass
-
-        assert not NotParent.opts.allow_inheritance
-
-        with pytest.raises(exceptions.DocumentDefinitionError) as exc:
-            @self.instance.register
-            class ImpossibleChildDoc1(NotParent):
-                pass
-        assert exc.value.args[0] == ("Document"
-            " <Implementation class 'tests.test_document.NotParent'>"
-            " doesn't allow inheritance")
-
         @self.instance.register
         class NotAbstractParent(Document):
-            class Meta:
-                allow_inheritance = True
+            pass
 
         with pytest.raises(exceptions.DocumentDefinitionError) as exc:
             @self.instance.register
@@ -590,13 +552,11 @@ class TestConfig(BaseTest):
         @self.instance.register
         class ParentWithCol1(Document):
             class Meta:
-                allow_inheritance = True
                 collection_name = 'col1'
 
         @self.instance.register
         class ParentWithCol2(Document):
             class Meta:
-                allow_inheritance = True
                 collection_name = 'col2'
 
         with pytest.raises(exceptions.DocumentDefinitionError) as exc:
diff --git a/tests/test_embedded_document.py b/tests/test_embedded_document.py
index a3d9484..b6bebb9 100644
--- a/tests/test_embedded_document.py
+++ b/tests/test_embedded_document.py
@@ -273,9 +273,6 @@ class TestEmbeddedDocument(BaseTest):
         class ConcreteChild(AbstractParent, AlienClass):
             c = fields.IntField()
 
-            class Meta:
-                allow_inheritance = True
-
         @self.instance.register
         class ConcreteGrandChild(AbstractChild):
             d = fields.IntField()
@@ -317,34 +314,9 @@ class TestEmbeddedDocument(BaseTest):
 
 
     def test_bad_inheritance(self):
-        with pytest.raises(exceptions.DocumentDefinitionError) as exc:
-            @self.instance.register
-            class BadAbstract(EmbeddedDocument):
-                class Meta:
-                    allow_inheritance = False
-                    abstract = True
-        assert exc.value.args[0] == "Abstract embedded document cannot disable inheritance"
-
-        @self.instance.register
-        class NotParent(EmbeddedDocument):
-            class Meta:
-                allow_inheritance = False
-
-        with pytest.raises(exceptions.DocumentDefinitionError) as exc:
-            @self.instance.register
-            class ImpossibleChild1(NotParent):
-                pass
-        assert exc.value.args[0] == ("EmbeddedDocument"
-            " <Implementation class 'tests.test_embedded_document.NotParent'>"
-            " doesn't allow inheritance")
-
         @self.instance.register
         class NotAbstractParent(EmbeddedDocument):
-            class Meta:
-                allow_inheritance = True
-
-        # Unlike Document, EmbeddedDocument should allow inheritance by default
-        assert NotAbstractParent.opts.allow_inheritance
+            pass
 
         with pytest.raises(exceptions.DocumentDefinitionError) as exc:
             @self.instance.register
diff --git a/tests/test_indexes.py b/tests/test_indexes.py
index ffb3b27..f5cae8f 100644
--- a/tests/test_indexes.py
+++ b/tests/test_indexes.py
@@ -68,7 +68,6 @@ class TestIndexes(BaseTest):
             last_name = fields.StrField()
 
             class Meta:
-                allow_inheritance = True
                 indexes = ['last_name']
 
         @self.instance.register
diff --git a/tests/test_inheritance.py b/tests/test_inheritance.py
index ef3dabf..b24e366 100644
--- a/tests/test_inheritance.py
+++ b/tests/test_inheritance.py
@@ -13,9 +13,6 @@ class TestInheritance(BaseTest):
         class Parent(Document):
             last_name = fields.StrField()
 
-            class Meta:
-                allow_inheritance = True
-
         @self.instance.register
         class Child(Parent):
             first_name = fields.StrField()
@@ -36,18 +33,15 @@ class TestInheritance(BaseTest):
             last_name = fields.StrField()
 
             class Meta:
-                allow_inheritance = True
                 collection_name = 'parent_col'
 
         assert Parent.opts.abstract is False
-        assert Parent.opts.allow_inheritance is True
 
         @self.instance.register
         class Child(Parent):
             first_name = fields.StrField()
 
         assert Child.opts.abstract is False
-        assert Child.opts.allow_inheritance is False
         assert Child.opts.collection_name == 'parent_col'
         assert Child.collection.name == 'parent_col'
         Child(first_name='John', last_name='Doe')
@@ -70,7 +64,6 @@ class TestInheritance(BaseTest):
                 abstract = True
 
         assert AbstractDoc.opts.abstract is True
-        assert AbstractDoc.opts.allow_inheritance is True
         # Cannot instanciate also an abstract document
         with pytest.raises(exceptions.AbstractDocumentError):
             AbstractDoc()
@@ -81,14 +74,12 @@ class TestInheritance(BaseTest):
                 abstract = True
 
         assert StillAbstractDoc.opts.abstract is True
-        assert StillAbstractDoc.opts.allow_inheritance is True
 
         @self.instance.register
         class ConcreteDoc(AbstractDoc):
             pass
 
         assert ConcreteDoc.opts.abstract is False
-        assert ConcreteDoc.opts.allow_inheritance is False
         assert ConcreteDoc().abs_field == 'from abstract'
 
     def test_non_document_inheritance(self):
diff --git a/tests/test_marshmallow.py b/tests/test_marshmallow.py
index 962d8eb..f03efed 100644
--- a/tests/test_marshmallow.py
+++ b/tests/test_marshmallow.py
@@ -27,9 +27,6 @@ class TestMarshmallow(BaseTest):
             name = fields.StrField()
             birthday = fields.DateTimeField()
 
-            class Meta:
-                allow_inheritance = True
-
         self.User = self.instance.register(User)
 
     def test_by_field(self):
@@ -55,16 +52,10 @@ class TestMarshmallow(BaseTest):
         class MyDocument(Document):
             MA_BASE_SCHEMA_CLS = ExcludeBaseSchema
 
-            class Meta:
-                allow_inheritance = True
-
         @self.instance.register
         class MyEmbeddedDocument(EmbeddedDocument):
             MA_BASE_SCHEMA_CLS = ExcludeBaseSchema
 
-            class Meta:
-                allow_inheritance = True
-
         # Now, all our objects will generate "exclude" marshmallow schemas
         @self.instance.register
         class Accessory(MyEmbeddedDocument):