Assigning a Model Instance to a Foreign Key Attribute Doesn't Cache the Instance

[Andy McCurdy <sedrik@…>]

Currently, when assigning a model instance to a forkeign key attribute doesn't cache the model instance being assigned. Rather, it simply deletes the prior cached attribute.

This causes two undesired behaviors:

• If code later in the execution process references the FK attribute rather than the model instance, a sql query is run to fetch the result, because it's not cached.

• It makes race conditions easy in post-save signals, where in many cases it's easy to end up with multiple instances of the same object.

Oddly enough, it seems GenericForeignKey gets this right. On line# 92 of django/contrib/contenttypes/generic.py, the instance being assigned gets cached on the model. However, on line# 225 of django/db/models/fields/related.py, the cached attribute simple gets deleted.

I believe line# 225 should be changed to save the instance being assigned on the cached attribute.

[Philippe Raoult]

See #17 and #5514. This is not strictly a duplicate but will be solved when those are closed.

[Jacob]

As Andy says, the fix is easy (see the patch I just attached). However, I'm not 100% sure of the ramifications, so I'm going to leave this open for the moment and see if I can't get someone else to take a peek and figure out if this is correct or just papering over a bigger issue.

[Jacob]

After ​discussion on django-dev, I've made a patch that somewhat changes FK assignment to really fix this issue. Specifically:

• Raise a ValueError if you try to assign an object of the wrong type to a FK.
• Raise a ValueError if you try to assign None to an FK that's null=False.
• Cache the value in __set__.

See the tests is the patch for examples.

Again, This is slightly backwards-incompatible, but only the second point -- dealing with None -- is likely to bite anyone. And then only if they're assigning None to a ForeignKey(null=False). I think it's a neccisary breakage, but I'm not committing just yet to let anyone with complaints speak up.

[Jacob]

(In [7574]) Fixed #6886: Tightened up ForeignKey and OneToOne field assignment. Specifically:

• Raise a ValueError if you try to assign the wrong type of object.
• Raise a ValueError if you try to assign None to a field not specified with null=True.
• Cache the set value at set time instead of just at lookup time.

This is a slightly backwards-incompatible change; see BackwardsIncompatibleChanges for more details.

[Gary Wilson]

(In [8185]) Fixed #8070 -- Cache related objects passed to Model init as keyword arguments. Also:

• Model init no longer performs a database query to refetch the related objects it is passed.
• Model init now caches unsaved related objects correctly, too. (Previously, accessing the field would raise DoesNotExist error for null=False fields.)
• Added tests for assigning None to null=True ForeignKey fields (refs #6886).