Skip to content

Commit

Permalink
Add deprecation warning for routing middleware
Browse files Browse the repository at this point in the history
While routing 'works' without an application plugin routing will not
work correctly.

Fixes #12740
  • Loading branch information
markstory committed Nov 28, 2018
1 parent ee8eb1c commit 9b48a65
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 12 deletions.
6 changes: 6 additions & 0 deletions src/Routing/Middleware/RoutingMiddleware.php
Expand Up @@ -59,6 +59,12 @@ class RoutingMiddleware
*/
public function __construct(HttpApplicationInterface $app = null, $cacheConfig = null)
{
if ($app === null) {
deprecationWarning(
'RoutingMiddleware should be passed an application instance. ' .
'Failing to do so can cause plugin routes to not behave correctly.'
);
}
$this->app = $app;
$this->cacheConfig = $cacheConfig;
}
Expand Down
38 changes: 27 additions & 11 deletions tests/TestCase/Routing/Middleware/RoutingMiddlewareTest.php
Expand Up @@ -61,7 +61,7 @@ public function testRedirectResponse()
$response = new Response();
$next = function ($req, $res) {
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$response = $middleware($request, $response, $next);

$this->assertEquals(301, $response->getStatusCode());
Expand All @@ -82,7 +82,7 @@ public function testRedirectResponseWithHeaders()
$response = new Response('php://memory', 200, ['X-testing' => 'Yes']);
$next = function ($req, $res) {
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$response = $middleware($request, $response, $next);

$this->assertEquals(301, $response->getStatusCode());
Expand All @@ -109,7 +109,7 @@ public function testRouterSetParams()
];
$this->assertEquals($expected, $req->getAttribute('params'));
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$middleware($request, $response, $next);
}

Expand All @@ -134,7 +134,7 @@ public function testPreservingExistingParams()
];
$this->assertEquals($expected, $req->getAttribute('params'));
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$middleware($request, $response, $next);
}

Expand Down Expand Up @@ -207,7 +207,7 @@ public function testRouterNoopOnController()
$next = function ($req, $res) {
$this->assertEquals(['controller' => 'Articles'], $req->getAttribute('params'));
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$middleware($request, $response, $next);
}

Expand All @@ -222,7 +222,7 @@ public function testMissingRouteNotCaught()
$response = new Response();
$next = function ($req, $res) {
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$middleware($request, $response, $next);
}

Expand Down Expand Up @@ -259,7 +259,7 @@ public function testFakedRequestMethodParsed()
$this->assertEquals($expected, $req->getAttribute('params'));
$this->assertEquals('PATCH', $req->getMethod());
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$middleware($request, $response, $next);
}

Expand Down Expand Up @@ -299,7 +299,7 @@ public function testInvokeScopedMiddleware()

return $res;
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$result = $middleware($request, $response, $next);
$this->assertSame($response, $result, 'Should return result');
$this->assertSame(['second', 'first', 'last'], $this->log);
Expand Down Expand Up @@ -344,7 +344,7 @@ public function testInvokeScopedMiddlewareReturnResponse()
$next = function ($req, $res) {
$this->fail('Should not be invoked as first should be ignored.');
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$result = $middleware($request, $response, $next);

$this->assertSame($response, $result, 'Should return result');
Expand Down Expand Up @@ -387,7 +387,7 @@ public function testInvokeScopedMiddlewareReturnResponseMainScope()
$next = function ($req, $res) {
$this->fail('Should not be invoked as second should be ignored.');
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$result = $middleware($request, $response, $next);

$this->assertSame($response, $result, 'Should return result');
Expand Down Expand Up @@ -438,7 +438,7 @@ public function testInvokeScopedMiddlewareIsolatedScopes($url, $expected)

return $res;
};
$middleware = new RoutingMiddleware();
$middleware = new RoutingMiddleware($this->app());
$result = $middleware($request, $response, $next);
$this->assertSame($response, $result, 'Should return result');
$this->assertSame($expected, $this->log);
Expand Down Expand Up @@ -543,4 +543,20 @@ public function testCacheConfigNotFound()

Cache::drop('_cake_router_');
}

/**
* Create a stub application for testing.
*
* @return \Cake\Core\HttpApplicationInterface
*/
protected function app()
{
$mock = $this->createMock(Application::class);
$mock->method('routes')
->will($this->returnCallback(function ($routes) {
return $routes;
}));

return $mock;
}
}
2 changes: 2 additions & 0 deletions tests/TestCase/TestSuite/IntegrationTestTraitTest.php
Expand Up @@ -624,6 +624,7 @@ public function testAssertTemplateAfterCellRender()
public function testArrayUrls()
{
$this->post(['controller' => 'Posts', 'action' => 'index', '_method' => 'POST']);
$this->assertResponseOk();
$this->assertEquals('value', $this->viewVariable('test'));
}

Expand All @@ -638,6 +639,7 @@ public function testArrayUrlsEmptyRouter()
$this->assertFalse(Router::$initialized);

$this->post(['controller' => 'Posts', 'action' => 'index']);
$this->assertResponseOk();
$this->assertEquals('value', $this->viewVariable('test'));
}

Expand Down
3 changes: 2 additions & 1 deletion tests/test_app/TestApp/Application.php
Expand Up @@ -48,7 +48,7 @@ public function console($commands)
*/
public function middleware($middleware)
{
$middleware->add(new RoutingMiddleware());
$middleware->add(new RoutingMiddleware($this));
$middleware->add(function ($req, $res, $next) {
/** @var \Cake\Http\ServerRequest $res */
$res = $next($req, $res);
Expand All @@ -69,6 +69,7 @@ public function routes($routes)
{
$routes->scope('/app', function (RouteBuilder $routes) {
$routes->connect('/articles', ['controller' => 'Articles']);
$routes->fallbacks();
});
}
}

0 comments on commit 9b48a65

Please sign in to comment.