set default action return codes to "fail=1"

because otherwise most tests will have to manually set an "actions"
block

diff --git a/doc/antora/modules/reference/pages/unlang/transaction.adoc b/doc/antora/modules/reference/pages/unlang/transaction.adoc
index e2b34fd7987fd6de31f0b5a7793712f8ffeaa4ae..09c3d09c7a4c15390d17c6fd61a1d3708e0190ad 100644 (file)
--- a/doc/antora/modules/reference/pages/unlang/transaction.adoc
+++ b/doc/antora/modules/reference/pages/unlang/transaction.adoc
@@ -17,21 +17,13 @@ been made will be reverted.
 
 == Caveats
 
-The `transaction` keyword currently has some major limitations.
+The `transaction` sets its own action defaults for return codes
+`fail`, `invalid`, and `disallow`.  The priority for those return
+codes is set to `1`, instead of the default `return`.  This behavior
+allows for the caller to edit attributes, but continue processing if
+something goes wrong.
 
-The first limitation is that most processing sections have a default
-action of `return` for failures.  This setting means that failures
-cause the server to stop processing the entire section.  The
-`transaction` keyword is no different.
-
-If instead the server should continue processing the next statement
-after the `transaction`, then the `transaction` section must finish
-with an xref:unlang/actions.adoc[actions] subsection, as given in the
-example below.  This subsection over-ride sthe default action of
-`return` for failures, and allows processing to continue after the
-`transaction`.
-
-The second limitation of `transaction` is that it can only revert
+The main limitation of `transaction` is that it can only revert
 attribute editing which is done via the xref:unlang/edit.adoc[edit]
 statements.  If a module performs attribute editing (e.g. `sql`,
 `files`, etc.), then those edits are not reverted.
@@ -40,21 +32,21 @@ Similarly, a `transaction` cannot undo operations on external
 databases.  For example, any data which was inserted into `sql` during
 a `transaction` statement will remain in `sql`.
 
-.Examples
+.Example
+
+In this example, if the SQL `select` statement fails, then the
+`&reply.Framed-IP-Address` attribute is _not_ updated, and the
+`transaction` section returns `fail`.
 
 [source,unlang]
 ----
 transaction {
-    &reply.Filter-Id := %exec
-    sql
-
-    actions {
-        fail = 1
-    }
+    &reply.Filter-Id := %sql("SELECT ...")
+    &reply.Framed-IP-Address := 192.0.2.1
 }
 ----
 
-The last entry in a `transaction` section can also be an xref:unlang/actions.adoc[actions] subsection.
+The last entry in a `transaction` section can also be an xref:unlang/actions.adoc[actions] subsection.  If set, those actions over-ride any previously set defaults.
 
 == Grouping Edits
 
@@ -63,7 +55,7 @@ xref:unlang/edit.adoc[edit] instructions.  When edit instructions are
 grouped, then the edits are done in an atomic transaction.  That is,
 either all of the edits succeed, or none of them do.
 
-For now, the only purpose of `transaction` is to group edits.
+For now, the only purpose of `transaction` is to group edits.  If a module is used inside of 
 
 // Copyright (C) 2023 Network RADIUS SAS.  Licenced under CC-by-NC 4.0.
 // This documentation was developed by Network RADIUS SAS.

diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index b03e31caaa15424912b52928e87662470425919e..b3af9d0f81546920175a81251725099d0d7b6567 100644 (file)
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -2183,7 +2183,7 @@ static bool compile_action_subsection(unlang_t *c, CONF_SECTION *cs, CONF_SECTIO
 }
 
 
-static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ctx_in)
+static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ctx_in, bool set_action_defaults)
 {
        CONF_ITEM       *ci = NULL;
        unlang_t        *c, *single;
@@ -2439,7 +2439,7 @@ static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ct
         *      Set the default actions, if they haven't already been
         *      set by an "actions" section above.
         */
-       compile_action_defaults(c, unlang_ctx);
+       if (set_action_defaults) compile_action_defaults(c, unlang_ctx);
 
        return c;
 }
@@ -2487,7 +2487,7 @@ static unlang_t *compile_section(unlang_t *parent, unlang_compile_t *unlang_ctx,
                MEM(c->debug_name = talloc_typed_asprintf(c, "%s %s", name1, name2));
        }
 
-       return compile_children(g, unlang_ctx);
+       return compile_children(g, unlang_ctx, true);
 }
 
 
@@ -2629,6 +2629,7 @@ static bool transaction_ok(CONF_SECTION *cs, bool *all_edits)
 
 static unlang_t *compile_transaction(unlang_t *parent, unlang_compile_t *unlang_ctx, CONF_SECTION *cs)
 {
+       unlang_group_t *g;
        unlang_t *c;
        bool all_edits = true;
        unlang_compile_t unlang_ctx2;
@@ -2639,6 +2640,11 @@ static unlang_t *compile_transaction(unlang_t *parent, unlang_compile_t *unlang_
                .type_name = "unlang_transaction_t",
        };
 
+       /*
+        *      The transaction is empty, ignore it.
+        */
+       if (!cf_item_next(cs, NULL)) return UNLANG_IGNORE;
+
        if (!transaction_ok(cs, &all_edits)) return NULL;
 
        /*
@@ -2653,10 +2659,27 @@ static unlang_t *compile_transaction(unlang_t *parent, unlang_compile_t *unlang_
 
        unlang_ctx2.all_edits = all_edits;
 
-       c = compile_section(parent, &unlang_ctx2, cs, &transaction);
-       if (!c || (c == UNLANG_IGNORE)) return c;
+       /*
+        *      We always create a group, even if the section is empty.
+        */
+       g = group_allocate(parent, cs, &transaction);
+       if (!g) return NULL;
+
+       c = unlang_group_to_generic(g);
+       c->debug_name = c->name = cf_section_name1(cs);
+
+       if (!compile_children(g, &unlang_ctx2, false)) return NULL;
+
+       /*
+        *      The default for a failed transaction is to continue on
+        *      failure.
+        */
+       if (!c->actions.actions[RLM_MODULE_FAIL])     c->actions.actions[RLM_MODULE_FAIL] = 1;
+       if (!c->actions.actions[RLM_MODULE_INVALID])  c->actions.actions[RLM_MODULE_INVALID] = 1;
+       if (!c->actions.actions[RLM_MODULE_DISALLOW]) c->actions.actions[RLM_MODULE_DISALLOW] = 1;
+
+       compile_action_defaults(c, unlang_ctx);
 
-       c->type = UNLANG_TYPE_TRANSACTION;
        return c;
 }
 

diff --git a/src/tests/keywords/transaction b/src/tests/keywords/transaction
index f26ec8c3be02242c52dda8aef8abec34b2354861..222e10b9e0bf5aa14dd0a3ec20c04e972c33a87b 100644 (file)
--- a/src/tests/keywords/transaction
+++ b/src/tests/keywords/transaction
@@ -10,10 +10,6 @@ transaction {
        fail
 
        &bar := "nope"
-
-       actions {
-               fail = 1
-       }
 }
 
 #