jasny / switch-route Goto Github PK

Thanks for a very useful script.
Tried this out as (and will use it in a project) it seemed logical that a switch approach would be a lot faster than traditional approaches to switching.
In my usecase of 50 odd routes its in the order of magnitude faster :) .. than others including fastroute :)

With regards to the routes a route of /user/shop/{id}/products/ do not correspond to url of /user/shop/cakestore/products/ .. not found but /user/shop/{id}/products does .. Is there a way to allow trailing slash in route so that route (example ) /user/shop/{shop}/products/ corresponds to /user/shop/cakestore/products/ AND /user/shop/{shop}/products/{id} corresponds to /user/shop/cakestore/products/45

As it works now a trailing slash is identified as another segment..

(I mainly route to a php file ['include' => 'products.php'] and need an array of variables in my case $params['{$vars[key]}'=>$segments[$vars[value]..] AND $params[0] for trimmed $path value )

Might be good to consider adding variable access for include files.. but since the level of difficulty adding in the return for 'include' route is quite low in my case its not necessary.. BUT allow for trailing slash in routes would be great..

I think the idea is brilliant to generate a route script like this..

Cheers

What do you think about using PHP-Parser for generating a code?
For example:

# src/Generator/AbstractGenerate.php
    protected function generateSwitchStmt(array $structure, int $level = 0): Switch_
    {
        $condition = new Coalesce(
            new ArrayDimFetch(new Variable('segments'), new LNumber($level)),
            new String_("\0")
        );
        $cases = [];
        foreach ($structure as $segment => $sub) {
            /** @var String_|null $caseCondition */
            $caseCondition = '*' === $segment ? null : new String_($segment);
            $caseStmts = [];
            if ($sub instanceof Endpoint) {
                $endpointSwitchCases = [];
                foreach ($sub->getUniqueRoutes() as [$methods, $route, $vars]) {
                    foreach ($methods as $method) {
                        $endpointSwitchCases[] = new Case_(
                            new String_($method),
                            [
                                // generate route statement
                            ]
                        );
                    }
                }
                $endpointSwitchStmt = new Switch_(new Variable('method'), $endpointSwitchCases);
                $caseStmts[] = $endpointSwitchStmt;
            } else {
                $caseStmts[] = $this->generateSwitchStmt($sub, $level + 1);
            }
            $caseStmts[] = new Break_(new LNumber($level + 1));
            $cases[] = new Case_(
                $caseCondition,
                $caseStmts
            );
        }
        return new Switch_($condition, $cases);
    }

And use:

    protected function generateSwitch(array $structure, int $level = 0): string
    {
        $factory = new BuilderFactory;
        $node = $factory->namespace('App\\SwitchRoute') // may be need wrap into function?
            ->addStmt($this->generateSwitchStmt($structure, $level))
            ->getNode();
        $code = (new Standard())->prettyPrintFile([$node]);
        return $code;
    }

will produce something like this:

<?php

namespace App\SwitchRoute;

switch ($segments[0] ?? '\000') {
    case '\000':
        switch ($method) {
            case 'GET':
                break;
        }
        break 1;
    case 'users':
        switch ($segments[1] ?? '\000') {
            case '\000':
                switch ($method) {
                    case 'GET':
                        break;
                    case 'POST':
                        break;
                }
                break 2;
            default:
              ...

Then can remove generateEndpoint method. Also need to handle generateNs and no more identations needed. If you interested, I can try to implement. But need your thoughts.

Can Endpoint contain constants for allowed methods?

   #\Jasny\SwitchRoute\Endpoint
    const METHOD_HEAD = 'HEAD';
    const METHOD_GET = 'GET';
    const METHOD_POST = 'POST';
    const METHOD_PUT = 'PUT';
    const METHOD_PATCH = 'PATCH';
    const METHOD_DELETE = 'DELETE';
    const METHOD_PURGE = 'PURGE';
    const METHOD_OPTIONS = 'OPTIONS';
    const METHOD_TRACE = 'TRACE';
    const METHOD_CONNECT = 'CONNECT';

In this case, we can simply use Endpoint :: METHOD_ * anywhere without fear of mis-naming. Also possible implement something like:

    #\Jasny\SwitchRoute\Endpoint::isAvailableMethod
    private function isAvailableMethod(string $method): bool
    {
        return $method === @constant(sprintf('%s::METHOD_%s', __CLASS__, $method));
    }

and then check method and remove the initialization in uppercase:

    #\Jasny\SwitchRoute\Endpoint::withRoute
    public function withRoute(string $method, $route, array $vars): self
    {
        if (!$this->isAvailableMethod($method)) {
            throw new InvalidRouteException("Unknown route method '$method'", 1);
        }
        if (isset($this->routes[$method])) {
            throw new InvalidRouteException("Duplicate route for '$method {$this->path}'");
        }

        $copy = clone $this;
        $copy->routes[$method] = $route;
        $copy->vars[$method] = $vars;

        return $copy;
    }

As you can see, this is also an example of why we also need to check a code (#3 (comment)). But of course, you can add one more exception, but it looks good. Just specify a subtype by code.

It should be trivial to switch from a path variable to a query parameter (or vice-versa). Using different actions prevents having to deal with too many situations in one action.

The syntax might be;

[
    "GET /users?(page={page})"                 => ["action" => "list-users"],
    "GET /users?filter={filter}&(page={page})" => ["action" => "query-users"],
    "GET /users?search={search}&(page={page})" => ["action" => "search-users"],
]

Using round braces means the query parameter is optional. In the above situation the list-users-filtered action is invoked if the request has a filter parameter.

Challenge is that if all query parameters that aren't defined are ignored, the first route is also valid if there is a filter query parameter. Also if a request has both a search and filter query param, what should be used?

Sorry to come out of nowhere with this, but I hope you don't mind if I ask a question about your scrutinizer.yml configuration.

I contribute to a number of PHP projects that use Scrutinizer, and we're fed-up with its proprietary static checker. While we generally like Scrutinizer's UI for displaying issues, we would prefer to use PHPStan (and only PHPStan) for static checking. According to the Scrutinizer documentation, it is possible to specify any style checkers, and as long as they output the expected "checkstyle" format, they are compatible with Scrutinizer's reporting, and can complement (or replace) the native checkers performed by "php-scrutinizer-run".

Amazingly, after doing a very targeted github search, your 3 repos are the only ones that seem to be attempting to configure phpstan this way.

So, to get around to my question: does this work for you? I can't seem to figure out how this is supposed to surface in the Scrutinizer UI. I've set up my repo the same way (in a PR branch), and I know that my "phpstan-checkstyle.xml" is being generated and stuffed with errors, but I don't see any hint of the generated information in the Scrutinizer interface.

I notice that you both use phpstan and run php-scrutinizer-run. Why is that? Are you also having trouble getting the phpstan reports to display?

Sorry to bomb you at random with all these questions, and I'll understand if you choose not to get involved.

Also I saw disadvantage (IMO). I can't set FQCN for route.controller key. I.e., ['controller' => IndexController::class].

You can, it just requires you to need to specify in the Invoker constructor. Maybe the default should be that you specify the FQCN. And other things as presets.

Originally posted by @jasny in #3 (comment)