Update federated filesystem directories timestamp when their child re…

…sources change - Add unit tests for removeDocument() and storeDocument() Resolves: https://www.pivotaltracker.com/story/show/70989992
fcrepo · Jul 25, 2014 · db1ac82 · db1ac82
1 parent 386906e
commit db1ac82
Show file tree

Hide file tree

Showing 4 changed files with 142 additions and 3 deletions.
diff --git a/fcrepo-connector-file/src/main/java/org/fcrepo/connector/file/FedoraFileSystemConnector.java b/fcrepo-connector-file/src/main/java/org/fcrepo/connector/file/FedoraFileSystemConnector.java
@@ -21,6 +21,7 @@
 import static org.fcrepo.jcr.FedoraJcrTypes.FEDORA_DATASTREAM;
 import static org.fcrepo.jcr.FedoraJcrTypes.FEDORA_RESOURCE;
 import static org.fcrepo.jcr.FedoraJcrTypes.JCR_CREATED;
+import static org.fcrepo.jcr.FedoraJcrTypes.JCR_LASTMODIFIED;
 import static org.fcrepo.kernel.utils.ContentDigest.asURI;
 import static org.modeshape.jcr.api.JcrConstants.JCR_DATA;
 import static org.modeshape.jcr.api.JcrConstants.JCR_PRIMARY_TYPE;
@@ -34,11 +35,13 @@
 import java.io.IOException;
 import java.util.Map;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.infinispan.schematic.document.Document;
 import org.modeshape.connector.filesystem.ExternalJsonSidecarExtraPropertyStore;
 import org.modeshape.connector.filesystem.FileSystemConnector;
 import org.modeshape.jcr.api.value.DateTime;
 import org.modeshape.jcr.api.nodetype.NodeTypeManager;
+import org.modeshape.jcr.federation.spi.DocumentChanges;
 import org.modeshape.jcr.federation.spi.DocumentReader;
 import org.modeshape.jcr.federation.spi.DocumentWriter;
 import org.modeshape.jcr.value.BinaryValue;
@@ -118,6 +121,9 @@ public Document getDocumentById(final String id) {
 
         final DocumentReader docReader = readDocument(doc);
         final DocumentWriter docWriter = writeDocument(doc);
+        final long lastmod = fileFor(id).lastModified();
+        LOGGER.debug("Adding lastModified={}", lastmod);
+        docWriter.addProperty(JCR_LASTMODIFIED, lastmod);
 
         final String primaryType = docReader.getPrimaryTypeName();
 
@@ -262,4 +268,50 @@ private void saveProperties(final DocumentReader docReader) {
         extraProperties.save();
     }
 
+    /* Override write operations to also update the parent file's timestamp, so
+       its Last-Modified header correctly reflects changes to children. */
+    @Override
+    public boolean removeDocument( final String id ) {
+        if ( super.removeDocument(id) ) {
+            touchParent(id);
+            return true;
+        }
+        return false;
+    }
+
+    @Override
+    public void storeDocument( final Document document ) {
+        super.storeDocument( document );
+        touchParent(readDocument(document).getDocumentId());
+    }
+
+    @Override
+    public void updateDocument( final DocumentChanges changes ) {
+        super.updateDocument( changes );
+        touchParent( changes.getDocumentId() );
+    }
+
+    /**
+     * Find the parent file, and set its timestamp to the current time.  This
+     * timestamp will be used for populating the Last-Modified header.
+    **/
+    protected void touchParent( final String id ) {
+        if ( !isRoot(id) ) {
+            final File file = fileFor(id);
+            final File parent = file.getParentFile();
+            parent.setLastModified( System.currentTimeMillis() );
+        }
+    }
+
+    /* Overriding so unit test can mock. */
+    @Override
+    @VisibleForTesting
+    protected File fileFor( final String id ) {
+        return super.fileFor(id);
+    }
+    @Override
+    @VisibleForTesting
+    protected DocumentReader readDocument( final Document document ) {
+        return super.readDocument(document);
+    }
 }
diff --git a/...connector-file/src/test/java/org/fcrepo/connector/file/FedoraFileSystemConnectorTest.java b/...connector-file/src/test/java/org/fcrepo/connector/file/FedoraFileSystemConnectorTest.java
@@ -24,6 +24,7 @@
 import org.modeshape.jcr.ExecutionContext;
 import org.modeshape.jcr.api.nodetype.NodeTypeManager;
 import org.modeshape.jcr.cache.document.DocumentTranslator;
+import org.modeshape.jcr.federation.spi.DocumentReader;
 import org.modeshape.jcr.federation.spi.ExtraPropertiesStore;
 import org.modeshape.jcr.value.BinaryValue;
 import org.modeshape.jcr.value.NameFactory;
@@ -47,10 +48,15 @@
 import static org.fcrepo.jcr.FedoraJcrTypes.CONTENT_DIGEST;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 import static org.modeshape.jcr.api.JcrConstants.JCR_DATA;
 import static org.modeshape.jcr.api.JcrConstants.NT_FILE;
