Merge pull request #962 from UNC-Libraries/bxc-2624-dep-cleanup

BXC-2624 - Fix deposit cleanup failures

Author: smithjp

deposit/src/main/java/edu/unc/lib/deposit/CleanupDepositJob.java

@@ -74,7 +74,7 @@ private File deleteFile(URI uri) {
             Files.delete(cFile.toPath());
             LOG.info("Deleted file: {}", cFile.getAbsoluteFile());
         } catch (NoSuchFileException e) {
-            LOG.warn("Cannot cleanup file {}, it does not exist", uri);
+            LOG.debug("Cannot cleanup file {}, it does not exist", uri);
         } catch (IOException e) {
             LOG.error("Cannot delete a staged file: " + uri.toString(), e);
         }

deposit/src/main/java/edu/unc/lib/deposit/validate/ValidateFileAvailabilityJob.java

@@ -28,6 +28,7 @@
 
 import edu.unc.lib.deposit.work.AbstractDepositJob;
 import edu.unc.lib.dl.fedora.PID;
+import edu.unc.lib.dl.persist.api.ingest.IngestSource;
 import edu.unc.lib.dl.persist.api.ingest.IngestSourceManager;
 import edu.unc.lib.dl.persist.api.ingest.UnknownIngestSourceException;
 import edu.unc.lib.dl.rdf.CdrDeposit;
