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

UpdateModelMixin invlidate select_related objects cache #5241

Closed
KushGoyal opened this issue Jun 29, 2017 · 2 comments
Closed

UpdateModelMixin invlidate select_related objects cache #5241

KushGoyal opened this issue Jun 29, 2017 · 2 comments

Comments

@KushGoyal
Copy link

In update method of UpdateModelMixin _prefetched_objects_cache is forcibly invalidated. Same should be done for select_related objects cache.

Steps to reproduce

# models

class Game(models.Model):
    name = models.CharField(max_length=20)
    company = models.ForeignKey(Company)

class Company(models.Model):
    name = models.CharField(max_length=20)

# serializers

class CompanySerializer(serializers.ModelSerializer):
    
    class Meta:
        model = Company
        fields = '__all__'

class GameSerializer(serializers.ModelSerializer):
    company_id = serializers.IntegerField()
    company = CompanySerializer(read_only=True)

    class Meta:
        model = Company
        fields = '__all__'

# views

class GameUpdateDeleteApi(generics.RetrieveUpdateDestroyAPIView):
    serializer_class = GameSerializer

        def get_queryset(self):
            return Game.objects.all().select_related('company')

Expected behavior

send request to game update api with new company_id value. It should update id of company object on game object. And then the updated company object should return with the game object.

request

{
    "name":"angry birds",
    "company_id":3
}

response

{
   "name":"angry birds",
   "company_id":3,
   "company": { "id": 3, "name":"Rovio" }
}

Actual behavior

on sending the update request with new company id, the game object updates correctly but the company object in the response is the old one.

request

{
    "name":"angry birds",
    "company_id":3
}

response

{
   "name":"angry birds",
   "company_id":3,
   "company": { "id": 2, "name":"old company" }
}

If you remove the select_related method on the queryset and try this, then the correct response is returned.

So if it is possible to invalidate the select related cache then the issue should get resolved.

@xordoquy
Copy link
Collaborator

xordoquy commented Jul 4, 2017

There are so many things that can turn can with this use case that I'd advise to use an explicit code to update the instance rather than relying on the default one.
On a side note, invalidating the select_related will have a huge performance issue with the most common use case - nested serializers.

@xordoquy xordoquy closed this as completed Jul 4, 2017
@K0Te
Copy link
Contributor

K0Te commented Aug 8, 2018

It is very counter-intuitive that DRF invalidates prefetch_related() cache, but not select_related().
select_related() works with default update in my case, but not with custom one, as custom does not use reverse name to update/delete related entity.
It took me a lot of time to troubleshoot misbehavior in my project and it might be easy to introduce similar bug in the future.

BTW, it seems that select_related() cache used to be invalidated between changes #4553 and #4668, in case when _prefetched_objects_cache was also present.

Nested serializer performance would be fine if only original select_related() could be re-applied, so that it would result in single DB query. However using .get_object() for this would lead to #4661 again.

Unfortunately I don't see how this can be fixed in generic way in my project, which would avoid potential bugs in the future.
I would be glad to help to fix this one in DRF, but I don't know how to avoid all described issues.

@encode encode deleted a comment from reichert Aug 8, 2018
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

3 participants