From 76fe5f6fce3a62b12537415e49d4c3ed17af9393 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Sat, 18 Mar 2017 10:35:18 -0400 Subject: [PATCH] ResourceHttpMessageWriter refactoring Fold ResourceRegionHttpMessageWriter into ResourceHttpMessageWriter. The latter was a private helper (not meant to be exposed) and the two have much in common now sharing a number of private helper methods. The combined class does not extend AbstractServerHttpMessageConverter from which it was not using anything. Internally the combined class now delegates directly to ResourceEncoder or ResourceRegionEncoder as needed. The former is no longer wrapped with EncoderHttpMessageWriter which is not required since "resource" MediaType determination is a bit different. The consolidation makes it easy to see the entire algorithm in one place especially for server side rendering (and HTTP ranges). It also allows for consistent determination of the "resource" MediaType via MediaTypeFactory for all use cases. --- .../http/codec/ResourceHttpMessageWriter.java | 251 +++++++++++------- .../ResourceRegionHttpMessageWriter.java | 158 ----------- .../codec/ResourceHttpMessageWriterTests.java | 122 ++++++--- .../ResourceRegionHttpMessageWriterTests.java | 136 ---------- .../server/ResourceHandlerFunctionTests.java | 8 +- .../resource/ResourceWebHandlerTests.java | 1 + 6 files changed, 231 insertions(+), 445 deletions(-) delete mode 100644 spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java delete mode 100644 spring-web/src/test/java/org/springframework/http/codec/ResourceRegionHttpMessageWriterTests.java 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 eb8fa0c45b..2f72b7210d 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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,7 +18,6 @@ package org.springframework.http.codec; import java.io.File; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -32,8 +31,11 @@ import reactor.core.publisher.Mono; 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; import org.springframework.core.io.support.ResourceRegion; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpRange; @@ -46,113 +48,100 @@ import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.MimeTypeUtils; +import static java.util.Collections.emptyMap; + /** - * Implementation of {@link HttpMessageWriter} that can write - * {@link Resource Resources}. + * {@code HttpMessageWriter} that can write a {@link Resource}. * - *

For a Resource reader simply use {@link ResourceDecoder} wrapped with + *

Also an implementation of {@code ServerHttpMessageWriter} with support + * for writing one or more {@link ResourceRegion}'s based on the HTTP ranges + * specified in the request. + * + *

For reading to a Resource, use {@link ResourceDecoder} wrapped with * {@link DecoderHttpMessageReader}. * * @author Arjen Poutsma * @author Brian Clozel + * @author Rossen Stoyanchev * @since 5.0 + * @see ResourceEncoder + * @see ResourceRegionEncoder + * @see HttpRange */ -public class ResourceHttpMessageWriter extends AbstractServerHttpMessageWriter { +public class ResourceHttpMessageWriter implements ServerHttpMessageWriter { - public static final String HTTP_RANGE_REQUEST_HINT = ResourceHttpMessageWriter.class.getName() + ".httpRange"; + private static final ResolvableType REGION_TYPE = ResolvableType.forClass(ResourceRegion.class); - private ResourceRegionHttpMessageWriter resourceRegionHttpMessageWriter; + + private final ResourceEncoder encoder; + + private final ResourceRegionEncoder regionEncoder; + + private final List mediaTypes; public ResourceHttpMessageWriter() { - super(new EncoderHttpMessageWriter<>(new ResourceEncoder())); - this.resourceRegionHttpMessageWriter = new ResourceRegionHttpMessageWriter(); + this(ResourceEncoder.DEFAULT_BUFFER_SIZE); } public ResourceHttpMessageWriter(int bufferSize) { - super(new EncoderHttpMessageWriter<>(new ResourceEncoder(bufferSize))); - this.resourceRegionHttpMessageWriter = new ResourceRegionHttpMessageWriter(bufferSize); + this.encoder = new ResourceEncoder(bufferSize); + this.regionEncoder = new ResourceRegionEncoder(bufferSize); + this.mediaTypes = MediaType.asMediaTypes(this.encoder.getEncodableMimeTypes()); } @Override - protected Map resolveWriteHints(ResolvableType streamType, ResolvableType elementType, - MediaType mediaType, ServerHttpRequest request) { - - List httpRanges = request.getHeaders().getRange(); - if (!httpRanges.isEmpty()) { - return Collections.singletonMap(ResourceHttpMessageWriter.HTTP_RANGE_REQUEST_HINT, httpRanges); - } - return Collections.emptyMap(); + public boolean canWrite(ResolvableType elementType, MediaType mediaType) { + return this.encoder.canEncode(elementType, mediaType); } + @Override + public List getWritableMediaTypes() { + return this.mediaTypes; + } + + + // HttpMessageWriter (client and server): single Resource + @Override public Mono write(Publisher inputStream, ResolvableType elementType, - MediaType mediaType, ReactiveHttpOutputMessage outputMessage, Map hints) { + MediaType mediaType, ReactiveHttpOutputMessage message, Map hints) { - return Mono.from(Flux.from(inputStream). - take(1). - concatMap(resource -> { - HttpHeaders headers = outputMessage.getHeaders(); - addHeaders(headers, resource, mediaType); - return writeContent(resource, elementType, outputMessage, hints); - })); + return Mono.from(inputStream).then(resource -> + writeResource(resource, elementType, mediaType, message, hints)); } - @Override - @SuppressWarnings("unchecked") - public Mono write(Publisher inputStream, ResolvableType streamType, - ResolvableType elementType, MediaType mediaType, ServerHttpRequest request, - ServerHttpResponse response, Map hints) { - try { - response.getHeaders().set(HttpHeaders.ACCEPT_RANGES, "bytes"); - Map mergedHints = new HashMap<>(hints); - mergedHints.putAll(resolveWriteHints(streamType, elementType, mediaType, request)); - if (mergedHints.containsKey(HTTP_RANGE_REQUEST_HINT)) { - response.setStatusCode(HttpStatus.PARTIAL_CONTENT); - List httpRanges = (List) mergedHints.get(HTTP_RANGE_REQUEST_HINT); - if (httpRanges.size() > 1) { - final String boundary = MimeTypeUtils.generateMultipartBoundaryString(); - mergedHints.put(ResourceRegionHttpMessageWriter.BOUNDARY_STRING_HINT, boundary); - } - Flux regions = Flux.from(inputStream) - .flatMap(resource -> Flux.fromIterable(HttpRange.toResourceRegions(httpRanges, resource))); + private Mono writeResource(Resource resource, ResolvableType type, MediaType mediaType, + ReactiveHttpOutputMessage message, Map hints) { - return this.resourceRegionHttpMessageWriter - .writeRegions(regions, mediaType, response, mergedHints); - } - else { - return write(inputStream, elementType, mediaType, response, mergedHints); - } - } - catch (IllegalArgumentException exc) { - response.setStatusCode(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE); - return response.setComplete(); - } - } + HttpHeaders headers = message.getHeaders(); + MediaType resourceMediaType = getResourceMediaType(mediaType, resource); + headers.setContentType(resourceMediaType); - protected void addHeaders(HttpHeaders headers, Resource resource, MediaType mediaType) { - if (headers.getContentType() == null) { - if (mediaType == null || !mediaType.isConcrete() || - MediaType.APPLICATION_OCTET_STREAM.equals(mediaType)) { - mediaType = Optional.ofNullable(MediaTypeFactory.getMediaType(resource)). - orElse(MediaType.APPLICATION_OCTET_STREAM); - } - headers.setContentType(mediaType); - } if (headers.getContentLength() < 0) { - contentLength(resource).ifPresent(headers::setContentLength); + lengthOf(resource).ifPresent(headers::setContentLength); } + + return zeroCopy(resource, null, message) + .orElseGet(() -> { + Mono input = Mono.just(resource); + DataBufferFactory factory = message.bufferFactory(); + Flux body = this.encoder.encode(input, factory, type, resourceMediaType, hints); + return message.writeWith(body); + }); } - /** - * Determine, if possible, the contentLength of the given resource without reading it. - * @param resource the resource instance - * @return the contentLength of the resource - */ - private OptionalLong contentLength(Resource resource) { - // Don't try to determine contentLength on InputStreamResource - cannot be read afterwards... - // Note: custom InputStreamResource subclasses could provide a pre-calculated content length! + private static MediaType getResourceMediaType(MediaType type, Resource resource) { + if (type != null && type.isConcrete() && !type.equals(MediaType.APPLICATION_OCTET_STREAM)) { + return type; + } + type = MediaTypeFactory.getMediaType(resource); + return type != null ? type : MediaType.APPLICATION_OCTET_STREAM; + } + + private static OptionalLong lengthOf(Resource resource) { + // Don't consume InputStream... if (InputStreamResource.class != resource.getClass()) { try { return OptionalLong.of(resource.contentLength()); @@ -163,34 +152,96 @@ public class ResourceHttpMessageWriter extends AbstractServerHttpMessageWriter writeContent(Resource resource, ResolvableType type, - ReactiveHttpOutputMessage outputMessage, Map hints) { + private static Optional> zeroCopy(Resource resource, ResourceRegion region, + ReactiveHttpOutputMessage message) { - if (outputMessage instanceof ZeroCopyHttpOutputMessage) { - Optional file = getFile(resource); - if (file.isPresent()) { - ZeroCopyHttpOutputMessage zeroCopyResponse = - (ZeroCopyHttpOutputMessage) outputMessage; - - return zeroCopyResponse.writeWith(file.get(), 0, file.get().length()); - } - } - - // non-zero copy fallback, using ResourceEncoder - return super.write(Mono.just(resource), type, - outputMessage.getHeaders().getContentType(), outputMessage, hints); - } - - private static Optional getFile(Resource resource) { - if (resource.isFile()) { - try { - return Optional.of(resource.getFile()); - } - catch (IOException ex) { - // should not happen + if (message instanceof ZeroCopyHttpOutputMessage) { + if (resource.isFile()) { + try { + File file = resource.getFile(); + long pos = region != null ? region.getPosition() : 0; + long count = region != null ? region.getCount() : file.length(); + return Optional.of(((ZeroCopyHttpOutputMessage) message).writeWith(file, pos, count)); + } + catch (IOException ex) { + // should not happen + } } } return Optional.empty(); } + + // ServerHttpMessageWriter (server only): single Resource or sub-regions + + @Override + @SuppressWarnings("unchecked") + public Mono write(Publisher inputStream, ResolvableType streamType, + ResolvableType elementType, MediaType mediaType, ServerHttpRequest request, + ServerHttpResponse response, Map hints) { + + HttpHeaders headers = response.getHeaders(); + headers.set(HttpHeaders.ACCEPT_RANGES, "bytes"); + + List ranges; + try { + ranges = request.getHeaders().getRange(); + } + catch (IllegalArgumentException ex) { + response.setStatusCode(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE); + return response.setComplete(); + } + + return Mono.from(inputStream).then(resource -> { + + if (ranges.isEmpty()) { + return writeResource(resource, elementType, mediaType, response, hints); + } + + response.setStatusCode(HttpStatus.PARTIAL_CONTENT); + List regions = HttpRange.toResourceRegions(ranges, resource); + MediaType resourceMediaType = getResourceMediaType(mediaType, resource); + + if (regions.size() == 1){ + ResourceRegion region = regions.get(0); + headers.setContentType(resourceMediaType); + lengthOf(resource).ifPresent(length -> { + long start = region.getPosition(); + long end = start + region.getCount() - 1; + end = Math.min(end, length - 1); + headers.add("Content-Range", "bytes " + start + '-' + end + '/' + length); + headers.setContentLength(end - start + 1); + }); + return writeSingleRegion(region, response); + } + else { + String boundary = MimeTypeUtils.generateMultipartBoundaryString(); + MediaType multipartType = MediaType.parseMediaType("multipart/byteranges;boundary=" + boundary); + headers.setContentType(multipartType); + Map theHints = new HashMap<>(hints); + theHints.put(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary); + return encodeAndWriteRegions(Flux.fromIterable(regions), resourceMediaType, response, theHints); + } + }); + } + + private Mono writeSingleRegion(ResourceRegion region, ReactiveHttpOutputMessage message) { + + return zeroCopy(region.getResource(), region, message) + .orElseGet(() -> { + Publisher input = Mono.just(region); + MediaType mediaType = message.getHeaders().getContentType(); + return encodeAndWriteRegions(input, mediaType, message, emptyMap()); + }); + } + + private Mono encodeAndWriteRegions(Publisher publisher, + MediaType mediaType, ReactiveHttpOutputMessage message, Map hints) { + + Flux body = this.regionEncoder.encode( + publisher, message.bufferFactory(), REGION_TYPE, mediaType, hints); + + return message.writeWith(body); + } + } diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java deleted file mode 100644 index 8fb17b4f6b..0000000000 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java +++ /dev/null @@ -1,158 +0,0 @@ -/* - * Copyright 2002-2017 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.codec; - -import java.io.File; -import java.io.IOException; -import java.util.Collections; -import java.util.Map; -import java.util.Optional; -import java.util.OptionalLong; - -import org.reactivestreams.Publisher; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; - -import org.springframework.core.ResolvableType; -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; -import org.springframework.core.io.support.ResourceRegion; -import org.springframework.http.MediaType; -import org.springframework.http.ReactiveHttpOutputMessage; -import org.springframework.http.ZeroCopyHttpOutputMessage; - -/** - * Package private helper for {@link ResourceHttpMessageWriter} to assist with - * writing {@link ResourceRegion ResourceRegion}s. - * - * @author Brian Clozel - * @author Rossen Stoyanchev - * @since 5.0 - */ -class ResourceRegionHttpMessageWriter { - - public static final String BOUNDARY_STRING_HINT = ResourceRegionHttpMessageWriter.class.getName() + ".boundaryString"; - - private static final ResolvableType TYPE = ResolvableType.forClass(ResourceRegion.class); - - - private final ResourceRegionEncoder encoder; - - - public ResourceRegionHttpMessageWriter() { - this.encoder = new ResourceRegionEncoder(); - } - - public ResourceRegionHttpMessageWriter(int bufferSize) { - this.encoder = new ResourceRegionEncoder(bufferSize); - } - - - public Mono writeRegions(Publisher inputStream, MediaType contentType, - ReactiveHttpOutputMessage outputMessage, Map hints) { - - if (hints != null && hints.containsKey(BOUNDARY_STRING_HINT)) { - String boundary = (String) hints.get(BOUNDARY_STRING_HINT); - hints.put(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary); - - MediaType multipartType = MediaType.parseMediaType("multipart/byteranges;boundary=" + boundary); - outputMessage.getHeaders().setContentType(multipartType); - - DataBufferFactory bufferFactory = outputMessage.bufferFactory(); - Flux body = this.encoder.encode(inputStream, bufferFactory, TYPE, contentType, hints); - return outputMessage.writeWith(body); - } - else { - return Mono.from(inputStream) - .then(region -> { - writeSingleResourceRegionHeaders(region, contentType, outputMessage); - return writeResourceRegion(region, outputMessage); - }); - } - } - - - private void writeSingleResourceRegionHeaders(ResourceRegion region, MediaType contentType, - ReactiveHttpOutputMessage outputMessage) { - - OptionalLong resourceLength = contentLength(region.getResource()); - resourceLength.ifPresent(length -> { - long start = region.getPosition(); - long end = start + region.getCount() - 1; - end = Math.min(end, length - 1); - outputMessage.getHeaders().add("Content-Range", "bytes " + start + '-' + end + '/' + length); - outputMessage.getHeaders().setContentLength(end - start + 1); - }); - outputMessage.getHeaders().setContentType(contentType); - } - - /** - * Determine, if possible, the contentLength of the given resource without reading it. - * @param resource the resource instance - * @return the contentLength of the resource - */ - private OptionalLong contentLength(Resource resource) { - // Don't try to determine contentLength on InputStreamResource - cannot be read afterwards... - // Note: custom InputStreamResource subclasses could provide a pre-calculated content length! - if (InputStreamResource.class != resource.getClass()) { - try { - return OptionalLong.of(resource.contentLength()); - } - catch (IOException ignored) { - } - } - return OptionalLong.empty(); - } - - private Mono writeResourceRegion(ResourceRegion region, ReactiveHttpOutputMessage outputMessage) { - - if (outputMessage instanceof ZeroCopyHttpOutputMessage) { - Optional file = getFile(region.getResource()); - if (file.isPresent()) { - ZeroCopyHttpOutputMessage zeroCopyResponse = - (ZeroCopyHttpOutputMessage) outputMessage; - - return zeroCopyResponse.writeWith(file.get(), region.getPosition(), region.getCount()); - } - } - - // non-zero copy fallback, using ResourceRegionEncoder - - DataBufferFactory bufferFactory = outputMessage.bufferFactory(); - MediaType contentType = outputMessage.getHeaders().getContentType(); - Map hints = Collections.emptyMap(); - - Flux body = this.encoder.encode(Mono.just(region), bufferFactory, TYPE, contentType, hints); - return outputMessage.writeWith(body); - } - - private static Optional getFile(Resource resource) { - if (resource.isFile()) { - try { - return Optional.of(resource.getFile()); - } - catch (IOException ex) { - // should not happen - } - } - return Optional.empty(); - } - -} diff --git a/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java b/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java index a30bee7a71..4ee1234eff 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -20,100 +20,134 @@ import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Map; -import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import org.springframework.core.ResolvableType; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.Resource; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpRange; import org.springframework.http.HttpStatus; -import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.util.MimeTypeUtils; +import org.springframework.util.StringUtils; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertThat; +import static org.springframework.http.MediaType.TEXT_PLAIN; import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get; /** * Unit tests for {@link ResourceHttpMessageWriter}. * * @author Brian Clozel + * @author Rossen Stoyanchev */ public class ResourceHttpMessageWriterTests { - private ResourceHttpMessageWriter writer = new ResourceHttpMessageWriter(); - - private MockServerHttpResponse response = new MockServerHttpResponse(); - - private Resource resource; + private static final Map HINTS = Collections.emptyMap(); - @Before - public void setUp() throws Exception { - String content = "Spring Framework test resource content."; - this.resource = new ByteArrayResource(content.getBytes(StandardCharsets.UTF_8)); - } + private final ResourceHttpMessageWriter writer = new ResourceHttpMessageWriter(); + + private final MockServerHttpResponse response = new MockServerHttpResponse(); + + private final Mono input = Mono.just(new ByteArrayResource( + "Spring Framework test resource content.".getBytes(StandardCharsets.UTF_8))); @Test - public void writableMediaTypes() throws Exception { + public void getWritableMediaTypes() throws Exception { assertThat(this.writer.getWritableMediaTypes(), containsInAnyOrder(MimeTypeUtils.APPLICATION_OCTET_STREAM, MimeTypeUtils.ALL)); } @Test - public void shouldWriteResource() throws Exception { - MockServerHttpRequest request = get("/").build(); - testWrite(request); + public void writeResource() throws Exception { - assertThat(this.response.getHeaders().getContentType(), is(MediaType.TEXT_PLAIN)); + testWrite(get("/").build()); + + assertThat(this.response.getHeaders().getContentType(), is(TEXT_PLAIN)); assertThat(this.response.getHeaders().getContentLength(), is(39L)); assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes")); - Mono result = this.response.getBodyAsString(); - StepVerifier.create(result) - .expectNext("Spring Framework test resource content.") + String content = "Spring Framework test resource content."; + StepVerifier.create(this.response.getBodyAsString()).expectNext(content).expectComplete().verify(); + } + + @Test + public void writeSingleRegion() throws Exception { + + testWrite(get("/").range(of(0, 5)).build()); + + assertThat(this.response.getHeaders().getContentType(), is(TEXT_PLAIN)); + assertThat(this.response.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE), is("bytes 0-5/39")); + assertThat(this.response.getHeaders().getContentLength(), is(6L)); + + StepVerifier.create(this.response.getBodyAsString()).expectNext("Spring").expectComplete().verify(); + } + + @Test + public void writeMultipleRegions() throws Exception { + + testWrite(get("/").range(of(0,5), of(7,15), of(17,20), of(22,38)).build()); + + HttpHeaders headers = this.response.getHeaders(); + String contentType = headers.getContentType().toString(); + String boundary = contentType.substring(30); + + assertThat(contentType, startsWith("multipart/byteranges;boundary=")); + + StepVerifier.create(this.response.getBodyAsString()) + .consumeNextWith(content -> { + String[] actualRanges = StringUtils.tokenizeToStringArray(content, "\r\n", false, true); + String[] expected = new String[] { + "--" + boundary, + "Content-Type: text/plain", + "Content-Range: bytes 0-5/39", + "Spring", + "--" + boundary, + "Content-Type: text/plain", + "Content-Range: bytes 7-15/39", + "Framework", + "--" + boundary, + "Content-Type: text/plain", + "Content-Range: bytes 17-20/39", + "test", + "--" + boundary, + "Content-Type: text/plain", + "Content-Range: bytes 22-38/39", + "resource content.", + "--" + boundary + "--" + }; + assertArrayEquals(expected, actualRanges); + }) .expectComplete() .verify(); } @Test - public void shouldWriteResourceRange() throws Exception { - MockServerHttpRequest request = get("/").range(HttpRange.createByteRange(0, 5)).build(); - testWrite(request); + public void invalidRange() throws Exception { - assertThat(this.response.getHeaders().getContentType(), is(MediaType.TEXT_PLAIN)); - assertThat(this.response.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE), is("bytes 0-5/39")); - assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes")); - assertThat(this.response.getHeaders().getContentLength(), is(6L)); - - Mono result = this.response.getBodyAsString(); - StepVerifier.create(result).expectNext("Spring").expectComplete().verify(); - } - - @Test - public void shouldSetRangeNotSatisfiableStatus() throws Exception { - MockServerHttpRequest request = get("/").header(HttpHeaders.RANGE, "invalid").build(); - testWrite(request); + testWrite(get("/").header(HttpHeaders.RANGE, "invalid").build()); assertThat(this.response.getHeaders().getFirst(HttpHeaders.ACCEPT_RANGES), is("bytes")); assertThat(this.response.getStatusCode(), is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE)); } + private void testWrite(MockServerHttpRequest request) { - Mono input = Mono.just(this.resource); - ResolvableType type = ResolvableType.forClass(Resource.class); - MediaType contentType = MediaType.TEXT_PLAIN; - Map hints = Collections.emptyMap(); - Mono mono = this.writer.write(input, null, type, contentType, request, this.response, hints); - StepVerifier.create(mono).expectNextCount(0).expectComplete().verify(); + Mono mono = this.writer.write(this.input, null, null, TEXT_PLAIN, request, this.response, HINTS); + StepVerifier.create(mono).expectComplete().verify(); + } + + private static HttpRange of(int first, int last) { + return HttpRange.createByteRange(first, last); } } diff --git a/spring-web/src/test/java/org/springframework/http/codec/ResourceRegionHttpMessageWriterTests.java b/spring-web/src/test/java/org/springframework/http/codec/ResourceRegionHttpMessageWriterTests.java deleted file mode 100644 index adef5264e8..0000000000 --- a/spring-web/src/test/java/org/springframework/http/codec/ResourceRegionHttpMessageWriterTests.java +++ /dev/null @@ -1,136 +0,0 @@ -/* - * Copyright 2002-2016 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.codec; - - -import java.nio.charset.StandardCharsets; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; -import reactor.test.StepVerifier; - -import org.springframework.core.io.ByteArrayResource; -import org.springframework.core.io.Resource; -import org.springframework.core.io.support.ResourceRegion; -import org.springframework.http.HttpHeaders; -import org.springframework.http.MediaType; -import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; -import org.springframework.util.MimeTypeUtils; -import org.springframework.util.StringUtils; - -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertThat; - -/** - * Unit tests for {@link ResourceRegionHttpMessageWriter}. - * @author Brian Clozel - */ -public class ResourceRegionHttpMessageWriterTests { - - private ResourceRegionHttpMessageWriter writer = new ResourceRegionHttpMessageWriter(); - - private MockServerHttpResponse response = new MockServerHttpResponse(); - - private Resource resource; - - @Rule - public ExpectedException expectedException = ExpectedException.none(); - - - @Before - public void setUp() throws Exception { - String content = "Spring Framework test resource content."; - this.resource = new ByteArrayResource(content.getBytes(StandardCharsets.UTF_8)); - } - - - @Test - public void shouldWriteResourceRegion() throws Exception { - - ResourceRegion region = new ResourceRegion(this.resource, 0, 6); - Map hints = Collections.emptyMap(); - - Mono mono = this.writer.writeRegions(Mono.just(region), MediaType.TEXT_PLAIN, this.response, hints); - StepVerifier.create(mono).expectComplete().verify(); - - assertThat(this.response.getHeaders().getContentType(), is(MediaType.TEXT_PLAIN)); - assertThat(this.response.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE), is("bytes 0-5/39")); - assertThat(this.response.getHeaders().getContentLength(), is(6L)); - - Mono result = response.getBodyAsString(); - StepVerifier.create(result).expectNext("Spring").expectComplete().verify(); - } - - @Test - public void shouldWriteMultipleResourceRegions() throws Exception { - Flux regions = Flux.just( - new ResourceRegion(this.resource, 0, 6), - new ResourceRegion(this.resource, 7, 9), - new ResourceRegion(this.resource, 17, 4), - new ResourceRegion(this.resource, 22, 17) - ); - String boundary = MimeTypeUtils.generateMultipartBoundaryString(); - Map hints = new HashMap<>(1); - hints.put(ResourceRegionHttpMessageWriter.BOUNDARY_STRING_HINT, boundary); - - Mono mono = this.writer.writeRegions(regions, MediaType.TEXT_PLAIN, this.response, hints); - StepVerifier.create(mono).expectComplete().verify(); - - HttpHeaders headers = this.response.getHeaders(); - assertThat(headers.getContentType().toString(), startsWith("multipart/byteranges;boundary=" + boundary)); - - Mono result = response.getBodyAsString(); - - StepVerifier.create(result) - .consumeNextWith(content -> { - String[] ranges = StringUtils - .tokenizeToStringArray(content, "\r\n", false, true); - String[] expected = new String[] { - "--" + boundary, - "Content-Type: text/plain", - "Content-Range: bytes 0-5/39", - "Spring", - "--" + boundary, - "Content-Type: text/plain", - "Content-Range: bytes 7-15/39", - "Framework", - "--" + boundary, - "Content-Type: text/plain", - "Content-Range: bytes 17-20/39", - "test", - "--" + boundary, - "Content-Type: text/plain", - "Content-Range: bytes 22-38/39", - "resource content.", - "--" + boundary + "--" - }; - assertArrayEquals(expected, ranges); - }) - .expectComplete() - .verify(); - } - -} diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/ResourceHandlerFunctionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/ResourceHandlerFunctionTests.java index 1789a6d4de..0e76e109a8 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/ResourceHandlerFunctionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/ResourceHandlerFunctionTests.java @@ -99,15 +99,9 @@ public class ResourceHandlerFunctionTests { return res.writeTo(exchange, HandlerStrategies.withDefaults()); }); - StepVerifier.create(result) - .expectComplete() - .verify(); - StepVerifier.create(result).expectComplete().verify(); + StepVerifier.create(mockResponse.getBody()).expectComplete().verify(); - StepVerifier.create(mockResponse.getBody()) - .expectComplete() - .verify(); assertEquals(MediaType.TEXT_PLAIN, mockResponse.getHeaders().getContentType()); assertEquals(this.resource.contentLength(), mockResponse.getHeaders().getContentLength()); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index cf3a5af6e4..5edfb4118f 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -52,6 +52,7 @@ import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.accept.CompositeContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder; import org.springframework.web.server.MethodNotAllowedException; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import static org.junit.Assert.assertEquals;