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

Fixes #23447: Ignore ordering by a count field left out of the REST API response

Arthur 5 дней назад
Родитель
Сommit
397abb0ba2
3 измененных файлов с 74 добавлено и 1 удалено
  1. 43 0
      netbox/dcim/tests/test_api.py
  2. 1 1
      netbox/netbox/settings.py
  3. 30 0
      netbox/virtualization/tests/test_api.py

+ 43 - 0
netbox/dcim/tests/test_api.py

@@ -1425,6 +1425,13 @@ class RackTestCase(APIViewTestCases.APIViewTestCase):
             },
             },
         ]
         ]
 
 
+    def test_ordering_by_count_without_field(self):
+        """Ordering by a count left out of the response is ignored rather than failing the request."""
+        self.add_permissions('dcim.view_rack')
+        response = self.client.get(f'{self._get_list_url()}?ordering=-device_count&fields=id,name', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        self.assertEqual([row['name'] for row in response.data['results']], ['Rack 1', 'Rack 2', 'Rack 3'])
+
     def test_get_rack_elevation(self):
     def test_get_rack_elevation(self):
         """
         """
         GET a single rack elevation.
         GET a single rack elevation.
@@ -4336,6 +4343,42 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
             filters.append(f'device_id={device.pk}')
             filters.append(f'device_id={device.pk}')
         return [filters[0], '&'.join(filters)]
         return [filters[0], '&'.join(filters)]
 
 
+    def test_ordering_by_count_annotation(self):
+        """Ordering by a count annotation sorts a response that renders it and is ignored by one that does not."""
+        self.add_permissions('dcim.view_interface', 'extras.view_exporttemplate')
+        interfaces = {name: Interface.objects.get(name=name) for name in ('Interface 2', 'Interface 3')}
+        IPAddress.objects.bulk_create((
+            IPAddress(address='192.0.2.1/24', assigned_object=interfaces['Interface 2']),
+            IPAddress(address='192.0.2.2/24', assigned_object=interfaces['Interface 2']),
+            IPAddress(address='192.0.2.3/24', assigned_object=interfaces['Interface 3']),
+        ))
+        export_template = ExportTemplate.objects.create(
+            name='Interfaces', template_code='{% for interface in queryset %}{{ interface.name }},{% endfor %}'
+        )
+        export_template.object_types.set([ObjectType.objects.get_for_model(Interface)])
+        url = f'{self._get_list_url()}?ordering=-_count_ipaddresses'
+
+        for params in ('', '&fields=id,name,count_ipaddresses'):
+            with self.subTest(params=params):
+                response = self.client.get(url + params, **self.header)
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+                self.assertEqual(
+                    [row['name'] for row in response.data['results']], ['Interface 2', 'Interface 3', 'Interface 1']
+                )
+
+        # Responses without the count carry no annotation to order by, so they keep the default ordering
+        for params in ('&brief=1', '&fields=id,name', '&export=Interfaces'):
+            with self.subTest(params=params):
+                response = self.client.get(url + params, **self.header)
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+                if 'export' in params:
+                    self.assertEqual(response.content.decode(), 'Interface 1,Interface 2,Interface 3,')
+                else:
+                    self.assertEqual(
+                        [row['name'] for row in response.data['results']],
+                        ['Interface 1', 'Interface 2', 'Interface 3'],
+                    )
+
     def test_bulk_delete_child_interfaces(self):
     def test_bulk_delete_child_interfaces(self):
         interface1 = Interface.objects.get(name='Interface 1')
         interface1 = Interface.objects.get(name='Interface 1')
         device = interface1.device
         device = interface1.device

+ 1 - 1
netbox/netbox/settings.py

@@ -793,7 +793,7 @@ REST_FRAMEWORK = {
     ),
     ),
     'DEFAULT_FILTER_BACKENDS': (
     'DEFAULT_FILTER_BACKENDS': (
         'django_filters.rest_framework.DjangoFilterBackend',
         'django_filters.rest_framework.DjangoFilterBackend',
-        'rest_framework.filters.OrderingFilter',
+        'netbox.api.filters.OrderingFilter',
     ),
     ),
     'DEFAULT_METADATA_CLASS': 'netbox.api.metadata.BulkOperationMetadata',
     'DEFAULT_METADATA_CLASS': 'netbox.api.metadata.BulkOperationMetadata',
     'DEFAULT_PAGINATION_CLASS': 'netbox.api.pagination.NetBoxPagination',
     'DEFAULT_PAGINATION_CLASS': 'netbox.api.pagination.NetBoxPagination',

+ 30 - 0
netbox/virtualization/tests/test_api.py

@@ -1138,6 +1138,36 @@ class VMInterfaceTestCase(APIViewTestCases.APIViewTestCase):
         # Unannotated instances count through the relations
         # Unannotated instances count through the relations
         self.assertEqual((vm_interface.count_ipaddresses, vm_interface.count_fhrp_groups), (2, 1))
         self.assertEqual((vm_interface.count_ipaddresses, vm_interface.count_fhrp_groups), (2, 1))
 
 
+    def test_ordering_by_count_annotation(self):
+        """Ordering by a count annotation sorts a response that renders it and is ignored by one that does not."""
+        self.add_permissions('virtualization.view_vminterface')
+        groups = FHRPGroup.objects.bulk_create((
+            FHRPGroup(protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=1),
+            FHRPGroup(protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=2),
+        ))
+        interfaces = {name: VMInterface.objects.get(name=name) for name in ('Interface 2', 'Interface 3')}
+        FHRPGroupAssignment.objects.bulk_create((
+            FHRPGroupAssignment(group=groups[0], interface=interfaces['Interface 2'], priority=100),
+            FHRPGroupAssignment(group=groups[1], interface=interfaces['Interface 2'], priority=100),
+            FHRPGroupAssignment(group=groups[0], interface=interfaces['Interface 3'], priority=100),
+        ))
+        url = f'{self._get_list_url()}?ordering=-_count_fhrp_groups'
+
+        response = self.client.get(url, **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        self.assertEqual(
+            [row['name'] for row in response.data['results']], ['Interface 2', 'Interface 3', 'Interface 1']
+        )
+
+        # Responses without the count carry no annotation to order by, so they keep the default ordering
+        for params in ('&brief=1', '&fields=id,name'):
+            with self.subTest(params=params):
+                response = self.client.get(url + params, **self.header)
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+                self.assertEqual(
+                    [row['name'] for row in response.data['results']], ['Interface 1', 'Interface 2', 'Interface 3']
+                )
+
     def test_export_template_prefetches_related_objects(self):
     def test_export_template_prefetches_related_objects(self):
         """An API export renders model instances, so it prefetches each counted relation once."""
         """An API export renders model instances, so it prefetches each counted relation once."""
         self.add_permissions('virtualization.view_vminterface', 'extras.view_exporttemplate')
         self.add_permissions('virtualization.view_vminterface', 'extras.view_exporttemplate')