Просмотр исходного кода

Remove custom validate_unique() methods

jeremystretch 3 лет назад
Родитель
Сommit
ec6457bcd3

+ 81 - 0
netbox/dcim/migrations/0162_unique_constraints.py

@@ -0,0 +1,81 @@
+# Generated by Django 4.1.1 on 2022-09-14 20:57
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dcim', '0161_cabling_cleanup'),
+    ]
+
+    operations = [
+        migrations.RemoveConstraint(
+            model_name='location',
+            name='dcim_location_name',
+        ),
+        migrations.RemoveConstraint(
+            model_name='location',
+            name='dcim_location_slug',
+        ),
+        migrations.RemoveConstraint(
+            model_name='region',
+            name='dcim_region_name',
+        ),
+        migrations.RemoveConstraint(
+            model_name='region',
+            name='dcim_region_slug',
+        ),
+        migrations.RemoveConstraint(
+            model_name='sitegroup',
+            name='dcim_sitegroup_name',
+        ),
+        migrations.RemoveConstraint(
+            model_name='sitegroup',
+            name='dcim_sitegroup_slug',
+        ),
+        migrations.AlterUniqueTogether(
+            name='device',
+            unique_together=set(),
+        ),
+        migrations.AddConstraint(
+            model_name='device',
+            constraint=models.UniqueConstraint(fields=('name', 'site', 'tenant'), name='dcim_device_unique_name_site_tenant'),
+        ),
+        migrations.AddConstraint(
+            model_name='device',
+            constraint=models.UniqueConstraint(condition=models.Q(('tenant__isnull', True)), fields=('name', 'site'), name='dcim_device_unique_name_site', violation_error_message='Device name must be unique per site.'),
+        ),
+        migrations.AddConstraint(
+            model_name='device',
+            constraint=models.UniqueConstraint(fields=('rack', 'position', 'face'), name='dcim_device_unique_rack_position_face'),
+        ),
+        migrations.AddConstraint(
+            model_name='device',
+            constraint=models.UniqueConstraint(fields=('virtual_chassis', 'vc_position'), name='dcim_device_unique_virtual_chassis_vc_position'),
+        ),
+        migrations.AddConstraint(
+            model_name='location',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True)), fields=('site', 'name'), name='dcim_location_name', violation_error_message='A location with this name already exists within the specified site.'),
+        ),
+        migrations.AddConstraint(
+            model_name='location',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True)), fields=('site', 'slug'), name='dcim_location_slug', violation_error_message='A location with this slug already exists within the specified site.'),
+        ),
+        migrations.AddConstraint(
+            model_name='region',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True)), fields=('name',), name='dcim_region_name', violation_error_message='A top-level region with this name already exists.'),
+        ),
+        migrations.AddConstraint(
+            model_name='region',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True)), fields=('slug',), name='dcim_region_slug', violation_error_message='A top-level region with this slug already exists.'),
+        ),
+        migrations.AddConstraint(
+            model_name='sitegroup',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True)), fields=('name',), name='dcim_sitegroup_name', violation_error_message='A top-level site group with this name already exists.'),
+        ),
+        migrations.AddConstraint(
+            model_name='sitegroup',
+            constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True)), fields=('slug',), name='dcim_sitegroup_slug', violation_error_message='A top-level site group with this slug already exists.'),
+        ),
+    ]

+ 19 - 21
netbox/dcim/models/devices.py

@@ -651,10 +651,25 @@ class Device(NetBoxModel, ConfigContextModel):
 
 
     class Meta:
     class Meta:
         ordering = ('_name', 'pk')  # Name may be null
         ordering = ('_name', 'pk')  # Name may be null
-        unique_together = (
-            ('site', 'tenant', 'name'),  # See validate_unique below
-            ('rack', 'position', 'face'),
-            ('virtual_chassis', 'vc_position'),
+        constraints = (
+            models.UniqueConstraint(
+                name='dcim_device_unique_name_site_tenant',
+                fields=('name', 'site', 'tenant')
+            ),
+            models.UniqueConstraint(
+                name='dcim_device_unique_name_site',
+                fields=('name', 'site'),
+                condition=Q(tenant__isnull=True),
+                violation_error_message="Device name must be unique per site."
+            ),
+            models.UniqueConstraint(
+                name='dcim_device_unique_rack_position_face',
+                fields=('rack', 'position', 'face')
+            ),
+            models.UniqueConstraint(
+                name='dcim_device_unique_virtual_chassis_vc_position',
+                fields=('virtual_chassis', 'vc_position')
+            ),
         )
         )
 
 
     def __str__(self):
     def __str__(self):
@@ -679,23 +694,6 @@ class Device(NetBoxModel, ConfigContextModel):
     def get_absolute_url(self):
     def get_absolute_url(self):
         return reverse('dcim:device', args=[self.pk])
         return reverse('dcim:device', args=[self.pk])
 
 
