diff --git a/CHANGES.txt b/CHANGES.txt index 274f45dc7e..903c933255 100644 --- 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 index 9068c25c28..06b1af346d 100644 --- 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 Collection loadAll(Class type, Collection ids, SortOrder .setPagination(pagination); try (Response response = session.requestHandler().execute((GraphModelRequest) qry)) { - new GraphEntityMapper(session.metaData(), session.context()).map(type, response); - return lookup(type, ids); + Iterable mapped = new GraphEntityMapper(session.metaData(), session.context()).map(type, response); + Set results = new LinkedHashSet<>(); + for (T entity : mapped) { + if (includeMappedEntity(ids, entity)) { + results.add(entity); + } + } + return results; } } @@ -92,26 +99,11 @@ public Collection loadAll(Class type, Collection ids, SortOrder return loadAll(type, ids, sortOrder, pagination, 1); } - private Collection lookup(Class type, Collection ids) { - - Set results = new HashSet<>(); - ClassInfo typeInfo = session.metaData().classInfo(type.getName()); - - for (Long id : ids) { + private boolean includeMappedEntity(Collection 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 index 89acb63651..12147b2582 100644 --- 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 ids = Arrays.asList(beatlesId, led.getId(), bonJovi.getId()); + SortOrder sortOrder = new SortOrder(); + sortOrder.add(SortOrder.Direction.ASC, "name"); + Iterable artists = session.loadAll(Artist.class, ids, sortOrder); + assertNotNull(artists); + List 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)); + + } }