Skip to content

Commit 33f1d68

Browse files
committed
fix(server): preserve count and Cypher transport results
1 parent f776885 commit 33f1d68

11 files changed

Lines changed: 242 additions & 5 deletions

File tree

‎docs/upgrade-tinkerpop-3.8.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ registry limited to untyped V1 messages. Standard graph-file IO and typed
7878
GraphSON serializers keep `HugeGraphIoRegistry` alone and use the TinkerPop
7979
Tree format for their negotiated version.
8080

81+
Groovy interpolated strings (`GString`, including subclasses) return as standard
82+
strings over GraphBinary, including inside lists and maps. For example,
83+
`def value = 42; "r3-value:${value}"` returns `"r3-value:42"`. The untyped GraphSON
84+
path retains the legacy GString bean map with its `values` and `strings` fields.
85+
86+
Cypher extension predicates use the current TinkerPop predicate constructor
87+
through the existing translator extension point. Computed-expression regex
88+
filters retain whole-string matching (`String.matches`), so `"marko"` matches
89+
`"mar.*"` and does not match `"ark"`.
90+
8191
Cypher results normalize HugeGraph IDs, including
8292
values nested in collections, maps and paths. Cyclic results and nesting deeper
8393
than 32 levels are rejected.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.hugegraph.opencypher;
19+
20+
import org.apache.tinkerpop.gremlin.process.traversal.P;
21+
import org.apache.tinkerpop.gremlin.process.traversal.PBiPredicate;
22+
import org.opencypher.gremlin.translation.traversal.TraversalGremlinPredicates;
23+
import org.opencypher.gremlin.traversal.CustomPredicate;
24+
25+
/**
26+
* Adapt the extension factories compiled against the pre-3.8 P constructor.
27+
* The original predicates retain Cypher type checks and whole-string regex matching.
28+
*/
29+
public final class CypherGremlinPredicates extends TraversalGremlinPredicates {
30+
31+
@Override
32+
public P<Object> regexMatch(Object value) {
33+
return predicate(CustomPredicate.cypherRegex, value);
34+
}
35+
36+
@Override
37+
public P<Object> isNode() {
38+
return predicate(CustomPredicate.cypherIsNode, null);
39+
}
40+
41+
@Override
42+
public P<Object> isRelationship() {
43+
return predicate(CustomPredicate.cypherIsRelationship, null);
44+
}
45+
46+
@Override
47+
public P<Object> isString() {
48+
return predicate(CustomPredicate.cypherIsString, null);
49+
}
50+
51+
private static P<Object> predicate(CustomPredicate predicate, Object value) {
52+
PBiPredicate<Object, Object> adapter = predicate::test;
53+
return new P<>(adapter, value);
54+
}
55+
}

