rsinger86/drf-flex-fields

FlexFieldsFilterBackend does not handle "GenericForeignKey"

KrYpTeD974 opened this issue · 2 comments

When calling a ViewSet that has both the FlexFieldsFilterBackend and a serializer containing a field pointing to a GenericForeignKey, we get the error : AttributeError: 'GenericForeignKey' object has no attribute 'attname'

It can easily be reproduced with the following classes :
models

class TaggedItem(models.Model):
    tag = models.SlugField()
    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
    object_id = models.PositiveIntegerField()
    content_object = GenericForeignKey('content_type', 'object_id')
    link = ForeignKey(SomeTaggableModel, null=True, on_delete=models.CASCADE)

serializers

class TaggedItemSerializer(FlexFieldsSerializerMixin, ModelSerializer):
    content_object = PrimaryKeyRelatedField(read_only=True) 
    class Meta:
        model = TaggedItem
        fields = (
            "id",
            "content_type",
            "object_id",
            "content_object"
        )

Please notice that the PrimaryKeyRelatedField is only here for simplicity, in real life it could be a GenericRelatedField from
rest-framework-generic-relations.

views

class TaggedItemViewSet(ModelViewSet):
    serializer_class = TaggedItemSerializer
    queryset = TaggedItem.objects.all()
    filter_backends = [FlexFieldsFilterBackend]

After digging a bit, I found that the field gets included in the queryset.only(...) and crashes probably because it is not a concrete field. A GenericForeignKey should, instead, be included in the queryset.prefetch_related(...) fields.

As a Fix I, updated the FlexFieldsFilterBackend to include some new conditions so that the field is "routed" into the prefetch_related fields.

To differenciate the GenericForeignKey from a ForeignKey, I considered that a GenericForeignKey field has :
is_relation = True, many_to_one = True and concrete = False, which seems to work but I am not really sure if there are other edge cases.

class FlexFieldsFilterBackend(FlexFieldsDocsFilterBackend):
    def filter_queryset(
        self, request: Request, queryset: QuerySet, view: GenericViewSet
    ):
        if (
            not issubclass(view.get_serializer_class(), FlexFieldsSerializerMixin)
            or request.method != "GET"
        ):
            return queryset

        auto_remove_fields_from_query = getattr(
            view, "auto_remove_fields_from_query", True
        )
        auto_select_related_on_query = getattr(
            view, "auto_select_related_on_query", True
        )
        required_query_fields = list(getattr(view, "required_query_fields", []))

        serializer = view.get_serializer(  # type: FlexFieldsSerializerMixin
            context=view.get_serializer_context()
        )

        serializer.apply_flex_fields(
            serializer.fields, serializer._flex_options_rep_only
        )
        serializer._flex_fields_rep_applied = True

        model_fields = []
        nested_model_fields = []
        for field in serializer.fields.values():
            model_field = self._get_field(field.source, queryset.model)
            if model_field:
                model_fields.append(model_field)
                if field.field_name in serializer.expanded_fields or \
                        (model_field.is_relation and not model_field.many_to_one) or \
                        (model_field.is_relation and model_field.many_to_one and not model_field.concrete):  # Include GenericForeignKey
                    nested_model_fields.append(model_field)

        if auto_remove_fields_from_query:
            queryset = queryset.only(
                *(
                    required_query_fields
                    + [
                        model_field.name
                        for model_field in model_fields if (
                            not model_field.is_relation or
                            model_field.many_to_one and model_field.concrete)  # Exclude not concrete fields
                    ]
                )
            )

        if auto_select_related_on_query and nested_model_fields:
            queryset = queryset.select_related(
                *(
                    model_field.name
                    for model_field in nested_model_fields if (
                            model_field.is_relation and
                            model_field.many_to_one and
                            model_field.concrete)  # Exclude GenericForeignKey
                )
            )

            queryset = queryset.prefetch_related(*(
                model_field.name for model_field in nested_model_fields if
                (model_field.is_relation and not model_field.many_to_one) or
                (model_field.is_relation and model_field.many_to_one and not model_field.concrete)  # Include GenericForeignKey)
                )
            )

        return queryset

I am going to submit a PullRequest that also includes tests for this case.

@rsinger86 Do you have any feedback on this issue and PR ?