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 2019/11/13 20:15:34 UTC
[tinkerpop] branch TINKERPOP-2314 created (now ff2c6c5)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch TINKERPOP-2314
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.
at ff2c6c5 TINKERPOP-2314 Employ by(String) for Map when possible
This branch includes the following new commits:
new ff2c6c5 TINKERPOP-2314 Employ by(String) for Map when possible
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-2314 Employ by(String) for Map when
possible
Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2314
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit ff2c6c56d6c80db89382e43ed56a50c0b16851bb
Author: stephen <sp...@gmail.com>
AuthorDate: Wed Nov 13 15:14:36 2019 -0500
TINKERPOP-2314 Employ by(String) for Map when possible
Also improve errors around incorrect types that might be called using this modulator overload.
---
CHANGELOG.asciidoc | 4 ++
docs/src/upgrade/release-3.4.x.asciidoc | 66 ++++++++++++++++++
.../traversal/lambda/ElementValueTraversal.java | 24 ++++++-
.../lambda/ElementValueTraversalTest.java | 78 ++++++++++++++++++++++
gremlin-test/features/map/Project.feature | 18 ++++-
.../process/traversal/step/map/ProjectTest.java | 22 ++++++
6 files changed, 210 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 45bda6a..6b54aee 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -25,6 +25,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
This release also includes changes from <<release-3-3-10, 3.3.10>>.
+* Expanded the use of `by(String)` modulator so that it can work on `Map` as well as `Element`.
+* Improved error messaging for `by(String)` so that it is more clear as to what the problem is
* Bump to Netty 4.1.42
* GraphBinary: Introduced our own `Buffer` API as a way to wrap Netty's Buffer API. Moved `GraphBinaryReader`, `GraphBinaryWriter` and `TypeSerializer<T>` to gremlin-core.
@@ -488,6 +490,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Bump to Tornado 5.x for gremlin-python.
* Deprecated `TraversalStrategies.applyStrategies()`.
* Reverted: Modified Java driver to use IP address rather than hostname to create connections.
+* Expanded the use of `by(String)` modulator so that it can work on `Map` as well as `Element`.
+* Improved error messaging for `by(String)` so that it is more clear as to what the problem is.
[[release-3-3-9]]
=== TinkerPop 3.3.9 (Release Date: October 14, 2019)
diff --git a/docs/src/upgrade/release-3.4.x.asciidoc b/docs/src/upgrade/release-3.4.x.asciidoc
index 8fce2e7..9a64523 100644
--- a/docs/src/upgrade/release-3.4.x.asciidoc
+++ b/docs/src/upgrade/release-3.4.x.asciidoc
@@ -28,6 +28,72 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
Please see the link:https://github.com/apache/tinkerpop/blob/3.4.5/CHANGELOG.asciidoc#release-3-4-5[changelog] for a
complete list of all the modifications that are part of this release.
+=== Upgrading for Users
+
+==== by(String) Modulator
+
+It is quite common to use the `by(String)` modulator when doing some for of selection operation where the `String` is
+the key to the value of the current `Traverser`, demonstrated as follows:
+
+[source,text]
+----
+gremlin> g.V().project('name').by('name')
+==>[name:marko]
+==>[name:vadas]
+==>[name:lop]
+==>[name:josh]
+==>[name:ripple]
+==>[name:peter]
+gremlin> g.V().order().by('name').values('name')
+==>josh
+==>lop
+==>marko
+==>peter
+==>ripple
+==>vadas
+----
+
+Of course, this approach usually only works when the current `Traverser` is an `Element`. If it is not an element, the
+error is swift and severe:
+
+[source,text]
+----
+gremlin> g.V().valueMap().project('x').by('name')
+java.util.LinkedHashMap cannot be cast to org.apache.tinkerpop.gremlin.structure.Element
+Type ':help' or ':h' for help.
+Display stack trace? [yN]n
+----
+
+and while it is perhaps straightforward to see the problem in the above example, it is not always clear exactly where
+the mistake is. The above example is the typical misuse of `by(String)` and comes when one tries to treat a `Map` the
+same way as an `Element` (which is quite reasonable).
+
+In 3.4.5, the issue of using `by(String)` on a `Map` and the error messaging have been resolved as follows:
+
+[source,text]
+----
+gremlin> g.V().valueMap().project('x').by('name')
+==>[x:[marko]]
+==>[x:[vadas]]
+==>[x:[lop]]
+==>[x:[josh]]
+==>[x:[ripple]]
+==>[x:[peter]]
+gremlin> g.V().elementMap().order().by('name')
+==>[id:4,label:person,name:josh,age:32]
+==>[id:3,label:software,name:lop,lang:java]
+==>[id:1,label:person,name:marko,age:29]
+==>[id:6,label:person,name:peter,age:35]
+==>[id:5,label:software,name:ripple,lang:java]
+==>[id:2,label:person,name:vadas,age:27]
+gremlin> g.V().values('name').project('x').by('name')
+The by("name") modulator can only be applied to a traverser that is an Element or a Map - it is being applied to [marko] a String class instead
+Type ':help' or ':h' for help.
+Display stack trace? [yN]n
+----
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2314[TINKERPOP-2314]
+
=== Upgrading for Providers
==== Graph Driver Providers
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversal.java
index fbfff62..73f73a6 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversal.java
@@ -22,9 +22,15 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
import org.apache.tinkerpop.gremlin.structure.Element;
+import java.util.Map;
import java.util.Objects;
/**
+ * More efficiently extracts a value from an {@link Element} or {@code Map} to avoid strategy application costs. Note
+ * that as of 3.4.5 this class is now poorly named as it was originally designed to work with {@link Element} only.
+ * In future 3.5.0 this class will likely be renamed but to ensure that we get this revised functionality for
+ * {@code Map} without introducing a breaking change this name will remain the same.
+ *
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
public final class ElementValueTraversal<V> extends AbstractLambdaTraversal<Element, V> {
@@ -48,7 +54,23 @@ public final class ElementValueTraversal<V> extends AbstractLambdaTraversal<Elem
@Override
public void addStart(final Traverser.Admin<Element> start) {
- this.value = null == this.bypassTraversal ? start.get().value(this.propertyKey) : TraversalUtil.apply(start, this.bypassTraversal);
+ if (null == this.bypassTraversal) {
+ // playing a game of type erasure here.....technically we can process either Map or Element here in this
+ // case after 3.4.5. and obviously users get weird errors along those lines here anyway. will fix up the
+ // generics in 3.5.0 when we can take some breaking changes. seemed like this feature would make Gremlin
+ // a lot nicer and given the small footprint of the change this seemed like a good hack to take.
+ final Object o = start.get();
+ if (o instanceof Element)
+ this.value = ((Element) o).value(propertyKey);
+ else if (o instanceof Map)
+ this.value = (V) ((Map) o).get(propertyKey);
+ else
+ throw new IllegalStateException(String.format(
+ "The by(\"%s\") modulator can only be applied to a traverser that is an Element or a Map - it is being applied to [%s] a %s class instead",
+ propertyKey, o, o.getClass().getSimpleName()));
+ } else {
+ this.value = TraversalUtil.apply(start, this.bypassTraversal);
+ }
}
public String getPropertyKey() {
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversalTest.java
new file mode 100644
index 0000000..2752a3d
--- /dev/null
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversalTest.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.process.traversal.lambda;
+
+import org.apache.tinkerpop.gremlin.process.traversal.traverser.B_O_Traverser;
+import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.structure.VertexProperty;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class ElementValueTraversalTest {
+
+ @Test
+ public void shouldWorkOnVertex() {
+ final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
+ final Vertex v = mock(Vertex.class);
+ when(v.value("age")).thenReturn(29);
+ t.addStart(new B_O_Traverser(v, 1).asAdmin());
+ assertEquals(29, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnEdge() {
+ final ElementValueTraversal<Double> t = new ElementValueTraversal<>("weight");
+ final Edge e = mock(Edge.class);
+ when(e.value("weight")).thenReturn(1.0d);
+ t.addStart(new B_O_Traverser(e, 1).asAdmin());
+ assertEquals(1.0d, t.next(), 0.00001d);
+ }
+
+ @Test
+ public void shouldWorkOnVertexProperty() {
+ final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
+ final VertexProperty vp = mock(VertexProperty.class);
+ when(vp.value("age")).thenReturn(29);
+ t.addStart(new B_O_Traverser(vp, 1).asAdmin());
+ assertEquals(29, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnMap() {
+ final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
+ final Map<String,Integer> m = new HashMap<>();
+ m.put("age", 29);
+ t.addStart(new B_O_Traverser(m, 1).asAdmin());
+ assertEquals(29, t.next().intValue());
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void shouldThrowExceptionWhenTryingUnsupportedType() {
+ final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
+ t.addStart(new B_O_Traverser(29, 1).asAdmin());
+ t.next();
+ }
+}
diff --git a/gremlin-test/features/map/Project.feature b/gremlin-test/features/map/Project.feature
index 8974312..f3666b8 100644
--- a/gremlin-test/features/map/Project.feature
+++ b/gremlin-test/features/map/Project.feature
@@ -52,4 +52,20 @@ Feature: Step - project()
| lop |
| lop |
| lop |
- | ripple |
\ No newline at end of file
+ | ripple |
+
+ Scenario: g_V_valueMap_projectXxX_byXselectXnameXX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().valueMap().project("x").by(__.select("name"))
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | m[{"x":["marko"]}] |
+ | m[{"x":["josh"]}] |
+ | m[{"x":["vadas"]}] |
+ | m[{"x":["peter"]}] |
+ | m[{"x":["lop"]}] |
+ | m[{"x":["ripple"]}] |
\ No newline at end of file
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProjectTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProjectTest.java
index 6bfae90..af71500 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProjectTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProjectTest.java
@@ -27,6 +27,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.junit.Test;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -42,6 +43,8 @@ public abstract class ProjectTest extends AbstractGremlinProcessTest {
public abstract Traversal<Vertex, String> get_g_V_outXcreatedX_projectXa_bX_byXnameX_byXinXcreatedX_countX_order_byXselectXbX__descX_selectXaX();
+ public abstract Traversal<Vertex, Map<String, Object>> get_g_V_valueMap_projectXxX_byXselectXnameXX();
+
@Test
@LoadGraphWith(MODERN)
public void g_V_hasLabelXpersonX_projectXa_bX_byXoutE_countX_byXageX() {
@@ -67,6 +70,20 @@ public abstract class ProjectTest extends AbstractGremlinProcessTest {
assertEquals("ripple", names.get(3));
}
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_V_valueMap_projectXxX_byXselectXnameXX() {
+ final Traversal<Vertex, Map<String, Object>> traversal = get_g_V_valueMap_projectXxX_byXselectXnameXX();
+ printTraversalForm(traversal);
+ checkResults(makeMapList(1,
+ "x", Collections.singletonList("marko"),
+ "x", Collections.singletonList("vadas"),
+ "x", Collections.singletonList("lop"),
+ "x", Collections.singletonList("josh"),
+ "x", Collections.singletonList("ripple"),
+ "x", Collections.singletonList("peter")), traversal);
+ }
+
public static class Traversals extends ProjectTest {
@Override
@@ -78,5 +95,10 @@ public abstract class ProjectTest extends AbstractGremlinProcessTest {
public Traversal<Vertex, String> get_g_V_outXcreatedX_projectXa_bX_byXnameX_byXinXcreatedX_countX_order_byXselectXbX__descX_selectXaX() {
return g.V().out("created").project("a", "b").by("name").by(__.in("created").count()).order().by(__.select("b"), Order.desc).select("a");
}
+
+ @Override
+ public Traversal<Vertex, Map<String, Object>> get_g_V_valueMap_projectXxX_byXselectXnameXX() {
+ return g.V().valueMap().project("x").by(__.select("name"));
+ }
}
}