Fix single valued Dexterity UID references not settable

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

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):

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)

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