-    def validate_unique(self, exclude=None):
-
-        # Check for a duplicate name on a device assigned to the same Site and no Tenant. This is necessary
-        # because Django does not consider two NULL fields to be equal, and thus will not trigger a violation
-        # of the uniqueness constraint without manual intervention.
-        if self.name and hasattr(self, 'site') and self.tenant is None:
-            if Device.objects.exclude(pk=self.pk).filter(
-                    name=self.name,
-                    site=self.site,
-                    tenant__isnull=True
-            ):
-                raise ValidationError({
-                    'name': 'A device with this name already exists.'
-                })
-
-        super().validate_unique(exclude)
-
     def clean(self):
     def clean(self):
         super().clean()
         super().clean()
 
 

+ 12 - 48
netbox/dcim/models/sites.py

@@ -67,7 +67,8 @@ class Region(NestedGroupModel):
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('name',),
                 fields=('name',),
                 name='dcim_region_name',
                 name='dcim_region_name',
-                condition=Q(parent=None)
+                condition=Q(parent__isnull=True),
+                violation_error_message="A top-level region with this name already exists."
             ),
             ),
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('parent', 'slug'),
                 fields=('parent', 'slug'),
@@ -76,24 +77,11 @@ class Region(NestedGroupModel):
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('slug',),
                 fields=('slug',),
                 name='dcim_region_slug',
                 name='dcim_region_slug',
-                condition=Q(parent=None)
+                condition=Q(parent__isnull=True),
+                violation_error_message="A top-level region with this slug already exists."
             ),
             ),
         )
         )
 
 
-    def validate_unique(self, exclude=None):
-        if self.parent is None:
-            regions = Region.objects.exclude(pk=self.pk)
-            if regions.filter(name=self.name, parent__isnull=True).exists():
-                raise ValidationError({
-                    'name': 'A region with this name already exists.'
-                })
-            if regions.filter(slug=self.slug, parent__isnull=True).exists():
-                raise ValidationError({
-                    'name': 'A region with this slug already exists.'
-                })
-
-        super().validate_unique(exclude=exclude)
-
     def get_absolute_url(self):
     def get_absolute_url(self):
         return reverse('dcim:region', args=[self.pk])
         return reverse('dcim:region', args=[self.pk])
 
 
@@ -153,7 +141,8 @@ class SiteGroup(NestedGroupModel):
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('name',),
                 fields=('name',),
                 name='dcim_sitegroup_name',
                 name='dcim_sitegroup_name',
-                condition=Q(parent=None)
+                condition=Q(parent__isnull=True),
+                violation_error_message="A top-level site group with this name already exists."
             ),
             ),
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('parent', 'slug'),
                 fields=('parent', 'slug'),
@@ -162,24 +151,11 @@ class SiteGroup(NestedGroupModel):
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('slug',),
                 fields=('slug',),
                 name='dcim_sitegroup_slug',
                 name='dcim_sitegroup_slug',
-                condition=Q(parent=None)
+                condition=Q(parent__isnull=True),
+                violation_error_message="A top-level site group with this slug already exists."
             ),
             ),
         )
         )
 
 
-    def validate_unique(self, exclude=None):
-        if self.parent is None:
-            site_groups = SiteGroup.objects.exclude(pk=self.pk)
-            if site_groups.filter(name=self.name, parent__isnull=True).exists():
-                raise ValidationError({
-                    'name': 'A site group with this name already exists.'
-                })
-            if site_groups.filter(slug=self.slug, parent__isnull=True).exists():
-                raise ValidationError({
-                    'name': 'A site group with this slug already exists.'
-                })
-
-        super().validate_unique(exclude=exclude)
-
     def get_absolute_url(self):
     def get_absolute_url(self):
         return reverse('dcim:sitegroup', args=[self.pk])
         return reverse('dcim:sitegroup', args=[self.pk])
 
 
@@ -384,7 +360,8 @@ class Location(NestedGroupModel):
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('site', 'name'),
                 fields=('site', 'name'),
                 name='dcim_location_name',
                 name='dcim_location_name',
-                condition=Q(parent=None)
+                condition=Q(parent__isnull=True),
+                violation_error_message="A location with this name already exists within the specified site."
             ),
             ),
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('site', 'parent', 'slug'),
                 fields=('site', 'parent', 'slug'),
@@ -393,24 +370,11 @@ class Location(NestedGroupModel):
             models.UniqueConstraint(
             models.UniqueConstraint(
                 fields=('site', 'slug'),
                 fields=('site', 'slug'),
                 name='dcim_location_slug',
                 name='dcim_location_slug',
-                condition=Q(parent=None)
+                condition=Q(parent__isnull=True),
+                violation_error_message="A location with this slug already exists within the specified site."
             ),
             ),
         )
         )
 
 
-    def validate_unique(self, exclude=None):
-        if self.parent is None:
-            locations = Location.objects.exclude(pk=self.pk)
-            if locations.filter(name=self.name, site=self.site, parent__isnull=True).exists():
-                raise ValidationError({
-                    "name": f"A location with this name in site {self.site} already exists."
-                })
-            if locations.filter(slug=self.slug, site=self.site, parent__isnull=True).exists():
-                raise ValidationError({
-                    "name": f"A location with this slug in site {self.site} already exists."
-                })
-
-        super().validate_unique(exclude=exclude)
-
     @classmethod
     @classmethod
     def get_prerequisite_models(cls):
     def get_prerequisite_models(cls):
         return [Site, ]
         return [Site, ]

+ 3 - 3
netbox/dcim/tests/test_models.py

@@ -384,7 +384,7 @@ class DeviceTestCase(TestCase):
             site=self.site,
             site=self.site,
             device_type=self.device_type,
             device_type=self.device_type,
             device_role=self.device_role,
             device_role=self.device_role,
-            name=''
+            name=None
         )
         )
         device1.save()
         device1.save()
 
 
@@ -392,12 +392,12 @@ class DeviceTestCase(TestCase):
             site=device1.site,
             site=device1.site,
             device_type=device1.device_type,
             device_type=device1.device_type,
             device_role=device1.device_role,
             device_role=device1.device_role,
-            name=''
+            name=None
         )
         )
         device2.full_clean()
         device2.full_clean()
         device2.save()
         device2.save()
 
 
-        self.assertEqual(Device.objects.filter(name='').count(), 2)
+        self.assertEqual(Device.objects.filter(name__isnull=True).count(), 2)
 
 
     def test_device_duplicate_names(self):
     def test_device_duplicate_names(self):
 
 

+ 25 - 0
netbox/virtualization/migrations/0033_unique_constraints.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.1.1 on 2022-09-14 20:57
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('virtualization', '0032_virtualmachine_update_sites'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='virtualmachine',
+            unique_together=set(),
+        ),
+        migrations.AddConstraint(
+            model_name='virtualmachine',
+            constraint=models.UniqueConstraint(fields=('name', 'cluster', 'tenant'), name='virtualization_virtualmachine_unique_name_cluster_tenant'),
+        ),
+        migrations.AddConstraint(
+            model_name='virtualmachine',
+            constraint=models.UniqueConstraint(condition=models.Q(('tenant__isnull', True)), fields=('name', 'cluster'), name='virtualization_virtualmachine_unique_name_cluster', violation_error_message='Virtual machine name must be unique per site.'),
+        ),
+    ]

+ 13 - 17
netbox/virtualization/models.py

@@ -2,6 +2,7 @@ from django.contrib.contenttypes.fields import GenericRelation
 from django.core.exceptions import ValidationError
 from django.core.exceptions import ValidationError
 from django.core.validators import MinValueValidator
 from django.core.validators import MinValueValidator
 from django.db import models
 from django.db import models
+from django.db.models import Q
 from django.urls import reverse
 from django.urls import reverse
 
 
 from dcim.models import BaseInterface, Device
 from dcim.models import BaseInterface, Device
@@ -309,9 +310,18 @@ class VirtualMachine(NetBoxModel, ConfigContextModel):
 
 
     class Meta:
     class Meta:
         ordering = ('_name', 'pk')  # Name may be non-unique
         ordering = ('_name', 'pk')  # Name may be non-unique
-        unique_together = [
-            ['cluster', 'tenant', 'name']
-        ]
+        constraints = (
+            models.UniqueConstraint(
+                name='virtualization_virtualmachine_unique_name_cluster_tenant',
+                fields=('name', 'cluster', 'tenant')
+            ),
+            models.UniqueConstraint(
+                name='virtualization_virtualmachine_unique_name_cluster',
+                fields=('name', 'cluster'),
+                condition=Q(tenant__isnull=True),
+                violation_error_message="Virtual machine name must be unique per site."
+            ),
+        )
 
 
     def __str__(self):
     def __str__(self):
         return self.name
         return self.name
@@ -323,20 +333,6 @@ class VirtualMachine(NetBoxModel, ConfigContextModel):
     def get_absolute_url(self):
     def get_absolute_url(self):
         return reverse('virtualization:virtualmachine', args=[self.pk])
         return reverse('virtualization:virtualmachine', args=[self.pk])
 
 
-    def validate_unique(self, exclude=None):
-
-        # Check for a duplicate name on a VM assigned to the same Cluster and no Tenant. This is necessary
-        # because Django does not consider two NULL fields to be equal, and thus will not trigger a violation
-        # of the uniqueness constraint without manual intervention.
-        if self.tenant is None and VirtualMachine.objects.exclude(pk=self.pk).filter(
-                name=self.name, cluster=self.cluster, tenant__isnull=True
-        ):
-            raise ValidationError({
-                'name': 'A virtual machine with this name already exists in the assigned cluster.'
-            })
-
-        super().validate_unique(exclude)
-
     def clean(self):
     def clean(self):
         super().clean()
         super().clean()