lib: fix order of northbound operations

When ordering operations, destroys must always come before other
operations, to correctly cover the change of a "case" in a "choice".

The problem can be reproduced with the following commands:
```
access-list test seq 1 permit 10.0.0.0/8
access-list test seq 1 permit host 10.0.0.1
access-list test seq 1 permit 10.0.0.0/8
```

Before this commit, the order of changes would be the following:
- `access-list test seq 1 permit 10.0.0.0/8`
  - `modify` for `ipv4-prefix`
- `access-list test seq 1 permit host 10.0.0.1`
  - `destroy` for `ipv4-prefix`
  - `modify` for `host`
- `access-list test seq 1 permit 10.0.0.0/8`
  - `modify` for `ipv4-prefix`
  - `destroy` for `host`

As `destroy` for `host` is called last, it rewrites the fields that were
filled by `modify` callback of `ipv4-prefix`. This commit fixes this
problem by always calling `destroy` callbacks first.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

1 files changed, 19 insertions, 4 deletions

diff --git a/lib/northbound.c b/lib/northbound.c
index 6b31b818c..8d9302a81 100644
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -391,14 +391,27 @@ void nb_config_replace(struct nb_config *config_dst,
 static inline int nb_config_cb_compare(const struct nb_config_cb *a,
 				       const struct nb_config_cb *b)
 {
+	bool a_destroy = a->operation == NB_CB_DESTROY;
+	bool b_destroy = b->operation == NB_CB_DESTROY;
+
+	/*
+	 * Sort by operation first. All "destroys" must come first, to correctly
+	 * process the change of a "case" inside a "choice". The old "case" must
+	 * be deleted before the new "case" is created.
+	 */
+	if (a_destroy && !b_destroy)
+		return -1;
+	if (!a_destroy && b_destroy)
+		return 1;
+
 	/*
-	 * Sort by priority first. If the operation is "destroy", reverse the
-	 * order, so that the dependencies are destroyed before the dependants.
+	 * Then sort by priority. If the operation is "destroy", reverse the
+	 * order, so that the dependants are deleted before the dependencies.
 	 */
 	if (a->nb_node->priority < b->nb_node->priority)
-		return a->operation != NB_CB_DESTROY ? -1 : 1;
+		return !a_destroy ? -1 : 1;
 	if (a->nb_node->priority > b->nb_node->priority)
-		return a->operation != NB_CB_DESTROY ? 1 : -1;
+		return !a_destroy ? 1 : -1;
 
 	/*
 	 * Preserve the order of the configuration changes as told by libyang.
@@ -1814,6 +1827,7 @@ nb_apply_finish_cb_new(struct nb_config_cbs *cbs, const struct nb_node *nb_node,
 	struct nb_config_cb *cb;
 
 	cb = XCALLOC(MTYPE_TMP, sizeof(*cb));
+	cb->operation = NB_CB_APPLY_FINISH;
 	cb->nb_node = nb_node;
 	cb->dnode = dnode;
 	RB_INSERT(nb_config_cbs, cbs, cb);
@@ -1828,6 +1842,7 @@ nb_apply_finish_cb_find(struct nb_config_cbs *cbs,
 {
 	struct nb_config_cb s;
 
+	s.operation = NB_CB_APPLY_FINISH;
 	s.seq = 0;
 	s.nb_node = nb_node;
 	s.dnode = dnode;