Refactor GlobalRuleDefinitionExecutor.buildAlteredRuleConfiguration (a…

…pache#29897)
hunkier · Jan 29, 2024 · 734ecb0 · 734ecb0
1 parent 81c259a
commit 734ecb0
Show file tree

Hide file tree

Showing 15 changed files with 29 additions and 28 deletions.
diff --git a/...sphere/distsql/handler/type/rdl/rule/engine/global/GlobalRuleDefinitionExecuteEngine.java b/...sphere/distsql/handler/type/rdl/rule/engine/global/GlobalRuleDefinitionExecuteEngine.java
@@ -61,7 +61,7 @@ private RuleConfiguration findCurrentRuleConfiguration(final Collection<RuleConf
 
     @SuppressWarnings("unchecked")
     private RuleConfiguration processUpdate(final Collection<RuleConfiguration> ruleConfigs, final RuleDefinitionStatement sqlStatement, final RuleConfiguration currentRuleConfig) {
-        RuleConfiguration result = executor.buildAlteredRuleConfiguration(currentRuleConfig, sqlStatement);
+        RuleConfiguration result = executor.buildAlteredRuleConfiguration(sqlStatement, currentRuleConfig);
         ruleConfigs.remove(currentRuleConfig);
         ruleConfigs.add(result);
         return result;

diff --git a/.../distsql/handler/type/rdl/rule/engine/legacy/LegacyGlobalRuleDefinitionExecuteEngine.java b/.../distsql/handler/type/rdl/rule/engine/legacy/LegacyGlobalRuleDefinitionExecuteEngine.java
@@ -52,8 +52,8 @@ public void executeUpdate() {
         contextManager.getInstanceContext().getModeContextManager().alterGlobalRuleConfiguration(processUpdate(ruleConfigs, sqlStatement, executor, currentRuleConfig));
     }
 
-    private RuleConfiguration findCurrentRuleConfiguration(final Collection<RuleConfiguration> ruleConfigurations, final Class<? extends RuleConfiguration> ruleConfigClass) {
-        for (RuleConfiguration each : ruleConfigurations) {
+    private RuleConfiguration findCurrentRuleConfiguration(final Collection<RuleConfiguration> ruleConfigs, final Class<? extends RuleConfiguration> ruleConfigClass) {
+        for (RuleConfiguration each : ruleConfigs) {
             if (ruleConfigClass.isAssignableFrom(each.getClass())) {
                 return each;
             }
@@ -62,11 +62,11 @@ private RuleConfiguration findCurrentRuleConfiguration(final Collection<RuleConf
     }
 
     @SuppressWarnings({"rawtypes", "unchecked"})
-    private Collection<RuleConfiguration> processUpdate(final Collection<RuleConfiguration> ruleConfigurations,
+    private Collection<RuleConfiguration> processUpdate(final Collection<RuleConfiguration> ruleConfigs,
                                                         final RuleDefinitionStatement sqlStatement, final GlobalRuleDefinitionExecutor globalRuleUpdater, final RuleConfiguration currentRuleConfig) {
-        Collection<RuleConfiguration> result = new LinkedList<>(ruleConfigurations);
+        Collection<RuleConfiguration> result = new LinkedList<>(ruleConfigs);
         result.remove(currentRuleConfig);
-        result.add(globalRuleUpdater.buildAlteredRuleConfiguration(currentRuleConfig, sqlStatement));
+        result.add(globalRuleUpdater.buildAlteredRuleConfiguration(sqlStatement, currentRuleConfig));
         return result;
     }
 }
diff --git a/...shardingsphere/distsql/handler/type/rdl/rule/spi/global/GlobalRuleDefinitionExecutor.java b/...shardingsphere/distsql/handler/type/rdl/rule/spi/global/GlobalRuleDefinitionExecutor.java
@@ -46,7 +46,7 @@ public interface GlobalRuleDefinitionExecutor<T extends RuleDefinitionStatement,
      * @param sqlStatement SQL statement
      * @return built altered rule configuration
      */
-    RuleConfiguration buildAlteredRuleConfiguration(R currentRuleConfig, T sqlStatement);
+    RuleConfiguration buildAlteredRuleConfiguration(T sqlStatement, R currentRuleConfig);
 
     /**
      * Get rule configuration class.

diff --git a/...pache/shardingsphere/globalclock/distsql/handler/update/AlterGlobalClockRuleExecutor.java b/...pache/shardingsphere/globalclock/distsql/handler/update/AlterGlobalClockRuleExecutor.java
@@ -31,7 +31,7 @@ public void checkBeforeUpdate(final AlterGlobalClockRuleStatement sqlStatement,
     }
 
     @Override
-    public GlobalClockRuleConfiguration buildAlteredRuleConfiguration(final GlobalClockRuleConfiguration currentRuleConfig, final AlterGlobalClockRuleStatement sqlStatement) {
+    public GlobalClockRuleConfiguration buildAlteredRuleConfiguration(final AlterGlobalClockRuleStatement sqlStatement, final GlobalClockRuleConfiguration currentRuleConfig) {
         return new GlobalClockRuleConfiguration(sqlStatement.getType(), sqlStatement.getProvider(), sqlStatement.isEnabled(), sqlStatement.getProps());
     }
 

diff --git a/...e/shardingsphere/globalclock/distsql/handler/update/AlterGlobalClockRuleExecutorTest.java b/...e/shardingsphere/globalclock/distsql/handler/update/AlterGlobalClockRuleExecutorTest.java
@@ -34,7 +34,7 @@ class AlterGlobalClockRuleExecutorTest {
     void assertExecute() {
         AlterGlobalClockRuleExecutor executor = new AlterGlobalClockRuleExecutor();
         AlterGlobalClockRuleStatement sqlStatement = new AlterGlobalClockRuleStatement("TSO", "redis", Boolean.TRUE, PropertiesBuilder.build(new Property("host", "127.0.0.1")));
-        GlobalClockRuleConfiguration actual = executor.buildAlteredRuleConfiguration(getSQLParserRuleConfiguration(), sqlStatement);
+        GlobalClockRuleConfiguration actual = executor.buildAlteredRuleConfiguration(sqlStatement, getSQLParserRuleConfiguration());
         assertThat(actual.getType(), is("TSO"));
         assertThat(actual.getProvider(), is("redis"));
         assertTrue(actual.isEnabled());

diff --git a/...e/shardingsphere/sqlfederation/distsql/handler/update/AlterSQLFederationRuleExecutor.java b/...e/shardingsphere/sqlfederation/distsql/handler/update/AlterSQLFederationRuleExecutor.java
@@ -33,7 +33,7 @@ public void checkBeforeUpdate(final AlterSQLFederationRuleStatement sqlStatement
     }
 
     @Override
-    public SQLFederationRuleConfiguration buildAlteredRuleConfiguration(final SQLFederationRuleConfiguration currentRuleConfig, final AlterSQLFederationRuleStatement sqlStatement) {
+    public SQLFederationRuleConfiguration buildAlteredRuleConfiguration(final AlterSQLFederationRuleStatement sqlStatement, final SQLFederationRuleConfiguration currentRuleConfig) {
         boolean sqlFederationEnabled = null == sqlStatement.getSqlFederationEnabled() ? currentRuleConfig.isSqlFederationEnabled() : sqlStatement.getSqlFederationEnabled();
         boolean allQueryUseSQLFederation = null == sqlStatement.getAllQueryUseSQLFederation() ? currentRuleConfig.isAllQueryUseSQLFederation() : sqlStatement.getAllQueryUseSQLFederation();
         CacheOption executionPlanCache = null == sqlStatement.getExecutionPlanCache()

diff --git a/...ardingsphere/sqlfederation/distsql/handler/update/AlterSQLFederationRuleExecutorTest.java b/...ardingsphere/sqlfederation/distsql/handler/update/AlterSQLFederationRuleExecutorTest.java
@@ -33,7 +33,7 @@ class AlterSQLFederationRuleExecutorTest {
     void assertExecute() {
         AlterSQLFederationRuleExecutor executor = new AlterSQLFederationRuleExecutor();
         AlterSQLFederationRuleStatement sqlStatement = new AlterSQLFederationRuleStatement(true, true, new CacheOptionSegment(64, 512L));
-        SQLFederationRuleConfiguration actual = executor.buildAlteredRuleConfiguration(getSQLFederationRuleConfiguration(), sqlStatement);
+        SQLFederationRuleConfiguration actual = executor.buildAlteredRuleConfiguration(sqlStatement, getSQLFederationRuleConfiguration());
         assertTrue(actual.isSqlFederationEnabled());
         assertTrue(actual.isAllQueryUseSQLFederation());
         assertThat(actual.getExecutionPlanCache().getInitialCapacity(), is(64));

diff --git a/...a/org/apache/shardingsphere/parser/distsql/handler/update/AlterSQLParserRuleExecutor.java b/...a/org/apache/shardingsphere/parser/distsql/handler/update/AlterSQLParserRuleExecutor.java
@@ -33,7 +33,7 @@ public void checkBeforeUpdate(final AlterSQLParserRuleStatement sqlStatement, fi
     }
 
     @Override
-    public SQLParserRuleConfiguration buildAlteredRuleConfiguration(final SQLParserRuleConfiguration currentRuleConfig, final AlterSQLParserRuleStatement sqlStatement) {
+    public SQLParserRuleConfiguration buildAlteredRuleConfiguration(final AlterSQLParserRuleStatement sqlStatement, final SQLParserRuleConfiguration currentRuleConfig) {
         CacheOption parseTreeCache = null == sqlStatement.getParseTreeCache()
                 ? currentRuleConfig.getParseTreeCache()
                 : createCacheOption(currentRuleConfig.getParseTreeCache(), sqlStatement.getParseTreeCache());

diff --git a/...g/apache/shardingsphere/parser/distsql/handler/update/AlterSQLParserRuleExecutorTest.java b/...g/apache/shardingsphere/parser/distsql/handler/update/AlterSQLParserRuleExecutorTest.java
@@ -32,7 +32,7 @@ class AlterSQLParserRuleExecutorTest {
     void assertExecute() {
         AlterSQLParserRuleExecutor executor = new AlterSQLParserRuleExecutor();
         AlterSQLParserRuleStatement sqlStatement = new AlterSQLParserRuleStatement(new CacheOptionSegment(64, 512L), new CacheOptionSegment(1000, 1000L));
-        SQLParserRuleConfiguration actual = executor.buildAlteredRuleConfiguration(getSQLParserRuleConfiguration(), sqlStatement);
+        SQLParserRuleConfiguration actual = executor.buildAlteredRuleConfiguration(sqlStatement, getSQLParserRuleConfiguration());
         assertThat(actual.getSqlStatementCache().getInitialCapacity(), is(1000));
         assertThat(actual.getSqlStatementCache().getMaximumSize(), is(1000L));
         assertThat(actual.getParseTreeCache().getInitialCapacity(), is(64));

diff --git a/...e/shardingsphere/sqltranslator/distsql/handler/update/AlterSQLTranslatorRuleExecutor.java b/...e/shardingsphere/sqltranslator/distsql/handler/update/AlterSQLTranslatorRuleExecutor.java
@@ -34,7 +34,7 @@ public void checkBeforeUpdate(final AlterSQLTranslatorRuleStatement sqlStatement
     }
 
     @Override
-    public SQLTranslatorRuleConfiguration buildAlteredRuleConfiguration(final SQLTranslatorRuleConfiguration currentRuleConfig, final AlterSQLTranslatorRuleStatement sqlStatement) {
+    public SQLTranslatorRuleConfiguration buildAlteredRuleConfiguration(final AlterSQLTranslatorRuleStatement sqlStatement, final SQLTranslatorRuleConfiguration currentRuleConfig) {
         return new SQLTranslatorRuleConfiguration(sqlStatement.getProvider().getName(), sqlStatement.getProvider().getProps(),
                 null == sqlStatement.getUseOriginalSQLWhenTranslatingFailed() ? currentRuleConfig.isUseOriginalSQLWhenTranslatingFailed() : sqlStatement.getUseOriginalSQLWhenTranslatingFailed());
     }

diff --git a/...ardingsphere/sqltranslator/distsql/handler/update/AlterSQLTranslatorRuleExecutorTest.java b/...ardingsphere/sqltranslator/distsql/handler/update/AlterSQLTranslatorRuleExecutorTest.java
@@ -37,8 +37,8 @@ class AlterSQLTranslatorRuleExecutorTest {
     @Test
     void assertExecute() {
         AlterSQLTranslatorRuleExecutor executor = new AlterSQLTranslatorRuleExecutor();
-        SQLTranslatorRuleConfiguration actual = executor.buildAlteredRuleConfiguration(createSQLTranslatorRuleConfiguration(),
-                new AlterSQLTranslatorRuleStatement(new AlgorithmSegment("JOOQ", PropertiesBuilder.build(new Property("foo", "bar"))), null));
+        SQLTranslatorRuleConfiguration actual = executor.buildAlteredRuleConfiguration(
+                new AlterSQLTranslatorRuleStatement(new AlgorithmSegment("JOOQ", PropertiesBuilder.build(new Property("foo", "bar"))), null), createSQLTranslatorRuleConfiguration());
         assertThat(actual.getType(), is("JOOQ"));
         assertFalse(actual.getProps().isEmpty());
         String props = PropertiesConverter.convert(actual.getProps());

diff --git a/...va/org/apache/shardingsphere/traffic/distsql/handler/update/AlterTrafficRuleExecutor.java b/...va/org/apache/shardingsphere/traffic/distsql/handler/update/AlterTrafficRuleExecutor.java
@@ -43,16 +43,16 @@ public final class AlterTrafficRuleExecutor implements GlobalRuleDefinitionExecu
 
     @Override
     public void checkBeforeUpdate(final AlterTrafficRuleStatement sqlStatement, final TrafficRuleConfiguration currentRuleConfig) {
-        checkRuleNames(currentRuleConfig, sqlStatement);
+        checkRuleNames(sqlStatement, currentRuleConfig);
         checkAlgorithmNames(sqlStatement);
     }
 
-    private void checkRuleNames(final TrafficRuleConfiguration currentRuleConfig, final AlterTrafficRuleStatement sqlStatement) {
-        Collection<String> notExistRuleNames = getNotExistRuleNames(currentRuleConfig, sqlStatement);
+    private void checkRuleNames(final AlterTrafficRuleStatement sqlStatement, final TrafficRuleConfiguration currentRuleConfig) {
+        Collection<String> notExistRuleNames = getNotExistRuleNames(sqlStatement, currentRuleConfig);
         ShardingSpherePreconditions.checkState(notExistRuleNames.isEmpty(), () -> new MissingRequiredRuleException("Traffic", notExistRuleNames));
     }
 
-    private Collection<String> getNotExistRuleNames(final TrafficRuleConfiguration currentRuleConfig, final AlterTrafficRuleStatement sqlStatement) {
+    private Collection<String> getNotExistRuleNames(final AlterTrafficRuleStatement sqlStatement, final TrafficRuleConfiguration currentRuleConfig) {
         Collection<String> currentRuleNames = currentRuleConfig.getTrafficStrategies().stream().map(TrafficStrategyConfiguration::getName).collect(Collectors.toSet());
         return sqlStatement.getSegments().stream().map(TrafficRuleSegment::getName).filter(each -> !currentRuleNames.contains(each)).collect(Collectors.toSet());
     }
@@ -67,7 +67,7 @@ private void checkAlgorithmNames(final AlterTrafficRuleStatement sqlStatement) {
     }
 
     @Override
-    public TrafficRuleConfiguration buildAlteredRuleConfiguration(final TrafficRuleConfiguration currentRuleConfig, final AlterTrafficRuleStatement sqlStatement) {
+    public TrafficRuleConfiguration buildAlteredRuleConfiguration(final AlterTrafficRuleStatement sqlStatement, final TrafficRuleConfiguration currentRuleConfig) {
         TrafficRuleConfiguration result = new TrafficRuleConfiguration();
         TrafficRuleConfiguration configFromSQLStatement = TrafficRuleConverter.convert(sqlStatement.getSegments());
         result.getTrafficStrategies().addAll(createToBeAlteredStrategyConfigurations(currentRuleConfig, configFromSQLStatement));

diff --git a/...rg/apache/shardingsphere/traffic/distsql/handler/update/AlterTrafficRuleExecutorTest.java b/...rg/apache/shardingsphere/traffic/distsql/handler/update/AlterTrafficRuleExecutorTest.java
@@ -63,7 +63,7 @@ void assertExecuteWithLoadBalancerCannotBeNull() {
         TrafficRuleSegment trafficRuleSegment = new TrafficRuleSegment("rule_name_1", Arrays.asList("olap", "order_by"),
                 new AlgorithmSegment("DISTSQL.FIXTURE", new Properties()), null);
         AlterTrafficRuleExecutor executor = new AlterTrafficRuleExecutor();
-        TrafficRuleConfiguration actual = executor.buildAlteredRuleConfiguration(createTrafficRuleConfiguration(), new AlterTrafficRuleStatement(Collections.singleton(trafficRuleSegment)));
+        TrafficRuleConfiguration actual = executor.buildAlteredRuleConfiguration(new AlterTrafficRuleStatement(Collections.singleton(trafficRuleSegment)), createTrafficRuleConfiguration());
         assertThat(actual.getTrafficStrategies().size(), is(2));
         assertThat(actual.getTrafficAlgorithms().size(), is(2));
         assertThat(actual.getLoadBalancers().size(), is(1));
@@ -77,7 +77,7 @@ void assertExecute() {
                 "rule_name_2", Collections.emptyList(), new AlgorithmSegment("DISTSQL.FIXTURE", new Properties()), new AlgorithmSegment("DISTSQL.FIXTURE", new Properties()));
         AlterTrafficRuleExecutor executor = new AlterTrafficRuleExecutor();
         TrafficRuleConfiguration actual =
-                executor.buildAlteredRuleConfiguration(createTrafficRuleConfiguration(), new AlterTrafficRuleStatement(Arrays.asList(trafficRuleSegment1, trafficRuleSegment2)));
+                executor.buildAlteredRuleConfiguration(new AlterTrafficRuleStatement(Arrays.asList(trafficRuleSegment1, trafficRuleSegment2)), createTrafficRuleConfiguration());
         assertThat(actual.getTrafficStrategies().size(), is(2));
         assertThat(actual.getTrafficAlgorithms().size(), is(2));
         assertThat(actual.getLoadBalancers().size(), is(2));

diff --git a/...pache/shardingsphere/transaction/distsql/handler/update/AlterTransactionRuleExecutor.java b/...pache/shardingsphere/transaction/distsql/handler/update/AlterTransactionRuleExecutor.java
@@ -69,7 +69,7 @@ private void checkTransactionManagerProviderType(final ShardingSphereTransaction
     }
 
     @Override
-    public TransactionRuleConfiguration buildAlteredRuleConfiguration(final TransactionRuleConfiguration currentRuleConfig, final AlterTransactionRuleStatement sqlStatement) {
+    public TransactionRuleConfiguration buildAlteredRuleConfiguration(final AlterTransactionRuleStatement sqlStatement, final TransactionRuleConfiguration currentRuleConfig) {
         return new TransactionRuleConfiguration(sqlStatement.getDefaultType(), sqlStatement.getProvider().getProviderType(), sqlStatement.getProvider().getProps());
     }
 

diff --git a/...e/shardingsphere/transaction/distsql/handler/update/AlterTransactionRuleExecutorTest.java b/...e/shardingsphere/transaction/distsql/handler/update/AlterTransactionRuleExecutorTest.java
@@ -48,8 +48,9 @@ class AlterTransactionRuleExecutorTest {
     void assertExecuteWithXA() {
         when(ShardingSphereServiceLoader.getServiceInstances(ShardingSphereTransactionManager.class)).thenReturn(Collections.singleton(new ShardingSphereTransactionManagerFixture()));
         AlterTransactionRuleExecutor executor = new AlterTransactionRuleExecutor();
-        TransactionRuleConfiguration actual = executor.buildAlteredRuleConfiguration(createTransactionRuleConfiguration(), new AlterTransactionRuleStatement("XA",
-                new TransactionProviderSegment("Atomikos", PropertiesBuilder.build(new Property("host", "127.0.0.1"), new Property("databaseName", "jbossts")))));
+        AlterTransactionRuleStatement sqlStatement = new AlterTransactionRuleStatement(
+                "XA", new TransactionProviderSegment("Atomikos", PropertiesBuilder.build(new Property("host", "127.0.0.1"), new Property("databaseName", "jbossts"))));
+        TransactionRuleConfiguration actual = executor.buildAlteredRuleConfiguration(sqlStatement, createTransactionRuleConfiguration());
         assertThat(actual.getDefaultType(), is("XA"));
         assertThat(actual.getProviderType(), is("Atomikos"));
         assertFalse(actual.getProps().isEmpty());
@@ -61,8 +62,8 @@ void assertExecuteWithXA() {
     @Test
     void assertExecuteWithLocal() {
         AlterTransactionRuleExecutor executor = new AlterTransactionRuleExecutor();
-        TransactionRuleConfiguration actual =
-                executor.buildAlteredRuleConfiguration(createTransactionRuleConfiguration(), new AlterTransactionRuleStatement("LOCAL", new TransactionProviderSegment("", new Properties())));
+        TransactionRuleConfiguration actual = executor.buildAlteredRuleConfiguration(
+                new AlterTransactionRuleStatement("LOCAL", new TransactionProviderSegment("", new Properties())), createTransactionRuleConfiguration());
         assertThat(actual.getDefaultType(), is("LOCAL"));
         assertThat(actual.getProviderType(), is(""));
     }