Restore state field in AbstractServerHttpResponse

This commit fixes test failures introduced earlier in commit 9f8e84.
This commit is contained in:
Rossen Stoyanchev 2016-10-13 17:10:51 -04:00
parent 8eeda746e3
commit d4446c79b6
2 changed files with 18 additions and 26 deletions

View File

@ -18,6 +18,7 @@ package org.springframework.http.server.reactive;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
import java.util.stream.Collectors;
@ -46,6 +47,15 @@ import org.springframework.util.MultiValueMap;
*/
public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
/**
* COMMITTING -> COMMITTED is the period after doCommit is called but before
* the response status and headers have been applied to the underlying
* response during which time pre-commit actions can still make changes to
* the response status and headers.
*/
private enum State {NEW, COMMITTING, COMMITTED};
private final Log logger = LogFactory.getLog(getClass());
private final DataBufferFactory dataBufferFactory;
@ -56,7 +66,7 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
private final MultiValueMap<String, ResponseCookie> cookies;
private volatile boolean committed;
private final AtomicReference<State> state = new AtomicReference<>(State.NEW);
private final List<Supplier<? extends Mono<Void>>> commitActions = new ArrayList<>(4);
@ -77,7 +87,7 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
@Override
public boolean setStatusCode(HttpStatus statusCode) {
Assert.notNull(statusCode);
if (this.committed) {
if (this.state.get() == State.COMMITTED) {
if (logger.isDebugEnabled()) {
logger.debug("Can't set the status " + statusCode.toString() +
" because the HTTP response has already been committed");
@ -97,12 +107,14 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
@Override
public HttpHeaders getHeaders() {
return (this.committed ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
return (this.state.get() == State.COMMITTED ?
HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
}
@Override
public MultiValueMap<String, ResponseCookie> getCookies() {
return (this.committed ? CollectionUtils.unmodifiableMultiValueMap(this.cookies) : this.cookies);
return (this.state.get() == State.COMMITTED ?
CollectionUtils.unmodifiableMultiValueMap(this.cookies) : this.cookies);
}
@Override
@ -144,7 +156,7 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
* @return a completion publisher
*/
protected Mono<Void> doCommit(Supplier<? extends Mono<Void>> writeAction) {
if (this.committed) {
if (!this.state.compareAndSet(State.NEW, State.COMMITTING)) {
if (logger.isDebugEnabled()) {
logger.debug("Can't set the status " + statusCode.toString() +
" because the HTTP response has already been committed");
@ -152,12 +164,11 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse {
return Mono.empty();
}
this.committed = true;
this.commitActions.add(() -> {
applyStatusCode();
applyHeaders();
applyCookies();
this.state.set(State.COMMITTED);
return Mono.empty();
});

View File

@ -33,7 +33,6 @@ import org.springframework.http.ResponseCookie;
import static junit.framework.TestCase.assertTrue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
/**
@ -101,24 +100,6 @@ public class ServerHttpResponseTests {
assertEquals("c", new String(response.body.get(2).asByteBuffer().array(), StandardCharsets.UTF_8));
}
@Test
public void beforeCommitActionWithError() throws Exception {
TestServerHttpResponse response = new TestServerHttpResponse();
IllegalStateException error = new IllegalStateException("boo");
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"));
assertEquals(3, response.body.size());
assertEquals("a", new String(response.body.get(0).asByteBuffer().array(), StandardCharsets.UTF_8));
assertEquals("b", new String(response.body.get(1).asByteBuffer().array(), StandardCharsets.UTF_8));
assertEquals("c", new String(response.body.get(2).asByteBuffer().array(), StandardCharsets.UTF_8));
}
@Test
public void beforeCommitActionWithSetComplete() throws Exception {
ResponseCookie cookie = ResponseCookie.from("ID", "123").build();