You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by bi...@apache.org on 2018/09/28 15:10:39 UTC

[kylin] 01/01: KYLIN-2924 enable google error-prone in compile phase

This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch KYLIN-2924
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 95e77a5af9a94ed0435024f1bc2467c9bb0edca1
Author: Billy Liu <bi...@apache.org>
AuthorDate: Fri Sep 28 23:09:41 2018 +0800

    KYLIN-2924 enable google error-prone in compile phase
---
 .../org/apache/kylin/common/util/SparkEntry.java   |  2 +-
 .../org/apache/kylin/cube/model/DimensionDesc.java |  2 +-
 .../job/impl/threadpool/DefaultScheduler.java      | 10 ++----
 .../apache/kylin/measure/topn/TopNCounterTest.java |  3 +-
 .../kylin/metrics/lib/impl/InstantReservoir.java   |  5 +--
 pom.xml                                            | 42 ++++++++++++++++++++++
 .../kylin/rest/controller/ModelController.java     |  4 +--
 .../kylin/rest/controller/ProjectController.java   |  4 +--
 .../apache/kylin/rest/util/AclPermissionUtil.java  |  9 +++--
 .../apache/kylin/tool/AbstractInfoExtractor.java   |  2 +-
 10 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/core-common/src/main/java/org/apache/kylin/common/util/SparkEntry.java b/core-common/src/main/java/org/apache/kylin/common/util/SparkEntry.java
index ce11b91..7022237 100644
--- a/core-common/src/main/java/org/apache/kylin/common/util/SparkEntry.java
+++ b/core-common/src/main/java/org/apache/kylin/common/util/SparkEntry.java
@@ -32,7 +32,7 @@ public final class SparkEntry {
             throw new IllegalArgumentException(String.valueOf("-className is required"));
         }
         final String className = args[1];
-        final Object o = Class.<AbstractApplication> forName(className).newInstance();
+        final Object o = Class.forName(className).newInstance();
         if (!(o instanceof AbstractApplication)) {
             throw new IllegalArgumentException(String.valueOf(className + " is not a subClass of AbstractApplication"));
         }
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/model/DimensionDesc.java b/core-cube/src/main/java/org/apache/kylin/cube/model/DimensionDesc.java
index dbbd4e8..42e3663 100644
--- a/core-cube/src/main/java/org/apache/kylin/cube/model/DimensionDesc.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/model/DimensionDesc.java
@@ -170,7 +170,7 @@ public class DimensionDesc implements java.io.Serializable {
             return false;
         }
 
-        if (derived != null ? !derived.equals(that.derived) : that.derived != null) {
+        if (derived != null ? !Arrays.equals(derived, that.derived) : that.derived != null) {
             return false;
         }
 
diff --git a/core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DefaultScheduler.java b/core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DefaultScheduler.java
index df3f6fa..5dd2c7c 100644
--- a/core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DefaultScheduler.java
+++ b/core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DefaultScheduler.java
@@ -47,15 +47,11 @@ import com.google.common.collect.Maps;
  */
 public class DefaultScheduler implements Scheduler<AbstractExecutable>, ConnectionStateListener {
 
-    private static DefaultScheduler INSTANCE = null;
+    private static DefaultScheduler INSTANCE;
 
-    public static DefaultScheduler getInstance() {
+    public synchronized static DefaultScheduler getInstance() {
         if (INSTANCE == null) {
-            synchronized (DefaultScheduler.class) {
-                if (INSTANCE == null) {
-                    INSTANCE = createInstance();
-                }
-            }
+            INSTANCE = createInstance();
         }
         return INSTANCE;
     }
diff --git a/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterTest.java b/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterTest.java
index 2105d37..9343701 100644
--- a/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterTest.java
+++ b/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterTest.java
@@ -98,7 +98,8 @@ public class TopNCounterTest {
         return tempFile.getAbsolutePath();
     }
 
-    //@Test
+    @Ignore
+    @Test
     public void testSingleSpaceSaving() throws IOException {
         String dataFile = prepareTestDate();
         TopNCounterTest.SpaceSavingConsumer spaceSavingCounter = new TopNCounterTest.SpaceSavingConsumer(
diff --git a/core-metrics/src/main/java/org/apache/kylin/metrics/lib/impl/InstantReservoir.java b/core-metrics/src/main/java/org/apache/kylin/metrics/lib/impl/InstantReservoir.java
index 41b53cf..7b6d710 100644
--- a/core-metrics/src/main/java/org/apache/kylin/metrics/lib/impl/InstantReservoir.java
+++ b/core-metrics/src/main/java/org/apache/kylin/metrics/lib/impl/InstantReservoir.java
@@ -18,6 +18,7 @@
 
 package org.apache.kylin.metrics.lib.impl;
 
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.kylin.metrics.lib.ActiveReservoirListener;
@@ -49,7 +50,7 @@ public class InstantReservoir extends AbstractActiveReservoir {
             if (!notifyListenerOfUpdatedRecord(listener, record)) {
                 ifSucceed = false;
                 logger.warn(
-                        "It fails to notify listener " + listener.toString() + " of updated record " + record.getKey());
+                        "It fails to notify listener " + listener.toString() + " of updated record " + Arrays.toString(record.getKey()));
             }
         }
         if (!ifSucceed) {
@@ -64,7 +65,7 @@ public class InstantReservoir extends AbstractActiveReservoir {
     }
 
     private boolean notifyListenerHAOfUpdatedRecord(Record record) {
-        logger.info("The HA listener " + listenerHA.toString() + " for updated record " + record.getKey()
+        logger.info("The HA listener " + listenerHA.toString() + " for updated record " + Arrays.toString(record.getKey())
                 + " will be started");
         if (!notifyListenerOfUpdatedRecord(listenerHA, record)) {
             logger.error("The HA listener also fails!!!");
diff --git a/pom.xml b/pom.xml
index 5c3b2cb..51b8766 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1303,7 +1303,28 @@
               <fork>true</fork>
               <meminitial>1024m</meminitial>
               <maxmem>2048m</maxmem>
+              <compilerId>javac-with-errorprone</compilerId>
+              <forceJavacCompilerUse>true</forceJavacCompilerUse>
+              <showWarnings>true</showWarnings>
+              <source>${javaVersion}</source>
+              <target>${javaVersion}</target>
+              <compilerArgs>
+                <!--<arg>-XepAllErrorsAsWarnings</arg>-->
+                <arg>-XepDisableWarningsInGeneratedCode</arg>
+              </compilerArgs>
             </configuration>
+            <dependencies>
+              <dependency>
+                <groupId>org.codehaus.plexus</groupId>
+                <artifactId>plexus-compiler-javac-errorprone</artifactId>
+                <version>2.8.5</version>
+              </dependency>
+              <dependency>
+                <groupId>com.google.errorprone</groupId>
+                <artifactId>error_prone_core</artifactId>
+                <version>2.3.1</version>
+              </dependency>
+            </dependencies>
           </plugin>
 
           <plugin>
@@ -1450,7 +1471,28 @@
               <fork>true</fork>
               <meminitial>1024m</meminitial>
               <maxmem>2048m</maxmem>
+              <compilerId>javac-with-errorprone</compilerId>
+              <forceJavacCompilerUse>true</forceJavacCompilerUse>
+              <showWarnings>true</showWarnings>
+              <source>${javaVersion}</source>
+              <target>${javaVersion}</target>
+              <compilerArgs>
+                <!--<arg>-XepAllErrorsAsWarnings</arg>-->
+                <arg>-XepDisableWarningsInGeneratedCode</arg>
+              </compilerArgs>
             </configuration>
+            <dependencies>
+              <dependency>
+                <groupId>org.codehaus.plexus</groupId>
+                <artifactId>plexus-compiler-javac-errorprone</artifactId>
+                <version>2.8.5</version>
+              </dependency>
+              <dependency>
+                <groupId>com.google.errorprone</groupId>
+                <artifactId>error_prone_core</artifactId>
+                <version>2.3.1</version>
+              </dependency>
+            </dependencies>
           </plugin>
 
           <plugin>
diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/ModelController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/ModelController.java
index 43d67ab..e7de9f4 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/controller/ModelController.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/controller/ModelController.java
@@ -116,8 +116,8 @@ public class ModelController extends BasicController {
         if (!ValidateUtil.isAlphanumericUnderscore(modelDesc.getName())) {
             throw new BadRequestException(
                     String.format(Locale.ROOT,
-                            "Invalid model name %s, only letters, numbers and underscore " + "supported."),
-                    modelDesc.getName());
+                            "Invalid model name %s, only letters, numbers and underscore supported.",
+                    modelDesc.getName()));
         }
 
         try {
diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java
index ecea557..26ea52f 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java
@@ -130,8 +130,8 @@ public class ProjectController extends BasicController {
         if (!ValidateUtil.isAlphanumericUnderscore(projectDesc.getName())) {
             throw new BadRequestException(
                     String.format(Locale.ROOT,
-                            "Invalid Project name %s, only letters, numbers and underscore " + "supported."),
-                    projectDesc.getName());
+                            "Invalid Project name %s, only letters, numbers and underscore supported.",
+                    projectDesc.getName()));
         }
 
         ProjectInstance createdProj = null;
diff --git a/server-base/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java b/server-base/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java
index 3737648..acf0f77 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java
@@ -35,12 +35,11 @@ public class AclPermissionUtil {
     }
 
     public static List<String> transformAuthorities(Collection<? extends GrantedAuthority> authorities) {
-        List<String> ret = new ArrayList<String>();
-        for (GrantedAuthority auth : authorities) {
-            if (!authorities.contains(auth.getAuthority())) {
-                ret.add(auth.getAuthority());
-            }
+        Set<String> groups = new HashSet<>();
+        for (GrantedAuthority authority : authorities) {
+            groups.add(authority.getAuthority());
         }
+        List<String> ret = new ArrayList<>(groups);
         return ret;
     }
 
diff --git a/tool/src/main/java/org/apache/kylin/tool/AbstractInfoExtractor.java b/tool/src/main/java/org/apache/kylin/tool/AbstractInfoExtractor.java
index 54f7604..3797512 100644
--- a/tool/src/main/java/org/apache/kylin/tool/AbstractInfoExtractor.java
+++ b/tool/src/main/java/org/apache/kylin/tool/AbstractInfoExtractor.java
@@ -102,7 +102,7 @@ public abstract class AbstractInfoExtractor extends AbstractApplication {
 
         // create new folder to contain the output
         String packageName = packageType.toLowerCase(Locale.ROOT) + "_"
-                + new SimpleDateFormat("YYYY_MM_dd_HH_mm_ss", Locale.ROOT).format(new Date());
+                + new SimpleDateFormat("yyyy_MM_dd_HH_mm_ss", Locale.ROOT).format(new Date());
         if (!isSubmodule && new File(exportDest).exists()) {
             exportDest = exportDest + packageName + "/";
         }