REST: top priorities for Drupal 8.3.x

As #2757967: API-first initiative already indicated, the goal is Make Drupal 8’s REST best-in-class.

First, let's start with moving over the uncompleted top priorities from #2721489: REST: top priorities for Drupal 8.2.x. Minus the one thing that can still happen in 8.2: addition of more test coverage: #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method.

This gives us a nice starting point, and shows how much still remains to be done from the (very ambitious) ambitions for 8.2. (Note that of the 34 issues are tagged with 8.2.0 release notes, 9 are for REST module!, that's more than 25%.)

And one bit of good news already: #2291055: REST resources for anonymous users: register was ready just barely in time for the 8.2.0 beta. Core committers decided to not commit it to 8.2.x anymore, but it has already been committed to 8.3.x! In other words: we're off to a flying start! :)

+1 for Serialization support for image style URLs!

Added #2768651: Let TimestampItem (de)normalize to/from RFC3339 timestamps, not UNIX timestamps, for better DX.

One very important area where rest.module in D8 is lacking today, is dogfooding. Tests are good. And we need more tests, which is why we're working on #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method.

But nothing beats dogfooding. (i.e. we build features on top of our own REST API.) For most of Drupal's features, it makes more sense to let PHP code call PHP code. Putting a REST API in between would be silly: lots of layers (PHP -> HTTP request -> PHP, which involves the networking stack, TCP/IP stack, HTTP parsing and generating, etc) that we don't actually need. So, it really only makes sense for Drupal for features that are not written in PHP. We have one of those now: Outside In — #2762505: Introduce "outside in" quick edit pattern to core.

One of the top priorities for REST (also for 8.2.x) was REST API CRUD support for config entities. We did the "R" but we still have to do the C, U and D. Why? Because validation of config entities does not live in the config entity API, but in their forms… which means our REST API can't call the same validation logic. So we first have to fix #2164373: [META] Untie config validation from form validation — enables validatable Recipes, decoupled admin UIs … and #1928868: Typed config incorrectly implements Typed Data interfaces before we can do #2300677: JSON:API POST/PATCH support for fully validatable config entities. Which is why it didn't go anywhere for 8.2.

So, let's make #2300677 the absolute top priority for 8.3. Because dogfooding, but also because it's the most commonly requested and expected capability that we don't have.

Oops. Clone issue doesn't actually clone an issue.

I just realized that there is a common thread for 2 of the top priority issues above, and there's a third in the queue: they're all gaps in how serialization works.

1. Serialization gap: image style URLs: #2517030: Add a URL formatter for the image field
2. Serialization gap: timezone handling unclear: #2768651: Let TimestampItem (de)normalize to/from RFC3339 timestamps, not UNIX timestamps, for better DX
3. Serialization gap: processed text: #2626924: Include processed text in normalizations: "text" field type's "processed" computed property should be non-internal and carry cacheability metadata

(Added that last one.)

Moved that set of three to the second place in the set of priorities, because they undermine the trustworthiness/perceived reliability of our REST API.

Thanks to https://www.previousnext.com.au/blog/we-could-add-default-content-drupal..., I've been made aware of more gaps in our serialization:

1. Serialization gap: taxonomy term's parent term: #2543726: Make $term->parent behave like any other entity reference field, to fix REST and Migrate support and de-customize its Views integration
2. Serialization gap: menu link content entities' content references: #2577923: MenuLinkContent entities pointing to nodes are not deployable: LinkItem should have a "target_uuid" computed property
3. Serialization gaps: path fields omitted from normalized entities: #2649646: Normalize path fields as part of the entity: allow REST clients to know the path alias

Thanks to #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, I was able to close #2772413: REST GET fails on entity/taxonomy_vocabulary/{id} 403 Forbidden with error, but in doing so the follow-up #2808233: REST 403 responses don't tell the user *why* access is not granted: requires deep Drupal understanding to figure out was filed. That's another very important way to improve DX.

Thanks to #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method I found another DX/RX problem: #2811133: Confusing error response set by ContentTypeHeaderMatcher when forgetting the Content-Type request header.

And another: #2813755: JSON responses encoded inconsistently: make them all RFC4627-compliant.

And another: #2813853: RequestHandler has its own error handling rather than leaving it to KernelEvents::EXCEPTION event subscribers.

And another: #2815845: Importing (deploying) REST resource config entities should automatically do the necessary route rebuilding.

Yay, good news for a change! #2811133: Confusing error response set by ContentTypeHeaderMatcher when forgetting the Content-Type request header landed, and was even backported to 8.2!

Opened more issues in the past 10 days:

1. Authentication WTF wrt anon: #2817737: A REST resource's "auth" configuration MUST be non-empty, which prevents configuring it for anon access only
2. Authentication WTF wrt cookies: #2817745: Add test coverage to prove that REST resource's "auth" configuration is also not allowing global authentication providers like "cookie" when not listed
3. HAL denormalization failing ungracefully: #2820743: FieldNormalizers are very unforgiving, impossible to debug error response given

And forgot one: #2820364: Entity + Field + Property validation constraints are processed in the incorrect order.

#2768651: Let TimestampItem (de)normalize to/from RFC3339 timestamps, not UNIX timestamps, for better DX was just refocused to only deal with TimestampItem (which lives in core itself), and #2824717: Add a format constraint to DateTimeItem to provide REST error message was filed against datetime.module to deal with DateTimeItem. Adding that new issue to the IS.

#2517030: Add a URL formatter for the image field was scoped to only REST export displays in Views. #2825487: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field was created to also address the "real" rest.module-affecting scope.

That was further split up, so the scope of each issue is more manageable: #2825812: ImageItem should have an "derivatives" computed property, to expose all image style URLs + #2825814: [PP-1] Allow URL query argument to limit the image style URLs returned by ImageItemNormalizer. The latter is a nice-to-have, so only adding the first to the serialization gap top priorities.

#2795965: REST requests with invalid X-CSRF-Token header get "missing " mesage was created as a follow-up to #2681911: REST requests without X-CSRF-Token header: unhelpful response significantly hinders DX, should receive a 401 response, which was the second DX top priority for REST in 8.2.x. I already got it fixed several weeks ago, but forgot to add it here.

@tedbow just remarked that entity reference field normalizations have target_type + target_uuid, but ignore those upon denormalization. That is of course wrong. Opened #2827164: Entity reference field normalization has target_type and target_uuid, but not used in denormalization, as a follow-up to #2060677: Add target_type, target_uuid to serialized output of entity reference fields in non-HAL formats.

Adding all the other relevant issues uncovered by #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method that are sufficiently important but weren't yet listed here.

1. #2800873: Add XML GET REST test coverage, work around XML encoder quirks
2. #2805279: Routing system + authentication system + format-specific routes (e.g. those in rest.module) = frustrating, unhelpful 403 responses instead of 406 responses
3. #2805281: ?_format=hal_json error responses are application/json, yet should be application/hal+json
4. #2807263: Impossible to write unit tests involving Vocabulary, because TAXONOMY_HIERARCHY_(DISABLED|SINGLE|MULTIPLE) are defined in taxonomy.module instead of VocabularyInterface
5. #2807501: ResourceResponse::$responseData is serialized, but is never used: unserializing it on every Page Cache hit is a huge performance penalty
6. #2808217: To be able to view Vocabulary config entities via REST, one should not have to grant the 'administer taxonomy' permission
7. #2817727: Add test coverage to prove controller is called *after* authentication validation
8. #2820888: Cookie authentication: the user.login.http route never supports the 'hal_json' format or some other format, depending on module order
9. #2821013: EntityResource's field validation 403 message is inconsistent between POST and PATCH
10. #2821077: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors
11. #2824408: To be able to create/update/delete Term entities via REST, one should not have to grant the 'administer taxonomy' permission
12. #2824827: \Drupal\hal\Normalizer\ContentEntityNormalizer::denormalize() fails with fatal PHP error when bundles are missing from link relation types
13. #2825347: 'Accept' header still is used for determining response format of error responses, ?_format= has no effect AND serialization module's exception subscriber does not support status code 500 error responses
14. #2826391: CsrfAccessCheck should have proper error feedback for invalid/missing CSRF token query argument just like CsrfRequestHeaderAccessCheck
15. #2826407: PATCH + POST allowed format validation happens in RequestHandler::handle(), rather than in the routing system

Most of them belong in the DX bucket, some of them belong in the Any use case bucket, a single one belongs in the Fully decoupled bucket and a few belong in a new Test coverage: velocity+maintainability+reliability+predictability bucket.

… that means I did not list any of these as top priorities:

1. #2820315: Blocks' 'view' operation is interpreted as "render on page" instead of "read this config entity"
2. #2821711: Apache always sets Content-Type: text/html, even for DELETE requests
3. #2822611: Document why UserInterface + FileInterface + MenuLinkContentInterface + … extend \Drupal\Core\Entity\ContentEntityInterface

#2807501: ResourceResponse::$responseData is serialized, but is never used: unserializing it on every Page Cache hit is a huge performance penalty landed, yay!

#2826391: CsrfAccessCheck should have proper error feedback for invalid/missing CSRF token query argument just like CsrfRequestHeaderAccessCheck landed.

While working on #2829327: Review JSON API module for core-readiness because we want to get the JSON API module into core as an experimental module (see #2757967: API-first initiative), I ran into at least one blocker: #2758897: Move rest module's "link manager" services to serialization module.

I think it makes sense to include "JSON API blockers" as top priorities for 8.3 too. So, added a new section for that to the IS, and added that issue as the first one for now.

#2820888: Cookie authentication: the user.login.http route never supports the 'hal_json' format or some other format, depending on module order landed.

#2805281: ?_format=hal_json error responses are application/json, yet should be application/hal+json is bad for DX and it turns out to be a JSON API blocker — see #2829977-13: SerializableHttpException and Drupal\jsonapi\Error\* are not necessary.

#2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method landed!

Unpostponed:

1. #2135829: EntityResource: translations support
2. #2821013: EntityResource's field validation 403 message is inconsistent between POST and PATCH
3. #2808233: REST 403 responses don't tell the user *why* access is not granted: requires deep Drupal understanding to figure out
4. #2820364: Entity + Field + Property validation constraints are processed in the incorrect order
5. #2813853: RequestHandler has its own error handling rather than leaving it to KernelEvents::EXCEPTION event subscribers
6. #2113345: Define a mechanism for custom link relationships
7. #2824572: Write EntityResourceTestBase subclasses for every other entity type.
8. #2824576: Delete old REST test coverage: (Read|Create|Update|Delete)Test, deprecate RESTTestBase
9. #2626924: Include processed text in normalizations: "text" field type's "processed" computed property should be non-internal and carry cacheability metadata
10. … and many more issues that were not yet explicitly marked as postponed (because there was other work that could be done in the mean time and/or they were far from RTBC and hence didn't need to be postponed explicitly yet).

Adding #2824576: Delete old REST test coverage: (Read|Create|Update|Delete)Test, deprecate RESTTestBase and #2824572: Write EntityResourceTestBase subclasses for every other entity type. to the Test coverage: velocity+maintainability+reliability+predictability section.

#2824576: Delete old REST test coverage: (Read|Create|Update|Delete)Test, deprecate RESTTestBase landed.

Added #2835683: Remove HTML from EntityResource validation 422 exception message to the list of DX problems to fix.

#2813755: JSON responses encoded inconsistently: make them all RFC4627-compliant landed, which unblocked #2739617: Make it easier to write on4xx() exception subscribers :)

#2739617: Make it easier to write on4xx() exception subscribers blocks #2805281: ?_format=hal_json error responses are application/json, yet should be application/hal+json, also adding that to the top priorities.

#2739617: Make it easier to write on4xx() exception subscribers landed. This unblocked #2805281: ?_format=hal_json error responses are application/json, yet should be application/hal+json. (Which in turn is blocking #2827084: EntityNormalizer::denormalize should not throw UnexpectedValueException, but \Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException, so we get a 422 response instead of 400.)

Yeah this one is not really a plan that needs to be approved. This is all just work we need to fix the brokenness/gaps in the existing REST API. As such, I don't think it needs framework/product manager sign-off.

The predecessor was #2721489: REST: top priorities for Drupal 8.2.x, which also didn't get (or need) sign-off.

But, at this point, I really don't know anymore. maybe this is the right place? Then #2721489: REST: top priorities for Drupal 8.2.x should also be moved.

#2824827: \Drupal\hal\Normalizer\ContentEntityNormalizer::denormalize() fails with fatal PHP error when bundles are missing from link relation types landed!

#2805281: ?_format=hal_json error responses are application/json, yet should be application/hal+json landed, which unblocked both #2813853: RequestHandler has its own error handling rather than leaving it to KernelEvents::EXCEPTION event subscribers and #2827084: EntityNormalizer::denormalize should not throw UnexpectedValueException, but \Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException, so we get a 422 response instead of 400!

Added #2838949: HttpException Handling Does Not Pass Headers to Responses — results e.g. in missing Allow header for 405 responses.

#2817727: Add test coverage to prove controller is called *after* authentication validation landed.

YAY!

Per #39 and #49, also moved #2721489 to this queue and marked as Active initiative: #2721489-51: REST: top priorities for Drupal 8.2.x.

#2820743: FieldNormalizers are very unforgiving, impossible to debug error response given landed!

#2813853: RequestHandler has its own error handling rather than leaving it to KernelEvents::EXCEPTION event subscribers landed.

#2838949: HttpException Handling Does Not Pass Headers to Responses — results e.g. in missing Allow header for 405 responses landed!

Added #2844046: REST Resource config entities do not respect the status (enabled/disabled).

#2113345: Define a mechanism for custom link relationships has landed, which is not listed in the IS, but it was a hard blocker for #2293697: EntityResource POST routes all use the confusing default: use entity types' https://www.drupal.org/link-relations/create link template if available since August 2016. I rerolled #2293697, and it will hopefully be committed soon, at last!

#2758897: Move rest module's "link manager" services to serialization module finally landed!

Added #2838144: Update "bundle missing" error message in entity denormalization to indicate which field is actually missing.

#2838144: Update "bundle missing" error message in entity denormalization to indicate which field is actually missing landed!

Forgot to strike through #2835683: Remove HTML from EntityResource validation 422 exception message.

WOOT, #2808233: REST 403 responses don't tell the user *why* access is not granted: requires deep Drupal understanding to figure out landed! :)

And on the same day, #2815845: Importing (deploying) REST resource config entities should automatically do the necessary route rebuilding landed too! This unblocked #2844046: REST Resource config entities do not respect the status (enabled/disabled), which is also listed as a top priority. Good progress today :)

#2827084: EntityNormalizer::denormalize should not throw UnexpectedValueException, but \Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException, so we get a 422 response instead of 400 landed! (And apparently that wasn't listed here.)

Thanks everybody for helping out here!

I'm proud of the progress we've made. We've tackled lots of tricky problems. rest.module is in a much better place now! We fixed the majority of things listed here.

Note that of the 53 issues are tagged with 8.3.0 release notes, 5 are for REST module!, that's almost 10%!

Now it's time to move on to 8.4.x. I've opened #2852860: REST: top priorities for Drupal 8.4.x for that, which is the continuation of this issue.

Thanks, and see you around!

(Sister post at #2721489-48: REST: top priorities for Drupal 8.2.x.)

Any use case

4 issues

Fully decoupled

2 issues (all of them!)

DX

15 issues (over half!)

Test coverage

4 issues (all really important ones, with just the less important ones remaining to get us to coverage for all entity types)

JSON API blockers

2 issues (all of them!)

Many of them were very difficult to land, requiring lots of consensus building, or requiring blockers to be solved that aren't listed here, often in other subsystems. Many of the ones that did not land are already well on their way, making it very likely that they will land in 8.4.

(#2827084 landed already, but only in 8.4, so moved that to the 8.4 top priorities issue.)