Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

M2M querysets not being invalidated on instance deletion #407

Open
tumb1er opened this issue Aug 16, 2021 · 6 comments
Open

M2M querysets not being invalidated on instance deletion #407

tumb1er opened this issue Aug 16, 2021 · 6 comments

Comments

@tumb1er
Copy link
Contributor

tumb1er commented Aug 16, 2021

We have a model with a non-autoincremental primary key, which has an m2m relation.

After deleting and creating an instance with same primary keys, we are getting stale m2m cache.
This is mostly because of Django not sending m2m_changed on instance deletion (see https://code.djangoproject.com/ticket/17688).

I'll add a PR with a failing test later, but have no good decision of fixing this on the cacheops side. Also, the bug was very difficult to catch, lots of conditions must be satisfied, so I'd like to propose to add a workaround for Django on the cacheops side.

class Profile(Model):
   user = OneToOneField(User, CASCADE, primary_key=True)
   roles = ManyToManyField(Role, blank=True)

user = User.objects.create()
profile = Profile.objects.create(user=user)
role = Role.objects.create()

profile.roles.set([role])

assert len(list(profile.roles.all())) == 1

profile.delete()  # Here m2m relation is not invalidated

profile = Profile.objects.create(user=user)

assert len(list(profile.roles.all())) == 0  # Assertion fails

# Workaround

@receiver(pre_delete, sender=Profile)
def invalidate_m2m_after_delete(sender, instance, **kwargs):
    invalidate_dict(Profile.roles.through, {'profile_id': instance.pk})
@Suor
Copy link
Owner

Suor commented Aug 16, 2021

Ok, waiting for a test.

tumb1er added a commit to tumb1er/django-cacheops that referenced this issue Aug 16, 2021
@tumb1er
Copy link
Contributor Author

tumb1er commented Aug 16, 2021

Failing test here #408

I see multiple ways:

  1. Catch pre_delete and traverse all m2m relations with calling "invalidate_dict" as in example workaround
  2. Add m2m-related queries to "profile" conj (now it's on "through" model and on "role" side - see below)
  3. Wait till Django issue will be fixed :(
1629124268.381713 [0 lua] "sadd" "schemes:profiles_role" ""
1629124268.381742 [0 lua] "sadd" "conj:profiles_role:" "q:d7b4c0ab39dd2d595b2dc6f70463e1a8"
1629124268.381784 [0 lua] "ttl" "conj:profiles_role:"
1629124268.381807 [0 lua] "expire" "conj:profiles_role:" "172810"
1629124268.381835 [0 lua] "sadd" "schemes:profiles_profile_roles" "profile_id"
1629124268.381871 [0 lua] "sadd" "conj:profiles_profile_roles:profile_id=17" "q:d7b4c0ab39dd2d595b2dc6f70463e1a8"
1629124268.381910 [0 lua] "ttl" "conj:profiles_profile_roles:profile_id=17"
1629124268.381937 [0 lua] "expire" "conj:profiles_profile_roles:profile_id=17" "172810"

@Suor
Copy link
Owner

Suor commented Aug 16, 2021

So, it's Django issue? Did you file a ticket there?

@tumb1er
Copy link
Contributor Author

tumb1er commented Aug 16, 2021

It's a ten years old ticket without any movement on last 5 years.
https://code.djangoproject.com/ticket/17688

So I think there will be no fix at Django side

@Suor
Copy link
Owner

Suor commented Aug 17, 2021

I'll take a look.

@Suor
Copy link
Owner

Suor commented Aug 30, 2021

Unfortunately there is no quick fix for this.

Suor pushed a commit that referenced this issue Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants