From eedc90818f5b42d43d3486d37e5831ed20712274 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Sun, 10 Jan 2016 06:25:12 -0500 Subject: [PATCH] Re-introduce writeHeaders() in ServerHttpResponse This commit brings back the writeHeaders method on ServerHttpResponse that was once added (2a6a4f) and then removed (9c7151). This version is a little simpler since writeHeaders doesn't explicitly flush/send headers which runtimes are expected to do by default. Instead the main purpose of writeHeaders now is to ensure changes made via HttpHeaders are applied to the underlying runtime response at some point and we now do that once at the very end. This approach provides the most flexibility (vs keeping HttpHeaders in sync) because it allows a full and consistent set of mutative operations for both headers and cookies (to be added) regardless of the API exposed by the underlying runtime. --- .../http/ExtendedHttpHeaders.java | 103 ------------------ .../reactive/ReactorServerHttpResponse.java | 35 +++--- .../reactive/RxNettyServerHttpResponse.java | 36 +++--- .../server/reactive/ServerHttpResponse.java | 16 ++- .../reactive/ServletServerHttpResponse.java | 48 ++++---- .../reactive/UndertowServerHttpResponse.java | 35 +++--- .../web/server/WebToHttpHandlerAdapter.java | 2 +- .../reactive/MockServerHttpResponse.java | 4 + ...mpleUrlHandlerMappingIntegrationTests.java | 2 - 9 files changed, 87 insertions(+), 194 deletions(-) delete mode 100644 spring-web-reactive/src/main/java/org/springframework/http/ExtendedHttpHeaders.java diff --git a/spring-web-reactive/src/main/java/org/springframework/http/ExtendedHttpHeaders.java b/spring-web-reactive/src/main/java/org/springframework/http/ExtendedHttpHeaders.java deleted file mode 100644 index f5fa19922bf..00000000000 --- a/spring-web-reactive/src/main/java/org/springframework/http/ExtendedHttpHeaders.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright 2002-2015 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.http; - -import java.util.ArrayList; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; - -/** - * Variant of HttpHeaders (to be merged into HttpHeaders) that supports the - * registration of {@link HeaderChangeListener}s. - * - *

