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: 吴伟杰 <[email protected]>
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);

Reply via email to