[JENKINS-64261] BaseFileContent.InputStreamSupplier #649
Conversation
| IOUtils.copy(new TruncatedInputStream(is, maxSize), os); | ||
| } | ||
| void writeTo(OutputStream os) throws IOException { | ||
| try (InputStream is = inputStreamSupplier.get()) { |
| try { | ||
| try (InputStream is = inputStreamSupplier.get()) { | ||
| if (maxSize == -1) { | ||
| try (BufferedReader reader = new BufferedReader(new InputStreamReader(is, ENCODING))) { | ||
| String s; | ||
| while ((s = reader.readLine()) != null) { | ||
| String filtered = ContentFilter.filter(filter, secretsFilterFunction.apply(s)) + "\n"; | ||
| IOUtils.write(filtered, os, ENCODING); | ||
| } | ||
| try (InputStream is = inputStreamSupplier.get()) { |
There was a problem hiding this comment.
Double try block seems unnecessary. Dates to #206.
There was a problem hiding this comment.
Correct, could just be included down in the trys in each conditions.
| return getInputStream(); | ||
| } catch (IOException e) { | ||
| LOGGER.log(Level.WARNING, "Error opening file " + file, e); | ||
| return null; |
|
|
||
| /** | ||
| * Utility class with the common logic to FileContent and UnfilteredFileContent. | ||
| * | ||
| * @author Stephen Connolly, M Ramón León | ||
| */ | ||
| @Restricted(NoExternalUse.class) |
There was a problem hiding this comment.
Why was this ever restricted in #174? It was never public.
| @@ -67,27 +69,9 @@ class BaseFileContent { | |||
|
|
|||
| private static final String ENCODING = "UTF-8"; | |||
|
|
|||
| /** | |||
| * @deprecated (as it is placed in the api package we keep backward compatibility, no relevant usage was found) | |||
There was a problem hiding this comment.
Why was this left in place in c6d20da? The class was not exposed outside the package.
|
|
||
| @FunctionalInterface | ||
| interface InputStreamSupplier { | ||
| InputStream get() throws IOException; |
There was a problem hiding this comment.
Letting the caller handle the IOException. #174 appears to have been catching FileNotFoundException for this reason, but that could not actually have been thrown from the body of the try block: other IOExceptions from handling OutputStream, perhaps, but the Supplier permitted no IOException so FileNotFoundException was being caught earlier.
There was a problem hiding this comment.
Thanks Jesse. I think this is https://issues.jenkins.io/browse/JENKINS-64261.
Overall looks good to me.
BaseFileContent.InputStreamSupplier
Noticed in a support bundle several exceptions of the form
This seems to be due to a
Supplier<InputStream>returningnull, which was not expected by the caller. #190 fixed this forFileContentbut notUnfilteredFileContent.