From a5a67ac67990009ed7dcb28d0728edc4a902f5c6 Mon Sep 17 00:00:00 2001 From: cn337131 <141730190+cn337131@users.noreply.github.com> Date: Mon, 21 Oct 2024 11:44:06 +0000 Subject: [PATCH] Gh-3319: Fixes ability to get orphan vertices from edges (#3320) * bug fix, tests and schema fix * checkstyle fixes * typos --------- Co-authored-by: wb36499 <166839644+wb36499@users.noreply.github.com> --- .../gchq/gaffer/tinkerpop/GafferPopGraph.java | 42 +++++++++++++------ .../traversal/util/GafferVertexUtils.java | 28 +++++++------ .../gaffer/tinkerpop/GafferPopGraphIT.java | 42 +++++++++++++------ 3 files changed, 76 insertions(+), 36 deletions(-) diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java index 593cd088205..e416951b825 100755 --- a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java +++ b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java @@ -35,6 +35,7 @@ import org.slf4j.LoggerFactory; import uk.gov.gchq.gaffer.data.element.Element; +import uk.gov.gchq.gaffer.data.element.id.EntityId; import uk.gov.gchq.gaffer.data.elementdefinition.view.View; import uk.gov.gchq.gaffer.graph.Graph; import uk.gov.gchq.gaffer.graph.GraphConfig; @@ -51,6 +52,7 @@ import uk.gov.gchq.gaffer.operation.impl.get.GetAllElements; import uk.gov.gchq.gaffer.operation.impl.get.GetElements; import uk.gov.gchq.gaffer.operation.io.Input; +import uk.gov.gchq.gaffer.store.operation.GetSchema; import uk.gov.gchq.gaffer.store.schema.Schema; import uk.gov.gchq.gaffer.tinkerpop.generator.GafferEdgeGenerator; import uk.gov.gchq.gaffer.tinkerpop.generator.GafferEntityGenerator; @@ -80,6 +82,7 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -540,7 +543,6 @@ public Iterator adjVerticesWithView(final Object vertexId, final Directi * Given an iterable of vertex ids, adjacent vertices will be returned. * If you provide any optional labels then you must provide edge labels and the vertex * labels - any missing labels will cause the elements to be filtered out. - * This method will not return 'id' vertices, only vertices that exist as entities in Gaffer. * * @param vertexIds the iterable of vertex ids to start at. * @param direction the direction along edges to travel @@ -830,16 +832,21 @@ private Iterator adjVerticesWithSeedsAndView(final List see throw new UnsupportedOperationException("There could be a lot of vertices, so please add some seeds"); } - final Iterable result = execute(new OperationChain.Builder() + final Iterable getAdjEntitySeeds = execute(new OperationChain.Builder() .first(new GetAdjacentIds.Builder() - .input(seeds) - .view(view) - .inOutType(getInOutType(direction)) - .build()) - // GetAdjacentIds provides list of entity seeds so run a GetElements to get the actual Entities - .then(new GetElements.Builder() - .view(createAllEntitiesView()) - .build()) + .input(seeds) + .view(view) + .inOutType(getInOutType(direction)) + .build()) + .build()); + + List seedList = StreamSupport.stream(getAdjEntitySeeds.spliterator(), false).collect(Collectors.toList()); + + // GetAdjacentIds provides list of entity seeds so run a GetElements to get the actual Entities + final Iterable result = execute(new OperationChain.Builder() + .first(new GetElements.Builder() + .input(seedList) + .build()) .build()); // Translate results to Gafferpop elements @@ -850,7 +857,13 @@ private Iterator adjVerticesWithSeedsAndView(final List see .map(e -> (Vertex) e) .iterator(); - return translatedResults.iterator(); + // Check for seeds that are not entities but are vertices on an edge (orphan vertices) + Iterable chainedIterable = translatedResults; + for (final EntityId seed : seedList) { + Iterable orphanVertices = GafferVertexUtils.getOrphanVertices(result, this, seed.getVertex()); + chainedIterable = IterableUtils.chainedIterable(chainedIterable, orphanVertices); + } + return chainedIterable.iterator(); } private Iterator edgesWithSeedsAndView(final List seeds, final Direction direction, final View view) { @@ -922,7 +935,12 @@ private View createView(final String... labels) { View view = null; if (null != labels && 0 < labels.length) { final View.Builder viewBuilder = new View.Builder(); - final Schema schema = graph.getSchema(); + final Schema schema = execute(new OperationChain.Builder() + .first(new GetSchema.Builder() + .compact(true) + .build()) + .options(opOptions) + .build()); for (final String label : labels) { if (schema.isEntity(label)) { viewBuilder.entity(label); diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/process/traversal/util/GafferVertexUtils.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/process/traversal/util/GafferVertexUtils.java index bdef1c4295c..be409317801 100644 --- a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/process/traversal/util/GafferVertexUtils.java +++ b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/process/traversal/util/GafferVertexUtils.java @@ -18,20 +18,24 @@ import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import uk.gov.gchq.gaffer.data.element.Edge; import uk.gov.gchq.gaffer.data.element.Element; import uk.gov.gchq.gaffer.data.element.Entity; import uk.gov.gchq.gaffer.tinkerpop.GafferPopGraph; import uk.gov.gchq.gaffer.tinkerpop.GafferPopVertex; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.StreamSupport; public final class GafferVertexUtils { + private static final Logger LOGGER = LoggerFactory.getLogger(GafferVertexUtils.class); private GafferVertexUtils() { // Utility class @@ -58,6 +62,7 @@ public static Iterable getOrphanVertices(final Iterable e.equals(id))) .collect(Collectors.toList()); + orphanVertexIds.forEach(id -> LOGGER.debug("Getting orphan vertices for vertex {}", id)); return (orphanVertexIds.isEmpty()) ? Collections.emptyList() : extractOrphanVerticesFromEdges(result, graph, orphanVertexIds); } @@ -71,19 +76,18 @@ public static Iterable getOrphanVertices(final Iterable extractOrphanVerticesFromEdges(final Iterable result, final GafferPopGraph graph, final List orphanVertexIds) { - return StreamSupport.stream(result.spliterator(), false) + List orphanVertices = new ArrayList<>(); + StreamSupport.stream(result.spliterator(), false) .filter(Edge.class::isInstance) .map(e -> (Edge) e) - .map(e -> { - if (orphanVertexIds.contains(e.getSource())) { - return new GafferPopVertex(GafferPopGraph.ID_LABEL, GafferCustomTypeFactory.parseForGraphSONv3(e.getSource()), graph); - } else if (orphanVertexIds.contains(e.getDestination())) { - return new GafferPopVertex(GafferPopGraph.ID_LABEL, GafferCustomTypeFactory.parseForGraphSONv3(e.getDestination()), graph); - } else { - return null; + .forEach(e -> { + if (orphanVertexIds.contains(e.getSource()) || orphanVertexIds.equals(e.getSource())) { + orphanVertices.add(new GafferPopVertex(GafferPopGraph.ID_LABEL, GafferCustomTypeFactory.parseForGraphSONv3(e.getSource()), graph)); } - }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + if (orphanVertexIds.contains(e.getDestination()) || orphanVertexIds.equals(e.getDestination())) { + orphanVertices.add(new GafferPopVertex(GafferPopGraph.ID_LABEL, GafferCustomTypeFactory.parseForGraphSONv3(e.getDestination()), graph)); + } + }); + return orphanVertices; } } diff --git a/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraphIT.java b/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraphIT.java index 341067f09e1..07701d92a76 100644 --- a/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraphIT.java +++ b/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraphIT.java @@ -54,6 +54,7 @@ */ class GafferPopGraphIT { private static final String TEST_NAME_FORMAT = "({0}) {displayName}"; + private static final String VERTEX_ONLY_ID_STRING = "7"; private static GafferPopGraph mapStore; private static GafferPopGraph accumuloStore; @@ -303,28 +304,45 @@ void shouldAddE(String graph, GraphTraversalSource g) { @MethodSource("provideTraversals") void shouldSeedWithVertexOnlyEdge(String graph, GraphTraversalSource g) { // Edge has a vertex but not an entity in the graph - Gaffer only feature - String vertexOnlyId = "7"; - mapStore.addEdge(new GafferPopEdge("knows", GafferPopModernTestUtils.MARKO.getId(), vertexOnlyId, mapStore)); - accumuloStore.addEdge(new GafferPopEdge("knows", GafferPopModernTestUtils.MARKO.getId(), vertexOnlyId, accumuloStore)); + // [1 - knows -> 7] + mapStore.addEdge(new GafferPopEdge("knows", GafferPopModernTestUtils.MARKO.getId(), VERTEX_ONLY_ID_STRING, mapStore)); + accumuloStore.addEdge(new GafferPopEdge("knows", GafferPopModernTestUtils.MARKO.getId(), VERTEX_ONLY_ID_STRING, accumuloStore)); - List result = g.V("7").toList(); + List result = g.V(VERTEX_ONLY_ID_STRING).toList(); assertThat(result) .extracting(r -> r.id()) - .contains(vertexOnlyId); + .contains(VERTEX_ONLY_ID_STRING); reset(); } @ParameterizedTest(name = TEST_NAME_FORMAT) @MethodSource("provideTraversals") - void shouldTraverseEdgeWithVertexOnlySeed(String graph, GraphTraversalSource g) { - // Edge has a vertex but not an entity in the graph - Gaffer only feature - String vertexOnlyId = "7"; - mapStore.addEdge(new GafferPopEdge("knows", GafferPopModernTestUtils.MARKO.getId(), vertexOnlyId, mapStore)); - accumuloStore.addEdge(new GafferPopEdge("knows", GafferPopModernTestUtils.MARKO.getId(), vertexOnlyId, accumuloStore)); + void shouldTraverseEdgeWithVertexOnlyEdge(String graph, GraphTraversalSource g) { + // Edge has a two vertices with no entities in the graph - Gaffer only feature + // [8 - knows -> 7] + mapStore.addEdge(new GafferPopEdge("knows", "8", VERTEX_ONLY_ID_STRING, mapStore)); + accumuloStore.addEdge(new GafferPopEdge("knows", "8", VERTEX_ONLY_ID_STRING, accumuloStore)); - List> result = g.V("7").inE().outV().elementMap().toList(); + List result = g.V(VERTEX_ONLY_ID_STRING).inE().inV().toList(); assertThat(result) - .containsExactly(MARKO.getPropertyMap()); + .extracting(r -> r.id()) + .contains(VERTEX_ONLY_ID_STRING); + List result2 = g.V(VERTEX_ONLY_ID_STRING).inE().outV().toList(); + assertThat(result2) + .extracting(r -> r.id()) + .contains("8"); + List result3 = g.V("8").outE().inV().toList(); + assertThat(result3) + .extracting(r -> r.id()) + .contains(VERTEX_ONLY_ID_STRING); + List result4 = g.V("8").outE().outV().toList(); + assertThat(result4) + .extracting(r -> r.id()) + .contains("8"); + List resultLabel = g.V("8").out("knows").toList(); + assertThat(resultLabel) + .extracting(r -> r.id()) + .containsOnly(VERTEX_ONLY_ID_STRING); reset(); }