Update retrieve() in RestClient to execute and extract in single scope

[rstoyanchev]

The retrieve() workflow in RestClient splits the exchange over two method calls. The call to retrieve() executes the request but doesn't extract, and the subsequent call to one of the ResponseSpec methods actually extracts the response.

This makes sense but it's also reasonable to conclude you can just call retrieve() if you don't care for the response. In that case, however, the observation scope remains open, the status is not checked, and the connection is not closed.

retrieve() should merely a step towards fully declaring the exchange before, and only once a ResponseSpec method is selected should we proceed with execution and response extraction. Same way it works with the RestTemplate.

This will make the retrieve() workflow a true shortcut for calling exchange(), fully scoped within a call to exchange() with a built-in ExchangeFunction based on the ResponseSpec selection. If an application actually wants to split execution and extraction into separate steps, it can use the exchange variants.

[fmasa]

Could this behavior change be mentioned in the changelog? 🙏

[bclozel]

@fmasa can you elaborate which behavior change surprised you while upgrading?

[fmasa]

@bclozel The 6.2.0 changed the behavior of retrieve. Previously it actually executed the HTTP request but now it does not (and the needs to call ResponseSpec.

We had one fire&forget request that stopped being called after upgrading to Spring 6.2.0 and when going through release notes I did not find any mention of this. 🙂

[bclozel]

Thanks, I'll add a note in the release notes. Out of curiosity: we added an annotation to warn people about this in #33818. IDEs should highlight this on your screen. Does it work for you?

I know this doesn't replace a warning in the release notes but just wanted to make sure this shows for future usages.

[bclozel]

See https://github.com/spring-projects/spring-framework/wiki/Spring-Framework-6.2-Release-Notes#http-clients

[fmasa]

@bclozel Thanks a lot 🙇

[fmasa]

For the annotation: I don't see anything in IDEA. But quick search shows that it's not supported there yet?.

We also use detekt which catches that 🙂

[denniseffing]

@rstoyanchev The commit for this change also introduced the @Nullable annotation on the exchange method, this breaks our code compilation when upgrading. We presume it was always nullable but the annotation was just missing? Anyway, to fix it correctly on our side I'd like to understand under which circumstances the exchange method may actually return null. Could you elaborate on that?

[rstoyanchev]

exchange is a way to use a callback to convert the response to an object. That callback can return null, so this was a straightforward case of adding nullability information where it was previously missing. It was added on both the exchange methods, and on the underlying ExchangeFunction callback in #33779.