@@ -73,9 +74,13 @@ public void runJob() {
             try {
                 URI manifestURI = URI.create(entry.getValue());
                 // If no ingest source can be found for the file, then file not available
-                sourceManager.getIngestSourceForUri(manifestURI);
+                IngestSource source = sourceManager.getIngestSourceForUri(manifestURI);
+                if (!source.exists(manifestURI)) {
+                    log.debug("Failed find staged file {} in deposit {}", href, getDepositUUID());
+                    badlyStagedFiles.add(href);
+                }
             } catch (UnknownIngestSourceException e) {
-                log.debug("Failed find staged file {} in deposit {}", href, getDepositUUID(), e);
+                log.debug("Could not determine staging location for {} in deposit {}", href, getDepositUUID(), e);
                 badlyStagedFiles.add(href);
             }
 

deposit/src/test/java/edu/unc/lib/deposit/CleanupDepositJobTest.java

@@ -31,6 +31,8 @@
 
 import java.io.File;
 import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -202,6 +204,21 @@ public void cleanupMissingStagingFolderTest() throws Exception {
         assertDepositCleanedUp();
     }
 
+    @Test
+    public void cleanupFileAlreadyRemovedTest() throws Exception {
+        addIngestedFilesToModel();
+        Files.delete(Paths.get(stagingPath, "project/folderA/ingested"));
+
+        when(ingestSource.isReadOnly()).thenReturn(false);
+        when(sourceManager.getIngestSourceForUri(any(URI.class))).thenReturn(ingestSource);
+
+        job.run();
+
+        assertIngestedFilesEmptyFoldersDeleted(stagingFolder);
+
+        assertDepositCleanedUp();
+    }
+
     /**
      * Test that registering multiple staging locations with different policies
      * works correctly

deposit/src/test/java/edu/unc/lib/deposit/validate/ValidateFileAvailabilityJobTest.java

@@ -91,6 +91,7 @@ public void init() throws Exception {
         when(depositStatusFactory.getState(anyString()))
                 .thenReturn(DepositState.running);
         when(sourceManager.getIngestSourceForUri(any(URI.class))).thenReturn(ingestSource);
+        when(ingestSource.exists(any(URI.class))).thenReturn(true);
 
         depositPid = job.getDepositPID();
 
@@ -143,12 +144,26 @@ public void absolutePresentTest() {
     }
 
     @Test(expected = JobFailedException.class)
-    public void missingFileTest() {
+    public void invalidSourceTest() {
         Model model = job.getWritableModel();
         Bag depBag = model.createBag(depositPid.getRepositoryPath());
 
         when(sourceManager.getIngestSourceForUri(any(URI.class)))
-                .thenThrow(new UnknownIngestSourceException("missing"));
+                .thenThrow(new UnknownIngestSourceException("Not source"));
+
+        addFileObject(depBag, "missing.pdf");
+
+        job.closeModel();
+
+        job.run();
+    }
+
+    @Test(expected = JobFailedException.class)
+    public void missingFileTest() {
+        Model model = job.getWritableModel();
+        Bag depBag = model.createBag(depositPid.getRepositoryPath());
+
+        when(ingestSource.exists(any(URI.class))).thenReturn(false);
 
         addFileObject(depBag, "missing.pdf");
 

fcrepo-clients/src/main/java/edu/unc/lib/dl/persist/api/ingest/IngestSource.java

@@ -86,6 +86,14 @@ public interface IngestSource {
      */
     boolean isValidUri(URI uri);
 
+    /**
+     * Returns true if the provided URI exists within this ingest source
+     *
+     * @param uri
+     * @return
+     */
+    boolean exists(URI uri);
+
     /**
      * List the potential candidates for ingest from this source location.
      *

persistence/src/main/java/edu/unc/lib/dl/persist/services/ingest/FilesystemIngestSource.java

@@ -149,13 +149,23 @@ private boolean isValidPath(Path path) {
         for (String pattern : patterns) {
             PathMatcher matcher = FileSystems.getDefault().getPathMatcher("glob:" + baseString + "/" + pattern + "*");
             if (matcher.matches(path)) {
-                return path.toFile().exists();
+                return true;
             }
         }
 
         return false;
     }
 
+    @Override
+    public boolean exists(URI uri) {
+        Path path = Paths.get(uri).normalize();
+        if (!path.startsWith(basePath)) {
+            return false;
+        }
+
+        return Files.exists(path);
+    }
+
     @Override
     public List<IngestSourceCandidate> listCandidates() {
         List<IngestSourceCandidate> candidates = new ArrayList<>();
@@ -249,7 +259,7 @@ private Bag getBagitBag(Path filePath) throws IOException, UnparsableVersionExce
     public URI resolveRelativePath(String relative) {
         Path candidatePath = basePath.resolve(relative).normalize();
 
-        if (!isValidPath(candidatePath)) {
+        if (!isValidPath(candidatePath) || Files.notExists(candidatePath)) {
             throw new InvalidIngestSourceCandidateException("Invalid ingest source path " + relative);
         }
 

persistence/src/test/java/edu/unc/lib/dl/persist/services/ingest/FilesystemIngestSourceTest.java

@@ -115,7 +115,7 @@ public void isValidUriDoesNotExist() throws Exception {
 
         URI testUri = Paths.get(sourceFolderPath.toString(), "someSubPath").toUri();
 
-        assertFalse(ingestSource.isValidUri(testUri));
+        assertTrue(ingestSource.isValidUri(testUri));
     }
 
     @Test
@@ -184,6 +184,31 @@ public void isValidUriNonFileUri() throws Exception {
         assertFalse(ingestSource.isValidUri(uri));
     }
 
+    @Test
+    public void existsNotInBase() throws Exception {
+        ingestSource.setBase(sourceFolderPath.toUri().toString());
+
+        URI uri = URI.create("file:///some/weird/path");
+        assertFalse(ingestSource.exists(uri));
+    }
+
+    @Test
+    public void existsInBaseNotFound() throws Exception {
+        ingestSource.setBase(sourceFolderPath.toUri().toString());
+
+        URI uri = sourceFolderPath.resolve("unknown/path.txt").toUri();
+        assertFalse(ingestSource.exists(uri));
+    }
+
+    @Test
+    public void existsSubpathExists() throws Exception {
+        ingestSource.setBase(sourceFolderPath.toUri().toString());
+
+        Path path = sourceFolderPath.resolve("someSubPath/child");
+        Files.createDirectories(path);
+        assertTrue(ingestSource.exists(path.toUri()));
+    }
+
     @Test
     public void listCandidatesBag() throws Exception {
         ingestSource.setId("testsource");
@@ -307,6 +332,29 @@ public void resolveRelativePathBag() throws Exception {
         assertEquals(sourceFolderPath.resolve("bag_with_files").toUri(), candUri);
     }
 
+    @Test
+    public void resolveRelativePathBagNested() throws Exception {
+        ingestSource.setId("testsource");
+        ingestSource.setBase(sourceFolderPath.toUri().toString());
+        ingestSource.setPatterns(asList("*"));
+
+        addBagToSource(sourceFolderPath);
+
+        URI candUri = ingestSource.resolveRelativePath("bag_with_files/data/test1.txt");
+        assertEquals(sourceFolderPath.resolve("bag_with_files/data/test1.txt").toUri(), candUri);
+    }
+
+    @Test(expected = InvalidIngestSourceCandidateException.class)
+    public void resolveRelativePathBagNestedNotExist() throws Exception {
+        ingestSource.setId("testsource");
+        ingestSource.setBase(sourceFolderPath.toUri().toString());
+        ingestSource.setPatterns(asList("*"));
+
+        addBagToSource(sourceFolderPath);
+
+        ingestSource.resolveRelativePath("bag_with_files/data/ohno.txt");
+    }
+
     @Test(expected = InvalidIngestSourceCandidateException.class)
     public void resolveRelativePathOutsideOfSource() throws Exception {
         ingestSource.setId("testsource");

persistence/src/test/java/edu/unc/lib/dl/persist/services/ingest/IngestSourceManagerImplTest.java

@@ -29,6 +29,7 @@
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 
+import java.net.URI;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
@@ -315,6 +316,65 @@ public void testListCandidatesNoMatchingSource() throws Exception {
         assertTrue("No candidates expected", candidates.isEmpty());
     }
 
+    @Test
+    public void testGetIngestSourceForUri_NestedUri() throws Exception {
+        configPath = createConfigFile(createBasicConfig("testsource", destPid));
+        Path source2FolderPath = tmpFolder.newFolder().toPath();
+
+        Map<String, Object> source2 = createConfig(
+                "testsource2",
+                "Source 2",
+                source2FolderPath,
+                asList("*"),
+                destPid);
+        configPath = createConfigFile(source2, createBasicConfig("testsource", destPid));
+
+        Path targetPath = addBagToSource(sourceFolderPath);
+
+        initializeManager();
+
+        IngestSource source = sourceMan.getIngestSourceForUri(targetPath.toUri());
+        assertEquals("testsource", source.getId());
+    }
+
+    @Test
+    public void testGetIngestSourceForUri_ValidPathTargetNotExist() throws Exception {
+        configPath = createConfigFile(createBasicConfig("testsource", destPid));
+
+        initializeManager();
+
+        URI targetUri = sourceFolderPath.resolve("my_target").toUri();
+
+        IngestSource source = sourceMan.getIngestSourceForUri(targetUri);
+        assertEquals("testsource", source.getId());
+    }
+
+    @Test
+    public void testGetIngestSourceForUri_NestedPath() throws Exception {
+        configPath = createConfigFile(createBasicConfig("testsource", destPid));
+
+        Path bagPath = addBagToSource(sourceFolderPath);
+        Path nestedPath = bagPath.resolve("data/test1.txt");
+
+        initializeManager();
+
+        IngestSource source = sourceMan.getIngestSourceForUri(nestedPath.toUri());
+        assertEquals("testsource", source.getId());
+    }
+
+    @Test
+    public void testGetIngestSourceForUri_NestedNotExist() throws Exception {
+        configPath = createConfigFile(createBasicConfig("testsource", destPid));
+
+        Path bagPath = addBagToSource(sourceFolderPath);
+        Path nestedPath = bagPath.resolve("data/somewhere.txt");
+
+        initializeManager();
+
+        IngestSource source = sourceMan.getIngestSourceForUri(nestedPath.toUri());
+        assertEquals("testsource", source.getId());
+    }
+
     private PID makePid() {
         return PIDs.get(UUID.randomUUID().toString());
     }

persistence/src/test/java/edu/unc/lib/dl/persist/services/ingest/IngestSourceTestHelper.java

@@ -48,14 +48,16 @@ public class IngestSourceTestHelper {
     private IngestSourceTestHelper() {
     }
 
-    public static void addBagToSource(Path ingestSourcePath) throws Exception {
+    public static Path addBagToSource(Path ingestSourcePath) throws Exception {
         String bagName = "bag_with_files";
-        addBagToSource(ingestSourcePath, bagName, bagName);
+        return addBagToSource(ingestSourcePath, bagName, bagName);
     }
 
-    public static void addBagToSource(Path ingestSourcePath, String bagName, String destName) throws Exception {
+    public static Path addBagToSource(Path ingestSourcePath, String bagName, String destName) throws Exception {
         File original = new File("src/test/resources/ingestSources/" + bagName);
-        FileUtils.copyDirectory(original, ingestSourcePath.resolve(destName).toFile());
+        Path bagPath = ingestSourcePath.resolve(destName);
+        FileUtils.copyDirectory(original, bagPath.toFile());
+        return bagPath;
     }
 
     public static IngestSourceCandidate findCandidateByPath(String patternMatched,