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

Router alt nested route #20

Merged
merged 10 commits into from
Feb 2, 2020
Merged

Router alt nested route #20

merged 10 commits into from
Feb 2, 2020

Conversation

yiiliveext
Copy link
Contributor

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Tests pass? ✔️

This is a little bit better implementation of the Route class.

There are a few significant disadvantages of the current implementation.

  1. It is the ability of jumping over handlers.
    Example
//Route
Route::get('/test')
                ->to(new ActionCaller(SiteController::class, 'test', $container))
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Sweet');
                    return $response;
                })
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Home');
                    return $response;
                })
                ->name('site/test'),
//Action
public function test(RouterInterface $router, ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    {
        $response = $this->responseFactory->createResponse();
        $response->getBody()->write('My');

        return $response;
    }

Everything is ok now. The output for both implementations is

My Sweet Home

What about breaking it? Let's try it!

Route::get('/test')
                ->to(new ActionCaller(SiteController::class, 'test', $container))
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Sweet');
                    return $response;
                })
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $handler->handle($request);
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Home');
                    return $response;
                })
                ->name('site/test'),

The output for the current inplementation is

We were unable to find the page /test. Home

The output for my implementation is

My Sweet Home
Route::get('/test')
                ->to(new ActionCaller(SiteController::class, 'test', $container))
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $handler->handle($request);
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Sweet');
                    return $response;
                })
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Home');
                    return $response;
                })
                ->name('site/test'),

The output for the current inplementation is

We were unable to find the page /test. Sweet Home

The output for my implementation is

My Sweet Home
  1. Ability to get an infinite loop.
Route::get('/test')
                ->to(new ActionCaller(SiteController::class, 'test', $container))
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $handler->dispath($request);
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Sweet');
                    return $response;
                })
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Home');
                    return $response;
                })
                ->name('site/test'),

The result for the current inplementation is

Infinite loop, HTTP ERROR 500

My implementation doesn't have the dispatch() method.

Route::get('/test')
                ->to(new ActionCaller(SiteController::class, 'test', $container))
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $handler->process($request, $handle);
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Sweet');
                    return $response;
                })
                ->prepend(function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($container) {
                    $response = $handler->handle($request);
                    $response->getBody()->write(' Home');
                    return $response;
                })
                ->name('site/test'),

The result for the current inplementation is

Infinite loop, HTTP ERROR 500

My implementation doesn't have the process() method.

Basically the only problem is that we don't have the Route object (as handler) in each middleware.
But I've created a fix for that in the router-fastroute package yiiliveext/router-fastroute@9384900. This is even better. We have the ability to get the current Route object from anywhere we can get RouterInterface::class from the container.

@samdark samdark added the status:code review The pull request needs review. label Feb 2, 2020
src/Route.php Show resolved Hide resolved
src/Route.php Show resolved Hide resolved
src/Route.php Outdated Show resolved Hide resolved
src/Route.php Outdated Show resolved Hide resolved
src/RouterFactory.php Show resolved Hide resolved
tests/RouteTest.php Outdated Show resolved Hide resolved
tests/RouteTest.php Outdated Show resolved Hide resolved
tests/RouteTest.php Outdated Show resolved Hide resolved
tests/RouteTest.php Outdated Show resolved Hide resolved
@samdark
Copy link
Member

samdark commented Feb 2, 2020

Overall 👍 but there are some important points to consider.

@samdark samdark added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Feb 2, 2020
@samdark samdark added this to the 1.0.0-alpha1 milestone Feb 2, 2020
@samdark samdark added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Feb 2, 2020
@samdark samdark merged commit b166898 into yiisoft:master Feb 2, 2020
@samdark
Copy link
Member

samdark commented Feb 2, 2020

Thanks!

@samdark samdark removed the status:code review The pull request needs review. label Feb 2, 2020
samdark added a commit to yiisoft/demo that referenced this pull request Feb 2, 2020
samdark added a commit to yiisoft/docs that referenced this pull request Feb 2, 2020
samdark added a commit to yiisoft/router-fastroute that referenced this pull request Feb 2, 2020
samdark added a commit that referenced this pull request Feb 2, 2020
samdark added a commit to yiisoft/yii-web that referenced this pull request Feb 2, 2020
@yiiliveext yiiliveext deleted the router-alt-nested-route branch February 3, 2020 00:33
devanych pushed a commit to yiisoft/yii-web that referenced this pull request Feb 1, 2021
devanych pushed a commit to yiisoft/yii-web that referenced this pull request Feb 1, 2021
devanych pushed a commit to yiisoft/yii-web that referenced this pull request Feb 1, 2021
devanych pushed a commit to yiisoft/yii-web that referenced this pull request Feb 1, 2021
devanych pushed a commit to yiisoft/yii-web that referenced this pull request Feb 1, 2021
real-master-100 pushed a commit to real-master-100/router-yii that referenced this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants