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

Zend\Mvc\Application::run returns ResponseInterface. #4831

Closed
wants to merge 1 commit into from
Closed

Zend\Mvc\Application::run returns ResponseInterface. #4831

wants to merge 1 commit into from

Conversation

reenl
Copy link

@reenl reenl commented Jul 14, 2013

This fixes a bug where the run function returns a \Zend\Mvc\Application in a lot of cases. According to the phpdoc and the documentation it should return a Zend\Stdlib\ResponseInterface.

@Maks3w
Copy link
Member

Maks3w commented Jul 14, 2013

Agree but due a potential BC Break I suggest apply it in the next major version

@reenl
Copy link
Author

reenl commented Jul 15, 2013

e2d8c14 and 643a99e where BC breaks, this restores the error somewhat.

At the moment the return value of ::run is unusable because you can't know for sure what is returned (note that it still returns the ResponseInterface in some cases before my PR). BC break or not, this is a bug and should be fixed.

The documentation should be updated too, whatever fix we do because this is simply not true no matter how we choose to fix it:

<?php
$application = $serviceManager->get('Application');
$application->bootstrap();
$response = $application->run();
$response->send();

Because a response doesn't have a send method, and the application does not return a response. And depending the routes / application environment this could be a fatal error.

Either we choose to always return Application or ResponseInterface. I can of course send this PR to develop if you wish.

If you tell me your wishes, I can make the changes.

@weierophinney
Copy link
Member

It's a documentation issue at this point. We modified how Zend\Mvc\Application works, as there were issues with simply rendering the response (if you are doing a stream, for instance, an echo was a bad idea). So, the run() method's signature should be updated, and documentation updated. We already updated the skeleton application some time ago, and messaged the change when it was released.

@reenl
Copy link
Author

reenl commented Jul 17, 2013

Note that it still returns a resonseinterface in some cases.

@reenl reenl deleted the bugfix-invalid-return branch July 18, 2013 07:38
@weierophinney
Copy link
Member

@reenl I'll correct hose; they're legacy at best, and the method should be consistent.

weierophinney added a commit to weierophinney/zendframework that referenced this pull request Jul 18, 2013
- Always return the application instance. (and fix docblock to indicate this)
- If an alternate response object is returned by an event, give it to
  the application instance.
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.

3 participants