Problem/Motivation
Discovered in #2932031-22: Comprehensive JSON API integration test coverage phase 2: for every entity type, complex use-cases:
Here,
contact_messageis another special case, because those entities are never stored. That means they can only bePOSTed. All other verbs are not allowed. And no routes should be created for them. We have no way to signal this just yet. Core's REST module achieves this viacontact_rest_resource_alter()overriding the default REST resource plugin class.
I was contemplating making JSON API respect this automatically for any entity type using theDrupal\Core\Entity\ContentEntityNullStoragestorage handler. But because JSON API has a single route for multiple methods, that's less easy to achieve, and merits more debate. So, created a new issue for that: [this issue]
See #2843755: EntityResource: Provide comprehensive test coverage for Message entity for the solution that core's REST module chose.
Proposed resolution
TBD
Remaining tasks
TBD
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2944977-8.patch | 13.99 KB | wim leers |
| #8 | interdiff.txt | 945 bytes | wim leers |
| #5 | 2944977-5.patch | 14.05 KB | wim leers |
Comments
Comment #2
gabesulliceCan you elaborate the difficulties?
Comment #3
wim leersNote that I also had to add a work-around to allow the
POSTtest forMessageto pass, because JSON API blindly generates aLocationresponse header, even if that created resource never is actually stored, and is therefore never even accessible via JSON API:Therefore the
selfandrelationshipsURLs in the JSON API response document are all really just URIs, not URLs: they only identify something, they're unable to locate something, because it is not stored, therefore unlocatable.Comment #4
wim leersSure!
In
\Drupal\jsonapi\Routing\Routes::routes():IOW: many routes support multiple methods. Which means we can't do the same as we do in core REST and its test coverage. Quoting the test coverage there (from
\Drupal\Tests\rest\Functional\EntityResource\Message\MessageResourceTestBase):It's okay that not exactly the same solution (and test coverage) will work. But it does mean it merits further discussion. That's what this issue is for :)
Comment #5
wim leersEt voila. This does what we need to happen, to prevent WTFs when trying to interact with
contact_messageentities. It's very similar to what #2843755: EntityResource: Provide comprehensive test coverage for Message entity did forrest.module.Comment #6
gabesullice❤️
Nit: this can be on one line and it saves an extra variable floating around.
$collection_route->setMethods($resource_type->isLocatable() ? ['GET', 'POST'] : ['POST']);Comment #7
gabesulliceComment #8
wim leers#6.2: ✔️
Comment #9
gabesulliceComment #10
wim leersYAY for fewer
@todos!Comment #12
wim leersOops, saved my comment before pushing 😊
Comment #13
wim leersJSON API was setting a
Locationheader even for unlocatable resource types. That was the bug. i.e. it was generating a link with a URL that is never going to return anything but an error.So, recategorizing this as a bug.