5715.

Initial change in the structure.  Just porting the /history
validation to use the 'form' API for validating GET arguments.

Subsequent changes (for GET requests) will follow the same pattern.

3 files changed, 104 insertions, 88 deletions

diff --git a/talerbank/app/middleware.py b/talerbank/app/middleware.py
index 220fa94..dd1b3ac 100644
--- a/talerbank/app/middleware.py
+++ b/talerbank/app/middleware.py
@@ -8,7 +8,7 @@ from .views import \
      LoginFailed, RejectNoRightsException)
 from .schemas import \
     (URLParameterMissing, URLParameterMalformed,
-     JSONFieldException)
+     JSONFieldException, URLParamValidationError)
 from .amount import \
     (CurrencyMismatch, BadFormatAmount,
      NumberTooBig, NegativeNumber)
@@ -72,6 +72,13 @@ class ExceptionMiddleware:
             DebitLimitException: 3,
             URLParameterMissing: 8,
             URLParameterMalformed: 9,
+
+            ##
+            # This one unified class kills the distinction
+            # between a parameter being missing or malformed.
+            # It simplifies the error handling though, and
+            # MUST be reflected in taler_error_codes.h!
+            URLParamValidationError: 9,
             JSONFieldException: 6,
             CurrencyMismatch: 11,
             BadFormatAmount: 11,
diff --git a/talerbank/app/schemas.py b/talerbank/app/schemas.py
index cf11bdc..b75ff6a 100644
--- a/talerbank/app/schemas.py
+++ b/talerbank/app/schemas.py
@@ -24,11 +24,67 @@ from validictory import validate
 from validictory.validator import \
     (RequiredFieldValidationError,
      FieldValidationError)
-
 from django.conf import settings
+from django.core.exceptions import ValidationError
+from django import forms
+from django.core.validators import RegexValidator
 
 
 ##
+# Exception class to be raised when at least one expected URL
+# parameter is either not found or malformed.
+class URLParamValidationError(ValidationError):
+
+    ##
+    # Init method.
+    #
+    # @param self the object itself.
+    # @param param the missing URL parameter name.
+    # @param http_status_code the HTTP response code to return
+    #        to the caller (client).
+    def __init__(self, error, http_status_code):
+        self.hint = ["%s: %s, " % (k, error[k]) for k in error]
+        self.http_status_code = http_status_code
+        super().__init__()
+
+
+##
+# Form specification that validates GET parameters from a
+# /history request.
+class HistoryParams(forms.Form):
+    auth = forms.CharField(
+        validators=[RegexValidator(
+            "^basic$",
+            message="Only 'basic' is allowed")])
+
+    cancelled = forms.CharField(
+        required=False,
+        empty_value="show",
+        validators=[RegexValidator(
+            "^(omit|show)$",
+            message="Only 'omit' or 'show' are valid")])
+
+    # FIXME: adjust min/max values.
+    delta = forms.IntegerField()
+    # FIXME: adjust min/max values.
+    start = forms.IntegerField(required=False)
+
+    ordering = forms.CharField(
+        required=False,
+        empty_value="descending",
+        validators=[RegexValidator(
+            "^(ascending|descending)$",
+            message="Only 'ascending' or 'descending' are valid")])
+
+    direction = forms.CharField(
+        validators=[RegexValidator(
+            "^(debit|credit|both|cancel\+|cancel-)$",
+            message="Only: debit/credit/both/cancel+/cancel-")])
+
+    # FIXME: adjust min/max values.
+    account_number = forms.IntegerField(required=False)
+
+##
 # Exception class to be raised when a expected URL parameter
 # is not found.
 class URLParameterMissing(ValueError):
@@ -86,7 +142,6 @@ AMOUNT_SCHEMA = {
     "type": "string",
     "pattern": "^[A-Za-z0-9_-]+:([0-9]+)\.?([0-9]+)?$"}
 
