Compare commits

..

1 Commits

Author SHA1 Message Date
Arthur
8cbfe94fba fix performance regression for Site save, use bulk_update for cached fields 2026-01-14 16:30:40 -08:00
7 changed files with 21 additions and 111 deletions

View File

@@ -30,13 +30,13 @@ jobs:
uses: actions/checkout@v4 uses: actions/checkout@v4
- name: Initialize CodeQL - name: Initialize CodeQL
uses: github/codeql-action/init@v4 uses: github/codeql-action/init@v3
with: with:
languages: ${{ matrix.language }} languages: ${{ matrix.language }}
build-mode: ${{ matrix.build-mode }} build-mode: ${{ matrix.build-mode }}
config-file: .github/codeql/codeql-config.yml config-file: .github/codeql/codeql-config.yml
- name: Perform CodeQL Analysis - name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v4 uses: github/codeql-action/analyze@v3
with: with:
category: "/language:${{matrix.language}}" category: "/language:${{matrix.language}}"

View File

@@ -44,4 +44,3 @@ class DataFileSerializer(NetBoxModelSerializer):
'id', 'url', 'display_url', 'display', 'source', 'path', 'last_updated', 'size', 'hash', 'id', 'url', 'display_url', 'display', 'source', 'path', 'last_updated', 'size', 'hash',
] ]
brief_fields = ('id', 'url', 'display', 'path') brief_fields = ('id', 'url', 'display', 'path')
read_only_fields = ['path', 'last_updated', 'size', 'hash']

View File

@@ -12,7 +12,7 @@ from django.core.validators import RegexValidator
from django.db import models from django.db import models
from django.urls import reverse from django.urls import reverse
from django.utils import timezone from django.utils import timezone
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext as _
from netbox.constants import CENSOR_TOKEN, CENSOR_TOKEN_CHANGED from netbox.constants import CENSOR_TOKEN, CENSOR_TOKEN_CHANGED
from netbox.models import PrimaryModel from netbox.models import PrimaryModel
@@ -128,9 +128,7 @@ class DataSource(JobsMixin, PrimaryModel):
# Ensure URL scheme matches selected type # Ensure URL scheme matches selected type
if self.backend_class.is_local and self.url_scheme not in ('file', ''): if self.backend_class.is_local and self.url_scheme not in ('file', ''):
raise ValidationError({ raise ValidationError({
'source_url': _("URLs for local sources must start with {scheme} (or specify no scheme)").format( 'source_url': "URLs for local sources must start with file:// (or specify no scheme)"
scheme='file://'
)
}) })
def save(self, *args, **kwargs): def save(self, *args, **kwargs):

View File

@@ -211,12 +211,16 @@ def sync_cached_scope_fields(instance, created, **kwargs):
for model in (Prefix, Cluster, WirelessLAN): for model in (Prefix, Cluster, WirelessLAN):
qs = model.objects.filter(**filters) qs = model.objects.filter(**filters)
# Bulk update cached fields to avoid O(N) performance issues with large datasets.
# This does not trigger post_save signals, avoiding spurious change log entries.
objects_to_update = []
for obj in qs: for obj in qs:
# Recompute cache using the same logic as save() # Recompute cache using the same logic as save()
obj.cache_related_objects() obj.cache_related_objects()
obj.save(update_fields=[ objects_to_update.append(obj)
'_location',
'_site', if objects_to_update:
'_site_group', model.objects.bulk_update(
'_region', objects_to_update,
]) ['_location', '_site', '_site_group', '_region']
)

View File

@@ -28,7 +28,7 @@ class ConfigContextProfileSerializer(PrimaryModelSerializer):
) )
data_file = DataFileSerializer( data_file = DataFileSerializer(
nested=True, nested=True,
required=False read_only=True
) )
class Meta: class Meta:
@@ -143,7 +143,7 @@ class ConfigContextSerializer(OwnerMixin, ChangeLogMessageSerializer, ValidatedM
) )
data_file = DataFileSerializer( data_file = DataFileSerializer(
nested=True, nested=True,
required=False read_only=True
) )
class Meta: class Meta:

View File

