Required properties removed from component if PATCH processed first

[rvinzent]

Describe the bug
I drilled down into the source to find what was causing issues with required properties in some _Request components when COMPONENT_SPLIT_REQUEST is set to True: #243

I believe I've finally found the issue. In short, it seems to be related to a registered component for a PATCH request being re-used for a POST request when they are not strictly equivalent. This issue is very hard to reproduce properly, but I will try to describe here.

You must process an unrelated PATCH request that happens to use this component in the body prior to processing this component in a POST request context.

To Reproduce

• COMPONENT_SPLIT_REQUEST = True
• COMPONENT_SPLIT_PATCH = True

class XModel(models.Model):
    read_only_field = models.TextField()
    some_field = models.TextField()


class YModel(models.Model):
    x_field = models.ForeignKey(XModel, on_delete=models.CASCADE)


class XSerializer(serializers.Serializer):
    read_only_field = serializers.CharField(read_only=True)
    some_field = serializers.CharField()


class XViewSet(viewsets.ModelViewSet):
    http_method_names = ("post", "patch")
    serializer_class = XSerializer
    queryset = XModel.objects.all()


class YSerializer(serializers.Serializer):
    x_field = XSerializer(read_only=True)


class YViewSet(viewsets.ModelViewSet):
    http_method_names = ("patch",)
    serializer_class = YSerializer
    queryset = YModel.objects.all()

# urls.py

# YView endpoints must be processed first
router.register("AAAA-y-view", YViewSet)
router.register("ZZZZ-x-view")

Note that the bug will not occur if a POST request is processed first for this component. I have confirmed the behavior by placing a breakpoint here.

When you hit the breakpoint, manually call self._map_serializer(serializer, direction) to see the generated schema. As expected, required properties are removed from the PatchedXRequest component.

The issue occurs when an unrelated PATCH method is processed first, and this causes XSerializer to be added to the component registry.

1. PATCH request for y-view that uses XSerializer as some participating component
2. a schema is generated, and added to the component registry without required fields due to this check
3. subsequent calls (e.g. the POST request for x-view) will hit the component registry and pull the first generated schema that does not contain the required fields!

This schema is produced. Note the missing required fields.

    X:
      type: object
      properties:
        read_only_field:
          type: string
          readOnly: true
        some_field:
          type: string
      required:
      - read_only_field
      - some_field
    XRequest:    # missing required fields!
      type: object
      properties:
        some_field:
          type: string
    Y:
      type: object
      properties:
        x_field:
          allOf:
          - $ref: '#/components/schemas/X'
          readOnly: true
      required:
      - x_field

I've tried to add as much reproduction as I can, but I hope the gist is clear enough. Basically, sometimes components are first registered in some kind of PATCH context, which excludes required fields, and when it comes time to create the Request component for the POST request, it finds the existing component in the registry that does not have the required fields. If _map_serializer is called again manually, it produces different schema than that returned from the component registry.

I've finally gotten a repro, but I cannot tell what part of it triggers the behavior. Something I've noticed is that sometimes _Request components are generated for serializers that are always read only. The components are unused in the generated spec.

I can't paste my original source unfortunately but please let me know if you need anything else! Thank you!

[tfranzel]

i can vaguely remember a similar issue. i think i get the gist and will try to get to the bottom of this. it might take a moment for me schedule some time and i may have some feedback questions then, but i'm sure we can sort this out.

[tfranzel]

thank you for providing the reproduction. it was quite helpful! as far as i understand this should only happen with COMPONENT_SPLIT_PATCH=True and read_only nested serializers. there was a disparity between naming and generating components for this particular case. the patched component (without required) was registered under the non-patched name and after that reused as it already "existed" in the registry.

the fix is basically this single line. the rest was just cascading changes and another related problem that surfaced on closer inspection. 6febe8f#diff-4d081c8c817b2246109e52886d6b1f078858a2d5c99ea6e694796e39a7d96d77L1005

i tried to break down the problem without COMPONENT_SPLIT_REQUEST. can you confirm your problem was resolved by this fix?

[rvinzent]

@tfranzel fixed, thank you!