diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java index 2e478a1db4..a1132ef6a9 100644 --- a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java @@ -21,12 +21,15 @@ import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; @@ -38,15 +41,20 @@ import org.springframework.util.MultiValueMap; * Base class for {@link ServerHttpResponse} implementations. * * @author Rossen Stoyanchev + * @author Sebastien Deleuze */ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { + private static Log logger = LogFactory.getLog(AbstractServerHttpResponse.class); + private static final int STATE_NEW = 1; private static final int STATE_COMMITTING = 2; private static final int STATE_COMMITTED = 3; + private HttpStatus statusCode; + private final HttpHeaders headers; private final MultiValueMap cookies; @@ -70,6 +78,10 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { return this.dataBufferFactory; } + protected HttpStatus getStatusCode() { + return statusCode; + } + @Override public HttpHeaders getHeaders() { if (STATE_COMMITTED == this.state.get()) { @@ -106,6 +118,7 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { }); mono = mono.then(() -> { this.state.set(STATE_COMMITTED); + writeStatusCode(); writeHeaders(); writeCookies(); return Mono.empty(); @@ -114,6 +127,12 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { return mono; } + /** + * Implement this method to write the status code to the underlying response. + * This method is called once only. + */ + protected abstract void writeStatusCode(); + /** * Implement this method to apply header changes from {@link #getHeaders()} * to the underlying response. This method is called once only. @@ -143,4 +162,18 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { return applyBeforeCommit(); } + @Override + public boolean setStatusCode(HttpStatus statusCode) { + Assert.notNull(statusCode); + if (STATE_NEW == this.state.get()) { + this.statusCode = statusCode; + return true; + } + else if (logger.isDebugEnabled()) { + logger.debug("Can't set the status " + statusCode.toString() + + " because the HTTP response has already been committed"); + } + return false; + } + } diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java index fd2467ec09..5ca6ca3bc8 100644 --- a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpResponse.java @@ -60,11 +60,6 @@ public class ReactorServerHttpResponse extends AbstractServerHttpResponse return this.channel; } - @Override - public void setStatusCode(HttpStatus status) { - getReactorChannel().status(HttpResponseStatus.valueOf(status.value())); - } - @Override protected Mono writeWithInternal(Publisher publisher) { return Flux.from(publisher) @@ -75,6 +70,14 @@ public class ReactorServerHttpResponse extends AbstractServerHttpResponse .then(); } + @Override + protected void writeStatusCode() { + HttpStatus statusCode = this.getStatusCode(); + if (statusCode != null) { + getReactorChannel().status(HttpResponseStatus.valueOf(statusCode.value())); + } + } + @Override protected void writeHeaders() { for (String name : getHeaders().keySet()) { diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpResponse.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpResponse.java index 61aac8cc6f..80aadad457 100644 --- a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpResponse.java +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/RxNettyServerHttpResponse.java @@ -59,11 +59,6 @@ public class RxNettyServerHttpResponse extends AbstractServerHttpResponse { return this.response; } - @Override - public void setStatusCode(HttpStatus status) { - this.response.setStatus(HttpResponseStatus.valueOf(status.value())); - } - @Override protected Mono writeWithInternal(Publisher body) { Observable content = RxJava1ObservableConverter.from(body).map(this::toByteBuf); @@ -75,6 +70,14 @@ public class RxNettyServerHttpResponse extends AbstractServerHttpResponse { return (buffer instanceof FlushingDataBuffer ? new FlushingByteBuf(byteBuf) : byteBuf); } + @Override + protected void writeStatusCode() { + HttpStatus statusCode = this.getStatusCode(); + if (statusCode != null) { + this.response.setStatus(HttpResponseStatus.valueOf(statusCode.value())); + } + } + @Override protected void writeHeaders() { for (String name : getHeaders().keySet()) { diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java index 4e641499f5..c458cfcc7e 100644 --- a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java @@ -27,14 +27,17 @@ import org.springframework.util.MultiValueMap; * Represents a reactive server-side HTTP response. * * @author Arjen Poutsma + * @author Sebastien Deleuze */ public interface ServerHttpResponse extends ReactiveHttpOutputMessage { /** * Set the HTTP status code of the response. * @param status the HTTP status as an {@link HttpStatus} enum value + * @return {@code false} if the status code has not been set because the HTTP response + * is already committed, {@code true} if it has been set correctly. */ - void setStatusCode(HttpStatus status); + boolean setStatusCode(HttpStatus status); /** * Return a mutable map with the cookies to send to the server. diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java index 926315c204..60cca26f23 100644 --- a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java @@ -63,13 +63,16 @@ public class ServletServerHttpResponse extends AbstractServerHttpResponse { } @Override - public void setStatusCode(HttpStatus status) { - getServletResponse().setStatus(status.value()); + protected Mono writeWithInternal(Publisher publisher) { + return this.responseBodyWriter.apply(publisher); } @Override - protected Mono writeWithInternal(Publisher publisher) { - return this.responseBodyWriter.apply(publisher); + protected void writeStatusCode() { + HttpStatus statusCode = this.getStatusCode(); + if (statusCode != null) { + getServletResponse().setStatus(statusCode.value()); + } } @Override diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java index 0dfb54f26b..73fba1d28a 100644 --- a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpResponse.java @@ -66,12 +66,6 @@ public class UndertowServerHttpResponse extends AbstractServerHttpResponse return this.exchange; } - @Override - public void setStatusCode(HttpStatus status) { - Assert.notNull(status); - getUndertowExchange().setStatusCode(status.value()); - } - @Override protected Mono writeWithInternal(Publisher publisher) { return Mono.from(s -> { @@ -107,6 +101,14 @@ public class UndertowServerHttpResponse extends AbstractServerHttpResponse } } + @Override + protected void writeStatusCode() { + HttpStatus statusCode = this.getStatusCode(); + if (statusCode != null) { + getUndertowExchange().setStatusCode(statusCode.value()); + } + } + @Override protected void writeHeaders() { for (Map.Entry> entry : getHeaders().entrySet()) { diff --git a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java index 30dbda1681..c12ba69f94 100644 --- a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java +++ b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java @@ -49,8 +49,9 @@ public class MockServerHttpResponse implements ServerHttpResponse { @Override - public void setStatusCode(HttpStatus status) { + public boolean setStatusCode(HttpStatus status) { this.status = status; + return true; } public HttpStatus getStatus() { diff --git a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java index 6eff9434bf..a5e5637605 100644 --- a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java @@ -28,7 +28,6 @@ import reactor.core.publisher.Mono; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DefaultDataBufferFactory; -import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; import static junit.framework.TestCase.assertTrue; @@ -36,6 +35,7 @@ import static org.junit.Assert.*; /** * @author Rossen Stoyanchev + * @author Sebastien Deleuze */ public class ServerHttpResponseTests { @@ -47,6 +47,7 @@ public class ServerHttpResponseTests { TestServerHttpResponse response = new TestServerHttpResponse(); response.writeWith(Flux.just(wrap("a"), wrap("b"), wrap("c"))).block(); + assertTrue(response.statusCodeWritten); assertTrue(response.headersWritten); assertTrue(response.cookiesWritten); @@ -62,6 +63,7 @@ public class ServerHttpResponseTests { IllegalStateException error = new IllegalStateException("boo"); response.writeWith(Flux.error(error)).otherwise(ex -> Mono.empty()).block(); + assertFalse(response.statusCodeWritten); assertFalse(response.headersWritten); assertFalse(response.cookiesWritten); assertTrue(response.body.isEmpty()); @@ -72,6 +74,7 @@ public class ServerHttpResponseTests { TestServerHttpResponse response = new TestServerHttpResponse(); response.setComplete().block(); + assertTrue(response.statusCodeWritten); assertTrue(response.headersWritten); assertTrue(response.cookiesWritten); assertTrue(response.body.isEmpty()); @@ -87,6 +90,7 @@ public class ServerHttpResponseTests { }); response.writeWith(Flux.just(wrap("a"), wrap("b"), wrap("c"))).block(); + assertTrue(response.statusCodeWritten); assertTrue(response.headersWritten); assertTrue(response.cookiesWritten); assertSame(cookie, response.getCookies().getFirst("ID")); @@ -104,6 +108,7 @@ public class ServerHttpResponseTests { response.beforeCommit(() -> Mono.error(error)); response.writeWith(Flux.just(wrap("a"), wrap("b"), wrap("c"))).block(); + assertTrue("beforeCommit action errors should be ignored", response.statusCodeWritten); assertTrue("beforeCommit action errors should be ignored", response.headersWritten); assertTrue("beforeCommit action errors should be ignored", response.cookiesWritten); assertNull(response.getCookies().get("ID")); @@ -124,6 +129,7 @@ public class ServerHttpResponseTests { }); response.setComplete().block(); + assertTrue(response.statusCodeWritten); assertTrue(response.headersWritten); assertTrue(response.cookiesWritten); assertTrue(response.body.isEmpty()); @@ -139,6 +145,8 @@ public class ServerHttpResponseTests { private static class TestServerHttpResponse extends AbstractServerHttpResponse { + private boolean statusCodeWritten; + private boolean headersWritten; private boolean cookiesWritten; @@ -150,7 +158,9 @@ public class ServerHttpResponseTests { } @Override - public void setStatusCode(HttpStatus status) { + public void writeStatusCode() { + assertFalse(this.statusCodeWritten); + this.statusCodeWritten = true; } @Override