This is an automated email from the ASF dual-hosted git repository.

zhangliang 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 c923b467649 Refactor RuleItemChangedBuilder (#34567)
c923b467649 is described below

commit c923b4676496c85e0eb86c2e13e7b0894d8aaf74
Author: Liang Zhang <zhangli...@apache.org>
AuthorDate: Thu Feb 6 15:27:51 2025 +0800

    Refactor RuleItemChangedBuilder (#34567)
    
    * Refactor RuleItemChangedBuilder
    
    * Refactor RuleItemChangedBuilder
---
 .../standalone/changed/RuleItemChangedBuilder.java | 54 +++++++++++++---------
 .../StandaloneMetaDataManagerPersistService.java   |  8 +---
 ...tandaloneMetaDataManagerPersistServiceTest.java |  5 +-
 3 files changed, 36 insertions(+), 31 deletions(-)

diff --git 
a/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/changed/RuleItemChangedBuilder.java
 
b/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/changed/RuleItemChangedBuilder.java
index f6138390ad9..1ab454f16ea 100644
--- 
a/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/changed/RuleItemChangedBuilder.java
+++ 
b/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/changed/RuleItemChangedBuilder.java
@@ -46,9 +46,12 @@ public final class RuleItemChangedBuilder {
      * @param changedType data changed type
      * @return built rule item
      */
-    public Optional<RuleChangedItem> build(final String databaseName, final 
String activeVersionKey, final Integer activeVersion, final Type changedType) {
+    public Optional<RuleChangedItem> build(final String databaseName, final 
String activeVersionKey, final int activeVersion, final Type changedType) {
         for (RuleNodePathProvider each : 
ShardingSphereServiceLoader.getServiceInstances(RuleNodePathProvider.class)) {
-            Optional<RuleChangedItem> result = build(each.getRuleNodePath(), 
databaseName, activeVersionKey, activeVersion, changedType);
+            if 
(!each.getRuleNodePath().getRoot().isValidatedPath(activeVersionKey)) {
+                continue;
+            }
+            Optional<RuleChangedItem> result = build(databaseName, 
activeVersionKey, activeVersion, changedType, each);
             if (result.isPresent()) {
                 return result;
             }
@@ -56,38 +59,43 @@ public final class RuleItemChangedBuilder {
         return Optional.empty();
     }
     
-    private Optional<RuleChangedItem> build(final RuleNodePath ruleNodePath, 
final String databaseName, final String activeVersionKey, final Integer 
activeVersion, final Type changedType) {
-        if (!ruleNodePath.getRoot().isValidatedPath(activeVersionKey) || 
Type.DELETED != changedType && null == activeVersion) {
-            return Optional.empty();
+    private Optional<RuleChangedItem> build(final String databaseName, final 
String activeVersionKey, final int activeVersion, final Type changedType, final 
RuleNodePathProvider pathProvider) {
+        if (Type.UPDATED == changedType) {
+            return buildAlterItem(pathProvider.getRuleNodePath(), 
databaseName, activeVersionKey, activeVersion);
+        }
+        if (Type.DELETED == changedType) {
+            return buildDropItem(pathProvider.getRuleNodePath(), databaseName, 
activeVersionKey);
         }
+        return Optional.empty();
+    }
+    
+    private Optional<RuleChangedItem> buildAlterItem(final RuleNodePath 
ruleNodePath, final String databaseName, final String activeVersionKey, final 
int activeVersion) {
         for (Entry<String, NamedRuleItemNodePath> entry : 
ruleNodePath.getNamedItems().entrySet()) {
-            Optional<String> itemName;
-            if (Type.ADDED == changedType || Type.UPDATED == changedType) {
-                itemName = 
entry.getValue().getNameByActiveVersion(activeVersionKey);
-            } else {
-                itemName = 
entry.getValue().getNameByItemPath(activeVersionKey);
-            }
+            Optional<String> itemName = 
entry.getValue().getNameByActiveVersion(activeVersionKey);
             if (itemName.isPresent()) {
-                return Optional.of(create(databaseName, itemName.get(), 
activeVersionKey, activeVersion, changedType, 
ruleNodePath.getRoot().getRuleType() + "." + entry.getKey()));
+                return Optional.of(new AlterNamedRuleItem(databaseName, 
itemName.get(), activeVersionKey, activeVersion, 
ruleNodePath.getRoot().getRuleType() + "." + entry.getKey()));
             }
         }
         for (Entry<String, UniqueRuleItemNodePath> entry : 
ruleNodePath.getUniqueItems().entrySet()) {
             if (entry.getValue().isActiveVersionPath(activeVersionKey)) {
-                return Optional.of(create(databaseName, activeVersionKey, 
activeVersion, changedType, ruleNodePath.getRoot().getRuleType() + "." + 
entry.getKey()));
+                return Optional.of(new AlterUniqueRuleItem(databaseName, 
activeVersionKey, activeVersion, ruleNodePath.getRoot().getRuleType() + "." + 
entry.getKey()));
             }
         }
         return Optional.empty();
     }
     
-    private RuleChangedItem create(final String databaseName, final String 
itemName, final String activeVersionKey, final Integer activeVersion, final 
Type changedType, final String type) {
-        return Type.ADDED == changedType || Type.UPDATED == changedType
-                ? new AlterNamedRuleItem(databaseName, itemName, 
activeVersionKey, activeVersion, type)
-                : new DropNamedRuleItem(databaseName, itemName, type);
-    }
-    
-    private RuleChangedItem create(final String databaseName, final String 
activeVersionKey, final Integer activeVersion, final Type changedType, final 
String type) {
-        return Type.ADDED == changedType || Type.UPDATED == changedType
-                ? new AlterUniqueRuleItem(databaseName, activeVersionKey, 
activeVersion, type)
-                : new DropUniqueRuleItem(databaseName, type);
+    private Optional<RuleChangedItem> buildDropItem(final RuleNodePath 
ruleNodePath, final String databaseName, final String activeVersionKey) {
+        for (Entry<String, NamedRuleItemNodePath> entry : 
ruleNodePath.getNamedItems().entrySet()) {
+            Optional<String> itemName = 
entry.getValue().getNameByItemPath(activeVersionKey);
+            if (itemName.isPresent()) {
+                return Optional.of(new DropNamedRuleItem(databaseName, 
itemName.get(), ruleNodePath.getRoot().getRuleType() + "." + entry.getKey()));
+            }
+        }
+        for (Entry<String, UniqueRuleItemNodePath> entry : 
ruleNodePath.getUniqueItems().entrySet()) {
+            if (entry.getValue().isActiveVersionPath(activeVersionKey)) {
+                return Optional.of(new DropUniqueRuleItem(databaseName, 
ruleNodePath.getRoot().getRuleType() + "." + entry.getKey()));
+            }
+        }
+        return Optional.empty();
     }
 }
diff --git 
a/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistService.java
 
b/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistService.java
index 2af668a21fc..9e4230e840f 100644
--- 
a/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistService.java
+++ 
b/mode/type/standalone/core/src/main/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistService.java
@@ -213,7 +213,7 @@ public final class StandaloneMetaDataManagerPersistService 
implements MetaDataMa
         
metaDataPersistFacade.getMetaDataVersionService().switchActiveVersion(metaDataVersions);
         for (MetaDataVersion each : metaDataVersions) {
             // TODO double check here, when ruleItemEvent not existed or not 
AlterRuleItemEvent @haoran
-            Optional<RuleChangedItem> ruleItemChanged = 
buildRuleChangedItem(databaseName, each, Type.UPDATED);
+            Optional<RuleChangedItem> ruleItemChanged = 
ruleItemChangedBuilder.build(databaseName, each.getActiveVersionNodePath(), 
each.getNextActiveVersion(), Type.UPDATED);
             if (ruleItemChanged.isPresent() && ruleItemChanged.get() 
instanceof AlterRuleItem) {
                 
metaDataContextManager.getDatabaseRuleItemManager().alter((AlterRuleItem) 
ruleItemChanged.get());
             }
@@ -221,10 +221,6 @@ public final class StandaloneMetaDataManagerPersistService 
implements MetaDataMa
         clearServiceCache();
     }
     
-    private Optional<RuleChangedItem> buildRuleChangedItem(final String 
databaseName, final MetaDataVersion metaDataVersion, final Type type) {
-        return ruleItemChangedBuilder.build(databaseName, 
metaDataVersion.getActiveVersionNodePath(), 
metaDataVersion.getNextActiveVersion(), type);
-    }
-    
     @Override
     public void removeRuleConfigurationItem(final String databaseName, final 
RuleConfiguration toBeRemovedRuleConfig) throws SQLException {
         if (null == toBeRemovedRuleConfig) {
@@ -232,7 +228,7 @@ public final class StandaloneMetaDataManagerPersistService 
implements MetaDataMa
         }
         Collection<MetaDataVersion> metaDataVersions = 
metaDataPersistFacade.getDatabaseRuleService().delete(databaseName, 
Collections.singleton(toBeRemovedRuleConfig));
         for (MetaDataVersion each : metaDataVersions) {
-            Optional<RuleChangedItem> ruleItemChanged = 
buildRuleChangedItem(databaseName, each, Type.DELETED);
+            Optional<RuleChangedItem> ruleItemChanged = 
ruleItemChangedBuilder.build(databaseName, each.getActiveVersionNodePath(), 
each.getNextActiveVersion(), Type.DELETED);
             // TODO double check here, when ruleItemEvent not existed or not 
AlterRuleItemEvent @haoran
             if (ruleItemChanged.isPresent() && ruleItemChanged.get() 
instanceof DropRuleItem) {
                 
metaDataContextManager.getDatabaseRuleItemManager().drop((DropRuleItem) 
ruleItemChanged.get());
diff --git 
a/mode/type/standalone/core/src/test/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistServiceTest.java
 
b/mode/type/standalone/core/src/test/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistServiceTest.java
index cd94a06b081..ef0b2e58b74 100644
--- 
a/mode/type/standalone/core/src/test/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistServiceTest.java
+++ 
b/mode/type/standalone/core/src/test/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistServiceTest.java
@@ -52,6 +52,7 @@ import java.util.Properties;
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyCollection;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.mock;
@@ -174,7 +175,7 @@ class StandaloneMetaDataManagerPersistServiceTest {
         when(metaDataPersistFacade.getDatabaseRuleService().persist("foo_db", 
Collections.singleton(ruleConfig))).thenReturn(metaDataVersion);
         AlterRuleItem alterRuleItem = mock(AlterRuleItem.class);
         RuleItemChangedBuilder ruleItemChangedBuilder = 
mock(RuleItemChangedBuilder.class);
-        when(ruleItemChangedBuilder.build(eq("foo_db"), any(), any(), 
any())).thenReturn(Optional.of(alterRuleItem));
+        when(ruleItemChangedBuilder.build(eq("foo_db"), any(), anyInt(), 
any())).thenReturn(Optional.of(alterRuleItem));
         setRuleConfigurationEventBuilder(ruleItemChangedBuilder);
         metaDataManagerPersistService.alterRuleConfiguration("foo_db", 
ruleConfig);
         
verify(metaDataPersistFacade.getMetaDataVersionService()).switchActiveVersion(metaDataVersion);
@@ -194,7 +195,7 @@ class StandaloneMetaDataManagerPersistServiceTest {
         when(metaDataPersistFacade.getDatabaseRuleService().delete("foo_db", 
Collections.singleton(ruleConfig))).thenReturn(metaDataVersion);
         RuleItemChangedBuilder ruleItemChangedBuilder = 
mock(RuleItemChangedBuilder.class);
         DropRuleItem dropRuleItem = mock(DropRuleItem.class);
-        when(ruleItemChangedBuilder.build(eq("foo_db"), any(), any(), 
any())).thenReturn(Optional.of(dropRuleItem));
+        when(ruleItemChangedBuilder.build(eq("foo_db"), any(), anyInt(), 
any())).thenReturn(Optional.of(dropRuleItem));
         setRuleConfigurationEventBuilder(ruleItemChangedBuilder);
         metaDataManagerPersistService.removeRuleConfigurationItem("foo_db", 
ruleConfig);
         
verify(metaDataContextManager.getDatabaseRuleItemManager()).drop(any(DropRuleItem.class));

Reply via email to