Introduce QueryResult class for QueryEngine caching (#1333)

* Make QueryEngine cache bypass flag part of Query

* Forming the cache key is not free, so don't a use stub cache

* Add QueryResult class

* Add pageTotals to QueryResult

* Pass page totals through QueryResult instead of Pagination

* Codacy doesn't know @value makes fields private

elide-core/src/main/java/com/yahoo/elide/core/pagination/PaginationImpl.java

@@ -64,17 +64,18 @@ public enum PaginationKey { offset, number, size, limit, totals }
     private static final String PAGE_KEYS_CSV = PAGE_KEYS.keySet().stream().collect(Collectors.joining(", "));
 
     @Getter
-    private Integer offset;
+    private final int offset;
 
     @Getter
-    private Integer limit;
+    private final int limit;
 
-    private Boolean generateTotals;
+    private final boolean generateTotals;
 
-    private Boolean isDefault;
+    @Getter
+    private final boolean defaultInstance;
 
     @Getter
-    private Class<?> entityClass;
+    private final Class<?> entityClass;
 
     /**
      * Constructor.
@@ -95,7 +96,7 @@ public PaginationImpl(Class<?> entityClass,
                            Boolean pageByPages) {
 
         this.entityClass = entityClass;
-        this.isDefault = (clientOffset == null && clientLimit == null && generateTotals == null);
+        this.defaultInstance = (clientOffset == null && clientLimit == null && generateTotals == null);
 
         Paginate paginate = entityClass != null ? (Paginate) entityClass.getAnnotation(Paginate.class) : null;
 
@@ -107,7 +108,7 @@ public PaginationImpl(Class<?> entityClass,
 
         String pageSizeLabel = pageByPages ? "size" : "limit";
 
-        if (limit > maxLimit && !isDefault) {
+        if (limit > maxLimit && !defaultInstance) {
             throw new InvalidValueException("Pagination "
                     + pageSizeLabel + " must be less than or equal to " + maxLimit);
         }
@@ -138,19 +139,10 @@ public PaginationImpl(Class<?> entityClass,
      * @return true if page totals should be returned.
      */
     @Override
-    public Boolean returnPageTotals() {
+    public boolean returnPageTotals() {
         return generateTotals;
     }
 
-    /**
-     * Whether or not the client requested pagination or the system defaults are in effect.
-     * @return True if the system defaults are in effect.
-     */
-    @Override
-    public Boolean isDefaultInstance() {
-        return isDefault;
-    }
-
     /**
      * Given json-api paging params, generate page and pageSize values from query params.
      *

elide-core/src/main/java/com/yahoo/elide/request/Pagination.java

@@ -14,35 +14,35 @@ public interface Pagination {
     /**
      * Default offset (in records) it client does not provide one.
      */
-    public static final int DEFAULT_OFFSET = 0;
+    int DEFAULT_OFFSET = 0;
 
     /**
      * Default page limit (in records) it client does not provide one.
      */
-    public static final int DEFAULT_PAGE_LIMIT = 500;
+    int DEFAULT_PAGE_LIMIT = 500;
 
     /**
      * Maximum allowable page limit (in records).
      */
-    public static final int MAX_PAGE_LIMIT = 10000;
+    int MAX_PAGE_LIMIT = 10000;
 
     /**
      * Get the page offset.
      * @return record offset.
      */
-    Integer getOffset();
+    int getOffset();
 
     /**
      * Get the page limit.
      * @return record limit.
      */
-    Integer getLimit();
+    int getLimit();
 
     /**
      * Whether or not to fetch the collection size or not.
      * @return true if the client wants the total size of the collection.
      */
-    Boolean returnPageTotals();
+    boolean returnPageTotals();
 
     /**
      * Get the total size of the collection
@@ -60,5 +60,5 @@ public interface Pagination {
      * Is this the default instance (not present).
      * @return true if pagination wasn't requested.  False otherwise.
      */
-    public Boolean isDefaultInstance();
+    boolean isDefaultInstance();
 }

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/AggregationDataStoreTransaction.java

@@ -9,6 +9,7 @@
 import com.yahoo.elide.core.RequestScope;
 import com.yahoo.elide.datastores.aggregation.metadata.models.Table;
 import com.yahoo.elide.datastores.aggregation.query.Query;
+import com.yahoo.elide.datastores.aggregation.query.QueryResult;
 import com.yahoo.elide.request.EntityProjection;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -53,7 +54,11 @@ public void createObject(Object entity, RequestScope scope) {
     @Override
     public Iterable<Object> loadObjects(EntityProjection entityProjection, RequestScope scope) {
         Query query = buildQuery(entityProjection, scope);
-        return queryEngine.executeQuery(query, true);
+        QueryResult result = queryEngine.executeQuery(query);
+        if (entityProjection.getPagination() != null && entityProjection.getPagination().returnPageTotals()) {
+            entityProjection.getPagination().setPageTotals(result.getPageTotals());
+        }
+        return result.getData();
     }
 
     @Override

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/EntityProjectionTranslator.java

@@ -14,6 +14,7 @@
 import com.yahoo.elide.datastores.aggregation.metadata.models.Table;
 import com.yahoo.elide.datastores.aggregation.metadata.models.TimeDimension;
 import com.yahoo.elide.datastores.aggregation.query.ColumnProjection;
+import com.yahoo.elide.datastores.aggregation.query.ImmutablePagination;
 import com.yahoo.elide.datastores.aggregation.query.MetricProjection;
 import com.yahoo.elide.datastores.aggregation.query.Query;
 import com.yahoo.elide.datastores.aggregation.query.TimeDimensionProjection;
@@ -72,7 +73,7 @@ public Query getQuery() {
                 .whereFilter(whereFilter)
                 .havingFilter(havingFilter)
                 .sorting(entityProjection.getSorting())
-                .pagination(entityProjection.getPagination())
+                .pagination(ImmutablePagination.from(entityProjection.getPagination()))
                 .build();
         QueryValidator validator = new QueryValidator(query, getAllFields(), dictionary);
         validator.validate();

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/QueryEngine.java

@@ -18,6 +18,7 @@
 import com.yahoo.elide.datastores.aggregation.query.ColumnProjection;
 import com.yahoo.elide.datastores.aggregation.query.MetricProjection;
 import com.yahoo.elide.datastores.aggregation.query.Query;
+import com.yahoo.elide.datastores.aggregation.query.QueryResult;
 import com.yahoo.elide.datastores.aggregation.query.TimeDimensionProjection;
 import com.yahoo.elide.request.Argument;
 
@@ -91,11 +92,7 @@ public QueryEngine(MetaDataStore metaDataStore, Cache cache) {
         populateMetaData(metaDataStore);
         this.tables = metaDataStore.getMetaData(Table.class).stream()
                 .collect(Collectors.toMap(Table::getId, Functions.identity()));
-        if (cache != null) {
-            this.cache = cache;
-        } else {
-            this.cache = new NoCache();
-        }
+        this.cache = cache;
     }
 
     /**
@@ -163,11 +160,10 @@ private void populateMetaData(MetaDataStore metaDataStore) {
      * Executes the specified {@link Query} against a specific persistent storage, which understand the provided
      * {@link Query}. Results may be taken from a cache, if configured.
      *
-     * @param query    The query customized for a particular persistent storage or storage client
-     * @param useCache Whether to use the cache, if configured
+     * @param query The query customized for a particular persistent storage or storage client
      * @return query results
      */
-    public abstract Iterable<Object> executeQuery(Query query, boolean useCache);
+    public abstract QueryResult executeQuery(Query query);
 
     /**
      * Returns the schema for a given entity class.
@@ -177,16 +173,4 @@ private void populateMetaData(MetaDataStore metaDataStore) {
     public Table getTable(String classAlias) {
         return tables.get(classAlias);
     }
-
-    private static class NoCache implements Cache {
-        @Override
-        public Iterable<Object> get(Object key) {
-            return null;
-        }
-
-        @Override
-        public void put(Object key, Iterable<Object> result) {
-            // Do nothing
-        }
-    }
 }

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/query/Cache.java

@@ -6,22 +6,22 @@
 package com.yahoo.elide.datastores.aggregation.query;
 
 /**
- * A cache for Query results.
+ * A cache for {@link QueryResult}s.
  */
 public interface Cache {
     /**
-     * Load Query result from cache. Exceptions should be passed through.
+     * Load QueryResult from cache. Exceptions should be passed through.
      *
      * @param key    a key to look up in the cache.
      * @return query results from cache, or null if not found.
      */
-    Iterable<Object> get(Object key);
+    QueryResult get(Object key);
 
     /**
      * Insert results into cache.
      *
      * @param key    the key to associate with the query
      * @param result the result to cache with the key
      */
-    void put(Object key, Iterable<Object> result);
+    void put(Object key, QueryResult result);
 }

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/query/ImmutablePagination.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2020, Yahoo Inc.
+ * Licensed under the Apache License, Version 2.0
+ * See LICENSE file in project root for terms.
+ */
+
+package com.yahoo.elide.datastores.aggregation.query;
+
+import com.yahoo.elide.request.Pagination;
+
+import lombok.Value;
+
+/**
+ * An immutable Pagination. Doesn't support getPageTotals/setPageTotals; page totals must be returned via QueryResult.
+ */
+@Value
+public class ImmutablePagination implements Pagination {
+
+    private int offset;
+    private int limit;
+    private boolean defaultInstance;
+    private boolean returnPageTotals;
+
+    public static ImmutablePagination from(Pagination src) {
+        if (src instanceof ImmutablePagination) {
+            return (ImmutablePagination) src;
+        } else if (src != null) {
+            return new ImmutablePagination(
+                    src.getOffset(), src.getLimit(), src.isDefaultInstance(), src.returnPageTotals());
+        } else {
+            return null;
+        }
+    }
+
+    @Override
+    public boolean returnPageTotals() {
+        return returnPageTotals;
+    }
+
+    @Override
+    public Long getPageTotals() {
+        return null;
+    }
+
+    @Override
+    public void setPageTotals(Long pageTotals) {
+        throw new UnsupportedOperationException("ImmutablePagination does not support setPageTotals");
+    }
+}

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/query/Query.java

@@ -9,9 +9,9 @@
 import com.yahoo.elide.core.filter.expression.FilterExpression;
 import com.yahoo.elide.datastores.aggregation.QueryEngine;
 import com.yahoo.elide.datastores.aggregation.metadata.models.Table;
-import com.yahoo.elide.request.Pagination;
 import com.yahoo.elide.request.Sorting;
 import lombok.Builder;
+import lombok.NonNull;
 import lombok.Singular;
 import lombok.Value;
 
@@ -27,22 +27,28 @@
 @Value
 @Builder
 public class Query {
-    Table table;
+    @NonNull
+    private Table table;
 
     @Singular
-    List<MetricProjection> metrics;
+    private List<MetricProjection> metrics;
 
     @Singular
-    Set<ColumnProjection> groupByDimensions;
+    private Set<ColumnProjection> groupByDimensions;
 
     @Singular
-    Set<TimeDimensionProjection> timeDimensions;
+    private Set<TimeDimensionProjection> timeDimensions;
 
-    FilterExpression whereFilter;
-    FilterExpression havingFilter;
-    Sorting sorting;
-    Pagination pagination;
-    RequestScope scope;
+    private FilterExpression whereFilter;
+    private FilterExpression havingFilter;
+    private Sorting sorting;
+    private ImmutablePagination pagination;
+    private RequestScope scope;
+
+    /**
+     * Whether to bypass the {@link QueryEngine} cache for this query.
+     */
+    private boolean bypassingCache;
 
     /**
      * Returns all the dimensions regardless of type.

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/query/QueryResult.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2019, Yahoo Inc.
+ * Licensed under the Apache License, Version 2.0
+ * See LICENSE file in project root for terms.
+ */
+package com.yahoo.elide.datastores.aggregation.query;
+
+import com.yahoo.elide.datastores.aggregation.QueryEngine;
+import com.yahoo.elide.request.Pagination;
+
+import lombok.Builder;
+import lombok.NonNull;
+import lombok.Value;
+
+/**
+ * A {@link QueryResult} contains the results from {@link QueryEngine#executeQuery(Query)}.
+ */
+@Value
+@Builder
+public class QueryResult {
+    @NonNull
+    private Iterable<Object> data;
+
+    /**
+     * Total record count. Null unless Query had Pagination with {@link Pagination#returnPageTotals()} set.
+     */
+    private Long pageTotals;
+}

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/queryengines/AbstractEntityHydrator.java

@@ -27,7 +27,7 @@
 import java.util.stream.Collectors;
 
 /**
- * {@link AbstractEntityHydrator} hydrates the entity loaded by {@link QueryEngine#executeQuery(Query, boolean)}.
+ * {@link AbstractEntityHydrator} hydrates the entity loaded by {@link QueryEngine#executeQuery(Query)}.
  * <p>
  * {@link AbstractEntityHydrator} is not thread-safe and should be accessed by only 1 thread in this application,
  * because it uses {@link StitchList}. See {@link StitchList} for more details.
@@ -49,8 +49,8 @@ public abstract class AbstractEntityHydrator {
     /**
      * Constructor.
      *
-     * @param results The loaded objects from {@link QueryEngine#executeQuery(Query, boolean)}
-     * @param query  The query passed to {@link QueryEngine#executeQuery(Query, boolean)} to load the objects
+     * @param results The loaded objects from {@link QueryEngine#executeQuery(Query)}
+     * @param query  The query passed to {@link QueryEngine#executeQuery(Query)} to load the objects
      * @param entityDictionary  An object that sets entity instance values and provides entity metadata info
      */
     public AbstractEntityHydrator(List<Object> results, Query query, EntityDictionary entityDictionary) {