Use ArchiveStreamReader in configserver

2 files changed, 86 insertions, 96 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java
index 0672f13fd6a..30fbfd48f78 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java
@@ -1,23 +1,21 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.application;
 
-import com.google.common.io.ByteStreams;
+import com.yahoo.compress.ArchiveStreamReader;
+import com.yahoo.compress.ArchiveStreamReader.Options;
+import com.yahoo.vespa.config.server.http.BadRequestException;
+import com.yahoo.vespa.config.server.http.InternalServerException;
+import com.yahoo.vespa.config.server.http.v2.ApplicationApiHandler;
 
 import java.io.File;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.logging.Level;
-import com.yahoo.vespa.config.server.http.BadRequestException;
-import com.yahoo.vespa.config.server.http.InternalServerException;
-import com.yahoo.vespa.config.server.http.v2.ApplicationApiHandler;
-import org.apache.commons.compress.archivers.ArchiveEntry;
-import org.apache.commons.compress.archivers.ArchiveInputStream;
-import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
-import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream;
-
 import java.util.logging.Logger;
-import java.util.zip.GZIPInputStream;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
 
@@ -29,7 +27,12 @@ import static com.yahoo.yolean.Exceptions.uncheck;
 public class CompressedApplicationInputStream implements AutoCloseable {
 
     private static final Logger log = Logger.getLogger(CompressedApplicationInputStream.class.getPackage().getName());
-    private final ArchiveInputStream ais;
+
+    private final ArchiveStreamReader reader;
+
+    private CompressedApplicationInputStream(ArchiveStreamReader reader) {
+        this.reader = reader;
+    }
 
     /**
      * Create an instance of a compressed application from an input stream.
@@ -40,42 +43,25 @@ public class CompressedApplicationInputStream implements AutoCloseable {
      */
     public static CompressedApplicationInputStream createFromCompressedStream(InputStream is, String contentType) {
         try {
-            ArchiveInputStream ais = getArchiveInputStream(is, contentType);
-            return createFromCompressedStream(ais);
-        } catch (IOException e) {
+            switch (contentType) {
+                case ApplicationApiHandler.APPLICATION_X_GZIP:
+                    return new CompressedApplicationInputStream(ArchiveStreamReader.ofTarGzip(is, Options.standard()));
+                case ApplicationApiHandler.APPLICATION_ZIP:
+                    return new CompressedApplicationInputStream(ArchiveStreamReader.ofZip(is, Options.standard()));
+                default:
+                    throw new BadRequestException("Unable to decompress");
+            }
+        } catch (UncheckedIOException e) {
             throw new InternalServerException("Unable to create compressed application stream", e);
         }
     }
 
-    static CompressedApplicationInputStream createFromCompressedStream(ArchiveInputStream ais) {
-        return new CompressedApplicationInputStream(ais);
-    }
-
-    private static ArchiveInputStream getArchiveInputStream(InputStream is, String contentTypeHeader) throws IOException {
-        ArchiveInputStream ais;
-        switch (contentTypeHeader) {
-            case ApplicationApiHandler.APPLICATION_X_GZIP:
-                ais = new TarArchiveInputStream(new GZIPInputStream(is));
-                break;
-            case ApplicationApiHandler.APPLICATION_ZIP:
-                ais = new ZipArchiveInputStream(is);
-                break;
-            default:
-                throw new BadRequestException("Unable to decompress");
-        }
-        return ais;
-    }
-
-    private CompressedApplicationInputStream(ArchiveInputStream ais) {
-        this.ais = ais;
-    }
-
     /**
      * Close this stream.
      * @throws IOException if the stream could not be closed
      */
     public void close() throws IOException {
-        ais.close();
+        reader.close();
     }
 
     File decompress() throws IOException {
@@ -83,45 +69,44 @@ public class CompressedApplicationInputStream implements AutoCloseable {
     }
 
     public File decompress(File dir) throws IOException {
-        decompressInto(dir);
+        decompressInto(dir.toPath());
         dir = findActualApplicationDir(dir);
         return dir;
     }
 
-    private void decompressInto(File application) throws IOException {
-        log.log(Level.FINE, () -> "Application is in " + application.getAbsolutePath());
+    private void decompressInto(Path dir) throws IOException {
+        if (!Files.isDirectory(dir)) throw new IllegalArgumentException("Not a directory: " + dir.toAbsolutePath());
+        log.log(Level.FINE, () -> "Application is in " + dir.toAbsolutePath());
         int entries = 0;
-        ArchiveEntry entry;
-        while ((entry = ais.getNextEntry()) != null) {
-            log.log(Level.FINE, "Unpacking %s", entry.getName());
-            File outFile = new File(application, entry.getName());
-            // FIXME/TODO: write more tests that break this logic. I have a feeling it is not very robust.
-            if (entry.isDirectory()) {
-                if (!(outFile.exists() && outFile.isDirectory())) {
-                    log.log(Level.FINE, () -> "Creating dir: " + outFile.getAbsolutePath());
-                    boolean res = outFile.mkdirs();
-                    if (!res) {
-                        log.log(Level.WARNING, "Could not create dir " + entry.getName());
-                    }
-                }
-            } else {
-                log.log(Level.FINE, () -> "Creating output file: " + outFile.getAbsolutePath());
-
-                // Create parent dir if necessary
-                String parent = outFile.getParent();
-                new File(parent).mkdirs();
-
-                FileOutputStream fos = new FileOutputStream(outFile);
-                ByteStreams.copy(ais, fos);
-                fos.close();
+        Path tmpFile = null;
+        OutputStream tmpStream = null;
+        try {
+            tmpFile = createTempFile(dir);
+            tmpStream = Files.newOutputStream(tmpFile);
+            ArchiveStreamReader.ArchiveFile file;
+            while ((file = reader.readNextTo(tmpStream)) != null) {
+                tmpStream.close();
+                log.log(Level.FINE, "Creating output file: " + file.path());
+                Path dstFile = dir.resolve(file.path().toString());
+                Files.createDirectories(dstFile.getParent());
+                Files.move(tmpFile, dstFile);
+                tmpFile = createTempFile(dir);
+                tmpStream = Files.newOutputStream(tmpFile);
+                entries++;
             }
-            entries++;
+        } finally {
+            if (tmpStream != null) tmpStream.close();
+            if (tmpFile != null) Files.deleteIfExists(tmpFile);
         }
         if (entries == 0) {
-            log.log(Level.WARNING, "Not able to read any entries from " + application.getName());
+            log.log(Level.WARNING, "Not able to decompress any entries to " + dir);
         }
     }
 
+    private static Path createTempFile(Path applicationDir) throws IOException {
+        return Files.createTempFile(applicationDir, "application", null);
+    }
+
     private File findActualApplicationDir(File application) {
         // If application is in e.g. application/, use that as root for UnpackedApplication
         // TODO: Vespa 8: Remove application/ directory support
@@ -131,4 +116,5 @@ public class CompressedApplicationInputStream implements AutoCloseable {
         }
         return application;
     }
+
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java
index 23444ac53d6..e8c7cff4e75 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java
@@ -2,10 +2,10 @@
 package com.yahoo.vespa.config.server.application;
 
 import com.google.common.io.ByteStreams;
+import com.yahoo.vespa.config.server.http.InternalServerException;
+import com.yahoo.yolean.Exceptions;
 import org.apache.commons.compress.archivers.ArchiveOutputStream;
-import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
 import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
-import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream;
 import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
 import org.junit.Test;
 
@@ -15,11 +15,10 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
-import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -64,7 +63,7 @@ public class CompressedApplicationInputStreamTest {
     @Test
     public void require_that_valid_tar_application_can_be_unpacked() throws IOException {
         File outFile = createTarFile();
-        CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(outFile))));
+        CompressedApplicationInputStream unpacked = streamFromTarGz(outFile);
         File outApp = unpacked.decompress();
         assertTestApp(outApp);
     }
@@ -91,7 +90,7 @@ public class CompressedApplicationInputStreamTest {
 
         archiveOutputStream.close();
 
-        CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(outFile))));
+        CompressedApplicationInputStream unpacked = streamFromTarGz(outFile);
         File outApp = unpacked.decompress();
         assertEquals("application", outApp.getName()); // gets the name of the subdir
         assertTestApp(outApp);
@@ -100,38 +99,25 @@ public class CompressedApplicationInputStreamTest {
     @Test
     public void require_that_valid_zip_application_can_be_unpacked() throws IOException {
         File outFile = createZipFile();
-        CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(
-                new ZipArchiveInputStream(new FileInputStream(outFile)));
+        CompressedApplicationInputStream unpacked = streamFromZip(outFile);
         File outApp = unpacked.decompress();
         assertTestApp(outApp);
     }
 
     @Test
     public void require_that_gnu_tared_file_can_be_unpacked() throws IOException, InterruptedException {
-        File tmpTar = File.createTempFile("myapp", ".tar");
-        Process p = new ProcessBuilder("tar", "-C", "src/test/resources/deploy/validapp", "--exclude=.svn", "-cvf", tmpTar.getAbsolutePath(), ".").start();
-        p.waitFor();
-        p = new ProcessBuilder("gzip", tmpTar.getAbsolutePath()).start();
-        p.waitFor();
-        File gzFile = new File(tmpTar.getAbsolutePath() + ".gz");
+        File gzFile = createTarGz("src/test/resources/deploy/validapp");
         assertTrue(gzFile.exists());
-        CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(
-                new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(gzFile))));
+        CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(new FileInputStream(gzFile), "application/x-gzip");
         File outApp = unpacked.decompress();
         assertTestApp(outApp);
     }
 
     @Test
     public void require_that_nested_app_can_be_unpacked() throws IOException, InterruptedException {
-        File tmpTar = File.createTempFile("myapp", ".tar");
-        Process p = new ProcessBuilder("tar", "-C", "src/test/resources/deploy/advancedapp", "--exclude=.svn", "-cvf", tmpTar.getAbsolutePath(), ".").start();
-        p.waitFor();
-        p = new ProcessBuilder("gzip", tmpTar.getAbsolutePath()).start();
-        p.waitFor();
-        File gzFile = new File(tmpTar.getAbsolutePath() + ".gz");
+        File gzFile = createTarGz("src/test/resources/deploy/advancedapp");
         assertTrue(gzFile.exists());
-        CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(
-                new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(gzFile))));
+        CompressedApplicationInputStream unpacked = streamFromTarGz(gzFile);
         File outApp = unpacked.decompress();
         List<File> files = Arrays.asList(outApp.listFiles());
         assertEquals(5, files.size());
