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.

spring-web-reactive/src/main/java/org/springframework/http/ExtendedHttpHeaders.java

@@ -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.
- *
- * <p>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<HeaderChangeListener> 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<String> values = new LinkedList<>();
-		values.add(value);
-		put(name, values);
-	}
-
-	@Override
-	public List<String> put(String key, List<String> values) {
-		for (HeaderChangeListener listener : this.listeners) {
-			listener.headerPut(key, values);
-		}
-		return super.put(key, values);
-	}
-
-	@Override
-	public List<String> remove(Object key) {
-		for (HeaderChangeListener listener : this.listeners) {
-			listener.headerRemoved((String) key);
-		}
-		return super.remove(key);
-	}
-
-	@Override
-	public void putAll(Map<? extends String, ? extends List<String>> map) {
-		for (Entry<? extends String, ? extends List<String>> entry : map.entrySet()) {
-			put(entry.getKey(), entry.getValue());
-		}
-		super.putAll(map);
-	}
-
-	@Override
-	public void clear() {
-		for (Entry<? extends String, ? extends List<String>> entry : super.entrySet()) {
-			remove(entry.getKey(), entry.getValue());
-		}
-		super.clear();
-	}
-
-
-	public interface HeaderChangeListener {
-
-		void headerAdded(String name, String value);
-
-		void headerPut(String key, List<String> values);
-
-		void headerRemoved(String key);
-
-	}
-
-}

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<?, Buffer> 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<Void> setBodyInternal(Publisher<ByteBuffer> 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<String> 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;
 		}
 	}
 

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<Void> setBodyInternal(Publisher<ByteBuffer> publisher) {
+		writeHeaders();
 		Observable<byte[]> content = RxJava1Converter.from(publisher).map(this::toBytes);
 		Observable<Void> 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<String> 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;
 		}
 	}
 
-}
+}

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.
+	 *
+	 * <p><strong>Note:</strong> After this method is called,
+	 * {@link #getHeaders() headers} become read-only and any additional calls
+	 * to this method are ignored.
+	 */
+	void writeHeaders();
+
 }

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<Publisher<ByteBuffer>, Mono<Void>> 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<Void> setBodyInternal(Publisher<ByteBuffer> 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<String> 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<String, List<String>> 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;
 		}
 	}
 
-}
+}

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<Publisher<ByteBuffer>, Mono<Void>> 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<Void> setBodyInternal(Publisher<ByteBuffer> 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<String> 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<String, List<String>> entry : this.headers.entrySet()) {
+				HttpString headerName = HttpString.tryFromString(entry.getKey());
+				this.exchange.getResponseHeaders().addAll(headerName, entry.getValue());
+			}
+			this.headersWritten = true;
 		}
 	}
 

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<Void> 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) {

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() {
+	}
+
 }

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;