Skip to content

Commit 34dc358

Browse files
google-labs-jules[bot]ddalex
authored andcommitted
Fix: SerializerMethodField as HiddenField in unique_together
Corrects an issue where a `SerializerMethodField` that shares its name with a model field involved in a `unique_together` constraint (or `UniqueConstraint`) could be incorrectly treated as a `HiddenField`. This typically occurred if the underlying model field was nullable or had a default value, and the `SerializerMethodField` was not being correctly prioritized during the uniqueness metadata processing. The `ModelSerializer.get_uniqueness_extra_kwargs` method has been updated to ensure that if a field name corresponding to a model field in a uniqueness constraint is explicitly declared as a `SerializerMethodField` on the serializer, it is not converted into a `HiddenField`. A test case has been added to `tests/test_model_serializer.py` (`TestSerializerMethodFieldInUniqueTogether`) to verify this behavior. NOTE: Due to persistent timeouts in the testing environment, I could not complete full test suite verification. Multiple attempts to run tests (full suite, specific module, specific class) timed out. The implemented fix is based on code analysis and the added specific test case logic. Relying on CI/CD pipeline for full test validation.
1 parent 33d59fe commit 34dc358

File tree

2 files changed

+118
-2
lines changed

2 files changed

+118
-2
lines changed

rest_framework/serializers.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,12 @@ def get_uniqueness_extra_kwargs(self, field_names, declared_fields, extra_kwargs
14831483
hidden_fields = {}
14841484
uniqueness_extra_kwargs = {}
14851485

1486+
# Identify declared SerializerMethodFields to prevent them from becoming HiddenFields
1487+
serializer_method_field_names = {
1488+
name for name, field_instance in declared_fields.items()
1489+
if isinstance(field_instance, SerializerMethodField)
1490+
}
1491+
14861492
for unique_constraint_name in unique_constraint_names:
14871493
# Get the model field that is referred too.
14881494
unique_constraint_field = model._meta.get_field(unique_constraint_name)
@@ -1507,8 +1513,10 @@ def get_uniqueness_extra_kwargs(self, field_names, declared_fields, extra_kwargs
15071513
elif default is not empty:
15081514
# The corresponding field is not present in the
15091515
# serializer. We have a default to use for it, so
1510-
# add in a hidden field that populates it.
1511-
hidden_fields[unique_constraint_name] = HiddenField(default=default)
1516+
# add in a hidden field that populates it,
1517+
# unless it's a SerializerMethodField.
1518+
if unique_constraint_name not in serializer_method_field_names:
1519+
hidden_fields[unique_constraint_name] = HiddenField(default=default)
15121520

15131521
# Update `extra_kwargs` with any new options.
15141522
for key, value in uniqueness_extra_kwargs.items():

tests/test_model_serializer.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,32 @@ class Meta:
13461346
fields = ('name',)
13471347

13481348

1349+
# --- Test models and serializers for SerializerMethodField in unique_together ---
1350+
class UniqueTestModel(models.Model):
1351+
name = models.CharField(max_length=100)
1352+
description = models.CharField(max_length=100, null=True, blank=True)
1353+
other_field = models.CharField(max_length=100, default="default_value")
1354+
1355+
class Meta:
1356+
unique_together = [("name", "description")]
1357+
1358+
def __str__(self):
1359+
return f"{self.name} - {self.description or 'No description'}"
1360+
1361+
1362+
class UniqueTestModelSerializer(serializers.ModelSerializer):
1363+
description = serializers.SerializerMethodField()
1364+
1365+
class Meta:
1366+
model = UniqueTestModel
1367+
fields = ["name", "description", "other_field"]
1368+
1369+
def get_description(self, obj):
1370+
if obj.description:
1371+
return f"Serialized: {obj.description}"
1372+
return "Serialized: No description provided"
1373+
1374+
13491375
class Issue6110Test(TestCase):
13501376
def test_model_serializer_custom_manager(self):
13511377
instance = Issue6110ModelSerializer().create({'name': 'test_name'})
@@ -1395,3 +1421,85 @@ class Meta:
13951421
serializer.save()
13961422

13971423
self.assertEqual(instance.char_field, 'value changed by signal')
1424+
1425+
1426+
# --- Test case for SerializerMethodField in unique_together ---
1427+
class TestSerializerMethodFieldInUniqueTogether(TestCase):
1428+
def test_serializer_method_field_not_hidden_in_unique_together(self):
1429+
# 1. Instantiate the serializer
1430+
serializer = UniqueTestModelSerializer()
1431+
1432+
# 2. Assert that the 'description' field is not a HiddenField
1433+
self.assertFalse(
1434+
isinstance(serializer.fields['description'], serializers.HiddenField),
1435+
"Field 'description' should not be a HiddenField."
1436+
)
1437+
1438+
# 3. Assert that 'description' is a SerializerMethodField
1439+
self.assertTrue(
1440+
isinstance(serializer.fields['description'], serializers.SerializerMethodField),
1441+
"Field 'description' should be a SerializerMethodField."
1442+
)
1443+
1444+
# 4. Assert that the 'description' field is present in the serializer's output
1445+
instance = UniqueTestModel.objects.create(name="TestName", description="TestDesc")
1446+
serializer_output = UniqueTestModelSerializer(instance).data
1447+
self.assertIn("description", serializer_output)
1448+
self.assertEqual(serializer_output["description"], "Serialized: TestDesc")
1449+
1450+
instance_no_desc = UniqueTestModel.objects.create(name="TestNameNoDesc")
1451+
serializer_output_no_desc = UniqueTestModelSerializer(instance_no_desc).data
1452+
self.assertEqual(serializer_output_no_desc["description"], "Serialized: No description provided")
1453+
1454+
# 5. Perform a validation test
1455+
# Create an initial instance
1456+
UniqueTestModel.objects.create(name="UniqueName", description="UniqueDesc")
1457+
1458+
# Attempt to validate data that would violate unique_together
1459+
invalid_data = {"name": "UniqueName", "description": "UniqueDesc", "other_field": "some_value"}
1460+
serializer_invalid = UniqueTestModelSerializer(data=invalid_data)
1461+
with self.assertRaises(serializers.ValidationError) as context:
1462+
serializer_invalid.is_valid(raise_exception=True)
1463+
self.assertIn("non_field_errors", context.exception.detail) # Check for unique_together error
1464+
self.assertTrue(any("unique test model with this name and description already exists" in str(err)
1465+
for err_list in context.exception.detail.values() for err in err_list))
1466+
1467+
1468+
# Attempt to validate data that would violate unique_together (with null description)
1469+
UniqueTestModel.objects.create(name="UniqueNameNull", description=None)
1470+
invalid_data_null = {"name": "UniqueNameNull", "description": None, "other_field": "some_value"}
1471+
serializer_invalid_null = UniqueTestModelSerializer(data=invalid_data_null)
1472+
1473+
with self.assertRaises(serializers.ValidationError) as context_null:
1474+
serializer_invalid_null.is_valid(raise_exception=True)
1475+
1476+
self.assertIn("non_field_errors", context_null.exception.detail)
1477+
self.assertTrue(any("unique test model with this name and description already exists" in str(err)
1478+
for err_list in context_null.exception.detail.values() for err in err_list))
1479+
1480+
1481+
# Attempt to validate valid data
1482+
valid_data = {"name": "NewName", "description": "NewDesc", "other_field": "another_value"}
1483+
serializer_valid = UniqueTestModelSerializer(data=valid_data)
1484+
self.assertTrue(serializer_valid.is_valid(raise_exception=True))
1485+
self.assertEqual(serializer_valid.validated_data['name'], "NewName")
1486+
# Note: 'description' from SerializerMethodField won't be in validated_data for writes by default.
1487+
# The key here is that the serializer construction and unique_together validation works.
1488+
# The model's description field will be None or the provided value during create/update.
1489+
1490+
# Validate data where description is not provided (should use model field's null=True)
1491+
valid_data_no_desc = {"name": "NameOnly"} # description and other_field will use defaults or be None
1492+
serializer_valid_no_desc = UniqueTestModelSerializer(data=valid_data_no_desc)
1493+
self.assertTrue(serializer_valid_no_desc.is_valid(raise_exception=True))
1494+
self.assertIsNone(serializer_valid_no_desc.validated_data.get('description'))
1495+
self.assertEqual(serializer_valid_no_desc.validated_data['other_field'], "default_value") # check other_field default
1496+
1497+
# Check saving the instance
1498+
saved_instance = serializer_valid_no_desc.save()
1499+
self.assertEqual(saved_instance.name, "NameOnly")
1500+
self.assertIsNone(saved_instance.description)
1501+
self.assertEqual(saved_instance.other_field, "default_value")
1502+
1503+
# Check that SerializerMethodField value is used in representation after save
1504+
output_after_save = UniqueTestModelSerializer(saved_instance).data
1505+
self.assertEqual(output_after_save['description'], "Serialized: No description provided")

0 commit comments

Comments
 (0)