address @cdeszaq's comments

yahoo · Apr 7, 2017 · 3a33f14 · 3a33f14
1 parent 816b0f8
commit 3a33f14
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 24 deletions.
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/util/IntervalUtils.java b/fili-core/src/main/java/com/yahoo/bard/webservice/util/IntervalUtils.java
@@ -6,14 +6,16 @@
 import com.yahoo.bard.webservice.config.SystemConfigProvider;
 import com.yahoo.bard.webservice.data.time.ZonedTimeGrain;
 import com.yahoo.bard.webservice.druid.model.query.Granularity;
+import com.yahoo.bard.webservice.table.PhysicalTable;
+import com.yahoo.bard.webservice.table.PhysicalTableSchema;
+import com.yahoo.bard.webservice.table.resolver.GranularityComparator;
 
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 import org.joda.time.Interval;
 import org.joda.time.base.AbstractInterval;
 
 import java.util.Collection;
-import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -32,6 +34,8 @@
  */
 public class IntervalUtils {
 
+    private static final GranularityComparator COMPARATOR = GranularityComparator.getInstance();
+
     public static final SystemConfig SYSTEM_CONFIG = SystemConfigProvider.getInstance();
 
     public static final String ALIGNMENT_EPOCH_STRING = SYSTEM_CONFIG.getStringProperty(
@@ -213,18 +217,17 @@ public static Optional<DateTime> firstMoment(Collection<? extends Collection<Int
     }
 
     /**
-     * Returns the coarsest ZonedTimeGrain among a set of ZonedTimeGrains.
-     * <p>
-     * If the set of ZonedTimeGrains is empty, return null.
+     * Returns the coarsest ZonedTimeGrain among a set of PhysicalTables.
      *
-     * @param zonedTimeGrains  A set of ZonedTimeGrains among which the coarsest ZonedTimeGrain is to be returned.
+     * @param physicalTables  A set of PhysicalTables among which the coarsest ZonedTimeGrain is to be found.
      *
-     * @return the coarsest ZonedTimeGrain among a set of ZonedTimeGrains
+     * @return the coarsest ZonedTimeGrain among a set of PhysicalTables
      */
-    public static ZonedTimeGrain getCoarsestTimeGrain(Collection<ZonedTimeGrain> zonedTimeGrains) {
-        return zonedTimeGrains.stream()
-                .sorted(Comparator.comparing(ZonedTimeGrain::getEstimatedDuration))
+    public static Optional<ZonedTimeGrain> getCoarsestTimeGrain(Collection<PhysicalTable> physicalTables) {
+        return physicalTables.stream()
+                .sorted(COMPARATOR)
                 .findFirst()
-                .orElse(null);
+                .map(PhysicalTable::getSchema)
+                .map(PhysicalTableSchema::getTimeGrain);
     }
 }
diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/util/IntervalUtilsSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/util/IntervalUtilsSpec.groovy
@@ -3,17 +3,20 @@
 package com.yahoo.bard.webservice.util
 
 import static com.yahoo.bard.webservice.data.time.DefaultTimeGrain.DAY
+import static com.yahoo.bard.webservice.data.time.DefaultTimeGrain.HOUR
+import static com.yahoo.bard.webservice.data.time.DefaultTimeGrain.MINUTE
 import static com.yahoo.bard.webservice.data.time.DefaultTimeGrain.MONTH
 import static com.yahoo.bard.webservice.data.time.DefaultTimeGrain.WEEK
 import static com.yahoo.bard.webservice.data.time.DefaultTimeGrain.YEAR
 
 import com.yahoo.bard.webservice.data.time.ZonedTimeGrain
 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.table.PhysicalTableSchema
 
 import org.joda.time.DateTime
 import org.joda.time.DateTimeZone
-import org.joda.time.Duration
 import org.joda.time.Interval
 
 import spock.lang.Specification
@@ -329,29 +332,43 @@ class IntervalUtilsSpec extends Specification {
     }
 
     @Unroll
-    def "getCoarsestTimeGrain returns duration of #expected among #duration1, #duration2, #duration3"() {
+    def "getCoarsestTimeGrain returns coarsest grain in the case of #description"() {
         given:
-        ZonedTimeGrain zonedTimeGrain1 = Mock(ZonedTimeGrain)
-        ZonedTimeGrain zonedTimeGrain2 = Mock(ZonedTimeGrain)
-        ZonedTimeGrain zonedTimeGrain3 = Mock(ZonedTimeGrain)
+        PhysicalTableSchema schema1 = Mock(PhysicalTableSchema)
+        PhysicalTableSchema schema2 = Mock(PhysicalTableSchema)
+        PhysicalTableSchema schema3 = Mock(PhysicalTableSchema)
 
-        zonedTimeGrain1.getEstimatedDuration() >> new Duration(duration1)
-        zonedTimeGrain2.getEstimatedDuration() >> new Duration(duration2)
-        zonedTimeGrain3.getEstimatedDuration() >> new Duration(duration3)
+        schema1.getTimeGrain() >> new ZonedTimeGrain(timeGrain1, DateTimeZone.forID(timeZone1))
+        schema2.getTimeGrain() >> new ZonedTimeGrain(timeGrain2, DateTimeZone.forID(timeZone2))
+        schema3.getTimeGrain() >> new ZonedTimeGrain(timeGrain3, DateTimeZone.forID(timeZone3))
+
+        PhysicalTable physicalTable1 = Mock(PhysicalTable)
+        PhysicalTable physicalTable2 = Mock(PhysicalTable)
+        PhysicalTable physicalTable3 = Mock(PhysicalTable)
+
+        physicalTable1.getSchema() >> schema1
+        physicalTable2.getSchema() >> schema2
+        physicalTable3.getSchema() >> schema3
 
         expect:
-        IntervalUtils.getCoarsestTimeGrain([zonedTimeGrain1, zonedTimeGrain2, zonedTimeGrain3]).getEstimatedDuration().equals(new Duration(expected))
+        IntervalUtils.getCoarsestTimeGrain(
+                [
+                        physicalTable1,
+                        physicalTable2,
+                        physicalTable3
+                ]
+        ).get() == new ZonedTimeGrain(expectedTimeGrain, DateTimeZone.forID(expectedTimeZone))
 
         where:
-        duration1 | duration2 | duration3 | expected
-        1         | 2         | 3         | 1
-        1         | 1         | 3         | 1
-        3         | 3         | 3         | 3
+        timeGrain1 | timeGrain2 | timeGrain3 | timeZone1         | timeZone2             | timeZone3         | expectedTimeGrain | expectedTimeZone  | description
+        DAY        | DAY        | DAY        | 'America/Chicago' | 'America/Los_Angeles' | 'America/Phoenix' | DAY               | 'America/Chicago' | 'same grain but different time zones'
+        MINUTE     | HOUR       | DAY        | 'America/Phoenix' | 'America/Phoenix'     | 'America/Phoenix' | DAY               | 'America/Phoenix' | 'same time zone but different grans'
+        MINUTE     | HOUR       | DAY        | 'America/Chicago' | 'America/Los_Angeles' | 'America/Phoenix' | DAY               | 'America/Phoenix' | 'different grains and different time zones'
     }
 
     def "getCoarsestTimeGrain returns null on empty input time grain collections"() {
         expect:
-        IntervalUtils.getCoarsestTimeGrain(Collections.emptyList()) == null
+        IntervalUtils.getCoarsestTimeGrain(Collections.emptyList()) == Optional.empty()
     }
 
     /**