Correctly buffer input/output streams for files

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/fs/FileUtils.java

@@ -15,10 +15,15 @@
  */
 package io.github.ascopes.protobufmavenplugin.fs;
 
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystemNotFoundException;
 import java.nio.file.Files;
+import java.nio.file.OpenOption;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.nio.file.attribute.PosixFilePermission;
@@ -146,4 +151,18 @@ public static List<Path> rebaseFileTree(
 
     return Collections.unmodifiableList(newPaths);
   }
+
+  public static InputStream newBufferedInputStream(
+      Path path,
+      OpenOption... options
+  ) throws IOException {
+    return new BufferedInputStream(Files.newInputStream(path, options));
+  }
+
+  public static OutputStream newBufferedOutputStream(
+      Path path,
+      OpenOption... options
+  ) throws IOException {
+    return new BufferedOutputStream(Files.newOutputStream(path, options));
+  }
 }

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/fs/UriResourceFetcher.java

@@ -148,26 +148,29 @@ private Optional<Path> handleOtherUri(URI uri, String extension) throws Resoluti
       conn.connect();
 
       try (
-          var responseStream = new BufferedInputStream(conn.getInputStream());
-          var fileStream = new SizeAwareBufferedOutputStream(Files.newOutputStream(targetFile))
+          var responseInputStream = new BufferedInputStream(conn.getInputStream());
+          var fileOutputStream = FileUtils.newBufferedOutputStream(targetFile)
       ) {
-        responseStream.transferTo(fileStream);
-        log.info("Downloaded '{}' to '{}' ({} bytes)", uri, targetFile, fileStream.size);
+        responseInputStream.transferTo(fileOutputStream);
       }
 
-      return Optional.of(targetFile);
-
-    } catch (FileNotFoundException ex) {
-
-      // May be raised during the call to .getInputStream(), or the call to .connect(),
-      // depending on the implementation.
-      log.warn("No resource at '{}' appears to exist!", uri);
+      var fileSize = Files.size(targetFile);
+      log.info("Downloaded '{}' to '{}' ({} bytes)", uri, fileSize);
 
-      return Optional.empty();
+      return Optional.of(targetFile);
 
     } catch (IOException ex) {
       log.debug("Failed to download '{}' to '{}'", uri, targetFile, ex);
-      throw new ResolutionException("Failed to download '" + uri + "' to '" + targetFile + "'", ex);
+
+      if (ex instanceof FileNotFoundException) {
+        // May be raised during the call to .getInputStream(), or the call to .connect(),
+        // depending on the implementation.
+        log.warn("No resource at '{}' exists", uri);
+        return Optional.empty();
+      } else {
+        throw new ResolutionException(
+            "Failed to download '" + uri + "' to '" + targetFile + "'", ex);
+      }
     }
   }
 
@@ -185,6 +188,11 @@ private Path targetFile(URL url, String extension) {
   }
 
   private URL parseUrlWithAnyHandler(URI uri) throws ResolutionException {
+    // We use ServiceLoader directly for this so that we can load handlers from
+    // other non-system classloaders. URL's internals only consider the default
+    // system/boot classloader, which differs to our runtime classloader that
+    // runs on top of ClassWorlds in Maven, meaning we cannot load custom schemes
+    // via normal mechanisms.
     var customHandler = ServiceLoader
         .load(URLStreamHandlerProvider.class, getClass().getClassLoader())
         .stream()
@@ -202,35 +210,4 @@ private URL parseUrlWithAnyHandler(URI uri) throws ResolutionException {
       throw new ResolutionException("URI '" + uri + "' is invalid: " + ex, ex);
     }
   }
-
-  /**
-   * Buffers an output stream, and keeps track of how many bytes
-   * were written.
-   */
-  private static final class SizeAwareBufferedOutputStream extends OutputStream {
-    private final OutputStream delegate;
-    private long size;
-
-    private SizeAwareBufferedOutputStream(OutputStream delegate) {
-      this.delegate = new BufferedOutputStream(delegate);
-      size = 0;
-    }
-
-    @Override
-    public void close() throws IOException {
-      flush();
-      delegate.close();
-    }
-
-    @Override
-    public void flush() throws IOException {
-      delegate.flush();
-    }
-
-    @Override
-    public void write(int nextByte) throws IOException {
-      ++size;
-      delegate.write(nextByte);
-    }
-  }
 }

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/JvmPluginResolver.java

