From ee5a70883ce4518ccea1be30572493ba95d61b8d Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 3 Jun 2026 15:24:01 -0700 Subject: [PATCH 1/6] Implement Tree change Assisted-by: Kiro: Claude Opus 4.8 --- CHANGELOG.asciidoc | 1 + docs/src/reference/the-traversal.asciidoc | 13 +- docs/src/upgrade/release-4.x.x.asciidoc | 34 +++ .../process/traversal/step/map/TreeStep.java | 4 +- .../step/sideEffect/TreeSideEffectStep.java | 4 +- .../process/traversal/step/util/Tree.java | 284 +++++++++++++----- .../io/binary/types/TreeSerializer.java | 11 +- .../io/graphson/GraphSONSerializersV1.java | 14 +- .../io/graphson/GraphSONSerializersV2.java | 15 +- .../io/graphson/GraphSONSerializersV3.java | 14 +- .../io/graphson/GraphSONSerializersV4.java | 14 +- .../util/detached/DetachedFactory.java | 16 +- .../util/reference/ReferenceFactory.java | 16 +- .../process/traversal/step/util/TreeTest.java | 251 +++++++++++++--- .../util/function/TreeSupplierTest.java | 4 +- 15 files changed, 530 insertions(+), 165 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 097aacb7c52..97a76db3ab9 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -27,6 +27,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Added configurable CORS `allowedOrigins` setting to Gremlin Server; warns when wildcard origin is used alongside authentication. * Fixed `ByteBuf` leak in `GraphBinaryMessageSerializerV4` when serialization throws an `IOException`. +* Changed `Tree` to no longer extend `HashMap`; it is now a final class with a tree-shaped API (`childAt`, `hasChild`, `contains`, `findSubtree`, `getOrCreateChild`, `getNodesAtDepth`, `getLeafNodes`, `nodeCount`) and is no longer a `Map`. * Added typed numeric wrappers and `preciseNumbers` connection option to `gremlin-javascript` for explicit control over numeric type serialization and deserialization. * Added `NextN(n)` to `Traversal` in `gremlin-go` for batched result iteration, providing API parity with `next(n)` in the Java, Python, and .NET GLVs, and updated the Go translators in `gremlin-core` and `gremlin-javascript` to emit `NextN(n)` for the batched form. * Added Provider Defined Types (PDT) support — graph providers can define custom types via `@ProviderDefined` annotation that serialize/deserialize seamlessly across all GLVs without driver-side configuration. Replaces TP3 custom type mechanism. diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index 75b7aba7b36..548c2beb35a 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -5156,14 +5156,14 @@ It is important to see how the paths of all the emanating traversers are united image::tree-step2.png[width=500] -The resultant tree data structure can then be manipulated (see `Tree` JavaDoc). +The resultant tree data structure can then be navigated with its tree-shaped API such as `childAt()`, `findSubtree()`, `getNodesAtDepth()`, and `getLeafNodes()` (see `Tree` JavaDoc). [gremlin-groovy,modern] ---- tree = g.V().out().out().tree().by('name').next() -tree['marko'] -tree['marko']['josh'] -tree.getObjectsAtDepth(3) +tree.childAt('marko') +tree.childAt('marko').childAt('josh') +tree.getNodesAtDepth(2) ---- Note that when using `by()`-modulation, tree nodes are combined based on projection uniqueness, not on the @@ -5179,6 +5179,11 @@ g.V().has('name','josh').out('created').values('name'). <1> When the `tree()` is created, vertex 3 and 5 are unique and thus, form unique branches in the tree structure. <2> When the `tree()` is `by()`-modulated by `label`, then vertex 3 and 5 are both "software" and thus are merged to a single node in the tree. +NOTE: A `Tree` keys its children by node value, so sibling branches that resolve to the same value (whether the +original objects or a `by()`-projection) collapse into a single node. This means structure can be lost when distinct +paths share a value at the same depth. If every path must be preserved, use `path()`; if a queryable result that +retains element identity, cycles, and parallel edges is required, use `subgraph()`. + The `tree()` step can also take a side-effect key as an argument. When using this form, the `Tree` is is built up in a side-effect as each traverser passes through. The `Tree` can later be accessed by either `select()` or `cap()`. diff --git a/docs/src/upgrade/release-4.x.x.asciidoc b/docs/src/upgrade/release-4.x.x.asciidoc index c5c45a284eb..ab298eb28a8 100644 --- a/docs/src/upgrade/release-4.x.x.asciidoc +++ b/docs/src/upgrade/release-4.x.x.asciidoc @@ -635,6 +635,34 @@ effort to the old custom serializer approach but is entirely optional for basic * <> +==== Tree No Longer Extends HashMap + +`Tree` no longer extends `HashMap`. It is now a `final` class that holds a `Map` internally and exposes a +tree-shaped API instead of `Map` methods. This is a breaking change for code that treated a `tree()` result as a +`Map`. + +Replacements for the common `Map`-based access patterns: + +[options="header"] +|======================= +|3.x (`Tree` as `Map`) |4.x (`Tree` API) +|`tree.get(key)` |`tree.childAt(key)` (throws if absent) or `tree.findSubtree(key)` (recursive, returns `Optional`) +|`tree.containsKey(key)` |`tree.hasChild(key)` +|`tree.keySet()` |`tree.rootNodes()` +|`tree.size()` |`tree.rootNodes().size()` for root entries, or `tree.nodeCount()` for total nodes +|`getObjectsAtDepth(d)` |`getNodesAtDepth(d)` (now 0-based: depth 0 returns the roots) +|`getLeafObjects()` |`getLeafNodes()` +|======================= + +A few Gremlin patterns that worked only because `Tree` was a `Map` (for example `select(keys)`, `count(local)`, +and `unfold()` applied to a `Tree`) no longer compose. Process the `Tree` result client-side after `next()`, or +reshape the upstream traversal. + +`isLeaf()` no longer throws on an empty tree (it returns `true`), and a `Tree` keeps the long-standing limitation +that sibling branches resolving to the same value collapse into one node; use `path()` or `subgraph()` when full +path structure must be preserved. + + === Upgrading for Providers ==== Graph System Providers @@ -654,6 +682,12 @@ benefiting all driver users transparently. See <> for full details on annotation usage, field filtering, nested types, and ServiceLoader registration. +`Tree` no longer extends `HashMap`. Provider code that inspected or rebuilt a `Tree` via `Map` methods +(`get`, `put`, `keySet`, `entrySet`, `size`) must use the tree-shaped API instead: read with `rootNodes()` + +`childAt(key)` and build with `getOrCreateChild(key)` + `addTree(subtree)`. The GraphSON (`g:Tree`) and +GraphBinary (`0x2b`) wire formats are unchanged, so no serialized data or cross-version compatibility is affected +— only the in-memory Java type changed. + ==== Graph Driver Providers ===== Request Interceptors diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java index 930d2d8a55e..ecc26e96950 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java @@ -85,9 +85,7 @@ public Tree projectTraverser(final Traverser.Admin traverser) { final TraversalProduct product = TraversalUtil.produce(path.get(i), this.traversalRing.next()); if (product.isProductive()) { final Object object = product.get(); - if (!depth.containsKey(object)) - depth.put(object, new Tree<>()); - depth = (Tree) depth.get(object); + depth = depth.getOrCreateChild(object); } } this.traversalRing.reset(); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java index 60f4f0b1ef4..dd2f171cb41 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java @@ -61,9 +61,7 @@ protected void sideEffect(final Traverser.Admin traverser) { final Path path = traverser.path(); for (int i = 0; i < path.size(); i++) { final Object object = TraversalUtil.applyNullable(path.get(i), this.traversalRing.next()); - if (!depth.containsKey(object)) - depth.put(object, new Tree<>()); - depth = (Tree) depth.get(object); + depth = depth.getOrCreateChild(object); } this.traversalRing.reset(); this.getTraversal().getSideEffects().add(this.sideEffectKey, root); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java index 71ece422412..653ad291643 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java @@ -20,125 +20,249 @@ import java.io.Serializable; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; /** + * A tree data structure with a tree-shaped public API that is composed of a {@link HashMap}. + *

+ * Internal storage is a plain {@link HashMap}, so iteration order is not promised and sibling-key collisions still + * collapse (a known limitation, tracked as TINKERPOP-1693). + * * @author Marko A. Rodriguez (http://markorodriguez.com) */ -public class Tree extends HashMap> implements Serializable { +public final class Tree implements Serializable { + + private final Map> children = new HashMap<>(); public Tree() { - super(); } + /** + * Creates a tree with the given values as root keys, each mapping to an empty {@link Tree}. + */ @SafeVarargs - public Tree(final T... children) { - this(); - for (final T t : children) { - this.put(t, new Tree<>()); + public Tree(final T... rootValues) { + for (final T t : rootValues) { + this.children.put(t, new Tree<>()); } } - @SafeVarargs - public Tree(final Map.Entry>... children) { - this(); - for (final Map.Entry> entry : children) { - this.put(entry.getKey(), entry.getValue()); + // ------------------------------------------------------------------ + // navigation + // ------------------------------------------------------------------ + + /** + * Returns the set of keys at the root of this tree. + */ + public Set rootNodes() { + return this.children.keySet(); + } + + /** + * Returns the child subtree for the given key. + * + * @throws IllegalArgumentException if no child exists for the given key + */ + public Tree childAt(final T key) { + if (!this.children.containsKey(key)) { + throw new IllegalArgumentException("Tree has no child for key: " + key); } + return this.children.get(key); } + /** + * Returns {@code true} if the given key is an immediate child of this tree. + */ + public boolean hasChild(final T key) { + return this.children.containsKey(key); + } - public List> getTreesAtDepth(final int depth) { - List> currentDepth = Collections.singletonList(this); - for (int i = 0; i < depth; i++) { - if (i == depth - 1) { - return currentDepth; - } else { - final List> temp = new ArrayList>(); - for (final Tree t : currentDepth) { - temp.addAll(t.values()); - } - currentDepth = temp; + /** + * Returns {@code true} if the given value appears as a key anywhere in this tree (recursive). + */ + public boolean contains(final T value) { + if (this.children.containsKey(value)) { + return true; + } + for (final Tree sub : this.children.values()) { + if (sub.contains(value)) { + return true; } } - return Collections.emptyList(); + return false; + } + + /** + * Recursively searches the tree for the first subtree rooted at {@code key} and returns it. The search visits + * direct children first and then recurses, but iteration order across siblings is unspecified. + */ + public Optional> findSubtree(final T key) { + if (this.children.containsKey(key)) { + return Optional.of(this.children.get(key)); + } + for (final Tree sub : this.children.values()) { + final Optional> found = sub.findSubtree(key); + if (found.isPresent()) { + return found; + } + } + return Optional.empty(); + } + + /** + * Returns the existing child for {@code key}, or inserts and returns a new empty {@link Tree} if absent. + */ + public Tree getOrCreateChild(final T key) { + Tree child = this.children.get(key); + if (null == child) { + child = new Tree<>(); + this.children.put(key, child); + } + return child; + } + + // ------------------------------------------------------------------ + // structural + // ------------------------------------------------------------------ + + /** + * Returns {@code true} if this tree has no children. An empty tree is considered a leaf. + */ + public boolean isLeaf() { + return this.children.isEmpty(); + } + + /** + * Returns the total number of nodes (keys) in the tree, counted recursively. + */ + public int nodeCount() { + int count = this.children.size(); + for (final Tree sub : this.children.values()) { + count += sub.nodeCount(); + } + return count; } - public List getObjectsAtDepth(final int depth) { - final List list = new ArrayList(); + /** + * Returns the keys at the given depth. Depth {@code 0} returns the root keys. + */ + public List getNodesAtDepth(final int depth) { + final List list = new ArrayList<>(); for (final Tree t : this.getTreesAtDepth(depth)) { - list.addAll(t.keySet()); + list.addAll(t.children.keySet()); } return list; } - public List> getLeafTrees() { - final List> leaves = new ArrayList<>(); + /** + * Returns the trees at the given depth. Depth {@code 0} returns a singleton list containing this tree. + * Depths beyond the tree's height return an empty list. + */ + public List> getTreesAtDepth(final int depth) { + if (depth < 0) { + return Collections.emptyList(); + } List> currentDepth = Collections.singletonList(this); - boolean allLeaves = false; - while (!allLeaves) { - allLeaves = true; - final List> temp = new ArrayList<>(); + for (int i = 0; i < depth; i++) { + final List> next = new ArrayList<>(); for (final Tree t : currentDepth) { - if (t.isLeaf()) { - for (Map.Entry> t2 : t.entrySet()) { - leaves.add(new Tree(t2)); - } - } else { - allLeaves = false; - temp.addAll(t.values()); - } + next.addAll(t.children.values()); } - currentDepth = temp; - + if (next.isEmpty()) { + return Collections.emptyList(); + } + currentDepth = next; } + return currentDepth; + } + + /** + * Returns all keys whose subtrees are leaves. + */ + public List getLeafNodes() { + final List leaves = new ArrayList<>(); + collectLeafKeys(leaves); return leaves; } - public List getLeafObjects() { - final List leaves = new ArrayList(); - for (final Tree t : this.getLeafTrees()) { - leaves.addAll(t.keySet()); + private void collectLeafKeys(final List out) { + for (final Map.Entry> entry : this.children.entrySet()) { + if (entry.getValue().isLeaf()) { + out.add(entry.getKey()); + } else { + entry.getValue().collectLeafKeys(out); + } } - return leaves; } - public boolean isLeaf() { - final Collection> values = this.values(); - return values.iterator().next().isEmpty(); + /** + * Returns single-key trees representing each leaf key in this tree, preserving the original + * key-to-empty-subtree mapping. + */ + public List> getLeafTrees() { + final List> leaves = new ArrayList<>(); + collectLeafTrees(leaves); + return leaves; + } + private void collectLeafTrees(final List> out) { + for (final Map.Entry> entry : this.children.entrySet()) { + if (entry.getValue().isLeaf()) { + final Tree leaf = new Tree<>(); + leaf.children.put(entry.getKey(), entry.getValue()); + out.add(leaf); + } else { + entry.getValue().collectLeafTrees(out); + } + } } - public void addTree(final Tree tree) { - tree.forEach((k, t) -> { - if (this.containsKey(k)) { - this.get(k).addTree(t); + // ------------------------------------------------------------------ + // composition + // ------------------------------------------------------------------ + + /** + * Recursively merges {@code other} into this tree. For overlapping keys, child subtrees are merged in turn. + * For keys present only in {@code other}, the corresponding subtree reference is adopted directly. + */ + public void addTree(final Tree other) { + other.children.forEach((k, t) -> { + if (this.children.containsKey(k)) { + this.children.get(k).addTree(t); } else { - this.put(k, t); + this.children.put(k, t); } }); } + /** + * Splits this tree into one tree per root key. If the tree has a single root, returns a singleton list + * containing this tree. + */ public List> splitParents() { - if (this.keySet().size() == 1) { + if (this.children.size() == 1) { return Collections.singletonList(this); - } else { - final List> parents = new ArrayList<>(); - this.forEach((k, t) -> { - final Tree parentTree = new Tree<>(); - parentTree.put(k, t); - parents.add(parentTree); - }); - return parents; } + final List> parents = new ArrayList<>(); + this.children.forEach((k, t) -> { + final Tree parentTree = new Tree<>(); + parentTree.children.put(k, t); + parents.add(parentTree); + }); + return parents; } + // ------------------------------------------------------------------ + // output + // ------------------------------------------------------------------ + /** - * Produce a formatted string representation of the tree structure. + * Produces a formatted string representation of the tree structure using a {@code |--} ASCII style. */ public String prettyPrint() { final StringBuilder builder = new StringBuilder(); @@ -148,10 +272,36 @@ public String prettyPrint() { } private void prettyPrint(final StringBuilder builder, final String prefix) { - for (Map.Entry> entry : this.entrySet()) { + for (final Map.Entry> entry : this.children.entrySet()) { builder.append(prefix).append("|--").append(entry.getKey()); builder.append(System.lineSeparator()); entry.getValue().prettyPrint(builder, prefix + " "); } } + + // ------------------------------------------------------------------ + // identity + // ------------------------------------------------------------------ + + /** + * Structural recursive equality. Order across siblings is irrelevant; two trees are equal if they have the + * same set of keys and each key's subtree is recursively equal. + */ + @Override + public boolean equals(final Object other) { + if (this == other) return true; + if (!(other instanceof Tree)) return false; + final Tree that = (Tree) other; + return this.children.equals(that.children); + } + + @Override + public int hashCode() { + return this.children.hashCode(); + } + + @Override + public String toString() { + return this.children.toString(); + } } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/TreeSerializer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/TreeSerializer.java index 5aab638812d..dcce24a7ea4 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/TreeSerializer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/TreeSerializer.java @@ -37,7 +37,9 @@ protected Tree readValue(final Buffer buffer, final GraphBinaryReader context) t final Tree result = new Tree(); for (int i = 0; i < length; i++) { - result.put(context.read(buffer), context.readValue(buffer, Tree.class, false)); + final Object key = context.read(buffer); + final Tree value = context.readValue(buffer, Tree.class, false); + result.getOrCreateChild(key).addTree(value); } return result; @@ -45,11 +47,12 @@ protected Tree readValue(final Buffer buffer, final GraphBinaryReader context) t @Override protected void writeValue(final Tree value, final Buffer buffer, final GraphBinaryWriter context) throws IOException { - buffer.writeInt(value.size()); + final java.util.Set rootNodes = value.rootNodes(); + buffer.writeInt(rootNodes.size()); - for (Object key : value.keySet()) { + for (final Object key : rootNodes) { context.write(key, buffer); - context.writeValue(value.get(key), buffer, false); + context.writeValue(value.childAt(key), buffer, false); } } } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV1.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV1.java index 4841b482b4e..db4d4a8d604 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV1.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV1.java @@ -50,7 +50,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.TimeUnit; /** @@ -284,14 +283,13 @@ private static void ser(final Tree tree, final JsonGenerator jsonGenerator, fina throws IOException { jsonGenerator.writeStartObject(); if (typeSerializer != null) jsonGenerator.writeStringField(GraphSONTokens.CLASS, HashMap.class.getName()); - - Set> set = tree.entrySet(); - for(Map.Entry entry : set) - { - jsonGenerator.writeObjectFieldStart(entry.getKey().id().toString()); + + for (final Object key : tree.rootNodes()) { + final Element elementKey = (Element) key; + jsonGenerator.writeObjectFieldStart(elementKey.id().toString()); if (typeSerializer != null) jsonGenerator.writeStringField(GraphSONTokens.CLASS, HashMap.class.getName()); - jsonGenerator.writeObjectField(GraphSONTokens.KEY, entry.getKey()); - jsonGenerator.writeObjectField(GraphSONTokens.VALUE, entry.getValue()); + jsonGenerator.writeObjectField(GraphSONTokens.KEY, elementKey); + jsonGenerator.writeObjectField(GraphSONTokens.VALUE, tree.childAt(key)); jsonGenerator.writeEndObject(); } jsonGenerator.writeEndObject(); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java index 8b52323c945..c3113ebfe41 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java @@ -68,7 +68,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.TimeUnit; import static org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONUtil.safeWriteObjectField; @@ -282,11 +281,10 @@ public TreeJacksonSerializer() { @Override public void serialize(final Tree tree, final JsonGenerator jsonGenerator, final SerializerProvider serializerProvider) throws IOException, JsonGenerationException { jsonGenerator.writeStartArray(); - final Set> set = tree.entrySet(); - for (Map.Entry entry : set) { + for (final Object key : tree.rootNodes()) { jsonGenerator.writeStartObject(); - jsonGenerator.writeObjectField(GraphSONTokens.KEY, entry.getKey()); - jsonGenerator.writeObjectField(GraphSONTokens.VALUE, entry.getValue()); + jsonGenerator.writeObjectField(GraphSONTokens.KEY, key); + jsonGenerator.writeObjectField(GraphSONTokens.VALUE, tree.childAt(key)); jsonGenerator.writeEndObject(); } jsonGenerator.writeEndArray(); @@ -692,7 +690,12 @@ public Tree deserialize(final JsonParser jsonParser, final DeserializationContex final List data = deserializationContext.readValue(jsonParser, List.class); final Tree t = new Tree(); for (Map entry : data) { - t.put(entry.get(GraphSONTokens.KEY), entry.get(GraphSONTokens.VALUE)); + final Object key = entry.get(GraphSONTokens.KEY); + final Object value = entry.get(GraphSONTokens.VALUE); + final Tree child = t.getOrCreateChild(key); + if (value instanceof Tree) { + child.addTree((Tree) value); + } } return t; } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java index 6df9141f5b2..b6761dc5291 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java @@ -303,11 +303,10 @@ public TreeJacksonSerializer() { @Override public void serialize(final Tree tree, final JsonGenerator jsonGenerator, final SerializerProvider serializerProvider) throws IOException, JsonGenerationException { jsonGenerator.writeStartArray(); - final Set> set = tree.entrySet(); - for (Map.Entry entry : set) { + for (final Object key : tree.rootNodes()) { jsonGenerator.writeStartObject(); - jsonGenerator.writeObjectField(GraphSONTokens.KEY, entry.getKey()); - jsonGenerator.writeObjectField(GraphSONTokens.VALUE, entry.getValue()); + jsonGenerator.writeObjectField(GraphSONTokens.KEY, key); + jsonGenerator.writeObjectField(GraphSONTokens.VALUE, tree.childAt(key)); jsonGenerator.writeEndObject(); } jsonGenerator.writeEndArray(); @@ -730,7 +729,12 @@ public Tree deserialize(final JsonParser jsonParser, final DeserializationContex final List data = deserializationContext.readValue(jsonParser, List.class); final Tree t = new Tree(); for (Map entry : data) { - t.put(entry.get(GraphSONTokens.KEY), entry.get(GraphSONTokens.VALUE)); + final Object key = entry.get(GraphSONTokens.KEY); + final Object value = entry.get(GraphSONTokens.VALUE); + final Tree child = t.getOrCreateChild(key); + if (value instanceof Tree) { + child.addTree((Tree) value); + } } return t; } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV4.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV4.java index 66361a0f05d..d68458b6717 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV4.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV4.java @@ -306,11 +306,10 @@ public TreeJacksonSerializer() { @Override public void serialize(final Tree tree, final JsonGenerator jsonGenerator, final SerializerProvider serializerProvider) throws IOException, JsonGenerationException { jsonGenerator.writeStartArray(); - final Set> set = tree.entrySet(); - for (Map.Entry entry : set) { + for (final Object key : tree.rootNodes()) { jsonGenerator.writeStartObject(); - jsonGenerator.writeObjectField(GraphSONTokens.KEY, entry.getKey()); - jsonGenerator.writeObjectField(GraphSONTokens.VALUE, entry.getValue()); + jsonGenerator.writeObjectField(GraphSONTokens.KEY, key); + jsonGenerator.writeObjectField(GraphSONTokens.VALUE, tree.childAt(key)); jsonGenerator.writeEndObject(); } jsonGenerator.writeEndArray(); @@ -581,7 +580,12 @@ public Tree deserialize(final JsonParser jsonParser, final DeserializationContex final List data = deserializationContext.readValue(jsonParser, List.class); final Tree t = new Tree(); for (Map entry : data) { - t.put(entry.get(GraphSONTokens.KEY), entry.get(GraphSONTokens.VALUE)); + final Object key = entry.get(GraphSONTokens.KEY); + final Object value = entry.get(GraphSONTokens.VALUE); + final Tree child = t.getOrCreateChild(key); + if (value instanceof Tree) { + child.addTree((Tree) value); + } } return t; } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/detached/DetachedFactory.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/detached/DetachedFactory.java index 0c9b17db982..8fbddf4550d 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/detached/DetachedFactory.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/detached/DetachedFactory.java @@ -106,11 +106,19 @@ public static D detach(final Object object, final boolean withProperties) { set.add(DetachedFactory.detach(item, withProperties)); } return (D) set; + } else if (object instanceof Tree) { + final Tree newTree = new Tree(); + final Tree sourceTree = (Tree) object; + for (final Object key : sourceTree.rootNodes()) { + final Object detachedKey = DetachedFactory.detach(key, withProperties); + final Tree detachedSubtree = (Tree) DetachedFactory.detach(sourceTree.childAt(key), withProperties); + newTree.getOrCreateChild(detachedKey).addTree(detachedSubtree); + } + return (D) newTree; } else if (object instanceof Map) { - final Map map = object instanceof Tree ? new Tree() : - object instanceof LinkedHashMap ? - new LinkedHashMap(((Map) object).size()) : - new HashMap(((Map) object).size()); + final Map map = object instanceof LinkedHashMap ? + new LinkedHashMap(((Map) object).size()) : + new HashMap(((Map) object).size()); for (final Map.Entry entry : ((Map) object).entrySet()) { map.put(DetachedFactory.detach(entry.getKey(), withProperties), DetachedFactory.detach(entry.getValue(), withProperties)); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/reference/ReferenceFactory.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/reference/ReferenceFactory.java index fa595311a06..63e746f8fe5 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/reference/ReferenceFactory.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/reference/ReferenceFactory.java @@ -102,11 +102,19 @@ public static D detach(final Object object) { set.add(ReferenceFactory.detach(item)); } return (D) set; + } else if (object instanceof Tree) { + final Tree newTree = new Tree(); + final Tree sourceTree = (Tree) object; + for (final Object key : sourceTree.rootNodes()) { + final Object detachedKey = ReferenceFactory.detach(key); + final Tree detachedSubtree = (Tree) ReferenceFactory.detach(sourceTree.childAt(key)); + newTree.getOrCreateChild(detachedKey).addTree(detachedSubtree); + } + return (D) newTree; } else if (object instanceof Map) { - final Map map = object instanceof Tree ? new Tree() : - object instanceof LinkedHashMap ? - new LinkedHashMap(((Map) object).size()) : - new HashMap(((Map) object).size()); + final Map map = object instanceof LinkedHashMap ? + new LinkedHashMap(((Map) object).size()) : + new HashMap(((Map) object).size()); for (final Map.Entry entry : ((Map) object).entrySet()) { map.put(ReferenceFactory.detach(entry.getKey()), ReferenceFactory.detach(entry.getValue())); } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/TreeTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/TreeTest.java index 5bf6e43533b..4e2710ecbbd 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/TreeTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/TreeTest.java @@ -24,15 +24,17 @@ import org.hamcrest.Matchers; import org.junit.Test; -import java.util.AbstractMap; import java.util.Arrays; import java.util.List; -import java.util.Map; +import java.util.Optional; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; /** * @author Marko A. Rodriguez (http://markorodriguez.com) @@ -40,69 +42,215 @@ */ public class TreeTest extends StepTest { + /** + * Helper that builds a subtree shaped like {@code key -> subtree}. + */ + private static Tree branch(final T key, final Tree subtree) { + final Tree wrapper = new Tree<>(); + wrapper.getOrCreateChild(key).addTree(subtree); + return wrapper; + } + @Test public void shouldProvideValidDepths() { - Tree tree = new Tree(); - tree.put("marko", new Tree(TreeTest.createTree("a", new Tree("a1", "a2")), TreeTest.createTree("b", new Tree("b1", "b2", "b3")))); - tree.put("josh", new Tree("1", "2")); + final Tree tree = new Tree<>(); + final Tree markoSubtree = new Tree<>(); + markoSubtree.addTree(branch("a", new Tree<>("a1", "a2"))); + markoSubtree.addTree(branch("b", new Tree<>("b1", "b2", "b3"))); + tree.getOrCreateChild("marko").addTree(markoSubtree); + tree.getOrCreateChild("josh").addTree(new Tree<>("1", "2")); + + assertEquals(2, tree.getNodesAtDepth(0).size()); + assertTrue(tree.getNodesAtDepth(0).containsAll(Arrays.asList("marko", "josh"))); + assertEquals(4, tree.getNodesAtDepth(1).size()); + assertEquals(5, tree.getNodesAtDepth(2).size()); + assertEquals(0, tree.getNodesAtDepth(3).size()); + assertEquals(0, tree.getNodesAtDepth(4).size()); + assertEquals(0, tree.getNodesAtDepth(5).size()); - assertEquals(0, tree.getObjectsAtDepth(0).size()); - assertEquals(2, tree.getObjectsAtDepth(1).size()); - assertEquals(4, tree.getObjectsAtDepth(2).size()); - assertEquals(5, tree.getObjectsAtDepth(3).size()); - assertEquals(0, tree.getObjectsAtDepth(4).size()); - assertEquals(0, tree.getObjectsAtDepth(5).size()); + assertEquals(1, tree.getTreesAtDepth(0).size()); + assertEquals(tree, tree.getTreesAtDepth(0).get(0)); - assertEquals(2, tree.get("josh").size()); - assertEquals(0, tree.get("marko").get("b").get("b1").size()); - assertEquals(3, tree.get("marko").get("b").size()); - assertNull(tree.get("marko").get("c")); + assertEquals(2, tree.childAt("josh").rootNodes().size()); + assertTrue(tree.childAt("marko").childAt("b").childAt("b1").isLeaf()); + assertEquals(3, tree.childAt("marko").childAt("b").rootNodes().size()); + assertFalse(tree.childAt("marko").hasChild("c")); } @Test public void shouldProvideValidLeaves() { - Tree tree = new Tree(); - tree.put("marko", new Tree(TreeTest.createTree("a", new Tree("a1", "a2")), TreeTest.createTree("b", new Tree("b1", "b2", "b3")))); - tree.put("josh", new Tree("1", "2")); + final Tree tree = new Tree<>(); + final Tree markoSubtree = new Tree<>(); + markoSubtree.addTree(branch("a", new Tree<>("a1", "a2"))); + markoSubtree.addTree(branch("b", new Tree<>("b1", "b2", "b3"))); + tree.getOrCreateChild("marko").addTree(markoSubtree); + tree.getOrCreateChild("josh").addTree(new Tree<>("1", "2")); assertEquals(7, tree.getLeafTrees().size()); - for (Tree t : tree.getLeafTrees()) { - assertEquals(1, t.keySet().size()); - final String key = t.keySet().iterator().next(); + for (final Tree t : tree.getLeafTrees()) { + assertEquals(1, t.rootNodes().size()); + final String key = t.rootNodes().iterator().next(); assertTrue(Arrays.asList("a1", "a2", "b1", "b2", "b3", "1", "2").contains(key)); } - assertEquals(7, tree.getLeafObjects().size()); - for (String s : tree.getLeafObjects()) { + assertEquals(7, tree.getLeafNodes().size()); + for (final String s : tree.getLeafNodes()) { assertTrue(Arrays.asList("a1", "a2", "b1", "b2", "b3", "1", "2").contains(s)); } } @Test public void shouldMergeTreesCorrectly() { - Tree tree1 = new Tree<>(); - tree1.put("1", new Tree(TreeTest.createTree("1_1", new Tree("1_1_1")), TreeTest.createTree("1_2", new Tree("1_2_1")))); - Tree tree2 = new Tree<>(); - tree2.put("1", new Tree(TreeTest.createTree("1_1", new Tree("1_1_1")), TreeTest.createTree("1_2", new Tree("1_2_2")))); + final Tree tree1 = new Tree<>(); + final Tree tree1OneSubtree = new Tree<>(); + tree1OneSubtree.addTree(branch("1_1", new Tree<>("1_1_1"))); + tree1OneSubtree.addTree(branch("1_2", new Tree<>("1_2_1"))); + tree1.getOrCreateChild("1").addTree(tree1OneSubtree); + + final Tree tree2 = new Tree<>(); + final Tree tree2OneSubtree = new Tree<>(); + tree2OneSubtree.addTree(branch("1_1", new Tree<>("1_1_1"))); + tree2OneSubtree.addTree(branch("1_2", new Tree<>("1_2_2"))); + tree2.getOrCreateChild("1").addTree(tree2OneSubtree); - Tree mergeTree = new Tree<>(); + final Tree mergeTree = new Tree<>(); mergeTree.addTree(tree1); mergeTree.addTree(tree2); - assertEquals(1, mergeTree.size()); - assertEquals(0, mergeTree.getObjectsAtDepth(0).size()); - assertEquals(1, mergeTree.getObjectsAtDepth(1).size()); - assertEquals(2, mergeTree.getObjectsAtDepth(2).size()); - assertEquals(3, mergeTree.getObjectsAtDepth(3).size()); - assertTrue(mergeTree.getObjectsAtDepth(3).contains("1_1_1")); - assertTrue(mergeTree.getObjectsAtDepth(3).contains("1_2_1")); - assertTrue(mergeTree.getObjectsAtDepth(3).contains("1_2_2")); + assertEquals(1, mergeTree.rootNodes().size()); + assertEquals(1, mergeTree.getNodesAtDepth(0).size()); + assertEquals(2, mergeTree.getNodesAtDepth(1).size()); + assertEquals(3, mergeTree.getNodesAtDepth(2).size()); + assertEquals(0, mergeTree.getNodesAtDepth(3).size()); + assertTrue(mergeTree.getNodesAtDepth(2).contains("1_1_1")); + assertTrue(mergeTree.getNodesAtDepth(2).contains("1_2_1")); + assertTrue(mergeTree.getNodesAtDepth(2).contains("1_2_2")); + } + + @Test + public void shouldGetOrCreateChild() { + final Tree tree = new Tree<>(); + final Tree child = tree.getOrCreateChild("a"); + assertNotNull(child); + assertTrue(child.isLeaf()); + // calling again returns the same instance + assertSame(child, tree.getOrCreateChild("a")); + + // mutating the returned subtree is observable through the parent + child.getOrCreateChild("a1"); + assertTrue(tree.childAt("a").hasChild("a1")); + } + + @Test + public void shouldAllowNullKeys() { + // HashMap allows null keys; the new Tree must preserve this (TreeSideEffectStep relies on it). + final Tree tree = new Tree<>(); + final Tree nullChild = tree.getOrCreateChild(null); + assertNotNull(nullChild); + assertTrue(tree.hasChild(null)); + assertEquals(1, tree.rootNodes().size()); + } + + @Test + public void shouldThrowOnMissingChildAt() { + final Tree tree = new Tree<>(); + tree.getOrCreateChild("a"); + try { + tree.childAt("missing"); + fail("expected IllegalArgumentException for missing key"); + } catch (final IllegalArgumentException iae) { + assertThat(iae.getMessage(), Matchers.containsString("missing")); + } + } + + @Test + public void shouldDetectImmediateAndRecursiveContainment() { + final Tree tree = new Tree<>(); + tree.getOrCreateChild("root").getOrCreateChild("child").getOrCreateChild("grandchild"); + + assertTrue(tree.hasChild("root")); + assertFalse(tree.hasChild("child")); + assertFalse(tree.hasChild("grandchild")); + + assertTrue(tree.contains("root")); + assertTrue(tree.contains("child")); + assertTrue(tree.contains("grandchild")); + assertFalse(tree.contains("nope")); + } + + @Test + public void shouldFindSubtreeRecursively() { + final Tree tree = new Tree<>(); + tree.getOrCreateChild("root").getOrCreateChild("child").getOrCreateChild("grandchild"); + + final Optional> found = tree.findSubtree("grandchild"); + assertTrue(found.isPresent()); + assertTrue(found.get().isLeaf()); + + assertFalse(tree.findSubtree("missing").isPresent()); + } + + @Test + public void shouldReportNodeCount() { + final Tree tree = new Tree<>(); + assertEquals(0, tree.nodeCount()); + tree.getOrCreateChild("a"); + assertEquals(1, tree.nodeCount()); + tree.getOrCreateChild("b").getOrCreateChild("b1"); + assertEquals(3, tree.nodeCount()); + } + + @Test + public void shouldReportLeafForEmptyTree() { + assertTrue(new Tree().isLeaf()); + + // a node with children is not a leaf + final Tree tree = new Tree<>(); + tree.getOrCreateChild("a"); + assertFalse(tree.isLeaf()); + } + + @Test + public void shouldBeStructurallyEqualOrderInsensitive() { + final Tree a = new Tree<>(); + a.getOrCreateChild("x").getOrCreateChild("x1"); + a.getOrCreateChild("y"); + + final Tree b = new Tree<>(); + b.getOrCreateChild("y"); + b.getOrCreateChild("x").getOrCreateChild("x1"); + + assertEquals(a, b); + assertEquals(a.hashCode(), b.hashCode()); + + final Tree c = new Tree<>(); + c.getOrCreateChild("x"); + c.getOrCreateChild("y"); + assertFalse(a.equals(c)); + } + + @Test + public void shouldSplitParents() { + final Tree single = new Tree<>(); + single.getOrCreateChild("only").getOrCreateChild("child"); + final List> singleSplit = single.splitParents(); + assertEquals(1, singleSplit.size()); + assertEquals(single, singleSplit.get(0)); + + final Tree multi = new Tree<>(); + multi.getOrCreateChild("a").getOrCreateChild("a1"); + multi.getOrCreateChild("b").getOrCreateChild("b1"); + final List> multiSplit = multi.splitParents(); + assertEquals(2, multiSplit.size()); + for (final Tree parent : multiSplit) { + assertEquals(1, parent.rootNodes().size()); + } } @Test public void testPrettyPrintSingleNode() { final Tree tree = new Tree<>(); - tree.put("root", new Tree<>()); + tree.getOrCreateChild("root"); final String expected = "|--root"; assertEquals(expected, tree.prettyPrint()); @@ -111,11 +259,9 @@ public void testPrettyPrintSingleNode() { @Test public void testPrettyPrintMultipleNodes() { final Tree tree = new Tree<>(); - final Tree child1 = new Tree<>(); - final Tree child2 = new Tree<>(); - tree.put("root", new Tree<>()); - tree.get("root").put("child1", child1); - tree.get("root").put("child2", child2); + final Tree root = tree.getOrCreateChild("root"); + root.getOrCreateChild("child1"); + root.getOrCreateChild("child2"); // either can be expected since Tree doesn't maintain node order final String expected1 = "|--root" + System.lineSeparator() + @@ -130,11 +276,7 @@ public void testPrettyPrintMultipleNodes() { @Test public void testPrettyPrintNestedTree() { final Tree tree = new Tree<>(); - final Tree child1 = new Tree<>(); - final Tree grandchild = new Tree<>(); - tree.put("root", new Tree<>()); - tree.get("root").put("child1", child1); - tree.get("root").get("child1").put("grandchild", grandchild); + tree.getOrCreateChild("root").getOrCreateChild("child1").getOrCreateChild("grandchild"); final String expected = "|--root" + System.lineSeparator() + " |--child1" + System.lineSeparator() + @@ -149,8 +291,17 @@ public void testPrettyPrintEmptyTree() { assertEquals(expected, tree.prettyPrint()); } - private static Map.Entry> createTree(T key, Tree tree) { - return new AbstractMap.SimpleEntry<>(key, tree); + @Test + public void shouldRenderToStringAsUnderlyingMap() { + assertEquals("{}", new Tree().toString()); + + final Tree single = new Tree<>(); + single.getOrCreateChild("root"); + assertEquals("{root={}}", single.toString()); + + final Tree nested = new Tree<>(); + nested.getOrCreateChild("root").getOrCreateChild("child"); + assertEquals("{root={child={}}}", nested.toString()); } @Override diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/function/TreeSupplierTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/function/TreeSupplierTest.java index 880b5b7d73f..a13c0bf97bb 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/function/TreeSupplierTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/function/TreeSupplierTest.java @@ -34,12 +34,12 @@ public class TreeSupplierTest { @Test public void shouldSupplyTree() { - assertEquals(0, TreeSupplier.instance().get().size()); + assertEquals(0, TreeSupplier.instance().get().nodeCount()); } @Test public void shouldSupplyTreeInstance() { - assertEquals(0, TreeSupplier.instance().get().size()); + assertEquals(0, TreeSupplier.instance().get().nodeCount()); assertThat(TreeSupplier.instance().get(), instanceOf(Tree.class)); } From 1b2eaabc554d1609477210a3b3224672994cc3ee Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Thu, 4 Jun 2026 16:45:26 -0700 Subject: [PATCH 2/6] Changed format for Gryo and fixed tests --- .../traversal/step/map/CountLocalStep.java | 2 ++ .../structure/io/gryo/GryoSerializersV1.java | 25 +++++++++++++++++++ .../structure/io/gryo/GryoSerializersV3.java | 25 +++++++++++++++++++ .../structure/io/gryo/GryoVersion.java | 4 +-- .../gremlin/features/StepDefinition.java | 14 +++++------ .../util/ser/AbstractRoundTripTest.java | 8 +++--- .../binary/TypeSerializerFailureTests.java | 2 +- 7 files changed, 66 insertions(+), 14 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java index 8fb2b5c0f8e..4a2616628ad 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java @@ -21,6 +21,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Path; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree; import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; @@ -44,6 +45,7 @@ protected Long map(final Traverser.Admin traverser) { final S item = traverser.get(); return (item instanceof Collection) ? ((Collection) item).size() : (item instanceof Map) ? ((Map) item).size() + : (item instanceof Tree) ? ((Tree) item).rootNodes().size() : (item instanceof Path) ? ((Path) item).size() : IteratorUtils.count(IteratorUtils.asIterator(item)); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV1.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV1.java index 6e731250549..27cd177efd7 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV1.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV1.java @@ -23,6 +23,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Path; import org.apache.tinkerpop.gremlin.process.traversal.Text; import org.apache.tinkerpop.gremlin.process.traversal.TextP; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree; import org.apache.tinkerpop.gremlin.process.traversal.util.AndP; import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP; import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; @@ -264,4 +265,28 @@ public DefaultRemoteTraverser read(final KryoShim kr return new DefaultRemoteTraverser<>(o, input.readLong()); } } + + public final static class TreeSerializer implements SerializerShim { + @Override + public void write(final KryoShim kryo, final O output, final Tree tree) { + final java.util.Set keys = tree.rootNodes(); + output.writeInt(keys.size()); + for (final Object key : keys) { + kryo.writeClassAndObject(output, key); + this.write(kryo, output, (Tree) tree.childAt(key)); + } + } + + @Override + public Tree read(final KryoShim kryo, final I input, final Class clazz) { + final Tree tree = new Tree(); + final int size = input.readInt(); + for (int i = 0; i < size; i++) { + final Object key = kryo.readClassAndObject(input); + final Tree child = this.read(kryo, input, clazz); + tree.getOrCreateChild(key).addTree(child); + } + return tree; + } + } } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3.java index 5a0beec2aec..0b748813f51 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3.java @@ -23,6 +23,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Path; import org.apache.tinkerpop.gremlin.process.traversal.Text; import org.apache.tinkerpop.gremlin.process.traversal.TextP; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree; import org.apache.tinkerpop.gremlin.process.traversal.util.AndP; import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP; import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalMetrics; @@ -447,6 +448,30 @@ public Metrics read(final KryoShim kryo, final I inp } } + public final static class TreeSerializer implements SerializerShim { + @Override + public void write(final KryoShim kryo, final O output, final Tree tree) { + final java.util.Set keys = tree.rootNodes(); + output.writeInt(keys.size()); + for (final Object key : keys) { + kryo.writeClassAndObject(output, key); + this.write(kryo, output, (Tree) tree.childAt(key)); + } + } + + @Override + public Tree read(final KryoShim kryo, final I input, final Class clazz) { + final Tree tree = new Tree(); + final int size = input.readInt(); + for (int i = 0; i < size; i++) { + final Object key = kryo.readClassAndObject(input); + final Tree child = this.read(kryo, input, clazz); + tree.getOrCreateChild(key).addTree(child); + } + return tree; + } + } + private static void writeElementProperties(final KryoShim kryo, final OutputShim output, final Element element) { final Iterator properties = element.properties(); output.writeBoolean(properties.hasNext()); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java index 86f4edd1f0d..dd05a5da70d 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java @@ -381,7 +381,7 @@ public static List> initV3Registrations() { add(GryoTypeReg.of(ReservedKeysVerificationStrategy.class, 190)); add(GryoTypeReg.of(TraverserSet.class, 58)); - add(GryoTypeReg.of(Tree.class, 61)); + add(GryoTypeReg.of(Tree.class, 61, new GryoSerializersV3.TreeSerializer())); add(GryoTypeReg.of(HashSet.class, 62)); add(GryoTypeReg.of(BulkSet.class, 64)); add(GryoTypeReg.of(Metrics.class, 69, new GryoSerializersV3.MetricsSerializer())); @@ -560,7 +560,7 @@ public static List> initV1Registrations() { add(GryoTypeReg.of(TraverserSet.class, 58)); - add(GryoTypeReg.of(Tree.class, 61)); + add(GryoTypeReg.of(Tree.class, 61, new GryoSerializersV1.TreeSerializer())); add(GryoTypeReg.of(HashSet.class, 62)); add(GryoTypeReg.of(BulkSet.class, 64)); add(GryoTypeReg.of(MutableMetrics.class, 69)); diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java index 45f29c17b6d..59bc869f63d 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java @@ -545,20 +545,20 @@ public void theGraphShouldBeATreeWithAStructureOf(final String asciiTree) { final List roots = parseTree(asciiTree); // Validate that the tree matches the data in roots - assertEquals(roots.size(), tree.keySet().size()); + assertEquals(roots.size(), tree.rootNodes().size()); for (TreeNode root : roots) { assertThat(String.format("Tree not matching at %s", root.getValue()), - tree.containsKey(root.getValue()), CoreMatchers.is(true)); - validateTreeStructure((Tree) tree.get(root.getValue()), root); + tree.hasChild(root.getValue()), CoreMatchers.is(true)); + validateTreeStructure(tree.childAt(root.getValue()), root); } } - private void validateTreeStructure(final Tree actualTree, final TreeNode expectedNode) { - assertEquals(expectedNode.getChildren().size(), actualTree.keySet().size()); + private void validateTreeStructure(final Tree actualTree, final TreeNode expectedNode) { + assertEquals(expectedNode.getChildren().size(), actualTree.rootNodes().size()); for (TreeNode child : expectedNode.getChildren()) { assertThat(String.format("Tree not matching at %s", child.getValue()), - actualTree.containsKey(child.getValue()), CoreMatchers.is(true)); - validateTreeStructure(actualTree.get(child.getValue()), child); + actualTree.hasChild(child.getValue()), CoreMatchers.is(true)); + validateTreeStructure(actualTree.childAt(child.getValue()), child); } } diff --git a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java index 36b40b88b47..cfc23e99ebc 100644 --- a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java +++ b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/AbstractRoundTripTest.java @@ -98,10 +98,10 @@ public static Collection input() throws Exception { final Tree tree = new Tree<>(); final Tree subTree = new Tree<>(); final Tree subSubTree = new Tree<>(); - subSubTree.put(new ReferenceVertex(1, "animal"), new Tree<>()); - subSubTree.put(new ReferenceVertex(2, "animal"), new Tree<>()); - subTree.put(new ReferenceVertex(100, "animal"), subSubTree); - tree.put(new ReferenceVertex(1000, "animal"), subTree); + subSubTree.getOrCreateChild(new ReferenceVertex(1, "animal")); + subSubTree.getOrCreateChild(new ReferenceVertex(2, "animal")); + subTree.getOrCreateChild(new ReferenceVertex(100, "animal")).addTree(subSubTree); + tree.getOrCreateChild(new ReferenceVertex(1000, "animal")).addTree(subTree); final MutableMetrics metrics = new MutableMetrics("id1", "name1"); metrics.setDuration(123, TimeUnit.MICROSECONDS); diff --git a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/TypeSerializerFailureTests.java b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/TypeSerializerFailureTests.java index e8573a1763a..0ff8db09cfa 100644 --- a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/TypeSerializerFailureTests.java +++ b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/ser/binary/TypeSerializerFailureTests.java @@ -65,7 +65,7 @@ public static Collection input() { final MutableMetrics metrics = new MutableMetrics("a metric", null); final Tree tree = new Tree<>(); - tree.put(vertex, null); + tree.getOrCreateChild(vertex); // Provide instances that are malformed for serialization to fail return Arrays.asList( From 994ec2eacc0cd6b861463d22389ece9041900122 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 5 Jun 2026 09:43:15 -0700 Subject: [PATCH 3/6] Updated docs and removed count(local) support Assisted-by: Kiro: Claude Opus 4.8 --- docs/src/upgrade/release-4.x.x.asciidoc | 11 ++++-- .../traversal/step/map/CountLocalStep.java | 4 ++- .../process/traversal/step/util/Tree.java | 4 +-- .../step/map/CountLocalStepTest.java | 20 +++++++++++ .../test/cucumber/feature-steps.js | 2 -- .../test/cucumber/gremlin.js | 2 -- .../gremlin/driver/remote/RemoteWorld.java | 1 - .../language/translator/translations.json | 34 ------------------- .../test/features/sideEffect/Tree.feature | 33 ------------------ .../spark/SparkGraphFeatureIntegrateTest.java | 1 - .../gremlin/tinkergraph/TinkerWorld.java | 1 - 11 files changed, 33 insertions(+), 80 deletions(-) diff --git a/docs/src/upgrade/release-4.x.x.asciidoc b/docs/src/upgrade/release-4.x.x.asciidoc index ab298eb28a8..7be918d7d7d 100644 --- a/docs/src/upgrade/release-4.x.x.asciidoc +++ b/docs/src/upgrade/release-4.x.x.asciidoc @@ -656,7 +656,8 @@ Replacements for the common `Map`-based access patterns: A few Gremlin patterns that worked only because `Tree` was a `Map` (for example `select(keys)`, `count(local)`, and `unfold()` applied to a `Tree`) no longer compose. Process the `Tree` result client-side after `next()`, or -reshape the upstream traversal. +reshape the upstream traversal. The old `count(local)` (which returned the root-node count) is replaced by +`rootNodes().size()` for the root count, or `nodeCount()` for the total node count. `isLeaf()` no longer throws on an empty tree (it returns `true`), and a `Tree` keeps the long-standing limitation that sibling branches resolving to the same value collapse into one node; use `path()` or `subgraph()` when full @@ -685,8 +686,12 @@ registration. `Tree` no longer extends `HashMap`. Provider code that inspected or rebuilt a `Tree` via `Map` methods (`get`, `put`, `keySet`, `entrySet`, `size`) must use the tree-shaped API instead: read with `rootNodes()` + `childAt(key)` and build with `getOrCreateChild(key)` + `addTree(subtree)`. The GraphSON (`g:Tree`) and -GraphBinary (`0x2b`) wire formats are unchanged, so no serialized data or cross-version compatibility is affected -— only the in-memory Java type changed. +GraphBinary (`0x2b`) wire formats are unchanged, so no GraphSON- or GraphBinary-serialized data or cross-version +compatibility is affected. The Gryo `Tree` wire format, however, did change: in 3.x a `Tree` was serialized via +Kryo's default `Map` serializer because it extended `HashMap`, whereas in 4.x it has an explicit `Tree` +serializer. As a result, Gryo-serialized `Tree` data written by 3.x cannot be read by 4.x and vice versa. The +Gryo type registration id (`61`) is unchanged and 4.x-to-4.x Gryo round-trips correctly; this Gryo break is +expected for the 4.0.0 major release. ==== Graph Driver Providers diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java index 4a2616628ad..9c8886e2b7d 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java @@ -43,9 +43,11 @@ public CountLocalStep(final Traversal.Admin traversal) { @Override protected Long map(final Traverser.Admin traverser) { final S item = traverser.get(); + if (item instanceof Tree) + throw new IllegalArgumentException("count(local) is not supported on Tree; Call next() and use the " + + "Tree API: rootNodes().size() for the root-node count, or nodeCount() for the total node count."); return (item instanceof Collection) ? ((Collection) item).size() : (item instanceof Map) ? ((Map) item).size() - : (item instanceof Tree) ? ((Tree) item).rootNodes().size() : (item instanceof Path) ? ((Path) item).size() : IteratorUtils.count(IteratorUtils.asIterator(item)); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java index 653ad291643..99432a5e19f 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java @@ -57,10 +57,10 @@ public Tree(final T... rootValues) { // ------------------------------------------------------------------ /** - * Returns the set of keys at the root of this tree. + * Returns the set of keys at the root of this tree. The returned set is unmodifiable. */ public Set rootNodes() { - return this.children.keySet(); + return Collections.unmodifiableSet(this.children.keySet()); } /** diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java index ce9a99fcd58..77c06bf6e6f 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java @@ -20,12 +20,19 @@ import org.apache.tinkerpop.gremlin.process.traversal.Scope; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree; +import org.apache.tinkerpop.gremlin.process.traversal.traverser.B_O_Traverser; +import org.junit.Test; import java.util.Collections; import java.util.List; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + /** * @author Daniel Kuppitz (http://gremlin.guru) */ @@ -35,4 +42,17 @@ public class CountLocalStepTest extends StepTest { protected List getTraversals() { return Collections.singletonList(__.count(Scope.local)); } + + @Test + public void shouldThrowOnTree() { + final Traversal.Admin traversal = __.count(Scope.local).asAdmin(); + final CountLocalStep step = (CountLocalStep) traversal.getSteps().get(0); + + final Tree tree = new Tree<>(); + tree.getOrCreateChild("a").getOrCreateChild("b"); + final Traverser.Admin traverser = new B_O_Traverser<>(tree, 1L); + + final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> step.map(traverser)); + assertTrue(ex.getMessage().contains("not supported on Tree")); + } } diff --git a/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js b/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js index bb956a2538d..907412ac2c1 100644 --- a/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js +++ b/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js @@ -90,8 +90,6 @@ const ignoredScenarios = { 'g_VX1X_out_out_out_tree': new IgnoreError(ignoreReason.treeStepNotSupported), 'g_VX1X_outE_inV_bothE_otherV_tree': new IgnoreError(ignoreReason.treeStepNotSupported), 'g_VX1X_outE_inV_bothE_otherV_tree_byXnameX_byXlabelX': new IgnoreError(ignoreReason.treeStepNotSupported), - 'g_V_out_treeXaX_selectXaX_countXlocalX': new IgnoreError(ignoreReason.treeStepNotSupported), - 'g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX': new IgnoreError(ignoreReason.treeStepNotSupported), // floating point issues 'g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack': new IgnoreError(ignoreReason.floatingPointIssues), 'g_withSackX2X_V_sackXdivX_byXconstantX4_0XX_sack': new IgnoreError(ignoreReason.floatingPointIssues), diff --git a/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js b/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js index d4e6e97e758..93708fefce4 100644 --- a/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js +++ b/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js @@ -2201,8 +2201,6 @@ const gremlins = { g_VX1X_out_out_out_tree: [function({g}) { return g.V().out().out().out().tree() }], g_VX1X_outE_inV_bothE_otherV_tree: [function({g, vid1}) { return g.V(vid1).outE().inV().bothE().otherV().tree() }], g_VX1X_outE_inV_bothE_otherV_tree_byXnameX_byXlabelX: [function({g, vid1}) { return g.V(vid1).outE().inV().bothE().otherV().tree().by("name").by(T.label) }], - g_V_out_treeXaX_selectXaX_countXlocalX: [function({g}) { return g.V().out().tree("a").select("a").count(Scope.local) }], - g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX: [function({g}) { return g.V().out().local(__.tree("a").select("a").count(Scope.local)) }], g_io_writeXkryoX: [function({g}) { return g.io("tinkerpop-modern-v3.kryo").write() }], g_io_write_withXwriter_gryoX: [function({g}) { return g.io("tinkerpop-modern-v3.kryo").with_(IO.writer, IO.gryo).write() }], g_io_writeXgraphsonX: [function({g}) { return g.io("tinkerpop-modern-v3.json").write() }], diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java index 42bc9b5deea..ddc48092c2f 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java @@ -159,7 +159,6 @@ public void beforeEachScenario(Scenario scenario) { // TINKERPOP-3209 case "g_V_localXgroupCountXaX_selectXaX_countXlocalXX": case "g_V_localXgroupXaX_byXnameX_by_selectXaX_countXlocalXX": - case "g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX": throw new AssumptionViolatedException("Selecting side effects inside local() produces inconsistent results in OLAP (See TINKERPOP-3209)"); default: // Do nothing diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json index f152691348e..8fe7421aa7d 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json @@ -44794,40 +44794,6 @@ } ] }, - { - "scenario": "g_V_out_treeXaX_selectXaX_countXlocalX", - "traversals": [ - { - "original": "g.V().out().tree(\"a\").select(\"a\").count(local)", - "language": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", - "canonical": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", - "anonymized": "g.V().out().tree(string0).select(string0).count(Scope.local)", - "dotnet": "g.V().Out().Tree(\"a\").Select(\"a\").Count(Scope.Local)", - "go": "g.V().Out().Tree(\"a\").Select(\"a\").Count(gremlingo.Scope.Local)", - "groovy": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", - "java": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", - "javascript": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", - "python": "g.V().out().tree('a').select('a').count(Scope.local)" - } - ] - }, - { - "scenario": "g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX", - "traversals": [ - { - "original": "g.V().out().local(tree(\"a\").select(\"a\").count(local))", - "language": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", - "canonical": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", - "anonymized": "g.V().out().local(__.tree(string0).select(string0).count(Scope.local))", - "dotnet": "g.V().Out().Local(__.Tree(\"a\").Select(\"a\").Count(Scope.Local))", - "go": "g.V().Out().Local(gremlingo.T__.Tree(\"a\").Select(\"a\").Count(gremlingo.Scope.Local))", - "groovy": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", - "java": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", - "javascript": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", - "python": "g.V().out().local(__.tree('a').select('a').count(Scope.local))" - } - ] - }, { "scenario": "g_io_writeXkryoX", "traversals": [ diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature index efc44e02c0c..19fd429de85 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature @@ -206,36 +206,3 @@ Feature: Step - tree() |--josh |--peter """ - - Scenario: g_V_out_treeXaX_selectXaX_countXlocalX - Given the modern graph - And the traversal of - """ - g.V().out().tree("a").select("a").count(local) - """ - When iterated to list - Then the result should be unordered - | result | - | d[3].l | - | d[3].l | - | d[3].l | - | d[3].l | - | d[3].l | - | d[3].l | - - @InsertionOrderingRequired - Scenario: g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX - Given the modern graph - And the traversal of - """ - g.V().out().local(tree("a").select("a").count(local)) - """ - When iterated to list - Then the result should be unordered - | result | - | d[1].l | - | d[1].l | - | d[1].l | - | d[2].l | - | d[2].l | - | d[3].l | diff --git a/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java b/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java index 884b27ac8d8..99db7b60e7e 100644 --- a/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java +++ b/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java @@ -98,7 +98,6 @@ public class SparkGraphFeatureIntegrateTest { add(Pair.with("g_V_untilXloops_isX2XX_repeatXout_order_byXnameXX_valuesXnameX", skipReasonOrdering)); add(Pair.with("g_V_localXgroupCountXaX_selectXaX_countXlocalXX", skipReasonTINKERPOP3209)); add(Pair.with("g_V_localXgroupXaX_byXnameX_by_selectXaX_countXlocalXX", skipReasonTINKERPOP3209)); - add(Pair.with("g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX", skipReasonTINKERPOP3209)); }}; public static final class ServiceModule extends AbstractModule { diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java index 1a6ff95e020..29f1d4e0b65 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java @@ -344,7 +344,6 @@ public void beforeEachScenario(final Scenario scenario) { // TINKERPOP-3209 case "g_V_localXgroupCountXaX_selectXaX_countXlocalXX": case "g_V_localXgroupXaX_byXnameX_by_selectXaX_countXlocalXX": - case "g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX": throw new AssumptionViolatedException("Selecting side effects inside local() produces inconsistent results in OLAP (See TINKERPOP-3209)"); default: // Do nothing From 5ae6bf3df6eb0b84c0d0f115bebd6c30a81c7a48 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Thu, 11 Jun 2026 14:02:59 -0700 Subject: [PATCH 4/6] Address comments Assisted-by: Kiro: Claude Opus 4.8 --- CHANGELOG.asciidoc | 2 + docs/src/reference/the-traversal.asciidoc | 9 +-- docs/src/upgrade/release-4.x.x.asciidoc | 23 +++++-- .../traversal/step/map/CountLocalStep.java | 6 +- .../process/traversal/step/util/Tree.java | 7 +- .../step/map/CountLocalStepTest.java | 42 ++++++++---- .../gremlin/groovy/loaders/SugarLoader.groovy | 13 ++++ .../gremlin/groovy/TreeSubscriptTest.groovy | 68 +++++++++++++++++++ .../test/cucumber/gremlin.js | 2 + .../language/translator/translations.json | 34 ++++++++++ .../test/features/sideEffect/Tree.feature | 33 +++++++++ 11 files changed, 210 insertions(+), 29 deletions(-) create mode 100644 gremlin-groovy/src/test/groovy/org/apache/tinkerpop/gremlin/groovy/TreeSubscriptTest.groovy diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 97a76db3ab9..594070c05cf 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -28,6 +28,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Added configurable CORS `allowedOrigins` setting to Gremlin Server; warns when wildcard origin is used alongside authentication. * Fixed `ByteBuf` leak in `GraphBinaryMessageSerializerV4` when serialization throws an `IOException`. * Changed `Tree` to no longer extend `HashMap`; it is now a final class with a tree-shaped API (`childAt`, `hasChild`, `contains`, `findSubtree`, `getOrCreateChild`, `getNodesAtDepth`, `getLeafNodes`, `nodeCount`) and is no longer a `Map`. +* Changed `count(local)` on a `Tree` to return the total node count (`Tree.nodeCount()`) instead of the root-entry count. +* Changed Tree class in Java to not extend from HashMap and offered a new tree-shaped API for navigation. * Added typed numeric wrappers and `preciseNumbers` connection option to `gremlin-javascript` for explicit control over numeric type serialization and deserialization. * Added `NextN(n)` to `Traversal` in `gremlin-go` for batched result iteration, providing API parity with `next(n)` in the Java, Python, and .NET GLVs, and updated the Go translators in `gremlin-core` and `gremlin-javascript` to emit `NextN(n)` for the batched form. * Added Provider Defined Types (PDT) support — graph providers can define custom types via `@ProviderDefined` annotation that serialize/deserialize seamlessly across all GLVs without driver-side configuration. Replaces TP3 custom type mechanism. diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index 548c2beb35a..7d40629e28e 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -1481,8 +1481,9 @@ g.V().hasLabel('person').outE('created').count().map {it.get() * 10}.path() <2> <1> `count()`-step is a <> meaning that all of the previous traversers are folded into a new traverser. <2> The path of the traverser emanating from `count()` starts at `count()`. -IMPORTANT: `count(local)` counts the current, local object (not the objects in the traversal stream). This works for -`Collection`- and `Map`-type objects. For any other object, a count of 1 is returned. +IMPORTANT: `count(local)` counts the current, local object (not the objects in the traversal stream). For a +`Collection` it returns the number of elements, for a `Map` the number of entries, for a `Path` the number of +objects in the path, and for a `Tree` the total number of nodes in the tree. For any other object, a count of 1 is returned. *Additional References* @@ -5161,8 +5162,8 @@ The resultant tree data structure can then be navigated with its tree-shaped API [gremlin-groovy,modern] ---- tree = g.V().out().out().tree().by('name').next() -tree.childAt('marko') -tree.childAt('marko').childAt('josh') +tree.childAt('marko') // with the SugarPlugin: tree['marko'] +tree.childAt('marko').childAt('josh') // with the SugarPlugin: tree['marko']['josh'] tree.getNodesAtDepth(2) ---- diff --git a/docs/src/upgrade/release-4.x.x.asciidoc b/docs/src/upgrade/release-4.x.x.asciidoc index 7be918d7d7d..b2a3bdf09eb 100644 --- a/docs/src/upgrade/release-4.x.x.asciidoc +++ b/docs/src/upgrade/release-4.x.x.asciidoc @@ -654,15 +654,21 @@ Replacements for the common `Map`-based access patterns: |`getLeafObjects()` |`getLeafNodes()` |======================= -A few Gremlin patterns that worked only because `Tree` was a `Map` (for example `select(keys)`, `count(local)`, -and `unfold()` applied to a `Tree`) no longer compose. Process the `Tree` result client-side after `next()`, or -reshape the upstream traversal. The old `count(local)` (which returned the root-node count) is replaced by -`rootNodes().size()` for the root count, or `nodeCount()` for the total node count. +A few Gremlin patterns that worked only because `Tree` was a `Map` (for example `select(keys)` and `unfold()` +applied to a `Tree`) no longer compose. Process the `Tree` result client-side after `next()`, or reshape the +upstream traversal. + +`count(local)` on a `Tree` has changed semantics. Previously, because `Tree` was a `Map`, it returned the +number of root entries; it now returns the total number of nodes in the tree (`Tree.nodeCount()`), consistent +with how `count(local)` counts the contents of other local objects (for example the objects in a `Path`). Use +`rootNodes().size()` on the materialized `Tree` if the root-entry count is required. `isLeaf()` no longer throws on an empty tree (it returns `true`), and a `Tree` keeps the long-standing limitation that sibling branches resolving to the same value collapse into one node; use `path()` or `subgraph()` when full path structure must be preserved. +See: link:https://lists.apache.org/thread/o0nqh6kmrkdht531655p351ldjll045d[[DISCUSS] Make Tree no longer extend HashMap] + === Upgrading for Providers @@ -687,6 +693,15 @@ registration. (`get`, `put`, `keySet`, `entrySet`, `size`) must use the tree-shaped API instead: read with `rootNodes()` + `childAt(key)` and build with `getOrCreateChild(key)` + `addTree(subtree)`. The GraphSON (`g:Tree`) and GraphBinary (`0x2b`) wire formats are unchanged, so no GraphSON- or GraphBinary-serialized data or cross-version +===== Tree Serialization and API Changes + +`Tree` no longer extends `HashMap`. + +Providers that use the reference (de)serializers receive a `Tree` instance from TinkerPop. Code that inspected or +rebuilt that `Tree` via `Map` methods (`get`, `put`, `keySet`, `entrySet`, `size`) must use the tree-shaped API +instead: read with `rootNodes()` + `childAt(key)` and build with `getOrCreateChild(key)` + `addTree(subtree)`. + +The GraphSON (`g:Tree`) and GraphBinary (`0x2b`) wire formats are unchanged, so no GraphSON- or GraphBinary-serialized data or cross-version compatibility is affected. The Gryo `Tree` wire format, however, did change: in 3.x a `Tree` was serialized via Kryo's default `Map` serializer because it extended `HashMap`, whereas in 4.x it has an explicit `Tree` serializer. As a result, Gryo-serialized `Tree` data written by 3.x cannot be read by 4.x and vice versa. The diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java index 9c8886e2b7d..912150ffbb8 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStep.java @@ -43,10 +43,8 @@ public CountLocalStep(final Traversal.Admin traversal) { @Override protected Long map(final Traverser.Admin traverser) { final S item = traverser.get(); - if (item instanceof Tree) - throw new IllegalArgumentException("count(local) is not supported on Tree; Call next() and use the " + - "Tree API: rootNodes().size() for the root-node count, or nodeCount() for the total node count."); - return (item instanceof Collection) ? ((Collection) item).size() + return (item instanceof Tree) ? ((Tree) item).nodeCount() + : (item instanceof Collection) ? ((Collection) item).size() : (item instanceof Map) ? ((Map) item).size() : (item instanceof Path) ? ((Path) item).size() : IteratorUtils.count(IteratorUtils.asIterator(item)); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java index 99432a5e19f..aa64f0df7fa 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Tree.java @@ -28,10 +28,11 @@ import java.util.Set; /** - * A tree data structure with a tree-shaped public API that is composed of a {@link HashMap}. + * A tree data structure with a tree-shaped public API. *

- * Internal storage is a plain {@link HashMap}, so iteration order is not promised and sibling-key collisions still - * collapse (a known limitation, tracked as TINKERPOP-1693). + * Iteration order over the nodes at a given level is not promised. Because children are keyed by node value, + * sibling branches that resolve to the same value are represented as a single node; callers that require every + * path to be preserved should use {@link org.apache.tinkerpop.gremlin.process.traversal.Path} instead. * * @author Marko A. Rodriguez (http://markorodriguez.com) */ diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java index 77c06bf6e6f..0126a366a70 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/CountLocalStepTest.java @@ -30,8 +30,7 @@ import java.util.Collections; import java.util.List; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; /** * @author Daniel Kuppitz (http://gremlin.guru) @@ -43,16 +42,31 @@ protected List getTraversals() { return Collections.singletonList(__.count(Scope.local)); } - @Test - public void shouldThrowOnTree() { - final Traversal.Admin traversal = __.count(Scope.local).asAdmin(); - final CountLocalStep step = (CountLocalStep) traversal.getSteps().get(0); - - final Tree tree = new Tree<>(); - tree.getOrCreateChild("a").getOrCreateChild("b"); - final Traverser.Admin traverser = new B_O_Traverser<>(tree, 1L); - - final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> step.map(traverser)); - assertTrue(ex.getMessage().contains("not supported on Tree")); - } + @Test + public void shouldCountTotalNodesOnTree() { + final Traversal.Admin traversal = __.count(Scope.local).asAdmin(); + final CountLocalStep step = (CountLocalStep) traversal.getSteps().get(0); + + // tree: a -> b (two total nodes) + final Tree tree = new Tree<>(); + tree.getOrCreateChild("a").getOrCreateChild("b"); + final Traverser.Admin traverser = new B_O_Traverser<>(tree, 1L); + + // count(local) on a Tree returns the total node count (Tree.nodeCount()), not the root-entry count + assertEquals(Long.valueOf(2L), step.map(traverser)); + } + + @Test + public void shouldCountTotalNodesOnMultiRootTree() { + final Traversal.Admin traversal = __.count(Scope.local).asAdmin(); + final CountLocalStep step = (CountLocalStep) traversal.getSteps().get(0); + + // tree with two roots and nested children: x -> x1, y (three total nodes) + final Tree tree = new Tree<>(); + tree.getOrCreateChild("x").getOrCreateChild("x1"); + tree.getOrCreateChild("y"); + final Traverser.Admin traverser = new B_O_Traverser<>(tree, 1L); + + assertEquals(Long.valueOf(3L), step.map(traverser)); + } } diff --git a/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/SugarLoader.groovy b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/SugarLoader.groovy index f80b651c771..5f33f277237 100644 --- a/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/SugarLoader.groovy +++ b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/SugarLoader.groovy @@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.groovy.loaders import org.apache.tinkerpop.gremlin.process.traversal.Traversal import org.apache.tinkerpop.gremlin.process.traversal.Traverser +import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__ @@ -76,6 +77,7 @@ class SugarLoader { Vertex.metaClass.mixin(VertexCategory.class); Edge.metaClass.mixin(ElementCategory.class); VertexProperty.metaClass.mixin(ElementCategory.class); + Tree.metaClass.mixin(TreeCategory.class); } public static class TraverserCategory { @@ -88,6 +90,17 @@ class SugarLoader { } } + public static class TreeCategory { + // Groovy subscript on a Tree: tree['marko'] -> tree.childAt('marko'). + public static final Object getAt(final Tree tree, final String key) { + return tree.childAt(key); + } + + public static final Object getAt(final Tree tree, final Object key) { + return tree.childAt(key); + } + } + public static class ElementCategory { public static final Object get(final Element element, final String key) { final Property property = element.property(key); diff --git a/gremlin-groovy/src/test/groovy/org/apache/tinkerpop/gremlin/groovy/TreeSubscriptTest.groovy b/gremlin-groovy/src/test/groovy/org/apache/tinkerpop/gremlin/groovy/TreeSubscriptTest.groovy new file mode 100644 index 00000000000..b2d068e2c4f --- /dev/null +++ b/gremlin-groovy/src/test/groovy/org/apache/tinkerpop/gremlin/groovy/TreeSubscriptTest.groovy @@ -0,0 +1,68 @@ +/* + * 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.groovy + +import org.apache.tinkerpop.gremlin.groovy.loaders.SugarLoader +import org.apache.tinkerpop.gremlin.groovy.util.SugarTestHelper +import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree +import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory +import org.junit.Before +import org.junit.Test + +import static org.junit.Assert.assertEquals +import static org.junit.Assert.assertTrue +import static org.junit.Assert.fail + +/** + * Verifies that, with the Gremlin Groovy sugar loaded, the subscript operator (tree[key]) on a + * {@link Tree} resolves to {@code childAt(key)}, restoring the tree['marko']['josh'] syntax after + * {@code Tree} stopped extending {@code HashMap}. + */ +class TreeSubscriptTest { + + @Before + public void setup() throws Exception { + SugarTestHelper.clearRegistry() + SugarLoader.load() + } + + @Test + public void shouldSupportSubscriptViaSugar() { + def g = TinkerFactory.createModern().traversal() + Tree tree = g.V(1).out().out().tree().by('name').next() + + assertEquals(tree.childAt('marko'), tree['marko']) + assertEquals(tree.childAt('marko').childAt('josh'), tree['marko']['josh']) + assertTrue(tree['marko']['josh'].rootNodes().contains('ripple')) + assertTrue(tree['marko']['josh'].rootNodes().contains('lop')) + } + + @Test + public void shouldThrowOnMissingSubscriptKey() { + def g = TinkerFactory.createModern().traversal() + Tree tree = g.V(1).out().out().tree().by('name').next() + + try { + tree['nonexistent'] + fail("expected IllegalArgumentException for missing subscript key") + } catch (IllegalArgumentException iae) { + assertTrue(iae.message.contains('nonexistent')) + } + } +} diff --git a/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js b/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js index 93708fefce4..d4e6e97e758 100644 --- a/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js +++ b/gremlin-js/gremlin-javascript/test/cucumber/gremlin.js @@ -2201,6 +2201,8 @@ const gremlins = { g_VX1X_out_out_out_tree: [function({g}) { return g.V().out().out().out().tree() }], g_VX1X_outE_inV_bothE_otherV_tree: [function({g, vid1}) { return g.V(vid1).outE().inV().bothE().otherV().tree() }], g_VX1X_outE_inV_bothE_otherV_tree_byXnameX_byXlabelX: [function({g, vid1}) { return g.V(vid1).outE().inV().bothE().otherV().tree().by("name").by(T.label) }], + g_V_out_treeXaX_selectXaX_countXlocalX: [function({g}) { return g.V().out().tree("a").select("a").count(Scope.local) }], + g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX: [function({g}) { return g.V().out().local(__.tree("a").select("a").count(Scope.local)) }], g_io_writeXkryoX: [function({g}) { return g.io("tinkerpop-modern-v3.kryo").write() }], g_io_write_withXwriter_gryoX: [function({g}) { return g.io("tinkerpop-modern-v3.kryo").with_(IO.writer, IO.gryo).write() }], g_io_writeXgraphsonX: [function({g}) { return g.io("tinkerpop-modern-v3.json").write() }], diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json index 8fe7421aa7d..f152691348e 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/language/translator/translations.json @@ -44794,6 +44794,40 @@ } ] }, + { + "scenario": "g_V_out_treeXaX_selectXaX_countXlocalX", + "traversals": [ + { + "original": "g.V().out().tree(\"a\").select(\"a\").count(local)", + "language": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", + "canonical": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", + "anonymized": "g.V().out().tree(string0).select(string0).count(Scope.local)", + "dotnet": "g.V().Out().Tree(\"a\").Select(\"a\").Count(Scope.Local)", + "go": "g.V().Out().Tree(\"a\").Select(\"a\").Count(gremlingo.Scope.Local)", + "groovy": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", + "java": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", + "javascript": "g.V().out().tree(\"a\").select(\"a\").count(Scope.local)", + "python": "g.V().out().tree('a').select('a').count(Scope.local)" + } + ] + }, + { + "scenario": "g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX", + "traversals": [ + { + "original": "g.V().out().local(tree(\"a\").select(\"a\").count(local))", + "language": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", + "canonical": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", + "anonymized": "g.V().out().local(__.tree(string0).select(string0).count(Scope.local))", + "dotnet": "g.V().Out().Local(__.Tree(\"a\").Select(\"a\").Count(Scope.Local))", + "go": "g.V().Out().Local(gremlingo.T__.Tree(\"a\").Select(\"a\").Count(gremlingo.Scope.Local))", + "groovy": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", + "java": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", + "javascript": "g.V().out().local(__.tree(\"a\").select(\"a\").count(Scope.local))", + "python": "g.V().out().local(__.tree('a').select('a').count(Scope.local))" + } + ] + }, { "scenario": "g_io_writeXkryoX", "traversals": [ diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature index 19fd429de85..c239ad053ba 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Tree.feature @@ -206,3 +206,36 @@ Feature: Step - tree() |--josh |--peter """ + + Scenario: g_V_out_treeXaX_selectXaX_countXlocalX + Given the modern graph + And the traversal of + """ + g.V().out().tree("a").select("a").count(local) + """ + When iterated to list + Then the result should be unordered + | result | + | d[9].l | + | d[9].l | + | d[9].l | + | d[9].l | + | d[9].l | + | d[9].l | + + @InsertionOrderingRequired + Scenario: g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX + Given the modern graph + And the traversal of + """ + g.V().out().local(tree("a").select("a").count(local)) + """ + When iterated to list + Then the result should be unordered + | result | + | d[2].l | + | d[3].l | + | d[4].l | + | d[6].l | + | d[7].l | + | d[9].l | From fb01988a5d82cc12cfdf5cc513fa3c34085fa3c1 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 12 Jun 2026 10:10:10 -0700 Subject: [PATCH 5/6] Reverted feature test deletions --- gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js | 2 ++ .../org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java | 1 + .../tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java | 1 + .../org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java | 1 + 4 files changed, 5 insertions(+) diff --git a/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js b/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js index 907412ac2c1..bb956a2538d 100644 --- a/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js +++ b/gremlin-js/gremlin-javascript/test/cucumber/feature-steps.js @@ -90,6 +90,8 @@ const ignoredScenarios = { 'g_VX1X_out_out_out_tree': new IgnoreError(ignoreReason.treeStepNotSupported), 'g_VX1X_outE_inV_bothE_otherV_tree': new IgnoreError(ignoreReason.treeStepNotSupported), 'g_VX1X_outE_inV_bothE_otherV_tree_byXnameX_byXlabelX': new IgnoreError(ignoreReason.treeStepNotSupported), + 'g_V_out_treeXaX_selectXaX_countXlocalX': new IgnoreError(ignoreReason.treeStepNotSupported), + 'g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX': new IgnoreError(ignoreReason.treeStepNotSupported), // floating point issues 'g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack': new IgnoreError(ignoreReason.floatingPointIssues), 'g_withSackX2X_V_sackXdivX_byXconstantX4_0XX_sack': new IgnoreError(ignoreReason.floatingPointIssues), diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java index ddc48092c2f..42bc9b5deea 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java @@ -159,6 +159,7 @@ public void beforeEachScenario(Scenario scenario) { // TINKERPOP-3209 case "g_V_localXgroupCountXaX_selectXaX_countXlocalXX": case "g_V_localXgroupXaX_byXnameX_by_selectXaX_countXlocalXX": + case "g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX": throw new AssumptionViolatedException("Selecting side effects inside local() produces inconsistent results in OLAP (See TINKERPOP-3209)"); default: // Do nothing diff --git a/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java b/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java index 99db7b60e7e..884b27ac8d8 100644 --- a/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java +++ b/spark-gremlin/src/test/java/org/apache/tinkerpop/gremlin/spark/SparkGraphFeatureIntegrateTest.java @@ -98,6 +98,7 @@ public class SparkGraphFeatureIntegrateTest { add(Pair.with("g_V_untilXloops_isX2XX_repeatXout_order_byXnameXX_valuesXnameX", skipReasonOrdering)); add(Pair.with("g_V_localXgroupCountXaX_selectXaX_countXlocalXX", skipReasonTINKERPOP3209)); add(Pair.with("g_V_localXgroupXaX_byXnameX_by_selectXaX_countXlocalXX", skipReasonTINKERPOP3209)); + add(Pair.with("g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX", skipReasonTINKERPOP3209)); }}; public static final class ServiceModule extends AbstractModule { diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java index 29f1d4e0b65..1a6ff95e020 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerWorld.java @@ -344,6 +344,7 @@ public void beforeEachScenario(final Scenario scenario) { // TINKERPOP-3209 case "g_V_localXgroupCountXaX_selectXaX_countXlocalXX": case "g_V_localXgroupXaX_byXnameX_by_selectXaX_countXlocalXX": + case "g_V_out_order_byXnameX_localXtreeXaX_selectXaX_countXlocalXX": throw new AssumptionViolatedException("Selecting side effects inside local() produces inconsistent results in OLAP (See TINKERPOP-3209)"); default: // Do nothing From 49bb284e2bdbb9870bab05969a0a9a0bc842184f Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 12 Jun 2026 15:43:56 -0700 Subject: [PATCH 6/6] Improve doc example --- docs/src/reference/the-traversal.asciidoc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index 7d40629e28e..a2bc194fb6e 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -5162,9 +5162,12 @@ The resultant tree data structure can then be navigated with its tree-shaped API [gremlin-groovy,modern] ---- tree = g.V().out().out().tree().by('name').next() -tree.childAt('marko') // with the SugarPlugin: tree['marko'] -tree.childAt('marko').childAt('josh') // with the SugarPlugin: tree['marko']['josh'] +tree.childAt('marko') +tree.childAt('marko').childAt('josh') tree.getNodesAtDepth(2) +// The sugar plugin offer convenience accessors +tree['marko'] +tree['marko']['josh'] ---- Note that when using `by()`-modulation, tree nodes are combined based on projection uniqueness, not on the