Merge pull request #3484 from open-formulieren/fix/2668-make-fewer-re…

…quests

[#2688] Make fewer requests

src/openapi.yaml

@@ -8670,6 +8670,14 @@ components:
           additionalProperties: {}
           nullable: true
           description: For jq, pass a string containing the filter expression
+        cacheTimeout:
+          type: integer
+          maximum: 2147483647
+          minimum: 0
+          nullable: true
+          description: The responses for service fetch are cached to prevent repeating
+            the same request multiple times when performing the logic check. If specified,
+            the cached responses will expire after the timeout (in seconds).
       required:
       - id
       - name

src/openforms/js/components/admin/form_design/data/complete-form.js

@@ -316,6 +316,11 @@ const saveVariables = async (state, csrftoken) => {
             variable.serviceFetchConfiguration.queryParams
           );
         }
+
+        // cacheTimeout is a number, but if empty it needs to be null
+        if (variable.serviceFetchConfiguration.cacheTimeout === '') {
+          variable.serviceFetchConfiguration.cacheTimeout = null;
+        }
       }
     }
   });

src/openforms/js/components/admin/form_design/variables/ServiceFetchConfigurationForm.js

@@ -8,7 +8,7 @@ import ActionButton from 'components/admin/forms/ActionButton';
 import Field from 'components/admin/forms/Field';
 import Fieldset from 'components/admin/forms/Fieldset';
 import FormRow from 'components/admin/forms/FormRow';
-import {TextInput} from 'components/admin/forms/Inputs';
+import {NumberInput, TextInput} from 'components/admin/forms/Inputs';
 import JsonWidget from 'components/admin/forms/JsonWidget';
 import MappingArrayInput from 'components/admin/forms/MappingArrayInput';
 import Select from 'components/admin/forms/Select';
