T8989: update to vyos's get_value() method (#29)

* T8989: update to vyos's get_value() method

* T8989: Refactor vyos.py with new get_value() wrapper

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* T8989: Refactor return statement to handle None values

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* T8989: get_values - AI fixes, add unit test

* T8989: get_values - AI fixes, add unit test

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Daniil Baturin <daniil@vyos.io>

3 files changed, 94 insertions, 3 deletions

diff --git a/changelogs/fragments/t8989_vyos_get_value.yml b/changelogs/fragments/t8989_vyos_get_value.yml
new file mode 100644
index 0000000..f6b7043
--- /dev/null
+++ b/changelogs/fragments/t8989_vyos_get_value.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - plugins/module_utils/vyos.py - Add VyOSModule.get_value() wrapper for the "returnValue" retrieve operation.
diff --git a/plugins/module_utils/vyos.py b/plugins/module_utils/vyos.py
index f7c17f9..b7a519e 100644
--- a/plugins/module_utils/vyos.py
+++ b/plugins/module_utils/vyos.py
@@ -320,7 +320,8 @@ class VyOSModule:
         that's actually already correct.
         """
         result = self._client.retrieve_return_value(path)
-        return result.get("data") or ""
+        data = result.get("data")
+        return "" if data is None else data
 
     def apply_commands(self, commands):
         if not commands:
@@ -372,7 +373,8 @@ class VyOSModule:
         rather than being indistinguishable from a valid empty response.
         """
         result = self._client.show(path)
-        return result.get("data") or ""
+        data = result.get("data")
+        return "" if data is None else data
 
     def save_config(self, file_path=None):
         """Save the running configuration to disk."""
diff --git a/tests/unit/test_module_utils_vyos.py b/tests/unit/test_module_utils_vyos.py
index 920e9a5..213a2fb 100644
--- a/tests/unit/test_module_utils_vyos.py
+++ b/tests/unit/test_module_utils_vyos.py
@@ -14,7 +14,12 @@ __metaclass__ = type
 
 import unittest
 
-from ansible_collections.vyos.rest.plugins.module_utils.vyos import cast_by_spec
+from unittest.mock import MagicMock
+
+from ansible_collections.vyos.rest.plugins.module_utils.vyos import (
+    VyOSModule,
+    cast_by_spec,
+)
 
 
 class TestCastBySpecIntCollapse(unittest.TestCase):
@@ -51,5 +56,86 @@ class TestCastBySpecIntCollapse(unittest.TestCase):
         self.assertIsNone(entry["distance"])
 
 
+class TestGetValue(unittest.TestCase):
+    """Regression tests for a confirmed bug (Copilot): get_value()
+    previously did `result.get("data") or ""`, which incorrectly
+    converts any falsy-but-valid scalar (0, False, an already-empty
+    string) into an empty string -- indistinguishable from the value
+    being genuinely unset. Fixed to only treat a missing "data" key
+    (None) as unset, leaving every other value -- including falsy
+    ones -- exactly as returned."""
+
+    def _vyos_with_data(self, data):
+        vyos = VyOSModule.__new__(VyOSModule)
+        vyos._client = MagicMock()
+        vyos._client.retrieve_return_value.return_value = {"data": data}
+        return vyos
+
+    def test_zero_preserved_not_emptied(self):
+        vyos = self._vyos_with_data(0)
+        result = vyos.get_value(["some", "path"])
+        self.assertEqual(result, 0)
+        self.assertIs(type(result), int)
+
+    def test_false_preserved_not_emptied(self):
+        vyos = self._vyos_with_data(False)
+        self.assertIs(vyos.get_value(["some", "path"]), False)
+
+    def test_genuine_string_value_passes_through(self):
+        vyos = self._vyos_with_data("vyos-core-01")
+        self.assertEqual(vyos.get_value(["some", "path"]), "vyos-core-01")
+
+    def test_already_empty_string_stays_empty(self):
+        vyos = self._vyos_with_data("")
+        self.assertEqual(vyos.get_value(["some", "path"]), "")
+
+    def test_missing_data_key_becomes_empty_string(self):
+        vyos = VyOSModule.__new__(VyOSModule)
+        vyos._client = MagicMock()
+        vyos._client.retrieve_return_value.return_value = {}
+        self.assertEqual(vyos.get_value(["some", "path"]), "")
+
+
+class TestShow(unittest.TestCase):
+    """Regression tests for the same confirmed bug class as
+    TestGetValue, found independently in show(): `result.get("data")
+    or ""` incorrectly converts any falsy-but-valid scalar (0, False,
+    an already-empty string) from an operational show command into an
+    empty string -- indistinguishable from the command genuinely
+    returning nothing. Fixed to only treat a missing "data" key
+    (None) as empty, leaving every other value -- including falsy
+    ones -- exactly as returned."""
+
+    def _vyos_with_data(self, data):
+        vyos = VyOSModule.__new__(VyOSModule)
+        vyos._client = MagicMock()
+        vyos._client.show.return_value = {"data": data}
+        return vyos
+
+    def test_zero_preserved_not_emptied(self):
+        vyos = self._vyos_with_data(0)
+        result = vyos.show(["some", "op", "path"])
+        self.assertEqual(result, 0)
+        self.assertIs(type(result), int)
+
+    def test_false_preserved_not_emptied(self):
+        vyos = self._vyos_with_data(False)
+        self.assertIs(vyos.show(["some", "op", "path"]), False)
+
+    def test_genuine_output_passes_through(self):
+        vyos = self._vyos_with_data("interface eth0 up")
+        self.assertEqual(vyos.show(["some", "op", "path"]), "interface eth0 up")
+
+    def test_already_empty_string_stays_empty(self):
+        vyos = self._vyos_with_data("")
+        self.assertEqual(vyos.show(["some", "op", "path"]), "")
+
+    def test_missing_data_key_becomes_empty_string(self):
+        vyos = VyOSModule.__new__(VyOSModule)
+        vyos._client = MagicMock()
+        vyos._client.show.return_value = {}
+        self.assertEqual(vyos.show(["some", "op", "path"]), "")
+
+
 if __name__ == "__main__":
     unittest.main()