From a6d2ad7265c66c397b9dc3dd1c341b4c76aebcfe Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 16 Dec 2024 08:54:24 +0100 Subject: [PATCH] :recycle: [#3457] Refactor payments module to apply plugin mechanism consistently 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 | 4 ++ src/openforms/payments/base.py | 27 ++++++++---- src/openforms/payments/contrib/demo/plugin.py | 12 ++++-- .../payments/contrib/ogone/plugin.py | 42 +++++++++---------- .../payments/contrib/ogone/typing.py | 7 ++++ src/openforms/payments/models.py | 7 +++- src/openforms/payments/tests/test_views.py | 8 ++-- src/openforms/payments/views.py | 12 ++++-- 8 files changed, 78 insertions(+), 41 deletions(-) create mode 100644 src/openforms/payments/contrib/ogone/typing.py diff --git a/pyright.pyproject.toml b/pyright.pyproject.toml index cc79387715..1343b5afcf 100644 --- a/pyright.pyproject.toml +++ b/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/", diff --git a/src/openforms/payments/base.py b/src/openforms/payments/base.py index 5a1b59ad29..b21b92e2c7 100644 --- a/src/openforms/payments/base.py +++ b/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}, diff --git a/src/openforms/payments/contrib/demo/plugin.py b/src/openforms/payments/contrib/demo/plugin.py index 5c51590385..62524b72c6 100644 --- a/src/openforms/payments/contrib/demo/plugin.py +++ b/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() diff --git a/src/openforms/payments/contrib/ogone/plugin.py b/src/openforms/payments/contrib/ogone/plugin.py index 3f7b45e444..ffba9e267d 100644 --- a/src/openforms/payments/contrib/ogone/plugin.py +++ b/src/openforms/payments/contrib/ogone/plugin.py @@ -1,6 +1,6 @@ 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 _ @@ -8,6 +8,7 @@ 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) diff --git a/src/openforms/payments/contrib/ogone/typing.py b/src/openforms/payments/contrib/ogone/typing.py new file mode 100644 index 0000000000..c8944d68dd --- /dev/null +++ b/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 diff --git a/src/openforms/payments/models.py b/src/openforms/payments/models.py index 20257134e7..4bee299582 100644 --- a/src/openforms/payments/models.py +++ b/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") diff --git a/src/openforms/payments/tests/test_views.py b/src/openforms/payments/tests/test_views.py index 1086cbd867..19b8cd6f2d 100644 --- a/src/openforms/payments/tests/test_views.py +++ b/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) diff --git a/src/openforms/payments/views.py b/src/openforms/payments/views.py index 09960d5c8f..a571b5e0ab 100644 --- a/src/openforms/payments/views.py +++ b/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