Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gh-3219: Fix inV() and outV() missing results #3221

Merged
merged 12 commits into from
May 29, 2024
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2023 Crown Copyright
* Copyright 2016-2024 Crown Copyright
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,23 +26,26 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import uk.gov.gchq.gaffer.data.element.Element;
import uk.gov.gchq.gaffer.data.elementdefinition.view.View;
import uk.gov.gchq.gaffer.operation.OperationChain;
import uk.gov.gchq.gaffer.operation.data.EntitySeed;
import uk.gov.gchq.gaffer.operation.impl.get.GetElements;
import uk.gov.gchq.gaffer.tinkerpop.generator.GafferPopElementGenerator;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

/**
* A <code>GafferPopEdge</code> is an {@link GafferPopElement} and {@link Edge}.
* <p>
* inVertex() and outVertex() methods are not supported as it is possible for a
* edge to have multiple in vertices and multiple out vertices (due to the mapping
* to a TinkerPop vertex to Gaffer {@link uk.gov.gchq.gaffer.data.element.Entity}.
* Use vertices(Direction) instead.
* </p>
* <p>
* An ID is required to be a List which contains the source, group, and
* destination of an edge.
* </p>
Expand All @@ -54,7 +57,8 @@ public final class GafferPopEdge extends GafferPopElement implements Edge {
private final GafferPopVertex inVertex;
private final GafferPopVertex outVertex;

public GafferPopEdge(final String label, final Object outVertex, final Object inVertex, final GafferPopGraph graph) {
public GafferPopEdge(final String label, final Object outVertex, final Object inVertex,
final GafferPopGraph graph) {
super(label, Arrays.asList(getVertexId(outVertex), getVertexId(inVertex)), graph);
this.outVertex = getValidVertex(outVertex, graph);
this.inVertex = getValidVertex(inVertex, graph);
Expand Down Expand Up @@ -97,7 +101,6 @@ public <V> Iterator<Property<V>> properties(final String... propertyKeys) {
}
}


/**
* Updates the properties attached to this Edge but without modifying the
* underlying graph.
Expand All @@ -107,8 +110,8 @@ public <V> Iterator<Property<V>> properties(final String... propertyKeys) {
* able to create a representative GafferPopEdge but without modifying the
* one stored in the graph.
*
* @param <V> Value type
* @param key The key
* @param <V> Value type
* @param key The key
* @param value The value
* @return The property
*/
Expand All @@ -127,11 +130,11 @@ public <V> Property<V> propertyWithoutUpdate(final String key, final V value) {
public Iterator<Vertex> vertices(final Direction direction) {
switch (direction) {
case OUT:
return IteratorUtils.of(outVertex);
return IteratorUtils.of(outVertex());
case IN:
return IteratorUtils.of(inVertex);
return IteratorUtils.of(inVertex());
default:
return IteratorUtils.of(outVertex, inVertex);
return IteratorUtils.of(outVertex(), inVertex());
}
}

Expand All @@ -153,12 +156,12 @@ public String toString() {

@Override
public Vertex outVertex() {
return outVertex;
return getVertex(outVertex);
}

@Override
public Vertex inVertex() {
return inVertex;
return getVertex(inVertex);
}

/**
Expand All @@ -184,39 +187,49 @@ private static Object getVertexId(final Object vertex) {
* Determines the GafferPopVertex object to connect this GafferPopEdge on.
*
* @param vertex The vertex object or ID
* @param graph The graph
* @param graph The graph
* @return A valid GafferPopVertex based on the supplied object or ID.
*/
private static GafferPopVertex getValidVertex(final Object vertex, final GafferPopGraph graph) {
// Determine if we can cast the vertex object we have been supplied
if (vertex instanceof Vertex) {
return (GafferPopVertex) vertex;
}

// As a fallback assume its the vertex ID object and construct with the ID label.
// As a fallback assume its the vertex ID object and construct with the ID
// label.
return new GafferPopVertex(GafferPopGraph.ID_LABEL, vertex, graph);
}

/**
* Runs a search to determine the correct Entity to use when a vertex ID
* is supplied to an edge. Note that this may slow performance.
*
* @param vertex The vertex object or ID
* @return A valid Vertex based on the supplied object or ID.
*/
private Vertex getVertex(final GafferPopVertex vertex) {
OperationChain<Iterable<? extends Element>> findBasedOnID = new OperationChain.Builder()
.first(new GetElements.Builder()
.input(new EntitySeed(vertex.id()))
tb06904 marked this conversation as resolved.
Show resolved Hide resolved
.view(new View.Builder().allEntities(true).build())
.build())
.build();

Iterable<? extends Element> result = graph().execute(findBasedOnID);

final GafferPopElementGenerator generator = new GafferPopElementGenerator(graph());

Optional<Vertex> foundEntity = StreamSupport.stream(result.spliterator(), false)
.map(generator::_apply)
.filter(Vertex.class::isInstance)
.map(e -> (Vertex) e)
.findFirst();

if (foundEntity.isPresent()) {
return foundEntity.get();
}

/*
* TODO: Review whether a search should be carried out to determine the correct
* Entity to use to construct the GafferPopVertex to add this edge too.
* Currently a default label will be used if a vertex ID is given to this
* method which may result in an incorrect mapping of a GafferPop
* 'label' to a Gaffer 'group'.
*
* A basic example of a search is given below:
*
* OperationChain<Iterable<? extends Element>> findBasedOnID = new OperationChain.Builder()
* .first(new GetElements.Builder().input(new EntitySeed(vertex)).build())
* .build();
*
* Iterable<? extends Element> result = graph.execute(findBasedOnID);
* Object foundEntity = StreamSupport.stream(result.spliterator(), false)
* .filter(Entity.class::isInstance)
* .findFirst()
* .get();
*
* return new GafferPopVertexGenerator(graph)._apply((Element) foundEntity);
*/
return vertex;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2023 Crown Copyright
* Copyright 2017-2024 Crown Copyright
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,6 +23,7 @@
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import uk.gov.gchq.gaffer.commonutil.TestGroups;
import uk.gov.gchq.gaffer.commonutil.TestPropertyNames;
Expand All @@ -49,6 +50,7 @@ public void shouldConstructEdge() {
final GafferPopGraph graph = mock(GafferPopGraph.class);
final GafferPopVertex outVertex = new GafferPopVertex(GafferPopGraph.ID_LABEL, SOURCE, graph);
final GafferPopVertex inVertex = new GafferPopVertex(GafferPopGraph.ID_LABEL, DEST, graph);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

// When
final GafferPopEdge edge = new GafferPopEdge(TestGroups.EDGE, outVertex, inVertex, graph);
Expand All @@ -71,6 +73,7 @@ public void shouldPreserveVertexLabelWhenSuppliedVertexObject() {
final GafferPopGraph graph = mock(GafferPopGraph.class);
final GafferPopVertex outVertex = new GafferPopVertex("label", SOURCE, graph);
final GafferPopVertex inVertex = new GafferPopVertex("label", DEST, graph);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

// When
final GafferPopEdge edge = new GafferPopEdge(TestGroups.EDGE, outVertex, inVertex, graph);
Expand Down Expand Up @@ -172,6 +175,7 @@ public void shouldCreateReadableToString() {
// Given
final GafferPopGraph graph = mock(GafferPopGraph.class);
final GafferPopEdge edge = new GafferPopEdge(TestGroups.EDGE, SOURCE, DEST, graph);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

// Then
assertThat(edge).hasToString(StringFactory.edgeString(edge));
Expand All @@ -183,6 +187,7 @@ public void shouldReturnOutVertex() {
final GafferPopGraph graph = mock(GafferPopGraph.class);
final GafferPopVertex outVertex = new GafferPopVertex(GafferPopGraph.ID_LABEL, SOURCE, graph);
final GafferPopVertex inVertex = new GafferPopVertex(GafferPopGraph.ID_LABEL, DEST, graph);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

// When
final GafferPopEdge edge = new GafferPopEdge(TestGroups.EDGE, outVertex, inVertex, graph);
Expand All @@ -197,6 +202,7 @@ public void shouldReturnInVertex() {
final GafferPopGraph graph = mock(GafferPopGraph.class);
final GafferPopVertex outVertex = new GafferPopVertex(GafferPopGraph.ID_LABEL, SOURCE, graph);
final GafferPopVertex inVertex = new GafferPopVertex(GafferPopGraph.ID_LABEL, DEST, graph);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

// When
final GafferPopEdge edge = new GafferPopEdge(TestGroups.EDGE, outVertex, inVertex, graph);
Expand All @@ -213,6 +219,7 @@ public void shouldCreateNewGafferPopVertexWithVertexId() {
final GafferPopVertex inVertex = mock(GafferPopVertex.class);
when(inVertex.id()).thenReturn("inVertextId");
when(outVertex.id()).thenReturn("outVertextId");
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

// When
final GafferPopEdge edge = new GafferPopEdge(TestGroups.EDGE, outVertex, inVertex, graph);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,29 @@ void shouldGroupVerticesByLabelAndProvideCount() {
.hasFieldOrPropertyWithValue(SOFTWARE_GROUP, 2L);
}

@Test
void shouldGetAllVerticesConnectedToOutGoingEdgeOfGivenVertex() {
// Given
GraphTraversalSource g = gafferPopGraph.traversal();

// When
List<Object> result = g.V(VERTEX_PERSON_1).outE().inV().values("name").toList();

assertThat(result).containsExactly(EXPECTED_PERSON_2_VERTEX_MAP.get("name"),
EXPECTED_SOFTWARE_1_VERTEX_MAP.get("name"));
}

@Test
void shouldGetPropertiesOfIncomingVerticesForSpecificVertex() {
// Given
GraphTraversalSource g = gafferPopGraph.traversal();

// When
List<Map<Object, Object>> result = g.V(VERTEX_PERSON_2).inE().outV().elementMap().toList();

assertThat(result).containsAnyOf(EXPECTED_PERSON_1_VERTEX_MAP);
}

@Test
void shouldGetEdgesById() {
// Given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@
import org.apache.tinkerpop.gremlin.structure.VertexProperty.Cardinality;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import uk.gov.gchq.gaffer.commonutil.TestGroups;
import uk.gov.gchq.gaffer.commonutil.TestPropertyNames;
import uk.gov.gchq.gaffer.data.elementdefinition.view.View;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Iterator;
Expand All @@ -40,6 +42,7 @@
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class GafferPopVertexTest {
// Common mocks for tests
Expand Down Expand Up @@ -247,6 +250,7 @@ void shouldDelegateEdgesToGraph() {
final GafferPopGraph graph = mock(GafferPopGraph.class);
final GafferPopVertex vertex = new GafferPopVertex(TestGroups.ENTITY, GafferPopGraph.ID_LABEL, graph);
final Iterable<Edge> resultEdges = Arrays.asList(((Edge) new GafferPopEdge(GafferPopGraph.ID_LABEL, vertex, vertex, graph)));
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());
given(graph.edgesWithView(GafferPopGraph.ID_LABEL, Direction.IN, new View.Builder()
.edges(Collections.singletonList(TestGroups.ENTITY))
.build()))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2023 Crown Copyright
* Copyright 2017-2024 Crown Copyright
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@
package uk.gov.gchq.gaffer.tinkerpop.generator;

import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import uk.gov.gchq.gaffer.commonutil.TestGroups;
import uk.gov.gchq.gaffer.commonutil.TestPropertyNames;
Expand All @@ -26,6 +27,9 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.util.ArrayList;

public class GafferEdgeGeneratorTest {
@Test
Expand All @@ -38,6 +42,7 @@ public void shouldConvertGafferPopEdgeToGafferEdge() {
final String propValue = "property value";
final GafferPopEdge gafferPopEdge = new GafferPopEdge(TestGroups.EDGE, source, dest, graph);
gafferPopEdge.property(TestPropertyNames.STRING, propValue);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

final GafferEdgeGenerator generator = new GafferEdgeGenerator();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2023 Crown Copyright
* Copyright 2017-2024 Crown Copyright
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@
package uk.gov.gchq.gaffer.tinkerpop.generator;

import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import uk.gov.gchq.gaffer.commonutil.TestGroups;
import uk.gov.gchq.gaffer.commonutil.TestPropertyNames;
Expand All @@ -29,12 +30,16 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.util.ArrayList;

public class GafferPopEdgeGeneratorTest {
@Test
public void shouldConvertGafferEdgeToGafferPopReadOnlyEdge() {
// Given
final GafferPopGraph graph = mock(GafferPopGraph.class);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

final String source = "source";
final String dest = "dest";
Expand Down Expand Up @@ -62,6 +67,7 @@ public void shouldConvertGafferEdgeToGafferPopReadOnlyEdge() {
public void shouldConvertGafferEdgeToGafferPopReadWriteEdge() {
// Given
final GafferPopGraph graph = mock(GafferPopGraph.class);
when(graph.execute(Mockito.any())).thenReturn(new ArrayList<>());

final String source = "source";
final String dest = "dest";
Expand Down