Skip to content

Commit

Permalink
🐛 [#4131] Ensure that plugin validators are not AND-ed together
Browse files Browse the repository at this point in the history
The frontend applies OR-logic when evaluating validation plugins
on a Formio component, which the backend did not replicate
correctly.
  • Loading branch information
sergei-maertens committed Apr 8, 2024
1 parent 46f97fb commit 107845c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/openforms/formio/components/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def build_serializer_field(
)

if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

Check warning on line 178 in src/openforms/formio/components/custom.py

View check run for this annotation

Codecov / codecov/patch

src/openforms/formio/components/custom.py#L178

Added line #L178 was not covered by tests

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -290,7 +290,7 @@ def build_serializer_field(

validators = [BSNValidator()]
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

extra["validators"] = validators

Expand Down
12 changes: 6 additions & 6 deletions src/openforms/formio/components/vanilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def build_serializer_field(

# Run plugin validators at the end after all basic checks have been performed.
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -144,7 +144,7 @@ def build_serializer_field(

validators = []
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -271,7 +271,7 @@ def build_serializer_field(

# Run plugin validators at the end after all basic checks have been performed.
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -359,7 +359,7 @@ def build_serializer_field(

validators = []
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -400,7 +400,7 @@ def build_serializer_field(self, component: Component) -> serializers.BooleanFie
if required:
validators.append(validate_required_checkbox)
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -493,7 +493,7 @@ def build_serializer_field(self, component: Component) -> serializers.FloatField

validators = []
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down
33 changes: 26 additions & 7 deletions src/openforms/validations/drf_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
.. todo:: use type hints from drf-stubs when we add those to base dependencies.
"""

from collections.abc import Iterable

from rest_framework import serializers

from openforms.submissions.models import Submission
Expand All @@ -22,18 +24,35 @@ class PluginValidator:

requires_context = True

def __init__(self, plugin: str):
assert plugin in register, "Invalid plugin identifier specified"
self.plugin = plugin
def __init__(self, plugins: Iterable[str]):
assert all(
plugin in register for plugin in plugins
), "Invalid plugin identifier specified"
self.plugins = plugins

# FIXME: after deserializing the input data, this may actually be any python type,
# so the JSONValue type hint is not accurate.
def __call__(self, value: JSONValue, field: serializers.Field) -> None:
"""
Validate that the data conforms to at least one of the specified plugins.
Validation plugins on our Formio components operate on an OR-basis rather than
AND - the input is considered valid as soon as one of the validators treats
it as valid.
"""
submission = field.context.get("submission", None)
assert isinstance(
submission, Submission
), "You must pass the submission in the serializer context"
result = register.validate(self.plugin, value=value, submission=submission)
if result.is_valid:
return
raise serializers.ValidationError(result.messages, code="invalid")

# evaluate all plugins to collect the error messages in case none is valid
messages: list[str] = []
for plugin in self.plugins:
result = register.validate(plugin, value=value, submission=submission)
# as soon as one is valid -> abort, there are no validation messages to
# display
if result.is_valid:
return
messages += result.messages

raise serializers.ValidationError(messages, code="invalid")

0 comments on commit 107845c

Please sign in to comment.