Merge pull request #3909 from StackStorm/disable-dict-conversion-client

Add flag to opt-in to dict conversion when colons are detected

st2client/st2client/commands/action.py

@@ -303,6 +303,15 @@ def _add_common_options(self):
         detail_arg_grp.add_argument('--tail', action='store_true',
                                     help='Automatically start tailing new execution.')
 
+        # Flag to opt-in to functionality introduced in PR #3670. More robust parsing
+        # of complex datatypes is planned for 2.6, so this flag will be deprecated soon
+        detail_arg_grp.add_argument('--auto-dict', action='store_true', dest='auto_dict',
+                                    default=False, help='Automatically convert list items to '
+                                    'dictionaries when colons are detected. '
+                                    '(NOTE - this parameter and its functionality will be '
+                                    'deprecated in the next release in favor of a more '
+                                    'robust conversion method)')
+
         return root_arg_grp
 
     def _print_execution_details(self, execution, args, **kwargs):
@@ -543,7 +552,7 @@ def transform_object(value):
                     result[key] = value
             return result
 
-        def transform_array(value, action_params=None):
+        def transform_array(value, action_params=None, auto_dict=False):
             action_params = action_params or {}
 
             # Sometimes an array parameter only has a single element:
@@ -575,13 +584,14 @@ def transform_array(value, action_params=None):
 
             # When each values in this array represent dict type, this converts
             # the 'result' to the dict type value.
-            if all([isinstance(x, str) and ':' in x for x in result]):
+            if all([isinstance(x, str) and ':' in x for x in result]) and auto_dict:
                 result_dict = {}
                 for (k, v) in [x.split(':') for x in result]:
                     # To parse values using the 'transformer' according to the type which is
                     # specified in the action metadata, calling 'normalize' method recursively.
                     if 'properties' in action_params and k in action_params['properties']:
-                        result_dict[k] = normalize(k, v, action_params['properties'])
+                        result_dict[k] = normalize(k, v, action_params['properties'],
+                                                   auto_dict=auto_dict)
                     else:
                         result_dict[k] = v
                 return [result_dict]
@@ -611,7 +621,7 @@ def get_param_type(key, action_params=None):
 
             return None
 
-        def normalize(name, value, action_params=None):
+        def normalize(name, value, action_params=None, auto_dict=False):
             """ The desired type is contained in the action meta-data, so we can look that up
                 and call the desired "caster" function listed in the "transformer" dict
             """
@@ -629,7 +639,7 @@ def normalize(name, value, action_params=None):
             # also leverage that to cast each array item to the correct type.
             param_type = get_param_type(name, action_params)
             if param_type == 'array' and name in action_params:
-                return transformer[param_type](value, action_params[name])
+                return transformer[param_type](value, action_params[name], auto_dict=auto_dict)
             elif param_type:
                 return transformer[param_type](value)
 
@@ -669,9 +679,9 @@ def normalize(name, value, action_params=None):
                     else:
                         # This permits multiple declarations of argument only in the array type.
                         if get_param_type(k) == 'array' and k in result:
-                            result[k] += normalize(k, v)
+                            result[k] += normalize(k, v, auto_dict=args.auto_dict)
                         else:
-                            result[k] = normalize(k, v)
+                            result[k] = normalize(k, v, auto_dict=args.auto_dict)
 
                 except Exception as e:
                     # TODO: Move transformers in a separate module and handle

st2client/tests/unit/test_action.py

@@ -377,6 +377,60 @@ def test_param_array_object_conversion(self):
         }
         httpclient.HTTPClient.post.assert_called_with('/executions', expected)
 
