update transaction functionality so it's useful

edits don't fail.  So we update the documentation to match.

grouped edits are atomic only if they're in a transaction.  So we
update the documentation to match.

But since edit statements never fail, the transaction keyword will
never detect that.  So we allow if/else/elsif statements inside of
a transaction.  And update the documentation to match.

The example documentation is also update to note that you have to
explicitly check that the assignment failed

diff --git a/doc/antora/modules/reference/pages/unlang/edit.adoc b/doc/antora/modules/reference/pages/unlang/edit.adoc
index c47c199da30eff2b3347a154100be6da453e1309..471ed4443d23ae5df9012f98ac89d515f28d9640 100644 (file)
--- a/doc/antora/modules/reference/pages/unlang/edit.adoc
+++ b/doc/antora/modules/reference/pages/unlang/edit.adoc
@@ -60,16 +60,14 @@ You _cannot_ convert an `update` section to the new syntax simply by
 removing the `update` keyword.
 ====
 
-== Atomic "Transactions" and Errors
+== Assignment Failures
 
-The edit process is atomic, in that either all of the attributes are
-modified, or none of them are modified.  If the edit fails for any
-reason, then all of the results are discarded, and the edit does not
-affect any attributes.
+If the right side of an edit fails, then the assignment does nothing,
+and the variable named on the left hand side is not updated.
 
-Note also that the server tracks overflows, underflows, and division
-by zero.  These issues are caught, and cause the problematic
-calculation to fail.
+For mathematical expressions, the server tracks overflows, underflows,
+and division by zero.  These issues are caught, and cause the
+problematic calculation to fail.
 
 For example, if an attribute is of type `uint8`, then it can only
 contain 8-bit integers.  Any attempt to overflow the value to more
@@ -86,11 +84,12 @@ and do not result in any changes.
 
 === Grouping Edits
 
-Multiple attributes may be grouped into a set by using the xref:unlang/transaction.adoc[transaction]
-keyword.  When changes are done in a `transaction`, then either all of the
-changes are applied, or none of them are applied.  This functionality
-is best used to conditionally apply attribute changes, generally when
-retrieving data from a database.
+Multiple attributes may be grouped into a set by using the
+xref:unlang/transaction.adoc[transaction] keyword.  When changes are
+done in a `transaction`, then either all of the changes are applied,
+or none of them are applied.  This functionality is best used to
+conditionally apply attribute changes, generally when retrieving data
+from a database.
 
 == List Editing
 

diff --git a/doc/antora/modules/reference/pages/unlang/transaction.adoc b/doc/antora/modules/reference/pages/unlang/transaction.adoc
index 4cee695b8f2a3926b2324a2306fe55c73b3f8cb1..bd3ae5e176963a2ea6b5e0d8143a05c465032fd9 100644 (file)
--- a/doc/antora/modules/reference/pages/unlang/transaction.adoc
+++ b/doc/antora/modules/reference/pages/unlang/transaction.adoc
@@ -15,37 +15,53 @@ been made will be reverted.
 
 [ statements ]:: The `unlang` commands which will be executed.
 
-== Caveats
+A `transaction` section should only contain attribute editing which is
+done via the xref:unlang/edit.adoc[edit] statements, or conditions
+(xref:unlang/edit.adoc[if] / xref:unlang/edit.adoc[else] /
+xref:unlang/edit.adoc[elsif]).  It can also contain
+xref:unlang/return_codes.adoc[rcodes] such as `fail` or `ok`.
 
-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.
+== Caveats
 
-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.
+An Unlang `transaction` cannot undo operations on external databases.
+For example, any data which was inserted into `sql` during a
+`transaction` statement will remain in `sql` even if the Unlang
+`transaction` fails.  This is because Unlang transactions are
+independent of any database transactions.
 
-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`.
+The `transaction` keyword sets its own return codes for `fail`,
+`invalid`, and `disallow` to be set the the priority `1`, instead of
+the default `return`.  This behavior means that a failed `transaction`
+will cause the interpreter to proceed to the next instruction, instead
+of returning.
 
 .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`.
