JSON API should check entity access during routing, not in controller, for the "individual" route

+++ b/src/Routing/Routes.php
@@ -172,6 +172,7 @@ class Routes implements ContainerInjectionInterface {
+    $individual_route->setRequirement('_entity_access', "{$entity_type_id}.view"); // what about update/delete?

I'd be unopposed to routes per HTTP method.

Code looks good. However it seems that there is a misalignment between the previous error codes and the current error codes. I'm OK with that as long as:

1. The new codes still make sense semantically.
2. The new codes are not against the spec.

1. +++ b/src/Routing/Routes.php
   @@ -140,14 +140,29 @@ class Routes implements ContainerInjectionInterface {
   +    // Allow anybody access because "view" and "view label" access are checked
   +    // in the controller.
   

   "view" and "view label" can't be checked in the router, right?

2. +++ b/src/Routing/Routes.php
   @@ -140,14 +140,29 @@ class Routes implements ContainerInjectionInterface {
   +    if ($resource_type->isMutable()) {
   +      $collection_create_route = new Route($collection_route->getPath());
   +      $collection_create_route->setMethods(['POST']);
   +      $collection_create_route->addDefaults(['serialization_class' => JsonApiDocumentTopLevel::class]);
   +      $create_requirement = sprintf("%s:%s", $resource_type->getEntityTypeId(), $resource_type->getBundle());
   +      $collection_create_route->setRequirement('_entity_create_access', $create_requirement);
   +      $collection_create_route->setRequirement('_csrf_request_header_token', 'TRUE');
   +    }
   

   Now that it's more feasible, let's just move this route definition into the "individual" method in Routes.php. It was odd that it was in the collection method to begin with since it's dealing with individual entities.

3. +++ b/src/Routing/Routes.php
   @@ -201,13 +216,24 @@ class Routes implements ContainerInjectionInterface {
   +      $individual_update_route = new Route($individual_route->getPath());
   

   I think this is where the "create" route belongs.

Now that it's more feasible, let's just move this route definition into the "individual" method in Routes.php. It was odd that it was in the collection method to begin with since it's dealing with individual entities.

Oh boy, was I ever more wrong? It is not easy to move that to the other section and make the code flow nicely. I hope no one burnt time on that.

+++ b/src/Routing/Routes.php
@@ -140,14 +140,29 @@ class Routes implements ContainerInjectionInterface {
+    // The 'collection' route must always exist. So if the resource type is not
+    // locatable, assign the "create" route definition to it.
+    if ($resource_type->isLocatable()) {
+      $routes->add(static::getRouteName($resource_type, 'collection'), $collection_route);
+      if ($resource_type->isMutable()) {
+        $routes->add(static::getRouteName($resource_type, 'collection.post'), $collection_create_route);
+      }
+    }
+    if (!$resource_type->isLocatable() && $resource_type->isMutable()) {
+      $routes->add(static::getRouteName($resource_type, 'collection'), $collection_create_route);
+    }

This was really confusing to me. I tried to move things around and comment it in a way that seemed clearer.

Otherwise, this looks good to go.

+++ b/src/Routing/Routes.php
@@ -190,47 +195,65 @@ class Routes implements ContainerInjectionInterface {
+    if ($resource_type->isMutable()) {
...
+      $individual_update_route->setMethods(['PATCH']);
+      $individual_update_route->addDefaults(['serialization_class' => JsonApiDocumentTopLevel::class]);
+      $individual_update_route->setRequirement('_entity_access', "{$entity_type_id}.update");
+      $individual_update_route->setRequirement('_csrf_request_header_token', 'TRUE');
+      $routes->add(static::getRouteName($resource_type, 'individual.patch'), $individual_update_route);
+      $individual_remove_route = new Route("/{$path}/{{$entity_type_id}}");
+      $individual_remove_route->setMethods(['DELETE']);
+      $individual_remove_route->setRequirement('_entity_access', "{$entity_type_id}.delete");
+      $individual_remove_route->setRequirement('_csrf_request_header_token', 'TRUE');
+      $routes->add(static::getRouteName($resource_type, 'individual.delete'), $individual_remove_route);
+    }

It's nice to see that we essentially ended up in the same place. This is almost exactly what I had. There's one thing that #24 gets wrong though... To be "updatable" or "deletable" it must also be "locatable". That forces one to wrap these in yet another isLocatable() condition and that makes it messy.

I think we should go with something close to #20. I have one final concern about it:

+++ b/src/Routing/Routes.php
@@ -139,15 +139,29 @@ class Routes implements ContainerInjectionInterface {
+      $collection_create_route_name = $resource_type->isLocatable() ? 'collection.post' : 'collection';
+      $routes->add(static::getRouteName($resource_type, $collection_create_route_name), $collection_create_route);
...
     // `/jsonapi/node/article/{uuid}/relationships/uid`.

This will probably lead to a major headache for someone trying to do Url::fromRoute('jsonapi.$resource_type.collection.post') because somewhat mysteriously and apparently randomly, that route will cease to exist.

Perhaps we should just fix our tests rather than add this strange logic.

I'll try that.

IMO, this is ready unless tests fail (meaning I probably missed a usage of Url::fromRoute('jsonapi.$resource_type.collection')).

Note that the interdiff is against #20.

Test coverage++

I did miss a special case of Url::fromRoute('jsonapi.$resource_type.collection') in the entry point. Now that's more robust and won't provide links to collection URLs that don't exist :)

Darnit 🤗. Hit save too soon, patch attached.

Fixed the nit on commit, since @Wim Leers said "this is RTBC" and this is blocks #2953346: Define related/relationship routes per field, not dynamically (with route parameters that need validating).