Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/develop' into gh-3229-tinkerpop-…
Browse files Browse the repository at this point in the history
…predicate-serialisation-bug
  • Loading branch information
tb06904 committed Jun 6, 2024
2 parents 6f607c6 + 8828a0e commit 7f42df4
Show file tree
Hide file tree
Showing 5 changed files with 216 additions and 32 deletions.
8 changes: 8 additions & 0 deletions library/tinkerpop/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
<tinkerpop.version>3.7.1</tinkerpop.version>
<google-guice.version>4.2.3</google-guice.version>
<cucumber.version>7.15.0</cucumber.version>
<!-- Note later versions require scala version 2.12.x -->
<cypher-gremlin.version>1.0.0</cypher-gremlin.version>

<!-- Don't run any of the standard tinkerpop tests by default-->
<test>!GafferPopGraphStructureStandardTest,!GafferPopFeatureTest,!GafferPopGraphProcessStandardTest</test>
Expand All @@ -53,6 +55,12 @@
<artifactId>gremlin-server</artifactId>
<version>${tinkerpop.version}</version>
</dependency>
<!-- Cypher translator -->
<dependency>
<groupId>org.opencypher.gremlin</groupId>
<artifactId>translation</artifactId>
<version>${cypher-gremlin.version}</version>
</dependency>