-
 ##
 # Definition that withdraw request bodies have to match.
 WITHDRAW_SESSION_SCHEMA = {
@@ -173,32 +228,6 @@ HISTORY_RANGE_REQUEST_SCHEMA = {
 
 
 ##
-# Definition for /history request URL parameters.
-HISTORY_REQUEST_SCHEMA = {
-    "type": "object",
-    "properties": {
-        "auth": {"type": "string", "pattern": "^basic$"},
-        "cancelled": {"type": "string",
-                      "pattern": "^(omit|show)$",
-                      "required": False},
-        "delta": {"type": "string",
-                  "pattern": r"^([\+-])?([0-9])+$"},
-        "start": {"type": "string",
-                  "pattern": "^([0-9]+)$",
-                  "required": False},
-        "ordering": {"type": "string",
-                     "pattern": r"^(ascending|descending)$",
-                     "required": False},
-        "direction": {"type": "string",
-                      "pattern": r"^(debit|credit|both|cancel\+|cancel-)$"},
-        "account_number": {"type": "string",
-                           "pattern": "^([0-9]+)$",
-                           "required": False}
-    }
-}
-
-
-##
 # Definition for /add/incoming request bodies.
 INCOMING_REQUEST_SCHEMA = {
     "type": "object",
@@ -210,7 +239,6 @@ INCOMING_REQUEST_SCHEMA = {
     }
 }
 
-
 ##
 # Definition for PIN/TAN request URL parameters.
 PIN_TAN_ARGS = {
@@ -268,13 +296,6 @@ def validate_reject(data):
     validate(data, REJECT_REQUEST_SCHEMA)
 
 ##
-# Check /history input data.
-#
-# @param data dict representing /history's GET parameters.
-def validate_history(data):
-    validate(data, HISTORY_REQUEST_SCHEMA)
-
-##
 # Check /history-range input data.
 #
 # @param data dict representing /history's GET parameters.
@@ -319,7 +340,6 @@ def check_withdraw_session(data):
 def validate_data(request, data):
     switch = {
         "/reject": validate_reject,
-        "/history": validate_history,
         "/history-range": validate_history_range,
         "/admin/add/incoming": validate_add_incoming,
         "/pin/verify": check_withdraw_session,
diff --git a/talerbank/app/views.py b/talerbank/app/views.py
index f4af47c..3d1f7bc 100644
--- a/talerbank/app/views.py
+++ b/talerbank/app/views.py
@@ -44,7 +44,7 @@ from django.shortcuts import render, redirect
 from datetime import datetime
 from .models import BankAccount, BankTransaction
 from .amount import Amount
-from .schemas import validate_data
+from .schemas import validate_data, HistoryParams, URLParamValidationError
 LOGGER = logging.getLogger(__name__)
 
 ##
@@ -259,7 +259,7 @@ def profile_page(request):
         account_no=request.user.bankaccount.account_no,
         wt_form=wtf,
         history=extract_history(request.user.bankaccount,
-                                True),
+                                -1 * (UINT64_MAX / 2 / 2))
     )
     if settings.TALER_SUGGESTED_EXCHANGE:
         context["suggested_exchange"] = settings.TALER_SUGGESTED_EXCHANGE
@@ -459,12 +459,14 @@ def logout_view(request):
 #        entries will be younger / older than @a start.
 # @return the history array.
 def extract_history(account,
-                    descending,
-                    delta=None,
-                    start=UINT64_MAX,
-                    sign="-"):
+                    delta,
+                    start=UINT64_MAX):
     history = []
-    qs = query_history(account, "both", delta, start, sign, True)
+    qs = query_history(account,
+                       "both",
+                       delta,
+                       start,
+                       "descending")
     for item in qs:
         if item.credit_account == account:
             counterpart = item.debit_account
@@ -514,9 +516,9 @@ def serve_public_accounts(request, name=None, page=None):
                                 "both",
                                 # Note: the parameter below is used for slicing arrays
                                 # and django/python is not allowing slicing with big numbers.
-                                (UINT64_MAX / 2 ) / 2,
-                                start=0,
-                                sign="+").count()
+                                UINT64_MAX / 2 / 2,
+                                0,
+                                "descending").count()
     DELTA = 30
     # '//' operator is NO floating point.
     num_pages = max(num_records // DELTA, 1)
@@ -525,10 +527,8 @@ def serve_public_accounts(request, name=None, page=None):
 
     # Retrieve DELTA records younger than 'start_row' (included).
     history = extract_history(user.bankaccount,
-                              True,
                               DELTA * page,
-                              0,
-                              "+")[DELTA * (page - 1):(DELTA * page)]
+                              0)[DELTA * (page - 1):(DELTA * page)]
 
     pages = list(range(1, num_pages + 1))
 
@@ -636,34 +636,31 @@ def query_history_range(bank_account,
 #        * cancel-: only entries where the querying user cancelled
 #                   the _paying_ of money will be returned.
 # @param delta how many history entries will be contained in the
-#        array (will be passed as-is to the internal routine
-#        @a query_history).
+#        array.
 # @param start any history will be searched starting from this
 #        value (which is a row ID), and going to the past or to
 #        the future (depending on the user choice).  However, this
 #        value itself will not be included in the history.
 # @param sign this value ("+"/"-") determines whether the history
 #        entries will be younger / older than @a start.
-# @param descending if True, then the results will have the
-#        youngest entry in the first position.
+# @param ordering "descending" or anything else (for "ascending").
 def query_history(bank_account,
                   direction,
                   delta,
                   start,
-                  sign,
-                  descending=True):
+                  ordering):
 
-    sign_filter = {
-        "+": Q(id__gt=start),
-        "-": Q(id__lt=start),
-    }
+    def sign_filter(delta):
+        if 0 > delta:
+            return Q(id__lt=start)
+        return Q(id__gt=start) 
 
     qs = BankTransaction.objects.filter(
         direction_switch(bank_account, direction),
-        sign_filter.get(sign))
+        sign_filter(delta))
     
-    order = "-id" if descending else "id"
-    return qs.order_by(order)[:delta]
+    order = "-id" if "descending" == ordering else "id"
+    return qs.order_by(order)[:abs(delta)]
 
 
 ##
@@ -733,8 +730,6 @@ def serve_history_range(request, user_account):
         return HttpResponse(status=204)
     return JsonResponse(dict(data=history), status=200)
 
-
-
 ##
 # Serve a request of /history.
 #
@@ -744,33 +739,27 @@ def serve_history_range(request, user_account):
 @require_GET
 @login_via_headers
 def serve_history(request, user_account):
-    validate_data(request, request.GET.dict())
+    get_params = HistoryParams(request.GET.dict())
+    if not get_params.is_valid():
+      raise URLParamValidationError(get_params.errors, 400)
 
-    # delta (it does exist: enforced by the check above.)
-    parsed_delta = re.search(r"([\+-])?([0-9]+)",
-                             request.GET.get("delta"))
-    # normalize the sign.
-    sign = parsed_delta.group(1)
-    sign = sign if sign else "+"
+    delta = get_params.cleaned_data.get("delta")
+    start = get_params.cleaned_data.get("start")
 
-    # Ordering.
-    ordering = request.GET.get("ordering", "descending")
-    start = request.GET.get("start")
-
-    if not start:
-        start = 0 if "+" == sign else UINT64_MAX
+    if None == start:
+        start = 0 if 0 <= delta else UINT64_MAX
 
     qs = query_history(user_account.bankaccount,
-                       request.GET.get("direction"),
-                       int(parsed_delta.group(2)),
-                       int(start),
-                       sign,
-                       "descending" == ordering)
+                       get_params.cleaned_data.get("direction"),
+                       delta,
+                       start,
+                       get_params.cleaned_data.get("ordering"))
+
+    history = build_history_response(
+        qs,
+        get_params.cleaned_data.get("cancelled"),
+        user_account)
 
-    history = build_history_response(qs,
-                                     request.GET.get("cancelled",
-                                                     "show"),
-                                     user_account)
     if not history:
         return HttpResponse(status=204)
     return JsonResponse(dict(data=history), status=200)