From 3aa7fc4c1f88402791c41136be4a8ecc22ace9a5 Mon Sep 17 00:00:00 2001 From: Jeff Burke Date: Tue, 6 Feb 2024 13:39:33 -0800 Subject: [PATCH 1/5] CADC-12859 add getResponseFormat method to set the archive return type --- cadc-pkg-server/build.gradle | 2 +- .../org/opencadc/pkg/server/PackageItem.java | 6 ++-- .../opencadc/pkg/server/PackageRunner.java | 30 ++++++++++++++----- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/cadc-pkg-server/build.gradle b/cadc-pkg-server/build.gradle index 7eaeb3f9..6da263b7 100644 --- a/cadc-pkg-server/build.gradle +++ b/cadc-pkg-server/build.gradle @@ -14,7 +14,7 @@ sourceCompatibility = 1.8 group = 'org.opencadc' -version = '1.2.1' +version = '1.2.3' description = 'OpenCADC CADC package server library' def git_url = 'https://github.com/opencadc/dal' diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageItem.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageItem.java index a7a8bd6f..a8218774 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageItem.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageItem.java @@ -96,7 +96,7 @@ public PackageItem(String relativePath) { } /** - * Creates a resource for a package. The relative path is used to create + * Creates a PackageItem for a file. The relative path is used to create * the file structure inside a package. * * @param relativePath path to the resource in the package. @@ -115,11 +115,11 @@ public PackageItem(String relativePath, URL content) { } /** - * Creates a resource for a package. The relative path is used to create + * Creates a PackageItem for a symbolic link. The relative path is used to create * the file structure inside a package. * * @param relativePath path to the resource in the package. - * @param linkTarget path to the resource in the package. + * @param linkTarget relative path to the link target in the package. */ public PackageItem(String relativePath, String linkTarget) { if (!StringUtil.hasText(relativePath)) { diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageRunner.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageRunner.java index 538da647..9a922ae1 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageRunner.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageRunner.java @@ -152,10 +152,31 @@ public void run() { log.info(logInfo.end()); } + /** + * Get the expected initial ExecutionPhase of the Job when the PackageRunner runs. + * + * @return ExecutionPhase + */ protected ExecutionPhase getInitialPhase() { return ExecutionPhase.QUEUED; } + /** + * Get the MIME type of the package. Defaults to application/x-tar + * if the RESPONSEFORMAT job parameter is not provided. + * + * @return MIME type + */ + protected String getResponseFormat() { + // Package type is in RESPONSEFORMAT Job parameter (optional) + String responseFormat = ParameterUtil.findParameterValue("RESPONSEFORMAT", job.getParameterList()); + if (!StringUtil.hasLength(responseFormat)) { + // Not provided, set default + responseFormat = TarWriter.MIME_TYPE; + } + return responseFormat; + } + private void doIt() { ExecutionPhase ep; PackageWriter writer = null; @@ -254,19 +275,12 @@ private ByteCountOutputStream initOutputStream(String mimeType, String contentDi private PackageWriter initWriter() throws IOException { - // Package type is in RESPONSEFORMAT Job parameter (optional) - String responseFormat = ParameterUtil.findParameterValue("RESPONSEFORMAT", job.getParameterList()); - - if (!StringUtil.hasLength(responseFormat)) { - // Not provided, set default - responseFormat = TarWriter.MIME_TYPE; - } - StringBuilder cdisp = new StringBuilder(); cdisp.append("inline;filename="); cdisp.append(packageName); // Initialize the writer, underlying syncoutput and output streams. + String responseFormat = getResponseFormat(); if (responseFormat.equals(ZipWriter.MIME_TYPE)) { cdisp.append(ZipWriter.EXTENSION); this.bcOutputStream = initOutputStream(ZipWriter.MIME_TYPE, cdisp.toString()); From 2008f715769bec86926499702a537bc57662838e Mon Sep 17 00:00:00 2001 From: Jeff Burke Date: Thu, 8 Feb 2024 10:27:32 -0800 Subject: [PATCH 2/5] CADC-12859 enforce directory relative paths do not start with a / and do end with / --- .../java/org/opencadc/pkg/server/PackageWriter.java | 6 +++++- .../main/java/org/opencadc/pkg/server/TarWriter.java | 10 ++++++++++ .../main/java/org/opencadc/pkg/server/ZipWriter.java | 7 +++++++ .../org/opencadc/pkg/server/PackageWriterTest.java | 6 ++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java index 7c49f52e..750130cc 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java @@ -141,8 +141,10 @@ public void write(PackageItem packageItem) throws IOException, InterruptedExcept } else { writeHTTPFile(packageItem); } - } else { + } else if (packageItem.isSymbolicLink()) { writeSymbolicLink(packageItem); + } else { + throw new IllegalArgumentException("Unknown PackageItem type: " + packageItem); } } @@ -181,6 +183,7 @@ private void writeFile(PackageItem packageItem) InputStream stream = fileURL.openStream(); MultiBufferIO multiBufferIO = new MultiBufferIO(); multiBufferIO.copy(stream, archiveOutputStream); + stream.close(); archiveOutputStream.closeArchiveEntry(); } @@ -211,6 +214,7 @@ private void writeHTTPFile(PackageItem packageItem) InputStream getIOStream = get.getInputStream(); MultiBufferIO multiBufferIO = new MultiBufferIO(); multiBufferIO.copy(getIOStream, archiveOutputStream); + getIOStream.close(); archiveOutputStream.closeArchiveEntry(); } diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java index 1629d4d4..25a54e9d 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java @@ -69,7 +69,10 @@ package org.opencadc.pkg.server; +import java.io.IOException; import java.io.OutputStream; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Date; import org.apache.commons.compress.archivers.ArchiveEntry; import org.apache.commons.compress.archivers.tar.TarArchiveEntry; @@ -102,6 +105,13 @@ ArchiveEntry createFileEntry(String relativePath, long size, Date lastModifiedDa ArchiveEntry createDirectoryEntry(String relativePath) { log.debug("directory ArchiveEntry: " + relativePath); + if (relativePath.startsWith("/")) { + relativePath = relativePath.substring(1); + } + if (!relativePath.endsWith("/")) { + relativePath = relativePath + "/"; + } + return new TarArchiveEntry(relativePath, TarConstants.LF_DIR); } diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java index bc5f5bf4..3ad3219f 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java @@ -103,6 +103,13 @@ ArchiveEntry createFileEntry(String relativePath, long size, Date lastModifiedDa ArchiveEntry createDirectoryEntry(String relativePath) { log.debug("directory ArchiveEntry: " + relativePath); + if (relativePath.startsWith("/")) { + relativePath = relativePath.substring(1); + } + if (!relativePath.endsWith("/")) { + relativePath = relativePath + "/"; + } + ZipArchiveEntry entry = new ZipArchiveEntry(relativePath); entry.setUnixMode(UnixStat.DIR_FLAG); return entry; diff --git a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java index ea06c913..4f712469 100644 --- a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java +++ b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java @@ -69,11 +69,13 @@ package org.opencadc.pkg.server; +import ca.nrc.cadc.util.Log4jInit; import java.io.File; import java.io.FileOutputStream; import java.net.URL; import java.util.ArrayList; import java.util.List; +import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.junit.Assert; import org.junit.Test; @@ -81,6 +83,10 @@ public class PackageWriterTest { private static final Logger log = Logger.getLogger(PackageWriterTest.class); + static { + Log4jInit.setLevel("org.opencadc.pkg.server", Level.INFO); + } + @Test public void testCreateTarFile() { try { From 4cc9d8e636002b2e68d8f7347f5c7b3ac2ab9ec3 Mon Sep 17 00:00:00 2001 From: Jeff Burke Date: Fri, 9 Feb 2024 06:37:52 -0800 Subject: [PATCH 3/5] CADC-12859 unit test udpates --- .../pkg/server/PackageWriterTest.java | 69 ++++++++++++------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java index 4f712469..4f821b0f 100644 --- a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java +++ b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java @@ -84,7 +84,7 @@ public class PackageWriterTest { private static final Logger log = Logger.getLogger(PackageWriterTest.class); static { - Log4jInit.setLevel("org.opencadc.pkg.server", Level.INFO); + Log4jInit.setLevel("org.opencadc.pkg.server", Level.DEBUG); } @Test @@ -140,34 +140,57 @@ public void testCreateZipFile() { protected List getTestPackageItems() { // Create PackageItems for testing // Files are in test/resources - String dir1Path = "some/path/"; - PackageItem dir1 = new PackageItem(dir1Path); - log.debug(dir1); + String some = "some/"; + PackageItem someDir = new PackageItem(some); + log.debug(someDir); - String dir2Path = "some/empty/path/"; - PackageItem dir2 = new PackageItem(dir2Path); - log.debug(dir2); + String somePath = "some/path/"; + PackageItem somePathDir = new PackageItem(somePath); + log.debug(somePathDir); - String file1Path = "some/path/GovCanada.gif"; - URL file1URL = getClass().getClassLoader().getResource("GovCanada.gif"); - PackageItem file1 = new PackageItem(file1Path, file1URL); - log.debug(file1); + String someEmpty = "some/empty/"; + PackageItem someEmptyDir = new PackageItem(someEmpty); + log.debug(someEmptyDir); - String file2Path = "another/path/SymbolCanada.gif"; - URL file2URL = getClass().getClassLoader().getResource("SymbolCanada.gif"); - PackageItem file2 = new PackageItem(file2Path, file2URL); - log.debug(file2); + String someEmptyPath = "some/empty/path/"; + PackageItem someEmptyPathDir = new PackageItem(someEmptyPath); + log.debug(someEmptyPathDir); - String link1Path = "some/path/link2SymbolCanada.gif"; - PackageItem link1 = new PackageItem(link1Path, file2Path); - log.debug(link1); + String govCanadaGif = "some/path/GovCanada.gif"; + URL govCanadaURL = getClass().getClassLoader().getResource("GovCanada.gif"); + PackageItem govCanadaFile = new PackageItem(govCanadaGif, govCanadaURL); + log.debug(govCanadaFile); + + String another = "another/"; + PackageItem anotherDir = new PackageItem(another); + log.debug(anotherDir); + + String anotherPath = "another/path/"; + PackageItem anotherPathDir = new PackageItem(anotherPath); + log.debug(anotherPathDir); + + String symbolCanadaGif = "another/path/SymbolCanada.gif"; + URL symbolCanadaURL = getClass().getClassLoader().getResource("SymbolCanada.gif"); + PackageItem symbolCanadaFile = new PackageItem(symbolCanadaGif, symbolCanadaURL); + log.debug(symbolCanadaFile); + + String linkPath = "some/path/link2SymbolCanada.gif"; + PackageItem link = new PackageItem(linkPath, "../../another/path/SymbolCanada.gif"); + log.debug(link); List packageItems = new ArrayList<>(); - packageItems.add(dir1); - packageItems.add(dir2); - packageItems.add(file1); - packageItems.add(file2); - packageItems.add(link1); + packageItems.add(someDir); + packageItems.add(somePathDir); + packageItems.add(someEmptyPathDir); + packageItems.add(govCanadaFile); + packageItems.add(anotherDir); + packageItems.add(anotherPathDir); + packageItems.add(symbolCanadaFile); + packageItems.add(link); + + String fooPath = "foo"; + PackageItem foo = new PackageItem(fooPath); + packageItems.add(foo); return packageItems; } From 72d624fb36989ebb828f4c6add4e9028c5080835 Mon Sep 17 00:00:00 2001 From: Jeff Burke Date: Mon, 12 Feb 2024 09:40:20 -0800 Subject: [PATCH 4/5] CADC-12859 unit test to info logging --- .../test/java/org/opencadc/pkg/server/PackageWriterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java index 4f821b0f..da24ff16 100644 --- a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java +++ b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java @@ -84,7 +84,7 @@ public class PackageWriterTest { private static final Logger log = Logger.getLogger(PackageWriterTest.class); static { - Log4jInit.setLevel("org.opencadc.pkg.server", Level.DEBUG); + Log4jInit.setLevel("org.opencadc.pkg.server", Level.INFO); } @Test From 81a4e8ca6950192d9b5e6261c3abe09856c1a0aa Mon Sep 17 00:00:00 2001 From: Jeff Burke Date: Mon, 12 Feb 2024 10:15:50 -0800 Subject: [PATCH 5/5] CADC-12859 code review rework --- .../src/main/java/org/opencadc/pkg/server/PackageWriter.java | 5 ++++- .../src/main/java/org/opencadc/pkg/server/TarWriter.java | 4 +++- .../src/main/java/org/opencadc/pkg/server/ZipWriter.java | 3 +++ .../test/java/org/opencadc/pkg/server/PackageWriterTest.java | 4 ++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java index 750130cc..17bdb3f9 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/PackageWriter.java @@ -85,6 +85,7 @@ import java.util.Date; import org.apache.commons.compress.archivers.ArchiveEntry; import org.apache.commons.compress.archivers.ArchiveOutputStream; +import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.log4j.Logger; public abstract class PackageWriter { @@ -233,7 +234,9 @@ private void writeSymbolicLink(PackageItem packageItem) archiveOutputStream.putArchiveEntry(archiveEntry); // entry content is the symbolic link target path - archiveOutputStream.write(linkTarget.getBytes(StandardCharsets.UTF_8)); + if (archiveEntry instanceof ZipArchiveEntry) { + archiveOutputStream.write(linkTarget.getBytes(StandardCharsets.UTF_8)); + } archiveOutputStream.closeArchiveEntry(); } diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java index 25a54e9d..34ceb07f 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/TarWriter.java @@ -91,6 +91,7 @@ public TarWriter(OutputStream ostream) { ((TarArchiveOutputStream)super.archiveOutputStream).setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX); } + @Override ArchiveEntry createFileEntry(String relativePath, long size, Date lastModifiedDate) { log.debug(String.format("file ArchiveEntry: %s %s %s", relativePath, size, lastModifiedDate)); @@ -102,6 +103,7 @@ ArchiveEntry createFileEntry(String relativePath, long size, Date lastModifiedDa return entry; } + @Override ArchiveEntry createDirectoryEntry(String relativePath) { log.debug("directory ArchiveEntry: " + relativePath); @@ -115,12 +117,12 @@ ArchiveEntry createDirectoryEntry(String relativePath) { return new TarArchiveEntry(relativePath, TarConstants.LF_DIR); } + @Override ArchiveEntry createSymbolicLinkEntry(String relativePath, String linkTarget) { log.debug(String.format("symbolic link ArchiveEntry: %s -> %s", relativePath, linkTarget)); TarArchiveEntry entry = new TarArchiveEntry(relativePath, TarConstants.LF_SYMLINK); entry.setLinkName(linkTarget); - entry.setSize(linkTarget.length()); return entry; } diff --git a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java index 3ad3219f..1818116d 100644 --- a/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java +++ b/cadc-pkg-server/src/main/java/org/opencadc/pkg/server/ZipWriter.java @@ -88,6 +88,7 @@ public ZipWriter(OutputStream ostream) { super(new ZipArchiveOutputStream(ostream)); } + @Override ArchiveEntry createFileEntry(String relativePath, long size, Date lastModifiedDate) { log.debug(String.format("file ArchiveEntry: %s %s %s", relativePath, size, lastModifiedDate)); @@ -100,6 +101,7 @@ ArchiveEntry createFileEntry(String relativePath, long size, Date lastModifiedDa return entry; } + @Override ArchiveEntry createDirectoryEntry(String relativePath) { log.debug("directory ArchiveEntry: " + relativePath); @@ -115,6 +117,7 @@ ArchiveEntry createDirectoryEntry(String relativePath) { return entry; } + @Override ArchiveEntry createSymbolicLinkEntry(String relativePath, String linkRelativePath) { log.debug(String.format("symbolic link ArchiveEntry: %s -> %s", relativePath, linkRelativePath)); diff --git a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java index da24ff16..f5d4485e 100644 --- a/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java +++ b/cadc-pkg-server/src/test/java/org/opencadc/pkg/server/PackageWriterTest.java @@ -93,7 +93,7 @@ public void testCreateTarFile() { List testPackageItems = getTestPackageItems(); File tarFile = File.createTempFile("tartest", ".tar"); - log.debug("tar archive: " + tarFile.getAbsolutePath()); + log.info("tar archive: " + tarFile.getAbsolutePath()); FileOutputStream fos = new FileOutputStream(tarFile); TarWriter fw = new TarWriter(fos); for (PackageItem pi : testPackageItems) { @@ -118,7 +118,7 @@ public void testCreateZipFile() { List testPackageItems = getTestPackageItems(); File zipFile = File.createTempFile("ziptest", ".zip"); - log.debug("zip archive: " + zipFile.getAbsolutePath()); + log.info("zip archive: " + zipFile.getAbsolutePath()); FileOutputStream fos = new FileOutputStream(zipFile); PackageWriter fw = new ZipWriter(fos); for (PackageItem pi : testPackageItems) {