diff --git a/docs/changelog.rst b/docs/changelog.rst index 3478b8d..b88353a 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. +- #117 Answer a refused field value with 400, not 500 - #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/fieldmanagers.py b/src/senaite/jsonapi/fieldmanagers.py index befd2b2..d12d34d 100644 --- a/src/senaite/jsonapi/fieldmanagers.py +++ b/src/senaite/jsonapi/fieldmanagers.py @@ -31,6 +31,7 @@ from senaite.jsonapi import api from senaite.jsonapi import logger from senaite.jsonapi import underscore as u +from senaite.jsonapi.exceptions import BadRequestError from senaite.jsonapi.interfaces import IFieldManager from zope import interface from zope.interface import implementer @@ -86,7 +87,12 @@ def _set(self, instance, value, **kw): # TODO: Check security on the field level return self.field.set(instance, value) except (WrongType, WrongContainedType): - raise TypeError("WrongType: Field={} Value={}".format( + # A value the schema refuses came from the request, so this + # is a bad request and not a server fault. Raised as a + # TypeError it answered 500, and a caller composing many + # objects could not tell which payload was at fault, + # because the type carried no more than the message. + raise BadRequestError("WrongType: Field={} Value={}".format( self.field, value)) def _get(self, instance, **kw): @@ -369,8 +375,8 @@ def _set(self, instance, value, **kw): # validate the value error = self.field.validate(value, instance) if error: - raise ValueError("Invalid value for field {}: {}" - .format(self.name, error)) + raise BadRequestError("Invalid value for field {}: {}" + .format(self.name, error)) # id fields take only strings if self.name == "id": @@ -598,8 +604,9 @@ def set(self, instance, value, **kw): # noqa # Handle non multi valued fields if not self.multi_valued: if len(ref) > 1: - raise ValueError("Multiple values given for single valued " - "field {}".format(repr(self.field))) + raise BadRequestError( + "Multiple values given for single valued " + "field {}".format(repr(self.field))) else: ref = ref[0] return self._set(instance, ref, **kw) @@ -735,8 +742,9 @@ def set(self, instance, value, **kw): # noqa # (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))) + raise BadRequestError( + "Multiple values given for single valued " + "field {}".format(repr(self.field))) refs = refs[0] if refs else None return self._set(instance, refs, **kw)