From a4246db5ba77e43eb727a17b40298cad5c50a72a Mon Sep 17 00:00:00 2001
From: Olof hagsand <olof@hagsand.se>
Date: Thu, 4 Jan 2024 14:33:26 +0100
Subject: [PATCH] Fixed: [Notification is not sent when a service parameter is
 changed](https://github.com/clicon/clixon-controller/issues/89)

---
 CHANGELOG.md           |  1 +
 src/controller_rpc.c   | 14 +++++-----
 test/test-c-service.sh | 58 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 870abbf..023c4fd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@ Expected: February 2024
 
 ### Corrected Bugs
 
+* Fixed: [Notification is not sent when a service parameter is changed](https://github.com/clicon/clixon-controller/issues/89)
 * Fixed: [Commit/connect transaction may lock datastore with no info or method to break it]([Commit/connect transaction may lock datastore with no info or method to break)
 * Fixed: [pull config from device also does commit local config](https://github.com/clicon/clixon-controller/issues/82)
 * Fixed: [Commit don't push configuration if service is configured](https://github.com/clicon/clixon-controller/issues/78)
diff --git a/src/controller_rpc.c b/src/controller_rpc.c
index 4e0fcc9..fc95914 100644
--- a/src/controller_rpc.c
+++ b/src/controller_rpc.c
@@ -549,13 +549,13 @@ actions_timeout_unregister(controller_transaction *ct)
 
 /*! Get candidate and running, compute diff and return notification
  *
- * @param[in]  h       Clixon handle
- * @param[in]  ct      Transaction
- * @param[in]  td      Local diff transaction
+ * @param[in]  h        Clixon handle
+ * @param[in]  ct       Transaction
+ * @param[in]  td       Local diff transaction
  * @param[out] services 0:  No service configuration, 1: Service config
- * @param[out] cvv     Vector of changed service instances, on the form name:<service> value:<instance>
- * @retval     0       OK, cb including notify msg (or not)
- * @retval    -1       Error
+ * @param[out] cvv      Vector of changed service instances, on the form name:<service> value:<instance>
+ * @retval     0        OK, cb including notify msg (or not)
+ * @retval    -1        Error
  * @see devices_diff   where diff is constructed
  */
 static int
@@ -589,7 +589,7 @@ controller_actions_diff(clixon_handle           h,
     if (x0s){
         xn = NULL;
         while ((xn = xml_child_each(x0s, xn,  CX_ELMNT)) != NULL){
-            if (xml_flag(xn, XML_FLAG_DEL) == 0)
+            if (xml_flag(xn, XML_FLAG_CHANGE|XML_FLAG_DEL) == 0)
                 continue;
             /* Assume first entry is key, Alt: get key via YANG */
             if ((xi = xml_find_type(xn, NULL, NULL, CX_ELMNT)) == NULL ||
diff --git a/test/test-c-service.sh b/test/test-c-service.sh
index f7858bd..1404ceb 100755
--- a/test/test-c-service.sh
+++ b/test/test-c-service.sh
@@ -93,7 +93,6 @@ cat<<EOF > $CFD/action-command.xml
   <CONTROLLER_ACTION_COMMAND xmlns="http://clicon.org/controller-config">${BINDIR}/clixon_controller_service -f $CFG</CONTROLLER_ACTION_COMMAND>
 </clixon-config>
 EOF
-#  <CONTROLLER_ACTION_COMMAND xmlns="http://clicon.org/controller-config">${BINDIR}/clixon_controller_service -f $CFG -D 3 -lf/tmp/services.log</CONTROLLER_ACTION_COMMAND>
 
 cat<<EOF > $CFD/autocli.xml
 <clixon-config xmlns="http://clicon.org/config">
@@ -690,6 +689,60 @@ if $BE; then
     stop_backend -f $CFG
 fi
 
+# Edit sub-service fields https://github.com/clicon/clixon-controller/issues/89
+if $BE; then
+    new "Start new backend -s running -f $CFG -D $DBG"
+    sudo clixon_backend -s running -f $CFG -D $DBG
+fi
+
+new "Wait backend 4"
+wait_backend
+
+new "open connections"
+expectpart "$(${clixon_cli} -1f $CFG connect open)" 0 ""
+
+new "Verify open devices"
+sleep $sleep
+
+imax=5
+for i in $(seq 1 $imax); do
+    res=$(${clixon_cli} -1f $CFG show devices | grep OPEN | wc -l)
+    if [ "$res" = "$nr" ]; then
+        break;
+    fi
+    echo "retry $i after sleep"
+    sleep $sleep
+done
+if [ $i -eq $imax ]; then
+    err1 "$nr open devices" "$res"
+fi
+
+new "Add Sx"
+expectpart "$(${clixon_cli} -m configure -1f $CFG set services testA foo params Sx)" 0 "^$"
+
+new "Add Sy"
+expectpart "$(${clixon_cli} -m configure -1f $CFG set services testA foo params Sy)" 0 "^$"
+
+new "commit base"
+expectpart "$(${clixon_cli} -m configure -1f $CFG commit 2>&1)" 0 "OK"
+
+new "Check Sy"
+expectpart "$(${clixon_cli} -1f $CFG show configuration devices device openconfig1 config interfaces interface Sy)" 0 "<name>Sy</name>"
+
+new "remove Sy"
+expectpart "$(${clixon_cli} -m configure -1f $CFG delete services testA foo params Sy)" 0 "^$"
+
+new "commit remove"
+expectpart "$(${clixon_cli} -m configure -1f $CFG commit 2>&1)" 0 "OK"
+
+new "Check not Sy"
+expectpart "$(${clixon_cli} -1f $CFG show configuration devices device openconfig1 config interfaces interface Sy)" 0 --not-- "<name>Sy</name>"
+
+if $BE; then
+    new "Kill old backend"
+    stop_backend -f $CFG
+fi
+
 # Simulated error
 cat<<EOF > $CFD/action-command.xml
 <clixon-config xmlns="http://clicon.org/config">
@@ -702,7 +755,7 @@ if $BE; then
     sudo clixon_backend -s running -f $CFG -D $DBG
 fi
 
-new "Wait backend 4"
+new "Wait backend 5"
 wait_backend
 
 new "open connections"
@@ -761,7 +814,6 @@ if [ -n "$match" ]; then
     err "<ok/>" "$ret"
 fi
 
-
 new "commit"
 expectpart "$(${clixon_cli} -m configure -1f $CFG commit 2>&1)" 0 "simulated error"