imbajin commented on code in PR #2994:
URL: https://github.com/apache/hugegraph/pull/2994#discussion_r3966286315
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtilOptimizeTest.java:
##########
@@ -316,6 +486,384 @@ public void testIsPositiveLabelContainer() {
P.within(Collections.emptyList()))));
}
+ @Test
+ public void testPageIsConsumedBeforeUnsafeLabel() {
+ HugeGraph graph = Mockito.mock(HugeGraph.class);
+ Traversal.Admin<?, ?> traversal = traversal(
+ __.V().has("~page", "").limit(2).has(T.label,
P.neq("author")), graph);
+ HugeGraphStep<?, ?> graphStep = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(graphStep, traversal);
+ Assert.assertTrue(graphStep.queryInfo().paging());
+ Assert.assertFalse(hasStepExists(traversal, "~page"));
+ Assert.assertTrue(hasStepExists(traversal, T.label.getAccessor()));
+
+ traversal = traversal(
+ __.V().outE().has("~page", "").limit(2).has(T.label,
P.neq("knows")), graph);
+ HugeVertexStep<?> vertexStep = replaceVertexStep(traversal);
+ TraversalUtil.extractHasContainer(vertexStep, traversal);
+ Assert.assertTrue(vertexStep.queryInfo().paging());
+ Assert.assertFalse(hasStepExists(traversal, "~page"));
+ Assert.assertTrue(hasStepExists(traversal, T.label.getAccessor()));
+ }
+
+ @Test
+ public void testLocalSearchWithoutGraphUsesRuntimeAnalyzer() throws
Exception {
+ HugeGraph graph = Mockito.mock(HugeGraph.class);
+
Mockito.when(graph.searchPredicate("(alpha)")).thenReturn("alpha"::equals);
+ HugeGraph other = Mockito.mock(HugeGraph.class);
+
Mockito.when(other.searchPredicate("(alpha)")).thenReturn("different"::equals);
+ for (P<?> search : new P<?>[]{Text.contains("(alpha)"),
+
Text.contains("(alpha)").or(P.eq("beta")).and(P.neq("excluded"))}) {
+ for (GraphTraversal<?, ?> chain : new GraphTraversal<?, ?>[]{
+ __.V().has("body",
search).limit(10).hasLabel(P.neq("author")),
+ __.outE().has("body",
search).limit(10).hasLabel(P.neq("knows"))}) {
+ Traversal.Admin<?, ?> traversal = chain.asAdmin();
+ Step<?, ?> source;
+ if (traversal.getStartStep() instanceof GraphStep) {
+ HugeGraphStep<?, ?> step = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(step, traversal);
+ source = step;
+ } else {
+ HugeVertexStep<?> step = replaceVertexStep(traversal);
+ TraversalUtil.extractHasContainer(step, traversal);
+ source = step;
+ }
+ Assert.assertTrue(((QueryHolder)
source).getHasContainers().isEmpty());
+ HasContainer local = ((HasStep<?>)
source.getNextStep()).getHasContainers().get(0);
+ Assert.assertEquals(search, local.getPredicate());
+ Assert.assertFalse(Text.contains("(alpha)").test("alpha"));
+ local = roundTrip(local);
+ Assert.assertTrue(local.test(searchVertex(graph, "alpha")));
+ Assert.assertFalse(local.test(searchVertex(graph,
"alphabet")));
+ Assert.assertEquals(search instanceof ConnectiveP,
+ local.test(searchVertex(graph, "beta")));
+ Assert.assertFalse(local.test(searchVertex(graph,
"excluded")));
+ HasContainer clone = local.clone();
+ Assert.assertFalse(clone.test(searchVertex(other, "alpha")));
+ Assert.assertTrue(clone.test(searchVertex(other,
"different")));
+ Assert.assertTrue(local.test(searchVertex(graph, "alpha")));
+ }
+ }
+ }
+
+ @Test
+ public void testPositiveLabelPushdownBeforeUnsafeChild() {
+ HugeGraph graph = positiveLabelGraph();
+ for (P<?> label : new P<?>[]{P.eq("person"), P.within("person", "fan"),
+ P.within(Collections.emptyList())}) {
+ Traversal.Admin<?, ?> traversal = traversal(__.V().has(T.label,
label).has("age", 18)
+ .where(__.out().hasLabel(P.neq("software"))), graph);
+ HugeGraphStep<?, ?> source = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(source, traversal);
+ Assert.assertEquals(1, source.getHasContainers().size());
+ Assert.assertEquals(label,
source.getHasContainers().get(0).getPredicate());
+ Assert.assertFalse(hasStepExists(traversal,
T.label.getAccessor()));
+ Assert.assertTrue(hasStepExists(traversal, "age"));
+ TraversalUtil.extractHasContainer(source, traversal);
+ Assert.assertEquals(1, source.getHasContainers().size());
+ }
+ }
+
+ @Test
+ public void testPositiveLabelPushdownDoesNotCrossRange() {
+ Traversal.Admin<?, ?> traversal =
traversal(__.V().hasLabel("person").limit(2)
+ .hasLabel("fan").hasLabel(P.neq("software")),
positiveLabelGraph());
+ HugeGraphStep<?, ?> source = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(source, traversal);
+ Assert.assertEquals(1, source.getHasContainers().size());
+ Assert.assertEquals(P.eq("person"),
source.getHasContainers().get(0).getPredicate());
+ Assert.assertTrue(hasStepExists(traversal, T.label.getAccessor()));
+ }
+
+ @Test
+ public void testPositiveLabelFallbackKeepsUnsupportedCandidatesLocal() {
+ HugeGraph graph = positiveLabelGraph();
+ VertexLabel disabled = new VertexLabel(graph, IdGenerator.of(3L),
"disabled");
+ disabled.enableLabelIndex(false);
+ Mockito.when(graph.vertexLabel("disabled")).thenReturn(disabled);
+ Mockito.when(graph.vertexLabel("missing"))
+ .thenThrow(new IllegalArgumentException("Undefined vertex
label"));
+ for (P<?> label : new P<?>[]{P.eq("disabled"), P.eq("missing"),
+ P.eq(IdGenerator.of(-1L)), P.within("person",
IdGenerator.of(-1L)),
+ P.within("person", "missing"),
P.eq(Collections.singletonList("person"))}) {
+ Traversal.Admin<?, ?> traversal = traversal(__.V().has(T.label,
label)
+ .hasLabel(P.neq("software")), graph);
+ HugeGraphStep<?, ?> source = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(source, traversal);
+ Assert.assertTrue(source.getHasContainers().isEmpty());
+ Assert.assertTrue(hasStepExists(traversal, T.label.getAccessor()));
+ }
+ }
+
+ private static HugeGraph positiveLabelGraph() {
+ HugeGraph graph = Mockito.mock(HugeGraph.class);
+ Mockito.when(graph.vertexLabel("person"))
+ .thenReturn(new VertexLabel(graph, IdGenerator.of(1L),
"person"));
+ Mockito.when(graph.vertexLabel("fan"))
+ .thenReturn(new VertexLabel(graph, IdGenerator.of(2L), "fan"));
+ return graph;
+ }
+
+ @Test
+ public void testLocalSearchPreservesPredicateAndSerialization() throws
Exception {
+ HugeGraph graph = Mockito.mock(HugeGraph.class);
+
Mockito.when(graph.searchPredicate("(alpha)")).thenReturn("alpha"::equals);
+ for (P<?> predicate : new P<?>[]{Text.contains("(alpha)"),
+
Text.contains("(alpha)").or(P.eq("beta")).and(P.neq("excluded"))}) {
+ Traversal.Admin<?, ?> traversal = traversal(
+ __.V().has("body",
predicate).limit(10).hasLabel(P.neq("other")), graph);
+ HugeGraphStep<?, ?> source = replaceGraphStep(traversal);
+ HasContainer original = ((HasStep<?>)
source.getNextStep()).getHasContainers().get(0);
+ TraversalUtil.extractHasContainer(source, traversal);
+ HasContainer local = ((HasStep<?>)
source.getNextStep()).getHasContainers().get(0);
+ Assert.assertEquals(predicate, local.getPredicate());
+ Assert.assertEquals(original.hashCode(), local.hashCode());
+ Assert.assertEquals(original.toString(), local.toString());
+ HasContainer restored = roundTrip(local);
+ Assert.assertTrue(restored.test(searchVertex(graph, "alpha")));
+ Assert.assertEquals(predicate instanceof ConnectiveP,
+ restored.test(searchVertex(graph, "beta")));
+ Assert.assertFalse(restored.test(searchVertex(graph, "excluded")));
+ // A warmed matcher must not serialize its graph/analyzer closure.
+ restored = roundTrip(restored);
+ Assert.assertEquals(predicate, restored.getPredicate());
+ Assert.assertTrue(restored.test(searchVertex(graph, "alpha")));
+ Assert.assertTrue(restored.clone().test(searchVertex(graph,
"alpha")));
+
+ HugeGraph otherGraph = Mockito.mock(HugeGraph.class);
+
Mockito.when(otherGraph.searchPredicate("(alpha)")).thenReturn("different"::equals);
+ Assert.assertFalse(restored.test(searchVertex(otherGraph,
"alpha")));
+ Assert.assertTrue(restored.test(searchVertex(otherGraph,
"different")));
+ Assert.assertTrue(restored.clone().test(searchVertex(graph,
"alpha")));
+ }
+ }
+
+ private static HasContainer roundTrip(HasContainer container) throws
Exception {
+ ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+ try (ObjectOutputStream output = new ObjectOutputStream(bytes)) {
+ output.writeObject(container);
+ }
+ try (ObjectInputStream input = new ObjectInputStream(
+ new ByteArrayInputStream(bytes.toByteArray()))) {
+ return (HasContainer) input.readObject();
+ }
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testLocalSearchMatcherTracksPredicateMutation() {
+ HugeGraph graph = Mockito.mock(HugeGraph.class);
+
Mockito.when(graph.searchPredicate("(alpha)")).thenReturn("alpha"::equals);
+
Mockito.when(graph.searchPredicate("(beta)")).thenReturn("beta"::equals);
+ Traversal.Admin<?, ?> traversal = traversal(
+ __.V().has("body", Text.contains("(alpha)"))
+ .hasLabel(P.neq("other")), graph);
+ HugeGraphStep<?, ?> source = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(source, traversal);
+ HasContainer local = ((HasStep<?>)
source.getNextStep()).getHasContainers().get(0);
+ Assert.assertTrue(local.test(searchVertex(graph, "alpha")));
+ Assert.assertTrue(local.test(searchVertex(graph, "alpha")));
+ Mockito.verify(graph, Mockito.times(1)).searchPredicate("(alpha)");
+ HasContainer clone = local.clone();
+ ((P<Object>) local.getPredicate()).setValue("(beta)");
+ Assert.assertFalse(local.test(searchVertex(graph, "alpha")));
+ Assert.assertTrue(local.test(searchVertex(graph, "beta")));
+ Assert.assertTrue(clone.test(searchVertex(graph, "alpha")));
+ Assert.assertFalse(clone.test(searchVertex(graph, "beta")));
+
+ traversal = traversal(__.V().has("body",
Text.contains("(alpha)").or(P.eq("gamma")))
+ .hasLabel(P.neq("other")), graph);
+ source = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(source, traversal);
+ local = ((HasStep<?>) source.getNextStep()).getHasContainers().get(0);
+ Assert.assertTrue(local.test(searchVertex(graph, "alpha")));
+ ConnectiveP<Object> connective = (ConnectiveP<Object>)
local.getPredicate();
+ connective.getPredicates().get(0).setValue("(beta)");
+ Assert.assertFalse(local.test(searchVertex(graph, "alpha")));
+ Assert.assertTrue(local.test(searchVertex(graph, "beta")));
+ Assert.assertTrue(local.test(searchVertex(graph, "gamma")));
+ connective.or(P.eq("delta"));
+ Assert.assertTrue(local.test(searchVertex(graph, "delta")));
+ }
+
+ @SuppressWarnings("unchecked")
+ private static Vertex searchVertex(HugeGraph graph, String value) {
+ Vertex vertex = Mockito.mock(Vertex.class);
+ VertexProperty<Object> property = Mockito.mock(VertexProperty.class);
+ Mockito.when(vertex.graph()).thenReturn(graph);
+ Mockito.when(vertex.properties("body"))
+ .thenAnswer(ignored ->
Collections.singletonList(property).iterator());
+ Mockito.when(property.element()).thenReturn(vertex);
+ Mockito.when(property.value()).thenReturn(value);
+ return vertex;
+ }
+
+ @Test
+ public void testLocalLabelIdWithoutGraphAndAfterClone() {
+ Id id = IdGenerator.of(12L);
+ Traversal.Admin<?, ?> traversal = __.V().has(T.label, id).limit(10)
+ .hasLabel(P.neq("other")).asAdmin();
+ HugeGraphStep<?, ?> source = replaceGraphStep(traversal);
+ TraversalUtil.extractHasContainer(source, traversal);
+ HasContainer filter = ((HasStep<?>)
source.getNextStep()).getHasContainers().get(0);
+ HugeVertex vertex = Mockito.mock(HugeVertex.class);
+ Mockito.when(vertex.schemaLabel()).thenReturn(new VertexLabel(null,
id, "v"));
+ Assert.assertTrue(filter.test(vertex));
+ Assert.assertTrue(filter.clone().test(vertex));
+ Mockito.when(vertex.schemaLabel()).thenReturn(new VertexLabel(null,
IdGenerator.of(13L), "v"));
+ Assert.assertFalse(filter.test(vertex));
+ Assert.assertFalse(filter.clone().test(vertex));
+ }
+
+ @Test
+ public void testSearchPredicateDefaultCompatibility() throws Exception {
+ Assert.assertTrue(HugeGraph.class.getMethod("searchPredicate",
String.class).isDefault());
+ HugeGraph graph = Mockito.mock(HugeGraph.class,
Mockito.CALLS_REAL_METHODS);
Review Comment:
⚠️ **This added regression test fails with the project's Mockito setup.**
`Mockito.mock(HugeGraph.class, Mockito.CALLS_REAL_METHODS)` throws
`org.mockito.exceptions.base.MockitoException: Cannot call abstract real method
on java object` when `graph.searchPredicate("word")` is invoked, so the
expected `UnsupportedOperationException` is never reached. I reproduced it with
`mvn test -pl hugegraph-server/hugegraph-test -am -P unit-test,memory
-Dsurefire.failIfNoSpecifiedTests=false -Dtest=TraversalUtilOptimizeTest`; 91
tests ran and this one failed. Please invoke the default method through a
testable concrete/proxy fixture or adjust the test so the unit-test profile
passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
