Rename the entity parameter from the entity type ID to 'entity' for all routes

What do we gain from this? Slightly more consistency/less variation in code?

Ah, so this is a necessary step to be able to do this:

Without doing this, we'd need to do some gymnastics with a route enhancer to make this work because we can't change the name of the PHP argument to the controller methods.

i.e. to use a route enhancer to achieve the same.

Correct?

But aren't they already being upcast by core's route enhancer?

Thanks for #7. That helped!

The patch here looks good, but is blocked on #2987608: Move deserialization from RequestHandler to JsonApiParamEnhancer. Marking this postponed.

Also, to ensure I understand the end goal:

+++ b/src/Controller/RequestHandler.php
@@ -45,9 +45,8 @@ class RequestHandler {
-    $entity_type_id = $resource_type->getEntityTypeId();
-    if ($entity = $request->get($entity_type_id)) {
-      $parameters[$entity_type_id] = $entity;
+    if ($entity = $request->get('entity')) {
+      $parameters['entity'] = $entity;
     }

This code change is not the end goal, right?

If we want that entity to be automatically passed to the controller method

This is, right? Which would mean something like #2720233: Use arguments resolver in RequestHandler to have consistent parameter ordering, right? Do you plan to do that as part of #2987610: Remove RequestHandler class and service and add EntityResource methods to each route definition?

Thanks for confirming! 🙏

Sounds good! 👌

I would like to see a patch for that final issue then, so we can see how all these steps actually simplify things.

Rebased; #2949807: Spec Compliance: Error responses are missing the `jsonapi` top-level member conflicted with this slightly.

AFAICT this could happen before #2987608: Move deserialization from RequestHandler to JsonApiParamEnhancer? That issue is far trickier to get done than this one.

See #6 and #7 for the reason why we want to do this. I think this is a worthwhile simplification.

I am still unclear what we gain from this (@Wim Leers also asked about this in #4).

From #5:

The routing system inspects method parameter names to associate them with provided route parameters. These parameters are drawn from route defaults or the URL. Without doing this, we'd need to do some gymnastics with a route enhancer to make this work because we can't change the name of the PHP argument to the controller methods.

One would assume that would translate on code simplification (hinted in #7), as the gymnastics were removed. However the patch RTBCed in #25 is purely cosmetic and doesn't come with such simplifications.

With that in mind, I'm unsure about the value of this given that:

1. We move away from how Drupal core implements the parameter enhancers (our current implementation was drawn from there).
2. We break internal APIs, which may be used by JSON API Extras, JSON API Defaults, …

I don't feel strongly against this issue, but I'd like you to give it a second thought before merging. After that, if you still think the benefits I fail to see outweight the inconviniencies listed here, do not hesitate to merge.

Right, I think @gabesullice should show more clearly what the benefits are that this enables in next steps.

In my understanding, it boils down to this:

1. \Drupal\jsonapi\Controller\EntityResource::getIndividual() and other methods on that class have an EntityInterface $entity parameter
2. Right now we have custom logic in \Drupal\jsonapi\Controller\RequestHandler::handle() to map the route's parameter to the controller:
   

       $entity_type_id = $resource_type->getEntityTypeId();
       if ($entity = $request->get($entity_type_id)) {
         $parameters[$entity_type_id] = $entity;
       }
   

3. If we'd rename it from node, user etc to just entity, then RequestHandler can just use Symfony's ArgumentsResolver, which brings us one step closer to getting rid of RequestHandler altogether
4. Which would mean that we end up just with a single class with many controller methods: EntityResource. This means less code to maintain, fewer intermediate steps, and path paved for future customizations/features.

Correct, Gabe?

FWIW, I did a grep in JSON API Extras but was not able to find anywhere that this change would break. If it does break something, I'll pledge to write the patch ✋

👍