Gh-3106: Refactor GafferPopNamedOperationService

library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/service/GafferPopNamedOperationService.java

@@ -31,7 +31,20 @@
 
 import java.util.Arrays;
 import java.util.Map;
+import java.util.stream.StreamSupport;
 
+/**
+ * Service for running Gaffer Named Operations
+ *
+ * <p>This service should be called at the start
+ * of a traversal.</p>
+ *
+ * @param <I> Ignored
+ * @param <R> Ignored
+ *
+ * @see <a href="https://tinkerpop.apache.org/docs/3.7.0/reference/#call-step">
+ * Tinkerpop Call Step Documentation</a>
+ */
 public class GafferPopNamedOperationService<I, R> implements Service<I, R> {
     private final GafferPopGraph graph;
 
@@ -44,44 +57,79 @@ public Type getType() {
         return Type.Start;
     }
 
+    /**
+     * Executes the Service, either calling an existing Named Operation or
+     * adding a new Named Operation.
+     *
+     * @param ctx Unused
+     * @param params {@link Map} containing a key of either "execute" or
+     * "add" and value with name to execute or operation chain to add
+     * @return {@link CloseableIterator} with results or empty if adding
+     * a Named Operation
+     */
     @Override
     public CloseableIterator<R> execute(final ServiceCallContext ctx, final Map params) {
         if (params.containsKey("execute")) {
-            final String name = (String) params.get("execute");
-            Iterable<NamedOperationDetail> namedOps = graph.execute(new OperationChain.Builder().first(new GetAllNamedOperations()).build());
-            for (final NamedOperationDetail namedOp : namedOps) {
-                if (namedOp.getOperationName().equals(name)) {
-                    if (namedOp.getOperationChainWithDefaultParams().getOutputClass().equals(Iterable.class)) {
-                        return (CloseableIterator<R>) CloseableIterator.of(graph.execute(new OperationChain.Builder()
-                                .first(new NamedOperation.Builder()
-                                        .name(name)
-                                        .build())
-                                .then(new GenerateObjects.Builder<GafferPopElement>()
-                                        .generator(new GafferPopElementGenerator(graph))
-                                        .build())
-                                .build()).iterator());
-                    } else {
-                        return CloseableIterator.of(Arrays.asList(graph.execute(new OperationChain.Builder()
-                                .first(new NamedOperation.Builder<Void, R>()
-                                        .name(name)
-                                        .build())
-                                .build())).iterator());
-                    }
-                }
-            }
-            throw new IllegalStateException("Named Operation not found");
+            return executeNamedOperation((String) params.get("execute"));
         } else if (params.containsKey("add")) {
-            final Map<String, Object> addParams = (Map) params.get("add");
-            graph.execute(new OperationChain.Builder()
-                    .first(new AddNamedOperation.Builder()
-                            .name((String) addParams.get("name"))
-                            .operationChain((String) addParams.get("opChain"))
+            return addNamedOperation((Map<String, String>) params.get("add"));
+        } else {
+            throw new IllegalStateException("Missing parameter, either 'execute' or 'add' expected");
+        }
+    }
+
+    /**
+     * Find the requested Named Operation and depending on its output class
+     * either execute and then convert all {@link uk.gov.gchq.gaffer.data.element.Element}
+     * to {@link GafferPopElement}, to just execute and return the result as
+     * an Iterable.
+     *
+     * @param name Name of the Named Operation to execute
+     * @return results of the operation execution
+     */
+    protected CloseableIterator<R> executeNamedOperation(final String name) {
+        // Fetch details for the requested Named Operation or throw an exception if it's not found
+        Iterable<NamedOperationDetail> allNamedOps = graph.execute(new OperationChain.Builder().first(new GetAllNamedOperations()).build());
+        NamedOperationDetail namedOperation = StreamSupport.stream(allNamedOps.spliterator(), false)
+                .filter(nd -> nd.getOperationName().equals(name))
+                .findFirst()
+                .orElseThrow(() -> new IllegalStateException("Named Operation not found"));
+
+        // If the Named Operation outputs an Iterable, then execute and convert Elements to GafferPopElement
+        // Else execute and wrap the output as an iterator
+        if (namedOperation.getOperationChainWithDefaultParams().getOutputClass().equals(Iterable.class)) {
+            return (CloseableIterator<R>) CloseableIterator.of(graph.execute(new OperationChain.Builder()
+                    .first(new NamedOperation.Builder()
+                            .name(name)
                             .build())
-                    .build());
-            return CloseableIterator.empty();
+                    .then(new GenerateObjects.Builder<GafferPopElement>()
+                            .generator(new GafferPopElementGenerator(graph))
+                            .build())
+                    .build()).iterator());
         } else {
-            throw new IllegalStateException("Missing parameter");
+            return CloseableIterator.of(Arrays.asList(graph.execute(new OperationChain.Builder()
+                    .first(new NamedOperation.Builder<Void, R>()
+                            .name(name)
+                            .build())
+                    .build())).iterator());
         }
     }
 
+    /**
+     * Add a new Named Operation with the supplied Name and Operation Chain.
+     *
+     * @param addParams {@link Map} with Key "name" containing the name to give
+     * the new Named Operation being added and key "OpChain" containing the
+     * operation chain to use with this Named Operation.
+     * @return Empty
+     */
+    protected CloseableIterator<R> addNamedOperation(final Map<String, String> addParams) {
+        graph.execute(new OperationChain.Builder()
+                .first(new AddNamedOperation.Builder()
+                        .name(addParams.get("name"))
+                        .operationChain(addParams.get("opChain"))
+                        .build())
+                .build());
+        return CloseableIterator.empty();
+    }
 }

library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/service/GafferPopNamedOperationServiceIT.java

@@ -86,7 +86,7 @@ void shouldThrowExceptionForMissingParameter() {
         // Then
         assertThatThrownBy(() -> g.call("namedoperation", params).toList())
                 .isExactlyInstanceOf(IllegalStateException.class)
-                .hasMessage("Missing parameter");
+                .hasMessage("Missing parameter, either 'execute' or 'add' expected");
     }
 
     @Test

library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/service/GafferPopNamedOperationServiceTest.java

@@ -50,6 +50,6 @@ void shouldThrowErrorForInvalidParams() {
         // When / Then
         assertThatThrownBy(() -> namedOpService.execute(null, Collections.singletonMap("invalid", "invalid")))
                 .isExactlyInstanceOf(IllegalStateException.class)
-                .hasMessage("Missing parameter");
+                .hasMessage("Missing parameter, either 'execute' or 'add' expected");
     }
 }