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