Skip to content

Commit 52c9e54

Browse files
committed
Address PR feedback, clean up new regression test
The new regression test now avoids casting the query to a string and inspecting the string, which was brittle at best. The new approach asserts directly against `queryset.distinct` for the main query and then finds the subquery that we expect to have distinct set and verifies that is in fact the case. I also realized that the use of `connection.query_log` was problematic, in that it didn't seem to return any queries as expected. This meant that the test was actually not making any assertions since none of the code inside of the for loop over `device_queries` ever ran.
1 parent f51b377 commit 52c9e54

File tree

1 file changed

+32
-22
lines changed

1 file changed

+32
-22
lines changed

netbox/extras/tests/test_models.py

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
1-
import re
21
import tempfile
32
from pathlib import Path
43

54
from django.contrib.contenttypes.models import ContentType
65
from django.core.files.uploadedfile import SimpleUploadedFile
7-
from django.db import connection
86
from django.forms import ValidationError
97
from django.test import tag, TestCase
108

119
from core.models import DataSource, ObjectType
1210
from dcim.models import Device, DeviceRole, DeviceType, Location, Manufacturer, Platform, Region, Site, SiteGroup
13-
from extras.models import ConfigContext, ConfigContextProfile, ConfigTemplate, ImageAttachment, Tag
11+
from extras.models import ConfigContext, ConfigContextProfile, ConfigTemplate, ImageAttachment, Tag, TaggedItem
1412
from tenancy.models import Tenant, TenantGroup
1513
from utilities.exceptions import AbortRequest
1614
from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine
@@ -661,26 +659,38 @@ def test_config_context_annotation_query_optimization(self):
661659
Verifies that DISTINCT is only used in tag subquery where needed,
662660
not on the main device query which is expensive for large datasets.
663661
"""
664-
# Use existing test device from ConfigContextTest setup
665662
device = Device.objects.first()
666-
667-
# connection.queries_log.clear()
668-
669-
# Execute annotation
670-
list(Device.objects.filter(pk=device.pk).annotate_config_context_data())
671-
672-
device_queries = [q['sql'] for q in connection.queries_log if 'dcim_device' in q['sql']]
673-
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)
663+
queryset = Device.objects.filter(pk=device.pk).annotate_config_context_data()
664+
665+
# Main device query should NOT use DISTINCT
666+
self.assertFalse(queryset.query.distinct)
667+
668+
# Check that tag subqueries DO use DISTINCT by inspecting the annotation
669+
config_annotation = queryset.query.annotations.get('config_context_data')
670+
self.assertIsNotNone(config_annotation)
671+
672+
def find_tag_subqueries(where_node):
673+
"""Find subqueries in WHERE clause that relate to tag filtering"""
674+
subqueries = []
675+
676+
def traverse(node):
677+
if hasattr(node, 'children'):
678+
for child in node.children:
679+
try:
680+
if child.rhs.query.model is TaggedItem:
681+
subqueries.append(child.rhs.query)
682+
except AttributeError:
683+
traverse(child)
684+
traverse(where_node)
685+
return subqueries
686+
687+
# Find subqueries in the WHERE clause that should have DISTINCT
688+
tag_subqueries = find_tag_subqueries(config_annotation.query.where)
689+
distinct_subqueries = [sq for sq in tag_subqueries if sq.distinct]
690+
691+
# Verify we found at least one DISTINCT subquery for tags
692+
self.assertEqual(len(distinct_subqueries), 1)
693+
self.assertTrue(distinct_subqueries[0].distinct)
684694

685695

686696
class ConfigTemplateTest(TestCase):

0 commit comments

Comments
 (0)