Create configured FinalHandler if one is not available in the container #309
Conversation
|
Closing & re-opening to trigger a new Travis build, since my pending |
|
@weierophinney now that this build will pass once Travis finishes running again, what do you think? Basically, instead of defaulting an unset Alternatively, I could create a |
| $config = $container->has('config') ? $container->get('config') : []; | ||
| $options = isset($config['final_handler']['options']) ? $config['final_handler']['options'] : []; | ||
| $finalHandler = new FinalHandler($options, null); | ||
| } |
There was a problem hiding this comment.
I'd extract the above logic to a private method, so that the logic here can be simply something like $finalHandler = $this->marshalFinalHandler($container);.
I like this approach, but it would require existing users to update their configuration to add a dependency map for it. As such, let's stick to an internal, private method for this as suggested above. |
|
One more thing: update the |
|
@weierophinney done and done |
| @@ -22,7 +22,7 @@ | |||
| "zendframework/zend-diactoros": "^1.1", | |||
| "zendframework/zend-expressive-router": "^1.1", | |||
| "zendframework/zend-expressive-template": "^1.0.1", | |||
| "zendframework/zend-stratigility": "^1.1" | |||
| "zendframework/zend-stratigility": "^1.2.0" | |||
There was a problem hiding this comment.
They're semantically equivalent. ^ will allow from that version to next
major. (vs ~, which limits to the same minor version.)
On Mar 21, 2016 6:23 PM, "Michaël Gallego" [email protected] wrote:
In composer.json
#309 (comment)
:@@ -22,7 +22,7 @@
"zendframework/zend-diactoros": "^1.1",
"zendframework/zend-expressive-router": "^1.1",
"zendframework/zend-expressive-template": "^1.0.1",
"zendframework/zend-stratigility": "^1.1""zendframework/zend-stratigility": "^1.2.0"^1.2 instead?
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/zendframework/zend-expressive/pull/309/files/889ed8b8ddeb9c92de319184aa9377d22d9112f5#r56914722
…-handler Create configured FinalHandler if one is not available in the container
|
Thanks for the patch, @michaelmoussa! Merged to develop for release with 1.1.0. |
Backport zendframework#309 for release with 1.1.0. Conflicts: composer.json src/Container/ApplicationFactory.php
Backport zendframework#309 for release with 1.1.0. Conflicts: composer.json src/Container/ApplicationFactory.php
@weierophinney this piggybacks on what we discussed earlier with regard to the issue that inspired this PR on the Stratigility repo.
The first part of the other PR fixes the stack traces in production issue, and the second adds the
setOriginalResponse($response);to theFinalHandlerin order to allow someone to create a properly configuredFinalHandlerahead of time. TheApplicationwould then inject the original response when it calls->getFinalHandler().The purpose of this new PR is to provide support for configuring the
FinalHandlerthat Expressive automatically creates without needing to create a new factory class. Developers would just need to add the desired final handler options to their config.There's a caveat, though - it depends on the Stratigility PR.
This version of this PR should not be merged until the Stratigility PR is merged, a new version is tagged, and Expressive's
composer.jsonentry forzendframework/zend-stratigilityreflects the dependency on that version. This implementation needsFinalHandlerto supportsetOriginalResponse, otherwise,200 OKresponses will return 404s instead (since no original response is being injected into theFinalHandler).I don't think the full solution would be considered a BC break, though, since people who were providing their own
FinalHandlerwill not be affected, and people who were relying on the default will not notice a thing.Let me know if you think a proper
Zend\Expressive\FinalHandlerFactorythat folks need to explicitly add to their dependencies config would be a better solution, otherwise, I'll take care of thecomposer.jsonupdate once the other PR is merged and tagged and update this PR.