PR feedback.

Clean up to Strings and Equals.
CHANGELOG

CHANGELOG.md

@@ -530,6 +530,8 @@ Changes:
 
 ### Deprecated:
 
+- [Deprecated DefaultingDictionary usage in DefaultingVolatileIntervalsService](https://github.com/yahoo/fili/pull/259)
+
 - [`RequestLog::switchTiming` has been deprecated](https://github.com/yahoo/fili/pull/141)
     - `RequestLog::switchTiming` is very context-dependent, and therefore brittle. In particular, adding any additional
       timers inside code called by a timed block may result in the original timer not stopping properly. All usages of
@@ -621,6 +623,13 @@ New Capabilities & Enhancements:
 
 ### Added:
 
+- [Added Dimension Value implementation for PartitionTableDefinition]
+    * Added `DimensionIdFilter` implementation of  `DataSourceFilter`
+    * Created `DimensionListPartitionTableDefinition` 
+
+- [Added 'hasAnyRows' to SearchProvider interface](https://github.com/yahoo/fili/pull/259)
+    * Has Any Rows allows implementations to optimize queries which only need to identify existence of matches
+
 - [Customize Logging Format in RequestLog](https://github.com/yahoo/fili/pull/81)
 
 - [Can Populate Dimension Rows from an AVRO file](https://github.com/yahoo/fili/pull/86)

fili-core/src/main/java/com/yahoo/bard/webservice/data/config/table/DimensionListPartitionTableDefinition.java

@@ -33,7 +33,7 @@ public class DimensionListPartitionTableDefinition extends PhysicalTableDefiniti
      * @param timeGrain  Zoned time grain of the table
      * @param metricNames  The Set of metric names on the table
      * @param dimensionConfigs  Set of dimensions on the table as dimension configs
-     * @param tablePartDefinitions A map from table names to the dimension value matches for that table
+     * @param tablePartDefinitions A map from table names to a map of dimension names to sets of values for those dimensions.  The named table will match if for every dimension named at least one of the set of values is part of the query.
      */
     public DimensionListPartitionTableDefinition(
             TableName name,

fili-core/src/main/java/com/yahoo/bard/webservice/data/volatility/DefaultingVolatileIntervalsService.java

@@ -5,6 +5,7 @@
 import com.yahoo.bard.webservice.druid.model.query.AllGranularity;
 import com.yahoo.bard.webservice.druid.model.query.Granularity;
 import com.yahoo.bard.webservice.table.PhysicalTable;
+import com.yahoo.bard.webservice.util.DefaultingDictionary;
 import com.yahoo.bard.webservice.util.IntervalUtils;
 import com.yahoo.bard.webservice.util.SimplifiedIntervalList;
 
@@ -18,7 +19,7 @@
 import java.util.Map;
 
 /**
- * An implementation of VolatileIntervalsService backed by DefaultingDictionary.
+ * An implementation of VolatileIntervalsService.
  */
 public class DefaultingVolatileIntervalsService implements VolatileIntervalsService {
 
@@ -50,6 +51,20 @@ public DefaultingVolatileIntervalsService(VolatileIntervalsFunction defaultInter
         this(defaultIntervalsFunction, Collections.emptyMap());
     }
 
+    /**
+     * Use the map of specific functions for physical tables.
+     *
+     * @param intervalsFunctions  the map of specific functions for physical tables
+     *
+     * @deprecated Simply use maps and default values
+     */
+    @Deprecated
+    public DefaultingVolatileIntervalsService(
+                DefaultingDictionary<PhysicalTable, VolatileIntervalsFunction> intervalsFunctions
+    ) {
+        this(intervalsFunctions.getDefaultValue(), intervalsFunctions);
+    }
+
     /**
      * Use a default function with overrides for particular tables.
      *

fili-core/src/main/java/com/yahoo/bard/webservice/table/BasePhysicalTable.java

@@ -142,7 +142,9 @@ protected void setAvailability(Availability availability) {
     }
 
     @Override
-    public boolean equals(final Object obj) {
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
         if (obj instanceof BasePhysicalTable) {
             BasePhysicalTable that = (BasePhysicalTable) obj;
             return Objects.equals(name.asName(), that.name.asName())

fili-core/src/main/java/com/yahoo/bard/webservice/table/BaseSchema.java

@@ -50,6 +50,11 @@ public Granularity getGranularity() {
         return granularity;
     }
 
+    @Override
+    public String toString() {
+        return String.format("%s: columns: %s granularity: %s", getClass().getSimpleName(), columns, granularity);
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {

fili-core/src/main/java/com/yahoo/bard/webservice/table/PartitionCompositeTable.java

@@ -56,9 +56,4 @@ private static Availability buildAvailability(Map<PhysicalTable, DataSourceFilte
         return new PartitionAvailability(dataSourceFilterMap.entrySet().stream()
                 .collect(Collectors.toMap(entry -> entry.getKey().getAvailability(), Map.Entry::getValue)));
     }
-
-    @Override
-    public String toString() {
-        return getName() + ":" + getSchema().getTimeGrain() + ":" + getAvailability();
-    }
 }

fili-core/src/main/java/com/yahoo/bard/webservice/table/PhysicalTableSchema.java

@@ -37,8 +37,9 @@ public PhysicalTableSchema(
         super(timeGrain, columns);
         this.timeGrain = timeGrain;
 
-        this.logicalToPhysicalColumnNames = Collections.unmodifiableMap(new LinkedHashMap<>
-                (logicalToPhysicalColumnNames));
+        this.logicalToPhysicalColumnNames = Collections.unmodifiableMap(
+                new LinkedHashMap<>(logicalToPhysicalColumnNames)
+        );
         this.physicalToLogicalColumnNames = Collections.unmodifiableMap(
                 this.logicalToPhysicalColumnNames.entrySet().stream().collect(
                         Collectors.groupingBy(
@@ -101,10 +102,14 @@ public ZonedTimeGrain getTimeGrain() {
     }
 
     @Override
-    public boolean equals(final Object o) {
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
         if (o instanceof PhysicalTableSchema) {
             PhysicalTableSchema that = (PhysicalTableSchema) o;
-            return Objects.equals(timeGrain, that.timeGrain)
+            return super.equals(o)
+                    && Objects.equals(timeGrain, that.timeGrain)
                     && Objects.equals(logicalToPhysicalColumnNames, that.logicalToPhysicalColumnNames)
                     && Objects.equals(physicalToLogicalColumnNames, that.physicalToLogicalColumnNames);
         }
@@ -113,11 +118,11 @@ public boolean equals(final Object o) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(timeGrain, logicalToPhysicalColumnNames, physicalToLogicalColumnNames);
+        return Objects.hash(super.hashCode(), timeGrain, logicalToPhysicalColumnNames, physicalToLogicalColumnNames);
     }
 
     @Override
     public String toString() {
-        return "time grain: " + timeGrain + ": logicalToPhysicalColumnNames: " + logicalToPhysicalColumnNames;
+        return String.format("%s logicalToPhysicalNameMap: %s", super.toString(), logicalToPhysicalColumnNames);
     }
 }

fili-core/src/main/java/com/yahoo/bard/webservice/table/availability/ConcreteAvailability.java

@@ -87,7 +87,7 @@ public String toString() {
     }
 
     @Override
-    public boolean equals(final Object obj) {
+    public boolean equals(Object obj) {
         if (obj instanceof ConcreteAvailability) {
             ConcreteAvailability that = (ConcreteAvailability) obj;
             return Objects.equals(name.asName(), that.name.asName())

fili-core/src/main/java/com/yahoo/bard/webservice/table/availability/MetricUnionAvailability.java

@@ -192,13 +192,13 @@ public String toString() {
     }
 
     @Override
-    public boolean equals(final Object obj) {
+    public boolean equals(Object obj) {
         if (obj instanceof MetricUnionAvailability) {
             MetricUnionAvailability that = (MetricUnionAvailability) obj;
             return Objects.equals(metricNames, that.metricNames)
                     && Objects.equals(availabilitiesToMetricNames, that.availabilitiesToMetricNames);
         }
-        return super.equals(obj);
+        return false;
     }
 
     @Override

fili-core/src/main/java/com/yahoo/bard/webservice/web/util/PaginationParameters.java

@@ -39,10 +39,7 @@ public class PaginationParameters {
     /**
      * Return only the first result.
      */
-    public static final PaginationParameters ONE_RESULT = new PaginationParameters(
-            1,
-            1
-    );
+    public static final PaginationParameters ONE_RESULT = new PaginationParameters(1, 1);
 
     private final int perPage;
     private final int page;

fili-core/src/test/groovy/com/yahoo/bard/webservice/data/config/table/DimensionListPartitionTableDefinitionSpec.groovy

@@ -10,7 +10,6 @@ import com.yahoo.bard.webservice.data.dimension.Dimension
 import com.yahoo.bard.webservice.data.dimension.DimensionDictionary
 import com.yahoo.bard.webservice.data.dimension.DimensionField
 import com.yahoo.bard.webservice.metadata.DataSourceMetadataService
-import com.yahoo.bard.webservice.table.Column
 import com.yahoo.bard.webservice.table.PartitionCompositeTable
 import com.yahoo.bard.webservice.table.PhysicalTable
 import com.yahoo.bard.webservice.table.PhysicalTableSchema
@@ -77,8 +76,8 @@ class DimensionListPartitionTableDefinitionSpec extends Specification {
         DimensionListPartitionTableDefinition definition = new DimensionListPartitionTableDefinition(
                 TableName.of("partition"),
                 DAY_UTC,
-                Collections.emptySet(),
-                Collections.emptySet(),
+                [] as Set,
+                [] as Set,
                 mappings
         )
 
@@ -87,7 +86,7 @@ class DimensionListPartitionTableDefinitionSpec extends Specification {
         PartitionCompositeTable expected = new PartitionCompositeTable(
                 TableName.of("partition"),
                 DAY_UTC,
-                (Set<Column>) Collections.emptySet(),
+                [] as Set,
                 [:],
                 [(part1): new DimensionIdFilter([(testDimension): (["part1Value"] as Set)]),
                  (part2): new DimensionIdFilter([(testDimension): (["part2Value"] as Set)])]

fili-core/src/test/groovy/com/yahoo/bard/webservice/data/dimension/DimensionRowSpec.groovy

@@ -17,6 +17,7 @@ class DimensionRowSpec extends Specification {
         keyField.name >> keyFieldFieldName
         nonKeyField.name >> nonKeyFieldName
     }
+
     def "healthy initialization returns correct state"() {
         setup:
         DimensionRow testRow = new DimensionRow(keyField, [(keyField): keyValue, (nonKeyField): nonKeyValue])

fili-core/src/test/groovy/com/yahoo/bard/webservice/data/volatility/DefaultingVolatileIntervalsServiceSpec.groovy

@@ -13,6 +13,7 @@ import org.joda.time.DateTime
 import org.joda.time.Interval
 
 import spock.lang.Specification
+
 /**
  * Test the default intervals service
  */
@@ -63,9 +64,7 @@ class DefaultingVolatileIntervalsServiceSpec extends Specification {
         allGrainQuery.intervals >> fullRange
         allGrainQuery.granularity >> AllGranularity.INSTANCE
 
-        intervalsFunctions = new HashMap<>()
-        intervalsFunctions.put(table2, mockFunction2)
-        intervalsFunctions.put(table3, mockFunction3)
+        intervalsFunctions = [(table2): mockFunction2, (table3): mockFunction3]
     }
 
 

fili-core/src/test/groovy/com/yahoo/bard/webservice/table/resolver/DimensionIdFilterSpec.groovy

@@ -60,7 +60,7 @@ class DimensionIdFilterSpec extends Specification {
         searchProvider.hasAnyRows(_) >> searchRowsExist
 
         expect: "filter matches when there are no constraints or they exist and return rows from search provider"
-        expected == filter.emptyConstraintOrAnyRows(dimension, constraintMap)
+        filter.emptyConstraintOrAnyRows(dimension, constraintMap) == expected
 
         where:
         hasApiFilter  | emptyApiFilter | searchRowsExist | expected | description
@@ -116,10 +116,10 @@ class DimensionIdFilterSpec extends Specification {
         }
 
         expect: "True as long as the dimension that doesnt return rows is not constrained"
-        dimensionIdFilter.apply(constraint) == success
+        dimensionIdFilter.apply(constraint) == expected
 
         where:
-        constrainFailure | constrainSuccess | constrainOther | success
+        constrainFailure | constrainSuccess | constrainOther | expected
         true             | true             | true           | false
         true             | true             | false          | false
         true             | false            | true           | false