🚧 [#5139] Remove DataContainer usage from iter_evaluate_rules

It's a quick test to see what happens if we were to remove the DataContainer from iter_evaluate_rules, and replace it with a FormioData instance.

Also added support for FormioData in recursive_apply to fix several failing tests.

test_two_actions_on_the_same_variable illustrates what happens when two actions are performed on the same variable.

Author: viktorvanwijk

src/openforms/formio/datastructures.py

@@ -162,12 +162,14 @@ class FormioData(UserDict):
 
     data: dict[str, JSONValue]
     _keys: set[str]
+    updates: dict[str, JSONValue]
     """
     A collection of flattened key names, for quicker __contains__ access
     """
 
     def __init__(self, *args, **kwargs):
         self._keys = set()
+        self.updates = {}
         super().__init__(*args, **kwargs)
 
     def __getitem__(self, key: str):
@@ -206,3 +208,6 @@ def __contains__(self, key: object) -> bool:
             return True
         except KeyError:
             return False
+
+    def track_updates(self, m: dict[str, JSONValue]) -> None:
+        self.updates.update(m)

src/openforms/formio/utils.py

@@ -447,6 +447,7 @@ def recursive_apply(
     JSON serialization unless transform_leaf flag is set to True where func is
     applied to the nested value as well.
     """
+    from openforms.formio.datastructures import FormioData
     match input:
         # string primitive - we can throw it into the template engine
         case str():
@@ -460,7 +461,7 @@ def recursive_apply(
             ]
 
         # mapping - map every key/value pair recursively
-        case dict():
+        case dict() | FormioData():
             return {
                 key: recursive_apply(nested_bit, func, transform_leaf, *args, **kwargs)
                 for key, nested_bit in input.items()

src/openforms/submissions/form_logic.py

@@ -21,6 +21,7 @@
     from .models import Submission, SubmissionStep
 
 
+# TODO-5139: replace `data` type with `FormioData`?
 @elasticapm.capture_span(span_type="app.submissions.logic")
 def evaluate_form_logic(
     submission: "Submission",
@@ -84,7 +85,8 @@ def evaluate_form_logic(
     submission_variables_state.set_values(data)
 
     rules = get_rules_to_evaluate(submission, step)
-    data_container = DataContainer(state=submission_variables_state)
+    initial_data = submission_variables_state.to_python()
+    data_for_evaluation = submission_variables_state.to_python()
 
     # 5. Evaluate the logic rules in order
     mutation_operations = []
@@ -97,16 +99,30 @@ def evaluate_form_logic(
     ):
         for operation in iter_evaluate_rules(
             rules,
-            data_container,
+            data_for_evaluation,
             submission=submission,
         ):
             mutation_operations.append(operation)
 
+    submission_variables_state.set_values(data_for_evaluation.updates)
+
     # 6. The variable state is now completely resolved - we can start processing the
     # dynamic configuration and side effects.
 
     # Calculate which data has changed from the initial, for the step.
-    updated_step_data = data_container.get_updated_step_data(step)
+    # TODO-5139: this should ideally be built up from mutation returns inside
+    #  iter_evaluate_rules
+    relevant_variables = submission_variables_state.get_variables_in_submission_step(
+        step, include_unsaved=True
+    )
+
+    updated_step_data = FormioData()
+    for key, variable in relevant_variables.items():
+        if (
+            not variable.form_variable
+            or variable.value != variable.form_variable.initial_value
+        ):
+            updated_step_data[key] = variable.value
     step.data = DirtyData(updated_step_data.data)
 
     # 7. finally, apply the dynamic configuration
@@ -129,8 +145,6 @@ def evaluate_form_logic(
     for mutation in mutation_operations:
         mutation.apply(step, config_wrapper)
 
-    initial_data = FormioData(data_container.initial_data)
-
     # XXX: See #2340 and #2409 - we need to clear the values of components that are
     # (eventually) hidden BEFORE we do any further processing. This is only a bandaid
     # fix, as the (stale) data has potentially been input for other logic rules.
@@ -155,11 +169,11 @@ def evaluate_form_logic(
             continue
 
         # clear the value
-        data_container.update({key: empty_value})
+        data_for_evaluation.update({key: empty_value})
         data_diff[key] = empty_value
 
     # 7.2 Interpolate the component configuration with the variables.
-    inject_variables(config_wrapper, data_container.data)
+    inject_variables(config_wrapper, data_for_evaluation.data)
 
     # 7.3 Handle custom formio types - TODO: this needs to be lifted out of
     # :func:`get_dynamic_configuration` so that it can use variables.
@@ -206,12 +220,14 @@ def check_submission_logic(
     submission_variables_state = submission.load_submission_value_variables_state()
     if unsaved_data:
         submission_variables_state.set_values(unsaved_data)
-    data_container = DataContainer(state=submission_variables_state)
+    data_for_evaluation = submission_variables_state.to_python()
 
     mutation_operations: list[ActionOperation] = []
-    for operation in iter_evaluate_rules(rules, data_container, submission):
+    for operation in iter_evaluate_rules(rules, data_for_evaluation, submission):
         mutation_operations.append(operation)
 
+    submission_variables_state.set_values(data_for_evaluation.updates)
+
     # we loop over all steps because we have validations that ensure unique component
     # keys across multiple steps for the whole form.
     #

src/openforms/submissions/logic/actions.py

@@ -175,8 +175,12 @@ def eval(
         context: DataMapping,
         submission: Submission,
     ) -> DataMapping:
+        state = submission.load_submission_value_variables_state()
+        value_variable = state.variables[self.variable]
         with log_errors(self.value, self.rule):
-            return {self.variable: jsonLogic(self.value, context)}
+            result = jsonLogic(self.value, context)
+            value_variable.to_python(result)
+            return {self.variable: value_variable.to_python(result)}
 
 
 @dataclass

src/openforms/submissions/logic/datastructures.py

@@ -37,13 +37,24 @@ def data(self) -> DataMapping:
         :return: A datamapping (key: variable key, value: variable value) ready for
           (template context) evaluation.
         """
+        # TODO: this .to_python is needed for conversion of date/datetimes. Not sure yet
+        #  if logic evaluation uses this in any way, though. Maybe we can get rid of it.
         dynamic_values = {
             key: variable.to_python() for key, variable in self.state.variables.items()
         }
         static_values = self.state.static_data()
         nested_data = FormioData({**dynamic_values, **static_values})
         return nested_data.data
 
+    @property
+    def data_without_to_python(self):
+        dynamic_values = {
+            key: variable.value for key, variable in self.state.variables.items()
+        }
+        static_values = self.state.static_data()
+        nested_data = FormioData({**dynamic_values, **static_values})
+        return nested_data.data
+
     def update(self, updates: DataMapping) -> None:
         """
         Update the dynamic data state.

src/openforms/submissions/logic/rules.py

@@ -3,11 +3,11 @@
 import elasticapm
 from json_logic import jsonLogic
 
+from openforms.formio.datastructures import FormioData
 from openforms.forms.models import FormLogic, FormStep
 
 from ..models import Submission, SubmissionStep
 from .actions import ActionOperation
-from .datastructures import DataContainer
 from .log_utils import log_errors
 
 
@@ -108,7 +108,7 @@ def get_current_step(submission: Submission) -> SubmissionStep | None:
 
 def iter_evaluate_rules(
     rules: Iterable[FormLogic],
-    data_container: DataContainer,
+    data_container: FormioData,
     submission: Submission,
 ) -> Iterator[ActionOperation]:
     """
@@ -137,15 +137,18 @@ def iter_evaluate_rules(
             triggered = False
             with log_errors(rule.json_logic_trigger, rule):
                 triggered = bool(
-                    jsonLogic(rule.json_logic_trigger, data_container.data)
+                    jsonLogic(rule.json_logic_trigger, data_container)
                 )
 
             if not triggered:
                 continue
 
             for operation in rule.action_operations:
                 if mutations := operation.eval(
-                    data_container.data, submission=submission
+                    data_container, submission=submission
                 ):
+                    # TODO-5139: not sure if FormioData should track the changes, seems
+                    #  like a task for iter_evaluate_rules itself.
                     data_container.update(mutations)
+                    data_container.track_updates(mutations)
                 yield operation

src/openforms/submissions/models/submission_value_variable.py

@@ -14,7 +14,7 @@
 
 from openforms.formio.service import FormioData
 from openforms.forms.models.form_variable import FormVariable
-from openforms.typing import DataMapping, JSONEncodable, JSONObject, JSONSerializable
+from openforms.typing import DataMapping, JSONEncodable, JSONObject, JSONSerializable, JSONValue
 from openforms.utils.date import format_date_value, parse_datetime, parse_time
 from openforms.variables.constants import FormVariableDataTypes
 from openforms.variables.service import VariablesRegistry, get_static_variables
@@ -253,6 +253,22 @@ def set_values(self, data: DataMapping) -> None:
                 continue
             variable.value = new_value
 
+    # TODO-5139: can this be combined with SubmissionValueVariablesState.get_data?
+    def to_python(self) -> FormioData:
+        """
+        Collect the total picture of data/variable values.
+
+        The dynamic values are augmented with the static variables.
+
+        :return: A datamapping (key: variable key, value: native python object for the
+            value) ready for (template context) evaluation.
+        """
+        dynamic_values = {
+            key: variable.to_python() for key, variable in self.variables.items()
+        }
+        static_values = self.static_data()
+        return FormioData({**dynamic_values, **static_values})
+
 
 class SubmissionValueVariableManager(models.Manager):
     def bulk_create_or_update_from_data(
@@ -361,7 +377,7 @@ class Meta:
     def __str__(self):
         return _("Submission value variable {key}").format(key=self.key)
 
-    def to_python(self) -> Any:
+    def to_python(self, value: JSONValue = None) -> Any:
         """
         Deserialize the value into the appropriate python type.
 
@@ -371,7 +387,10 @@ def to_python(self) -> Any:
         to correctly interpret the data. For the time being, this is not needed yet
         as we focus on NL first.
         """
-        if self.value is None:
+        if value is None:
+            value = self.value
+
+        if value is None:
             return None
 
         # it's possible a submission value variable exists without the form variable
@@ -390,7 +409,7 @@ def to_python(self) -> Any:
                     "submission_id": self.submission_id,
                 },
             )
-            return self.value
+            return value
 
         # we expect JSON types to have been properly stored (and thus not as string!)
         data_type = self.form_variable.data_type
@@ -402,37 +421,37 @@ def to_python(self) -> Any:
             FormVariableDataTypes.float,
             FormVariableDataTypes.array,
         ):
-            return self.value
+            return value
 
-        if self.value and data_type == FormVariableDataTypes.date:
-            if isinstance(self.value, date):
-                return self.value
-            formatted_date = format_date_value(self.value)
+        if value and data_type == FormVariableDataTypes.date:
+            if isinstance(value, date):
+                return value
+            formatted_date = format_date_value(value)
             naive_date = parse_date(formatted_date)
             if naive_date is not None:
                 aware_date = timezone.make_aware(datetime.combine(naive_date, time.min))
                 return aware_date.date()
 
-            maybe_naive_datetime = parse_datetime(self.value)
+            maybe_naive_datetime = parse_datetime(value)
             if maybe_naive_datetime is None:
                 return
 
             if timezone.is_aware(maybe_naive_datetime):
                 return maybe_naive_datetime.date()
             return timezone.make_aware(maybe_naive_datetime).date()
 
-        if self.value and data_type == FormVariableDataTypes.datetime:
-            if isinstance(self.value, datetime):
-                return self.value
-            maybe_naive_datetime = parse_datetime(self.value)
+        if value and data_type == FormVariableDataTypes.datetime:
+            if isinstance(value, datetime):
+                return value
+            maybe_naive_datetime = parse_datetime(value)
             if maybe_naive_datetime is None:
                 return
 
             if timezone.is_aware(maybe_naive_datetime):
                 return maybe_naive_datetime
             return timezone.make_aware(maybe_naive_datetime)
 
-        if self.value and data_type == FormVariableDataTypes.time:
-            return parse_time(self.value)
+        if value and data_type == FormVariableDataTypes.time:
+            return parse_time(value)
 
-        return self.value
+        return value

src/openforms/submissions/tests/form_logic/test_modify_variables.py

@@ -19,6 +19,50 @@
 
 
 class VariableModificationTests(TestCase):
+    def test_two_actions_on_the_same_variable(self):
+        form = FormFactory.create()
+        form_step = FormStepFactory.create(
+            form=form,
+            form_definition__configuration={
+                "components": [
+                    {"type": "date", "key": "date"},
+                ]
+            },
+        )
+        submission = SubmissionFactory.create(form=form)
+        submission_step = SubmissionStepFactory.create(
+            submission=submission,
+            form_step=form_step,
+            data={},
+        )
+
+        FormLogicFactory.create(
+            form=form,
+            json_logic_trigger=True,
+            actions=[
+                {
+                    "variable": "date",
+                    "action": {
+                        "type": "variable",
+                        "value": "2025-06-06",
+                    },
+                },
+                {
+                    "variable": "date",
+                    "action": {
+                        "type": "variable",
+                        "value": {"+": [{"var": "date"}, {"duration": "P1M"}]},
+                    },
+                }
+            ],
+        )
+
+        evaluate_form_logic(submission, submission_step, submission.data)
+
+        variables_state = submission.load_submission_value_variables_state()
+
+        self.assertEqual(str(variables_state.variables["date"].value), "2025-07-06")
+
     def test_modify_variable_related_to_step_being_edited(self):
         form = FormFactory.create()
         step1 = FormStepFactory.create(