Inspection rework

yahoo · Oct 22, 2019 · d4bdee2 · d4bdee2
1 parent 24bed08
commit d4bdee2
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 245 deletions.
diff --git a/elide-core/src/main/java/com/yahoo/elide/core/EntityDictionary.java b/elide-core/src/main/java/com/yahoo/elide/core/EntityDictionary.java
@@ -1429,7 +1429,7 @@ public void addArgumentsToAttributes(Class<?> cls, String attributeName, Set<Arg
      * @param attributeName attribute name to which argument has to be added
      * @param argument A single argument
      */
-    public void addArgumentToAttributes(Class<?> cls, String attributeName, ArgumentType argument) {
+    public void addArgumentToAttribute(Class<?> cls, String attributeName, ArgumentType argument) {
         this.addArgumentsToAttributes(cls, attributeName, Sets.newHashSet(argument));
     }
 

diff --git a/elide-datastore/elide-datastore-aggregation/pom.xml.releaseBackup b/elide-datastore/elide-datastore-aggregation/pom.xml.releaseBackup
diff --git a/...ggregation/src/main/java/com/yahoo/elide/datastores/aggregation/AggregationDataStore.java b/...ggregation/src/main/java/com/yahoo/elide/datastores/aggregation/AggregationDataStore.java
@@ -35,7 +35,7 @@ public void populateEntityDictionary(EntityDictionary dictionary) {
         /* Add 'grain' argument to each TimeDimensionColumn */
         for (Schema schema: queryEngine.getSchemas()) {
             for (TimeDimensionColumn timeDim : schema.getTimeDimensions()) {
-                dictionary.addArgumentToAttributes(schema.getEntityClass(), timeDim.getName(),
+                dictionary.addArgumentToAttribute(schema.getEntityClass(), timeDim.getName(),
                         new ArgumentType("grain", String.class));
             }
         }

diff --git a/...tion/src/main/java/com/yahoo/elide/datastores/aggregation/AggregationDataStoreHelper.java b/...tion/src/main/java/com/yahoo/elide/datastores/aggregation/AggregationDataStoreHelper.java
@@ -102,55 +102,53 @@ private void splitFilters() {
         }
     }
 
-    //TODO - Add tests in the next PR.
     /**
      * Gets time dimensions based on relationships and attributes from {@link EntityProjection}.
      *
      * @return projections for time dimension columns
      * @throws InvalidOperationException Thrown if a requested time grain is not supported.
      */
     private Set<TimeDimensionProjection> resolveTimeDimensions() {
+        return entityProjection.getAttributes().stream()
+                .filter(attribute -> schema.getTimeDimension(attribute.getName()) != null)
+                .map(attribute -> {
+                    TimeDimensionColumn timeDim = schema.getTimeDimension(attribute.getName());
 
-        Set<TimeDimensionProjection> timeDims = new LinkedHashSet<>();
-        //Only attributes can be time dimensions
-        entityProjection.getAttributes().stream().forEach((attribute -> {
-            TimeDimensionColumn timeDim = schema.getTimeDimension(attribute.getName());
-            if (timeDim == null) {
-                return;
-            }
-
-            Argument timeGrainArgument = attribute.getArguments().stream()
-                    .filter(attr -> attr.getName().equals("grain"))
-                    .findAny()
-                    .orElse(null);
-
-            TimeGrainDefinition requestedGrainDefinition;
-            if (timeGrainArgument == null) {
+                    Argument timeGrainArgument = attribute.getArguments().stream()
+                            .filter(attr -> attr.getName().equals("grain"))
+                            .findAny()
+                            .orElse(null);
 
-                //The first grain is the default.
-                requestedGrainDefinition = timeDim.getSupportedGrains().iterator().next();
-            } else {
-                String requestedGrainName = timeGrainArgument.getValue().toString().toUpperCase(Locale.ENGLISH);
+                    TimeGrainDefinition requestedGrainDefinition;
+                    if (timeGrainArgument == null) {
 
-                TimeGrain requestedGrain;
-                try {
-                    requestedGrain = TimeGrain.valueOf(requestedGrainName);
-                } catch (IllegalArgumentException e) {
-                    throw new InvalidOperationException(String.format("Invalid grain %s", requestedGrainName));
-                }
+                        //The first grain is the default.
+                        requestedGrainDefinition = timeDim.getSupportedGrains().stream()
+                                .findFirst()
+                                .orElseThrow(() -> new IllegalStateException(
+                                        String.format("Requested default grain, no grain defined on %s",
+                                                attribute.getName())));
+                    } else {
+                        String requestedGrainName = timeGrainArgument.getValue().toString().toUpperCase(Locale.ENGLISH);
 
-                requestedGrainDefinition = timeDim.getSupportedGrains().stream()
-                        .filter(supportedGrainDef -> supportedGrainDef.grain().equals(requestedGrain))
-                        .findAny()
-                        .orElseThrow(() -> new InvalidOperationException(
-                                String.format("Requested grain %s, not supported on %s",
-                                        requestedGrainName, attribute.getName())));
-            }
+                        TimeGrain requestedGrain;
+                        try {
+                            requestedGrain = TimeGrain.valueOf(requestedGrainName);
+                        } catch (IllegalArgumentException e) {
+                            throw new InvalidOperationException(String.format("Invalid grain %s", requestedGrainName));
+                        }
 
-            timeDims.add(timeDim.toProjectedDimension(requestedGrainDefinition));
-        }));
+                        requestedGrainDefinition = timeDim.getSupportedGrains().stream()
+                                .filter(supportedGrainDef -> supportedGrainDef.grain().equals(requestedGrain))
+                                .findAny()
+                                .orElseThrow(() -> new InvalidOperationException(
+                                        String.format("Requested grain %s, not supported on %s",
+                                                requestedGrainName, attribute.getName())));
+                    }
 
-        return timeDims;
+                    return timeDim.toProjectedDimension(requestedGrainDefinition);
+                })
+                .collect(Collectors.toCollection(LinkedHashSet::new));
     }
 
     /**

diff --git a/...c/main/java/com/yahoo/elide/datastores/aggregation/queryengines/sql/schema/SQLSchema.java b/...c/main/java/com/yahoo/elide/datastores/aggregation/queryengines/sql/schema/SQLSchema.java
@@ -75,7 +75,7 @@ protected DimensionColumn constructDimension(String dimensionField,
             String columnName = getColumnName(entityClass, dimensionField);
 
             if (dim instanceof TimeDimensionColumn) {
-               return new SQLTimeDimensionColumn((TimeDimensionColumn) dim, columnName, getAlias());
+                return new SQLTimeDimensionColumn((TimeDimensionColumn) dim, columnName, getAlias());
             }
             return new SQLDimensionColumn(dim, columnName, getAlias());
         }

diff --git a/...om/yahoo/elide/datastores/aggregation/queryengines/sql/schema/SQLTimeDimensionColumn.java b/...om/yahoo/elide/datastores/aggregation/queryengines/sql/schema/SQLTimeDimensionColumn.java
@@ -11,8 +11,6 @@
 import com.yahoo.elide.datastores.aggregation.schema.dimension.TimeDimensionColumn;
 import com.yahoo.elide.datastores.aggregation.time.TimeGrain;
 
-import com.google.common.base.Preconditions;
-
 import java.util.Set;
 import java.util.TimeZone;
 
@@ -63,11 +61,9 @@ public Set<TimeGrainDefinition> getSupportedGrains() {
      * @return Something like "table_alias.column_name"
      */
     public String getColumnReference(TimeGrain requestedGrain) {
-        Preconditions.checkArgument(getSupportedGrains().stream()
-                .anyMatch((grainDef -> grainDef.grain().equals(requestedGrain))));
-
         TimeGrainDefinition definition = getSupportedGrains().stream()
-                .filter(grainDef -> grainDef.grain().equals(requestedGrain)).findFirst().get();
+                .filter(grainDef -> grainDef.grain().equals(requestedGrain)).findFirst()
+                .orElseThrow(() -> new IllegalStateException("Requested time grain not supported."));
 
         //TODO - We will likely migrate to a templating language when we support parameterized metrics.
         return String.format(definition.expression(), getColumnReference());

diff --git a/...store-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/schema/Schema.java b/...store-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/schema/Schema.java
@@ -53,7 +53,7 @@
 public class Schema {
 
     @Getter
-    public final Class<?> entityClass;
+    protected final Class<?> entityClass;
     @Getter
     protected final Set<Metric> metrics;
     protected final Map<String, DimensionColumn> dimensions;

diff --git a/...-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/example/SubCountry.java b/...-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/example/SubCountry.java
@@ -18,7 +18,7 @@
 import javax.persistence.Id;
 
 /**
- * A root level entity for testing AggregationDataStore with @Subselect annotation
+ * A root level entity for testing AggregationDataStore with @Subselect annotation.
  */
 @Data
 @Entity