Add more test cases on StatePersistService (apache#32911)

terrymanu · Sep 17, 2024 · 2df99bf · 2df99bf
1 parent 7f1b9c6
commit 2df99bf
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 20 deletions.
diff --git a/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/ContextManager.java b/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/ContextManager.java
@@ -33,7 +33,6 @@
 import org.apache.shardingsphere.infra.metadata.database.schema.builder.GenericSchemaBuilderMaterial;
 import org.apache.shardingsphere.infra.metadata.database.schema.model.ShardingSphereSchema;
 import org.apache.shardingsphere.infra.spi.ShardingSphereServiceLoader;
-import org.apache.shardingsphere.infra.state.cluster.ClusterState;
 import org.apache.shardingsphere.mode.manager.listener.ContextManagerLifecycleListener;
 import org.apache.shardingsphere.mode.metadata.MetaDataContextManager;
 import org.apache.shardingsphere.mode.metadata.MetaDataContexts;
@@ -70,7 +69,7 @@ public ContextManager(final MetaDataContexts metaDataContexts, final ComputeNode
         this.computeNodeInstanceContext = computeNodeInstanceContext;
         metaDataContextManager = new MetaDataContextManager(this.metaDataContexts, computeNodeInstanceContext, repository);
         persistServiceFacade = new PersistServiceFacade(repository, computeNodeInstanceContext.getModeConfiguration(), metaDataContextManager);
-        stateContext = new StateContext(persistServiceFacade.getStatePersistService().loadClusterState().orElse(ClusterState.OK));
+        stateContext = new StateContext(persistServiceFacade.getStatePersistService().load());
         executorEngine = ExecutorEngine.createExecutorEngineWithSize(metaDataContexts.getMetaData().getProps().<Integer>getValue(ConfigurationPropertyKey.KERNEL_EXECUTOR_SIZE));
         for (ContextManagerLifecycleListener each : ShardingSphereServiceLoader.getServiceInstances(ContextManagerLifecycleListener.class)) {
             each.onInitialized(this);

diff --git a/...ore/src/main/java/org/apache/shardingsphere/mode/persist/service/StatePersistService.java b/...ore/src/main/java/org/apache/shardingsphere/mode/persist/service/StatePersistService.java
@@ -23,8 +23,6 @@
 import org.apache.shardingsphere.metadata.persist.node.ComputeNode;
 import org.apache.shardingsphere.mode.spi.PersistRepository;
 
-import java.util.Optional;
-
 /**
  * State persist service.
  */
@@ -36,19 +34,19 @@ public final class StatePersistService {
     /**
      * Update cluster state.
      *
-     * @param state cluster state
+     * @param state to be updated cluster state
      */
-    public void updateClusterState(final ClusterState state) {
+    public void update(final ClusterState state) {
         repository.persist(ComputeNode.getClusterStateNodePath(), state.name());
     }
 
     /**
      * Load cluster state.
      *
-     * @return cluster state
+     * @return loaded cluster state
      */
-    public Optional<ClusterState> loadClusterState() {
+    public ClusterState load() {
         String value = repository.query(ComputeNode.getClusterStateNodePath());
-        return Strings.isNullOrEmpty(value) ? Optional.empty() : Optional.of(ClusterState.valueOf(value));
+        return Strings.isNullOrEmpty(value) ? ClusterState.OK : ClusterState.valueOf(value);
     }
 }
diff --git a/...src/test/java/org/apache/shardingsphere/mode/persist/service/StatePersistServiceTest.java b/...src/test/java/org/apache/shardingsphere/mode/persist/service/StatePersistServiceTest.java
@@ -18,31 +18,46 @@
 package org.apache.shardingsphere.mode.persist.service;
 
 import org.apache.shardingsphere.infra.state.cluster.ClusterState;
-import org.apache.shardingsphere.metadata.persist.node.ComputeNode;
 import org.apache.shardingsphere.mode.spi.PersistRepository;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
 class StatePersistServiceTest {
 
+    private StatePersistService statePersistService;
+
     @Mock
     private PersistRepository repository;
 
+    @BeforeEach
+    void setUp() {
+        statePersistService = new StatePersistService(repository);
+    }
+
+    @Test
+    void assertUpdate() {
+        statePersistService.update(ClusterState.OK);
+        verify(repository).persist("/nodes/compute_nodes/status", ClusterState.OK.name());
+    }
+
     @Test
-    void assertUpdateClusterStateClusterStateWithoutPath() {
-        StatePersistService statePersistService = new StatePersistService(repository);
-        statePersistService.updateClusterState(ClusterState.OK);
-        verify(repository).persist(ComputeNode.getClusterStateNodePath(), ClusterState.OK.name());
+    void assertLoad() {
+        when(repository.query("/nodes/compute_nodes/status")).thenReturn(ClusterState.READ_ONLY.name());
+        assertThat(statePersistService.load(), is(ClusterState.READ_ONLY));
     }
 
     @Test
-    void assertLoadClusterStateClusterState() {
-        new StatePersistService(repository).loadClusterState();
-        verify(repository).query(ComputeNode.getClusterStateNodePath());
+    void assertLoadWithEmptyState() {
+        when(repository.query("/nodes/compute_nodes/status")).thenReturn("");
+        assertThat(statePersistService.load(), is(ClusterState.OK));
     }
 }
diff --git a/...che/shardingsphere/proxy/backend/handler/distsql/ral/updatable/UnlockClusterExecutor.java b/...che/shardingsphere/proxy/backend/handler/distsql/ral/updatable/UnlockClusterExecutor.java
@@ -43,7 +43,7 @@ public void executeUpdate(final UnlockClusterStatement sqlStatement, final Conte
         if (lockContext.tryLock(lockDefinition, 3000L)) {
             try {
                 checkState(contextManager);
-                contextManager.getPersistServiceFacade().getStatePersistService().updateClusterState(ClusterState.OK);
+                contextManager.getPersistServiceFacade().getStatePersistService().update(ClusterState.OK);
                 // TODO unlock snapshot info if locked
             } finally {
                 lockContext.unlock(lockDefinition);

diff --git a/.../java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterReadWriteLockStrategy.java b/.../java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterReadWriteLockStrategy.java
@@ -28,7 +28,7 @@ public class ClusterReadWriteLockStrategy implements ClusterLockStrategy {
 
     @Override
     public void lock() {
-        ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().updateClusterState(ClusterState.UNAVAILABLE);
+        ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().update(ClusterState.UNAVAILABLE);
     }
 
     @Override

diff --git a/...main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterWriteLockStrategy.java b/...main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterWriteLockStrategy.java
@@ -28,7 +28,7 @@ public class ClusterWriteLockStrategy implements ClusterLockStrategy {
 
     @Override
     public void lock() {
-        ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().updateClusterState(ClusterState.READ_ONLY);
+        ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().update(ClusterState.READ_ONLY);
     }
 
     @Override