diff --git a/netbox/dcim/migrations/0162_unique_constraints.py b/netbox/dcim/migrations/0162_unique_constraints.py new file mode 100644 index 000000000..08c113f50 --- /dev/null +++ b/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.'), + ), + ] diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py index 7858960a1..eb21e532b 100644 --- a/netbox/dcim/models/devices.py +++ b/netbox/dcim/models/devices.py @@ -651,10 +651,25 @@ class Device(NetBoxModel, ConfigContextModel): class Meta: 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): @@ -679,23 +694,6 @@ class Device(NetBoxModel, ConfigContextModel): def get_absolute_url(self): 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): super().clean() diff --git a/netbox/dcim/models/sites.py b/netbox/dcim/models/sites.py index f5c8e6d9d..90f855741 100644 --- a/netbox/dcim/models/sites.py +++ b/netbox/dcim/models/sites.py @@ -67,7 +67,8 @@ class Region(NestedGroupModel): models.UniqueConstraint( fields=('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( fields=('parent', 'slug'), @@ -76,24 +77,11 @@ class Region(NestedGroupModel): models.UniqueConstraint( fields=('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): return reverse('dcim:region', args=[self.pk]) @@ -153,7 +141,8 @@ class SiteGroup(NestedGroupModel): models.UniqueConstraint( fields=('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( fields=('parent', 'slug'), @@ -162,24 +151,11 @@ class SiteGroup(NestedGroupModel): models.UniqueConstraint( fields=('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): return reverse('dcim:sitegroup', args=[self.pk]) @@ -384,7 +360,8 @@ class Location(NestedGroupModel): models.UniqueConstraint( fields=('site', '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( fields=('site', 'parent', 'slug'), @@ -393,24 +370,11 @@ class Location(NestedGroupModel): models.UniqueConstraint( fields=('site', '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 def get_prerequisite_models(cls): return [Site, ] diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index 0e02b0de5..acde02ecd 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -384,7 +384,7 @@ class DeviceTestCase(TestCase): site=self.site, device_type=self.device_type, device_role=self.device_role, - name='' + name=None ) device1.save() @@ -392,12 +392,12 @@ class DeviceTestCase(TestCase): site=device1.site, device_type=device1.device_type, device_role=device1.device_role, - name='' + name=None ) device2.full_clean() 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): diff --git a/netbox/virtualization/migrations/0033_unique_constraints.py b/netbox/virtualization/migrations/0033_unique_constraints.py new file mode 100644 index 000000000..fe02881b0 --- /dev/null +++ b/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.'), + ), + ] diff --git a/netbox/virtualization/models.py b/netbox/virtualization/models.py index 4acbe6daf..1b0a6ba06 100644 --- a/netbox/virtualization/models.py +++ b/netbox/virtualization/models.py @@ -2,6 +2,7 @@ from django.contrib.contenttypes.fields import GenericRelation from django.core.exceptions import ValidationError from django.core.validators import MinValueValidator from django.db import models +from django.db.models import Q from django.urls import reverse from dcim.models import BaseInterface, Device @@ -309,9 +310,18 @@ class VirtualMachine(NetBoxModel, ConfigContextModel): class Meta: 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): return self.name @@ -323,20 +333,6 @@ class VirtualMachine(NetBoxModel, ConfigContextModel): def get_absolute_url(self): 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): super().clean()