Skip to content

Commit d94ce0a

Browse files
jhoellerjhoeller
committed
Avoid package dependency cycles
1 parent 65e01ea commit d94ce0a

File tree

6 files changed

+76
-51
lines changed

6 files changed

+76
-51
lines changed

‎spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import reactor.core.publisher.Mono;
2929

3030
import org.springframework.core.ResolvableType;
31+
import org.springframework.core.io.InputStreamResource;
32+
import org.springframework.core.io.Resource;
3133
import org.springframework.core.io.buffer.DataBuffer;
3234
import org.springframework.core.io.buffer.DataBufferFactory;
3335
import org.springframework.core.io.buffer.DataBufferUtils;
@@ -134,7 +136,7 @@ private byte[] getAsciiBytes(String in) {
134136
private byte[] getContentRangeHeader(ResourceRegion region) {
135137
long start = region.getPosition();
136138
long end = start + region.getCount() - 1;
137-
OptionalLong contentLength = ResourceUtils.contentLength(region.getResource());
139+
OptionalLong contentLength = contentLength(region.getResource());
138140
if (contentLength.isPresent()) {
139141
return getAsciiBytes("Content-Range: bytes " + start + "-" + end + "/" + contentLength.getAsLong() + "\r\n\r\n");
140142
}
@@ -143,4 +145,22 @@ private byte[] getContentRangeHeader(ResourceRegion region) {
143145
}
144146
}
145147

148+
/**
149+
* Determine, if possible, the contentLength of the given resource without reading it.
150+
* @param resource the resource instance
151+
* @return the contentLength of the resource
152+
*/
153+
private OptionalLong contentLength(Resource resource) {
154+
// Don't try to determine contentLength on InputStreamResource - cannot be read afterwards...
155+
// Note: custom InputStreamResource subclasses could provide a pre-calculated content length!
156+
if (InputStreamResource.class != resource.getClass()) {
157+
try {
158+
return OptionalLong.of(resource.contentLength());
159+
}
160+
catch (IOException ignored) {
161+
}
162+
}
163+
return OptionalLong.empty();
164+
}
165+
146166
}

‎spring-core/src/main/java/org/springframework/util/ResourceUtils.java

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,11 @@
1818

1919
import java.io.File;
2020
import java.io.FileNotFoundException;
21-
import java.io.IOException;
2221
import java.net.MalformedURLException;
2322
import java.net.URI;
2423
import java.net.URISyntaxException;
2524
import java.net.URL;
2625
import java.net.URLConnection;
27-
import java.util.OptionalLong;
28-
29-
import org.springframework.core.io.InputStreamResource;
30-
import org.springframework.core.io.Resource;
3126

3227
/**
3328
* Utility methods for resolving resource locations to files in the
@@ -389,23 +384,4 @@ public static void useCachesIfNecessary(URLConnection con) {
389384
con.setUseCaches(con.getClass().getSimpleName().startsWith("JNLP"));
390385
}
391386

392-
/**
393-
* Determine, if possible, the contentLength of the given resource
394-
* without reading it.
395-
* @param resource the resource instance
396-
* @return the contentLength of the resource
397-
*/
398-
public static OptionalLong contentLength(Resource resource) {
399-
// Don't try to determine contentLength on InputStreamResource - cannot be read afterwards...
400-
// Note: custom InputStreamResource subclasses could provide a pre-calculated content length!
401-
if (InputStreamResource.class != resource.getClass()) {
402-
try {
403-
return OptionalLong.of(resource.contentLength());
404-
}
405-
catch (IOException ignored) {
406-
}
407-
}
408-
return OptionalLong.empty();
409-
}
410-
411387
}

‎spring-web-reactive/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -527,9 +527,7 @@ public void partialContentInvalidRangeHeader() throws Exception {
527527

528528
TestSubscriber.subscribe(this.handler.handle(this.exchange))
529529
.assertErrorWith(throwable -> {
530-
assertThat(throwable, instanceOf(ResponseStatusException.class));
531-
ResponseStatusException exc = (ResponseStatusException) throwable;
532-
assertThat(exc.getStatus(), is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE));
530+
assertThat(throwable, instanceOf(IllegalArgumentException.class));
533531
});
534532
}
535533

‎spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.Optional;
26+
import java.util.OptionalLong;
2627

