use try-with-resources statement in ResourceRegionHttpMessageConverter#35051
use try-with-resources statement in ResourceRegionHttpMessageConverter#35051Pankraz76 wants to merge 1 commit intospring-projects:mainfrom
try-with-resources statement in ResourceRegionHttpMessageConverter#35051Conversation
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.
|
|
||
| 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
Pankraz76
force-pushed
the
fix-ResourceRegionHttpMessageConverter
branch
from
4373d57 to
b317def
Compare
| println(out); | ||
| println(out); | ||
| // Printing content | ||
| StreamUtils.copyRange(in, out, start, end); |
| 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.
| 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.
| // 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? |
3 participants
before:
now: