Skip to content

Commit 2453412

Browse files
rstoyanchevrstoyanchev
committed
Polishing in WebAsyncManager
See gh-34192
1 parent 875cc82 commit 2453412

File tree

4 files changed

+105
-95
lines changed

4 files changed

+105
-95
lines changed

‎spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java

+58-48
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -288,65 +288,75 @@ public boolean setErrorResult(Object result) {
288288
}
289289

290290

291-
final DeferredResultProcessingInterceptor getInterceptor() {
292-
return new DeferredResultProcessingInterceptor() {
293-
@Override
294-
public <S> boolean handleTimeout(NativeWebRequest request, DeferredResult<S> deferredResult) {
295-
boolean continueProcessing = true;
296-
try {
297-
if (timeoutCallback != null) {
298-
timeoutCallback.run();
299-
}
300-
}
301-
finally {
302-
Object value = timeoutResult.get();
303-
if (value != RESULT_NONE) {
304-
continueProcessing = false;
305-
try {
306-
setResultInternal(value);
307-
}
308-
catch (Throwable ex) {
309-
logger.debug("Failed to handle timeout result", ex);
310-
}
311-
}
291+
final DeferredResultProcessingInterceptor getLifecycleInterceptor() {
292+
return new LifecycleInterceptor();
293+
}
294+
295+
296+
/**
297+
* Handles a DeferredResult value when set.
298+
*/
299+
@FunctionalInterface
300+
public interface DeferredResultHandler {
301+
302+
void handleResult(@Nullable Object result);
303+
}
304+
305+
306+
/**
307+
* Instance interceptor to receive Servlet container notifications.
308+
*/
309+
private class LifecycleInterceptor implements DeferredResultProcessingInterceptor {
310+
311+
@Override
312+
public <S> boolean handleTimeout(NativeWebRequest request, DeferredResult<S> result) {
313+
boolean continueProcessing = true;
314+
try {
315+
if (timeoutCallback != null) {
316+
timeoutCallback.run();
312317
}
313-
return continueProcessing;
314318
}
315-
@Override
316-
public <S> boolean handleError(NativeWebRequest request, DeferredResult<S> deferredResult, Throwable t) {
317-
try {
318-
if (errorCallback != null) {
319-
errorCallback.accept(t);
320-
}
321-
}
322-
finally {
319+
finally {
320+
Object value = timeoutResult.get();
321+
if (value != RESULT_NONE) {
322+
continueProcessing = false;
323323
try {
324-
setResultInternal(t);
324+
setResultInternal(value);
325325
}
326326
catch (Throwable ex) {
327-
logger.debug("Failed to handle error result", ex);
327+
logger.debug("Failed to handle timeout result", ex);
328328
}
329329
}
330-
return false;
331330
}
332-
@Override
333-
public <S> void afterCompletion(NativeWebRequest request, DeferredResult<S> deferredResult) {
334-
expired = true;
335-
if (completionCallback != null) {
336-
completionCallback.run();
331+
return continueProcessing;
332+
}
333+
334+
@Override
335+
public <S> boolean handleError(NativeWebRequest request, DeferredResult<S> result, Throwable t) {
336+
try {
337+
if (errorCallback != null) {
338+
errorCallback.accept(t);
337339
}
338340
}
339-
};
340-
}
341-
341+
finally {
342+
try {
343+
setResultInternal(t);
344+
}
345+
catch (Throwable ex) {
346+
logger.debug("Failed to handle error result", ex);
347+
}
348+
}
349+
return false;
350+
}
342351

343-
/**
344-
* Handles a DeferredResult value when set.
345-
*/
346-
@FunctionalInterface
347-
public interface DeferredResultHandler {
352+
@Override
353+
public <S> void afterCompletion(NativeWebRequest request, DeferredResult<S> result) {
354+
expired = true;
355+
if (completionCallback != null) {
356+
completionCallback.run();
357+
}
358+
}
348359

349-
void handleResult(@Nullable Object result);
350360
}
351361

352362
}

‎spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java

+38-38
Original file line numberDiff line numberDiff line change
@@ -382,36 +382,6 @@ public void startCallableProcessing(final WebAsyncTask<?> webAsyncTask, Object..
382382
}
383383
}
384384

385-
private void setConcurrentResultAndDispatch(@Nullable Object result) {
386-
Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null");
387-
synchronized (WebAsyncManager.this) {
388-
if (!this.state.compareAndSet(State.ASYNC_PROCESSING, State.RESULT_SET)) {
389-
if (logger.isDebugEnabled()) {
390-
logger.debug("Async result already set: [" + this.state.get() +
391-
"], ignored result for " + formatUri(this.asyncWebRequest));
392-
}
393-
return;
394-
}
395-
396-
this.concurrentResult = result;
397-
if (logger.isDebugEnabled()) {
398-
logger.debug("Async result set for " + formatUri(this.asyncWebRequest));
399-
}
400-
401-
if (this.asyncWebRequest.isAsyncComplete()) {
402-
if (logger.isDebugEnabled()) {
403-
logger.debug("Async request already completed for " + formatUri(this.asyncWebRequest));
404-
}
405-
return;
406-
}
407-
408-
if (logger.isDebugEnabled()) {
409-
logger.debug("Performing async dispatch for " + formatUri(this.asyncWebRequest));
410-
}
411-
this.asyncWebRequest.dispatch();
412-
}
413-
}
414-
415385
/**
416386
* Start concurrent request processing and initialize the given
417387
* {@link DeferredResult} with a {@link DeferredResultHandler} that saves
@@ -443,7 +413,7 @@ public void startDeferredResultProcessing(
443413
}
444414

445415
List<DeferredResultProcessingInterceptor> interceptors = new ArrayList<>();
446-
interceptors.add(deferredResult.getInterceptor());
416+
interceptors.add(deferredResult.getLifecycleInterceptor());
447417
interceptors.addAll(this.deferredResultInterceptors.values());
448418
interceptors.add(timeoutDeferredResultInterceptor);
449419

@@ -508,6 +478,36 @@ private void startAsyncProcessing(Object[] processingContext) {
508478
this.asyncWebRequest.startAsync();
509479
}
510480

481+
private void setConcurrentResultAndDispatch(@Nullable Object result) {
482+
Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null");
483+
synchronized (WebAsyncManager.this) {
484+
if (!this.state.compareAndSet(State.ASYNC_PROCESSING, State.RESULT_SET)) {
485+
if (logger.isDebugEnabled()) {
486+
logger.debug("Async result already set: [" + this.state.get() + "], " +
487+
"ignored result for " + formatUri(this.asyncWebRequest));
488+
}
489+
return;
490+
}
491+
492+
this.concurrentResult = result;
493+
if (logger.isDebugEnabled()) {
494+
logger.debug("Async result set for " + formatUri(this.asyncWebRequest));
495+
}
496+
497+
if (this.asyncWebRequest.isAsyncComplete()) {
498+
if (logger.isDebugEnabled()) {
499+
logger.debug("Async request already completed for " + formatUri(this.asyncWebRequest));
500+
}
501+
return;
502+
}
503+
504+
if (logger.isDebugEnabled()) {
505+
logger.debug("Performing async dispatch for " + formatUri(this.asyncWebRequest));
506+
}
507+
this.asyncWebRequest.dispatch();
508+
}
509+
}
510+
511511
private static String formatUri(AsyncWebRequest asyncWebRequest) {
512512
HttpServletRequest request = asyncWebRequest.getNativeRequest(HttpServletRequest.class);
513513
return (request != null ? "\"" + request.getRequestURI() + "\"" : "servlet container");
@@ -517,13 +517,13 @@ private static String formatUri(AsyncWebRequest asyncWebRequest) {
517517
/**
518518
* Represents a state for {@link WebAsyncManager} to be in.
519519
* <p><pre>
520-
* NOT_STARTED <------+
521-
* | |
522-
* v |
523-
* ASYNC_PROCESSING |
524-
* | |
525-
* v |
526-
* RESULT_SET -------+
520+
* +------> NOT_STARTED <------+
521+
* | | |
522+
* | v |
523+
* | ASYNC_PROCESSING |
524+
* | | |
525+
* | v |
526+
* <-------+ RESULT_SET -------+
527527
* </pre>
528528
* @since 5.3.33
529529
*/

‎spring-web/src/test/java/org/springframework/web/context/request/async/DeferredResultTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -93,7 +93,7 @@ void onCompletion() throws Exception {
9393
DeferredResult<String> result = new DeferredResult<>();
9494
result.onCompletion(() -> sb.append("completion event"));
9595

96-
result.getInterceptor().afterCompletion(null, null);
96+
result.getLifecycleInterceptor().afterCompletion(null, null);
9797

9898
assertThat(result.isSetOrExpired()).isTrue();
9999
assertThat(sb.toString()).isEqualTo("completion event");
@@ -109,7 +109,7 @@ void onTimeout() throws Exception {
109109
result.setResultHandler(handler);
110110
result.onTimeout(() -> sb.append("timeout event"));
111111

112-
result.getInterceptor().handleTimeout(null, null);
112+
result.getLifecycleInterceptor().handleTimeout(null, null);
113113

114114
assertThat(sb.toString()).isEqualTo("timeout event");
115115
assertThat(result.setResult("hello")).as("Should not be able to set result a second time").isFalse();
@@ -127,7 +127,7 @@ void onError() throws Exception {
127127
Exception e = new Exception();
128128
result.onError(t -> sb.append("error event"));
129129

130-
result.getInterceptor().handleError(null, null, e);
130+
result.getLifecycleInterceptor().handleError(null, null, e);
131131

132132
assertThat(sb.toString()).isEqualTo("error event");
133133
assertThat(result.setResult("hello")).as("Should not be able to set result a second time").isFalse();

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -333,8 +333,8 @@ public void run() {
333333
this.subscription.request(1);
334334
}
335335
catch (final Throwable ex) {
336-
if (logger.isTraceEnabled()) {
337-
logger.trace("Send for " + this.emitter + " failed: " + ex);
336+
if (logger.isDebugEnabled()) {
337+
logger.debug("Send for " + this.emitter + " failed: " + ex);
338338
}
339339
terminate();
340340
this.emitter.completeWithError(ex);
@@ -347,8 +347,8 @@ public void run() {
347347
Throwable ex = this.error;
348348
this.error = null;
349349
if (ex != null) {
350-
if (logger.isTraceEnabled()) {
351-
logger.trace("Publisher for " + this.emitter + " failed: " + ex);
350+
if (logger.isDebugEnabled()) {
351+
logger.debug("Publisher for " + this.emitter + " failed: " + ex);
352352
}
353353
this.emitter.completeWithError(ex);
354354
}

0 commit comments

Comments
 (0)