diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 67c6e0d61cb..efecacce2f7 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -138,6 +138,7 @@ This release also includes changes from <>. * Modified `limit()`, `skip()`, range()` in `repeat()` to track per-iteration counters. * Moved `Traverser` loop logic into new interface `NL_SL_Traverser` and changed loop-supporting `Traverser`s to extend the common interface. * Fixed `sample()` in `repeat()` computer algorithm to retain the configured sample size per loop +* Fixed `JavaTranslator` to handle scenarios where the last argument to `has()` is null which previously could cause an exception. == TinkerPop 3.7.0 (Gremfir Master of the Pan Flute) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java index fa56a2cd4fd..4a9bfd6426a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java @@ -21,8 +21,8 @@ import org.apache.commons.configuration2.BaseConfiguration; import org.apache.commons.configuration2.Configuration; -import org.apache.commons.configuration2.MapConfiguration; import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; +import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.Translator; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; @@ -301,8 +301,8 @@ private Object invokeMethod(final Object delegate, final Class returnType, fi } } - // special case has() where the first arg is null - sometimes this can end up with the T being - // null and in 3.5.x that generates an exception which raises badly in the translator. it is + // special cases of has() where the first or last arg is null - sometimes this can end up with the T or P being + // null which generates an exception which raises badly in the translator. it is // safer to force this to the String form by letting this "found" version pass. In java this // form of GraphTraversal can't be produced because of validations for has(T, ...) but in // other language it might be allowed which means that has((T) null, ...) from something like @@ -312,9 +312,17 @@ private Object invokeMethod(final Object delegate, final Class returnType, fi // badly bytecode should either change to use gremlin-language and go away or bytecode should // get a safer way to be translated to a traversal with more explicit calls that don't rely // on reflection. - if (methodName.equals(GraphTraversal.Symbols.has) && newArguments.length > 0 && null == newArguments[0] && - method.getParameterTypes()[0].isAssignableFrom(org.apache.tinkerpop.gremlin.structure.T.class)) { - found = false; + Class[] parametersTypes = method.getParameterTypes(); + if (methodName.equals(GraphTraversal.Symbols.has) && newArguments.length > 0) { + //first case has((T)null, ...) instead of has((String)null, ...) + if (null == newArguments[0] && + parametersTypes[0].isAssignableFrom(org.apache.tinkerpop.gremlin.structure.T.class)) { + found = false; + } else if (newArguments[newArguments.length - 1] == null && parametersTypes[newArguments.length - 1] == P.class) { + //the second case has(String, String, (P)(null)) instead of has(String,String, (Object)null) + //or has(String, (P)(null)) instead of has(String, (Object)null) + found = false; + } } if (found) { diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslatorTest.java new file mode 100644 index 00000000000..18b7c59057c --- /dev/null +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslatorTest.java @@ -0,0 +1,89 @@ +/* + * 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.jsr223; + +import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class JavaTranslatorTest { + private GraphTraversalSource g = EmptyGraph.instance().traversal(); + private JavaTranslator> translator = JavaTranslator.of(EmptyGraph.instance().traversal()); + + @Test + public void shouldTranslateHasWithObjectThirdArgValue() { + final Bytecode bytecode = new Bytecode(); + bytecode.addStep("E"); + bytecode.addStep("has", "knows", "weight", 1.0); + final Traversal.Admin translation = translator.translate(bytecode); + assertEquals(g.E().has("knows", "weight", 1.0).asAdmin(), translation); + } + + @Test + public void shouldTranslateHasWithPredicateThirdArgValue() { + final Bytecode bytecode = new Bytecode(); + bytecode.addStep("E"); + bytecode.addStep("has", "knows", "weight", P.eq(1.0)); + final Traversal.Admin translation = translator.translate(bytecode); + assertEquals(g.E().has("knows", "weight", P.eq(1.0)).asAdmin(), translation); + } + + @Test + public void shouldTranslateHasWithNullThirdArgValue() { + final Bytecode bytecode = new Bytecode(); + bytecode.addStep("E"); + bytecode.addStep("has", "knows", "weight", null); + final Traversal.Admin translation = translator.translate(bytecode); + assertEquals(g.E().has("knows", "weight", (String) null).asAdmin(), translation); + } + + @Test + public void shouldTranslateHasWithObjectSecondArgValue() { + final Bytecode bytecode = new Bytecode(); + bytecode.addStep("E"); + bytecode.addStep("has", "weight", 1.0); + final Traversal.Admin translation = translator.translate(bytecode); + assertEquals(g.E().has("weight", 1.0).asAdmin(), translation); + } + + @Test + public void shouldTranslateHasWithPredicateSecondArgValue() { + final Bytecode bytecode = new Bytecode(); + bytecode.addStep("E"); + bytecode.addStep("has", "weight", P.eq(1.0)); + final Traversal.Admin translation = translator.translate(bytecode); + assertEquals(g.E().has("weight", P.eq(1.0)).asAdmin(), translation); + } + + @Test + public void shouldTranslateHasWithNullSecondArgValue() { + final Bytecode bytecode = new Bytecode(); + bytecode.addStep("E"); + bytecode.addStep("has", "weight", null); + final Traversal.Admin translation = translator.translate(bytecode); + assertEquals(g.E().has("weight", (String) null).asAdmin(), translation); + } + +} \ No newline at end of file