Skip to content

Commit

Permalink
gchqgh-1992 FedGraph runs operations against graphs that cant merge.
Browse files Browse the repository at this point in the history
  • Loading branch information
GCHQDev404 committed Dec 18, 2018
1 parent 6a96927 commit 02aaa3b
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
import uk.gov.gchq.gaffer.store.schema.Schema;
import uk.gov.gchq.gaffer.store.schema.ViewValidator;
import uk.gov.gchq.gaffer.user.User;
import uk.gov.gchq.koryphe.ValidationResult;

import java.util.Arrays;
import java.util.Collection;

import static java.util.Objects.nonNull;

/**
* Validation class for validating {@link uk.gov.gchq.gaffer.operation.OperationChain}s against {@link ViewValidator}s using the Federated Store schemas.
Expand All @@ -37,4 +43,40 @@ public FederatedOperationChainValidator(final ViewValidator viewValidator) {
protected Schema getSchema(final Operation operation, final User user, final Store store) {
return ((FederatedStore) store).getSchema(operation, user);
}

@Override
protected void validateViews(final Operation op, final User user, final Store store, final ValidationResult validationResult) {
validateAllGraphsIdViews(op, user, store, validationResult, getGraphIds(op, user, (FederatedStore) store));
}

private void validateAllGraphsIdViews(final Operation op, final User user, final Store store, final ValidationResult validationResult, final Collection<String> graphIds) {
ValidationResult errors = new ValidationResult();
ValidationResult current = errors;

for (final String graphId : graphIds) {
current = new ValidationResult();
final Operation clone = op.shallowClone();
clone.addOption(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, graphId);
super.validateViews(clone, user, store, current);
if (current.isValid()) {
break;
} else {
errors.add(current);
}
}

if (!current.isValid()) {
validationResult.add(errors);
}
}

private Collection<String> getGraphIds(final Operation op, final User user, final FederatedStore store) {
return nonNull(op) && nonNull(getGraphIds(op)) && !getGraphIds(op).isEmpty()
? Arrays.asList(getGraphIds(op).split(","))
: store.getAllGraphIds(user);
}

private String getGraphIds(final Operation op) {
return op.getOption(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.common.collect.Lists;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

import uk.gov.gchq.gaffer.accumulostore.AccumuloProperties;
Expand Down Expand Up @@ -158,6 +159,7 @@ public void shouldGetCorrectDefaultViewForAChosenGraphOperation() throws
assertFalse(a.iterator().hasNext());
}

@Ignore(value = "see FederatedStoreTest#shouldRunOperationRegardlessOfSchemaFailure()")
@Test
public void shouldThrowWhenSelectedGraphsSchemaClash() throws Exception {
final HashMapGraphLibrary library = new HashMapGraphLibrary();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
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.data.elementdefinition.exception.SchemaException;
import uk.gov.gchq.gaffer.data.elementdefinition.view.View;
import uk.gov.gchq.gaffer.data.util.ElementUtil;
import uk.gov.gchq.gaffer.federatedstore.operation.AddGraph;
import uk.gov.gchq.gaffer.federatedstore.operation.GetAllGraphIds;
import uk.gov.gchq.gaffer.federatedstore.operation.RemoveGraph;
Expand All @@ -57,6 +59,7 @@
import uk.gov.gchq.gaffer.store.StoreTrait;
import uk.gov.gchq.gaffer.store.library.GraphLibrary;
import uk.gov.gchq.gaffer.store.library.HashMapGraphLibrary;
import uk.gov.gchq.gaffer.store.operation.GetSchema;
import uk.gov.gchq.gaffer.store.operation.GetTraits;
import uk.gov.gchq.gaffer.store.schema.Schema;
import uk.gov.gchq.gaffer.store.schema.Schema.Builder;
Expand All @@ -70,6 +73,8 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -108,6 +113,8 @@ public class FederatedStoreTest {
private static final String PATH_ACC_STORE_PROPERTIES_2 = "properties/singleUseMockAccStore.properties";
private static final String PATH_ACC_STORE_PROPERTIES_ALT = "properties/singleUseMockAccStoreAlt.properties";
private static final String PATH_BASIC_ENTITY_SCHEMA_JSON = "schema/basicEntitySchema.json";
private static final String PATH_ENTITY_A_SCHEMA_JSON = "schema/entityASchema.json";
private static final String PATH_ENTITY_B_SCHEMA_JSON = "schema/entityBSchema.json";
private static final String PATH_BASIC_EDGE_SCHEMA_JSON = "schema/basicEdgeSchema.json";
private static final String EXCEPTION_NOT_THROWN = "exception not thrown";
public static final String UNUSUAL_KEY = "unusualKey";
Expand Down Expand Up @@ -1243,4 +1250,86 @@ private void clearCache() {
private void clearLibrary() {
HashMapGraphLibrary.clear();
}

@Test
public void shouldRunOpAgainstGraphsThatCantMerge() throws OperationException {
//given
final Entity A = new Entity.Builder()
.group("entityA")
.vertex("A")
.build();

final Entity B = new Entity.Builder()
.group("entityB")
.vertex(7)
.build();

final ArrayList<Entity> expectedAB = Lists.newArrayList(A, B);
final ArrayList<Entity> expectedA = Lists.newArrayList(A);
final ArrayList<Entity> expectedB = Lists.newArrayList(B);

addGraphWithPaths("graphA", PATH_ACC_STORE_PROPERTIES_1, PATH_ENTITY_A_SCHEMA_JSON);
addGraphWithPaths("graphB", PATH_ACC_STORE_PROPERTIES_1, PATH_ENTITY_B_SCHEMA_JSON);

store.execute(new AddElements.Builder()
.input(A)
.option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphA")
.build(), userContext);

store.execute(new AddElements.Builder()
.input(B)
.option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphB")
.build(), userContext);

try {
//when
store.execute(new GetSchema.Builder().build(), userContext);
fail("exception expected");
} catch (final SchemaException e) {
//then
assertTrue(e.getMessage(), Pattern.compile("Unable to merge the schemas for all of your federated graphs: \\[graph., graph.\\]\\. You can limit which graphs to query for using the operation option: gaffer\\.federatedstore\\.operation\\.graphIds").matcher(e.getMessage()).matches());
}

//when
final CloseableIterable<? extends Element> responseGraphsWithNoView = store.execute(new GetAllElements.Builder().build(), userContext);
//then
ElementUtil.assertElementEquals(expectedAB, responseGraphsWithNoView);

//when
final CloseableIterable<? extends Element> responseGraphA = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphA").build(), userContext);
final CloseableIterable<? extends Element> responseGraphB = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphB").build(), userContext);
final CloseableIterable<? extends Element> responseGraphAWithAView = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphA").view(new View.Builder().entity("entityA").build()).build(), userContext);
final CloseableIterable<? extends Element> responseGraphBWithBView = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphB").view(new View.Builder().entity("entityB").build()).build(), userContext);
final CloseableIterable<? extends Element> responseGraphsWithAView = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphA,graphB").view(new View.Builder().entity("entityA").build()).build(), userContext);
final CloseableIterable<? extends Element> responseGraphsWithBView = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphA,graphB").view(new View.Builder().entity("entityB").build()).build(), userContext);
//then
ElementUtil.assertElementEquals(expectedA, responseGraphA);
ElementUtil.assertElementEquals(expectedB, responseGraphB);
ElementUtil.assertElementEquals(expectedA, responseGraphAWithAView);
ElementUtil.assertElementEquals(expectedB, responseGraphBWithBView);
ElementUtil.assertElementEquals(expectedA, responseGraphsWithAView);
ElementUtil.assertElementEquals(expectedB, responseGraphsWithBView);

try {
//when
CloseableIterable<? extends Element> responseGraphAWithBView = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphA").view(new View.Builder().entity("entityB").build()).build(), userContext);
fail("exception expected");
} catch (Exception e) {
//then
assertEquals("Operation chain is invalid. Validation errors: \n" +
"View for operation uk.gov.gchq.gaffer.operation.impl.get.GetAllElements is not valid. \n" +
"Entity group entityB does not exist in the schema", e.getMessage());
}

try {
//when
final CloseableIterable<? extends Element> responseGraphBWithAView = store.execute(new GetAllElements.Builder().option(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, "graphB").view(new View.Builder().entity("entityA").build()).build(), userContext);
fail("exception expected");
} catch (Exception e) {
//then
assertEquals("Operation chain is invalid. Validation errors: \n" +
"View for operation uk.gov.gchq.gaffer.operation.impl.get.GetAllElements is not valid. \n" +
"Entity group entityA does not exist in the schema", e.getMessage());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"entities": {
"entityA": {
"vertex": "vertex.string",
"properties": {
"property1": "simpleProperty"
}
}
},
"types": {
"vertex.string": {
"class": "java.lang.String"
},
"simpleProperty": {
"class": "java.lang.Integer",
"aggregateFunction": {
"class": "uk.gov.gchq.koryphe.impl.binaryoperator.Sum"
},
"serialiser": {
"class": "uk.gov.gchq.gaffer.serialisation.implementation.raw.CompactRawIntegerSerialiser"
}
}
},
"vertexSerialiser": {
"class": "uk.gov.gchq.gaffer.serialisation.implementation.StringSerialiser"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"entities": {
"entityB": {
"vertex": "vertex.int",
"properties": {
"property1": "simpleProperty"
}
}
},
"types": {
"vertex.int": {
"class": "java.lang.Integer"
},
"simpleProperty": {
"class": "java.lang.Integer",
"aggregateFunction": {
"class": "uk.gov.gchq.koryphe.impl.binaryoperator.Sum"
},
"serialiser": {
"class": "uk.gov.gchq.gaffer.serialisation.implementation.raw.CompactRawIntegerSerialiser"
}
}
},
"vertexSerialiser": {
"class": "uk.gov.gchq.gaffer.serialisation.implementation.raw.CompactRawIntegerSerialiser"
}
}

0 comments on commit 02aaa3b

Please sign in to comment.