From 53da1e47942f22a56e761d786ba89d05ca55a224 Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Fri, 6 Mar 2009 04:51:05 +0000 Subject: [PATCH] Fixed #9649 -- Better error handling in model creation. Previously, you could explicitly assign None to a non-null ForeignKey (or other) field when creating the model (Child(parent=None), etc). We now throw an exception when you do that, which matches the behaviour when you assign None to the attribute after creation. Thanks to ales.zoulek@gmail.com and ondrej.kohout@gmail.com for some analysis of this problem. git-svn-id: http://code.djangoproject.com/svn/django/trunk@9983 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/base.py | 13 +++++++------ tests/regressiontests/many_to_one_regress/models.py | 13 ++++++++++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 50a701dce2..e704db2918 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -224,12 +224,13 @@ class Model(object): # keywords, or default. for field in fields_iter: - rel_obj = None + is_related_object = False if kwargs: if isinstance(field.rel, ManyToOneRel): try: # Assume object instance was passed in. rel_obj = kwargs.pop(field.name) + is_related_object = True except KeyError: try: # Object instance wasn't passed in -- must be an ID. @@ -245,11 +246,11 @@ class Model(object): val = kwargs.pop(field.attname, field.get_default()) else: val = field.get_default() - # If we got passed a related instance, set it using the field.name - # instead of field.attname (e.g. "user" instead of "user_id") so - # that the object gets properly cached (and type checked) by the - # RelatedObjectDescriptor. - if rel_obj: + if is_related_object: + # If we are passed a related instance, set it using the + # field.name instead of field.attname (e.g. "user" instead of + # "user_id") so that the object gets properly cached (and type + # checked) by the RelatedObjectDescriptor. setattr(self, field.name, rel_obj) else: setattr(self, field.attname, val) diff --git a/tests/regressiontests/many_to_one_regress/models.py b/tests/regressiontests/many_to_one_regress/models.py index 2039bd5ddf..3f35bef62c 100644 --- a/tests/regressiontests/many_to_one_regress/models.py +++ b/tests/regressiontests/many_to_one_regress/models.py @@ -1,5 +1,5 @@ """ -Regression tests for a few FK bugs: #1578, #6886 +Regression tests for a few ForeignKey bugs. """ from django.db import models @@ -106,6 +106,17 @@ Traceback (most recent call last): ... ValueError: Cannot assign "": "Child.parent" must be a "Parent" instance. +# Nor can you explicitly assign None to Child.parent during object creation +# (regression for #9649). +>>> Child(name='xyzzy', parent=None) +Traceback (most recent call last): + ... +ValueError: Cannot assign None: "Child.parent" does not allow null values. +>>> Child.objects.create(name='xyzzy', parent=None) +Traceback (most recent call last): + ... +ValueError: Cannot assign None: "Child.parent" does not allow null values. + # Creation using keyword argument should cache the related object. >>> p = Parent.objects.get(name="Parent") >>> c = Child(parent=p)