Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -1749,15 +1749,15 @@ def get_value(self, dictionary):
# We override the default field access in order to support
# dictionaries in HTML forms.
if html.is_html_input(dictionary):
return html.parse_html_dict(dictionary, prefix=self.field_name)
return html.parse_html_dict(dictionary, prefix=self.field_name, default=empty)
return dictionary.get(self.field_name, empty)

def to_internal_value(self, data):
"""
Dicts of native values <- Dicts of primitive datatypes.
"""
if html.is_html_input(data):
data = html.parse_html_dict(data)
data = html.parse_html_dict(data, default=data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't a better default be an empty dict here?

Suggested change
data = html.parse_html_dict(data, default=data)
data = html.parse_html_dict(data, default={})

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this but it actually breaks test_querydict_dict_input. When to_internal_value gets called, the data is already a MultiValueDict with parsed keys. parse_html_dict with no prefix looks for dot-prefixed keys which don't match, so default={} would lose the already-parsed data. Keeping default=data as a fallback preserves it correctly. Happy to discuss further though!

if not isinstance(data, dict):
self.fail('not_a_dict', input_type=type(data).__name__)
if not self.allow_empty and len(data) == 0:
Expand Down
2 changes: 1 addition & 1 deletion rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ def get_value(self, dictionary):
# We override the default field access in order to support
# nested HTML forms.
if html.is_html_input(dictionary):
return html.parse_html_dict(dictionary, prefix=self.field_name) or empty
return html.parse_html_dict(dictionary, prefix=self.field_name, default=empty)
return dictionary.get(self.field_name, empty)

def run_validation(self, data=empty):
Expand Down
7 changes: 5 additions & 2 deletions rest_framework/utils/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def parse_html_list(dictionary, prefix='', default=None):
return [ret[item] for item in sorted(ret)] if ret else default


def parse_html_dict(dictionary, prefix=''):
def parse_html_dict(dictionary, prefix='', default=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks consistent with what we do for parsing html list 👍🏻 :

#5927

"""
Used to support dictionary values in HTML forms.

Expand All @@ -81,6 +81,9 @@ def parse_html_dict(dictionary, prefix=''):
'email': 'example@example.com'
}
}

:returns a MultiValueDict of the parsed data, or the value specified in
``default`` if the dict field was not present in the input
"""
ret = MultiValueDict()
regex = re.compile(r'^%s\.(.+)$' % re.escape(prefix))
Expand All @@ -92,4 +95,4 @@ def parse_html_dict(dictionary, prefix=''):
value = dictionary.getlist(field)
ret.setlist(key, value)

return ret
return ret if ret else default
50 changes: 50 additions & 0 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -2496,6 +2496,56 @@ def test_allow_empty_disallowed(self):

assert exc_info.value.detail == ['This dictionary may not be empty.']

def test_querydict_dict_input(self):
"""
DictField should correctly parse HTML form (QueryDict) input
with dot-separated keys.
"""
class TestSerializer(serializers.Serializer):
data = serializers.DictField(child=serializers.CharField())

serializer = TestSerializer(data=QueryDict('data.a=1&data.b=2'))
assert serializer.is_valid()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small detail, but can we please tweak these asserrt to show the serializer errors when they fail? That will give much better test output:

Suggested change
assert serializer.is_valid()
assert serializer.is_valid(), serializer.errors

Some for the other assert serializer.is_valid() in the other tests

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, updated all three to assert serializer.is_valid(), serializer.errors.

assert serializer.validated_data == {'data': {'a': '1', 'b': '2'}}

def test_querydict_dict_input_no_values_uses_default(self):
"""
When no matching keys are present in the QueryDict and a default
is set, the field should return the default value.
"""
class TestSerializer(serializers.Serializer):
a = serializers.IntegerField(required=True)
data = serializers.DictField(default=lambda: {'x': 'y'})

serializer = TestSerializer(data=QueryDict('a=1'))
assert serializer.is_valid()
assert serializer.validated_data == {'a': 1, 'data': {'x': 'y'}}

def test_querydict_dict_input_no_values_no_default_and_not_required(self):
"""
When no matching keys are present in the QueryDict, there is no
default, and the field is not required, the field should be
skipped entirely from validated_data.
"""
class TestSerializer(serializers.Serializer):
data = serializers.DictField(required=False)

serializer = TestSerializer(data=QueryDict(''))
assert serializer.is_valid()
assert serializer.validated_data == {}

def test_querydict_dict_input_no_values_required(self):
"""
When no matching keys are present in the QueryDict and the field
is required, validation should fail.
"""
class TestSerializer(serializers.Serializer):
data = serializers.DictField(required=True)

serializer = TestSerializer(data=QueryDict(''))
assert not serializer.is_valid()
assert 'data' in serializer.errors


class TestNestedDictField(FieldValues):
"""
Expand Down