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 5c8e40d7ec4 Validate rule configurations before persistence (#39888)
5c8e40d7ec4 is described below
commit 5c8e40d7ec45ef1ab5f619e523d71ed4782cec69
Author: Liang Zhang <[email protected]>
AuthorDate: Fri Sep 18 20:14:43 2026 +0800
Validate rule configurations before persistence (#39888)
* Add @SingletonSPI on MCPRuleDistSQLStatementClassifier
* Validate rule configurations before persistence
---
.../rule/builder/global/GlobalRulesBuilderTest.java | 6 ++----
.../config/database/DatabaseRulePersistService.java | 7 +++----
.../config/global/GlobalRulePersistService.java | 2 ++
.../database/DatabaseRulePersistServiceTest.java | 20 ++++++++++++++++++++
.../config/global/GlobalRulePersistServiceTest.java | 21 +++++++++++++++++++++
5 files changed, 48 insertions(+), 8 deletions(-)
diff --git
a/infra/common/src/test/java/org/apache/shardingsphere/infra/rule/builder/global/GlobalRulesBuilderTest.java
b/infra/common/src/test/java/org/apache/shardingsphere/infra/rule/builder/global/GlobalRulesBuilderTest.java
index 8ccf0326eea..5a447093eec 100644
---
a/infra/common/src/test/java/org/apache/shardingsphere/infra/rule/builder/global/GlobalRulesBuilderTest.java
+++
b/infra/common/src/test/java/org/apache/shardingsphere/infra/rule/builder/global/GlobalRulesBuilderTest.java
@@ -51,8 +51,7 @@ class GlobalRulesBuilderTest {
void assertBuildRulesWithInvalidRuleConfiguration() {
FixtureGlobalRuleConfiguration ruleConfig = new
FixtureGlobalRuleConfiguration();
ruleConfig.setName("");
- assertThrows(InvalidRuleConfigurationException.class,
- () ->
GlobalRulesBuilder.buildRules(Collections.singletonList(ruleConfig),
Collections.singleton(buildDatabase()), mock()));
+ assertThrows(InvalidRuleConfigurationException.class, () ->
GlobalRulesBuilder.buildRules(Collections.singletonList(ruleConfig),
Collections.singleton(buildDatabase()), mock()));
}
@Test
@@ -65,8 +64,7 @@ class GlobalRulesBuilderTest {
void assertBuildSingleRulesWithInvalidRuleConfiguration() {
FixtureGlobalRuleConfiguration ruleConfig = new
FixtureGlobalRuleConfiguration();
ruleConfig.setName("");
- assertThrows(InvalidRuleConfigurationException.class,
- () -> GlobalRulesBuilder.buildSingleRules(ruleConfig,
Collections.singleton(buildDatabase()), mock()));
+ assertThrows(InvalidRuleConfigurationException.class, () ->
GlobalRulesBuilder.buildSingleRules(ruleConfig,
Collections.singleton(buildDatabase()), mock()));
}
private ShardingSphereDatabase buildDatabase() {
diff --git
a/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistService.java
b/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistService.java
index 36c87bdd311..bddcdb11dcd 100644
---
a/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistService.java
+++
b/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistService.java
@@ -18,6 +18,7 @@
package org.apache.shardingsphere.mode.metadata.persist.config.database;
import org.apache.shardingsphere.infra.config.rule.RuleConfiguration;
+import
org.apache.shardingsphere.infra.config.rule.validator.RuleConfigurationValidator;
import
org.apache.shardingsphere.infra.yaml.config.pojo.rule.YamlRuleConfiguration;
import
org.apache.shardingsphere.infra.yaml.config.swapper.rule.YamlRuleConfigurationSwapperEngine;
import
org.apache.shardingsphere.mode.metadata.persist.version.VersionPersistService;
@@ -36,7 +37,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
-import java.util.Objects;
import java.util.stream.Collectors;
/**
@@ -79,9 +79,7 @@ public final class DatabaseRulePersistService {
Collection<DatabaseRuleNodePath> nodePaths = new LinkedList<>();
nodePaths.addAll(getUniqueItemNodePaths(databaseName,
databaseRuleNode.getRuleType(), databaseRuleNode.getUniqueItems()));
nodePaths.addAll(getNamedItemNodePaths(databaseName,
databaseRuleNode.getRuleType(), databaseRuleNode.getNamedItems()));
- return nodePaths.stream()
- .map(each -> new RuleNodeTuple(each,
versionPersistService.loadContent(new
VersionNodePath(each)))).filter(Objects::nonNull)
- .collect(Collectors.toList());
+ return nodePaths.stream().map(each -> new RuleNodeTuple(each,
versionPersistService.loadContent(new
VersionNodePath(each)))).collect(Collectors.toList());
}
private Collection<DatabaseRuleNodePath> getUniqueItemNodePaths(final
String databaseName, final String ruleType, final Collection<String>
uniqueItems) {
@@ -105,6 +103,7 @@ public final class DatabaseRulePersistService {
* @return meta data versions
*/
public Collection<MetaDataVersion> persist(final String databaseName,
final Collection<RuleConfiguration> configs) {
+ RuleConfigurationValidator.validate(configs);
Collection<MetaDataVersion> result = new LinkedList<>();
for (YamlRuleConfiguration each :
yamlSwapperEngine.swapToYamlRuleConfigurations(configs)) {
result.addAll(persistTuples(tupleSwapperEngine.swapToTuples(databaseName,
each)));
diff --git
a/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistService.java
b/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistService.java
index 1713863d83b..c2386a7083f 100644
---
a/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistService.java
+++
b/mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistService.java
@@ -18,6 +18,7 @@
package org.apache.shardingsphere.mode.metadata.persist.config.global;
import org.apache.shardingsphere.infra.config.rule.RuleConfiguration;
+import
org.apache.shardingsphere.infra.config.rule.validator.RuleConfigurationValidator;
import
org.apache.shardingsphere.infra.yaml.config.pojo.rule.YamlGlobalRuleConfiguration;
import
org.apache.shardingsphere.infra.yaml.config.pojo.rule.YamlRuleConfiguration;
import
org.apache.shardingsphere.infra.yaml.config.swapper.rule.YamlRuleConfigurationSwapperEngine;
@@ -78,6 +79,7 @@ public final class GlobalRulePersistService {
* @param globalRuleConfigs global rule configurations
*/
public void persist(final Collection<RuleConfiguration> globalRuleConfigs)
{
+ RuleConfigurationValidator.validate(globalRuleConfigs);
for (YamlRuleConfiguration each :
yamlSwapperEngine.swapToYamlRuleConfigurations(globalRuleConfigs)) {
persistTuple(tupleSwapperEngine.swapToTuple((YamlGlobalRuleConfiguration)
each));
}
diff --git
a/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistServiceTest.java
b/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistServiceTest.java
index 5e3efb09e48..6214a4eed8e 100644
---
a/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistServiceTest.java
+++
b/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/database/DatabaseRulePersistServiceTest.java
@@ -17,7 +17,9 @@
package org.apache.shardingsphere.mode.metadata.persist.config.database;
+import lombok.Getter;
import org.apache.shardingsphere.infra.config.rule.RuleConfiguration;
+import
org.apache.shardingsphere.infra.exception.kernel.metadata.rule.InvalidRuleConfigurationException;
import org.apache.shardingsphere.mode.node.path.version.MetaDataVersion;
import org.apache.shardingsphere.mode.spi.repository.PersistRepository;
import
org.apache.shardingsphere.test.infra.fixture.rule.MockedRuleConfiguration;
@@ -27,13 +29,17 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
+import javax.validation.constraints.NotBlank;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
@@ -85,6 +91,13 @@ class DatabaseRulePersistServiceTest {
assertThat(actual.iterator().next().getActiveVersion(), is(11));
}
+ @Test
+ void assertPersistWithInvalidRuleConfiguration() {
+ Collection<RuleConfiguration> ruleConfigs = Arrays.asList(new
MockedRuleConfiguration("foo_value"), new InvalidRuleConfiguration());
+ assertThrows(InvalidRuleConfigurationException.class, () ->
persistService.persist("foo_db", ruleConfigs));
+ verifyNoInteractions(repository);
+ }
+
@Test
void assertDeleteWithRuleType() {
persistService.delete("foo_db", "foo_rule");
@@ -97,4 +110,11 @@ class DatabaseRulePersistServiceTest {
assertThat(actual.size(), is(1));
assertThat(actual.iterator().next().getActiveVersion(), is(0));
}
+
+ @Getter
+ private static final class InvalidRuleConfiguration implements
RuleConfiguration {
+
+ @NotBlank
+ private final String name = "";
+ }
}
diff --git
a/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistServiceTest.java
b/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistServiceTest.java
index 3b4bb8f77bd..3357d09836b 100644
---
a/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistServiceTest.java
+++
b/mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/persist/config/global/GlobalRulePersistServiceTest.java
@@ -17,7 +17,10 @@
package org.apache.shardingsphere.mode.metadata.persist.config.global;
+import lombok.Getter;
import org.apache.shardingsphere.infra.config.rule.RuleConfiguration;
+import
org.apache.shardingsphere.infra.config.rule.scope.GlobalRuleConfiguration;
+import
org.apache.shardingsphere.infra.exception.kernel.metadata.rule.InvalidRuleConfigurationException;
import
org.apache.shardingsphere.mode.metadata.persist.version.VersionPersistService;
import org.apache.shardingsphere.mode.spi.repository.PersistRepository;
import
org.apache.shardingsphere.test.infra.fixture.rule.global.MockedGlobalRuleConfiguration;
@@ -27,7 +30,10 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
+import javax.validation.constraints.NotBlank;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
@@ -36,6 +42,7 @@ import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
@@ -89,4 +96,18 @@ class GlobalRulePersistServiceTest {
verify(repository).persist("/rules/global_fixture/versions/0", "name:
foo_value" + System.lineSeparator());
verify(repository).persist("/rules/global_fixture/active_version",
"0");
}
+
+ @Test
+ void assertPersistWithInvalidRuleConfiguration() {
+ Collection<RuleConfiguration> ruleConfigs = Arrays.asList(new
MockedGlobalRuleConfiguration("foo_value"), new
InvalidGlobalRuleConfiguration());
+ assertThrows(InvalidRuleConfigurationException.class, () ->
globalRulePersistService.persist(ruleConfigs));
+ verifyNoInteractions(repository);
+ }
+
+ @Getter
+ private static final class InvalidGlobalRuleConfiguration implements
GlobalRuleConfiguration {
+
+ @NotBlank
+ private final String name = "";
+ }
}
