commit f5987b2b67a3951b349be4b6e3e57947b771e425
parent f3b438ab533526bb6bb0d2b76ee940d363a74039
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 5 Aug 2026 18:37:50 +0200
better fix for 500/404 product category issue: ensure failed request do not modify DB, add tests
Diffstat:
9 files changed, 280 insertions(+), 23 deletions(-)
diff --git a/src/backend/taler-merchant-httpd_patch-private-products-PRODUCT_ID.c b/src/backend/taler-merchant-httpd_patch-private-products-PRODUCT_ID.c
@@ -404,10 +404,14 @@ TMH_private_patch_products_ID (
{
char cat_str[24];
+ /* 'no_cat' is the 1-based index into 'cats' of the unknown
+ category; report the category itself to the client. */
+ GNUNET_assert (no_cat > 0);
+ GNUNET_assert (no_cat <= (ssize_t) num_cats);
GNUNET_snprintf (cat_str,
sizeof (cat_str),
"%llu",
- (unsigned long long) no_cat);
+ (unsigned long long) cats[no_cat - 1]);
ret = TALER_MHD_reply_with_error (connection,
MHD_HTTP_NOT_FOUND,
TALER_EC_MERCHANT_GENERIC_CATEGORY_UNKNOWN,
diff --git a/src/backend/taler-merchant-httpd_post-private-products.c b/src/backend/taler-merchant-httpd_post-private-products.c
@@ -408,10 +408,14 @@ TMH_private_post_products (const struct TMH_RequestHandler *rh,
char nocats[24];
GNUNET_break_op (0);
+ /* 'no_cat' is the 1-based index into 'cats' of the unknown
+ category; report the category itself to the client. */
+ GNUNET_assert (no_cat > 0);
+ GNUNET_assert (no_cat <= (ssize_t) num_cats);
GNUNET_snprintf (nocats,
sizeof (nocats),
"%llu",
- (unsigned long long) no_cat);
+ (unsigned long long) cats[no_cat - 1]);
ret = TALER_MHD_reply_with_error (
connection,
MHD_HTTP_NOT_FOUND,
diff --git a/src/backenddb/insert_product.sql b/src/backenddb/insert_product.sql
@@ -76,6 +76,23 @@ THEN
END IF;
END IF;
+-- Check all categories exist. Must happen before we modify anything,
+-- as we do not roll back on the out_no_cat return path.
+FOR i IN 1..COALESCE(array_length(ina_categories,1),0)
+LOOP
+ ini_cat=ina_categories[i];
+
+ PERFORM
+ FROM merchant_categories
+ WHERE category_serial=ini_cat;
+ IF NOT FOUND
+ THEN
+ out_no_cat=i;
+ out_conflict=FALSE;
+ RETURN;
+ END IF;
+END LOOP;
+
INSERT INTO merchant_inventory
(product_id
,product_name
@@ -203,15 +220,6 @@ FOR i IN 1..COALESCE(array_length(ina_categories,1),0)
LOOP
ini_cat=ina_categories[i];
- PERFORM
- FROM merchant_categories
- WHERE category_serial=ini_cat;
- IF NOT FOUND
- THEN
- out_no_cat=i;
- RETURN;
- END IF;
-
INSERT INTO merchant_product_categories
(product_serial
,category_serial)
diff --git a/src/backenddb/update_product.sql b/src/backenddb/update_product.sql
@@ -112,6 +112,23 @@ THEN
out_lost_reduced=TRUE;
RETURN;
END IF;
+
+-- Check all categories exist. Must happen before we modify anything,
+-- as we do not roll back on the out_no_cat return path.
+FOR i IN 1..COALESCE(array_length(ina_categories,1),0)
+LOOP
+ ini_cat=ina_categories[i];
+
+ PERFORM
+ FROM merchant_categories
+ WHERE category_serial=ini_cat;
+ IF NOT FOUND
+ THEN
+ out_no_cat=i;
+ RETURN;
+ END IF;
+END LOOP;
+
IF rec.allow_fractional_quantity
AND (NOT in_allow_fractional_quantity)
THEN
@@ -130,15 +147,6 @@ FOR i IN 1..COALESCE(array_length(ina_categories,1),0)
LOOP
ini_cat=ina_categories[i];
- PERFORM
- FROM merchant_categories
- WHERE category_serial=ini_cat;
- IF NOT FOUND
- THEN
- out_no_cat=i;
- RETURN;
- END IF;
-
INSERT INTO merchant_product_categories
(product_serial
,category_serial)
diff --git a/src/include/taler/taler_merchant_testing_lib.h b/src/include/taler/taler_merchant_testing_lib.h
@@ -635,6 +635,34 @@ TALER_TESTING_cmd_merchant_patch_product_with_unit_prices (
/**
+ * Define a "PATCH /products/$ID" CMD with category IDs.
+ *
+ * @param label command label.
+ * @param merchant_url base URL of the merchant serving the
+ * PATCH /product request.
+ * @param product_id the ID of the product to update
+ * @param description name of the product
+ * @param unit unit of measurement
+ * @param price price of the product
+ * @param num_cats length of the @a cats array
+ * @param cats array of category IDs
+ * @param http_status expected HTTP response code.
+ * @return the command.
+ */
+struct TALER_TESTING_Command
+TALER_TESTING_cmd_merchant_patch_product_with_categories (
+ const char *label,
+ const char *merchant_url,
+ const char *product_id,
+ const char *description,
+ const char *unit,
+ const char *price,
+ unsigned int num_cats,
+ const uint64_t *cats,
+ unsigned int http_status);
+
+
+/**
* Define a "GET /products" CMD.
*
* @param label command label.
diff --git a/src/lib/merchant_api_patch-private-products-PRODUCT_ID.c b/src/lib/merchant_api_patch-private-products-PRODUCT_ID.c
@@ -426,6 +426,7 @@ TALER_MERCHANT_patch_private_product_start (
TALER_MERCHANT_PATCH_PRIVATE_PRODUCT_RESULT_CLOSURE *cb_cls)
{
json_t *req_obj;
+ json_t *categories;
CURL *eh;
char unit_total_stock_buf[64];
@@ -449,6 +450,20 @@ TALER_MERCHANT_patch_private_product_start (
unit_total_stock_buf,
sizeof (unit_total_stock_buf));
+ if (0 == pph->num_cats)
+ {
+ categories = NULL;
+ }
+ else
+ {
+ categories = json_array ();
+ GNUNET_assert (NULL != categories);
+ for (unsigned int i = 0; i < pph->num_cats; i++)
+ GNUNET_assert (0 ==
+ json_array_append_new (categories,
+ json_integer (pph->cats[i])));
+ }
+
req_obj = GNUNET_JSON_PACK (
GNUNET_JSON_pack_string ("product_name",
(NULL == pph->product_name)
@@ -458,6 +473,9 @@ TALER_MERCHANT_patch_private_product_start (
pph->description),
GNUNET_JSON_pack_object_incref ("description_i18n",
pph->description_i18n),
+ GNUNET_JSON_pack_allow_null (
+ GNUNET_JSON_pack_array_steal ("categories",
+ categories)),
GNUNET_JSON_pack_string ("unit",
pph->unit),
TALER_JSON_pack_amount_array ("unit_price",
@@ -473,8 +491,9 @@ TALER_MERCHANT_patch_private_product_start (
pph->total_lost),
GNUNET_JSON_pack_object_incref ("address",
pph->address),
- GNUNET_JSON_pack_timestamp ("next_restock",
- pph->next_restock));
+ GNUNET_JSON_pack_allow_null (
+ GNUNET_JSON_pack_timestamp ("next_restock",
+ pph->next_restock)));
if (pph->have_unit_allow_fraction &&
pph->unit_allow_fraction)
{
diff --git a/src/testing/test_merchant_api.c b/src/testing/test_merchant_api.c
@@ -2932,6 +2932,128 @@ run (void *cls,
TALER_TESTING_cmd_end ()
};
+ /* Products with categories. Runs last, as it adds a category and
+ products which would otherwise show up in the category- and
+ product-based expectations of the 'templates' batch. */
+ struct TALER_TESTING_Command product_categories[] = {
+ /* 3rd category: the 'templates' batch already created two. */
+ TALER_TESTING_cmd_merchant_post_categories ("post-category-pc",
+ merchant_url,
+ "Category Products",
+ json_pack ("{s:s}",
+ "en",
+ "Category Products"),
+ 3,
+ MHD_HTTP_OK),
+ /* Unknown category must yield 404, not a DB failure (500). */
+ TALER_TESTING_cmd_merchant_post_products_with_categories (
+ "post-products-pc-cat-nx",
+ merchant_url,
+ "pc-product-nx",
+ "product with unknown category",
+ "test-unit",
+ "EUR:3",
+ 1,
+ (const uint64_t[]) { 424242 },
+ false,
+ 0,
+ MHD_HTTP_NOT_FOUND),
+ /* Category serials are positive; 0 is malformed. */
+ TALER_TESTING_cmd_merchant_post_products_with_categories (
+ "post-products-pc-cat-zero",
+ merchant_url,
+ "pc-product-zero",
+ "product with zero category",
+ "test-unit",
+ "EUR:3",
+ 1,
+ (const uint64_t[]) { 0 },
+ false,
+ 0,
+ MHD_HTTP_BAD_REQUEST),
+ TALER_TESTING_cmd_merchant_get_product (
+ "get-product-pc-nx",
+ merchant_url,
+ "pc-product-nx",
+ MHD_HTTP_NOT_FOUND,
+ NULL),
+ TALER_TESTING_cmd_merchant_post_products_with_categories (
+ "post-products-pc",
+ merchant_url,
+ "pc-product",
+ "product with category",
+ "test-unit",
+ "EUR:3",
+ 1,
+ (const uint64_t[]) { 3 },
+ false,
+ 0,
+ MHD_HTTP_NO_CONTENT),
+ /* Same for PATCH: unknown category must yield 404, not 500. The
+ rejected update must not have been applied either, hence the
+ deviating description and the GET checking against the POST. */
+ TALER_TESTING_cmd_merchant_patch_product_with_categories (
+ "patch-products-pc-cat-nx",
+ merchant_url,
+ "pc-product",
+ "product with unapplied update",
+ "test-unit",
+ "EUR:3",
+ 1,
+ (const uint64_t[]) { 424242 },
+ MHD_HTTP_NOT_FOUND),
+ TALER_TESTING_cmd_merchant_get_product (
+ "get-product-pc-unchanged",
+ merchant_url,
+ "pc-product",
+ MHD_HTTP_OK,
+ "post-products-pc"),
+ TALER_TESTING_cmd_merchant_patch_product_with_categories (
+ "patch-products-pc-cat-zero",
+ merchant_url,
+ "pc-product",
+ "product with unapplied update",
+ "test-unit",
+ "EUR:3",
+ 1,
+ (const uint64_t[]) { 0 },
+ MHD_HTTP_BAD_REQUEST),
+ TALER_TESTING_cmd_merchant_get_product (
+ "get-product-pc-unchanged-2",
+ merchant_url,
+ "pc-product",
+ MHD_HTTP_OK,
+ "post-products-pc"),
+ /* Duplicate categories are de-duplicated, not rejected. */
+ TALER_TESTING_cmd_merchant_patch_product_with_categories (
+ "patch-products-pc-cat-dup",
+ merchant_url,
+ "pc-product",
+ "product with category",
+ "test-unit",
+ "EUR:3",
+ 2,
+ (const uint64_t[]) { 3, 3 },
+ MHD_HTTP_NO_CONTENT),
+ TALER_TESTING_cmd_merchant_patch_product_with_categories (
+ "patch-products-pc-cat-ok",
+ merchant_url,
+ "pc-product",
+ "product with category",
+ "test-unit",
+ "EUR:3",
+ 1,
+ (const uint64_t[]) { 3 },
+ MHD_HTTP_NO_CONTENT),
+ TALER_TESTING_cmd_merchant_get_product (
+ "get-product-pc",
+ merchant_url,
+ "pc-product",
+ MHD_HTTP_OK,
+ "patch-products-pc-cat-ok"),
+ TALER_TESTING_cmd_end ()
+ };
+
struct TALER_TESTING_Command commands[] = {
/* general setup */
TALER_TESTING_cmd_run_fakebank (
@@ -3302,6 +3424,8 @@ run (void *cls,
tokens),
TALER_TESTING_cmd_batch ("donau",
donau),
+ TALER_TESTING_cmd_batch ("product-categories",
+ product_categories),
TALER_TESTING_cmd_merchant_get_statisticsamount ("stats-refund",
merchant_url,
"refunds-granted",
diff --git a/src/testing/testing_api_cmd_patch_product.c b/src/testing/testing_api_cmd_patch_product.c
@@ -153,6 +153,16 @@ struct PatchProductState
struct GNUNET_TIME_Timestamp next_restock;
/**
+ * Categories array length.
+ */
+ unsigned int num_cats;
+
+ /**
+ * Categories array.
+ */
+ uint64_t *cats;
+
+ /**
* Expected HTTP response code.
*/
unsigned int http_status;
@@ -267,6 +277,8 @@ patch_product_cb (struct PatchProductState *pis,
{
case MHD_HTTP_NO_CONTENT:
break;
+ case MHD_HTTP_BAD_REQUEST:
+ break;
case MHD_HTTP_UNAUTHORIZED:
break;
case MHD_HTTP_FORBIDDEN:
@@ -318,7 +330,9 @@ patch_product_run (void *cls,
TALER_MERCHANT_patch_private_product_option_total_stock_val (pis->total_stock),
TALER_MERCHANT_patch_private_product_option_total_lost (pis->total_lost),
TALER_MERCHANT_patch_private_product_option_address (pis->address),
- TALER_MERCHANT_patch_private_product_option_next_restock (pis->next_restock)));
+ TALER_MERCHANT_patch_private_product_option_next_restock (pis->next_restock),
+ TALER_MERCHANT_patch_private_product_option_categories (pis->num_cats,
+ pis->cats)));
GNUNET_assert (NULL != pis->iph);
if (pis->use_fractional)
{
@@ -411,6 +425,7 @@ patch_product_cleanup (void *cls,
}
if (pis->owns_unit_prices)
GNUNET_free (pis->unit_prices);
+ GNUNET_free (pis->cats);
json_decref (pis->description_i18n);
GNUNET_free (pis->image);
json_decref (pis->taxes);
@@ -584,4 +599,49 @@ TALER_TESTING_cmd_merchant_patch_product_with_unit_prices (
}
+struct TALER_TESTING_Command
+TALER_TESTING_cmd_merchant_patch_product_with_categories (
+ const char *label,
+ const char *merchant_url,
+ const char *product_id,
+ const char *description,
+ const char *unit,
+ const char *price,
+ unsigned int num_cats,
+ const uint64_t *cats,
+ unsigned int http_status)
+{
+ struct TALER_TESTING_Command cmd;
+
+ cmd = TALER_TESTING_cmd_merchant_patch_product (
+ label,
+ merchant_url,
+ product_id,
+ description,
+ json_pack ("{s:s}", "en", description),
+ unit,
+ price,
+ "",
+ json_array (),
+ 4, /* total stock */
+ 0, /* total lost */
+ json_pack ("{s:s}", "street", "my street"),
+ GNUNET_TIME_UNIT_ZERO_TS,
+ http_status);
+ {
+ struct PatchProductState *pps = cmd.cls;
+
+ pps->num_cats = num_cats;
+ if (0 < num_cats)
+ {
+ pps->cats = GNUNET_new_array (num_cats,
+ uint64_t);
+ for (unsigned int i = 0; i < num_cats; i++)
+ pps->cats[i] = cats[i];
+ }
+ }
+ return cmd;
+}
+
+
/* end of testing_api_cmd_patch_product.c */
diff --git a/src/testing/testing_api_cmd_post_products.c b/src/testing/testing_api_cmd_post_products.c
@@ -270,6 +270,8 @@ post_products_cb (struct PostProductsState *pis,
{
case MHD_HTTP_NO_CONTENT:
break;
+ case MHD_HTTP_BAD_REQUEST:
+ break;
case MHD_HTTP_UNAUTHORIZED:
break;
case MHD_HTTP_FORBIDDEN:
