attempt to fix #8353

3 files changed, 89 insertions, 85 deletions

diff --git a/src/backend/taler-merchant-httpd_get-orders-ID.c b/src/backend/taler-merchant-httpd_get-orders-ID.c
index aeaba030..6b7bbb9f 100644
--- a/src/backend/taler-merchant-httpd_get-orders-ID.c
+++ b/src/backend/taler-merchant-httpd_get-orders-ID.c
@@ -45,16 +45,16 @@
 enum Phase
 {
   GOP_INIT = 0,
-  GOP_LOOKUP_TERMS,
-  GOP_PARSE_CONTRACT,
-  GOP_CHECK_CLIENT_ACCESS,
-  GOP_REDIRECT_TO_PAID_ORDER,
-  GOP_CHECK_CLAIMED,
-  GOP_CHECK_PAID,
-  GOP_CHECK_REFUNDED,
-  GOP_RETURN_STATUS,
-  GOP_RETURN_MHD_YES,
-  GOP_RETURN_MHD_NO
+  GOP_LOOKUP_TERMS = 1,
+  GOP_PARSE_CONTRACT = 2,
+  GOP_CHECK_CLIENT_ACCESS = 3,
+  GOP_CHECK_PAID = 4,
+  GOP_REDIRECT_TO_PAID_ORDER = 5,
+  GOP_HANDLE_UNPAID = 6,
+  GOP_CHECK_REFUNDED = 7,
+  GOP_RETURN_STATUS = 8,
+  GOP_RETURN_MHD_YES = 9,
+  GOP_RETURN_MHD_NO = 10
 };
 
 
@@ -210,7 +210,12 @@ struct GetOrderData
   bool claimed;
 
   /**
-   * Set to true if this payment has been refunded and
+   * Set to true if this order was paid.
+   */
+  bool paid;
+
+  /**
+   * Set to true if this order has been refunded and
    * @e refund_amount is initialized.
    */
   bool refunded;
@@ -1063,6 +1068,38 @@ send_pay_request (struct GetOrderData *god,
 
 
 /**
+ * Check if the order has been paid.
+ *
+ * @param[in,out] god request context
+ */
+static void
+phase_check_paid (struct GetOrderData *god)
+{
+  enum GNUNET_DB_QueryStatus qs;
+  struct TALER_PrivateContractHashP h_contract;
+
+  god->paid = false;
+  qs = TMH_db->lookup_order_status (
+    TMH_db->cls,
+    god->hc->instance->settings.id,
+    god->order_id,
+    &h_contract,
+    &god->paid);
+  if (0 > qs)
+  {
+    /* Always report on hard error as well to enable diagnostics */
+    GNUNET_break (GNUNET_DB_STATUS_HARD_ERROR == qs);
+    phase_fail (god,
+                MHD_HTTP_INTERNAL_SERVER_ERROR,
+                TALER_EC_GENERIC_DB_FETCH_FAILED,
+                "lookup_order_status");
+    return;
+  }
+  god->phase++;
+}
+
+
+/**
  * Check if the client already paid for an equivalent
  * order under this session, and if so redirect to
  * that order.
@@ -1087,7 +1124,7 @@ phase_redirect_to_paid_order (struct GetOrderData *god)
     char *already_paid_order_id = NULL;
     enum GNUNET_DB_QueryStatus qs;
 
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                 "Running re-purchase detection for %s/%s\n",
                 god->session_id,
                 god->fulfillment_url);
@@ -1096,8 +1133,7 @@ phase_redirect_to_paid_order (struct GetOrderData *god)
       god->hc->instance->settings.id,
       god->fulfillment_url,
       god->session_id,
-      TALER_EXCHANGE_YNA_NO !=
-      god->allow_refunded_for_repurchase,
+      TALER_EXCHANGE_YNA_NO != god->allow_refunded_for_repurchase,
       &already_paid_order_id);
     if (qs < 0)
     {
@@ -1112,9 +1148,10 @@ phase_redirect_to_paid_order (struct GetOrderData *god)
                   "order by fulfillment");
       return false;
     }
-    if ( (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) ||
-         (0 != strcmp (god->order_id,
-                       already_paid_order_id)) )
+    if ( (! god->paid) &&
+         ( (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) ||
+           (0 != strcmp (god->order_id,
+                         already_paid_order_id)) ) )
     {
       bool ret;
 
@@ -1136,31 +1173,6 @@ phase_redirect_to_paid_order (struct GetOrderData *god)
 
 
 /**
- * Check if order is unclaimed, and if so request
- * payment.
- *
- * @param[in,out] god request context
- * @return true to exit due to suspension
- */
-static bool
-phase_check_claimed (struct GetOrderData *god)
-{
-  if (god->claimed)
-  {
-    god->phase++;
-    return false;
-  }
-  /* Order is unclaimed, no need to check for payments or even
-     refunds, simply always generate payment request */
-  GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-              "Order unclaimed, sending pay request for order %s\n",
-              god->order_id);
-  return send_pay_request (god,
-                           NULL);
-}
-
-
-/**
  * Check if the order has been paid, and if not
  * request payment.
  *
@@ -1168,39 +1180,27 @@ phase_check_claimed (struct GetOrderData *god)
  * @return true to exit due to suspension
  */
 static bool
-phase_check_paid (struct GetOrderData *god)
+phase_handle_unpaid (struct GetOrderData *god)
 {
-  enum GNUNET_DB_QueryStatus qs;
-  struct TALER_PrivateContractHashP h_contract;
-  bool paid;
-
-  qs = TMH_db->lookup_order_status (
-    TMH_db->cls,
-    god->hc->instance->settings.id,
-    god->order_id,
-    &h_contract,
-    &paid);
-  if (0 >= qs)
+  if (god->paid)
   {
-    /* Always report on hard error as well to enable diagnostics */
-    GNUNET_break (GNUNET_DB_STATUS_HARD_ERROR == qs);
-    phase_fail (god,
-                MHD_HTTP_INTERNAL_SERVER_ERROR,
-                TALER_EC_GENERIC_DB_FETCH_FAILED,
-                "lookup_order_status");
+    god->phase++;
     return false;
   }
-  GNUNET_break (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs);
-  if (! paid)
+  if (god->claimed)
   {
     GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                 "Order claimed but unpaid, sending pay request for order %s\n",
                 god->order_id);
-    return send_pay_request (god,
-                             NULL);
   }
-  god->phase++;
-  return false;
+  else
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+                "Order unclaimed, sending pay request for order %s\n",
+                god->order_id);
+  }
+  return send_pay_request (god,
+                           NULL);
 }
 
 
@@ -1647,16 +1647,15 @@ TMH_get_orders_ID (const struct TMH_RequestHandler *rh,
     case GOP_CHECK_CLIENT_ACCESS:
       phase_check_client_access (god);
       break;
+    case GOP_CHECK_PAID:
+      phase_check_paid (god);
+      break;
     case GOP_REDIRECT_TO_PAID_ORDER:
       if (phase_redirect_to_paid_order (god))
         return MHD_YES;
       break;
-    case GOP_CHECK_CLAIMED:
-      if (phase_check_claimed (god))
-        return MHD_YES;
-      break;
-    case GOP_CHECK_PAID:
-      if (phase_check_paid (god))
+    case GOP_HANDLE_UNPAID:
+      if (phase_handle_unpaid (god))
         return MHD_YES;
       break;
     case GOP_CHECK_REFUNDED:
diff --git a/src/backenddb/pg_lookup_order_by_fulfillment.c b/src/backenddb/pg_lookup_order_by_fulfillment.c
index 291bd857..e600df32 100644
--- a/src/backenddb/pg_lookup_order_by_fulfillment.c
+++ b/src/backenddb/pg_lookup_order_by_fulfillment.c
@@ -25,13 +25,15 @@
 #include "pg_lookup_order_by_fulfillment.h"
 #include "pg_helper.h"
 
+
 enum GNUNET_DB_QueryStatus
-TMH_PG_lookup_order_by_fulfillment (void *cls,
-                                    const char *instance_id,
-                                    const char *fulfillment_url,
-                                    const char *session_id,
-                                    bool allow_refunded_for_repurchase,
-                                    char **order_id)
+TMH_PG_lookup_order_by_fulfillment (
+  void *cls,
+  const char *instance_id,
+  const char *fulfillment_url,
+  const char *session_id,
+  bool allow_refunded_for_repurchase,
+  char **order_id)
 {
   struct PostgresClosure *pg = cls;
   struct GNUNET_PQ_QueryParam params[] = {
@@ -51,7 +53,7 @@ TMH_PG_lookup_order_by_fulfillment (void *cls,
   PREPARE (pg,
            "lookup_order_by_fulfillment",
            "SELECT"
-           " mct.order_id"
+           "  mct.order_id"
            " FROM merchant_contract_terms mct"
            " LEFT JOIN merchant_refunds mref"
            "   USING (order_serial)"
@@ -62,8 +64,15 @@ TMH_PG_lookup_order_by_fulfillment (void *cls,
            "           FROM merchant_instances"
            "          WHERE merchant_id=$1)"
            "   AND ((CAST($4 AS BOOL)) OR"
-           "        mref.refund_serial IS NULL)");
-
+           "        mref.refund_serial IS NULL)"
+           /* Theoretically, multiple paid orders
+              for the same fulfillment URL could
+              exist for this session_id -- if a
+              wallet was broken and did multiple
+              payments without repurchase detection.
+              So we need to limit to 1 when returning! */
+           " ORDER BY order_id DESC"
+           " LIMIT 1;");
   return GNUNET_PQ_eval_prepared_singleton_select (pg->conn,
                                                    "lookup_order_by_fulfillment",
                                                    params,
diff --git a/src/testing/test_merchant_api.c b/src/testing/test_merchant_api.c
index bfa534d5..80ec3dad 100644
--- a/src/testing/test_merchant_api.c
+++ b/src/testing/test_merchant_api.c
@@ -1601,7 +1601,6 @@ run (void *cls,
       true,
       "post-order-repurchase-original",
       MHD_HTTP_OK),
-#ifdef XFAIL
     TALER_TESTING_cmd_wallet_get_order2 (
       "repurchase-wallet-check-primary-order-refunded",
       merchant_url,
@@ -1612,7 +1611,6 @@ run (void *cls,
       true,
       "post-order-repurchase-original",
       MHD_HTTP_OK),
-#endif
     TALER_TESTING_cmd_merchant_get_order3 (
       "repurchase-check-refunded",
       merchant_url,
@@ -1929,7 +1927,6 @@ run (void *cls,
       merchant_url,
       "product-2",
       MHD_HTTP_CONFLICT),
-#if 1
     TALER_TESTING_cmd_batch ("pay",
                              pay),
     TALER_TESTING_cmd_batch ("double-spending",
@@ -1946,7 +1943,6 @@ run (void *cls,
                              webhooks),
     TALER_TESTING_cmd_batch ("auth",
                              auth),
-#endif
     TALER_TESTING_cmd_batch ("repurchase",
                              repurchase),
     /**