Deprecations and backports for 2.2.0 release #140

weierophinney · 2018-01-15T23:28:36Z

This patch provides a number of changes for a new minor version in order to help developers prepare for the upcoming 3.0.0 release. They include:

Marking deprecated classes
- Route
- InvalidMiddlewareException
- InvalidRequestTypeException
- CallableDelegateDecorator
- CallableInteropMiddlewareWrapper
- CallableMiddlewareWrapper
- CallableMiddlewareWrapperFactory
- NoopFinalHandler
Backporting classes/functions:
- CallableMiddlewareDecorator and the associated middleware() utility function
- DoublePassMiddlewareDecorator and the associated doublePassMiddleware() utility function
- PathMiddlewareDecorator and the associated path() utility function
Modifying internals:
- Updating MiddlewarePipe::pipe() to:
  - trigger an E_USER_DEPRECATED error when two arguments are provided
  - decorate middleware in a PathMiddlewareDecorator instance when two arguments are provided
  - decorate PSR-15-compatible callables using CallableMiddlewareDecorator instances
  - decorate double-pass callables using DoublePassMiddlewareDecorator instances
- Updating Route to:
  - decorate middleware in a PathMiddlewareDecorator instance when a non-root path is provided, and the middleware isn't already a decorator
- Updating Next to:
  - remove path segregation, and instead only process middleware

These changes are mostly backwards compatible; the only ones that may affect users are the following:

Callables are decorated using different classes. If an extension relied on the old decorators, they will need to be updated. Since this was considered an internal detail, I'm unsure if this will or should have much impact.
Path segregated middleware is now decorated. Again, this should only affect extensions that were introspecting middleware in either the MiddlewarePipe, Route, or Next workflow. Again, I'm unsure if this will or should have much impact.

The patch removes some tests from Next, as they are now covered by the PathMiddlewareDecorator tests and new integration tests; existing behavior remains unchanged, however.

Finally, all changes are documented, with recommendations on how to prepare your application to remain compatible for version 3.

This patch marks as deprecated classes we plan to remove in version 3.0.0.

This patch backports the following classes from the release-3.0.0 development series: - `Zend\Stratigility\Middleware\CallableMiddlewareDecorator`, for decorating middleware that follows the PSR-15 signature. - `Zend\Stratigility\Middleware\DoublePassMiddlewareDecorator`, for decorating callable middleware that follows the double pass middleware signature. - `Zend\Stratigility\Middleware\PathMiddlewareDecorator`, for providing path-segregated middleware. This required also extracting the class `Zend\Stratigility\Middleware\PathRequestHandlerDecorator`, which is used internally in order to decorate the request handler passed to middleware it decorates. All classes and their were updated to adapt to the http-middleware-compatibility shims.

This patch deprecates using the two-argument form of `MiddlewarePipe::pipe()` in favor of using a `PathMiddlewareDecorator` instance instead. It accomplishes this via the following: - When a non-empty, non-root path argument is provided along with middleware, `MiddlewarePipe::pipe()` will trigger an `E_USER_DEPRECATED` error to notify the user that path segregation should be accomplished using the `PathMiddlewareDecorator`. - When `MiddlewarePipe::pipe()` is called with two arguments, the path argument is not a root path, and the middleware is not a `PathMiddlewareDecorator` instance, it will be decorated as one. - The constructor of `Route` will trigger an `E_USER_DEPRECATED` error if a non root path argument is provided with a non-`PathMiddlewareDecorator` middleware instance. It will then decorate the middleware in a `PathMiddlewareDecorator` instance. - `Next` no longer handles path segregation internally. The changes to `Next` are _almost_ a BC break. However, it includes some code to cast `Route` instances that represent path-segregated middleware to `PathMiddlewareDecorator` instances in the case that they are not already (which can only happen with custom extensions). As such, behavior is retained with this patch. In updating tests, I discovered some edge cases to using the `PathMiddlewareDecorator` approach; in particular, a path-segregated middleware should be able to propagate changes to the request when calling the handler passed to it. As such, the `PathRequestHandlerDecorator` now updates the request URI with the path from the original request URI only.

This patch updates the logic in `MiddlewarePipe` as follows: - Detection of callable middleware, either interop or double-pass, now triggers an `E_USER_DEPRECATED` error indicating that this feature is deprecated, and that users should decorate their middleware before passing it to `pipe()`, recommending the backported decorator classes. - By default, decoration of interop middleware is now done with a `CallableMiddlewareDecorator` instance. - By default, decoration of double-pass middleware is now done with a `DoublePassMiddlewareDecorator` instance, unless a `CallableMiddlewareWrapperFactory` is composed. Tests were refactored to decorate callable middleware when not testing the decoration features. When testing the decoration features, tests were updated to detect emission of the deprecation notices. The `RouteTest` was updated to follow the same patterns as used in the `MiddlewarePipeTest` for detection of the deprecation notice in order to remove the necessity to close over a reference.

In particular, `__invoke()` double-pass middleware implementations.