‎hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/opencypher/CypherOpProcessor.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
import org.opencypher.gremlin.translation.ir.TranslationWriter;
5656
import org.opencypher.gremlin.translation.ir.model.GremlinStep;
5757
import org.opencypher.gremlin.translation.translator.Translator;
58+
import org.opencypher.gremlin.translation.traversal.TraversalGremlinBindings;
59+
import org.opencypher.gremlin.translation.traversal.TraversalGremlinSteps;
5860
import org.opencypher.gremlin.traversal.ParameterNormalizer;
5961
import org.opencypher.gremlin.traversal.ProcedureContext;
6062
import org.opencypher.gremlin.traversal.ReturnNormalizer;
@@ -139,7 +141,9 @@ private void evalCypher(Context context) throws OpProcessorException {
139141
.build(translatorDefinition);
140142

141143
Translator<GraphTraversal, P> traversalTranslator = Translator.builder()
142-
.traversal(g)
144+
.custom(new TraversalGremlinSteps(g),
145+
new CypherGremlinPredicates(),
146+
new TraversalGremlinBindings())
143147
.build(translatorDefinition);
144148

145149
Seq<GremlinStep> ir = ast.translate(strTranslator.flavor(),

‎hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/io/HugeGraphTypeSerializerRegistryBuilder.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@
4444
import org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer;
4545
import org.apache.tinkerpop.gremlin.structure.io.binary.types.TransformSerializer;
4646

47+
import groovy.lang.GString;
48+
4749
public final class HugeGraphTypeSerializerRegistryBuilder
4850
extends TypeSerializerRegistry.Builder {
4951

@@ -59,6 +61,9 @@ public final class HugeGraphTypeSerializerRegistryBuilder
5961
private static final TypeSerializer<File> FILE_TRANSFORM_SERIALIZER =
6062
new WireTransformSerializer<>(file -> Map.of("file", file.getName()));
6163

64+
private static final TypeSerializer<GString> GSTRING_TRANSFORM_SERIALIZER =
65+
new WireTransformSerializer<>(GString::toString);
66+
6267
public HugeGraphTypeSerializerRegistryBuilder() {
6368
this.withFallbackResolver(type -> {
6469
if (Id.class.isAssignableFrom(type)) {
@@ -76,6 +81,9 @@ public HugeGraphTypeSerializerRegistryBuilder() {
7681
if (File.class.isAssignableFrom(type)) {
7782
return FILE_TRANSFORM_SERIALIZER;
7883
}
84+
if (GString.class.isAssignableFrom(type)) {
85+
return GSTRING_TRANSFORM_SERIALIZER;
86+
}
7987
return null;
8088
});
8189
}

‎hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeCountStrategy.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,11 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
187187
}
188188

189189
/*
190-
* HugeGraph extracts RangeGlobalStep into backend queries. A
191-
* negative upper bound is never useful for count(), and would
192-
* become an invalid backend range like [0, -3).
190+
* HugeGraph extracts RangeGlobalStep into backend queries.
191+
* Non-positive upper bounds cannot preserve count(). A zero
192+
* bound from without(-1) would replace a nonempty count with 0.
193193
*/
194-
if (highRange != null && highRange < 0L) {
194+
if (highRange != null && highRange <= 0L) {
195195
highRange = null;
196196
}
197197

‎hugegraph-server/hugegraph-dist/src/assembly/travis/run-gremlin-console-smoke-test.sh‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ try {
6969
"'${SMOKE_MARKER}-' + count"
7070
results = client.submit(remoteScript).all().get()
7171
println(results[0].object)
72+
interpolation = client.submit('def value = 42; "r3-value:\${value}"').all().get()
73+
assert interpolation.size() == 1
74+
assert interpolation[0].object instanceof String
75+
assert interpolation[0].object == 'r3-value:42'
76+
nested = client.submit('def value = 42; [["r3-value:\${value}"], ' +
77+
'[value: "r3-value:\${value}"]]').all().get()
78+
assert nested.size() == 2
79+
assert nested[0].object == ['r3-value:42']
80+
assert nested[1].object == [value: 'r3-value:42']
81+
println('gremlin-console-gstring-ok')
7282
} finally {
7383
client.close()
7484
cluster.close()
@@ -81,3 +91,4 @@ EOF
8191
) | tee "$SMOKE_LOG"
8292

8393
grep -q "$SMOKE_MARKER" "$SMOKE_LOG"
94+
grep -q "gremlin-console-gstring-ok" "$SMOKE_LOG"

‎hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,22 @@ public void testPost() {
6060
this.testCypherQueryAndContains(QUERY, QUERY_RESULT);
6161
}
6262

63+
@Test
64+
public void testComputedRegexExecutesExtensionPredicate() {
65+
String query = "MATCH (n:person) WHERE (n.name + '') =~ 'mar.*' " +
66+
"RETURN n.name AS name";
67+
String content = this.testCypherQueryAndContains(query, "marko");
68+
Assert.assertEquals(List.of(Map.of("name", "marko")), assertCypherSuccessData(content));
69+
}
70+
71+
@Test
72+
public void testComputedRegexRequiresWholeStringMatch() {
73+
String query = "MATCH (n:person) WHERE (n.name + '') =~ 'ark' " +
74+
"RETURN n.name AS name";
75+
String content = this.testCypherQueryAndContains(query, "data");
76+
Assert.assertTrue(assertCypherSuccessData(content).isEmpty());
77+
}
78+
6379
@Test
6480
public void testCreate() {
6581
this.testCypherQueryAndContains("CREATE (n:person { name : 'test', " +

‎hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/CountStrategyCoreTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,43 @@ public void testCountUnknownNumericPredicateKeepsFinalResults() {
484484
customFalse.or(P.eq(0L)), P.not(custom).or(P.eq(0L))));
485485
}
486486

487+
@Test
488+
public void testCountWithoutNegativeValuesKeepsEmptyCount() {
489+
this.initSchema();
490+
commitTx();
491+
assertNegativeWithoutCounts(graph().traversal(), 0L);
492+
}
493+
494+
@Test
495+
public void testCountWithoutNegativeValuesKeepsNonemptyAndNestedCount() {
496+
this.initSchema();
497+
Vertex source = graph().addVertex(T.label, "person", "name", "source");
498+
for (int i = 0; i < 5; i++) {
499+
Vertex target = graph().addVertex(T.label, "person", "name", "target" + i);
500+
source.addEdge("knows", target);
501+
}
502+
commitTx();
503+
GraphTraversalSource g = graph().traversal();
504+
assertNegativeWithoutCounts(g, 6L);
505+
for (P<Long> predicate : List.of(P.without(-1L), P.without(-2L, -1L))) {
506+
Assert.assertEquals(Collections.singletonList(source.id()),
507+
g.V().filter(__.out("knows").count().is(predicate.clone())
508+
.is(P.gt(0L))).id().toList());
509+
}
510+
}
511+
512+
private static void assertNegativeWithoutCounts(GraphTraversalSource g, long count) {
513+
GraphTraversalSource control = g.withoutStrategies(HugeCountStrategy.class);
514+
for (P<Long> predicate : List.of(P.without(-1L), P.without(-2L, -1L))) {
515+
Assert.assertEquals(Collections.singletonList(count),
516+
control.V().count().is(predicate.clone()).toList());
517+
Assert.assertEquals(Collections.singletonList(count),
518+
g.V().count().is(predicate.clone()).toList());
519+
Assert.assertEquals(count > 0L ? Collections.singletonList(count) : Collections.emptyList(),
520+
g.V().count().is(predicate.clone()).is(P.gt(0L)).toList());
521+
}
522+
}
523+
487524
@Test
488525
public void testCountWithoutUsesBoundAboveAllExcludedCounts() {
489526
this.initSchema();

‎hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
import org.apache.hugegraph.unit.id.IdTest;
8585
import org.apache.hugegraph.unit.id.IdUtilTest;
8686
import org.apache.hugegraph.unit.id.SplicingIdGeneratorTest;
87+
import org.apache.hugegraph.unit.opencypher.CypherGremlinPredicatesTest;
8788
import org.apache.hugegraph.unit.rocksdb.RocksDBCountersTest;
8889
import org.apache.hugegraph.unit.rocksdb.RocksDBSessionTest;
8990
import org.apache.hugegraph.unit.rocksdb.RocksDBSessionsTest;
@@ -125,6 +126,7 @@
125126
/* api gremlin */
126127
GremlinQueryAPITest.class,
127128
CypherClientTest.class,
129+
CypherGremlinPredicatesTest.class,
128130
WsAndHttpBasicAuthHandlerTest.class,
129131
GraphSpaceGroupAPITest.class,
130132
GraphSpaceAuthPayloadTest.class,

‎hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/config/GremlinConfigCompatibilityTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@
7575
import org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV1;
7676
import org.apache.tinkerpop.gremlin.util.ser.MessageTextSerializer;
7777
import org.junit.Test;
78+
79+
import groovy.lang.GString;
80+
import groovy.lang.GroovyShell;
7881
import org.mockito.Mockito;
7982
import org.yaml.snakeyaml.Yaml;
8083
import org.yaml.snakeyaml.error.YAMLException;
@@ -540,6 +543,44 @@ public void testDefaultGraphSONSerializerIncludesHugeGraphElementProperties()
540543
Assert.assertContains("0.75", pathJson);
541544
}
542545

546+
@Test
547+
public void testDriverConfigsReturnEvaluatedGroovyStrings() throws Exception {
548+
Object value = new GroovyShell().evaluate("def value = 42; \"r3-value:${value}\"");
549+
Assert.assertInstanceOf(GString.class, value);
550+
Settings.SerializerSettings serverSettings = readGremlinServerSettings().serializers.get(0);
551+
MessageSerializer<?> server = newMessageSerializer(serverSettings.className);
552+
server.configure(config(serverSettings.config), Collections.emptyMap());
553+
List<Object> response = List.of(value, List.of(value), Map.of("value", value));
554+
List<Object> expected = List.of("r3-value:42", List.of("r3-value:42"),
555+
Map.of("value", "r3-value:42"));
556+
for (String config : DRIVER_CONFIGS) {
557+
RemoteSerializerSettings settings = readRemoteSerializerSettings(config);
558+
MessageSerializer<?> client = newMessageSerializer(settings.className);
559+
client.configure(config(settings.config), Collections.emptyMap());
560+
Assert.assertEquals(config, expected,
561+
roundTripBinaryResponse(server, client, response).getResult().getData());
562+
}
563+
Assert.assertEquals(expected,
564+
roundTripBinaryResponse(server, new GraphBinaryMessageSerializerV1(), response)
565+
.getResult().getData());
566+
}
567+
568+
@Test
569+
public void testUntypedGraphsonKeepsEvaluatedGroovyStringBean() throws Exception {
570+
Object value = new GroovyShell().evaluate("def value = 42; \"r3-value:${value}\"");
571+
Settings.SerializerSettings settings = readGremlinServerSettings().serializers.stream()
572+
.filter(s -> s.className.endsWith(
573+
"GraphSONUntypedMessageSerializerV1"))
574+
.findFirst().orElseThrow();
575+
MessageTextSerializer<?> serializer = (MessageTextSerializer<?>) newMessageSerializer(settings.className);
576+
serializer.configure(config(settings.config), Collections.emptyMap());
577+
Object result = roundTripResponse(serializer, value).getResult().getData();
578+
Assert.assertInstanceOf(Map.class, result);
579+
Map<?, ?> bean = (Map<?, ?>) result;
580+
Assert.assertEquals(List.of(42), bean.get("values"));
581+
Assert.assertEquals(List.of("r3-value:", ""), bean.get("strings"));
582+
}
583+
543584
@Test
544585
public void testDriverConfigsReturnOptionalAndFile() throws Exception {
545586
Settings.SerializerSettings serverSettings = readGremlinServerSettings()

0 commit comments

Comments
 (0)