From 08e788997895787e923abb5b94f665ec1f4083a3 Mon Sep 17 00:00:00 2001 From: Dominik Przybysz Date: Wed, 21 Feb 2024 09:01:39 +0100 Subject: [PATCH 1/2] SNOW-1063844: Configure braces check for condition and loop bodies --- .github/pull_request_template.md | 5 +++-- README.rst | 10 ++++++---- pom.xml | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 84ae71dcc..9ef1e0ed3 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -3,9 +3,10 @@ SNOW-XXXXX ## Pre-review self checklist +- [ ] PR branch is updated with all the changes from `master` branch - [ ] The code is correctly formatted (run `mvn -P check-style validate`) -- [ ] I don't expose unnecessary new public API (run `mvn verify` and inspect `target/japicmp/japicmp.html`) -- [ ] Pull request name is prefixed with `SNOW-XXXX: ` +- [ ] New public API is not unnecessary exposed (run `mvn verify` and inspect `target/japicmp/japicmp.html`) +- [ ] The pull request name is prefixed with `SNOW-XXXX: ` ## External contributors - please answer these questions before submitting a pull request. Thanks! diff --git a/README.rst b/README.rst index 55248b392..c2e296e95 100644 --- a/README.rst +++ b/README.rst @@ -151,10 +151,12 @@ You may import the coding style from IntelliJ so that the coding style can be ap - In the **File** -> **Settings/Plugins**, and install `google-java-format` plugin. - Enable `google-java-format` for the JDBC project. - In the source code window, select **Code** -> **Reformat** to apply the coding style. -- Additionally configure IDE to not use wildcard imports in **File** -> **Ecitor** -> **Code Style** -> **Java** set: - - **Use single class import** - - **Class count to use import with '*'** to 1000 - - **Names count to use static import with '*'** to 1000 +- Additionally configure IDE in **File** -> **Editor** -> **Code Style** -> **Java** to + - not use wildcard imports (tab **Imports**): + - **Use single class import** + - **Class count to use import with '*'** to 1000 + - **Names count to use static import with '*'** to 1000 + - always use braces in ``if/while/for/do..while`` in (tab **Wrapping and Braces**) Tests ===== diff --git a/pom.xml b/pom.xml index a1d94f9d1..514ffd381 100644 --- a/pom.xml +++ b/pom.xml @@ -595,6 +595,7 @@ + From 6884e0dac86c6c641857ab07636d0b6859120e2d Mon Sep 17 00:00:00 2001 From: Dominik Przybysz Date: Wed, 21 Feb 2024 09:02:52 +0100 Subject: [PATCH 2/2] SNOW-1063844: Add missing braces to condition and loop bodies --- .../client/config/SFClientConfig.java | 16 +++-- .../client/core/ExecTimeTelemetryData.java | 72 ++++++++++++++----- .../client/core/QueryContextCache.java | 4 +- .../net/snowflake/client/core/ResultUtil.java | 3 +- .../net/snowflake/client/core/SFSession.java | 11 ++- .../snowflake/client/core/SessionUtil.java | 6 +- .../net/snowflake/client/core/UUIDUtils.java | 8 ++- .../jdbc/SnowflakePreparedStatementV1.java | 8 ++- .../cloud/storage/SnowflakeAzureClient.java | 8 ++- .../cloud/storage/SnowflakeGCSClient.java | 12 +++- .../jdbc/cloud/storage/SnowflakeS3Client.java | 8 ++- .../DatabaseMetaDataInternalLatestIT.java | 4 +- .../snowflake/client/jdbc/HeartbeatIT.java | 4 +- .../client/jdbc/ResultSetLatestIT.java | 3 +- .../client/jdbc/SnowflakeDriverIT.java | 8 ++- 15 files changed, 129 insertions(+), 46 deletions(-) diff --git a/src/main/java/net/snowflake/client/config/SFClientConfig.java b/src/main/java/net/snowflake/client/config/SFClientConfig.java index cd352f1a6..1029b1167 100644 --- a/src/main/java/net/snowflake/client/config/SFClientConfig.java +++ b/src/main/java/net/snowflake/client/config/SFClientConfig.java @@ -35,8 +35,12 @@ public void setConfigFilePath(String configFilePath) { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } SFClientConfig that = (SFClientConfig) o; return Objects.equals(commonProps, that.commonProps); } @@ -78,8 +82,12 @@ public void setLogPath(String logPath) { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } CommonProps that = (CommonProps) o; return Objects.equals(logLevel, that.logLevel) && Objects.equals(logPath, that.logPath); } diff --git a/src/main/java/net/snowflake/client/core/ExecTimeTelemetryData.java b/src/main/java/net/snowflake/client/core/ExecTimeTelemetryData.java index 1d48873cc..d4dd1ecf0 100644 --- a/src/main/java/net/snowflake/client/core/ExecTimeTelemetryData.java +++ b/src/main/java/net/snowflake/client/core/ExecTimeTelemetryData.java @@ -48,92 +48,128 @@ public ExecTimeTelemetryData() { } public void setBindStart() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.bindStart = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setOCSPStatus(Boolean ocspEnabled) { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.ocspEnabled = ocspEnabled; } public void setBindEnd() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.bindEnd = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setHttpClientStart() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.httpClientStart = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setHttpClientEnd() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.httpClientEnd = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setGzipStart() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.gzipStart = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setGzipEnd() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.gzipEnd = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setQueryEnd() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.queryEnd = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setQueryId(String queryId) { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.queryId = queryId; } public void setProcessResultChunkStart() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.processResultChunkStart = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setProcessResultChunkEnd() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.processResultChunkEnd = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setResponseIOStreamStart() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.responseIOStreamStart = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setResponseIOStreamEnd() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.responseIOStreamEnd = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setCreateResultSetStart() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.createResultSetStart = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void setCreateResultSetEnd() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.createResultSetEnd = SnowflakeUtil.getEpochTimeInMicroSeconds(); } public void incrementRetryCount() { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.retryCount++; } public void setRequestId(String requestId) { - if (!this.sendData) return; + if (!this.sendData) { + return; + } this.requestId = requestId; } public void addRetryLocation(String location) { - if (!this.sendData) return; + if (!this.sendData) { + return; + } if (Strings.isNullOrEmpty(this.retryLocations)) { this.retryLocations = location; } else { diff --git a/src/main/java/net/snowflake/client/core/QueryContextCache.java b/src/main/java/net/snowflake/client/core/QueryContextCache.java index ea6e37cd5..ea47e6167 100644 --- a/src/main/java/net/snowflake/client/core/QueryContextCache.java +++ b/src/main/java/net/snowflake/client/core/QueryContextCache.java @@ -340,7 +340,9 @@ public QueryContextDTO serializeQueryContextDTO() { logCacheEntries(); TreeSet elements = getElements(); - if (elements.size() == 0) return null; + if (elements.size() == 0) { + return null; + } try { QueryContextDTO queryContextDTO = new QueryContextDTO(); diff --git a/src/main/java/net/snowflake/client/core/ResultUtil.java b/src/main/java/net/snowflake/client/core/ResultUtil.java index f973bb1b1..8581df1fc 100644 --- a/src/main/java/net/snowflake/client/core/ResultUtil.java +++ b/src/main/java/net/snowflake/client/core/ResultUtil.java @@ -411,8 +411,9 @@ public static long calculateUpdateCount(SFBaseResultSet resultSet) || statementType == SFStatementType.MERGE || statementType == SFStatementType.MULTI_INSERT) { int columnCount = resultSet.getMetaData().getColumnCount(); - for (int i = 0; i < columnCount; i++) + for (int i = 0; i < columnCount; i++) { updateCount += resultSet.getLong(i + 1); // add up number of rows updated + } } else { updateCount = 0; } diff --git a/src/main/java/net/snowflake/client/core/SFSession.java b/src/main/java/net/snowflake/client/core/SFSession.java index a603fd4b3..20850b4f0 100644 --- a/src/main/java/net/snowflake/client/core/SFSession.java +++ b/src/main/java/net/snowflake/client/core/SFSession.java @@ -683,8 +683,11 @@ public synchronized void open() throws SFException, SnowflakeSQLException { "Query context cache is {}", ((disableQueryContextCache) ? "disabled" : "enabled")); // Initialize QCC - if (!disableQueryContextCache) qcc = new QueryContextCache(this.getQueryContextCacheSize()); - else qcc = null; + if (!disableQueryContextCache) { + qcc = new QueryContextCache(this.getQueryContextCacheSize()); + } else { + qcc = null; + } // start heartbeat for this session so that the master token will not expire startHeartbeatForThisSession(); @@ -814,7 +817,9 @@ public void close() throws SFException, SnowflakeSQLException { getClientInfo().clear(); // qcc can be null, if disabled. - if (qcc != null) qcc.clearCache(); + if (qcc != null) { + qcc.clearCache(); + } isClosed = true; } diff --git a/src/main/java/net/snowflake/client/core/SessionUtil.java b/src/main/java/net/snowflake/client/core/SessionUtil.java index 305b5b220..9a51cd8ae 100644 --- a/src/main/java/net/snowflake/client/core/SessionUtil.java +++ b/src/main/java/net/snowflake/client/core/SessionUtil.java @@ -475,9 +475,11 @@ private static SFLoginOutput newSession( // Fix for HikariCP refresh token issue:SNOW-533673. // If token value is not set but password field is set then // the driver treats password as token. - if (loginInput.getToken() != null) + if (loginInput.getToken() != null) { data.put(ClientAuthnParameter.TOKEN.name(), loginInput.getToken()); - else data.put(ClientAuthnParameter.TOKEN.name(), loginInput.getPassword()); + } else { + data.put(ClientAuthnParameter.TOKEN.name(), loginInput.getPassword()); + } } else if (authenticatorType == ClientAuthnDTO.AuthenticatorType.SNOWFLAKE_JWT) { data.put(ClientAuthnParameter.AUTHENTICATOR.name(), authenticatorType.name()); diff --git a/src/main/java/net/snowflake/client/core/UUIDUtils.java b/src/main/java/net/snowflake/client/core/UUIDUtils.java index 75ba56ce9..e57e977d2 100644 --- a/src/main/java/net/snowflake/client/core/UUIDUtils.java +++ b/src/main/java/net/snowflake/client/core/UUIDUtils.java @@ -24,8 +24,12 @@ private static UUID UUIDImpl() { long msb = 0; long lsb = 0; - for (int i = 0; i < 8; i++) msb = (msb << 8) | (randomBytes[i] & 0xff); - for (int i = 8; i < 16; i++) lsb = (lsb << 8) | (randomBytes[i] & 0xff); + for (int i = 0; i < 8; i++) { + msb = (msb << 8) | (randomBytes[i] & 0xff); + } + for (int i = 8; i < 16; i++) { + lsb = (lsb << 8) | (randomBytes[i] & 0xff); + } return new UUID(msb, lsb); } diff --git a/src/main/java/net/snowflake/client/jdbc/SnowflakePreparedStatementV1.java b/src/main/java/net/snowflake/client/jdbc/SnowflakePreparedStatementV1.java index b2dea22c0..10a2743b8 100644 --- a/src/main/java/net/snowflake/client/jdbc/SnowflakePreparedStatementV1.java +++ b/src/main/java/net/snowflake/client/jdbc/SnowflakePreparedStatementV1.java @@ -903,10 +903,14 @@ VariableTypeArray executeBatchInternalWithArrayBind(boolean isLong) throws SQLEx if (updateCount == batchSize) { if (isLong) { updateCounts = new VariableTypeArray(null, new long[updateCount]); - for (int idx = 0; idx < updateCount; idx++) updateCounts.longArr[idx] = 1; + for (int idx = 0; idx < updateCount; idx++) { + updateCounts.longArr[idx] = 1; + } } else { updateCounts = new VariableTypeArray(new int[updateCount], null); - for (int idx = 0; idx < updateCount; idx++) updateCounts.intArr[idx] = 1; + for (int idx = 0; idx < updateCount; idx++) { + updateCounts.intArr[idx] = 1; + } } } else { if (isLong) { diff --git a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeAzureClient.java b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeAzureClient.java index be54535ff..8977d154b 100644 --- a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeAzureClient.java +++ b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeAzureClient.java @@ -536,7 +536,9 @@ public void upload( blob.uploadMetadata(null, transferOptions, opContext); // close any open streams in the "toClose" list and return - for (FileInputStream is : toClose) IOUtils.closeQuietly(is); + for (FileInputStream is : toClose) { + IOUtils.closeQuietly(is); + } return; } catch (Exception ex) { @@ -566,7 +568,9 @@ public void upload( } while (retryCount <= getMaxRetries()); - for (FileInputStream is : toClose) IOUtils.closeQuietly(is); + for (FileInputStream is : toClose) { + IOUtils.closeQuietly(is); + } throw new SnowflakeSQLException( queryId, diff --git a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java index 98040b91b..25b3bc9dc 100644 --- a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java +++ b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java @@ -694,7 +694,9 @@ public void upload( logger.debug("Upload successful", false); // close any open streams in the "toClose" list and return - for (FileInputStream is : toClose) IOUtils.closeQuietly(is); + for (FileInputStream is : toClose) { + IOUtils.closeQuietly(is); + } return; } @@ -716,7 +718,9 @@ public void upload( logger.debug("Upload successful", false); // close any open streams in the "toClose" list and return - for (FileInputStream is : toClose) IOUtils.closeQuietly(is); + for (FileInputStream is : toClose) { + IOUtils.closeQuietly(is); + } return; } catch (Exception ex) { @@ -747,7 +751,9 @@ public void upload( } while (retryCount <= getMaxRetries()); - for (FileInputStream is : toClose) IOUtils.closeQuietly(is); + for (FileInputStream is : toClose) { + IOUtils.closeQuietly(is); + } throw new SnowflakeSQLLoggedException( queryId, diff --git a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeS3Client.java b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeS3Client.java index b5af0aef5..190493b69 100644 --- a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeS3Client.java +++ b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeS3Client.java @@ -578,7 +578,9 @@ public ExecutorService newExecutor() { myUpload.waitForCompletion(); // get out - for (FileInputStream is : toClose) IOUtils.closeQuietly(is); + for (FileInputStream is : toClose) { + IOUtils.closeQuietly(is); + } return; } catch (Exception ex) { @@ -610,7 +612,9 @@ public ExecutorService newExecutor() { } } while (retryCount <= getMaxRetries()); - for (FileInputStream is : toClose) IOUtils.closeQuietly(is); + for (FileInputStream is : toClose) { + IOUtils.closeQuietly(is); + } throw new SnowflakeSQLLoggedException( queryId, diff --git a/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataInternalLatestIT.java b/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataInternalLatestIT.java index cdc50ab35..43e8272fb 100644 --- a/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataInternalLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataInternalLatestIT.java @@ -268,7 +268,9 @@ public void testGetTablesRaceCondition() })); } executorService.shutdown(); - for (int i = 0; i < 10; i++) futures.get(i).get(); + for (int i = 0; i < 10; i++) { + futures.get(i).get(); + } } } } diff --git a/src/test/java/net/snowflake/client/jdbc/HeartbeatIT.java b/src/test/java/net/snowflake/client/jdbc/HeartbeatIT.java index c0863d3b5..16e8364d6 100644 --- a/src/test/java/net/snowflake/client/jdbc/HeartbeatIT.java +++ b/src/test/java/net/snowflake/client/jdbc/HeartbeatIT.java @@ -144,7 +144,9 @@ public void testSuccess() throws Exception { })); } executorService.shutdown(); - for (int idx = 0; idx < concurrency; idx++) futures.get(idx).get(); + for (int idx = 0; idx < concurrency; idx++) { + futures.get(idx).get(); + } } /** diff --git a/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java b/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java index 68e6d512f..f9e5f0293 100644 --- a/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java @@ -542,8 +542,7 @@ public void testResultChunkDownloaderException() throws SQLException { try { // Normally this step won't cause too long. Because we will get exception once trying to get // result from the first chunk downloader - while (resultSet.next()) - ; + while (resultSet.next()) {} fail("Should not reach here. Last next() command is supposed to throw an exception"); } catch (SnowflakeSQLException ex) { // pass, do nothing diff --git a/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverIT.java b/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverIT.java index 1501f58ea..aa82813f0 100644 --- a/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverIT.java +++ b/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverIT.java @@ -1801,14 +1801,18 @@ public void testBind() throws Throwable { + "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?," + " ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"); - for (int idx = 0; idx < 16; idx++) addBindBatch(preparedStatement, sqlDate); + for (int idx = 0; idx < 16; idx++) { + addBindBatch(preparedStatement, sqlDate); + } updateCounts = preparedStatement.executeBatch(); // GS optimizes this into one insert execution assertEquals("Number of update counts", 16, updateCounts.length); - for (int idx = 0; idx < 16; idx++) assertEquals("update count", 1, updateCounts[idx]); + for (int idx = 0; idx < 16; idx++) { + assertEquals("update count", 1, updateCounts[idx]); + } } finally { if (regularStatement != null) { regularStatement.execute("DROP TABLE testBind");