For use with HTTP server response implementations that wish to propagate - * header header changes to the underlying runtime as they occur. - * - * @author Rossen Stoyanchev - */ -public class ExtendedHttpHeaders extends HttpHeaders { - - private final List listeners = new ArrayList<>(1); - - - public ExtendedHttpHeaders() { - } - - public ExtendedHttpHeaders(HeaderChangeListener listener) { - this.listeners.add(listener); - } - - - @Override - public void add(String name, String value) { - for (HeaderChangeListener listener : this.listeners) { - listener.headerAdded(name, value); - } - super.add(name, value); - } - - @Override - public void set(String name, String value) { - List values = new LinkedList<>(); - values.add(value); - put(name, values); - } - - @Override - public List put(String key, List values) { - for (HeaderChangeListener listener : this.listeners) { - listener.headerPut(key, values); - } - return super.put(key, values); - } - - @Override - public List remove(Object key) { - for (HeaderChangeListener listener : this.listeners) { - listener.headerRemoved((String) key); - } - return super.remove(key); - } - - @Override - public void putAll(Map> map) { - for (Entry> entry : map.entrySet()) { - put(entry.getKey(), entry.getValue()); - } - super.putAll(map); - } - - @Override - public void clear() { - for (Entry> entry : super.entrySet()) { - remove(entry.getKey(), entry.getValue()); - } - super.clear(); - } - - - public interface HeaderChangeListener { - - void headerAdded(String name, String value); - - void headerPut(String key, List values); - - void headerRemoved(String key); - - } - -} 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 58a8947440e..f1c7af00ef6 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 @@ -16,7 +16,6 @@ package org.springframework.http.server.reactive; import java.nio.ByteBuffer; -import java.util.List; import org.reactivestreams.Publisher; import reactor.Flux; @@ -25,7 +24,6 @@ import reactor.io.buffer.Buffer; import reactor.io.net.http.HttpChannel; import reactor.io.net.http.model.Status; -import org.springframework.http.ExtendedHttpHeaders; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.util.Assert; @@ -42,11 +40,13 @@ public class ReactorServerHttpResponse implements ServerHttpResponse { private final HttpHeaders headers; + private boolean headersWritten = false; + public ReactorServerHttpResponse(HttpChannel response) { Assert.notNull("'response' must not be null."); this.channel = response; - this.headers = new ExtendedHttpHeaders(new ReactorHeaderChangeListener()); + this.headers = new HttpHeaders(); } @@ -61,7 +61,7 @@ public class ReactorServerHttpResponse implements ServerHttpResponse { @Override public HttpHeaders getHeaders() { - return this.headers; + return (this.headersWritten ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers); } @Override @@ -70,26 +70,19 @@ public class ReactorServerHttpResponse implements ServerHttpResponse { } protected Mono setBodyInternal(Publisher publisher) { + writeHeaders(); return Mono.from(getReactorChannel().writeWith(Flux.from(publisher).map(Buffer::new))); } - - private class ReactorHeaderChangeListener implements ExtendedHttpHeaders.HeaderChangeListener { - - @Override - public void headerAdded(String name, String value) { - getReactorChannel().responseHeaders().add(name, value); - } - - @Override - public void headerPut(String key, List values) { - getReactorChannel().responseHeaders().remove(key); - getReactorChannel().responseHeaders().add(key, values); - } - - @Override - public void headerRemoved(String key) { - getReactorChannel().responseHeaders().remove(key); + @Override + public void writeHeaders() { + if (!this.headersWritten) { + for (String name : this.headers.keySet()) { + for (String value : this.headers.get(name)) { + this.channel.responseHeaders().add(name, value); + } + } + this.headersWritten = true; } } 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 05ed55e8c3d..c60eaaded8d 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 @@ -17,7 +17,6 @@ package org.springframework.http.server.reactive; import java.nio.ByteBuffer; -import java.util.List; import io.netty.handler.codec.http.HttpResponseStatus; import io.reactivex.netty.protocol.http.server.HttpServerResponse; @@ -27,7 +26,6 @@ import reactor.Mono; import reactor.core.publisher.convert.RxJava1Converter; import rx.Observable; -import org.springframework.http.ExtendedHttpHeaders; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.util.Assert; @@ -44,11 +42,13 @@ public class RxNettyServerHttpResponse implements ServerHttpResponse { private final HttpHeaders headers; + private boolean headersWritten = false; + public RxNettyServerHttpResponse(HttpServerResponse response) { Assert.notNull("'response', response must not be null."); this.response = response; - this.headers = new ExtendedHttpHeaders(new RxNettyHeaderChangeListener()); + this.headers = new HttpHeaders(); } @@ -63,7 +63,7 @@ public class RxNettyServerHttpResponse implements ServerHttpResponse { @Override public HttpHeaders getHeaders() { - return this.headers; + return (this.headersWritten ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers); } @Override @@ -72,6 +72,7 @@ public class RxNettyServerHttpResponse implements ServerHttpResponse { } protected Mono setBodyInternal(Publisher publisher) { + writeHeaders(); Observable content = RxJava1Converter.from(publisher).map(this::toBytes); Observable completion = getRxNettyResponse().writeBytes(content); return RxJava1Converter.from(completion).after(); @@ -83,26 +84,15 @@ public class RxNettyServerHttpResponse implements ServerHttpResponse { return bytes; } - - private class RxNettyHeaderChangeListener implements ExtendedHttpHeaders.HeaderChangeListener { - - @Override - public void headerAdded(String name, String value) { - getRxNettyResponse().addHeader(name, value); - } - - @Override - public void headerPut(String key, List values) { - getRxNettyResponse().removeHeader(key); - for (String value : values) { - getRxNettyResponse().addHeader(key, value); + @Override + public void writeHeaders() { + if (!this.headersWritten) { + for (String name : this.headers.keySet()) { + for (String value : this.headers.get(name)) + this.response.addHeader(name, value); } - } - - @Override - public void headerRemoved(String key) { - getRxNettyResponse().removeHeader(key); + this.headersWritten = true; } } -} +} \ No newline at end of file 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 4b31cf19c39..82320e98584 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 @@ -16,6 +16,8 @@ package org.springframework.http.server.reactive; +import org.reactivestreams.Publisher; + import org.springframework.http.HttpStatus; import org.springframework.http.ReactiveHttpOutputMessage; @@ -31,5 +33,17 @@ public interface ServerHttpResponse extends ReactiveHttpOutputMessage { * @param status the HTTP status as an {@link HttpStatus} enum value */ void setStatusCode(HttpStatus status); - + + /** + * Use this method to apply header changes made via {@link #getHeaders()} to + * the underlying server response. By default changes made via + * {@link #getHeaders()} are cached until a call to {@link #setBody} + * implicitly applies header changes or until this method is called. + * + *

Note: After this method is called, + * {@link #getHeaders() headers} become read-only and any additional calls + * to this method are ignored. + */ + void writeHeaders(); + } 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 0fc838af418..27b67989554 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 @@ -17,7 +17,9 @@ package org.springframework.http.server.reactive; import java.nio.ByteBuffer; +import java.nio.charset.Charset; import java.util.List; +import java.util.Map; import java.util.function.Function; import javax.servlet.http.HttpServletResponse; @@ -25,9 +27,9 @@ import org.reactivestreams.Publisher; import reactor.Flux; import reactor.Mono; -import org.springframework.http.ExtendedHttpHeaders; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.util.Assert; /** @@ -43,6 +45,8 @@ public class ServletServerHttpResponse implements ServerHttpResponse { private final HttpHeaders headers; + private boolean headersWritten = false; + public ServletServerHttpResponse(HttpServletResponse response, Function, Mono> responseBodyWriter) { @@ -51,7 +55,7 @@ public class ServletServerHttpResponse implements ServerHttpResponse { Assert.notNull(responseBodyWriter, "'responseBodyWriter' must not be null"); this.response = response; this.responseBodyWriter = responseBodyWriter; - this.headers = new ExtendedHttpHeaders(new ServletHeaderChangeListener()); + this.headers = new HttpHeaders(); } @@ -66,7 +70,7 @@ public class ServletServerHttpResponse implements ServerHttpResponse { @Override public HttpHeaders getHeaders() { - return this.headers; + return (this.headersWritten ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers); } @Override @@ -75,29 +79,29 @@ public class ServletServerHttpResponse implements ServerHttpResponse { } protected Mono setBodyInternal(Publisher publisher) { + writeHeaders(); return this.responseBodyWriter.apply(publisher); } - - private class ServletHeaderChangeListener implements ExtendedHttpHeaders.HeaderChangeListener { - - @Override - public void headerAdded(String name, String value) { - getServletResponse().addHeader(name, value); - } - - @Override - public void headerPut(String key, List values) { - // We can only add but not remove - for (String value : values) { - getServletResponse().addHeader(key, value); + @Override + public void writeHeaders() { + if (!this.headersWritten) { + for (Map.Entry> entry : this.headers.entrySet()) { + String headerName = entry.getKey(); + for (String headerValue : entry.getValue()) { + this.response.addHeader(headerName, headerValue); + } } - } - - @Override - public void headerRemoved(String key) { - // No Servlet support for removing headers + MediaType contentType = this.headers.getContentType(); + if (this.response.getContentType() == null && contentType != null) { + this.response.setContentType(contentType.toString()); + } + Charset charset = (contentType != null ? contentType.getCharSet() : null); + if (this.response.getCharacterEncoding() == null && charset != null) { + this.response.setCharacterEncoding(charset.name()); + } + this.headersWritten = true; } } -} +} \ No newline at end of file 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 2d303c4cd02..08032cf1e23 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 @@ -18,6 +18,7 @@ package org.springframework.http.server.reactive; import java.nio.ByteBuffer; import java.util.List; +import java.util.Map; import java.util.function.Function; import io.undertow.server.HttpServerExchange; @@ -26,7 +27,6 @@ import org.reactivestreams.Publisher; import reactor.Flux; import reactor.Mono; -import org.springframework.http.ExtendedHttpHeaders; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.util.Assert; @@ -45,6 +45,8 @@ public class UndertowServerHttpResponse implements ServerHttpResponse { private final HttpHeaders headers; + private boolean headersWritten = false; + public UndertowServerHttpResponse(HttpServerExchange exchange, Function, Mono> responseBodyWriter) { @@ -53,7 +55,7 @@ public class UndertowServerHttpResponse implements ServerHttpResponse { Assert.notNull(responseBodyWriter, "'responseBodyWriter' must not be null"); this.exchange = exchange; this.responseBodyWriter = responseBodyWriter; - this.headers = new ExtendedHttpHeaders(new UndertowHeaderChangeListener()); + this.headers = new HttpHeaders(); } @@ -69,7 +71,7 @@ public class UndertowServerHttpResponse implements ServerHttpResponse { @Override public HttpHeaders getHeaders() { - return this.headers; + return (this.headersWritten ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers); } @Override @@ -78,27 +80,18 @@ public class UndertowServerHttpResponse implements ServerHttpResponse { } protected Mono setBodyInternal(Publisher publisher) { + writeHeaders(); return this.responseBodyWriter.apply(publisher); } - - private class UndertowHeaderChangeListener implements ExtendedHttpHeaders.HeaderChangeListener { - - @Override - public void headerAdded(String name, String value) { - HttpString headerName = HttpString.tryFromString(name); - getUndertowExchange().getResponseHeaders().add(headerName, value); - } - - @Override - public void headerPut(String key, List values) { - HttpString headerName = HttpString.tryFromString(key); - getUndertowExchange().getResponseHeaders().putAll(headerName, values); - } - - @Override - public void headerRemoved(String key) { - getUndertowExchange().getResponseHeaders().remove(key); + @Override + public void writeHeaders() { + if (!this.headersWritten) { + for (Map.Entry> entry : this.headers.entrySet()) { + HttpString headerName = HttpString.tryFromString(entry.getKey()); + this.exchange.getResponseHeaders().addAll(headerName, entry.getValue()); + } + this.headersWritten = true; } } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java b/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java index b2d45e7b6f4..aace5d7885d 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java @@ -37,7 +37,7 @@ public class WebToHttpHandlerAdapter extends WebHandlerDecorator implements Http @Override public Mono handle(ServerHttpRequest request, ServerHttpResponse response) { WebServerExchange exchange = createWebServerExchange(request, response); - return getDelegate().handle(exchange); + return getDelegate().handle(exchange).doOnTerminate((aVoid, ex) -> response.writeHeaders()); } protected WebServerExchange createWebServerExchange(ServerHttpRequest request, ServerHttpResponse response) { 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 0a282a95b34..0aab9caaf3a 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 @@ -60,4 +60,8 @@ public class MockServerHttpResponse implements ServerHttpResponse { return this.body; } + @Override + public void writeHeaders() { + } + } diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingIntegrationTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingIntegrationTests.java index 8bd43ba8309..dfc0fb1a563 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingIntegrationTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingIntegrationTests.java @@ -32,8 +32,6 @@ import org.springframework.http.RequestEntity; import org.springframework.http.ResponseEntity; import org.springframework.http.server.reactive.AbstractHttpHandlerIntegrationTests; import org.springframework.http.server.reactive.HttpHandler; -import org.springframework.http.server.reactive.ServerHttpRequest; -import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; import org.springframework.web.reactive.DispatcherHandler;