commit d67d0a862d7a23e7d9a3f15d0920b1ba6450d7a6
parent 3bca078e5fe5730303f4bb32ab1b886c912748c2
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 5 Aug 2026 23:56:48 +0200
address FIXME: make order deletion atomic
Diffstat:
4 files changed, 211 insertions(+), 32 deletions(-)
diff --git a/src/backenddb/delete_order.c b/src/backenddb/delete_order.c
@@ -38,44 +38,33 @@ TALER_MERCHANTDB_delete_order (
GNUNET_PQ_query_param_bool (force),
GNUNET_PQ_query_param_end
};
- struct GNUNET_PQ_QueryParam params2[] = {
- GNUNET_PQ_query_param_string (order_id),
- GNUNET_PQ_query_param_end
+ bool deleted;
+ struct GNUNET_PQ_ResultSpec rs[] = {
+ GNUNET_PQ_result_spec_bool ("deleted",
+ &deleted),
+ GNUNET_PQ_result_spec_end
};
enum GNUNET_DB_QueryStatus qs;
- enum GNUNET_DB_QueryStatus qs2;
GNUNET_assert (NULL != pg->current_merchant_id);
GNUNET_assert (0 == strcmp (instance_id,
pg->current_merchant_id));
- // FIXME: replace two statements by stored procedure!
+ /* Note: this MUST be a single stored procedure: deleting the order and
+ the associated contract terms in two separate statements would run in
+ two separate transactions (the caller does not open one), and a failure
+ between them would orphan the contract terms. */
TMH_PQ_prepare_anon (pg,
- "WITH mc AS"
- "(SELECT paid"
- " FROM merchant_contract_terms"
- " WHERE order_id=$1) "
- "DELETE"
- " FROM merchant_orders mo"
- " WHERE order_id=$1"
- " AND ( (pay_deadline < $2)"
- " OR (NOT EXISTS (SELECT paid FROM mc))"
- " OR ($3 AND (FALSE=(SELECT paid FROM mc))) );");
- qs = GNUNET_PQ_eval_prepared_non_select (pg->conn,
- "",
- params);
- if ( (qs < 0) || (! force) )
+ "SELECT"
+ " out_deleted AS deleted"
+ " FROM merchant_do_delete_order"
+ "($1,$2,$3);");
+ qs = GNUNET_PQ_eval_prepared_singleton_select (pg->conn,
+ "",
+ params,
+ rs);
+ if (qs <= 0)
return qs;
- TMH_PQ_prepare_anon (pg,
- "DELETE"
- " FROM merchant_contract_terms"
- " WHERE order_id=$1"
- " AND NOT paid;");
- qs2 = GNUNET_PQ_eval_prepared_non_select (pg->conn,
- "",
- params2);
- if (qs2 < 0)
- return qs2;
- if (qs2 > 0)
- return qs2;
- return qs;
+ return deleted
+ ? GNUNET_DB_STATUS_SUCCESS_ONE_RESULT
+ : GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
}
diff --git a/src/backenddb/delete_order.sql b/src/backenddb/delete_order.sql
@@ -0,0 +1,70 @@
+--
+-- This file is part of TALER
+-- Copyright (C) 2026 Taler Systems SA
+--
+-- TALER is free software; you can redistribute it and/or modify it under the
+-- terms of the GNU General Public License as published by the Free Software
+-- Foundation; either version 3, or (at your option) any later version.
+--
+-- TALER is distributed in the hope that it will be useful, but WITHOUT ANY
+-- WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+-- A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+--
+-- You should have received a copy of the GNU General Public License along with
+-- TALER; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+--
+
+DROP FUNCTION IF EXISTS merchant_do_delete_order;
+CREATE FUNCTION merchant_do_delete_order (
+ IN in_order_id TEXT,
+ IN in_now INT8,
+ IN in_force BOOLEAN,
+ OUT out_deleted BOOLEAN)
+LANGUAGE plpgsql
+AS $$
+DECLARE
+ my_paid BOOLEAN;
+ my_claimed BOOLEAN;
+BEGIN
+
+-- An order that was claimed by a wallet has a row in
+-- merchant_contract_terms; the row in merchant_orders (if any) then
+-- merely holds the not-yet-claimed proposal. Both deletions must
+-- happen together, or a failure between them would leave the contract
+-- terms behind without the order. Doing both in one stored procedure
+-- gives us that atomicity even though the caller runs in autocommit.
+SELECT paid
+ INTO my_paid
+ FROM merchant_contract_terms
+ WHERE order_id=in_order_id
+ FOR UPDATE;
+my_claimed = FOUND;
+
+DELETE FROM merchant_orders
+ WHERE order_id=in_order_id
+ AND ( (pay_deadline < in_now)
+ OR (NOT my_claimed)
+ OR (in_force AND (NOT my_paid)) );
+out_deleted = FOUND;
+
+IF NOT in_force
+THEN
+ RETURN;
+END IF;
+
+DELETE FROM merchant_contract_terms
+ WHERE order_id=in_order_id
+ AND NOT paid;
+IF FOUND
+THEN
+ out_deleted = TRUE;
+END IF;
+
+END $$;
+
+COMMENT ON FUNCTION merchant_do_delete_order(TEXT, INT8, BOOLEAN)
+ IS 'Deletes an order. The unclaimed proposal is removed if the pay deadline'
+ ' has passed, if the order was never claimed, or (with in_force) if it'
+ ' was claimed but not paid. With in_force the claimed-but-unpaid'
+ ' contract terms are removed in the same transaction. Returns'
+ ' out_deleted=TRUE if anything was deleted.';
diff --git a/src/backenddb/sql-schema/meson.build b/src/backenddb/sql-schema/meson.build
@@ -35,6 +35,7 @@ sql_instance_procedures = [
'../pg_statistics_helpers.sql',
'../pg_do_handle_inventory_changes.sql',
'../pg_do_handle_category_changes.sql',
+ '../delete_order.sql',
'../delete_product.sql',
'../delete_unit.sql',
'../insert_unit.sql',
diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c
@@ -3333,6 +3333,113 @@ test_delete_order (const struct InstanceData *instance,
/**
+ * Runs @a sql directly on the database connection of the test. Used to
+ * install the fault-injection trigger of #test_delete_order_atomic().
+ *
+ * @param sql the SQL statement(s) to execute
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+orders_exec_sql (const char *sql)
+{
+ struct GNUNET_PQ_ExecuteStatement es[] = {
+ GNUNET_PQ_make_execute (sql),
+ GNUNET_PQ_EXECUTE_STATEMENT_END
+ };
+
+ TEST_COND_RET_ON_FAIL (GNUNET_OK ==
+ GNUNET_PQ_exec_statements (pg->conn,
+ es),
+ "Direct SQL execution failed\n");
+ return 0;
+}
+
+
+/**
+ * Tests that a forced order deletion removes the row in
+ * merchant_orders and the row in merchant_contract_terms atomically.
+ *
+ * The caller of TALER_MERCHANTDB_delete_order() does not open a
+ * transaction, so doing this in two statements would run them in two
+ * separate autocommit transactions: the first would stay committed
+ * when the second one fails, leaving orphaned contract terms behind.
+ * We provoke exactly that by installing a trigger that makes the
+ * deletion of the contract terms fail, and then check that the order
+ * itself survived.
+ *
+ * @param instance the instance to delete the order from.
+ * @param order the order to delete; must be claimed and unpaid.
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+test_delete_order_atomic (const struct InstanceData *instance,
+ const struct OrderData *order)
+{
+ struct TALER_MerchantPostDataHashP h_post_data;
+
+ TEST_SET_INSTANCE (instance->instance.id,
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT);
+ TEST_RET_ON_FAIL (orders_exec_sql (
+ "CREATE FUNCTION test_block_ct_delete ()"
+ " RETURNS TRIGGER"
+ " LANGUAGE plpgsql"
+ " AS $$"
+ " BEGIN"
+ " RAISE EXCEPTION"
+ " 'simulated failure deleting contract terms';"
+ " END $$;"
+ "CREATE TRIGGER test_block_ct_delete_trigger"
+ " BEFORE DELETE ON merchant_contract_terms"
+ " FOR EACH ROW"
+ " EXECUTE FUNCTION test_block_ct_delete ();"));
+ TEST_COND_RET_ON_FAIL (GNUNET_DB_STATUS_HARD_ERROR ==
+ TALER_MERCHANTDB_delete_order (pg,
+ instance->instance.id,
+ order->id,
+ true),
+ "Deleting the order should have failed\n");
+ TEST_RET_ON_FAIL (orders_exec_sql (
+ "DROP TRIGGER test_block_ct_delete_trigger"
+ " ON merchant_contract_terms;"
+ "DROP FUNCTION test_block_ct_delete ();"));
+ TEST_COND_RET_ON_FAIL (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT ==
+ TALER_MERCHANTDB_get_order (pg,
+ instance->instance.id,
+ order->id,
+ NULL,
+ &h_post_data,
+ NULL),
+ "Order was deleted even though deleting its"
+ " contract terms failed\n");
+ /* Without the trigger in the way, both rows must now go. */
+ TEST_COND_RET_ON_FAIL (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT ==
+ TALER_MERCHANTDB_delete_order (pg,
+ instance->instance.id,
+ order->id,
+ true),
+ "Forced deletion of a claimed, unpaid order failed\n");
+ TEST_COND_RET_ON_FAIL (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS ==
+ TALER_MERCHANTDB_get_order (pg,
+ instance->instance.id,
+ order->id,
+ NULL,
+ &h_post_data,
+ NULL),
+ "Order survived its deletion\n");
+ TEST_COND_RET_ON_FAIL (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS ==
+ TALER_MERCHANTDB_get_contract_terms (
+ pg,
+ instance->instance.id,
+ order->id,
+ NULL,
+ NULL,
+ NULL),
+ "Contract terms survived the order deletion\n");
+ return 0;
+}
+
+
+/**
* Test inserting contract terms for an order.
*
* @param instance the instance.
@@ -3995,6 +4102,18 @@ run_test_orders (struct TestOrders_Closure *cls)
TEST_RET_ON_FAIL (test_delete_order (&cls->instance,
&cls->orders[2],
GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+
+ /* Deleting an order and its contract terms must be atomic.
+ orders[1] was fully removed above, so we can re-create it, this
+ time claimed (=> with contract terms) and unpaid. */
+ TEST_RET_ON_FAIL (test_insert_order (&cls->instance,
+ &cls->orders[1],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ TEST_RET_ON_FAIL (test_insert_contract_terms (&cls->instance,
+ &cls->orders[1],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ TEST_RET_ON_FAIL (test_delete_order_atomic (&cls->instance,
+ &cls->orders[1]));
return 0;
}