@@ -1,5 +1,4 @@
import datetime import datetime
import hashlib
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.urls import reverse from django.urls import reverse
@@ -8,7 +7,7 @@ from rest_framework import status
from core.choices import ManagedFileRootPathChoices from core.choices import ManagedFileRootPathChoices
from core.events import * from core.events import *
from core.models import DataFile, DataSource, ObjectType from core.models import ObjectType
from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Rack, Location, RackRole, Site from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Rack, Location, RackRole, Site
from extras.choices import * from extras.choices import *
from extras.models import * from extras.models import *
@@ -732,51 +731,6 @@ class ConfigContextProfileTest(APIViewTestCases.APIViewTestCase):
) )
ConfigContextProfile.objects.bulk_create(profiles) ConfigContextProfile.objects.bulk_create(profiles)
def test_update_data_source_and_data_file(self):
"""
Regression test: Ensure data_source and data_file can be assigned via the API.
This specifically covers PATCHing a ConfigContext with integer IDs for both fields.
"""
self.add_permissions(
'core.view_datafile',
'core.view_datasource',
'extras.view_configcontextprofile',
'extras.change_configcontextprofile',
)
config_context_profile = ConfigContextProfile.objects.first()
# Create a data source and file
datasource = DataSource.objects.create(
name='Data Source 1',
type='local',
source_url='file:///tmp/netbox-datasource/',
)
# Generate a valid dummy YAML file
file_data = b'profile: configcontext\n'
datafile = DataFile.objects.create(
source=datasource,
path='dir1/file1.yml',
last_updated=now(),
size=len(file_data),
hash=hashlib.sha256(file_data).hexdigest(),
data=file_data,
)
url = self._get_detail_url(config_context_profile)
payload = {
'data_source': datasource.pk,
'data_file': datafile.pk,
}
response = self.client.patch(url, payload, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
config_context_profile.refresh_from_db()
self.assertEqual(config_context_profile.data_source_id, datasource.pk)
self.assertEqual(config_context_profile.data_file_id, datafile.pk)
self.assertEqual(response.data['data_source']['id'], datasource.pk)
self.assertEqual(response.data['data_file']['id'], datafile.pk)
class ConfigContextTest(APIViewTestCases.APIViewTestCase): class ConfigContextTest(APIViewTestCases.APIViewTestCase):
model = ConfigContext model = ConfigContext
@@ -858,51 +812,6 @@ class ConfigContextTest(APIViewTestCases.APIViewTestCase):
rendered_context = device.get_config_context() rendered_context = device.get_config_context()
self.assertEqual(rendered_context['bar'], 456) self.assertEqual(rendered_context['bar'], 456)
def test_update_data_source_and_data_file(self):
"""
Regression test: Ensure data_source and data_file can be assigned via the API.
This specifically covers PATCHing a ConfigContext with integer IDs for both fields.
"""
self.add_permissions(
'core.view_datafile',
'core.view_datasource',
'extras.view_configcontext',
'extras.change_configcontext',
)
config_context = ConfigContext.objects.first()
# Create a data source and file
datasource = DataSource.objects.create(
name='Data Source 1',
type='local',
source_url='file:///tmp/netbox-datasource/',
)
# Generate a valid dummy YAML file
file_data = b'context: config\n'
datafile = DataFile.objects.create(
source=datasource,
path='dir1/file1.yml',
last_updated=now(),
size=len(file_data),
hash=hashlib.sha256(file_data).hexdigest(),
data=file_data,
)
url = self._get_detail_url(config_context)
payload = {
'data_source': datasource.pk,
'data_file': datafile.pk,
}
response = self.client.patch(url, payload, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
config_context.refresh_from_db()
self.assertEqual(config_context.data_source_id, datasource.pk)
self.assertEqual(config_context.data_file_id, datafile.pk)
self.assertEqual(response.data['data_source']['id'], datasource.pk)
self.assertEqual(response.data['data_file']['id'], datafile.pk)
class ConfigTemplateTest(APIViewTestCases.APIViewTestCase): class ConfigTemplateTest(APIViewTestCases.APIViewTestCase):
model = ConfigTemplate model = ConfigTemplate

View File

@@ -8,7 +8,7 @@ msgid ""
msgstr "" msgstr ""
"Project-Id-Version: PACKAGE VERSION\n" "Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n" "Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2026-01-15 05:05+0000\n" "POT-Creation-Date: 2026-01-13 05:05+0000\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n" "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -12489,8 +12489,8 @@ msgstr ""
msgid "Delete Selected" msgid "Delete Selected"
msgstr "" msgstr ""
#: netbox/netbox/plugins/navigation.py:53 #: netbox/netbox/plugins/navigation.py:55
#: netbox/netbox/plugins/navigation.py:89 #: netbox/netbox/plugins/navigation.py:88
msgid "Permissions must be passed as a tuple or list." msgid "Permissions must be passed as a tuple or list."
msgstr "" msgstr ""
@@ -12498,7 +12498,7 @@ msgstr ""
msgid "Buttons must be passed as a tuple or list." msgid "Buttons must be passed as a tuple or list."
msgstr "" msgstr ""
#: netbox/netbox/plugins/navigation.py:95 #: netbox/netbox/plugins/navigation.py:92
msgid "Button color must be a choice within ButtonColorChoices." msgid "Button color must be a choice within ButtonColorChoices."
msgstr "" msgstr ""