From 759f4f6388ada4073cae86a571c08490a423cef4 Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Tue, 6 Oct 2026 23:32:06 +0200 Subject: [PATCH] Fix single valued Dexterity UID references not settable Two separate faults, both of which end as a WrongContainedType or a WrongType on a field the request never mentions. #106 unwrapped a single valued reference from its list to a bare UID. An Archetypes UIDReferenceField needs that, and refuses a list with "[...] is not supported". A Dexterity UIDReferenceField derives from zope.schema List and validates as one, so the bare UID fails before it is ever stored, and setting e.g. a department's manager through the API stopped working. The unwrapping moves to the Archetypes field manager, through a to_field_value hook, so each side is handed the shape its own validator expects. The second fault is older and was hiding behind the first. A Dexterity type migrated from Archetypes keeps the old capitalised name as a BBB property, and a payload written against the old API uses it. get_field looked the name up exactly, so the schema field was not found and the data manager fell back to the BBB setter. Some of those setters convert the value, and many hand it to the mutator as it came: a duration stays the mapping JSON carried, a UID stays unicode. The schema spellings are tried after the exact name, and only a candidate naming a real field is used. There are 140 of these BBB properties on Dexterity content, and 112 of them name a schema field. Lowering the first letter reaches 43; the rest are snake case. A few keep neither convention (a sample template calls its field `samplepoint`), which is why the plain lower case form is tried too. The new doctest covers both faults, and fails if either fix is reverted. --- docs/changelog.rst | 1 + src/senaite/jsonapi/api/__init__.py | 36 +++- src/senaite/jsonapi/fieldmanagers.py | 37 +++- .../jsonapi/tests/doctests/uidreferences.rst | 173 ++++++++++++++++++ 4 files changed, 236 insertions(+), 11 deletions(-) create mode 100644 src/senaite/jsonapi/tests/doctests/uidreferences.rst diff --git a/docs/changelog.rst b/docs/changelog.rst index 3478b8d..ab864b5 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,7 @@ Changelog applied. Uninstalling from the same panel removes the PAS plugin and the per-user JWT signing secrets. +- #115 Fix single valued Dexterity UID references not settable - #109 Add a partition operation endpoint - #106 Fix single-valued UID reference fields not settable through the JSON API - #112 Allow updating the Laboratory through the API diff --git a/src/senaite/jsonapi/api/__init__.py b/src/senaite/jsonapi/api/__init__.py index 5750c13..3561a0a 100644 --- a/src/senaite/jsonapi/api/__init__.py +++ b/src/senaite/jsonapi/api/__init__.py @@ -20,6 +20,7 @@ import datetime import json +import re from Acquisition import ImplicitAcquisitionWrapper from bika.lims import api @@ -315,11 +316,44 @@ def get_fields(brain_or_object): return api.get_fields(obj) +def to_snake_case(name): + """Convert a CamelCase name to snake_case + + Acronyms are kept together: `AccreditationBodyURL` becomes + `accreditation_body_url`, not `..._u_r_l`. + """ + name = re.sub(r"(.)([A-Z][a-z]+)", r"\1_\2", name) + return re.sub(r"([a-z0-9])([A-Z])", r"\1_\2", name).lower() + + def get_field(brain_or_object, name, default=None): """Return the named field + + A Dexterity type that was migrated from Archetypes keeps the old + capitalised name as a BBB property, and a payload written against + the old API uses it. The schema field is named in snake case, so + the exact lookup misses and the caller falls back to the BBB + setter, which stores whatever it is given: a UID that arrived from + a JSON body as unicode then fails the field's own value type, and + says so on the whole object rather than on the value. + + The schema spellings are tried after the exact name, and only a + candidate that names a real field is used, so a name that converts + to nothing in particular is still nothing in particular. The plain + lower case form is there because the convention is not kept + everywhere: a sample template calls its field `samplepoint`. """ fields = get_fields(brain_or_object) - return fields.get(name, default) + candidates = [ + name, + name[:1].lower() + name[1:], + to_snake_case(name), + name.lower(), + ] + for candidate in candidates: + if candidate in fields: + return fields[candidate] + return default def get_behaviors(brain_or_object): diff --git a/src/senaite/jsonapi/fieldmanagers.py b/src/senaite/jsonapi/fieldmanagers.py index befd2b2..436ed43 100644 --- a/src/senaite/jsonapi/fieldmanagers.py +++ b/src/senaite/jsonapi/fieldmanagers.py @@ -729,17 +729,20 @@ def set(self, instance, value, **kw): # noqa # convert all references to UIDs refs = [str(api.get_uid(ref)) for ref in refs if ref] - # Single valued fields expect a scalar value, not a list. Passing a - # list makes the field validator of e.g. an AT UIDReferenceField - # reject the value with "[...] is not supported", so unwrap it here - # (None clears the reference). Multi valued fields keep the list. - if not self.multi_valued: - if len(refs) > 1: - raise ValueError("Multiple values given for single valued " - "field {}".format(repr(self.field))) - refs = refs[0] if refs else None + if not self.multi_valued and len(refs) > 1: + raise ValueError("Multiple values given for single valued " + "field {}".format(repr(self.field))) - return self._set(instance, refs, **kw) + return self._set(instance, self.to_field_value(refs), **kw) + + def to_field_value(self, refs): + """Shape the UIDs the way this field's own validator wants them. + + The two implementations disagree on a single valued field, and + both validate before they store, so the value has to arrive in + the shape each one expects. + """ + return refs @implementer(IFieldManager) @@ -750,10 +753,24 @@ def __init__(self, field): super(ATUIDReferenceFieldManager, self).__init__(field) self.multi_valued = field.multiValued + def to_field_value(self, refs): + """A single valued AT field wants the UID itself. + + Handing it a list makes its validator refuse the value with + "[...] is not supported". None clears the reference. + """ + if not self.multi_valued: + return refs[0] if refs else None + return refs + @implementer(IFieldManager) class DXUIDReferenceFieldManager(UIDReferenceFieldMixin, ZopeSchemaFieldManager): """Adapter to get/set the value of DX based UIDReferenceFields + + A single valued field keeps the list. The DX UIDReferenceField + derives from zope.schema List and validates as one, so a bare UID + fails its own validation before it is ever stored. """ def __init__(self, field): super(DXUIDReferenceFieldManager, self).__init__(field) diff --git a/src/senaite/jsonapi/tests/doctests/uidreferences.rst b/src/senaite/jsonapi/tests/doctests/uidreferences.rst new file mode 100644 index 0000000..715db4e --- /dev/null +++ b/src/senaite/jsonapi/tests/doctests/uidreferences.rst @@ -0,0 +1,173 @@ +UID REFERENCES +-------------- + +Setting a reference to another object through the field managers. + +A single valued field is the interesting case, because the two field +implementations want the value in different shapes and both validate +before they store. An Archetypes UIDReferenceField refuses a list with +"[...] is not supported". A Dexterity UIDReferenceField derives from +`zope.schema.List` and refuses a bare UID with WrongType. + +The payload is sent as JSON here, which is what makes the UIDs unicode: +a value that reaches the field unconverted fails its ASCIILine value +type, and the object is then refused on the whole-object validation +rather than where the value went in. + +Running this test from the buildout directory: + + bin/test test_doctests -t uidreferences + + +Test Setup +~~~~~~~~~~ + +Needed Imports: + + >>> import json + >>> import transaction + >>> from plone.app.testing import setRoles + >>> from plone.app.testing import TEST_USER_ID + + >>> from bika.lims import api + +Functional Helpers: + + >>> def post(url, data): + ... url = "{}/{}".format(api_url, url) + ... browser.post(url, json.dumps(data), "application/json") + ... return browser.contents + + >>> def get_item_object(response): + ... items = json.loads(response).get("items") + ... assert(len(items) == 1) + ... return api.get_object(items[0]["uid"]) + + >>> def create(data): + ... return get_item_object(post("create", data)) + + >>> def update(data): + ... return get_item_object(post("update", data)) + +Variables: + + >>> portal = self.portal + >>> portal_url = portal.absolute_url() + >>> api_url = "{}/@@API/senaite/v1".format(portal_url) + >>> setup = portal.setup + >>> bika_setup = portal.bika_setup + >>> browser = self.getBrowser() + >>> setRoles(portal, TEST_USER_ID, ["LabManager", "Manager"]) + >>> transaction.commit() + +Something to point at: + + >>> data = {"portal_type": "LabContact", + ... "parent_path": api.get_path(bika_setup.bika_labcontacts), + ... "Firstname": "Anna", + ... "Surname": "Meyer"} + >>> contact = create(data) + + >>> data = {"portal_type": "Department", + ... "parent_path": api.get_path(setup.departments), + ... "title": "Chemistry", + ... "department_id": "CHEM", + ... "manager": api.get_uid(contact)} + >>> department = create(data) + + +A single valued Dexterity reference +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +An analysis category names one responsible department, and the field is +declared `multi_valued=False`: + + >>> data = {"portal_type": "AnalysisCategory", + ... "parent_path": api.get_path(setup.analysiscategories), + ... "title": "Water Chemistry", + ... "department": api.get_uid(department)} + >>> category = create(data) + >>> api.get_uid(category.getDepartment()) == api.get_uid(department) + True + +The stored UID is a native string, which is what the field's ASCIILine +value type accepts: + + >>> from senaite.core.content.analysiscategory import ( + ... IAnalysisCategorySchema) + >>> field = IAnalysisCategorySchema["department"] + >>> isinstance(field.get_raw(category), str) + True + +The same through the BBB name the type kept from its Archetypes days. +It has to end up in the same shape, because the object is validated as +a whole afterwards: + + >>> data = {"portal_type": "AnalysisCategory", + ... "parent_path": api.get_path(setup.analysiscategories), + ... "title": "Microbiology", + ... "Department": api.get_uid(department)} + >>> category2 = create(data) + >>> isinstance(field.get_raw(category2), str) + True + +A BBB name of more than one word has to reach the field as well. The +schema names it in snake case, so `RetentionPeriod` has to find +`retention_period` rather than `retentionPeriod`. That field is a +duration, which only its field manager knows how to build out of the +mapping JSON can carry: + + >>> data = {"portal_type": "SamplePoint", + ... "parent_path": api.get_path(setup.samplepoints), + ... "title": "Reservoir inlet", + ... "SamplingFrequency": {"days": 7}} + >>> sample_point = create(data) + >>> sample_point.sampling_frequency + datetime.timedelta(7) + +A list of one is accepted as well: + + >>> data = {"uid": api.get_uid(category2), + ... "department": [api.get_uid(department)]} + >>> category2 = update(data) + >>> api.get_uid(category2.getDepartment()) == api.get_uid(department) + True + +More than one is refused, because the field holds a single reference: + + >>> data = {"portal_type": "Department", + ... "parent_path": api.get_path(setup.departments), + ... "title": "Microbiology", + ... "department_id": "MICRO", + ... "manager": api.get_uid(contact)} + >>> other = create(data) + + >>> data = {"uid": api.get_uid(category2), + ... "department": [api.get_uid(department), api.get_uid(other)]} + >>> category2 = update(data) + Traceback (most recent call last): + ... + HTTPError: HTTP Error 400: Bad Request + + +A single valued Archetypes reference +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +An analysis service points at one default method, and that field is an +Archetypes UIDReferenceField, which wants the UID itself: + + >>> data = {"portal_type": "Method", + ... "parent_path": api.get_path(setup.methods), + ... "title": "Photometric Determination"} + >>> method = create(data) + + >>> data = {"portal_type": "AnalysisService", + ... "parent_path": api.get_path(bika_setup.bika_analysisservices), + ... "title": "Nitrate", + ... "Keyword": "NO3", + ... "Category": api.get_uid(category), + ... "Methods": [api.get_uid(method)], + ... "Method": api.get_uid(method)} + >>> service = create(data) + >>> api.get_uid(service.getMethod()) == api.get_uid(method) + True