contrueCT commented on code in PR #3167:
URL: https://github.com/apache/hugegraph/pull/3167#discussion_r3841358286
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/ShortestPathTraverser.java:
##########
@@ -184,33 +185,36 @@ public PathSet forward(boolean all) {
while (this.pathResults.hasNextKey()) {
Id source = this.pathResults.nextKey();
- Iterator<Edge> edges = edgesOfVertex(source, this.direction,
- this.labels, degree);
- edges = skipSuperNodeIfNeeded(edges, this.degree,
- this.skipDegree);
+ Iterator<Edge> sourceEdges = edgesOfVertex(
Review Comment:
Fixed in 74e87019. Both multi-label overloads now use one exception-safe
acquisition helper that tracks every opened child iterator. If a later label
query fails, all previously opened iterators are closed; the query failure
remains primary and any close failures are suppressed. The normal
ExtendableIterator close path now also attempts every child even when one close
fails. Added Map and List acquisition-failure tests plus a multi-label wrapper
close-failure test.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/ShortestPathTraverser.java:
##########
@@ -184,33 +185,36 @@ public PathSet forward(boolean all) {
while (this.pathResults.hasNextKey()) {
Id source = this.pathResults.nextKey();
- Iterator<Edge> edges = edgesOfVertex(source, this.direction,
- this.labels, degree);
- edges = skipSuperNodeIfNeeded(edges, this.degree,
- this.skipDegree);
+ Iterator<Edge> sourceEdges = edgesOfVertex(
+ source, this.direction, this.labels, degree);
+ try {
+ Iterator<Edge> edges = skipSuperNodeIfNeeded(
+ sourceEdges, this.degree, this.skipDegree);
- this.vertexCount += 1L;
+ this.vertexCount += 1L;
- while (edges.hasNext()) {
- HugeEdge edge = (HugeEdge) edges.next();
- Id target = edge.id().otherVertexId();
+ while (edges.hasNext()) {
+ HugeEdge edge = (HugeEdge) edges.next();
+ Id target = edge.id().otherVertexId();
- this.edgeResults.addEdge(source, target, edge);
+ this.edgeResults.addEdge(source, target, edge);
- PathSet paths = this.pathResults.findPath(target,
- t ->
!this.superNode(t,
-
this.direction),
- all, false);
+ PathSet paths = this.pathResults.findPath(
+ target,
+ t -> !this.superNode(t, this.direction),
+ all, false);
- if (paths.isEmpty()) {
- continue;
- }
- results.addAll(paths);
- if (!all) {
- return paths;
+ if (paths.isEmpty()) {
+ continue;
+ }
+ results.addAll(paths);
+ if (!all) {
+ return paths;
+ }
}
+ } finally {
Review Comment:
Fixed in 74e87019. The forward, backward, and super-node paths now retain
the traversal/query failure and attach a close failure as suppressed; a close
failure is propagated directly only when there is no primary failure. Added
focused tests for all three primary-plus-close-failure paths and the close-only
wrapper path.
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/traversal/ShortestPathTraverserTest.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * 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.hugegraph.unit.traversal;
+
+import java.util.ArrayDeque;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.Map;
+
+import org.apache.hugegraph.HugeGraph;
+import org.apache.hugegraph.backend.id.EdgeId;
+import org.apache.hugegraph.backend.id.Id;
+import org.apache.hugegraph.backend.id.IdGenerator;
+import org.apache.hugegraph.config.CoreOptions;
+import org.apache.hugegraph.structure.HugeEdge;
+import org.apache.hugegraph.testutil.Assert;
+import org.apache.hugegraph.traversal.algorithm.HugeTraverser.Path;
+import org.apache.hugegraph.traversal.algorithm.ShortestPathTraverser;
+import org.apache.hugegraph.type.define.CollectionType;
+import org.apache.hugegraph.type.define.Directions;
+import org.apache.hugegraph.unit.BaseUnitTest;
+import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class ShortestPathTraverserTest extends BaseUnitTest {
+
+ @Test
+ public void testCloseEdgesWhenPathFoundForward() {
+ Id source = IdGenerator.of(1L);
+ Id target = IdGenerator.of(2L);
+ TrackingIterator edges = edges(edgeTo(target));
+ TestTraverser traverser = new TestTraverser(edges);
+
+ Path path = shortestPath(traverser, source, target, 1, 0L);
+
+ Assert.assertEquals(Arrays.asList(source, target), path.vertices());
+ Assert.assertTrue(edges.closed());
+ }
+
+ @Test
+ public void testCloseEdgesWhenPathFoundBackward() {
+ Id source = IdGenerator.of(1L);
+ Id middle = IdGenerator.of(2L);
+ Id target = IdGenerator.of(3L);
+ TrackingIterator forwardEdges = edges(edgeTo(middle));
+ TrackingIterator backwardEdges = edges(edgeTo(middle));
+ TestTraverser traverser = new TestTraverser(forwardEdges,
+ backwardEdges);
+
+ Path path = shortestPath(traverser, source, target, 2, 0L);
+
+ Assert.assertEquals(Arrays.asList(source, middle, target),
+ path.vertices());
+ Assert.assertTrue(forwardEdges.closed());
+ Assert.assertTrue(backwardEdges.closed());
+ }
+
+ @Test
+ public void testCloseEdgesWhenCheckingSuperNode() {
+ Id source = IdGenerator.of(1L);
+ Id target = IdGenerator.of(2L);
+ TrackingIterator sourceEdges = edges(edgeTo(target));
Review Comment:
Fixed in 74e87019. The coverage now includes the actual skipDegree
threshold, a target that reaches the super-node threshold, the real label
ExtendableIterator plus LimitIterator wrapper chain, allShortestPaths(),
multi-label acquisition failures, close-only failures, and throwing iterators
for forward, backward, and super-node paths. ShortestPathTraverserTest passes
all 13 tests locally; ExtendableIteratorTest passes all 13 tests.
--
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]