<!-- Runtime dependencies -->
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
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.operation.io.Output;
import uk.gov.gchq.gaffer.store.schema.Schema;
import uk.gov.gchq.gaffer.tinkerpop.generator.GafferEdgeGenerator;
import uk.gov.gchq.gaffer.tinkerpop.generator.GafferEntityGenerator;
Expand Down Expand Up @@ -245,6 +244,7 @@ public enum DefaultIdManager {
private final ServiceRegistry serviceRegistry;

private static final Logger LOGGER = LoggerFactory.getLogger(GafferPopGraph.class);
private static final String GET_ALL_DEBUG_MSG = "Requested a GetAllElements, results will be truncated to: {}.";

public GafferPopGraph(final Configuration configuration) {
this(configuration, createGraph(configuration));
Expand Down Expand Up @@ -393,19 +393,17 @@ public void addEdge(final GafferPopEdge edge) {
@Override
public Iterator<Vertex> vertices(final Object... vertexIds) {
final boolean getAll = null == vertexIds || 0 == vertexIds.length;
final Integer getAllElementsLimit = variables.getAllElementsLimit();

final OperationChain<Iterable<? extends Element>> getOperation;

if (getAll) {
LOGGER.debug("Requested a GetAllElements, results will be truncated to: {}.", getAllElementsLimit);
LOGGER.debug(GET_ALL_DEBUG_MSG, variables.getAllElementsLimit());
getOperation = new Builder()
.first(new GetAllElements.Builder()
.view(new View.Builder()
.entities(graph.getSchema().getEntityGroups())
.build())
.build())
.then(new Limit<Element>(getAllElementsLimit, true))
.then(new Limit<Element>(variables.getAllElementsLimit(), true))
.build();
} else {
getOperation = new Builder()
Expand All @@ -428,10 +426,10 @@ public Iterator<Vertex> vertices(final Object... vertexIds) {
.map(e -> (Vertex) e)
.iterator();

if (getAll && IterableUtils.size(translatedResults) == getAllElementsLimit) {
if (getAll && IterableUtils.size(translatedResults) == variables.getAllElementsLimit()) {
LOGGER.warn(
"Result size is equal to configured limit ({}). Results may have been truncated",
getAllElementsLimit);
variables.getAllElementsLimit());
}
return translatedResults.iterator();
}
Expand Down Expand Up @@ -576,18 +574,17 @@ public Iterator<Vertex> adjVerticesWithView(final Iterable<Object> vertexIds, fi
@Override
public Iterator<Edge> edges(final Object... elementIds) {
final boolean getAll = null == elementIds || 0 == elementIds.length;
final Integer getAllElementsLimit = variables.getAllElementsLimit();

final OperationChain<Iterable<? extends Element>> getOperation;

if (getAll) {
LOGGER.debug("Requested a GetAllElements, results will be truncated to: {}.", getAllElementsLimit);
LOGGER.debug(GET_ALL_DEBUG_MSG, variables.getAllElementsLimit());
getOperation = new Builder()
.first(new GetAllElements.Builder()
.view(new View.Builder()
.edges(graph.getSchema().getEdgeGroups())
.build())
.build())
.then(new Limit<>(getAllElementsLimit, true))
.then(new Limit<>(variables.getAllElementsLimit(), true))
.build();
} else {
getOperation = new Builder()
Expand All @@ -612,10 +609,10 @@ public Iterator<Edge> edges(final Object... elementIds) {
.map(e -> (Edge) e)
.iterator();

if (getAll && IterableUtils.size(translatedResults) == getAllElementsLimit) {
if (getAll && IterableUtils.size(translatedResults) == variables.getAllElementsLimit()) {
LOGGER.warn(
"Result size is equal to configured limit ({}). Results may have been truncated",
getAllElementsLimit);
variables.getAllElementsLimit());
}
return translatedResults.iterator();
}
Expand Down Expand Up @@ -789,15 +786,21 @@ private Iterator<GafferPopVertex> verticesWithSeedsAndView(final List<ElementSee
.build();
}

final Output<Iterable<? extends Element>> getOperation;
final OperationChain<Iterable<? extends Element>> getOperation;
if (getAll) {
getOperation = new GetAllElements.Builder()
.view(entitiesView)
LOGGER.debug(GET_ALL_DEBUG_MSG, variables.getAllElementsLimit());
getOperation = new Builder()
.first(new GetAllElements.Builder()
.view(entitiesView)
.build())
.then(new Limit<>(variables.getAllElementsLimit(), true))
.build();
} else {
getOperation = new GetElements.Builder()
.input(seeds)
.view(entitiesView)
getOperation = new Builder()
.first(new GetElements.Builder()
.input(seeds)
.view(entitiesView)
.build())
.build();

if (null == entitiesView || entitiesView.getEntityGroups().contains(ID_LABEL)) {
Expand All @@ -810,9 +813,7 @@ private Iterator<GafferPopVertex> verticesWithSeedsAndView(final List<ElementSee
}

// Run operation on graph
final Iterable<? extends Element> result = execute(new OperationChain.Builder()
.first(getOperation)
.build());
final Iterable<? extends Element> result = execute(getOperation);

// Translate results to Gafferpop elements
final GafferPopElementGenerator generator = new GafferPopElementGenerator(this);
Expand Down Expand Up @@ -868,23 +869,27 @@ private Iterator<Edge> edgesWithSeedsAndView(final List<ElementSeed> seeds, fina
.build();
}

final Output<Iterable<? extends Element>> getOperation;
final OperationChain<Iterable<? extends Element>> getOperation;
if (getAll) {
getOperation = new GetAllElements.Builder()
.view(edgesView)
LOGGER.debug(GET_ALL_DEBUG_MSG, variables.getAllElementsLimit());
getOperation = new Builder()
.first(new GetAllElements.Builder()
.view(edgesView)
.build())
.then(new Limit<>(variables.getAllElementsLimit(), true))
.build();
} else {
getOperation = new GetElements.Builder()
.input(seeds)
.view(edgesView)
.inOutType(getInOutType(direction))
getOperation = new Builder()
.first(new GetElements.Builder()
.input(seeds)
.view(edgesView)
.inOutType(getInOutType(direction))
.build())
.build();
}

// Run requested chain on the graph
final Iterable<? extends Element> result = execute(new Builder()
.first(getOperation)
.build());
final Iterable<? extends Element> result = execute(getOperation);

// Translate results to Gafferpop elements
final GafferPopElementGenerator generator = new GafferPopElementGenerator(this, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,56 @@
import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.NoOpBarrierStep;
import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
import org.opencypher.gremlin.translation.CypherAst;
import org.opencypher.gremlin.translation.translator.Translator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import uk.gov.gchq.gaffer.tinkerpop.process.traversal.step.GafferPopGraphStep;

import java.util.Optional;

/**
* The {@link GraphStep} strategy for GafferPop, this will replace the default
* {@link GraphStep} of a query to add Gaffer optimisations. Such as gathering
* any {@link HasStep}s so that a Gaffer View can be constructed for the query.
* Will also handle the translation any Cypher queries passed in a with() step
* into a Gremlin traversal.
*
* <pre>
* g.V().hasLabel() // replaced by GafferPopGraphStep
* g.E().hasLabel() // replaced by GafferPopGraphStep
* g.with("cypher", "query") // translated to Gremlin traversal
* </pre>
*/
public final class GafferPopGraphStepStrategy extends AbstractTraversalStrategy<TraversalStrategy.ProviderOptimizationStrategy> implements TraversalStrategy.ProviderOptimizationStrategy {
private static final Logger LOGGER = LoggerFactory.getLogger(GafferPopGraphStepStrategy.class);
private static final GafferPopGraphStepStrategy INSTANCE = new GafferPopGraphStepStrategy();

/**
* Key used in a with step to include a opencypher query traversal
*/
public static final String CYPHER_KEY = "cypher";

private GafferPopGraphStepStrategy() {
}

@Override
public void apply(final Admin<?, ?> traversal) {
// Check for any options on the traversal
Optional<OptionsStrategy> optionsStrategy = traversal.getStrategies().getStrategy(OptionsStrategy.class);
// Translate and add a cypher traversal in if that key has been set
if (optionsStrategy.isPresent() && optionsStrategy.get().getOptions().containsKey(CYPHER_KEY)) {
LOGGER.info("Replacing traversal with translated Cypher query");
CypherAst ast = CypherAst.parse((String) optionsStrategy.get().getOptions().get(CYPHER_KEY));
Admin<?, ?> translatedCypher = ast.buildTranslation(Translator.builder().traversal().build()).asAdmin();

// Add the cypher traversal
TraversalHelper.insertTraversal(0, translatedCypher, traversal);
LOGGER.debug("New traversal is: {}", traversal);
}

TraversalHelper.getStepsOfClass(GraphStep.class, traversal).forEach(originalGraphStep -> {
// Replace the current GraphStep with a GafferPopGraphStep
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/*
* Copyright 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.
* 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 uk.gov.gchq.gaffer.tinkerpop.process.traversal.strategy;

import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.opencypher.v9_0.util.SyntaxException;

import uk.gov.gchq.gaffer.mapstore.MapStoreProperties;
import uk.gov.gchq.gaffer.tinkerpop.GafferPopGraph;
import uk.gov.gchq.gaffer.tinkerpop.process.traversal.step.GafferPopHasStepIT;
import uk.gov.gchq.gaffer.tinkerpop.util.GafferPopModernTestUtils;

import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static uk.gov.gchq.gaffer.tinkerpop.process.traversal.strategy.optimisation.GafferPopGraphStepStrategy.CYPHER_KEY;
import static uk.gov.gchq.gaffer.tinkerpop.util.GafferPopModernTestUtils.JOSH;
import static uk.gov.gchq.gaffer.tinkerpop.util.GafferPopModernTestUtils.MARKO;
import static uk.gov.gchq.gaffer.tinkerpop.util.GafferPopModernTestUtils.MODERN_CONFIGURATION;
import static uk.gov.gchq.gaffer.tinkerpop.util.GafferPopModernTestUtils.PETER;
import static uk.gov.gchq.gaffer.tinkerpop.util.GafferPopModernTestUtils.VADAS;

public class GafferPopCypherIT {

private static final MapStoreProperties MAP_STORE_PROPERTIES = MapStoreProperties.loadStoreProperties("/tinkerpop/map-store.properties");
private static GraphTraversalSource g;

@BeforeAll
public static void beforeAll() {
GafferPopGraph gafferPopGraph = GafferPopModernTestUtils.createModernGraph(GafferPopHasStepIT.class, MAP_STORE_PROPERTIES, MODERN_CONFIGURATION);
g = gafferPopGraph.traversal();
}

@Test
void shouldTranslateCypherWithSeededID() {
// Given
final String cypherQuery = "MATCH (p:person) WHERE ID(p) = '1' RETURN p";
// When
// Check we can do a seeded query with ID 1
Map<Object, Object> results = ((LinkedHashMap<Object, Object>) g
.with(CYPHER_KEY, cypherQuery)
.call()
.next());
// Make sure only one result
assertThat(results).size().isOne();

// The cypher translator will return a property map of the matched node so get that
Map<Object, Object> resultMap = (LinkedHashMap<Object, Object>) results.get("p");

assertThat(resultMap).containsAllEntriesOf(MARKO.getPropertyMap());
}

@Test
void shouldTranslateCypherWithPredicate() {
// Given
// Get names of all people older than 30
final String cypherQuery = "MATCH (p:person) WHERE p.age > 30 RETURN p.name";

// When
List<Object> results = g
.with(CYPHER_KEY, cypherQuery)
.call()
.toList();
// Flatten the results as they will be like [{p.name=peter}, {p.name=josh} ...]
List<Object> flattenedResults = results.stream()
.flatMap(result ->((LinkedHashMap<Object, Object>) result).values().stream())
.collect(Collectors.toList());

// Then
assertThat(flattenedResults).containsExactlyInAnyOrder(PETER.getName(), JOSH.getName());
}

@Test
void shouldTranslateCypherEdgeTraversal() {
// Given
// Finds all the people marko 'knows'
final String cypherQuery = "MATCH (marko:person {name: 'marko'})-[:knows]->(p) RETURN p.name";

// When
List<Object> results = g
.with(CYPHER_KEY, cypherQuery)
.call()
.toList();
// Flatten the results as they will be like [{p.name=peter}, {p.name=josh} ...]
List<Object> flattenedResults = results.stream()
.flatMap(result -> ((LinkedHashMap<Object, Object>) result).values().stream())
.collect(Collectors.toList());

// Then
assertThat(flattenedResults).containsExactlyInAnyOrder(JOSH.getName(), VADAS.getName());
}

@Test
void shouldRejectInvalidCypher() {
// Given
// Bad query that's missing the WHERE clause
final String malformedQuery = "MATCH (p:person) WHERE RETURN p";

// When
GraphTraversal traversal = g
.with(CYPHER_KEY, malformedQuery)
.call();

// Then
assertThatExceptionOfType(SyntaxException.class)
.isThrownBy(() -> traversal.toList());
}

}
Loading

0 comments on commit 7f42df4

Please sign in to comment.