yahoo · cdeszaq · Apr 14, 2017 · Apr 14, 2017 · Apr 14, 2017 · Apr 14, 2017
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,10 @@ Current
 -------
 ### Added:
 
+- [BaseCompositePhysicalTable](https://github.com/yahoo/fili/pull/242)
+    * `ConcretePhysicalTable` provides common operations, such as validating coarsest ZonedTimeGrain, for composite
+    tables.
+
 - [Add Reciprocal `satisfies()` relationship complementing `satisfiedBy()` on Granularity](https://github.com/yahoo/fili/issues/222)
 
 - [MetricUnionAvailability and MetricUnionCompositeTable](https://github.com/yahoo/fili/pull/193)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/table/BaseCompositePhysicalTable.java b/fili-core/src/main/java/com/yahoo/bard/webservice/table/BaseCompositePhysicalTable.java
@@ -0,0 +1,82 @@
+// Copyright 2017 Yahoo Inc.
+// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
+package com.yahoo.bard.webservice.table;
+
+import com.yahoo.bard.webservice.data.config.names.TableName;
+import com.yahoo.bard.webservice.data.time.ZonedTimeGrain;
+import com.yahoo.bard.webservice.table.availability.Availability;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Set;
+
+import javax.validation.constraints.NotNull;
+
+/**
+ * An implementation of BasePhysicalTable that contains multiple tables.
+ */
+public class BaseCompositePhysicalTable extends BasePhysicalTable {
+
+    private static final Logger LOG = LoggerFactory.getLogger(BaseCompositePhysicalTable.class);
+
+    /**
+     * Constructor.
+     *
+     * @param name  Name that represents set of fact table names that are put together under this table
+     * @param timeGrain  The time grain of the table. The time grain has to satisfy all grains of the tables
+     * @param columns  The columns for this table
+     * @param physicalTables  A set of PhysicalTables that are put together under this table. The
+     * tables shall have zoned time grains that all satisfy the provided timeGrain
+     * @param logicalToPhysicalColumnNames  Mappings from logical to physical names
+     * @param availability  The Availability of this table
+     */
+    public BaseCompositePhysicalTable(
+            @NotNull TableName name,
+            @NotNull ZonedTimeGrain timeGrain,
+            @NotNull Set<Column> columns,
+            @NotNull Set<PhysicalTable> physicalTables,
+            @NotNull Map<String, String> logicalToPhysicalColumnNames,
+            @NotNull Availability availability
+    ) {
+        super(
+                name,
+                timeGrain,
+                columns,
+                logicalToPhysicalColumnNames,
+                availability
+        );
+        verifyGrainSatisfiesAllTables(getSchema().getTimeGrain(), physicalTables, name);
+    }
+
+    /**
+     * Verifies that the coarsest ZonedTimeGrain satisfies all tables.
+     *
+     * @param coarsestTimeGrain  The coarsest ZonedTimeGrain to be verified
+     * @param physicalTables  A set of PhysicalTables whose ZonedTimeGrains are checked to make sure
+     * they all satisfies with the given coarsest ZonedTimeGrain
+     * @param name  Name of the current MetricUnionCompositeTable that represents set of fact table names
+     * joined together
+     *
+     * @throws IllegalArgumentException when there is no mutually satisfying grain among the table's time grains
+     */
+    private static void verifyGrainSatisfiesAllTables(
+            ZonedTimeGrain coarsestTimeGrain,
+            Set<PhysicalTable> physicalTables,
+            TableName name
+    ) throws IllegalArgumentException {
+        if (!physicalTables.stream()
+                .map(PhysicalTable::getSchema)
+                .map(PhysicalTableSchema::getTimeGrain)
+                .allMatch(coarsestTimeGrain::satisfies)) {
+            String message = String.format(
+                    "There is no mutually satisfying grain among: %s for current table %s",
+                    physicalTables,
+                    name.asName()
+            );
+            LOG.error(message);
+            throw new IllegalArgumentException(message);
+        }
+    }
+}
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/table/MetricUnionCompositeTable.java b/fili-core/src/main/java/com/yahoo/bard/webservice/table/MetricUnionCompositeTable.java
@@ -5,10 +5,6 @@
 import com.yahoo.bard.webservice.data.config.names.TableName;
 import com.yahoo.bard.webservice.data.time.ZonedTimeGrain;
 import com.yahoo.bard.webservice.table.availability.MetricUnionAvailability;
-import com.yahoo.bard.webservice.util.IntervalUtils;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.util.Map;
 import java.util.Set;
@@ -55,71 +51,32 @@
  * and this joined table is backed by the <tt>MetricUnionAvailability</tt>
  *
  */
-public class MetricUnionCompositeTable extends BasePhysicalTable {
-    private static final Logger LOG = LoggerFactory.getLogger(MetricUnionCompositeTable.class);
+public class MetricUnionCompositeTable extends BaseCompositePhysicalTable {
 
     /**
      * Constructor.
      *
      * @param name  Name that represents set of fact table names joined together
+     * @param timeGrain  The time grain of the table. The time grain has to satisfy all grains of the tables
      * @param columns  The columns for this table
-     * @param physicalTables  A set of <tt>PhysicalTable</tt>s whose same metric schema is to be joined together. The
-     * tables will be used to construct MetricUnionAvailability, as well as to compute common/coarsest time grain among
-     * them. The <tt>PhysicalTable</tt>s needs to have mutually satisfying time grains in order to calculate the
-     * common/coarsest time grain.
+     * @param physicalTables  A set of PhysicalTables that are put together under this table. The
+     * tables shall have zoned time grains that all satisfy the provided timeGrain
      * @param logicalToPhysicalColumnNames  Mappings from logical to physical names
      */
     public MetricUnionCompositeTable(
             @NotNull TableName name,
+            @NotNull ZonedTimeGrain timeGrain,
             @NotNull Set<Column> columns,
             @NotNull Set<PhysicalTable> physicalTables,
             @NotNull Map<String, String> logicalToPhysicalColumnNames
     ) {
         super(
                 name,
-                IntervalUtils.getCoarsestTimeGrain(physicalTables).orElseThrow(() -> {
-                    String message = String.format(
-                            "At least 1 physical table needs to be provided in order to calculate " +
-                                    "coarsest time grain for %s",
-                            name.asName()
-                    );
-                    LOG.error(message);
-                    return new IllegalArgumentException(message);
-                }),
+                timeGrain,
                 columns,
+                physicalTables,
                 logicalToPhysicalColumnNames,
                 new MetricUnionAvailability(physicalTables, columns)
         );
-        verifyGrainSatisfiesAllTables(getSchema().getTimeGrain(), physicalTables, name);
-    }
-
-    /**
-     * Verifies that the coarsest <tt>ZonedTimeGrain</tt> satisfies all tables.
-     *
-     * @param coarsestTimeGrain  The coarsest <tt>ZonedTimeGrain</tt> to be verified
-     * @param physicalTables  A set of <tt>PhysicalTable</tt>s whose <tt>ZonedTimeGrain</tt>s are checked to make sure
-     * they all satisfies with the given coarsest <tt>ZonedTimeGrain</tt>
-     * @param name  Name of the current <tt>MetricUnionCompositeTable</tt> that represents set of fact table names
-     * joined together
-     *
-     * @throws IllegalArgumentException when there is no mutually satisfying grain among the table's time grains
-     */
-    private static void verifyGrainSatisfiesAllTables(
-            ZonedTimeGrain coarsestTimeGrain,
-            Set<PhysicalTable> physicalTables,
-            TableName name
-    ) throws IllegalArgumentException {
-        if (!physicalTables.stream()
-                .map(PhysicalTable::getSchema)
-                .map(PhysicalTableSchema::getTimeGrain)
-                .allMatch(grain -> grain.satisfiedBy(coarsestTimeGrain))) {
-            String message = String.format("There is no mutually satisfying grain among: %s for current table " +
-                    "%s",
-                    physicalTables,
-                    name.asName()
-            );
-            LOG.error(message);
-            throw new IllegalArgumentException(message);
-        }
     }
 }
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
@@ -4,10 +4,7 @@
 
 import com.yahoo.bard.webservice.config.SystemConfig;
 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;
@@ -215,19 +212,4 @@ public static Optional<DateTime> firstMoment(Collection<? extends Collection<Int
                 .map(Interval::getStart)
                 .reduce(Utils::getMinValue);
     }
-
-    /**
-     * Returns the coarsest ZonedTimeGrain among a set of PhysicalTables.
-     *
-     * @param physicalTables  A set of PhysicalTables among which the coarsest ZonedTimeGrain is to be found.
-     *
-     * @return the coarsest ZonedTimeGrain among a set of PhysicalTables
-     */
-    public static Optional<ZonedTimeGrain> getCoarsestTimeGrain(Collection<PhysicalTable> physicalTables) {
-        return physicalTables.stream()
-                .sorted(COMPARATOR)
-                .findFirst()
-                .map(PhysicalTable::getSchema)
-                .map(PhysicalTableSchema::getTimeGrain);
-    }
 }
diff --git a/...able/MetricUnionCompositeTableSpec.groovy → ...ble/BaseCompositePhysicalTableSpec.groovy b/...able/MetricUnionCompositeTableSpec.groovy → ...ble/BaseCompositePhysicalTableSpec.groovy
@@ -4,29 +4,11 @@ package com.yahoo.bard.webservice.table
 
 import com.yahoo.bard.webservice.data.config.names.TableName
 import com.yahoo.bard.webservice.data.time.ZonedTimeGrain
+import com.yahoo.bard.webservice.table.availability.Availability
 
 import spock.lang.Specification
 
-class MetricUnionCompositeTableSpec extends Specification {
-    TableName tableName
-
-    def setup() {
-        tableName =  TableName.of("table1")
-    }
-
-    def "Constructor throws illegal argument exception on empty physical tables"() {
-        when:
-        MetricUnionCompositeTable metricUnionCompositeTable = new MetricUnionCompositeTable(
-                tableName,
-                [] as Set,
-                [] as Set,
-                [:]
-        )
-
-        then:
-        IllegalArgumentException illegalArgumentException = thrown()
-        illegalArgumentException.message == 'At least 1 physical table needs to be provided in order to calculate coarsest time grain for table1'
-    }
+class BaseCompositePhysicalTableSpec extends Specification {
 
     def "verifyGrainSatisfiesAllTables throws illegal argument exception on non-mutually satisfying grain among physical tables"() {
         given:
@@ -50,7 +32,7 @@ class MetricUnionCompositeTableSpec extends Specification {
         physicalTable2.getSchema() >> schema2
 
         when:
-        MetricUnionCompositeTable.verifyGrainSatisfiesAllTables(coarsestTimeGrain, [physicalTable1, physicalTable2] as Set, tableName)
+        MetricUnionCompositeTable.verifyGrainSatisfiesAllTables(coarsestTimeGrain, [physicalTable1, physicalTable2] as Set, TableName.of("table1"))
 
         then:
         IllegalArgumentException illegalArgumentException = thrown()

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
@@ -332,51 +332,6 @@ class IntervalUtilsSpec extends Specification {
         DAY   | ["2012-02-03/2017-04-04"] | ["2012-02-03/2012-05-04", "2017-02-03/2017-04-04"]
     }
 
-    @Unroll
-    def "getCoarsestTimeGrain returns coarsest grain in the case of #description"() {
-        given:
-        PhysicalTableSchema schema1 = Mock(PhysicalTableSchema)
-        PhysicalTableSchema schema2 = Mock(PhysicalTableSchema)
-        PhysicalTableSchema schema3 = Mock(PhysicalTableSchema)
-
-        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(
-                [
-                        physicalTable1,
-                        physicalTable2,
-                        physicalTable3
-                ]
-        ).get() == new ZonedTimeGrain(expectedTimeGrain, DateTimeZone.forID(expectedTimeZone))
-
-        where:
-        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, with DAY as the coarsest grain, and different time zones'
-        HOUR       | DAY        | WEEK       | 'America/Chicago' | 'America/Los_Angeles' | 'America/Phoenix' | WEEK              | 'America/Phoenix' | 'different grains, with WEEK as the coarsest grain, and different time zones'
-        DAY        | WEEK       | MONTH      | 'America/Chicago' | 'America/Los_Angeles' | 'America/Phoenix' | MONTH             | 'America/Phoenix' | 'different grains, with MONTH as the coarsest grain, and different time zones'
-        WEEK       | MONTH      | QUARTER    | 'America/Chicago' | 'America/Los_Angeles' | 'America/Phoenix' | QUARTER           | 'America/Phoenix' | 'different grains, with QUARTER as the coarsest grain, and different time zones'
-        MONTH      | QUARTER    | YEAR       | 'America/Chicago' | 'America/Los_Angeles' | 'America/Phoenix' | YEAR              | 'America/Phoenix' | 'different grains, with YEAR as the coarsest grain, and different time zones'
-
-    }
-
-    def "getCoarsestTimeGrain returns empty on empty input physical table collections"() {
-        expect:
-        IntervalUtils.getCoarsestTimeGrain([]) == Optional.empty()
-    }
-
     /**
      * Returns the instant at which this year began.
      *