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

Dynamic ChoiceField choices collision #1114

Closed
Lucianod28 opened this issue Nov 26, 2023 · 6 comments
Closed

Dynamic ChoiceField choices collision #1114

Lucianod28 opened this issue Nov 26, 2023 · 6 comments

Comments

@Lucianod28
Copy link

Describe the bug
I have these serializers:

class FloatWithUnitSerializer(serializers.Serializer):
    value = serializers.FloatField()

    def __init__(self, unit_type, *args, **kwargs):
        if unit_type == "length":
            self.fields["unit"] = serializers.ChoiceField(choices=LengthUnits.choices)
        else:
            self.fields["unit"] = serializers.ChoiceField(choices=WeightUnits.choices)
        super().__init__(*args, **kwargs)
    # Custom to_representation and to_internal_value not shown

class CargoSerializer(UnitMixin, serializers.ModelSerializer):
    length = FloatWithUnitSerializer(unit_type="length", required=False, source="*")
    height = FloatWithUnitSerializer(unit_type="length", required=False, source="*")
    weight = FloatWithUnitSerializer(unit_type="weight", required=False, source="*")

    class Meta:
        model = models.Cargo
        fields = ["id", "length", "height", "weight"]

And these choices:

class LengthUnits(models.IntegerChoices):
    FEET = 0
    YARDS = 1
    METERS = 2

class WeightUnits(models.IntegerChoices):
    US_TONS = 0
    POUNDS = 1
    KILOGRAMS = 2
    METRIC_TONS = 3

LengthUnits and WeightUnits have different values. However, when I generate my schema, length, height, and weight all share the same type, with WeightUnits used as the choices. So the schema incorrectly allows "3" as input for length and height units. I expected length and height to have a type with LengthUnits as unit choices and weight to separately have WeightUnits as unit choices.

@tfranzel
Copy link
Owner

Yes this a known collision bug that was introduced when we added the enum description label feature.
duplicate of #790 and #1104

you try and review #1113 if you like

@Lucianod28
Copy link
Author

Got it, I saw those but thought they were different since my enums have different shapes. I'll take a look at 1113.

@tfranzel
Copy link
Owner

well that might be another related issue. due to the way DRF handles this, the enum names is lost. We can only generate a from out of the field name, and if that looks like a supoptimal solution, we emit a warning.

you might also need to use the setting ENUM_NAME_OVERRIDES for disambiguation of the names.

@Lucianod28
Copy link
Author

I tried with #1113 and the issue remains

@tfranzel
Copy link
Owner

tfranzel commented Nov 27, 2023

at closer inspection I now understand what you are doing. No, that is not allowed like that

You are dynamically changing the serializer class on demand. Spectacular parses each serializer (class/class name tuple) once and reuses the the generated schema, when encountered again. So you only see the result of the first occurrence.

There are 2 solutions.

  1. make the unit field explicit and polymorhic via decoration. (prob not what you want since its always generic and every usage would not contain the concrete info on whether it is length or weight)

  2. slightly adapt your logic to force spectacular to create multiple serializer components:

from drf_spectacular.drainage import set_override

        def __init__(self, unit_type, *args, **kwargs):
            if unit_type == "length":
                set_override(self, 'component_name', "FloatWithLength")
                self.fields["unit"] = serializers.ChoiceField(choices=LengthUnits.choices)
            else:
                set_override(self, 'component_name', "FloatWithWeight")
                self.fields["unit"] = serializers.ChoiceField(choices=WeightUnits.choices)
            super().__init__(*args, **kwargs)
        # Custom to_representation and to_internal_value not shown

this is basically what @extend_schema_serializer(component_name=...) does, just in a dynamic fashion.

But I would also recommend to use ENUM_NAME_OVERRIDES with

"ENUM_NAME_OVERRIDES": {
   "WeightEnum": "import.path.to.WeightUnits", 
   ...
}

to have better names for you enums since, as mentioned, DRF/Django throws away the Text/IntegerChoices class names

@Lucianod28
Copy link
Author

Thank you, that makes sense. Option 2 works!

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

2 participants