This patch updates the documentation to use examples that follow current code and standards. This includes: - Using standards-based signatures _everywhere_, except when detailing what the double pass signature is. - Updating the "Creating Middleware" chapter to emphasize standards-based middleware over double-pass middleware, and the proposed v2.2.0 middleware decorators over the legacy middleware "wrappers". (A similar approach was used in the API chapter.) - Using the `PathMiddlewareDecorator` when demonstrating how to segregate middleware by path. - Removing redundant discussion of http-interop, and pushing it into a single location, the Creating Middleware chapter.

- `path()` for creating `PathMiddlewareDecorator` instances. - `middleware()` for creating `CallableMiddlewareDecorator` instances. - `doublePassMiddleware()` for creating `DoublePassMiddlewareDecorator` instances.

michalbundyra · 2018-01-16T10:03:01Z

docs/book/api.md

+- Deprecated since 2.2.0
+
+This is a middleware decorator for PHP callables that have a signature
+compatible with http-interop/http-middleware version 0.4.1. Please see the


and 0.5.0 since 2.1.0

michalbundyra · 2018-01-16T10:05:20Z

docs/book/api.md

+````
+function Zend\Stratigility\path(
+    string $pathPrefix,
+    Interop\Http\Server\MiddlewareInterface $middleware


If so it's compatible only with http-interoop 0.5.0

I'll update this to use the polyfill interfaces; the decorator classes already do.

michalbundyra · 2018-01-16T10:06:59Z

docs/book/api.md

+
+Stratigility provides the following utility functions.
+
+### path


should we quote function name with `?

Not in headers.

michalbundyra · 2018-01-16T10:09:31Z

docs/book/creating-middleware.md

+>
+> The `CallableInteropMiddlewareWrapper` is deprecated starting in version
+> 2.2.0, and will be removed entirely for version 3.0.0. We recommend updating
+> your code to use http-middleware 0.5.0 and the `CallableMiddlewareDecorator`


It's not possible with expressive to update to 0.5.0 because we never released expressive compatible with http-interop 0.5.0.

I think we'll need to provide a new minor release of Expressive that allows this. That said, the decorator will also work with 0.4.1, so I'll also update this description.

you mean expressive v2 with support of http-interop 0.5.0 ? 😱

michalbundyra · 2018-01-16T10:10:34Z

docs/book/creating-middleware.md

+
+Our first double-pass middleware decorator is
+`Zend\Stratigility\Middleware\CallableMiddlewareWrapper`, which implements the
+http-middleware 0.4.1 `MiddlewareInterface`:


It works also with 0.5.0

michalbundyra · 2018-01-16T10:24:23Z

test/Middleware/PathMiddlewareDecoratorTest.php

+     * @dataProvider nestedPathCombinations
+     */
+    public function testNestedMiddlewareOnlyMatchesAtPathBoundaries(
+        string $prefix,


we can't use these typehints as long as we support also PHP 5.6 in version 2.

Yep, already fixed this in another commit that's been pushed, along with a number of anonymous class declarations.

michalbundyra · 2018-01-16T10:29:25Z

src/Exception/PathOutOfSyncException.php

+    /**
+     * @param string $pathPrefix
+     * @param string $path
+     * @return self


What about @internal annotation? Do we have this method in v3?

Yes, the method is there as well.

I can see other use cases for this, particularly if you were to create routing middleware that operates similarly. As such, I'd rather not mark it internal at this time.

michalbundyra · 2018-01-16T10:30:17Z

src/Middleware/DoublePassMiddlewareDecorator.php

+<?php
+/**
+ * @see       https://github.com/zendframework/zend-stratigility for the canonical source repository
+ * @copyright Copyright (c) 2017 Zend Technologies USA Inc. (http://www.zend.com)


2018 and https link

This branch uses the http, not https, scheme in the docheader. I'll update those in a later PR.

I'll update the dates for the new classes momentarily.

michalbundyra · 2018-01-16T10:32:46Z

src/Middleware/PathMiddlewareDecorator.php

+    /**
+     * @param string $segment
+     * @param string $path
+     * @return string


missing @throws annotation (Exception\PathOutOfSyncException)

michalbundyra · 2018-01-16T10:34:48Z

src/Next.php

-        if ('/' === substr($segment, -1)) {
-            // Re-try by submitting with / stripped from end of segment
-            return $this->getTruncatedPath(rtrim($segment, '/'), $path);
+        if (! in_array($path, ['', '/'])


why it's not strict ? (3rd param to true)

michalbundyra · 2018-01-16T16:21:34Z

docs/book/creating-middleware.md

+external router instance to attempt to route a request and delegate to the
+middleware matched.
+
+## MiddlewareInterface


PSR-15 MiddlewareInterface or Stratigility MiddlewareInterface?

Standards one; Stratigility's was deprecated prior to 2.0, and no longer consumed as of that version. The narrative only mentions the proposed standard versions.

Updates backported code and related tests to ensure it tests correctly against PHP versions not supported by the release-3.0.0 branch but currently supported by the develop branch.

This allows it to be used with either the 0.4.1 or 0.5.0 versions of http-interop/http-middleware.

- Removes unnecessary aliases - Use strict flag with `in_array` - Ensure `@throws` annotations are present

- New revisions of malukenho/docheader require symfony/console, and thus we need to ensure that we get a version of that dep that satisfies our current PHP version.

- Do not define a composer script for it, as it is not a defined dependency of the shipped package - Use the new package name (php-coveralls/php-coveralls, vs satooshi/php-coveralls) - Update the invocation of the script within the travis configuration

michalbundyra · 2018-01-16T17:41:05Z

docs/book/api.md

@@ -207,6 +241,19 @@ you will need to:
 - Create and return a concrete response type, OR
 - Operate on a response returned by invoking the delegate.

+### RequestHandler invocation
+
+- Since 2.1.0


Hm, 2.1.0 or 2.2.0 ?

That's been since 2.1.0, when we started supporting http-interop 0.5.0.

michalbundyra · 2018-01-16T17:47:28Z

src/Route.php

@@ -43,6 +45,19 @@ public function __construct($path, ServerMiddlewareInterface $handler)
        }

        $this->path    = $path;
+
+        if (! in_array($this->path, ['', '/'])


strict mode please

Adds unit tests to cover all behaviors of CallableDelegateDecorator.

Each of `__invoke()`, `process()`, and `next()` are deprecated, and removed in v3.0.0.

A major change such as this one should not decrease code quality. As such, adds tests and/or `@codeCoverageIgnore` annotations in order to boost coverage. Also marks an entire class to ignore for the coverage report, as testing that class is unnecessary.

Close #140

weierophinney added 7 commits January 10, 2018 15:30

Marks deprecated classes

a5b6608

This patch marks as deprecated classes we plan to remove in version 3.0.0.

Mark additional deprecated methods

a6f43cc

In particular, `__invoke()` double-pass middleware implementations.

Backports the utility functions from the release-3.0.0 branch

c3d1fc4

- `path()` for creating `PathMiddlewareDecorator` instances. - `middleware()` for creating `CallableMiddlewareDecorator` instances. - `doublePassMiddleware()` for creating `DoublePassMiddlewareDecorator` instances.

michalbundyra reviewed Jan 16, 2018

View reviewed changes

weierophinney added 8 commits January 16, 2018 10:22

Ensure all new code is compatible with PHP versions supported in v2

f0ff0ff

Updates backported code and related tests to ensure it tests correctly against PHP versions not supported by the release-3.0.0 branch but currently supported by the develop branch.

Ensure the path() function accepts the middleware interface polyfill

810448a

This allows it to be used with either the 0.4.1 or 0.5.0 versions of http-interop/http-middleware.

Updates copyright ranges for all class/function files

662c581

Incorporates feedback

3e2342c

- Removes unnecessary aliases - Use strict flag with `in_array` - Ensure `@throws` annotations are present

Add more legacy deps to Travis builds

24220db

- New revisions of malukenho/docheader require symfony/console, and thus we need to ensure that we get a version of that dep that satisfies our current PHP version.

Fixes docheaders on new files to follow template

6cc234b

Require PHP 7.2 to pass

33b8c9d

weierophinney added this to the 2.2.0 milestone Jan 16, 2018

michalbundyra reviewed Jan 16, 2018

View reviewed changes

weierophinney added 4 commits January 16, 2018 13:39

Use strict comparisons for in_array

04659bd

Adds tests for CallableDelegateDecorator

9f14eb5

Adds unit tests to cover all behaviors of CallableDelegateDecorator.

Mark more method deprecations in Next

b3f6ada

Each of `__invoke()`, `process()`, and `next()` are deprecated, and removed in v3.0.0.

Improve test coverage

e3711a6

A major change such as this one should not decrease code quality. As such, adds tests and/or `@codeCoverageIgnore` annotations in order to boost coverage. Also marks an entire class to ignore for the coverage report, as testing that class is unnecessary.

weierophinney merged commit e3711a6 into zendframework:develop Jan 25, 2018

weierophinney added a commit that referenced this pull request Jan 25, 2018

Adds CHANGELOG entries for #140

e2fbcdc

weierophinney added a commit that referenced this pull request Jan 25, 2018

Merge branch 'feature/deprecations' into develop

b682c19

Close #140

weierophinney deleted the feature/deprecations branch January 25, 2018 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecations and backports for 2.2.0 release #140

Deprecations and backports for 2.2.0 release #140

weierophinney commented Jan 15, 2018

michalbundyra Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

michalbundyra Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018

weierophinney Jan 16, 2018

michalbundyra Jan 16, 2018


		Stratigility provides the following utility functions.

		### path

Deprecations and backports for 2.2.0 release #140

Deprecations and backports for 2.2.0 release #140

Conversation

weierophinney commented Jan 15, 2018

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

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

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