Opened 2 years ago

Last modified 2 years ago

#33984 closed Bug

Related managers cache gets stale after saving a fetched model with new PK — at Version 1

Reported by: joeli Owned by: nobody
Component: Database layer (models, ORM) Version: 4.1
Severity: Release blocker Keywords:
Cc: Keryn Knight Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by joeli)

I'm upgrading our codebase from Django 3.2 to 4.1, and came upon a regression when running our test suite. I bisected it down to commit 4f8c7fd9d9 Fixed #32980 -- Made models cache related managers: https://github.com/django/django/commit/4f8c7fd9d91b35e2c2922de4bb50c8c8066cbbc6

The main problem is that when you have fetched a model containing a m2m field from the database, and access its m2m field the manager gets cached. If you then set the model's .pk to None and do a .save() to save it as a copy of the old one, the related manager cache is not cleared. Here's some code with inline comments to demonstrate:

# models.py
from django.db import models

class Tag(models.Model):
    tag = models.SlugField(max_length=64, unique=True)

    def __str__(self):
        return self.tag

class Thing(models.Model):
    tags = models.ManyToManyField(Tag, blank=True)

    def clone(self) -> 'Thing':
        # To clone a thing, we first save a list of the tags it has
        tags = list(self.tags.all())

        # Then set its pk to none and save, creating the copy
        self.pk = None
        self.save()

        # In django 3.2 the following sets the original tags for the new instance.
        # In 4.x it's a no-op because self.tags returns the old instance's manager.
        self.tags.set(tags)
        return self

    @staticmethod
    def has_bug():
        one, _ = Tag.objects.get_or_create(tag='one')
        two, _ = Tag.objects.get_or_create(tag='two')
        
        obj = Thing.objects.create()
        obj.tags.set([one, two])
        new_thing = obj.clone()

        # new_thing.tags.all() returns the expected tags, but it is actually returning obj.tags.all()
        # If we fetch new_thing again it returns the actual tags for new_thing, which is empty.
        #
        # Even `new_thing.refresh_from_db()` -- which is not required with 3.x -- does not help.
        # `new_thing._state.related_managers_cache.clear()` works, but feels like something I
        # shouldn't have to do.
        return list(new_thing.tags.all()) != list(Thing.objects.get(pk=new_thing.pk).tags.all())

Change History (1)

comment:1 by joeli, 2 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top