Skip to content

Commit

Permalink
Rename filter variables and methods in DataApiRequest (#507)
Browse files Browse the repository at this point in the history
  • Loading branch information
mpardesh authored and archolewa committed Aug 25, 2017
1 parent 118a4ff commit 6c64862
Show file tree
Hide file tree
Showing 14 changed files with 81 additions and 50 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ Current

### Changed:


- [Rename filter variables and methods in DataApiRequest](https://github.com/yahoo/fili/pull/507)
* The method names `getFilter` and `getFilters` can be confusing, as well as the `filters` variable

- [Decoupled from static dimension lookup building]()
* Instead of `ModelUtils`, create an interface for `ExtractionFunctionDimension` and rebase `LookupDimension` and `RegisteredLookupDimension` on that interface.
* `LookupDimensionToDimensionSpec` now uses only the Extraction interface to decide how to serialize dimensions.
Expand Down Expand Up @@ -87,6 +91,9 @@ Current

### Deprecated:

- [Rename filter variables and methods in DataApiRequest](https://github.com/yahoo/fili/pull/507)
* Deprecated `getFilters` in favor of `getApiFilters` and `getFilter` in favor of `getDruidFilter`

- [Add dimension dictionary to metric loader](https://github.com/yahoo/fili/pull/317)
* Deprecated single argument version of `loadMetricDictionary` in `MetricLoader`, favor additional dimension dictionary argument `loadMetricDictionary` instead

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public DruidAggregationQuery<?> buildQuery(
request.getGranularity(),
request.getTimeZone(),
request.getDimensions(),
request.getFilter(),
request.getDruidFilter(),
request.getIntervals(),
druidTopNMetric,
request.getTopN().getAsInt()
Expand All @@ -137,7 +137,7 @@ public DruidAggregationQuery<?> buildQuery(
table,
request.getGranularity(),
request.getTimeZone(),
request.getFilter(),
request.getDruidFilter(),
request.getIntervals()
) :
buildGroupByQuery(
Expand All @@ -146,7 +146,7 @@ public DruidAggregationQuery<?> buildQuery(
request.getGranularity(),
request.getTimeZone(),
request.getDimensions(),
request.getFilter(),
request.getDruidFilter(),
request.getHaving(),
request.getIntervals(),
druidOrderBy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public DataSourceConstraint(DataApiRequest dataApiRequest, DruidAggregationQuery
this.filterDimensions = Collections.unmodifiableSet(dataApiRequest.getFilterDimensions());
this.metricDimensions = Collections.unmodifiableSet(templateDruidQuery.getMetricDimensions());
this.metricNames = Collections.unmodifiableSet(templateDruidQuery.getDependentFieldNames());
this.apiFilters = Collections.unmodifiableMap(dataApiRequest.getFilters());
this.apiFilters = Collections.unmodifiableMap(dataApiRequest.getApiFilters());
this.allDimensions = Collections.unmodifiableSet(Stream.of(
getRequestDimensions().stream(),
getFilterDimensions().stream(),
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private void logRequestMetrics(DataApiRequest request, Boolean readCache, DruidQ
new DataRequest(
table,
request.getIntervals(),
request.getFilters().values(),
request.getApiFilters().values(),
metrics,
dimensions,
druidQuery.getDataSource().getNames(),
Expand Down Expand Up @@ -419,7 +419,7 @@ public void getData(

try (TimedPhase timer = RequestLog.startTiming("logRequestMetrics")) {
logRequestMetrics(apiRequest, readCache, druidQuery);
RequestLog.record(new DruidFilterInfo(apiRequest.getFilter()));
RequestLog.record(new DruidFilterInfo(apiRequest.getDruidFilter()));
}

// Process the request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ import com.yahoo.bard.webservice.druid.model.query.TopNQuery
import com.yahoo.bard.webservice.metadata.DataSourceMetadataService
import com.yahoo.bard.webservice.table.ConstrainedTable
import com.yahoo.bard.webservice.table.TableTestUtils
import com.yahoo.bard.webservice.table.StrictPhysicalTable
import com.yahoo.bard.webservice.table.PhysicalTable
import com.yahoo.bard.webservice.table.resolver.DefaultPhysicalTableResolver
import com.yahoo.bard.webservice.web.ApiFilter
import com.yahoo.bard.webservice.web.ApiHaving
Expand Down Expand Up @@ -121,7 +119,7 @@ class DruidQueryBuilderSpec extends Specification {
apiRequest.getGranularity() >> HOUR.buildZonedTimeGrain(UTC)
apiRequest.getTimeZone() >> UTC
apiRequest.getDimensions() >> ([resources.d1] as Set)
apiRequest.getFilters() >> apiFilters
apiRequest.getApiFilters() >> apiFilters
apiRequest.getLogicalMetrics() >> ([lm1] as Set)
apiRequest.getIntervals() >> intervals
apiRequest.getFilterDimensions() >> []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class DimensionToDefaultDimensionSpecSpec extends Specification {
apiRequest.getTopN() >> OptionalInt.empty()
apiRequest.getSorts() >> ([])
apiRequest.getCount() >> OptionalInt.empty()
apiRequest.getFilters() >> Collections.emptyMap()
apiRequest.getApiFilters() >> Collections.emptyMap()
}

def "Serialize to apiName when apiName and physicalName of a dimension is the same"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class LookupDimensionToDimensionSpecSpec extends Specification{
apiRequest.getTopN() >> OptionalInt.empty()
apiRequest.getSorts() >> ([])
apiRequest.getCount() >> OptionalInt.empty()
apiRequest.getFilters() >> Collections.emptyMap()
apiRequest.getApiFilters() >> Collections.emptyMap()
}

def "Given lookup dimension with no namespace serialize using dimension serializer"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class RegisteredLookupDimensionToDimensionSpecSpec extends Specification{
apiRequest.getTopN() >> OptionalInt.empty()
apiRequest.getSorts() >> ([])
apiRequest.getCount() >> OptionalInt.empty()
apiRequest.getFilters() >> Collections.emptyMap()
apiRequest.getApiFilters() >> Collections.emptyMap()
}

def "Given registered lookup dimension with no lookup serialize using dimension serializer"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class DefaultPhysicalTableResolverSpec extends Specification {
apiRequest.granularity >> granularity
apiRequest.filterDimensions >> prototype['filterDimensions']
apiRequest.logicalMetrics >> prototype['logicalMetrics']
apiRequest.getFilters() >> Collections.emptyMap()
apiRequest.getApiFilters() >> Collections.emptyMap()
return apiRequest
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class QueryPlanningConstraintSpec extends Specification {
DataApiRequest dataApiRequest = Mock(DataApiRequest)
dataApiRequest.getDimensions() >> ([] as Set)
dataApiRequest.getFilterDimensions() >> ([] as Set)
dataApiRequest.getFilters() >> [:]
dataApiRequest.getApiFilters() >> [:]
dataApiRequest.getIntervals() >> ([] as Set)
dataApiRequest.getLogicalMetrics() >> ([] as Set)
dataApiRequest.getGranularity() >> (DefaultTimeGrain.DAY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class SchemaPhysicalTableMatcherSpec extends Specification {
query.getDependentFieldNames() >> ([] as Set)
request.getFilterDimensions() >> []
request.getDimensions() >> (dimSet)
request.getFilters() >> Collections.emptyMap()
request.getApiFilters() >> Collections.emptyMap()
request.getIntervals() >> []
request.getLogicalMetrics() >> []

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class PartialDataRequestHandlerSpec extends Specification {
physicalTable.getAvailableIntervals() >> availableIntervals
apiRequest.getDimensions() >> Collections.emptySet()
apiRequest.getFilterDimensions() >> Collections.emptySet()
apiRequest.getFilters() >> Collections.emptyMap()
apiRequest.getApiFilters() >> Collections.emptyMap()
apiRequest.getGranularity() >> DefaultTimeGrain.DAY
groupByQuery.getMetricDimensions() >> Collections.emptySet()
groupByQuery.getDependentFieldNames() >> Collections.emptySet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ protected DataApiRequest internalApply(DataApiRequest request, ContainerRequestC

validateSecurityFilters(securityContext.getUserPrincipal(), securityFilters);

Map<Dimension, Set<ApiFilter>> revisedFilters = mergeSecurityFilters(request.getFilters(), securityFilters);
Map<Dimension, Set<ApiFilter>> revisedFilters = mergeSecurityFilters(request.getApiFilters(), securityFilters);

return request.withFilters(revisedFilters);
}
Expand Down

0 comments on commit 6c64862

Please sign in to comment.