Refuse a field name that is not a field

docs/changelog.rst

@@ -15,6 +15,8 @@ Changelog
    applied. Uninstalling from the same panel removes the PAS plugin
    and the per-user JWT signing secrets.
 
+- #116 Refuse a field name that is not a field
+- #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
@@ -47,6 +48,15 @@
 
 SKIP_UPDATE_FIELDS = ["id", ]
 
+# Keys that address the object or steer the request rather than naming
+# one of its fields. Everything else in a record is taken for a field.
+# `filename`, `mimetype` and `content_type` are read from the payload
+# by the file and image field managers, which take them as keyword
+# arguments rather than from a field of their own.
+CONTROL_FIELDS = ("content_type", "filename", "id", "mimetype",
+                  "parent_path", "parent_uid", "path", "portal_type",
+                  "transition", "uid")
+
 
 # -----------------------------------------------------------------------------
 #   JSON API (CRUD) Functions (called by the route providers)
@@ -315,11 +325,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/api/mutation.py

@@ -33,6 +33,7 @@
 """
 
 import copy
+import difflib
 
 import transaction
 from AccessControl import Unauthorized
@@ -48,12 +49,14 @@
 from senaite.jsonapi.api import convert_physical_paths_to_objects
 from senaite.jsonapi.api import do_transition_for
 from senaite.jsonapi.api import find_objects
+from senaite.jsonapi.api import get_fields
 from senaite.jsonapi.api import get_object
 from senaite.jsonapi.api import get_object_by_path
 from senaite.jsonapi.api import get_object_by_record
 from senaite.jsonapi.api import get_object_by_uid
 from senaite.jsonapi.api import is_root
 from senaite.jsonapi.api import make_items_for
+from senaite.jsonapi.api import CONTROL_FIELDS
 from senaite.jsonapi.api import SKIP_UPDATE_FIELDS
 from senaite.jsonapi.exceptions import BadRequestError
 from senaite.jsonapi.exceptions import ForbiddenError
@@ -290,6 +293,24 @@ def create_analysisrequest(container, **data):
     return _create_ar(container, request, data)
 
 
+def no_such_field(content, name):
+    """Say that a field is not there, and which one was probably meant
+
+    A name that matches no field and no setter used to be dropped with
+    a line in the log, and the request answered that the object had
+    been created or updated. A caller asking for `Service` instead of
+    `services` was told that all was well and got back an object with
+    nothing in it.
+    """
+    names = sorted(get_fields(content))
+    close = difflib.get_close_matches(name, names, n=1, cutoff=0.6)
+    message = "No field named '%s' on %s" % (
+        name, bika_api.get_portal_type(content))
+    if close:
+        message = "%s. Did you mean '%s'?" % (message, close[0])
+    return message
+
+
 def update_object_with_data(content, record):
     """Update `content` with the fields from `record`.
 
@@ -310,7 +331,7 @@ def update_object_with_data(content, record):
             raise BadRequestError("Update for this object is not allowed")
 
         purged = copy.deepcopy(record)
-        for key in SKIP_UPDATE_FIELDS:
+        for key in set(SKIP_UPDATE_FIELDS) | set(CONTROL_FIELDS):
             purged.pop(key, None)
 
         for k, v in purged.items():
@@ -322,8 +343,7 @@ def update_object_with_data(content, record):
                 raise BadRequestError(str(exc))
 
             if success is False:
-                logger.warning("update_object_with_data::skipping key=%r", k)
-                continue
+                raise BadRequestError(no_such_field(content, k))
             logger.debug("update_object_with_data::field %r updated", k)
 
         # Validate the whole object only for the field-manager path, where

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)