Index: tests/modeltests/on_delete/__init__.py =================================================================== Index: tests/modeltests/on_delete/tests.py =================================================================== --- tests/modeltests/on_delete/tests.py (revision 0) +++ tests/modeltests/on_delete/tests.py (revision 0) @@ -0,0 +1,144 @@ +from django.test import TestCase +from django.db import models, IntegrityError + +class R(models.Model): + is_default = models.BooleanField(default=False) + + def __str__(self): + return "%s" % self.pk + +get_default_r = lambda: R.objects.get_or_create(is_default=True)[0] + +class S(models.Model): + r = models.ForeignKey(R) + +class T(models.Model): + s = models.ForeignKey(S) + +class A(models.Model): + name = models.CharField(max_length=10) + + auto = models.ForeignKey(R, related_name="auto_set") + auto_nullable = models.ForeignKey(R, null=True, related_name='auto_nullable_set') + setnull = models.ForeignKey(R, on_delete=models.SET_NULL, null=True, related_name='setnull_set') + setdefault = models.ForeignKey(R, on_delete=models.SET_DEFAULT, default=get_default_r, related_name='setdefault_set') + setdefault_none = models.ForeignKey(R, on_delete=models.SET_DEFAULT, default=None, null=True, related_name='setnull_nullable_set') + cascade = models.ForeignKey(R, on_delete=models.CASCADE, related_name='cascade_set') + cascade_nullable = models.ForeignKey(R, on_delete=models.CASCADE, null=True, related_name='cascade_nullable_set') + protect = models.ForeignKey(R, on_delete=models.PROTECT, null=True) + donothing = models.ForeignKey(R, on_delete=models.DO_NOTHING, null=True, related_name='donothing_set') + +def create_a(name): + a = A(name=name) + for name in ('auto', 'auto_nullable', 'setnull', 'setdefault', 'setdefault_none', 'cascade', 'cascade_nullable', 'protect', 'donothing'): + r = R.objects.create() + setattr(a, name, r) + a.save() + return a + +class OnDeleteTests(TestCase): + def test_basics(self): + DEFAULT = get_default_r() + + a = create_a('auto') + a.auto.delete() + self.failIf(A.objects.filter(name='auto').exists()) + + a = create_a('auto_nullable') + a.auto_nullable.delete() + self.failUnlessEqual([a], list(A.objects.all())) + + a = create_a('setnull') + a.setnull.delete() + a = A.objects.get(pk=a.pk) + self.failUnlessEqual(None, a.setnull) + + a = create_a('setdefault') + a.setdefault.delete() + a = A.objects.get(pk=a.pk) + self.failUnlessEqual(DEFAULT, a.setdefault) + + a = create_a('setdefault_none') + a.setdefault_none.delete() + a = A.objects.get(pk=a.pk) + self.failUnlessEqual(None, a.setdefault_none) + + a = create_a('cascade') + a.cascade.delete() + self.failIf(A.objects.filter(name='cascade').exists()) + + a = create_a('cascade_nullable') + a.cascade_nullable.delete() + self.failIf(A.objects.filter(name='cascade_nullable').exists()) + + a = create_a('protect') + self.assertRaises(IntegrityError, a.protect.delete) + + # Testing DO_NOTHING is a bit harder: It would raise IntegrityError for a normal model, + # so we connect to pre_delete and set the fk to a known value. + replacement_r = R.objects.create() + def check_do_nothing(sender, **kwargs): + obj = kwargs['instance'] + obj.donothing_set.update(donothing=replacement_r) + models.signals.pre_delete.connect(check_do_nothing) + a = create_a('do_nothing') + a.donothing.delete() + a = A.objects.get(pk=a.pk) + self.failUnlessEqual(replacement_r, a.donothing) + models.signals.pre_delete.disconnect(check_do_nothing) + + # cleanup + A.objects.all().update(protect=None, donothing=None) + R.objects.all().delete() + self.failIf(A.objects.exists()) + + def test_cache_update(self): + deleted = [] + related_setnull_sets = [] + def pre_delete(sender, **kwargs): + obj = kwargs['instance'] + deleted.append(obj) + if isinstance(obj, R): + related_setnull_sets.append(list(a.pk for a in obj.setnull_set.all())) + + models.signals.pre_delete.connect(pre_delete) + a = create_a('cache_update_setnull') + a.setnull.delete() + + a = create_a('cache_update_setnull') + a.cascade.delete() + + for obj in deleted: + self.failUnlessEqual(None, obj.pk) + + for pk_list in related_setnull_sets: + for a in A.objects.filter(id__in=pk_list): + self.failUnlessEqual(None, a.setnull) + + models.signals.pre_delete.disconnect(pre_delete) + + def test_deletion_order(self): + pre_delete_order = [] + post_delete_order = [] + + def log_post_delete(sender, **kwargs): + pre_delete_order.append((sender, kwargs['instance'].pk)) + + def log_pre_delete(sender, **kwargs): + post_delete_order.append((sender, kwargs['instance'].pk)) + + models.signals.post_delete.connect(log_post_delete) + models.signals.pre_delete.connect(log_pre_delete) + + r = R.objects.create(pk=1) + s1 = S.objects.create(pk=1, r=r) + s2 = S.objects.create(pk=2, r=r) + t1 = T.objects.create(pk=1, s=s1) + t2 = T.objects.create(pk=2, s=s2) + r.delete() + self.failUnlessEqual(pre_delete_order, [(T, 2), (T, 1), (S, 2), (S, 1), (R, 1)]) + self.failUnlessEqual(post_delete_order, [(T, 1), (T, 2), (S, 1), (S, 2), (R, 1)]) + + models.signals.post_delete.disconnect(log_post_delete) + models.signals.post_delete.disconnect(log_pre_delete) + Index: tests/modeltests/on_delete/models.py =================================================================== Index: tests/modeltests/delete/models.py =================================================================== --- tests/modeltests/delete/models.py (revision 11706) +++ tests/modeltests/delete/models.py (working copy) @@ -44,38 +44,6 @@ __test__ = {'API_TESTS': """ ### Tests for models A,B,C,D ### -## First, test the CollectedObjects data structure directly - ->>> from django.db.models.query import CollectedObjects - ->>> g = CollectedObjects() ->>> g.add("key1", 1, "item1", None) -False ->>> g["key1"] -{1: 'item1'} ->>> g.add("key2", 1, "item1", "key1") -False ->>> g.add("key2", 2, "item2", "key1") -False ->>> g["key2"] -{1: 'item1', 2: 'item2'} ->>> g.add("key3", 1, "item1", "key1") -False ->>> g.add("key3", 1, "item1", "key2") -True ->>> g.ordered_keys() -['key3', 'key2', 'key1'] - ->>> g.add("key2", 1, "item1", "key3") -True ->>> g.ordered_keys() -Traceback (most recent call last): - ... -CyclicDependency: There is a cyclic dependency of items to be processed. - - -## Second, test the usage of CollectedObjects by Model.delete() - # Due to the way that transactions work in the test harness, # doing m.delete() here can work but fail in a real situation, # since it may delete all objects, but not in the right order. @@ -111,11 +79,6 @@ >>> c1.save() >>> d1 = D(c=c1, a=a1) >>> d1.save() - ->>> o = CollectedObjects() ->>> a1._collect_sub_objects(o) ->>> o.keys() -[, , , ] >>> a1.delete() # Same again with a known bad order @@ -130,54 +93,17 @@ >>> c2.save() >>> d2 = D(c=c2, a=a2) >>> d2.save() - ->>> o = CollectedObjects() ->>> a2._collect_sub_objects(o) ->>> o.keys() -[, , , ] >>> a2.delete() ### Tests for models E,F - nullable related fields ### -## First, test the CollectedObjects data structure directly - ->>> g = CollectedObjects() ->>> g.add("key1", 1, "item1", None) -False ->>> g.add("key2", 1, "item1", "key1", nullable=True) -False ->>> g.add("key1", 1, "item1", "key2") -True ->>> g.ordered_keys() -['key1', 'key2'] - -## Second, test the usage of CollectedObjects by Model.delete() - >>> e1 = E() >>> e1.save() >>> f1 = F(e=e1) >>> f1.save() >>> e1.f = f1 >>> e1.save() - -# Since E.f is nullable, we should delete F first (after nulling out -# the E.f field), then E. - ->>> o = CollectedObjects() ->>> e1._collect_sub_objects(o) ->>> o.keys() -[, ] - -# temporarily replace the UpdateQuery class to verify that E.f is actually nulled out first ->>> import django.db.models.sql ->>> class LoggingUpdateQuery(django.db.models.sql.UpdateQuery): -... def clear_related(self, related_field, pk_list): -... print "CLEARING FIELD",related_field.name -... return super(LoggingUpdateQuery, self).clear_related(related_field, pk_list) ->>> original_class = django.db.models.sql.UpdateQuery ->>> django.db.models.sql.UpdateQuery = LoggingUpdateQuery >>> e1.delete() -CLEARING FIELD f >>> e2 = E() >>> e2.save() @@ -185,18 +111,6 @@ >>> f2.save() >>> e2.f = f2 >>> e2.save() - -# Same deal as before, though we are starting from the other object. - ->>> o = CollectedObjects() ->>> f2._collect_sub_objects(o) ->>> o.keys() -[, ] - >>> f2.delete() -CLEARING FIELD f - -# Put this back to normal ->>> django.db.models.sql.UpdateQuery = original_class """ } Index: django/db/models/base.py =================================================================== --- django/db/models/base.py (revision 11706) +++ django/db/models/base.py (working copy) @@ -523,7 +523,7 @@ (model_class, {pk_val: obj, pk_val: obj, ...}), ...] """ pk_val = self._get_pk_val() - if seen_objs.add(self.__class__, pk_val, self, parent, nullable): + if seen_objs.add(self, parent, nullable): return for related in self._meta.get_all_related_objects(): @@ -534,7 +534,7 @@ except ObjectDoesNotExist: pass else: - sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null) + related.field.rel.on_delete(self, related, sub_obj, seen_objs) else: # To make sure we can access all elements, we can't use the # normal manager on the related object. So we work directly @@ -547,7 +547,7 @@ raise AssertionError("Should never get here.") delete_qs = rel_descriptor.delete_manager(self).all() for sub_obj in delete_qs: - sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null) + related.field.rel.on_delete(self, related, sub_obj, seen_objs) # Handle any ancestors (for the model-inheritance case). We do this by # traversing to the most remote parent classes -- those with no parents Index: django/db/models/fields/related.py =================================================================== --- django/db/models/fields/related.py (revision 11706) +++ django/db/models/fields/related.py (working copy) @@ -1,4 +1,4 @@ -from django.db import connection, transaction +from django.db import connection, transaction, IntegrityError from django.db.backends import util from django.db.models import signals, get_model from django.db.models.fields import AutoField, Field, IntegerField, PositiveIntegerField, PositiveSmallIntegerField, FieldDoesNotExist @@ -18,6 +18,31 @@ RECURSIVE_RELATIONSHIP_CONSTANT = 'self' +def CASCADE(obj, related, sub_obj, seen_objs): + sub_obj._collect_sub_objects(seen_objs, obj.__class__) + +def PROTECT(obj, related, sub_obj, seen_objs): + msg = '[Django] Cannot delete a parent object: a foreign key constraint fails (ForeignKey `%s` (pk=`%s`) references `%s` (pk=`%s`))' % ( + sub_obj.__class__, sub_obj.pk, + obj.__class__, obj.pk, + ) + raise IntegrityError(msg) + +def SET(value): + def set_on_delete(obj, related, sub_obj, seen_objs): + seen_objs.add(sub_obj, related.field, value) + return set_on_delete + +def SET_NULL(obj, related, sub_obj, seen_objs): + seen_objs.add_field_update(sub_obj, related.field, None) + +def SET_DEFAULT(obj, related, sub_obj, seen_objs): + seen_objs.add_field_update(sub_obj, related.field, related.field.get_default()) + +def DO_NOTHING(obj, related, sub_obj, seen_objs): + pass + + pending_lookups = {} def add_lazy_relation(cls, field, relation, operation): @@ -628,7 +653,8 @@ class ManyToOneRel(object): def __init__(self, to, field_name, related_name=None, - limit_choices_to=None, lookup_overrides=None, parent_link=False): + limit_choices_to=None, lookup_overrides=None, parent_link=False, + on_delete=None): try: to._meta except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT @@ -641,6 +667,7 @@ self.lookup_overrides = lookup_overrides or {} self.multiple = True self.parent_link = parent_link + self.on_delete = on_delete def get_related_field(self): """ @@ -655,10 +682,12 @@ class OneToOneRel(ManyToOneRel): def __init__(self, to, field_name, related_name=None, - limit_choices_to=None, lookup_overrides=None, parent_link=False): + limit_choices_to=None, lookup_overrides=None, parent_link=False, + on_delete=None): super(OneToOneRel, self).__init__(to, field_name, related_name=related_name, limit_choices_to=limit_choices_to, - lookup_overrides=lookup_overrides, parent_link=parent_link) + lookup_overrides=lookup_overrides, parent_link=parent_link, + on_delete=on_delete) self.multiple = False class ManyToManyRel(object): @@ -692,12 +721,20 @@ assert not to._meta.abstract, "%s cannot define a relation with abstract class %s" % (self.__class__.__name__, to._meta.object_name) to_field = to_field or to._meta.pk.name kwargs['verbose_name'] = kwargs.get('verbose_name', None) + + on_delete = kwargs.pop('on_delete', None) + if on_delete is None: + if kwargs.get('null', False): + on_delete = SET_NULL + else: + on_delete = CASCADE kwargs['rel'] = rel_class(to, to_field, related_name=kwargs.pop('related_name', None), limit_choices_to=kwargs.pop('limit_choices_to', None), lookup_overrides=kwargs.pop('lookup_overrides', None), - parent_link=kwargs.pop('parent_link', False)) + parent_link=kwargs.pop('parent_link', False), + on_delete=on_delete) Field.__init__(self, **kwargs) self.db_index = True @@ -742,6 +779,14 @@ target = self.rel.to._meta.db_table cls._meta.duplicate_targets[self.column] = (target, "o2m") + on_delete = self.rel.on_delete + if on_delete == SET_NULL and not self.null: + specification = "'on_delete=SET_NULL'" + raise ValueError("%s specified for %s '%s.%s', but the field is not nullable." % (specification, type(self).__name__, cls.__name__, name)) + if on_delete == SET_DEFAULT and not self.has_default(): + specification = "'on_delete=SET_DEFAULT'" + raise ValueError("%s specified for %s '%s.%s', but the field has no default value." % (specification, type(self).__name__, cls.__name__, name)) + def contribute_to_related_class(self, cls, related): setattr(cls, related.get_accessor_name(), ForeignRelatedObjectsDescriptor(related)) Index: django/db/models/__init__.py =================================================================== --- django/db/models/__init__.py (revision 11706) +++ django/db/models/__init__.py (working copy) @@ -11,6 +11,7 @@ from django.db.models.fields.subclassing import SubfieldBase from django.db.models.fields.files import FileField, ImageField from django.db.models.fields.related import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel, ManyToManyRel, OneToOneRel +from django.db.models.fields.related import CASCADE, PROTECT, SET, SET_NULL, SET_DEFAULT, DO_NOTHING from django.db.models import signals # Admin stages. Index: django/db/models/query.py =================================================================== --- django/db/models/query.py (revision 11706) +++ django/db/models/query.py (working copy) @@ -1011,60 +1011,27 @@ else: forced_managed = False try: - ordered_classes = seen_objs.keys() - except CyclicDependency: - # If there is a cyclic dependency, we cannot in general delete the - # objects. However, if an appropriate transaction is set up, or if the - # database is lax enough, it will succeed. So for now, we go ahead and - # try anyway. - ordered_classes = seen_objs.unordered_keys() + try: + seen_objs.sort() + except CyclicDependency: + pass - obj_pairs = {} - try: - for cls in ordered_classes: - items = seen_objs[cls].items() - items.sort() - obj_pairs[cls] = items + for obj in seen_objs: + signals.pre_delete.send(sender=obj.__class__, instance=obj) + + seen_objs.execute_update() - # Pre-notify all instances to be deleted. - for pk_val, instance in items: - signals.pre_delete.send(sender=cls, instance=instance) + seen_objs.execute_delete_related() + + seen_objs.reverse_instances() + + seen_objs.execute_delete() - pk_list = [pk for pk,instance in items] - del_query = sql.DeleteQuery(cls, connection) - del_query.delete_batch_related(pk_list) + for obj in seen_objs: + signals.post_delete.send(sender=obj.__class__, instance=obj) + + seen_objs.update_instances() - update_query = sql.UpdateQuery(cls, connection) - for field, model in cls._meta.get_fields_with_model(): - if (field.rel and field.null and field.rel.to in seen_objs and - filter(lambda f: f.column == field.rel.get_related_field().column, - field.rel.to._meta.fields)): - if model: - sql.UpdateQuery(model, connection).clear_related(field, - pk_list) - else: - update_query.clear_related(field, pk_list) - - # Now delete the actual data. - for cls in ordered_classes: - items = obj_pairs[cls] - items.reverse() - - pk_list = [pk for pk,instance in items] - del_query = sql.DeleteQuery(cls, connection) - del_query.delete_batch(pk_list) - - # Last cleanup; set NULLs where there once was a reference to the - # object, NULL the primary key of the found objects, and perform - # post-notification. - for pk_val, instance in items: - for field in cls._meta.fields: - if field.rel and field.null and field.rel.to in seen_objs: - setattr(instance, field.attname, None) - - signals.post_delete.send(sender=cls, instance=instance) - setattr(instance, cls._meta.pk.attname, None) - if forced_managed: transaction.commit() else: Index: django/db/models/query_utils.py =================================================================== --- django/db/models/query_utils.py (revision 11706) +++ django/db/models/query_utils.py (working copy) @@ -39,22 +39,21 @@ """ def __init__(self, previously_seen=None): + # {model: {(field, value): set([instances])}} + self.field_updates = {} self.data = {} self.children = {} if previously_seen: - self.blocked = previously_seen.blocked - for cls, seen in previously_seen.data.items(): - self.blocked.setdefault(cls, SortedDict()).update(seen) + self.blocked = set(previously_seen) + self.blocked.update(previously_seen.blocked) else: - self.blocked = {} + self.blocked = set() - def add(self, model, pk, obj, parent_model, nullable=False): + def add(self, obj, parent_model, nullable=False): """ Adds an item to the container. Arguments: - * model - the class of the object being added. - * pk - the primary key. * obj - the object itself. * parent_model - the model of the parent object that this object was reached through. @@ -62,68 +61,95 @@ Returns True if the item already existed in the structure and False otherwise. - """ - if pk in self.blocked.get(model, {}): + """ + if obj in self.blocked: return True - d = self.data.setdefault(model, SortedDict()) - retval = pk in d - d[pk] = obj + model = obj.__class__ + instances = self.data.setdefault(model, []) + seen_before = obj in instances + if not seen_before: + instances.append(obj) # Nullable relationships can be ignored -- they are nulled out before # deleting, and therefore do not affect the order in which objects # have to be deleted. if parent_model is not None and not nullable: - self.children.setdefault(parent_model, []).append(model) - return retval + self.children.setdefault(parent_model, set()).add(model) + return seen_before + + def add_field_update(self, obj, field, value): + updates = self.field_updates.setdefault(obj.__class__, dict()) + updates.setdefault((field, value), set()).add(obj) + + def __iter__(self): + for instances in self.data.itervalues(): + for obj in instances: + yield obj - def __contains__(self, key): - return self.data.__contains__(key) - - def __getitem__(self, key): - return self.data[key] - def __nonzero__(self): return bool(self.data) - def iteritems(self): - for k in self.ordered_keys(): - yield k, self[k] - - def items(self): - return list(self.iteritems()) - - def keys(self): - return self.ordered_keys() - - def ordered_keys(self): - """ - Returns the models in the order that they should be dealt with (i.e. - models with no dependencies first). - """ - dealt_with = SortedDict() - # Start with items that have no children + def sort(self): + for instances in self.data.itervalues(): + instances.sort(key=lambda obj: obj.pk) + + sorted_models = [] models = self.data.keys() - while len(dealt_with) < len(models): + while len(sorted_models) < len(models): found = False for model in models: - if model in dealt_with: + if model in sorted_models: continue - children = self.children.setdefault(model, []) - if len([c for c in children if c not in dealt_with]) == 0: - dealt_with[model] = None + children = self.children.get(model) + if not children or not children.difference(sorted_models): + sorted_models.append(model) found = True if not found: - raise CyclicDependency( - "There is a cyclic dependency of items to be processed.") + raise CyclicDependency() + self.data = SortedDict([(model, self.data[model]) for model in sorted_models]) - return dealt_with.keys() + def reverse_instances(self): + for instances in self.data.itervalues(): + instances.reverse() - def unordered_keys(self): - """ - Fallback for the case where is a cyclic dependency but we don't care. - """ - return self.data.keys() + def execute_delete_related(self): + from django.db.models import connection, sql + for model, instances in self.data.iteritems(): + pk_list = [obj.pk for obj in instances] + del_query = sql.DeleteQuery(model, connection) + del_query.delete_batch_related(pk_list) + def execute_delete(self): + from django.db.models import connection, sql + for model, instances in self.data.iteritems(): + pk_list = [obj.pk for obj in instances] + del_query = sql.DeleteQuery(model, connection) + del_query.delete_batch(pk_list) + + def execute_update(self): + from django.db.models import connection, sql + from django.db.models.sql.constants import GET_ITERATOR_CHUNK_SIZE + for model, instances_for_fieldvalues in self.field_updates.iteritems(): + for (field, value), instances in instances_for_fieldvalues.iteritems(): + pk_field = model._meta.pk + pk_list = [obj.pk for obj in instances] + for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE): + query = sql.UpdateQuery(model, connection) + query.where = query.where_class() + query.where.add((sql.where.Constraint(None, pk_field.column, pk_field), 'in', pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]), sql.where.AND) + query.add_update_values({field.name: value}) + query.execute_sql() + + def update_instances(self): + for model, instances_for_fieldvalues in self.field_updates.iteritems(): + for (field, value), instances in instances_for_fieldvalues.iteritems(): + for obj in instances: + setattr(obj, field.attname, value) + for model, instances in self.data.iteritems(): + for instance in instances: + setattr(instance, model._meta.pk.attname, None) + + class QueryWrapper(object): """ A type that indicates the contents are an SQL fragment and the associate