You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by wu...@apache.org on 2022/12/12 09:41:12 UTC

[shardingsphere-elasticjob] branch master updated: Avoid NPE in InstanceService.getAvailableJobInstances (#2164)

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

wuweijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere-elasticjob.git


The following commit(s) were added to refs/heads/master by this push:
     new 1cbeacf2b Avoid NPE in InstanceService.getAvailableJobInstances (#2164)
1cbeacf2b is described below

commit 1cbeacf2b3fdb440a0d2a9da7c1cbcf20118ff64
Author: 吴伟杰 <wu...@apache.org>
AuthorDate: Mon Dec 12 17:41:07 2022 +0800

    Avoid NPE in InstanceService.getAvailableJobInstances (#2164)
---
 .../elasticjob/lite/internal/instance/InstanceService.java       | 9 +++++++--
 .../elasticjob/lite/internal/instance/InstanceServiceTest.java   | 8 ++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceService.java b/elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceService.java
index 0fb0c3baf..1fe987b11 100644
--- a/elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceService.java
+++ b/elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceService.java
@@ -69,9 +69,14 @@ public final class InstanceService {
     public List<JobInstance> getAvailableJobInstances() {
         List<JobInstance> result = new LinkedList<>();
         for (String each : jobNodeStorage.getJobNodeChildrenKeys(InstanceNode.ROOT)) {
-            JobInstance jobInstance = YamlEngine.unmarshal(jobNodeStorage.getJobNodeData(instanceNode.getInstancePath(each)), JobInstance.class);
+            // TODO It's better to make it atomic
+            String jobNodeData = jobNodeStorage.getJobNodeData(instanceNode.getInstancePath(each));
+            if (null == jobNodeData) {
+                continue;
+            }
+            JobInstance jobInstance = YamlEngine.unmarshal(jobNodeData, JobInstance.class);
             if (null != jobInstance && serverService.isEnableServer(jobInstance.getServerIp())) {
-                result.add(new JobInstance(each));
+                result.add(jobInstance);
             }
         }
         return result;
diff --git a/elasticjob-lite/elasticjob-lite-core/src/test/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceServiceTest.java b/elasticjob-lite/elasticjob-lite-core/src/test/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceServiceTest.java
index 5bc3ee7d2..2e7cd55de 100644
--- a/elasticjob-lite/elasticjob-lite-core/src/test/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceServiceTest.java
+++ b/elasticjob-lite/elasticjob-lite-core/src/test/java/org/apache/shardingsphere/elasticjob/lite/internal/instance/InstanceServiceTest.java
@@ -79,6 +79,14 @@ public final class InstanceServiceTest {
         assertThat(instanceService.getAvailableJobInstances(), is(Collections.singletonList(new JobInstance("127.0.0.1@-@0"))));
     }
     
+    @Test
+    public void assertGetAvailableJobInstancesWhenInstanceRemoving() {
+        when(jobNodeStorage.getJobNodeChildrenKeys(InstanceNode.ROOT)).thenReturn(Arrays.asList("127.0.0.1@-@0", "127.0.0.2@-@0"));
+        when(jobNodeStorage.getJobNodeData("instances/127.0.0.1@-@0")).thenReturn("jobInstanceId: 127.0.0.1@-@0\nlabels: labels\nserverIp: 127.0.0.1\n");
+        when(serverService.isEnableServer("127.0.0.1")).thenReturn(true);
+        assertThat(instanceService.getAvailableJobInstances(), is(Collections.singletonList(new JobInstance("127.0.0.1@-@0"))));
+    }
+    
     @Test
     public void assertIsLocalJobInstanceExisted() {
         when(jobNodeStorage.isJobNodeExisted("instances/127.0.0.1@-@0")).thenReturn(true);