Skip to content

Commit f51b377

Browse files
committed
Add performance regression test for config context annotation
The test verifies that: - Main device queries do not use expensive DISTINCT operations - Tag subqueries properly use DISTINCT to prevent duplicates from issue #5387 This ensures the optimization from issue #20327 (moving .distinct() from maintaining query to tag subquery) cannot be accidentally reverted while maintaining the correctness guarantees for issues #5314 and #5387.
1 parent e5fc23f commit f51b377

File tree

1 file changed

+54
-20
lines changed

1 file changed

+54
-20
lines changed

netbox/extras/tests/test_models.py

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import re
12
import tempfile
23
from pathlib import Path
34

45
from django.contrib.contenttypes.models import ContentType
56
from django.core.files.uploadedfile import SimpleUploadedFile
7+
from django.db import connection
68
from django.forms import ValidationError
79
from django.test import tag, TestCase
810

@@ -536,6 +538,34 @@ def test_virtualmachine_site_context(self):
536538
vms[1].get_config_context()
537539
)
538540

541+
def test_valid_local_context_data(self):
542+
device = Device.objects.first()
543+
device.local_context_data = None
544+
device.clean()
545+
546+
device.local_context_data = {"foo": "bar"}
547+
device.clean()
548+
549+
def test_invalid_local_context_data(self):
550+
device = Device.objects.first()
551+
552+
device.local_context_data = ""
553+
with self.assertRaises(ValidationError):
554+
device.clean()
555+
556+
device.local_context_data = 0
557+
with self.assertRaises(ValidationError):
558+
device.clean()
559+
560+
device.local_context_data = False
561+
with self.assertRaises(ValidationError):
562+
device.clean()
563+
564+
device.local_context_data = 'foo'
565+
with self.assertRaises(ValidationError):
566+
device.clean()
567+
568+
@tag('regression')
539569
def test_multiple_tags_return_distinct_objects(self):
540570
"""
541571
Tagged items use a generic relationship, which results in duplicate rows being returned when queried.
@@ -573,6 +603,7 @@ def test_multiple_tags_return_distinct_objects(self):
573603
self.assertEqual(ConfigContext.objects.get_for_object(device).count(), 1)
574604
self.assertEqual(device.get_config_context(), annotated_queryset[0].get_config_context())
575605

606+
@tag('regression')
576607
def test_multiple_tags_return_distinct_objects_with_separate_config_contexts(self):
577608
"""
578609
Tagged items use a generic relationship, which results in duplicate rows being returned when queried.
@@ -621,32 +652,35 @@ def test_multiple_tags_return_distinct_objects_with_separate_config_contexts(sel
621652
self.assertEqual(ConfigContext.objects.get_for_object(device).count(), 2)
622653
self.assertEqual(device.get_config_context(), annotated_queryset[0].get_config_context())
623654

624-
def test_valid_local_context_data(self):
625-
device = Device.objects.first()
626-
device.local_context_data = None
627-
device.clean()
628-
629-
device.local_context_data = {"foo": "bar"}
630-
device.clean()
655+
@tag('performance', 'regression')
656+
def test_config_context_annotation_query_optimization(self):
657+
"""
658+
Regression test for issue #20327: Ensure config context annotation
659+
doesn't use expensive DISTINCT on main query.
631660
632-
def test_invalid_local_context_data(self):
661+
Verifies that DISTINCT is only used in tag subquery where needed,
662+
not on the main device query which is expensive for large datasets.
663+
"""
664+
# Use existing test device from ConfigContextTest setup
633665
device = Device.objects.first()
634666

635-
device.local_context_data = ""
636-
with self.assertRaises(ValidationError):
637-
device.clean()
667+
# connection.queries_log.clear()
638668

639-
device.local_context_data = 0
640-
with self.assertRaises(ValidationError):
641-
device.clean()
669+
# Execute annotation
670+
list(Device.objects.filter(pk=device.pk).annotate_config_context_data())
642671

643-
device.local_context_data = False
644-
with self.assertRaises(ValidationError):
645-
device.clean()
672+
device_queries = [q['sql'] for q in connection.queries_log if 'dcim_device' in q['sql']]
646673

647-
device.local_context_data = 'foo'
648-
with self.assertRaises(ValidationError):
649-
device.clean()
674+
for query in device_queries:
675+
# Main device query should NOT use DISTINCT
676+
self.assertNotIn('SELECT DISTINCT "dcim_device"', query)
677+
678+
# Tag sub-query _should_ use DISTINCT
679+
tag_distinct_pattern = re.compile(
680+
r'SELECT DISTINCT \w+\."tag_id".*FROM "extras_taggeditem" \w+ INNER JOIN "django_content_type"'
681+
)
682+
distinct_tag_pattern_found = any(tag_distinct_pattern.search(query) for query in device_queries)
683+
self.assertTrue(distinct_tag_pattern_found)
650684

651685

652686
class ConfigTemplateTest(TestCase):

0 commit comments

Comments
 (0)