Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

FastRouteRouter->addRoute() does not throw a RuntimeException of called after either match() or geerateUri() #60

Open
2 tasks done
MatthiasKuehneEllerhold opened this issue Oct 1, 2018 · 3 comments

Comments

@MatthiasKuehneEllerhold
Copy link

The Zend\Expressive\Router\RouterInface states on addRoute():

    /**
     * ...
     *
     * The method MUST raise Exception\RuntimeException if called after either `match()`
     * or `generateUri()` have already been called, to ensure integrity of the
     * router between invocations of either of those methods.
     *
     * @throws Exception\RuntimeException when called after match() or
     *     generateUri() have been called.
     */
    public function addRoute(Route $route) : void;

But no RuntimeException will be thrown / raised.

Code to reproduce the issue

class TestMiddleware implements \Psr\Http\Server\MiddlewareInterface
{
  public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface {
    return $handler->handle($request);
  }
}

$router = new \Zend\Expressive\Router\FastRouteRouter();
$router->addRoute('/test', new TestMiddleware());
$router->match($request);
$router->addRoute('/test2', new TestMiddleware());

Expected results

A Zend\Expressive\Router\Exception\RuntimeException is thrown indicating that no routes may be added after calling match().

Actual results

Scripts run without an exception.

Consequences

Is this a bug in the RouterInterface of "zend-expressive-router" or in the FastRouteRouter of this repo?
(Same "error" (?) is in the "zend-expressive-zendrouter").

@geerteltink
Copy link
Member

I guess this is a typo or something that's never implemented. I've checked all routers and they all do this (including older versions):

    public function addRoute(Route $route) : void
    {
        $this->routesToInject[] = $route;
    }

I guess you can even call it a feature to add more routes after you matched a route already :)

And to be honest, why would you have that check? When adding a lot of routes it only creates overhead.

@MatthiasKuehneEllerhold
Copy link
Author

So it'd be fine to remove this part of the docblock from zend-expressive?

@weierophinney
Copy link
Member

This repository has been closed and moved to mezzio/mezzio-fastroute; a new issue has been opened at mezzio/mezzio-fastroute#2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants