sanity in filter logic for pg_lookup_trnasfers

2 files changed, 111 insertions, 319 deletions

diff --git a/src/backenddb/pg_lookup_transfers.c b/src/backenddb/pg_lookup_transfers.c
index 782bb43f..a2d22719 100644
--- a/src/backenddb/pg_lookup_transfers.c
+++ b/src/backenddb/pg_lookup_transfers.c
@@ -1,6 +1,6 @@
 /*
    This file is part of TALER
-   Copyright (C) 2022 Taler Systems SA
+   Copyright (C) 2022-2024 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
@@ -25,7 +25,6 @@
 #include "pg_lookup_transfers.h"
 #include "pg_helper.h"
 
-/* FIXME: filter-logic can be written more cleanly, with less duplication! */
 
 /**
  * Closure for #lookup_transfers_cb().
@@ -52,10 +51,6 @@ struct LookupTransfersContext
    */
   enum GNUNET_DB_QueryStatus qs;
 
-  /**
-   * Filter to apply by verification status.
-   */
-  enum TALER_EXCHANGE_YesNoAll verified;
 };
 
 
@@ -81,10 +76,9 @@ lookup_transfers_cb (void *cls,
     char *payto_uri;
     char *exchange_url;
     uint64_t transfer_serial_id;
-    struct GNUNET_TIME_Timestamp execution_time;
-    enum TALER_EXCHANGE_YesNoAll verified;
-    uint8_t verified8;
-    uint8_t confirmed8;
+    struct GNUNET_TIME_Timestamp execution_time = GNUNET_TIME_UNIT_FOREVER_TS;
+    bool verified;
+    bool confirmed;
     struct GNUNET_PQ_ResultSpec rs[] = {
       TALER_PQ_result_spec_amount_with_currency ("credit_amount",
                                                  &credit_amount),
@@ -96,12 +90,14 @@ lookup_transfers_cb (void *cls,
                                     &exchange_url),
       GNUNET_PQ_result_spec_uint64 ("credit_serial",
                                     &transfer_serial_id),
-      GNUNET_PQ_result_spec_timestamp ("execution_time",
-                                       &execution_time),
-      GNUNET_PQ_result_spec_auto_from_type ("verified",
-                                            &verified8),
-      GNUNET_PQ_result_spec_auto_from_type ("confirmed",
-                                            &confirmed8),
+      GNUNET_PQ_result_spec_allow_null (
+        GNUNET_PQ_result_spec_timestamp ("execution_time",
+                                         &execution_time),
+        NULL),
+      GNUNET_PQ_result_spec_bool ("verified",
+                                  &verified),
+      GNUNET_PQ_result_spec_bool ("confirmed",
+                                  &confirmed),
       GNUNET_PQ_result_spec_end
     };
 
@@ -114,23 +110,15 @@ lookup_transfers_cb (void *cls,
       ltc->qs = GNUNET_DB_STATUS_HARD_ERROR;
       return;
     }
-    if (0 == verified8)
-      verified = TALER_EXCHANGE_YNA_NO;
-    else
-      verified = TALER_EXCHANGE_YNA_YES;
-    if ( (ltc->verified == TALER_EXCHANGE_YNA_ALL) ||
-         (ltc->verified == verified) )
-    {
-      ltc->cb (ltc->cb_cls,
-               &credit_amount,
-               &wtid,
-               payto_uri,
-               exchange_url,
-               transfer_serial_id,
-               execution_time,
-               TALER_EXCHANGE_YNA_YES == verified,
-               0 != confirmed8);
-    }
+    ltc->cb (ltc->cb_cls,
+             &credit_amount,
+             &wtid,
+             payto_uri,
+             exchange_url,
+             transfer_serial_id,
+             execution_time,
+             verified,
+             confirmed);
     GNUNET_PQ_cleanup_result (rs);
   }
   ltc->qs = num_results;
@@ -151,298 +139,102 @@ TMH_PG_lookup_transfers (void *cls,
 {
   struct PostgresClosure *pg = cls;
   uint64_t plimit = (uint64_t) ((limit < 0) ? -limit : limit);
+  bool by_time = ( (! GNUNET_TIME_absolute_is_never (before.abs_time)) ||
+                   (! GNUNET_TIME_absolute_is_zero (after.abs_time)) );
   struct LookupTransfersContext ltc = {
     .cb = cb,
     .cb_cls = cb_cls,
-    .pg = pg,
-    .verified = verified
+    .pg = pg
+  };
+  struct GNUNET_PQ_QueryParam params[] = {
+    GNUNET_PQ_query_param_string (instance_id),
+    GNUNET_PQ_query_param_timestamp (&before),
+    GNUNET_PQ_query_param_timestamp (&after),
+    GNUNET_PQ_query_param_uint64 (&offset),
+    GNUNET_PQ_query_param_uint64 (&plimit),
+    NULL == payto_uri
+    ? GNUNET_PQ_query_param_null () /* NULL: do not filter by payto URI */
+    : GNUNET_PQ_query_param_string (payto_uri),
+    GNUNET_PQ_query_param_bool (! by_time),     /* $7: filter by time? */
+    GNUNET_PQ_query_param_bool (TALER_EXCHANGE_YNA_ALL == verified), /* filter by verified? */
+    GNUNET_PQ_query_param_bool (TALER_EXCHANGE_YNA_YES == verified),
+    GNUNET_PQ_query_param_end
   };
   enum GNUNET_DB_QueryStatus qs;
-  bool by_time;
 
-  by_time = ( (! GNUNET_TIME_absolute_is_never (before.abs_time)) ||
-              (! GNUNET_TIME_absolute_is_zero (after.abs_time)) );
   check_connection (pg);
-  if (by_time)
-  {
-    if (NULL != payto_uri)
-    {
-      struct GNUNET_PQ_QueryParam params[] = {
-        GNUNET_PQ_query_param_string (instance_id),
-        GNUNET_PQ_query_param_timestamp (&before),
-        GNUNET_PQ_query_param_timestamp (&after),
-        GNUNET_PQ_query_param_uint64 (&offset),
-        GNUNET_PQ_query_param_uint64 (&plimit),
-        GNUNET_PQ_query_param_string (payto_uri),
-        GNUNET_PQ_query_param_end
-      };
-
-      PREPARE (pg,
-               "lookup_transfers_time_payto_asc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",merchant_transfer_signatures.execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE execution_time < $2"
-               "   AND execution_time >= $3"
-               "   AND credit_serial > $4"
-               "   AND REGEXP_REPLACE(payto_uri,'\\?.*','')"
-               "      =REGEXP_REPLACE($6,'\\?.*','')"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial ASC"
-               " LIMIT $5");
-      PREPARE (pg,
-               "lookup_transfers_time_payto_desc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",merchant_transfer_signatures.execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE execution_time < $2"
-               "   AND execution_time >= $3"
-               "   AND credit_serial < $4"
-               "   AND REGEXP_REPLACE(payto_uri,'\\?.*','')"
-               "      =REGEXP_REPLACE($6,'\\?.*','')"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial DESC"
-               " LIMIT $5");
-      qs = GNUNET_PQ_eval_prepared_multi_select (
-        pg->conn,
-        (limit > 0)
-        ? "lookup_transfers_time_payto_asc"
-        : "lookup_transfers_time_payto_desc",
-        params,
-        &lookup_transfers_cb,
-        &ltc);
-    }
-    else
-    {
-      struct GNUNET_PQ_QueryParam params[] = {
-        GNUNET_PQ_query_param_string (instance_id),
-        GNUNET_PQ_query_param_timestamp (&before),
-        GNUNET_PQ_query_param_timestamp (&after),
-        GNUNET_PQ_query_param_uint64 (&offset),
-        GNUNET_PQ_query_param_uint64 (&plimit),
-        GNUNET_PQ_query_param_end
-      };
-
-      PREPARE (pg,
-               "lookup_transfers_time_asc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",merchant_transfer_signatures.execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE execution_time < $2"
-               "   AND execution_time >= $3"
-               "   AND credit_serial > $4"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial ASC"
-               " LIMIT $5");
-      PREPARE (pg,
-               "lookup_transfers_time_desc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",merchant_transfer_signatures.execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE execution_time < $2"
-               "   AND execution_time >= $3"
-               "   AND credit_serial < $4"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial DESC"
-               " LIMIT $5");
-      qs = GNUNET_PQ_eval_prepared_multi_select (
-        pg->conn,
-        (limit > 0)
-        ? "lookup_transfers_time_asc"
-        : "lookup_transfers_time_desc",
-        params,
-        &lookup_transfers_cb,
-        &ltc);
-    }
-  }
-  else
-  {
-    if (NULL != payto_uri)
-    {
-      struct GNUNET_PQ_QueryParam params[] = {
-        GNUNET_PQ_query_param_string (instance_id),
-        GNUNET_PQ_query_param_uint64 (&offset),
-        GNUNET_PQ_query_param_uint64 (&plimit),
-        GNUNET_PQ_query_param_string (payto_uri),
-        GNUNET_PQ_query_param_end
-      };
-
-      PREPARE (pg,
-               "lookup_transfers_payto_asc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",CASE WHEN (merchant_transfer_signatures.execution_time) IS NULL"
-               "   THEN 9223372036854775807" /* largest BIGINT possible */
-               "   ELSE merchant_transfer_signatures.execution_time"
-               " END AS execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  LEFT JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE credit_serial > $2"
-               "   AND REGEXP_REPLACE(payto_uri,'\\?.*','')"
-               "      =REGEXP_REPLACE($4,'\\?.*','')"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial ASC"
-               " LIMIT $3");
-      PREPARE (pg,
-               "lookup_transfers_payto_desc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",CASE WHEN (merchant_transfer_signatures.execution_time) IS NULL"
-               "   THEN 9223372036854775807" /* largest BIGINT possible */
-               "   ELSE merchant_transfer_signatures.execution_time"
-               " END AS execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  LEFT JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE credit_serial < $2"
-               "   AND REGEXP_REPLACE(payto_uri,'\\?.*','')"
-               "      =REGEXP_REPLACE($4,'\\?.*','')"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial DESC"
-               " LIMIT $3");
-      qs = GNUNET_PQ_eval_prepared_multi_select (
-        pg->conn,
-        (limit > 0)
-        ? "lookup_transfers_payto_asc"
-        : "lookup_transfers_payto_desc",
-        params,
-        &lookup_transfers_cb,
-        &ltc);
-    }
-    else
-    {
-      struct GNUNET_PQ_QueryParam params[] = {
-        GNUNET_PQ_query_param_string (instance_id),
-        GNUNET_PQ_query_param_uint64 (&offset),
-        GNUNET_PQ_query_param_uint64 (&plimit),
-        GNUNET_PQ_query_param_end
-      };
-
-      PREPARE (pg,
-               "lookup_transfers_desc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",CASE WHEN (merchant_transfer_signatures.execution_time) IS NULL"
-               "   THEN 9223372036854775807" /* largest BIGINT possible */
-               "   ELSE merchant_transfer_signatures.execution_time"
-               " END AS execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  LEFT JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE credit_serial < $2"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial DESC"
-               " LIMIT $3");
-      PREPARE (pg,
-               "lookup_transfers_asc",
-               "SELECT"
-               " mt.credit_amount"
-               ",wtid"
-               ",merchant_accounts.payto_uri"
-               ",exchange_url"
-               ",credit_serial"
-               ",CASE WHEN (merchant_transfer_signatures.execution_time) IS NULL"
-               "   THEN 9223372036854775807" /* largest BIGINT possible */
-               "   ELSE merchant_transfer_signatures.execution_time"
-               " END AS execution_time"
-               ",verified"
-               ",confirmed"
-               " FROM merchant_transfers mt"
-               "  JOIN merchant_accounts USING (account_serial)"
-               "  LEFT JOIN merchant_transfer_signatures USING (credit_serial)"
-               " WHERE credit_serial > $2"
-               "   AND merchant_serial ="
-               "     (SELECT merchant_serial"
-               "        FROM merchant_instances"
-               "       WHERE merchant_id=$1)"
-               " ORDER BY credit_serial ASC"
-               " LIMIT $3");
-      qs = GNUNET_PQ_eval_prepared_multi_select (
-        pg->conn,
-        (limit > 0)
-        ? "lookup_transfers_asc"
-        : "lookup_transfers_desc",
-        params,
-        &lookup_transfers_cb,
-        &ltc);
-    }
-  }
+  PREPARE (pg,
+           "lookup_transfers_asc",
+           "SELECT"
+           " mt.credit_amount"
+           ",wtid"
+           ",mac.payto_uri"
+           ",exchange_url"
+           ",credit_serial"
+           ",mts.execution_time"
+           ",verified"
+           ",confirmed"
+           " FROM merchant_transfers mt"
+           "  JOIN merchant_accounts mac"
+           "    USING (account_serial)"
+           "  LEFT JOIN merchant_transfer_signatures mts"
+           "    USING (credit_serial)"
+           " WHERE ( $7 OR "
+           "         (mts.execution_time IS NOT NULL AND"
+           "          mts.execution_time < $2 AND"
+           "          mts.execution_time >= $3) )"
+           "   AND ( $8 OR "
+           "         (verified = $9) )"
+           "   AND ( (CAST($6 AS TEXT) IS NULL) OR "
+           "         (REGEXP_REPLACE(mac.payto_uri,'\\?.*','')"
+           "         =REGEXP_REPLACE($6,'\\?.*','')) )"
+           "   AND merchant_serial ="
+           "     (SELECT merchant_serial"
+           "        FROM merchant_instances"
+           "       WHERE merchant_id=$1)"
+           "   AND (credit_serial > $4)"
+           " ORDER BY credit_serial ASC"
+           " LIMIT $5");
+  PREPARE (pg,
+           "lookup_transfers_desc",
+           "SELECT"
+           " mt.credit_amount"
+           ",wtid"
+           ",mac.payto_uri"
+           ",exchange_url"
+           ",credit_serial"
+           ",mts.execution_time"
+           ",verified"
+           ",confirmed"
+           " FROM merchant_transfers mt"
+           "  JOIN merchant_accounts mac"
+           "    USING (account_serial)"
+           "  LEFT JOIN merchant_transfer_signatures mts"
+           "    USING (credit_serial)"
+           " WHERE ( $7 OR "
+           "         (mts.execution_time IS NOT NULL AND"
+           "          mts.execution_time < $2 AND"
+           "          mts.execution_time >= $3) )"
+           "   AND ( $8 OR "
+           "         (verified = $9) )"
+           "   AND ( (CAST($6 AS TEXT) IS NULL) OR "
+           "         (REGEXP_REPLACE(mac.payto_uri,'\\?.*','')"
+           "         =REGEXP_REPLACE($6,'\\?.*','')) )"
+           "   AND merchant_serial ="
+           "     (SELECT merchant_serial"
+           "        FROM merchant_instances"
+           "       WHERE merchant_id=$1)"
+           "   AND (credit_serial < $4)"
+           " ORDER BY credit_serial DESC"
+           " LIMIT $5");
+  qs = GNUNET_PQ_eval_prepared_multi_select (
+    pg->conn,
+    (limit > 0)
+    ? "lookup_transfers_asc"
+    : "lookup_transfers_desc",
+    params,
+    &lookup_transfers_cb,
+    &ltc);
   if (0 >= qs)
     return qs;
   return ltc.qs;
diff --git a/src/testing/testing_api_cmd_get_transfers.c b/src/testing/testing_api_cmd_get_transfers.c
index b8548229..b5b05295 100644
--- a/src/testing/testing_api_cmd_get_transfers.c
+++ b/src/testing/testing_api_cmd_get_transfers.c
@@ -109,7 +109,7 @@ get_transfers_cb (
     if (gtr->details.ok.transfers_length != gts->transfers_length)
     {
       GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                  "Transfers length does not match (%u/%u)\n",
+                  "Transfers length does not match (got %u/want %u)\n",
                   gtr->details.ok.transfers_length,
                   gts->transfers_length);
       TALER_TESTING_interpreter_fail (gts->is);