You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by to...@apache.org on 2022/11/24 18:40:18 UTC
[shardingsphere] branch master updated: Remove SQLCheckResult (#22394)
This is an automated email from the ASF dual-hosted git repository.
totalo 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 cd37b391dfd Remove SQLCheckResult (#22394)
cd37b391dfd is described below
commit cd37b391dfde4bd77da47ea00756fa9cff3507fe
Author: Liang Zhang <zh...@apache.org>
AuthorDate: Fri Nov 25 02:40:11 2022 +0800
Remove SQLCheckResult (#22394)
---
agent/plugins/tracing/pom.xml | 6 ++--
.../sharding/spi/ShardingAuditAlgorithm.java | 4 +--
.../fixture/ShardingAuditAlgorithmFixture.java | 10 +++----
...MLShardingConditionsShardingAuditAlgorithm.java | 13 ++++-----
.../checker/audit/ShardingAuditChecker.java | 18 ++++--------
...sphere.infra.executor.check.checker.SQLChecker} | 0
...ardingConditionsShardingAuditAlgorithmTest.java | 19 ++++---------
.../sharding/checker/ShardingAuditCheckerTest.java | 27 +++++++++---------
.../DropShardingTableRuleStatementUpdater.java | 2 +-
.../shardingsphere/infra/check/SQLCheckResult.java | 33 ----------------------
.../infra/executor/check/SQLCheckEngine.java | 9 ++----
.../executor/check/{ => checker}/SQLChecker.java | 6 ++--
.../check/{ => checker}/SQLCheckerFactory.java | 2 +-
.../check/exception}/SQLCheckException.java | 2 +-
.../check/{ => checker}/SQLCheckerFactoryTest.java | 4 +--
.../{ => checker}/fixture/SQLCheckerFixture.java | 12 ++++----
...sphere.infra.executor.check.checker.SQLChecker} | 2 +-
.../authority/checker/AuthorityChecker.java | 27 ++++++++----------
...sphere.infra.executor.check.checker.SQLChecker} | 0
.../authority/checker/AuthorityCheckerTest.java | 10 +++----
.../discovery/command/DiscoveryDistSQLCommand.java | 2 +-
.../command/MGRPrimaryReplicaCommand.java | 2 +-
.../fixture/ITShardingAuditAlgorithmFixture.java | 10 +++----
23 files changed, 77 insertions(+), 143 deletions(-)
diff --git a/agent/plugins/tracing/pom.xml b/agent/plugins/tracing/pom.xml
index 5a94696240f..a506bdecc00 100644
--- a/agent/plugins/tracing/pom.xml
+++ b/agent/plugins/tracing/pom.xml
@@ -29,11 +29,11 @@
<name>${project.artifactId}</name>
<modules>
- <module>test</module>
- <module>jaeger</module>
- <module>zipkin</module>
<module>opentracing</module>
<module>opentelemetry</module>
+ <module>jaeger</module>
+ <module>zipkin</module>
+ <module>test</module>
</modules>
<properties>
diff --git a/features/sharding/api/src/main/java/org/apache/shardingsphere/sharding/spi/ShardingAuditAlgorithm.java b/features/sharding/api/src/main/java/org/apache/shardingsphere/sharding/spi/ShardingAuditAlgorithm.java
index eba2e17e6dc..ba24f545aca 100644
--- a/features/sharding/api/src/main/java/org/apache/shardingsphere/sharding/spi/ShardingAuditAlgorithm.java
+++ b/features/sharding/api/src/main/java/org/apache/shardingsphere/sharding/spi/ShardingAuditAlgorithm.java
@@ -18,7 +18,6 @@
package org.apache.shardingsphere.sharding.spi;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
import org.apache.shardingsphere.infra.config.algorithm.ShardingSphereAlgorithm;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
@@ -37,7 +36,6 @@ public interface ShardingAuditAlgorithm extends ShardingSphereAlgorithm {
* @param params SQL parameters
* @param grantee grantee
* @param database database
- * @return SQL check result
*/
- SQLCheckResult check(SQLStatementContext<?> sqlStatementContext, List<Object> params, Grantee grantee, ShardingSphereDatabase database);
+ void check(SQLStatementContext<?> sqlStatementContext, List<Object> params, Grantee grantee, ShardingSphereDatabase database);
}
diff --git a/features/sharding/api/src/test/java/org/apache/shardingsphere/sharding/fixture/ShardingAuditAlgorithmFixture.java b/features/sharding/api/src/test/java/org/apache/shardingsphere/sharding/fixture/ShardingAuditAlgorithmFixture.java
index a58902bd707..79147266af6 100644
--- a/features/sharding/api/src/test/java/org/apache/shardingsphere/sharding/fixture/ShardingAuditAlgorithmFixture.java
+++ b/features/sharding/api/src/test/java/org/apache/shardingsphere/sharding/fixture/ShardingAuditAlgorithmFixture.java
@@ -18,7 +18,6 @@
package org.apache.shardingsphere.sharding.fixture;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.sharding.spi.ShardingAuditAlgorithm;
@@ -29,17 +28,16 @@ import java.util.Properties;
public final class ShardingAuditAlgorithmFixture implements ShardingAuditAlgorithm {
@Override
- public Properties getProps() {
- return new Properties();
+ public void init(final Properties props) {
}
@Override
- public void init(final Properties props) {
+ public void check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final ShardingSphereDatabase database) {
}
@Override
- public SQLCheckResult check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final ShardingSphereDatabase database) {
- return new SQLCheckResult(true, "");
+ public Properties getProps() {
+ return new Properties();
}
@Override
diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithm.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithm.java
index b5ed76796af..1387dedd562 100644
--- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithm.java
+++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithm.java
@@ -19,9 +19,10 @@ package org.apache.shardingsphere.sharding.algorithm.audit;
import lombok.Getter;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
+import org.apache.shardingsphere.infra.executor.check.exception.SQLCheckException;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
+import org.apache.shardingsphere.infra.util.exception.ShardingSpherePreconditions;
import org.apache.shardingsphere.sharding.route.engine.condition.engine.ShardingConditionEngine;
import org.apache.shardingsphere.sharding.route.engine.condition.engine.ShardingConditionEngineFactory;
import org.apache.shardingsphere.sharding.rule.ShardingRule;
@@ -46,19 +47,17 @@ public final class DMLShardingConditionsShardingAuditAlgorithm implements Shardi
@SuppressWarnings({"rawtypes", "unchecked"})
@Override
- public SQLCheckResult check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final ShardingSphereDatabase database) {
+ public void check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final ShardingSphereDatabase database) {
if (sqlStatementContext.getSqlStatement() instanceof DMLStatement) {
ShardingRule rule = database.getRuleMetaData().getSingleRule(ShardingRule.class);
if (rule.isAllBroadcastTables(sqlStatementContext.getTablesContext().getTableNames())
|| sqlStatementContext.getTablesContext().getTableNames().stream().noneMatch(rule::isShardingTable)) {
- return new SQLCheckResult(true, "");
+ return;
}
ShardingConditionEngine shardingConditionEngine = ShardingConditionEngineFactory.createShardingConditionEngine(sqlStatementContext, database, rule);
- if (shardingConditionEngine.createShardingConditions(sqlStatementContext, params).isEmpty()) {
- return new SQLCheckResult(false, "Not allow DML operation without sharding conditions");
- }
+ ShardingSpherePreconditions.checkState(!shardingConditionEngine.createShardingConditions(sqlStatementContext, params).isEmpty(),
+ () -> new SQLCheckException("Not allow DML operation without sharding conditions"));
}
- return new SQLCheckResult(true, "");
}
@Override
diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/checker/audit/ShardingAuditChecker.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/checker/audit/ShardingAuditChecker.java
index 4b109fa6857..1c219f35fc9 100644
--- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/checker/audit/ShardingAuditChecker.java
+++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/checker/audit/ShardingAuditChecker.java
@@ -19,8 +19,7 @@ package org.apache.shardingsphere.sharding.checker.audit;
import org.apache.shardingsphere.infra.binder.statement.CommonSQLStatementContext;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
-import org.apache.shardingsphere.infra.executor.check.SQLChecker;
+import org.apache.shardingsphere.infra.executor.check.checker.SQLChecker;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.sharding.api.config.strategy.audit.ShardingAuditStrategyConfiguration;
@@ -45,27 +44,22 @@ public final class ShardingAuditChecker implements SQLChecker<ShardingRule> {
}
@Override
- public SQLCheckResult check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee,
- final String currentDatabase, final Map<String, ShardingSphereDatabase> databases, final ShardingRule rule) {
+ public void check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee,
+ final String currentDatabase, final Map<String, ShardingSphereDatabase> databases, final ShardingRule rule) {
Collection<ShardingAuditStrategyConfiguration> auditStrategies = getShardingAuditStrategies(sqlStatementContext, rule);
if (auditStrategies.isEmpty()) {
- return new SQLCheckResult(true, "");
+ return;
}
Collection<String> disableAuditNames = sqlStatementContext instanceof CommonSQLStatementContext
? ((CommonSQLStatementContext<?>) sqlStatementContext).getSqlHintExtractor().findDisableAuditNames()
: Collections.emptyList();
for (ShardingAuditStrategyConfiguration auditStrategy : auditStrategies) {
for (String auditorName : auditStrategy.getAuditorNames()) {
- if (auditStrategy.isAllowHintDisable() && disableAuditNames.contains(auditorName.toLowerCase())) {
- continue;
- }
- SQLCheckResult result = rule.getAuditors().get(auditorName).check(sqlStatementContext, params, grantee, databases.get(currentDatabase.toLowerCase()));
- if (!result.isPassed()) {
- return result;
+ if (!auditStrategy.isAllowHintDisable() || !disableAuditNames.contains(auditorName.toLowerCase())) {
+ rule.getAuditors().get(auditorName).check(sqlStatementContext, params, grantee, databases.get(currentDatabase.toLowerCase()));
}
}
}
- return new SQLCheckResult(true, "");
}
@Override
diff --git a/features/sharding/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.SQLChecker b/features/sharding/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.checker.SQLChecker
similarity index 100%
rename from features/sharding/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.SQLChecker
rename to features/sharding/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.checker.SQLChecker
diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithmTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithmTest.java
index 847cb04e24b..fc76a728af7 100644
--- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithmTest.java
+++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/audit/DMLShardingConditionsShardingAuditAlgorithmTest.java
@@ -18,8 +18,8 @@
package org.apache.shardingsphere.sharding.algorithm.audit;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
import org.apache.shardingsphere.infra.config.algorithm.AlgorithmConfiguration;
+import org.apache.shardingsphere.infra.executor.check.exception.SQLCheckException;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.database.rule.ShardingSphereRuleMetaData;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
@@ -34,8 +34,6 @@ import org.junit.Test;
import java.util.Collections;
import java.util.Properties;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.MatcherAssert.assertThat;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
@@ -64,7 +62,7 @@ public final class DMLShardingConditionsShardingAuditAlgorithmTest {
@Test
public void assertNotDMLStatementCheck() {
when(sqlStatementContext.getSqlStatement()).thenReturn(mock(DDLStatement.class));
- assertCheckResult(shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database), true, "");
+ shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database);
verify(database, times(0)).getRuleMetaData();
}
@@ -73,7 +71,7 @@ public final class DMLShardingConditionsShardingAuditAlgorithmTest {
when(sqlStatementContext.getSqlStatement()).thenReturn(mock(DMLStatement.class));
when(database.getRuleMetaData()).thenReturn(new ShardingSphereRuleMetaData(Collections.singletonList(rule)));
when(rule.isAllBroadcastTables(sqlStatementContext.getTablesContext().getTableNames())).thenReturn(true);
- assertCheckResult(shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database), true, "");
+ shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database);
}
@Test
@@ -81,20 +79,15 @@ public final class DMLShardingConditionsShardingAuditAlgorithmTest {
when(sqlStatementContext.getSqlStatement()).thenReturn(mock(DMLStatement.class));
when(database.getRuleMetaData()).thenReturn(new ShardingSphereRuleMetaData(Collections.singletonList(rule)));
when(rule.isAllBroadcastTables(sqlStatementContext.getTablesContext().getTableNames())).thenReturn(false);
- assertCheckResult(shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database), true, "");
+ shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database);
}
- @Test
+ @Test(expected = SQLCheckException.class)
public void assertEmptyShardingConditionsCheck() {
when(sqlStatementContext.getSqlStatement()).thenReturn(mock(DMLStatement.class));
when(database.getRuleMetaData()).thenReturn(new ShardingSphereRuleMetaData(Collections.singletonList(rule)));
when(rule.isAllBroadcastTables(sqlStatementContext.getTablesContext().getTableNames())).thenReturn(false);
when(rule.isShardingTable("t_order")).thenReturn(true);
- assertCheckResult(shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database), false, "Not allow DML operation without sharding conditions");
- }
-
- private void assertCheckResult(final SQLCheckResult checkResult, final boolean isPassed, final String errorMessage) {
- assertThat(checkResult.isPassed(), is(isPassed));
- assertThat(checkResult.getErrorMessage(), is(errorMessage));
+ shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), database);
}
}
diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/checker/ShardingAuditCheckerTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/checker/ShardingAuditCheckerTest.java
index 7bea8867795..1273218fe4d 100644
--- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/checker/ShardingAuditCheckerTest.java
+++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/checker/ShardingAuditCheckerTest.java
@@ -18,13 +18,14 @@
package org.apache.shardingsphere.sharding.checker;
import org.apache.shardingsphere.infra.binder.statement.CommonSQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
+import org.apache.shardingsphere.infra.executor.check.exception.SQLCheckException;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.sharding.api.config.strategy.audit.ShardingAuditStrategyConfiguration;
import org.apache.shardingsphere.sharding.checker.audit.ShardingAuditChecker;
import org.apache.shardingsphere.sharding.rule.ShardingRule;
import org.apache.shardingsphere.sharding.rule.TableRule;
+import org.apache.shardingsphere.sharding.spi.ShardingAuditAlgorithm;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -39,6 +40,7 @@ import java.util.Optional;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -73,30 +75,27 @@ public final class ShardingAuditCheckerTest {
@Test
public void assertCheckSQLStatementPass() {
- when(rule.getAuditors().get("auditor_1").check(sqlStatementContext, Collections.emptyList(), grantee, databases.get("foo_db"))).thenReturn(new SQLCheckResult(true, ""));
- assertCheckResult(new ShardingAuditChecker().check(sqlStatementContext, Collections.emptyList(), grantee, "foo_db", databases, rule), true, "");
+ new ShardingAuditChecker().check(sqlStatementContext, Collections.emptyList(), grantee, "foo_db", databases, rule);
verify(rule.getAuditors().get("auditor_1"), times(1)).check(sqlStatementContext, Collections.emptyList(), grantee, databases.get("foo_db"));
}
@Test
public void assertSQCheckPassByDisableAuditNames() {
- when(rule.getAuditors().get("auditor_1").check(sqlStatementContext, Collections.emptyList(), grantee, databases.get("foo_db"))).thenReturn(new SQLCheckResult(false, ""));
when(auditStrategy.isAllowHintDisable()).thenReturn(true);
- assertCheckResult(new ShardingAuditChecker().check(sqlStatementContext, Collections.emptyList(), grantee, "foo_db", databases, rule), true, "");
+ new ShardingAuditChecker().check(sqlStatementContext, Collections.emptyList(), grantee, "foo_db", databases, rule);
verify(rule.getAuditors().get("auditor_1"), times(0)).check(sqlStatementContext, Collections.emptyList(), grantee, databases.get("foo_db"));
}
@Test
public void assertSQLCheckNotPass() {
- when(rule.getAuditors().get("auditor_1").check(sqlStatementContext, Collections.emptyList(), grantee, databases.get("foo_db")))
- .thenReturn(new SQLCheckResult(false, "Not allow DML operation without sharding conditions"));
- assertCheckResult(new ShardingAuditChecker().check(
- sqlStatementContext, Collections.emptyList(), grantee, "foo_db", databases, rule), false, "Not allow DML operation without sharding conditions");
+ ShardingAuditAlgorithm auditAlgorithm = rule.getAuditors().get("auditor_1");
+ doThrow(new SQLCheckException("Not allow DML operation without sharding conditions"))
+ .when(auditAlgorithm).check(sqlStatementContext, Collections.emptyList(), grantee, databases.get("foo_db"));
+ try {
+ new ShardingAuditChecker().check(sqlStatementContext, Collections.emptyList(), grantee, "foo_db", databases, rule);
+ } catch (final SQLCheckException ex) {
+ assertThat(ex.getMessage(), is("SQL check failed, error message: Not allow DML operation without sharding conditions."));
+ }
verify(rule.getAuditors().get("auditor_1"), times(1)).check(sqlStatementContext, Collections.emptyList(), grantee, databases.get("foo_db"));
}
-
- private void assertCheckResult(final SQLCheckResult checkResult, final boolean isPassed, final String errorMessage) {
- assertThat(checkResult.isPassed(), is(isPassed));
- assertThat(checkResult.getErrorMessage(), is(errorMessage));
- }
}
diff --git a/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/update/DropShardingTableRuleStatementUpdater.java b/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/update/DropShardingTableRuleStatementUpdater.java
index 4b32f68d0d9..4f91fd4174a 100644
--- a/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/update/DropShardingTableRuleStatementUpdater.java
+++ b/features/sharding/distsql/handler/src/main/java/org/apache/shardingsphere/sharding/distsql/handler/update/DropShardingTableRuleStatementUpdater.java
@@ -119,7 +119,7 @@ public final class DropShardingTableRuleStatementUpdater extends AbstractDropSha
dropUnusedAuditor(currentRuleConfig);
return false;
}
-
+
private void dropUnusedKeyGenerator(final ShardingRuleConfiguration currentRuleConfig) {
Collection<String> inUsedKeyGenerators = currentRuleConfig.getTables().stream().map(ShardingTableRuleConfiguration::getKeyGenerateStrategy).filter(Objects::nonNull)
.map(KeyGenerateStrategyConfiguration::getKeyGeneratorName).collect(Collectors.toSet());
diff --git a/infra/common/src/main/java/org/apache/shardingsphere/infra/check/SQLCheckResult.java b/infra/common/src/main/java/org/apache/shardingsphere/infra/check/SQLCheckResult.java
deleted file mode 100644
index d336d25716b..00000000000
--- a/infra/common/src/main/java/org/apache/shardingsphere/infra/check/SQLCheckResult.java
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.shardingsphere.infra.check;
-
-import lombok.Getter;
-import lombok.RequiredArgsConstructor;
-
-/**
- * SQL check result.
- */
-@RequiredArgsConstructor
-@Getter
-public final class SQLCheckResult {
-
- private final boolean isPassed;
-
- private final String errorMessage;
-}
diff --git a/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLCheckEngine.java b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLCheckEngine.java
index b8c5b70f7f6..9919d60c69c 100644
--- a/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLCheckEngine.java
+++ b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLCheckEngine.java
@@ -20,8 +20,8 @@ package org.apache.shardingsphere.infra.executor.check;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckException;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
+import org.apache.shardingsphere.infra.executor.check.checker.SQLChecker;
+import org.apache.shardingsphere.infra.executor.check.checker.SQLCheckerFactory;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
@@ -71,10 +71,7 @@ public final class SQLCheckEngine {
public static void check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Collection<ShardingSphereRule> rules,
final String currentDatabase, final Map<String, ShardingSphereDatabase> databases, final Grantee grantee) {
for (Entry<ShardingSphereRule, SQLChecker> entry : SQLCheckerFactory.getInstance(rules).entrySet()) {
- SQLCheckResult checkResult = entry.getValue().check(sqlStatementContext, params, grantee, currentDatabase, databases, entry.getKey());
- if (!checkResult.isPassed()) {
- throw new SQLCheckException(checkResult.getErrorMessage());
- }
+ entry.getValue().check(sqlStatementContext, params, grantee, currentDatabase, databases, entry.getKey());
}
}
diff --git a/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLChecker.java b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/checker/SQLChecker.java
similarity index 87%
rename from infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLChecker.java
rename to infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/checker/SQLChecker.java
index bbc6ea9a3e0..b3ecf1310ef 100644
--- a/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLChecker.java
+++ b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/checker/SQLChecker.java
@@ -15,10 +15,9 @@
* limitations under the License.
*/
-package org.apache.shardingsphere.infra.executor.check;
+package org.apache.shardingsphere.infra.executor.check.checker;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
@@ -54,9 +53,8 @@ public interface SQLChecker<T extends ShardingSphereRule> extends OrderedSPI<T>
* @param currentDatabase current database
* @param databases databases
* @param rule rule
- * @return SQL check result
*/
- SQLCheckResult check(SQLStatementContext<?> sqlStatementContext, List<Object> params, Grantee grantee, String currentDatabase, Map<String, ShardingSphereDatabase> databases, T rule);
+ void check(SQLStatementContext<?> sqlStatementContext, List<Object> params, Grantee grantee, String currentDatabase, Map<String, ShardingSphereDatabase> databases, T rule);
/**
* Check User.
diff --git a/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLCheckerFactory.java b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/checker/SQLCheckerFactory.java
similarity index 96%
rename from infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLCheckerFactory.java
rename to infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/checker/SQLCheckerFactory.java
index 66134f4f7fb..d7a1c439980 100644
--- a/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/SQLCheckerFactory.java
+++ b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/checker/SQLCheckerFactory.java
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-package org.apache.shardingsphere.infra.executor.check;
+package org.apache.shardingsphere.infra.executor.check.checker;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
diff --git a/infra/common/src/main/java/org/apache/shardingsphere/infra/check/SQLCheckException.java b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/exception/SQLCheckException.java
similarity index 95%
rename from infra/common/src/main/java/org/apache/shardingsphere/infra/check/SQLCheckException.java
rename to infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/exception/SQLCheckException.java
index dad4c06eda3..4036ec8b63c 100644
--- a/infra/common/src/main/java/org/apache/shardingsphere/infra/check/SQLCheckException.java
+++ b/infra/executor/src/main/java/org/apache/shardingsphere/infra/executor/check/exception/SQLCheckException.java
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-package org.apache.shardingsphere.infra.check;
+package org.apache.shardingsphere.infra.executor.check.exception;
import org.apache.shardingsphere.infra.util.exception.external.sql.type.kernel.KernelSQLException;
import org.apache.shardingsphere.infra.util.exception.external.sql.sqlstate.XOpenSQLState;
diff --git a/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/SQLCheckerFactoryTest.java b/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/checker/SQLCheckerFactoryTest.java
similarity index 91%
rename from infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/SQLCheckerFactoryTest.java
rename to infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/checker/SQLCheckerFactoryTest.java
index 9955fab1d35..a496cc1a824 100644
--- a/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/SQLCheckerFactoryTest.java
+++ b/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/checker/SQLCheckerFactoryTest.java
@@ -15,9 +15,9 @@
* limitations under the License.
*/
-package org.apache.shardingsphere.infra.executor.check;
+package org.apache.shardingsphere.infra.executor.check.checker;
-import org.apache.shardingsphere.infra.executor.check.fixture.SQLCheckerFixture;
+import org.apache.shardingsphere.infra.executor.check.checker.fixture.SQLCheckerFixture;
import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
import org.apache.shardingsphere.test.fixture.rule.MockedRule;
import org.junit.Test;
diff --git a/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/fixture/SQLCheckerFixture.java b/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/checker/fixture/SQLCheckerFixture.java
similarity index 79%
rename from infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/fixture/SQLCheckerFixture.java
rename to infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/checker/fixture/SQLCheckerFixture.java
index b8622998a20..1bed5d05e00 100644
--- a/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/fixture/SQLCheckerFixture.java
+++ b/infra/executor/src/test/java/org/apache/shardingsphere/infra/executor/check/checker/fixture/SQLCheckerFixture.java
@@ -15,14 +15,13 @@
* limitations under the License.
*/
-package org.apache.shardingsphere.infra.executor.check.fixture;
+package org.apache.shardingsphere.infra.executor.check.checker.fixture;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
-import org.apache.shardingsphere.infra.executor.check.SQLChecker;
-import org.apache.shardingsphere.test.fixture.rule.MockedRule;
+import org.apache.shardingsphere.infra.executor.check.checker.SQLChecker;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
+import org.apache.shardingsphere.test.fixture.rule.MockedRule;
import java.util.List;
import java.util.Map;
@@ -36,9 +35,8 @@ public final class SQLCheckerFixture implements SQLChecker<MockedRule> {
}
@Override
- public SQLCheckResult check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final String currentDatabase,
- final Map<String, ShardingSphereDatabase> databases, final MockedRule rule) {
- return null;
+ public void check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final String currentDatabase,
+ final Map<String, ShardingSphereDatabase> databases, final MockedRule rule) {
}
@Override
diff --git a/infra/executor/src/test/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.SQLChecker b/infra/executor/src/test/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.checker.SQLChecker
similarity index 90%
rename from infra/executor/src/test/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.SQLChecker
rename to infra/executor/src/test/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.checker.SQLChecker
index 2c5b7a748eb..885a94e9c93 100644
--- a/infra/executor/src/test/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.SQLChecker
+++ b/infra/executor/src/test/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.checker.SQLChecker
@@ -15,4 +15,4 @@
# limitations under the License.
#
-org.apache.shardingsphere.infra.executor.check.fixture.SQLCheckerFixture
\ No newline at end of file
+org.apache.shardingsphere.infra.executor.check.checker.fixture.SQLCheckerFixture
diff --git a/kernel/authority/core/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java b/kernel/authority/core/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
index c4d324485c3..50d5513a4f6 100644
--- a/kernel/authority/core/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
+++ b/kernel/authority/core/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
@@ -22,11 +22,12 @@ import org.apache.shardingsphere.authority.model.PrivilegeType;
import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
import org.apache.shardingsphere.authority.rule.AuthorityRule;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
-import org.apache.shardingsphere.infra.executor.check.SQLChecker;
+import org.apache.shardingsphere.infra.executor.check.checker.SQLChecker;
+import org.apache.shardingsphere.infra.executor.check.exception.SQLCheckException;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
+import org.apache.shardingsphere.infra.util.exception.ShardingSpherePreconditions;
import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement;
import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.AlterDatabaseStatement;
import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.AlterTableStatement;
@@ -61,24 +62,18 @@ public final class AuthorityChecker implements SQLChecker<AuthorityRule> {
}
@Override
- public SQLCheckResult check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee,
- final String currentDatabase, final Map<String, ShardingSphereDatabase> databases, final AuthorityRule rule) {
+ public void check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee,
+ final String currentDatabase, final Map<String, ShardingSphereDatabase> databases, final AuthorityRule rule) {
if (null == grantee) {
- return new SQLCheckResult(true, "");
+ return;
}
Optional<ShardingSpherePrivileges> privileges = rule.findPrivileges(grantee);
- if (!privileges.isPresent()) {
- return new SQLCheckResult(false, String.format("Access denied for user '%s'@'%s'", grantee.getUsername(), grantee.getHostname()));
- }
- if (null != currentDatabase && !privileges.filter(optional -> optional.hasPrivileges(currentDatabase)).isPresent()) {
- return new SQLCheckResult(false, String.format("Unknown database '%s'", currentDatabase));
- }
+ ShardingSpherePreconditions.checkState(privileges.isPresent(), () -> new SQLCheckException(String.format("Access denied for user '%s'@'%s'", grantee.getUsername(), grantee.getHostname())));
+ ShardingSpherePreconditions.checkState(null == currentDatabase || privileges.filter(optional -> optional.hasPrivileges(currentDatabase)).isPresent(),
+ () -> new SQLCheckException(String.format("Unknown database '%s'", currentDatabase)));
PrivilegeType privilegeType = getPrivilege(sqlStatementContext.getSqlStatement());
- return privileges.map(optional -> {
- boolean isPassed = optional.hasPrivileges(Collections.singletonList(privilegeType));
- String errorMessage = isPassed || null == privilegeType ? "" : String.format("Access denied for operation %s", privilegeType.name());
- return new SQLCheckResult(isPassed, errorMessage);
- }).orElseGet(() -> new SQLCheckResult(false, ""));
+ boolean hasPrivileges = privileges.get().hasPrivileges(Collections.singletonList(privilegeType));
+ ShardingSpherePreconditions.checkState(hasPrivileges, () -> new SQLCheckException(String.format("Access denied for operation %s.", null == privilegeType ? "" : privilegeType.name())));
}
@Override
diff --git a/kernel/authority/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.SQLChecker b/kernel/authority/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.checker.SQLChecker
similarity index 100%
rename from kernel/authority/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.SQLChecker
rename to kernel/authority/core/src/main/resources/META-INF/services/org.apache.shardingsphere.infra.executor.check.checker.SQLChecker
diff --git a/kernel/authority/core/src/test/java/org/apache/shardingsphere/authority/checker/AuthorityCheckerTest.java b/kernel/authority/core/src/test/java/org/apache/shardingsphere/authority/checker/AuthorityCheckerTest.java
index 2e596050c6a..0fdb78bf1d3 100644
--- a/kernel/authority/core/src/test/java/org/apache/shardingsphere/authority/checker/AuthorityCheckerTest.java
+++ b/kernel/authority/core/src/test/java/org/apache/shardingsphere/authority/checker/AuthorityCheckerTest.java
@@ -23,8 +23,8 @@ import org.apache.shardingsphere.infra.binder.statement.ddl.CreateTableStatement
import org.apache.shardingsphere.infra.binder.statement.dml.InsertStatementContext;
import org.apache.shardingsphere.infra.binder.statement.dml.SelectStatementContext;
import org.apache.shardingsphere.infra.config.algorithm.AlgorithmConfiguration;
-import org.apache.shardingsphere.infra.executor.check.SQLChecker;
-import org.apache.shardingsphere.infra.executor.check.SQLCheckerFactory;
+import org.apache.shardingsphere.infra.executor.check.checker.SQLChecker;
+import org.apache.shardingsphere.infra.executor.check.checker.SQLCheckerFactory;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
import org.junit.Test;
@@ -78,8 +78,8 @@ public final class AuthorityCheckerTest {
SelectStatementContext selectStatementContext = mock(SelectStatementContext.class);
CreateTableStatementContext createTableStatementContext = mock(CreateTableStatementContext.class);
InsertStatementContext insertStatementContext = mock(InsertStatementContext.class);
- assertTrue(sqlChecker.check(selectStatementContext, Collections.emptyList(), new Grantee("root", "localhost"), "db0", Collections.emptyMap(), rule).isPassed());
- assertTrue(sqlChecker.check(insertStatementContext, Collections.emptyList(), new Grantee("root", "localhost"), "db0", Collections.emptyMap(), rule).isPassed());
- assertTrue(sqlChecker.check(createTableStatementContext, Collections.emptyList(), new Grantee("root", "localhost"), "db0", Collections.emptyMap(), rule).isPassed());
+ sqlChecker.check(selectStatementContext, Collections.emptyList(), new Grantee("root", "localhost"), "db0", Collections.emptyMap(), rule);
+ sqlChecker.check(insertStatementContext, Collections.emptyList(), new Grantee("root", "localhost"), "db0", Collections.emptyMap(), rule);
+ sqlChecker.check(createTableStatementContext, Collections.emptyList(), new Grantee("root", "localhost"), "db0", Collections.emptyMap(), rule);
}
}
diff --git a/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/DiscoveryDistSQLCommand.java b/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/DiscoveryDistSQLCommand.java
index 6e5d9588d57..546579c0210 100644
--- a/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/DiscoveryDistSQLCommand.java
+++ b/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/DiscoveryDistSQLCommand.java
@@ -32,7 +32,7 @@ public final class DiscoveryDistSQLCommand {
@XmlElement(name = "register-storage-unit")
@Getter
- private String registerStorageUnit;
+ private String registerStorageUnit;
@XmlElement(name = "create-discovery-rule")
@Getter
diff --git a/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/MGRPrimaryReplicaCommand.java b/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/MGRPrimaryReplicaCommand.java
index 7d8706786ed..fa68b610806 100644
--- a/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/MGRPrimaryReplicaCommand.java
+++ b/test/integration-test/discovery/src/test/java/org/apache/shardingsphere/test/integration/discovery/command/MGRPrimaryReplicaCommand.java
@@ -32,7 +32,7 @@ public final class MGRPrimaryReplicaCommand {
@XmlElement(name = "build-primary-node-sql")
@Getter
- private String buildPrimaryNodeSQL;
+ private String buildPrimaryNodeSQL;
@XmlElement(name = "build-replica-node-sql")
@Getter
diff --git a/test/integration-test/fixture/src/test/java/org/apache/shardingsphere/test/integration/fixture/ITShardingAuditAlgorithmFixture.java b/test/integration-test/fixture/src/test/java/org/apache/shardingsphere/test/integration/fixture/ITShardingAuditAlgorithmFixture.java
index aa875ecbb43..6e95b8245c4 100644
--- a/test/integration-test/fixture/src/test/java/org/apache/shardingsphere/test/integration/fixture/ITShardingAuditAlgorithmFixture.java
+++ b/test/integration-test/fixture/src/test/java/org/apache/shardingsphere/test/integration/fixture/ITShardingAuditAlgorithmFixture.java
@@ -18,7 +18,6 @@
package org.apache.shardingsphere.test.integration.fixture;
import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.check.SQLCheckResult;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.sharding.spi.ShardingAuditAlgorithm;
@@ -29,17 +28,16 @@ import java.util.Properties;
public final class ITShardingAuditAlgorithmFixture implements ShardingAuditAlgorithm {
@Override
- public Properties getProps() {
- return new Properties();
+ public void init(final Properties props) {
}
@Override
- public void init(final Properties props) {
+ public void check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final ShardingSphereDatabase database) {
}
@Override
- public SQLCheckResult check(final SQLStatementContext<?> sqlStatementContext, final List<Object> params, final Grantee grantee, final ShardingSphereDatabase database) {
- return new SQLCheckResult(true, "");
+ public Properties getProps() {
+ return new Properties();
}
@Override