Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response headers are unavailable outside the WebApiContext #6

Open
fgm opened this issue May 15, 2014 · 21 comments
Open

Response headers are unavailable outside the WebApiContext #6

fgm opened this issue May 15, 2014 · 21 comments

Comments

@fgm
Copy link
Contributor

fgm commented May 15, 2014

Since $response has been set to private and $headers is currently only used for request headers, contexts inheriting WebApiContext have no way to access the response headers.

Suggested change:

  • either mark $response as protected instead of private
  • or add $this->headers = $this->response->getHeaders(); at the end of sendRequest(), making headers usable for both request and response
  • or provide separate properties+accessors for request and response headers.
@stof
Copy link
Member

stof commented May 15, 2014

the second option is invalid. Request headers and response headers are 2 totally different stuff. They should not be mixed in the same property (it would break stuff when sending a second request).

@fgm
Copy link
Contributor Author

fgm commented May 15, 2014

Then 1 or 3 ? I prefer 1 as it does not decide on behalf of the users, and keeps their options open.

2 could probably work too, but that would require awkward scenarios resetting the headers before adding new ones, so I agree it is not a good idea.

@stof
Copy link
Member

stof commented May 15, 2014

2 cannot work, because it mean you loose the configured request headers when receiving a response. and we cannot reset the array before sendign a request, as this would defeat the whole point of adding this array in the first place.

I don't like 1 because it means that the response property becomes an extension point of the context, for which we cannot do any change without BC break. This would restrict our possibilities too much for the future. Thus, each user of the response property would have to be careful when using it, because its type is Response|null

I prefer doing a bigger refactoring to include things coming from the RestExtension. See #3.
It is just that I have only a limited amount of time, and my priorities for the moment are to release MinkExtension and Symfony2Extension (which are ready for the release) as well as Mink 1.6 (the plan is the end of the weekend).

@dready
Copy link

dready commented Jun 11, 2014

Hi!

Could you at least make the response object available via getter...

it's not possible to add additional assertions in our contexts as our hands are tight at the moment.

i could send a pull request if you wish.

best regards, thomas

@pasxel
Copy link

pasxel commented Sep 17, 2014

either mark $response as protected instead of private
+1

@dready
Copy link

dready commented Sep 17, 2014

@pasxel i would not recommend that as then you cannot guarantee what someone will do with $response. Better having a getter for it. but that's just my opinion..

@pasxel
Copy link

pasxel commented Sep 17, 2014

@dready Yes, I agree. The main thing to get response in the child class.

@dantleech
Copy link
Contributor

+1

1 similar comment
@tomasz-janiczek
Copy link

+1

@dready
Copy link

dready commented Feb 5, 2015

actually there is a pull request waiting for this...
#10

@AndyWendt
Copy link

👍 I NEED TO GET TO THE RESPONSE

@juliobetta
Copy link

For god's sake, let us get access to response.

@kwisatz
Copy link

kwisatz commented Apr 16, 2015

👍 Required since we would like to use our own, unescaped regexes, etc.

@dantleech
Copy link
Contributor

ping @stof

@njbarrett
Copy link

Yeah, we need to access the response somehow.

@fabioginzel
Copy link

I need access response +1

@mattjanssen
Copy link

This is really silly. The best part of this extension is the setting up and constructing of the response object. But then we can't get to it! Copy-paste copy-paste copy-paste. We need the response object. 😾

@jurgenhaas
Copy link

This is important to us as well. Is this project still maintained?

@dantleech
Copy link
Contributor

@jurgenhaas i would say not /cc @stof

@juliobetta
Copy link

The best thing we can do now is to fork the project, modify it as we need and add these lines to composer.json:

"require-dev": {
    "behat/web-api-extension": "dev-master"
},
"repositories": [
    {
        "type": "vcs",
        "url": "https://github.com/yourGithubAccount/WebApiExtension"
    }
]

@jurgenhaas
Copy link

Is there a way to use this approach with composer require ...?

I have tried composer require --prefer-source https://github.com/jurgenhaas/WebApiExtension.git behat/web-api-extension but that complaints about minimum-stability. Note that my project uses only stable releases for the other packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests