Skip to content

Commit 89d746d

Browse files
committed
Avoid async dispatch if completed in AsyncServerResponse
This commit checks whether the CompletableFuture<ServerResponse> passed to AsyncServerResponse.async has been completed, and if so returns a CompletedAsyncServerResponse that simply delegates to the completed response, instead of the DefaultAsyncServerResponse that uses async dispatch. Closes gh-32223
1 parent 3ddc512 commit 89d746d

File tree

5 files changed

+147
-35
lines changed

5 files changed

+147
-35
lines changed

spring-webmvc/src/main/java/org/springframework/web/servlet/function/AsyncServerResponse.java

+44-2
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@
1818

1919
import java.time.Duration;
2020
import java.util.concurrent.CompletableFuture;
21+
import java.util.concurrent.ExecutionException;
2122

2223
import org.reactivestreams.Publisher;
2324

25+
import org.springframework.core.ReactiveAdapter;
2426
import org.springframework.core.ReactiveAdapterRegistry;
27+
import org.springframework.lang.Nullable;
28+
import org.springframework.util.Assert;
2529

2630
/**
2731
* Asynchronous subtype of {@link ServerResponse} that exposes the future
@@ -53,7 +57,7 @@ public interface AsyncServerResponse extends ServerResponse {
5357
* @return the asynchronous response
5458
*/
5559
static AsyncServerResponse create(Object asyncResponse) {
56-
return DefaultAsyncServerResponse.create(asyncResponse, null);
60+
return createInternal(asyncResponse, null);
5761
}
5862

5963
/**
@@ -69,7 +73,45 @@ static AsyncServerResponse create(Object asyncResponse) {
6973
* @return the asynchronous response
7074
*/
7175
static AsyncServerResponse create(Object asyncResponse, Duration timeout) {
72-
return DefaultAsyncServerResponse.create(asyncResponse, timeout);
76+
return createInternal(asyncResponse, timeout);
77+
}
78+
79+
private static AsyncServerResponse createInternal(Object asyncResponse, @Nullable Duration timeout) {
80+
Assert.notNull(asyncResponse, "AsyncResponse must not be null");
81+
82+
CompletableFuture<ServerResponse> futureResponse = toCompletableFuture(asyncResponse);
83+
if (futureResponse.isDone() &&
84+
!futureResponse.isCancelled() &&
85+
!futureResponse.isCompletedExceptionally()) {
86+
87+
try {
88+
ServerResponse completedResponse = futureResponse.get();
89+
return new CompletedAsyncServerResponse(completedResponse);
90+
}
91+
catch (InterruptedException | ExecutionException ignored) {
92+
// fall through to use DefaultAsyncServerResponse
93+
}
94+
}
95+
return new DefaultAsyncServerResponse(futureResponse, timeout);
96+
}
97+
98+
@SuppressWarnings("unchecked")
99+
private static CompletableFuture<ServerResponse> toCompletableFuture(Object obj) {
100+
if (obj instanceof CompletableFuture<?> futureResponse) {
101+
return (CompletableFuture<ServerResponse>) futureResponse;
102+
}
103+
else if (DefaultAsyncServerResponse.reactiveStreamsPresent) {
104+
ReactiveAdapterRegistry registry = ReactiveAdapterRegistry.getSharedInstance();
105+
ReactiveAdapter publisherAdapter = registry.getAdapter(obj.getClass());
106+
if (publisherAdapter != null) {
107+
Publisher<ServerResponse> publisher = publisherAdapter.toPublisher(obj);
108+
ReactiveAdapter futureAdapter = registry.getAdapter(CompletableFuture.class);
109+
if (futureAdapter != null) {
110+
return (CompletableFuture<ServerResponse>) futureAdapter.fromPublisher(publisher);
111+
}
112+
}
113+
}
114+
throw new IllegalArgumentException("Asynchronous type not supported: " + obj.getClass());
73115
}
74116

75117
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.web.servlet.function;
18+
19+
import java.io.IOException;
20+
21+
import jakarta.servlet.ServletException;
22+
import jakarta.servlet.http.Cookie;
23+
import jakarta.servlet.http.HttpServletRequest;
24+
import jakarta.servlet.http.HttpServletResponse;
25+
26+
import org.springframework.http.HttpHeaders;
27+
import org.springframework.http.HttpStatusCode;
28+
import org.springframework.lang.Nullable;
29+
import org.springframework.util.Assert;
30+
import org.springframework.util.MultiValueMap;
31+
import org.springframework.web.servlet.ModelAndView;
32+
33+
/**
34+
* {@link AsyncServerResponse} implementation for completed futures.
35+
*
36+
* @author Arjen Poutsma
37+
* @since 6.2
38+
*/
39+
final class CompletedAsyncServerResponse implements AsyncServerResponse {
40+
41+
private final ServerResponse serverResponse;
42+
43+
44+
CompletedAsyncServerResponse(ServerResponse serverResponse) {
45+
Assert.notNull(serverResponse, "ServerResponse must not be null");
46+
this.serverResponse = serverResponse;
47+
}
48+
49+
@Override
50+
public ServerResponse block() {
51+
return this.serverResponse;
52+
}
53+
54+
@Override
55+
public HttpStatusCode statusCode() {
56+
return this.serverResponse.statusCode();
57+
}
58+
59+
@Override
60+
@Deprecated
61+
public int rawStatusCode() {
62+
return this.serverResponse.rawStatusCode();
63+
}
64+
65+
@Override
66+
public HttpHeaders headers() {
67+
return this.serverResponse.headers();
68+
}
69+
70+
@Override
71+
public MultiValueMap<String, Cookie> cookies() {
72+
return this.serverResponse.cookies();
73+
}
74+
75+
@Nullable
76+
@Override
77+
public ModelAndView writeTo(HttpServletRequest request, HttpServletResponse response, Context context)
78+
throws ServletException, IOException {
79+
80+
return this.serverResponse.writeTo(request, response, context);
81+
}
82+
}

spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultAsyncServerResponse.java

+1-30
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,10 @@
2929
import jakarta.servlet.http.Cookie;
3030
import jakarta.servlet.http.HttpServletRequest;
3131
import jakarta.servlet.http.HttpServletResponse;
32-
import org.reactivestreams.Publisher;
3332

34-
import org.springframework.core.ReactiveAdapter;
35-
import org.springframework.core.ReactiveAdapterRegistry;
3633
import org.springframework.http.HttpHeaders;
3734
import org.springframework.http.HttpStatusCode;
3835
import org.springframework.lang.Nullable;
39-
import org.springframework.util.Assert;
4036
import org.springframework.util.ClassUtils;
4137
import org.springframework.util.MultiValueMap;
4238
import org.springframework.web.context.request.async.AsyncWebRequest;
@@ -62,7 +58,7 @@ final class DefaultAsyncServerResponse extends ErrorHandlingServerResponse imple
6258
private final Duration timeout;
6359

6460

65-
private DefaultAsyncServerResponse(CompletableFuture<ServerResponse> futureResponse, @Nullable Duration timeout) {
61+
DefaultAsyncServerResponse(CompletableFuture<ServerResponse> futureResponse, @Nullable Duration timeout) {
6662
this.futureResponse = futureResponse;
6763
this.timeout = timeout;
6864
}
@@ -167,29 +163,4 @@ private DeferredResult<ServerResponse> createDeferredResult(HttpServletRequest r
167163
});
168164
return result;
169165
}
170-
171-
@SuppressWarnings({"rawtypes", "unchecked"})
172-
public static AsyncServerResponse create(Object obj, @Nullable Duration timeout) {
173-
Assert.notNull(obj, "Argument to async must not be null");
174-
175-
if (obj instanceof CompletableFuture futureResponse) {
176-
return new DefaultAsyncServerResponse(futureResponse, timeout);
177-
}
178-
else if (reactiveStreamsPresent) {
179-
ReactiveAdapterRegistry registry = ReactiveAdapterRegistry.getSharedInstance();
180-
ReactiveAdapter publisherAdapter = registry.getAdapter(obj.getClass());
181-
if (publisherAdapter != null) {
182-
Publisher<ServerResponse> publisher = publisherAdapter.toPublisher(obj);
183-
ReactiveAdapter futureAdapter = registry.getAdapter(CompletableFuture.class);
184-
if (futureAdapter != null) {
185-
CompletableFuture<ServerResponse> futureResponse =
186-
(CompletableFuture<ServerResponse>) futureAdapter.fromPublisher(publisher);
187-
return new DefaultAsyncServerResponse(futureResponse, timeout);
188-
}
189-
}
190-
}
191-
throw new IllegalArgumentException("Asynchronous type not supported: " + obj.getClass());
192-
}
193-
194-
195166
}

spring-webmvc/src/main/java/org/springframework/web/servlet/function/ServerResponse.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ static BodyBuilder unprocessableEntity() {
246246
* @since 5.3
247247
*/
248248
static ServerResponse async(Object asyncResponse) {
249-
return DefaultAsyncServerResponse.create(asyncResponse, null);
249+
return AsyncServerResponse.create(asyncResponse);
250250
}
251251

252252
/**
@@ -267,7 +267,7 @@ static ServerResponse async(Object asyncResponse) {
267267
* @since 5.3.2
268268
*/
269269
static ServerResponse async(Object asyncResponse, Duration timeout) {
270-
return DefaultAsyncServerResponse.create(asyncResponse, timeout);
270+
return AsyncServerResponse.create(asyncResponse, timeout);
271271
}
272272

273273
/**

spring-webmvc/src/test/java/org/springframework/web/servlet/function/DefaultAsyncServerResponseTests.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,29 @@
2828
class DefaultAsyncServerResponseTests {
2929

3030
@Test
31-
void block() {
31+
void blockCompleted() {
3232
ServerResponse wrappee = ServerResponse.ok().build();
3333
CompletableFuture<ServerResponse> future = CompletableFuture.completedFuture(wrappee);
3434
AsyncServerResponse response = AsyncServerResponse.create(future);
3535

3636
assertThat(response.block()).isSameAs(wrappee);
3737
}
3838

39+
@Test
40+
void blockNotCompleted() {
41+
ServerResponse wrappee = ServerResponse.ok().build();
42+
CompletableFuture<ServerResponse> future = CompletableFuture.supplyAsync(() -> {
43+
try {
44+
Thread.sleep(500);
45+
return wrappee;
46+
}
47+
catch (InterruptedException ex) {
48+
throw new RuntimeException(ex);
49+
}
50+
});
51+
AsyncServerResponse response = AsyncServerResponse.create(future);
52+
53+
assertThat(response.block()).isSameAs(wrappee);
54+
}
55+
3956
}

0 commit comments

Comments
 (0)