From 3f5513241e03e635146b5ca3279add0572b95fe1 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 29 Jan 2025 15:35:49 +0100 Subject: [PATCH] :recycle: [#4689] Move upload processing for Objects API v2 Moved the file component key/value processing to the mapped variable processing instead of special casing this in the registration handler, since we will need access to the component types to handle editgrids which have file uploads inside, as these have different data paths *and* will require recursion as well since there can be editgrids inside editgrids that have this problem. The alternative is special casing repeating groups too, which breaks the mechanism to do component-specific post-processing in the dedicated function. This also updates the query for the submission variables so that if we have a more exact data path for an upload (inside a repeating group) that we use that instead of messing with the container editgrid which incorrectly gets replaced now. For uploads *not* in repeating groups, this data path is empty because it's identical to the variable key, so we can coalesce there at the DB level to calculate this in a unified way. See also #2713 that highlights the difficulties with how file uploads are now processed, which requires some proper re-structuring. --- .../contrib/objects_api/handlers/v2.py | 45 +++++++++++--- .../objects_api/submission_registration.py | 58 ++++++++++--------- 2 files changed, 70 insertions(+), 33 deletions(-) diff --git a/src/openforms/registrations/contrib/objects_api/handlers/v2.py b/src/openforms/registrations/contrib/objects_api/handlers/v2.py index c1399cbef2..c3b29b29b1 100644 --- a/src/openforms/registrations/contrib/objects_api/handlers/v2.py +++ b/src/openforms/registrations/contrib/objects_api/handlers/v2.py @@ -5,6 +5,7 @@ from collections.abc import Sequence from dataclasses import dataclass from datetime import date, datetime +from typing import cast from glom import Assign, Path, glom @@ -56,7 +57,31 @@ def process_mapped_variable( JSONValue | date | datetime ), # can't narrow it down yet, as the type depends on the component type component: Component | None = None, + attachment_urls: dict[str, list[str]] | None = None, ) -> AssignmentSpec | Sequence[AssignmentSpec]: + """ + Apply post-processing to a mapped variable. + + A mapped variable may have additional options that specify the behaviour of how the + values are translated before they end up in the Objects API record. Often, these + transformations are dependent on the component type being processed. + + :arg mapping: The mapping of form variable to destination path, including possible + component-specific configuration options that influence the mapping behaviour. + :arg value: The raw value of the form variable for the submission being processed. + The type/shape of the value depends on the variable/component data type being + processed and even the component configuration (such as multiple True/False). + :arg component: If the variable corresponds to a Formio component, the component + definition is provided, otherwise ``None``. + :arg attachment_urls: The registration plugin uploads attachments to a Documents API + and provides the API resource URL for each attachment. Keys are the data paths in + the (Formio) submission data, e.g. `myAttachment` or ``repeatingGroups.2.file``. + + :returns: A single assignment spec or collection of assignment specs that specify + which value needs to be written to which "object path" for the record data, for + possible deep assignments. + """ + target_path = Path(*bits) if (bits := mapping.get("target_path")) else None # normalize non-primitive date/datetime values so that they're ready for JSON @@ -95,14 +120,20 @@ def process_mapped_variable( assert target_path is not None return AssignmentSpec(destination=target_path, value=value) - # multiple files - return an array - case {"type": "file", "multiple": True}: - assert isinstance(value, list) + case {"type": "file", **rest}: + assert attachment_urls is not None + multiple = rest.get("multiple", False) + upload_urls = attachment_urls[mapping["variable_key"]] - # single file - return only one element - case {"type": "file"}: - assert isinstance(value, list) - value = value[0] if value else "" + transformed_value = str | list[str] + + # for multiple uploads, replace the Formio file dicts with our upload URLs + if multiple: + transformed_value = upload_urls + # single file - return only one element *if* there are uploads + else: + transformed_value = upload_urls[0] if upload_urls else "" + value = cast(JSONValue, transformed_value) case {"type": "map"}: assert isinstance(value, dict) diff --git a/src/openforms/registrations/contrib/objects_api/submission_registration.py b/src/openforms/registrations/contrib/objects_api/submission_registration.py index 06e3396758..3f73dbeb8a 100644 --- a/src/openforms/registrations/contrib/objects_api/submission_registration.py +++ b/src/openforms/registrations/contrib/objects_api/submission_registration.py @@ -15,7 +15,9 @@ override, ) -from django.db.models import F +from django.db import models +from django.db.models import F, Value +from django.db.models.functions import Coalesce, NullIf from openforms.authentication.service import AuthAttribute from openforms.contrib.objects_api.clients import ( @@ -478,11 +480,19 @@ def get_attachment_urls_by_key(submission: Submission) -> dict[str, list[str]]: attachments = ObjectsAPISubmissionAttachment.objects.filter( submission_file_attachment__submission_variable__submission=submission ).annotate( - variable_key=F("submission_file_attachment__submission_variable__key") + data_path=Coalesce( + NullIf( + F("submission_file_attachment___component_data_path"), + Value(""), + ), + # fall back to variable/component key if no explicit data path is set + F("submission_file_attachment__submission_variable__key"), + output_field=models.TextField(), + ), ) for attachment_meta in attachments: key: str = ( - attachment_meta.variable_key # pyright: ignore[reportAttributeAccessIssue] + attachment_meta.data_path # pyright: ignore[reportAttributeAccessIssue] ) urls_map[key].append(attachment_meta.document_url) return urls_map @@ -528,28 +538,21 @@ def get_record_data( variable = None value: JSONValue | date | datetime - # special casing documents - we transform the formio file upload data into - # the api resource URLs for the uploaded documents in the Documens API. - # Normalizing to string/array of strings is done later via - # process_mapped_variable which receives the component configuration. - if key in urls_map: - value = urls_map[key] # pyright: ignore[reportAssignmentType] - else: - try: - value = all_values[key] - except KeyError: - logger.info( - "Expected key %s to be present in the submission (%s) variables, " - "but it wasn't. Ignoring it.", - key, - submission.uuid, - extra={ - "submission": submission.uuid, - "key": key, - "mapping_config": mapping, - }, - ) - continue + try: + value = all_values[key] + except KeyError: + logger.info( + "Expected key %s to be present in the submission (%s) variables, " + "but it wasn't. Ignoring it.", + key, + submission.uuid, + extra={ + "submission": submission.uuid, + "key": key, + "mapping_config": mapping, + }, + ) + continue # Look up if the key points to a form component that provides additional # context for how to process the value. @@ -562,7 +565,10 @@ def get_record_data( # process the value so that we can assign it to the record data as requested assignment_spec = process_mapped_variable( - mapping=mapping, value=value, component=component + mapping=mapping, + value=value, + component=component, + attachment_urls=urls_map, ) if isinstance(assignment_spec, AssignmentSpec): assignment_specs.append(assignment_spec)