You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2020/03/19 20:31:04 UTC
[tinkerpop] branch TINKERPOP-2351 created (now 40d50ea)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch TINKERPOP-2351
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.
at 40d50ea TINKERPOP-2351 Fixed bug in Order when enum is a key in Map
This branch includes the following new commits:
new 40d50ea TINKERPOP-2351 Fixed bug in Order when enum is a key in Map
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[tinkerpop] 01/01: TINKERPOP-2351 Fixed bug in Order when enum is a
key in Map
Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2351
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 40d50eaa126f378f9cf752bdfe5949a1f00f1eaa
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Thu Mar 19 16:30:19 2020 -0400
TINKERPOP-2351 Fixed bug in Order when enum is a key in Map
---
CHANGELOG.asciidoc | 1 +
.../tinkerpop/gremlin/process/traversal/Order.java | 20 +++++--
.../gremlin/process/traversal/OrderTest.java | 3 +
gremlin-test/features/map/Order.feature | 17 +++++-
.../process/traversal/step/map/OrderTest.java | 65 +++++++++++++++++-----
5 files changed, 86 insertions(+), 20 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 7b9ec50..c7db3b4 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -27,6 +27,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Added `trustStoreType` such that keystore and truststore can be of different types in the Java driver.
* Added session support to gremlin-javascript.
* Modified Gremlin Server to close the session when the channel itself is closed.
+* Fixed bug in `Order` where comparisons of `enum` types wouldn't compare with `String` values.
* Added `maxWaitForClose` configuration option to the Java driver.
* Deprecated `maxWaitForSessionClose` in the Java driver.
* Remove invalid service descriptors from gremlin-shaded
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java
index 618cdfd..8a24b1b 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java
@@ -98,9 +98,13 @@ public enum Order implements Comparator<Object> {
asc {
@Override
public int compare(final Object first, final Object second) {
- return first instanceof Number && second instanceof Number
- ? NumberHelper.compare((Number) first, (Number) second)
- : Comparator.<Comparable>naturalOrder().compare((Comparable) first, (Comparable) second);
+ // need to convert enum to string representations for comparison or else you can get cast exceptions.
+ // this typically happens when sorting local on the keys of maps that contain T
+ final Object f = first instanceof Enum<?> ? ((Enum<?>) first).name() : first;
+ final Object s = second instanceof Enum<?> ? ((Enum<?>) second).name() : second;
+ return f instanceof Number && s instanceof Number
+ ? NumberHelper.compare((Number) f, (Number) s)
+ : Comparator.<Comparable>naturalOrder().compare((Comparable) f, (Comparable) s);
}
@Override
@@ -117,9 +121,13 @@ public enum Order implements Comparator<Object> {
desc {
@Override
public int compare(final Object first, final Object second) {
- return first instanceof Number && second instanceof Number
- ? NumberHelper.compare((Number) second, (Number) first)
- : Comparator.<Comparable>reverseOrder().compare((Comparable) first, (Comparable) second);
+ // need to convert enum to string representations for comparison or else you can get cast exceptions.
+ // this typically happens when sorting local on the keys of maps that contain T
+ final Object f = first instanceof Enum<?> ? ((Enum<?>) first).name() : first;
+ final Object s = second instanceof Enum<?> ? ((Enum<?>) second).name() : second;
+ return f instanceof Number && s instanceof Number
+ ? NumberHelper.compare((Number) s, (Number) f)
+ : Comparator.<Comparable>reverseOrder().compare((Comparable) f, (Comparable) s);
}
@Override
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OrderTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OrderTest.java
index 0db2039..b2d3084 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OrderTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OrderTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.process.traversal;
+import org.apache.tinkerpop.gremlin.structure.T;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -58,6 +59,8 @@ public class OrderTest {
{Order.desc, Arrays.asList(100L, 1L, -1L, 0L), Arrays.asList(100L, 1L, 0L, -1L)},
{Order.asc, Arrays.asList(100, 1, -1, 0), Arrays.asList(-1, 0, 1, 100)},
{Order.desc, Arrays.asList(100, 1, -1, 0), Arrays.asList(100, 1, 0, -1)},
+ {Order.asc, Arrays.asList("b", "a", T.id, "c", "d"), Arrays.asList("a", "b", "c", "d", T.id)},
+ {Order.desc, Arrays.asList("b", "a", T.id, "c", "d"), Arrays.asList(T.id, "d", "c", "b", "a")},
{Order.incr, Arrays.asList("b", "a", "c", "d"), Arrays.asList("a", "b", "c", "d")},
{Order.decr, Arrays.asList("b", "a", "c", "d"), Arrays.asList("d", "c", "b", "a")},
{Order.incr, Arrays.asList(formatter.parse("1-Jan-2018"), formatter.parse("1-Jan-2020"), formatter.parse("1-Jan-2008")),
diff --git a/gremlin-test/features/map/Order.feature b/gremlin-test/features/map/Order.feature
index 1d4331f..8debc4f 100644
--- a/gremlin-test/features/map/Order.feature
+++ b/gremlin-test/features/map/Order.feature
@@ -367,4 +367,19 @@ Feature: Step - order()
Then nothing should happen because
"""
TODO
- """
\ No newline at end of file
+ """
+
+ Scenario: g_VX1X_valueMapXtrueX_orderXlocalX_byXkeys_descXunfold
+ Given the modern graph
+ And using the parameter v1Id defined as "v[marko].id"
+ And the traversal of
+ """
+ g.V(v1Id).valueMap(true).order(Scope.local).by(Column.keys, Order.desc).unfold()
+ """
+ When iterated to list
+ Then the result should be ordered
+ | result |
+ | m[{"name":["marko"]}] |
+ | m[{"t[label]":"person"}] |
+ | m[{"t[id]":"v[marko].id"}] |
+ | m[{"age":[29]}] |
\ No newline at end of file
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java
index b2fafb9..aaf87a9 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java
@@ -42,9 +42,13 @@ import java.util.Map;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.GRATEFUL;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
+import static org.apache.tinkerpop.gremlin.process.traversal.Order.desc;
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.outE;
+import static org.apache.tinkerpop.gremlin.structure.Column.keys;
+import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -104,6 +108,8 @@ public abstract class OrderTest extends AbstractGremlinProcessTest {
public abstract Traversal<Vertex, Map.Entry<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_unfold_order_byXvalues_descX();
+ public abstract Traversal<Vertex, Map.Entry<Object, Object>> get_g_VX1X_valueMapXtrueX_orderXlocalX_byXkeys_descXunfold(final Object v1Id);
+
@Test
@LoadGraphWith(MODERN)
public void g_V_name_order() {
@@ -449,6 +455,34 @@ public abstract class OrderTest extends AbstractGremlinProcessTest {
assertFalse(traversal.hasNext());
}
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_VX1X_valueMapXtrueX_orderXlocalX_byXkeys_descXunfold() {
+ final Traversal<Vertex, ?> traversal = get_g_VX1X_valueMapXtrueX_orderXlocalX_byXkeys_descXunfold(convertToVertexId(graph, "marko"));
+ printTraversalForm(traversal);
+
+ final Object name = traversal.next();
+ assertEquals("name", getKey(name));
+ final Object label = traversal.next();
+ assertEquals(T.label, getKey(label));
+ final Object id = traversal.next();
+ assertEquals(T.id, getKey(id));
+ final Object age = traversal.next();
+ assertEquals("age", getKey(age));
+
+ assertThat(traversal.hasNext(), is(false));
+ }
+
+ public Object getKey(final Object kv) {
+ // remotes return LinkedHashMap and embedded returns Map.Entry :/
+ if (kv instanceof Map.Entry)
+ return ((Map.Entry) kv).getKey();
+ else if (kv instanceof Map)
+ return ((Map) kv).keySet().iterator().next();
+ else
+ throw new IllegalStateException("Returned value should be a Map or Map.Entry but got: " + kv.getClass().getName());
+ }
+
public static class Traversals extends OrderTest {
@Override
@@ -483,7 +517,7 @@ public abstract class OrderTest extends AbstractGremlinProcessTest {
@Override
public Traversal<Vertex, Double> get_g_V_outE_order_byXweight_descX_weight() {
- return g.V().outE().order().by("weight", Order.desc).values("weight");
+ return g.V().outE().order().by("weight", desc).values("weight");
}
@Override
@@ -507,27 +541,27 @@ public abstract class OrderTest extends AbstractGremlinProcessTest {
map.put(3, (int) v.get().value("age") * 3);
map.put(4, (int) v.get().value("age"));
return map;
- }).order(Scope.local).by(Column.values, Order.desc).by(Column.keys, Order.asc);
+ }).order(Scope.local).by(Column.values, desc).by(keys, Order.asc);
}
@Override
public Traversal<Vertex, Vertex> get_g_V_order_byXoutE_count_descX() {
- return g.V().order().by(outE().count(), Order.desc);
+ return g.V().order().by(outE().count(), desc);
}
@Override
public Traversal<Vertex, Map<String, List<Vertex>>> get_g_V_group_byXlabelX_byXname_order_byXdescX_foldX() {
- return g.V().<String, List<Vertex>>group().by(T.label).by(__.values("name").order().by(Order.desc).fold());
+ return g.V().<String, List<Vertex>>group().by(T.label).by(__.values("name").order().by(desc).fold());
}
@Override
public Traversal<Vertex, List<Double>> get_g_V_localXbothE_weight_foldX_order_byXsumXlocalX_descX() {
- return g.V().local(__.bothE().<Double>values("weight").fold()).order().by(__.sum(Scope.local), Order.desc);
+ return g.V().local(__.bothE().<Double>values("weight").fold()).order().by(__.sum(Scope.local), desc);
}
@Override
public Traversal<Vertex, Map<String, Object>> get_g_V_asXvX_mapXbothE_weight_foldX_sumXlocalX_asXsX_selectXv_sX_order_byXselectXsX_descX() {
- return g.V().as("v").map(__.bothE().<Double>values("weight").fold()).sum(Scope.local).as("s").select("v", "s").order().by(__.select("s"), Order.desc);
+ return g.V().as("v").map(__.bothE().<Double>values("weight").fold()).sum(Scope.local).as("s").select("v", "s").order().by(__.select("s"), desc);
}
@Override
@@ -542,32 +576,32 @@ public abstract class OrderTest extends AbstractGremlinProcessTest {
@Override
public Traversal<Vertex, String> get_g_V_hasLabelXpersonX_order_byXvalueXageX_descX_name() {
- return g.V().hasLabel("person").order().<Vertex>by(v -> v.value("age"), Order.desc).values("name");
+ return g.V().hasLabel("person").order().<Vertex>by(v -> v.value("age"), desc).values("name");
}
@Override
public Traversal<Vertex, String> get_g_V_properties_order_byXkey_descX_key() {
- return g.V().properties().order().by(T.key, Order.desc).key();
+ return g.V().properties().order().by(T.key, desc).key();
}
@Override
public Traversal<Vertex, Vertex> get_g_V_hasXsong_name_OHBOYX_outXfollowedByX_outXfollowedByX_order_byXperformancesX_byXsongType_descX() {
- return g.V().has("song", "name", "OH BOY").out("followedBy").out("followedBy").order().by("performances").by("songType", Order.desc);
+ return g.V().has("song", "name", "OH BOY").out("followedBy").out("followedBy").order().by("performances").by("songType", desc);
}
@Override
public Traversal<Vertex, String> get_g_V_both_hasLabelXpersonX_order_byXage_descX_limitX5X_name() {
- return g.V().both().hasLabel("person").order().by("age", Order.desc).limit(5).values("name");
+ return g.V().both().hasLabel("person").order().by("age", desc).limit(5).values("name");
}
@Override
public Traversal<Vertex, String> get_g_V_both_hasLabelXpersonX_order_byXage_descX_name() {
- return g.V().both().hasLabel("person").order().by("age", Order.desc).values("name");
+ return g.V().both().hasLabel("person").order().by("age", desc).values("name");
}
@Override
public Traversal<Vertex, String> get_g_V_hasLabelXsongX_order_byXperformances_descX_byXnameX_rangeX110_120X_name() {
- return g.V().hasLabel("song").order().by("performances", Order.desc).by("name").range(110, 120).values("name");
+ return g.V().hasLabel("song").order().by("performances", desc).by("name").range(110, 120).values("name");
}
@Override
@@ -577,7 +611,12 @@ public abstract class OrderTest extends AbstractGremlinProcessTest {
@Override
public Traversal<Vertex, Map.Entry<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_unfold_order_byXvalues_descX() {
- return g.V().hasLabel("person").group().by("name").by(outE().values("weight").sum()).<Map.Entry<String, Number>>unfold().order().by(Column.values, Order.desc);
+ return g.V().hasLabel("person").group().by("name").by(outE().values("weight").sum()).<Map.Entry<String, Number>>unfold().order().by(Column.values, desc);
+ }
+
+ @Override
+ public Traversal<Vertex, Map.Entry<Object, Object>> get_g_VX1X_valueMapXtrueX_orderXlocalX_byXkeys_descXunfold(final Object v1Id) {
+ return g.V(v1Id).valueMap(true).order(Scope.local).by(keys, desc).unfold();
}
}
}