changes: optionally include values in ChangeDeleted

Allows the user to choose whether to retrieve the values of nodes
deleted in ChangeDeleted when generating the subscription (by default,
False). This now also includes List and Container nodes.

Signed-off-by: Adrián Vázquez <[email protected]>

Author: avazquezrd

sysrepo/change.py

@@ -31,6 +31,7 @@ def parse(
         prev_list: str,
         prev_dflt: bool,
         include_implicit_defaults: bool = True,
+        include_deleted_values: bool = False,
     ) -> "Change":
         """
         Parse an operation code and values from libsysrepo.so and return a Change
@@ -64,6 +65,8 @@ def parse(
             Previous value default flag, depends on the operation.
         :arg include_implicit_defaults:
             Include implicit default values into the data dictionaries.
+        :arg include_deleted_values:
+            Include deleted nodes values.
         """
         if operation == lib.SR_OP_CREATED:
             if not node.should_print(
@@ -83,8 +86,8 @@ def parse(
                 prev_dflt=prev_dflt,
             )
         if operation == lib.SR_OP_DELETED:
-            if isinstance(node, (libyang.DLeaf, libyang.DLeafList)):
-                value = node.value()
+            if include_deleted_values:
+                value = _node_value(node, include_implicit_defaults)
             else:
                 value = None
             return ChangeDeleted(node.path(), value)

sysrepo/session.py

@@ -293,6 +293,7 @@ def subscribe_module_change(
         private_data: Any = None,
         asyncio_register: bool = False,
         include_implicit_defaults: bool = True,
+        include_deleted_values: bool = False,
         extra_info: bool = False,
     ) -> None:
         """
@@ -329,6 +330,8 @@ def subscribe_module_change(
             monitored read file descriptors. Implies `no_thread=True`.
         :arg include_implicit_defaults:
             Include implicit default nodes in changes.
+        :arg include_deleted_values:
+            Include deleted nodes values in changes.
         :arg extra_info:
             When True, the given callback is called with extra keyword arguments
             containing extra information of the sysrepo session that gave origin to the
@@ -343,6 +346,7 @@ def subscribe_module_change(
             private_data,
             asyncio_register=asyncio_register,
             include_implicit_defaults=include_implicit_defaults,
+            include_deleted_values=include_deleted_values,
             extra_info=extra_info,
         )
         sub_p = ffi.new("sr_subscription_ctx_t **")
@@ -732,7 +736,10 @@ def subscribe_notification(
 
     # begin: changes
     def get_changes(
-        self, xpath: str, include_implicit_defaults: bool = True
+        self,
+        xpath: str,
+        include_implicit_defaults: bool = True,
+        include_deleted_values: bool = False,
     ) -> Iterator[Change]:
         """
         Return an iterator that will yield all pending changes in the current session.
@@ -743,6 +750,8 @@ def get_changes(
             appended to the XPath.
         :arg include_implicit_defaults:
             Include implicit default nodes.
+        :arg include_deleted_values:
+            Include deleted node values into the returned Change objects.
 
         :returns:
             An iterator that will yield `sysrepo.Change` objects.
@@ -778,6 +787,7 @@ def get_changes(
                             prev_list=c2str(prev_list_p[0]),
                             prev_dflt=bool(prev_dflt_p[0]),
                             include_implicit_defaults=include_implicit_defaults,
+                            include_deleted_values=include_deleted_values,
                         )
                 except Change.Skip:
                     pass

sysrepo/subscription.py

@@ -33,6 +33,7 @@ def __init__(
         asyncio_register: bool = False,
         strict: bool = False,
         include_implicit_defaults: bool = True,
+        include_deleted_values: bool = False,
         extra_info: bool = False,
     ):
         """
@@ -50,6 +51,9 @@ def __init__(
         :arg include_implicit_defaults:
             If True, include implicit default nodes into Change objects passed to module
             change callbacks and into input parameters passed to RPC/action callbacks.
+        :arg include_deleted_values:
+            If True, include the complete deleted node values into Change objects passed
+            to module change callbacks.
         :arg extra_info:
             When True, the given callback is called with extra keyword arguments
             containing extra information of the sysrepo session that gave origin to the
@@ -64,6 +68,7 @@ def __init__(
         self.asyncio_register = asyncio_register
         self.strict = strict
         self.include_implicit_defaults = include_implicit_defaults
+        self.include_deleted_values = include_deleted_values
         self.extra_info = extra_info
         if asyncio_register:
             self.loop = asyncio.get_event_loop()
@@ -242,6 +247,7 @@ def module_change_callback(session, sub_id, module, xpath, event, req_id, priv):
                     session.get_changes(
                         root_xpath + "//.",
                         include_implicit_defaults=subscription.include_implicit_defaults,
+                        include_deleted_values=subscription.include_deleted_values,
                     )
                 )
                 task = subscription.loop.create_task(
@@ -270,6 +276,7 @@ def module_change_callback(session, sub_id, module, xpath, event, req_id, priv):
                 session.get_changes(
                     root_xpath + "//.",
                     include_implicit_defaults=subscription.include_implicit_defaults,
+                    include_deleted_values=subscription.include_deleted_values,
                 )
             )
             callback(event_name, req_id, changes, private_data, **extra_info)

tests/test_subs_module_change.py

@@ -111,11 +111,11 @@ def module_change_cb(event, req_id, changes, private_data):
                 ),
                 sysrepo.ChangeDeleted(
                     "/sysrepo-example:conf/network/interface[name='eth0']/name",
-                    "eth0",
+                    None,
                 ),
                 sysrepo.ChangeDeleted(
                     "/sysrepo-example:conf/network/interface[name='eth0']/up",
-                    True,
+                    None,
                 ),
                 sysrepo.ChangeCreated(
                     "/sysrepo-example:conf/network/interface[name='eth2']",
@@ -195,6 +195,86 @@ def module_change_cb(event, req_id, changes, private_data):
             ch_sess.replace_config(sent_config, "sysrepo-example", strict=True)
             self.assertEqual(current_config, sent_config)
 
+    def test_module_change_sub_with_deleted_values(self):
+        priv = object()
+        current_config = {}
+        expected_changes = []
+
+        def module_change_cb(event, req_id, changes, private_data):
+            self.assertIn(event, ("change", "done", "abort"))
+            self.assertIs(private_data, priv)
+            if event in ("change", "done"):
+                self.assertEqual(changes, expected_changes)
+            if event == "done":
+                sysrepo.update_config_cache(current_config, changes)
+
+        self.sess.subscribe_module_change(
+            "sysrepo-example",
+            "/sysrepo-example:conf",
+            module_change_cb,
+            private_data=priv,
+            include_deleted_values=True,
+        )
+
+        with self.conn.start_session("running") as ch_sess:
+            # 1.
+            sent_config = {
+                "conf": {
+                    "system": {"hostname": "bar"},
+                    "network": {"interface": [{"name": "eth0", "up": True}]},
+                }
+            }
+            expected_changes = [
+                sysrepo.ChangeCreated("/sysrepo-example:conf/system/hostname", "bar"),
+                sysrepo.ChangeCreated(
+                    "/sysrepo-example:conf/network/interface[name='eth0']",
+                    {"name": "eth0", "up": True},
+                    after="",
+                ),
+                sysrepo.ChangeCreated(
+                    "/sysrepo-example:conf/network/interface[name='eth0']/name", "eth0"
+                ),
+                sysrepo.ChangeCreated(
+                    "/sysrepo-example:conf/network/interface[name='eth0']/up", True
+                ),
+            ]
+            ch_sess.replace_config(sent_config, "sysrepo-example", strict=True)
+            self.assertEqual(current_config, sent_config)
+            # 2.
+            sent_config = {
+                "conf": {
+                    "system": {"hostname": "bar"},
+                    "network": {"interface": [{"name": "eth2", "up": False}]},
+                }
+            }
+            expected_changes = [
+                sysrepo.ChangeDeleted(
+                    "/sysrepo-example:conf/network/interface[name='eth0']",
+                    {"name": "eth0", "up": True},
+                ),
+                sysrepo.ChangeDeleted(
+                    "/sysrepo-example:conf/network/interface[name='eth0']/name",
+                    "eth0",
+                ),
+                sysrepo.ChangeDeleted(
+                    "/sysrepo-example:conf/network/interface[name='eth0']/up",
+                    True,
+                ),
+                sysrepo.ChangeCreated(
+                    "/sysrepo-example:conf/network/interface[name='eth2']",
+                    {"name": "eth2", "up": False},
+                    after="",
+                ),
+                sysrepo.ChangeCreated(
+                    "/sysrepo-example:conf/network/interface[name='eth2']/name", "eth2"
+                ),
+                sysrepo.ChangeCreated(
+                    "/sysrepo-example:conf/network/interface[name='eth2']/up", False
+                ),
+            ]
+            ch_sess.replace_config(sent_config, "sysrepo-example", strict=True)
+            self.assertEqual(current_config, sent_config)
+
     def test_module_change_sub_with_extra_info(self):
         priv = object()
         calls = []