♻️ [#3457] Refactor payments module to apply plugin mechanism consist…

…ently

All other modules operate on the idea that the generic code populates
the plugin options from the generic information and the specified
options serializer, which allows plugins to be generically typed and
not needing to perform additional input validation or conversion from
the raw JSON data for the options to the Python/Django objects that
are easier to work with.

This does change the public API of the registry/plugins to take an
extra argument, but enables us to apply static type checking (which
already reveals some problems) and simpler plugin code.

pyright.pyproject.toml

@@ -10,6 +10,7 @@ include = [
     "src/openforms/dmn/registry.py",
     "src/openforms/formio/registry.py",
     "src/openforms/formio/rendering/registry.py",
+    "src/openforms/payments/base.py",
     "src/openforms/payments/registry.py",
     "src/openforms/pre_requests/base.py",
     "src/openforms/pre_requests/registry.py",
@@ -29,6 +30,9 @@ include = [
     "src/openforms/forms/api/serializers/logic/action_serializers.py",
     # Payments
     "src/openforms/payments/models.py",
+    "src/openforms/payments/contrib/ogone/client.py",
+    "src/openforms/payments/contrib/ogone/plugin.py",
+    "src/openforms/payments/views.py",
     # Interaction with the outside world
     "src/openforms/contrib/zgw/service.py",
     "src/openforms/contrib/objects_api/",

src/openforms/payments/base.py

@@ -1,9 +1,12 @@
+from __future__ import annotations
+
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Mapping
+from typing import TYPE_CHECKING, Mapping, TypedDict
 
 from django.http import HttpRequest, HttpResponse
 
 from rest_framework import serializers
+from rest_framework.request import Request
 
 from openforms.plugins.plugin import AbstractBasePlugin
 from openforms.utils.mixins import JsonSchemaSerializerMixin
@@ -32,38 +35,46 @@ class EmptyOptions(JsonSchemaSerializerMixin, serializers.Serializer):
     pass
 
 
-class BasePlugin(AbstractBasePlugin):
+class Options(TypedDict):
+    pass
+
+
+class BasePlugin[OptionsT: Options](AbstractBasePlugin):
     return_method = "GET"
     webhook_method = "POST"
-    configuration_options = EmptyOptions
+    configuration_options: type[serializers.Serializer] = EmptyOptions
 
     # override
 
     def start_payment(
         self,
         request: HttpRequest,
-        payment: "SubmissionPayment",
+        payment: SubmissionPayment,
+        options: OptionsT,
     ) -> PaymentInfo:
         raise NotImplementedError()
 
     def handle_return(
-        self, request: HttpRequest, payment: "SubmissionPayment"
+        self,
+        request: Request,
+        payment: SubmissionPayment,
+        options: OptionsT,
     ) -> HttpResponse:
         raise NotImplementedError()
 
-    def handle_webhook(self, request: HttpRequest) -> "SubmissionPayment":
+    def handle_webhook(self, request: Request) -> SubmissionPayment:
         raise NotImplementedError()
 
     # helpers
 
-    def get_start_url(self, request: HttpRequest, submission: "Submission") -> str:
+    def get_start_url(self, request: HttpRequest, submission: Submission) -> str:
         return reverse_plus(
             "payments:start",
             kwargs={"uuid": submission.uuid, "plugin_id": self.identifier},
             request=request,
         )
 
-    def get_return_url(self, request: HttpRequest, payment: "SubmissionPayment") -> str:
+    def get_return_url(self, request: HttpRequest, payment: SubmissionPayment) -> str:
         return reverse_plus(
             "payments:return",
             kwargs={"uuid": payment.uuid},

src/openforms/payments/contrib/demo/plugin.py

@@ -1,3 +1,5 @@
+from typing import TypedDict
+
 from django.http import HttpResponseRedirect
 from django.utils.translation import gettext_lazy as _
 
@@ -11,16 +13,20 @@
 from ...registry import register
 
 
+class NoOptions(TypedDict):
+    pass
+
+
 @register("demo")
-class DemoPayment(BasePlugin):
+class DemoPayment(BasePlugin[NoOptions]):
     verbose_name = _("Demo")
     is_demo_plugin = True
 
-    def start_payment(self, request, payment):
+    def start_payment(self, request, payment, options):
         url = self.get_return_url(request, payment)
         return PaymentInfo(url=url, data={})
 
-    def handle_return(self, request, payment):
+    def handle_return(self, request, payment, options):
         payment.status = PaymentStatus.completed
         payment.save()
 

src/openforms/payments/contrib/ogone/plugin.py

@@ -1,13 +1,14 @@
 import logging
 
-from django.http import HttpResponseBadRequest, HttpResponseRedirect
+from django.http import HttpRequest, HttpResponseBadRequest, HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
 import requests
 from rest_framework import serializers
 from rest_framework.exceptions import ParseError
+from rest_framework.request import Request
 
 from openforms.api.fields import PrimaryKeyRelatedAsChoicesField
 from openforms.config.data import Entry
@@ -18,12 +19,13 @@
 
 from ...base import BasePlugin
 from ...constants import PAYMENT_STATUS_FINAL, UserAction
-from ...contrib.ogone.client import OgoneClient
-from ...contrib.ogone.constants import OgoneStatus
-from ...contrib.ogone.exceptions import InvalidSignature
-from ...contrib.ogone.models import OgoneMerchant
 from ...models import SubmissionPayment
 from ...registry import register
+from .client import OgoneClient
+from .constants import OgoneStatus
+from .exceptions import InvalidSignature
+from .models import OgoneMerchant
+from .typing import PaymentOptions
 
 logger = logging.getLogger(__name__)
 
@@ -41,18 +43,17 @@ class OgoneOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer):
 
 
 @register("ogone-legacy")
-class OgoneLegacyPaymentPlugin(BasePlugin):
+class OgoneLegacyPaymentPlugin(BasePlugin[PaymentOptions]):
     verbose_name = _("Ogone legacy")
     configuration_options = OgoneOptionsSerializer
 
-    def start_payment(self, request, payment: SubmissionPayment):
+    def start_payment(
+        self, request: HttpRequest, payment: SubmissionPayment, options: PaymentOptions
+    ):
         # decimal to cents
         amount_cents = int((payment.amount * 100).to_integral_exact())
 
-        merchant = get_object_or_404(
-            OgoneMerchant, id=payment.plugin_options["merchant_id"]
-        )
-        client = OgoneClient(merchant)
+        client = OgoneClient(options["merchant_id"])
 
         return_url = self.get_return_url(request, payment)
 
@@ -69,14 +70,13 @@ def start_payment(self, request, payment: SubmissionPayment):
         )
         return info
 
-    def handle_return(self, request, payment: SubmissionPayment):
+    def handle_return(
+        self, request: Request, payment: SubmissionPayment, options: PaymentOptions
+    ):
         action = request.query_params.get(RETURN_ACTION_PARAM)
         payment_id = request.query_params[PAYMENT_ID_PARAM]
 
-        merchant = get_object_or_404(
-            OgoneMerchant, id=payment.plugin_options["merchant_id"]
-        )
-        client = OgoneClient(merchant)
+        client = OgoneClient(options["merchant_id"])
 
         try:
             params = client.get_validated_params(request.query_params)
@@ -107,7 +107,7 @@ def handle_return(self, request, payment: SubmissionPayment):
         )
         return HttpResponseRedirect(redirect_url)
 
-    def handle_webhook(self, request):
+    def handle_webhook(self, request: Request):
         # unvalidated data
         order_id = case_insensitive_get(request.data, "orderID")
         if not order_id:
@@ -120,10 +120,10 @@ def handle_webhook(self, request):
             raise ParseError("missing PAYID")
 
         payment = get_object_or_404(SubmissionPayment, public_order_id=order_id)
-        merchant = get_object_or_404(
-            OgoneMerchant, id=payment.plugin_options["merchant_id"]
-        )
-        client = OgoneClient(merchant)
+        options_serializer = self.configuration_options(data=payment.plugin_options)
+        options_serializer.is_valid(raise_exception=True)
+        options: PaymentOptions = options_serializer.validated_data
+        client = OgoneClient(options["merchant_id"])
 
         try:
             params = client.get_validated_params(request.data)

src/openforms/payments/contrib/ogone/typing.py

@@ -0,0 +1,7 @@
+from typing import TypedDict
+
+from .models import OgoneMerchant
+
+
+class PaymentOptions(TypedDict):
+    merchant_id: OgoneMerchant  # FIXME: key is badly named in the serializer

src/openforms/payments/models.py

@@ -2,7 +2,7 @@
 
 import uuid
 from decimal import Decimal
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, ClassVar
 
 from django.db import models, transaction
 from django.utils.translation import gettext_lazy as _
@@ -92,6 +92,7 @@ def get_completed_public_order_ids(self) -> list[str]: ...
 
 
 class SubmissionPayment(models.Model):
+    id: int | None
     uuid = models.UUIDField(_("UUID"), unique=True, default=uuid.uuid4)
     created = models.DateTimeField(auto_now_add=True)
 
@@ -139,7 +140,9 @@ class SubmissionPayment(models.Model):
         help_text=_("The ID assigned to the payment by the payment provider."),
     )
 
