gh-2823 FederatedStore unsupported operations are forwarded to subgra…

…phs. improvements
gchq · Nov 15, 2022 · 1eb20b7 · 1eb20b7
1 parent 248ddb4
commit 1eb20b7
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 8 deletions.
diff --git a/...ation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStore.java b/...ation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/FederatedStore.java
@@ -629,7 +629,7 @@ protected Object doUnhandledOperation(final Operation operation, final Context c
                         .doOperation(operation, context, this);
             }
         } catch (final Exception e) {
-            throw new GafferRuntimeException(String.format("Error with FederatedStore forwarding unhandled operation to sub-graphs due to: %s", e.getMessage()), e);
+            throw new UnsupportedOperationException(String.format("Operation class %s is not supported by the FederatedStore. Error occurred forwarding unhandled operation to sub-graphs due to: %s", operation.getClass().getName(), e.getMessage()), e);
         }
     }
 

diff --git a/...a/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedAddGraphHandlerParent.java b/...a/uk/gov/gchq/gaffer/federatedstore/operation/handler/FederatedAddGraphHandlerParent.java
@@ -85,8 +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);
         }
 
-        // TODO FS remove if required
-        // addGenericHandler((FederatedStore) store, graph);
+        addGenericHandler((FederatedStore) store, graph);
 
         return null;
     }

diff --git a/...n/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTest.java b/...n/federated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreTest.java
@@ -17,6 +17,7 @@
 package uk.gov.gchq.gaffer.federatedstore;
 
 import com.google.common.collect.Sets;
+import org.apache.commons.lang3.exception.CloneFailedException;
 import org.assertj.core.api.InstanceOfAssertFactories;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
@@ -50,7 +51,6 @@
 import uk.gov.gchq.gaffer.jsonserialisation.JSONSerialiser;
 import uk.gov.gchq.gaffer.operation.Operation;
 import uk.gov.gchq.gaffer.operation.OperationException;
-import uk.gov.gchq.gaffer.operation.impl.OperationImpl;
 import uk.gov.gchq.gaffer.operation.impl.add.AddElements;
 import uk.gov.gchq.gaffer.operation.impl.get.GetAllElements;
 import uk.gov.gchq.gaffer.store.Context;
@@ -69,6 +69,7 @@
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -270,13 +271,28 @@ public void shouldNotAllowOverwritingOfGraphWithinFederatedScope() throws Except
     @Test
     public void shouldThrowAppropriateExceptionWhenHandlingAnUnsupportedOperation() {
         // Given
-        final Operation op = new OperationImpl();
+        final Operation unknownOperation = new Operation() {
+            @Override
+            public Operation shallowClone() throws CloneFailedException {
+                return this;
+            }
+
+            @Override
+            public Map<String, String> getOptions() {
+                return null;
+            }
+
+            @Override
+            public void setOptions(final Map<String, String> options) {
+
+            }
+        };
         // When
         // Expected an UnsupportedOperationException rather than an OperationException
 
         // Then
-        assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> store.handleOperation(op, new Context()))
-                .withMessage("Operation class uk.gov.gchq.gaffer.operation.impl.OperationImpl is not supported by the FederatedStore.");
+        assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> store.handleOperation(unknownOperation, new Context()))
+                .withMessageContaining("Operation class uk.gov.gchq.gaffer.federatedstore.FederatedStoreTest$1 is not supported by the FederatedStore.");
     }
 
     @Test

diff --git a/...ov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedUnhandledOperationTest.java b/...ov/gchq/gaffer/federatedstore/operation/handler/impl/FederatedUnhandledOperationTest.java
@@ -25,6 +25,7 @@
 import uk.gov.gchq.gaffer.federatedstore.FederatedStoreProperties;
 import uk.gov.gchq.gaffer.federatedstore.FederatedStoreTestUtil;
 import uk.gov.gchq.gaffer.federatedstore.operation.AddGraph;
+import uk.gov.gchq.gaffer.graph.Graph;
 import uk.gov.gchq.gaffer.operation.Operation;
 import uk.gov.gchq.gaffer.operation.impl.add.AddElements;
 import uk.gov.gchq.gaffer.operation.impl.get.GetAllElements;
@@ -61,13 +62,22 @@ void afterEach() {
 
     @Test
     public void shouldFailWithUnsupportedOperation() throws Exception {
-        //make custom store with no GetAllElements
         final String testExceptionMessage = "TestExceptionMessage";
+
+        //make custom store with no GetAllElements
         FederatedStore federatedStoreWithNoGetAllElements = new FederatedStore() {
             @Override
             protected void addAdditionalOperationHandlers() {
                 super.addAdditionalOperationHandlers();
+                //No support for GetAllElements
                 addOperationHandler(GetAllElements.class, null);
+                //No support for adding Generic Handlers to Federation.
+                addOperationHandler(AddGraph.class, new FederatedAddGraphHandler() {
+                    @Override
+                    protected void addGenericHandler(final FederatedStore store, final Graph graph) {
+                        //nothing
+                    }
+                });
             }
 
             @Override
@@ -102,7 +112,15 @@ public void shouldDelegateUnknownOperationToSubGraphs() throws Exception {
             @Override
             protected void addAdditionalOperationHandlers() {
                 super.addAdditionalOperationHandlers();
+                //No support for GetAllElements
                 addOperationHandler(GetAllElements.class, null);
+                //No support for adding Generic Handlers to Federation.
+                addOperationHandler(AddGraph.class, new FederatedAddGraphHandler() {
+                    @Override
+                    protected void addGenericHandler(final FederatedStore store, final Graph graph) {
+                        //nothing
+                    }
+                });
             }
         };
         federatedStoreWithNoGetAllElements.initialise(GRAPH_ID_TEST_FEDERATED_STORE, null, federatedStoreProperties);