Merge pull request #1743 from UNC-Libraries/bxc-4604

sharonluong · web-flow · commit 5de6bd7232aa · 2024-05-30T14:15:19.000-04:00
BXC-4604 - Use label as filename for TechMD extract
diff --git a/common-utils/src/main/java/edu/unc/lib/boxc/common/http/MimetypeHelpers.java b/common-utils/src/main/java/edu/unc/lib/boxc/common/http/MimetypeHelpers.java
@@ -21,7 +21,7 @@ private MimetypeHelpers() {
      * @return true if the given mimetype appears to be a valid mimetype
      */
     public static boolean isValidMimetype(String mimetype) {
-        if (StringUtils.isBlank(mimetype)) {
+        if (StringUtils.isBlank(mimetype) || mimetype.contains("symlink")) {
             return false;
         }
         return VALID_MIMETYPE_PATTERN.matcher(mimetype).matches();
diff --git a/common-utils/src/test/java/edu/unc/lib/boxc/common/http/MimetypeHelpersTest.java b/common-utils/src/test/java/edu/unc/lib/boxc/common/http/MimetypeHelpersTest.java
@@ -0,0 +1,61 @@
+package edu.unc.lib.boxc.common.http;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * @author bbpennel
+ */
+public class MimetypeHelpersTest {
+    @Test
+    public void isValidMimetypeNullTest() {
+        String mimetype = null;
+        assertFalse(MimetypeHelpers.isValidMimetype(mimetype));
+    }
+
+    @Test
+    public void isValidMimetypeBlankTest() {
+        String mimetype = "";
+        assertFalse(MimetypeHelpers.isValidMimetype(mimetype));
+    }
+
+    @Test
+    public void isValidMimetypeSymlinkTest() {
+        String mimetype = "inode/symlink";
+        assertFalse(MimetypeHelpers.isValidMimetype(mimetype));
+    }
+
+    @Test
+    public void isValidMimetypeInvalidFormatTest() {
+        String mimetype = "invalid format";
+        assertFalse(MimetypeHelpers.isValidMimetype(mimetype));
+    }
+
+    @Test
+    public void isValidMimetypeNefTest() {
+        String mimetype = "image/x-nikon-nef";
+        assertTrue(MimetypeHelpers.isValidMimetype(mimetype));
+    }
+
+    @Test
+    public void formatMimetypeNullTest() {
+        String mimetype = null;
+        assertNull(MimetypeHelpers.formatMimetype(mimetype));
+    }
+
+    @Test
+    public void formatMimetypeWithParamsTest() {
+        String mimetype = "text/plain; charset=UTF-8";
+        assertEquals("text/plain", MimetypeHelpers.formatMimetype(mimetype));
+    }
+
+    @Test
+    public void formatMimetypeRegularTest() {
+        String mimetype = "text/plain";
+        assertEquals("text/plain", MimetypeHelpers.formatMimetype(mimetype));
+    }
+}
diff --git a/deposit-app/src/main/java/edu/unc/lib/boxc/deposit/validate/ExtractTechnicalMetadataJob.java b/deposit-app/src/main/java/edu/unc/lib/boxc/deposit/validate/ExtractTechnicalMetadataJob.java
@@ -7,6 +7,7 @@
 import edu.unc.lib.boxc.deposit.work.JobFailedException;
 import edu.unc.lib.boxc.deposit.work.JobInterruptedException;
 import edu.unc.lib.boxc.model.api.ids.PID;
+import edu.unc.lib.boxc.model.api.rdf.CdrDeposit;
 import edu.unc.lib.boxc.model.fcrepo.ids.DatastreamPids;
 import edu.unc.lib.boxc.model.fcrepo.ids.PIDs;
 import org.apache.commons.io.FilenameUtils;
@@ -124,8 +125,10 @@ public void runJob() {
                 String stagedPath = stagedPair.getValue();
                 PID originalPid = DatastreamPids.getOriginalFilePid(objPid);
                 final String providedMimetype = getProvidedMimetype(originalPid, model);
+                final String providedLabel = getProvidedLabel(objPid, model);
 
-                submitTask(new ExtractTechnicalMetadataRunnable(objPid, originalPid, stagedPath, providedMimetype));
+                submitTask(new ExtractTechnicalMetadataRunnable(objPid, originalPid, stagedPath,
+                        providedMimetype, providedLabel));
             }
 
             waitForCompletion();
@@ -144,6 +147,16 @@ private String getProvidedMimetype(PID originalPid, Model model) {
         }
     }
 
+    private String getProvidedLabel(PID filePid, Model model) {
+        Resource fileResc = model.getResource(filePid.getRepositoryPath());
+        Statement labelStmt = fileResc.getProperty(CdrDeposit.label);
+        if (labelStmt != null) {
+            return labelStmt.getString();
+        } else {
+            return null;
+        }
+    }
+
     @Override
     protected void registrationAction() {
         List<Object> results = new ArrayList<>();
@@ -177,14 +190,16 @@ private class ExtractTechnicalMetadataRunnable implements Runnable {
         private PID originalPid;
         private String stagedPath;
         private String providedMimetype;
+        private String providedLabel;
         private ExtractTechnicalMetadataResult result = new ExtractTechnicalMetadataResult();
 
         public ExtractTechnicalMetadataRunnable(PID objPid, PID originalPid, String stagedPath,
-                String providedMimetype) {
+                String providedMimetype, String providedLabel) {
             this.objPid = objPid;
             this.originalPid = originalPid;
             this.stagedPath = stagedPath;
             this.providedMimetype = providedMimetype;
+            this.providedLabel = providedLabel;
             result.objPid = objPid;
             result.originalPid = originalPid;
             result.hasProvidedMimetype = providedMimetype != null;
@@ -198,8 +213,10 @@ public void run() {
 
             interruptJobIfStopped();
 
+            // Symlink the file before processing
+            Path linkPath = makeSymlinkForStagedPath(stagedPath, providedLabel);
             // Generate the FITS report as a document
-            Document fitsDoc = getFitsDocument(objPid, stagedPath);
+            Document fitsDoc = getFitsDocument(objPid, linkPath);
 
             try {
                 // Create the PREMIS report wrapper for the FITS results
@@ -222,6 +239,13 @@ public void run() {
             } catch (Exception e) {
                 failJob(e, "Failed to extract FITS details for file '{0}' with id {1} from document:\n{2}",
                         stagedPath, objPid.getId(), getXMLOutputter().outputString(fitsDoc));
+            } finally {
+                try {
+                    Files.delete(linkPath);
+                    Files.delete(linkPath.getParent());
+                } catch (IOException e) {
+                    log.warn("Failed to delete symlink", e);
+                }
             }
         }
 
@@ -261,7 +285,6 @@ private void addFileIdentification(Document fitsDoc, Element premisObjCharsEl) {
          * Overrides the mimetype for this object in the deposit model when the FITS
          * generated value is preferred.
          *
-         * @param objResc
          * @param fitsExtractMimetype
          */
         private void overrideDepositMimetype(String fitsExtractMimetype) {
@@ -301,31 +324,48 @@ private void overrideDepositMimetype(String fitsExtractMimetype) {
      * XML document
      *
      * @param objPid
+     * @param filePath
+     * @return
+     */
+    private Document getFitsDocument(PID objPid, Path filePath) {
+        if (shouldProcessWithWebService(filePath)) {
+            return extractUsingWebService(objPid, filePath);
+        } else {
+            return extractUsingCLI(objPid, filePath);
+        }
+    }
+
+    /**
+     * Creates a symlink to the provided stagedUri, where the symlink is sanitized of problematic characters
+     * and uses the label as the filename to ensure the original file extension is present, if available.
+     * @param objPid
      * @param stagedUriString
+     * @param label
      * @return
      */
-    private Document getFitsDocument(PID objPid, String stagedUriString) {
+    protected Path makeSymlinkForStagedPath(String stagedUriString, String label)  {
+        // Resolve the path from a URI and make it absolute
         URI stagedUri = URI.create(stagedUriString);
         Path stagedPath;
         if (!stagedUri.isAbsolute()) {
             stagedPath = Paths.get(getDepositDirectory().toString(), stagedUriString);
         } else {
             stagedPath = Paths.get(stagedUri);
         }
-
-        if (shouldProcessWithWebService(stagedPath)) {
-            return extractUsingWebService(objPid, stagedPath);
-        } else {
-            return extractUsingCLI(objPid, stagedPath);
+        try {
+            // Create a symlink to the file to make use of the original filename and avoid issues with non-ascii characters
+            var parentDir = TMP_PATH.resolve(Long.toString(System.nanoTime()));
+            Files.createDirectories(parentDir);
+            String symlinkName = label != null ? label : stagedPath.getFileName().toString();
+            var linkPath = sanitizeCliPath(parentDir.resolve(symlinkName));
+            Files.createSymbolicLink(linkPath, stagedPath);
+            return linkPath;
+        } catch (IOException e) {
+            throw new JobFailedException("Failed to create symlink for file " + stagedPath, e);
         }
     }
 
     private boolean shouldProcessWithWebService(Path path) {
-        // FITS cannot currently handle file paths that contain unicode characters
-        if (!CharMatcher.ascii().matchesAllOf(path.toString())) {
-            log.debug("File {} not applicable for web service due to unacceptable characters", path);
-            return false;
-        }
         String filename = path.getFileName().toString();
         String extension = FilenameUtils.getExtension(filename).toLowerCase();
         if (FILE_EXTS_FOR_CLI.contains(extension)) {
@@ -392,35 +432,9 @@ protected Path sanitizeCliPath(Path stagedPath) {
         return Paths.get(path);
     }
 
-    /**
-     * Symlink the provided file into the temp directory, inside of parent directory based on the id of the object.
-     * Filename of the symlink is based off the sanitized form of the path.
-     * @param objPid
-     * @param sanitizedPath Sanitized version of the staging path, does not need to resolve to a file
-     * @param stagedPath Original staging path of the file, must resolve to the file being linked
-     * @return Symlink path
-     * @throws IOException
-     */
-    protected Path symlinkFile(PID objPid, Path sanitizedPath, Path stagedPath) throws IOException {
-        var parentDir = TMP_PATH.resolve(objPid.getId());
-        Files.createDirectories(parentDir);
-        var linkPath = parentDir.resolve(sanitizedPath.getFileName());
-        Files.createSymbolicLink(linkPath, stagedPath);
-        return linkPath;
-    }
-
-    private Document extractUsingCLI(PID objPid, Path stagedPath) {
+    private Document extractUsingCLI(PID objPid, Path targetPath) {
         String stdout = null;
-        Path fileLink = null;
         try {
-            Path targetPath = stagedPath;
-            var sanitizedPath = sanitizeCliPath(stagedPath);
-            // Create a symlink to the file to avoid problems with non-ascii characters and reserved linux characters
-            // otherwise there will be encoding mismatches between boxc, the terminal, and FITS.
-            if (!stagedPath.equals(sanitizedPath)) {
-                fileLink = symlinkFile(objPid, sanitizedPath, stagedPath);
-                targetPath = fileLink;
-            }
             String[] command = new String[] { fitsCommandPath.toString(), "-i", targetPath.toString() };
             Process process = Runtime.getRuntime().exec(command);
             int exitCode = process.waitFor();
@@ -434,17 +448,7 @@ private Document extractUsingCLI(PID objPid, Path stagedPath) {
             return createSAXBuilder().build(new ByteArrayInputStream(stdout.getBytes(UTF_8)));
         } catch (IOException | JDOMException | InterruptedException e) {
             failJob(e, "Failed to generate report for file {0} with id {1}, output was:\n{2}",
-                    stagedPath, objPid.getId(), stdout);
-        } finally {
-            // Cleanup symlink and parent directory containing symlink, if a symlink was used
-            if (fileLink != null) {
-                try {
-                    Files.delete(fileLink);
-                    Files.delete(fileLink.getParent());
-                } catch (IOException e) {
-                    log.warn("Failed to cleanup symlink", e);
-                }
-            }
+                    targetPath, objPid.getId(), stdout);
         }
         return null;
     }
diff --git a/deposit-app/src/test/java/edu/unc/lib/boxc/deposit/validate/ExtractTechnicalMetadataJobTest.java b/deposit-app/src/test/java/edu/unc/lib/boxc/deposit/validate/ExtractTechnicalMetadataJobTest.java
@@ -98,6 +98,8 @@ public class ExtractTechnicalMetadataJobTest extends AbstractDepositJobTest {
     private final static String UNKNOWN_MD5 = "2748ba561254b629c2103cb2e1be3fc2";
     private final static String UNKNOWN_FORMAT = "Unknown";
 
+    private static final Path TMP_PATH = Paths.get(System.getProperty("java.io.tmpdir"));
+
     @Mock
     private CloseableHttpClient httpClient;
     @Mock
@@ -429,14 +431,39 @@ public void unicodeFilenameTest() throws Exception {
         Bag workBag = model.createBag(workPid.getRepositoryPath());
         workBag.addProperty(RDF.type, Cdr.Work);
         depositBag.add(workBag);
-        String filename = "weird\uD83D\uDC7D.txt";
-        File sourceFile = tmpFolder.resolve(filename).toFile();
-        PID filePid = addFileObject(workBag, sourceFile.getAbsolutePath(), IMAGE_MIMETYPE, IMAGE_MD5);
+        String filename = "weird\uD83D\uDC7D.mov";
+        Path sourcePath = tmpFolder.resolve(filename);
+        Files.createFile(sourcePath);
+        PID filePid = addFileObject(workBag, sourcePath.toFile().toURI().toString(), IMAGE_MIMETYPE, IMAGE_MD5);
+
+        job.closeModel();
+
+        job.run();
+
+        verifyFileResults(filePid, IMAGE_MIMETYPE, IMAGE_FORMAT, IMAGE_MD5, 1);
+    }
+
+    @Test
+    public void filenameFromLabelTest() throws Exception {
+        respondWithFile("/fitsReports/imageReport.xml");
+
+        // Create the work object which nests the file
+        PID workPid = makePid(RepositoryPathConstants.CONTENT_BASE);
+        Bag workBag = model.createBag(workPid.getRepositoryPath());
+        workBag.addProperty(RDF.type, Cdr.Work);
+        depositBag.add(workBag);
+        String filename = "ambiguous_file_name";
+        Path sourcePath = tmpFolder.resolve(filename);
+        Files.createFile(sourcePath);
+        PID filePid = addFileObject(workBag, sourcePath.toFile().toURI().toString(), IMAGE_MIMETYPE, IMAGE_MD5);
+        Resource fileResc = model.getResource(filePid.getRepositoryPath());
+        fileResc.addProperty(CdrDeposit.label, "boxys_favorite_file.nef");
 
         job.closeModel();
 
         job.run();
 
+        verifyRequestParameters("boxys_favorite_file.nef");
         verifyFileResults(filePid, IMAGE_MIMETYPE, IMAGE_FORMAT, IMAGE_MD5, 1);
     }
 
@@ -503,15 +530,23 @@ public void sanitizeCliPathReservedTest() throws Exception {
     }
 
     @Test
-    public void symlinkFileTest() throws Exception {
-        PID pid = makePid();
-        var originalPath = tmpFolder.resolve("file.txt");
-        var sanitizedPath = tmpFolder.resolve("fi_l_e.txt");
-        var result = job.symlinkFile(pid, sanitizedPath, originalPath);
-
-        assertEquals("fi_l_e.txt", result.getFileName().toString());
-        assertEquals(pid.getId(), result.getParent().getFileName().toString());
+    public void makeSymlinkForStagedPathProblemCharactersTest() throws Exception {
+        var originalPath = tmpFolder.resolve("fil£.txt");
+        var result = job.makeSymlinkForStagedPath(originalPath.toUri().toString(), null);
+
+        assertEquals("fil_.txt", result.getFileName().toString());
+        assertTrue(Files.isSymbolicLink(result));
+        assertEquals(originalPath, Files.readSymbolicLink(result));
+    }
+
+    @Test
+    public void makeSymlinkForStagedPathWithLabelTest() throws Exception {
+        var originalPath = tmpFolder.resolve("file");
+        var result = job.makeSymlinkForStagedPath(originalPath.toUri().toString(), "boxys_favorite_file.nef");
+
+        assertEquals("boxys_favorite_file.nef", result.getFileName().toString());
         assertTrue(Files.isSymbolicLink(result));
+        assertEquals(originalPath, Files.readSymbolicLink(result));
     }
 
     private HttpUriRequest getRequest() throws Exception {
@@ -529,11 +564,12 @@ private String getSubmittedFilePath(HttpUriRequest request) {
     }
 
     private void verifyRequestParameters(String expectedFilepath) throws Exception {
-        String absFilePath = Paths.get(depositDir.getAbsolutePath(), expectedFilepath).toString();
         HttpUriRequest request = getRequest();
         String submittedPath = getSubmittedFilePath(request);
 
-        assertEquals(absFilePath.replace("/", "%2F"), submittedPath, "FITS service not called with the expected path");
+        String failMessage = "FITS service called with wrong path. Expected " + expectedFilepath + " but got " + submittedPath;
+        assertTrue(submittedPath.startsWith(TMP_PATH.toString().replace("/", "%2F")), failMessage);
+        assertTrue(submittedPath.endsWith("%2F" + Paths.get(expectedFilepath).getFileName()), failMessage);
     }
 
     private void verifyFileResults(PID filePid, String expectedMimetype, String expectedFormat,

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ private MimetypeHelpers() {`
`21`	`21`	`* @return true if the given mimetype appears to be a valid mimetype`
`22`	`22`	`*/`
`23`	`23`	`public static boolean isValidMimetype(String mimetype) {`
`24`		`- if (StringUtils.isBlank(mimetype)) {`
	`24`	`+ if (StringUtils.isBlank(mimetype) \|\| mimetype.contains("symlink")) {`
`25`	`25`	`return false;`
`26`	`26`	`}`
`27`	`27`	`return VALID_MIMETYPE_PATTERN.matcher(mimetype).matches();`