Add an option to not follow redirects (302) and add a unit test for that - #602
Conversation
|
@kdavisk6 I have rebased on master and reworked the imports to keep it clean (my IntelliJ optimizes imports on the fly). It should now be acceptable? |
| connection.setReadTimeout(options.readTimeoutMillis()); | ||
| connection.setAllowUserInteraction(false); | ||
| connection.setInstanceFollowRedirects(true); | ||
| connection.setInstanceFollowRedirects(options.isFollowRedirects()); |
There was a problem hiding this comment.
Won't this impact other clients too?
There was a problem hiding this comment.
The other clients would need to be updated to use the option.
There was a problem hiding this comment.
I made the option default to true, so the previous clients will keep the previous behavior.
You have to explicitely set the option to false to have the new behavior.
There was a problem hiding this comment.
This change need to be included to the okhttp and ribbon
https://github.com/OpenFeign/feign/blob/master/okhttp/src/main/java/feign/okhttp/OkHttpClient.java
https://github.com/OpenFeign/feign/blob/master/ribbon/src/main/java/feign/ribbon/RibbonClient.java
There was a problem hiding this comment.
@velo I have added implementations for Ribbon, OkHttp and LbClient as well (since unit tests of Ribbon use it too).
Add unit tests for these.
|
One of the tests failed. Could you please take a look? |
|
I did and corrected. I just forgot to run the entire testing suite after commiting the new options. |
…hat (#602) * Add an option to not follow redirects (302) and add a unit test for that * Implement followRedirect options for Ribbon Client and OkHTTP. Add unit tests for these. * Fix last failing unit test with IClientConfig options handling
…hat (#602) * Add an option to not follow redirects (302) and add a unit test for that * Implement followRedirect options for Ribbon Client and OkHTTP. Add unit tests for these. * Fix last failing unit test with IClientConfig options handling
4 participants
This change allows the Options() constructor to specify whether the HttpConnection should follow redirects or not. The default is yes (as has been from the beginning) but you can now specify that the connection should return the 3xx response if received.
This might fix the #249 issue. At least it solves it in my case: I wanted to receive the 302 response of the API I'm interrogating and act upon that.