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

Is there a way to force schema to output a Dict instead of an Array? #395

Closed
pablodgonzalez opened this issue May 20, 2021 · 12 comments
Closed
Labels
bug Something isn't working fix confirmation pending issue has been fixed and confirmation from issue reporter is pending

Comments

@pablodgonzalez
Copy link

pablodgonzalez commented May 20, 2021

Describe the bug
Hi, I've changed the default list_serializer_class by an DictSerializer but I have not found a way to change the schema according it. Maybe there is some way to do it but i don't know. I tried with extension schemas, and fileds and custom serializerextensions without success.

class BrandDictSerializer(serializers.ModelSerializer)
    Meta:
        model = Brand
        fieds = (id, name)
        list_serializer_class = DictSerializer

class DictSerializer(serializers.ListSerializer):
    dict_key = id

    @property
    def data(self):
        ret = super(serializers.ListSerializer, self).data
        return ReturnDict(ret, serializer=self)

    def to_representation(self, data):
        items = super(DictSerializer, self).to_representation(data)
        return {item[self.dict_key]: item for item in items}

Expected behavior
I expect the schema get an object type with additionalProperties of type Brand

"BrandDic": {
      "type": "object",
      additionalProperties:
      "properties": {
                    "id": {
                        "type": "integer",
                        "readOnly": true
                    },
                    "brand": {
                        "$ref": "#/components/schemas/Brand"
                    }
  }

@tfranzel
Copy link
Owner

Hi! DRF is certainly a customization rabbit hole. never seen it being modified there. pretty ingenious!

SerializerExtension is the right thing to do. the reason the extension is not called is that ListSerializer contains magic meta-programming and has a special place in spectacular's processing. the list gets unwrapped before the extension can be called (link below). It would work if DictSerializer would not be derived from ListSerializer

component = self.resolve_serializer(serializer.child, 'request')

this issue is partially related to #391 (and by association #351)

@pablodgonzalez
Copy link
Author

Thanks @tfranzel , So, If I understood you I need change DictSerializer (or better BrandDictSerializer) to receive the array values but no using it with "many=True". Is it possible???

@tfranzel
Copy link
Owner

not even sure if i would call this a bug. you derive from serializers.ListSerializer but then make it so that it is not a list anymore. that is basically the violated assumption that spectacular makes.

SerializerExtensions at the moment work for everything except ListSerializer (and ListField as it doing the same thing)

If you copy the contents of serializers.ListSerializer to DictSerializer and make it not derived from ListSerializer, the extension should work.

as you already, know XXX(many=True) is just syntactic sugar for list_serializer_class(child=XXX). so the many=True is not the problem but the isinstance(DictSerializer(child=BrandDictSerializer()), ListSerializer) == True

@pablodgonzalez
Copy link
Author

pablodgonzalez commented May 25, 2021

hi @tfranzel, How are you? I think now, need your help.
Well I he changed a little my implementation but, right now drf-spectacular needs some "fix". You should see it.

class DictSerializer(serializers.BaseSerializer):

    dict_key = 'id'
    child = None
    many=True

    def __init__(self, *args, **kwargs):
        self.child = kwargs.pop('child', copy.deepcopy(self.child))
        self.dict_key = kwargs.pop('dict_key', 'id')
        assert self.child is not None, '`child` is a required argument.'
        assert not inspect.isclass(self.child), '`child` has not been instantiated.'
        super().__init__(*args, **kwargs)
        self.child.bind(field_name='', parent=self)

    def get_initial(self):
        if hasattr(self, 'initial_data'):
            return self.to_representation(self.initial_data)
        return {}

    def get_value(self, dictionary):
        """
        Given the input dictionary, return the field value.
        """
        # We override the default field access in order to support
        # dict in HTML forms.
        if html.is_html_input(dictionary):
            return html.parse_html_dict(dictionary, prefix=self.field_name)
        return dictionary.get(self.field_name, empty)

    @property
    def data(self):
        """
        Overriden to return a ReturnDict instead of a ReturnList.
        """
        ret = super().data
        return ReturnDict(self.data, serializer=self)

    def to_representation(self, data):
        """
        Converts the data from a list to a dictionary.
        """
        items = serializers.ListSerializer(data=data, child=self.child).to_representation(data)
        return {item[self.dict_key]: item for item in items}

class DictModelSerializer(serializers.ModelSerializer):

    @classmethod
    def many_init(cls, *args, **kwargs):
        is_dict = kwargs.pop('dict', False)
        dict_key = kwargs.pop('dict_key', 'id')
        if not is_dict:
            return super().many_init(cls,*args, **kwargs)

        child_serializer = cls(*args, **kwargs)
        list_kwargs = {
            'child': child_serializer,
            'dict_key': dict_key
        }
        list_kwargs.update({
            key: value for key, value in kwargs.items()
            if key in serializers.LIST_SERIALIZER_KWARGS
        })
        meta = getattr(cls, 'Meta', None)
        dict_serializer_class = getattr(meta, 'list_serializer_class', DictSerializer)
        return dict_serializer_class(*args, **list_kwargs)

class ContainerSerializer(serializers.Serializer):
  brands = BrandSerializer(source='*', dict=True, many=True)
  other = OtherSerializer()

class BrandSerializer(DictModelSerializer):
  class Meta:
    model = Brand
    fields = ('id', 'country')

class OpenAPIDictSerializerExtension(OpenApiSerializerExtension):
    target_class = 'mymodule.DictSerializer'

    def get_name(self):
        return self.target.child.__class__.__name__.replace("Serializer", "Dict")

    def map_serializer(self, auto_schema, direction):
        sub_components = []
        serializer = self.target
        meta = auto_schema._get_serializer_field_meta(self.target)
        component = auto_schema.resolve_serializer(self.target.child, direction)
        content = build_basic_type(OpenApiTypes.OBJECT)
        content['additionalProperties'] = auto_schema._map_serializer_field(self.target.child, direction)

        return append_meta(content, meta) if component else None

I get this output

"Container": {
        "brands": {
            "1": {
                "id": 1,
                "country": 10
            },
            "3": {
                "id": 3,
                "country": 10
            }
        },
     "other": "other"
    }

The output is what i expect but the schema don't
I expect the below schema

Container:
  type: object
      properties:
        other:
          type: string
          readOnly: true
        brands:
          $ref: '#/components/schemas/BrandDict'

BrandDict:
   type: object
   additionalProperties:
      $ref: '#/components/schemas/Brand'

Brand:
      type: object
      properties:
        id:
          type: integer
          readOnly: true
        country:
          type: string

Instead Of I get just

Container:
  type: object
      properties:
        other:
          type: string
          readOnly: true

And the issue is in the the resolve_serializer method

discard_component = (
# components with empty schemas serve no purpose
not component.schema
# concrete component without properties are likely only transactional so discard
or (component.schema.get('type') == 'object' and not component.schema.get('properties'))
)
if discard_component:
del self.registry[component]
return ResolvedComponent(None, None) # sentinel
return component

I think Just need add

and not component.schema.get('additionalProperties')

What do you think?

@tfranzel
Copy link
Owner

@pablodgonzalez ahh i'm really sry. this one fell of the waggon and i totally forgot about it.

that code is pretty new and not even release yet. we inverted the logic from "keep this" to "discard this". of course we missed a case. 😄 it should also account for additionalProperties easy fix.

@tfranzel tfranzel added the bug Something isn't working label May 31, 2021
@pablodgonzalez
Copy link
Author

@tfranzel I know it is a new code, in the old logic has not additionalProperties either, but yeah! we going to ahead. I will waiting for this change. Thanks so much!

tfranzel added a commit that referenced this issue May 31, 2021
@tfranzel tfranzel added the fix confirmation pending issue has been fixed and confirmation from issue reporter is pending label May 31, 2021
@tfranzel
Copy link
Owner

@pablodgonzalez that should do it. would be great if you could test that this satisfies your use-case.

@pablodgonzalez
Copy link
Author

@tfranzel Yes!!! I have test it already. My question is about to if it is the only place where the change is needs if it is.
Thanks in advance!

@tfranzel
Copy link
Owner

from my current understanding this should be it. everything else should work just the same. components will not be removed so easily anymore. Extensions need no changes. was that your question?

@pablodgonzalez
Copy link
Author

Yes! All is right then! Thanks for your support!

@tfranzel tfranzel closed this as completed Jun 1, 2021
@tfranzel
Copy link
Owner

fyi: there is now a mechanism to modify ListSerializer behavior by writing a OpenApiSerializerExtension (68e2b1b)

@pablodgonzalez
Copy link
Author

Greate! Thanks @tfranzel for remember this thread. Good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix confirmation pending issue has been fixed and confirmation from issue reporter is pending
Projects
None yet
Development

No branches or pull requests

2 participants