Gh-3209: GetAll Limit fails on Proxy Store

core/operation/src/main/java/uk/gov/gchq/gaffer/operation/OperationChain.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2023 Crown Copyright
+ * Copyright 2016-2024 Crown Copyright
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -66,7 +66,7 @@
 public class OperationChain<OUT> implements Output<OUT>,
         Operations<Operation> {
     private List<Operation> operations;
-    private Map<String, String> options;
+    private Map<String, String> options = new HashMap<>();
 
     public OperationChain() {
         this(new ArrayList<>());
@@ -184,7 +184,11 @@ public Map<String, String> getOptions() {
 
     @Override
     public void setOptions(final Map<String, String> options) {
-        this.options = options;
+        if (options == null) {
+            this.options = new HashMap<>();
+        } else {
+            this.options = options;
+        }
     }
 
     @Override

core/operation/src/test/java/uk/gov/gchq/gaffer/operation/impl/ValidateOperationChainTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2021 Crown Copyright
+ * Copyright 2018-2024 Crown Copyright
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
 
 import java.util.Set;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 
@@ -51,7 +52,7 @@ public void shouldJSONSerialiseAndDeserialise() throws SerialisationException {
         final ValidateOperationChain deserialisedOp = JSONSerialiser.deserialise(json, ValidateOperationChain.class);
 
         // Then
-        assertEquals(validateOperationChain.getOperationChain(), deserialisedOp.getOperationChain());
+        assertThat(deserialisedOp.getOperationChain()).isEqualTo(validateOperationChain.getOperationChain());
     }
 
     @Test

library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java

@@ -380,28 +380,30 @@ public void addEdge(final GafferPopEdge edge) {
     public Iterator<Vertex> vertices(final Object... vertexIds) {
         final boolean getAll = null == vertexIds || 0 == vertexIds.length;
 
-        final Output<Iterable<? extends Element>> getOperation;
+        final OperationChain<Iterable<? extends Element>> getOperation;
+
         if (getAll) {
+            LOGGER.debug("Requested a GetAllElements, results will be truncated to: {}.", getAllElementsLimit);
             getOperation = new Builder()
                     .first(new GetAllElements.Builder()
                             .view(new View.Builder()
                                     .entities(graph.getSchema().getEntityGroups())
                                     .build())
                             .build())
-                    .then(new Limit<>(getAllElementsLimit, true))
+                    .then(new Limit<Element>(getAllElementsLimit, true))
                     .build();
         } else {
-            getOperation = new GetElements.Builder()
+            getOperation = new Builder()
+                .first(new GetElements.Builder()
                     .input(getElementSeeds(Arrays.asList(vertexIds)))
                     .view(new View.Builder()
                             .entities(graph.getSchema().getEntityGroups())
                             .build())
-                    .build();
+                    .build())
+                .build();
         }
         // Run requested chain on the graph
-        final Iterable<? extends Element> result = execute(new Builder()
-                .first(getOperation)
-                .build());
+        final Iterable<? extends Element> result = execute(getOperation);
 
         // Translate results to Gafferpop elements
         final GafferPopElementGenerator generator = new GafferPopElementGenerator(this);
@@ -411,12 +413,11 @@ public Iterator<Vertex> vertices(final Object... vertexIds) {
                 .map(e -> (Vertex) e)
                 .iterator();
 
-        final int resultSize = IterableUtils.size(translatedResults);
-        if (getAll && resultSize >= getAllElementsLimit) {
-            LOGGER.warn("Result size is equal to configured limit ({}). Results may have been truncated", getAllElementsLimit);
+        if (getAll && IterableUtils.size(translatedResults) == getAllElementsLimit) {
+            LOGGER.warn(
+                "Result size is equal to configured limit ({}). Results may have been truncated",
+                 getAllElementsLimit);
         }
-
-
         return translatedResults.iterator();
     }
 
@@ -540,8 +541,9 @@ public Iterator<Vertex> adjVerticesWithView(final Iterable<Object> vertexIds, fi
     public Iterator<Edge> edges(final Object... elementIds) {
         final boolean getAll = null == elementIds || 0 == elementIds.length;
 
-        final Output<Iterable<? extends Element>> getOperation;
+        final OperationChain<Iterable<? extends Element>> getOperation;
         if (getAll) {
+            LOGGER.debug("Requested a GetAllElements, results will be truncated to: {}.", getAllElementsLimit);
             getOperation = new Builder()
                 .first(new GetAllElements.Builder()
                         .view(new View.Builder()
@@ -551,18 +553,19 @@ public Iterator<Edge> edges(final Object... elementIds) {
                 .then(new Limit<>(getAllElementsLimit, true))
                 .build();
         } else {
-            getOperation = new GetElements.Builder()
+            getOperation = new Builder()
+                .first(new GetElements.Builder()
                     .input(getElementSeeds(Arrays.asList(elementIds)))
                     .view(new View.Builder()
-                            .edges(graph.getSchema().getEdgeGroups())
-                            .build())
-                    .build();
+                        .edges(graph.getSchema().getEdgeGroups())
+                        .build())
+                    .build())
+                .build();
         }
 
         // Run requested chain on the graph
-        final Iterable<? extends Element> result = execute(new Builder()
-                .first(getOperation)
-                .build());
+        final Iterable<? extends Element> result = execute(getOperation);
+
 
         // Translate results to Gafferpop elements
         final GafferPopElementGenerator generator = new GafferPopElementGenerator(this);
@@ -572,11 +575,11 @@ public Iterator<Edge> edges(final Object... elementIds) {
                 .map(e -> (Edge) e)
                 .iterator();
 
-        final int resultSize = IterableUtils.size(translatedResults);
-        if (getAll && resultSize >= getAllElementsLimit) {
-            LOGGER.warn("Result size is equal to configured limit ({}). Results may have been truncated", getAllElementsLimit);
+        if (getAll && IterableUtils.size(translatedResults) == getAllElementsLimit) {
+            LOGGER.warn(
+                "Result size is equal to configured limit ({}). Results may have been truncated",
+                getAllElementsLimit);
         }
-
         return translatedResults.iterator();
     }
 

library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/process/traversal/step/GafferPopGraphStep.java

@@ -108,7 +108,7 @@ private Iterator<? extends Vertex> vertices(final GafferPopGraph graph) {
         }
 
         // linear scan as fallback
-        return IteratorUtils.filter(graph.vertices(Arrays.asList(this.ids)), vertex -> HasContainer.testAll(vertex, hasContainers));
+        return IteratorUtils.filter(graph.vertices(this.ids), vertex -> HasContainer.testAll(vertex, hasContainers));
     }
 
     /**

store-implementation/proxy-store/src/main/java/uk/gov/gchq/gaffer/proxystore/response/deserialiser/impl/DefaultResponseDeserialiser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 Crown Copyright
+ * Copyright 2021-2024 Crown Copyright
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -13,15 +13,21 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package uk.gov.gchq.gaffer.proxystore.response.deserialiser.impl;
 
 import com.fasterxml.jackson.core.type.TypeReference;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import uk.gov.gchq.gaffer.exception.SerialisationException;
 import uk.gov.gchq.gaffer.jsonserialisation.JSONSerialiser;
+import uk.gov.gchq.gaffer.operation.serialisation.TypeReferenceImpl;
 import uk.gov.gchq.gaffer.proxystore.response.deserialiser.ResponseDeserialiser;
 
 public class DefaultResponseDeserialiser<O> implements ResponseDeserialiser<O> {
+    private static final Logger LOGGER = LoggerFactory.getLogger(DefaultResponseDeserialiser.class);
 
     private final TypeReference<O> typeReference;
 
@@ -36,8 +42,21 @@ public O deserialise(final String jsonString) throws SerialisationException {
             // The input is likely a plain java.lang.String object, so return as-is
             return (O) jsonString;
         } else {
-            // The input is likely a valid JSON value type, so process using the deserialiser
-            return JSONSerialiser.deserialise(encodeString(jsonString), typeReference);
+            // For some operations like 'Limit' we may not know the output type up front so it
+            // will be left as a generic '?', this means some objects will not be deserialised correctly.
+            try {
+                // Check if we can deserialise to common outputs like list of elements.
+                // This is a workaround until proxy store has been refactored
+                if (typeReference.getType().getTypeName().equals(Iterable.class.getName() + "<?>")) {
+                    return (O) JSONSerialiser.deserialise(encodeString(jsonString), new TypeReferenceImpl.IterableElement());
+                } else {
+                    return JSONSerialiser.deserialise(encodeString(jsonString), typeReference);
+                }
+            } catch (final SerialisationException e) {
+                // The input is likely a valid JSON value type, so process using the deserialiser
+                LOGGER.error("Unable to deserialse Iterable<?> as Iterable<Elements> using default deserialisation", e);
+                return JSONSerialiser.deserialise(encodeString(jsonString), typeReference);
+            }
         }
     }
 }