Session.loadAll() sorts by SortOrder specified instead of by Ids. Fixes

#302.
neo4j · Dec 13, 2016 · 0801eec · 0801eec
1 parent d983ed2
commit 0801eec
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 27 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,3 +1,8 @@
+2.0.7-SNAPSHOT
+--------------
+o Fixes issue where session.loadAll would sort by ids instead of by the sort order specified. Fixes #302.
+
+
 2.0.6
 --------------
 o Support for Neo4j Bolt Driver 1.0.6

diff --git a/core/src/main/java/org/neo4j/ogm/session/delegates/LoadByIdsDelegate.java b/core/src/main/java/org/neo4j/ogm/session/delegates/LoadByIdsDelegate.java
@@ -12,11 +12,15 @@
  */
 package org.neo4j.ogm.session.delegates;
 
-import org.neo4j.ogm.annotation.RelationshipEntity;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import org.neo4j.ogm.context.GraphEntityMapper;
 import org.neo4j.ogm.cypher.query.AbstractRequest;
 import org.neo4j.ogm.cypher.query.Pagination;
 import org.neo4j.ogm.cypher.query.SortOrder;
-import org.neo4j.ogm.context.GraphEntityMapper;
+import org.neo4j.ogm.entity.io.EntityAccessManager;
 import org.neo4j.ogm.metadata.ClassInfo;
 import org.neo4j.ogm.model.GraphModel;
 import org.neo4j.ogm.request.GraphModelRequest;
@@ -25,12 +29,9 @@
 import org.neo4j.ogm.session.Neo4jSession;
 import org.neo4j.ogm.session.request.strategy.QueryStatements;
 
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
-
 /**
  * @author Vince Bickers
+ * @author Luanne Misquitta
  */
 public class LoadByIdsDelegate implements Capability.LoadByIds {
 
@@ -52,8 +53,14 @@ public <T> Collection<T> loadAll(Class<T> type, Collection<Long> ids, SortOrder
                 .setPagination(pagination);
 
         try (Response<GraphModel> response = session.requestHandler().execute((GraphModelRequest) qry)) {
-            new GraphEntityMapper(session.metaData(), session.context()).map(type, response);
-            return lookup(type, ids);
+            Iterable<T> mapped = new GraphEntityMapper(session.metaData(), session.context()).map(type, response);
+            Set<T> results = new LinkedHashSet<>();
+            for (T entity : mapped) {
+                if (includeMappedEntity(ids, entity)) {
+                    results.add(entity);
+                }
+            }
+            return results;
         }
     }
 
@@ -92,26 +99,11 @@ public <T> Collection<T> loadAll(Class<T> type, Collection<Long> ids, SortOrder
         return loadAll(type, ids, sortOrder, pagination, 1);
     }
 
-    private <T> Collection<T> lookup(Class<T> type, Collection<Long> ids) {
-
-        Set<T> results = new HashSet<>();
-        ClassInfo typeInfo = session.metaData().classInfo(type.getName());
-
-        for (Long id : ids) {
+    private <T>  boolean includeMappedEntity(Collection<Long> ids, T mapped) {
 
-            Object ref;
+        final ClassInfo classInfo = session.metaData().classInfo(mapped);
 
-            if (typeInfo.annotationsInfo().get(RelationshipEntity.CLASS) == null) {
-                ref = session.context().getNodeEntity(id);
-            } else {
-                ref = session.context().getRelationshipEntity(id);
-            }
-            try {
-                results.add(type.cast(ref));
-            } catch (ClassCastException cce) {
-                // do nothing, the object is not loadable in the domain;
-            }
-        }
-        return results;
+        Object id = EntityAccessManager.getIdentityPropertyReader(classInfo).read(mapped);
+        return ids.contains(id);
     }
 }
diff --git a/core/src/test/java/org/neo4j/ogm/persistence/session/capability/LoadCapabilityTest.java b/core/src/test/java/org/neo4j/ogm/persistence/session/capability/LoadCapabilityTest.java
@@ -20,9 +20,11 @@
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 
 import org.junit.After;
 import org.junit.Before;
@@ -659,6 +661,43 @@ public void shouldRefreshEntityStateWhenReloadedOnCleanSession() {
         assertEquals(0, ledZeppelin.getAlbums().size());
     }
 
+    /**
+     * @see Issue 302
+     */
+    @Test
+    public void shouldMaintainSortOrderWhenLoadingByIds() {
+        Artist led = new Artist("Led Zeppelin");
+        session.save(led);
+        Artist bonJovi = new Artist("Bon Jovi");
+        session.save(bonJovi);
+
+        List<Long> ids = Arrays.asList(beatlesId, led.getId(), bonJovi.getId());
+        SortOrder sortOrder = new SortOrder();
+        sortOrder.add(SortOrder.Direction.ASC, "name");
+        Iterable<Artist> artists = session.loadAll(Artist.class, ids, sortOrder);
+        assertNotNull(artists);
+        List<String> artistNames = new ArrayList<>();
+        for (Artist artist : artists) {
+            artistNames.add(artist.getName());
+        }
+        assertEquals("Bon Jovi", artistNames.get(0));
+        assertEquals("Led Zeppelin", artistNames.get(1));
+        assertEquals("The Beatles", artistNames.get(2));
+
+
+        sortOrder = new SortOrder();
+        sortOrder.add(SortOrder.Direction.DESC, "name");
+        artists = session.loadAll(Artist.class, ids, sortOrder);
+        assertNotNull(artists);
+        artistNames = new ArrayList<>();
+        for (Artist artist : artists) {
+            artistNames.add(artist.getName());
+        }
+        assertEquals("The Beatles", artistNames.get(0));
+        assertEquals("Led Zeppelin", artistNames.get(1));
+        assertEquals("Bon Jovi", artistNames.get(2));
+
+    }
 
 
 }