This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch gvalue in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit ee43de1be589c9b5de12397201c6174756c5b1e1 Author: Stephen Mallette <[email protected]> AuthorDate: Thu Aug 8 16:24:37 2024 -0400 wip --- .../tinkerpop/gremlin/process/traversal/P.java | 20 +++++++++++++++++--- .../process/traversal/dsl/graph/GraphTraversal.java | 7 +++++++ .../gremlin/process/traversal/step/GValue.java | 4 ++-- .../process/traversal/step/map/GraphStep.java | 13 +++++++++++++ .../process/traversal/step/util/AbstractStep.java | 19 ++++++++++++++++++- .../process/traversal/step/util/HasContainer.java | 6 ++++-- .../gremlin/process/traversal/step/GValueTest.java | 4 ++-- .../traversal/step/sideEffect/TinkerGraphStep.java | 14 ++++++++------ 8 files changed, 71 insertions(+), 16 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java index 56e1946368..622a3c3e3a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java @@ -25,7 +25,9 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; import java.io.Serializable; import java.util.Arrays; import java.util.Collection; +import java.util.List; import java.util.function.Predicate; +import java.util.stream.Collectors; /** * Predefined {@code Predicate} values that can be used to define filters to {@code has()} and {@code where()}. @@ -75,10 +77,22 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { @Override public boolean test(final V testValue) { - if (this.value instanceof GValue) + if (this.value instanceof GValue) { return this.biPredicate.test(testValue, ((GValue<V>) this.value).get()); - else - return this.biPredicate.test(testValue, this.value); + } else { + // this might be a bunch of GValue that need to be resolved. zomg + if (this.value instanceof List) { + return this.biPredicate.test(testValue, (V) ((List) this.value).stream().map(o -> { + if (o instanceof GValue) { + return ((GValue) o).get(); + } else { + return o; + } + }).collect(Collectors.toList())); + } else { + return this.biPredicate.test(testValue, this.value); + } + } } @Override diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java index 0a392fc334..a7379fd914 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java @@ -25,6 +25,7 @@ import org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.PageRank import org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.PeerPressureVertexProgramStep; import org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.ProgramVertexProgramStep; import org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.ShortestPathVertexProgramStep; +import org.apache.tinkerpop.gremlin.process.traversal.Contains; import org.apache.tinkerpop.gremlin.process.traversal.DT; import org.apache.tinkerpop.gremlin.process.traversal.Failure; import org.apache.tinkerpop.gremlin.process.traversal.Merge; @@ -2837,6 +2838,12 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { } else { this.asAdmin().getBytecode().addStep(Symbols.hasId, id, otherIds); + // the logic for dealing with hasId([]) is sketchy historically, just trying to maintain what we were + // originally testing prior to GValue. + if (id instanceof GValue && ((GValue) id).getType().isCollection()) { + return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.id.getAccessor(), new P(Contains.within, id))); + } + //using ArrayList given P.within() turns all arguments into lists final List<Object> ids = new ArrayList<>(); if (id instanceof Object[]) { diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java index 41caaea1a2..da0694acdc 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValue.java @@ -246,14 +246,14 @@ public class GValue<V> implements Cloneable, Serializable { /** * Create a new {@code GValue} for a list value. */ - public static GValue<List> ofList(final List value) { + public static <T> GValue<List<T>> ofList(final List<T> value) { return new GValue<>(GType.LIST, value); } /** * Create a new {@code GValue} for a list value with a specified name. */ - public static GValue<List> ofList(final String name, final List value) { + public static <T> GValue<List<T>> ofList(final String name, final List<T> value) { return new GValue<>(name, GType.LIST, value); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java index 5c03500d6e..784ef087f3 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java @@ -43,6 +43,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.function.Supplier; @@ -56,6 +57,7 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen protected Parameters parameters = new Parameters(); protected final Class<E> returnClass; protected GValue<?>[] ids; + protected boolean legacyLogicForPassingNoIds = false; protected transient Supplier<Iterator<E>> iteratorSupplier; protected boolean isStart; protected boolean done = false; @@ -152,10 +154,21 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen * Gets the ids associated with this step as literal values rather than {@link GValue} objects. */ public Object[] getResolvedIds() { + if (legacyLogicForPassingNoIds) return null; return resolveToValues(this.ids); } public void addIds(final Object... newIds) { + // there is some logic that has been around for a long time that used to set ids to null. it only happened here + // in this method and only occurred when the ids were already null or empty and the newIds were length 1 and + // an instance of List and that list was empty. so basically it would trigger for something like g.V().hasId([]) + // which in turn would trigger an empty iterator in TinkerGraphStep and zero results. trying to maintain that + // logic now with GValue in the mix is tough because the context of what the meaning is gets lost by the time + // you get to calling getResolvedIds(). using a flag to try to maintain that legacy logic, but ultimately, all + // this needs to be rethought. + this.legacyLogicForPassingNoIds = newIds.length == 1 && ((newIds[0] instanceof List && ((List) newIds[0]).isEmpty()) || + (newIds[0] instanceof GValue && ((GValue) newIds[0]).getType().isCollection() && ((List) ((GValue) newIds[0]).get()).isEmpty())); + final GValue[] gvalues = convertToGValues(tryUnrollSingleCollectionArgument(newIds)); this.ids = ArrayUtils.addAll(this.ids, gvalues); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java index 15ddacdc91..dd6f5fb849 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java @@ -28,9 +28,11 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversal; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalInterruptedException; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashSet; +import java.util.List; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; @@ -245,7 +247,22 @@ public abstract class AbstractStep<S, E> implements Step<S, E> { } /** - * Converts {@link GValue} objects the argument array to their values to prevent them from leaking to the Graph API. + * Converts {@link GValue} objects arguments to their values to prevent them from leaking to the Graph API. + * Providers extending from this step should use this method to get actual values to prevent any {@link GValue} + * objects from leaking to the Graph API. + */ + protected Object[] resolveToValues(final List<GValue<?>> gvalues) { + // convert gvalues to array + final Object[] newIds = new Object[gvalues.size()]; + int i = 0; + for (final GValue<?> gvalue : gvalues) { + newIds[i++] = gvalue.get(); + } + return newIds; + } + + /** + * Converts {@link GValue} objects argument array to their values to prevent them from leaking to the Graph API. * Providers extending from this step should use this method to get actual values to prevent any {@link GValue} * objects from leaking to the Graph API. */ diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java index c20c68fde0..d456b09df6 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java @@ -19,10 +19,12 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.util; import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.step.GType; import org.apache.tinkerpop.gremlin.structure.Element; import org.apache.tinkerpop.gremlin.structure.Property; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; +import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; import java.io.Serializable; import java.util.Collection; @@ -154,11 +156,11 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element> if (predicateValue instanceof Collection) { final Collection collection = (Collection) predicateValue; if (!collection.isEmpty()) { - return ((Collection) predicateValue).stream().allMatch(c -> null == c || c instanceof String); + return ((Collection) predicateValue).stream().allMatch(c -> null == c || c instanceof String || (c instanceof GValue && ((GValue) c).getType() == GType.STRING)); } } - return predicateValue instanceof String; + return predicateValue instanceof String || (predicateValue instanceof GValue && ((GValue) predicateValue).getType() == GType.STRING); } return false; diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java index d1742e3e29..f4ae56dff8 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/GValueTest.java @@ -203,7 +203,7 @@ public class GValueTest { @Test public void shouldCreateGValueFromList() { final List<String> list = Arrays.asList("value1", "value2"); - final GValue<List> gValue = GValue.ofList(list); + final GValue<List<String>> gValue = GValue.ofList(list); assertEquals(list, gValue.get()); assertEquals(GType.LIST, gValue.getType()); assertThat(gValue.isVariable(), is(false)); @@ -212,7 +212,7 @@ public class GValueTest { @Test public void shouldCreateGValueFromListWithName() { final List<String> list = Arrays.asList("value1", "value2"); - final GValue<List> gValue = GValue.ofList("varName", list); + final GValue<List<String>> gValue = GValue.ofList("varName", list); assertEquals(list, gValue.get()); assertEquals(GType.LIST, gValue.getType()); assertEquals("varName", gValue.getName()); diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java index e529d78390..9715c0e4e4 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java @@ -71,11 +71,12 @@ public final class TinkerGraphStep<S, E extends Element> extends GraphStep<S, E> final AbstractTinkerGraph graph = (AbstractTinkerGraph) this.getTraversal().getGraph().get(); final HasContainer indexedContainer = getIndexKey(Edge.class); Iterator<Edge> iterator; + final Object[] resolvedIds = this.getResolvedIds(); // ids are present, filter on them first - if (null == this.ids) + if (null == resolvedIds) iterator = Collections.emptyIterator(); - else if (this.ids.length > 0) - iterator = this.iteratorList(graph.edges(this.getResolvedIds())); + else if (resolvedIds.length > 0) + iterator = this.iteratorList(graph.edges(resolvedIds)); else iterator = null == indexedContainer ? this.iteratorList(graph.edges()) : @@ -93,11 +94,12 @@ public final class TinkerGraphStep<S, E extends Element> extends GraphStep<S, E> final AbstractTinkerGraph graph = (AbstractTinkerGraph) this.getTraversal().getGraph().get(); final HasContainer indexedContainer = getIndexKey(Vertex.class); Iterator<? extends Vertex> iterator; + final Object[] resolvedIds = this.getResolvedIds(); // ids are present, filter on them first - if (null == this.ids) + if (null == resolvedIds) iterator = Collections.emptyIterator(); - else if (this.ids.length > 0) - iterator = this.iteratorList(graph.vertices(this.getResolvedIds())); + else if (resolvedIds.length > 0) + iterator = this.iteratorList(graph.vertices(resolvedIds)); else iterator = (null == indexedContainer ? this.iteratorList(graph.vertices()) :