-    objects = SubmissionPaymentManager()
+    objects: ClassVar[  # pyright: ignore[reportIncompatibleVariableOverride]
+        SubmissionPaymentManager
+    ] = SubmissionPaymentManager()
 
     class Meta:
         verbose_name = _("submission payment details")

src/openforms/payments/tests/test_views.py

@@ -23,14 +23,14 @@ class Plugin(BasePlugin):
     return_method = "GET"
     webhook_method = "POST"
 
-    def start_payment(self, request, payment):
+    def start_payment(self, request, payment, options):
         return PaymentInfo(type="get", url="http://testserver/foo")
 
-    def handle_return(self, request, payment):
+    def handle_return(self, request, payment, options):
         return HttpResponseRedirect(payment.submission.form_url)
 
     def handle_webhook(self, request):
-        return None
+        return SubmissionPayment(id=None)
 
 
 class ViewsTests(TestCase):
@@ -221,7 +221,7 @@ def handle_webhook(self, request):
         return payment
 
     def handle_return(
-        self, request: HttpRequest, payment: "SubmissionPayment"
+        self, request: HttpRequest, payment: "SubmissionPayment", options
     ) -> HttpResponse:
         return HttpResponseRedirect(payment.submission.form_url)
 

src/openforms/payments/views.py

@@ -129,7 +129,9 @@ def post(self, request, uuid: str, plugin_id: str):
             submission.price,
         )
 
-        info = plugin.start_payment(request, payment)
+        options = plugin.configuration_options(data=payment.plugin_options)
+        options.is_valid(raise_exception=True)
+        info = plugin.start_payment(request, payment, options.validated_data)
         logevent.payment_flow_start(payment, plugin)
         return Response(self.get_serializer(instance=info).data)
 
@@ -222,8 +224,10 @@ def _handle_return(self, request, uuid: str):
         if plugin.return_method.upper() != request.method.upper():
             raise MethodNotAllowed(request.method)
 
+        options = plugin.configuration_options(data=payment.plugin_options)
         try:
-            response = plugin.handle_return(request, payment)
+            options.is_valid(raise_exception=True)
+            response = plugin.handle_return(request, payment, options.validated_data)
         except Exception as e:
             logevent.payment_flow_failure(payment, plugin, e)
             raise
@@ -383,7 +387,9 @@ def get_context_data(self, **kwargs):
                 submission.price,
             )
 
-            info = plugin.start_payment(self.request, payment)
+            options = plugin.configuration_options(data=payment.plugin_options)
+            options.is_valid(raise_exception=True)
+            info = plugin.start_payment(self.request, payment, options.validated_data)
             logevent.payment_flow_start(payment, plugin, from_email=True)
 
             context["url"] = info.url