+    @mock.patch.object(
+        models.ResourceManager, 'get_by_ref_or_id',
+        mock.MagicMock(side_effect=get_by_ref))
+    @mock.patch.object(
+        models.ResourceManager, 'get_by_name',
+        mock.MagicMock(side_effect=get_by_name))
+    @mock.patch.object(
+        httpclient.HTTPClient, 'post',
+        mock.MagicMock(return_value=base.FakeResponse(json.dumps(LIVE_ACTION), 200, 'OK')))
+    def test_param_dict_conversion_flag(self):
+        """Ensure that the automatic conversion to dict based on colons only occurs with the flag
+        """
+
+        self.shell.run(
+            [
+                'run',
+                'mockety.mock2',
+                'list=key1:value1,key2:value2',
+                '--auto-dict'
+            ]
+        )
+        expected = {
+            'action': 'mockety.mock2',
+            'user': None,
+            'parameters': {
+                'list': [
+                    {
+                        'key1': 'value1',
+                        'key2': 'value2'
+                    }
+                ]
+            }
+        }
+        httpclient.HTTPClient.post.assert_called_with('/executions', expected)
+
+        self.shell.run(
+            [
+                'run',
+                'mockety.mock2',
+                'list=key1:value1,key2:value2'
+            ]
+        )
+        expected = {
+            'action': 'mockety.mock2',
+            'user': None,
+            'parameters': {
+                'list': [
+                    'key1:value1',
+                    'key2:value2'
+                ]
+            }
+        }
+        httpclient.HTTPClient.post.assert_called_with('/executions', expected)
+
     @mock.patch.object(
         models.ResourceManager, 'get_by_ref_or_id',
         mock.MagicMock(side_effect=get_by_ref))

st2client/tests/unit/test_command_actionrun.py

@@ -57,7 +57,47 @@ def test_get_params_types(self):
         self.assertEqual(runner.runner_parameters, orig_runner_params, 'Runner params modified.')
         self.assertEqual(action.parameters, orig_action_params, 'Action params modified.')
 
+    def test_opt_in_dict_auto_convert(self):
+        """Test ability for user to opt-in to dict convert functionality
+        """
+
+        runner = RunnerType()
+        runner.runner_parameters = {}
+
+        action = Action()
+        action.ref = 'test.action'
+        action.parameters = {
+            'param_array': {'type': 'array'},
+        }
+
+        subparser = mock.Mock()
+        command = ActionRunCommand(action, self, subparser, name='test')
+
+        mockarg = mock.Mock()
+        mockarg.inherit_env = False
+        mockarg.parameters = [
+            'param_array=foo:bar,foo2:bar2',
+        ]
+
+        mockarg.auto_dict = False
+        param = command._get_action_parameters_from_args(action=action, runner=runner, args=mockarg)
+        self.assertEqual(param['param_array'], ['foo:bar', 'foo2:bar2'])
+
+        mockarg.auto_dict = True
+        param = command._get_action_parameters_from_args(action=action, runner=runner, args=mockarg)
+        self.assertEqual(param['param_array'], [{'foo': 'bar', 'foo2': 'bar2'}])
+
+        # set auto_dict back to default
+        mockarg.auto_dict = False
+
     def test_get_params_from_args(self):
+        """test_get_params_from_args
+
+        This tests the details of the auto-dict conversion, assuming it's enabled. Please
+        see test_opt_in_dict_auto_convert for a test of detecting whether or not this
+        functionality is enabled.
+        """
+
         runner = RunnerType()
         runner.runner_parameters = {}
 
@@ -84,6 +124,7 @@ def test_get_params_from_args(self):
 
         mockarg = mock.Mock()
         mockarg.inherit_env = False
+        mockarg.auto_dict = True
         mockarg.parameters = [
             'param_string=hoge',
             'param_integer=123',
@@ -116,7 +157,17 @@ def test_get_params_from_args(self):
             self.assertTrue(isinstance(param['qux'], dict))
             self.assertTrue(isinstance(param['quux'], bool))
 
+        # set auto_dict back to default
+        mockarg.auto_dict = False
+
     def test_get_params_from_args_with_multiple_declarations(self):
+        """test_get_params_from_args_with_multiple_declarations
+
+        This tests the details of the auto-dict conversion, assuming it's enabled. Please
+        see test_opt_in_dict_auto_convert for a test of detecting whether or not this
+        functionality is enabled.
+        """
+
         runner = RunnerType()
         runner.runner_parameters = {}
 
@@ -133,6 +184,7 @@ def test_get_params_from_args_with_multiple_declarations(self):
 
         mockarg = mock.Mock()
         mockarg.inherit_env = False
+        mockarg.auto_dict = True
         mockarg.parameters = [
             'param_string=hoge',  # This value will be overwritten with the next declaration.
             'param_string=fuga',
@@ -151,3 +203,6 @@ def test_get_params_from_args_with_multiple_declarations(self):
             {'foo': '1', 'bar': '2'},
             {'hoge': 'A', 'fuga': 'B'}
         ])
+
+        # set auto_dict back to default
+        mockarg.auto_dict = False