Skip to content
New issue

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

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

Already on GitHub? # to your account

TestRestTemplate does not allow redirects to be customized #43258

Closed
wants to merge 1 commit into from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Nov 22, 2024

Closes GH-27360

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 22, 2024
@quaff quaff changed the title Add redirects method to RestTemplateBuilder Add withRedirects method to TestRestTemplate and deprecate HttpClientOption.ENABLE_REDIRECTS Nov 22, 2024
@philwebb philwebb changed the title Add withRedirects method to TestRestTemplate and deprecate HttpClientOption.ENABLE_REDIRECTS Add withRedirects method to TestRestTemplate and deprecate HttpClientOption.ENABLE_REDIRECTS Nov 22, 2024
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Nov 22, 2024
@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review and removed for: team-meeting An issue we'd like to discuss as a team to make progress labels Dec 3, 2024
@philwebb philwebb self-assigned this Dec 4, 2024
@philwebb philwebb added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Dec 4, 2024
@philwebb philwebb modified the milestones: 3.4.x, 3.4.1 Dec 4, 2024
philwebb pushed a commit that referenced this pull request Dec 4, 2024
Add `redirects(...)` method to `RestTemplateBuilder` to allow redirect
customization. This new method is required in 3.4 since the default
redirect strategy for some clients has changed and users need a way
to restore the old behavior.

See gh-43258
philwebb added a commit that referenced this pull request Dec 4, 2024
Remove deprecations and new methods in `TestRestTemplate` in favor of
passing in a configured `RestTemplateBuilder`.

See gh-43258
@philwebb philwebb closed this in b7f3627 Dec 4, 2024
@philwebb
Copy link
Member

philwebb commented Dec 4, 2024

Thanks @quaff. I've simplified things a bit in 8b83afd so that we're no longer needing so many changes to TestRestTemplateBuilder. For folks that want specific redirect behavior they can pass in a configured RestTemplateBuilder and use your new redirects(...) method.

There's still some more improvements I'd like to make to TestRestTemplateBuilder in #43422, but those can wait until 3.5

@quaff
Copy link
Contributor Author

quaff commented Dec 5, 2024

Thanks @quaff. I've simplified things a bit in 8b83afd so that we're no longer needing so many changes to TestRestTemplateBuilder. For folks that want specific redirect behavior they can pass in a configured RestTemplateBuilder and use your new redirects(...) method.

There's still some more improvements I'd like to make to TestRestTemplateBuilder in #43422, but those can wait until 3.5

@philwebb You removed TestRestTemplate::withRedirects in 8b83afd, then how to create a new TestRestTemplate base on the injected one?

@philwebb
Copy link
Member

philwebb commented Dec 5, 2024

That's a good point, I overlooked the injection. I was keen to not add more public API to TestRestTemplate, but duplicating org.springframework.boot.test.web.client.TestRestTemplateContextCustomizer.TestRestTemplateFactory in tests will be a bit cumbersome.

I'll reopen the issue to consider things a bit more.

@philwebb philwebb reopened this Dec 5, 2024
@philwebb philwebb closed this in 8ca8ab1 Dec 6, 2024
@philwebb
Copy link
Member

philwebb commented Dec 6, 2024

Thanks again for the feedback @quaff. I've added a withRequestFactorySettings methods that should allow you to apply settings that include alternative redirects. Let me know if that works for you.

@quaff
Copy link
Contributor Author

quaff commented Dec 9, 2024

Thanks again for the feedback @quaff. I've added a withRequestFactorySettings methods that should allow you to apply settings that include alternative redirects. Let me know if that works for you.

Still not able to update redirects base on underlying settings, I created #43444.

@philwebb
Copy link
Member

philwebb commented Dec 9, 2024

@quaff Do you need to be able to update just the redirects? I assumed that using a complete ClientHttpRequestFactorySettings would be enough for most people.

@philwebb philwebb reopened this Dec 9, 2024
@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Dec 9, 2024
@quaff
Copy link
Contributor Author

quaff commented Dec 10, 2024

@quaff Do you need to be able to update just the redirects?

Yes.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 10, 2024
@philwebb philwebb changed the title Add withRedirects method to TestRestTemplate and deprecate HttpClientOption.ENABLE_REDIRECTS TestRestTemplate does not allow redirects to be customized Dec 10, 2024
@philwebb philwebb closed this in e72546d Dec 10, 2024
@quaff
Copy link
Contributor Author

quaff commented Dec 11, 2024

I like the UnaryOperator as customizer, thanks. @philwebb

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
status: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable redirects by default in TestRestTemplate for all HTTP clients
3 participants