AltSchool / dynamic-rest

Dynamic extensions for Django REST Framework
MIT License
831 stars 109 forks source link

Confusing behavior of DynamicField.requires #373

Open d-ryzhykau opened 1 month ago

d-ryzhykau commented 1 month ago

If the last part of the path in DynamicField.requires attribute is a relation and not a field name, the prefetch is not done. Example:

# models.py
from django.db import models

class Artist(models.Model):
    name = models.CharField(max_length=10)

class Album(models.Model):
    artist = models.ForeignKey(Artist, on_delete=models.CASCADE)
    name = models.CharField(max_length=256)

class Song(models.Model):
    album = models.CharField(max_length=256)
    name = models.ForeignKey(Artist, on_delete=models.CASCADE)

# serializers.py
from dynamic_rest.serializers import DynamicModelSerializer
from dynamic_rest.fields import DynamicMethodField

class SongSerializer(DynamicModelSerializer):
    display_name = DynamicMethodField(requires=["album.artist"])

    class Meta:
        model = Song
        fields = ["pk", "display_name"]

    def get_display_name(self, song):
        return f"{song.album.artist.name} - {song.name} (from '{song.album.name}')"

If the display_name is requested, song.album will be prefetched, but song.album.artist won't be and will result in N+1 queries for each serialized Song object. This can be resolved by using album.artist.name, album.artist. or album.artist.* as the value of display_name.requires. Is this the expected behavior? If so, it would be helpful to have it mentioned explicitly in the docs. Otherwise, it's necessary to fix the DynamicFilterBackend._build_implicit_prefetches method to include prefetches of relations with empty path remainders. Either way I'll be glad to provide a PR with fixes.