2728
import org.reactivestreams.Publisher;
2829
import reactor.core.publisher.Flux;
@@ -31,6 +32,7 @@
3132
import org.springframework.core.ResolvableType;
3233
import org.springframework.core.codec.ResourceDecoder;
3334
import org.springframework.core.codec.ResourceEncoder;
35+
import org.springframework.core.io.InputStreamResource;
3436
import org.springframework.core.io.Resource;
3537
import org.springframework.core.io.support.ResourceRegion;
3638
import org.springframework.http.HttpHeaders;
@@ -43,8 +45,6 @@
4345
import org.springframework.http.server.reactive.ServerHttpRequest;
4446
import org.springframework.http.server.reactive.ServerHttpResponse;
4547
import org.springframework.util.MimeTypeUtils;
46-
import org.springframework.util.ResourceUtils;
47-
import org.springframework.web.server.ResponseStatusException;
4848

4949
/**
5050
* Implementation of {@link HttpMessageWriter} that can write
@@ -74,18 +74,14 @@ public ResourceHttpMessageWriter(int bufferSize) {
7474
this.resourceRegionHttpMessageWriter = new ResourceRegionHttpMessageWriter(bufferSize);
7575
}
7676

77+
7778
@Override
7879
protected Map<String, Object> resolveWriteHints(ResolvableType streamType, ResolvableType elementType,
7980
MediaType mediaType, ServerHttpRequest request) {
80-
try {
81-
List<HttpRange> httpRanges = request.getHeaders().getRange();
82-
if (!httpRanges.isEmpty()) {
83-
return Collections.singletonMap(ResourceHttpMessageWriter.HTTP_RANGE_REQUEST_HINT, httpRanges);
84-
}
85-
}
86-
catch (IllegalArgumentException ex) {
87-
throw new ResponseStatusException(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE,
88-
"Could not parse Range request header", ex);
81+
82+
List<HttpRange> httpRanges = request.getHeaders().getRange();
83+
if (!httpRanges.isEmpty()) {
84+
return Collections.singletonMap(ResourceHttpMessageWriter.HTTP_RANGE_REQUEST_HINT, httpRanges);
8985
}
9086
return Collections.emptyMap();
9187
}
@@ -139,12 +135,31 @@ protected void addHeaders(HttpHeaders headers, Resource resource, MediaType medi
139135
headers.setContentType(mediaType);
140136
}
141137
if (headers.getContentLength() < 0) {
142-
ResourceUtils.contentLength(resource).ifPresent(headers::setContentLength);
138+
contentLength(resource).ifPresent(headers::setContentLength);
143139
}
144140
}
145141

142+
/**
143+
* Determine, if possible, the contentLength of the given resource without reading it.
144+
* @param resource the resource instance
145+
* @return the contentLength of the resource
146+
*/
147+
private OptionalLong contentLength(Resource resource) {
148+
// Don't try to determine contentLength on InputStreamResource - cannot be read afterwards...
149+
// Note: custom InputStreamResource subclasses could provide a pre-calculated content length!
150+
if (InputStreamResource.class != resource.getClass()) {
151+
try {
152+
return OptionalLong.of(resource.contentLength());
153+
}
154+
catch (IOException ignored) {
155+
}
156+
}
157+
return OptionalLong.empty();
158+
}
159+
146160
private Mono<Void> writeContent(Resource resource, ResolvableType type,
147161
ReactiveHttpOutputMessage outputMessage, Map<String, Object> hints) {
162+
148163
if (outputMessage instanceof ZeroCopyHttpOutputMessage) {
149164
Optional<File> file = getFile(resource);
150165
if (file.isPresent()) {

‎spring-web/src/main/java/org/springframework/http/codec/ResourceRegionHttpMessageWriter.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
import org.springframework.core.ResolvableType;
3030
import org.springframework.core.codec.ResourceRegionEncoder;
31+
import org.springframework.core.io.InputStreamResource;
3132
import org.springframework.core.io.Resource;
3233
import org.springframework.core.io.support.ResourceRegion;
3334
import org.springframework.http.MediaType;
@@ -80,7 +81,7 @@ public Mono<Void> write(Publisher<? extends ResourceRegion> inputStream, Resolva
8081
private void writeSingleResourceRegionHeaders(ResourceRegion region, MediaType contentType,
8182
ReactiveHttpOutputMessage outputMessage) {
8283

83-
OptionalLong resourceLength = ResourceUtils.contentLength(region.getResource());
84+
OptionalLong resourceLength = contentLength(region.getResource());
8485
resourceLength.ifPresent(length -> {
8586
long start = region.getPosition();
8687
long end = start + region.getCount() - 1;
@@ -91,6 +92,24 @@ private void writeSingleResourceRegionHeaders(ResourceRegion region, MediaType c
9192
outputMessage.getHeaders().setContentType(contentType);
9293
}
9394

95+
/**
96+
* Determine, if possible, the contentLength of the given resource without reading it.
97+
* @param resource the resource instance
98+
* @return the contentLength of the resource
99+
*/
100+
private OptionalLong contentLength(Resource resource) {
101+
// Don't try to determine contentLength on InputStreamResource - cannot be read afterwards...
102+
// Note: custom InputStreamResource subclasses could provide a pre-calculated content length!
103+
if (InputStreamResource.class != resource.getClass()) {
104+
try {
105+
return OptionalLong.of(resource.contentLength());
106+
}
107+
catch (IOException ignored) {
108+
}
109+
}
110+
return OptionalLong.empty();
111+
}
112+
94113
private Mono<Void> writeResourceRegion(ResourceRegion region,
95114
ResolvableType type, ReactiveHttpOutputMessage outputMessage) {
96115
if (outputMessage instanceof ZeroCopyHttpOutputMessage) {

‎spring-web/src/test/java/org/springframework/http/codec/ResourceHttpMessageWriterTests.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,9 @@
1616

1717
package org.springframework.http.codec;
1818

19-
import static org.hamcrest.Matchers.*;
20-
import static org.junit.Assert.*;
21-
2219
import java.nio.charset.StandardCharsets;
2320
import java.util.Collections;
2421

25-
import org.hamcrest.Matchers;
2622
import org.junit.Before;
2723
import org.junit.Rule;
2824
import org.junit.Test;
@@ -40,16 +36,18 @@
4036
import org.springframework.core.io.buffer.support.DataBufferTestUtils;
4137
import org.springframework.http.HttpHeaders;
4238
import org.springframework.http.HttpRange;
43-
import org.springframework.http.HttpStatus;
4439
import org.springframework.http.MediaType;
4540
import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
4641
import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
4742
import org.springframework.tests.TestSubscriber;
4843
import org.springframework.util.MimeTypeUtils;
49-
import org.springframework.web.server.ResponseStatusException;
44+
45+
import static org.hamcrest.Matchers.*;
46+
import static org.junit.Assert.*;
5047

5148
/**
5249
* Unit tests for {@link ResourceHttpMessageWriter}.
50+
*
5351
* @author Brian Clozel
5452
*/
5553
public class ResourceHttpMessageWriterTests {
@@ -72,6 +70,7 @@ public void setUp() throws Exception {
7270
this.resource = new ByteArrayResource(content.getBytes(StandardCharsets.UTF_8));
7371
}
7472

73+
7574
@Test
7675
public void writableMediaTypes() throws Exception {
7776
assertThat(this.writer.getWritableMediaTypes(),
@@ -80,7 +79,6 @@ public void writableMediaTypes() throws Exception {
8079

8180
@Test
8281
public void shouldWriteResource() throws Exception {
83-
8482
TestSubscriber.subscribe(this.writer.write(Mono.just(resource), null, ResolvableType.forClass(Resource.class),
8583
MediaType.TEXT_PLAIN, this.request, this.response, Collections.emptyMap())).assertComplete();
8684

@@ -93,7 +91,6 @@ public void shouldWriteResource() throws Exception {
9391

9492
@Test
9593
public void shouldWriteResourceRange() throws Exception {
96-
9794
this.request.getHeaders().setRange(Collections.singletonList(HttpRange.createByteRange(0, 5)));
9895

9996
TestSubscriber.subscribe(this.writer.write(Mono.just(resource), null, ResolvableType.forClass(Resource.class),
@@ -111,10 +108,9 @@ public void shouldWriteResourceRange() throws Exception {
111108
public void shouldThrowErrorInvalidRange() throws Exception {
112109
this.request.getHeaders().set(HttpHeaders.RANGE, "invalid");
113110

114-
this.expectedException.expect(ResponseStatusException.class);
111+
this.expectedException.expect(IllegalArgumentException.class);
115112
TestSubscriber.subscribe(this.writer.write(Mono.just(resource), null, ResolvableType.forClass(Resource.class),
116113
MediaType.TEXT_PLAIN, this.request, this.response, Collections.emptyMap()));
117-
this.expectedException.expect(Matchers.hasProperty("status", is(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE)));
118114
}
119115

120116
private Mono<String> reduceToString(Publisher<DataBuffer> buffers, DataBufferFactory bufferFactory) {
@@ -127,4 +123,5 @@ private Mono<String> reduceToString(Publisher<DataBuffer> buffers, DataBufferFac
127123
})
128124
.map(buffer -> DataBufferTestUtils.dumpString(buffer, StandardCharsets.UTF_8));
129125
}
126+
130127
}

0 commit comments

Comments
 (0)