Skip to content

Conversation

mgriffin
Copy link

The followRedirect() method has changed to automatically follow redirects. I've changed the documentation to reflect this and left a note on how to change it back to the way it used to work.

it automatically. You can examine the response and force a redirection
afterwards with the ``followRedirect()`` method::
When a request returns a redirect response, the client follows it
automatically. You can prevent this by issuing the ``followRedirects()``
Copy link
Member

Choose a reason for hiding this comment

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

I'd use API links here (same below):

:method:`Symfony\\Component\\HttpKernel\\Client::followRedirects`

Copy link
Author

Choose a reason for hiding this comment

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

I was following the style that was already there. I can change it if that's now the preferred style

Copy link
Member

Choose a reason for hiding this comment

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

It's always good to have the API link in the docs. Maybe no one noticed this the first time.

Copy link
Member

Choose a reason for hiding this comment

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

in fact, this section (and article) is from 2011 (when the docs looked like this). The API links where added in 2012, so it did not exists when writing this.

mgriffin and others added 2 commits December 31, 2013 23:31
The followRedirect() method has changed to automatically follow redirects. I've changed the documentation to reflect this and left a note on how to change it back to the way it used to work.
@mgriffin
Copy link
Author

How does this look?

@xabbuh
Copy link
Member

xabbuh commented Jan 2, 2014

👍

@weaverryan
Copy link
Member

Hey Mike!

I'm actually not sure this is correct, We'll it may be more complicated. See the original PR that made this change at #1257. So, the default Client (in the BrowserKit component) does follow redirects automatically. But in a Symfony framework functional test, we're actually using the FrameworkBundle Client, which extends the Client in HttpKernel. As you can see, it reverses that setting and sets it to not follow redirects.

Are you seeing different behavior? And if so, do you have a case where the default is different or am I possibly missing something in the code?

Thanks!

@mgriffin
Copy link
Author

mgriffin commented Jan 7, 2014

Ah, that's really confusing! 😃

I was coming from the silex docs, they link to the symfony2 ones. I missed that symphony uses the FrameworkBundle Client while silex uses the BrowserKit component. I guess I went too deep in trying to fix the documentation!

Is there a reason that the two methods work in opposite ways?

@weaverryan
Copy link
Member

Hey Mike!

That makes more sense, and that is confusing! I don't remember/know why there is a difference. However, I did look at the Silex docs, and it looks like even there (by extending this class: https://github.com/fabpot/Silex/blob/master/src/Silex/WebTestCase.php#L14, via the docs: http://silex.sensiolabs.org/doc/testing.html#webtestcase), you should still end up using the HttpKernel Client directly, which still means that followRedirects is set to false. Is there another resource that pointed you to use the BrowserKit directly? Or something else going on?

Thanks!

@mgriffin
Copy link
Author

The only thing that I can see that might be different is that the SIlex docs say that you have to install the dependencies manually to use the Symfony WebTestCase. Then it says to add symfony/browser-kit in to the composer.json file.

After digging a little deeper, it seems that HttpKernel\Client extends Browser-Kit\Client and even though it sets followRedirects to false in the constructor, it then calls the parent constructor which sets it back to true.

It's a little convoluted, so it is.

@wouterj
Copy link
Member

wouterj commented Jan 12, 2014

@mgriffin the BrowerKit Client doesn't set followRedirects to true in the constructor.

@mgriffin
Copy link
Author

Hmm, that's right. But it does on my machine...

Ah hah! It doesn't now, with version 2.4.1. But the silex docs say to use >=2.3,<2.4-dev. When I tried to use that, it installed 2.3 which does set followRedirects in the constructor.

I checked to see if using 2.4 works and for this problem it does. Time to go and see about changing toe silex docs, I guess.

Thanks for all the help.

@xabbuh
Copy link
Member

xabbuh commented Jan 12, 2014

But in 2.3 the followRedirects property is set to false after the parent constructor has been called.

@mgriffin
Copy link
Author

@xabbuh Where does it do that?

I've tried it a number of different ways.

When I load browser-kit v2.3 with composer as per the silex docs, I have to set $client->followRedirects(false) if I want to check the bits before the redirect. I can then use $crawler->followRedirect() to follow the redirect and run tests on the page that is eventually shown.

If I then change composer.json to load browser-kit v2.4, I don't have to set $client->followRedirects(false), the redirect is not automatically followed.

I'm not sure what else to do here, but the functionality has definitely changed there. I'm happy I can work around it, but I wanted to make sure that no one else has to run in to the same problem.

@xabbuh
Copy link
Member

xabbuh commented Jan 15, 2014

@weaverryan
Copy link
Member

Yes, and you can see the slight change between 2.3 and 2.4 (notice its positioning relative to the parent construct), though I can't think of how this would make a difference.

https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/HttpKernel/Client.php#L49
https://github.com/symfony/symfony/blob/2.4/src/Symfony/Component/HttpKernel/Client.php#L47

In both cases, if you're using the Client in HttpKernel (which both Symfony and Silex do), followRedirects should be set to false. So, it's interesting @mgriffin that you see a difference in behavior between the versions: there is a slight code difference (above links), but I can't see how this would cause the different behavior.

Can anyone else spot why that would cause Silex to follow redirects by default? I think we're going to ultimately make no change to the docs, but I would love to know if we're missing anything on this :).

Thanks!

@stof
Copy link
Member

stof commented Mar 18, 2014

@weaverryan the behavior is only different for custom child classes overwriting setServerParameter (which is called in the parent constructor): they can now rely on the proper flag being set by the constructor. IIRC, this was the reason of the change.

I don't see why Silex would behave differently though

@weaverryan
Copy link
Member

I'm going to close this issue. As it's written, it is correct. There were possibly some subtle differences with Silex and different versions of Symfony (which further confuse this issue), but I don't see anything that's actually wrong with the docs. Though, we there's a clarifying point we can add to help, I'd be open to that :).

Cheers!

@weaverryan weaverryan closed this Apr 2, 2014
@mgriffin
Copy link
Author

mgriffin commented Apr 3, 2014

That's fair enough. I could never understand what I was doing differently, but I got it working in the end.

If it crops up again, I'll re submit it 😈 😄

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

Successfully merging this pull request may close these issues.

5 participants