allow for nested edit lists

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index 36f288d6c070348dc311d72c2eaede7912055ea5..cf456cbdf027f055884ac33c2edc6feb5554a45d 100644 (file)
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -1587,7 +1587,7 @@ static void edit_state_init_internal(request_t *request, unlang_frame_state_edit
         *      generally be large enough for most edits.
         */
        if (!el) {
-               MEM(state->el = fr_edit_list_alloc(state, map_list_num_elements(map_list)));
+               MEM(state->el = fr_edit_list_alloc(state, map_list_num_elements(map_list), NULL));
        }
 
        current->ctx = state;

diff --git a/src/lib/util/edit.c b/src/lib/util/edit.c
index 783b8a8cee6fb519be7d1ed17568b1b2f111cc2b..132ed051cd7466556267a3b67c13fa0208861898 100644 (file)
--- a/src/lib/util/edit.c
+++ b/src/lib/util/edit.c
@@ -58,6 +58,7 @@ typedef enum {
        FR_EDIT_VALUE,                  //!< edit a VP in place
        FR_EDIT_CLEAR,                  //!< clear the children of a structural entry.
        FR_EDIT_INSERT,                 //!< insert a VP into a list, after another one.
+       FR_EDIT_CHILD,                  //!< child edit list
 } fr_edit_op_t;
 
 #if 0
@@ -73,24 +74,6 @@ static const char *edit_names[4] = {
 };
 #endif
 
-/** Track a series of edits.
- *
- */
-struct fr_edit_list_s {
-       /*
-        *      List of undo changes to be made, in order.
-        */
-       fr_dlist_head_t undo;
-
-       fr_dlist_head_t ignore;         //!< lists to ignore
-
-       /*
-        *      VPs which were inserted, and then over-written by a
-        *      later edit.
-        */
-       fr_pair_list_t  deleted_pairs;
-};
-
 /** Track one particular edit.
  */
 typedef struct {
@@ -103,6 +86,7 @@ typedef struct {
                union {
                        fr_value_box_t  data;   //!< original data
                        fr_pair_list_t  children;  //!< original child list, for "clear"
+                       fr_edit_list_t  *child_edit;
                };
 
                struct {
@@ -112,6 +96,27 @@ typedef struct {
        };
 } fr_edit_t;
 
+/** Track a series of edits.
+ *
+ */
+struct fr_edit_list_s {
+       /*
+        *      List of undo changes to be made, in order.
+        */
+       fr_dlist_head_t undo;
+
+       fr_dlist_head_t ignore;         //!< lists to ignore
+
+       /*
+        *      VPs which were inserted, and then over-written by a
+        *      later edit.
+        */
+       fr_pair_list_t  deleted_pairs;
+
+       fr_edit_list_t  *parent;        //!< for nested transactions
+       fr_edit_t       *e;             //!< so we don't have to loop over parent edits on abort
+};
+
 typedef struct {
        fr_dlist_t      entry;
        fr_pair_list_t  *list;          //!< list to ignore (never dereferenced)
@@ -170,6 +175,10 @@ static int edit_undo(fr_edit_t *e)
                 */
                fr_pair_delete(e->list, vp);
                break;
+
+       case FR_EDIT_CHILD:
+               fr_edit_list_abort(e->child_edit);
+               break;
        }
 
        return 0;
@@ -210,6 +219,14 @@ void fr_edit_list_abort(fr_edit_list_t *el)
                 */
        }
 
+       /*
+        *      There's a parent, we remove ourselves from the parent undo list.
+        */
+       if (el->parent) {
+               fr_dlist_remove(&el->parent->undo, el->e);
+               talloc_free(el->e);
+       }
+
        talloc_free(el);
 }
 
@@ -234,6 +251,8 @@ static int edit_record(fr_edit_list_t *el, fr_edit_op_t op, fr_pair_t *vp, fr_pa
        fr_assert(el != NULL);
        fr_assert(vp != NULL);
 
+       fr_assert(op != FR_EDIT_CHILD); /* only used by fr_edit_alloc() */
+
        /*
         *      When we insert a structural type, we also want to
         *      not track edits to it's children.  The "ignore list"
@@ -449,6 +468,10 @@ static int edit_record(fr_edit_list_t *el, fr_edit_op_t op, fr_pair_t *vp, fr_pa
                        e->op = FR_EDIT_DELETE;
                        fr_dlist_remove(&el->undo, e);
                        goto delete;
+
+               case FR_EDIT_CHILD: /* e->vp==NULL, so this should never happen */
+                       fr_assert(0);
+                       return -1;
                }
        } /* loop over existing edits */
 
