From 3ee6a5f0f9ac9840fbf1a285da6e06bd9f6c3fd5 Mon Sep 17 00:00:00 2001 From: GCHQDev404 <45399082+GCHQDev404@users.noreply.github.com> Date: Wed, 2 Nov 2022 12:52:09 +0000 Subject: [PATCH] gh-2457 double caching issue (#2595) * gh-2457-double-caching-issue weak initial step, requires synchronisation with FederatedGraphStorage and further testing. * gh-2457-double-caching-issue remove FederatedGraphStorage local map, using cache only. * gh-2457-double-caching-issue remove FederatedGraphStorage test fixes * gh-2457-double-caching-issue remove FederatedGraphStorage review. * gh-2457-double-caching-removing-graphstorage minimising use of GraphSerialisable.getGraph() * gh-2457-double-caching-removing-graphstorage gh-2478 JobTracker cache can have Suffix name. * gh-2457 double caching issue fix for persisting graph names in tests. * Merge remote-tracking branch 'origin/v2-alpha' into gh-2357-federatedstore-federated-operation-merge-alpha2 !!!With 1 failing class of Tests!!! * gh-2457 GraphSerialisable not being able to Mock has failing tests. changing GraphSerialisable causes backwards compatability issues. * gh-2457 Fixed GraphSerialisable equals. * gh-2457 checkstyle * gh-2457 PR requests. * gh-2457 PR requests. * gh-2457 PR requests. --- .../java/uk/gov/gchq/gaffer/cache/Cache.java | 16 +- .../java/uk/gov/gchq/gaffer/graph/Graph.java | 30 +- .../gchq/gaffer/graph/GraphSerialisable.java | 195 +++++--- .../gaffer/graph/GraphSerialisableTest.java | 7 +- .../gchq/gaffer/jobtracker/JobTracker.java | 27 +- .../java/uk/gov/gchq/gaffer/store/Store.java | 5 +- .../gchq/gaffer/store/StoreProperties.java | 32 +- .../federatedstore/FederatedGraphStorage.java | 452 ++++++++---------- .../gaffer/federatedstore/FederatedStore.java | 31 +- .../federatedstore/FederatedStoreCache.java | 21 +- .../FederatedStoreProperties.java | 17 +- .../FederatedOperationChainValidator.java | 10 +- .../FederatedAddGraphHandlerParent.java | 7 +- .../impl/FederatedOperationHandler.java | 12 +- .../FederatedGraphStorageTest.java | 121 +++-- .../FederatedStoreAuthTest.java | 13 +- .../FederatedStoreCacheTest.java | 11 +- .../FederatedStoreDefaultGraphsTest.java | 26 +- .../FederatedStoreGetTraitsTest.java | 6 +- .../FederatedStoreGraphLibraryTest.java | 8 +- .../FederatedStoreGraphVisibilityTest.java | 2 +- .../FederatedStoreMultiCacheTest.java | 2 +- .../FederatedStorePublicAccessTest.java | 2 +- .../federatedstore/FederatedStoreTest.java | 125 +++-- .../FederatedStoreTestUtil.java | 19 +- .../FederatedStoreWrongGraphIDsTest.java | 2 +- .../AbstractStandaloneFederatedStoreIT.java | 3 + .../integration/FederatedAdminIT.java | 16 +- .../FederatedStoreRecursionIT.java | 7 +- .../FederatedOperationHandlerTest.java | 44 +- .../impl/FederatedAddGraphHandlerTest.java | 30 +- ...FederatedAddGraphWithHooksHandlerTest.java | 31 +- .../impl/FederatedAggregateHandlerTest.java | 11 +- .../FederatedOperationChainHandlerTest.java | 2 +- .../impl/FederatedRemoveGraphHandlerTest.java | 13 +- .../singleUseFederatedStore.properties | 1 + ...pStorePropertiesGraphSerialisableTest.java | 4 +- 37 files changed, 761 insertions(+), 600 deletions(-) diff --git a/core/cache/src/main/java/uk/gov/gchq/gaffer/cache/Cache.java b/core/cache/src/main/java/uk/gov/gchq/gaffer/cache/Cache.java index 9e42174b66a..4dc2b6dd816 100644 --- a/core/cache/src/main/java/uk/gov/gchq/gaffer/cache/Cache.java +++ b/core/cache/src/main/java/uk/gov/gchq/gaffer/cache/Cache.java @@ -1,5 +1,5 @@ /* - * Copyright 2018-2020 Crown Copyright + * Copyright 2018-2022 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package uk.gov.gchq.gaffer.cache; import uk.gov.gchq.gaffer.cache.exception.CacheOperationException; +import uk.gov.gchq.gaffer.core.exception.GafferRuntimeException; import java.util.Collections; import java.util.Set; @@ -55,8 +56,17 @@ protected void addToCache(final String key, final V value, final boolean overwri } public Set getAllKeys() { - final Set allKeysFromCache = CacheServiceLoader.getService().getAllKeysFromCache(cacheName); - return (null == allKeysFromCache) ? null : Collections.unmodifiableSet(allKeysFromCache); + try { + final Set allKeysFromCache; + if (CacheServiceLoader.isEnabled()) { + allKeysFromCache = CacheServiceLoader.getService().getAllKeysFromCache(cacheName); + } else { + throw new GafferRuntimeException("Cache is not enabled, check it was Initialised"); + } + return (null == allKeysFromCache) ? null : Collections.unmodifiableSet(allKeysFromCache); + } catch (final Exception e) { + throw new GafferRuntimeException("Error getting all keys", e); + } } /** diff --git a/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/Graph.java b/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/Graph.java index fa436dc95c5..eee3a2baefd 100644 --- a/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/Graph.java +++ b/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/Graph.java @@ -18,6 +18,8 @@ import com.google.common.collect.Lists; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.builder.EqualsBuilder; +import org.apache.commons.lang3.builder.HashCodeBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -447,8 +449,7 @@ public String getDescription() { * Returns all the {@link StoreTrait}s for the contained {@link Store} * implementation * - * @return a {@link Set} of all of the {@link StoreTrait}s that the store - * has. + * @return a {@link Set} of all of the {@link StoreTrait}s that the store has. */ @Deprecated public Set getStoreTraits() { @@ -975,4 +976,29 @@ private Schema cloneSchema(final Schema schema) { return null != schema ? schema.clone() : null; } } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + + if (o == null || getClass() != o.getClass()) { + return false; + } + + final Graph graph = (Graph) o; + + return new EqualsBuilder() + .append(new GraphSerialisable.Builder(this).build(), new GraphSerialisable.Builder(graph).build()) + .isEquals(); + } + + @Override + public int hashCode() { + return new HashCodeBuilder(17, 37) + .append(store) + .append(config) + .toHashCode(); + } } diff --git a/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/GraphSerialisable.java b/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/GraphSerialisable.java index ea43a6b97c3..6f7d8a6d57d 100644 --- a/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/GraphSerialisable.java +++ b/core/graph/src/main/java/uk/gov/gchq/gaffer/graph/GraphSerialisable.java @@ -23,8 +23,8 @@ import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; -import uk.gov.gchq.gaffer.commonutil.StringUtil; import uk.gov.gchq.gaffer.commonutil.ToStringBuilder; +import uk.gov.gchq.gaffer.core.exception.GafferRuntimeException; import uk.gov.gchq.gaffer.exception.SerialisationException; import uk.gov.gchq.gaffer.jsonserialisation.JSONSerialiser; import uk.gov.gchq.gaffer.store.StoreProperties; @@ -33,9 +33,11 @@ import java.io.InputStream; import java.io.Serializable; +import java.util.Arrays; import java.util.Properties; import static java.util.Objects.isNull; +import static java.util.Objects.nonNull; /** * A Serialisable object which holds the contents for creating Graphs. @@ -45,38 +47,42 @@ * @see GraphSerialisable.Builder */ @JsonDeserialize(builder = GraphSerialisable.Builder.class) -public final class GraphSerialisable implements Serializable { +public class GraphSerialisable implements Serializable { private static final long serialVersionUID = 2684203367656032583L; - - private transient Schema deserialisedSchema; - private final byte[] schema; - - private transient StoreProperties deserialisedProperties; - private final Properties properties; - - private final byte[] config; - private transient GraphConfig deserialisedConfig; + private final byte[] serialisedSchema; + private final byte[] serialisedProperties; + private final byte[] serialisedConfig; + private transient Schema schema; + private transient StoreProperties storeProperties; + private transient GraphConfig config; private transient Graph graph; - private GraphSerialisable(final GraphConfig config, final Schema schema, final Properties properties) { - this.deserialisedSchema = schema; + public GraphSerialisable(final GraphConfig config, final Schema schema, final StoreProperties storeProperties) { + this(config, schema, storeProperties.getProperties()); + } + + public GraphSerialisable(final GraphConfig config, final Schema schema, final Properties properties) { try { - this.schema = null == schema ? null : JSONSerialiser.serialise(schema, true); + this.serialisedSchema = isNull(schema) ? null : JSONSerialiser.serialise(schema, true); } catch (final SerialisationException e) { throw new IllegalArgumentException("Unable to serialise schema", e); } - this.deserialisedConfig = config; try { - this.config = null == config ? null : JSONSerialiser.serialise(config, true); + this.serialisedConfig = isNull(config) ? null : JSONSerialiser.serialise(config, true); } catch (final SerialisationException e) { throw new IllegalArgumentException("Unable to serialise config", e); } - this.deserialisedProperties = StoreProperties.loadStoreProperties(properties); - this.properties = properties; + + try { + this.serialisedProperties = isNull(properties) ? null : JSONSerialiser.serialise(properties, true); + } catch (final SerialisationException e) { + throw new IllegalArgumentException("Unable to serialise properties", e); + } } + /** * @return returns a new {@link Graph} built from the contents of a this * class. @@ -93,11 +99,15 @@ public Graph getGraph() { */ @JsonIgnore public Graph getGraph(final GraphLibrary library) { - if (null == graph) { + if (isNull(graph)) { + + //TODO FS use GraphDelegate to create and verify just like if a User was Adding. + //graph = GraphDelegate.createGraph( + graph = new Graph.Builder() - .addSchema(getDeserialisedSchema()) - .addStoreProperties(getDeserialisedProperties()) - .config(getDeserialisedConfig()) + .addSchema(getSchema()) + .addStoreProperties(getStoreProperties()) + .config(getConfig()) .config(new GraphConfig.Builder() .library(library) .build()) @@ -110,14 +120,14 @@ public Graph getGraph(final GraphLibrary library) { @Override public boolean equals(final Object obj) { final Boolean rtn; - if (null == obj || !(obj instanceof GraphSerialisable)) { + if (isNull(obj) || !(obj instanceof GraphSerialisable)) { rtn = false; } else { final GraphSerialisable that = (GraphSerialisable) obj; rtn = new EqualsBuilder() - .append(this.getConfig(), that.getConfig()) - .append(this.getSchema(), that.getSchema()) - .append(this.getProperties(), that.getProperties()) + .appendSuper(Arrays.equals(this.getSerialisedConfig(), that.getSerialisedConfig())) + .appendSuper(Arrays.equals(this.getSerialisedSchema(), that.getSerialisedSchema())) + .appendSuper(Arrays.equals(this.getSerialisedProperties(), that.getSerialisedProperties())) .build(); } return rtn; @@ -126,74 +136,109 @@ public boolean equals(final Object obj) { @Override public String toString() { return new ToStringBuilder(this) - .append("config", StringUtil.toString(this.getConfig())) - .append("schema", StringUtil.toString(this.getSchema())) - .append("properties", this.getProperties()) + .append("config", getConfig()) + .append("schema", getSchema()) + .append("properties", getStoreProperties().getProperties()) .build(); } @Override public int hashCode() { return new HashCodeBuilder(13, 31) - .append(this.getConfig()) - .append(this.getSchema()) - .append(this.getProperties()) + .append(this.getSerialisedConfig()) + .append(this.getSerialisedSchema()) + .append(this.getSerialisedProperties()) .build(); } @JsonIgnore - public Schema getDeserialisedSchema() { - if (null == deserialisedSchema) { - if (null == graph) { - deserialisedSchema = null != schema ? Schema.fromJson(schema) : null; + private Schema _getDeserialisedSchema() { + if (isNull(schema)) { + if (isNull(graph)) { + schema = null != serialisedSchema ? Schema.fromJson(serialisedSchema) : null; } else { - deserialisedSchema = graph.getSchema(); + schema = graph.getSchema(); } } - return deserialisedSchema; + return schema; } - public byte[] getSchema() { - return schema; + @JsonIgnore + public Schema getSchema() { + return getSchema(null); } @JsonIgnore - public StoreProperties getDeserialisedProperties() { - if (null == deserialisedProperties) { - if (null == graph) { - deserialisedProperties = null != properties ? StoreProperties.loadStoreProperties(properties) : null; - } else { - deserialisedProperties = graph.getStoreProperties(); - } + public Schema getSchema(final GraphLibrary graphLibrary) { + Schema schema = _getDeserialisedSchema(); + if (isNull(schema) && nonNull(graphLibrary)) { + schema = graphLibrary.getSchema(graph.getGraphId()); } - return deserialisedProperties; + return schema; } - public Properties getProperties() { - return properties; + @JsonIgnore + public String getGraphId() { + GraphConfig graphConfig = getConfig(); + return nonNull(graphConfig) + ? graphConfig.getGraphId() + : null; + } + + public byte[] getSerialisedSchema() { + return serialisedSchema; } @JsonIgnore - public GraphConfig getDeserialisedConfig() { - if (null == deserialisedConfig) { - if (null == graph) { - deserialisedConfig = null != config ? new GraphConfig.Builder().json(config).build() : null; + private StoreProperties _getDeserialisedStoreProperties() { + if (isNull(storeProperties)) { + if (isNull(graph)) { + try { + storeProperties = null != serialisedProperties ? StoreProperties.loadStoreProperties(JSONSerialiser.deserialise(serialisedProperties, Properties.class)) : null; + } catch (final SerialisationException e) { + throw new GafferRuntimeException("Unable to deserialise properties"); + } } else { - deserialisedConfig = graph.getConfig(); + storeProperties = graph.getStoreProperties(); } } - return deserialisedConfig; + return storeProperties; } @JsonIgnore - public String getGraphId() { - return getDeserialisedConfig().getGraphId(); + public StoreProperties getStoreProperties() { + return getStoreProperties(null); } - public byte[] getConfig() { + @JsonIgnore + public StoreProperties getStoreProperties(final GraphLibrary graphLibrary) { + StoreProperties properties = _getDeserialisedStoreProperties(); + if (isNull(properties) && nonNull(graphLibrary)) { + properties = graphLibrary.getProperties(graph.getGraphId()); + } + return properties; + } + + public byte[] getSerialisedProperties() { + return serialisedProperties; + } + + @JsonIgnore + public GraphConfig getConfig() { + if (isNull(config)) { + if (isNull(graph)) { + config = null != serialisedConfig ? new GraphConfig.Builder().json(serialisedConfig).build() : null; + } else { + config = graph.getConfig(); + } + } return config; } + public byte[] getSerialisedConfig() { + return serialisedConfig; + } + @JsonPOJOBuilder(buildMethodName = "build", withPrefix = "") public static class Builder { @@ -201,6 +246,24 @@ public static class Builder { private Properties properties; private GraphConfig config; + public Builder() { + } + + @JsonIgnore + public Builder(final Graph graph) { + this(); + schema(graph.getSchema()); + properties(graph.getStoreProperties().getProperties()); + config(graph.getConfig()); + } + + public Builder(final GraphSerialisable graphSerialisable) { + this(); + schema(graphSerialisable.getSchema()); + properties(graphSerialisable.getStoreProperties()); + config(graphSerialisable.getConfig()); + } + public Builder schema(final Schema schema) { this.schema = schema; return _self(); @@ -217,7 +280,7 @@ public Builder properties(final Properties properties) { } public Builder properties(final StoreProperties properties) { - if (null == properties) { + if (isNull(properties)) { this.properties = null; } else { this.properties = properties.getProperties(); @@ -234,11 +297,12 @@ public Builder config(final GraphConfig config) { return _self(); } - @JsonIgnore - public Builder graph(final Graph graph) { - schema = graph.getSchema(); - properties = graph.getStoreProperties().getProperties(); - config = graph.getConfig(); + public Builder mergeConfig(final GraphConfig config) { + this.config = new GraphConfig.Builder() + .merge(this.config) + .merge(config) + .build(); + return _self(); } @@ -253,5 +317,4 @@ public GraphSerialisable build() { return new GraphSerialisable(config, schema, properties); } } - } diff --git a/core/graph/src/test/java/uk/gov/gchq/gaffer/graph/GraphSerialisableTest.java b/core/graph/src/test/java/uk/gov/gchq/gaffer/graph/GraphSerialisableTest.java index ed854e629ad..6f55e2ca18c 100644 --- a/core/graph/src/test/java/uk/gov/gchq/gaffer/graph/GraphSerialisableTest.java +++ b/core/graph/src/test/java/uk/gov/gchq/gaffer/graph/GraphSerialisableTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2020 Crown Copyright + * Copyright 2017-2022 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -91,9 +91,8 @@ public void shouldSerialiseAndDeserialise() throws Exception { @Test public void shouldConsumeGraph() { // Given - final Graph graph = new Graph.Builder().addSchema(schema).addStoreProperties(new StoreProperties(properties)) - .config(config).build(); - final GraphSerialisable result = new GraphSerialisable.Builder().graph(graph).build(); + final Graph graph = new Graph.Builder().addSchema(schema).addStoreProperties(new StoreProperties(properties)).config(config).build(); + final GraphSerialisable result = new GraphSerialisable.Builder(graph).build(); // When / Then assertThat(result).isEqualTo(expected); diff --git a/core/operation/src/main/java/uk/gov/gchq/gaffer/jobtracker/JobTracker.java b/core/operation/src/main/java/uk/gov/gchq/gaffer/jobtracker/JobTracker.java index de9cb186de7..fcfd1a1aa25 100644 --- a/core/operation/src/main/java/uk/gov/gchq/gaffer/jobtracker/JobTracker.java +++ b/core/operation/src/main/java/uk/gov/gchq/gaffer/jobtracker/JobTracker.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2020 Crown Copyright + * Copyright 2016-2022 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,13 +26,25 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import static java.util.Objects.nonNull; + /** * A {@code JobTracker} is an entry in a Gaffer cache service which is used to store * details of jobs submitted to the graph. */ public class JobTracker { - private static final String CACHE_NAME = "JobTracker"; + private String cacheName; + + public static final String CACHE_SERVICE_NAME_PREFIX = "JobTracker"; + + public JobTracker() { + this(null); + } + + public JobTracker(final String cacheNameSuffix) { + cacheName = String.format("%s%s", CACHE_SERVICE_NAME_PREFIX, nonNull(cacheNameSuffix) ? "_" + cacheNameSuffix.toLowerCase() : ""); + } /** * Add or update the job details relating to a job in the job tracker cache. @@ -43,7 +55,7 @@ public class JobTracker { public void addOrUpdateJob(final JobDetail jobDetail, final User user) { validateJobDetail(jobDetail); try { - CacheServiceLoader.getService().putInCache(CACHE_NAME, jobDetail.getJobId(), jobDetail); + CacheServiceLoader.getService().putInCache(cacheName, jobDetail.getJobId(), jobDetail); } catch (final CacheOperationException e) { throw new RuntimeException("Failed to add jobDetail " + jobDetail.toString() + " to the cache", e); } @@ -57,7 +69,7 @@ public void addOrUpdateJob(final JobDetail jobDetail, final User user) { * @return the {@link JobDetail} object for the requested job */ public JobDetail getJob(final String jobId, final User user) { - return CacheServiceLoader.getService().getFromCache(CACHE_NAME, jobId); + return CacheServiceLoader.getService().getFromCache(cacheName, jobId); } /** @@ -83,7 +95,7 @@ public Iterable getAllScheduledJobs() { private Iterable getAllJobsMatching(final User user, final Predicate jobDetailPredicate) { - final Set jobIds = CacheServiceLoader.getService().getAllKeysFromCache(CACHE_NAME); + final Set jobIds = CacheServiceLoader.getService().getAllKeysFromCache(cacheName); final List jobs = jobIds.stream() .filter(Objects::nonNull) .map(jobId -> getJob(jobId, user)) @@ -99,7 +111,7 @@ private Iterable getAllJobsMatching(final User user, final Predicate< */ public void clear() { try { - CacheServiceLoader.getService().clearCache(CACHE_NAME); + CacheServiceLoader.getService().clearCache(cacheName); } catch (final CacheOperationException e) { throw new RuntimeException("Failed to clear job tracker cache", e); } @@ -115,4 +127,7 @@ private void validateJobDetail(final JobDetail jobDetail) { } } + public String getCacheName() { + return cacheName; + } } diff --git a/core/store/src/main/java/uk/gov/gchq/gaffer/store/Store.java b/core/store/src/main/java/uk/gov/gchq/gaffer/store/Store.java index bd94e11b8f5..af371e81af3 100644 --- a/core/store/src/main/java/uk/gov/gchq/gaffer/store/Store.java +++ b/core/store/src/main/java/uk/gov/gchq/gaffer/store/Store.java @@ -253,8 +253,7 @@ public static Store createStore(final String graphId, final Schema schema, final final String storeClass = storeProperties.getStoreClass(); if (isNull(storeClass)) { - throw new IllegalArgumentException(String - .format("The Store class name was not found in the store properties for key: %s, GraphId: %s", + throw new IllegalArgumentException(String.format("The Store class name was not found in the store properties for key: %s, GraphId: %s", StoreProperties.STORE_CLASS, graphId)); } @@ -804,7 +803,7 @@ protected void validateSchema(final ValidationResult validationResult, final Ser protected JobTracker createJobTracker() { if (properties.getJobTrackerEnabled()) { - return new JobTracker(); + return new JobTracker(getProperties().getCacheServiceNameSuffix()); } return null; } diff --git a/core/store/src/main/java/uk/gov/gchq/gaffer/store/StoreProperties.java b/core/store/src/main/java/uk/gov/gchq/gaffer/store/StoreProperties.java index 7c37f153b60..9e3a522218b 100644 --- a/core/store/src/main/java/uk/gov/gchq/gaffer/store/StoreProperties.java +++ b/core/store/src/main/java/uk/gov/gchq/gaffer/store/StoreProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2020 Crown Copyright + * Copyright 2017-2022 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import uk.gov.gchq.gaffer.cache.util.CacheProperties; import uk.gov.gchq.gaffer.commonutil.DebugUtil; import uk.gov.gchq.gaffer.commonutil.StreamUtil; import uk.gov.gchq.gaffer.commonutil.ToStringBuilder; @@ -72,6 +73,19 @@ public class StoreProperties implements Cloneable { public static final String ADMIN_AUTH = "gaffer.store.admin.auth"; + /** + * This is used.... + * eg.gaffer.cache.service.class="uk.gov.gchq.gaffer.cache.impl.HashMapCacheService" + */ + public static final String CACHE_SERVICE_CLASS = CacheProperties.CACHE_SERVICE_CLASS; + + /** + * This is used... + * CASE INSENSITIVE + * e.g. gaffer.cache.service.name.suffix="v2" + */ + public static final String CACHE_SERVICE_NAME_SUFFIX = "gaffer.cache.service.name.suffix"; + /** * CSV of extra packages to be included in the reflection scanning. */ @@ -421,6 +435,22 @@ public void setAdminAuth(final String adminAuth) { set(ADMIN_AUTH, adminAuth); } + public void setCacheServiceClass(final String cacheServiceClassString) { + set(CACHE_SERVICE_CLASS, cacheServiceClassString); + } + + public String getCacheServiceClass(final String defaultValue) { + return get(CACHE_SERVICE_CLASS, defaultValue); + } + + public void setCacheServiceNameSuffix(final String suffix) { + set(CACHE_SERVICE_NAME_SUFFIX, suffix); + } + + public String getCacheServiceNameSuffix() { + return get(CACHE_SERVICE_NAME_SUFFIX, null); + } + public Properties getProperties() { return props; } diff --git a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorage.java b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorage.java index 44efa8e285f..593e41eea66 100644 --- a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorage.java +++ b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorage.java @@ -27,13 +27,13 @@ import uk.gov.gchq.gaffer.cache.exception.CacheOperationException; import uk.gov.gchq.gaffer.commonutil.JsonUtil; import uk.gov.gchq.gaffer.commonutil.exception.OverwritingException; +import uk.gov.gchq.gaffer.commonutil.pair.Pair; +import uk.gov.gchq.gaffer.core.exception.GafferRuntimeException; import uk.gov.gchq.gaffer.data.elementdefinition.exception.SchemaException; import uk.gov.gchq.gaffer.federatedstore.exception.StorageException; import uk.gov.gchq.gaffer.federatedstore.operation.FederatedOperation; -import uk.gov.gchq.gaffer.graph.Graph; import uk.gov.gchq.gaffer.graph.GraphConfig; import uk.gov.gchq.gaffer.graph.GraphSerialisable; -import uk.gov.gchq.gaffer.operation.OperationException; import uk.gov.gchq.gaffer.store.Context; import uk.gov.gchq.gaffer.store.library.GraphLibrary; import uk.gov.gchq.gaffer.store.operation.GetSchema; @@ -41,15 +41,13 @@ import uk.gov.gchq.gaffer.store.schema.Schema.Builder; import uk.gov.gchq.gaffer.user.User; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -57,23 +55,30 @@ import static java.util.Objects.isNull; import static java.util.Objects.nonNull; +import static uk.gov.gchq.gaffer.cache.util.CacheProperties.CACHE_SERVICE_CLASS; public class FederatedGraphStorage { - private static final Logger LOGGER = LoggerFactory.getLogger(FederatedGraphStorage.class); public static final boolean DEFAULT_DISABLED_BY_DEFAULT = false; public static final String ERROR_ADDING_GRAPH_TO_CACHE = "Error adding graph, GraphId is known within the cache, but %s is different. GraphId: %s"; public static final String USER_IS_ATTEMPTING_TO_OVERWRITE = "User is attempting to overwrite a graph within FederatedStore. GraphId: %s"; public static final String ACCESS_IS_NULL = "Can not put graph into storage without a FederatedAccess key."; public static final String GRAPH_IDS_NOT_VISIBLE = "The following graphIds are not visible or do not exist: %s"; - private final Map> storage = new HashMap<>(); - private final FederatedStoreCache federatedStoreCache = new FederatedStoreCache(); + private static final Logger LOGGER = LoggerFactory.getLogger(FederatedGraphStorage.class); + private final FederatedStoreCache federatedStoreCache; private Boolean isCacheEnabled = false; private GraphLibrary graphLibrary; + public FederatedGraphStorage() { + this(null); + } + + public FederatedGraphStorage(final String cacheNameSuffix) { + federatedStoreCache = new FederatedStoreCache(cacheNameSuffix); + } + protected void startCacheServiceLoader() throws StorageException { - if (CacheServiceLoader.isEnabled()) { - isCacheEnabled = true; - makeAllGraphsFromCache(); + if (!CacheServiceLoader.isEnabled()) { + throw new StorageException("Cache is not enabled for the FederatedStore, Set a value in StoreProperties for " + CACHE_SERVICE_CLASS); } } @@ -108,29 +113,22 @@ public void put(final GraphSerialisable graph, final FederatedAccess access) thr String graphId = graph.getGraphId(); try { if (null == access) { - throw new IllegalArgumentException(ACCESS_IS_NULL); + throw new StorageException(new IllegalArgumentException(ACCESS_IS_NULL)); } if (null != graphLibrary) { - graphLibrary.checkExisting(graphId, graph.getDeserialisedSchema(), graph.getDeserialisedProperties()); + graphLibrary.checkExisting(graphId, graph.getSchema(graphLibrary), graph.getStoreProperties(graphLibrary)); } - validateExisting(graph); - final Graph builtGraph = graph.getGraph(); - if (isCacheEnabled()) { - addToCache(builtGraph, access); - } + validateExisting(graphId); + + addToCache(graph, access); - Set existingGraphs = storage.get(access); - if (null == existingGraphs) { - existingGraphs = new HashSet<>(); - existingGraphs.add(builtGraph); - storage.put(access, existingGraphs); - } else { - existingGraphs.add(builtGraph); - } } catch (final Exception e) { - throw new StorageException("Error adding graph " + graphId + " to storage due to: " + e.getMessage(), e); + throw new StorageException(String.format("Error adding graph %s%s", graphId, + nonNull(e.getMessage()) + ? (" to storage due to: " + e.getMessage()) + : "."), e); } } else { throw new StorageException("Graph cannot be null"); @@ -145,16 +143,16 @@ public void put(final GraphSerialisable graph, final FederatedAccess access) thr * @return visible graphIds. */ public List getAllIds(final User user) { - return getIdsFrom(getUserGraphStream(entry -> entry.getKey().hasReadAccess(user))); + return getIdsFrom(getUserGraphStream(federatedAccess -> federatedAccess.hasReadAccess(user))); } public List getAllIds(final User user, final String adminAuth) { - return getIdsFrom(getUserGraphStream(entry -> entry.getKey().hasReadAccess(user, adminAuth))); + return getIdsFrom(getUserGraphStream(federatedAccess -> federatedAccess.hasReadAccess(user, adminAuth))); } - private List getIdsFrom(final Stream allStream) { + private List getIdsFrom(final Stream allStream) { final List rtn = allStream - .map(Graph::getGraphId) + .map(GraphSerialisable::getGraphId) .distinct() .collect(Collectors.toList()); @@ -167,8 +165,8 @@ private List getIdsFrom(final Stream allStream) { * @param user to match visibility against. * @return visible graphs */ - public Collection getAll(final User user) { - final Set rtn = getUserGraphStream(entry -> entry.getKey().hasReadAccess(user)) + public Collection getAll(final User user) { + final Set rtn = getUserGraphStream(federatedAccess -> federatedAccess.hasReadAccess(user)) .collect(Collectors.toCollection(LinkedHashSet::new)); return Collections.unmodifiableCollection(rtn); } @@ -184,40 +182,23 @@ public Collection getAll(final User user) { * @see #isValidToView(User, FederatedAccess) */ public boolean remove(final String graphId, final User user) { - return remove(graphId, entry -> entry.getKey().hasWriteAccess(user)); + return remove(graphId, federatedAccess -> federatedAccess.hasWriteAccess(user)); } protected boolean remove(final String graphId, final User user, final String adminAuth) { - return remove(graphId, entry -> entry.getKey().hasWriteAccess(user, adminAuth)); - } - - private boolean remove(final String graphId, final Predicate>> entryPredicateForGraphRemoval) { - return storage.entrySet().stream() - .filter(entryPredicateForGraphRemoval) - .map(entry -> { - boolean isRemoved = false; - final Set graphs = entry.getValue(); - if (null != graphs) { - HashSet remove = new HashSet<>(); - for (final Graph graph : graphs) { - if (graph.getGraphId().equals(graphId)) { - remove.add(graph); - deleteFromCache(graphId); - isRemoved = true; - } - } - graphs.removeAll(remove); - } - return isRemoved; - }) - .collect(Collectors.toSet()) - .contains(true); + return remove(graphId, federatedAccess -> federatedAccess.hasWriteAccess(user, adminAuth)); } - private void deleteFromCache(final String graphId) { - if (isCacheEnabled()) { - federatedStoreCache.deleteGraphFromCache(graphId); + private boolean remove(final String graphId, final Predicate accessPredicate) { + FederatedAccess accessFromCache = federatedStoreCache.getAccessFromCache(graphId); + boolean rtn; + if (nonNull(accessFromCache) ? accessPredicate.test(accessFromCache) : false) { + federatedStoreCache.deleteFromCache(graphId); + rtn = true; + } else { + rtn = false; } + return rtn; } /** @@ -228,7 +209,7 @@ private void deleteFromCache(final String graphId) { * @param graphIds the graphIds to get graphs for. * @return visible graphs from the given graphIds. */ - public Collection get(final User user, final List graphIds) { + public Collection get(final User user, final List graphIds) { return get(user, graphIds, null); } @@ -241,18 +222,18 @@ public Collection get(final User user, final List graphIds) { * @param adminAuth adminAuths role * @return visible graphs from the given graphIds. */ - public List get(final User user, final List graphIds, final String adminAuth) { + public List get(final User user, final List graphIds, final String adminAuth) { if (null == user) { return Collections.emptyList(); } validateAllGivenGraphIdsAreVisibleForUser(user, graphIds, adminAuth); - Stream graphs = getStream(user, graphIds); + Stream graphs = getStream(user, graphIds); if (null != graphIds) { //This maintains order with the requested Ids. graphs = graphs.sorted(Comparator.comparingInt(g -> graphIds.indexOf(g.getGraphId()))); } - final List rtn = graphs.distinct().collect(Collectors.toList()); + final List rtn = graphs.distinct().collect(Collectors.toList()); return Collections.unmodifiableList(rtn); } @@ -264,22 +245,22 @@ public Schema getSchema(final FederatedOperation operation, final } final List graphIds = isNull(operation) ? null : operation.getGraphIds(); - final Stream graphs = getStream(context.getUser(), graphIds); + final Stream graphs = getStream(context.getUser(), graphIds); final Builder schemaBuilder = new Builder(); try { if (nonNull(operation) && operation.hasPayloadOperation() && operation.payloadInstanceOf(GetSchema.class) && ((GetSchema) operation.getPayloadOperation()).isCompact()) { - graphs.forEach(g -> { + graphs.forEach(gs -> { try { - schemaBuilder.merge(g.execute((GetSchema) operation.getPayloadOperation(), context)); - } catch (final OperationException e) { - throw new RuntimeException("Unable to fetch schema from graph " + g.getGraphId(), e); + schemaBuilder.merge(gs.getGraph().execute((GetSchema) operation.getPayloadOperation(), context)); + } catch (final Exception e) { + throw new GafferRuntimeException("Unable to fetch schema from graph " + gs.getGraphId(), e); } }); } else { - graphs.forEach(g -> schemaBuilder.merge(g.getSchema())); + graphs.forEach(g -> schemaBuilder.merge(g.getSchema(graphLibrary))); } } catch (final SchemaException e) { - final List resultGraphIds = getStream(context.getUser(), graphIds).map(Graph::getGraphId).collect(Collectors.toList()); + final List resultGraphIds = getStream(context.getUser(), graphIds).map(GraphSerialisable::getGraphId).collect(Collectors.toList()); throw new SchemaException("Unable to merge the schemas for all of your federated graphs: " + resultGraphIds + ". You can limit which graphs to query for using the FederatedOperation.graphIds.", e); } return schemaBuilder.build(); @@ -289,21 +270,17 @@ private void validateAllGivenGraphIdsAreVisibleForUser(final User user, final Co if (null != graphIds) { final Collection visibleIds = getAllIds(user, adminAuth); if (!visibleIds.containsAll(graphIds)) { - final Set notVisibleIds = new HashSet<>(graphIds); + final List notVisibleIds = new ArrayList<>(graphIds); notVisibleIds.removeAll(visibleIds); throw new IllegalArgumentException(String.format(GRAPH_IDS_NOT_VISIBLE, notVisibleIds)); } } } - private void validateExisting(final GraphSerialisable graph) throws StorageException { - final String graphId = graph.getGraphId(); - for (final Set graphs : storage.values()) { - for (final Graph g : graphs) { - if (g.getGraphId().equals(graphId)) { - throw new OverwritingException((String.format(USER_IS_ATTEMPTING_TO_OVERWRITE, graphId))); - } - } + private void validateExisting(final String graphId) throws StorageException { + boolean exists = federatedStoreCache.getAllGraphIds().contains(graphId); + if (exists) { + throw new StorageException(new OverwritingException((String.format(USER_IS_ATTEMPTING_TO_OVERWRITE, graphId)))); } } @@ -324,140 +301,92 @@ private boolean isValidToView(final User user, final FederatedAccess access) { * @return a stream of graphs for the given graphIds and the user has visibility for. * If graphIds is null then only enabled by default graphs are returned that the user can see. */ - private Stream getStream(final User user, final Collection graphIds) { + private Stream getStream(final User user, final Collection graphIds) { + Stream rtn; if (isNull(graphIds)) { - return storage.entrySet() - .stream() - .filter(entry -> isValidToView(user, entry.getKey())) + rtn = federatedStoreCache.getAllGraphIds().stream() + .map(g -> federatedStoreCache.getFromCache(g)) + .filter(pair -> isValidToView(user, pair.getSecond())) //not visible unless graphId is requested. - .filter(entry -> !entry.getKey().isDisabledByDefault()) - .flatMap(entry -> entry.getValue().stream()); + .filter(pair -> !pair.getSecond().isDisabledByDefault()) + .map(pair -> pair.getFirst()); + } else { + rtn = federatedStoreCache.getAllGraphIds().stream() + .map(g -> federatedStoreCache.getFromCache(g)) + .filter(pair -> isValidToView(user, pair.getSecond())) + .filter(pair -> graphIds.contains(pair.getFirst().getGraphId())) + .map(pair -> pair.getFirst()); } - - return storage.entrySet() - .stream() - .filter(entry -> isValidToView(user, entry.getKey())) - .flatMap(entry -> entry.getValue().stream()) - .filter(graph -> graphIds.contains(graph.getGraphId())); + return rtn; } /** * @param readAccessPredicate to filter graphs. * @return a stream of graphs the user has visibility for. */ - private Stream getUserGraphStream(final Predicate>> readAccessPredicate) { - return storage.entrySet() - .stream() - .filter(readAccessPredicate) - .flatMap(entry -> entry.getValue().stream()); + private Stream getUserGraphStream(final Predicate readAccessPredicate) { + return federatedStoreCache.getAllGraphIds().stream() + .map(graphId -> federatedStoreCache.getFromCache(graphId)) + .filter(pair -> readAccessPredicate.test(pair.getSecond())) + .map(Pair::getFirst); } - private void addToCache(final Graph newGraph, final FederatedAccess access) { - final String graphId = newGraph.getGraphId(); - if (federatedStoreCache.contains(graphId)) { - validateSameAsFromCache(newGraph, graphId); + private void addToCache(final GraphSerialisable newGraph, final FederatedAccess access) { + if (federatedStoreCache.contains(newGraph.getGraphId())) { + validateSameAsFromCache(newGraph); } else { try { federatedStoreCache.addGraphToCache(newGraph, access, false); } catch (final OverwritingException e) { - throw new OverwritingException((String.format("User is attempting to overwrite a graph within the cacheService. GraphId: %s", graphId))); + throw new OverwritingException((String.format("User is attempting to overwrite a graph within the cacheService. GraphId: %s", newGraph.getGraphId()))); } catch (final CacheOperationException e) { throw new RuntimeException(e); } } } - private void validateSameAsFromCache(final Graph newGraph, final String graphId) { - final Graph fromCache = federatedStoreCache.getGraphSerialisableFromCache(graphId).getGraph(graphLibrary); - if (!newGraph.getStoreProperties().getProperties().equals(fromCache.getStoreProperties().getProperties())) { - throw new RuntimeException(String.format(ERROR_ADDING_GRAPH_TO_CACHE, GraphConfigEnum.PROPERTIES.toString(), graphId)); - } else { - if (!JsonUtil.equals(newGraph.getSchema().toJson(false), fromCache.getSchema().toJson(false))) { - throw new RuntimeException(String.format(ERROR_ADDING_GRAPH_TO_CACHE, GraphConfigEnum.SCHEMA.toString(), graphId)); - } else { - if (!newGraph.getGraphId().equals(fromCache.getGraphId())) { - throw new RuntimeException(String.format(ERROR_ADDING_GRAPH_TO_CACHE, "GraphId", graphId)); - } - } - } - } + private void validateSameAsFromCache(final GraphSerialisable newGraph) { + String graphId = newGraph.getGraphId(); - public void setGraphLibrary(final GraphLibrary graphLibrary) { - this.graphLibrary = graphLibrary; - } - - /** - * Enum for the Graph Properties or Schema - */ - public enum GraphConfigEnum { - SCHEMA("schema"), PROPERTIES("properties"); - - private final String value; + GraphSerialisable fromCache = federatedStoreCache.getGraphSerialisableFromCache(graphId); - GraphConfigEnum(final String value) { - this.value = value; + if (!newGraph.getStoreProperties(graphLibrary).getProperties().equals(fromCache.getStoreProperties(graphLibrary))) { + throw new RuntimeException(String.format(ERROR_ADDING_GRAPH_TO_CACHE, GraphConfigEnum.PROPERTIES.toString(), graphId)); } - - @Override - public String toString() { - return value; + if (!JsonUtil.equals(newGraph.getSchema(graphLibrary).toJson(false), fromCache.getSchema(graphLibrary).toJson(false))) { + throw new RuntimeException(String.format(ERROR_ADDING_GRAPH_TO_CACHE, GraphConfigEnum.SCHEMA.toString(), graphId)); } - - } - - private Boolean isCacheEnabled() { - boolean rtn = false; - if (isCacheEnabled) { - if (federatedStoreCache.getCache() == null) { - throw new RuntimeException("No cache has been set, please initialise the FederatedStore instance"); - } - rtn = true; + if (!newGraph.getGraphId().equals(fromCache.getGraphId())) { + throw new RuntimeException(String.format(ERROR_ADDING_GRAPH_TO_CACHE, "GraphId", graphId)); } - return rtn; } - private void makeGraphFromCache(final String graphId) throws StorageException { - final GraphSerialisable graph = federatedStoreCache.getGraphSerialisableFromCache(graphId); - final FederatedAccess accessFromCache = federatedStoreCache.getAccessFromCache(graphId); - put(graph, accessFromCache); - } - - private void makeAllGraphsFromCache() throws StorageException { - final Set allGraphIds = federatedStoreCache.getAllGraphIds(); - for (final String graphId : allGraphIds) { - try { - makeGraphFromCache(graphId); - } catch (final Exception e) { - LOGGER.error("Skipping graphId: {} due to: {}", graphId, e.getMessage(), e); - } - } + public void setGraphLibrary(final GraphLibrary graphLibrary) { + this.graphLibrary = graphLibrary; } protected Map getAllGraphsAndAccess(final User user, final List graphIds) { return getAllGraphsAndAccess(graphIds, access -> access != null && access.hasReadAccess(user)); } - protected Map getAllGraphsAndAccessAsAdmin(final User user, final List graphIds, final String adminAuth) { + protected Map getAllGraphsAndAccess(final User user, final List graphIds, final String adminAuth) { return getAllGraphsAndAccess(graphIds, access -> access != null && access.hasReadAccess(user, adminAuth)); } private Map getAllGraphsAndAccess(final List graphIds, final Predicate accessPredicate) { - return storage.entrySet() - .stream() + return federatedStoreCache.getAllGraphIds().stream() + .map(graphId -> federatedStoreCache.getFromCache(graphId)) //filter on FederatedAccess - .filter(e -> accessPredicate.test(e.getKey())) - //convert to Map - .flatMap(entry -> entry.getValue().stream().collect(Collectors.toMap(Graph::getGraphId, g -> entry.getKey())).entrySet().stream()) + .filter(pair -> accessPredicate.test(pair.getSecond())) //filter on if graph required? - .filter(entry -> { - final boolean isGraphIdRequested = nonNull(graphIds) && graphIds.contains(entry.getKey()); + .filter(pair -> { + final boolean isGraphIdRequested = nonNull(graphIds) && graphIds.contains(pair.getFirst().getGraphId()); final boolean isAllGraphIdsRequired = isNull(graphIds) || graphIds.isEmpty(); return isGraphIdRequested || isAllGraphIdsRequired; }) - .collect(Collectors.toMap(Entry::getKey, Entry::getValue)); + .collect(Collectors.toMap(pair -> pair.getFirst().getGraphId(), Pair::getSecond)); } - public boolean changeGraphAccess(final String graphId, final FederatedAccess newFederatedAccess, final User requestingUser) throws StorageException { return changeGraphAccess(graphId, newFederatedAccess, access -> access.hasWriteAccess(requestingUser)); } @@ -468,29 +397,13 @@ public boolean changeGraphAccess(final String graphId, final FederatedAccess new private boolean changeGraphAccess(final String graphId, final FederatedAccess newFederatedAccess, final Predicate accessPredicate) throws StorageException { boolean rtn; - final Graph graphToMove = getGraphToMove(graphId, accessPredicate); + final GraphSerialisable graphToUpdate = getGraphToUpdate(graphId, accessPredicate); - if (nonNull(graphToMove)) { + if (nonNull(graphToUpdate)) { //remove graph to be moved - FederatedAccess oldAccess = null; - for (final Entry> entry : storage.entrySet()) { - entry.getValue().removeIf(graph -> graph.getGraphId().equals(graphId)); - oldAccess = entry.getKey(); - } + remove(graphId, federatedAccess -> true); - //add the graph being moved. - this.put(new GraphSerialisable.Builder().graph(graphToMove).build(), newFederatedAccess); - - if (isCacheEnabled()) { - //Update cache - try { - federatedStoreCache.addGraphToCache(graphToMove, newFederatedAccess, true/*true because graphLibrary should have throw error*/); - } catch (final CacheOperationException e) { - String message = String.format("Error occurred updating graphAccess. GraphStorage=updated, Cache=outdated. graphId:%s. Recovery is possible from a restart if a persistent cache is being used, otherwise contact admin", graphId); - LOGGER.error("{} graphStorage access:{} cache access:{}", message, newFederatedAccess, oldAccess); - throw new StorageException(message, e); - } - } + updateCacheWithNewAccess(graphId, newFederatedAccess, graphToUpdate); rtn = true; } else { @@ -499,6 +412,16 @@ private boolean changeGraphAccess(final String graphId, final FederatedAccess ne return rtn; } + private void updateCacheWithNewAccess(final String graphId, final FederatedAccess newFederatedAccess, final GraphSerialisable graphToUpdate) throws StorageException { + try { + this.put(new GraphSerialisable.Builder(graphToUpdate).build(), newFederatedAccess); + } catch (final Exception e) { + String message = String.format("Error occurred updating graphAccess. GraphStorage=updated, Cache=outdated. graphId:%s. Recovery is possible from a restart if a persistent cache is being used, otherwise contact admin", graphId); + LOGGER.error("{} graphStorage access:{} cache access:{}", message, newFederatedAccess, federatedStoreCache.getAccessFromCache(graphId)); + throw new StorageException(message, e); + } + } + public boolean changeGraphId(final String graphId, final String newGraphId, final User requestingUser) throws StorageException { return changeGraphId(graphId, newGraphId, access -> access.hasWriteAccess(requestingUser)); } @@ -509,63 +432,17 @@ public boolean changeGraphId(final String graphId, final String newGraphId, fina private boolean changeGraphId(final String graphId, final String newGraphId, final Predicate accessPredicate) throws StorageException { boolean rtn; - final Graph graphToMove = getGraphToMove(graphId, accessPredicate); - - if (nonNull(graphToMove)) { - FederatedAccess key = null; - //remove graph to be moved from storage - for (final Entry> entry : storage.entrySet()) { - final boolean removed = entry.getValue().removeIf(graph -> graph.getGraphId().equals(graphId)); - if (removed) { - key = entry.getKey(); - break; - } - } + final GraphSerialisable graphToUpdate = getGraphToUpdate(graphId, accessPredicate); - //Update Tables - String storeClass = graphToMove.getStoreProperties().getStoreClass(); - if (nonNull(storeClass) && storeClass.startsWith(AccumuloStore.class.getPackage().getName())) { - /* - * This logic is only for Accumulo derived stores Only. - * For updating table names to match graphs names. - * - * uk.gov.gchq.gaffer.accumulostore.[AccumuloStore, SingleUseAccumuloStore, - * MiniAccumuloStore, SingleUseMiniAccumuloStore] - */ - try { - Connector connection = TableUtils.getConnector((AccumuloProperties) graphToMove.getStoreProperties()); - - if (connection.tableOperations().exists(graphId)) { - connection.tableOperations().offline(graphId); - connection.tableOperations().rename(graphId, newGraphId); - connection.tableOperations().online(newGraphId); - } - } catch (final Exception e) { - LOGGER.warn("Error trying to update tables for graphID:{} graphToMove:{}", graphId, graphToMove, e); - } - } + if (nonNull(graphToUpdate)) { + //get access before removing old graphId. + FederatedAccess access = federatedStoreCache.getAccessFromCache(graphId); + //Removed first, to stop a sync issue when sharing the cache with another store. + remove(graphId, federatedAccess -> true); - final GraphConfig configWithNewGraphId = cloneGraphConfigWithNewGraphId(newGraphId, graphToMove); - - //add the graph being renamed. - GraphSerialisable newGraphSerialisable = new GraphSerialisable.Builder() - .graph(graphToMove) - .config(configWithNewGraphId) - .build(); - this.put(newGraphSerialisable, key); - - //Update cache - if (isCacheEnabled()) { - try { - //Overwrite cache = true because the graphLibrary should have thrown an error before this point. - federatedStoreCache.addGraphToCache(newGraphSerialisable, key, true); - } catch (final CacheOperationException e) { - String message = "Contact Admin for recovery. Error occurred updating graphId. GraphStorage=updated, Cache=outdated graphId."; - LOGGER.error("{} graphStorage graphId:{} cache graphId:{}", message, newGraphId, graphId); - throw new StorageException(message, e); - } - federatedStoreCache.deleteGraphFromCache(graphId); - } + updateTablesWithNewGraphId(newGraphId, graphToUpdate); + + updateCacheWithNewGraphId(newGraphId, graphToUpdate, access); rtn = true; } else { @@ -574,33 +451,78 @@ private boolean changeGraphId(final String graphId, final String newGraphId, fin return rtn; } - private GraphConfig cloneGraphConfigWithNewGraphId(final String newGraphId, final Graph graphToMove) { - return new GraphConfig.Builder() - .json(new GraphSerialisable.Builder().graph(graphToMove).build().getConfig()) - .graphId(newGraphId) + private void updateCacheWithNewGraphId(final String newGraphId, final GraphSerialisable graphToUpdate, final FederatedAccess access) throws StorageException { + //rename graph + GraphSerialisable updatedGraphSerialisable = new GraphSerialisable.Builder(graphToUpdate) + .config(cloneGraphConfigWithNewGraphId(newGraphId, graphToUpdate)) .build(); + + try { + this.put(updatedGraphSerialisable, access); + } catch (final Exception e) { + String s = "Contact Admin for recovery. Error occurred updating graphId. GraphStorage=updated, Cache=outdated graphId."; + LOGGER.error("{} graphStorage graphId:{} cache graphId:{}", s, newGraphId, graphToUpdate.getGraphId()); + throw new StorageException(s, e); + } } - private Graph getGraphToMove(final String graphId, final Predicate accessPredicate) { - Graph graphToMove = null; - for (final Entry> entry : storage.entrySet()) { - if (accessPredicate.test(entry.getKey())) { - //select graph to be moved - for (final Graph graph : entry.getValue()) { - if (graph.getGraphId().equals(graphId)) { - if (isNull(graphToMove)) { - //1st match, store graph and continue. - graphToMove = graph; - } else { - //2nd match. - throw new IllegalStateException("graphIds are unique, but more than one graph was found with the same graphId: " + graphId); - } - } + private void updateTablesWithNewGraphId(final String newGraphId, final GraphSerialisable graphToUpdate) throws StorageException { + //Update Tables + String graphId = graphToUpdate.getGraphId(); + String storeClass = graphToUpdate.getStoreProperties().getStoreClass(); + if (nonNull(storeClass) && storeClass.startsWith(AccumuloStore.class.getPackage().getName())) { + /* + * This logic is only for Accumulo derived stores Only. + * For updating table names to match graphs names. + * + * uk.gov.gchq.gaffer.accumulostore.[AccumuloStore, SingleUseAccumuloStore, + * MiniAccumuloStore, SingleUseMiniAccumuloStore] + */ + try { + Connector connection = TableUtils.getConnector((AccumuloProperties) graphToUpdate.getStoreProperties()); + + if (connection.tableOperations().exists(graphId)) { + connection.tableOperations().offline(graphId); + connection.tableOperations().rename(graphId, newGraphId); + connection.tableOperations().online(newGraphId); } + } catch (final Exception e) { + LOGGER.warn("Error trying to update tables for graphID:{} graphToMove:{}", graphId, graphToUpdate, e); } } - return graphToMove; } + private GraphConfig cloneGraphConfigWithNewGraphId(final String newGraphId, final GraphSerialisable graphToUpdate) { + return new GraphConfig.Builder() + .json(new GraphSerialisable.Builder(graphToUpdate).build().getSerialisedConfig()) + .graphId(newGraphId) + .build(); + } + + private GraphSerialisable getGraphToUpdate(final String graphId, final Predicate accessPredicate) { + Pair fromCache = federatedStoreCache.getFromCache(graphId); + return accessPredicate.test(fromCache.getSecond()) + ? fromCache.getFirst() + : null; + } + /** + * Enum for the Graph Properties or Schema + */ + //TODO FS Why is there an enum? + public enum GraphConfigEnum { + SCHEMA("schema"), PROPERTIES("properties"); + + private final String value; + + GraphConfigEnum(final String value) { + this.value = value; + } + + @Override + public String toString() { + return value; + } + + } } diff --git a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStore.java b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStore.java index f6b19fd137a..9c143b92e9f 100644 --- a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStore.java +++ b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStore.java @@ -21,6 +21,7 @@ import uk.gov.gchq.gaffer.access.predicate.AccessPredicate; import uk.gov.gchq.gaffer.access.predicate.user.NoAccessUserPredicate; +import uk.gov.gchq.gaffer.core.exception.GafferRuntimeException; import uk.gov.gchq.gaffer.data.element.Element; import uk.gov.gchq.gaffer.data.element.id.EntityId; import uk.gov.gchq.gaffer.federatedstore.exception.StorageException; @@ -51,7 +52,6 @@ import uk.gov.gchq.gaffer.federatedstore.operation.handler.impl.FederatedOutputIterableHandler; import uk.gov.gchq.gaffer.federatedstore.operation.handler.impl.FederatedRemoveGraphHandler; import uk.gov.gchq.gaffer.federatedstore.schema.FederatedViewValidator; -import uk.gov.gchq.gaffer.graph.Graph; import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.named.operation.AddNamedOperation; import uk.gov.gchq.gaffer.named.view.AddNamedView; @@ -110,14 +110,14 @@ * * @see #initialise(String, Schema, StoreProperties) * @see Store - * @see Graph + * @see uk.gov.gchq.gaffer.graph.Graph */ public class FederatedStore extends Store { public static final String FEDERATED_STORE_PROCESSED = "FederatedStore.processed."; public static final String FED_STORE_GRAPH_ID_VALUE_NULL_OR_EMPTY = "FedStoreGraphId_value_null_or_empty"; private static final Logger LOGGER = LoggerFactory.getLogger(Store.class); private static final List ALL_IDS = new ArrayList<>(); - private final FederatedGraphStorage graphStorage = new FederatedGraphStorage(); + private FederatedGraphStorage graphStorage; private final int id; private Set customPropertiesAuths; private Boolean isPublicAccessAllowed = Boolean.valueOf(IS_PUBLIC_ACCESS_ALLOWED_DEFAULT); @@ -144,6 +144,7 @@ public FederatedStore() { */ @Override public void initialise(final String graphId, final Schema unused, final StoreProperties properties) throws StoreException { + graphStorage = new FederatedGraphStorage(properties.getCacheServiceNameSuffix()); super.initialise(graphId, new Schema(), properties); customPropertiesAuths = getCustomPropertiesAuths(); isPublicAccessAllowed = Boolean.valueOf(getProperties().getIsPublicAccessAllowed()); @@ -152,7 +153,11 @@ public void initialise(final String graphId, final Schema unused, final StorePro @Override public void setGraphLibrary(final GraphLibrary library) { super.setGraphLibrary(library); - graphStorage.setGraphLibrary(library); + if (nonNull(graphStorage)) { + graphStorage.setGraphLibrary(library); + } else { + throw new GafferRuntimeException("Error adding GraphLibrary, Initialise the FederatedStore first."); + } } /** @@ -329,8 +334,8 @@ public Set getTraits() { * @param operation the requesting operation, graphs are returned only once per operation. * @return the graph collection. */ - public List getGraphs(final User user, final List graphIds, final IFederationOperation operation) { - List rtn = new ArrayList<>(); + public List getGraphs(final User user, final List graphIds, final IFederationOperation operation) { + List rtn = new ArrayList<>(); if (nonNull(operation)) { boolean isFedStoreIdPreexisting = addFedStoreIdToOperation(operation); if (isFedStoreIdPreexisting) { @@ -353,7 +358,7 @@ public List getGraphs(final User user, final List graphIds, final LOGGER.info("A get graphs request was made with empty graphIds"); } String adminAuth = operation.isUserRequestingAdminUsage() ? this.getProperties().getAdminAuth() : null; - rtn.addAll(graphStorage.get(user, graphIds, adminAuth)); + rtn.addAll(new ArrayList<>(graphStorage.get(user, graphIds, adminAuth))); } } else { LOGGER.warn("getGraphs was requested with null Operation, this will return no graphs."); @@ -404,7 +409,7 @@ private static boolean isValueForFedStoreIdNullOrEmpty(final Operation operation public Map getAllGraphsAndAuths(final User user, final List graphIds, final boolean userRequestingAdminUsage) { return userRequestingAdminUsage - ? graphStorage.getAllGraphsAndAccessAsAdmin(user, graphIds, this.getProperties().getAdminAuth()) + ? graphStorage.getAllGraphsAndAccess(user, graphIds, this.getProperties().getAdminAuth()) : graphStorage.getAllGraphsAndAccess(user, graphIds); } @@ -505,11 +510,13 @@ protected Class getRequiredParentSerialiserClass() { @Override protected void startCacheServiceLoader(final StoreProperties properties) { + //this line sets the property map with the default value if required. + properties.setCacheServiceClass(properties.getCacheServiceClass(FederatedStoreProperties.CACHE_SERVICE_CLASS_DEFAULT)); super.startCacheServiceLoader(properties); try { graphStorage.startCacheServiceLoader(); - } catch (final StorageException e) { - throw new RuntimeException(e.getMessage(), e); + } catch (final Exception e) { + throw new RuntimeException("Error occurred while starting cache. " + e.getMessage(), e); } } @@ -557,7 +564,7 @@ public FederatedStore setAdminConfiguredDefaultGraphIds(final List admin return this; } - private List getDefaultGraphs(final User user, final IFederationOperation operation) { + private List getDefaultGraphs(final User user, final IFederationOperation operation) { boolean isAdminRequestingOverridingDefaultGraphs = operation.isUserRequestingAdminUsage() @@ -570,7 +577,7 @@ private List getDefaultGraphs(final User user, final IFederationOperation //This operation has already been processes once, by this store. String keyForProcessedFedStoreId = getKeyForProcessedFedStoreId(); operation.addOption(keyForProcessedFedStoreId, null); // value is null, but key is still found. - List graphs = getGraphs(user, adminConfiguredDefaultGraphIds, operation); + List graphs = getGraphs(user, adminConfiguredDefaultGraphIds, operation); //put it back operation.addOption(keyForProcessedFedStoreId, getValueForProcessedFedStoreId()); return graphs; diff --git a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCache.java b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCache.java index 38bc6af9507..79695777692 100644 --- a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCache.java +++ b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCache.java @@ -25,6 +25,7 @@ import java.util.Set; import static java.util.Objects.isNull; +import static java.util.Objects.nonNull; /** * Wrapper around the {@link uk.gov.gchq.gaffer.cache.CacheServiceLoader} to provide an interface for @@ -32,9 +33,17 @@ */ public class FederatedStoreCache extends Cache> { public static final String ERROR_ADDING_GRAPH_TO_CACHE_GRAPH_ID_S = "Error adding graph to cache. graphId: %s"; + private static final String CACHE_SERVICE_NAME_PREFIX = "federatedStoreGraphs"; public FederatedStoreCache() { - super("federatedStoreGraphs"); + this(null); + } + + public FederatedStoreCache(final String cacheNameSuffix) { + super(String.format("%s%s", CACHE_SERVICE_NAME_PREFIX, + nonNull(cacheNameSuffix) + ? "_" + cacheNameSuffix.toLowerCase() + : "")); } /** @@ -55,7 +64,7 @@ public Set getAllGraphIds() { * @throws CacheOperationException if there was an error trying to add to the cache */ public void addGraphToCache(final Graph graph, final FederatedAccess access, final boolean overwrite) throws CacheOperationException { - addGraphToCache(new GraphSerialisable.Builder().graph(graph).build(), access, overwrite); + addGraphToCache(new GraphSerialisable.Builder(graph).build(), access, overwrite); } /** @@ -81,14 +90,14 @@ public void deleteGraphFromCache(final String graphId) { } /** - * Retrieve the {@link Graph} with the specified ID from the cache. + * Retrieve the {@link GraphSerialisable} with the specified ID from the cache. * * @param graphId the ID of the {@link Graph} to retrieve - * @return the {@link Graph} related to the specified ID + * @return the {@link GraphSerialisable} related to the specified ID */ - public Graph getGraphFromCache(final String graphId) { + public GraphSerialisable getGraphFromCache(final String graphId) { final GraphSerialisable graphSerialisable = getGraphSerialisableFromCache(graphId); - return (isNull(graphSerialisable)) ? null : graphSerialisable.getGraph(); + return (isNull(graphSerialisable)) ? null : graphSerialisable; } /** diff --git a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreProperties.java b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreProperties.java index 37016ba883e..f81719c9af6 100644 --- a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreProperties.java +++ b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2020 Crown Copyright + * Copyright 2017-2022 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ package uk.gov.gchq.gaffer.federatedstore; -import uk.gov.gchq.gaffer.cache.util.CacheProperties; +import uk.gov.gchq.gaffer.cache.impl.HashMapCacheService; import uk.gov.gchq.gaffer.store.StoreProperties; import java.io.InputStream; @@ -40,12 +40,7 @@ public class FederatedStoreProperties extends StoreProperties { public static final String CUSTOM_PROPERTIES_AUTHS = "gaffer.federatedstore.customPropertiesAuths"; public static final String CUSTOM_PROPERTIES_AUTHS_DEFAULT = null; - /** - * This is used.... - * eg.gaffer.federatedstore.cache.service.class="uk.gov.gchq.gaffer.cache.impl.HashMapCacheService" - */ - public static final String CACHE_SERVICE_CLASS = CacheProperties.CACHE_SERVICE_CLASS; - public static final String CACHE_SERVICE_CLASS_DEFAULT = null; + public static final String CACHE_SERVICE_CLASS_DEFAULT = HashMapCacheService.class.getCanonicalName(); public FederatedStoreProperties() { super(FederatedStore.class); @@ -67,11 +62,7 @@ public void setCustomPropertyAuths(final String auths) { set(CUSTOM_PROPERTIES_AUTHS, auths); } - public void setCacheProperties(final String cacheServiceClassString) { - set(CACHE_SERVICE_CLASS, cacheServiceClassString); - } - - public String getCacheProperties() { + public String getCacheServiceClass() { return get(CACHE_SERVICE_CLASS, CACHE_SERVICE_CLASS_DEFAULT); } diff --git a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java index 3fdf3561a70..d7f8b9c8514 100644 --- a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java +++ b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java @@ -18,7 +18,7 @@ import uk.gov.gchq.gaffer.data.elementdefinition.view.View; import uk.gov.gchq.gaffer.federatedstore.FederatedStore; -import uk.gov.gchq.gaffer.graph.Graph; +import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.operation.Operation; import uk.gov.gchq.gaffer.store.Context; import uk.gov.gchq.gaffer.store.Store; @@ -93,16 +93,16 @@ protected void validateViews(final Operation op, final User user, final Store st .graphIds(graphIds) .setUserRequestingAdminUsage(op instanceof IFederationOperation && ((IFederationOperation) op).isUserRequestingAdminUsage()) .build(); - Collection graphs = ((FederatedStore) store).getGraphs(user, graphIds, clonedOp); - for (final Graph graph : graphs) { - String graphId = graph.getGraphId(); + Collection graphSerialisables = ((FederatedStore) store).getGraphs(user, graphIds, clonedOp); + for (final GraphSerialisable graphSerialisable : graphSerialisables) { + String graphId = graphSerialisable.getGraphId(); final boolean graphIdValid = ((FederatedStore) store).getAllGraphIds(user).contains(graphId); // If graphId is not valid, then there is no schema to validate a view against. if (graphIdValid) { currentResult = new ValidationResult(); clonedOp.graphIdsCSV(graphId); // Deprecated function still in use due to Federated GetTraits bug with DYNAMIC_SCHEMA - if (!graph.getStoreTraits().contains(StoreTrait.DYNAMIC_SCHEMA)) { + if (!graphSerialisable.getGraph().getStoreTraits().contains(StoreTrait.DYNAMIC_SCHEMA)) { super.validateViews(clonedOp, user, store, currentResult); } if (currentResult.isValid()) { diff --git a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedAddGraphHandlerParent.java b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedAddGraphHandlerParent.java index 3d8597e5bdf..23091e13134 100644 --- a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedAddGraphHandlerParent.java +++ b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedAddGraphHandlerParent.java @@ -66,8 +66,11 @@ public Void doOperation(final OP operation, final Context context, final Store s throw new OperationException(String.format(ERROR_BUILDING_GRAPH_GRAPH_ID_S, operation.getGraphId()), e); } + Graph graph; try { - //Add GraphSerialisable with Access values. + //created at this position to capture errors before adding to cache. + graph = graphSerialisable.getGraph(); + ((FederatedStore) store).addGraphs( operation.getGraphAuths(), context.getUser().getUserId(), @@ -82,7 +85,7 @@ public Void doOperation(final OP operation, final Context context, final Store s throw new OperationException(String.format(ERROR_ADDING_GRAPH_GRAPH_ID_S, operation.getGraphId()), e); } - addGenericHandler((FederatedStore) store, graphSerialisable.getGraph()); + addGenericHandler((FederatedStore) store, graph); return null; } diff --git a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationHandler.java b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationHandler.java index d843c430f6d..317f6b0402e 100644 --- a/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationHandler.java +++ b/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationHandler.java @@ -20,6 +20,7 @@ import uk.gov.gchq.gaffer.federatedstore.operation.FederatedOperation; import uk.gov.gchq.gaffer.federatedstore.util.FederatedStoreUtil; import uk.gov.gchq.gaffer.graph.Graph; +import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.operation.Operation; import uk.gov.gchq.gaffer.operation.OperationException; import uk.gov.gchq.gaffer.operation.io.Output; @@ -55,9 +56,10 @@ public Object doOperation(final FederatedOperation operation, fin private Iterable getAllGraphResults(final FederatedOperation operation, final Context context, final FederatedStore store) throws OperationException { try { List results; - final Collection graphs = getGraphs(operation, context, store); + final Collection graphs = getGraphs(operation, context, store); results = new ArrayList<>(graphs.size()); - for (final Graph graph : graphs) { + for (final GraphSerialisable graphSerialisable : graphs) { + final Graph graph = graphSerialisable.getGraph(); final Operation updatedOp = FederatedStoreUtil.updateOperationForGraph(operation.getUnClonedPayload(), graph); if (updatedOp != null) { @@ -72,7 +74,7 @@ private Iterable getAllGraphResults(final FederatedOperation oper } } catch (final Exception e) { if (!operation.isSkipFailedFederatedExecution()) { - throw new OperationException(FederatedStoreUtil.createOperationErrorMsg(operation, graph.getGraphId(), e), e); + throw new OperationException(FederatedStoreUtil.createOperationErrorMsg(operation, graphSerialisable.getGraphId(), e), e); } } } @@ -102,8 +104,8 @@ private Object mergeResults(final Iterable resultsFromAllGraphs, final Federated } } - private List getGraphs(final FederatedOperation operation, final Context context, final FederatedStore store) { - List graphs = store.getGraphs(context.getUser(), operation.getGraphIds(), operation); + private List getGraphs(final FederatedOperation operation, final Context context, final FederatedStore store) { + List graphs = store.getGraphs(context.getUser(), operation.getGraphIds(), operation); return nonNull(graphs) ? graphs diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorageTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorageTest.java index 49f85ec09e4..f6afc61dc9e 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorageTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorageTest.java @@ -24,8 +24,11 @@ import uk.gov.gchq.gaffer.access.predicate.NoAccessPredicate; import uk.gov.gchq.gaffer.access.predicate.UnrestrictedAccessPredicate; import uk.gov.gchq.gaffer.accumulostore.AccumuloProperties; +import uk.gov.gchq.gaffer.cache.CacheServiceLoader; +import uk.gov.gchq.gaffer.cache.ICacheService; +import uk.gov.gchq.gaffer.cache.impl.HashMapCacheService; +import uk.gov.gchq.gaffer.cache.util.CacheProperties; import uk.gov.gchq.gaffer.federatedstore.exception.StorageException; -import uk.gov.gchq.gaffer.graph.Graph; import uk.gov.gchq.gaffer.graph.GraphConfig; import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.store.library.GraphLibrary; @@ -37,6 +40,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Properties; import java.util.Set; import static java.util.Collections.singleton; @@ -50,6 +54,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static uk.gov.gchq.gaffer.federatedstore.FederatedGraphStorage.GRAPH_IDS_NOT_VISIBLE; +import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreProperties.CACHE_SERVICE_CLASS_DEFAULT; import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.ACCUMULO_STORE_SINGLE_USE_PROPERTIES; import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.EDGES; import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.ENTITIES; @@ -59,6 +64,7 @@ import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.contextBlankUser; import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.contextTestUser; import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.loadAccumuloStoreProperties; +import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.resetForFederatedTests; import static uk.gov.gchq.gaffer.store.TestTypes.DIRECTED_EITHER; import static uk.gov.gchq.gaffer.user.StoreUser.AUTH_1; import static uk.gov.gchq.gaffer.user.StoreUser.AUTH_2; @@ -93,13 +99,17 @@ public class FederatedGraphStorageTest { @BeforeEach public void setUp() throws Exception { + resetForFederatedTests(); + FederatedStoreProperties federatedStoreProperties = new FederatedStoreProperties(); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_DEFAULT); + CacheServiceLoader.initialise(federatedStoreProperties.getProperties()); graphStorage = new FederatedGraphStorage(); } @Test public void shouldValidateAssumptionStartWithNoGraphs() throws Exception { //when - final Collection graphs = graphStorage.get(nullUser, null); + final Collection graphs = graphStorage.get(nullUser, null); //then assertThat(graphs).isEmpty(); } @@ -184,9 +194,9 @@ public void shouldGetGraphForAddingUser() throws Exception { //access contains adding user graphStorage.put(graphSerialisableA, auth1Access); //when - final Collection allGraphs = graphStorage.getAll(testUser()); + final Collection allGraphs = graphStorage.getAll(testUser()); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -194,7 +204,7 @@ public void shouldNotGetGraphForAddingUserWhenBlockingReadAccessPredicateConfigu //given graphStorage.put(graphSerialisableA, blockingReadAccess); //when - final Collection allGraphs = graphStorage.getAll(testUser()); + final Collection allGraphs = graphStorage.getAll(testUser()); //then assertThat(allGraphs).isEmpty(); } @@ -204,9 +214,9 @@ public void shouldGetGraphForAuthUser() throws Exception { //given graphStorage.put(graphSerialisableA, auth1Access); //when - final Collection allGraphs = graphStorage.getAll(authUser()); + final Collection allGraphs = graphStorage.getAll(authUser()); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -214,9 +224,9 @@ public void shouldGetDisabledGraphWhenGetAll() throws Exception { //given graphStorage.put(graphSerialisableA, disabledByDefaultAccess); //when - final Collection allGraphs = graphStorage.getAll(authUser()); + final Collection allGraphs = graphStorage.getAll(authUser()); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -224,7 +234,7 @@ public void shouldNotGetGraphForBlankUser() throws Exception { //given graphStorage.put(graphSerialisableA, auth1Access); //when - final Collection allGraphs = graphStorage.getAll(blankUser()); + final Collection allGraphs = graphStorage.getAll(blankUser()); //then assertThat(allGraphs).isEmpty(); } @@ -234,9 +244,9 @@ public void shouldGetGraphForBlankUserWhenPermissiveReadAccessPredicateConfigure //given graphStorage.put(graphSerialisableA, permissiveReadAccess); //then - final Collection allGraphs = graphStorage.getAll(blankUser()); + final Collection allGraphs = graphStorage.getAll(blankUser()); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -244,9 +254,9 @@ public void shouldGetGraphForAddingUserWithCorrectId() throws Exception { //given graphStorage.put(graphSerialisableA, auth1Access); //when - final Collection allGraphs = graphStorage.get(testUser(), singletonList(GRAPH_ID_A)); + final Collection allGraphs = graphStorage.get(testUser(), singletonList(GRAPH_ID_A)); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -264,9 +274,9 @@ public void shouldGetGraphForAuthUserWithCorrectId() throws Exception { //given graphStorage.put(graphSerialisableA, auth1Access); //when - final Collection allGraphs = graphStorage.get(authUser(), singletonList(GRAPH_ID_A)); + final Collection allGraphs = graphStorage.get(authUser(), singletonList(GRAPH_ID_A)); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -274,9 +284,9 @@ public void shouldGetDisabledGraphForAuthUserWithCorrectId() throws Exception { //given graphStorage.put(graphSerialisableA, disabledByDefaultAccess); //when - final Collection allGraphs = graphStorage.get(authUser(), singletonList(GRAPH_ID_A)); + final Collection allGraphs = graphStorage.get(authUser(), singletonList(GRAPH_ID_A)); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -284,7 +294,7 @@ public void shouldNotGetDisabledGraphForAuthUserWhenNoIdsProvided() throws Excep //given graphStorage.put(graphSerialisableA, disabledByDefaultAccess); //when - final Collection allGraphs = graphStorage.get(authUser(), null); + final Collection allGraphs = graphStorage.get(authUser(), null); //then assertThat(allGraphs).isEmpty(); } @@ -304,9 +314,9 @@ public void shouldGetGraphForBlankUserWithCorrectIdWhenPermissiveReadAccessPredi //given graphStorage.put(graphSerialisableA, permissiveReadAccess); //when - final Collection allGraphs = graphStorage.get(blankUser(), singletonList(GRAPH_ID_A)); + final Collection allGraphs = graphStorage.get(blankUser(), singletonList(GRAPH_ID_A)); //then - assertThat(allGraphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(allGraphs).containsExactly(graphSerialisableA); } @Test @@ -421,7 +431,7 @@ public void shouldRemoveForAddingUser() throws Exception { graphStorage.put(graphSerialisableA, auth1Access); //when final boolean remove = graphStorage.remove(GRAPH_ID_A, testUser()); - final Collection graphs = graphStorage.getAll(testUser()); + final Collection graphs = graphStorage.getAll(testUser()); //when assertTrue(remove); assertThat(graphs).isEmpty(); @@ -433,10 +443,10 @@ public void shouldNotRemoveForAddingUserWhenBlockingWriteAccessPredicateConfigur graphStorage.put(graphSerialisableA, blockingWriteAccess); //when final boolean remove = graphStorage.remove(GRAPH_ID_A, testUser()); - final Collection graphs = graphStorage.getAll(testUser()); + final Collection graphs = graphStorage.getAll(testUser()); //then assertFalse(remove); - assertThat(graphs).containsExactly(graphSerialisableA.getGraph()); + assertThat(graphs).containsExactly(graphSerialisableA); } @Test @@ -477,14 +487,14 @@ public void shouldGetGraphsInOrder() throws Exception { final List configBA = Arrays.asList(GRAPH_ID_B, GRAPH_ID_A); // When - final Collection graphsAB = graphStorage.get(authUser(), configAB); - final Collection graphsBA = graphStorage.get(authUser(), configBA); + final Collection graphsAB = graphStorage.get(authUser(), configAB); + final Collection graphsBA = graphStorage.get(authUser(), configBA); // Then // A B - assertThat(graphsAB).containsExactly(graphSerialisableA.getGraph(), graphSerialisableB.getGraph()); + assertThat(graphsAB).containsExactly(graphSerialisableA, graphSerialisableB); // B A - assertThat(graphsBA).containsExactly(graphSerialisableB.getGraph(), graphSerialisableA.getGraph()); + assertThat(graphsBA).containsExactly(graphSerialisableB, graphSerialisableA); } @Test @@ -494,7 +504,7 @@ public void shouldNotAddGraphWhenLibraryThrowsExceptionDuringAdd() throws Except String testMockException = "testMockException"; Mockito.doThrow(new RuntimeException(testMockException)) .when(mock) - .checkExisting(GRAPH_ID_A, graphSerialisableA.getDeserialisedSchema(), graphSerialisableA.getDeserialisedProperties()); + .checkExisting(GRAPH_ID_A, graphSerialisableA.getSchema(), graphSerialisableA.getStoreProperties()); graphStorage.setGraphLibrary(mock); //then @@ -551,7 +561,8 @@ public void checkSchemaNotLeakedWhenOverwritingExistingGraph() throws Exception .build(), auth1Access)) - .withMessage("Error adding graph " + GRAPH_ID_A + " to storage due to: " + String.format(FederatedGraphStorage.USER_IS_ATTEMPTING_TO_OVERWRITE, GRAPH_ID_A)) + .withMessageContaining("Error adding graph " + GRAPH_ID_A + " to storage due to: ") + .withMessageContaining(String.format(FederatedGraphStorage.USER_IS_ATTEMPTING_TO_OVERWRITE, GRAPH_ID_A)) .withFailMessage("error message should not contain details about schema") .withMessageNotContainingAny(UNUSUAL_TYPE, GROUP_EDGE, GROUP_ENT); } @@ -584,7 +595,8 @@ public void checkSchemaNotLeakedWhenAlreadyExistsUnderDifferentAccess() throws E // When / Then assertThatExceptionOfType(StorageException.class) .isThrownBy(() -> graphStorage.put(graph, altAuth2Access)) - .withMessage("Error adding graph " + GRAPH_ID_A + " to storage due to: " + String.format(FederatedGraphStorage.USER_IS_ATTEMPTING_TO_OVERWRITE, GRAPH_ID_A)) + .withMessageContaining("Error adding graph " + GRAPH_ID_A + " to storage due to: ") + .withMessageContaining(String.format(FederatedGraphStorage.USER_IS_ATTEMPTING_TO_OVERWRITE, GRAPH_ID_A)) .withFailMessage("error message should not contain details about schema") .withMessageNotContainingAny(UNUSUAL_TYPE, GROUP_EDGE, GROUP_ENT); } @@ -626,7 +638,8 @@ public void checkSchemaNotLeakedWhenAlreadyExistsUnderDifferentAccessWithOtherGr // When / Then assertThatExceptionOfType(StorageException.class) .isThrownBy(() -> graphStorage.put(graph2, auth1Access)) - .withMessage("Error adding graph " + GRAPH_ID_B + " to storage due to: " + String.format(FederatedGraphStorage.USER_IS_ATTEMPTING_TO_OVERWRITE, GRAPH_ID_B)) + .withMessageContaining("Error adding graph " + GRAPH_ID_B + " to storage due to: ") + .withMessageContaining(String.format(FederatedGraphStorage.USER_IS_ATTEMPTING_TO_OVERWRITE, GRAPH_ID_B)) .withFailMessage("error message should not contain details about schema") .withMessageNotContainingAny(UNUSUAL_TYPE, GROUP_EDGE, GROUP_ENT); @@ -652,4 +665,48 @@ private SchemaEntityDefinition getEntityDefinition(final int i) { .vertex(STRING + i) .build(); } + + + @Test + public void shouldAddGraphWithCacheEnabled() throws StorageException { + //given + final Properties serviceLoaderProperties = new Properties(); + serviceLoaderProperties.setProperty(CacheProperties.CACHE_SERVICE_CLASS, HashMapCacheService.class.getName()); + CacheServiceLoader.initialise(serviceLoaderProperties); + graphStorage.startCacheServiceLoader(); + final ICacheService cacheService = CacheServiceLoader.getService(); + + //when + graphStorage.put(graphSerialisableA, auth1Access); + final Collection allIds = graphStorage.getAllIds(authUser()); + + //then + assertEquals(1, cacheService.getCache("federatedStoreGraphs").getAllValues().size()); + assertEquals(1, allIds.size()); + assertEquals(GRAPH_ID_A, allIds.iterator().next()); + + } + + @Test + public void shouldAddGraphReplicatedBetweenInstances() throws StorageException { + //given + final Properties serviceLoaderProperties = new Properties(); + serviceLoaderProperties.setProperty(CacheProperties.CACHE_SERVICE_CLASS, HashMapCacheService.class.getName()); + CacheServiceLoader.initialise(serviceLoaderProperties); + final ICacheService cacheService = CacheServiceLoader.getService(); + final FederatedGraphStorage otherGraphStorage = new FederatedGraphStorage(); + graphStorage.startCacheServiceLoader(); + + //when + otherGraphStorage.startCacheServiceLoader(); + otherGraphStorage.put(graphSerialisableA, auth1Access); + final Collection allIds = graphStorage.getAllIds(authUser()); + + //then + assertEquals(1, cacheService.getCache("federatedStoreGraphs").getAllValues().size()); + assertEquals(1, allIds.size()); + assertEquals(GRAPH_ID_A, allIds.iterator().next()); + + } + } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreAuthTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreAuthTest.java index 26464edc2b3..287f0a0fe5c 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreAuthTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreAuthTest.java @@ -25,7 +25,7 @@ import uk.gov.gchq.gaffer.federatedstore.operation.GetAllGraphIds; import uk.gov.gchq.gaffer.federatedstore.operation.IFederationOperation; import uk.gov.gchq.gaffer.federatedstore.operation.handler.impl.FederatedAddGraphHandler; -import uk.gov.gchq.gaffer.graph.Graph; +import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.operation.OperationException; import uk.gov.gchq.gaffer.store.Context; import uk.gov.gchq.gaffer.store.schema.Schema; @@ -67,7 +67,7 @@ public void setUp() throws Exception { FederatedStoreProperties federatedStoreProperties; federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); federatedStore.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedStoreProperties); @@ -81,9 +81,9 @@ public void shouldAddGraphWithAuth() throws Exception { addGraphWith(AUTH_1, new Schema(), testUser()); //when - Collection testUserGraphs = federatedStore.getGraphs(testUser(), null, mock); - Collection authUserGraphs = federatedStore.getGraphs(authUser(), null, mock); - Collection blankUserGraphs = federatedStore.getGraphs(blankUser(), null, ignore); + Collection testUserGraphs = federatedStore.getGraphs(testUser(), null, mock); + Collection authUserGraphs = federatedStore.getGraphs(authUser(), null, mock); + Collection blankUserGraphs = federatedStore.getGraphs(blankUser(), null, ignore); //then assertThat(authUserGraphs).hasSize(1); @@ -123,7 +123,8 @@ public void shouldNotShowHiddenGraphsInError() throws Exception { final OperationException e = assertThrows(OperationException.class, () -> addGraphWith("nonMatchingAuth", schema, testUser())); assertThat(e).message() - .contains(String.format("Error adding graph %s to storage due to: User is attempting to overwrite a graph within FederatedStore. GraphId: %s", GRAPH_ID_ACCUMULO, GRAPH_ID_ACCUMULO)) + .contains("Error adding graph " + GRAPH_ID_ACCUMULO + " to storage due to:") + .contains("User is attempting to overwrite a graph within FederatedStore. GraphId: " + GRAPH_ID_ACCUMULO) .withFailMessage("error message should not contain details about schema") .doesNotContain(unusualType) .doesNotContain(groupEdge) diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCacheTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCacheTest.java index 14fdb7d34a3..684947c1922 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCacheTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreCacheTest.java @@ -26,6 +26,7 @@ import uk.gov.gchq.gaffer.commonutil.exception.OverwritingException; import uk.gov.gchq.gaffer.graph.Graph; import uk.gov.gchq.gaffer.graph.GraphConfig; +import uk.gov.gchq.gaffer.graph.GraphSerialisable; import java.util.Properties; import java.util.Set; @@ -72,14 +73,14 @@ public void shouldAddAndGetGraphToCache() throws CacheOperationException { federatedStoreCache.addGraphToCache(testGraph, null, false); //when - Graph cached = federatedStoreCache.getGraphFromCache(GRAPH_ID_ACCUMULO); + GraphSerialisable cached = federatedStoreCache.getGraphFromCache(GRAPH_ID_ACCUMULO); //then assertThat(cached) .isNotNull() - .returns(testGraph.getGraphId(), from(Graph::getGraphId)) - .returns(testGraph.getSchema(), from(Graph::getSchema)) - .returns(testGraph.getStoreProperties(), from(Graph::getStoreProperties)); + .returns(testGraph.getGraphId(), from(GraphSerialisable::getGraphId)) + .returns(testGraph.getSchema(), from(GraphSerialisable::getSchema)) + .returns(testGraph.getStoreProperties(), from(GraphSerialisable::getStoreProperties)); } @Test @@ -131,7 +132,7 @@ public void shouldNotThrowExceptionIfGraphIdToGetIsNull() throws CacheOperationE //given federatedStoreCache.addGraphToCache(testGraph, null, false); //when - final Graph graphFromCache = assertDoesNotThrow(() -> federatedStoreCache.getGraphFromCache(null)); + final GraphSerialisable graphFromCache = assertDoesNotThrow(() -> federatedStoreCache.getGraphFromCache(null)); //then assertThat(graphFromCache).isNull(); } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreDefaultGraphsTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreDefaultGraphsTest.java index a0961001b73..b3e81156f0c 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreDefaultGraphsTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreDefaultGraphsTest.java @@ -17,10 +17,12 @@ package uk.gov.gchq.gaffer.federatedstore; import com.google.common.collect.Lists; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import uk.gov.gchq.gaffer.federatedstore.operation.GetAllGraphInfo; +import uk.gov.gchq.gaffer.store.schema.Schema; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.from; @@ -31,6 +33,18 @@ public class FederatedStoreDefaultGraphsTest { + private FederatedStore federatedStore; + + @BeforeEach + public void before() throws Exception { + federatedStore = loadFederatedStoreFrom("DefaultedGraphIds.json"); + assertThat(federatedStore) + .isNotNull() + .returns(Lists.newArrayList("defaultJsonGraphId"), from(FederatedStore::getAdminConfiguredDefaultGraphIds)); + + federatedStore.initialise(FederatedStoreTestUtil.GRAPH_ID_TEST_FEDERATED_STORE, new Schema(), new FederatedStoreProperties()); + } + @Disabled @Test public void testDisableByDefault() { @@ -51,12 +65,6 @@ public void testDisableByDefaultButIsDefaultListOfGraphs() { @Test public void shouldGetDefaultedGraphIdFromJsonConfig() throws Exception { - //Given - FederatedStore federatedStore = loadFederatedStoreFrom("DefaultedGraphIds.json"); - assertThat(federatedStore) - .isNotNull() - .returns(Lists.newArrayList("defaultJsonGraphId"), from(FederatedStore::getAdminConfiguredDefaultGraphIds)); - //when final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> federatedStore.getGraphs(testUser(), null, new GetAllGraphInfo())); //then @@ -65,12 +73,6 @@ public void shouldGetDefaultedGraphIdFromJsonConfig() throws Exception { @Test public void shouldNotChangeExistingDefaultedGraphId() throws Exception { - //Given - FederatedStore federatedStore = loadFederatedStoreFrom("DefaultedGraphIds.json"); - assertThat(federatedStore) - .isNotNull() - .returns(Lists.newArrayList("defaultJsonGraphId"), from(FederatedStore::getAdminConfiguredDefaultGraphIds)); - //when federatedStore.setAdminConfiguredDefaultGraphIdsCSV("other"); diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGetTraitsTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGetTraitsTest.java index 46d8cf00b94..36248a39cb3 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGetTraitsTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGetTraitsTest.java @@ -273,8 +273,7 @@ public void shouldGetCurrentTraitsForAddingUser() throws Exception { @Test public void shouldGetCurrentTraitsForAddingUserButSelectedGraphsOnly() throws Exception { // given - final GraphSerialisable accumuloGraphSerialised2 = new GraphSerialisable.Builder() - .graph(accumuloGraphSerialised.getGraph()) + final GraphSerialisable accumuloGraphSerialised2 = new GraphSerialisable.Builder(accumuloGraphSerialised) .config(new GraphConfig(GRAPH_ID_ACCUMULO + 2)) .build(); @@ -300,8 +299,7 @@ public void shouldGetCurrentTraitsForAddingUserButSelectedGraphsOnly() throws Ex @Test public void shouldGetNonCurrentTraitsForAddingUserButSelectedGraphsOnly() throws Exception { //given - final GraphSerialisable accumuloGraphSerialised2 = new GraphSerialisable.Builder() - .graph(accumuloGraphSerialised.getGraph()) + final GraphSerialisable accumuloGraphSerialised2 = new GraphSerialisable.Builder(accumuloGraphSerialised) .config(new GraphConfig(GRAPH_ID_ACCUMULO + 2)) .build(); diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphLibraryTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphLibraryTest.java index 18aeeeb923e..45d29ae3c37 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphLibraryTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphLibraryTest.java @@ -78,12 +78,12 @@ public void setUp() throws Exception { library.add(GRAPH_ID_B, build, properties.clone()); federatedStore = new FederatedStore(); - federatedStore.setGraphLibrary(library); - FederatedStoreProperties fedProps = new FederatedStoreProperties(); - fedProps.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + FederatedStoreProperties fedProperties = new FederatedStoreProperties(); + fedProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); - federatedStore.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, fedProps); + federatedStore.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, fedProperties); + federatedStore.setGraphLibrary(library); } @Test diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphVisibilityTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphVisibilityTest.java index a256262239a..a3e86c0d7c9 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphVisibilityTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGraphVisibilityTest.java @@ -65,7 +65,7 @@ public static void tearDownCache() { public void setUp() throws Exception { resetForFederatedTests(); FederatedStoreProperties federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); federatedGraph = new Builder() .config(new GraphConfig.Builder() diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreMultiCacheTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreMultiCacheTest.java index 5d836c1ab6c..7218de89b43 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreMultiCacheTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreMultiCacheTest.java @@ -57,7 +57,7 @@ public void setUp() throws Exception { resetForFederatedTests(); federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); federatedStoreProperties.set(HashMapCacheService.STATIC_CACHE, String.valueOf(true)); federatedStore = new FederatedStore(); federatedStore.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedStoreProperties); diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStorePublicAccessTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStorePublicAccessTest.java index 2bc8ff225d5..db06a027e2b 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStorePublicAccessTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStorePublicAccessTest.java @@ -64,7 +64,7 @@ public void setUp() throws Exception { resetForFederatedTests(); federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); store = new FederatedStore(); } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTest.java index 09312bfa5ed..cc3ef68b8c8 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTest.java @@ -164,8 +164,8 @@ public void setUp() throws Exception { store = new FederatedStore(); - store.setGraphLibrary(library); store.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedProperties); + store.setGraphLibrary(library); blankUserContext = contextBlankUser(); blankUser = blankUser(); @@ -189,18 +189,18 @@ public void tearDown() throws Exception { @Test public void shouldLoadGraphsWithIds() throws Exception { //given - final Collection before = store.getGraphs(blankUser, null, new GetAllGraphIds()); + final Collection before = store.getGraphs(blankUser, null, new GetAllGraphIds()); //when addGraphWithIds(ACC_ID_2, ID_PROPS_ACC_2, ID_SCHEMA_EDGE); addGraphWithIds(ACC_ID_1, ID_PROPS_ACC_1, ID_SCHEMA_ENTITY); //then - final Collection graphs = store.getGraphs(blankUser, null, new GetAllGraphIds()); + final Collection graphs = store.getGraphs(blankUser, null, new GetAllGraphIds()); assertThat(before).size().isEqualTo(0); final List graphNames = asList(ACC_ID_1, ACC_ID_2); - for (final Graph graph : graphs) { + for (final GraphSerialisable graph : graphs) { assertThat(graphNames).contains(graph.getGraphId()); } assertThat(graphs).size().isEqualTo(2); @@ -591,7 +591,7 @@ public void shouldAddGraphWithPropertiesAndSchemaFromGraphLibrary() throws Excep // Then assertThat(store.getGraphs(blankUser, null, new GetAllGraphIds())).hasSize(1); - final Graph graph = store.getGraphs(blankUser, getCleanStrings(ACC_ID_2), new GetAllGraphIds()).iterator().next(); + final GraphSerialisable graph = store.getGraphs(blankUser, getCleanStrings(ACC_ID_2), new GetAllGraphIds()).iterator().next(); assertThat(getSchemaFromPath(SCHEMA_ENTITY_BASIC_JSON)).isEqualTo(graph.getSchema()); assertThat(graph.getStoreProperties()).isEqualTo(propertiesAlt); } @@ -731,14 +731,14 @@ public void shouldReturnSpecificGraphsFromCSVString() throws Exception { final Collection unexpectedGraphs = graphLists.get(1); // When - final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings("mockGraphId1,mockGraphId2,mockGraphId4"), new GetAllGraphIds()); + final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings("mockGraphId1,mockGraphId2,mockGraphId4"), new GetAllGraphIds()); // Then assertThat(returnedGraphs) .hasSize(3) - .containsAll(toGraphs(expectedGraphs)); + .containsAll(expectedGraphs); - assertThat(checkUnexpected(toGraphs(unexpectedGraphs), returnedGraphs)).isFalse(); + assertThat(returnedGraphs).doesNotContainAnyElementsOf(unexpectedGraphs); } @Test @@ -747,10 +747,10 @@ public void shouldReturnEnabledByDefaultGraphsForNullString() throws Exception { populateGraphs(); // When - final Collection returnedGraphs = store.getGraphs(blankUser, null, new GetAllGraphIds()); + final Collection returnedGraphs = store.getGraphs(blankUser, null, new GetAllGraphIds()); // Then - final Set graphIds = returnedGraphs.stream().map(Graph::getGraphId).collect(Collectors.toSet()); + final Set graphIds = returnedGraphs.stream().map(GraphSerialisable::getGraphId).collect(Collectors.toSet()); assertThat(graphIds).containsExactly("mockGraphId0", "mockGraphId2", "mockGraphId4"); } @@ -760,10 +760,10 @@ public void shouldReturnNotReturnEnabledOrDisabledGraphsWhenNotInCsv() throws Ex populateGraphs(); // When - final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings("mockGraphId0,mockGraphId1"), new GetAllGraphIds()); + final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings("mockGraphId0,mockGraphId1"), new GetAllGraphIds()); // Then - final Set graphIds = returnedGraphs.stream().map(Graph::getGraphId).collect(Collectors.toSet()); + final Set graphIds = returnedGraphs.stream().map(GraphSerialisable::getGraphId).collect(Collectors.toSet()); assertThat(graphIds).containsExactly("mockGraphId0", "mockGraphId1"); } @@ -775,7 +775,7 @@ public void shouldReturnNoGraphsFromEmptyString() throws Exception { final Collection expectedGraphs = graphLists.get(0); // When - final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings(""), new GetAllGraphIds()); + final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings(""), new GetAllGraphIds()); // Then assertThat(returnedGraphs).withFailMessage(returnedGraphs.toString()).isEmpty(); @@ -790,80 +790,60 @@ public void shouldReturnGraphsWithLeadingCommaString() throws Exception { final Collection unexpectedGraphs = graphLists.get(1); // When - final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings(",mockGraphId2,mockGraphId4"), new GetAllGraphIds()); + final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings(",mockGraphId2,mockGraphId4"), new GetAllGraphIds()); // Then assertThat(returnedGraphs) .hasSize(2) - .containsAll(toGraphs(expectedGraphs)); + .containsAll(expectedGraphs); - assertThat(checkUnexpected(toGraphs(unexpectedGraphs), returnedGraphs)).isFalse(); + assertThat(returnedGraphs).doesNotContainAnyElementsOf(unexpectedGraphs); } @Test public void shouldAddGraphIdWithAuths() throws Exception { // Given - final Graph fedGraph = new Graph.Builder() - .config(new GraphConfig.Builder() - .graphId(GRAPH_ID_TEST_FEDERATED_STORE) - .library(library) - .build()) - .addStoreProperties(federatedProperties) - .build(); - - addGraphWithIds(ACC_ID_2, ID_PROPS_ACC_2, ID_SCHEMA_ENTITY); - - library.add(ACC_ID_2, getSchemaFromPath(SCHEMA_ENTITY_BASIC_JSON), propertiesAlt); + library.add(ACC_ID_1, getSchemaFromPath(SCHEMA_ENTITY_BASIC_JSON), propertiesAlt); // When - int before = 0; - for (@SuppressWarnings("unused") final String ignore : fedGraph.execute( - new GetAllGraphIds(), - blankUser)) { - before++; - } + Iterable before = store.execute(new GetAllGraphIds(), contextBlankUser()); - fedGraph.execute( - new AddGraph.Builder() + store.execute(new AddGraph.Builder() .graphAuths("auth") - .graphId(ACC_ID_2) + .graphId(ACC_ID_1) .build(), - blankUser); + contextBlankUser()); - int after = 0; - for (@SuppressWarnings("unused") final String ignore : fedGraph.execute( - new GetAllGraphIds(), - blankUser)) { - after++; - } + Iterable after = (Iterable) store.execute(new GetAllGraphIds(), contextBlankUser()); - fedGraph.execute(new AddElements.Builder() + store.execute(new AddElements.Builder() .input(new Entity.Builder() .group("BasicEntity") .vertex("v1") .build()) .build(), - blankUser); + contextBlankUser()); - final Iterable elements = fedGraph.execute( + final Iterable elements = store.execute( new GetAllElements(), - new User.Builder() + new Context(new User.Builder() .userId(TEST_USER_ID + "Other") .opAuth("auth") - .build()); + .build())); - final Iterable elements2 = fedGraph.execute(new GetAllElements(), - new User.Builder() + final Iterable elements2 = store.execute(new GetAllElements(), + new Context(new User.Builder() .userId(TEST_USER_ID + "Other") .opAuths("x") - .build()); - assertThat(elements2).isEmpty(); + .build())); + // Then - assertThat(before).isEqualTo(0); - assertThat(after).isEqualTo(1); + assertThat(before).isEmpty(); + assertThat(after).containsExactly(ACC_ID_1); assertThat(elements).isNotNull(); assertThat(elements.iterator()).hasNext(); + assertThat(elements2).isEmpty(); } @Test @@ -921,14 +901,14 @@ public void shouldReturnASingleGraph() throws Exception { final Collection unexpectedGraphs = graphLists.get(1); // When - final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings("mockGraphId1"), new GetAllGraphIds()); + final Collection returnedGraphs = store.getGraphs(blankUser, getCleanStrings("mockGraphId1"), new GetAllGraphIds()); // Then assertThat(returnedGraphs) .hasSize(1) - .containsAll(toGraphs(expectedGraphs)); + .containsAll(expectedGraphs); - assertThat(checkUnexpected(toGraphs(unexpectedGraphs), returnedGraphs)).isFalse(); + assertThat(returnedGraphs).doesNotContainAnyElementsOf(unexpectedGraphs); } private List toGraphs(final Collection graphSerialisables) { @@ -937,7 +917,7 @@ private List toGraphs(final Collection graphSerialisab @Test public void shouldThrowExceptionWithInvalidCacheClass() throws StoreException { - federatedProperties.setCacheProperties(INVALID_CACHE_SERVICE_CLASS_STRING); + federatedProperties.setCacheServiceClass(INVALID_CACHE_SERVICE_CLASS_STRING); CacheServiceLoader.shutdown(); @@ -948,7 +928,8 @@ public void shouldThrowExceptionWithInvalidCacheClass() throws StoreException { @Test public void shouldReuseGraphsAlreadyInCache() throws Exception { // Check cache is empty - federatedProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + CacheServiceLoader.shutdown(); + federatedProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); assertThat(CacheServiceLoader.getService()).isNull(); // initialise FedStore @@ -980,8 +961,9 @@ public void shouldReuseGraphsAlreadyInCache() throws Exception { @Test public void shouldInitialiseWithCache() throws StoreException { + CacheServiceLoader.shutdown(); assertThat(CacheServiceLoader.getService()).isNull(); - federatedProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); assertThat(CacheServiceLoader.getService()).isNull(); store.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedProperties); assertThat(CacheServiceLoader.getService()).isNotNull(); @@ -989,7 +971,7 @@ public void shouldInitialiseWithCache() throws StoreException { @Test public void shouldThrowExceptionWithoutInitialisation() throws StoreException { - federatedProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); store.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedProperties); // Given @@ -1004,7 +986,7 @@ public void shouldThrowExceptionWithoutInitialisation() throws StoreException { // When / Then assertThatExceptionOfType(Exception.class) .isThrownBy(() -> store.addGraphs(null, TEST_USER_ID, false, graphToAdd)) - .withMessageContaining("No cache has been set"); + .withStackTraceContaining("Cache is not enabled, check it was Initialised"); } @Test @@ -1022,7 +1004,7 @@ public void shouldNotThrowExceptionWhenInitialisedWithNoCacheClassInProperties() @Test public void shouldAddGraphsToCache() throws Exception { - federatedProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); store.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedProperties); // Given @@ -1039,11 +1021,11 @@ public void shouldAddGraphsToCache() throws Exception { assertThat(store.getGraphs(blankUser, getCleanStrings(ACC_ID_1), new GetAllGraphIds())).hasSize(1); // When - final Collection storeGraphs = store.getGraphs(blankUser, null, new GetAllGraphIds()); + final Collection storeGraphs = store.getGraphs(blankUser, null, new GetAllGraphIds()); // Then assertThat(CacheServiceLoader.getService().getAllKeysFromCache(CACHE_SERVICE_NAME)).contains(ACC_ID_1); - assertThat(storeGraphs).contains(graphToAdd.getGraph()); + assertThat(storeGraphs).contains(graphToAdd); // When store = new FederatedStore(); @@ -1054,7 +1036,7 @@ public void shouldAddGraphsToCache() throws Exception { @Test public void shouldAddMultipleGraphsToCache() throws Exception { - federatedProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); store.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedProperties); // Given @@ -1095,7 +1077,7 @@ public void shouldAddAGraphRemoveAGraphAndBeAbleToReuseTheGraphId() throws Excep addGraphWithPaths(ACC_ID_2, propertiesAlt, SCHEMA_EDGE_BASIC_JSON); // Then - final Collection graphs = store.getGraphs(blankUserContext.getUser(), getCleanStrings(ACC_ID_2), new GetAllGraphIds()); + final Collection graphs = store.getGraphs(blankUserContext.getUser(), getCleanStrings(ACC_ID_2), new GetAllGraphIds()); assertThat(graphs).hasSize(1); JsonAssert.assertEquals(JSONSerialiser.serialise(Schema.fromJson(StreamUtil.openStream(getClass(), SCHEMA_EDGE_BASIC_JSON))), JSONSerialiser.serialise(graphs.iterator().next().getSchema())); @@ -1104,7 +1086,8 @@ public void shouldAddAGraphRemoveAGraphAndBeAbleToReuseTheGraphId() throws Excep @Test public void shouldNotAddGraphToLibraryWhenReinitialisingFederatedStoreWithGraphFromCache() throws Exception { // Check cache is empty - federatedProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + CacheServiceLoader.shutdown(); + federatedProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); assertThat(CacheServiceLoader.getService()).isNull(); // initialise FedStore @@ -1113,7 +1096,7 @@ public void shouldNotAddGraphToLibraryWhenReinitialisingFederatedStoreWithGraphF // add something so it will be in the cache final GraphSerialisable graphToAdd = new GraphSerialisable.Builder() .config(new GraphConfig(ACC_ID_1)) - .properties(properties1) + .properties(properties1.clone()) .schema(StreamUtil.openStream(FederatedStoreTest.class, SCHEMA_EDGE_BASIC_JSON)) .build(); @@ -1128,10 +1111,10 @@ public void shouldNotAddGraphToLibraryWhenReinitialisingFederatedStoreWithGraphF // restart the store store = new FederatedStore(); - // clear and set the GraphLibrary again - store.setGraphLibrary(library); // initialise the FedStore store.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedProperties); + // clear and set the GraphLibrary again + store.setGraphLibrary(library); // check is in the cache still assertThat(CacheServiceLoader.getService().getAllKeysFromCache(CACHE_SERVICE_NAME)) diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTestUtil.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTestUtil.java index 747b6e341f0..05503043536 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTestUtil.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTestUtil.java @@ -37,6 +37,8 @@ import uk.gov.gchq.gaffer.store.schema.Schema; import uk.gov.gchq.gaffer.store.schema.SchemaEdgeDefinition; import uk.gov.gchq.gaffer.store.schema.SchemaEntityDefinition; +import uk.gov.gchq.gaffer.store.schema.TypeDefinition; +import uk.gov.gchq.koryphe.impl.binaryoperator.Sum; import java.io.IOException; import java.io.InputStream; @@ -85,9 +87,9 @@ public final class FederatedStoreTestUtil { public static final String FORMAT_VALUE_STRING = "value%s"; public static final String VALUE_1 = value(1); public static final String VALUE_2 = value(2); + public static final String INTEGER = "integer"; static final String CACHE_SERVICE_CLASS_STRING = "uk.gov.gchq.gaffer.cache.impl.HashMapCacheService"; static final Set GRAPH_AUTHS_ALL_USERS = ImmutableSet.of(ALL_USERS); - public static final String INTEGER = "integer"; private FederatedStoreTestUtil() { //no instance @@ -129,7 +131,7 @@ public static void addGraph(final FederatedStore federatedStore, final String gr .build()); } - public static Context contextBlankUser() { + public static Context contextBlankUser() { return new Context(blankUser()); } @@ -162,6 +164,17 @@ public static SchemaEntityDefinition entityBasicDefinition() { .build(); } + public static Schema basicEntitySchema() { + return new Schema.Builder() + .entity(GROUP_BASIC_ENTITY, entityBasicDefinition()) + .type(STRING, String.class) + .type(INTEGER, new TypeDefinition.Builder() + .clazz(Integer.class) + .aggregateFunction(new Sum()) + .build()) + .build(); + } + public static SchemaEdgeDefinition edgeDefinition() { return new SchemaEdgeDefinition.Builder() .source(STRING) @@ -184,6 +197,6 @@ public static Context contextTestUser() { } public static ListAssert assertThatGraphs(final Collection graphs, final GraphSerialisable... values) { - return assertThat(graphs.stream().map(g -> new GraphSerialisable.Builder().graph(g).build()).collect(Collectors.toList())).containsExactlyInAnyOrder(values); + return assertThat(graphs.stream().map(g -> new GraphSerialisable.Builder(g).build()).collect(Collectors.toList())).containsExactlyInAnyOrder(values); } } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreWrongGraphIDsTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreWrongGraphIDsTest.java index 32efe5adce9..86f51b52e3f 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreWrongGraphIDsTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreWrongGraphIDsTest.java @@ -69,7 +69,7 @@ public void setUp() throws Exception { federatedStore = new FederatedStore(); FederatedStoreProperties fedProps = new FederatedStoreProperties(); - fedProps.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + fedProps.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); federatedStore.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, fedProps); } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/AbstractStandaloneFederatedStoreIT.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/AbstractStandaloneFederatedStoreIT.java index 0461b4e4a9a..d9d145e4aee 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/AbstractStandaloneFederatedStoreIT.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/AbstractStandaloneFederatedStoreIT.java @@ -27,6 +27,8 @@ import uk.gov.gchq.gaffer.store.schema.Schema; import uk.gov.gchq.gaffer.user.User; +import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.resetForFederatedTests; + public abstract class AbstractStandaloneFederatedStoreIT { protected Graph graph; @@ -43,6 +45,7 @@ private static void _tearDown() throws Exception { @BeforeEach public void setUp() throws Exception { + resetForFederatedTests(); createGraph(); _setUp(); } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedAdminIT.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedAdminIT.java index bbadf43bfd2..15b843b12b4 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedAdminIT.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedAdminIT.java @@ -464,16 +464,17 @@ public void shouldChangeGraphIdForNonOwnedGraphAsAdminWhenRequestingAdminAccess( assertThat(graph.execute(new GetAllGraphIds(), user)).contains(GRAPH_ID_A); //when + final String graphIdB = GRAPH_ID_B + 17456; //TODO FS this hides a issue of graphId persisting for tests. final Boolean changed = graph.execute(new ChangeGraphId.Builder() .graphId(GRAPH_ID_A) - .newGraphId(GRAPH_ID_B) + .newGraphId(graphIdB) .setUserRequestingAdminUsage(true) .build(), ADMIN_USER); //then assertThat(changed).isTrue(); assertThat(graph.execute(new GetAllGraphIds(), user)).doesNotContain(GRAPH_ID_A) - .contains(GRAPH_ID_B); + .contains(graphIdB); } @@ -565,19 +566,20 @@ public void shouldChangeGraphIdInStorage() throws Exception { @Test public void shouldChangeGraphIdInCache() throws Exception { //given - String newName = "newName"; + String newName = "newName" + 23452335; //TODO FS this hides a issue of graphId persisting for tests. FederatedStoreCache federatedStoreCache = new FederatedStoreCache(); - final String graphA = GRAPH_ID_A; + graph.execute(new AddGraph.Builder() - .graphId(graphA) + .graphId(GRAPH_ID_A) .schema(new Schema()) .storeProperties(ACCUMULO_PROPERTIES) .build(), user); - assertThat(graph.execute(new GetAllGraphIds(), user)).contains(graphA); + + assertThat(graph.execute(new GetAllGraphIds(), user)).contains(GRAPH_ID_A); //when final Boolean changed = graph.execute(new ChangeGraphId.Builder() - .graphId(graphA) + .graphId(GRAPH_ID_A) .newGraphId(newName) .build(), user); diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedStoreRecursionIT.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedStoreRecursionIT.java index 8083a665ba9..2f2e55f59cb 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedStoreRecursionIT.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/integration/FederatedStoreRecursionIT.java @@ -161,12 +161,13 @@ private void createInnerProxyToOuterFederatedStore() throws OperationException { .build())), user); } - private void createTheInnerFederatedStore() throws - OperationException { + private void createTheInnerFederatedStore() throws OperationException { + FederatedStoreProperties properties = new FederatedStoreProperties(); + properties.setCacheServiceNameSuffix(INNER_FEDERATED_GRAPH); proxyToRestServiceFederatedGraph.execute(new AddGraph.Builder() .graphId(INNER_FEDERATED_GRAPH) .schema(new Schema()) - .storeProperties(new FederatedStoreProperties()) + .storeProperties(properties) .build(), user); } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedOperationHandlerTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedOperationHandlerTest.java index fd861b70001..298de0afd07 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedOperationHandlerTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedOperationHandlerTest.java @@ -36,6 +36,7 @@ import uk.gov.gchq.gaffer.federatedstore.operation.handler.impl.FederatedOperationHandler; import uk.gov.gchq.gaffer.graph.Graph; import uk.gov.gchq.gaffer.graph.GraphConfig; +import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.operation.Operation; import uk.gov.gchq.gaffer.operation.OperationChain; import uk.gov.gchq.gaffer.operation.OperationException; @@ -72,6 +73,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil.resetForFederatedTests; import static uk.gov.gchq.gaffer.federatedstore.util.FederatedStoreUtil.getCleanStrings; import static uk.gov.gchq.gaffer.federatedstore.util.FederatedStoreUtil.getFederatedOperation; import static uk.gov.gchq.gaffer.federatedstore.util.FederatedStoreUtil.getHardCodedDefaultMergeFunction; @@ -89,18 +91,21 @@ public class FederatedOperationHandlerTest { private Store mockStore2; private Store mockStore3; private Store mockStore4; - private Graph graph1; - private Graph graph2; - private Graph graph3; - private Graph graph4; + private GraphSerialisable graph1; + private GraphSerialisable graph2; + private GraphSerialisable graph3; + private GraphSerialisable graph4; @BeforeEach public void setUp() throws Exception { + resetForFederatedTests(); + testUser = testUser(); context = new Context(testUser); Schema unusedSchema = new Schema.Builder().build(); StoreProperties storeProperties = new StoreProperties(); + storeProperties.setStoreClass("MockedStore"); mockStore1 = getMockStoreThatAlwaysReturns(unusedSchema, storeProperties, output1); mockStore2 = getMockStoreThatAlwaysReturns(unusedSchema, storeProperties, output2); mockStore3 = getMockStoreThatAlwaysReturns(unusedSchema, storeProperties, output3); @@ -155,11 +160,20 @@ public final void shouldGetAllResultsFromGraphIds() throws Exception { validateMergeResultsFromFieldObjects(results, output1, output3); } - private Graph getGraphWithMockStore(final Store mockStore) { - return new Graph.Builder() + private GraphSerialisable getGraphWithMockStore(final Store mockStore) { + + final Graph graph = new Graph.Builder() .config(new GraphConfig(TEST_GRAPH_ID)) .store(mockStore) .build(); + + //TODO FS final GraphSerialisable Test bug, this is the only reason why it loosing final. can this test class be rewritten. + final GraphSerialisable mock = mock(GraphSerialisable.class); + when(mock.getGraph()).thenReturn(graph); + when(mock.getGraphId()).thenReturn(TEST_GRAPH_ID); + when(mock.getConfig()).thenReturn(new GraphConfig(TEST_GRAPH_ID)); + + return mock; } private Store getMockStoreThatAlwaysReturns(final Schema schema, final StoreProperties storeProperties, final Object willReturn) throws uk.gov.gchq.gaffer.operation.OperationException { @@ -279,11 +293,11 @@ public final void shouldPassGlobalsOnToSubstores() throws Exception { Store mockStore1 = getMockStore(unusedSchema, storeProperties); Store mockStore2 = getMockStore(concreteSchema, storeProperties); - Graph graph1 = getGraphWithMockStore(mockStore1); - Graph graph2 = getGraphWithMockStore(mockStore2); + GraphSerialisable graph1 = getGraphWithMockStore(mockStore1); + GraphSerialisable graph2 = getGraphWithMockStore(mockStore2); FederatedStore mockStore = mock(FederatedStore.class); - List linkedGraphs = asList(graph1, graph2); + List linkedGraphs = asList(graph1, graph2); when(mockStore.getGraphs(eq(testUser), eq(null), any())).thenReturn(linkedGraphs); @@ -317,7 +331,7 @@ public void shouldReturnEmptyOutputOfTypeIterableWhenResultsIsNull() throws Exce given(mockStore.execute(any(OperationChain.class), any(Context.class))).willReturn(null); FederatedStore federatedStore = Mockito.mock(FederatedStore.class); - List filteredGraphs = singletonList(getGraphWithMockStore(mockStore)); + List filteredGraphs = singletonList(getGraphWithMockStore(mockStore)); given(federatedStore.getGraphs(eq(testUser), eq((List) null), any(FederatedOperation.class))).willReturn(filteredGraphs); // When @@ -342,7 +356,7 @@ public void shouldProcessAIterableOfBooleanFromMultipleGraphs() throws Exception given(mockStore.execute(any(OperationChain.class), any(Context.class))).willReturn(singletonList(true)); FederatedStore federatedStore = Mockito.mock(FederatedStore.class); - List threeGraphsOfBoolean = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); + List threeGraphsOfBoolean = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); given(federatedStore.getGraphs(eq(testUser), eq((List) null), any(FederatedOperation.class))).willReturn(threeGraphsOfBoolean); // When @@ -364,7 +378,7 @@ public void shouldProcessABooleanNotJustIterablesFromMultipleGraphs() throws Exc given(mockStore.execute(any(OperationChain.class), any(Context.class))).willReturn(true); FederatedStore federatedStore = Mockito.mock(FederatedStore.class); - List threeGraphsOfBoolean = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); + List threeGraphsOfBoolean = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); given(federatedStore.getGraphs(eq(testUser), eq((List) null), any(FederatedOperation.class))).willReturn(threeGraphsOfBoolean); // When @@ -389,7 +403,7 @@ public void shouldProcessAIterableOfIntegersFromMultipleGraphs() throws Exceptio given(mockStore.execute(any(OperationChain.class), any(Context.class))).willReturn(singletonList(123)); FederatedStore federatedStore = Mockito.mock(FederatedStore.class); - List threeGraphsOfBoolean = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); + List threeGraphsOfBoolean = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); given(federatedStore.getGraphs(eq(testUser), eq((List) null), any(FederatedOperation.class))).willReturn(threeGraphsOfBoolean); // When @@ -414,7 +428,7 @@ public void shouldProcessAIterableOfNullFromMultipleGraphs() throws Exception { given(mockStore.execute(any(OperationChain.class), any(Context.class))).willReturn(singletonList((Object) null)); FederatedStore federatedStore = Mockito.mock(FederatedStore.class); - List threeGraphsOfNull = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); + List threeGraphsOfNull = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); given(federatedStore.getGraphs(eq(testUser), eq((List) null), any(FederatedOperation.class))).willReturn(threeGraphsOfNull); // When @@ -440,7 +454,7 @@ public void shouldReturnNulledOutputOfTypeIterableWhenResultsContainsOnlyNull() given(mockStore.execute(any(OperationChain.class), any(Context.class))).willReturn(null); FederatedStore federatedStore = Mockito.mock(FederatedStore.class); - List threeGraphsOfNull = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); + List threeGraphsOfNull = asList(getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore), getGraphWithMockStore(mockStore)); given(federatedStore.getGraphs(eq(testUser), eq((List) null), any(FederatedOperation.class))).willReturn(threeGraphsOfNull); // When diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphHandlerTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphHandlerTest.java index 8f362626f4e..a569f66d844 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphHandlerTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphHandlerTest.java @@ -30,7 +30,7 @@ import uk.gov.gchq.gaffer.federatedstore.FederatedStore; import uk.gov.gchq.gaffer.federatedstore.FederatedStoreProperties; import uk.gov.gchq.gaffer.federatedstore.operation.AddGraph; -import uk.gov.gchq.gaffer.graph.Graph; +import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.hdfs.operation.AddElementsFromHdfs; import uk.gov.gchq.gaffer.operation.impl.get.GetAllElements; import uk.gov.gchq.gaffer.store.Context; @@ -74,7 +74,7 @@ public void setUp() throws Exception { CacheServiceLoader.shutdown(); this.store = new FederatedStore(); federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); testUser = testUser(); authUser = authUser(); @@ -103,10 +103,10 @@ public void shouldAddGraph() throws Exception { new Context(testUser), store); - Collection graphs = store.getGraphs(testUser, null, new AddGraph()); + Collection graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(1); - final Graph next = graphs.iterator().next(); + final GraphSerialisable next = graphs.iterator().next(); assertThat(next.getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); assertThat(next.getSchema()).isEqualTo(expectedSchema); @@ -122,7 +122,7 @@ public void shouldAddGraph() throws Exception { graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(2); - final Iterator iterator = graphs.iterator(); + final Iterator iterator = graphs.iterator(); final HashSet set = new HashSet<>(); while (iterator.hasNext()) { set.add(iterator.next().getGraphId()); @@ -147,10 +147,10 @@ public void shouldAddDisabledByDefaultGraph() throws Exception { new Context(testUser), store); - final Collection enabledGraphs = store.getGraphs(testUser, null, new AddGraph()); + final Collection enabledGraphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(enabledGraphs).isEmpty(); - final Collection expectedGraphs = store.getGraphs(testUser, getCleanStrings(EXPECTED_GRAPH_ID), new AddGraph()); + final Collection expectedGraphs = store.getGraphs(testUser, getCleanStrings(EXPECTED_GRAPH_ID), new AddGraph()); assertThat(expectedGraphs).hasSize(1); assertThat(expectedGraphs.iterator().next().getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); } @@ -174,10 +174,10 @@ public void shouldAddGraphUsingLibrary() throws Exception { new Context(testUser), store); - Collection graphs = store.getGraphs(testUser, null, new AddGraph()); + Collection graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(1); - final Graph next = graphs.iterator().next(); + final GraphSerialisable next = graphs.iterator().next(); assertThat(next.getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); assertThat(next.getSchema()).isEqualTo(expectedSchema); @@ -195,7 +195,7 @@ public void shouldAddGraphUsingLibrary() throws Exception { graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(2); - final Iterator iterator = graphs.iterator(); + final Iterator iterator = graphs.iterator(); final HashSet set = new HashSet<>(); while (iterator.hasNext()) { set.add(iterator.next().getGraphId()); @@ -213,10 +213,10 @@ public void shouldThrowWhenOverwriteGraphIsDifferent() throws Exception { .type("string", String.class) .build(); - assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); - store.initialise(FEDERATEDSTORE_GRAPH_ID, new Schema(), federatedStoreProperties); + assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); + final FederatedAddGraphHandler federatedAddGraphHandler = new FederatedAddGraphHandler(); federatedAddGraphHandler.doOperation( @@ -246,10 +246,10 @@ public void shouldThrowWhenOverwriteGraphIsDifferent() throws Exception { public void shouldThrowWhenOverwriteGraphIsSameAndAccessIsDifferent() throws Exception { final Schema expectedSchema = new Schema.Builder().build(); - assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); - store.initialise(FEDERATEDSTORE_GRAPH_ID, new Schema(), federatedStoreProperties); + assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); + final FederatedAddGraphHandler federatedAddGraphHandler = new FederatedAddGraphHandler(); federatedAddGraphHandler.doOperation( @@ -305,7 +305,7 @@ public void shouldAddGraphIDOnlyWithAuths() throws Exception { new Context(authUser), store); - final Collection graphs = store.getGraphs(authUser, null, new AddGraph()); + final Collection graphs = store.getGraphs(authUser, null, new AddGraph()); assertThat(graphs).hasSize(1); assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); assertThat(graphs.iterator().next().getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphWithHooksHandlerTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphWithHooksHandlerTest.java index bbe8697030b..905f7cd7283 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphWithHooksHandlerTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAddGraphWithHooksHandlerTest.java @@ -30,7 +30,7 @@ import uk.gov.gchq.gaffer.federatedstore.FederatedStoreProperties; import uk.gov.gchq.gaffer.federatedstore.operation.AddGraph; import uk.gov.gchq.gaffer.federatedstore.operation.AddGraphWithHooks; -import uk.gov.gchq.gaffer.graph.Graph; +import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.graph.hook.GraphHook; import uk.gov.gchq.gaffer.graph.hook.Log4jLogger; import uk.gov.gchq.gaffer.operation.OperationException; @@ -50,6 +50,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -77,7 +78,7 @@ public void setUp() throws Exception { CacheServiceLoader.shutdown(); this.store = new FederatedStore(); federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); testUser = testUser(); authUser = authUser(); @@ -106,10 +107,10 @@ public void shouldAddGraph() throws Exception { new Context(testUser), store); - Collection graphs = store.getGraphs(testUser, null, new AddGraph()); + Collection graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(1); - final Graph next = graphs.iterator().next(); + final GraphSerialisable next = graphs.iterator().next(); assertThat(next.getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); assertThat(next.getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); @@ -125,7 +126,7 @@ public void shouldAddGraph() throws Exception { graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(2); - final Iterator iterator = graphs.iterator(); + final Iterator iterator = graphs.iterator(); final HashSet set = new HashSet<>(); while (iterator.hasNext()) { set.add(iterator.next().getGraphId()); @@ -153,10 +154,10 @@ public void shouldAddGraphUsingLibrary() throws Exception { new Context(testUser), store); - Collection graphs = store.getGraphs(testUser, null, new AddGraph()); + Collection graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(1); - final Graph next = graphs.iterator().next(); + final GraphSerialisable next = graphs.iterator().next(); assertThat(next.getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); assertThat(next.getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); @@ -174,7 +175,7 @@ public void shouldAddGraphUsingLibrary() throws Exception { graphs = store.getGraphs(testUser, null, new AddGraph()); assertThat(graphs).hasSize(2); - final Iterator iterator = graphs.iterator(); + final Iterator iterator = graphs.iterator(); final HashSet set = new HashSet<>(); while (iterator.hasNext()) { set.add(iterator.next().getGraphId()); @@ -192,10 +193,10 @@ public void shouldThrowWhenOverwriteGraphIsDifferent() throws Exception { .type("string", String.class) .build(); - assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); - store.initialise(FEDERATEDSTORE_GRAPH_ID, new Schema(), federatedStoreProperties); + assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); + final FederatedAddGraphWithHooksHandler federatedAddGraphWithHooksHandler = new FederatedAddGraphWithHooksHandler(); federatedAddGraphWithHooksHandler.doOperation( @@ -225,10 +226,10 @@ public void shouldThrowWhenOverwriteGraphIsDifferent() throws Exception { public void shouldThrowWhenOverwriteGraphIsSameAndAccessIsDifferent() throws Exception { final Schema expectedSchema = new Schema.Builder().build(); - assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); - store.initialise(FEDERATEDSTORE_GRAPH_ID, new Schema(), federatedStoreProperties); + assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); + final FederatedAddGraphWithHooksHandler federatedAddGraphWithHooksHandler = new FederatedAddGraphWithHooksHandler(); federatedAddGraphWithHooksHandler.doOperation( @@ -282,7 +283,7 @@ public void shouldAddGraphIDOnlyWithAuths() throws Exception { new Context(authUser), store); - final Collection graphs = store.getGraphs(authUser, null, new AddGraph()); + final Collection graphs = store.getGraphs(authUser, null, new AddGraph()); assertThat(graphs).hasSize(1); assertThat(store.getGraphs(testUser, null, new AddGraph())).hasSize(0); assertThat(graphs.iterator().next().getGraphId()).isEqualTo(EXPECTED_GRAPH_ID); @@ -338,9 +339,9 @@ public void shouldAddGraphWithHooks() throws Exception { new Context(testUser), store); - final Collection graphs = store.getGraphs(testUser, null, new AddGraph()); + final Collection graphs = store.getGraphs(testUser, null, new AddGraph()); - final List> graphHooks = graphs.iterator().next().getGraphHooks(); + final List> graphHooks = graphs.iterator().next().getConfig().getHooks().stream().map(GraphHook::getClass).collect(Collectors.toList()); assertThat(graphHooks).contains(Log4jLogger.class); } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAggregateHandlerTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAggregateHandlerTest.java index 6e5ccddd366..19b3a473cd3 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAggregateHandlerTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedAggregateHandlerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2022 Crown Copyright + * Copyright 2016-2022 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,6 +43,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Properties; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; @@ -95,6 +96,14 @@ public void shouldAggregateDuplicatesFromDiffStores() throws Exception { final String graphNameB = "b"; final Context context = new Context(new User()); + Properties properties = PROPERTIES.getProperties(); + AccumuloProperties propsA = new AccumuloProperties(); + propsA.setProperties(properties); + propsA.setInstance(properties.getProperty(AccumuloProperties.INSTANCE_NAME) + "A"); + AccumuloProperties propsB = new AccumuloProperties(); + propsB.setProperties(properties); + propsB.setInstance(properties.getProperty(AccumuloProperties.INSTANCE_NAME) + "B"); + fed.execute(new OperationChain.Builder() .first(new AddGraph.Builder() .graphId(graphNameA) diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationChainHandlerTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationChainHandlerTest.java index 8bd9ce2e07e..b8c86cda01e 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationChainHandlerTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedOperationChainHandlerTest.java @@ -227,7 +227,7 @@ public void shouldHandleChainWithExtraLimit() throws OperationException { // When final Iterable result = store.execute(opChain, context); - // Then - the result will contain 1 element from the first graph + // Then - the result will contain 1 element from the first graph //TODO FS why the first graph? ElementUtil.assertElementEquals(Collections.singletonList(elements[1]), result); } diff --git a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedRemoveGraphHandlerTest.java b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedRemoveGraphHandlerTest.java index 10f290a7f69..28dadd36bfb 100644 --- a/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedRemoveGraphHandlerTest.java +++ b/store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedRemoveGraphHandlerTest.java @@ -27,7 +27,6 @@ import uk.gov.gchq.gaffer.federatedstore.FederatedStore; import uk.gov.gchq.gaffer.federatedstore.FederatedStoreProperties; import uk.gov.gchq.gaffer.federatedstore.operation.RemoveGraph; -import uk.gov.gchq.gaffer.graph.Graph; import uk.gov.gchq.gaffer.graph.GraphConfig; import uk.gov.gchq.gaffer.graph.GraphSerialisable; import uk.gov.gchq.gaffer.store.Context; @@ -60,7 +59,7 @@ public void setUp() throws Exception { public void shouldRemoveGraphForAddingUser() throws Exception { FederatedStore store = new FederatedStore(); final FederatedStoreProperties federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); store.initialise(FEDERATEDSTORE_GRAPH_ID, null, federatedStoreProperties); @@ -79,7 +78,7 @@ public void shouldRemoveGraphForAddingUser() throws Exception { new Context(testUser), store); - Collection graphs = store.getGraphs(testUser, null, new RemoveGraph()); + Collection graphs = store.getGraphs(testUser, null, new RemoveGraph()); assertThat(graphs).isEmpty(); @@ -89,7 +88,7 @@ public void shouldRemoveGraphForAddingUser() throws Exception { public void shouldNotRemoveGraphForNonAddingUser() throws Exception { FederatedStore store = new FederatedStore(); final FederatedStoreProperties federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); store.initialise(FEDERATEDSTORE_GRAPH_ID, null, federatedStoreProperties); @@ -108,7 +107,7 @@ public void shouldNotRemoveGraphForNonAddingUser() throws Exception { new Context(testUser), store); - Collection graphs = store.getGraphs(testUser, null, new RemoveGraph()); + Collection graphs = store.getGraphs(testUser, null, new RemoveGraph()); assertThat(graphs).hasSize(1); @@ -118,7 +117,7 @@ public void shouldNotRemoveGraphForNonAddingUser() throws Exception { public void shouldNotRemoveGraphConfiguredWithNoAccessWritePredicate() throws Exception { FederatedStore store = new FederatedStore(); final FederatedStoreProperties federatedStoreProperties = new FederatedStoreProperties(); - federatedStoreProperties.setCacheProperties(CACHE_SERVICE_CLASS_STRING); + federatedStoreProperties.setCacheServiceClass(CACHE_SERVICE_CLASS_STRING); store.initialise(FEDERATEDSTORE_GRAPH_ID, null, federatedStoreProperties); @@ -146,7 +145,7 @@ public void shouldNotRemoveGraphConfiguredWithNoAccessWritePredicate() throws Ex new Context(testUser), store); - Collection graphs = store.getGraphs(testUser, null, new RemoveGraph()); + Collection graphs = store.getGraphs(testUser, null, new RemoveGraph()); assertThat(graphs).hasSize(1); } diff --git a/store-implementation/federated-store/src/test/resources/properties/singleUseFederatedStore.properties b/store-implementation/federated-store/src/test/resources/properties/singleUseFederatedStore.properties index 553ae9b1acd..f69f4a81ad2 100644 --- a/store-implementation/federated-store/src/test/resources/properties/singleUseFederatedStore.properties +++ b/store-implementation/federated-store/src/test/resources/properties/singleUseFederatedStore.properties @@ -15,3 +15,4 @@ # gaffer.store.class=uk.gov.gchq.gaffer.federatedstore.FederatedStore gaffer.store.properties.class=uk.gov.gchq.gaffer.federatedstore.FederatedStoreProperties +gaffer.cache.service.name.suffix=proxyToRestServiceFederatedGraph diff --git a/store-implementation/map-store/src/test/java/uk/gov/gchq/gaffer/mapstore/MapStorePropertiesGraphSerialisableTest.java b/store-implementation/map-store/src/test/java/uk/gov/gchq/gaffer/mapstore/MapStorePropertiesGraphSerialisableTest.java index 287f835b9da..9625901c2b6 100644 --- a/store-implementation/map-store/src/test/java/uk/gov/gchq/gaffer/mapstore/MapStorePropertiesGraphSerialisableTest.java +++ b/store-implementation/map-store/src/test/java/uk/gov/gchq/gaffer/mapstore/MapStorePropertiesGraphSerialisableTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2020 Crown Copyright + * Copyright 2017-2022 Crown Copyright * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -83,7 +83,7 @@ public void shouldConsumeGraph() throws Exception { final MapStoreProperties mapStoreProperties = new MapStoreProperties(); mapStoreProperties.setProperties(properties); final Graph graph = new Graph.Builder().addSchema(schema).addStoreProperties(mapStoreProperties).config(config).build(); - final GraphSerialisable result = new GraphSerialisable.Builder().graph(graph).build(); + final GraphSerialisable result = new GraphSerialisable.Builder(graph).build(); assertEquals(expected, result); } }