Make '_is_jsonapi' route option a route default

#5: that happened! Now retesting.

1. --- a/src/EventSubscriber/DefaultExceptionSubscriber.php
   +++ b/src/EventSubscriber/DefaultExceptionSubscriber.php
   

   The changes in this class feel out of scope based on the issue title and summary.

2. +++ b/src/ParamConverter/EntityUuidConverter.php
   @@ -43,7 +44,7 @@ class EntityUuidConverter extends EntityConverter {
   -      $route->getOption('_is_jsonapi') &&
   +      Routes::hasJsonApiParameters($route) &&
   
   +++ b/src/Routing/JsonApiParamEnhancer.php
   @@ -51,7 +51,7 @@ class JsonApiParamEnhancer implements RouteEnhancerInterface {
      public function applies(Route $route) {
   -    return Routes::isJsonApiRoute($route->getDefaults());
   +    return Routes::hasJsonApiParameters($route) && Routes::getResourceTypeFromRouteDefaults($route->getDefaults());
      }
   
   +++ b/src/Routing/RouteEnhancer.php
   @@ -18,7 +18,7 @@ class RouteEnhancer implements RouteEnhancerInterface {
      public function applies(Route $route) {
   -    return Routes::isJsonApiRoute($route->getDefaults());
   +    return Routes::hasJsonApiParameters($route) && Routes::getResourceTypeFromRouteDefaults($route->getDefaults());
      }
   

   These changes look fine, but they got me thinking…

   … what if instead of inspecting this, we'd simply check whether the route has a resource_type route parameter?

   Because all of these things only need to happen in that case.

   Then we wouldn't need these classes calling a method on Routes!

   I think I'm missing something… 🤔

3. +++ b/tests/src/Unit/Routing/JsonApiParamEnhancerTest.php
   @@ -51,17 +48,11 @@ class JsonApiParamEnhancerTest extends UnitTestCase {
   -    $request = $this->prophesize(Request::class);
   -    $query = $this->prophesize(ParameterBag::class);
   -    $query->get('filter')->willReturn(['filed1' => 'lorem']);
   -    $query->has(Argument::type('string'))->willReturn(FALSE);
   -    $query->has('filter')->willReturn(TRUE);
   -    $request->query = $query->reveal();
   -
        $defaults = $object->enhance([
          RouteObjectInterface::CONTROLLER_NAME => Routes::FRONT_CONTROLLER,
          Routes::RESOURCE_TYPE_KEY => new ResourceType('foo', 'bar', NULL),
   -    ], $request->reveal());
   +      Routes::JSON_API_ROUTE_FLAG => TRUE,
   +    ], Request::create('/jsonapi/foo/bar', 'GET', ['filter' => ['filed1' => 'lorem']]));
   

   I sure like seeing this vast simplification three times!

+++ b/src/Routing/Routes.php
@@ -21,6 +23,13 @@ use Symfony\Component\Routing\RouteCollection;
+   * A key with which to flag a route as belonging to the JSON API module.
...
+  const JSON_API_ROUTE_FLAG = '_is_jsonapi';

+++ b/tests/src/Unit/Routing/JsonApiParamEnhancerTest.php
@@ -32,17 +32,14 @@ class JsonApiParamEnhancerTest extends UnitTestCase {
+      Routes::JSON_API_ROUTE_FLAG => TRUE,
+      Routes::RESOURCE_TYPE_KEY => 'foo--bar',

Nit: Once "flag", once "key".

7 files changed, 57 insertions, 80 deletions.

Nice!

So, we'll still have/need that method regardless. It makes sense to just use it for the "applicability" test though.

Maybe I wasn't clear enough. Let me play devil's advocate.

Why not just

  public function applies(Route $route) {
    return $route->hasDefault(Routes::RESOURCE_TYPE_KEY);
  }

instead of

  public function applies(Route $route) {
    return (bool) Routes::getResourceTypeFromRouteDefaults($route->getDefaults());
  }

?

BTW, zero matches when grepping for _is_jsonapi in jsonapi_extras! 👌

It seemed like it was nicer to use the existing public static method already in place and not tie the enhancers to the implementation internal

Good argument!

My rationale for proposing #13 instead was to not have one class call another class' public static @internal method.

Neither is perfect, both can be argued for. I don't have a strong preference. I just want us to careful consider all options. So that we hopefully don't have to revisit this again. Let's go with yours.

Just one last nit:

+++ b/src/EventSubscriber/DefaultExceptionSubscriber.php
@@ -65,18 +61,18 @@ class DefaultExceptionSubscriber extends SerializationDefaultExceptionSubscriber
-    return $format === 'api_json' || ($route && $route->getOption('_is_jsonapi'));
...
+    return $request->getRequestFormat() === 'api_json' || $request->attributes->get(Routes::JSON_API_ROUTE_FLAG_KEY);

+++ b/src/ParamConverter/EntityUuidConverter.php
@@ -43,7 +44,7 @@ class EntityUuidConverter extends EntityConverter {
-      $route->getOption('_is_jsonapi') &&
+    return (bool) Routes::getResourceTypeFromRouteDefaults($route->getDefaults());

+++ b/src/Routing/JsonApiParamEnhancer.php
@@ -51,7 +51,7 @@ class JsonApiParamEnhancer implements RouteEnhancerInterface {
-    return Routes::isJsonApiRoute($route->getDefaults());
+    return (bool) Routes::getResourceTypeFromRouteDefaults($route->getDefaults());

+++ b/src/Routing/RouteEnhancer.php
@@ -18,7 +18,7 @@ class RouteEnhancer implements RouteEnhancerInterface {
-    return Routes::isJsonApiRoute($route->getDefaults());
+    return (bool) Routes::getResourceTypeFromRouteDefaults($route->getDefaults());

This is the inconsistency being cleaned up. Sometimes code was inspecting the route option, sometimes it was calling this method on Routes. But note that we're still not being consistent: DefaultExceptionSubscriber is now the outlier.

I didn't know where my remark would lead, but I'm glad to hear you liked it and made the patch quite a bit simpler thanks to it :)

+++ b/src/Routing/Routes.php
@@ -246,17 +246,18 @@ class Routes implements ContainerInjectionInterface {
-   * Gets a route's resource type from its defaults.
+   * Gets the resource type from a route or request's parameters.
...
-  public static function getResourceTypeFromRouteDefaults(array $defaults) {
...
+  public static function getResourceTypeFromParameters(array $parameters) {

One final nit now that we're renaming this function anyway: we're not getting the resource type (a \Drupal\jsonapi\ResourceType\ResourceType object), but the resource type name.

Oh and there are 5 unused use statements causing CS violations, easy fix!

Bumping to Normal because this makes JSON API's code base less brittle and more maintainable!

Simpler & more robust code AND nearly 30 LoC less. ✅

 src/Controller/RequestHandler.php                                   |  1 -
 src/EventSubscriber/DefaultExceptionSubscriber.php                  | 33 ++++++++++++++-------------------
 src/ParamConverter/EntityUuidConverter.php                          |  3 ++-
 src/Routing/JsonApiParamEnhancer.php                                |  4 ++--
 src/Routing/RouteEnhancer.php                                       |  4 ++--
 src/Routing/Routes.php                                              | 42 ++++++++++++++++++------------------------
 src/StackMiddleware/FormatSetter.php                                | 22 ++++++++++++++--------
 tests/src/Unit/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php |  1 -
 tests/src/Unit/Routing/JsonApiParamEnhancerTest.php                 | 46 ++++++++++++----------------------------------
 9 files changed, 64 insertions(+), 92 deletions(-)

🎉