#558 fixed storing/executing multiselect values in scheduled scripts

src/execution/execution_service.py

@@ -43,7 +43,10 @@ def get_active_executor(self, execution_id, user):
     def start_script(self, config, values, user: User):
         audit_name = user.get_audit_name()
 
-        executor = ScriptExecutor(config, values)
+        config.set_all_param_values(values)
+        normalized_values = dict(config.parameter_values)
+
+        executor = ScriptExecutor(config, normalized_values)
         execution_id = self._id_generator.next_id()
 
         audit_command = executor.get_secure_command()

src/model/parameter_config.py

@@ -284,9 +284,11 @@ def validate_value(self, value, *, ignore_required=False):
         value_string = self.value_to_repr(value)
 
         if self.no_value:
-            if value not in ['true', True, 'false', False]:
-                return 'should be boolean, but has value ' + value_string
-            return None
+            if isinstance(value, bool):
+                return None
+            if isinstance(value, str) and value.lower() in ['true', 'false']:
+                return None
+            return 'should be boolean, but has value ' + value_string
 
         if self.type == 'text':
             if (not is_empty(self.max_length)) and (len(value) > int(self.max_length)):

src/scheduling/schedule_service.py

@@ -84,7 +84,8 @@ def create_job(self, script_name, parameter_values, incoming_schedule_config, us
 
         id = self._id_generator.next_id()
 
-        job = SchedulingJob(id, user, schedule_config, script_name, parameter_values)
+        normalized_values = dict(config_model.parameter_values)
+        job = SchedulingJob(id, user, schedule_config, script_name, normalized_values)
 
         self.save_job(job)
 

src/tests/scheduling/schedule_service_test.py

@@ -7,11 +7,13 @@
 from unittest.mock import patch, ANY, MagicMock
 
 from auth.user import User
+from config.config_service import ConfigService
 from scheduling import schedule_service
 from scheduling.schedule_config import ScheduleConfig, InvalidScheduleException
 from scheduling.schedule_service import ScheduleService, InvalidUserException, UnavailableScriptException
 from scheduling.scheduling_job import SchedulingJob
 from tests import test_utils
+from tests.test_utils import AnyUserAuthorizer
 from utils import date_utils, audit_utils, file_utils
 
 mocked_now = date_utils.parse_iso_datetime('2020-07-24T12:30:59.000000Z')
@@ -40,13 +42,14 @@ def assert_schedule_calls(self, expected_job_time_pairs):
                                  schedule_method_job_arg.as_serializable_dict())
 
     def mock_schedule_model_with_secure_param(self):
-        model = test_utils.create_config_model('some-name', parameters=[{'name': 'p1', 'secure': True}])
-        model.schedulable = True
-
-        self.config_service.load_config_model.side_effect = lambda a, b, c: model
+        self.create_config('secure-config', parameters=[
+            {'name': 'p1', 'secure': True},
+            {'name': 'param_2', 'type': 'multiselect', 'values': ['hello', 'world', '1', '2', '3']},
+        ])
 
     def setUp(self) -> None:
         super().setUp()
+        test_utils.setup()
 
         self.patcher = patch('sched.scheduler')
         self.scheduler_mock = MagicMock()
@@ -55,19 +58,32 @@ def setUp(self) -> None:
         schedule_service._sleep = MagicMock()
         schedule_service._sleep.side_effect = lambda x: time.sleep(0.001)
 
-        self.unschedulable_scripts = set()
-        self.config_service = MagicMock()
-        self.config_service.load_config_model.side_effect = lambda name, b, c: test_utils.create_config_model(
-            name,
-            schedulable=name not in self.unschedulable_scripts)
+        self.config_service = ConfigService(AnyUserAuthorizer(), test_utils.temp_folder)
+
+        self.create_config('my_script_A')
+        self.create_config('unschedulable-script', scheduling_enabled=False)
 
         self.execution_service = MagicMock()
 
         self.schedule_service = ScheduleService(self.config_service, self.execution_service, test_utils.temp_folder)
 
         date_utils._mocked_now = mocked_now
 
