#2948666: Remove JSON API's use of $context['cacheable_metadata'] had the side-effect/benefit of ensuring that ResourceResponses always contain a data value that normalizes to a JsonApiDocumentTopLevelValue object.
It does that with:
// Having just normalized the data, we can associate its cacheability with
// the response object.
assert($jsonapi_doc_object instanceof JsonApiDocumentTopLevelNormalizerValue);
This makes sense. JSON API always returns a JSON API document.
#2933062: [>=8.5.4] Spec Compliance: Return 400 for unrecognized/unsupported query parameters added validation of JSON API query parameters. Correctly, it attempts to respond with an errors JSON API document with a relevant hyperlink to the JSON API spec.
Unfortunately, it does this by returning a ResourceResponse whose $data value is a complete JSON API document in array form.
Why doesn't it return a JsonApiDocumentTopLevel object? That's what it's for right? You'd think! But...
Wah, wahh, it can't because that class can't handle JSON API error documents. It's even on the class comment:
* @todo Add the missing required members: 'error' and 'meta' or document why not.
* @todo Add support for the missing optional members: 'jsonapi', 'links' and 'included' or document why not.
*/
class JsonApiDocumentTopLevel {
So, what to do? Could we throw an exception? We could, but we don't actually have a generic exception class that's customizable. IOW, if we threw an BadRequestException, we couldn't add that helpful hyperlink to the JSON API spec.
One way to solve this would be to create that exception class.
OTOH, we could make it possible to create a JsonApiDocumentTopLevel object with errors from scratch and add that directly to the ResourceResponse object that the request validator returns.
I propose we do this second object. We'll need to add a value object to src/Normalizer/Value named ErrorObject to mirror the JSON API terminology. That object should implement CacheableDependencyInterface. From there, we would make it possible to construct a JsonApiDocumentTopLevel object with either an iterator of entities (EntityCollection) or an iterator of errors.... and never the twain shall meet.
I ofc assume @Wim Leers will find a three line solution instead :P so in that case, disregard!
(I really would like to get to a point where we're constructing documents out of value objects, rather than returning a mishmash of jsonapi and non-jsonapi non-value objects that normalize to a nest of value objects, this could be a small first-step in that direction).
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 2982478-9.patch | 684 bytes | wim leers |
| #4 | 2982478-4.patch | 3.74 KB | wim leers |
Comments
Comment #2
gabesullice"Critical" because this will unbreak HEAD.
Comment #3
gabesulliceComment #4
wim leersOops, #2977600-13: Spec Compliance: `_format` is a disallowed query parameter name should have been posted here:
Comment #5
wim leersI've proven that to be unnecessary above. Long-term, I agree that's what probably should happen. That's what we'll need for #2949807: Spec Compliance: Error responses are missing the `jsonapi` top-level member most likely.
Proven unnecessary.
Not quite 3 lines, but I did manage to make it a net negative diff :D
Patch is green, so self-RTBC'ing and committing to get HEAD to be green again!
Comment #6
wim leersMore accurate title, reflecting the root cause & solution,.
Comment #8
wim leersComment #9
wim leersThis introduced 2 CS violations.