@@ -164,11 +150,29 @@ public class CompressedApplicationInputStreamTest {
         assertEquals(new File(bar, "lol").getAbsolutePath(), bar.listFiles()[0].getAbsolutePath());
     }
 
-
-    @Test(expected = IOException.class)
-    public void require_that_invalid_application_returns_error_when_unpacked() throws IOException {
+    @Test(expected = InternalServerException.class)
+    public void require_that_invalid_application_returns_error_when_unpacked() {
         File app = new File("src/test/resources/deploy/validapp/services.xml");
-        CompressedApplicationInputStream.createFromCompressedStream(
-                new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(app))));
+        streamFromTarGz(app);
     }
+
+    private static File createTarGz(String appDir) throws IOException, InterruptedException {
+        File tmpTar = File.createTempFile("myapp", ".tar");
+        Process p = new ProcessBuilder("tar", "-C", appDir, "--exclude=.svn", "--strip-components=1", "-cvf", tmpTar.getAbsolutePath(), ".").start();
+        p.waitFor();
+        p = new ProcessBuilder("gzip", tmpTar.getAbsolutePath()).start();
+        p.waitFor();
+        File gzFile = new File(tmpTar.getAbsolutePath() + ".gz");
+        assertTrue(gzFile.exists());
+        return gzFile;
+    }
+
+    private static CompressedApplicationInputStream streamFromZip(File zipFile) {
+        return Exceptions.uncheck(() -> CompressedApplicationInputStream.createFromCompressedStream(new FileInputStream(zipFile), "application/zip"));
+    }
+
+    private static CompressedApplicationInputStream streamFromTarGz(File tarFile) {
+        return Exceptions.uncheck(() -> CompressedApplicationInputStream.createFromCompressedStream(new FileInputStream(tarFile), "application/x-gzip"));
+    }
+
 }