@@ -258,9 +258,10 @@ private String determineMainClass(
   ) throws ResolutionException {
     try (
         var zip = FileUtils.openZipAsFileSystem(pluginPath);
-        var manifestStream = Files.newInputStream(zip.getPath("META-INF", "MANIFEST.MF"))
+        var manifestInputStream = FileUtils.newBufferedInputStream(
+            zip.getPath("META-INF", "MANIFEST.MF"))
     ) {
-      return new Manifest(manifestStream)
+      return new Manifest(manifestInputStream)
           .getMainAttributes()
           .getValue("Main-Class");
     } catch (IOException ex) {

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/sources/ProtoSourceResolver.java

@@ -98,7 +98,7 @@ private Optional<DescriptorListing> resolveDescriptor(
       return Optional.empty();
     }
 
-    try (var inputStream = Files.newInputStream(descriptorFilePath)) {
+    try (var inputStream = FileUtils.newBufferedInputStream(descriptorFilePath)) {
       return FileDescriptorSet.parseFrom(inputStream)
           .getFileList()
           .stream()

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/sources/incremental/IncrementalCacheManager.java

@@ -15,6 +15,7 @@
  */
 package io.github.ascopes.protobufmavenplugin.sources.incremental;
 
+import io.github.ascopes.protobufmavenplugin.fs.FileUtils;
 import io.github.ascopes.protobufmavenplugin.fs.TemporarySpace;
 import io.github.ascopes.protobufmavenplugin.sources.DescriptorListing;
 import io.github.ascopes.protobufmavenplugin.sources.FilesToCompile;
@@ -198,7 +199,7 @@ private Stream<FutureTask<Map.Entry<Path, String>>> generateDescriptorDigests(
   private FutureTask<Map.Entry<Path, String>> generateFileDigest(Path file) {
     return concurrentExecutor.submit(() -> {
       log.trace("Generating digest for {}", file);
-      try (var inputStream = Files.newInputStream(file)) {
+      try (var inputStream = FileUtils.newBufferedInputStream(file)) {
         var digest = Digests.sha512ForStream(inputStream);
         return Map.entry(file, digest);
       }

protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/fs/FileUtilsTest.java

@@ -20,6 +20,10 @@
 import static org.assertj.core.api.Assumptions.assumeThat;
 
 import io.github.ascopes.protobufmavenplugin.fixtures.TestFileSystem;
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -252,4 +256,42 @@ void rebaseFileTreeCopiesAllNodesBetweenFileSystems() throws IOException {
           );
     }
   }
+
+  @DisplayName(".newBufferedInputStream(Path, OpenOption... reads a file with a buffer")
+  @Test
+  void newBufferedInputStreamReadsFileWithBuffer(@TempDir Path dir) throws IOException {
+    // Given
+    var file = dir.resolve("file.txt");
+    Files.writeString(file, "Hello, World!");
+
+    // When
+    var os = new ByteArrayOutputStream();
+    try (var is = FileUtils.newBufferedInputStream(file)) {
+      assertThat(is).isInstanceOf(BufferedInputStream.class);
+      is.transferTo(os);
+    }
+
+    // Then
+    assertThat(os).asString().isEqualTo("Hello, World!");
+  }
+
+  @DisplayName(".newBufferedOutputStream(Path, OpenOption... writes a file with a buffer")
+  @Test
+  void newBufferedOutputStreamWritesFileWithBuffer(@TempDir Path dir) throws IOException {
+    // Given
+    var file = dir.resolve("file.txt");
+
+    // When
+    var is = new ByteArrayInputStream("Hello, World!".getBytes());
+    try (var os = FileUtils.newBufferedOutputStream(file)) {
+      assertThat(os).isInstanceOf(BufferedOutputStream.class);
+      is.transferTo(os);
+    }
+
+    // Then
+    assertThat(file)
+        .isRegularFile()
+        .content()
+        .isEqualTo("Hello, World!");
+  }
 }