From 33db635bb5a26ece279482dc69fe06735426c3fb Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Tue, 19 Dec 2023 18:13:12 +0100 Subject: [PATCH] [#3607] Refactor validation plugins ... to be up to date with the current OF standard Only implementation is changed in this commit, tests will follow --- src/openforms/contrib/brk/validators.py | 11 ++- src/openforms/contrib/kvk/validators.py | 36 ++++---- src/openforms/plugins/registry.py | 8 +- src/openforms/registrations/registry.py | 2 +- src/openforms/validations/base.py | 34 ++++++++ src/openforms/validations/registry.py | 82 ++++--------------- .../validations/validators/formats.py | 45 +++++----- 7 files changed, 102 insertions(+), 116 deletions(-) create mode 100644 src/openforms/validations/base.py diff --git a/src/openforms/contrib/brk/validators.py b/src/openforms/contrib/brk/validators.py index 369c5b4ad8..3baaec1a18 100644 --- a/src/openforms/contrib/brk/validators.py +++ b/src/openforms/contrib/brk/validators.py @@ -12,6 +12,7 @@ from openforms.authentication.constants import AuthAttribute from openforms.submissions.models import Submission +from openforms.validations.base import BasePlugin from openforms.validations.registry import register from .client import NoServiceConfigured, SearchParams, get_client @@ -56,15 +57,13 @@ class ValueSerializer(serializers.Serializer): value = AddressValueSerializer() -@register( - "brk-zakelijk-gerechtigd", - verbose_name=_("BRK - Zakelijk gerechtigd"), - for_components=("addressNL",), -) +@register("brk-Zaakgerechtigde") @deconstructible -class BRKZakelijkGerechtigdeValidator: +class BRKZaakgerechtigdeValidator(BasePlugin[AddressValue]): value_serializer = ValueSerializer + verbose_name = _("BRK - Zaakgerechtigde") + for_components = ("addressNL",) error_messages = { "no_bsn": _("No BSN is available to validate your address."), diff --git a/src/openforms/contrib/kvk/validators.py b/src/openforms/contrib/kvk/validators.py index 1af022e33b..4111fbde5f 100644 --- a/src/openforms/contrib/kvk/validators.py +++ b/src/openforms/contrib/kvk/validators.py @@ -7,7 +7,8 @@ from requests import RequestException from openforms.utils.validators import validate_digits, validate_rsin -from openforms.validations.registry import StringValueSerializer, register +from openforms.validations.base import BasePlugin +from openforms.validations.registry import register from .client import NoServiceConfigured, SearchParams, get_client @@ -20,7 +21,7 @@ class NumericBaseValidator: "too_short": _("%(type)s should have %(size)i characters."), } - def __call__(self, value): + def __call__(self, value: str): validate_digits(value) if len(value) != self.value_size: @@ -51,8 +52,6 @@ class KVKRemoteBaseValidator: query_param: Literal["kvkNummer", "rsin", "vestigingsnummer"] value_label: str - value_serializer = StringValueSerializer - error_messages = { "not_found": _("%(type)s does not exist."), "too_short": _("%(type)s should have %(size)i characters."), @@ -84,38 +83,43 @@ def __call__(self, value: str) -> bool: return True -@register("kvk-kvkNumber", verbose_name=_("KvK number"), for_components=("textfield",)) +@register("kvk-kvkNumber") @deconstructible -class KVKNumberRemoteValidator(KVKRemoteBaseValidator): +class KVKNumberRemoteValidator(BasePlugin[str], KVKRemoteBaseValidator): query_param = "kvkNummer" value_label = _("KvK number") - def __call__(self, value, submission): + verbose_name = _("KvK number") + for_components = ("textfield",) + + def __call__(self, value: str, submission): validate_kvk(value) super().__call__(value) -@register("kvk-rsin", verbose_name=_("KvK RSIN"), for_components=("textfield",)) +@register("kvk-rsin") @deconstructible -class KVKRSINRemoteValidator(KVKRemoteBaseValidator): +class KVKRSINRemoteValidator(BasePlugin[str], KVKRemoteBaseValidator): query_param = "rsin" value_label = _("RSIN") - def __call__(self, value, submission): + verbose_name = _("KvK RSIN") + for_components = ("textfield",) + + def __call__(self, value: str, submission): validate_rsin(value) super().__call__(value) -@register( - "kvk-branchNumber", - verbose_name=_("KvK branch number"), - for_components=("textfield",), -) +@register("kvk-branchNumber") @deconstructible class KVKBranchNumberRemoteValidator(KVKRemoteBaseValidator): query_param = "vestigingsnummer" value_label = _("Branch number") - def __call__(self, value, submission): + verbose_name = _("KvK branch number") + for_components = ("textfield",) + + def __call__(self, value: str, submission): validate_branchNumber(value) super().__call__(value) diff --git a/src/openforms/plugins/registry.py b/src/openforms/plugins/registry.py index 6ab7f005ca..ad61eb362d 100644 --- a/src/openforms/plugins/registry.py +++ b/src/openforms/plugins/registry.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING, Callable, Generic, TypeVar +from typing import TYPE_CHECKING, Callable, Generic, Iterator, TypeVar from django.db import OperationalError @@ -35,7 +35,7 @@ def __call__( def decorator(plugin_cls: type[PluginType_co]) -> type[PluginType_co]: if len(unique_identifier) > UNIQUE_ID_MAX_LENGTH: raise ValueError( - f"The unique identifier '{unique_identifier}' is longer then {UNIQUE_ID_MAX_LENGTH} characters." + f"The unique identifier '{unique_identifier}' is longer than {UNIQUE_ID_MAX_LENGTH} characters." ) if unique_identifier in self._registry: raise ValueError( @@ -50,7 +50,7 @@ def decorator(plugin_cls: type[PluginType_co]) -> type[PluginType_co]: return decorator - def check_plugin(self, plugin): + def check_plugin(self, plugin: PluginType_co): # validation hook pass @@ -63,7 +63,7 @@ def __getitem__(self, key: str): def __contains__(self, key: str): return key in self._registry - def iter_enabled_plugins(self): + def iter_enabled_plugins(self) -> Iterator[PluginType_co]: try: config = GlobalConfiguration.get_solo() assert isinstance(config, GlobalConfiguration) diff --git a/src/openforms/registrations/registry.py b/src/openforms/registrations/registry.py index 6fdf82f443..7c8497b465 100644 --- a/src/openforms/registrations/registry.py +++ b/src/openforms/registrations/registry.py @@ -10,7 +10,7 @@ class Registry(BaseRegistry[BasePlugin]): module = "registrations" - def check_plugin(self, plugin): + def check_plugin(self, plugin: BasePlugin): if not plugin.configuration_options: raise ValueError( "Please specify 'configuration_options' attribute for plugin class." diff --git a/src/openforms/validations/base.py b/src/openforms/validations/base.py new file mode 100644 index 0000000000..3d40c69bb4 --- /dev/null +++ b/src/openforms/validations/base.py @@ -0,0 +1,34 @@ +from abc import ABC, abstractmethod +from typing import ClassVar, Generic, TypeVar + +from rest_framework import serializers + +from openforms.plugins.plugin import AbstractBasePlugin +from openforms.submissions.models import Submission + +T = TypeVar("T") +"""A type variable representing the type of the value being validated by the plugin.""" + + +class StringValueSerializer(serializers.Serializer): + """A default serializer that accepts ``value`` as a string.""" + + value = serializers.CharField() + + +class BasePlugin(ABC, AbstractBasePlugin, Generic[T]): + + value_serializer: ClassVar[type[serializers.BaseSerializer]] = StringValueSerializer + """The serializer to be used to validate the value.""" + + for_components: ClassVar[tuple[str, ...]] = tuple() + """The components that can make use of this validator.""" + + @property + def is_enabled(self) -> bool: + # TODO always enabled for now, see: https://github.com/open-formulieren/open-forms/issues/1149 + return True + + @abstractmethod + def __call__(self, value: T, submission: Submission) -> bool: + pass diff --git a/src/openforms/validations/registry.py b/src/openforms/validations/registry.py index 670fe6c7df..b9c2f7cee4 100644 --- a/src/openforms/validations/registry.py +++ b/src/openforms/validations/registry.py @@ -1,6 +1,6 @@ import dataclasses import logging -from typing import Callable, Iterable, List, Type, Union +from typing import Callable, Iterable, List, Type, TypeVar, Union from django.core.exceptions import ValidationError as DJ_ValidationError from django.utils.translation import gettext_lazy as _ @@ -13,15 +13,9 @@ from openforms.submissions.models import Submission from openforms.typing import JSONValue -logger = logging.getLogger(__name__) - -ValidatorType = Callable[[JSONValue, Submission], bool] - - -class StringValueSerializer(serializers.Serializer): - """A default serializer that accepts ``value`` as a string.""" +from .base import BasePlugin - value = serializers.CharField() +logger = logging.getLogger(__name__) @dataclasses.dataclass() @@ -30,20 +24,6 @@ class ValidationResult: messages: List[str] = dataclasses.field(default_factory=list) -@dataclasses.dataclass() -class RegisteredValidator: - identifier: str - verbose_name: str - callable: ValidatorType - for_components: tuple[str] - is_demo_plugin: bool = False - # TODO always enabled for now, see: https://github.com/open-formulieren/open-forms/issues/1149 - is_enabled: bool = True - - def __call__(self, *args, **kwargs) -> bool: - return self.callable(*args, **kwargs) - - def flatten(iterables: Iterable) -> List[str]: def _flat(it): if isinstance(it, str): @@ -58,7 +38,10 @@ def _flat(it): return list(_flat(iterables)) -class Registry(BaseRegistry[RegisteredValidator]): +T = TypeVar("T") + + +class Registry(BaseRegistry[BasePlugin[T]]): """ A registry for the validations module plugins. @@ -70,51 +53,14 @@ class Registry(BaseRegistry[RegisteredValidator]): module = "validations" - def __call__( - self, - identifier: str, - verbose_name: str, - is_demo_plugin: bool = False, - for_components: tuple[str] = tuple(), - *args, - **kwargs, - ) -> Callable: - def decorator(validator: Union[Type, ValidatorType]): - if identifier in self._registry: - raise ValueError( - f"The unique identifier '{identifier}' is already present " - "in the registry." - ) - - call = validator - assert hasattr( - call, "value_serializer" - ), "Plugins must define a 'value_serializer' attribute" - - if isinstance(call, type): - call = validator() - if not callable(call): - raise ValueError(f"Validator '{identifier}' is not callable.") - - self._registry[identifier] = RegisteredValidator( - identifier=identifier, - verbose_name=verbose_name, - callable=call, - for_components=for_components, - is_demo_plugin=is_demo_plugin, - ) - return validator - - return decorator - @elasticapm.capture_span("app.validations.validate") def validate( - self, plugin_id: str, value: JSONValue, submission: Submission + self, plugin_id: str, value: T, submission: Submission ) -> ValidationResult: try: validator = self._registry[plugin_id] except KeyError: - logger.warning(f"called unregistered plugin_id '{plugin_id}'") + logger.warning("called unregistered plugin_id %s", plugin_id) return ValidationResult( False, messages=[ @@ -124,7 +70,7 @@ def validate( ], ) - if not getattr(validator.callable, "is_enabled", True): + if not getattr(validator, "is_enabled", True): return ValidationResult( False, messages=[ @@ -132,7 +78,7 @@ def validate( ], ) - SerializerClass = validator.callable.value_serializer + SerializerClass = validator.value_serializer serializer = SerializerClass(data={"value": value}) # first, run the cheap validation to check that the data actually conforms to the expected schema. @@ -151,6 +97,12 @@ def validate( else: return ValidationResult(True) + def check_plugin(self, plugin: BasePlugin) -> None: + if not hasattr(plugin, "value_serializer"): + raise ValueError( + f"Validator '{plugin.identifier}' must have a 'value_serializer' attribute." + ) + # Sentinel to provide the default registry. You an easily instantiate another # :class:`Registry` object to use as dependency injection in tests. diff --git a/src/openforms/validations/validators/formats.py b/src/openforms/validations/validators/formats.py index aecec3c260..435701b933 100644 --- a/src/openforms/validations/validators/formats.py +++ b/src/openforms/validations/validators/formats.py @@ -1,4 +1,5 @@ -from typing import TYPE_CHECKING, Optional, Protocol +from abc import ABC, abstractmethod +from typing import TYPE_CHECKING from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ @@ -6,25 +7,23 @@ import phonenumbers from phonenumbers import NumberParseException -from openforms.validations.registry import StringValueSerializer, register +from openforms.validations.base import BasePlugin +from openforms.validations.registry import register if TYPE_CHECKING: from phonenumbers.phonenumber import PhoneNumber -class ParsePhoneNumber(Protocol): - def __call__(self, value: str) -> "PhoneNumber": - ... # pragma: nocover - - -class PhoneNumberBaseValidator: - country: Optional[str] +class PhoneNumberBaseValidator(ABC): + country: str | None country_name: str = "" error_message = _("Not a valid %(country)s phone number") - value_serializer = StringValueSerializer - _parse_phonenumber: ParsePhoneNumber - def __call__(self, value, submission): + @abstractmethod + def _parse_phonenumber(self, value: str) -> "PhoneNumber": + pass + + def __call__(self, value: str, submission): z = self._parse_phonenumber(value) if not phonenumbers.is_possible_number(z) or not phonenumbers.is_valid_number( @@ -48,18 +47,17 @@ def __call__(self, value, submission): ) -@register( - "phonenumber-international", - verbose_name=_("International phone number"), - for_components=("phoneNumber",), -) -class InternationalPhoneNumberValidator(PhoneNumberBaseValidator): +@register("phonenumber-international") +class InternationalPhoneNumberValidator(BasePlugin[str], PhoneNumberBaseValidator): country = None country_name = _("international") error_message = _( "Not a valid international phone number. An example of a valid international phone number is +31612312312" ) + verbose_name = _("International phone number") + for_components = ("phoneNumber",) + def _parse_phonenumber(self, value: str) -> "PhoneNumber": try: return phonenumbers.parse(value, self.country) @@ -75,18 +73,17 @@ def _parse_phonenumber(self, value: str) -> "PhoneNumber": ) -@register( - "phonenumber-nl", - verbose_name=_("Dutch phone number"), - for_components=("phoneNumber",), -) -class DutchPhoneNumberValidator(PhoneNumberBaseValidator): +@register("phonenumber-nl") +class DutchPhoneNumberValidator(BasePlugin[str], PhoneNumberBaseValidator): country = "NL" country_name = _("Dutch") error_message = _( "Not a valid dutch phone number. An example of a valid dutch phone number is 0612312312" ) + verbose_name = _("Dutch phone number") + for_components = ("phoneNumber",) + def _parse_phonenumber(self, value: str) -> "PhoneNumber": try: return phonenumbers.parse(value, self.country)