minor bugfixes in /track/transfer logic, testcase now passes

4 files changed, 201 insertions, 38 deletions

diff --git a/src/backend/taler-merchant-httpd_track-transaction.c b/src/backend/taler-merchant-httpd_track-transaction.c
index da96f781..2d62eb62 100644
--- a/src/backend/taler-merchant-httpd_track-transaction.c
+++ b/src/backend/taler-merchant-httpd_track-transaction.c
@@ -613,7 +613,14 @@ MH_handler_track_transaction (struct TMH_RequestHandler *rh,
                 ret ? "OK" : "FAILED");
     return ret;
   }
-
+  if ( (NULL != tctx->fo) ||
+       (NULL != tctx->eh) )
+  {
+    /* likely old MHD version */
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "Not sure why we are here, should be suspended\n");
+    return MHD_YES; /* still work in progress */
+  }
   str = MHD_lookup_connection_value (connection,
                                      MHD_GET_ARGUMENT_KIND,
                                      "id");
@@ -671,7 +678,7 @@ MH_handler_track_transaction (struct TMH_RequestHandler *rh,
   tctx->timeout_task = GNUNET_SCHEDULER_add_delayed (TRACK_TIMEOUT,
                                                      &handle_track_transaction_timeout,
                                                      tctx);
-  return MHD_NO;
+  return MHD_YES;
 }
 
 
diff --git a/src/backend/taler-merchant-httpd_track-transfer.c b/src/backend/taler-merchant-httpd_track-transfer.c
index 034cc5ae..1e7ef255 100644
--- a/src/backend/taler-merchant-httpd_track-transfer.c
+++ b/src/backend/taler-merchant-httpd_track-transfer.c
@@ -259,6 +259,9 @@ wire_transfer_cb (void *cls,
   int ret;
 
   rctx->wdh = NULL;
+  GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+              "Got response code %u from exchange for /track/transfer\n",
+              http_status);
   if (MHD_HTTP_OK != http_status)
   {
     resume_track_transfer_with_response
@@ -285,7 +288,7 @@ wire_transfer_cb (void *cls,
   {
     rctx->current_detail = &details[i];
     rctx->check_transfer_result = GNUNET_NO;
-    ret = db->find_payments_by_id (rctx,
+    ret = db->find_payments_by_id (db->cls,
                                    details[i].transaction_id,
                                    &check_transfer,
                                    rctx);
@@ -353,6 +356,15 @@ process_track_transfer_with_exchange (void *cls,
                                             &rctx->wtid,
                                             &wire_transfer_cb,
                                             rctx);
+  if (NULL == rctx->wdh)
+  {
+    GNUNET_break (0);
+    resume_track_transfer_with_response
+      (rctx,
+       MHD_HTTP_INTERNAL_SERVER_ERROR,
+       TMH_RESPONSE_make_json_pack ("{s:s}",
+                                    "error", "failed to run /track/transfer on exchange"));
+  }
 }
 
 
@@ -461,6 +473,14 @@ MH_handler_track_transfer (struct TMH_RequestHandler *rh,
                 ret ? "OK" : "FAILED");
     return ret;
   }
+  if ( (NULL != rctx->fo) ||
+       (NULL != rctx->eh) )
+  {
+    /* likely old MHD version */
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "Not sure why we are here, should be suspended\n");
+    return MHD_YES; /* still work in progress */
+  }
 
   uri = MHD_lookup_connection_value (connection,
                                      MHD_GET_ARGUMENT_KIND,
@@ -502,6 +522,10 @@ MH_handler_track_transfer (struct TMH_RequestHandler *rh,
       MHD_destroy_response (rctx->response);
       rctx->response = NULL;
     }
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "Queueing response (%u) for /track/transfer (%s).\n",
+                (unsigned int) rctx->response_code,
+                ret ? "OK" : "FAILED");
     return ret;
   }
 
@@ -515,7 +539,7 @@ MH_handler_track_transfer (struct TMH_RequestHandler *rh,
     = GNUNET_SCHEDULER_add_delayed (TRACK_TIMEOUT,
                                     &handle_track_transfer_timeout,
                                     rctx);
-  return MHD_NO;
+  return MHD_YES;
 }
 
 /* end of taler-merchant-httpd_track-transfer.c */
diff --git a/src/lib/merchant_api_track_transfer.c b/src/lib/merchant_api_track_transfer.c
index 661d2906..61d9e1f0 100644
--- a/src/lib/merchant_api_track_transfer.c
+++ b/src/lib/merchant_api_track_transfer.c
@@ -77,13 +77,13 @@ struct TALER_MERCHANT_TrackTransferHandle
  * Any changes should likely be reflected there as well.
  *
  * @param wdh handle to the operation
- * @param json response we got
+ * @param json response we got as the 'details' from the exchange
  * @return #GNUNET_OK if we are done and all is well,
  *         #GNUNET_SYSERR if the response was bogus
  */
 static int
-check_track_transfer_response_ok (struct TALER_MERCHANT_TrackTransferHandle *wdh,
-                                  const json_t *json)
+parse_exchange_details_ok (struct TALER_MERCHANT_TrackTransferHandle *wdh,
+                           const json_t *json)
 {
   json_t *details_j;
   struct GNUNET_HashCode h_wire;
@@ -91,7 +91,7 @@ check_track_transfer_response_ok (struct TALER_MERCHANT_TrackTransferHandle *wdh
   struct TALER_MerchantPublicKeyP merchant_pub;
   unsigned int num_details;
   struct TALER_ExchangePublicKeyP exchange_pub;
-  struct GNUNET_JSON_Specification spec[] = {
+  struct GNUNET_JSON_Specification inner_spec[] = {
     TALER_JSON_spec_amount ("total", &total_amount),
     GNUNET_JSON_spec_fixed_auto ("merchant_pub", &merchant_pub),
     GNUNET_JSON_spec_fixed_auto ("H_wire", &h_wire),
@@ -102,7 +102,7 @@ check_track_transfer_response_ok (struct TALER_MERCHANT_TrackTransferHandle *wdh
 
   if (GNUNET_OK !=
       GNUNET_JSON_parse (json,
-                         spec,
+                         inner_spec,
                          NULL, NULL))
   {
     GNUNET_break_op (0);
@@ -144,12 +144,70 @@ check_track_transfer_response_ok (struct TALER_MERCHANT_TrackTransferHandle *wdh
              num_details,
              details);
   }
-  TALER_MERCHANT_track_transfer_cancel (wdh);
   return GNUNET_OK;
 }
 
 
 /**
+ * We got a #MHD_HTTP_OK response for the /track/transfer request.
+ * Check that the response is well-formed and if it is, call the
+ * callback.  If not, return an error code.
+ *
+ * This code is very similar to
+ * exchange_api_track_transfer.c::check_track_transfer_response_ok.
+ * (Except we do not check the signature, as that was done by the
+ * backend which we trust already.)
+ * Any changes should likely be reflected there as well.
+ *
+ * @param wdh handle to the operation
+ * @param json response we got
+ * @return #GNUNET_OK if we are done and all is well,
+ *         #GNUNET_SYSERR if the response was bogus
+ */
+static int
+check_track_transfer_response_ok (struct TALER_MERCHANT_TrackTransferHandle *wdh,
+                                  const json_t *json)
+{
+  int ret;
+  json_t *inner_j;
+  uint32_t exchange_status;
+  struct GNUNET_JSON_Specification outer_spec[] = {
+    GNUNET_JSON_spec_json ("details", &inner_j),
+    GNUNET_JSON_spec_uint32 ("exchange_status", &exchange_status),
+    GNUNET_JSON_spec_end()
+  };
+
+  if (GNUNET_OK !=
+      GNUNET_JSON_parse (json,
+                         outer_spec,
+                         NULL, NULL))
+  {
+    GNUNET_break_op (0);
+    return GNUNET_SYSERR;
+  }
+  switch (exchange_status)
+  {
+  case MHD_HTTP_OK:
+    ret = parse_exchange_details_ok (wdh,
+                                     inner_j);
+    break;
+    /* FIXME: other acceptable exchange status codes to handle here?
+       FIXME: implement proper way to pass exchange status vs. backend
+              status code to application! */
+  default:
+    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                "Unexpected exchange status code %u\n",
+                (unsigned int) exchange_status);
+    ret = GNUNET_SYSERR;
+    break;
+  }
+  if (GNUNET_OK == ret)
+    TALER_MERCHANT_track_transfer_cancel (wdh);
+  return ret;
+}
+
+
+/**
  * Function called when we're done processing the
  * HTTP /track/transfer request.
  *
@@ -213,8 +271,8 @@ handle_track_transfer_finished (void *cls,
  * @param backend_uri base URI of the backend
  * @param wtid base32 string indicating a wtid
  * @param exchange base URL of the exchange in charge of returning the wanted information
- * @param tracktransfer_cb the callback to call when a reply for this request is available
- * @param tracktransfer_cb_cls closure for @a contract_cb
+ * @param track_transfer_cb the callback to call when a reply for this request is available
+ * @param track_transfer_cb_cls closure for @a contract_cb
  * @return a handle for this request
  */
 struct TALER_MERCHANT_TrackTransferHandle *
@@ -222,8 +280,8 @@ TALER_MERCHANT_track_transfer (struct GNUNET_CURL_Context *ctx,
                                const char *backend_uri,
                                const struct TALER_WireTransferIdentifierRawP *wtid,
                                const char *exchange_uri,
-                               TALER_MERCHANT_TrackTransferCallback tracktransfer_cb,
-                               void *tracktransfer_cb_cls)
+                               TALER_MERCHANT_TrackTransferCallback track_transfer_cb,
+                               void *track_transfer_cb_cls)
 {
   struct TALER_MERCHANT_TrackTransferHandle *tdo;
   CURL *eh;
@@ -233,8 +291,9 @@ TALER_MERCHANT_track_transfer (struct GNUNET_CURL_Context *ctx,
                                                   sizeof (struct TALER_WireTransferIdentifierRawP));
   tdo = GNUNET_new (struct TALER_MERCHANT_TrackTransferHandle);
   tdo->ctx = ctx;
-  tdo->cb = tracktransfer_cb;
-  tdo->cb_cls = tracktransfer_cb_cls;
+  tdo->cb = track_transfer_cb;
+  tdo->cb_cls = track_transfer_cb_cls;
+  /* TODO: do we need to escape 'exchange_uri' here? */
   GNUNET_asprintf (&tdo->url,
                    "%s/track/transfer?wtid=%s&exchange=%s",
                    backend_uri,
diff --git a/src/lib/test_merchant_api.c b/src/lib/test_merchant_api.c
index 3732649c..6ebb5ff2 100644
--- a/src/lib/test_merchant_api.c
+++ b/src/lib/test_merchant_api.c
@@ -469,6 +469,11 @@ struct Command
       char *check_bank_ref;
 
       /**
+       * #OC_PAY command which we expect in the result.
+       */
+      char *expected_pay_ref;
+
+      /**
        * Handle to a /track/transfer operation
        */
       struct TALER_MERCHANT_TrackTransferHandle *tdo;
@@ -996,18 +1001,79 @@ track_transfer_cb (void *cls,
   struct Command *cmd = &is->commands[is->ip];
 
   cmd->details.track_transfer.tdo = NULL;
-  /* FIXME: properly test result vs. expecations... */
-  if (MHD_HTTP_OK == http_status)
+  if (cmd->expected_response_code != http_status)
   {
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "Ok from /track/transfer handler\n");
-    result = GNUNET_OK;
+    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                "Unexpected response code %u to command %s\n",
+                http_status,
+                cmd->label);
+    json_dumpf (json, stderr, 0);
+    fail (is);
+    return;
   }
-  else
+  switch (http_status)
   {
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "Not Ok from /track/transfer handler\n");
-    result = GNUNET_SYSERR;
+  case MHD_HTTP_OK:
+    {
+      const struct Command *ref;
+      unsigned int i;
+      int found;
+
+      ref = find_command (is,
+                          cmd->details.track_transfer.expected_pay_ref);
+      found = GNUNET_NO;
+      for (i=0;i<details_length;i++)
+      {
+        struct TALER_Amount amount_with_fee;
+        struct TALER_Amount amount_without_fee;
+        struct TALER_Amount deposit_fee;
+        const struct Command *cref;
+        struct TALER_CoinSpendPublicKeyP coin_pub;
+
+        GNUNET_assert (GNUNET_OK ==
+                       TALER_string_to_amount (ref->details.pay.amount_without_fee,
+                                               &amount_without_fee));
+        GNUNET_assert (GNUNET_OK ==
+                       TALER_string_to_amount (ref->details.pay.amount_with_fee,
+                                               &amount_with_fee));
+        GNUNET_assert (GNUNET_OK ==
+                       TALER_amount_subtract (&deposit_fee,
+                                              &amount_with_fee,
+                                              &amount_without_fee));
+
+	cref = find_command (is,
+                             ref->details.pay.coin_ref);
+	switch (cref->oc)
+	{
+	case OC_WITHDRAW_SIGN:
+          GNUNET_CRYPTO_eddsa_key_get_public (&cref->details.reserve_withdraw.coin_priv.eddsa_priv,
+                                              &coin_pub.eddsa_pub);
+	  break;
+	default:
+	  GNUNET_assert (0);
+	}
+
+        if ( (details[i].transaction_id == ref->details.pay.transaction_id) &&
+             (0 == TALER_amount_cmp (&details[i].coin_value,
+                                     &amount_with_fee)) &&
+             (0 == TALER_amount_cmp (&details[i].coin_fee,
+                                     &deposit_fee)) &&
+             (0 == memcmp (&details[i].coin_pub,
+                           &coin_pub,
+                           sizeof (struct TALER_CoinSpendPublicKeyP))) )
+          found = GNUNET_YES;
+      }
+      if (GNUNET_NO == found)
+      {
+        GNUNET_break (0);
+        json_dumpf (json, stderr, 0);
+        fail (is);
+        return;
+      }
+      break;
+    }
+  default:
+    break;
   }
   next_command (is);
 }
@@ -1039,18 +1105,25 @@ track_transaction_cb (void *cls,
   struct Command *cmd = &is->commands[is->ip];
 
   cmd->details.track_transaction.tth = NULL;
-  /* FIXME: properly test result vs. expecations... */
-  if (MHD_HTTP_OK == http_status)
+  if (cmd->expected_response_code != http_status)
   {
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "Ok from /track/transaction handler\n");
-    result = GNUNET_OK;
+    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                "Unexpected response code %u to command %s\n",
+                http_status,
+                cmd->label);
+    json_dumpf (json, stderr, 0);
+    fail (is);
+    return;
   }
-  else
+  /* FIXME: properly test result vs. expecations... */
+  switch (http_status)
   {
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "Not Ok from /track/transaction handler\n");
-    result = GNUNET_SYSERR;
+  case MHD_HTTP_OK:
+    {
+      break;
+    }
+  default:
+    break;
   }
   next_command (is);
 }
@@ -1386,7 +1459,7 @@ interpreter_run (void *cls)
           return;
         }
         cmd->details.pay.transaction_id = transaction_id;
-    }
+      }
 
       TALER_JSON_hash (ref->details.contract.contract,
 		       &h_contract);
@@ -1890,10 +1963,10 @@ run (void *cls)
 
     /* Trace the WTID back to the original transaction */
     { .oc = OC_TRACK_TRANSFER,
-      .label = "track-deposit-1",
+      .label = "track-transfer-1",
       .expected_response_code = MHD_HTTP_OK,
-      .details.track_transfer.check_bank_ref = "check_bank_transfer-499c"
-      /* FIXME: more needed here for actual checking... */
+      .details.track_transfer.check_bank_ref = "check_bank_transfer-499c",
+      .details.track_transfer.expected_pay_ref = "deposit-simple"
     },
 
     /* FIXME: also do reverse test: lookup WTID by transaction ID */