Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,13 @@ public function pipe($path, $middleware = null)
{
// Lazy-load middleware from the container when possible
$container = $this->container;
if (null === $middleware && is_string($path) && $container && $container->has($path, true)) {
if (null === $middleware && is_string($path) && $container && $container->has($path)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting for posterity: The boolean true argument existed because an earlier revision of zend-servicemanager for v3 defined it; when provided, it indicated whether or not to look in abstract factories when performing the check. We have since removed that flag in zend-servicemanager's develop branch as it violates the LSP, but also because it was leading to many subtle errors (it was far too easy to forget to add it, and have a false negative lookup).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has be done on #194. This PR is on top of that one for prevent merge conflicts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing to change, @Maks3w ; I made the comment in case somebody wonders what the purpose of that change was. The change is fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. :) Just the comment was better located on that PR :)

$middleware = $this->marshalLazyMiddlewareService($path, $container);
$path = '/';
} elseif (is_string($middleware)
&& ! is_callable($middleware)
&& $container
&& $container->has($middleware, true)
&& $container->has($middleware)
) {
$middleware = $this->marshalLazyMiddlewareService($middleware, $container);
} elseif (null === $middleware && is_callable($path)) {
Expand Down Expand Up @@ -263,13 +263,13 @@ public function pipeErrorHandler($path, $middleware = null)
{
// Lazy-load middleware from the container
$container = $this->container;
if (null === $middleware && is_string($path) && $container && $container->has($path, true)) {
if (null === $middleware && is_string($path) && $container && $container->has($path)) {
$middleware = $this->marshalLazyErrorMiddlewareService($path, $container);
$path = '/';
} elseif (is_string($middleware)
&& ! is_callable($middleware)
&& $container
&& $container->has($middleware, true)
&& $container->has($middleware)
) {
$middleware = $this->marshalLazyErrorMiddlewareService($middleware, $container);
} elseif (null === $middleware && is_callable($path)) {
Expand Down Expand Up @@ -542,7 +542,7 @@ private function checkForDuplicateRoute($path, $methods = null)
private function marshalMiddlewareFromContainer($middleware)
{
$container = $this->container;
if (! $container || ! $container->has($middleware, true)) {
if (! $container || ! $container->has($middleware)) {
return $middleware;
}

Expand Down
28 changes: 12 additions & 16 deletions test/ApplicationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

namespace ZendTest\Expressive;

use Interop\Container\ContainerInterface;
use PHPUnit_Framework_TestCase as TestCase;
use Prophecy\Argument;
use ReflectionProperty;
Expand All @@ -26,6 +25,8 @@
*/
class ApplicationTest extends TestCase
{
use ContainerTrait;

public function setUp()
{
$this->noopMiddleware = function ($req, $res, $next) {
Expand Down Expand Up @@ -377,9 +378,8 @@ public function testInvocationWillPipeRoutingMiddlewareIfNotAlreadyPiped()

$this->router->match($request)->willReturn(RouteResult::fromRouteMatch('foo', 'foo', []));

$container = $this->prophesize(ContainerInterface::class);
$container->has('foo')->willReturn(true);
$container->get('foo')->willReturn($middleware);
$container = $this->mockContainerInterface();
$this->injectServiceInContainer($container, 'foo', $middleware);

$app = new Application($this->router->reveal(), $container->reveal());

Expand Down Expand Up @@ -417,9 +417,8 @@ public function testPipingAllowsPassingMiddlewareServiceNameAsSoleArgument()
return 'invoked';
};

$container = $this->prophesize(ContainerInterface::class);
$container->has('foo', true)->willReturn(true);
$container->get('foo')->willReturn($middleware);
$container = $this->mockContainerInterface();
$this->injectServiceInContainer($container, 'foo', $middleware);

$app = new Application($this->router->reveal(), $container->reveal());
$app->pipe('foo');
Expand All @@ -444,9 +443,8 @@ public function testAllowsPipingErrorMiddlewareUsingServiceNameAsSoleArgument()
return 'invoked';
};

$container = $this->prophesize(ContainerInterface::class);
$container->has('foo', true)->willReturn(true);
$container->get('foo')->willReturn($middleware);
$container = $this->mockContainerInterface();
$this->injectServiceInContainer($container, 'foo', $middleware);

$app = new Application($this->router->reveal(), $container->reveal());
$app->pipeErrorHandler('foo');
Expand All @@ -471,9 +469,8 @@ public function testAllowsPipingMiddlewareAsServiceNameWithPath()
return 'invoked';
};

$container = $this->prophesize(ContainerInterface::class);
$container->has('foo', true)->willReturn(true);
$container->get('foo')->willReturn($middleware);
$container = $this->mockContainerInterface();
$this->injectServiceInContainer($container, 'foo', $middleware);

$app = new Application($this->router->reveal(), $container->reveal());
$app->pipe('/foo', 'foo');
Expand All @@ -498,9 +495,8 @@ public function testAllowsPipingErrorMiddlewareAsServiceNameWithPath()
return 'invoked';
};

$container = $this->prophesize(ContainerInterface::class);
$container->has('foo', true)->willReturn(true);
$container->get('foo')->willReturn($middleware);
$container = $this->mockContainerInterface();
$this->injectServiceInContainer($container, 'foo', $middleware);

$app = new Application($this->router->reveal(), $container->reveal());
$app->pipeErrorHandler('/foo', 'foo');
Expand Down
Loading