Simplify RequestHandler: make it no longer ContainerAware, split up ::handle()

The performance difference for point 2, measured with Page Cache turned off, best of 5, with testing this:

$ ab -c 1 -n 100 -H 'Authorization: Basic …' http://d8/node/1?_format=hal_json

There is no measurable difference. So then there is also no point in not injecting the serializer of course: better to do that, it's cleaner!

Point 2 was:

2. retrieves the serializer directly from the container, but does so unconditionally — if it's only retrieved for those requests that need it, then its service and the many services it depends upon only need to be initialized when necessary, which improves performance

IS updated.

This patch is relative to #2826407: PATCH + POST allowed format validation happens in RequestHandler::handle(), rather than in the routing system, hence this issue is marked postponed.

Now also fixing array() -> [].

RequestHandler::handle() is a 60-line function in which a lot of things happen. Splitting it up into multiple helper methods that are each called would make it far more maintainable.

I don't see why this is a good change. It just gives you a fun time if you actually debug that.

Huh? Isn't it the rule to use this whenever possible?

Ok. Will reroll this once #2826407: PATCH + POST allowed format validation happens in RequestHandler::handle(), rather than in the routing system lands. Thanks for checking!

#2826407 is now blocked on #2472337: Provide a lazy alternative to service collectors which just detects service IDs.

#2472337: Provide a lazy alternative to service collectors which just detects service IDs landed, #2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent has been rerolled and is now blocked on review. Once that's in, we can continue here!

As of #2858482-39: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent, #2858482 is blocked on another issue: #2883680: Force all route filters and route enhancers to be non-lazy. This therefore has a chain of 2 issues blocking it.

#2883680: Force all route filters and route enhancers to be non-lazy landed 2 months ago, ever since, we've only been blocked on #2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent, which has received another ~80 comments since #13 (😮), but is now RTBC! 🎉

Rebased #5.

Also: turns out this is no longer blocked on #2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent! Earlier iterations of #2858482 were changing RequestHandler probably, but that's no longer the case.

I forgot to update the unit test. Easy-peasy.

Done!

I'm not sure what you're asking, @neclimdul?

This is a controller, which doesn't have any BC guarantees: https://www.drupal.org/core/d8-bc-policy#controllers, so we're good :)

No — things in a route definition's defaults are never upcasted. We're not changing the logic you're citing at all: we're just making sure clearly separate concerns are in separate methods, making the RequestHandler easier to understand, maintain, debug.

Proof this is just being moved:

+++ b/core/modules/rest/src/RequestHandler.php
@@ -60,22 +68,51 @@ public function __construct(EntityStorageInterface $entity_storage, ConfigFactor
+    $resource_config_id = $route_match->getRouteObject()->getDefault('_rest_resource_config');
+    /** @var \Drupal\rest\RestResourceConfigInterface $resource_config */
+    $resource_config = $this->resourceStorage->load($resource_config_id);

@@ -89,18 +126,34 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
-    $resource_config_id = $route_match->getRouteObject()->getDefault('_rest_resource_config');
-    /** @var \Drupal\rest\RestResourceConfigInterface $resource_config */
-    $resource_config = $this->resourceStorage->load($resource_config_id);

Also, that'd be perfect to address in a follow-up, if it is indeed possible!

Yay, this already really helped: #1927648-501: Allow creation of file entities from binary data via REST requests shows that that this allows us to bring the number of changes in RequestHandler down from 140 lines to 60! (And each of the remaining changes is much, much simpler, no longer ambiguous.)

Thanks, @larowlan, and turns out that what you envisioned works, just not the upcasting bit (which is what I thought you meant and hence wrote#26). Already RTBC'd it :)