big revamp on xfconf, adding array values (#693)

* big revamp on xfconf, adding array values

* added changelog fragment

* Update changelogs/fragments/693-big-revamp-on-xfconf-adding-array-values.yml

Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>

Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>

changelogs/fragments/693-big-revamp-on-xfconf-adding-array-values.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- xfconf - add arrays support (https://github.com/ansible/ansible/issues/46308).

plugins/modules/system/xfconf.py

@@ -12,6 +12,7 @@ DOCUMENTATION = '''
 module: xfconf
 author:
     - "Joseph Benden (@jbenden)"
+    - "Alexei Znamensky (@russoz)"
 short_description: Edit XFCE4 Configurations
 description:
   - This module allows for the manipulation of Xfce 4 Configuration via
@@ -23,37 +24,63 @@ options:
       Xfconf repository that corresponds to the location for which all
       application properties/keys are stored. See man xfconf-query(1)
     required: yes
+    type: str
   property:
     description:
     - A Xfce preference key is an element in the Xfconf repository
       that corresponds to an application preference. See man xfconf-query(1)
     required: yes
+    type: str
   value:
     description:
     - Preference properties typically have simple values such as strings,
       integers, or lists of strings and integers. This is ignored if the state
-      is "get". See man xfconf-query(1)
+      is "get". For array mode, use a list of values. See man xfconf-query(1)
+    type: list
+    elements: raw
   value_type:
     description:
     - The type of value being set. This is ignored if the state is "get".
+      For array mode, use a list of types.
+    type: list
+    elements: str
     choices: [ int, bool, float, string ]
   state:
     description:
     - The action to take upon the property/value.
     choices: [ get, present, absent ]
     default: "present"
+  force_array:
+    description:
+    - Force array even if only one element
+    type: bool
+    default: 'no'
+    aliases: ['array']
+    version_added: 1.0.0
 '''
 
 EXAMPLES = """
 - name: Change the DPI to "192"
-  community.general.xfconf:
+  xfconf:
     channel: "xsettings"
     property: "/Xft/DPI"
     value_type: "int"
     value: "192"
-  become: True
-  become_user: johnsmith
 
+- name: Set workspace names (4)
+  xfconf:
+    channel: xfwm4
+    property: /general/workspace_names
+    value_type: string
+    value: ['Main', 'Work1', 'Work2', 'Tmp']
+
+- name: Set workspace names (1)
+  xfconf:
+    channel: xfwm4
+    property: /general/workspace_names
+    value_type: string
+    value: ['Main']
+    force_array: yes
 """
 
 RETURN = '''
@@ -77,132 +104,200 @@ RETURN = '''
     returned: success
     type: str
     sample: "192"
-...
+  previous_value:
+    description: The value of the preference key before executing the module (None for "get" state).
+    returned: success
+    type: str
+    sample: "96"
 '''
 
-import sys
-
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.six.moves import shlex_quote
 
 
-class XfconfPreference(object):
+class XfConfException(Exception):
-    def __init__(self, module, channel, property, value_type, value):
+    pass
+
+
+class XfConfProperty(object):
+    SET = "present"
+    GET = "get"
+    RESET = "absent"
+    VALID_STATES = (SET, GET, RESET)
+    VALID_VALUE_TYPES = ('int', 'bool', 'float', 'string')
+    previous_value = None
+    is_array = None
+
+    def __init__(self, module):
         self.module = module
-        self.channel = channel
+        self.channel = module.params['channel']
-        self.property = property
+        self.property = module.params['property']
-        self.value_type = value_type
+        self.value_type = module.params['value_type']
-        self.value = value
+        self.value = module.params['value']
+        self.force_array = module.params['force_array']
 
-    def call(self, call_type, fail_onerr=True):
+        self.cmd = "{0} --channel {1} --property {2}".format(
-        """ Helper function to perform xfconf-query operations """
+            module.get_bin_path('xfconf-query', True),
-        changed = False
-        out = ''
-
-        # Execute the call
-        cmd = "{0} --channel {1} --property {2}".format(self.module.get_bin_path('xfconf-query', True),
             shlex_quote(self.channel),
-                                                        shlex_quote(self.property))
+            shlex_quote(self.property)
+        )
+        self.method_map = dict(zip((self.SET, self.GET, self.RESET),
+                                   (self.set, self.get, self.reset)))
+
+        # @TODO This will not work with non-English translations, but xfconf-query does not return
+        #       distinct result codes for distinct outcomes.
+        self.does_not = 'Property "{0}" does not exist on channel "{1}".'.format(self.property, self.channel)
+
+        def run(cmd):
+            return module.run_command(cmd, check_rc=False)
+        self._run = run
+
+    def _execute_xfconf_query(self, args=None):
         try:
-            if call_type == 'set':
+            cmd = self.cmd
-                cmd += " --type {0} --create --set {1}".format(shlex_quote(self.value_type),
+            if args:
-                                                               shlex_quote(self.value))
+                cmd = "{0} {1}".format(cmd, args)
-            elif call_type == 'unset':
-                cmd += " --reset"
 
-            # Start external command
+            self.module.debug("Running cmd={0}".format(cmd))
-            rc, out, err = self.module.run_command(cmd, check_rc=False)
+            rc, out, err = self._run(cmd)
+            if err.rstrip() == self.does_not:
+                return None
+            if rc or len(err):
+                raise XfConfException('xfconf-query failed with error (rc={0}): {1}'.format(rc, err))
 
-            if rc != 0 or len(err) > 0:
+            return out.rstrip()
-                if fail_onerr:
-                    self.module.fail_json(msg='xfconf-query failed with error: %s' % (str(err)))
-            else:
-                changed = True
 
         except OSError as exception:
-            self.module.fail_json(msg='xfconf-query failed with exception: %s' % exception)
+            XfConfException('xfconf-query failed with exception: {0}'.format(exception))
-        return changed, out.rstrip()
+
+    def get(self):
+        previous_value = self._execute_xfconf_query()
+        if previous_value is None:
+            return
+
+        if "Value is an array with" in previous_value:
+            previous_value = previous_value.split("\n")
+            previous_value.pop(0)
+            previous_value.pop(0)
+
+        return previous_value
+
+    def reset(self):
+        self._execute_xfconf_query("--reset")
+        return None
+
+    @staticmethod
+    def _fix_bool(value):
+        if value.lower() in ("true", "false"):
+            value = value.lower()
+        return value
+
+    def _make_value_args(self, value, value_type):
+        if value_type == 'bool':
+            value = self._fix_bool(value)
+        return " --type '{1}' --set '{0}'".format(shlex_quote(value), shlex_quote(value_type))
+
+    def set(self):
+        args = "--create"
+        if self.is_array:
+            args += " --force-array"
+            for v in zip(self.value, self.value_type):
+                args += self._make_value_args(*v)
+        else:
+            args += self._make_value_args(self.value, self.value_type)
+        self._execute_xfconf_query(args)
+        return self.value
+
+    def call(self, state):
+        return self.method_map[state]()
+
+    def sanitize(self):
+        self.previous_value = self.get()
+
+        if self.value is None and self.value_type is None:
+            return
+        if (self.value is None) ^ (self.value_type is None):
+            raise XfConfException('Must set both "value" and "value_type"')
+
+        # stringify all values - in the CLI they will all be happy strings anyway
+        # and by doing this here the rest of the code can be agnostic to it
+        self.value = [str(v) for v in self.value]
+
+        values_len = len(self.value)
+        types_len = len(self.value_type)
+
+        if types_len == 1:
+            # use one single type for the entire list
+            self.value_type = self.value_type * values_len
+        elif types_len != values_len:
+            # or complain if lists' lengths are different
+            raise XfConfException('Same number of "value" and "value_type" needed')
+
+        # fix boolean values
+        self.value = [self._fix_bool(v[0]) if v[1] == 'bool' else v[0] for v in zip(self.value, self.value_type)]
+
+        # calculates if it is an array
+        self.is_array = self.force_array or isinstance(self.previous_value, list) or values_len > 1
+        if not self.is_array:
+            self.value = self.value[0]
+            self.value_type = self.value_type[0]
 
 
 def main():
+    facts_name = "xfconf"
     # Setup the Ansible module
     module = AnsibleModule(
         argument_spec=dict(
+            state=dict(default=XfConfProperty.SET,
+                       choices=XfConfProperty.VALID_STATES,
+                       type='str'),
             channel=dict(required=True, type='str'),
             property=dict(required=True, type='str'),
-            value_type=dict(required=False,
+            value_type=dict(required=False, type='list',
-                            choices=['int', 'bool', 'float', 'string'],
+                            elements='str', choices=XfConfProperty.VALID_VALUE_TYPES),
-                            type='str'),
+            value=dict(required=False, type='list', elements='raw'),
-            value=dict(required=False, default=None, type='str'),
+            force_array=dict(default=False, type='bool', aliases=['array']),
-            state=dict(default='present',
-                       choices=['present', 'get', 'absent'],
-                       type='str')
         ),
+        required_if=[
+            ('state', XfConfProperty.SET, ['value', 'value_type'])
+        ],
         supports_check_mode=True
     )
 
-    state_values = {"present": "set", "absent": "unset", "get": "get"}
+    state = module.params['state']
-
-    # Assign module values to dictionary values
-    channel = module.params['channel']
-    property = module.params['property']
-    value_type = module.params['value_type']
-    if module.params['value'].lower() == "true":
-        value = "true"
-    elif module.params['value'] == "false":
-        value = "false"
-    else:
-        value = module.params['value']
-
-    state = state_values[module.params['state']]
-
-    # Initialize some variables for later
-    change = False
-    new_value = ''
-
-    if state != "get":
-        if value is None or value == "":
-            module.fail_json(msg='State %s requires "value" to be set'
-                             % str(state))
-        elif value_type is None or value_type == "":
-            module.fail_json(msg='State %s requires "value_type" to be set'
-                             % str(state))
 
+    try:
         # Create a Xfconf preference
-    xfconf = XfconfPreference(module,
+        xfconf = XfConfProperty(module)
-                              channel,
+        xfconf.sanitize()
-                              property,
+
-                              value_type,
+        previous_value = xfconf.get()
-                              value)
+        facts = {
-    # Now we get the current value, if not found don't fail
+            facts_name: dict(
-    dummy, current_value = xfconf.call("get", fail_onerr=False)
+                channel=xfconf.channel,
+                property=xfconf.property,
+                value_type=xfconf.value_type,
+                value=previous_value,
+            )
+        }
+
+        if state == XfConfProperty.GET \
+                or (previous_value is not None
+                    and (state, set(previous_value)) == (XfConfProperty.SET, set(xfconf.value))):
+            module.exit_json(changed=False, ansible_facts=facts)
+            return
 
-    # Check if the current value equals the value we want to set.  If not, make
-    # a change
-    if current_value != value:
         # If check mode, we know a change would have occurred.
         if module.check_mode:
-            # So we will set the change to True
+            new_value = xfconf.value
-            change = True
-            # And set the new_value to the value that would have been set
-            new_value = value
-        # If not check mode make the change.
         else:
-            change, new_value = xfconf.call(state)
+            new_value = xfconf.call(state)
-    # If the value we want to set is the same as the current_value, we will
-    # set the new_value to the current_value for reporting
-    else:
-        new_value = current_value
 
-    facts = dict(xfconf={'changed': change,
+        facts[facts_name].update(value=new_value, previous_value=previous_value)
-                         'channel': channel,
+        module.exit_json(changed=True, ansible_facts=facts)
-                         'property': property,
-                         'value_type': value_type,
-                         'new_value': new_value,
-                         'previous_value': current_value,
-                         'playbook_value': module.params['value']})
 
-    module.exit_json(changed=change, ansible_facts=facts)
+    except Exception as e:
+        module.fail_json(msg="Failed with exception: {0}".format(e))
 
 
 if __name__ == '__main__':