@@ -68,6 +74,7 @@ public class FedoraFileSystemConnectorTest {
     private static Path directoryPath;
 
     private static File tmpFile;
+    private static File tmpFile2;
 
     @Mock
     private NamespaceRegistry mockRegistry;
@@ -110,12 +117,27 @@ public static void beforeClass() throws IOException {
             logger.error("Error creating: {} - {}", tmpFile.getAbsolutePath(),
                     e.getMessage());
         }
+
+        tmpFile2 =
+            createTempFile(directoryPath, "fedora-filesystemtestfile",
+                    "txt").toFile();
+        try (FileOutputStream outputStream = new FileOutputStream(tmpFile2)) {
+            outputStream.write("goodbye".getBytes());
+        } catch (final IOException e) {
+            logger.error("Error creating: {} - {}", tmpFile2.getAbsolutePath(),
+                    e.getMessage());
+        }
     }
 
     @AfterClass
     public static void afterClass() {
         try {
-            tmpFile.delete();
+            if ( tmpFile.exists() ) {
+                tmpFile.delete();
+            }
+            if ( tmpFile2.exists() ) {
+                tmpFile2.delete();
+            }
         } catch (final Exception e) {
             logger.error("Error deleting: " + tmpFile.getAbsolutePath()
                     + " - " + e.getMessage());
@@ -222,4 +244,24 @@ public void testSha1ContentDigestIsNotCached() {
         assert(sha1.contains(chksum));
     }
 
+    @Test
+    public void testRemoveDocument() throws IOException, RepositoryException {
+        final String id = "/" + tmpFile2.getName();
+        final FedoraFileSystemConnector spy = spy(connector);
+        assertTrue("Removing document should return true!", spy.removeDocument(id));
+        verify(spy).touchParent(id);
+    }
+
+    @Test
+    public void testStoreDocument() throws IOException, RepositoryException {
+        final String id = "/" + tmpFile.getName();
+        final DocumentReader reader = mock(DocumentReader.class);
+        final FedoraFileSystemConnector spy = spy(connector);
+        doReturn(tmpFile).when(spy).fileFor(anyString());
+        doReturn(reader).when(spy).readDocument(any(Document.class));
+        doReturn(id).when(reader).getDocumentId();
+        doReturn(NT_FILE).when(reader).getPrimaryTypeName();
+        spy.storeDocument(spy.getDocumentById(id));
+        verify(spy).touchParent(id);
+    }
 }
diff --git a/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraNodesIT.java b/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraNodesIT.java
@@ -32,6 +32,7 @@
 import static org.apache.jena.riot.WebContent.contentTypeNTriples;
 import static java.util.regex.Pattern.DOTALL;
 import static java.util.regex.Pattern.compile;
+import static java.util.TimeZone.getTimeZone;
 import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
 import static javax.ws.rs.core.Response.Status.CONFLICT;
 import static javax.ws.rs.core.Response.Status.CREATED;
@@ -71,14 +72,17 @@
 import static org.junit.Assert.fail;
 
 import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.StringWriter;
 import java.io.Writer;
 import java.net.URI;
+import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Locale;
 import java.util.Iterator;
 import java.util.UUID;
 
@@ -1644,4 +1648,45 @@ public void testLinkedDeletion() throws Exception {
         assertEquals("Linked to should still exist!", 200, getStatus(get));
     }
 
+    /**
+     * When I make changes to a resource in a federated filesystem, the parent
+     * folder's Last-Modified header should be updated.
+    **/
+    @Test
+    public void testLastModifiedUpdatedAfterUpdates() throws Exception {
+        final SimpleDateFormat df = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US);
+        df.setTimeZone(getTimeZone("GMT"));
+
+        // create directory containing a file in filesystem
+        final File fed = new File("target/test-classes/test-objects");
+        final String id = getRandomUniquePid();
+        final File dir = new File( fed, id );
+        final File child = new File( dir, "child" );
+        final long timestamp1 = System.currentTimeMillis();
+        dir.mkdir();
+        child.mkdir();
+        Thread.sleep(2000);
+
+        // check Last-Modified header is current
+        final HttpHead head1 = new HttpHead(serverAddress + "files/" + id);
+        final HttpResponse resp1 = client.execute(head1);
+        assertEquals( 200, resp1.getStatusLine().getStatusCode() );
+        final long lastmod1 = df.parse(resp1.getFirstHeader("Last-Modified").getValue()).getTime();
+        assertTrue( (timestamp1 - lastmod1) < 1000 ); // because rounding
+
+        // remove the file and wait for the TTL to expire
+        final long timestamp2 = System.currentTimeMillis();
+        child.delete();
+        Thread.sleep(2000);
+
+        // check Last-Modified header is updated
+        final HttpHead head2 = new HttpHead(serverAddress + "files/" + id);
+        final HttpResponse resp2 = client.execute(head2);
+        assertEquals( 200, resp2.getStatusLine().getStatusCode() );
+        final long lastmod2 = df.parse(resp2.getFirstHeader("Last-Modified").getValue()).getTime();
+        assertTrue( (timestamp2 - lastmod2) < 1000 ); // because rounding
+
+        assertFalse("Last-Modified headers should have changed", lastmod1 == lastmod2);
+    }
+
 }
diff --git a/fcrepo-http-api/src/test/resources/test_repository.json b/fcrepo-http-api/src/test/resources/test_repository.json
@@ -21,7 +21,7 @@
         "directoryPath" : "target/test-classes/test-objects",
         "readonly" : true,
         "extraPropertiesStorage": "json",
-        "cacheTtlSeconds" : 5,
+        "cacheTtlSeconds" : 2,
         "projections" : [ "default:/files => /" ]
       }
     },