From 4a576fcff095ddc2ef79af42e73445eb9456b0ae Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Wed, 16 Jun 2021 12:41:49 +0200
Subject: [PATCH] Tar archive generation: handled directory structure

---
 .../ia2/transfer/persistence/FileDAO.java     |  6 +-
 .../transfer/persistence/model/FileInfo.java  |  9 +++
 .../ia2/transfer/service/ArchiveService.java  | 45 ++++++++++++--
 .../transfer/service/ArchiveServiceTest.java  | 61 +++++++++++++++++--
 4 files changed, 111 insertions(+), 10 deletions(-)

diff --git a/src/main/java/it/inaf/ia2/transfer/persistence/FileDAO.java b/src/main/java/it/inaf/ia2/transfer/persistence/FileDAO.java
index 6a22024..9f5aa85 100644
--- a/src/main/java/it/inaf/ia2/transfer/persistence/FileDAO.java
+++ b/src/main/java/it/inaf/ia2/transfer/persistence/FileDAO.java
@@ -42,7 +42,7 @@ public class FileDAO {
                 + "content_type, content_encoding, content_length, content_md5,\n"
                 + "accept_views, provide_views, l.location_type, n.path <> n.relative_path AS virtual_parent,\n"
                 + "(SELECT user_name FROM users WHERE user_id = creator_id) AS username,\n"
-                + "base_path, get_os_path(n.node_id) AS os_path, ? AS vos_path\n"
+                + "base_path, get_os_path(n.node_id) AS os_path, ? AS vos_path, false AS is_directory\n"
                 + "FROM node n\n"
                 + "JOIN location l ON (n.location_id IS NOT NULL AND n.location_id = l.location_id) OR (n.location_id IS NULL AND l.location_id = ?)\n"
                 + "LEFT JOIN storage s ON s.storage_id = l.storage_dest_id\n"
@@ -132,7 +132,8 @@ public class FileDAO {
                 + "content_type, content_encoding, content_length, content_md5,\n"
                 + "accept_views, provide_views, l.location_type, n.path <> n.relative_path AS virtual_parent,\n"
                 + "(SELECT user_name FROM users WHERE user_id = creator_id) AS username,\n"
-                + "base_path, get_os_path(n.node_id) AS os_path, get_vos_path(n.node_id) AS vos_path\n"
+                + "base_path, get_os_path(n.node_id) AS os_path, get_vos_path(n.node_id) AS vos_path,\n"
+                + "type = 'container' AS is_directory\n"
                 + "FROM node n\n"
                 + "JOIN location l ON (n.location_id IS NOT NULL AND n.location_id = l.location_id) OR (n.location_id IS NULL AND l.location_id = ?)\n"
                 + "LEFT JOIN storage s ON s.storage_id = l.storage_dest_id\n"
@@ -172,6 +173,7 @@ public class FileDAO {
         fi.setContentLength(rs.getLong("content_length"));
         fi.setContentMd5(rs.getString("content_md5"));
         fi.setContentType(rs.getString("content_type"));
+        fi.setDirectory(rs.getBoolean("is_directory"));
 
         fillOsPath(fi, rs);
 
diff --git a/src/main/java/it/inaf/ia2/transfer/persistence/model/FileInfo.java b/src/main/java/it/inaf/ia2/transfer/persistence/model/FileInfo.java
index 3fb8d09..c27f8cd 100644
--- a/src/main/java/it/inaf/ia2/transfer/persistence/model/FileInfo.java
+++ b/src/main/java/it/inaf/ia2/transfer/persistence/model/FileInfo.java
@@ -14,6 +14,7 @@ public class FileInfo {
     private String virtualPath;
     private boolean isPublic;
     private boolean virtualParent;
+    private boolean directory;
     private List<String> groupRead;
     private List<String> groupWrite;
     private String ownerId;
@@ -89,6 +90,14 @@ public class FileInfo {
         this.isPublic = isPublic;
     }
 
+    public boolean isDirectory() {
+        return directory;
+    }
+
+    public void setDirectory(boolean directory) {
+        this.directory = directory;
+    }
+
     public boolean hasVirtualParent() {
         return virtualParent;
     }
diff --git a/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java b/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java
index 8ff8c96..b71db9f 100644
--- a/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java
+++ b/src/main/java/it/inaf/ia2/transfer/service/ArchiveService.java
@@ -14,6 +14,8 @@ import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Files;
+import java.util.List;
 import org.kamranzafar.jtar.TarEntry;
 import org.kamranzafar.jtar.TarOutputStream;
 import org.slf4j.Logger;
@@ -21,6 +23,7 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Service;
+import org.springframework.util.FileSystemUtils;
 
 @Service
 public class ArchiveService {
@@ -66,12 +69,23 @@ public class ArchiveService {
                 throw new IllegalStateException("Unable to create file " + archiveFile.getAbsolutePath());
             }
 
+            String commonParent = getCommonParent(job.getVosPaths());
+            // support directory used to generate folder inside tar files (path is redefined each time by TarEntry class)
+            File supportDir = Files.createTempDirectory("dir").toFile();
+
             try ( TarOutputStream tos = new TarOutputStream(
                     new BufferedOutputStream(new FileOutputStream(archiveFile)))) {
 
                 for (FileInfo fileInfo : fileDAO.getArchiveFileInfos(job.getVosPaths())) {
-                    // TODO: handle different locations
 
+                    String relPath = fileInfo.getVirtualPath().substring(commonParent.length());
+
+                    if (fileInfo.isDirectory()) {
+                        tos.putNextEntry(new TarEntry(supportDir, relPath));
+                        continue;
+                    }
+
+                    // TODO: handle different locations
                     if (!authorizationService.isDownloadable(fileInfo, job.getPrincipal())) {
                         // TODO: proper exception type
                         throw new RuntimeException("Unauthorized");
@@ -79,8 +93,10 @@ public class ArchiveService {
 
                     File file = new File(fileInfo.getOsPath());
                     LOG.trace("Adding file " + file.getAbsolutePath() + " to tar archive");
-                    writeFileIntoTarArchive(file, tos);
+                    writeFileIntoTarArchive(file, relPath, tos);
                 }
+            } finally {
+                FileSystemUtils.deleteRecursively(supportDir);
             }
             // TODO: update job status
 
@@ -89,8 +105,29 @@ public class ArchiveService {
         }
     }
 
-    private void writeFileIntoTarArchive(File file, TarOutputStream tos) throws IOException {
-        TarEntry tarEntry = new TarEntry(file, file.getName());
+    private String getCommonParent(List<String> vosPaths) {
+        String commonParent = null;
+        for (String vosPath : vosPaths) {
+            if (commonParent == null) {
+                commonParent = vosPath;
+            } else {
+                StringBuilder newCommonParent = new StringBuilder();
+                boolean same = true;
+                for (int i = 0; same && i < Math.min(commonParent.length(), vosPath.length()); i++) {
+                    if (commonParent.charAt(i) == vosPath.charAt(i)) {
+                        newCommonParent.append(commonParent.charAt(i));
+                    } else {
+                        same = false;
+                    }
+                }
+                commonParent = newCommonParent.toString();
+            }
+        }
+        return commonParent;
+    }
+
+    private void writeFileIntoTarArchive(File file, String path, TarOutputStream tos) throws IOException {
+        TarEntry tarEntry = new TarEntry(file, path);
 
         try ( InputStream is = new FileInputStream(file)) {
             tos.putNextEntry(tarEntry);
diff --git a/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java b/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java
index 09aadff..8852142 100644
--- a/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java
+++ b/src/test/java/it/inaf/ia2/transfer/service/ArchiveServiceTest.java
@@ -9,16 +9,21 @@ import it.inaf.ia2.transfer.auth.TokenPrincipal;
 import it.inaf.ia2.transfer.persistence.FileDAO;
 import it.inaf.ia2.transfer.persistence.model.FileInfo;
 import java.io.File;
+import java.io.FileInputStream;
 import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import org.junit.jupiter.api.AfterAll;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.kamranzafar.jtar.TarEntry;
+import org.kamranzafar.jtar.TarInputStream;
 import static org.mockito.ArgumentMatchers.any;
 import org.mockito.Mock;
 import static org.mockito.Mockito.when;
@@ -59,6 +64,8 @@ public class ArchiveServiceTest {
     @Test
     public void testTarGeneration() throws Exception {
 
+        String parent = "/path/to";
+
         File tmpParent = tmpDir.toPath().resolve("test1").toFile();
         File file1 = createFile(tmpParent, "dir1/a/b/file1");
         File file2 = createFile(tmpParent, "dir1/a/b/file2");
@@ -71,21 +78,65 @@ public class ArchiveServiceTest {
         job.setPrincipal(new TokenPrincipal("user123", "token123"));
         job.setJobId("abcdef");
         job.setType(ArchiveJob.Type.TAR);
-        job.setVosPaths(Arrays.asList("/path/to/file6"));//"/path/to/dir1", "/path/to/dir2",
+        job.setVosPaths(Arrays.asList(parent + "/dir1", parent + "/dir2", parent + "/file6"));
 
         when(authorizationService.isDownloadable(any(), any())).thenReturn(true);
 
         List<FileInfo> fileInfos = new ArrayList<>();
-        FileInfo fileInfo = new FileInfo();
-        fileInfo.setOsPath(file6.getAbsolutePath());
-        fileInfos.add(fileInfo);
+        addFileInfo(fileInfos, parent + "/file6", file6);
+        addDirInfo(fileInfos, parent + "/dir1");
+        addDirInfo(fileInfos, parent + "/dir1/a");
+        addDirInfo(fileInfos, parent + "/dir1/a/b");
+        addFileInfo(fileInfos, parent + "/dir1/a/b/file1", file1);
+        addFileInfo(fileInfos, parent + "/dir1/a/b/file2", file2);
+        addDirInfo(fileInfos, parent + "/dir2");
+        addDirInfo(fileInfos, parent + "/dir2/c");
+        addFileInfo(fileInfos, parent + "/dir2/c/file3", file3);
+        addFileInfo(fileInfos, parent + "/dir2/c/file4", file4);
+        addDirInfo(fileInfos, parent + "/dir2/c/d");
+        addFileInfo(fileInfos, parent + "/dir2/c/d/file5", file5);
 
         when(fileDAO.getArchiveFileInfos(any())).thenReturn(fileInfos);
 
         archiveService.createArchive(job);
 
         File result = tmpDir.toPath().resolve("user123").resolve("abcdef.tar").toFile();
+
         assertTrue(result.exists());
+
+        // verify result structure
+        List<String> expectedSequence = Arrays.asList("file6", "dir1/", "dir1/a/", "dir1/a/b/",
+                "dir1/a/b/file1", "dir1/a/b/file2", "dir2/", "dir2/c/", "dir2/c/file3", "dir2/c/file4",
+                "dir2/c/d/", "dir2/c/d/file5");
+
+        int i = 0;
+        try ( TarInputStream tis = new TarInputStream(new FileInputStream(result))) {
+            TarEntry entry;
+            while ((entry = tis.getNextEntry()) != null) {
+                assertFalse(i >= expectedSequence.size(), "Found more entries than in expected sequence");
+                assertEquals(expectedSequence.get(i), entry.getName());
+                if (!entry.isDirectory()) {
+                    assertEquals("some data", new String(tis.readAllBytes()));
+                }
+                i++;
+            }
+        }
+
+        assertFalse(i < expectedSequence.size(), "Found less entries than in expected sequence");
+    }
+
+    private void addFileInfo(List<FileInfo> fileInfos, String vosPath, File file) {
+        FileInfo fileInfo = new FileInfo();
+        fileInfo.setOsPath(file.getAbsolutePath());
+        fileInfo.setVirtualPath(vosPath);
+        fileInfos.add(fileInfo);
+    }
+
+    private void addDirInfo(List<FileInfo> fileInfos, String vosPath) {
+        FileInfo fileInfo = new FileInfo();
+        fileInfo.setVirtualPath(vosPath);
+        fileInfo.setDirectory(true);
+        fileInfos.add(fileInfo);
     }
 
     private File createFile(File parent, String path) throws Exception {
@@ -94,6 +145,8 @@ public class ArchiveServiceTest {
         for (int i = 0; i < files.length; i++) {
             File file = parent.toPath().resolve(files[i]).toFile();
             if (i == files.length - 1) {
+                // test os_path different from vos_path
+                file.renameTo(file.getParentFile().toPath().resolve(file.getName() + "-renamed").toFile());
                 file.createNewFile();
                 Files.write(file.toPath(), "some data".getBytes());
                 return file;
-- 
GitLab