@@ -464,6 +487,7 @@ static int edit_record(fr_edit_list_t *el, fr_edit_op_t op, fr_pair_t *vp, fr_pa
 
        switch (op) {
        case FR_EDIT_INVALID:
+       case FR_EDIT_CHILD:
        fail:
                talloc_free(e);
                return -1;
@@ -739,6 +763,10 @@ static int _edit_list_destructor(fr_edit_list_t *el)
                        fr_assert(fr_type_is_leaf(e->vp->vp_type));
                        fr_value_box_clear(&e->data);
                        break;
+
+               case FR_EDIT_CHILD:
+                       talloc_free(e->child_edit);
+                       break;
                }
        }
 
@@ -749,9 +777,27 @@ static int _edit_list_destructor(fr_edit_list_t *el)
        return 0;
 }
 
-fr_edit_list_t *fr_edit_list_alloc(TALLOC_CTX *ctx, int hint)
+/** Allocate an edit list.
+ *
+ *  Edit lists can be nested.  If the child list commits, then it does nothing
+ *  until the parent list commits.  On the other hand, if the child list aborts,
+ *  then the parent list might continue.
+ *
+ * @param ctx          talloc context.  Ignored if parent!=NULL
+ * @param hint         how many edits we are likely to allocate
+ * @param parent       a parent edit list
+ */
+fr_edit_list_t *fr_edit_list_alloc(TALLOC_CTX *ctx, int hint, fr_edit_list_t *parent)
 {
        fr_edit_list_t *el;
+       fr_edit_t *e;
+
+       /*
+        *      If we have nested transactions, then allocate this
+        *      list in the context of the parent.  Otherwise it will
+        *      be freed too soon.
+        */
+       if (parent) ctx = parent;
 
        el = talloc_zero_pooled_object(ctx, fr_edit_list_t, hint, hint * sizeof(fr_edit_t));
        if (!el) return NULL;
@@ -763,9 +809,42 @@ fr_edit_list_t *fr_edit_list_alloc(TALLOC_CTX *ctx, int hint)
 
        talloc_set_destructor(el, _edit_list_destructor);
 
+       el->parent = parent;
+
+       if (!parent) return el;
+
+       e = talloc_zero(parent, fr_edit_t);
+       if (!e) {
+               talloc_free(el);
+               return NULL;
+       }
+
+       /*
+        *      Insert the child into the tail of the current edit list.
+        */
+       e->op = FR_EDIT_CHILD;
+       e->vp = NULL;
+       e->child_edit = el;
+
+       fr_dlist_insert_tail(&parent->undo, e);
+       el->e = e;
+
        return el;
 }
 
+/** Commit an edit list.
+ *
+ *  If there are nested transactions, then this transaction is
+ *  committed only when the parent transaction has been commited.
+ *
+ */
+void fr_edit_list_commit(fr_edit_list_t *el)
+{
+       if (el->parent) return;
+
+       talloc_free(el);
+}
+
 /** Notes
  *
  *  Unlike "update" sections, edits are _not_ hierarchical.  If we're

diff --git a/src/lib/util/edit.h b/src/lib/util/edit.h
index 3cc0d66cf34a76b71650c2c8e1707b0bd9172fe2..2a5fd474dece04a0807cc5fdc5b48fed7369f810 100644 (file)
--- a/src/lib/util/edit.h
+++ b/src/lib/util/edit.h
@@ -33,11 +33,11 @@ extern "C" {
 
 typedef struct fr_edit_list_s fr_edit_list_t;
 
-fr_edit_list_t *fr_edit_list_alloc(TALLOC_CTX *ctx, int hint);
+fr_edit_list_t *fr_edit_list_alloc(TALLOC_CTX *ctx, int hint, fr_edit_list_t *parent);
 
 void fr_edit_list_abort(fr_edit_list_t *el);
 
-#define fr_edit_list_commit(_x) talloc_free(_x)
+void fr_edit_list_commit(fr_edit_list_t *el);
 
 /*
  *     Functions to modify #fr_pair_t

diff --git a/src/lib/util/edit_tests.c b/src/lib/util/edit_tests.c
index c992fe7a153e35e6e6bb4b8afeeea02514d4bef8..a16fac18c39d4892d495b4cff03844b924b1ff3a 100644 (file)
--- a/src/lib/util/edit_tests.c
+++ b/src/lib/util/edit_tests.c
@@ -141,7 +141,7 @@ static void test_pair_delete_head(void)
 
        vp = fr_pair_list_head(&local_pairs);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_pair_delete(el, &local_pairs, vp);
@@ -174,7 +174,7 @@ static void test_pair_delete_head_abort(void)
 
        vp = fr_pair_list_head(&local_pairs);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_pair_delete(el, &local_pairs, vp);
@@ -208,7 +208,7 @@ static void test_pair_delete_middle(void)
        vp = fr_pair_list_next(&local_pairs, vp);
        fr_assert(vp != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_pair_delete(el, &local_pairs, vp);
@@ -247,7 +247,7 @@ static void test_pair_delete_middle_abort(void)
        fr_assert(middle != NULL);
        TEST_CHECK(middle->da == fr_dict_attr_test_octets);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_pair_delete(el, &local_pairs, middle);
@@ -287,7 +287,7 @@ static void test_pair_delete_multiple(void)
        vp = fr_pair_list_next(&local_pairs, vp);
        fr_assert(vp != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_pair_delete(el, &local_pairs, vp); /* middle */
@@ -332,7 +332,7 @@ static void test_pair_delete_multiple_abort(void)
        fr_assert(vp != NULL);
        TEST_CHECK(vp->da == fr_dict_attr_test_octets);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_pair_delete(el, &local_pairs, vp); /* middle */
@@ -377,7 +377,7 @@ static void test_pair_edit_value(void)
        vp = fr_pair_list_head(&local_pairs);
        fr_assert(vp != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_save_pair_value(el, vp);
@@ -411,7 +411,7 @@ static void test_pair_edit_value_abort(void)
        vp = fr_pair_list_head(&local_pairs);
        fr_assert(vp != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_save_pair_value(el, vp);
@@ -446,7 +446,7 @@ static void test_pair_insert_after_head(void)
 
        add_pairs(&local_pairs);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        TEST_CHECK((vp = fr_pair_afrom_da(autofree, fr_dict_attr_test_string)) != NULL);
@@ -479,7 +479,7 @@ static void test_pair_insert_after_head_abort(void)
 
        add_pairs(&local_pairs);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        TEST_CHECK((vp = fr_pair_afrom_da(autofree, fr_dict_attr_test_string)) != NULL);
@@ -515,7 +515,7 @@ static void test_pair_insert_after_middle(void)
        middle = fr_pair_list_next(&local_pairs, vp);
        fr_assert(middle != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        TEST_CHECK((vp = fr_pair_afrom_da(autofree, fr_dict_attr_test_string)) != NULL);
@@ -548,7 +548,7 @@ static void test_pair_insert_after_middle_abort(void)
        middle = fr_pair_list_next(&local_pairs, vp);
        fr_assert(middle != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        TEST_CHECK((vp = fr_pair_afrom_da(autofree, fr_dict_attr_test_string)) != NULL);
@@ -582,7 +582,7 @@ static void test_pair_edit_value_delete(void)
        vp = fr_pair_list_head(&local_pairs);
        fr_assert(vp != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_save_pair_value(el, vp);
@@ -620,7 +620,7 @@ static void test_pair_edit_value_delete_abort(void)
        vp = fr_pair_list_head(&local_pairs);
        fr_assert(vp != NULL);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        rcode = fr_edit_list_save_pair_value(el, vp);
@@ -658,7 +658,7 @@ static void test_pair_insert_after_head_delete(void)
 
        add_pairs(&local_pairs);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        TEST_CHECK((vp = fr_pair_afrom_da(autofree, fr_dict_attr_test_string)) != NULL);
@@ -694,7 +694,7 @@ static void test_pair_insert_after_head_delete_abort(void)
 
        add_pairs(&local_pairs);
 
-       el = fr_edit_list_alloc(NULL, 5);
+       el = fr_edit_list_alloc(NULL, 5, NULL);
        fr_assert(el != NULL);
 
        TEST_CHECK((vp = fr_pair_afrom_da(autofree, fr_dict_attr_test_string)) != NULL);
@@ -715,6 +715,104 @@ static void test_pair_insert_after_head_delete_abort(void)
 }
 
 
+static void test_pair_edit_child_value(void)
+{
+       fr_pair_t       *vp;
+       fr_pair_list_t  local_pairs;
+       fr_edit_list_t  *el, *child;
+       int             rcode;
+
+       TEST_CASE("Add 3 pairs and change the value of the first one in a child transaction");
+
+       add_pairs(&local_pairs);
+
+       vp = fr_pair_list_head(&local_pairs);
+       fr_assert(vp != NULL);
+
+       el = fr_edit_list_alloc(NULL, 5, NULL);
+       fr_assert(el != NULL);
+
+       rcode = fr_edit_list_save_pair_value(el, vp);
+       TEST_CHECK(rcode == 0);
+
+       TEST_CHECK(vp->vp_uint32 == 0);
+
+       vp->vp_uint32 = 1;
+       TEST_CHECK(vp->vp_uint32 == 1);
+
+       child = fr_edit_list_alloc(NULL, 5, el);
+       fr_assert(child != NULL);
+
+       rcode = fr_edit_list_save_pair_value(child, vp); /* CHILD */
+       TEST_CHECK(rcode == 0);
+
+       TEST_CHECK(vp->vp_uint32 == 1);
+
+       vp->vp_uint32 = 2;
+       TEST_CHECK(vp->vp_uint32 == 2);
+
+       fr_edit_list_commit(child); /* should do nothing */
+
+       TEST_CHECK(vp->vp_uint32 == 2);
+
+       fr_edit_list_commit(el);
+
+       vp = fr_pair_list_head(&local_pairs);
+       TEST_CHECK(vp->da == fr_dict_attr_test_uint32);
+       TEST_CHECK(vp->vp_uint32 == 2);
+
+       expect3(&local_pairs);
+}
+
+static void test_pair_edit_child_value_abort(void)
+{
+       fr_pair_t       *vp;
+       fr_pair_list_t  local_pairs;
+       fr_edit_list_t  *el, *child;
+       int             rcode;
+
+       TEST_CASE("Add 3 pairs and change the value of the first one, then abort");
+
+       add_pairs(&local_pairs);
+
+       vp = fr_pair_list_head(&local_pairs);
+       fr_assert(vp != NULL);
+
+       el = fr_edit_list_alloc(NULL, 5, NULL);
+       fr_assert(el != NULL);
+
+       rcode = fr_edit_list_save_pair_value(el, vp);
+       TEST_CHECK(rcode == 0);
+
+       TEST_CHECK(vp->vp_uint32 == 0);
+
+       vp->vp_uint32 = 1;
+       TEST_CHECK(vp->vp_uint32 == 1);
+
+       child = fr_edit_list_alloc(NULL, 5, el);
+       fr_assert(child != NULL);
+
+       rcode = fr_edit_list_save_pair_value(child, vp); /* CHILD */
+       TEST_CHECK(rcode == 0);
+
+       TEST_CHECK(vp->vp_uint32 == 1);
+
+       vp->vp_uint32 = 2;
+       TEST_CHECK(vp->vp_uint32 == 2);
+
+       fr_edit_list_abort(child); /* CHILD */
+
+       TEST_CHECK(vp->vp_uint32 == 1);
+
+       fr_edit_list_commit(el);
+
+       vp = fr_pair_list_head(&local_pairs);
+       TEST_CHECK(vp->da == fr_dict_attr_test_uint32);
+       TEST_CHECK(vp->vp_uint32 == 1);
+
+       expect3(&local_pairs);
+}
+
 TEST_LIST = {
        /*
         *      Deletion.
@@ -755,5 +853,11 @@ TEST_LIST = {
        { "pair_insert_after_head_delete",       test_pair_insert_after_head_delete },
        { "pair_insert_after_head_delete_abort", test_pair_insert_after_head_delete_abort },
 
+       /*
+        *      Value modification in child list
+        */
+       { "pair_edit_child_value",              test_pair_edit_child_value },
+       { "pair_edit_child_value_abort",        test_pair_edit_child_value_abort },
+
        { NULL }
 };