+statements inside of the `transaction` section will return `fail`.
+The assignment to `reply.Framed-IP-Address` will then be reverted.
 
 [source,unlang]
 ----
 transaction {
-    reply.Filter-Id := %sql("SELECT ...")
+    string tmp
+    tmp := %sql("SELECT ...")
     reply.Framed-IP-Address := 192.0.2.1
+
+    if !tmp  {
+        fail
+    }
+
+    reply.Filter-Id := tmp
 }
 ----
 
+Note that xref:unlang/edit.adoc[edit] statements do not fail if the
+right hand side expression fails.  Instead, you must manually check if
+the assignment failed.  The example above does this with an
+intermediate variable.
+
 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.
@@ -57,7 +73,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.  None of
+For now, the main purpose of `transaction` is to group edits.  None of
 the modules support transactions.  If a module is used inside of a
 transaction, the server will return an error, and will not start.
 

diff --git a/src/lib/unlang/transaction.c b/src/lib/unlang/transaction.c
index 20265fdd4955ea311a78562dfb55a6812ece779e..92710f9088791404d3a26182f0b6b9fa618a8676 100644 (file)
--- a/src/lib/unlang/transaction.c
+++ b/src/lib/unlang/transaction.c
@@ -166,12 +166,15 @@ static bool transaction_ok(CONF_SECTION *cs)
                        subcs = cf_item_to_section(ci);
                        name = cf_section_name1(subcs);
 
-                       if (strcmp(name, "actions") == 0) continue;
-
                        /*
-                        *      Definitely an attribute editing thing.
+                        *      Allow limited keywords.
                         */
-                       if (*name == '&') continue;
+                       if ((strcmp(name, "actions") == 0) ||
+                           (strcmp(name, "if") == 0) ||
+                           (strcmp(name, "else") == 0) ||
+                           (strcmp(name, "elsif") == 0)) {
+                               continue;
+                       }
 
                        if (fr_list_assignment_op[cf_section_name2_quote(cs)]) continue;
 
@@ -195,8 +198,6 @@ static bool transaction_ok(CONF_SECTION *cs)
                         */
                        if (cf_pair_value(cp)) continue;
 
-                       if (*name == '&') continue;
-
                        /*
                         *      Allow rcodes via the "always" module.
                         */

diff --git a/src/tests/keywords/transaction-if b/src/tests/keywords/transaction-if
new file mode 100644 (file)
index 0000000..608b480
--- /dev/null
+++ b/src/tests/keywords/transaction-if
@@ -0,0 +1,35 @@
+#
+# PRE: transaction
+#
+string foo
+string bar
+
+transaction {
+       #
+       #  Bad argument to base64 decode, so it fails and doesn't return anything.
+       #
+       foo := %base64.decode('$$$')
+       bar := "nope"
+
+       if !foo {
+               fail
+       }       
+}
+
+#
+#  This should be rolled back.
+#
+if bar {
+       test_fail
+}
+
+#
+#  This should get rolled back, too!
+#
+if foo {
+       test_fail
+} else {
+       ok              # force auth success for the test framework
+}
+
+success

diff --git a/src/tests/keywords/xlat-alternation-with-func b/src/tests/keywords/xlat-alternation-with-func
index fc56ffc4b9928e0d66c0711bd8ce5635525b94ac..a2a31a16c76f982e910c01a468a7d99b8f55c137 100644 (file)
--- a/src/tests/keywords/xlat-alternation-with-func
+++ b/src/tests/keywords/xlat-alternation-with-func
@@ -8,8 +8,7 @@ string dummy_string
 test_string1 := "foo"
 test_string2 := "bar"
 
-
-if (!(%{%test.passthrough(%{dummy_string}) || %{test_string2}} == 'bar')) {
+if (!(%{%test.passthrough(dummy_string) || test_string2} == 'bar')) {
        test_fail
 }