From f25dd7aab702b71467665f1b1ddb4ece81e52220 Mon Sep 17 00:00:00 2001 From: Jad Seifeddine Date: Wed, 30 Jul 2025 13:56:32 +1000 Subject: [PATCH 1/3] Fix MAC address pagination duplicates by adding 'pk' to model ordering Add 'pk' to MACAddress model ordering to ensure deterministic results when multiple MAC addresses have the same value. This prevents the same MAC address from appearing on multiple pages during pagination. The issue occurred because Django's default ordering by 'mac_address' alone is non-deterministic when multiple records share the same MAC address value, causing inconsistent pagination results when the same MAC address is assigned to multiple interfaces on a device. Added regression test that verifies MAC addresses with identical values are properly ordered by their primary key, ensuring consistent pagination behavior across the application. Fixes netbox-community#19917 --- .../0209_alter_macaddress_options.py | 19 ++++++++++ netbox/dcim/models/devices.py | 2 +- netbox/dcim/tests/test_models.py | 36 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 netbox/dcim/migrations/0209_alter_macaddress_options.py diff --git a/netbox/dcim/migrations/0209_alter_macaddress_options.py b/netbox/dcim/migrations/0209_alter_macaddress_options.py new file mode 100644 index 00000000000..2f3b6de06e3 --- /dev/null +++ b/netbox/dcim/migrations/0209_alter_macaddress_options.py @@ -0,0 +1,19 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('dcim', '0208_devicerole_uniqueness'), + ] + + operations = [ + migrations.AlterModelOptions( + name='macaddress', + options={ + 'ordering': ('mac_address', 'pk'), + 'verbose_name': 'MAC address', + 'verbose_name_plural': 'MAC addresses' + }, + ), + ] diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py index abd29d4e3a9..e1ad9c15cb9 100644 --- a/netbox/dcim/models/devices.py +++ b/netbox/dcim/models/devices.py @@ -1276,7 +1276,7 @@ class MACAddress(PrimaryModel): ) class Meta: - ordering = ('mac_address',) + ordering = ('mac_address', 'pk',) verbose_name = _('MAC address') verbose_name_plural = _('MAC addresses') diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index be9f067d432..1037957660a 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -37,6 +37,19 @@ def setUpTestData(cls): cls.interface.primary_mac_address = cls.mac_a cls.interface.save() + shared_mac = '1234567890ab' + interfaces = [] + for i in range(10): + interfaces.append(Interface( + device=device, name=f'Interface {i:03d}', type='1000base-t' + )) + Interface.objects.bulk_create(interfaces) + + mac_addresses = [] + for interface in interfaces: + mac_addresses.append(MACAddress(mac_address=shared_mac, assigned_object=interface)) + MACAddress.objects.bulk_create(mac_addresses) + @tag('regression') def test_clean_will_not_allow_removal_of_assigned_object_if_primary(self): self.mac_a.assigned_object = None @@ -48,6 +61,29 @@ def test_clean_will_allow_removal_of_assigned_object_if_not_primary(self): self.mac_b.assigned_object = None self.mac_b.clean() + @tag('regression') + def test_mac_address_ordering_with_duplicates(self): + """ + Test that MAC addresses with the same value are ordered by their primary key (pk). + This ensures deterministic ordering for pagination when multiple records share the same MAC address. + """ + mac_addresses = list(MACAddress.objects.all()) + + mac_address_groups = {} + for mac in mac_addresses: + if mac.mac_address not in mac_address_groups: + mac_address_groups[mac.mac_address] = [] + mac_address_groups[mac.mac_address].append(mac) + + for mac_value, macs in mac_address_groups.items(): + if len(macs) > 1: + for i in range(1, len(macs)): + self.assertLess( + macs[i - 1].pk, + macs[i].pk, + f"MAC addresses with value {mac_value} are not ordered by primary key" + ) + class LocationTestCase(TestCase): From 375b1dc2ba61149dc71d5c52f3703e720d8a62c5 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 4 Aug 2025 09:24:49 -0400 Subject: [PATCH 2/3] Remove test --- ...options.py => 0209_macaddress_ordering.py} | 0 netbox/dcim/tests/test_models.py | 36 ------------------- 2 files changed, 36 deletions(-) rename netbox/dcim/migrations/{0209_alter_macaddress_options.py => 0209_macaddress_ordering.py} (100%) diff --git a/netbox/dcim/migrations/0209_alter_macaddress_options.py b/netbox/dcim/migrations/0209_macaddress_ordering.py similarity index 100% rename from netbox/dcim/migrations/0209_alter_macaddress_options.py rename to netbox/dcim/migrations/0209_macaddress_ordering.py diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index 1037957660a..be9f067d432 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -37,19 +37,6 @@ def setUpTestData(cls): cls.interface.primary_mac_address = cls.mac_a cls.interface.save() - shared_mac = '1234567890ab' - interfaces = [] - for i in range(10): - interfaces.append(Interface( - device=device, name=f'Interface {i:03d}', type='1000base-t' - )) - Interface.objects.bulk_create(interfaces) - - mac_addresses = [] - for interface in interfaces: - mac_addresses.append(MACAddress(mac_address=shared_mac, assigned_object=interface)) - MACAddress.objects.bulk_create(mac_addresses) - @tag('regression') def test_clean_will_not_allow_removal_of_assigned_object_if_primary(self): self.mac_a.assigned_object = None @@ -61,29 +48,6 @@ def test_clean_will_allow_removal_of_assigned_object_if_not_primary(self): self.mac_b.assigned_object = None self.mac_b.clean() - @tag('regression') - def test_mac_address_ordering_with_duplicates(self): - """ - Test that MAC addresses with the same value are ordered by their primary key (pk). - This ensures deterministic ordering for pagination when multiple records share the same MAC address. - """ - mac_addresses = list(MACAddress.objects.all()) - - mac_address_groups = {} - for mac in mac_addresses: - if mac.mac_address not in mac_address_groups: - mac_address_groups[mac.mac_address] = [] - mac_address_groups[mac.mac_address].append(mac) - - for mac_value, macs in mac_address_groups.items(): - if len(macs) > 1: - for i in range(1, len(macs)): - self.assertLess( - macs[i - 1].pk, - macs[i].pk, - f"MAC addresses with value {mac_value} are not ordered by primary key" - ) - class LocationTestCase(TestCase): From 3d2cf792220f2c5f1fcccb27febe125b27d79105 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 4 Aug 2025 09:32:08 -0400 Subject: [PATCH 3/3] Resolve migration conflict --- ...{0209_macaddress_ordering.py => 0210_macaddress_ordering.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename netbox/dcim/migrations/{0209_macaddress_ordering.py => 0210_macaddress_ordering.py} (86%) diff --git a/netbox/dcim/migrations/0209_macaddress_ordering.py b/netbox/dcim/migrations/0210_macaddress_ordering.py similarity index 86% rename from netbox/dcim/migrations/0209_macaddress_ordering.py rename to netbox/dcim/migrations/0210_macaddress_ordering.py index 2f3b6de06e3..5d719c5ada5 100644 --- a/netbox/dcim/migrations/0209_macaddress_ordering.py +++ b/netbox/dcim/migrations/0210_macaddress_ordering.py @@ -4,7 +4,7 @@ class Migration(migrations.Migration): dependencies = [ - ('dcim', '0208_devicerole_uniqueness'), + ('dcim', '0209_device_component_denorm_site_location'), ] operations = [