Update to http-middleware 0.4.1 #96

weierophinney · 2017-01-20T21:35:45Z

http-interop/http-middleware 0.4.1 is the latest revision, and likely the version that will be used during the FIG review period. It makes the following changes:

The namespace changes from Interop\Http\Middleware to Interop\Http\ServerMiddleware, to indicate that the specification is only dealing with server-side middleware.
The interface ServerMiddlewareInterface is renamed to MiddlewareInterface, as the namespace change makes it clear what type of middleware is being defined.
The signature of DelegateInterface::process now accepts specifically a ServerRequestInterface instead of a more general RequestInterface.

Updating to these changes means the following breaking changes:

We no longer support the interfaces as defined in 0.2.0.
Next::process() now typehints against ServerRequestInterface.

The above result in a few breaking changes in the develop branch:

Zend\Stratigility\Next and Zend\Stratigility\Delegate\CallableDelegateDecorator::process have changed signatures, due to the changes to DelegateInterface::process; if developers were extending these previously, their code now breaks.
Stratigility no longer accepts middleware implementing the http-middleware 0.2.0 signatures, as those were under different namespaces and/or names.

I'm currently targeting this at a 1.4.0 release; however, due to the breaking changes, I could target it at a 2.0.0 release. If consensus is to take this latter route, I'll update this repository to merge in the changes currently in the develop branch as well.

This targets develop, which is the 2.0.0 feature branch at this time.

- Changed import statements to reflect new namespace, class names. - Removed functionality in `Next` around validating a server request is received; no longer required with changes to http-middleware. - Removed validation of client-side middleware; no longer relevant with changes to http-middleware.

weierophinney · 2017-01-20T21:36:58Z

Pinging @michaelmoussa and @xtreamwayz for review as well.

geerteltink

I'm wondering why you choose to alias Interop\Http\ServerMiddleware\MiddlewareInterface as InteropMiddlewareInterface in src. In the tests ServerMiddlewareInterface is being used.

Why not use ServerMiddlewareInterface for both and keep it consistent? If you choose InteropMiddlewareInterface you might want to change it again once the PSR is accepted. ServerMiddlewareInterface could always be used and tells exactly what it is.

weierophinney · 2017-01-21T20:06:49Z

I'm wondering why you choose to alias Interop\Http\ServerMiddleware\MiddlewareInterface as InteropMiddlewareInterface in src. In the tests ServerMiddlewareInterface is being used.

That's a good catch.

The main reason I chose the verbiage "Interop" was to point out that it's from the http-interop project. That said, once this is ratified by php-fig, we'll be implementing both http-interop (which will be deprecated at that point) and the FIG interfaces — which do not include the "interop" verbiage. As such, I agree with you that the naming needs to change, and will push an update momentarily.

michaelmoussa · 2017-01-21T20:30:21Z

Pretty straightforward, looks 👍 to me BUT, depending on how optimistic you are about finalization of the FIG interfaces, I'd be inclined to target 2.0.0 for this. Breaking changes are breaking changes.

If timing works out we might even manage to lump in the updating of dependencies for the other Expressive components that had major version bumps recently too.

danizord

👍

weierophinney · 2017-01-23T14:21:55Z

I'd be inclined to target 2.0.0 for this. Breaking changes are breaking changes.

I hate having version inflation, but you're absolutely correct. This means, of course, that if there are any breaking changes to http-middleware between now and acceptance, we'll need a v3, but that also provides end-users with a better path forward.

I'm thinking with that, we'll also likely want to make Expressive's next version 2.0. We can provide migration tooling to help end-users adapt their existing, callable middleware into http-middleware-compatible middleware, and likely do that at the application level. (We already have some tools in place for this anyways.) Users would then be able to opt-in to the upgrade.

I'll get started today and merging in the other breaking changes from the dev-2.0.0 branch into this PR and/or re-target this PR against that branch.

…ddleware-0.4.1 Conflicts: CHANGELOG.md composer.lock src/Dispatch.php src/Middleware/ErrorHandler.php src/Middleware/NotFoundHandler.php src/MiddlewarePipe.php src/Next.php src/Route.php test/DispatchTest.php test/Middleware/CallableInteropMiddlewareWrapperTest.php test/Middleware/CallableMiddlewareWrapperTest.php test/Middleware/ErrorHandlerTest.php test/Middleware/NotFoundHandlerTest.php test/MiddlewarePipeTest.php test/NextTest.php

Use `ServerMiddlewareInterface` throughout project.

weierophinney · 2017-01-23T18:06:00Z

src/Route.php

     * @throws Exception\InvalidMiddlewareException if the $handler provided is
-     *     not an http-interop middleare type.
+     *     not an http-interop middleware type.
     */
    public function __construct($path, $handler)


I'm thinking I can use a typehint here instead of doing the checking within the body; however, doing so affects a ton of tests. I'll see how much work it is.

weierophinney · 2017-01-23T18:06:56Z

test/MiddlewarePipeTest.php

 * @license   https://framework.zend.com/license New BSD License
 */

 namespace ZendTest\Stratigility;

-use Interop\Http\Middleware\DelegateInterface;
-use Interop\Http\Middleware\ServerMiddlewareInterface;
-use PHPUnit\Framework\TestCase;


I wasn't aware namespacing was a thing in PHPUnit... I'll get all of these reverted.

I thought namespacing was added in version 6. But it seems there is a forward compatibility layer since version 5.4.

- 5.* series has namespaced variants.

Adds the typehint `ServerMiddlewareInterface` to the `$handler` argument of `Route`'s constructor, and removes the internal type verification as it's now redundant.

geerteltink · 2017-01-23T21:23:44Z

test/NextTest.php

-use Interop\Http\Middleware\DelegateInterface;
-use Interop\Http\Middleware\ServerMiddlewareInterface;
+use Interop\Http\ServerMiddleware\DelegateInterface;
+use Interop\Http\ServerMiddleware\MiddlewareInterface as ServerMiddlewareInterface;
 use PHPUnit_Framework_Assert as Assert;


There's also a PHPUnit\Framework\Assert namespace.

Edit: Since version 5.7.

Awesome; thanks. Updated!

- requires minimum 5.7 version of PHPUnit.

- Reference http-interop/http-middleware or http-middleware.

weierophinney · 2017-01-23T21:48:56Z

doc/book/api.md

-can be a `Next` instance, as it also implements that interface.
+Starting in version 1.3.0, `MiddlewarePipe` implements the
+http-interop/http-middleware server-side middleware interface, and thus provides
+a `process()` method. This method requires a `ServerRequestInterface` instance


I'm not specifying the interface name here, as it varies based on the http-middleware version; however, the semantics are the same.

weierophinney · 2017-01-23T21:49:50Z

doc/book/api.md

+Starting in version 1.3.0, `MiddlewarePipe` implements the
+http-interop/http-middleware server-side middleware interface, and thus provides
+a `process()` method. This method requires a `ServerRequestInterface` instance
+and an http-interop/http-middleware `DelegateInterface` instance on invocation;


I'm not specifying the full namespace here, as the namespace varies based on the http-middleware version. The interface name remains the same between the two versions, however, allowing calling it out by its name minus the namespace.

weierophinney · 2017-01-23T21:50:35Z

doc/book/api.md

@@ -175,8 +175,6 @@ you will need to:
 ### Providing an altered request:

 ```php
-use Interop\Http\Middleware\DelegateInterface;


I've removed the import statements throughout, as they'll vary based on which version of Stratigility, and thus http-middleware, you use. The interface name remains the same between versions.

weierophinney · 2017-01-23T21:51:36Z

doc/book/api.md

-middleware. Internally, if a response prototype is composed in the
-`MiddlewarePipe`, callable middleware piped to the `MiddlewarePipe` will be
+Starting in version 1.3.0, we offer the ability to work with
+http-interop/http-middleware. Internally, if a response prototype is composed in


I started using the verbiage "http-interop/http-middleware" throughout. I'm not sure if I got all of them, but I'm trying to be consistent, as the http-interop project also covers PSR-17 (factories), and may cover client-side middleware in the future.

weierophinney · 2017-01-23T21:52:17Z

doc/book/api.md

+
+// http-interop/http-middleware 0.4.1:
+use Interop\Http\ServerMiddleware\DelegateInterface;
+use Interop\Http\ServerMiddleware\MiddlewareInterface as ServerMiddlewareInterface;


This is the one place I provided commentary on the different versions, because it's a full example.

bakura10 · 2017-01-23T22:19:35Z

doc/book/migration/to-v2.md

+
+- The namespace changes from `Interop\Http\Middleware` to
+  `Interop\Http\ServerMiddleware`, signaling a change indicating that the project
+  now only targests server-side middleware.


@bakura10

- Per @bakura10

Also edited existing changelog entries to prevent confusion about what was finally done. Several were referencing changes that targeted http-middleware 0.2.0, and/or previous iterations in the develop branch that no longer have relevance or have different relevance at this time.

Close #96

weierophinney added 3 commits January 20, 2017 14:45

Added CHANGELOG stub for 1.4.0

5b915de

Update to http-middleware 0.4.1

77fca73

weierophinney added enhancement WIP labels Jan 20, 2017

weierophinney requested a review from Ocramius January 20, 2017 21:36

weierophinney added this to the 1.4.0 milestone Jan 20, 2017

geerteltink reviewed Jan 21, 2017

View reviewed changes

danizord approved these changes Jan 21, 2017

View reviewed changes

danizord mentioned this pull request Jan 21, 2017

Update for PSR-15 zf-fr/zfr-eb-worker#33

Closed

weierophinney force-pushed the feature/http-middleware-0.4.1 branch from ab7e173 to c718caa Compare January 23, 2017 17:51

weierophinney changed the base branch from dev-1.4.0 to develop January 23, 2017 17:51

weierophinney modified the milestones: 2.0.0, 1.4.0 Jan 23, 2017

weierophinney removed the WIP label Jan 23, 2017

weierophinney added 2 commits January 23, 2017 11:57

Updated license copyright for all changed files.

977b4e4

Ensure consistency in naming of interop middleware

707edc2

Use `ServerMiddlewareInterface` throughout project.

weierophinney commented Jan 23, 2017

View reviewed changes

weierophinney added 2 commits January 23, 2017 12:13

Use namespaced PHPUnit classes

3b8a0f5

- 5.* series has namespaced variants.

Add typehint for Route $handler argument

6dfd5e3

Adds the typehint `ServerMiddlewareInterface` to the `$handler` argument of `Route`'s constructor, and removes the internal type verification as it's now redundant.

weierophinney added the Needs Documentation label Jan 23, 2017

Updated documentation to discuss http-middleware 0.4.1 changes

e67b3f1

weierophinney removed the Needs Documentation label Jan 23, 2017

geerteltink reviewed Jan 23, 2017

View reviewed changes

weierophinney added 3 commits January 23, 2017 15:42

Use namespaced PHPUnit Assert class

8f39515

- requires minimum 5.7 version of PHPUnit.

Fixed grammar in new text.

6751fa3

More consistency fixes in documentation

4a02993

- Reference http-interop/http-middleware or http-middleware.

weierophinney commented Jan 23, 2017

View reviewed changes

bakura10 reviewed Jan 23, 2017

View reviewed changes

Fixed typo in documentation

c7f30bb

- Per @bakura10

weierophinney merged commit c7f30bb into zendframework:develop Jan 23, 2017

weierophinney added a commit that referenced this pull request Jan 23, 2017

Merge branch 'feature/http-middleware-0.4.1' into develop

43daae1

Close #96

weierophinney deleted the feature/http-middleware-0.4.1 branch January 23, 2017 22:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to http-middleware 0.4.1 #96

Update to http-middleware 0.4.1 #96

weierophinney commented Jan 20, 2017 •

edited

Loading

weierophinney commented Jan 20, 2017

geerteltink left a comment

weierophinney commented Jan 21, 2017

michaelmoussa commented Jan 21, 2017

danizord left a comment

weierophinney commented Jan 23, 2017

weierophinney Jan 23, 2017

weierophinney Jan 23, 2017

geerteltink Jan 23, 2017

geerteltink Jan 23, 2017 •

edited

Loading

weierophinney Jan 23, 2017

weierophinney Jan 23, 2017

weierophinney Jan 23, 2017

weierophinney Jan 23, 2017

weierophinney Jan 23, 2017

weierophinney Jan 23, 2017

bakura10 Jan 23, 2017

Update to http-middleware 0.4.1 #96

Update to http-middleware 0.4.1 #96

Conversation

weierophinney commented Jan 20, 2017 • edited Loading

weierophinney commented Jan 20, 2017

geerteltink left a comment

Choose a reason for hiding this comment

weierophinney commented Jan 21, 2017

michaelmoussa commented Jan 21, 2017

danizord left a comment

Choose a reason for hiding this comment

weierophinney commented Jan 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geerteltink Jan 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weierophinney commented Jan 20, 2017 •

edited

Loading

geerteltink Jan 23, 2017 •

edited

Loading