Making sure last-modified header is not earlier than created date, we…

…ird breakage
fcrepo · Aug 8, 2014 · e2ed144 · e2ed144
1 parent ae12d03
commit e2ed144
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 14 deletions.
diff --git a/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraLocksIT.java b/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraLocksIT.java
@@ -282,7 +282,7 @@ public void testLockLinkIsPresentLockedNode() throws IOException {
         final Node nodeURI = createURI(serverAddress + pid);
         final Node lockURI = createURI(serverAddress + pid + "/" + FCR_LOCK);
 
-        final GraphStore store = getGraphStore(getObjectProperties(pid));
+        final GraphStore store = getGraphStore(getObjectProperties(pid, lockToken));
         Assert.assertTrue("HAS_LOCK assertion should be in the object's RDF.",
                 store.contains(Node.ANY, nodeURI, HAS_LOCK.asNode(), lockURI));
         assertUnlockWithToken(pid, lockToken);
@@ -297,10 +297,10 @@ public void testLockLinkIsPresentOnChildrenOfDeepLockedNode() throws IOException
 
         final String lockToken = getLockToken(lockObject(pid, true));
 
+        final GraphStore store = getGraphStore(getObjectProperties(childPid, lockToken));
+
         final Node childNodeURI = createURI(serverAddress + childPid);
         final Node lockURI = createURI(serverAddress + pid + "/" + FCR_LOCK);
-
-        final GraphStore store = getGraphStore(getObjectProperties(childPid));
         Assert.assertTrue("HAS_LOCK assertion should be in the child object's RDF.",
                 store.contains(Node.ANY, childNodeURI, HAS_LOCK.asNode(), lockURI));
         assertUnlockWithToken(pid, lockToken);
@@ -339,7 +339,7 @@ public void testOmissionOfJCRCLocksRDF() throws IOException {
 
         final String lockToken = getLockToken(lockObject(pid));
 
-        final GraphStore rdf = getGraphStore(new HttpGet(serverAddress + pid));
+        final GraphStore rdf = getGraphStore(getObjectProperties(pid, lockToken));
 
         final Resource subject = createResource(serverAddress + pid);
         final String [] jcrLockTriples = new String[] {
@@ -403,7 +403,8 @@ public void testTransactionFailureOnCommit() throws IOException {
         Assert.assertEquals(CONFLICT.getStatusCode(), commitTransaction(txId).getStatusLine().getStatusCode());
 
         // verify that non-conflicting operation was not successful.
-        Assert.assertEquals(NOT_FOUND.getStatusCode(), getObjectProperties(inTxPid).getStatusLine().getStatusCode());
+        Assert.assertEquals(NOT_FOUND.getStatusCode(), getObjectProperties(inTxPid, lockToken)
+            .getStatusLine().getStatusCode());
     }
 
     /**
@@ -550,8 +551,10 @@ private HttpResponse commitTransaction(final String txId) throws IOException {
         return execute(commitTx);
     }
 
-    private HttpResponse getObjectProperties(final String pid) throws IOException {
-        return execute(new HttpGet(serverAddress + pid));
+    private HttpResponse getObjectProperties(final String pid, final String lockToken) throws IOException {
+        final HttpGet get = getObjMethod(pid);
+        addLockToken(get, lockToken);
+        return execute(get);
     }
 }
 
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
@@ -81,6 +81,7 @@
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Date;
 import java.util.List;
 import java.util.Locale;
 import java.util.Iterator;
@@ -108,6 +109,7 @@
 import org.apache.http.impl.client.cache.CachingHttpClientBuilder;
 import org.apache.http.util.EntityUtils;
 import org.fcrepo.http.commons.domain.RDFMediaType;
+import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.xml.sax.ErrorHandler;
@@ -123,6 +125,7 @@
 import com.hp.hpl.jena.rdf.model.Model;
 import com.hp.hpl.jena.rdf.model.Property;
 import com.hp.hpl.jena.rdf.model.Resource;
+import com.hp.hpl.jena.rdf.model.StmtIterator;
 import com.hp.hpl.jena.sparql.core.Quad;
 import com.hp.hpl.jena.update.GraphStore;
 
@@ -133,6 +136,17 @@
  */
 public class FedoraNodesIT extends AbstractResourceIT {
 
+    private SimpleDateFormat headerFormat;
+    private SimpleDateFormat tripleFormat;
+
+    @Before
+    public void setup() {
+        headerFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US);
+        headerFormat.setTimeZone(getTimeZone("GMT"));
+        tripleFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");
+        tripleFormat.setTimeZone(getTimeZone("GMT"));
+    }
+
     @Test
     public void testIngest() throws Exception {
         final String pid = getRandomUniquePid();
@@ -1654,8 +1668,6 @@ public void testLinkedDeletion() throws Exception {
     **/
     @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");
@@ -1671,7 +1683,7 @@ public void testLastModifiedUpdatedAfterUpdates() throws Exception {
         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();
+        final long lastmod1 = headerFormat.parse(resp1.getFirstHeader("Last-Modified").getValue()).getTime();
         assertTrue( (timestamp1 - lastmod1) < 1000 ); // because rounding
 
         // remove the file and wait for the TTL to expire
@@ -1683,7 +1695,7 @@ public void testLastModifiedUpdatedAfterUpdates() throws Exception {
         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();
+        final long lastmod2 = headerFormat.parse(resp2.getFirstHeader("Last-Modified").getValue()).getTime();
         assertTrue( (timestamp2 - lastmod2) < 1000 ); // because rounding
 
         assertFalse("Last-Modified headers should have changed", lastmod1 == lastmod2);
@@ -1704,4 +1716,33 @@ public void testUpdateObjectWithSpaces() throws Exception {
         assertEquals(NO_CONTENT.getStatusCode(), response.getStatusLine().getStatusCode());
     }
 
+    @Test
+    public void testCreatedAndModifiedDates() throws Exception {
+        final HttpResponse createResponse = createObject("");
+        final String location = createResponse.getFirstHeader("Location").getValue();
+        final HttpGet getObjMethod = new HttpGet(location);
+        final HttpResponse response = client.execute(getObjMethod);
+        final GraphStore results = getGraphStore(response);
+        final Model model = createModelForGraph(results.getDefaultGraph());
+        final Resource nodeUri = createResource(location);
+
+        final String lastmodString = response.getFirstHeader("Last-Modified").getValue();
+        final Date lastmodDate = headerFormat.parse(lastmodString);
+        final Date createdDateTriples = getDateFromModel( model, nodeUri,
+                createProperty(REPOSITORY_NAMESPACE + "created"));
+        final Date lastmodDateTriples = getDateFromModel( model, nodeUri,
+                createProperty(REPOSITORY_NAMESPACE + "lastModified"));
+        assertNotNull( createdDateTriples );
+        assertEquals( lastmodString, headerFormat.format(createdDateTriples) );
+        assertNotNull( lastmodDateTriples );
+        assertEquals( lastmodString, headerFormat.format(lastmodDateTriples) );
+    }
+
+    private Date getDateFromModel( final Model model, final Resource subj, final Property pred ) throws Exception {
+        final StmtIterator stmts = model.listStatements( subj, pred, (String)null );
+        if ( stmts.hasNext() ) {
+            return tripleFormat.parse(stmts.nextStatement().getString());
+        }
+        return null;
+    }
 }
diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/FedoraResourceImpl.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/FedoraResourceImpl.java
@@ -166,15 +166,24 @@ public Date getCreatedDate() throws RepositoryException {
      */
     @Override
     public Date getLastModifiedDate() throws RepositoryException {
+        final Date createdDate = getCreatedDate();
+
         if (node.hasProperty(JCR_LASTMODIFIED)) {
-            return new Date(node.getProperty(JCR_LASTMODIFIED).getDate()
-                            .getTimeInMillis());
+            final Date lastModifiedDate = new Date(node.getProperty(JCR_LASTMODIFIED).getDate().getTimeInMillis());
+
+            // make sure that lastModifiedDate isn't before createdDate
+            if ( createdDate != null && lastModifiedDate.before(createdDate) ) {
+                final Calendar cal = Calendar.getInstance();
+                cal.setTime(createdDate);
+                node.setProperty(JCR_LASTMODIFIED, cal);
+                return createdDate;
+            }
+            return lastModifiedDate;
         }
         LOGGER.debug(
                     "Could not get last modified date property for node {}",
                     node);
 
-        final Date createdDate = getCreatedDate();
 
         if (createdDate != null) {
             LOGGER.trace(

diff --git a/...po-kernel-impl/src/test/java/org/fcrepo/integration/kernel/impl/FedoraResourceImplIT.java b/...po-kernel-impl/src/test/java/org/fcrepo/integration/kernel/impl/FedoraResourceImplIT.java
@@ -145,6 +145,19 @@ public void testRandomNodeGraph() throws RepositoryException {
                 .contains(ANY, s, p, o));
     }
 
+    @Test
+    public void testLastModified() throws RepositoryException {
+        final String pid = UUID.randomUUID().toString();
+        final FedoraObject obj1 = objectService.createObject(session, "/" + pid);
+
+        session.save();
+        session.logout();
+        session = repo.login();
+
+        final FedoraObject obj2 = objectService.getObject(session, "/" + pid);
+        assertFalse( obj2.getLastModifiedDate().before(obj2.getCreatedDate()) );
+    }
+
     @Test
     public void testRepositoryRootGraph() throws RepositoryException {