Fixes #22750: Converts raw IDs to ObjectVars/MultiObjectVars
This commit is contained in:
parent
bb29246033
commit
113f5ea9d4
|
|
@ -2,6 +2,7 @@ import logging
|
|||
import traceback
|
||||
from contextlib import ExitStack
|
||||
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.db import DEFAULT_DB_ALIAS, router, transaction
|
||||
from django.utils.translation import gettext as _
|
||||
|
||||
|
|
@ -112,16 +113,53 @@ class ScriptJob(JobRunner):
|
|||
script = script_model.python_class()
|
||||
self.logger.debug(f"Loaded script {script.full_name}")
|
||||
|
||||
# Add files to form data
|
||||
if request:
|
||||
files = request.FILES
|
||||
for field_name, fileobj in files.items():
|
||||
data[field_name] = fileobj
|
||||
|
||||
# Add the current request as a property of the script
|
||||
script.request = request
|
||||
self.logger.debug(f"Request ID: {request.id if request else None}")
|
||||
|
||||
# Normalize incoming payload for the form: API callers submit variables under "data".
|
||||
payload = data or {}
|
||||
if isinstance(payload, dict) and 'data' in payload:
|
||||
payload = payload['data'] or {}
|
||||
|
||||
files = request.FILES if request else None
|
||||
if files:
|
||||
for field_name, fileobj in files.items():
|
||||
# merge into payload so script.run receives the uploaded files in data
|
||||
payload[field_name] = fileobj
|
||||
|
||||
# Validate & clean using the script's form so ObjectVar/MultiObjectVar IDs become model instances
|
||||
if hasattr(script, 'as_form') and callable(getattr(script, 'as_form')):
|
||||
try:
|
||||
form = script.as_form(data=payload, files=files)
|
||||
if not form.is_valid():
|
||||
raise AbortScript(f"Script input validation failed: {form.errors.as_json()}")
|
||||
|
||||
cleaned = form.cleaned_data
|
||||
|
||||
# Remove execution parameters
|
||||
for key in list(cleaned.keys()):
|
||||
if key.startswith('_'):
|
||||
cleaned.pop(key)
|
||||
|
||||
# Preserve uploaded files that were merged into the payload so scripts still see them
|
||||
# even if the Script's form doesn't declare file fields.
|
||||
if files:
|
||||
for fname, fobj in files.items():
|
||||
if fname not in cleaned:
|
||||
cleaned[fname] = fobj
|
||||
|
||||
# Use cleaned form data as the data passed into the script
|
||||
data = cleaned
|
||||
except AbortScript:
|
||||
# Re-raise for run_script() to log/handle
|
||||
raise
|
||||
except (ValidationError, TypeError, ValueError) as e:
|
||||
raise AbortScript(f"Error validating script input: {e!s}")
|
||||
else:
|
||||
# Script doesn't provide as_form (e.g., lightweight test double); keep `data` as-is.
|
||||
data = payload
|
||||
|
||||
if commit:
|
||||
self.logger.info("Executing script (commit enabled)")
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -6,7 +6,10 @@ from unittest.mock import MagicMock, patch
|
|||
from django.db import DEFAULT_DB_ALIAS
|
||||
from django.test import TestCase
|
||||
|
||||
from dcim.models import Site
|
||||
from extras.jobs import ScriptJob
|
||||
from extras.models import Script as ScriptModel
|
||||
from extras.scripts import ObjectVar, Script
|
||||
from utilities.exceptions import AbortScript
|
||||
|
||||
|
||||
|
|
@ -338,3 +341,74 @@ class ScriptJobRunTestCase(TestCase):
|
|||
runner.run(data={}, commit=False)
|
||||
|
||||
self.assertEqual(entered, ['proc_a'])
|
||||
|
||||
|
||||
class ScriptJobFormCleaningTestCase(TestCase):
|
||||
def test_run_converts_objectvar_id_to_model_instance(self):
|
||||
# Create a simple target object
|
||||
site = Site.objects.create(name="Test Site", slug="test-site")
|
||||
|
||||
# Real Script subclass that declares an ObjectVar and records received data
|
||||
class TestScript(Script):
|
||||
site = ObjectVar(label='Site', model=Site)
|
||||
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.received = None
|
||||
|
||||
def run(self, data, commit=True):
|
||||
# record what we received and return a sentinel
|
||||
self.received = data
|
||||
return "ok"
|
||||
|
||||
script_instance = TestScript()
|
||||
|
||||
# Make ScriptModel.objects.get() return a stub whose python_class() yields our instance
|
||||
script_model_stub = MagicMock()
|
||||
script_model_stub.python_class.return_value = script_instance
|
||||
|
||||
runner = _make_runner(object_id=1)
|
||||
with patch.object(ScriptModel.objects, 'get', return_value=script_model_stub):
|
||||
# Pass nested payload shape like the REST API: {"data": {"site": <pk>}}
|
||||
runner.run(data={'data': {'site': site.pk}}, request=None, commit=True)
|
||||
|
||||
# Assert the script received a model instance for 'site'
|
||||
self.assertIsNotNone(script_instance.received)
|
||||
self.assertIn('site', script_instance.received)
|
||||
self.assertIsInstance(script_instance.received['site'], Site)
|
||||
self.assertNotIn('_commit', script_instance.received)
|
||||
|
||||
def test_run_merges_request_files_into_data_for_real_script(self):
|
||||
# Create object
|
||||
site = Site.objects.create(name="Test Site 2", slug="test-site-2")
|
||||
|
||||
class TestScript(Script):
|
||||
site = ObjectVar(label='Site', model=Site)
|
||||
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.received = None
|
||||
|
||||
def run(self, data, commit=True):
|
||||
self.received = data
|
||||
actual_type = type(data['site'])
|
||||
return f"got type: {actual_type}"
|
||||
|
||||
script_instance = TestScript()
|
||||
script_model_stub = MagicMock()
|
||||
script_model_stub.python_class.return_value = script_instance
|
||||
|
||||
# Simulate a request with uploaded files
|
||||
fake_request = MagicMock()
|
||||
fake_request.FILES = {'upload': 'fileobj'}
|
||||
fake_request.id = None
|
||||
|
||||
runner = _make_runner(object_id=1)
|
||||
with patch.object(ScriptModel.objects, 'get', return_value=script_model_stub):
|
||||
runner.run(data={'data': {'site': site.pk}}, request=fake_request, commit=True)
|
||||
|
||||
# Assert both file merged and ObjectVar conversion happened
|
||||
self.assertIsNotNone(script_instance.received)
|
||||
self.assertIn('upload', script_instance.received)
|
||||
self.assertEqual(script_instance.received['upload'], 'fileobj')
|
||||
self.assertIsInstance(script_instance.received['site'], Site)
|
||||
|
|
|
|||
Loading…
Reference in New Issue