فهرست منبع

Fixes #23498: Announce v5.0 removal of finalize on export and config templates

Replace the v4.6.1 note in the export and config template docs with a deprecation warning naming v5.0 as the removal. Rendering a template with a stored finalize now emits a FutureWarning once per template per process. Render output is unchanged.
Jason Novinger 10 ساعت پیش
والد
کامیت
1acfeb8f30

+ 2 - 2
docs/models/extras/configtemplate.md

@@ -32,9 +32,9 @@ The `undefined` Jinja environment parameter, which must reference a Python class
 }
 ```
 
-!!! note "Changed in NetBox v4.6.1"
+!!! warning "Deprecation Warning"
 
-    The `finalize` parameter is deprecated and cannot be set on new or modified templates. Templates that already have it saved continue to honor it.
+    The `finalize` parameter has been deprecated as of NetBox v4.6.1, and is scheduled for removal in NetBox v5.0. It cannot be set on new or modified templates. Templates that already have it saved continue to honor it until then. Move any `finalize` logic into the template code itself.
 
 ### MIME Type
 

+ 2 - 2
docs/models/extras/exporttemplate.md

@@ -32,9 +32,9 @@ The `undefined` Jinja environment parameter, which must reference a Python class
 }
 ```
 
-!!! note "Changed in NetBox v4.6.1"
+!!! warning "Deprecation Warning"
 
-    The `finalize` parameter is deprecated and cannot be set on new or modified templates. Templates that already have it saved continue to honor it.
+    The `finalize` parameter has been deprecated as of NetBox v4.6.1, and is scheduled for removal in NetBox v5.0. It cannot be set on new or modified templates. Templates that already have it saved continue to honor it until then. Move any `finalize` logic into the template code itself.
 
 ### MIME Type
 

+ 8 - 0
netbox/extras/models/mixins.py

@@ -3,6 +3,7 @@ import importlib.util
 import logging
 import os
 import sys
+import warnings
 from collections import defaultdict
 
 from django.core.exceptions import ValidationError
@@ -253,6 +254,13 @@ class RenderTemplateMixin(models.Model):
                 type(self).__name__, self.pk,
             )
             return {key: value for key, value in params.items() if key != 'finalize'}
+        # The pk makes each template's text unique, so the default filter warns once per template.
+        # Do not set stacklevel: a caller's line would repeat the warning for each caller.
+        warnings.warn(
+            f'The "finalize" environment parameter stored on {type(self).__name__} (pk={self.pk}) is deprecated and '
+            'will be removed in NetBox v5.0. Move its logic into the template code.',
+            FutureWarning,
+        )
         return {**params, 'finalize': stored_finalize}
 
     def get_environment_params(self):

+ 111 - 0
netbox/extras/tests/test_models.py

@@ -2,6 +2,7 @@ import io
 import os
 import sys
 import tempfile
+import warnings
 from pathlib import Path
 from types import SimpleNamespace
 from unittest.mock import patch
@@ -1709,6 +1710,116 @@ class ExportTemplateStoredOnlyFinalizeTestCase(TestCase):
             t.get_environment_params()
 
 
+class StoredFinalizeDeprecationWarningTestCase(TestCase):
+    """
+    A stored 'finalize' stays honored until v5.0, so operators get a once-per-template FutureWarning and unchanged
+    output (#23498).
+    """
+    TEMPLATE_CODE = '{{ "netbox" }}'
+    SHARED_PK = 9001
+
+    def _create(self, model, name, **kwargs):
+        # create() does not call clean(). This matches a value stored before clean() blocked it.
+        return model.objects.create(name=name, template_code=self.TEMPLATE_CODE, **kwargs)
+
+    def _create_stored(self, model, name='stored'):
+        return self._create(model, name, environment_params={'finalize': 'builtins.len'})
+
+    def _render_recording(self, *templates):
+        """
+        Use the default filter, not 'always', so the once-per-template dedupe is exercised.
+        """
+        with warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter('default')
+            for template in templates:
+                template.render()
+        return [w for w in caught if issubclass(w.category, FutureWarning)]
+
+    def test_render_output_unchanged_with_stored_finalize(self):
+        for model in (ExportTemplate, ConfigTemplate):
+            with self.subTest(model=model.__name__):
+                self._create_stored(model)
+                t = model.objects.get(name='stored')
+                with warnings.catch_warnings():
+                    warnings.simplefilter('ignore')
+                    self.assertEqual(t.render(), '6')
+
+    def test_warning_names_model_pk_and_removal_version(self):
+        for model in (ExportTemplate, ConfigTemplate):
+            with self.subTest(model=model.__name__):
+                t = self._create_stored(model, name=f'stored-{model.__name__}')
+                with self.assertNoLogs('extras.models.mixins', 'WARNING'):
+                    caught = self._render_recording(t)
+                self.assertEqual(len(caught), 1)
+                self.assertIs(caught[0].category, FutureWarning)
+                message = str(caught[0].message)
+                self.assertIn(model.__name__, message)
+                self.assertIn(f'pk={t.pk}', message)
+                self.assertIn('v5.0', message)
+                # Must read differently from the "Ignoring the deprecated..." log for in-memory values
+                self.assertNotIn('Ignoring', message)
+
+    def test_warns_once_per_template_per_process(self):
+        first = self._create_stored(ExportTemplate, name='first')
+        second = self._create_stored(ExportTemplate, name='second')
+        caught = self._render_recording(first, first, second, first, second)
+        self.assertEqual(len(caught), 2)
+        self.assertIn(f'pk={first.pk}', str(caught[0].message))
+        self.assertIn(f'pk={second.pk}', str(caught[1].message))
+
+    def test_warns_once_across_different_callers(self):
+        """
+        Callers render from different source lines (views, API, scripts). The warning must be attributed to the
+        same site regardless, or the default filter would warn once per caller instead of once per template.
+        """
+        t = self._create_stored(ExportTemplate)
+
+        def render_a():
+            t.render()
+
+        def render_b():
+            t.render()
+
+        with warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter('default')
+            render_a()
+            render_b()
+            render_a()
+        caught = [w for w in caught if issubclass(w.category, FutureWarning)]
+        self.assertEqual(len(caught), 1)
+        self.assertTrue(caught[0].filename.endswith('extras/models/mixins.py'), caught[0].filename)
+
+    def test_warns_separately_for_models_sharing_a_pk(self):
+        params = {'finalize': 'builtins.len'}
+        export_template = self._create(ExportTemplate, 'export', pk=self.SHARED_PK, environment_params=params)
+        config_template = self._create(ConfigTemplate, 'config', pk=self.SHARED_PK, environment_params=params)
+        caught = self._render_recording(export_template, config_template)
+        self.assertEqual(len(caught), 2)
+
+    def test_no_warning_without_finalize(self):
+        for model in (ExportTemplate, ConfigTemplate):
+            with self.subTest(model=model.__name__):
+                t = self._create(model, f'plain-{model.__name__}', environment_params={'trim_blocks': True})
+                self.assertEqual(self._render_recording(t), [])
+
+    def test_no_warning_when_finalize_exists_only_in_memory(self):
+        t = self._create(ExportTemplate, 'saved')
+        t.environment_params = {'finalize': 'builtins.len'}
+        with self.assertLogs('extras.models.mixins', 'WARNING'):
+            caught = self._render_recording(t)
+        self.assertEqual(caught, [])
+
+    def test_no_warning_for_unsaved_template(self):
+        t = ExportTemplate(
+            name='unsaved',
+            template_code=self.TEMPLATE_CODE,
+            environment_params={'finalize': 'builtins.len'},
+        )
+        with self.assertLogs('extras.models.mixins', 'WARNING'):
+            caught = self._render_recording(t)
+        self.assertEqual(caught, [])
+
+
 class WebhookPayloadUrlValidationTestCase(TestCase):
     """Tests for Webhook.clean()'s validation of payload_url (#22828)."""