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

Added support for http-interop/http-middleware 0.5.0 (No BC Break) #46

Merged
merged 1 commit into from
Oct 9, 2017
Merged

Added support for http-interop/http-middleware 0.5.0 (No BC Break) #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 8 additions & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,18 @@
"config": {
"sort-packages": true
},
"repositories": [
{
"type": "git",
"url": "https://github.com/webimpress/zend-expressive-router.git"
Copy link

Choose a reason for hiding this comment

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

Why is this package not published on packagist.org?

Copy link
Member Author

Choose a reason for hiding this comment

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

@zf2timo This PR requires another one to be merged before - zendframework/zend-expressive-router#36
After merge PR in expressive-router, this PR will be updated.

}
],
"require": {
"http-interop/http-middleware": "^0.4.1",
"php": "^5.6 || ^7.0",
"psr/container": "^1.0",
"psr/http-message": "^1.0.1",
"zendframework/zend-expressive-router": "^2.1"
"webimpress/http-middleware-compatibility": "^0.1.1",
"zendframework/zend-expressive-router": "dev-feature/http-middleware-0.5"
},
"require-dev": {
"malukenho/docheader": "^0.1.5",
Expand Down
100 changes: 87 additions & 13 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions src/BodyParams/BodyParamsMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

namespace Zend\Expressive\Helper\BodyParams;

use Interop\Http\ServerMiddleware\DelegateInterface;
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Webimpress\HttpMiddlewareCompatibility\HandlerInterface as DelegateInterface;
use Webimpress\HttpMiddlewareCompatibility\MiddlewareInterface;

use const Webimpress\HttpMiddlewareCompatibility\HANDLER_METHOD;

class BodyParamsMiddleware implements MiddlewareInterface
{
Expand Down Expand Up @@ -77,7 +79,7 @@ public function clearStrategies()
public function process(ServerRequestInterface $request, DelegateInterface $delegate)
{
if (in_array($request->getMethod(), $this->nonBodyRequests)) {
return $delegate->process($request);
return $delegate->{HANDLER_METHOD}($request);
Copy link

Choose a reason for hiding this comment

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

Why is this changed? It increases the complexity and I think it has an negative impact on the performance.

Copy link
Member Author

Choose a reason for hiding this comment

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

The purpose of this PR is to provide Non-BC-Break change to support http-interop/http-middleware in version 0.4.1 and 0.5.0. We accomplish it by using separate repository - webimpress/http-middleware-compatibility. Now consumer of the library can decide what version of http-interop middleware to use. Please note that handler/delegator interface has been changed in 0.5.0 release and this is the simplest way (I can see right now) to provide that functionality.

Please see webimpress/http-middleware-compatibility library for more information.

Copy link

Choose a reason for hiding this comment

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

Okay, I can see your intend now. But does it really need a solution like this? The http-interop is not a stable release yet. So it could be possible that not only the name of the method changes, but also the signature changes. These changes can not be covered by your solution.
The signature changes can only be handled by a wrapper module - which is my preferred solution, because you do not have the "magic" with {HANDLER_METHOD}($request) .

Copy link
Member Author

Choose a reason for hiding this comment

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

@zf2timo Please note that signature has been changed (of middleware) and interfaces has different namespace. I covered it all in my library. This "magic" trick with method name is the simples solution what I can think of right now, and probably it will work until we have a final PSR-15.
Of course wrapper will be better, but unfortunately wrapper is going to provide BC break. We would rather avoid this right now, but we don't want stop users of using the latest version of http-interop middleware. With that solution you can use any version. This is temporary solution, the final one will be when PSR-15 is released.

I wouldn't be worried about performance. We define the constant only on autoloading. Other than that we just using aliases also defined on autoloading. In your library which consumes that library you can use http-interop directly. Whatever is easier for you. Finally, when PSR-15 will be released you will need to update your application anyway. And using this solution, it's quite possible that you will be able to do before all zend libraries implement that changes 😄

Copy link
Member

Choose a reason for hiding this comment

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

I don't see a likely performance change due to this, TBH. As @webimpress notes, the constant and aliases are defined when the autoloaders are first created, which means in production it will happen the first time the application is hit only, and not on subsequent requests.

Second, regarding this:

it could be possible that not only the name of the method changes, but also the signature changes

While this change from 0.4.1 to 0.5.0 is pretty major, I think this will likely be the last major change we have in the specification. 0.4.1 proved itself quite well; 0.5.0 is mainly around some re-naming to allow usage of the "request handler" (formerly the "delegate") outside the scope of the middleware interface; in terms of how the interfaces relate and work, they are exactly the same.

The main concern I have is that we have to make two changes: one now, to allow for both backwards and forwards compatibility, and another for the next major release, which removes this workaround in favor of the final PSR-15 interfaces. Having to make changes twice, across multiple libraries, is painful and often fragile (we forget the order in which the changes need to be made, or forget which libraries need changes, etc.).

The chief benefit I see to making the change now is that it provides an implementation of the current version of the PSR-15 specification, which will allow it to move forward towards an acceptance vote. As such, I'm weighing in as being in favor of this change at this time.

I've pointed out the library @webimpress has made to other members of the PSR-15 working group, and want to discuss with them briefly before I merge these, to make sure nothing is being missed. If they think it looks reasonable, I'll proceed in merging this and the other related changes.

}

$header = $request->getHeaderLine('Content-Type');
Expand All @@ -87,10 +89,10 @@ public function process(ServerRequestInterface $request, DelegateInterface $dele
}

// Matched! Parse and pass on to the next
return $delegate->process($strategy->parse($request));
return $delegate->{HANDLER_METHOD}($strategy->parse($request));
}

// No match; continue
return $delegate->process($request);
return $delegate->{HANDLER_METHOD}($request);
}
}
8 changes: 5 additions & 3 deletions src/ContentLengthMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

