From 6af12b18148d21f522591966862fb1faab4f6ac2 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 5 Mar 2024 17:14:42 -0500 Subject: [PATCH] Add tests for missing FilterSet filters --- netbox/circuits/tests/test_filtersets.py | 1 + netbox/core/tests/test_filtersets.py | 2 + netbox/dcim/tests/test_filtersets.py | 8 ++ netbox/extras/tests/test_filtersets.py | 30 +++--- netbox/ipam/tests/test_filtersets.py | 2 + netbox/users/tests/test_filtersets.py | 3 + netbox/utilities/testing/filtersets.py | 91 ++++++++++++++++++- .../virtualization/tests/test_filtersets.py | 1 + netbox/vpn/tests/test_filtersets.py | 4 +- 9 files changed, 126 insertions(+), 16 deletions(-) diff --git a/netbox/circuits/tests/test_filtersets.py b/netbox/circuits/tests/test_filtersets.py index 6553179ec..bbd2438d7 100644 --- a/netbox/circuits/tests/test_filtersets.py +++ b/netbox/circuits/tests/test_filtersets.py @@ -330,6 +330,7 @@ class CircuitTestCase(TestCase, ChangeLoggedFilterSetTests): class CircuitTerminationTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = CircuitTermination.objects.all() filterset = CircuitTerminationFilterSet + ignore_fields = ('cable',) @classmethod def setUpTestData(cls): diff --git a/netbox/core/tests/test_filtersets.py b/netbox/core/tests/test_filtersets.py index 8ff104142..aefb9eed0 100644 --- a/netbox/core/tests/test_filtersets.py +++ b/netbox/core/tests/test_filtersets.py @@ -10,6 +10,7 @@ from ..models import * class DataSourceTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = DataSource.objects.all() filterset = DataSourceFilterSet + ignore_fields = ('ignore_rules', 'parameters') @classmethod def setUpTestData(cls): @@ -70,6 +71,7 @@ class DataSourceTestCase(TestCase, ChangeLoggedFilterSetTests): class DataFileTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = DataFile.objects.all() filterset = DataFileFilterSet + ignore_fields = ('data',) @classmethod def setUpTestData(cls): diff --git a/netbox/dcim/tests/test_filtersets.py b/netbox/dcim/tests/test_filtersets.py index f1eeddbb5..8ec1da713 100644 --- a/netbox/dcim/tests/test_filtersets.py +++ b/netbox/dcim/tests/test_filtersets.py @@ -196,6 +196,7 @@ class SiteGroupTestCase(TestCase, ChangeLoggedFilterSetTests): class SiteTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = Site.objects.all() filterset = SiteFilterSet + ignore_fields = ('physical_address', 'shipping_address') @classmethod def setUpTestData(cls): @@ -467,6 +468,7 @@ class RackRoleTestCase(TestCase, ChangeLoggedFilterSetTests): class RackTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = Rack.objects.all() filterset = RackFilterSet + ignore_fields = ('units',) @classmethod def setUpTestData(cls): @@ -726,6 +728,7 @@ class RackTestCase(TestCase, ChangeLoggedFilterSetTests): class RackReservationTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = RackReservation.objects.all() filterset = RackReservationFilterSet + ignore_fields = ('units',) @classmethod def setUpTestData(cls): @@ -889,6 +892,7 @@ class ManufacturerTestCase(TestCase, ChangeLoggedFilterSetTests): class DeviceTypeTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = DeviceType.objects.all() filterset = DeviceTypeFilterSet + ignore_fields = ('front_image', 'rear_image') @classmethod def setUpTestData(cls): @@ -1880,6 +1884,7 @@ class PlatformTestCase(TestCase, ChangeLoggedFilterSetTests): class DeviceTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = Device.objects.all() filterset = DeviceFilterSet + ignore_fields = ('primary_ip4', 'primary_ip6', 'oob_ip', 'local_context_data') @classmethod def setUpTestData(cls): @@ -2332,6 +2337,7 @@ class DeviceTestCase(TestCase, ChangeLoggedFilterSetTests): class ModuleTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = Module.objects.all() filterset = ModuleFilterSet + ignore_fields = ('local_context_data',) @classmethod def setUpTestData(cls): @@ -3229,6 +3235,7 @@ class PowerOutletTestCase(TestCase, DeviceComponentFilterSetTests, ChangeLoggedF class InterfaceTestCase(TestCase, DeviceComponentFilterSetTests, ChangeLoggedFilterSetTests): queryset = Interface.objects.all() filterset = InterfaceFilterSet + ignore_fields = ('untagged_vlan',) @classmethod def setUpTestData(cls): @@ -5332,6 +5339,7 @@ class PowerFeedTestCase(TestCase, ChangeLoggedFilterSetTests): class VirtualDeviceContextTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = VirtualDeviceContext.objects.all() filterset = VirtualDeviceContextFilterSet + ignore_fields = ('primary_ip4', 'primary_ip6') @classmethod def setUpTestData(cls): diff --git a/netbox/extras/tests/test_filtersets.py b/netbox/extras/tests/test_filtersets.py index bec62c688..ccccaa793 100644 --- a/netbox/extras/tests/test_filtersets.py +++ b/netbox/extras/tests/test_filtersets.py @@ -23,9 +23,10 @@ from virtualization.models import Cluster, ClusterGroup, ClusterType User = get_user_model() -class CustomFieldTestCase(TestCase, BaseFilterSetTests): +class CustomFieldTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = CustomField.objects.all() filterset = CustomFieldFilterSet + ignore_fields = ('default',) @classmethod def setUpTestData(cls): @@ -155,9 +156,10 @@ class CustomFieldTestCase(TestCase, BaseFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) -class CustomFieldChoiceSetTestCase(TestCase, BaseFilterSetTests): +class CustomFieldChoiceSetTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = CustomFieldChoiceSet.objects.all() filterset = CustomFieldChoiceSetFilterSet + ignore_fields = ('extra_choices',) @classmethod def setUpTestData(cls): @@ -188,6 +190,7 @@ class CustomFieldChoiceSetTestCase(TestCase, BaseFilterSetTests): class WebhookTestCase(TestCase, BaseFilterSetTests): queryset = Webhook.objects.all() filterset = WebhookFilterSet + ignore_fields = ('additional_headers', 'body_template') @classmethod def setUpTestData(cls): @@ -252,6 +255,7 @@ class WebhookTestCase(TestCase, BaseFilterSetTests): class EventRuleTestCase(TestCase, BaseFilterSetTests): queryset = EventRule.objects.all() filterset = EventRuleFilterSet + ignore_fields = ('action_data', 'conditions') @classmethod def setUpTestData(cls): @@ -405,7 +409,7 @@ class EventRuleTestCase(TestCase, BaseFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) -class CustomLinkTestCase(TestCase, BaseFilterSetTests): +class CustomLinkTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = CustomLink.objects.all() filterset = CustomLinkFilterSet @@ -474,9 +478,10 @@ class CustomLinkTestCase(TestCase, BaseFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) -class SavedFilterTestCase(TestCase, BaseFilterSetTests): +class SavedFilterTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = SavedFilter.objects.all() filterset = SavedFilterFilterSet + ignore_fields = ('parameters',) @classmethod def setUpTestData(cls): @@ -647,9 +652,10 @@ class BookmarkTestCase(TestCase, BaseFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) -class ExportTemplateTestCase(TestCase, BaseFilterSetTests): +class ExportTemplateTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = ExportTemplate.objects.all() filterset = ExportTemplateFilterSet + ignore_fields = ('template_code', 'data_path') @classmethod def setUpTestData(cls): @@ -683,9 +689,10 @@ class ExportTemplateTestCase(TestCase, BaseFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) -class ImageAttachmentTestCase(TestCase, BaseFilterSetTests): +class ImageAttachmentTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = ImageAttachment.objects.all() filterset = ImageAttachmentFilterSet + ignore_fields = ('image',) @classmethod def setUpTestData(cls): @@ -760,12 +767,6 @@ class ImageAttachmentTestCase(TestCase, BaseFilterSetTests): } self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) - def test_created(self): - pk_list = self.queryset.values_list('pk', flat=True)[:2] - self.queryset.filter(pk__in=pk_list).update(created=datetime(2021, 1, 1, 0, 0, 0, tzinfo=timezone.utc)) - params = {'created': '2021-01-01T00:00:00'} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) - class JournalEntryTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = JournalEntry.objects.all() @@ -873,6 +874,7 @@ class JournalEntryTestCase(TestCase, ChangeLoggedFilterSetTests): class ConfigContextTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = ConfigContext.objects.all() filterset = ConfigContextFilterSet + ignore_fields = ('data', 'data_path') @classmethod def setUpTestData(cls): @@ -1096,9 +1098,10 @@ class ConfigContextTestCase(TestCase, ChangeLoggedFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) -class ConfigTemplateTestCase(TestCase, BaseFilterSetTests): +class ConfigTemplateTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = ConfigTemplate.objects.all() filterset = ConfigTemplateFilterSet + ignore_fields = ('template_code', 'environment_params', 'data_path') @classmethod def setUpTestData(cls): @@ -1193,6 +1196,7 @@ class TagTestCase(TestCase, ChangeLoggedFilterSetTests): class ObjectChangeTestCase(TestCase, BaseFilterSetTests): queryset = ObjectChange.objects.all() filterset = ObjectChangeFilterSet + ignore_fields = ('prechange_data', 'postchange_data') @classmethod def setUpTestData(cls): diff --git a/netbox/ipam/tests/test_filtersets.py b/netbox/ipam/tests/test_filtersets.py index bb4f50c21..3eecadb06 100644 --- a/netbox/ipam/tests/test_filtersets.py +++ b/netbox/ipam/tests/test_filtersets.py @@ -1733,6 +1733,7 @@ class VLANTestCase(TestCase, ChangeLoggedFilterSetTests): class ServiceTemplateTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = ServiceTemplate.objects.all() filterset = ServiceTemplateFilterSet + ignore_fields = ('ports',) @classmethod def setUpTestData(cls): @@ -1797,6 +1798,7 @@ class ServiceTemplateTestCase(TestCase, ChangeLoggedFilterSetTests): class ServiceTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = Service.objects.all() filterset = ServiceFilterSet + ignore_fields = ('ports',) @classmethod def setUpTestData(cls): diff --git a/netbox/users/tests/test_filtersets.py b/netbox/users/tests/test_filtersets.py index 5930285a9..5349244f7 100644 --- a/netbox/users/tests/test_filtersets.py +++ b/netbox/users/tests/test_filtersets.py @@ -15,6 +15,7 @@ User = get_user_model() class UserTestCase(TestCase, BaseFilterSetTests): queryset = User.objects.all() filterset = filtersets.UserFilterSet + ignore_fields = ('password',) @classmethod def setUpTestData(cls): @@ -132,6 +133,7 @@ class GroupTestCase(TestCase, BaseFilterSetTests): class ObjectPermissionTestCase(TestCase, BaseFilterSetTests): queryset = ObjectPermission.objects.all() filterset = filtersets.ObjectPermissionFilterSet + ignore_fields = ('actions', 'constraints') @classmethod def setUpTestData(cls): @@ -226,6 +228,7 @@ class ObjectPermissionTestCase(TestCase, BaseFilterSetTests): class TokenTestCase(TestCase, BaseFilterSetTests): queryset = Token.objects.all() filterset = filtersets.TokenFilterSet + ignore_fields = ('allowed_ips',) @classmethod def setUpTestData(cls): diff --git a/netbox/utilities/testing/filtersets.py b/netbox/utilities/testing/filtersets.py index 00f3d9745..7b19f2408 100644 --- a/netbox/utilities/testing/filtersets.py +++ b/netbox/utilities/testing/filtersets.py @@ -1,15 +1,47 @@ -from datetime import date, datetime, timezone +from datetime import datetime, timezone +from itertools import chain +from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation +from django.contrib.contenttypes.models import ContentType +from django.db.models import ForeignKey, ManyToManyField, ManyToManyRel, ManyToOneRel, OneToOneRel +from django.utils.module_loading import import_string +from taggit.managers import TaggableManager + +from core.models import ObjectType __all__ = ( 'BaseFilterSetTests', 'ChangeLoggedFilterSetTests', ) +IGNORE_MODELS = ( + ('core', 'AutoSyncRecord'), + ('core', 'ManagedFile'), + ('core', 'ObjectType'), + ('dcim', 'CablePath'), + ('extras', 'Branch'), + ('extras', 'CachedValue'), + ('extras', 'Dashboard'), + ('extras', 'ScriptModule'), + ('extras', 'StagedChange'), + ('extras', 'TaggedItem'), + ('users', 'UserConfig'), +) + +IGNORE_FIELDS = ( + 'comments', + 'custom_field_data', + 'level', # MPTT + 'lft', # MPTT + 'rght', # MPTT + 'tree_id', # MPTT +) + class BaseFilterSetTests: queryset = None filterset = None + ignore_fields = tuple() def test_id(self): """ @@ -19,6 +51,63 @@ class BaseFilterSetTests: self.assertGreater(self.queryset.count(), 2) self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + def test_missing_filters(self): + """ + Check for any model fields which do not have the required filter(s) defined. + """ + app_label = self.__class__.__module__.split('.')[0] + model = self.queryset.model + model_name = model.__name__ + + # Skip ignored models + if (app_label, model_name) in IGNORE_MODELS: + return + + # Import the FilterSet class & sanity check it + filterset = import_string(f'{app_label}.filtersets.{model_name}FilterSet') + self.assertEqual(model, filterset.Meta.model, "FilterSet model does not match!") + + filterset_fields = sorted(filterset.get_filters()) + + # Check for missing filters + for model_field in model._meta.get_fields(): + + # Skip private fields + if model_field.name.startswith('_'): + continue + + # Skip ignored fields + if model_field.name in chain(self.ignore_fields, IGNORE_FIELDS): + continue + + # One-to-one & one-to-many relationships + if issubclass(model_field.__class__, ForeignKey) or type(model_field) is OneToOneRel: + if model_field.related_model is ContentType: + # Relationships to ContentType (used as part of a GFK) do not need a filter + continue + elif model_field.related_model is ObjectType: + # Filters to ObjectType use 'app.model' rather than numeric PK, so we omit the _id suffix + filter_name = model_field.name + else: + filter_name = f'{model_field.name}_id' + self.assertIn(filter_name, filterset_fields, f'No filter found for {model_field.name}!') + + # TODO: Many-to-one & many-to-many relationships + elif type(model_field) in (ManyToOneRel, ManyToManyField, ManyToManyRel): + continue + + # TODO: Generic relationships + elif type(model_field) in (GenericForeignKey, GenericRelation): + continue + + # Tags + elif type(model_field) is TaggableManager: + self.assertIn('tag', filterset_fields, f'No filter found for {model_field.name}!') + + # All other fields + else: + self.assertIn(model_field.name, filterset_fields, f'No filter found for {model_field.name}!') + class ChangeLoggedFilterSetTests(BaseFilterSetTests): diff --git a/netbox/virtualization/tests/test_filtersets.py b/netbox/virtualization/tests/test_filtersets.py index 5c020e1b2..3d9e17a23 100644 --- a/netbox/virtualization/tests/test_filtersets.py +++ b/netbox/virtualization/tests/test_filtersets.py @@ -522,6 +522,7 @@ class VirtualMachineTestCase(TestCase, ChangeLoggedFilterSetTests): class VMInterfaceTestCase(TestCase, ChangeLoggedFilterSetTests): queryset = VMInterface.objects.all() filterset = VMInterfaceFilterSet + ignore_fields = ('untagged_vlan',) @classmethod def setUpTestData(cls): diff --git a/netbox/vpn/tests/test_filtersets.py b/netbox/vpn/tests/test_filtersets.py index d4e80750d..4d099a065 100644 --- a/netbox/vpn/tests/test_filtersets.py +++ b/netbox/vpn/tests/test_filtersets.py @@ -848,8 +848,8 @@ class L2VPNTerminationTestCase(TestCase, ChangeLoggedFilterSetTests): params = {'l2vpn': [l2vpns[0].slug, l2vpns[1].slug]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6) - def test_content_type(self): - params = {'assigned_object_type_id': ContentType.objects.get(model='vlan').pk} + def test_termination_type(self): + params = {'assigned_object_type': 'ipam.vlan'} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3) def test_interface(self):