Merge pull request #334 from databendlabs/feat/add-code-quality

hantmac · web-flow · commit c8ec37ee95da · 2025-07-15T16:53:11.000+08:00
feat: add code quality check ci
diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml
@@ -0,0 +1,41 @@
+name: Code Quality Check
+
+on:
+  pull_request:
+    branches: [ main, master ]
+  push:
+    branches: [ main, master ]
+
+jobs:
+  checkstyle:
+    name: Run Checkstyle
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up JDK 11
+        uses: actions/setup-java@v4
+        with:
+          java-version: '11'
+          distribution: 'temurin'
+          cache: 'maven'
+
+      - name: Run Checkstyle
+        run: |
+          mvn checkstyle:checkstyle
+          mvn checkstyle:check
+
+      - name: Report Checkstyle results
+        uses: jwgmeligmeyling/checkstyle-github-action@master
+        if: always()
+        with:
+          path: '**/checkstyle-result.xml'
+
+      - name: Upload Checkstyle results
+        uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: checkstyle-results
+          path: target/site/checkstyle.html
+          retention-days: 7
diff --git a/.gitignore b/.gitignore
@@ -1,3 +1,6 @@
+# Checkstyle
+target/checkstyle-*
+.checkstyle
 .idea/
 databend-jdbc/databend-jdbc-debug.log
 target/
diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+        "https://checkstyle.org/dtds/configuration_1_3.dtd">
+
+<module name="Checker">
+    <property name="charset" value="UTF-8"/>
+    <property name="severity" value="error"/>
+
+    <module name="TreeWalker">
+        <module name="Regexp">
+            <property name="format" value="System\.(out|err)\.print(ln)?"/>
+            <property name="message" value="Use logging framework instead of System.out.println"/>
+            <property name="illegalPattern" value="true"/>
+        </module>
+
+        <!-- 其他规则 -->
+        <module name="UnusedImports"/>
+        <module name="RedundantImport"/>
+        <module name="IllegalImport"/>
+    </module>
+</module>
diff --git a/databend-client/src/main/java/com/databend/client/DatabendClient.java b/databend-client/src/main/java/com/databend/client/DatabendClient.java
@@ -14,7 +14,6 @@
 
 package com.databend.client;
 
-import com.github.zafarkhaja.semver.Version;
 import okhttp3.Request;
 
 import java.io.Closeable;
diff --git a/databend-client/src/main/java/com/databend/client/DiscoveryNode.java b/databend-client/src/main/java/com/databend/client/DiscoveryNode.java
@@ -17,7 +17,6 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-import java.net.URI;
 
 import static com.google.common.base.MoreObjects.toStringHelper;
 
diff --git a/databend-client/src/test/java/com/databend/client/TestClientIT.java b/databend-client/src/test/java/com/databend/client/TestClientIT.java
@@ -42,7 +42,6 @@ public void testBasicQueryPagination() {
         ClientSettings settings = new ClientSettings(DATABEND_HOST);
         AtomicReference<String> lastNodeID = new AtomicReference<>();
         DatabendClient cli = new DatabendClientV1(client, "select 1", settings, null, lastNodeID);
-        System.out.println(cli.getResults().getData());
         Assert.assertEquals(cli.getQuery(), "select 1");
         Assert.assertEquals(cli.getSession().getDatabase(), DATABASE);
         Assert.assertNotNull(cli.getResults());
@@ -66,7 +65,6 @@ public void testConnectionRefused() {
             cli.getResults(); // This should trigger the connection attempt
             Assert.fail("Expected exception was not thrown");
         } catch (Exception e) {
-            System.out.println(e.getMessage());
             Assert.assertTrue(
                     e instanceof ConnectException || e.getCause() instanceof ConnectException, "Exception should be ConnectionException or contain ConnectionException as cause");
 
@@ -96,8 +94,6 @@ public void testBasicQueryIDHeader() {
             cli.advance();
             Assert.assertEquals(cli1.getAdditionalHeaders().get(X_Databend_Query_ID), expectedUUID1);
         }
-        System.out.println(cli1.getResults().getData());
-        System.out.println(cli1.getAdditionalHeaders());
         Assert.assertEquals(cli1.getAdditionalHeaders().get(X_Databend_Query_ID), expectedUUID1);
     }
 
@@ -111,9 +107,6 @@ public void testDiscoverNodes() {
         ClientSettings settings = new ClientSettings(DATABEND_HOST, DatabendSession.createDefault(), DEFAULT_QUERY_TIMEOUT, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_SOCKET_TIMEOUT, PaginationOptions.defaultPaginationOptions(), additionalHeaders, null, DEFAULT_RETRY_ATTEMPTS);
         List<DiscoveryNode> nodes = DatabendClientV1.discoverNodes(client, settings);
         Assert.assertFalse(nodes.isEmpty());
-        for (DiscoveryNode node : nodes) {
-            System.out.println(node.getAddress());
-        }
     }
 
     @Test(groups = {"it"})
@@ -129,7 +122,6 @@ public void testDiscoverNodesUnSupported() {
             DatabendClientV1.discoverNodes(client, settings);
             Assert.fail("Expected exception was not thrown");
         } catch (Exception e) {
-            System.out.println(e.getMessage());
             Assert.assertTrue(e instanceof UnsupportedOperationException, "Exception should be UnsupportedOperationException");
         }
     }
