Переглянути джерело

Fixes #22029: Recast empty string values on unique nullable fields as null (#22035)

Jeremy Stretch 2 місяців тому
батько
коміт
baa2ff3ade
2 змінених файлів з 56 додано та 0 видалено
  1. 36 0
      netbox/dcim/tests/test_models.py
  2. 20 0
      netbox/netbox/models/__init__.py

+ 36 - 0
netbox/dcim/tests/test_models.py

@@ -638,6 +638,42 @@ class DeviceTestCase(TestCase):
         device2.full_clean()
         device2.save()
 
+    def test_empty_asset_tag_coerced_to_null_on_clean(self):
+        """
+        An empty string assigned to a unique nullable CharField (e.g. asset_tag) must be coerced
+        to None on save so that multiple objects can be saved without violating the unique
+        constraint. Test that this is done on clean().
+        """
+        common_kwargs = {
+            'site': Site.objects.first(),
+            'device_type': DeviceType.objects.first(),
+            'role': DeviceRole.objects.first(),
+        }
+        device1 = Device(name='Device 1', asset_tag='', **common_kwargs)
+        device1.clean()
+        self.assertIsNone(device1.asset_tag)
+
+    def test_empty_asset_tag_coerced_to_null_on_save(self):
+        """
+        An empty string assigned to a unique nullable CharField (e.g. asset_tag) must be coerced
+        to None on save so that multiple objects can be saved without violating the unique
+        constraint. Test that this is done on save().
+        """
+        common_kwargs = {
+            'site': Site.objects.first(),
+            'device_type': DeviceType.objects.first(),
+            'role': DeviceRole.objects.first(),
+        }
+        device1 = Device(name='Device 1', asset_tag='', **common_kwargs)
+        device1.save()
+        device2 = Device(name='Device 2', asset_tag='', **common_kwargs)
+        device2.save()
+
+        device1.refresh_from_db()
+        device2.refresh_from_db()
+        self.assertIsNone(device1.asset_tag)
+        self.assertIsNone(device2.asset_tag)
+
     def test_device_label(self):
         device1 = Device(
             site=Site.objects.first(),

+ 20 - 0
netbox/netbox/models/__init__.py

@@ -58,6 +58,7 @@ class BaseModel(models.Model):
     This class provides some important overrides to Django's default functionality, such as
     - Overriding the default manager to use RestrictedQuerySet
     - Extending `clean()` to validate GenericForeignKey fields
+    - Extending `clean()` and `save()` to coerce empty strings to None on unique nullable CharFields
     """
 
     objects = RestrictedQuerySet.as_manager()
@@ -65,11 +66,26 @@ class BaseModel(models.Model):
     class Meta:
         abstract = True
 
+    def _coerce_nullable_unique_chars(self):
+        """
+        Coerce empty strings to None on unique nullable CharFields to avoid spurious
+        uniqueness violations (PostgreSQL treats two empty strings as duplicates).
+        """
+        for field in self._meta.concrete_fields:
+            if (
+                isinstance(field, models.CharField)
+                and field.null
+                and field.unique
+                and getattr(self, field.attname, None) == ''
+            ):
+                setattr(self, field.attname, None)
+
     def clean(self):
         """
         Validate the model for GenericForeignKey fields to ensure that the content type and object ID exist.
         """
         super().clean()
+        self._coerce_nullable_unique_chars()
 
         for field in self._meta.get_fields():
             if isinstance(field, GenericForeignKey):
@@ -97,6 +113,10 @@ class BaseModel(models.Model):
                     # update the GFK field value
                     setattr(self, field.name, obj)
 
+    def save(self, *args, **kwargs):
+        self._coerce_nullable_unique_chars()
+        super().save(*args, **kwargs)
+
 
 class ChangeLoggedModel(ChangeLoggingMixin, CustomValidationMixin, EventRulesMixin, BaseModel):
     """