From 010352163ba4d7f754c019912dc67cb31ef5702a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 4 Jun 2018 15:46:58 -0400 Subject: [PATCH] Eliminate the need for Encoder#getContentLength Issue: SPR-16892 --- .../core/codec/ByteArrayEncoder.java | 4 -- .../core/codec/ByteBufferEncoder.java | 4 -- .../core/codec/CharSequenceEncoder.java | 5 --- .../core/codec/DataBufferEncoder.java | 5 --- .../springframework/core/codec/Encoder.java | 11 ------ .../core/codec/ResourceEncoder.java | 15 -------- .../http/codec/EncoderHttpMessageWriter.java | 22 +++++------ .../http/codec/ResourceHttpMessageWriter.java | 23 ++++++++--- ...pingMessageConversionIntegrationTests.java | 38 ++++++++++++++----- 9 files changed, 55 insertions(+), 72 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java index 23ae1109cee..395e54c06eb 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java @@ -55,8 +55,4 @@ public class ByteArrayEncoder extends AbstractEncoder { return Flux.from(inputStream).map(bufferFactory::wrap); } - @Override - public Long getContentLength(byte[] bytes, @Nullable MimeType mimeType) { - return (long) bytes.length; - } } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java index 065c3222218..e1332fd8434 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java @@ -56,8 +56,4 @@ public class ByteBufferEncoder extends AbstractEncoder { return Flux.from(inputStream).map(bufferFactory::wrap); } - @Override - public Long getContentLength(ByteBuffer byteBuffer, @Nullable MimeType mimeType) { - return (long) byteBuffer.array().length; - } } diff --git a/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java index aa852229d00..5618dff729e 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java @@ -82,11 +82,6 @@ public class CharSequenceEncoder extends AbstractEncoder { return charset; } - @Override - public Long getContentLength(CharSequence data, @Nullable MimeType mimeType) { - return (long) data.toString().getBytes(getCharset(mimeType)).length; - } - /** * Create a {@code CharSequenceEncoder} that supports only "text/plain". */ diff --git a/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java index 5e25ddd1eaf..af4f9364b9e 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java @@ -55,9 +55,4 @@ public class DataBufferEncoder extends AbstractEncoder { return Flux.from(inputStream); } - @Override - public Long getContentLength(DataBuffer dataBuffer, @Nullable MimeType mimeType) { - return (long) dataBuffer.readableByteCount(); - } - } diff --git a/spring-core/src/main/java/org/springframework/core/codec/Encoder.java b/spring-core/src/main/java/org/springframework/core/codec/Encoder.java index 6522385b6b5..fb2d020ce50 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/Encoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/Encoder.java @@ -67,17 +67,6 @@ public interface Encoder { Flux encode(Publisher inputStream, DataBufferFactory bufferFactory, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints); - /** - * Return the length for the given item, if known. - * @param t the item to check - * @return the length in bytes, or {@code null} if not known. - * @since 5.0.5 - */ - @Nullable - default Long getContentLength(T t, @Nullable MimeType mimeType) { - return null; - } - /** * Return the list of mime types this encoder supports. */ diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java index 525fba93189..f60e0f25e0d 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java @@ -16,13 +16,11 @@ package org.springframework.core.codec; -import java.io.IOException; import java.util.Map; import reactor.core.publisher.Flux; import org.springframework.core.ResolvableType; -import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.Resource; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; @@ -70,17 +68,4 @@ public class ResourceEncoder extends AbstractSingleValueEncoder { return DataBufferUtils.read(resource, dataBufferFactory, this.bufferSize); } - @Override - public Long getContentLength(Resource resource, @Nullable MimeType mimeType) { - // Don't consume InputStream... - if (InputStreamResource.class != resource.getClass()) { - try { - return resource.contentLength(); - } - catch (IOException ignored) { - } - } - return null; - } - } diff --git a/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java index 96fcd886221..4157328fcca 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java @@ -28,6 +28,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; import org.springframework.core.codec.Encoder; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpOutputMessage; @@ -98,23 +99,18 @@ public class EncoderHttpMessageWriter implements HttpMessageWriter { @Nullable MediaType mediaType, ReactiveHttpOutputMessage message, Map hints) { MediaType contentType = updateContentType(message, mediaType); - HttpHeaders headers = message.getHeaders(); - - if (headers.getContentLength() < 0 && !headers.containsKey(HttpHeaders.TRANSFER_ENCODING)) { - if (inputStream instanceof Mono) { - // This works because we don't actually commit until after the first signal... - inputStream = ((Mono) inputStream).doOnNext(data -> { - Long contentLength = this.encoder.getContentLength(data, contentType); - if (contentLength != null) { - headers.setContentLength(contentLength); - } - }); - } - } Flux body = this.encoder.encode( inputStream, message.bufferFactory(), elementType, contentType, hints); + // Response is not committed until the first signal... + if (inputStream instanceof Mono) { + HttpHeaders headers = message.getHeaders(); + if (headers.getContentLength() < 0 && !headers.containsKey(HttpHeaders.TRANSFER_ENCODING)) { + body = body.doOnNext(data -> headers.setContentLength(data.readableByteCount())); + } + } + return (isStreamingMediaType(contentType) ? message.writeAndFlushWith(body.map(Flux::just)) : message.writeWith(body)); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java index c227d8431ea..caf515bc149 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java @@ -31,6 +31,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.codec.ResourceDecoder; import org.springframework.core.codec.ResourceEncoder; import org.springframework.core.codec.ResourceRegionEncoder; +import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.Resource; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; @@ -119,9 +120,9 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { headers.setContentType(resourceMediaType); if (headers.getContentLength() < 0) { - Long contentLength = this.encoder.getContentLength(resource, mediaType); - if (contentLength != null) { - headers.setContentLength(contentLength); + long length = lengthOf(resource); + if (length != -1) { + headers.setContentLength(length); } } @@ -141,6 +142,18 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { return MediaTypeFactory.getMediaType(resource).orElse(MediaType.APPLICATION_OCTET_STREAM); } + private static long lengthOf(Resource resource) { + // Don't consume InputStream... + if (InputStreamResource.class != resource.getClass()) { + try { + return resource.contentLength(); + } + catch (IOException ignored) { + } + } + return -1; + } + private static Optional> zeroCopy(Resource resource, @Nullable ResourceRegion region, ReactiveHttpOutputMessage message) { @@ -192,8 +205,8 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { if (regions.size() == 1){ ResourceRegion region = regions.get(0); headers.setContentType(resourceMediaType); - Long contentLength = this.encoder.getContentLength(resource, mediaType); - if (contentLength != null) { + long contentLength = lengthOf(resource); + if (contentLength != -1) { long start = region.getPosition(); long end = start + region.getCount() - 1; end = Math.min(end, contentLength - 1); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingMessageConversionIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingMessageConversionIntegrationTests.java index c825ce6a80b..cc94301f71f 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingMessageConversionIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingMessageConversionIntegrationTests.java @@ -128,45 +128,59 @@ public class RequestMappingMessageConversionIntegrationTests extends AbstractReq @Test public void personResponseBody() throws Exception { Person expected = new Person("Robert"); - assertEquals(expected, performGet("/person-response/person", JSON, Person.class).getBody()); + ResponseEntity responseEntity = performGet("/person-response/person", JSON, Person.class); + assertEquals(17, responseEntity.getHeaders().getContentLength()); + assertEquals(expected, responseEntity.getBody()); } @Test public void personResponseBodyWithCompletableFuture() throws Exception { Person expected = new Person("Robert"); - assertEquals(expected, performGet("/person-response/completable-future", JSON, Person.class).getBody()); + ResponseEntity responseEntity = performGet("/person-response/completable-future", JSON, Person.class); + assertEquals(17, responseEntity.getHeaders().getContentLength()); + assertEquals(expected, responseEntity.getBody()); } @Test public void personResponseBodyWithMono() throws Exception { Person expected = new Person("Robert"); - assertEquals(expected, performGet("/person-response/mono", JSON, Person.class).getBody()); + ResponseEntity responseEntity = performGet("/person-response/mono", JSON, Person.class); + assertEquals(17, responseEntity.getHeaders().getContentLength()); + assertEquals(expected, responseEntity.getBody()); } @Test public void personResponseBodyWithMonoDeclaredAsObject() throws Exception { Person expected = new Person("Robert"); - assertEquals(expected, performGet("/person-response/mono-declared-as-object", JSON, Person.class).getBody()); + ResponseEntity entity = performGet("/person-response/mono-declared-as-object", JSON, Person.class); + assertEquals(17, entity.getHeaders().getContentLength()); + assertEquals(expected, entity.getBody()); } @Test public void personResponseBodyWithSingle() throws Exception { Person expected = new Person("Robert"); - assertEquals(expected, performGet("/person-response/single", JSON, Person.class).getBody()); + ResponseEntity entity = performGet("/person-response/single", JSON, Person.class); + assertEquals(17, entity.getHeaders().getContentLength()); + assertEquals(expected, entity.getBody()); } @Test public void personResponseBodyWithMonoResponseEntity() throws Exception { Person expected = new Person("Robert"); - assertEquals(expected, performGet("/person-response/mono-response-entity", JSON, Person.class).getBody()); + ResponseEntity entity = performGet("/person-response/mono-response-entity", JSON, Person.class); + assertEquals(17, entity.getHeaders().getContentLength()); + assertEquals(expected, entity.getBody()); } @Test // SPR-16172 public void personResponseBodyWithMonoResponseEntityXml() throws Exception { - String actual = performGet("/person-response/mono-response-entity-xml", - new HttpHeaders(), String.class).getBody(); + String url = "/person-response/mono-response-entity-xml"; + ResponseEntity entity = performGet(url, new HttpHeaders(), String.class); + String actual = entity.getBody(); + assertEquals(91, entity.getHeaders().getContentLength()); assertEquals("" + "Robert", actual); } @@ -174,13 +188,17 @@ public class RequestMappingMessageConversionIntegrationTests extends AbstractReq @Test public void personResponseBodyWithList() throws Exception { List expected = asList(new Person("Robert"), new Person("Marie")); - assertEquals(expected, performGet("/person-response/list", JSON, PERSON_LIST).getBody()); + ResponseEntity> entity = performGet("/person-response/list", JSON, PERSON_LIST); + assertEquals(36, entity.getHeaders().getContentLength()); + assertEquals(expected, entity.getBody()); } @Test public void personResponseBodyWithPublisher() throws Exception { List expected = asList(new Person("Robert"), new Person("Marie")); - assertEquals(expected, performGet("/person-response/publisher", JSON, PERSON_LIST).getBody()); + ResponseEntity> entity = performGet("/person-response/publisher", JSON, PERSON_LIST); + assertEquals(-1, entity.getHeaders().getContentLength()); + assertEquals(expected, entity.getBody()); } @Test