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

Fix OPTIONS request for not found routes #73

Merged
merged 1 commit into from
Mar 21, 2018
Merged

Fix OPTIONS request for not found routes #73

merged 1 commit into from
Mar 21, 2018

Conversation

michalbundyra
Copy link
Member

Pass to next handler when route is not matched.

Fixes #72

  • Are you fixing a bug?
    • Detail how the bug is invoked currently.
    • Detail the original, incorrect behavior.
    • Detail the new, expected behavior.
    • Base your feature on the master branch, and submit against that branch.
    • Add a regression test that demonstrates the bug, and proves the fix.
    • Add a CHANGELOG.md entry for the fix.

Pass to next handler when route is not matched.
@@ -73,12 +73,11 @@ public function testReturnsResultOfHandlerWhenRouteSupportsOptionsExplicitly()
{
$route = $this->prophesize(Route::class);

$result = $this->prophesize(RouteResult::class);
$result->getMatchedRoute()->will([$route, 'reveal']);
$result = RouteResult::fromRoute($route->reveal());
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed here to not mocking the RouteResult because if we use more methods we need always extend our mock.
And what's more we can easily mock RouteResult wrong way. We can create new instance only by using fromRoute or fromRouteFailure but we can create invalid mock as we have to mock 4-5 public methods...

$result = $this->prophesize(RouteResult::class);
$result->getAllowedMethods()->willReturn($allowedMethods);
$result->getMatchedRoute()->willReturn(false);
$result = RouteResult::fromRouteFailure($allowedMethods);
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 same comment here as above about mocking RouteResult.


public function testReturnsResultOfHandlerWhenRouteNotFound()
{
$result = RouteResult::fromRouteFailure(Route::HTTP_METHOD_ANY);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main test, so in case route NOT FOUND we have allowed any methods, but the result is failure.
So isFailure === true and isMethodFailure === false. Then -> 404.

@geerteltink geerteltink added this to the 3.0.2 milestone Mar 21, 2018
@weierophinney weierophinney merged commit adb00ca into zendframework:master Mar 21, 2018
weierophinney added a commit that referenced this pull request Mar 21, 2018
weierophinney added a commit that referenced this pull request Mar 21, 2018
weierophinney added a commit that referenced this pull request Mar 21, 2018
Forward port #73

Conflicts:
	CHANGELOG.md
@weierophinney
Copy link
Member

Thanks, @webimpress!

@michalbundyra michalbundyra deleted the hotfix/72 branch March 21, 2018 16:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OPTIONS not found path was throw an error
3 participants