Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Reverse Delete Rules #2765

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,4 @@ that much better:
* oleksandr-l5 (https://github.com/oleksandr-l5)
* Ido Shraga (https://github.com/idoshr)
* Terence Honles (https://github.com/terencehonles)
* Chris Combs (https://github.com/combscCode)
18 changes: 13 additions & 5 deletions docs/guide/defining-documents.rst
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,23 @@ Dealing with deletion of referred documents
'''''''''''''''''''''''''''''''''''''''''''
By default, MongoDB doesn't check the integrity of your data, so deleting
documents that other documents still hold references to will lead to consistency
issues. Mongoengine's :class:`ReferenceField` adds some functionality to
safeguard against these kinds of database integrity problems, providing each
reference with a delete rule specification. A delete rule is specified by
supplying the :attr:`reverse_delete_rule` attributes on the
:class:`ReferenceField` definition, like this::
issues. Mongoengine's :class:`ReferenceField` and :class:`GenericReferenceField`
add some functionality to safeguard against these kinds of database integrity
problems, providing each reference with a delete rule specification. A delete
rule is specified by supplying the :attr:`reverse_delete_rule` attributes on the
:class:`ReferenceField` or :class:`GenericReferenceField` definition, like this::

class ProfilePage(Document):
employee = ReferenceField('Employee', reverse_delete_rule=mongoengine.CASCADE)

Note that in the case of :class:`GenericReferenceField` you'll also need to specify
an iterable of :attr:`choices` to get :attr:`reverse_delete_rule` working.
:attr:`choices` should consist of the documents that can be referenced by the
:class:`GenericReferenceField`.:

class ProfilePage(Document):
employee = GenericReferenceField(choices=['Employee'], reverse_delete_rule=mongoengine.CASCADE)

The declaration in this example means that when an :class:`Employee` object is
removed, the :class:`ProfilePage` that references that employee is removed as
well. If a whole batch of employees is removed, all profile pages that are
Expand Down
1 change: 1 addition & 0 deletions docs/tutorial.rst
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ To delete all the posts if a user is deleted set the rule::
comments = ListField(EmbeddedDocumentField(Comment))

See :class:`~mongoengine.fields.ReferenceField` for more information.
:class:`~mongoengine.fields.GenericReferenceField` also supports this feature.

.. note::
MapFields and DictFields currently don't support automatic handling of
Expand Down
2 changes: 2 additions & 0 deletions mongoengine/base/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
# common
"UPDATE_OPERATORS",
"_document_registry",
"_undefined_document_delete_rules",
"get_document",
"update_document_if_registered",
# datastructures
"BaseDict",
"BaseList",
Expand Down
21 changes: 20 additions & 1 deletion mongoengine/base/common.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
from collections import defaultdict

from mongoengine.errors import NotRegistered

__all__ = ("UPDATE_OPERATORS", "get_document", "_document_registry")
__all__ = (
"UPDATE_OPERATORS",
"get_document",
"update_document_if_registered",
"_document_registry",
"_undefined_document_delete_rules",
)


UPDATE_OPERATORS = {
Expand All @@ -23,6 +31,7 @@


_document_registry = {}
_undefined_document_delete_rules = defaultdict(list)


def get_document(name):
Expand Down Expand Up @@ -60,3 +69,13 @@ def get_doc_alias(doc_cls):
for doc_cls in _document_registry.values()
if get_doc_alias(doc_cls) == connection_alias
]


def update_document_if_registered(document):
"""Converts a string to a Document if registered in the registry."""
if isinstance(document, str):
try:
return get_document(document)
except NotRegistered:
pass
return document
23 changes: 20 additions & 3 deletions mongoengine/base/metaclasses.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import itertools
import warnings

from mongoengine.base.common import _document_registry
from mongoengine.base.common import (
_document_registry,
_undefined_document_delete_rules,
)
from mongoengine.base.fields import (
BaseField,
ComplexBaseField,
ObjectIdField,
)
from mongoengine.common import _import_class
from mongoengine.errors import InvalidDocumentError
from mongoengine.errors import InvalidDocumentError, NotRegistered
from mongoengine.queryset import (
DO_NOTHING,
DoesNotExist,
Expand Down Expand Up @@ -206,7 +209,14 @@ def __new__(mcs, name, bases, attrs):
"EmbeddedDocuments (field: %s)" % field.name
)
raise InvalidDocumentError(msg)
f.document_type.register_delete_rule(new_class, field.name, delete_rule)
try:
f.document_type.register_delete_rule(
new_class, field.name, delete_rule
Comment on lines +213 to +214
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the ideal world I would cut out document_type here and just make fields responsible for implementing register_delete_rule.

However, when I tried this I realized that it would break the interface for how to register delete rules and thus any users dynamically setting delete rules would be upset.

)
except NotRegistered:
_undefined_document_delete_rules[f.document_type_obj].append(
(new_class, field.name, delete_rule)
)

if (
field.name
Expand Down Expand Up @@ -362,6 +372,13 @@ def __new__(mcs, name, bases, attrs):

# Call super and get the new class
new_class = super_new(mcs, name, bases, attrs)
# Find any lazy delete rules and apply to current doc.
if new_class._class_name in _undefined_document_delete_rules:
rules_tuple_list = _undefined_document_delete_rules.pop(
new_class._class_name
)
for document_cls, field_name, rule in rules_tuple_list:
new_class.register_delete_rule(document_cls, field_name, rule)

meta = new_class._meta

Expand Down
77 changes: 74 additions & 3 deletions mongoengine/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
GeoJsonBaseField,
LazyReference,
ObjectIdField,
_undefined_document_delete_rules,
get_document,
update_document_if_registered,
)
from mongoengine.base.utils import LazyRegexCompiler
from mongoengine.common import _import_class
Expand Down Expand Up @@ -1120,8 +1122,7 @@ class ReferenceField(BaseField):
* DENY (3) - Prevent the deletion of the reference object.
* PULL (4) - Pull the reference from a :class:`~mongoengine.fields.ListField` of references

Alternative syntax for registering delete rules (useful when implementing
bi-directional delete rules)
Alternative syntax for registering delete rules

.. code-block:: python

Expand Down Expand Up @@ -1435,6 +1436,39 @@ def sync_all(self):
self.owner_document.objects(**filter_kwargs).update(**update_kwargs)


class GenericReferenceDeleteHandler:
"""Used to make delete rules work for GenericReferenceFields.

Since delete rules are registered on single documents, we'll always need
something like this to make a generic reference (AKA, a reference to
multiple documents) with delete rules work.
"""

def __init__(self, documents):
self.documents = documents

def __getattr__(self, name):
raise NotImplementedError(
f"{self.__name__} is intended only to be used "
"to enable generic reference delete rules. You "
"are trying to access undefined attributes."
)

def register_delete_rule(self, document_cls, field_name, rule):
for doc in self.documents:
doc = update_document_if_registered(doc)
if isinstance(doc, str):
_undefined_document_delete_rules[doc].append(
(
document_cls,
field_name,
rule,
)
)
else:
doc.register_delete_rule(document_cls, field_name, rule)


class GenericReferenceField(BaseField):
"""A reference to *any* :class:`~mongoengine.document.Document` subclass
that will be automatically dereferenced on access (lazily).
Expand All @@ -1445,6 +1479,31 @@ class GenericReferenceField(BaseField):
To solve this you should consider using the
:class:`~mongoengine.fields.GenericLazyReferenceField`.

Use the `reverse_delete_rule` to handle what should happen if the document
the field is referencing is deleted. EmbeddedDocuments, DictFields and
MapFields does not support reverse_delete_rule and an `InvalidDocumentError`
will be raised if trying to set on one of these Document / Field types.

The options are:

* DO_NOTHING (0) - don't do anything (default).
* NULLIFY (1) - Updates the reference to null.
* CASCADE (2) - Deletes the documents associated with the reference.
* DENY (3) - Prevent the deletion of the reference object.
* PULL (4) - Pull the reference from a :class:`~mongoengine.fields.ListField` of references

Alternative syntax for registering delete rules

.. code-block:: python

class Org(Document):
owner = ReferenceField('User')

class User(Document):
org = ReferenceField('Org', reverse_delete_rule=CASCADE)

User.register_delete_rule(Org, 'owner', DENY)

Comment on lines +1482 to +1506
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't showing up in sphinx when I ran make html-readthedocs, not sure why?

.. note ::
* Any documents used as a generic reference must be registered in the
document registry. Importing the model will automatically register
Expand All @@ -1453,8 +1512,11 @@ class GenericReferenceField(BaseField):
* You can use the choices param to limit the acceptable Document types
"""

def __init__(self, *args, **kwargs):
def __init__(self, *args, reverse_delete_rule=DO_NOTHING, **kwargs):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to call this param out in the GenericReferenceField docs but I couldn't seem to get it to show up properly, I'd make a docstring for the init but it wouldn't populate to the docs 🤷

choices = kwargs.pop("choices", None)
self.reverse_delete_rule = reverse_delete_rule
if self.reverse_delete_rule is not DO_NOTHING and not choices:
raise ValidationError("choices must be set to use reverse_delete_rules")
super().__init__(*args, **kwargs)
self.choices = []
# Keep the choices as a list of allowed Document class names
Expand All @@ -1472,6 +1534,15 @@ def __init__(self, *args, **kwargs):
"Document subclasses and/or str"
)

@property
def document_type(self):
# This property is exposed purely for enabling reverse_delete_rule
# on this class. Do not attempt to use it in any other way, if you
# do a NotImplementedError will be raised.
if not self.choices:
return None
return GenericReferenceDeleteHandler(self.choices)

def _validate_choices(self, value):
if isinstance(value, dict):
# If the field has not been dereferenced, it is still a dict
Expand Down
52 changes: 51 additions & 1 deletion tests/document/test_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@

from mongoengine import *
from mongoengine import signals
from mongoengine.base import _document_registry, get_document
from mongoengine.base import (
_document_registry,
_undefined_document_delete_rules,
get_document,
)
from mongoengine.connection import get_db
from mongoengine.context_managers import query_counter, switch_db
from mongoengine.errors import (
Expand Down Expand Up @@ -2516,6 +2520,52 @@ class BlogPost(Document):
author.delete()
assert self.Person.objects.count() == 1

def test_lazy_delete_rules(self):
"""Ensure that a document does not need to be defined to reference it
in a ReferenceField."""

assert not _undefined_document_delete_rules.get("BlogPostLazy")

# named "lazy" to ensure these Documents don't exist in the
# document registry
class CommentLazy(Document):
text = StringField()
post = ReferenceField("BlogPostLazy", reverse_delete_rule=CASCADE)

assert len(_undefined_document_delete_rules.get("BlogPostLazy")) == 1

class CommentDosLazy(Document):
textdos = StringField()
postdos = ReferenceField("BlogPostLazy", reverse_delete_rule=CASCADE)

assert len(_undefined_document_delete_rules.get("BlogPostLazy")) == 2

class BlogPostLazy(Document):
content = StringField()
author = ReferenceField(self.Person, reverse_delete_rule=CASCADE)

assert not _undefined_document_delete_rules.get("BlogPostLazy")

self.Person.drop_collection()
BlogPostLazy.drop_collection()
CommentLazy.drop_collection()

author = self.Person(name="Test User")
author.save()

post = BlogPostLazy(content="Watched some TV")
post.author = author
post.save()

comment = CommentLazy(text="Kudos.")
comment.post = post
comment.save()

# Delete the Person, which should lead to deletion of the BlogPost,
# and, recursively to the Comment, too
author.delete()
assert CommentLazy.objects.count() == 0

def subclasses_and_unique_keys_works(self):
class A(Document):
pass
Expand Down
Loading