Fix bug that collapsed duplicate versions.

Resolves: https://www.pivotaltracker.com/story/show/75910204
fcrepo · Aug 7, 2014 · 3845a80 · 3845a80
1 parent 5f7f724
commit 3845a80
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 3 deletions.
diff --git a/fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/responses/ViewHelpers.java b/fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/responses/ViewHelpers.java
@@ -15,6 +15,7 @@
  */
 package org.fcrepo.http.commons.responses;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.hp.hpl.jena.graph.Node.ANY;
 import static com.hp.hpl.jena.rdf.model.ResourceFactory.createResource;
 import static org.fcrepo.jcr.FedoraJcrTypes.FCR_CONTENT;
@@ -119,7 +120,10 @@ public Iterator<Node> getChildVersions(final DatasetGraph dataset,
     }
 
     /**
-     * Return an iterator of Quads for versions.
+     * Return an iterator of Quads for versions in order by the
+     * last modification date for the versioned content.  When
+     * multiple versions exist with the same last modification
+     * date, the order of those redundant versions is undefined.
      *
      * @param dataset
      * @param subject
@@ -135,8 +139,11 @@ public Iterator<Node> getOrderedVersions(final DatasetGraph dataset,
         while (versions.hasNext()) {
             quad = versions.next();
             date = getVersionDate(dataset, quad.getObject());
-            map.put(date == null || date.length() == 0 ?
-                    format.format(new Date()) : date, quad.getObject());
+            String key = isNullOrEmpty(date) ? format.format(new Date()) : date;
+            while (map.containsKey(key)) {
+                key = key + "1";
+            }
+            map.put(key, quad.getObject());
         }
         return map.values().iterator();
     }

diff --git a/fcrepo-http-commons/src/test/java/org/fcrepo/http/commons/responses/ViewHelpersTest.java b/fcrepo-http-commons/src/test/java/org/fcrepo/http/commons/responses/ViewHelpersTest.java
@@ -88,6 +88,34 @@ public void testGetVersions() {
                      version, testObj.getVersions(mem, createURI("http://localhost/fcrepo/abc")).next());
     }
 
+    @Test
+    public void testGetOrderedVersions() {
+        final Node resource = createURI("http://localhost/fcrepo/abc");
+        final Node v1 = createURI("http://localhost/fcrepo/abc/fcr:version/1");
+        final Node v2 = createURI("http://localhost/fcrepo/abc/fcr:version/2");
+        final Node v3 = createURI("http://localhost/fcrepo/abc/fcr:version/3");
+        final Date now = new Date();
+        final Date later = new Date();
+        later.setTime(later.getTime() + 10000l);
+
+        final DatasetGraph mem = createMem();
+        final Node anon = createAnon();
+        mem.add(anon, resource, HAS_VERSION.asNode(), v1);
+        mem.add(anon, v1, LAST_MODIFIED_DATE.asNode(),
+                createLiteral(now.toString()));
+        mem.add(anon, resource, HAS_VERSION.asNode(), v2);
+        mem.add(anon, v2, LAST_MODIFIED_DATE.asNode(),
+                createLiteral(now.toString()));
+        mem.add(anon, resource, HAS_VERSION.asNode(), v3);
+        mem.add(anon, v3, LAST_MODIFIED_DATE.asNode(),
+                createLiteral(later.toString()));
+
+        final Iterator<Node> versions = testObj.getOrderedVersions(mem, resource, HAS_VERSION);
+        final Node r1 = versions.next();
+        final Node r2 = versions.next();
+        final Node r3 = versions.next();
+        assertEquals("Latest version should be last.", v3, r3);
+    }
     @Test
     public void testGetChildVersions() {
         final DatasetGraph mem = createMem();