From ca388f064a550ac1ef965adb6bf2a22188b3f403 Mon Sep 17 00:00:00 2001
From: Alexei Znamensky <103110+russoz@users.noreply.github.com>
Date: Mon, 8 May 2023 06:22:53 +1200
Subject: [PATCH] snap: now using CmdRunner (#6468)

* snap: now using CmdRunner

* add changelog frag

* minor adjustment + comment

* simplified args for cmdrunner when retrieving snap options

* Update changelogs/fragments/6468-snap-cmdrunner.yml
---
 changelogs/fragments/6468-snap-cmdrunner.yml |   2 +
 plugins/module_utils/snap.py                 |  20 +++-
 plugins/modules/snap.py                      | 100 ++++++++-----------
 3 files changed, 63 insertions(+), 59 deletions(-)
 create mode 100644 changelogs/fragments/6468-snap-cmdrunner.yml

diff --git a/changelogs/fragments/6468-snap-cmdrunner.yml b/changelogs/fragments/6468-snap-cmdrunner.yml
new file mode 100644
index 0000000000..e5247ffbd0
--- /dev/null
+++ b/changelogs/fragments/6468-snap-cmdrunner.yml
@@ -0,0 +1,2 @@
+minor_changes:
+  - snap - refactor module to use ``CmdRunner`` to execute external commands (https://github.com/ansible-collections/community.general/pull/6468).
diff --git a/plugins/module_utils/snap.py b/plugins/module_utils/snap.py
index 1362b697af..3ae126dbfd 100644
--- a/plugins/module_utils/snap.py
+++ b/plugins/module_utils/snap.py
@@ -15,15 +15,29 @@ _alias_state_map = dict(
     info='aliases',
 )
 
+_state_map = dict(
+    present='install',
+    absent='remove',
+    enabled='enable',
+    disabled='disable',
+)
+
 
 def snap_runner(module, **kwargs):
     runner = CmdRunner(
         module,
-        module.get_bin_path("snap"),
+        "snap",
         arg_formats=dict(
-            state_alias=cmd_runner_fmt.as_map(_alias_state_map),
+            state_alias=cmd_runner_fmt.as_map(_alias_state_map),  # snap_alias only
             name=cmd_runner_fmt.as_list(),
-            alias=cmd_runner_fmt.as_list(),
+            alias=cmd_runner_fmt.as_list(),                       # snap_alias only
+            state=cmd_runner_fmt.as_map(_state_map),
+            _list=cmd_runner_fmt.as_fixed("list"),
+            _set=cmd_runner_fmt.as_fixed("set"),
+            get=cmd_runner_fmt.as_fixed(["get", "-d"]),
+            classic=cmd_runner_fmt.as_bool("--classic"),
+            channel=cmd_runner_fmt.as_func(lambda v: [] if v == 'stable' else ['--channel', '{0}'.format(v)]),
+            options=cmd_runner_fmt.as_list(),
         ),
         check_rc=False,
         **kwargs
diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py
index 4b798d6e2c..359b7fa323 100644
--- a/plugins/modules/snap.py
+++ b/plugins/modules/snap.py
@@ -154,28 +154,11 @@ import numbers
 
 from ansible.module_utils.common.text.converters import to_native
 
-from ansible_collections.community.general.plugins.module_utils.module_helper import (
-    CmdStateModuleHelper, ArgFormat
-)
+from ansible_collections.community.general.plugins.module_utils.module_helper import StateModuleHelper
+from ansible_collections.community.general.plugins.module_utils.snap import snap_runner
 
 
-__state_map = dict(
-    present='install',
-    absent='remove',
-    enabled='enable',
-    disabled='disable',
-    info='info',  # not public
-    list='list',  # not public
-    set='set',  # not public
-    get='get',  # not public
-)
-
-
-def _state_map(value):
-    return [__state_map[value]]
-
-
-class Snap(CmdStateModuleHelper):
+class Snap(StateModuleHelper):
     __disable_re = re.compile(r'(?:\S+\s+){5}(?P<notes>\S+)')
     __set_param_re = re.compile(r'(?P<snap_prefix>\S+:)?(?P<key>\S+)\s*=\s*(?P<value>.+)')
     module = dict(
@@ -189,16 +172,6 @@ class Snap(CmdStateModuleHelper):
         },
         supports_check_mode=True,
     )
-    command = "snap"
-    command_args_formats = dict(
-        actionable_snaps=dict(fmt=lambda v: v),
-        state=dict(fmt=_state_map),
-        classic=dict(fmt="--classic", style=ArgFormat.BOOLEAN),
-        channel=dict(fmt=lambda v: [] if v == 'stable' else ['--channel', '{0}'.format(v)]),
-        options=dict(fmt=list),
-        json_format=dict(fmt="-d", style=ArgFormat.BOOLEAN),
-    )
-    check_rc = False
 
     @staticmethod
     def _first_non_zero(a):
@@ -208,18 +181,35 @@ class Snap(CmdStateModuleHelper):
 
         return 0
 
-    def _run_multiple_commands(self, commands):
-        outputs = [(c,) + self.run_command(params=c) for c in commands]
-        results = ([], [], [], [])
-        for output in outputs:
-            for i in range(4):
-                results[i].append(output[i])
+    def __init_module__(self):
+        self.runner = snap_runner(self.module)
+
+    def _run_multiple_commands(self, commands, actionable_names, bundle=True):
+        results_cmd = []
+        results_rc = []
+        results_out = []
+        results_err = []
+
+        with self.runner(commands + ["name"]) as ctx:
+            if bundle:
+                rc, out, err = ctx.run(name=actionable_names)
+                results_cmd.append(commands + actionable_names)
+                results_rc.append(rc)
+                results_out.append(out)
+                results_err.append(err)
+            else:
+                for name in actionable_names:
+                    rc, out, err = ctx.run(name=name)
+                    results_cmd.append(commands + [name])
+                    results_rc.append(rc)
+                    results_out.append(out)
+                    results_err.append(err)
 
         return [
-            '; '.join([to_native(x) for x in results[0]]),
-            self._first_non_zero(results[1]),
-            '\n'.join(results[2]),
-            '\n'.join(results[3]),
+            '; '.join([to_native(x) for x in results_cmd]),
+            self._first_non_zero(results_rc),
+            '\n'.join(results_out),
+            '\n'.join(results_err),
         ]
 
     def convert_json_subtree_to_map(self, json_subtree, prefix=None):
@@ -245,8 +235,8 @@ class Snap(CmdStateModuleHelper):
         return self.convert_json_subtree_to_map(json_object)
 
     def retrieve_option_map(self, snap_name):
-        params = [{'state': 'get'}, {'name': snap_name}, {'json_format': True}]
-        rc, out, err = self.run_command(params=params)
+        with self.runner("get name") as ctx:
+            rc, out, err = ctx.run(name=snap_name)
 
         if rc != 0:
             return {}
@@ -266,10 +256,12 @@ class Snap(CmdStateModuleHelper):
         return option_map
 
     def is_snap_installed(self, snap_name):
-        return 0 == self.run_command(params=[{'state': 'list'}, {'name': snap_name}])[0]
+        rc, dummy, dummy = self.runner("_list name").run(name=snap_name)
+        return rc == 0
 
     def is_snap_enabled(self, snap_name):
-        rc, out, err = self.run_command(params=[{'state': 'list'}, {'name': snap_name}])
+        with self.runner("_list name") as ctx:
+            rc, out, err = ctx.run(name=snap_name)
         if rc != 0:
             return None
         result = out.splitlines()[1]
@@ -283,7 +275,7 @@ class Snap(CmdStateModuleHelper):
         self.changed = True
         self.vars.snaps_installed = actionable_snaps
 
-        if self.module.check_mode:
+        if self.check_mode:
             return
 
         params = ['state', 'classic', 'channel']  # get base cmd parts
@@ -291,10 +283,9 @@ class Snap(CmdStateModuleHelper):
         has_multiple_snaps = len(actionable_snaps) > 1
 
         if has_one_pkg_params and has_multiple_snaps:
-            commands = [params + [{'actionable_snaps': [s]}] for s in actionable_snaps]
+            self.vars.cmd, rc, out, err = self._run_multiple_commands(params, actionable_snaps, bundle=False)
         else:
-            commands = [params + [{'actionable_snaps': actionable_snaps}]]
-        self.vars.cmd, rc, out, err = self._run_multiple_commands(commands)
+            self.vars.cmd, rc, out, err = self._run_multiple_commands(params, actionable_snaps)
 
         if rc == 0:
             return
@@ -360,11 +351,9 @@ class Snap(CmdStateModuleHelper):
             if options_changed:
                 self.changed = True
 
-                if not self.module.check_mode:
-                    params = [{'state': 'set'}, {'name': snap_name}, {'options': options_changed}]
-
-                    rc, out, err = self.run_command(params=params)
-
+                if not self.check_mode:
+                    with self.runner("_set name options") as ctx:
+                        rc, out, err = ctx.run(name=snap_name, options=options_changed)
                     if rc != 0:
                         if 'has no "configure" hook' in err:
                             msg = "Snap '{snap}' does not have any configurable options".format(snap=snap_name)
@@ -383,12 +372,11 @@ class Snap(CmdStateModuleHelper):
             return
         self.changed = True
         self.vars[actionable_var] = actionable_snaps
-        if self.module.check_mode:
+        if self.check_mode:
             return
         if params is None:
             params = ['state']
-        commands = [params + [{'actionable_snaps': actionable_snaps}]]
-        self.vars.cmd, rc, out, err = self._run_multiple_commands(commands)
+        self.vars.cmd, rc, out, err = self._run_multiple_commands(params, actionable_snaps)
         if rc == 0:
             return
         msg = "Ooops! Snap operation failed while executing '{cmd}', please examine logs and " \