Allow Snap.get and Snap.set to respect the Python's built-in types (

#109)

lib/charms/operator_libs_linux/v2/snap.py

@@ -83,7 +83,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 1
+LIBPATCH = 2
 
 
 # Regex to locate 7-bit C1 ANSI sequences
@@ -310,23 +310,38 @@ def _snap_daemons(
         except CalledProcessError as e:
             raise SnapError("Could not {} for snap [{}]: {}".format(args, self._name, e.stderr))
 
-    def get(self, key) -> str:
-        """Fetch a snap configuration value.
+    def get(self, key: Optional[str], *, typed: bool = False) -> Any:
+        """Fetch snap configuration values.
 
         Args:
-            key: the key to retrieve
+            key: the key to retrieve. Default to retrieve all values for typed=True.
+            typed: set to True to retrieve typed values (set with typed=True).
+                Default is to return a string.
         """
+        if typed:
+            config = json.loads(self._snap("get", ["-d", key]))
+            if key:
+                return config.get(key)
+            return config
+
+        if not key:
+            raise TypeError("Key must be provided when typed=False")
+
         return self._snap("get", [key]).strip()
 
-    def set(self, config: Dict) -> str:
+    def set(self, config: Dict[str, Any], *, typed: bool = False) -> str:
         """Set a snap configuration value.
 
         Args:
            config: a dictionary containing keys and values specifying the config to set.
+           typed: set to True to convert all values in the config into typed values while
+                configuring the snap (set with typed=True). Default is not to convert.
         """
-        args = ['{}="{}"'.format(key, val) for key, val in config.items()]
+        if typed:
+            kv = [f"{key}={json.dumps(val)}" for key, val in config.items()]
+            return self._snap("set", ["-t"] + kv)
 
-        return self._snap("set", [*args])
+        return self._snap("set", [f"{key}={val}" for key, val in config.items()])
 
     def unset(self, key) -> str:
         """Unset a snap configuration value.

tests/integration/test_snap.py

@@ -62,20 +62,70 @@ def test_snap_refresh():
     assert hello_world.channel == "latest/candidate"
 
 
-def test_snap_set():
+def test_snap_set_and_get_with_typed():
     cache = snap.SnapCache()
     lxd = cache["lxd"]
     lxd.ensure(snap.SnapState.Latest, channel="latest")
+    configs = {
+        "true": True,
+        "false": False,
+        "null": None,
+        "integer": 1,
+        "float": 2.0,
+        "list": [1, 2.0, True, False, None],
+        "dict": {
+            "true": True,
+            "false": False,
+            "null": None,
+            "integer": 1,
+            "float": 2.0,
+            "list": [1, 2.0, True, False, None],
+        },
+        "criu.enable": "true",
+        "ceph.external": "false",
+    }
+
+    lxd.set(configs, typed=True)
+
+    assert lxd.get("true", typed=True)
+    assert not lxd.get("false", typed=True)
+    with pytest.raises(snap.SnapError):
+        lxd.get("null", typed=True)
+    assert lxd.get("integer", typed=True) == 1
+    assert lxd.get("float", typed=True) == 2.0
+    assert lxd.get("list", typed=True) == [1, 2.0, True, False, None]
+
+    # Note that `"null": None` will be missing here because `key=null` will not
+    # be set (because it means unset in snap). However, `key=[null]` will be
+    # okay, and that's why `None` exists in "list".
+    assert lxd.get("dict", typed=True) == {
+        "true": True,
+        "false": False,
+        "integer": 1,
+        "float": 2.0,
+        "list": [1, 2.0, True, False, None],
+    }
+
+    assert lxd.get("dict.true", typed=True)
+    assert not lxd.get("dict.false", typed=True)
+    with pytest.raises(snap.SnapError):
+        lxd.get("dict.null", typed=True)
+    assert lxd.get("dict.integer", typed=True) == 1
+    assert lxd.get("dict.float", typed=True) == 2.0
+    assert lxd.get("dict.list", typed=True) == [1, 2.0, True, False, None]
 
-    lxd.set({"ceph.external": "false", "criu.enable": "false"})
+    assert lxd.get("criu.enable", typed=True) == "true"
+    assert lxd.get("ceph.external", typed=True) == "false"
 
-    assert lxd.get("ceph.external") == "false"
-    assert lxd.get("criu.enable") == "false"
 
-    lxd.set({"ceph.external": "true", "criu.enable": "true"})
+def test_snap_set_and_get_untyped():
+    cache = snap.SnapCache()
+    lxd = cache["lxd"]
+    lxd.ensure(snap.SnapState.Latest, channel="latest")
 
-    assert lxd.get("ceph.external") == "true"
-    assert lxd.get("criu.enable") == "true"
+    lxd.set({"foo": "true", "bar": True}, typed=False)
+    assert lxd.get("foo", typed=False) == "true"
+    assert lxd.get("bar", typed=False) == "True"
 
 
 def test_unset_key_raises_snap_error():

tests/unit/test_snap.py

@@ -548,29 +548,28 @@ def raise_error(cmd, **kwargs):
         self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message)
 
     @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
-    def test_snap_set(self, mock_subprocess):
+    def test_snap_set_typed(self, mock_subprocess):
         foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic")
 
-        foo.set({"bar": "baz"})
+        config = {"n": 42, "s": "string", "d": {"nested": True}}
+
+        foo.set(config, typed=True)
         mock_subprocess.assert_called_with(
-            ["snap", "set", "foo", 'bar="baz"'],
+            ["snap", "set", "foo", "-t", "n=42", 's="string"', 'd={"nested": true}'],
             universal_newlines=True,
         )
 
-        foo.set({"bar": "baz", "qux": "quux"})
-        try:
-            mock_subprocess.assert_called_with(
-                ["snap", "set", "foo", 'bar="baz"', 'qux="quux"'],
-                universal_newlines=True,
-            )
-        except AssertionError:
-            # The ordering of this call can be unpredictable across Python versions.
-            # Unfortunately, the methods available to introspect the call list have also
-            # changed, so we do this, which is a little clunky.
-            mock_subprocess.assert_called_with(
-                ["snap", "set", "foo", 'qux="quux"', 'bar="baz"'],
-                universal_newlines=True,
-            )
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
+    def test_snap_set_untyped(self, mock_subprocess):
+        foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic")
+
+        config = {"n": 42, "s": "string", "d": {"nested": True}}
+
+        foo.set(config, typed=False)
+        mock_subprocess.assert_called_with(
+            ["snap", "set", "foo", "n=42", "s=string", "d={'nested': True}"],
+            universal_newlines=True,
+        )
 
     @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call")
     def test_system_set(self, mock_subprocess):