Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support closures when using cache #141

Open
tienv2i opened this issue Aug 22, 2017 · 6 comments
Open

Support closures when using cache #141

tienv2i opened this issue Aug 22, 2017 · 6 comments

Comments

@tienv2i
Copy link

tienv2i commented Aug 22, 2017

I got an error when I use cachedDispatcher, I is normal when I use simpleDispatcher
The error is:

Fatal error: Call to undefined method Closure::__set_state() in E:\biiblog\src\Bii\route.cache on line 7

This is my code:

    function &dispatch()
    {
        $dispatcher = \FastRoute\cachedDispatcher(
            function(\FastRoute\RouteCollector $r){
                foreach ($this->routers as $router) {
                    $r->addRoute($router['method'],$router['route'] ,$router['handler']);
                }
        },[
            'cacheFile' => __DIR__ . '/route.cache', /* required */
        ]);

        $uri=isset($_SERVER['PATH_INFO'])?rtrim($_SERVER['PATH_INFO'],'\/'):"/";

        $method=$_SERVER["REQUEST_METHOD"];
        $routeInfo=$dispatcher->dispatch($method, $uri);

        switch($routeInfo[0]) {
            case \FastRoute\Dispatcher::NOT_FOUND:
                //NOT FOUND
                die("Page not found");
                break;
            case \FastRoute\Dispatcher::METHOD_NOT_ALLOWED:
                //NOT ALLOWED
                die("Method not allowed");
                break;
            case \FastRoute\Dispatcher::FOUND:
                $handler = $routeInfo[1];
                $vars = $routeInfo[2];

                if (is_callable($handler)){
                    $this->displayData=$handler($vars);
                }
                elseif(is_string($handler) && preg_match_all('#^([A-Za-z\\\]+)@([A-Za-z]+)#',$handler, $matches))
                {
                    if(class_exists($matches[1][0]) && is_callable(array($matches[1][0],$matches[2][0])) )
                    {
                        $class=new $matches[1][0];
                        $action=$matches[2][0];
                        $this->displayData=$class->$action($vars);
                    }
                    else
                    {
                        die($handler.' is invalid');
                    }
                }else
                {
                    die('Route is not callable!');
                }
                // ... call $handler with $vars
                break;
        }
        return $this;
    }
@staabm
Copy link

staabm commented Aug 22, 2017

Fatal error: Call to undefined method Closure::__set_state() in E:\biiblog\src\Bii\route.cache on line 7

the error message sounds like you try to serialize()/var_export() a Closure object which seems to not support this kind of operation.

@tienv2i
Copy link
Author

tienv2i commented Aug 22, 2017

I found that it has that when I use handler as a function. It is normal when I use handler as a string. Is there any way to fix this?

This has error:

$r->addRoute('GET', '/phpinfo', function()
    {
        phpinfo();
    }); 

This works well:

$r->addRoute('GET', '/phpinfo', 'phpinfo'); 

@staabm
Copy link

staabm commented Aug 22, 2017

for the "builtin php Closures are not serializable" problem, exists a separate lib
https://github.com/jeremeamia/super_closure

your caching adapter needs to take care of that, in case you really wanne cache such Closure style routes.
not a FastRoute issue though.

@nikic
Copy link
Owner

nikic commented Sep 17, 2017

A possible workaround that avoids super_closure:

Closures generally don't play well with any kind of caching (they aren't even serializable). The standard way to solve this is to replace the closures with a unique identifier (say, an incrementing number) and have a map from that identifier to the closure. The second map will have to be always reconstructed at runtime, that's unavoidable.

In any case, the README should explicitly mention that closures as handlers cannot be cached.

@smallfish500
Copy link

Accepting closure serialization (@staabm) or explicitly mentioning it unsupported inda doc (@nikic)? Why not adding flexibility to the $handler parameter (as strictness remains not de rigueur while using php) ?
https://opis.io/closure

@lcobucci
Copy link
Collaborator

lcobucci commented Sep 2, 2019

@smallfish500 we should mention it in the docs for sure.

For v2.0 we need to look at which approach to take to make things cacheable. I'd say having the map sounds more appealing to me than introducing new dependencies but we need to explore these options and benchmark things.

@lcobucci lcobucci changed the title Error when use cachedDispatcher Support closures when using cache Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants