From c7440c3d012bedbc262d6351069bafc34a54a3d5 Mon Sep 17 00:00:00 2001 From: p29876 <165825455+p29876@users.noreply.github.com> Date: Tue, 28 Jan 2025 18:11:13 +0000 Subject: [PATCH 1/4] gh-3359 Reduce number of vertex lookups for edges Stop inV/outV performing a full vertex lookup Add new methods for doing this when we want to traverse an Edge --- .../gchq/gaffer/tinkerpop/GafferPopEdge.java | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java index a70792f6062..50f65f048e1 100644 --- a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java +++ b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java @@ -132,11 +132,11 @@ public Property propertyWithoutUpdate(final String key, final V value) { public Iterator vertices(final Direction direction) { switch (direction) { case OUT: - return IteratorUtils.of(outVertex()); + return IteratorUtils.of(getCompleteOutVertex()); case IN: - return IteratorUtils.of(inVertex()); + return IteratorUtils.of(getCompleteInVertex()); default: - return IteratorUtils.of(outVertex(), inVertex()); + return IteratorUtils.of(getCompleteOutVertex(), getCompleteInVertex()); } } @@ -156,13 +156,32 @@ public String toString() { return StringFactory.edgeString(this); } + @Override public Vertex outVertex() { + return outVertex; + } + + /** + * Performs a lookup for the outgoing vertex for this edge. + * The returned vertex will include all properties but incurs lookup cost + * @return The complete outgoing vertex + */ + public Vertex getCompleteOutVertex() { return getVertex(outVertex); } @Override public Vertex inVertex() { + return inVertex; + } + + /** + * Perform a full lookup for the incoming vertex for this edge. + * The returned vertex will include all properties but incurs lookup cost + * @return The complete incoming vertex + */ + public Vertex getCompleteInVertex() { return getVertex(inVertex); } From aa1610c8a5491339f09e5ba0fe0bf7d87eed5355 Mon Sep 17 00:00:00 2001 From: p29876 <165825455+p29876@users.noreply.github.com> Date: Tue, 28 Jan 2025 19:07:12 +0000 Subject: [PATCH 2/4] headers --- .../main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java index 50f65f048e1..64dc7b03221 100644 --- a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java +++ b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 Crown Copyright + * Copyright 2016-2025 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From b12fc2160adb920abf4e66f3881bd8464072e039 Mon Sep 17 00:00:00 2001 From: p29876 <165825455+p29876@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:06:15 +0000 Subject: [PATCH 3/4] tidy and add tests --- .../gchq/gaffer/tinkerpop/GafferPopEdge.java | 36 +++++++++---------- .../gaffer/tinkerpop/GafferPopGraphIT.java | 33 +++++++++++++++++ 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java index 64dc7b03221..83d50e9df7d 100644 --- a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java +++ b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java @@ -128,15 +128,17 @@ public Property propertyWithoutUpdate(final String key, final V value) { return newProperty; } + // This is the method TinkerPop uses when traversing edges + // lookup the 'full' vertices rather than returning the 'dummy' ID vertices @Override public Iterator vertices(final Direction direction) { switch (direction) { case OUT: - return IteratorUtils.of(getCompleteOutVertex()); + return IteratorUtils.of(lookupVertex(outVertex)); case IN: - return IteratorUtils.of(getCompleteInVertex()); + return IteratorUtils.of(lookupVertex(inVertex)); default: - return IteratorUtils.of(getCompleteOutVertex(), getCompleteInVertex()); + return IteratorUtils.of(lookupVertex(outVertex), lookupVertex(inVertex)); } } @@ -157,34 +159,28 @@ public String toString() { } + /** + * Gets the outgoing vertex of the edge. + * + * Note: the returned vertex will not have any properties set - only the ID + * if you need the 'full' vertex use {@link #vertices(Direction.OUT)} + */ @Override public Vertex outVertex() { return outVertex; } /** - * Performs a lookup for the outgoing vertex for this edge. - * The returned vertex will include all properties but incurs lookup cost - * @return The complete outgoing vertex + * Gets the incoming vertex of the edge. + * + * Note: the returned vertex will not have any properties set - only the ID + * if you need the 'full' vertex use {@link #vertices(Direction.IN)} */ - public Vertex getCompleteOutVertex() { - return getVertex(outVertex); - } - @Override public Vertex inVertex() { return inVertex; } - /** - * Perform a full lookup for the incoming vertex for this edge. - * The returned vertex will include all properties but incurs lookup cost - * @return The complete incoming vertex - */ - public Vertex getCompleteInVertex() { - return getVertex(inVertex); - } - /** * Gets the vertex ID object from the supplied vertex. * @@ -228,7 +224,7 @@ private static GafferPopVertex getValidVertex(final Object vertex, final GafferP * @param vertex The vertex object or ID * @return A valid Vertex based on the supplied object or ID. */ - private Vertex getVertex(final GafferPopVertex vertex) { + private Vertex lookupVertex(final GafferPopVertex vertex) { OperationChain> findBasedOnID = new OperationChain.Builder() .first(new GetElements.Builder() .input(new EntitySeed(vertex.id())) 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 91fb1225863..39fa2858ae7 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 @@ -37,6 +37,7 @@ import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; +import static uk.gov.gchq.gaffer.tinkerpop.util.modern.GafferPopModernTestUtils.AGE; import static uk.gov.gchq.gaffer.tinkerpop.util.modern.GafferPopModernTestUtils.CREATED; import static uk.gov.gchq.gaffer.tinkerpop.util.modern.GafferPopModernTestUtils.JOSH; import static uk.gov.gchq.gaffer.tinkerpop.util.modern.GafferPopModernTestUtils.KNOWS; @@ -277,6 +278,38 @@ void shouldGetEdgeById(String graph, GraphTraversalSource g) { }); } + @ParameterizedTest(name = TEST_NAME_FORMAT) + @MethodSource("provideTraversals") + void shouldLookupInVertex(String graph, GraphTraversalSource g) { + final List result = g.E("[1,knows,2]").inV().toList(); + + assertThat(result) + .hasSize(1); + + // Check that properties are set on the returned vertex + // i.e. a vertex lookup has been performed + Vertex vadas = result.get(0); + assertThat(vadas.id()).isEqualTo(VADAS.getId()); + assertThat(vadas.property(NAME).value()).isEqualTo(VADAS.getName()); + assertThat(vadas.property(AGE).value()).isEqualTo(VADAS.getAge()); + } + + @ParameterizedTest(name = TEST_NAME_FORMAT) + @MethodSource("provideTraversals") + void shouldLookupOutVertex(String graph, GraphTraversalSource g) { + final List result = g.E("[1,knows,2]").outV().toList(); + + assertThat(result) + .hasSize(1); + + // Check that properties are set on the returned vertex + // i.e. a vertex lookup has been performed + Vertex marko = result.get(0); + assertThat(marko.id()).isEqualTo(MARKO.getId()); + assertThat(marko.property(NAME).value()).isEqualTo(MARKO.getName()); + assertThat(marko.property(AGE).value()).isEqualTo(MARKO.getAge()); + } + @ParameterizedTest(name = TEST_NAME_FORMAT) @MethodSource("provideTraversals") void shouldAddV(String graph, GraphTraversalSource g) { From a4055ab3bec441b5b82961535d732ccc81d84739 Mon Sep 17 00:00:00 2001 From: p29876 <165825455+p29876@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:30:00 +0000 Subject: [PATCH 4/4] javadoc --- .../main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java index 83d50e9df7d..7e1b66979f3 100644 --- a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java +++ b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopEdge.java @@ -163,7 +163,7 @@ public String toString() { * Gets the outgoing vertex of the edge. * * Note: the returned vertex will not have any properties set - only the ID - * if you need the 'full' vertex use {@link #vertices(Direction.OUT)} + * if you need the 'full' vertex use {@link #vertices(Direction)} */ @Override public Vertex outVertex() { @@ -174,7 +174,7 @@ public Vertex outVertex() { * Gets the incoming vertex of the edge. * * Note: the returned vertex will not have any properties set - only the ID - * if you need the 'full' vertex use {@link #vertices(Direction.IN)} + * if you need the 'full' vertex use {@link #vertices(Direction)} */ @Override public Vertex inVertex() {