From ccbe96c434c40c1cee44718e73b32405e79c2964 Mon Sep 17 00:00:00 2001 From: Frantisek Hartman Date: Tue, 15 Aug 2017 18:09:50 +0200 Subject: [PATCH] Keep order for loadAll by objects or ids, fixes #196 Methods loadAll(Collection objects, ..) and loadAll(Collection, ..) without SortOrder parameter now keep order of the input collection. --- .../session/delegates/LoadByIdsDelegate.java | 32 ++++++++++++ .../delegates/LoadByInstancesDelegate.java | 4 +- .../org/neo4j/ogm/domain/music/Artist.java | 8 +++ .../capability/LoadCapabilityTest.java | 49 +++++++++++++++++++ 4 files changed, 91 insertions(+), 2 deletions(-) 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 93966d3b35..b6d9443d1b 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 @@ -14,7 +14,9 @@ import java.io.Serializable; import java.util.Collection; +import java.util.HashMap; import java.util.LinkedHashSet; +import java.util.Map; import java.util.Set; import org.neo4j.ogm.context.GraphEntityMapper; @@ -55,6 +57,10 @@ public Collection loadAll(Class type, Collect GraphModelRequest request = new DefaultGraphModelRequest(qry.getStatement(), qry.getParameters()); try (Response response = session.requestHandler().execute(request)) { Iterable mapped = new GraphEntityMapper(session.metaData(), session.context()).map(type, response); + + if (sortOrder.sortClauses().isEmpty()) { + return sortResultsByIds(type, ids, mapped); + } Set results = new LinkedHashSet<>(); for (T entity : mapped) { if (includeMappedEntity(ids, entity)) { @@ -65,6 +71,32 @@ public Collection loadAll(Class type, Collect } } + private Set sortResultsByIds(Class type, Collection ids, Iterable mapped) { + Map items = new HashMap<>(); + ClassInfo classInfo = session.metaData().classInfo(type.getName()); + + FieldInfo idField = classInfo.primaryIndexField(); + if (idField == null) { + idField = classInfo.identityField(); + } + + for (T t : mapped) { + Object id = idField.read(t); + if (id != null) { + items.put((ID) id, t); + } + } + + Set results = new LinkedHashSet<>(); + for (ID id : ids) { + T item = items.get(id); + if (item != null) { + results.add(item); + } + } + return results; + } + public Collection loadAll(Class type, Collection ids) { return loadAll(type, ids, new SortOrder(), null, 1); } diff --git a/core/src/main/java/org/neo4j/ogm/session/delegates/LoadByInstancesDelegate.java b/core/src/main/java/org/neo4j/ogm/session/delegates/LoadByInstancesDelegate.java index 1eb955460b..cfce8be182 100644 --- a/core/src/main/java/org/neo4j/ogm/session/delegates/LoadByInstancesDelegate.java +++ b/core/src/main/java/org/neo4j/ogm/session/delegates/LoadByInstancesDelegate.java @@ -14,7 +14,7 @@ import java.lang.reflect.Field; import java.util.Collection; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; import org.neo4j.ogm.cypher.query.Pagination; @@ -40,7 +40,7 @@ public Collection loadAll(Collection objects, SortOrder sortOrder, Pag return objects; } - Set ids = new HashSet<>(); + Set ids = new LinkedHashSet<>(); Class type = objects.iterator().next().getClass(); ClassInfo classInfo = session.metaData().classInfo(type.getName()); Field identityField = classInfo.getField(classInfo.identityField()); diff --git a/test/src/test/java/org/neo4j/ogm/domain/music/Artist.java b/test/src/test/java/org/neo4j/ogm/domain/music/Artist.java index f846c44feb..0e82e2927b 100644 --- a/test/src/test/java/org/neo4j/ogm/domain/music/Artist.java +++ b/test/src/test/java/org/neo4j/ogm/domain/music/Artist.java @@ -78,4 +78,12 @@ public void addAlbum(Album album) { public Long getId() { return id; } + + @Override + public String toString() { + return "Artist{" + + "id=" + id + + ", name='" + name + '\'' + + '}'; + } } diff --git a/test/src/test/java/org/neo4j/ogm/persistence/session/capability/LoadCapabilityTest.java b/test/src/test/java/org/neo4j/ogm/persistence/session/capability/LoadCapabilityTest.java index 0ccb5d6b6d..3bd04f9ffb 100644 --- a/test/src/test/java/org/neo4j/ogm/persistence/session/capability/LoadCapabilityTest.java +++ b/test/src/test/java/org/neo4j/ogm/persistence/session/capability/LoadCapabilityTest.java @@ -13,6 +13,7 @@ package org.neo4j.ogm.persistence.session.capability; +import static com.google.common.collect.Lists.newArrayList; import static org.assertj.core.api.Assertions.*; import java.io.IOException; @@ -678,4 +679,52 @@ public void shouldMaintainSortOrderWhenLoadingByIds() { assertThat(artistNames.get(1)).isEqualTo("Led Zeppelin"); assertThat(artistNames.get(2)).isEqualTo("Bon Jovi"); } + + @Test + public void loadAllByIdsShouldSortByIdsIfSortOrderIsNotProvided() throws Exception { + Artist beatles = session.load(Artist.class, beatlesId); + + Artist led = new Artist("Led Zeppelin"); + session.save(led); + Artist bonJovi = new Artist("Bon Jovi"); + session.save(bonJovi); + + Long ledId = led.getId(); + Long bonJoviId = bonJovi.getId(); + + Collection artists; + + artists = session.loadAll(Artist.class, newArrayList(beatlesId, ledId, bonJoviId)); + assertThat(artists).containsExactly(beatles, led, bonJovi); + + artists = session.loadAll(Artist.class, newArrayList(ledId, beatlesId, bonJoviId)); + assertThat(artists).containsExactly(led, beatles, bonJovi); + + artists = session.loadAll(Artist.class, newArrayList(ledId, bonJoviId, beatlesId)); + assertThat(artists).containsExactly(led, bonJovi, beatles); + } + + @Test + public void loadAllByInstancesShouldSortByIdsIfSortOrderIsNotProvided() throws Exception { + Artist beatles = session.load(Artist.class, beatlesId); + + Artist led = new Artist("Led Zeppelin"); + session.save(led); + Artist bonJovi = new Artist("Bon Jovi"); + session.save(bonJovi); + + Long ledId = led.getId(); + Long bonJoviId = bonJovi.getId(); + + Collection artists; + + artists = session.loadAll(newArrayList(beatles, led, bonJovi)); + assertThat(artists).containsExactly(beatles, led, bonJovi); + + artists = session.loadAll(newArrayList(led, beatles, bonJovi)); + assertThat(artists).containsExactly(led, beatles, bonJovi); + + artists = session.loadAll(newArrayList(led, bonJovi, beatles)); + assertThat(artists).containsExactly(led, bonJovi, beatles); + } }