Hotfix: Undeprecate PhpEnvironement Response methods #3543

Merged
merged 2 commits into from Jan 25, 2013

2 participants

@ralphschindler
Zend Framework member

Simply b/c facilities have been added so that Zend\Mvc can send responses does not remove the requirement that a PhpEnvironment\Response must be able to send itself.

Also, Zend\Http does not have a dependency on Zend\Mvc, so changes in behavior in Zend\Mvc should be of no consequence to Zend\Http.

@weierophinney
Zend Framework member
@ralphschindler
Zend Framework member

If Zend\Mvc's Application is setting up a listener that sends the response, then I guess it needs to take care of not sending a Response object as the return value of run(). If you want to be BC in applications that have ->send() in the, Application could return itself and have a send() method that is basically a NOOP, or return an empty object that can respond to send() with a NOOP.

The problem I ran into is this:
Zend\Authentication\Adapter\Http Digest needs to be able to alter the Response, and you might ideally want to exit() early and simply send() the contents when you realize that the Digest handshake is not yet complete - and you don't want any other dispatch() code to run b/c after all, authentication is not complete.

@ralphschindler
Zend Framework member

Also, I'm not sure what the intention was, but this is just changing the docblocks, which in turn will remove the line through the method call in IDE's. It doesn't seem like the code has changed- was it suppose to?

@weierophinney
Zend Framework member
@ralphschindler
Zend Framework member

That's a case when you should return a response from the listener - that will bubble out of the event loop and dispatch cycle. If you're using exit, you're doing something wrong.

I didn't know that was the case, this works for my particular use case as well.

I still think send() should not be deprecated, I am sure there of plenty non-MVC contextual use cases where someone will be using Zend\Http as a stand alone component, and might need to send() a response. Think web-service classes, or Oauth not in an MVC based application. My point still stands that requirement changes in MVC shouldn't impact the API and requirements of the Zend\Http component when the directionality of the dependency is MVC -> Http and not MVC <--> Http. :)

@weierophinney
Zend Framework member
@ralphschindler
Zend Framework member

Either/or:

a) create a nop object in stdlib that looks like this:

<?php
namespace Zend\Stdlib;
class Nop
{
    public function __get($name)
    {
        return $this;
    }
    public function __set($name, $value)
    {
        return $this;
    }
    public function __call($method, $args)
    {
        return $this;
    }
}

Or, we create an Zend\Mvc\Response which extends PhpEnvironment\Response that overrides send() with an empty body.

Which do you think is a better route?

@weierophinney
Zend Framework member

@ralphschindler Don't overcomplicate it. I'd simply change Application::run() such that:

// on line 308, which now reads:
return $this->completeRequest($event);

// to this:
$this->completeRequest($event);
return $this;

Then add a send() method with an empty body.

@ralphschindler
Zend Framework member

Sounds good, part of me wanted to avoid having a send() method in the Application object - but for all intents and purposes, it doesn't really matter, no one really interacts with an Application object much enough to get send() popping up in their code completion.

@ralphschindler ralphschindler Zend\Mvc\Application
run() returns the Application object which now has a deprecated
send() method.  This will ensure IDE's display a deprecated/
strike-through send() method when Zend\Mvc consumers (The skeleton
application applications) use send() from their index.php file.
e2d8c14
@weierophinney weierophinney added a commit that referenced this pull request Jan 25, 2013
@weierophinney weierophinney [#3543] Fix failing test
- A test was calling run()->getBody(); modified to call run() prior, and then
  getResponse()->getBody().
4417ddf
@weierophinney weierophinney added a commit that referenced this pull request Jan 25, 2013
@weierophinney weierophinney Merge branch 'hotfix/3543' into develop
Close #3543
3466f7b
@weierophinney weierophinney merged commit e2d8c14 into zendframework:develop Jan 25, 2013
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney [#3543] Fix failing test
- A test was calling run()->getBody(); modified to call run() prior, and then
  getResponse()->getBody().
edfae48
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/3543' into develop
Close #3543
d76e68e
@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/3543' into develop fc21a96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment