Add more test cases on ComputeNodePersistService (apache#32912)

terrymanu · Sep 17, 2024 · 76825c5 · 76825c5
1 parent 2df99bf
commit 76825c5
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 55 deletions.
diff --git a/...c/main/java/org/apache/shardingsphere/mode/persist/service/ComputeNodePersistService.java b/...c/main/java/org/apache/shardingsphere/mode/persist/service/ComputeNodePersistService.java
@@ -32,10 +32,12 @@
 import org.apache.shardingsphere.metadata.persist.node.ComputeNode;
 import org.apache.shardingsphere.mode.spi.PersistRepository;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 /**
  * Compute node persist service.
@@ -120,14 +122,10 @@ public Optional<Integer> loadInstanceWorkerId(final String instanceId) {
     /**
      * Load all compute node instances.
      *
-     * @return compute node instances
+     * @return loaded compute node instances
      */
     public Collection<ComputeNodeInstance> loadAllComputeNodeInstances() {
-        Collection<ComputeNodeInstance> result = new LinkedList<>();
-        for (InstanceType each : InstanceType.values()) {
-            result.addAll(loadComputeNodeInstances(each));
-        }
-        return result;
+        return Arrays.stream(InstanceType.values()).flatMap(each -> loadComputeNodeInstances(each).stream()).collect(Collectors.toList());
     }
 
     private Collection<ComputeNodeInstance> loadComputeNodeInstances(final InstanceType instanceType) {

diff --git a/...st/java/org/apache/shardingsphere/mode/persist/service/ComputeNodePersistServiceTest.java b/...st/java/org/apache/shardingsphere/mode/persist/service/ComputeNodePersistServiceTest.java
@@ -19,89 +19,109 @@
 
 import org.apache.shardingsphere.infra.instance.ComputeNodeInstance;
 import org.apache.shardingsphere.infra.instance.metadata.InstanceMetaData;
-import org.apache.shardingsphere.infra.instance.metadata.InstanceType;
 import org.apache.shardingsphere.infra.instance.metadata.proxy.ProxyInstanceMetaData;
 import org.apache.shardingsphere.infra.instance.yaml.YamlComputeNodeData;
 import org.apache.shardingsphere.infra.state.instance.InstanceState;
 import org.apache.shardingsphere.infra.util.yaml.YamlEngine;
-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 org.mockito.junit.jupiter.MockitoSettings;
-import org.mockito.quality.Strictness;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
-@MockitoSettings(strictness = Strictness.LENIENT)
 class ComputeNodePersistServiceTest {
 
+    private ComputeNodePersistService computeNodePersistService;
+
     @Mock
     private PersistRepository repository;
 
+    @BeforeEach
+    void setUp() {
+        computeNodePersistService = new ComputeNodePersistService(repository);
+    }
+
     @Test
     void assertRegisterOnline() {
         ComputeNodeInstance computeNodeInstance = new ComputeNodeInstance(new ProxyInstanceMetaData("foo_instance_id", 3307));
         computeNodeInstance.getLabels().add("test");
-        new ComputeNodePersistService(repository).registerOnline(computeNodeInstance);
-        verify(repository).persistEphemeral(eq("/nodes/compute_nodes/online/proxy/" + computeNodeInstance.getMetaData().getId()), anyString());
-        verify(repository).persistEphemeral(ComputeNode.getComputeNodeStateNodePath(computeNodeInstance.getMetaData().getId()), InstanceState.OK.name());
-        verify(repository).persistEphemeral(ComputeNode.getInstanceLabelsNodePath(computeNodeInstance.getMetaData().getId()), YamlEngine.marshal(Collections.singletonList("test")));
+        computeNodePersistService.registerOnline(computeNodeInstance);
+        verify(repository).persistEphemeral(eq("/nodes/compute_nodes/online/proxy/foo_instance_id"), anyString());
+        verify(repository).persistEphemeral("/nodes/compute_nodes/status/foo_instance_id", InstanceState.OK.name());
+        verify(repository).persistEphemeral("/nodes/compute_nodes/labels/foo_instance_id", YamlEngine.marshal(Collections.singletonList("test")));
     }
 
     @Test
     void assertPersistInstanceLabels() {
-        ComputeNodePersistService computeNodePersistService = new ComputeNodePersistService(repository);
-        InstanceMetaData instanceMetaData = new ProxyInstanceMetaData("foo_instance_id", 3307);
-        final String instanceId = instanceMetaData.getId();
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
         computeNodePersistService.persistInstanceLabels(instanceId, Collections.singletonList("test"));
-        verify(repository).persistEphemeral(ComputeNode.getInstanceLabelsNodePath(instanceId), YamlEngine.marshal(Collections.singletonList("test")));
-        computeNodePersistService.persistInstanceLabels(instanceId, Collections.emptyList());
-        verify(repository).persistEphemeral(ComputeNode.getInstanceLabelsNodePath(instanceId), YamlEngine.marshal(Collections.emptyList()));
+        verify(repository).persistEphemeral("/nodes/compute_nodes/labels/foo_instance_id", YamlEngine.marshal(Collections.singletonList("test")));
     }
 
     @Test
     void assertPersistInstanceWorkerId() {
-        InstanceMetaData instanceMetaData = new ProxyInstanceMetaData("foo_instance_id", 3307);
-        final String instanceId = instanceMetaData.getId();
-        new ComputeNodePersistService(repository).persistInstanceWorkerId(instanceId, 100);
-        verify(repository).persistEphemeral(ComputeNode.getInstanceWorkerIdNodePath(instanceId), String.valueOf(100));
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
+        computeNodePersistService.persistInstanceWorkerId(instanceId, 100);
+        verify(repository).persistEphemeral("/nodes/compute_nodes/worker_id/foo_instance_id", String.valueOf(100));
+    }
+
+    @Test
+    void assertLoadEmptyInstanceLabels() {
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
+        when(repository.query("/nodes/compute_nodes/labels/foo_instance_id")).thenReturn("");
+        assertTrue(computeNodePersistService.loadInstanceLabels(instanceId).isEmpty());
     }
 
     @Test
     void assertLoadInstanceLabels() {
-        InstanceMetaData instanceMetaData = new ProxyInstanceMetaData("foo_instance_id", 3307);
-        final String instanceId = instanceMetaData.getId();
-        new ComputeNodePersistService(repository).loadInstanceLabels(instanceId);
-        verify(repository).query(ComputeNode.getInstanceLabelsNodePath(instanceId));
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
+        when(repository.query("/nodes/compute_nodes/labels/foo_instance_id")).thenReturn("{xxx:xxx}");
+        assertFalse(computeNodePersistService.loadInstanceLabels(instanceId).isEmpty());
     }
 
     @Test
     void assertLoadComputeNodeState() {
-        InstanceMetaData instanceMetaData = new ProxyInstanceMetaData("foo_instance_id", 3307);
-        final String instanceId = instanceMetaData.getId();
-        new ComputeNodePersistService(repository).loadComputeNodeState(instanceId);
-        verify(repository).query(ComputeNode.getComputeNodeStateNodePath(instanceId));
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
+        when(repository.query("/nodes/compute_nodes/status/foo_instance_id")).thenReturn("OK");
+        assertThat(computeNodePersistService.loadComputeNodeState(instanceId), is("OK"));
     }
 
     @Test
     void assertLoadInstanceWorkerId() {
-        InstanceMetaData instanceMetaData = new ProxyInstanceMetaData("foo_instance_id", 3307);
-        final String instanceId = instanceMetaData.getId();
-        new ComputeNodePersistService(repository).loadInstanceWorkerId(instanceId);
-        verify(repository).query(ComputeNode.getInstanceWorkerIdNodePath(instanceId));
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
+        when(repository.query("/nodes/compute_nodes/worker_id/foo_instance_id")).thenReturn("1");
+        assertThat(computeNodePersistService.loadInstanceWorkerId(instanceId), is(Optional.of(1)));
+    }
+
+    @Test
+    void assertLoadWithEmptyInstanceWorkerId() {
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
+        when(repository.query("/nodes/compute_nodes/worker_id/foo_instance_id")).thenReturn("");
+        assertFalse(computeNodePersistService.loadInstanceWorkerId(instanceId).isPresent());
+    }
+
+    @Test
+    void assertLoadInstanceWorkerIdWithInvalidFormat() {
+        String instanceId = new ProxyInstanceMetaData("foo_instance_id", 3307).getId();
+        when(repository.query("/nodes/compute_nodes/worker_id/foo_instance_id")).thenReturn("a");
+        assertFalse(computeNodePersistService.loadInstanceWorkerId(instanceId).isPresent());
     }
 
     @Test
@@ -112,43 +132,36 @@ void assertLoadAllComputeNodeInstances() {
         yamlComputeNodeData0.setAttribute("127.0.0.1");
         yamlComputeNodeData0.setVersion("foo_version");
         when(repository.query("/nodes/compute_nodes/online/jdbc/foo_instance_3307")).thenReturn(YamlEngine.marshal(yamlComputeNodeData0));
-        YamlComputeNodeData yamlComputeNodeData1 = new YamlComputeNodeData();
-        yamlComputeNodeData1.setAttribute("127.0.0.1@3308");
-        yamlComputeNodeData1.setVersion("foo_version");
-        when(repository.query("/nodes/compute_nodes/online/proxy/foo_instance_3308")).thenReturn(YamlEngine.marshal(yamlComputeNodeData1));
-        List<ComputeNodeInstance> actual = new ArrayList<>(new ComputeNodePersistService(repository).loadAllComputeNodeInstances());
-        assertThat(actual.size(), is(2));
+        List<ComputeNodeInstance> actual = new ArrayList<>(computeNodePersistService.loadAllComputeNodeInstances());
+        assertThat(actual.size(), is(1));
         assertThat(actual.get(0).getMetaData().getId(), is("foo_instance_3307"));
         assertThat(actual.get(0).getMetaData().getIp(), is("127.0.0.1"));
-        assertThat(actual.get(1).getMetaData().getId(), is("foo_instance_3308"));
-        assertThat(actual.get(1).getMetaData().getIp(), is("127.0.0.1"));
-        assertThat(actual.get(1).getMetaData().getType(), is(InstanceType.PROXY));
-        assertThat(((ProxyInstanceMetaData) actual.get(1).getMetaData()).getPort(), is(3308));
     }
 
     @Test
     void assertLoadComputeNodeInstance() {
         InstanceMetaData instanceMetaData = new ProxyInstanceMetaData("foo_instance_id", 3307);
-        ComputeNodeInstance actual = new ComputeNodePersistService(repository).loadComputeNodeInstance(instanceMetaData);
+        ComputeNodeInstance actual = computeNodePersistService.loadComputeNodeInstance(instanceMetaData);
         assertThat(actual.getMetaData(), is(instanceMetaData));
     }
 
     @Test
     void assertGetUsedWorkerIds() {
-        new ComputeNodePersistService(repository).getAssignedWorkerIds();
-        verify(repository).getChildrenKeys(ComputeNode.getInstanceWorkerIdRootNodePath());
+        when(repository.getChildrenKeys("/nodes/compute_nodes/worker_id")).thenReturn(Arrays.asList("1", "2"));
+        when(repository.query("/nodes/compute_nodes/worker_id/1")).thenReturn(null);
+        when(repository.query("/nodes/compute_nodes/worker_id/2")).thenReturn("2");
+        assertThat(computeNodePersistService.getAssignedWorkerIds(), is(Collections.singleton(2)));
     }
 
     @Test
     void assertUpdateComputeNodeState() {
-        new ComputeNodePersistService(repository).updateComputeNodeState("foo_instance_id", InstanceState.OK);
-        verify(repository).persistEphemeral(ComputeNode.getComputeNodeStateNodePath("foo_instance_id"), InstanceState.OK.name());
+        computeNodePersistService.updateComputeNodeState("foo_instance_id", InstanceState.OK);
+        verify(repository).persistEphemeral("/nodes/compute_nodes/status/foo_instance_id", InstanceState.OK.name());
     }
 
     @Test
     void assertOffline() {
-        ComputeNodeInstance computeNodeInstance = new ComputeNodeInstance(new ProxyInstanceMetaData("foo_instance_id", 3307));
-        new ComputeNodePersistService(repository).offline(computeNodeInstance);
-        verify(repository).delete("/nodes/compute_nodes/online/proxy/" + computeNodeInstance.getMetaData().getId());
+        computeNodePersistService.offline(new ComputeNodeInstance(new ProxyInstanceMetaData("foo_instance_id", 3307)));
+        verify(repository).delete("/nodes/compute_nodes/online/proxy/foo_instance_id");
     }
 }
diff --git a/mode/core/src/test/resources/logback-test.xml b/mode/core/src/test/resources/logback-test.xml
@@ -0,0 +1,35 @@
+<?xml version="1.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.
+  -->
+
+<configuration>
+    <statusListener class="ch.qos.logback.core.status.NopStatusListener" />
+    <appender name="console" class="ch.qos.logback.core.ConsoleAppender">
+        <encoder>
+            <pattern>[%-5level] %d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %logger{36} - %msg%n</pattern>
+        </encoder>
+    </appender>
+    <logger name="org.apache.shardingsphere" level="warn" additivity="false">
+        <appender-ref ref="console" />
+    </logger>
+    <logger name="org.apache.shardingsphere.mode.persist.service.ComputeNodePersistService" level="off" />
+
+    <root>
+        <level value="error" />
+        <appender-ref ref="console" />
+    </root>
+</configuration>