Skip to content

Commit

Permalink
Fix sonar issue on WhereClauseShardingConditionEngine
Browse files Browse the repository at this point in the history
  • Loading branch information
@terrymanu
terrymanu committed Dec 24, 2024
1 parent f4c1815 commit f3344a7
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 45 deletions.
22 changes: 11 additions & 11 deletions .../java/org/apache/shardingsphere/sharding/cache/checker/ShardingRouteCacheableChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,34 +86,34 @@ 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());
}
key.getParameters().clear();
return result;
}

private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
private ShardingRouteCacheableCheckResult checkSelectCacheable(final SelectStatementContext statementContext, final List<Object> params) {
Collection<String> tableNames = new HashSet<>(statementContext.getTablesContext().getTableNames());
if (!shardingRule.isAllShardingTables(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
if (1 != tableNames.size() && !shardingRule.isAllBindingTables(tableNames) || containsNonCacheableShardingAlgorithm(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(statementContext, params);
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(statementContext, params);
return checkShardingConditionsCacheable(shardingConditions);
}

private ShardingRouteCacheableCheckResult checkUpdateCacheable(final UpdateStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
return checkUpdateOrDeleteCacheable(statementContext, params, database);
private ShardingRouteCacheableCheckResult checkUpdateCacheable(final UpdateStatementContext statementContext, final List<Object> params) {
return checkUpdateOrDeleteCacheable(statementContext, params);
}

private ShardingRouteCacheableCheckResult checkInsertCacheable(final InsertStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
Expand All @@ -140,11 +140,11 @@ private ShardingRouteCacheableCheckResult checkInsertCacheable(final InsertState
return checkShardingConditionsCacheable(shardingConditions);
}

private ShardingRouteCacheableCheckResult checkDeleteCacheable(final DeleteStatementContext statementContext, final List<Object> params, final ShardingSphereDatabase database) {
return checkUpdateOrDeleteCacheable(statementContext, params, database);
private ShardingRouteCacheableCheckResult checkDeleteCacheable(final DeleteStatementContext statementContext, final List<Object> params) {
return checkUpdateOrDeleteCacheable(statementContext, params);
}

private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQLStatementContext sqlStatementContext, final List<Object> params, final ShardingSphereDatabase database) {
private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQLStatementContext sqlStatementContext, final List<Object> params) {
Collection<String> tableNames = ((TableAvailable) sqlStatementContext).getTablesContext().getTableNames();
if (1 != tableNames.size()) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
Expand All @@ -153,7 +153,7 @@ private ShardingRouteCacheableCheckResult checkUpdateOrDeleteCacheable(final SQL
if (!isShardingTable || containsNonCacheableShardingAlgorithm(tableNames)) {
return new ShardingRouteCacheableCheckResult(false, Collections.emptyList());
}
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(database, shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params);
List<ShardingCondition> shardingConditions = new WhereClauseShardingConditionEngine(shardingRule, timestampServiceRule).createShardingConditions(sqlStatementContext, params);
return checkShardingConditionsCacheable(shardingConditions);
}

Expand Down
2 changes: 1 addition & 1 deletion ...ingsphere/sharding/route/engine/condition/engine/InsertClauseShardingConditionEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ private Object getShardingValue(final SimpleExpressionSegment expressionSegment,

private List<ShardingCondition> createShardingConditionsWithInsertSelect(final InsertStatementContext sqlStatementContext, final List<Object> 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<ShardingCondition> shardingConditions) {
Expand Down
2 changes: 1 addition & 1 deletion ...apache/shardingsphere/sharding/route/engine/condition/engine/ShardingConditionEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ public List<ShardingCondition> 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);
}
}
17 changes: 2 additions & 15 deletions ...dingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -103,19 +97,12 @@ private Collection<ShardingCondition> 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<Column, Collection<ShardingConditionValue>> createShardingConditionValueMap(final Collection<ExpressionSegment> predicates, final List<Object> params) {
Map<Column, Collection<ShardingConditionValue>> result = new HashMap<>(predicates.size(), 1F);
for (ExpressionSegment each : predicates) {
for (ColumnSegment columnSegment : ColumnExtractor.extract(each)) {
String tableName = columnSegment.getColumnBoundInfo().getOriginalTable().getValue();
Optional<String> shardingColumn = shardingRule.findShardingColumn(columnSegment.getColumnBoundInfo().getOriginalColumn().getValue(), tableName);
Optional<String> shardingColumn = rule.findShardingColumn(columnSegment.getColumnBoundInfo().getOriginalColumn().getValue(), tableName);
if (!shardingColumn.isPresent()) {
continue;
}
Expand Down
24 changes: 7 additions & 17 deletions ...sphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -60,21 +54,17 @@ class WhereClauseShardingConditionEngineTest {
private WhereClauseShardingConditionEngine shardingConditionEngine;

@Mock
private ShardingRule shardingRule;
private ShardingRule rule;

@Mock
private SelectStatementContext sqlStatementContext;

@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));
}

Expand All @@ -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<ShardingCondition> 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
Expand All @@ -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<ShardingCondition> 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));
}
}

0 comments on commit f3344a7

Please sign in to comment.