Przeglądaj źródła

Closes #21459: Avoid prefetching data for hidden table columns (#21460)

Jeremy Stretch 11 godzin temu
rodzic
commit
d5e8f7dafa

+ 2 - 2
netbox/core/views.py

@@ -684,7 +684,7 @@ class PluginListView(BasePluginView):
 
         plugins = [plugin for plugin in plugins if not plugin.hidden]
 
-        table = CatalogPluginTable(plugins, user=request.user)
+        table = CatalogPluginTable(plugins)
         table.configure(request)
 
         # If this is an HTMX request, return only the rendered table HTML
@@ -707,7 +707,7 @@ class PluginView(BasePluginView):
             raise Http404(_("Plugin {name} not found").format(name=name))
         plugin = plugins[name]
 
-        table = PluginVersionTable(plugin.release_recent_history, user=request.user)
+        table = PluginVersionTable(plugin.release_recent_history)
         table.configure(request)
 
         return render(request, 'core/plugin.html', {

+ 3 - 7
netbox/extras/views.py

@@ -1582,11 +1582,7 @@ class ScriptJobsView(BaseScriptView):
     def get(self, request, **kwargs):
         script = self.get_object(**kwargs)
 
-        jobs_table = ScriptJobTable(
-            data=script.jobs.all(),
-            orderable=False,
-            user=request.user
-        )
+        jobs_table = ScriptJobTable(data=script.jobs.all(), orderable=False)
         jobs_table.configure(request)
 
         return render(request, 'extras/script/jobs.html', {
@@ -1632,7 +1628,7 @@ class ScriptResultView(TableMixin, generic.ObjectView):
                         }
                         data.append(result)
 
-                table = ScriptResultsTable(data, user=request.user)
+                table = ScriptResultsTable(data)
                 table.configure(request)
             else:
                 # for legacy reports
@@ -1656,7 +1652,7 @@ class ScriptResultView(TableMixin, generic.ObjectView):
                             }
                             data.append(result)
 
-            table = ReportResultsTable(data, user=request.user)
+            table = ReportResultsTable(data)
             table.configure(request)
 
         return table

+ 38 - 31
netbox/netbox/tables/tables.py

@@ -53,43 +53,14 @@ class BaseTable(tables.Table):
             'class': 'table table-hover object-list',
         }
 
-    def __init__(self, *args, user=None, **kwargs):
-
+    # TODO: Remove user kwarg in NetBox v4.7
+    def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
         # Set default empty_text if none was provided
         if self.empty_text is None:
             self.empty_text = _("No {model_name} found").format(model_name=self._meta.model._meta.verbose_name_plural)
 
-        # Dynamically update the table's QuerySet to ensure related fields are pre-fetched
-        if isinstance(self.data, TableQuerysetData):
-
-            prefetch_fields = []
-            for column in self.columns:
-                if column.visible:
-                    model = getattr(self.Meta, 'model')
-                    accessor = column.accessor
-                    if accessor.startswith('custom_field_data__'):
-                        # Ignore custom field references
-                        continue
-                    prefetch_path = []
-                    for field_name in accessor.split(accessor.SEPARATOR):
-                        try:
-                            field = model._meta.get_field(field_name)
-                        except FieldDoesNotExist:
-                            break
-                        if isinstance(field, (RelatedField, ManyToOneRel)):
-                            # Follow ForeignKeys to the related model
-                            prefetch_path.append(field_name)
-                            model = field.remote_field.model
-                        elif isinstance(field, GenericForeignKey):
-                            # Can't prefetch beyond a GenericForeignKey
-                            prefetch_path.append(field_name)
-                            break
-                    if prefetch_path:
-                        prefetch_fields.append('__'.join(prefetch_path))
-            self.data.data = self.data.data.prefetch_related(*prefetch_fields)
-
     def _get_columns(self, visible=True):
         columns = []
         for name, column in self.columns.items():
@@ -145,6 +116,41 @@ class BaseTable(tables.Table):
             self.sequence.remove('actions')
             self.sequence.append('actions')
 
+    def _apply_prefetching(self):
+        """
+        Dynamically update the table's QuerySet to ensure related fields are pre-fetched
+        """
+        if not isinstance(self.data, TableQuerysetData):
+            return
+
+        prefetch_fields = []
+        for column in self.columns:
+            if not column.visible:
+                # Skip hidden columns
+                continue
+            model = getattr(self.Meta, 'model')  # Must be called *after* resolving columns
+            accessor = column.accessor
+            if accessor.startswith('custom_field_data__'):
+                # Ignore custom field references
+                continue
+            prefetch_path = []
+            for field_name in accessor.split(accessor.SEPARATOR):
+                try:
+                    field = model._meta.get_field(field_name)
+                except FieldDoesNotExist:
+                    break
+                if isinstance(field, (RelatedField, ManyToOneRel)):
+                    # Follow ForeignKeys to the related model
+                    prefetch_path.append(field_name)
+                    model = field.remote_field.model
+                elif isinstance(field, GenericForeignKey):
+                    # Can't prefetch beyond a GenericForeignKey
+                    prefetch_path.append(field_name)
+                    break
+            if prefetch_path:
+                prefetch_fields.append('__'.join(prefetch_path))
+        self.data.data = self.data.data.prefetch_related(*prefetch_fields)
+
     def configure(self, request):
         """
         Configure the table for a specific request context. This performs pagination and records
@@ -179,6 +185,7 @@ class BaseTable(tables.Table):
             columns = getattr(self.Meta, 'default_columns', self.Meta.fields)
 
         self._set_columns(columns)
+        self._apply_prefetching()
         if ordering is not None:
             self.order_by = ordering
 

+ 44 - 3
netbox/netbox/tests/test_tables.py

@@ -1,9 +1,50 @@
 from django.template import Context, Template
-from django.test import TestCase
+from django.test import RequestFactory, TestCase
 
-from dcim.models import Site
+from dcim.models import Device, Site
+from dcim.tables import DeviceTable
 from netbox.tables import NetBoxTable, columns
-from utilities.testing import create_tags
+from utilities.testing import create_tags, create_test_device, create_test_user
+
+
+class BaseTableTest(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        create_test_device('Test Device 1')
+        cls.user = create_test_user('testuser')
+
+    def test_prefetch_visible_columns(self):
+        """
+        Verify that the table queryset's prefetch_related lookups correspond to the user's
+        visible column preferences. Columns referencing related fields should only be
+        prefetched when those columns are visible.
+        """
+        request = RequestFactory().get('/')
+        request.user = self.user
+
+        # Scenario 1: 'rack' (simple FK) and 'region' (nested accessor: site__region) are visible
+        self.user.config.set(
+            'tables.DeviceTable.columns',
+            ['name', 'status', 'site', 'rack', 'region'],
+            commit=True,
+        )
+        table = DeviceTable(Device.objects.all())
+        table.configure(request)
+        prefetch_lookups = table.data.data._prefetch_related_lookups
+        self.assertIn('rack', prefetch_lookups)
+        self.assertIn('site__region', prefetch_lookups)
+
+        # Scenario 2: Local fields only; no prefetching
+        self.user.config.set(
+            'tables.DeviceTable.columns',
+            ['name', 'status', 'description'],
+            commit=True,
+        )
+        table = DeviceTable(Device.objects.all())
+        table.configure(request)
+        prefetch_lookups = table.data.data._prefetch_related_lookups
+        self.assertEqual(prefetch_lookups, tuple())
 
 
 class TagColumnTable(NetBoxTable):

+ 2 - 7
netbox/netbox/views/generic/feature_views.py

@@ -69,7 +69,6 @@ class ObjectChangeLogView(ConditionalLoginRequiredMixin, View):
         objectchanges_table = ObjectChangeTable(
             data=objectchanges,
             orderable=False,
-            user=request.user
         )
         objectchanges_table.configure(request)
 
@@ -153,7 +152,7 @@ class ObjectJournalView(ConditionalLoginRequiredMixin, View):
             assigned_object_type=content_type,
             assigned_object_id=obj.pk
         )
-        journalentry_table = JournalEntryTable(journalentries, user=request.user)
+        journalentry_table = JournalEntryTable(journalentries)
         journalentry_table.configure(request)
         journalentry_table.columns.hide('assigned_object_type')
         journalentry_table.columns.hide('assigned_object')
@@ -220,11 +219,7 @@ class ObjectJobsView(ConditionalLoginRequiredMixin, View):
 
         # Gather all Jobs for this object
         jobs = self.get_jobs(obj)
-        jobs_table = JobTable(
-            data=jobs,
-            orderable=False,
-            user=request.user
-        )
+        jobs_table = JobTable(data=jobs, orderable=False)
         jobs_table.configure(request)
 
         # Default to using "<app>/<model>.html" as the template, if it exists. Otherwise,

+ 1 - 1
netbox/netbox/views/generic/mixins.py

@@ -92,7 +92,7 @@ class TableMixin:
                 request.user.config.set(f'tables.{table}.columns', tableconfig.columns)
                 request.user.config.set(f'tables.{table}.ordering', tableconfig.ordering, commit=True)
 
-        table = self.table(data, user=request.user)
+        table = self.table(data)
         if 'pk' in table.base_columns and bulk_actions:
             table.columns.show('pk')
         table.configure(request)

+ 1 - 1
netbox/wireless/views.py

@@ -110,7 +110,7 @@ class WirelessLANView(generic.ObjectView):
         attached_interfaces = Interface.objects.restrict(request.user, 'view').filter(
             wireless_lans=instance
         )
-        interfaces_table = tables.WirelessLANInterfacesTable(attached_interfaces, user=request.user)
+        interfaces_table = tables.WirelessLANInterfacesTable(attached_interfaces)
         interfaces_table.configure(request)
 
         return {