Add support for route parameters

[alancolant]

Correction find schema when prefix contains parameters

Summary

I add the ability to set custom parameters to route prefix, like:

        'prefix'           => 'graphql/{game_slug}',

This fix use $request->route()->uri() instead of $request->getPathInfo() to fix the schemaName detection to always return null due to Str::startsWith

• …

---

Type of change:

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update
• [ ] Misc. change (internal, infrastructure, maintenance, etc.)

Checklist:

• [ ] Existing tests have been adapted and/or new tests have been added
• [ ] Add a CHANGELOG.md entry
• [ ] Update the README.md
• [ ] Code style has been fixed via composer fix-style

[alancolant]

Ok, let's imagine I want a dynamic URL parameter at my Graphql entry point.

I'll start by setting the prefix at the config level to "graphql/{my_parameter}" like this:

return [
    ...
    'route'      => [
        // The prefix for routes; do NOT use a leading slash!
        'prefix'           => 'graphql/{my_parameter}',
        ...
    ],
    ...
]

The route generated will therefore make it possible to retrieve this parameter in this way in my queries:

$myParameter = request()->route()->parameter('my_parameter')

This works for the current version's default schema.

But in case I have several schemas, a "public" and a "private".

The controller tries to detect the schema using

if (!Str::startsWith($path, $routePrefix)) {
    return null;
}

In case the URL is "/graphql/parameter_arbitrary_value/private"

// When $path is retrieved using $request->getPathInfo(), the condition become

if (!Str::startsWith('/graphql/parameter_arbitrary_value/private', '/graphql/{my_parameter}')) { 
    return null; // Always return null and default schema is applied
}


//But, when $path is retrieved using $request->route()->uri(), the condition become
if (!Str::startsWith('/graphql/{my_parameter}/private', '/graphql/{my_parameter}')) { 
    return null;
}

//Everything works fine

[mfn]

Thanks for the clarification, that helps a lot!

Can you please add a test for the PR showing the change in action?

It will also help me figure which approach is the best.

Thanks

[alancolant]

Here, 2 tests have been added, one for the default schema and a second for the custom schema.

[alancolant]

I think a better approach would be to declare one and only one graphql route:

$prefix/{schemaName?}

And in your controller use

$schemaName = $request->route()->parameter('schemaName',$config->get('graphql.default_schema', 'default'));
//Instead of:
$routePrefix = $config->get('graphql.route.prefix', 'graphql');
$schemaName = $this->findSchemaNameInRequest($request, "/$routePrefix") ?: $config->get('graphql.default_schema', 'default');

But this is probably going to be a breaking change, since there will no longer be the possibility to retrieve routes named 'graphql.custom' using route('graphql.custom'), but it will have to use route('graphql',['schemaName'=>'custom']) instead

[mfn]

💥 😄

I'm not opposed to bump a major version for a breaking change, if the improves are worth it.

The routing code has undergone some major change in recent years (if you're invested in this PR, I encourage you to check out the route.php history and the related changes in the controller) and received a LOT of duck tape fixes, so we really need to be very considerate about any changes here.

I already had the feeling this won't be an easy PR but now I'm confident it's not. I'm on vacation for a good part of the summer months and I need focus time to wrap my head around this before proceeding. I'll revisit once I've time, but that will likely take months :/

[mfn]

(I moved the PR in draft mode until we've consensus, I hope this is fine with you)

[alancolant]

Alright I am fine with it. I will on my side try to implement the suggestion above. I will just fix the problem you have detected for the moment. Good holidays

[alancolant]

To avoid breaking changes, I have reworked the routes.php file so that the specified routes remain independent: graphql, graphql.custom, graphql.default. However, instead of using a parameter at the route level, I have opted to automatically instantiate a middleware that injects the schemaName variable into $request->server for each route.

I find this approach more flexible as it allows us to eventually inject other information that can be retrieved in any controller using $request->server('graphql.*').

I also took the opportunity to declare and move the route declaration into the GraphQL.php class within a parseRoute function.

All these changes have been tested, and some tests have been modified to verify the presence of the GraphQLHttpMiddleware for each generated route.

I have also removed the findSchemaNameInRequest function from the controller since we no longer need to detect this name from the URL.

Looking forward to your feedback on this proposal.

---

I also think it could be interesting in a future major version to define a GraphQLContext class that will be automatically injected into all Queries.

In this case, for dynamic URLs, it will be possible to retrieve these parameters directly by using something like:

public function resolve($root, $args, GraphQLContext $context, ResolveInfo $resolveInfo, Closure $getSelectFields): LengthAwarePaginator
{
  $routeParameter = $context->getRouteParameters()['name_of_param'];
  $routeParameter = $context->getRouteParameter('name_of_param');
}