@@ -144,6 +144,32 @@ const ServiceFetchConfigurationForm = ({formik, selectExisting = false}) => {
               </Field>
             </FormRow>
 
+            <FormRow>
+              <Field
+                name="cacheTimeout"
+                fieldBox
+                required={false}
+                label={
+                  <FormattedMessage
+                    defaultMessage="Cache timeout"
+                    description="Label cache timeout"
+                  />
+                }
+                helpText={
+                  <FormattedMessage
+                    defaultMessage="After how many seconds should the cached response expire."
+                    description="Help text cache timeout"
+                  />
+                }
+              >
+                <NumberInput
+                  id="cacheTimeout"
+                  value={formik.values.cacheTimeout ?? ''}
+                  {...formik.getFieldProps('cacheTimeout')}
+                />
+              </Field>
+            </FormRow>
+
             <FormRow fields={['queryParams']}>
               <Field
                 name="queryParams"

src/openforms/js/components/admin/form_design/variables/ServiceFetchConfigurationPicker.js

@@ -25,6 +25,7 @@ const INITIAL_VALUES = {
   // These fields are mapped to mappingExpression on save
   jsonLogicExpression: {},
   jqExpression: '',
+  cacheTimeout: null,
 };
 
 const ServiceFetchConfigurationPicker = ({

src/openforms/submissions/form_logic.py

@@ -104,7 +104,10 @@ def evaluate_form_logic(
         name="collect_logic_operations", span_type="app.submissions.logic"
     ):
         for operation in iter_evaluate_rules(
-            rules, data_container, on_rule_check=evaluated_rules.append
+            rules,
+            data_container,
+            on_rule_check=evaluated_rules.append,
+            submission=submission,
         ):
             mutation_operations.append(operation)
 
@@ -228,7 +231,7 @@ def check_submission_logic(
     data_container = DataContainer(state=submission_variables_state)
 
     mutation_operations = []
-    for operation in iter_evaluate_rules(rules, data_container):
+    for operation in iter_evaluate_rules(rules, data_container, submission):
         # component mutations can be skipped as we're not in the context of a single
         # form step.
         if isinstance(operation, PropertyAction):

src/openforms/submissions/logic/actions.py

@@ -10,7 +10,7 @@
 from openforms.utils.json_logic import ComponentMeta
 from openforms.variables.models import ServiceFetchConfiguration
 
-from ..models import SubmissionStep
+from ..models import Submission, SubmissionStep
 from ..models.submission_step import DirtyData
 from .log_utils import log_errors
 from .service_fetching import perform_service_fetch
@@ -56,7 +56,10 @@ def apply(
         pass
 
     def eval(
-        self, context: DataMapping, log: Callable[[JSONValue], None]
+        self,
+        context: DataMapping,
+        log: Callable[[JSONValue], None],
+        submission: Submission,
     ) -> DataMapping | None:
         """
         Return a mapping [name/path -> new_value] with changes that are to be
@@ -201,6 +204,7 @@ def eval(
         self,
         context: DataMapping,
         log: Callable[[JSONValue], None],
+        submission: Submission,
     ) -> DataMapping:
         with log_errors(self.value, self.rule):
             log({"value": (value := jsonLogic(self.value, context))})
@@ -233,6 +237,7 @@ def eval(
         self,
         context: DataMapping,
         log: Callable[[JSONValue], None],
+        submission: Submission,
     ) -> DataMapping:
         # FIXME
         # https://github.com/open-formulieren/open-forms/issues/3052
@@ -246,7 +251,7 @@ def eval(
         else:  # the current way
             var = self.rule.form.formvariable_set.get(key=self.variable)
         with log_errors({}, self.rule):  # TODO proper error handling
-            result = perform_service_fetch(var, context)
+            result = perform_service_fetch(var, context, str(submission.uuid))
             log(asdict(result))
             return {var.key: result.value}
 

src/openforms/submissions/logic/rules.py

@@ -122,6 +122,7 @@ class EvaluatedRule:
 def iter_evaluate_rules(
     rules: Iterable[FormLogic],
     data_container: DataContainer,
+    submission: Submission,
     on_rule_check: Callable[[EvaluatedRule], None] = lambda noop: None,
 ) -> Iterator[ActionOperation]:
     """
@@ -161,7 +162,9 @@ def iter_evaluate_rules(
 
             for i, operation in enumerate(rule.action_operations):
                 log = partial(operator.setitem, evaluated_rule.action_log_data, i)
-                if mutations := operation.eval(data_container.data, log=log):
+                if mutations := operation.eval(
+                    data_container.data, log=log, submission=submission
+                ):
                     data_container.update(mutations)
                 yield operation
             on_rule_check(evaluated_rule)

src/openforms/submissions/logic/service_fetching.py

@@ -1,5 +1,9 @@
+import json
 from dataclasses import dataclass
 
+from django.core.cache import cache
+from django.core.cache.backends.base import DEFAULT_TIMEOUT
+
 import jq
 from json_logic import jsonLogic
 
@@ -18,7 +22,12 @@ class FetchResult:
     # response_headers: JSONObject
 
 
-def perform_service_fetch(var: FormVariable, context: DataMapping) -> FetchResult:
+sentinel = object()
+
+
+def perform_service_fetch(
+    var: FormVariable, context: DataMapping, submission_uuid: str = ""
+) -> FetchResult:
     """Fetch a value from a http-service, perform a transformation on it and
     return the result.
 
@@ -27,7 +36,11 @@ def perform_service_fetch(var: FormVariable, context: DataMapping) -> FetchResul
     and how to retrieve the data, and how to transform that data into something
     that can be stored in a form variable and be used in form fields and logic.
     The result is presented as a form variable value for a given submission
-    instance."""
+    instance.
+
+    The value returned by the request is cached using the submission UUID and the
+    arguments to the request (hashed to make a cache key).
+    """
 
     if not var.service_fetch_configuration:
         raise ValueError(
@@ -38,7 +51,22 @@ def perform_service_fetch(var: FormVariable, context: DataMapping) -> FetchResul
 
     client = fetch_config.service.build_client()
     request_args = fetch_config.request_arguments(context)
-    raw_value = client.request(**request_args)
+
+    if not submission_uuid:
+        raw_value = client.request(**request_args)
+    else:
+        cache_key = hash(submission_uuid + json.dumps(request_args, sort_keys=True))
+
+        raw_value = cache.get(cache_key, sentinel)
+        if raw_value is sentinel:
+            raw_value = client.request(**request_args)
+            cache.set(
+                cache_key,
+                raw_value,
+                timeout=fetch_config.cache_timeout
+                if fetch_config.cache_timeout is not None
+                else DEFAULT_TIMEOUT,
+            )
 
     match fetch_config.data_mapping_type, fetch_config.mapping_expression:
         case DataMappingTypes.jq, expression:

src/openforms/submissions/tests/test_variables/test_fetch_with_logic.py

@@ -2,6 +2,7 @@
 
 import requests_mock
 from factory.django import FileField
+from freezegun import freeze_time
 from rest_framework.reverse import reverse
 from rest_framework.test import APITestCase
 from zgw_consumers.constants import APITypes, AuthTypes
@@ -109,3 +110,128 @@ def test_it_handles_bad_service_responses(self, m):
         # if a prefill service is down or misconfigured, a submitter's values
         # shouldn't be overwritten
         self.assertEqual(response.data["step"]["data"], {})
+
+    @requests_mock.Mocker(case_sensitive=True)
+    def test_requests_not_made_multiple_times(self, m):
+        submission = SubmissionFactory.from_components(
+            [
+                {"type": "textfield", "key": "fieldA"},
+                {"type": "textfield", "key": "fieldB"},
+                {"type": "textfield", "key": "fieldC"},
+            ]
+        )
+        fetch_config1 = ServiceFetchConfigurationFactory.create(
+            service=self.service, path="get"
+        )
+        fetch_config2 = ServiceFetchConfigurationFactory.create(
+            service=self.service, path="get", query_params={"fieldC": ["{{ fieldC }}"]}
+        )
+
+        FormLogicFactory.create(
+            form=submission.form,
+            order=1,
+            json_logic_trigger=True,
+            actions=[
+                {
+                    "variable": "fieldA",
+                    "action": {
+                        "name": "Fetch some field from some server",
+                        "type": LogicActionTypes.fetch_from_service,
+                        "value": fetch_config1.id,
+                    },
+                }
+            ],
+        )
+        FormLogicFactory.create(
+            form=submission.form,
+            order=2,
+            json_logic_trigger=True,
+            actions=[
+                {
+                    "variable": "fieldB",
+                    "action": {
+                        "name": "Fetch some field from some server",
+                        "type": LogicActionTypes.fetch_from_service,
+                        "value": fetch_config2.id,
+                    },
+                }
+            ],
+        )
+
+        self._add_submission_to_session(submission)
+
+        m.get("https://httpbin.org/get", json=42)
+
+        endpoint = reverse(
+            "api:submission-steps-logic-check",
+            kwargs={
+                "submission_uuid": submission.uuid,
+                "step_uuid": submission.form.formstep_set.first().uuid,
+            },
+        )
+
+        response = self.client.post(endpoint, data={"data": {"fieldC": 42}})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(m.request_history), 2)
+        self.assertEqual(m.request_history[-1].url, "https://httpbin.org/get?fieldC=42")
+        self.assertEqual(m.request_history[-2].url, "https://httpbin.org/get")
+
+        response = self.client.post(endpoint, data={"data": {"fieldC": 43}})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(m.request_history), 3)
+        self.assertEqual(m.request_history[-1].url, "https://httpbin.org/get?fieldC=43")
+
+    @requests_mock.Mocker(case_sensitive=True)
+    def test_requests_cache_timeout(self, m):
+        submission = SubmissionFactory.from_components(
+            [
+                {"type": "textfield", "key": "fieldA"},
+            ]
+        )
+        fetch_config = ServiceFetchConfigurationFactory.create(
+            service=self.service, path="get", cache_timeout=30
+        )
+
+        FormLogicFactory.create(
+            form=submission.form,
+            order=1,
+            json_logic_trigger=True,
+            actions=[
+                {
+                    "variable": "fieldA",
+                    "action": {
+                        "name": "Fetch some field from some server",
+                        "type": LogicActionTypes.fetch_from_service,
+                        "value": fetch_config.id,
+                    },
+                }
+            ],
+        )
+
+        self._add_submission_to_session(submission)
+
+        m.get("https://httpbin.org/get", json=42)
+
+        endpoint = reverse(
+            "api:submission-steps-logic-check",
+            kwargs={
+                "submission_uuid": submission.uuid,
+                "step_uuid": submission.form.formstep_set.first().uuid,
+            },
+        )
+
+        with freeze_time("2023-02-21T18:00:00Z"):
+            response = self.client.post(endpoint, data={"data": {}})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(m.request_history), 1)
+        self.assertEqual(m.request_history[-1].url, "https://httpbin.org/get")
+
+        with freeze_time("2023-02-21T18:01:00Z"):
+            response = self.client.post(endpoint, data={"data": {}})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(m.request_history), 2)
+        self.assertEqual(m.request_history[-1].url, "https://httpbin.org/get")

src/openforms/variables/api/serializers.py

@@ -29,6 +29,7 @@ class Meta:
             "body",
             "data_mapping_type",
             "mapping_expression",
+            "cache_timeout",
         )
         validators = [
             ModelValidator[ServiceFetchConfiguration](validate_mapping_expression),

src/openforms/variables/migrations/0012_servicefetchconfiguration_cache_timeout.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.2.21 on 2023-09-20 08:30
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("variables", "0011_migrate_interpolation_format"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="servicefetchconfiguration",
+            name="cache_timeout",
+            field=models.PositiveIntegerField(
+                blank=True,
+                help_text="The responses for service fetch are cached to prevent repeating the same request multiple times when performing the logic check. If specified, the cached responses will expire after the timeout (in seconds).",
+                null=True,
+                verbose_name="cache timeout",
+            ),
+        ),
+    ]