From 0d8fe1d071365b2c99eca8dcfbcfd8126d4e3cf4 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 25 Jan 2017 13:00:22 -0600 Subject: [PATCH 1/2] Pipe should not decorate MiddlewarePipe instances In prepping Expressive to use Stratigility 2, I discovered that `MiddlewarePipe` instances piped into another `MiddlewarePipe` instance were being decorated as callable middleware; this should no longer happen. This patch adds a test to ensure it does not happen, and updates `pipe()` to not decorate callable `ServerMiddlewareInterface` instances. In doing so, the `isInteropMiddleware()` method is no longer relevant, and I removed it; since it was marked `private`, this is a backwards compatible change. --- src/MiddlewarePipe.php | 17 ++--------------- test/MiddlewarePipeTest.php | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/MiddlewarePipe.php b/src/MiddlewarePipe.php index f1d95e9..d5a5979 100644 --- a/src/MiddlewarePipe.php +++ b/src/MiddlewarePipe.php @@ -128,10 +128,9 @@ public function pipe($path, $middleware = null) $path = '/'; } - // Decorate callable middleware as http-interop middleware if we have - // a response prototype present. + // Decorate callable middleware as http-interop middleware if (is_callable($middleware) - && ! $this->isInteropMiddleware($middleware) + && ! $middleware instanceof ServerMiddlewareInterface ) { $middleware = $this->decorateCallableMiddleware($middleware); } @@ -266,18 +265,6 @@ private function getCallableMiddlewareDecorator() return $this->callableMiddlewareDecorator; } - /** - * Is the provided middleware argument http-interop middleware? - * - * @param mixed $middleware - * @return bool - */ - private function isInteropMiddleware($middleware) - { - return ! is_callable($middleware) - && $middleware instanceof ServerMiddlewareInterface; - } - /** * @param callable $middleware * @return \ReflectionFunctionAbstract diff --git a/test/MiddlewarePipeTest.php b/test/MiddlewarePipeTest.php index ab29c90..6e1dfe7 100644 --- a/test/MiddlewarePipeTest.php +++ b/test/MiddlewarePipeTest.php @@ -518,4 +518,19 @@ public function testWillDecorateCallableArrayMiddlewareWithoutErrors() $this->assertInstanceOf(CallableMiddlewareWrapper::class, $test); $this->assertAttributeSame($middleware, 'middleware', $test); } + + public function testPipeShouldNotWrapMiddlewarePipeInstancesAsCallableMiddleware() + { + $nested = new MiddlewarePipe(); + $pipeline = new MiddlewarePipe(); + + $pipeline->pipe($nested); + + $r = new ReflectionProperty($pipeline, 'pipeline'); + $r->setAccessible(true); + $queue = $r->getValue($pipeline); + + $route = $queue->dequeue(); + $this->assertSame($nested, $route->handler); + } } From cb75d91401b59978d3eee45137efe6ed1b673d41 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 25 Jan 2017 13:14:15 -0600 Subject: [PATCH 2/2] Added CHANGELOG for #98 --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19963b6..ea12cce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Versions prior to 1.0 were originally released as `phly/conduit`; please visit its [CHANGELOG](https://github.com/phly/conduit/blob/master/CHANGELOG.md) for details. -## 2.0.1 - TBD +## 2.0.1 - 2017-01-25 ### Added @@ -22,7 +22,10 @@ details. ### Fixed -- Nothing. +- [#98](https://github.com/zendframework/zend-stratigility/pull/98) fixes how + `Middleware::pipe()` handles `MiddlewarePipe` instances passed to it; + previously it was incorrectly wrapping them in `CallableMiddlewareWrapper` + instances; it now pipes them as-is. ## 2.0.0 - 2017-01-24