zendframework / zend-expressive-router

Router subcomponent for Expressive
BSD 3-Clause "New" or "Revised" License
28 stars 13 forks source link

Imports ImplicitHead, ImplicitOptions Middleware from zend-expressive #55

Closed weierophinney closed 6 years ago

weierophinney commented 6 years ago

Per the Expressive 2.2. roadmap, this patch imports the ImplicitHeadMiddleware and ImplicitOptionsMiddleware from zend-expressive v2, and places them in the Zend\Expressive\Router\Middleware namespace. They have been updated to allow compatibility with any version of http-interop middleware, via the webimpress/http-middleware-compatibility package (which was already a requirement).

This patch updates ImplicitHeadMiddleware to optionally allow passing a second argument representing a PHP callable capable of producing a PSR-7 StreamInterface instance.

Finally, it deprecates the implicitHead() and implicitOptions() methods of the Route class, as they are removed in version 3.

weierophinney commented 6 years ago

This patch is now ready for review.

weierophinney commented 6 years ago

I didn't check tests, but coveralls report shows very poor ImplicitHeadMiddleware coverage

There's something wrong with that report, I think. The testInvokesHandlerWhenRouteImplicitlySupportsHeadAndSupportsGet() method definitely exercises the lines marked as not covered via its own lines 141-151.

michalbundyra commented 6 years ago

@weierophinney

There's something wrong with that report, I think.

Checked with breakpoint, and no. The report is fine. We never stop there. What's more please note that variable defined in line 99: https://github.com/zendframework/zend-expressive-router/pull/55/files#diff-0c5f0c3fd11973365a718e7fe2685c81R99

is not used ! So definitely there is something wrong.

michalbundyra commented 6 years ago

@weierophinney

The testInvokesHandlerWhenRouteImplicitlySupportsHeadAndSupportsGet() method definitely exercises ...

again - checked with debugger and it quits in line 92 of the middleware:

        $route = $result->getMatchedRoute();
        if (! $route || ! $route->implicitHead()) { // <!-- this condition is satisfied
            return $handler->{HANDLER_METHOD}($request);
        }

screen shot 2018-03-08 at 14 19 50

weierophinney commented 6 years ago

Refactored the test, and fixed a bug in the middleware; let's see what coverage says now! :)