diff --git a/docs/guide/defining-documents.rst b/docs/guide/defining-documents.rst index 106d4ec8..de0e7272 100644 --- a/docs/guide/defining-documents.rst +++ b/docs/guide/defining-documents.rst @@ -193,6 +193,59 @@ as the constructor's argument:: class ProfilePage(Document): content = StringField() + +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:: + + class Employee(Document): + ... + profile_page = ReferenceField('ProfilePage', reverse_delete_rule=mongoengine.NULLIFY) + +The declaration in this example means that when an :class:`Employee` object is +removed, the :class:`ProfilePage` that belongs to that employee is removed as +well. If a whole batch of employees is removed, all profile pages that are +linked are removed as well. + +Its value can take any of the following constants: + +:const:`mongoengine.DO_NOTHING` + This is the default and won't do anything. Deletes are fast, but may cause + database inconsistency or dangling references. +:const:`mongoengine.DENY` + Deletion is denied if there still exist references to the object being + deleted. +:const:`mongoengine.NULLIFY` + Any object's fields still referring to the object being deleted are removed + (using MongoDB's "unset" operation), effectively nullifying the relationship. +:const:`mongoengine.CASCADE` + Any object containing fields that are refererring to the object being deleted + are deleted first. + + +.. warning:: + A safety note on setting up these delete rules! Since the delete rules are + not recorded on the database level by MongoDB itself, but instead at runtime, + in-memory, by the MongoEngine module, it is of the upmost importance + that the module that declares the relationship is loaded **BEFORE** the + delete is invoked. + + If, for example, the :class:`Employee` object lives in the + :mod:`payroll` app, and the :class:`ProfilePage` in the :mod:`people` + app, it is extremely important that the :mod:`people` app is loaded + before any employee is removed, because otherwise, MongoEngine could + never know this relationship exists. + + In Django, be sure to put all apps that have such delete rule declarations in + their :file:`models.py` in the :const:`INSTALLED_APPS` tuple. + + Generic reference fields '''''''''''''''''''''''' A second kind of reference field also exists, diff --git a/mongoengine/base.py b/mongoengine/base.py index 589042e2..ae9e1eb3 100644 --- a/mongoengine/base.py +++ b/mongoengine/base.py @@ -1,5 +1,6 @@ from queryset import QuerySet, QuerySetManager from queryset import DoesNotExist, MultipleObjectsReturned +from queryset import DO_NOTHING import sys import pymongo @@ -203,6 +204,10 @@ class DocumentMetaclass(type): new_class = super_new(cls, name, bases, attrs) for field in new_class._fields.values(): field.owner_document = new_class + delete_rule = getattr(field, 'reverse_delete_rule', DO_NOTHING) + if delete_rule != DO_NOTHING: + field.document_type.register_delete_rule(new_class, field.name, + delete_rule) module = attrs.get('__module__') @@ -271,6 +276,7 @@ class TopLevelDocumentMetaclass(DocumentMetaclass): 'index_drop_dups': False, 'index_opts': {}, 'queryset_class': QuerySet, + 'delete_rules': {}, } meta.update(base_meta) @@ -506,6 +512,7 @@ if sys.version_info < (2, 5): # Prior to Python 2.5, Exception was an old-style class import types def subclass_exception(name, parents, unused): + import types return types.ClassType(name, parents, {}) else: def subclass_exception(name, parents, module): diff --git a/mongoengine/document.py b/mongoengine/document.py index fef737db..e64092e8 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -100,6 +100,14 @@ class Document(BaseDocument): message = u'Could not delete document (%s)' % err.message raise OperationError(message) + @classmethod + def register_delete_rule(cls, document_cls, field_name, rule): + """This method registers the delete rules to apply when removing this + object. + """ + cls._meta['delete_rules'][(document_cls, field_name)] = rule + + def reload(self): """Reloads all attributes from the database. diff --git a/mongoengine/fields.py b/mongoengine/fields.py index 8a3721a9..3d78e9e3 100644 --- a/mongoengine/fields.py +++ b/mongoengine/fields.py @@ -1,4 +1,5 @@ from base import BaseField, ObjectIdField, ValidationError, get_document +from queryset import DO_NOTHING from document import Document, EmbeddedDocument from connection import _get_db from operator import itemgetter @@ -455,12 +456,13 @@ class ReferenceField(BaseField): access (lazily). """ - def __init__(self, document_type, **kwargs): + def __init__(self, document_type, reverse_delete_rule=DO_NOTHING, **kwargs): if not isinstance(document_type, basestring): if not issubclass(document_type, (Document, basestring)): raise ValidationError('Argument to ReferenceField constructor ' 'must be a document class or a string') self.document_type_obj = document_type + self.reverse_delete_rule = reverse_delete_rule super(ReferenceField, self).__init__(**kwargs) @property diff --git a/mongoengine/queryset.py b/mongoengine/queryset.py index 7d23e5ca..11d4706c 100644 --- a/mongoengine/queryset.py +++ b/mongoengine/queryset.py @@ -10,11 +10,18 @@ import copy import itertools __all__ = ['queryset_manager', 'Q', 'InvalidQueryError', - 'InvalidCollectionError'] + 'InvalidCollectionError', 'DO_NOTHING', 'NULLIFY', 'CASCADE', 'DENY'] + # The maximum number of items to display in a QuerySet.__repr__ REPR_OUTPUT_SIZE = 20 +# Delete rules +DO_NOTHING = 0 +NULLIFY = 1 +CASCADE = 2 +DENY = 3 + class DoesNotExist(Exception): pass @@ -947,6 +954,28 @@ class QuerySet(object): :param safe: check if the operation succeeded before returning """ + doc = self._document + + # Check for DENY rules before actually deleting/nullifying any other + # references + for rule_entry in doc._meta['delete_rules']: + document_cls, field_name = rule_entry + rule = doc._meta['delete_rules'][rule_entry] + if rule == DENY and document_cls.objects(**{field_name + '__in': self}).count() > 0: + msg = u'Could not delete document (at least %s.%s refers to it)' % \ + (document_cls.__name__, field_name) + raise OperationError(msg) + + for rule_entry in doc._meta['delete_rules']: + document_cls, field_name = rule_entry + rule = doc._meta['delete_rules'][rule_entry] + if rule == CASCADE: + document_cls.objects(**{field_name + '__in': self}).delete(safe=safe) + elif rule == NULLIFY: + document_cls.objects(**{field_name + '__in': self}).update( + safe_update=safe, + **{'unset__%s' % field_name: 1}) + self._collection.remove(self._query, safe=safe) @classmethod diff --git a/tests/document.py b/tests/document.py index c0567632..67c21a46 100644 --- a/tests/document.py +++ b/tests/document.py @@ -502,7 +502,7 @@ class DocumentTest(unittest.TestCase): try: recipient.save(validate=False) except ValidationError: - fail() + self.fail() def test_delete(self): """Ensure that document may be deleted using the delete method. @@ -624,6 +624,108 @@ class DocumentTest(unittest.TestCase): BlogPost.drop_collection() + + def test_reverse_delete_rule_cascade_and_nullify(self): + """Ensure that a referenced document is also deleted upon deletion. + """ + + class BlogPost(Document): + content = StringField() + author = ReferenceField(self.Person, reverse_delete_rule=CASCADE) + reviewer = ReferenceField(self.Person, reverse_delete_rule=NULLIFY) + + self.Person.drop_collection() + BlogPost.drop_collection() + + author = self.Person(name='Test User') + author.save() + + reviewer = self.Person(name='Re Viewer') + reviewer.save() + + post = BlogPost(content = 'Watched some TV') + post.author = author + post.reviewer = reviewer + post.save() + + reviewer.delete() + self.assertEqual(len(BlogPost.objects), 1) # No effect on the BlogPost + self.assertEqual(BlogPost.objects.get().reviewer, None) + + # Delete the Person, which should lead to deletion of the BlogPost, too + author.delete() + self.assertEqual(len(BlogPost.objects), 0) + + def test_reverse_delete_rule_cascade_recurs(self): + """Ensure that a chain of documents is also deleted upon cascaded + deletion. + """ + + class BlogPost(Document): + content = StringField() + author = ReferenceField(self.Person, reverse_delete_rule=CASCADE) + + class Comment(Document): + text = StringField() + post = ReferenceField(BlogPost, reverse_delete_rule=CASCADE) + + + author = self.Person(name='Test User') + author.save() + + post = BlogPost(content = 'Watched some TV') + post.author = author + post.save() + + comment = Comment(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() + self.assertEqual(len(Comment.objects), 0) + + self.Person.drop_collection() + BlogPost.drop_collection() + Comment.drop_collection() + + def test_reverse_delete_rule_deny(self): + """Ensure that a document cannot be referenced if there are still + documents referring to it. + """ + + class BlogPost(Document): + content = StringField() + author = ReferenceField(self.Person, reverse_delete_rule=DENY) + + self.Person.drop_collection() + BlogPost.drop_collection() + + author = self.Person(name='Test User') + author.save() + + post = BlogPost(content = 'Watched some TV') + post.author = author + post.save() + + # Delete the Person should be denied + self.assertRaises(OperationError, author.delete) # Should raise denied error + self.assertEqual(len(BlogPost.objects), 1) # No objects may have been deleted + self.assertEqual(len(self.Person.objects), 1) + + # Other users, that don't have BlogPosts must be removable, like normal + author = self.Person(name='Another User') + author.save() + + self.assertEqual(len(self.Person.objects), 2) + author.delete() + self.assertEqual(len(self.Person.objects), 1) + + self.Person.drop_collection() + BlogPost.drop_collection() + + def tearDown(self): self.Person.drop_collection() diff --git a/tests/fields.py b/tests/fields.py index 034ec61e..f24b5eda 100644 --- a/tests/fields.py +++ b/tests/fields.py @@ -523,29 +523,29 @@ class FieldTest(unittest.TestCase): Link.drop_collection() Post.drop_collection() Bookmark.drop_collection() - + link_1 = Link(title="Pitchfork") link_1.save() - + post_1 = Post(title="Behind the Scenes of the Pavement Reunion") post_1.save() - + bm = Bookmark(bookmark_object=post_1) bm.save() - + bm = Bookmark.objects(bookmark_object=post_1).first() - + self.assertEqual(bm.bookmark_object, post_1) self.assertTrue(isinstance(bm.bookmark_object, Post)) - + bm.bookmark_object = link_1 bm.save() - + bm = Bookmark.objects(bookmark_object=link_1).first() - + self.assertEqual(bm.bookmark_object, link_1) self.assertTrue(isinstance(bm.bookmark_object, Link)) - + Link.drop_collection() Post.drop_collection() Bookmark.drop_collection() @@ -555,23 +555,23 @@ class FieldTest(unittest.TestCase): """ class Link(Document): title = StringField() - + class Post(Document): title = StringField() - + class User(Document): bookmarks = ListField(GenericReferenceField()) - + Link.drop_collection() Post.drop_collection() User.drop_collection() - + link_1 = Link(title="Pitchfork") link_1.save() - + post_1 = Post(title="Behind the Scenes of the Pavement Reunion") post_1.save() - + user = User(bookmarks=[post_1, link_1]) user.save() diff --git a/tests/queryset.py b/tests/queryset.py index 34730aa4..d0cdf106 100644 --- a/tests/queryset.py +++ b/tests/queryset.py @@ -351,8 +351,6 @@ class QuerySetTest(unittest.TestCase): def test_filter_chaining(self): """Ensure filters can be chained together. """ - from datetime import datetime - class BlogPost(Document): title = StringField() is_published = BooleanField() @@ -848,6 +846,71 @@ class QuerySetTest(unittest.TestCase): self.Person.objects.delete() self.assertEqual(len(self.Person.objects), 0) + def test_reverse_delete_rule_cascade(self): + """Ensure cascading deletion of referring documents from the database. + """ + class BlogPost(Document): + content = StringField() + author = ReferenceField(self.Person, reverse_delete_rule=CASCADE) + BlogPost.drop_collection() + + me = self.Person(name='Test User') + me.save() + someoneelse = self.Person(name='Some-one Else') + someoneelse.save() + + BlogPost(content='Watching TV', author=me).save() + BlogPost(content='Chilling out', author=me).save() + BlogPost(content='Pro Testing', author=someoneelse).save() + + self.assertEqual(3, BlogPost.objects.count()) + self.Person.objects(name='Test User').delete() + self.assertEqual(1, BlogPost.objects.count()) + + def test_reverse_delete_rule_nullify(self): + """Ensure nullification of references to deleted documents. + """ + class Category(Document): + name = StringField() + + class BlogPost(Document): + content = StringField() + category = ReferenceField(Category, reverse_delete_rule=NULLIFY) + + BlogPost.drop_collection() + Category.drop_collection() + + lameness = Category(name='Lameness') + lameness.save() + + post = BlogPost(content='Watching TV', category=lameness) + post.save() + + self.assertEqual(1, BlogPost.objects.count()) + self.assertEqual('Lameness', BlogPost.objects.first().category.name) + Category.objects.delete() + self.assertEqual(1, BlogPost.objects.count()) + self.assertEqual(None, BlogPost.objects.first().category) + + def test_reverse_delete_rule_deny(self): + """Ensure deletion gets denied on documents that still have references + to them. + """ + class BlogPost(Document): + content = StringField() + author = ReferenceField(self.Person, reverse_delete_rule=DENY) + + BlogPost.drop_collection() + self.Person.drop_collection() + + me = self.Person(name='Test User') + me.save() + + post = BlogPost(content='Watching TV', author=me) + post.save() + + self.assertRaises(OperationError, self.Person.objects.delete) + def test_update(self): """Ensure that atomic updates work properly. """