Skip to content

Conversation

@UpStreamThomas
Copy link

Before this change, DefaultRestTestClient.mutate() was reusing the underlying builder all calls.

Building a new builder for each call clones the RestTestClientBuilder, protecting from side effects.

@UpStreamThomas UpStreamThomas force-pushed the fix-rest-test-client-mutate-side-effect branch from 7916536 to 3df1c62 Compare October 28, 2025 11:33
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 28, 2025
@UpStreamThomas UpStreamThomas force-pushed the fix-rest-test-client-mutate-side-effect branch from 3df1c62 to 5b30f1d Compare October 29, 2025 08:52
Copy link

@RemiVangrevelynghe RemiVangrevelynghe left a comment

Choose a reason for hiding this comment

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

Thanks !

@rstoyanchev rstoyanchev self-assigned this Oct 31, 2025
@rstoyanchev
Copy link
Contributor

Thanks for the pull request .This is done in .build() currently, but should be done in .mutate() instead.

@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 31, 2025
@rstoyanchev rstoyanchev added this to the 7.0.0 milestone Oct 31, 2025
@rstoyanchev rstoyanchev changed the title fix: RestTestClient.mutate() should not have side effects RestTestClient.mutate() should not have side effects Oct 31, 2025
@UpStreamThomas
Copy link
Author

Thanks for the pull request .This is done in .build() currently, but should be done in .mutate() instead.

I did not notice that that builder was cloned in the build() method, do you want me to remove it ?

@rstoyanchev
Copy link
Contributor

Yes, it should be removed from .build(), effectively moved to .mutate().

trecloux and others added 2 commits November 1, 2025 16:12
Before this change, DefaultRestTestClient.mutate() was reusing the
underlying builder all calls.
Building a new builder for each call clones the RestTestClientBuilder,
protecting from side effects.

Signed-off-by: Thomas Recloux <[email protected]>
@UpStreamThomas UpStreamThomas force-pushed the fix-rest-test-client-mutate-side-effect branch from 5b30f1d to c72052c Compare November 1, 2025 15:20
@UpStreamThomas
Copy link
Author

Yes, it should be removed from .build(), effectively moved to .mutate().

I added a commit with clone removed from build().
I also rebased the branch against main.

rstoyanchev pushed a commit that referenced this pull request Nov 3, 2025
@rstoyanchev
Copy link
Contributor

On closer look, we need to keep the cloning in build() to protect against changes to the original builder. I've added an additional test for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants