use try-with-resources statement in ResourceRegionHttpMessageConverter
#35051
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
Pankraz76
force-pushed
the
fix-ResourceRegionHttpMessageConverter
branch
from
39e24fc to
746ba02
Compare
| } | ||
| finally { | ||
| try { | ||
| in.close(); |
There was a problem hiding this comment.
these is no special handling at all.
we still ignore IO.
| @@ -173,19 +173,10 @@ protected void writeResourceRegion(ResourceRegion region, HttpOutputMessage outp | |||
| responseHeaders.add("Content-Range", "bytes " + start + '-' + end + '/' + resourceLength); | |||
| responseHeaders.setContentLength(rangeLength); | |||
|
|
|||
| InputStream in = region.getResource().getInputStream(); | |||
| // We cannot use try-with-resources here for the InputStream, since we have | |||
| // custom handling of the close() method in a finally-block. | |||
There was a problem hiding this comment.
these is no special handling at all.
we still ignore IO.
Pankraz76
force-pushed
the
fix-ResourceRegionHttpMessageConverter
branch
from
746ba02 to
4373d57
Compare
...ebsocket/src/main/java/org/springframework/web/socket/messaging/StompSubProtocolHandler.java
Outdated
Show resolved
Hide resolved
Pankraz76
force-pushed
the
fix-ResourceRegionHttpMessageConverter
branch
from
4373d57 to
b317def
Compare
| print(out, "Content-Range: bytes " + | ||
| region.getPosition() + '-' + (region.getPosition() + region.getCount() - 1) + | ||
| '/' + resourceLength); | ||
| println(out); | ||
| println(out); | ||
| // Printing content | ||
| StreamUtils.copyRange(in, out, start, end); |
| @@ -227,14 +219,14 @@ private void writeResourceRegionCollection(Collection<ResourceRegion> resourceRe | |||
| println(out); | |||
| } | |||
| long resourceLength = region.getResource().contentLength(); | |||
| end = Math.min(end, resourceLength - inputStreamPosition - 1); | |||
There was a problem hiding this comment.
min,max,modulo its all used very often from Math of course, as this is the happy path all around.
| print(out, "Content-Range: bytes " + | ||
| region.getPosition() + '-' + (region.getPosition() + region.getCount() - 1) + | ||
| '/' + resourceLength); | ||
| println(out); | ||
| println(out); | ||
| // Printing content | ||
| StreamUtils.copyRange(in, out, start, end); | ||
| copyRange(in, out, start, end); |
There was a problem hiding this comment.
its random impl. detail where this comes from we want to copyRange who how or what seems different concern.
...web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java
Show resolved
Hide resolved
| // custom handling of the close() method in a finally-block. | ||
| try { | ||
| StreamUtils.copyRange(in, outputMessage.getBody(), start, end); | ||
| try (var in = region.getResource().getInputStream()) { |
There was a problem hiding this comment.
could see type as random impl. detail, as well.
Signed-off-by: Vincent Potucek <[email protected]>
Pankraz76
force-pushed
the
fix-ResourceRegionHttpMessageConverter
branch
from
b317def to
af07e87
Compare
|
Sorry but we cannot process PRs like this. Its title says one thing but the content is all over the place, mixing stylistic changes, new attempts at introducing try-with-resources (and some changing the behavior again), a dozen comments and screenshots. Please be more considerate of our time and stop submitting similar PRs. |
bclozel
added
status: declined
|
yes im sorry. will give test for the catch as im interested now. dont understand so tests will help. can we merge test then at least to improve cover? |
before:
now: