Improved ModuleHelper.run_command() (#1867) (#1938)

* Improved run_command signature and behaviour

- extra_params has been removed from the signature
- params now can be either str or dict (containing the param value)

* Reverted the removal of the method parameter, and added changelog fragment

* Update changelogs/fragments/1867-modhelper-cmdmixin-dict-params.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/module_utils/module_helper.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* adjustement per PR

* Update plugins/module_utils/module_helper.py

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 585dd0b6ed)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

changelogs/fragments/1867-modhelper-cmdmixin-dict-params.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - module_helper module utils - ``CmdMixin.run_command()`` now accepts ``dict`` command arguments, providing the parameter and its value (https://github.com/ansible-collections/community.general/pull/1867).

plugins/module_utils/module_helper.py

@@ -296,17 +296,28 @@ class CmdMixin(object):
         param_list = params if params else self.module.params.keys()
 
         for param in param_list:
-            if param in self.module.argument_spec:
+            if isinstance(param, dict):
-                if param not in self.module.params:
+                if len(param) != 1:
+                    raise ModuleHelperException("run_command parameter as a dict must "
+                                                "contain only one key: {0}".format(param))
+                _param = list(param.keys())[0]
+                fmt = find_format(_param)
+                value = param[_param]
+            elif isinstance(param, str):
+                if param in self.module.argument_spec:
+                    fmt = find_format(param)
+                    value = self.module.params[param]
+                elif param in extra_params:
+                    fmt = find_format(param)
+                    value = extra_params[param]
+                else:
+                    self.module.deprecate("Cannot determine value for parameter: {0}. "
+                                          "From version 4.0.0 onwards this will generate an exception".format(param),
+                                          version="4.0.0", collection_name="community.general")
                     continue
-                fmt = find_format(param)
+
-                value = self.module.params[param]
             else:
-                if param not in extra_params:
+                raise ModuleHelperException("run_command parameter must be either a str or a dict: {0}".format(param))
-                    continue
-                fmt = find_format(param)
-                value = extra_params[param]
-            self.cmd_args = cmd_args
             cmd_args = add_arg_formatted_param(cmd_args, fmt, value)
 
         return cmd_args

plugins/modules/system/xfconf.py

@@ -238,7 +238,7 @@ class XFConfProperty(CmdMixin, StateMixin, ModuleHelper):
 
     def state_absent(self):
         self.vars.value = None
-        self.run_command(params=('channel', 'property', 'reset'), extra_params={"reset": True})
+        self.run_command(params=('channel', 'property', {'reset': True}))
         self.update_xfconf_output(previous_value=self.vars.previous_value,
                                   value=None)
 
@@ -267,17 +267,13 @@ class XFConfProperty(CmdMixin, StateMixin, ModuleHelper):
             isinstance(self.vars.previous_value, list) or \
             values_len > 1
 
-        params = ['channel', 'property', 'create']
+        params = ['channel', 'property', {'create': True}]
         if self.vars.is_array:
-            params.append('is_array')
+            params.append({'is_array': True})
-        params.append('values_and_types')
+        params.append({'values_and_types': (self.vars.value, value_type)})
-
-        extra_params = dict(values_and_types=(self.vars.value, value_type))
-        extra_params['create'] = True
-        extra_params['is_array'] = self.vars.is_array
 
         if not self.module.check_mode:
-            self.run_command(params=params, extra_params=extra_params)
+            self.run_command(params=params)
 
         if not self.vars.is_array:
             self.vars.value = self.vars.value[0]