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

Fix read_only + default unique_together validation. #5922

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions rest_framework/serializers.py
Expand Up @@ -441,6 +441,30 @@ def run_validation(self, data=empty):

return value

def _read_only_defaults(self):
fields = [
field for field in self.fields.values()
if (field.read_only) and (field.default != empty) and (field.source != '*') and ('.' not in field.source)
]

defaults = OrderedDict()
for field in fields:
try:
default = field.get_default()
except SkipField:
continue
defaults[field.field_name] = default

Choose a reason for hiding this comment

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

if we are using a field with field_name different from the field in model that is used as source then this is failing and it should be

defaults[field.source] = default

Copy link
Member

Choose a reason for hiding this comment

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

I somehow missed your comment on this, but you're correct. The code above specifically checks for a compatible field source, but uses the serializer field_name instead of the source model field.

Choose a reason for hiding this comment

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

i had reported this and tried to work on it, but couldnt continue
and along with this issue other issue is there defined in the Issue below
#7005
#7003

Copy link
Member

Choose a reason for hiding this comment

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

Hi @anveshagarwal. I'm currently following up on that PR. That's what prompted my comment. 😄

Copy link
Member

Choose a reason for hiding this comment

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

For posterity's sake, my reasoning above was slightly wrong. The source checks are an indicator that we should be operating on field.source instead of field.field_name, however the way to determine this is by looking at run_validation. Before it calls run_validators (then _read_only_defaults), it calls to_internal_value. At this point, the data has been converted to its source structure. So, it makes sense that the read-only defaults would also map to its source structure, instead of the raw field names.


return defaults

def run_validators(self, value):
"""
Add read_only fields with defaults to value before running validators.
"""
to_validate = self._read_only_defaults()
to_validate.update(value)
super(Serializer, self).run_validators(to_validate)

def to_internal_value(self, data):
"""
Dict of native values <- Dict of primitive datatypes.
Expand Down Expand Up @@ -1477,6 +1501,12 @@ def get_unique_together_validators(self):
if (field.source != '*') and ('.' not in field.source)
}

# Special Case: Add read_only fields with defaults.
field_names |= {
field.source for field in self.fields.values()
if (field.read_only) and (field.default != empty) and (field.source != '*') and ('.' not in field.source)
}

# Note that we make sure to check `unique_together` both on the
# base model class, but also on any parent classes.
validators = []
Expand Down
24 changes: 24 additions & 0 deletions tests/test_validators.py
Expand Up @@ -277,6 +277,30 @@ class Meta:
""")
assert repr(serializer) == expected

def test_read_only_fields_with_default(self):
"""
Special case of read_only + default DOES validate unique_together.
"""
class ReadOnlyFieldWithDefaultSerializer(serializers.ModelSerializer):
race_name = serializers.CharField(max_length=100, read_only=True, default='example')

class Meta:
model = UniquenessTogetherModel
fields = ('id', 'race_name', 'position')

data = {'position': 2}
serializer = ReadOnlyFieldWithDefaultSerializer(data=data)

assert len(serializer.validators) == 1
assert isinstance(serializer.validators[0], UniqueTogetherValidator)
assert serializer.validators[0].fields == ('race_name', 'position')
assert not serializer.is_valid()
assert serializer.errors == {
'non_field_errors': [
'The fields race_name, position must make a unique set.'
]
}

def test_allow_explict_override(self):
"""
Ensure validators can be explicitly removed..
Expand Down