Set 304 status on ServerResponse when ETag/LastModified match

This commit checks the Etag/LastModified headers on the incoming
request, and sets a 304 Not Modified status with no body when they
match, by delegating to ServerWebExchange.checkNotModified.

Issue: SPR-16348

spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultEntityResponseBuilder.java

@@ -224,10 +224,8 @@ class DefaultEntityResponseBuilder<T> implements EntityResponse.Builder<T> {
 		}
 
 		@Override
-		public Mono<Void> writeTo(ServerWebExchange exchange, Context context) {
+		protected Mono<Void> writeToInternal(ServerWebExchange exchange, Context context) {
-			ServerHttpResponse response = exchange.getResponse();
+			return inserter().insert(exchange.getResponse(), new BodyInserter.Context() {
-			writeStatusAndHeaders(response);
-			return inserter().insert(response, new BodyInserter.Context() {
 				@Override
 				public List<HttpMessageWriter<?>> messageWriters() {
 					return context.messageWriters();

spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultRenderingResponseBuilder.java

@@ -35,7 +35,6 @@ import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseCookie;
-import org.springframework.http.server.reactive.ServerHttpResponse;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.LinkedMultiValueMap;
@@ -184,9 +183,7 @@ class DefaultRenderingResponseBuilder implements RenderingResponse.Builder {
 		}
 
 		@Override
-		public Mono<Void> writeTo(ServerWebExchange exchange, Context context) {
+		protected Mono<Void> writeToInternal(ServerWebExchange exchange, Context context) {
-			ServerHttpResponse response = exchange.getResponse();
-			writeStatusAndHeaders(response);
 			MediaType responseContentType = exchange.getResponse().getHeaders().getContentType();
 			Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext());
 			Stream<ViewResolver> viewResolverStream = context.viewResolvers().stream();

spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java

@@ -17,8 +17,10 @@
 package org.springframework.web.reactive.function.server;
 
 import java.net.URI;
+import java.time.Instant;
 import java.time.ZonedDateTime;
 import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -278,6 +280,8 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder {
 
 	static abstract class AbstractServerResponse implements ServerResponse {
 
+		private static final Set<HttpMethod> SAFE_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD);
+
 		final int statusCode;
 
 		private final HttpHeaders headers;
@@ -319,7 +323,21 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder {
 			return this.cookies;
 		}
 
-		protected void writeStatusAndHeaders(ServerHttpResponse response) {
+		@Override
+		public final Mono<Void> writeTo(ServerWebExchange exchange, Context context) {
+			writeStatusAndHeaders(exchange.getResponse());
+
+			Instant lastModified = Instant.ofEpochMilli(headers().getLastModified());
+			HttpMethod httpMethod = exchange.getRequest().getMethod();
+			if (SAFE_METHODS.contains(httpMethod) && exchange.checkNotModified(headers().getETag(), lastModified)) {
+				return exchange.getResponse().setComplete();
+			}
+			else {
+				return writeToInternal(exchange, context);
+			}
+		}
+
+		private void writeStatusAndHeaders(ServerHttpResponse response) {
 			if (response instanceof AbstractServerHttpResponse) {
 				((AbstractServerHttpResponse) response).setStatusCodeValue(this.statusCode);
 			}
@@ -335,6 +353,8 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder {
 			copy(this.cookies, response.getCookies());
 		}
 
+		protected abstract Mono<Void> writeToInternal(ServerWebExchange exchange, Context context);
+
 		private static <K,V> void copy(MultiValueMap<K,V> src, MultiValueMap<K,V> dst) {
 			if (!src.isEmpty()) {
 				src.entrySet().stream()
@@ -358,8 +378,7 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder {
 		}
 
 		@Override
-		public Mono<Void> writeTo(ServerWebExchange exchange, Context context) {
+		protected Mono<Void> writeToInternal(ServerWebExchange exchange, Context context) {
-			writeStatusAndHeaders(exchange.getResponse());
 			return this.writeFunction.apply(exchange, context);
 		}
 	}
@@ -381,10 +400,8 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder {
 		}
 
 		@Override
-		public Mono<Void> writeTo(ServerWebExchange exchange, Context context) {
+		protected Mono<Void> writeToInternal(ServerWebExchange exchange, Context context) {
-			ServerHttpResponse response = exchange.getResponse();
+			return this.inserter.insert(exchange.getResponse(), new BodyInserter.Context() {
-			writeStatusAndHeaders(response);
-			return this.inserter.insert(response, new BodyInserter.Context() {
 				@Override
 				public List<HttpMessageWriter<?>> messageWriters() {
 					return context.messageWriters();

spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultEntityResponseBuilderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -18,6 +18,8 @@ package org.springframework.web.reactive.function.server;
 
 import java.nio.ByteBuffer;
 import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
@@ -44,6 +46,7 @@ import org.springframework.http.codec.EncoderHttpMessageWriter;
 import org.springframework.http.codec.HttpMessageWriter;
 import org.springframework.http.server.reactive.ServerHttpResponse;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
+import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
@@ -232,4 +235,52 @@ public class DefaultEntityResponseBuilderTests {
 		assertNotNull(exchange.getResponse().getBody());
 	}
 
+	@Test
+	public void notModifiedEtag() {
+		String etag = "\"foo\"";
+		EntityResponse<String> responseMono = EntityResponse.fromObject("bar")
+				.eTag(etag)
+				.build()
+				.block();
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com")
+				.header(HttpHeaders.IF_NONE_MATCH, etag)
+				.build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+
+		responseMono.writeTo(exchange, DefaultServerResponseBuilderTests.EMPTY_CONTEXT);
+
+		MockServerHttpResponse response = exchange.getResponse();
+		assertEquals(HttpStatus.NOT_MODIFIED, response.getStatusCode());
+		StepVerifier.create(response.getBody())
+				.expectError(IllegalStateException.class)
+				.verify();
+	}
+
+
+	@Test
+	public void notModifiedLastModified() {
+		ZonedDateTime now = ZonedDateTime.now();
+		ZonedDateTime oneMinuteBeforeNow = now.minus(1, ChronoUnit.MINUTES);
+
+		EntityResponse<String> responseMono = EntityResponse.fromObject("bar")
+				.lastModified(oneMinuteBeforeNow)
+				.build()
+				.block();
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com")
+				.header(HttpHeaders.IF_MODIFIED_SINCE,
+						DateTimeFormatter.RFC_1123_DATE_TIME.format(now))
+				.build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+
+		responseMono.writeTo(exchange, DefaultServerResponseBuilderTests.EMPTY_CONTEXT);
+
+		MockServerHttpResponse response = exchange.getResponse();
+		assertEquals(HttpStatus.NOT_MODIFIED, response.getStatusCode());
+		StepVerifier.create(response.getBody())
+				.expectError(IllegalStateException.class)
+				.verify();
+	}
+
 }

spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultRenderingResponseTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -16,6 +16,9 @@
 
 package org.springframework.web.reactive.function.server;
 
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -28,9 +31,11 @@ import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseCookie;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
+import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
@@ -40,7 +45,7 @@ import org.springframework.web.reactive.result.view.ViewResolver;
 import org.springframework.web.reactive.result.view.ViewResolverSupport;
 import org.springframework.web.server.ServerWebExchange;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
 /**
@@ -181,4 +186,52 @@ public class DefaultRenderingResponseTests {
 
 	}
 
+	@Test
+	public void notModifiedEtag() {
+		String etag = "\"foo\"";
+		RenderingResponse responseMono = RenderingResponse.create("bar")
+				.header(HttpHeaders.ETAG, etag)
+				.build()
+				.block();
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com")
+				.header(HttpHeaders.IF_NONE_MATCH, etag)
+				.build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+
+		responseMono.writeTo(exchange, DefaultServerResponseBuilderTests.EMPTY_CONTEXT);
+
+		MockServerHttpResponse response = exchange.getResponse();
+		assertEquals(HttpStatus.NOT_MODIFIED, response.getStatusCode());
+		StepVerifier.create(response.getBody())
+				.expectError(IllegalStateException.class)
+				.verify();
+	}
+
+	@Test
+	public void notModifiedLastModified() {
+		ZonedDateTime now = ZonedDateTime.now();
+		ZonedDateTime oneMinuteBeforeNow = now.minus(1, ChronoUnit.MINUTES);
+
+		RenderingResponse responseMono = RenderingResponse.create("bar")
+				.header(HttpHeaders.LAST_MODIFIED, DateTimeFormatter.RFC_1123_DATE_TIME.format(oneMinuteBeforeNow))
+				.build()
+				.block();
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com")
+				.header(HttpHeaders.IF_MODIFIED_SINCE,
+						DateTimeFormatter.RFC_1123_DATE_TIME.format(now))
+				.build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+
+		responseMono.writeTo(exchange, DefaultServerResponseBuilderTests.EMPTY_CONTEXT);
+
+		MockServerHttpResponse response = exchange.getResponse();
+		assertEquals(HttpStatus.NOT_MODIFIED, response.getStatusCode());
+		StepVerifier.create(response.getBody())
+				.expectError(IllegalStateException.class)
+				.verify();
+	}
+
+
 }

spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -18,6 +18,8 @@ package org.springframework.web.reactive.function.server;
 
 import java.net.URI;
 import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
@@ -33,19 +35,33 @@ import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseCookie;
+import org.springframework.http.codec.HttpMessageWriter;
+import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
+import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
-import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.reactive.result.view.ViewResolver;
 
 import static org.junit.Assert.*;
-import static org.mockito.Mockito.*;
 
 /**
  * @author Arjen Poutsma
  */
 public class DefaultServerResponseBuilderTests {
 
+	static final ServerResponse.Context EMPTY_CONTEXT = new ServerResponse.Context() {
+		@Override
+		public List<HttpMessageWriter<?>> messageWriters() {
+			return Collections.emptyList();
+		}
+
+		@Override
+		public List<ViewResolver> viewResolvers() {
+			return Collections.emptyList();
+		}
+	};
+
 	@Test
 	public void from() throws Exception {
 		ServerResponse other = ServerResponse.ok().header("foo", "bar").build().block();
@@ -287,13 +303,12 @@ public class DefaultServerResponseBuilderTests {
 				.header("MyKey", "MyValue")
 				.cookie(cookie).build();
 
-		ServerWebExchange exchange = mock(ServerWebExchange.class);
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com").build();
-		MockServerHttpResponse response = new MockServerHttpResponse();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
-		when(exchange.getResponse()).thenReturn(response);
-		ServerResponse.Context context = mock(ServerResponse.Context.class);
 
-		result.flatMap(res -> res.writeTo(exchange, context)).block();
+		result.flatMap(res -> res.writeTo(exchange, EMPTY_CONTEXT)).block();
 
+		MockServerHttpResponse response = exchange.getResponse();
 		assertEquals(HttpStatus.CREATED, response.getStatusCode());
 		assertEquals("MyValue", response.getHeaders().getFirst("MyKey"));
 		assertEquals("value", response.getCookies().getFirst("name").getValue());
@@ -305,13 +320,12 @@ public class DefaultServerResponseBuilderTests {
 		Mono<Void> mono = Mono.empty();
 		Mono<ServerResponse> result = ServerResponse.ok().build(mono);
 
-		ServerWebExchange exchange = mock(ServerWebExchange.class);
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com").build();
-		MockServerHttpResponse response = new MockServerHttpResponse();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
-		when(exchange.getResponse()).thenReturn(response);
-		ServerResponse.Context context = mock(ServerResponse.Context.class);
 
-		result.flatMap(res -> res.writeTo(exchange, context)).block();
+		result.flatMap(res -> res.writeTo(exchange, EMPTY_CONTEXT)).block();
 
+		MockServerHttpResponse response = exchange.getResponse();
 		StepVerifier.create(response.getBody()).expectComplete().verify();
 	}
 
@@ -322,5 +336,52 @@ public class DefaultServerResponseBuilderTests {
 		ServerResponse.ok().syncBody(mono);
 	}
 
+	@Test
+	public void notModifiedEtag() {
+		String etag = "\"foo\"";
+		ServerResponse responseMono = ServerResponse.ok()
+				.eTag(etag)
+				.syncBody("bar")
+				.block();
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com")
+				.header(HttpHeaders.IF_NONE_MATCH, etag)
+				.build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+
+		responseMono.writeTo(exchange, EMPTY_CONTEXT);
+
+		MockServerHttpResponse response = exchange.getResponse();
+		assertEquals(HttpStatus.NOT_MODIFIED, response.getStatusCode());
+		StepVerifier.create(response.getBody())
+				.expectError(IllegalStateException.class)
+				.verify();
+	}
+
+	@Test
+	public void notModifiedLastModified() {
+		ZonedDateTime now = ZonedDateTime.now();
+		ZonedDateTime oneMinuteBeforeNow = now.minus(1, ChronoUnit.MINUTES);
+
+		ServerResponse responseMono = ServerResponse.ok()
+				.lastModified(oneMinuteBeforeNow)
+				.syncBody("bar")
+				.block();
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com")
+				.header(HttpHeaders.IF_MODIFIED_SINCE,
+						DateTimeFormatter.RFC_1123_DATE_TIME.format(now))
+				.build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+
+		responseMono.writeTo(exchange, EMPTY_CONTEXT);
+
+		MockServerHttpResponse response = exchange.getResponse();
+		assertEquals(HttpStatus.NOT_MODIFIED, response.getStatusCode());
+		StepVerifier.create(response.getBody())
+				.expectError(IllegalStateException.class)
+				.verify();
+	}
+
 
 }