namespace Zend\Expressive\Helper;

use Interop\Http\ServerMiddleware\DelegateInterface;
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Psr\Http\Message\ServerRequestInterface;
use Webimpress\HttpMiddlewareCompatibility\HandlerInterface as DelegateInterface;
use Webimpress\HttpMiddlewareCompatibility\MiddlewareInterface;

use const Webimpress\HttpMiddlewareCompatibility\HANDLER_METHOD;

/**
* Middleware to inject a Content-Length response header.
Expand All @@ -25,7 +27,7 @@ class ContentLengthMiddleware implements MiddlewareInterface
*/
public function process(ServerRequestInterface $request, DelegateInterface $delegate)
{
$response = $delegate->process($request);
$response = $delegate->{HANDLER_METHOD}($request);
if ($response->hasHeader('Content-Length')) {
return $response;
}
Expand Down
8 changes: 5 additions & 3 deletions src/ServerUrlMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

namespace Zend\Expressive\Helper;

use Interop\Http\ServerMiddleware\DelegateInterface;
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Webimpress\HttpMiddlewareCompatibility\HandlerInterface as DelegateInterface;
use Webimpress\HttpMiddlewareCompatibility\MiddlewareInterface;

use const Webimpress\HttpMiddlewareCompatibility\HANDLER_METHOD;

class ServerUrlMiddleware implements MiddlewareInterface
{
Expand Down Expand Up @@ -42,6 +44,6 @@ public function process(ServerRequestInterface $request, DelegateInterface $dele
{
$this->helper->setUri($request->getUri());

return $delegate->process($request);
return $delegate->{HANDLER_METHOD}($request);
}
}
8 changes: 5 additions & 3 deletions src/UrlHelperMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@

namespace Zend\Expressive\Helper;

use Interop\Http\ServerMiddleware\DelegateInterface;
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Zend\Expressive\Router\RouteResult;
use Webimpress\HttpMiddlewareCompatibility\HandlerInterface as DelegateInterface;
use Webimpress\HttpMiddlewareCompatibility\MiddlewareInterface;

use const Webimpress\HttpMiddlewareCompatibility\HANDLER_METHOD;

/**
* Pipeline middleware for injecting a UrlHelper with a RouteResult.
Expand Down Expand Up @@ -49,6 +51,6 @@ public function process(ServerRequestInterface $request, DelegateInterface $dele
$this->helper->setRouteResult($result);
}

return $delegate->process($request);
return $delegate->{HANDLER_METHOD}($request);
}
}
8 changes: 5 additions & 3 deletions test/BodyParams/BodyParamsMiddlewareTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@

namespace ZendTest\Expressive\Helper\BodyParams;

use Interop\Http\ServerMiddleware\DelegateInterface;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Psr\Http\Message\ServerRequestInterface;
use Webimpress\HttpMiddlewareCompatibility\HandlerInterface as DelegateInterface;
use Zend\Diactoros\Response;
use Zend\Diactoros\ServerRequest;
use Zend\Diactoros\Stream;
use Zend\Expressive\Helper\BodyParams\BodyParamsMiddleware;
use Zend\Expressive\Helper\BodyParams\StrategyInterface;
use Zend\Expressive\Helper\Exception\MalformedRequestBodyException;

use const Webimpress\HttpMiddlewareCompatibility\HANDLER_METHOD;

class BodyParamsMiddlewareTest extends TestCase
{
/**
Expand Down Expand Up @@ -46,7 +48,7 @@ private function mockDelegate(callable $callback)
$delegate = $this->prophesize(DelegateInterface::class);

$delegate
->process(Argument::type(ServerRequestInterface::class))
->{HANDLER_METHOD}(Argument::type(ServerRequestInterface::class))
->will(function ($args) use ($callback) {
$request = $args[0];
return $callback($request);
Expand All @@ -60,7 +62,7 @@ private function mockDelegateToNeverTrigger()
$delegate = $this->prophesize(DelegateInterface::class);

$delegate
->process(Argument::type(ServerRequestInterface::class))
->{HANDLER_METHOD}(Argument::type(ServerRequestInterface::class))
->shouldNotBeCalled();

return $delegate;
Expand Down
6 changes: 4 additions & 2 deletions test/ContentLengthMiddlewareTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@

namespace ZendTest\Expressive\Helper;

use Interop\Http\ServerMiddleware\DelegateInterface;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\StreamInterface;
use Webimpress\HttpMiddlewareCompatibility\HandlerInterface as DelegateInterface;
use Zend\Expressive\Helper\ContentLengthMiddleware;

use const Webimpress\HttpMiddlewareCompatibility\HANDLER_METHOD;

class ContentLengthMiddlewareTest extends TestCase
{
public function setUp()
Expand All @@ -23,7 +25,7 @@ public function setUp()
$this->stream = $this->prophesize(StreamInterface::class);

$delegate = $this->prophesize(DelegateInterface::class);
$delegate->process($request)->will([$response, 'reveal']);
$delegate->{HANDLER_METHOD}($request)->will([$response, 'reveal']);
$this->delegate = $delegate->reveal();

$this->middleware = new ContentLengthMiddleware();
Expand Down
6 changes: 4 additions & 2 deletions test/ServerUrlMiddlewareTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@

namespace ZendTest\Expressive\Helper;

use Interop\Http\ServerMiddleware\DelegateInterface;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\UriInterface;
use Webimpress\HttpMiddlewareCompatibility\HandlerInterface as DelegateInterface;
use Zend\Diactoros\Response;
use Zend\Expressive\Helper\ServerUrlHelper;
use Zend\Expressive\Helper\ServerUrlMiddleware;

use const Webimpress\HttpMiddlewareCompatibility\HANDLER_METHOD;

class ServerUrlMiddlewareTest extends TestCase
{
public function testMiddlewareInjectsHelperWithUri()
Expand All @@ -31,7 +33,7 @@ public function testMiddlewareInjectsHelperWithUri()
$invoked = false;

$delegate = $this->prophesize(DelegateInterface::class);
$delegate->process(Argument::type(RequestInterface::class))->will(function ($req) use (&$invoked) {
$delegate->{HANDLER_METHOD}(Argument::type(RequestInterface::class))->will(function ($req) use (&$invoked) {
$invoked = true;

return new Response();
Expand Down
Loading