SOLR-18320 follow-up: streaming zipDirectory, drop dead passthrough wrapper

solr/core/src/java/org/apache/solr/cli/CreateTool.java

@@ -346,7 +346,7 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para
               + " using the Configsets V2 API");
       var uploadReq =
           new ConfigsetsApi.UploadConfigSet(
-              confName, new ByteArrayInputStream(zipConfigSet(confPath)));
+              confName, new ByteArrayInputStream(Utils.zipDirectory(confPath)));
       uploadReq.process(cloudSolrClient);
     }
 
@@ -388,11 +388,6 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para
     echo(endMessage);
   }
 
-  /** Zips the contents of a configset directory for upload. */
-  static byte[] zipConfigSet(Path confPath) throws IOException {
-    return Utils.zipDirectory(confPath);
-  }
-
   private Path getFullConfDir(Path solrInstallDir, Path confDirName) {
     return CLIUtils.getConfigSetsDir(solrInstallDir).resolve(confDirName);
   }

solr/core/src/test/org/apache/solr/cli/CreateToolTest.java

@@ -18,13 +18,17 @@
 package org.apache.solr.cli;
 
 import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.util.Utils;
 import org.apache.solr.util.SecurityJson;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -96,20 +100,31 @@ public void testZipConfigSetSkipsHiddenFilesAndIncludesDirectoryEntries() throws
     Path hiddenDir = Files.createDirectory(confDir.resolve(".hiddenDir"));
     Files.writeString(hiddenDir.resolve("nope.txt"), "should not be zipped either");
 
-    byte[] zipBytes = CreateTool.zipConfigSet(confDir);
+    byte[] zipBytes = Utils.zipDirectory(confDir);
+    Map<String, String> entries = readZipEntries(zipBytes);
 
-    Set<String> entryNames = new HashSet<>();
+    assertTrue(entries.containsKey("solrconfig.xml"));
+    assertTrue(entries.containsKey("lang/"));
+    assertTrue(entries.containsKey("lang/stopwords.txt"));
+    assertFalse(entries.containsKey(".hidden-file"));
+    assertTrue(entries.keySet().stream().noneMatch(name -> name.startsWith(".hiddenDir")));
+
+    // Equivalent, not necessarily byte-identical: ZipEntry defaults to the current time
+    // when unset, so two back-to-back calls can differ at the byte level.
+    ByteArrayOutputStream streamed = new ByteArrayOutputStream();
+    Utils.zipDirectory(confDir, streamed);
+    assertEquals(entries, readZipEntries(streamed.toByteArray()));
+  }
+
+  /** Maps each zip entry name to its contents (directory entries map to the empty string). */
+  private static Map<String, String> readZipEntries(byte[] zipBytes) throws IOException {
+    Map<String, String> entries = new HashMap<>();
     try (ZipInputStream zipIn = new ZipInputStream(new ByteArrayInputStream(zipBytes))) {
       ZipEntry entry;
       while ((entry = zipIn.getNextEntry()) != null) {
-        entryNames.add(entry.getName());
+        entries.put(entry.getName(), new String(zipIn.readAllBytes(), StandardCharsets.UTF_8));
       }
     }
-
-    assertTrue(entryNames.contains("solrconfig.xml"));
-    assertTrue(entryNames.contains("lang/"));
-    assertTrue(entryNames.contains("lang/stopwords.txt"));
-    assertFalse(entryNames.contains(".hidden-file"));
-    assertTrue(entryNames.stream().noneMatch(name -> name.startsWith(".hiddenDir")));
+    return entries;
   }
 }

solr/solrj/src/java/org/apache/solr/common/util/Utils.java

@@ -1064,41 +1064,55 @@ public static ByteBuffer toByteArray(InputStream is) throws IOException {
    */
   public static byte[] zipDirectory(Path rootPath) throws IOException {
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    try (ZipOutputStream zipOut = new ZipOutputStream(baos)) {
-      Files.walkFileTree(
-          rootPath,
-          new SimpleFileVisitor<>() {
-            @Override
-            public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
-                throws IOException {
-              if (Files.isHidden(dir)) {
-                return FileVisitResult.SKIP_SUBTREE;
-              }
-              String dirName = rootPath.relativize(dir).toString().replace('\\', '/');
-              if (!dirName.isEmpty()) {
-                if (!dirName.endsWith("/")) {
-                  dirName += "/";
-                }
-                zipOut.putNextEntry(new ZipEntry(dirName));
-                zipOut.closeEntry();
-              }
-              return FileVisitResult.CONTINUE;
-            }
+    zipDirectory(rootPath, baos);
+    return baos.toByteArray();
+  }
 
-            @Override
-            public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
-                throws IOException {
-              if (Files.isHidden(file)) {
-                return FileVisitResult.CONTINUE;
+  /**
+   * Zips the contents of {@code rootPath}, writing the archive to {@code out} instead of buffering
+   * it in memory. See {@link #zipDirectory(Path)} for the skipped/normalized entry rules. Does not
+   * close {@code out}.
+   *
+   * @param rootPath the directory to zip
+   * @param out the stream to write the zip archive to
+   */
+  public static void zipDirectory(Path rootPath, OutputStream out) throws IOException {
+    ZipOutputStream zipOut = new ZipOutputStream(out);
+    Files.walkFileTree(
+        rootPath,
+        new SimpleFileVisitor<>() {
+          @Override
+          public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
+              throws IOException {
+            if (Files.isHidden(dir)) {
+              return FileVisitResult.SKIP_SUBTREE;
+            }
+            String dirName = rootPath.relativize(dir).toString().replace('\\', '/');
+            if (!dirName.isEmpty()) {
+              if (!dirName.endsWith("/")) {
+                dirName += "/";
               }
-              String entryName = rootPath.relativize(file).toString().replace('\\', '/');
-              zipOut.putNextEntry(new ZipEntry(entryName));
-              Files.copy(file, zipOut);
+              zipOut.putNextEntry(new ZipEntry(dirName));
               zipOut.closeEntry();
+            }
+            return FileVisitResult.CONTINUE;
+          }
+
+          @Override
+          public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+              throws IOException {
+            if (Files.isHidden(file)) {
               return FileVisitResult.CONTINUE;
             }
-          });
-    }
-    return baos.toByteArray();
+            String entryName = rootPath.relativize(file).toString().replace('\\', '/');
+            zipOut.putNextEntry(new ZipEntry(entryName));
+            Files.copy(file, zipOut);
+            zipOut.closeEntry();
+            return FileVisitResult.CONTINUE;
+          }
+        });
+    // finish(), not close(): flushes the central directory without closing out, which this
+    // method doesn't own.
+    zipOut.finish();
   }
 }

solr/test-framework/src/java/org/apache/solr/cloud/RemoteSolrBackend.java

@@ -17,6 +17,7 @@
 package org.apache.solr.cloud;
 
 import java.io.IOException;
+import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
@@ -67,7 +68,9 @@ public SolrClient getSolrClient() {
   public void uploadConfigSet(Path configDir, String name) throws SolrServerException, IOException {
     Path tempZip = Files.createTempFile("configset-", ".zip");
     try {
-      Files.write(tempZip, Utils.zipDirectory(configDir));
+      try (OutputStream out = Files.newOutputStream(tempZip)) {
+        Utils.zipDirectory(configDir, out);
+      }
       new ConfigSetAdminRequest.Upload()
           .setConfigSetName(name)
           .setUploadFile(tempZip, "application/zip")