From f3344a7e2c9fb5511ac01c478be28e3a951e3928 Mon Sep 17 00:00:00 2001 From: zhangliang Date: Tue, 24 Dec 2024 22:13:19 +0800 Subject: [PATCH 1/2] Fix sonar issue on WhereClauseShardingConditionEngine --- .../ShardingRouteCacheableChecker.java | 22 ++++++++--------- .../InsertClauseShardingConditionEngine.java | 2 +- .../engine/ShardingConditionEngine.java | 2 +- .../WhereClauseShardingConditionEngine.java | 17 ++----------- ...hereClauseShardingConditionEngineTest.java | 24 ++++++------------- 5 files changed, 22 insertions(+), 45 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/cache/checker/ShardingRouteCacheableChecker.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/cache/checker/ShardingRouteCacheableChecker.java index 9a308d3d1d234..67b6160b7dd92 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/cache/checker/ShardingRouteCacheableChecker.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/cache/checker/ShardingRouteCacheableChecker.java @@ -86,13 +86,13 @@ private ShardingRouteCacheableCheckResult load(final Key key) { SQLStatementContext sqlStatementContext = key.getSqlStatementContext(); ShardingRouteCacheableCheckResult result; if (sqlStatementContext instanceof SelectStatementContext) { - result = checkSelectCacheable((SelectStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase()); + result = checkSelectCacheable((SelectStatementContext) sqlStatementContext, key.getParameters()); } else if (sqlStatementContext instanceof UpdateStatementContext) { - result = checkUpdateCacheable((UpdateStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase()); + result = checkUpdateCacheable((UpdateStatementContext) sqlStatementContext, key.getParameters()); } else if (sqlStatementContext instanceof InsertStatementContext) { result = checkInsertCacheable((InsertStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase()); } else if (sqlStatementContext instanceof DeleteStatementContext) { - result = checkDeleteCacheable((DeleteStatementContext) sqlStatementContext, key.getParameters(), key.getDatabase()); + result = checkDeleteCacheable((DeleteStatementContext) sqlStatementContext, key.getParameters()); } else { result = new ShardingRouteCacheableCheckResult(false, Collections.emptyList()); } @@ -100,7 +100,7 @@ private ShardingRouteCacheableCheckResult load(final Key key) { return result; } - private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectStatementContext statementContext, final List params, final ShardingSphereDatabase database) { + private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectStatementContext statementContext, final List params) { Collection tableNames = new HashSet<>(statementContext.getTablesContext().getTableNames()); if (!shardingRule.isAllShardingTables(tableNames)) { return new ShardingRouteCacheableCheckResult(false, Collections.emptyList()); @@ -108,12 +108,12 @@ private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectState if (1 != tableNames.size() && !shardingRule.isAllBindingTables(tableNames) || containsNonCacheableShardingAlgorithm(tableNames)) { return new ShardingRouteCacheableCheckResult(false, Collections.emptyList()); } - List shardingConditions = new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(statementContext, params); + List shardingConditions = new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(statementContext, params); return checkShardingConditionsCacheable(shardingConditions); } - private ShardingRouteCacheableCheckResult checkUpdateCacheable(final UpdateStatementContext statementContext, final List params, final ShardingSphereDatabase database) { - return checkUpdateOrDeleteCacheable(statementContext, params, database); + private ShardingRouteCacheableCheckResult checkUpdateCacheable(final UpdateStatementContext statementContext, final List params) { + return checkUpdateOrDeleteCacheable(statementContext, params); } private ShardingRouteCacheableCheckResult checkInsertCacheable(final InsertStatementContext statementContext, final List params, final ShardingSphereDatabase database) { @@ -140,11 +140,11 @@ private ShardingRouteCacheableCheckResult checkInsertCacheable(final InsertState return checkShardingConditionsCacheable(shardingConditions); } - private ShardingRouteCacheableCheckResult checkDeleteCacheable(final DeleteStatementContext statementContext, final List params, final ShardingSphereDatabase database) { - return checkUpdateOrDeleteCacheable(statementContext, params, database); + private ShardingRouteCacheableCheckResult checkDeleteCacheable(final DeleteStatementContext statementContext, final List params) { + return checkUpdateOrDeleteCacheable(statementContext, params); } - private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQLStatementContext sqlStatementContext, final List params, final ShardingSphereDatabase database) { + private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQLStatementContext sqlStatementContext, final List params) { Collection tableNames = ((TableAvailable) sqlStatementContext).getTablesContext().getTableNames(); if (1 != tableNames.size()) { return new ShardingRouteCacheableCheckResult(false, Collections.emptyList()); @@ -153,7 +153,7 @@ private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQL if (!isShardingTable || containsNonCacheableShardingAlgorithm(tableNames)) { return new ShardingRouteCacheableCheckResult(false, Collections.emptyList()); } - List shardingConditions = new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params); + List shardingConditions = new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params); return checkShardingConditionsCacheable(shardingConditions); } diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/InsertClauseShardingConditionEngine.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/InsertClauseShardingConditionEngine.java index 15c7cd5615339..7bee81e16c715 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/InsertClauseShardingConditionEngine.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/InsertClauseShardingConditionEngine.java @@ -164,7 +164,7 @@ private Object getShardingValue(final SimpleExpressionSegment expressionSegment, private List createShardingConditionsWithInsertSelect(final InsertStatementContext sqlStatementContext, final List params) { SelectStatementContext selectStatementContext = sqlStatementContext.getInsertSelectContext().getSelectStatementContext(); - return new LinkedList<>(new WhereClauseShardingConditionEngine(database, rule, timestampServiceRule).createShardingConditions(selectStatementContext, params)); + return new LinkedList<>(new WhereClauseShardingConditionEngine(rule, timestampServiceRule).createShardingConditions(selectStatementContext, params)); } private void appendGeneratedKeyConditions(final InsertStatementContext sqlStatementContext, final List shardingConditions) { diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/ShardingConditionEngine.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/ShardingConditionEngine.java index 72eff8f587ef9..512c94de9262e 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/ShardingConditionEngine.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/ShardingConditionEngine.java @@ -51,6 +51,6 @@ public List createShardingConditions(final SQLStatementContex TimestampServiceRule timestampServiceRule = globalRuleMetaData.getSingleRule(TimestampServiceRule.class); return sqlStatementContext instanceof InsertStatementContext ? new InsertClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions((InsertStatementContext) sqlStatementContext, params) - : new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params); + : new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params); } } diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngine.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngine.java index 8a9eb3f0aecf0..d558597ea3e31 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngine.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngine.java @@ -21,11 +21,7 @@ import lombok.RequiredArgsConstructor; import org.apache.shardingsphere.infra.binder.context.extractor.SQLStatementContextExtractor; import org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext; -import org.apache.shardingsphere.infra.binder.context.type.TableAvailable; import org.apache.shardingsphere.infra.binder.context.type.WhereAvailable; -import org.apache.shardingsphere.infra.database.core.type.DatabaseTypeRegistry; -import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase; -import org.apache.shardingsphere.infra.metadata.database.schema.model.ShardingSphereSchema; import org.apache.shardingsphere.sharding.exception.data.ShardingValueDataTypeException; import org.apache.shardingsphere.sharding.route.engine.condition.AlwaysFalseShardingCondition; import org.apache.shardingsphere.sharding.route.engine.condition.Column; @@ -63,9 +59,7 @@ @RequiredArgsConstructor public final class WhereClauseShardingConditionEngine { - private final ShardingSphereDatabase database; - - private final ShardingRule shardingRule; + private final ShardingRule rule; private final TimestampServiceRule timestampServiceRule; @@ -103,19 +97,12 @@ private Collection createShardingConditions(final ExpressionS return result; } - private ShardingSphereSchema getSchema(final SQLStatementContext sqlStatementContext, final ShardingSphereDatabase database) { - String defaultSchemaName = new DatabaseTypeRegistry(sqlStatementContext.getDatabaseType()).getDefaultSchemaName(database.getName()); - return sqlStatementContext instanceof TableAvailable - ? ((TableAvailable) sqlStatementContext).getTablesContext().getSchemaName().map(database::getSchema).orElseGet(() -> database.getSchema(defaultSchemaName)) - : database.getSchema(defaultSchemaName); - } - private Map> createShardingConditionValueMap(final Collection predicates, final List params) { Map> result = new HashMap<>(predicates.size(), 1F); for (ExpressionSegment each : predicates) { for (ColumnSegment columnSegment : ColumnExtractor.extract(each)) { String tableName = columnSegment.getColumnBoundInfo().getOriginalTable().getValue(); - Optional shardingColumn = shardingRule.findShardingColumn(columnSegment.getColumnBoundInfo().getOriginalColumn().getValue(), tableName); + Optional shardingColumn = rule.findShardingColumn(columnSegment.getColumnBoundInfo().getOriginalColumn().getValue(), tableName); if (!shardingColumn.isPresent()) { continue; } diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java index d85bba0435e9c..ba8b39a7c12b0 100644 --- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java +++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java @@ -17,12 +17,7 @@ package org.apache.shardingsphere.sharding.route.engine.condition.engine; -import org.apache.shardingsphere.infra.binder.context.segment.table.TablesContext; import org.apache.shardingsphere.infra.binder.context.statement.dml.SelectStatementContext; -import org.apache.shardingsphere.infra.config.props.ConfigurationProperties; -import org.apache.shardingsphere.infra.database.core.type.DatabaseType; -import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase; -import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader; import org.apache.shardingsphere.sharding.route.engine.condition.ShardingCondition; import org.apache.shardingsphere.sharding.route.engine.condition.value.ListShardingConditionValue; import org.apache.shardingsphere.sharding.route.engine.condition.value.RangeShardingConditionValue; @@ -45,11 +40,10 @@ import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.Properties; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -60,7 +54,7 @@ class WhereClauseShardingConditionEngineTest { private WhereClauseShardingConditionEngine shardingConditionEngine; @Mock - private ShardingRule shardingRule; + private ShardingRule rule; @Mock private SelectStatementContext sqlStatementContext; @@ -68,13 +62,9 @@ class WhereClauseShardingConditionEngineTest { @Mock private WhereSegment whereSegment; - @Mock - private TablesContext tablesContext; - @BeforeEach void setUp() { - shardingConditionEngine = new WhereClauseShardingConditionEngine(ShardingSphereDatabase.create("test_db", - TypedSPILoader.getService(DatabaseType.class, "FIXTURE"), new ConfigurationProperties(new Properties())), shardingRule, mock(TimestampServiceRule.class)); + shardingConditionEngine = new WhereClauseShardingConditionEngine(rule, mock(TimestampServiceRule.class)); when(sqlStatementContext.getWhereSegments()).thenReturn(Collections.singleton(whereSegment)); } @@ -87,10 +77,10 @@ void assertCreateShardingConditionsForSelectRangeStatement() { ExpressionSegment andSegment = new LiteralExpressionSegment(0, 0, and); BetweenExpression betweenExpression = new BetweenExpression(0, 0, left, betweenSegment, andSegment, false); when(whereSegment.getExpr()).thenReturn(betweenExpression); - when(shardingRule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col")); + when(rule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col")); List actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Collections.emptyList()); assertThat(actual.get(0).getStartIndex(), is(0)); - assertTrue(actual.get(0).getValues().get(0) instanceof RangeShardingConditionValue); + assertThat(actual.get(0).getValues().get(0), instanceOf(RangeShardingConditionValue.class)); } @Test @@ -101,9 +91,9 @@ void assertCreateShardingConditionsForSelectInStatement() { right.getItems().add(literalExpressionSegment); InExpression inExpression = new InExpression(0, 0, left, right, false); when(whereSegment.getExpr()).thenReturn(inExpression); - when(shardingRule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col")); + when(rule.findShardingColumn(any(), any())).thenReturn(Optional.of("foo_sharding_col")); List actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Collections.emptyList()); assertThat(actual.get(0).getStartIndex(), is(0)); - assertTrue(actual.get(0).getValues().get(0) instanceof ListShardingConditionValue); + assertThat(actual.get(0).getValues().get(0), instanceOf(ListShardingConditionValue.class)); } } From eac2b94c9f1a4daff509056a558af7a06981caf1 Mon Sep 17 00:00:00 2001 From: zhangliang Date: Tue, 24 Dec 2024 22:20:42 +0800 Subject: [PATCH 2/2] Fix sonar issue on PipelineDDLGenerator --- .../core/metadata/generator/PipelineDDLGenerator.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/metadata/generator/PipelineDDLGenerator.java b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/metadata/generator/PipelineDDLGenerator.java index 37d281e4da6a7..dbdb67da6bd2d 100644 --- a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/metadata/generator/PipelineDDLGenerator.java +++ b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/metadata/generator/PipelineDDLGenerator.java @@ -17,7 +17,8 @@ package org.apache.shardingsphere.data.pipeline.core.metadata.generator; -import lombok.RequiredArgsConstructor; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.shardingsphere.data.pipeline.core.sqlbuilder.dialect.DialectPipelineSQLBuilder; import org.apache.shardingsphere.infra.database.core.spi.DatabaseTypedSPILoader; @@ -31,7 +32,7 @@ /** * Pipeline DDL generator. */ -@RequiredArgsConstructor +@NoArgsConstructor(access = AccessLevel.PRIVATE) @Slf4j public final class PipelineDDLGenerator {