Skip to content

Commit 743fd1d

Browse files
andreachildandrii0lomakin
andreachild
and
andrii0lomakin
authored
Fix for JavaTranslator handling of has with null last argument (#3278)
Fixed JavaTranslator to handle scenarios for has where the second or third arguments are null. If a null second or third argument is encountered, the method overload with a predicate type for the last arg should be avoided as a null predicate will throw NullPointerException. Co-authored-by: Andrii Lomakin <[email protected]>
1 parent 337dc22 commit 743fd1d

File tree

3 files changed

+104
-6
lines changed

3 files changed

+104
-6
lines changed

‎CHANGELOG.asciidoc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ This release also includes changes from <<release-3-7-5, 3.7.5>>.
138138
* Modified `limit()`, `skip()`, range()` in `repeat()` to track per-iteration counters.
139139
* Moved `Traverser` loop logic into new interface `NL_SL_Traverser` and changed loop-supporting `Traverser`s to extend the common interface.
140140
* Fixed `sample()` in `repeat()` computer algorithm to retain the configured sample size per loop
141+
* Fixed `JavaTranslator` to handle scenarios where the last argument to `has()` is null which previously could cause an exception.
141142
142143
==== Bugs
143144

‎gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@
2121

2222
import org.apache.commons.configuration2.BaseConfiguration;
2323
import org.apache.commons.configuration2.Configuration;
24-
import org.apache.commons.configuration2.MapConfiguration;
2524
import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
25+
import org.apache.tinkerpop.gremlin.process.traversal.P;
2626
import org.apache.tinkerpop.gremlin.process.traversal.Translator;
2727
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
2828
import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource;
@@ -301,8 +301,8 @@ private Object invokeMethod(final Object delegate, final Class<?> returnType, fi
301301
}
302302
}
303303

304-
// special case has() where the first arg is null - sometimes this can end up with the T being
305-
// null and in 3.5.x that generates an exception which raises badly in the translator. it is
304+
// special cases of has() where the first or last arg is null - sometimes this can end up with the T or P being
305+
// null which generates an exception which raises badly in the translator. it is
306306
// safer to force this to the String form by letting this "found" version pass. In java this
307307
// form of GraphTraversal can't be produced because of validations for has(T, ...) but in
308308
// 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
312312
// badly bytecode should either change to use gremlin-language and go away or bytecode should
313313
// get a safer way to be translated to a traversal with more explicit calls that don't rely
314314
// on reflection.
315-
if (methodName.equals(GraphTraversal.Symbols.has) && newArguments.length > 0 && null == newArguments[0] &&
316-
method.getParameterTypes()[0].isAssignableFrom(org.apache.tinkerpop.gremlin.structure.T.class)) {
317-
found = false;
315+
Class<?>[] parametersTypes = method.getParameterTypes();
316+
if (methodName.equals(GraphTraversal.Symbols.has) && newArguments.length > 0) {
317+
//first case has((T)null, ...) instead of has((String)null, ...)
318+
if (null == newArguments[0] &&
319+
parametersTypes[0].isAssignableFrom(org.apache.tinkerpop.gremlin.structure.T.class)) {
320+
found = false;
321+
} else if (newArguments[newArguments.length - 1] == null && parametersTypes[newArguments.length - 1] == P.class) {
322+
//the second case has(String, String, (P)(null)) instead of has(String,String, (Object)null)
323+
//or has(String, (P)(null)) instead of has(String, (Object)null)
324+
found = false;
325+
}
318326
}
319327

320328
if (found) {

‎gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslatorTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.tinkerpop.gremlin.jsr223;
21+
22+
import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
23+
import org.apache.tinkerpop.gremlin.process.traversal.P;
24+
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
25+
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
26+
import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
27+
import org.junit.Test;
28+
29+
import static org.junit.Assert.assertEquals;
30+
31+
public class JavaTranslatorTest {
32+
private GraphTraversalSource g = EmptyGraph.instance().traversal();
33+
private JavaTranslator<GraphTraversalSource, Traversal.Admin<?, ?>> translator = JavaTranslator.of(EmptyGraph.instance().traversal());
34+
35+
@Test
36+
public void shouldTranslateHasWithObjectThirdArgValue() {
37+
final Bytecode bytecode = new Bytecode();
38+
bytecode.addStep("E");
39+
bytecode.addStep("has", "knows", "weight", 1.0);
40+
final Traversal.Admin<?, ?> translation = translator.translate(bytecode);
41+
assertEquals(g.E().has("knows", "weight", 1.0).asAdmin(), translation);
42+
}
43+
44+
@Test
45+
public void shouldTranslateHasWithPredicateThirdArgValue() {
46+
final Bytecode bytecode = new Bytecode();
47+
bytecode.addStep("E");
48+
bytecode.addStep("has", "knows", "weight", P.eq(1.0));
49+
final Traversal.Admin<?, ?> translation = translator.translate(bytecode);
50+
assertEquals(g.E().has("knows", "weight", P.eq(1.0)).asAdmin(), translation);
51+
}
52+
53+
@Test
54+
public void shouldTranslateHasWithNullThirdArgValue() {
55+
final Bytecode bytecode = new Bytecode();
56+
bytecode.addStep("E");
57+
bytecode.addStep("has", "knows", "weight", null);
58+
final Traversal.Admin<?, ?> translation = translator.translate(bytecode);
59+
assertEquals(g.E().has("knows", "weight", (String) null).asAdmin(), translation);
60+
}
61+
62+
@Test
63+
public void shouldTranslateHasWithObjectSecondArgValue() {
64+
final Bytecode bytecode = new Bytecode();
65+
bytecode.addStep("E");
66+
bytecode.addStep("has", "weight", 1.0);
67+
final Traversal.Admin<?, ?> translation = translator.translate(bytecode);
68+
assertEquals(g.E().has("weight", 1.0).asAdmin(), translation);
69+
}
70+
71+
@Test
72+
public void shouldTranslateHasWithPredicateSecondArgValue() {
73+
final Bytecode bytecode = new Bytecode();
74+
bytecode.addStep("E");
75+
bytecode.addStep("has", "weight", P.eq(1.0));
76+
final Traversal.Admin<?, ?> translation = translator.translate(bytecode);
77+
assertEquals(g.E().has("weight", P.eq(1.0)).asAdmin(), translation);
78+
}
79+
80+
@Test
81+
public void shouldTranslateHasWithNullSecondArgValue() {
82+
final Bytecode bytecode = new Bytecode();
83+
bytecode.addStep("E");
84+
bytecode.addStep("has", "weight", null);
85+
final Traversal.Admin<?, ?> translation = translator.translate(bytecode);
86+
assertEquals(g.E().has("weight", (String) null).asAdmin(), translation);
87+
}
88+
89+
}

0 commit comments

Comments
 (0)