gh-2455 FederatedStore GetTraits Audit. Removing dead code and correc…

…ting tests and behaviours.

core/operation/src/test/java/uk/gov/gchq/gaffer/user/StoreUser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 Crown Copyright
+ * Copyright 2017-2021 Crown Copyright
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@ public final class StoreUser {
     public static final String AUTH_USER_ID = "authUser";
     public static final String AUTH_1 = "auth1";
     public static final String AUTH_2 = "auth2";
+    public static final String UNUSED_AUTH_STRING = "unusedAuthString";
 
     private StoreUser() {
         // private to prevent instantiation
@@ -43,4 +44,8 @@ public static User authUser() {
     public static User blankUser() {
         return new User.Builder().build();
     }
+
+    public static User nullUser() {
+        return null;
+    }
 }

core/store/src/main/java/uk/gov/gchq/gaffer/store/operation/handler/GetTraitsHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 Crown Copyright
+ * Copyright 2017-2021 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,7 +17,6 @@
 package uk.gov.gchq.gaffer.store.operation.handler;
 
 import com.google.common.collect.Sets;
-import org.apache.commons.collections.CollectionUtils;
 
 import uk.gov.gchq.gaffer.commonutil.iterable.ChainedIterable;
 import uk.gov.gchq.gaffer.operation.OperationException;
@@ -31,6 +30,9 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static java.util.Objects.nonNull;
+import static org.apache.commons.collections.CollectionUtils.isNotEmpty;
+
 public class GetTraitsHandler implements OutputOperationHandler<GetTraits, Set<StoreTrait>> {
     private Set<StoreTrait> currentTraits;
 
@@ -49,19 +51,14 @@ public Set<StoreTrait> doOperation(final GetTraits operation, final Context cont
     private Set<StoreTrait> createCurrentTraits(final Store store) {
         final Set<StoreTrait> traits = Sets.newHashSet(store.getTraits());
         final Schema schema = store.getSchema();
-        final Iterable<SchemaElementDefinition> defs = new ChainedIterable<>(schema.getEntities().values(), schema.getEdges().values());
 
-        final boolean hasAggregatedGroups = CollectionUtils.isNotEmpty(schema.getAggregatedGroups());
-        final boolean hasVisibility = null != schema.getVisibilityProperty();
+        final boolean hasAggregatedGroups = isNotEmpty(schema.getAggregatedGroups());
+        final boolean hasVisibility = nonNull(schema.getVisibilityProperty());
         boolean hasGroupBy = false;
         boolean hasValidation = false;
-        for (final SchemaElementDefinition def : defs) {
-            if (!hasValidation && def.hasValidation()) {
-                hasValidation = true;
-            }
-            if (!hasGroupBy && CollectionUtils.isNotEmpty(def.getGroupBy())) {
-                hasGroupBy = true;
-            }
+        for (final SchemaElementDefinition def : new ChainedIterable<SchemaElementDefinition>(schema.getEntities().values(), schema.getEdges().values())) {
+            hasValidation = hasValidation || def.hasValidation();
+            hasGroupBy = hasGroupBy || isNotEmpty(def.getGroupBy());
             if (hasGroupBy && hasValidation) {
                 break;
             }

store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorage.java

@@ -332,45 +332,31 @@ public Schema getSchema(final Map<String, String> config, final User user) {
      * @return the set of {@link StoreTrait} that are common for all visible graphs
      */
     public Set<StoreTrait> getTraits(final GetTraits op, final Context context) {
-        final Set<StoreTrait> traits = Sets.newHashSet(StoreTrait.values());
-        if (null != op && op.isCurrentTraits()) {
+        boolean firstPass = true;
+        final Set<StoreTrait> traits = new HashSet<>();
+        if (null != op) {
             final List<String> graphIds = FederatedStoreUtil.getGraphIds(op.getOptions());
-            final Stream<Graph> graphs = getStream(context.getUser(), graphIds);
+            //final Set<Graph> graphs = get(context.getUser(), graphIds) would throw informative error if user couldn't view request graphs
+            final Set<Graph> graphs = getStream(context.getUser(), graphIds).collect(Collectors.toSet());
             final GetTraits getTraits = op.shallowClone();
-            graphs.forEach(g -> {
+            for (final Graph graph : graphs) {
                 try {
-                    traits.retainAll(g.execute(getTraits, context));
+                    Set<StoreTrait> execute = graph.execute(getTraits, context);
+                    if (firstPass) {
+                        traits.addAll(execute);
+                        firstPass = false;
+                    } else {
+                        traits.retainAll(execute);
+                    }
                 } catch (final OperationException e) {
-                    throw new RuntimeException("Unable to fetch traits from graph " + g.getGraphId(), e);
+                    throw new RuntimeException("Unable to fetch traits from graph " + graph.getGraphId(), e);
                 }
-            });
+            }
         }
 
         return traits;
     }
 
-    /**
-     * returns a set of {@link StoreTrait} that are common for all visible graphs.
-     * traits1 = [a,b,c]
-     * traits2 = [b,c]
-     * traits3 = [a,b]
-     * return [b]
-     *
-     * @param config containing optional graphIds csv.
-     * @param user   to match visibility against.
-     * @return the set of {@link StoreTrait} that are common for all visible graphs
-     */
-    public Set<StoreTrait> getTraits(final Map<String, String> config, final User user) {
-        final List<String> graphIds = FederatedStoreUtil.getGraphIds(config);
-        Collection<Graph> graphs = get(user, graphIds);
-
-        final Set<StoreTrait> traits = graphs.isEmpty() ? Sets.newHashSet() : Sets.newHashSet(StoreTrait.values());
-        for (final Graph graph : graphs) {
-            traits.retainAll(graph.getStoreTraits());
-        }
-        return traits;
-    }
-
     private void validateAllGivenGraphIdsAreVisibleForUser(final User user, final Collection<String> graphIds) {
         if (null != graphIds) {
             final Collection<String> visibleIds = getAllIds(user);

store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStore.java

@@ -334,7 +334,9 @@ public Schema getSchema(final Map<String, String> config, final User user) {
 
     /**
      * @return {@link Store#getTraits()}
+     * @deprecated should execute GetTraits Operation against the FederatedStore
      */
+    @Deprecated
     @Override
     public Set<StoreTrait> getTraits() {
         return StoreTrait.ALL_TRAITS;

store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedAccessAuthTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 Crown Copyright
+ * Copyright 2017-2021 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,13 +43,13 @@
 import static uk.gov.gchq.gaffer.user.StoreUser.AUTH_2;
 import static uk.gov.gchq.gaffer.user.StoreUser.AUTH_USER_ID;
 import static uk.gov.gchq.gaffer.user.StoreUser.TEST_USER_ID;
+import static uk.gov.gchq.gaffer.user.StoreUser.UNUSED_AUTH_STRING;
 import static uk.gov.gchq.gaffer.user.StoreUser.authUser;
 import static uk.gov.gchq.gaffer.user.StoreUser.blankUser;
 import static uk.gov.gchq.gaffer.user.StoreUser.testUser;
 
 public class FederatedAccessAuthTest {
 
-    private static final String AUTH_X = "X";
     private static final User TEST_USER = testUser();
     public static final User AUTH_USER = authUser();
 
@@ -108,7 +108,7 @@ public void shouldValidateWithListOfAuths() throws Exception {
 
         final FederatedAccess access = new FederatedAccess.Builder()
                 .addGraphAuths(asList(AUTH_1))
-                .addGraphAuths(asList(AUTH_X))
+                .addGraphAuths(asList(UNUSED_AUTH_STRING))
                 .build();
 
         assertTrue(access.hasReadAccess(AUTH_USER));

store-implementation/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorageTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 Crown Copyright
+ * Copyright 2017-2021 Crown Copyright
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,7 +32,6 @@
 import uk.gov.gchq.gaffer.graph.GraphConfig;
 import uk.gov.gchq.gaffer.graph.GraphSerialisable;
 import uk.gov.gchq.gaffer.store.Context;
-import uk.gov.gchq.gaffer.store.StoreTrait;
 import uk.gov.gchq.gaffer.store.library.GraphLibrary;
 import uk.gov.gchq.gaffer.store.schema.Schema;
 import uk.gov.gchq.gaffer.store.schema.SchemaEdgeDefinition;
@@ -414,49 +413,6 @@ public void shouldGetSchemaForBlankUserWhenPermissiveReadAccessPredicateConfigur
         assertEquals(e1, schema.getElement("e1"));
     }
 
-    @Test
-    public void shouldGetTraitsForAddingUser() throws Exception {
-        graphStorage.put(a, new FederatedAccess(Sets.newHashSet(X), X));
-        graphStorage.put(b, access);
-        final Set<StoreTrait> traits = graphStorage.getTraits(null, testUser);
-        assertNotEquals(5, traits.size(), "Revealing hidden traits");
-        assertEquals(10, traits.size());
-    }
-
-    @Test
-    public void shouldNotGetTraitsForAddingUserWhenBlockingReadAccessPredicateConfigured() throws Exception {
-        graphStorage.put(a, new FederatedAccess(Sets.newHashSet(X), X));
-        graphStorage.put(b, blockingReadAccess);
-        final Set<StoreTrait> traits = graphStorage.getTraits(null, blankUser);
-        assertEquals(0, traits.size(), "Revealing hidden traits");
-    }
-
-    @Test
-    public void shouldGetTraitsForAuthUser() throws Exception {
-        graphStorage.put(a, new FederatedAccess(Sets.newHashSet(X), X));
-        graphStorage.put(b, access);
-        final Set<StoreTrait> traits = graphStorage.getTraits(null, authUser);
-        assertNotEquals(5, traits.size(), "Revealing hidden traits");
-        assertEquals(10, traits.size());
-    }
-
-    @Test
-    public void shouldNotGetTraitsForBlankUser() throws Exception {
-        graphStorage.put(a, new FederatedAccess(Sets.newHashSet(X), X));
-        graphStorage.put(b, access);
-        final Set<StoreTrait> traits = graphStorage.getTraits(null, blankUser);
-        assertEquals(0, traits.size(), "Revealing hidden traits");
-    }
-
-    @Test
-    public void shouldGetTraitsForBlankUserWhenPermissiveReadAccessPredicateConfigured() throws Exception {
-        graphStorage.put(a, new FederatedAccess(Sets.newHashSet(X), X));
-        graphStorage.put(b, permissiveReadAccess);
-        final Set<StoreTrait> traits = graphStorage.getTraits(null, blankUser);
-        assertNotEquals(5, traits.size(), "Revealing hidden traits");
-        assertEquals(10, traits.size());
-    }
-
     @Test
     public void shouldRemoveForAddingUser() throws Exception {
         graphStorage.put(a, access);