-        test_utils.setup()
+    def create_config(self, name, scheduling_enabled=True, parameters=None):
+        if parameters is None:
+            parameters = [
+                {'name': 'p1'},
+                {'name': 'param_2', 'type': 'multiselect', 'values': ['hello', 'world', '1', '2', '3']},
+            ]
+
+        test_utils.write_script_config(
+            {
+                'name': name,
+                'script_path': 'echo 1',
+                'parameters': parameters,
+                'scheduling': {'enabled': scheduling_enabled}
+            },
+            name)
 
     def tearDown(self) -> None:
         super().tearDown()
@@ -98,11 +114,14 @@ def test_create_job_when_multiple(self):
         jobs = []
 
         for i in range(1, 3):
+            script_name = 'script-' + str(i)
+            self.create_config(script_name)
+
             job_prototype = create_job(
                 user_id='user-' + str(i),
-                script_name='script-' + str(i),
+                script_name=script_name,
                 repeatable=i % 2 == 1,
-                parameter_values={'p1': 'hi', 'p2': i})
+                parameter_values={'p1': 'hi', 'param_2': [str(i)]})
             job_id = self.call_create_job(job_prototype)
 
             self.assertEqual(str(i), job_id)
@@ -121,8 +140,7 @@ def test_create_job_when_user_none(self):
             'abc', {}, {}, None)
 
     def test_create_job_when_not_schedulable(self):
-        job_prototype = create_job()
-        self.unschedulable_scripts.add('my_script_A')
+        job_prototype = create_job(script_name='unschedulable-script')
 
         self.assertRaisesRegex(
             UnavailableScriptException,
@@ -133,11 +151,11 @@ def test_create_job_when_not_schedulable(self):
     def test_create_job_when_secure(self):
         self.mock_schedule_model_with_secure_param()
 
-        job_prototype = create_job()
+        job_prototype = create_job(script_name='secure-config')
 
         self.assertRaisesRegex(
             UnavailableScriptException,
-            'Script contains secure parameters',
+            'secure-config is not schedulable',
             self.call_create_job,
             job_prototype)
 
@@ -306,13 +324,12 @@ def test_execute_when_fails(self):
 
     def test_execute_when_not_schedulable(self):
         job = create_job(id=1,
+                         script_name='unschedulable-script',
                          repeatable=True,
                          start_datetime=mocked_now - timedelta(seconds=1),
                          repeat_unit='days',
                          repeat_period=1)
 
-        self.unschedulable_scripts.add(job.script_name)
-
         self.schedule_service._execute_job(job)
 
         self.execution_service.start_script.assert_not_called()

src/web/server.py

@@ -380,25 +380,22 @@ def post(self, user):
                 for key, value in self.form_reader.files.items():
                     parameter_values[key] = value.path
 
-            try:
-                config_model.set_all_param_values(parameter_values)
-                normalized_values = dict(config_model.parameter_values)
-            except InvalidValueException as e:
-                message = 'Invalid parameter %s value: %s' % (e.param_name, str(e))
-                LOGGER.error(message)
-                respond_error(self, 400, message)
-                return
-
             all_audit_names = user.audit_names
             LOGGER.info('Calling script %s. User %s', script_name, all_audit_names)
 
             execution_id = self.application.execution_service.start_script(
                 config_model,
-                normalized_values,
+                parameter_values,
                 user)
 
             self.write(str(execution_id))
 
+        except InvalidValueException as e:
+            message = 'Invalid parameter %s value: %s' % (e.param_name, str(e))
+            LOGGER.error(message)
+            respond_error(self, 422, message)
+            return
+
         except ConfigNotAllowedException:
             LOGGER.warning('Access to the script "' + script_name + '" is denied for ' + audit_name)
             respond_error(self, 403, 'Access to the script is denied')