Fix content type issue in ResourceRegionHttpMessageWriter
ResourceRegionHttpMessageWriter no longer extends from EncoderHttpMessageWriter freeing it to pass the correct content type into the encoder. Considering that the main benefit of EncoderHttpMessageWriter is to deal with content type fallback cases, there is nothing to be missed. Furthermore ResourceRegionHttpMessageWriter is a package private class that is used internally within ResourceHttpMessageWriter and never exposed externally as a an actual HttpMessageWriter. Issue: SPR-15358
This commit is contained in:
parent
2979b37ae3
commit
c735ffb39b
|
@ -119,9 +119,9 @@ public class ResourceHttpMessageWriter extends AbstractServerHttpMessageWriter<R
|
|||
.flatMap(resource -> Flux.fromIterable(HttpRange.toResourceRegions(httpRanges, resource)));
|
||||
|
||||
return this.resourceRegionHttpMessageWriter
|
||||
.write(regions, ResolvableType.forClass(ResourceRegion.class), mediaType, response, mergedHints);
|
||||
.writeRegions(regions, mediaType, response, mergedHints);
|
||||
}
|
||||
else {
|
||||
else {
|
||||
return write(inputStream, elementType, mediaType, response, mergedHints);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -24,54 +24,66 @@ 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;
|
||||
|
||||
/**
|
||||
* Implementation of {@link HttpMessageWriter} that can write
|
||||
* {@link ResourceRegion ResourceRegion}s.
|
||||
*
|
||||
* <p>Note that there is no {@link HttpMessageReader} counterpart.
|
||||
* 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 extends EncoderHttpMessageWriter<ResourceRegion> {
|
||||
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() {
|
||||
super(new ResourceRegionEncoder());
|
||||
this.encoder = new ResourceRegionEncoder();
|
||||
}
|
||||
|
||||
public ResourceRegionHttpMessageWriter(int bufferSize) {
|
||||
super(new ResourceRegionEncoder(bufferSize));
|
||||
this.encoder = new ResourceRegionEncoder(bufferSize);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Mono<Void> write(Publisher<? extends ResourceRegion> inputStream, ResolvableType type,
|
||||
MediaType contentType, ReactiveHttpOutputMessage outputMessage, Map<String, Object> hints) {
|
||||
|
||||
public Mono<Void> writeRegions(Publisher<? extends ResourceRegion> inputStream, MediaType contentType,
|
||||
ReactiveHttpOutputMessage outputMessage, Map<String, Object> hints) {
|
||||
|
||||
if (hints != null && hints.containsKey(BOUNDARY_STRING_HINT)) {
|
||||
String boundary = (String) hints.get(BOUNDARY_STRING_HINT);
|
||||
hints.put(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary);
|
||||
outputMessage.getHeaders()
|
||||
.setContentType(MediaType.parseMediaType("multipart/byteranges;boundary=" + boundary));
|
||||
return super.write(inputStream, type, contentType, outputMessage, hints);
|
||||
|
||||
MediaType multipartType = MediaType.parseMediaType("multipart/byteranges;boundary=" + boundary);
|
||||
outputMessage.getHeaders().setContentType(multipartType);
|
||||
|
||||
DataBufferFactory bufferFactory = outputMessage.bufferFactory();
|
||||
Flux<DataBuffer> 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, type, outputMessage);
|
||||
return writeResourceRegion(region, outputMessage);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
@ -109,8 +121,8 @@ class ResourceRegionHttpMessageWriter extends EncoderHttpMessageWriter<ResourceR
|
|||
return OptionalLong.empty();
|
||||
}
|
||||
|
||||
private Mono<Void> writeResourceRegion(ResourceRegion region,
|
||||
ResolvableType type, ReactiveHttpOutputMessage outputMessage) {
|
||||
private Mono<Void> writeResourceRegion(ResourceRegion region, ReactiveHttpOutputMessage outputMessage) {
|
||||
|
||||
if (outputMessage instanceof ZeroCopyHttpOutputMessage) {
|
||||
Optional<File> file = getFile(region.getResource());
|
||||
if (file.isPresent()) {
|
||||
|
@ -122,8 +134,13 @@ class ResourceRegionHttpMessageWriter extends EncoderHttpMessageWriter<ResourceR
|
|||
}
|
||||
|
||||
// non-zero copy fallback, using ResourceRegionEncoder
|
||||
return super.write(Mono.just(region), type,
|
||||
outputMessage.getHeaders().getContentType(), outputMessage, Collections.emptyMap());
|
||||
|
||||
DataBufferFactory bufferFactory = outputMessage.bufferFactory();
|
||||
MediaType contentType = outputMessage.getHeaders().getContentType();
|
||||
Map<String, Object> hints = Collections.emptyMap();
|
||||
|
||||
Flux<DataBuffer> body = this.encoder.encode(Mono.just(region), bufferFactory, TYPE, contentType, hints);
|
||||
return outputMessage.writeWith(body);
|
||||
}
|
||||
|
||||
private static Optional<File> getFile(Resource resource) {
|
||||
|
|
|
@ -23,7 +23,6 @@ import java.util.HashMap;
|
|||
import java.util.Map;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.junit.rules.ExpectedException;
|
||||
|
@ -31,7 +30,6 @@ import reactor.core.publisher.Flux;
|
|||
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.core.io.support.ResourceRegion;
|
||||
|
@ -41,8 +39,10 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse
|
|||
import org.springframework.util.MimeTypeUtils;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
||||
import static org.hamcrest.Matchers.*;
|
||||
import static org.junit.Assert.*;
|
||||
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}.
|
||||
|
@ -67,19 +67,13 @@ public class ResourceRegionHttpMessageWriterTests {
|
|||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void writableMediaTypes() throws Exception {
|
||||
assertThat(this.writer.getWritableMediaTypes(),
|
||||
containsInAnyOrder(MimeTypeUtils.APPLICATION_OCTET_STREAM, MimeTypeUtils.ALL));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldWriteResourceRegion() throws Exception {
|
||||
|
||||
ResourceRegion region = new ResourceRegion(this.resource, 0, 6);
|
||||
Map<String, Object> hints = Collections.emptyMap();
|
||||
|
||||
Mono<Void> mono = this.writer.write(Mono.just(region), ResolvableType.forClass(ResourceRegion.class),
|
||||
MediaType.TEXT_PLAIN, this.response, Collections.emptyMap());
|
||||
Mono<Void> 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));
|
||||
|
@ -91,7 +85,6 @@ public class ResourceRegionHttpMessageWriterTests {
|
|||
}
|
||||
|
||||
@Test
|
||||
@Ignore("Until issue resolved: ResourceRegion should not use response content-type")
|
||||
public void shouldWriteMultipleResourceRegions() throws Exception {
|
||||
Flux<ResourceRegion> regions = Flux.just(
|
||||
new ResourceRegion(this.resource, 0, 6),
|
||||
|
@ -103,8 +96,7 @@ public class ResourceRegionHttpMessageWriterTests {
|
|||
Map<String, Object> hints = new HashMap<>(1);
|
||||
hints.put(ResourceRegionHttpMessageWriter.BOUNDARY_STRING_HINT, boundary);
|
||||
|
||||
Mono<Void> mono = this.writer.write(regions, ResolvableType.forClass(ResourceRegion.class),
|
||||
MediaType.TEXT_PLAIN, this.response, hints);
|
||||
Mono<Void> mono = this.writer.writeRegions(regions, MediaType.TEXT_PLAIN, this.response, hints);
|
||||
StepVerifier.create(mono).expectComplete().verify();
|
||||
|
||||
HttpHeaders headers = this.response.getHeaders();
|
||||
|
|
|
@ -26,7 +26,6 @@ import java.util.List;
|
|||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Test;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
@ -538,7 +537,6 @@ public class ResourceWebHandlerTests {
|
|||
}
|
||||
|
||||
@Test
|
||||
@Ignore("Until issue resolved: ResourceRegion should not use response content-type")
|
||||
public void partialContentMultipleByteRanges() throws Exception {
|
||||
MockServerWebExchange exchange = MockServerHttpRequest.get("").header("Range", "bytes=0-1, 4-5, 8-9").toExchange();
|
||||
setPathWithinHandlerMapping(exchange, "foo.txt");
|
||||
|
|
Loading…
Reference in New Issue