This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 42d7749873c Fix remaining Java reliability and SonarCloud issues
(#40321)
42d7749873c is described below
commit 42d7749873c0fdf4dbe425950c8e8924b76b3399
Author: Liang Zhang <[email protected]>
AuthorDate: Wed Oct 21 20:42:06 2026 +0800
Fix remaining Java reliability and SonarCloud issues (#40321)
---
.../OpenGaussAuthenticationHexData.java | 8 +--
.../expr/groovy/GroovyInlineExpressionParser.java | 2 +-
.../handler/execute/MCPJdbcStatementExecutor.java | 24 +++++----
.../execute/MCPJdbcStatementExecutorTest.java | 51 ++++++++++++++++++
.../tool/model/ShardingWorkflowRequest.java | 60 +++++++++++-----------
.../mcp/support/workflow/model/WorkflowKind.java | 2 +-
.../support/workflow/model/WorkflowKindTest.java | 13 +++++
.../statement/type/MySQLDMLStatementVisitor.java | 4 +-
8 files changed, 116 insertions(+), 48 deletions(-)
diff --git
a/database/protocol/dialect/opengauss/src/main/java/org/apache/shardingsphere/database/protocol/opengauss/packet/authentication/OpenGaussAuthenticationHexData.java
b/database/protocol/dialect/opengauss/src/main/java/org/apache/shardingsphere/database/protocol/opengauss/packet/authentication/OpenGaussAuthenticationHexData.java
index b97b1dae8ff..fc13b2b0142 100644
---
a/database/protocol/dialect/opengauss/src/main/java/org/apache/shardingsphere/database/protocol/opengauss/packet/authentication/OpenGaussAuthenticationHexData.java
+++
b/database/protocol/dialect/opengauss/src/main/java/org/apache/shardingsphere/database/protocol/opengauss/packet/authentication/OpenGaussAuthenticationHexData.java
@@ -33,12 +33,12 @@ public final class OpenGaussAuthenticationHexData {
private final String nonce;
public OpenGaussAuthenticationHexData() {
- salt = generate(64);
- nonce = generate(8);
+ Random random = new SecureRandom();
+ salt = generate(64, random);
+ nonce = generate(8, random);
}
- private String generate(final int length) {
- Random random = new SecureRandom();
+ private String generate(final int length, final Random random) {
StringBuilder result = new StringBuilder(length);
for (int i = 0; i < result.capacity(); i++) {
result.append(Integer.toString(random.nextInt(0x10), 0x10));
diff --git
a/infra/expr/type/groovy/src/main/java/org/apache/shardingsphere/infra/expr/groovy/GroovyInlineExpressionParser.java
b/infra/expr/type/groovy/src/main/java/org/apache/shardingsphere/infra/expr/groovy/GroovyInlineExpressionParser.java
index 125eeb7f537..27e895f17dc 100644
---
a/infra/expr/type/groovy/src/main/java/org/apache/shardingsphere/infra/expr/groovy/GroovyInlineExpressionParser.java
+++
b/infra/expr/type/groovy/src/main/java/org/apache/shardingsphere/infra/expr/groovy/GroovyInlineExpressionParser.java
@@ -135,7 +135,7 @@ public final class GroovyInlineExpressionParser implements
InlineExpressionParse
private Object evaluate(final String expression) {
if (isConstantExpression(expression)) {
- return expression.replaceAll("^\"|\"$", "");
+ return expression.replaceAll("(^\")|(\"$)", "");
}
Script script = scriptCache.get(expression, SHELL::parse);
return null == script ? expression : script.run();
diff --git
a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutor.java
b/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutor.java
index 6afd67e567e..948bcc1ab8d 100644
---
a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutor.java
+++
b/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutor.java
@@ -156,7 +156,8 @@ public final class MCPJdbcStatementExecutor {
boolean originalReadOnly = connection.isReadOnly();
boolean originalAutoCommit = connection.getAutoCommit();
boolean autoCommitDisabled = false;
- SQLException failure = null;
+ Throwable failure = null;
+ SQLExecutionResult result;
try {
connection.setReadOnly(true);
if (originalAutoCommit) {
@@ -165,9 +166,11 @@ public final class MCPJdbcStatementExecutor {
}
try (Statement statement = connection.createStatement()) {
configureStatement(statement, executionRequest);
- return executeStatement(statement, executionRequest,
classificationResult);
+ result = executeStatement(statement, executionRequest,
classificationResult);
}
- } catch (final SQLException ex) {
+ // CHECKSTYLE:OFF
+ } catch (final SQLException | RuntimeException | Error ex) {
+ // CHECKSTYLE:ON
failure = ex;
throw ex;
} finally {
@@ -178,13 +181,14 @@ public final class MCPJdbcStatementExecutor {
failure = restoreAutoCommit(connection, originalAutoCommit,
failure);
}
failure = restoreReadOnly(connection, originalReadOnly, failure);
- if (null != failure) {
- throw failure;
- }
}
+ if (null != failure) {
+ throw (SQLException) failure;
+ }
+ return result;
}
- private SQLException rollbackReadOnlyConnection(final Connection
connection, final SQLException previousFailure) {
+ private Throwable rollbackReadOnlyConnection(final Connection connection,
final Throwable previousFailure) {
try {
connection.rollback();
return previousFailure;
@@ -193,7 +197,7 @@ public final class MCPJdbcStatementExecutor {
}
}
- private SQLException restoreAutoCommit(final Connection connection, final
boolean originalAutoCommit, final SQLException previousFailure) {
+ private Throwable restoreAutoCommit(final Connection connection, final
boolean originalAutoCommit, final Throwable previousFailure) {
try {
connection.setAutoCommit(originalAutoCommit);
return previousFailure;
@@ -202,7 +206,7 @@ public final class MCPJdbcStatementExecutor {
}
}
- private SQLException restoreReadOnly(final Connection connection, final
boolean originalReadOnly, final SQLException previousFailure) {
+ private Throwable restoreReadOnly(final Connection connection, final
boolean originalReadOnly, final Throwable previousFailure) {
try {
connection.setReadOnly(originalReadOnly);
return previousFailure;
@@ -211,7 +215,7 @@ public final class MCPJdbcStatementExecutor {
}
}
- private SQLException appendFailure(final SQLException previousFailure,
final SQLException failure) {
+ private Throwable appendFailure(final Throwable previousFailure, final
SQLException failure) {
if (null == previousFailure) {
return failure;
}
diff --git
a/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutorTest.java
b/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutorTest.java
index fdb2995e0fc..cf6c2dd9f07 100644
---
a/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutorTest.java
+++
b/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPJdbcStatementExecutorTest.java
@@ -133,9 +133,60 @@ class MCPJdbcStatementExecutorTest {
Arguments.of("rollback failure", false, true, false, false,
"rollback failed", List.of()),
Arguments.of("autoCommit restoration failure", false, false,
true, false, "autoCommit failed", List.of()),
Arguments.of("readOnly restoration failure", false, false,
false, true, "readOnly failed", List.of()),
+ Arguments.of("cleanup failures", false, true, true, true,
"rollback failed", List.of("autoCommit failed", "readOnly failed")),
Arguments.of("execution and cleanup failures", true, true,
true, true, "execution failed", List.of("rollback failed", "autoCommit failed",
"readOnly failed")));
}
+ @Test
+ void assertExecuteReadOnlyWithRuntimeFailure() throws SQLException {
+ Statement statement = mock(Statement.class);
+ IllegalStateException cause = new IllegalStateException("foo execution
failed");
+ when(statement.execute("SELECT 1")).thenThrow(cause);
+ Connection connection = createStatementConnection(statement);
+ when(connection.getAutoCommit()).thenReturn(true);
+ SQLException rollbackFailure = new SQLException("foo rollback failed");
+ SQLException autoCommitFailure = new SQLException("foo autoCommit
failed");
+ SQLException readOnlyFailure = new SQLException("foo readOnly failed");
+ doThrow(rollbackFailure).when(connection).rollback();
+ doThrow(autoCommitFailure).when(connection).setAutoCommit(true);
+ doThrow(readOnlyFailure).when(connection).setReadOnly(false);
+ RuntimeDatabaseConfiguration databaseConfig =
mock(RuntimeDatabaseConfiguration.class);
+ when(databaseConfig.openConnection("foo_db")).thenReturn(connection);
+ MCPJdbcStatementExecutor executor = new
MCPJdbcStatementExecutor(Map.of("foo_db", databaseConfig),
mock(MCPJdbcTransactionResourceManager.class));
+ SQLExecutionRequest request = new SQLExecutionRequest("foo_session",
"foo_db", "", "SELECT 1", 0, 0, true);
+ ClassificationResult classification = new
ClassificationResult(SupportedMCPStatement.QUERY, "SELECT", "SELECT 1", "",
List.of(), false);
+ IllegalStateException actual =
assertThrows(IllegalStateException.class, () -> executor.execute(request,
classification,
createDatabaseCapability(SchemaExecutionSemantics.FIXED_TO_DATABASE)));
+ assertThat(actual, sameInstance(cause));
+ assertThat(Arrays.asList(actual.getSuppressed()),
is(List.of(rollbackFailure, autoCommitFailure, readOnlyFailure)));
+ verify(statement).close();
+ verify(connection).close();
+ }
+
+ @Test
+ void assertExecuteReadOnlyWithError() throws SQLException {
+ Statement statement = mock(Statement.class);
+ Error cause = new Error("foo execution failed");
+ when(statement.execute("SELECT 1")).thenThrow(cause);
+ Connection connection = createStatementConnection(statement);
+ when(connection.getAutoCommit()).thenReturn(true);
+ SQLException rollbackFailure = new SQLException("foo rollback failed");
+ SQLException autoCommitFailure = new SQLException("foo autoCommit
failed");
+ SQLException readOnlyFailure = new SQLException("foo readOnly failed");
+ doThrow(rollbackFailure).when(connection).rollback();
+ doThrow(autoCommitFailure).when(connection).setAutoCommit(true);
+ doThrow(readOnlyFailure).when(connection).setReadOnly(false);
+ RuntimeDatabaseConfiguration databaseConfig =
mock(RuntimeDatabaseConfiguration.class);
+ when(databaseConfig.openConnection("foo_db")).thenReturn(connection);
+ MCPJdbcStatementExecutor executor = new
MCPJdbcStatementExecutor(Map.of("foo_db", databaseConfig),
mock(MCPJdbcTransactionResourceManager.class));
+ SQLExecutionRequest request = new SQLExecutionRequest("foo_session",
"foo_db", "", "SELECT 1", 0, 0, true);
+ ClassificationResult classification = new
ClassificationResult(SupportedMCPStatement.QUERY, "SELECT", "SELECT 1", "",
List.of(), false);
+ Error actual = assertThrows(Error.class, () ->
executor.execute(request, classification,
createDatabaseCapability(SchemaExecutionSemantics.FIXED_TO_DATABASE)));
+ assertThat(actual, sameInstance(cause));
+ assertThat(Arrays.asList(actual.getSuppressed()),
is(List.of(rollbackFailure, autoCommitFailure, readOnlyFailure)));
+ verify(statement).close();
+ verify(connection).close();
+ }
+
@Test
void assertExecuteReadOnlyWithAutoCommitAlreadyDisabled() throws
SQLException {
Statement statement = mock(Statement.class);
diff --git
a/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/model/ShardingWorkflowRequest.java
b/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/model/ShardingWorkflowRequest.java
index 683d8e0ab6e..b4c40c95db7 100644
---
a/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/model/ShardingWorkflowRequest.java
+++
b/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/model/ShardingWorkflowRequest.java
@@ -106,59 +106,59 @@ public final class ShardingWorkflowRequest extends
WorkflowRequest {
}
public void setRuleName(final String ruleName) {
- this.ruleName = normalize(ruleName);
+ this.ruleName = normalizeShardingValue(ruleName);
}
public void setDataNodes(final String dataNodes) {
- this.dataNodes = normalize(dataNodes);
+ this.dataNodes = normalizeShardingValue(dataNodes);
}
public void setStorageUnits(final String storageUnits) {
- this.storageUnits = normalize(storageUnits);
+ this.storageUnits = normalizeShardingValue(storageUnits);
}
public void setStrategyType(final String strategyType) {
- this.strategyType = normalize(strategyType);
+ this.strategyType = normalizeShardingValue(strategyType);
}
public void setShardingColumns(final String shardingColumns) {
- this.shardingColumns = normalize(shardingColumns);
+ this.shardingColumns = normalizeShardingValue(shardingColumns);
}
public void setDefaultStrategyType(final String defaultStrategyType) {
- this.defaultStrategyType = normalize(defaultStrategyType);
+ this.defaultStrategyType = normalizeShardingValue(defaultStrategyType);
}
public void setKeyGenerateColumn(final String keyGenerateColumn) {
- this.keyGenerateColumn = normalize(keyGenerateColumn);
+ this.keyGenerateColumn = normalizeShardingValue(keyGenerateColumn);
}
public void setKeyGeneratorName(final String keyGeneratorName) {
- this.keyGeneratorName = normalize(keyGeneratorName);
+ this.keyGeneratorName = normalizeShardingValue(keyGeneratorName);
}
public void setKeyGeneratorType(final String keyGeneratorType) {
- this.keyGeneratorType = normalize(keyGeneratorType);
+ this.keyGeneratorType = normalizeShardingValue(keyGeneratorType);
}
public void setSequenceName(final String sequenceName) {
- this.sequenceName = normalize(sequenceName);
+ this.sequenceName = normalizeShardingValue(sequenceName);
}
public void setKeyGenerateStrategyName(final String
keyGenerateStrategyName) {
- this.keyGenerateStrategyName = normalize(keyGenerateStrategyName);
+ this.keyGenerateStrategyName =
normalizeShardingValue(keyGenerateStrategyName);
}
public void setComponentType(final String componentType) {
- this.componentType = normalize(componentType);
+ this.componentType = normalizeShardingValue(componentType);
}
public void setComponentName(final String componentName) {
- this.componentName = normalize(componentName);
+ this.componentName = normalizeShardingValue(componentName);
}
public void setAllowHintDisable(final String allowHintDisable) {
- this.allowHintDisable = normalize(allowHintDisable);
+ this.allowHintDisable = normalizeShardingValue(allowHintDisable);
}
/**
@@ -186,20 +186,20 @@ public final class ShardingWorkflowRequest extends
WorkflowRequest {
private void overlayTo(final ShardingWorkflowRequest target) {
super.overlayTo(target);
- target.setRuleName(resolveValue(target.getRuleName(), ruleName));
- target.setDataNodes(resolveValue(target.getDataNodes(), dataNodes));
- target.setStorageUnits(resolveValue(target.getStorageUnits(),
storageUnits));
- target.setStrategyType(resolveValue(target.getStrategyType(),
strategyType));
- target.setShardingColumns(resolveValue(target.getShardingColumns(),
shardingColumns));
-
target.setDefaultStrategyType(resolveValue(target.getDefaultStrategyType(),
defaultStrategyType));
-
target.setKeyGenerateColumn(resolveValue(target.getKeyGenerateColumn(),
keyGenerateColumn));
- target.setKeyGeneratorName(resolveValue(target.getKeyGeneratorName(),
keyGeneratorName));
- target.setKeyGeneratorType(resolveValue(target.getKeyGeneratorType(),
keyGeneratorType));
- target.setSequenceName(resolveValue(target.getSequenceName(),
sequenceName));
-
target.setKeyGenerateStrategyName(resolveValue(target.getKeyGenerateStrategyName(),
keyGenerateStrategyName));
- target.setComponentType(resolveValue(target.getComponentType(),
componentType));
- target.setComponentName(resolveValue(target.getComponentName(),
componentName));
- target.setAllowHintDisable(resolveValue(target.getAllowHintDisable(),
allowHintDisable));
+ target.setRuleName(resolveShardingValue(target.getRuleName(),
ruleName));
+ target.setDataNodes(resolveShardingValue(target.getDataNodes(),
dataNodes));
+ target.setStorageUnits(resolveShardingValue(target.getStorageUnits(),
storageUnits));
+ target.setStrategyType(resolveShardingValue(target.getStrategyType(),
strategyType));
+
target.setShardingColumns(resolveShardingValue(target.getShardingColumns(),
shardingColumns));
+
target.setDefaultStrategyType(resolveShardingValue(target.getDefaultStrategyType(),
defaultStrategyType));
+
target.setKeyGenerateColumn(resolveShardingValue(target.getKeyGenerateColumn(),
keyGenerateColumn));
+
target.setKeyGeneratorName(resolveShardingValue(target.getKeyGeneratorName(),
keyGeneratorName));
+
target.setKeyGeneratorType(resolveShardingValue(target.getKeyGeneratorType(),
keyGeneratorType));
+ target.setSequenceName(resolveShardingValue(target.getSequenceName(),
sequenceName));
+
target.setKeyGenerateStrategyName(resolveShardingValue(target.getKeyGenerateStrategyName(),
keyGenerateStrategyName));
+
target.setComponentType(resolveShardingValue(target.getComponentType(),
componentType));
+
target.setComponentName(resolveShardingValue(target.getComponentName(),
componentName));
+
target.setAllowHintDisable(resolveShardingValue(target.getAllowHintDisable(),
allowHintDisable));
if (!referenceTables.isEmpty()) {
target.getReferenceTables().clear();
target.getReferenceTables().addAll(referenceTables);
@@ -211,11 +211,11 @@ public final class ShardingWorkflowRequest extends
WorkflowRequest {
target.getKeyGeneratorProperties().putAll(keyGeneratorProperties);
}
- private String resolveValue(final String previousValue, final String
currentValue) {
+ private String resolveShardingValue(final String previousValue, final
String currentValue) {
return currentValue.isEmpty() ? previousValue : currentValue;
}
- private String normalize(final String value) {
+ private String normalizeShardingValue(final String value) {
return null == value ? "" : value.trim();
}
}
diff --git
a/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKind.java
b/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKind.java
index cd14a310675..7d98e476caf 100644
---
a/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKind.java
+++
b/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKind.java
@@ -34,7 +34,7 @@ import java.util.regex.Pattern;
@EqualsAndHashCode
public final class WorkflowKind {
- private static final Pattern IDENTIFIER_PATTERN =
Pattern.compile("[a-z0-9]+(?:\\.[a-z0-9]+)+");
+ private static final Pattern IDENTIFIER_PATTERN =
Pattern.compile("[a-z0-9]+(?:\\.[a-z0-9]+)++");
private final String value;
diff --git
a/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKindTest.java
b/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKindTest.java
index b694ac2c9fc..7e70fbc4420 100644
---
a/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKindTest.java
+++
b/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/WorkflowKindTest.java
@@ -32,9 +32,22 @@ class WorkflowKindTest {
assertThat(actual.toString(), is("encrypt.rule"));
}
+ @Test
+ void assertValueOfWithLongValue() {
+ String value = "foo.".repeat(10000) + "bar";
+ assertThat(WorkflowKind.valueOf(value).getValue(), is(value));
+ }
+
@Test
void assertValueOfWithInvalidValue() {
IllegalArgumentException actual =
assertThrows(IllegalArgumentException.class, () ->
WorkflowKind.valueOf("encrypt_rule"));
assertThat(actual.getMessage(), is("Invalid workflow kind
`encrypt_rule`."));
}
+
+ @Test
+ void assertValueOfWithLongInvalidValue() {
+ String value = "foo.".repeat(10000);
+ IllegalArgumentException actual =
assertThrows(IllegalArgumentException.class, () -> WorkflowKind.valueOf(value));
+ assertThat(actual.getMessage(), is(String.format("Invalid workflow
kind `%s`.", value)));
+ }
}
diff --git
a/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDMLStatementVisitor.java
b/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDMLStatementVisitor.java
index 53e31de6510..844f641429c 100644
---
a/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDMLStatementVisitor.java
+++
b/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDMLStatementVisitor.java
@@ -210,7 +210,7 @@ public final class MySQLDMLStatementVisitor extends
MySQLStatementVisitor implem
// MARIADB CHANGED BEGIN
if (null != ctx.LEAD() || null != ctx.LAG() || null !=
ctx.FIRST_VALUE() || null != ctx.LAST_VALUE() || null != ctx.MEDIAN()) {
result.getParameters().add((ExpressionSegment) visit(ctx.expr()));
- appendLeadLagParameters(result.getParameters(), ctx.leadLagInfo());
+ appendWindowFunctionLeadLagParameters(result.getParameters(),
ctx.leadLagInfo());
}
// MARIADB CHANGED END
if (null != ctx.NTH_VALUE()) {
@@ -221,7 +221,7 @@ public final class MySQLDMLStatementVisitor extends
MySQLStatementVisitor implem
return result;
}
- private void appendLeadLagParameters(final Collection<ExpressionSegment>
parameters, final LeadLagInfoContext ctx) {
+ private void appendWindowFunctionLeadLagParameters(final
Collection<ExpressionSegment> parameters, final LeadLagInfoContext ctx) {
if (null == ctx) {
return;
}
