ListField default values not honored in multipart/form-data mode

[awbacker]

When sending a multipart/form-data request to a serializer with a ListField, the default value returned for the field is always the empty list []. Any default= passed to field will be ignored due to the way html input is handled in the get_value() function.

• This functionality works properly when posting json data
• May be related to ListField default values not honored in multipart/form-data mode #5807 but I don't think so.

Steps to reproduce

class SimpleSerializer(Serializer):
    a = IntegerField(required=True)
    c = ListField(default=lambda: [1, 2, 3])

# simulate a multipart/form-data post (querydict is a confusing name here)
s1 = SimpleSerializer(data=QueryDict("a=3"))
s1.is_valid(raise_exception=True)
print("html:", s1.validated_data)

# simulate a JSON post
s2 = SimpleSerializer(data={'a': 3})
s2.is_valid(raise_exception=True)
print("json:", s2.validated_data)

>>> html: OrderedDict([(u'a', 3), (u'c', [])])
>>> json: OrderedDict([(u'a', 3), (u'c', [1, 2, 3])])

Expected behavior

• Default value should be honored in HTML mode

Actual behavior

• Default value is always returned as []

Explanation and Workaround

The ListField.get_value() function always returns the value of html.parse_html_list from get_value when that field_name was not passed. There is a check at the top for a missing key, but that only affects partial posts. Missing keys on regular POSTs will still be processed normally and return [].

I'm not sure if it is OK to just remove that check for partial and always return empty, but that is what I have done for this version of my workaround. The list fields I use this way are in pure serializers and are only used for validating input. They are not used in ModelSerializers so for my case this is ok.

Initial Workaround

class ListFieldWithSaneDefault(ListField):
    """
    This is used ONLY as a base class for other fields.  When using it, please ensure that you
    always provide a default value (at least `default=lambda: []`) if the field is not required.
    Your derived class should take no parameters to __init__, it should be self contained
    """
    def get_value(self, dictionary):
        """
        When handling html multipart forms input (as opposed to json, which works properly)
        the base list field returns `[]` for _missing_ keys.  This override checks for that specific
        case and returns `empty` so that standard default-value processing takes over
        """
        if self.field_name not in dictionary:
            return empty
        return super(ListFieldWithSaneDefault, self).get_value(dictionary)

[tomchristie]

Okay. One sensible thing to do would be to try that in the repo, and see which (if any) tests fail,
and then take things from there.

[rpkilby]

May be related to #5807 but I don't think so.

That reference is to this PR. What PR did you mean to link to?

In general, ListField and DictField are not compatible with form media types. Did you run into this issue when using the browsable API, or is this endpoint intended to use the multipart/form-data media type? If it's the former, it's worth noting that #5702 has disabled rendering of ListField and DictField inputs in the browsable API.

[awbacker]

@rpkilby Its only related to the multipart form posts to api endpoints. I don't use the browsable api for updating (disabled that part, for the queries). I'm making the changes now, and will try to work in a test case and show a PR. So far nothing fails in testing, but I haven't added one for this specific issue for others to evaluate yet.

If anyone has feedback on the behavior (optional list is always [] in html forms mode) or how to handle it, I'd love to hear it. I'm sure I'm going to miss some finer points.

[anx-ckreuzberger]

It seems that I came across a similar issue related to multi-part forms.

I have the following serializer:

class KanbanBoardSerializer(serializers.ModelSerializer):
    kanban_board_columns = KanbanBoardColumnSerializer(
        read_only=False,
        many=True,
        required=False
    )

    class Meta:
        model = KanbanBoard
        fields = (
            'pk', 'title', 'kanban_board_columns', 'some_file'
        )

I then use a multi-part form (to upload to the file-field some_file).

Obviously, within the serializer I need to handle the kanban_board_columns field:

    def update(self, instance, validated_data):
        """
        Override update method of KanbanBoardSerializer such that we can handle sub-serializers for
        - kanban_board_columns
        :param instance: the instance that is being updated
        :param validated_data: validated data of the serializer
        :return: KanbanBoard
        """
        kanban_board_columns = None

        # get kanban board columns from validated data (if it is available)
        if 'kanban_board_columns' in validated_data:
            kanban_board_columns = validated_data.pop('kanban_board_columns')
            # handle inserts/updates/removes on kanban board columns

        # update kanban board instance
        instance = super(KanbanBoardSerializer, self).update(instance, validated_data)

        return instance

The result is that kanban_board_columns = validated_data.pop('kanban_board_columns') always returns an empty list [], even though it is definatley not set in the multi-part form data.

However, when I use JSON, the field is not being populated in validated_data.

[achichenin]

Hi, this is actually related to #2761. See, the reason for #2761 - get_value alway returned [] which broke all unpatched ListFields in the serializer.

The solution (a bit bizarre imo) was to add the following check:

        if self.field_name not in dictionary:
            if getattr(self.root, 'partial', False):
                return empty

While it fixed the issue it obviously made patching of ListFields impossible for "multipart/formdata", and introduced different behavior for POST and PATCH methods.

I think that now when html.parse_html_list properly returns empty this "partial" check should be removed like @awbacker suggested initially.

[MatejMijoski]

Is it safe now to remove the partial check from the get_value function and enable support for multipart/form-data in PATCH requests?