From 638e6cc7f8f817b0271711a5ee48dbba8b58a7e0 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 1 Sep 2016 15:58:46 -0400 Subject: [PATCH] Add checkNotModified support for ResponseEntity Issue: SPR-14522 --- .../ResponseEntityResultHandler.java | 13 ++ .../ResponseEntityResultHandlerTests.java | 116 ++++++++++++++++-- 2 files changed, 120 insertions(+), 9 deletions(-) diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java index 9f8aace5be..683e8a3a1c 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java @@ -15,6 +15,8 @@ */ package org.springframework.web.reactive.result.method.annotation; +import java.time.Instant; +import java.util.Arrays; import java.util.List; import java.util.Optional; @@ -26,6 +28,7 @@ import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ResolvableType; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.http.RequestEntity; import org.springframework.http.ResponseEntity; import org.springframework.http.codec.HttpMessageWriter; @@ -47,6 +50,8 @@ import org.springframework.web.server.ServerWebExchange; public class ResponseEntityResultHandler extends AbstractMessageWriterResultHandler implements HandlerResultHandler { + private static final List SAFE_METHODS = Arrays.asList(HttpMethod.GET, HttpMethod.HEAD); + /** * Constructor with {@link HttpMessageWriter}s and a @@ -145,6 +150,14 @@ public class ResponseEntityResultHandler extends AbstractMessageWriterResultHand .forEach(entry -> responseHeaders.put(entry.getKey(), entry.getValue())); } + String etag = entityHeaders.getETag(); + Instant lastModified = Instant.ofEpochMilli(entityHeaders.getLastModified()); + HttpMethod httpMethod = exchange.getRequest().getMethod(); + if (SAFE_METHODS.contains(httpMethod) && exchange.checkNotModified(etag, lastModified)) { + exchange.getResponse().setComplete(); + return Mono.empty(); + } + return writeBody(httpEntity.getBody(), bodyType, exchange); }); } diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java index 40fe1fb54d..bb19b22ffc 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java @@ -19,6 +19,8 @@ package org.springframework.web.reactive.result.method.annotation; import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -35,6 +37,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.codec.ByteBufferEncoder; import org.springframework.core.codec.CharSequenceEncoder; import org.springframework.core.io.buffer.support.DataBufferTestUtils; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -45,7 +48,6 @@ import org.springframework.http.codec.json.Jackson2JsonEncoder; import org.springframework.http.codec.xml.Jaxb2XmlEncoder; import org.springframework.http.server.reactive.MockServerHttpRequest; import org.springframework.http.server.reactive.MockServerHttpResponse; -import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.tests.TestSubscriber; import org.springframework.util.ObjectUtils; import org.springframework.web.reactive.HandlerResult; @@ -55,9 +57,14 @@ import org.springframework.web.reactive.result.ResolvableMethod; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; import org.springframework.web.server.session.MockWebSessionManager; +import org.springframework.web.server.session.WebSessionManager; -import static org.junit.Assert.*; -import static org.springframework.core.ResolvableType.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.springframework.core.ResolvableType.forClassWithGenerics; +import static org.springframework.http.ResponseEntity.ok; /** * Unit tests for {@link ResponseEntityResultHandler}. When adding a test also @@ -72,6 +79,8 @@ public class ResponseEntityResultHandlerTests { private ResponseEntityResultHandler resultHandler; + private MockServerHttpRequest request; + private MockServerHttpResponse response = new MockServerHttpResponse(); private ServerWebExchange exchange; @@ -80,8 +89,9 @@ public class ResponseEntityResultHandlerTests { @Before public void setUp() throws Exception { this.resultHandler = createHandler(); - ServerHttpRequest request = new MockServerHttpRequest(HttpMethod.GET, new URI("/path")); - this.exchange = new DefaultServerWebExchange(request, this.response, new MockWebSessionManager()); + this.request = new MockServerHttpRequest(HttpMethod.GET, new URI("/path")); + WebSessionManager manager = new MockWebSessionManager(); + this.exchange = new DefaultServerWebExchange(this.request, this.response, manager); } private ResponseEntityResultHandler createHandler(HttpMessageWriter... writers) { @@ -160,23 +170,93 @@ public class ResponseEntityResultHandlerTests { @Test public void handleReturnTypes() throws Exception { - Object returnValue = ResponseEntity.ok("abc"); + Object returnValue = ok("abc"); ResolvableType returnType = responseEntity(String.class); testHandle(returnValue, returnType); - returnValue = Mono.just(ResponseEntity.ok("abc")); + returnValue = Mono.just(ok("abc")); returnType = forClassWithGenerics(Mono.class, responseEntity(String.class)); testHandle(returnValue, returnType); - returnValue = Mono.just(ResponseEntity.ok("abc")); + returnValue = Mono.just(ok("abc")); returnType = forClassWithGenerics(Single.class, responseEntity(String.class)); testHandle(returnValue, returnType); - returnValue = Mono.just(ResponseEntity.ok("abc")); + returnValue = Mono.just(ok("abc")); returnType = forClassWithGenerics(CompletableFuture.class, responseEntity(String.class)); testHandle(returnValue, returnType); } + @Test + public void handleReturnValueLastModified() throws Exception { + Instant currentTime = Instant.now().truncatedTo(ChronoUnit.SECONDS); + Instant oneMinAgo = currentTime.minusSeconds(60); + this.request.getHeaders().setIfModifiedSince(currentTime.toEpochMilli()); + + ResponseEntity entity = ok().lastModified(oneMinAgo.toEpochMilli()).body("body"); + HandlerResult result = handlerResult(entity, responseEntity(String.class)); + this.resultHandler.handleResult(this.exchange, result).block(Duration.ofSeconds(5)); + + assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, null, oneMinAgo); + } + + @Test + public void handleReturnValueEtag() throws Exception { + String etagValue = "\"deadb33f8badf00d\""; + this.request.getHeaders().setIfNoneMatch(etagValue); + + ResponseEntity entity = ok().eTag(etagValue).body("body"); + HandlerResult result = handlerResult(entity, responseEntity(String.class)); + this.resultHandler.handleResult(this.exchange, result).block(Duration.ofSeconds(5)); + + assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, etagValue, Instant.MIN); + } + + @Test // SPR-14559 + public void handleReturnValueEtagInvalidIfNoneMatch() throws Exception { + this.request.getHeaders().setIfNoneMatch("unquoted"); + + ResponseEntity entity = ok().eTag("\"deadb33f8badf00d\"").body("body"); + HandlerResult result = handlerResult(entity, responseEntity(String.class)); + this.resultHandler.handleResult(this.exchange, result).block(Duration.ofSeconds(5)); + + assertEquals(HttpStatus.OK, this.response.getStatusCode()); + assertResponseBody("body"); + } + + @Test + public void handleReturnValueETagAndLastModified() throws Exception { + String eTag = "\"deadb33f8badf00d\""; + this.request.getHeaders().setIfNoneMatch(eTag); + + Instant currentTime = Instant.now().truncatedTo(ChronoUnit.SECONDS); + Instant oneMinAgo = currentTime.minusSeconds(60); + this.request.getHeaders().setIfModifiedSince(currentTime.toEpochMilli()); + + ResponseEntity entity = ok().eTag(eTag).lastModified(oneMinAgo.toEpochMilli()).body("body"); + HandlerResult result = handlerResult(entity, responseEntity(String.class)); + this.resultHandler.handleResult(this.exchange, result).block(Duration.ofSeconds(5)); + + assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, eTag, oneMinAgo); + } + + @Test + public void handleReturnValueChangedETagAndLastModified() throws Exception { + String etag = "\"deadb33f8badf00d\""; + String newEtag = "\"changed-etag-value\""; + this.request.getHeaders().setIfNoneMatch(etag); + + Instant currentTime = Instant.now().truncatedTo(ChronoUnit.SECONDS); + Instant oneMinAgo = currentTime.minusSeconds(60); + this.request.getHeaders().setIfModifiedSince(currentTime.toEpochMilli()); + + ResponseEntity entity = ok().eTag(newEtag).lastModified(oneMinAgo.toEpochMilli()).body("body"); + HandlerResult result = handlerResult(entity, responseEntity(String.class)); + this.resultHandler.handleResult(this.exchange, result).block(Duration.ofSeconds(5)); + + assertConditionalResponse(HttpStatus.OK, "body", newEtag, oneMinAgo); + } + private void testHandle(Object returnValue, ResolvableType type) { HandlerResult result = handlerResult(returnValue, type); @@ -203,6 +283,24 @@ public class ResponseEntityResultHandlerTests { DataBufferTestUtils.dumpString(buf, StandardCharsets.UTF_8))); } + private void assertConditionalResponse(HttpStatus status, String body, String etag, Instant lastModified) { + assertEquals(status, this.response.getStatusCode()); + if (body != null) { + assertResponseBody(body); + } + else { + assertNull(this.response.getBody()); + } + if (etag != null) { + assertEquals(1, this.response.getHeaders().get(HttpHeaders.ETAG).size()); + assertEquals(etag, this.response.getHeaders().getETag()); + } + if (lastModified.isAfter(Instant.EPOCH)) { + assertEquals(1, this.response.getHeaders().get(HttpHeaders.LAST_MODIFIED).size()); + assertEquals(lastModified.toEpochMilli(), this.response.getHeaders().getLastModified()); + } + } + @SuppressWarnings("unused") private static class TestController {