diff --git a/databend-client/src/test/java/com/databend/client/TestDiscoveryNodes.java b/databend-client/src/test/java/com/databend/client/TestDiscoveryNodes.java
@@ -2,7 +2,6 @@
 
 import com.databend.client.errors.QueryErrors;
 import com.fasterxml.jackson.core.JsonProcessingException;
-import io.airlift.json.JsonCodecFactory;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
diff --git a/databend-jdbc/src/main/java/com/databend/jdbc/DatabendConnection.java b/databend-jdbc/src/main/java/com/databend/jdbc/DatabendConnection.java
@@ -681,7 +681,6 @@ public void PingDatabendClientV1() throws SQLException {
             statement.execute("select 1");
             ResultSet r = statement.getResultSet();
             while (r.next()) {
-                //System.out.println(r.getInt(1));
             }
         } catch (SQLException e) {
             throw new DatabendFailedToPingException(String.format("failed to ping databend server: %s", e.getMessage()));
@@ -952,7 +951,7 @@ public void uploadStream(String stageName, String destPrefix, InputStream inputS
                 }
             }
         } catch (RuntimeException e) {
-            System.out.println(e.getMessage());
+            logger.warning("failed to upload input stream, file size is:" + fileSize / 1024.0 + e.getMessage());
             throw new SQLException(e);
         } catch (IOException e) {
             logger.warning("failed to upload input stream, file size is:" + fileSize / 1024.0 + e.getMessage());
@@ -982,7 +981,6 @@ public void copyIntoTable(String database, String tableName, DatabendCopyParams
         requireNonNull(p.getDatabaseTableName(), "tableName is null");
         requireNonNull(p.getDatabendStage(), "stage is null");
         String sql = getCopyIntoSql(database, p);
-        System.out.println(sql);
         Statement statement = this.createStatement();
         statement.execute(sql);
         ResultSet rs = statement.getResultSet();
@@ -1037,7 +1035,6 @@ String generalRequest(String path, String body)  throws SQLException {
                     }
                     return response.body().string();
                 } catch (IOException e) {
-                    System.out.println("e = " + e.getMessage());
                     if (e.getCause() instanceof ConnectException) {
                         if (failReason == null) {
                             failReason = e.getMessage();
diff --git a/databend-jdbc/src/main/java/com/databend/jdbc/DatabendStatement.java b/databend-jdbc/src/main/java/com/databend/jdbc/DatabendStatement.java
@@ -1,7 +1,6 @@
 package com.databend.jdbc;
 
 import com.databend.client.DatabendClient;
-import com.databend.client.DatabendSession;
 import com.databend.client.QueryResults;
 import com.databend.client.StageAttachment;
 import com.databend.jdbc.annotation.NotImplemented;
@@ -216,8 +215,6 @@ final boolean internalExecute(String sql, StageAttachment attachment) throws SQL
                         currentUpdateCount = results.getStats().getWriteProgress().getRows().intValue();
                     }
                 } else {
-                    // System.out.println("sql is : " + sql);
-                    // System.out.println("[DEBUG] Query Write Progress: " + results.getStats().getWriteProgress());
                     currentUpdateCount = results.getStats().getWriteProgress().getRows().intValue();
                 }
             }
diff --git a/databend-jdbc/src/main/java/com/databend/jdbc/PresignContext.java b/databend-jdbc/src/main/java/com/databend/jdbc/PresignContext.java
@@ -1,6 +1,5 @@
 package com.databend.jdbc;
 
-import com.databend.client.errors.QueryErrors;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import okhttp3.Headers;
diff --git a/databend-jdbc/src/main/java/com/databend/jdbc/examples/Examples.java b/databend-jdbc/src/main/java/com/databend/jdbc/examples/Examples.java
@@ -1,23 +1,26 @@
 package com.databend.jdbc.examples;
 
+
+import java.util.logging.Logger;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 
+
 class Examples {
     private static Connection createConnection()
             throws SQLException {
         String url = "jdbc:databend://localhost:8000";
         return DriverManager.getConnection(url, "databend", "databend");
     }
-
+    private static final Logger logger = Logger.getLogger(Examples.class.getPackage().getName());
     public static void main(String[] args) throws SQLException {
         // set up
         Connection c = createConnection();
-        System.out.println("-----------------");
-        System.out.println("drop all existing test table");
+        logger.info("-----------------");
+        logger.info("Databend JDBC Examples");
         // execute demo
         c.createStatement().execute("drop table if exists test_prepare_statement");
         c.createStatement().execute("create table test_prepare_statement (a int, b string)");
@@ -31,23 +34,21 @@ public static void main(String[] args) throws SQLException {
             statement.setString(2, "b");
             statement.addBatch();
             int[] result = statement.executeBatch();
-            System.out.println(result);
+            logger.info("Insert result: " + result[0]);
         }
         // update with PreparedStatement
         String updateSQL = "update test_prepare_statement set b = ? where a = ?";
         try (PreparedStatement statement = conn.prepareStatement(updateSQL)) {
             statement.setInt(2, 1);
-            // Attention: now setString(1, "c") will throw exception, need to setString(1, "'c'")
             statement.setString(1, "c");
             int result = statement.executeUpdate();
-            System.out.println(result);
+            logger.info("Update result: " + result);
         }
 
         // executeQuery and return ResultSet
         ResultSet r = conn.createStatement().executeQuery("select * from test_prepare_statement");
         while (r.next()) {
-            System.out.println(r.getInt(1));
-            System.out.println(r.getString(2));
+            logger.info("Row: " + r.getInt(1) + ", " + r.getString(2));
         }
 
         // replace into with PreparedStatement
@@ -57,25 +58,23 @@ public static void main(String[] args) throws SQLException {
             statement.setString(2, "d");
             statement.addBatch();
             int[] result = statement.executeBatch();
+            logger.info("Replace into result: " + result[0]);
         }
         ResultSet r2 = conn.createStatement().executeQuery("select * from test_prepare_statement");
         while (r2.next()) {
-            System.out.println(r2.getInt(1));
-            System.out.println(r2.getString(2));
+            logger.info("Row: " + r2.getInt(1) + ", " + r2.getString(2));
         }
         // delete with PreparedStatement
         String deleteSQL = "delete from test_prepare_statement where a = ?";
         try (PreparedStatement statement = conn.prepareStatement(deleteSQL)) {
             statement.setInt(1, 1);
             int result = statement.executeUpdate();
-            System.out.println(result);
+            logger.info("Delete result: " + result);
         }
         ResultSet r3 = conn.createStatement().executeQuery("select * from test_prepare_statement");
 //        Assert.assertEquals(0, r3.getRow());
         while (r3.next()) {
-            // noting print
-            System.out.println(r3.getInt(1));
-            System.out.println(r3.getString(2));
+            logger.info("Row: " + r3.getInt(1) + ", " + r3.getString(2));
         }
     }
 }
diff --git a/databend-jdbc/src/test/java/com/databend/jdbc/TestMultiHost.java b/databend-jdbc/src/test/java/com/databend/jdbc/TestMultiHost.java
@@ -83,7 +83,6 @@ public void testRandomLoadBalancing()
                 }
             }
         }
-        System.out.println("node8000: " + node8000 + ", node8002: " + node8002 + ", node8003: " + node8003 + ", unknown: " + unknown);
         Assert.assertTrue(node8000 > 0 && node8002 > 0 && node8003 > 0);
         Assert.assertEquals(unknown, 0);
         Assert.assertEquals(node8000 + node8002 + node8003, 100);
@@ -117,7 +116,6 @@ public void testRoundRobinLoadBalancing()
                 }
             }
         }
-        System.out.println("node8000: " + node8000 + ", node8002: " + node8002 + ", node8003: " + node8003 + ", unknown: " + unknown);
         Assert.assertEquals(node8000, 30);
         Assert.assertEquals(node8002, 30);
         Assert.assertEquals(node8003, 30);
@@ -187,7 +185,6 @@ public void testFailOver()
                 }
             }
         }
-        System.out.println("node8000: " + node8000 + ", node8002: " + node8002 + ", node8003: " + node8003 + ", unknown: " + unknown);
 
         Assert.assertEquals(node8000, 90);
         Assert.assertEquals(unknown, 0);
@@ -222,7 +219,6 @@ public void testAutoDiscovery()
                 }
             }
         }
-        System.out.println("node8000: " + node8000 + ", node8002: " + node8002 + ", node8003: " + node8003 + ", unknown: " + unknown);
 
         Assert.assertEquals(node8000, 31);
         Assert.assertEquals(node8002, 30);
@@ -261,9 +257,6 @@ public void testAutoDiscoveryUriParsing() throws SQLException {
         discoveryNodes.add(DiscoveryNode.create("127.0.0.1:8002"));
         discoveryNodes.add(DiscoveryNode.create("127.0.0.1:8003"));
         List<URI> uris = nodes.parseURI(discoveryNodes);
-        for (URI u : uris) {
-            System.out.println(u);
-        }
         Assert.assertEquals(uris.size(), 3);
         Assert.assertEquals(uris2.size(), 3);
         Assert.assertEquals(uris2, uris);
diff --git a/pom.xml b/pom.xml
@@ -50,6 +50,7 @@
         <dep.errorprone.version>2.15.0</dep.errorprone.version>
         <dep.semver.version>0.9.0</dep.semver.version>
         <air.javadoc.lint>-missing</air.javadoc.lint>
+        <checkstyle.config.location>${maven.multiModuleProjectDirectory}/config/checkstyle/checkstyle.xml</checkstyle.config.location>
     </properties>
 
     <modules>
@@ -222,14 +223,24 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-checkstyle-plugin</artifactId>
+                <version>3.3.0</version>
                 <configuration>
-                    <configLocation>
-                        https://raw.githubusercontent.com/checkstyle/checkstyle/master/src/main/resources/google_checks.xml
-                    </configLocation>
+                    <configLocation>${checkstyle.config.location}</configLocation>
+                    <encoding>UTF-8</encoding>
                     <consoleOutput>true</consoleOutput>
                     <failsOnError>true</failsOnError>
                     <linkXRef>false</linkXRef>
+                    <includeTestSourceDirectory>false</includeTestSourceDirectory>
                 </configuration>
+                <executions>
+                    <execution>
+                        <id>validate</id>
+                        <phase>validate</phase>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
             </plugin>
         </plugins>
         <pluginManagement>
@@ -239,7 +250,12 @@
                     <artifactId>maven-shade-plugin</artifactId>
                     <version>3.5.1</version>
                 </plugin>
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-checkstyle-plugin</artifactId>
+                    <version>3.3.0</version>
+                </plugin>
             </plugins>
         </pluginManagement>
     </build>
-</project>
+</project>

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,6 @@`
`1`	`1`	`package com.databend.jdbc;`
`2`	`2`
`3`	`3`	`import com.databend.client.DatabendClient;`
`4`		`-import com.databend.client.DatabendSession;`
`5`	`4`	`import com.databend.client.QueryResults;`
`6`	`5`	`import com.databend.client.StageAttachment;`
`7`	`6`	`import com.databend.jdbc.annotation.NotImplemented;`
`@@ -216,8 +215,6 @@ final boolean internalExecute(String sql, StageAttachment attachment) throws SQL`
`216`	`215`	`currentUpdateCount = results.getStats().getWriteProgress().getRows().intValue();`
`217`	`216`	`}`
`218`	`217`	`} else {`
`219`		`- // System.out.println("sql is : " + sql);`
`220`		`- // System.out.println("[DEBUG] Query Write Progress: " + results.getStats().getWriteProgress());`
`221`	`218`	`currentUpdateCount = results.getStats().getWriteProgress().getRows().intValue();`
`222`	`219`	`}`
`223`	`220`	`}`