[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1734: Add null check in SnapshotService to avoid NPE

[GitBox <gi...@apache.org>]

Technoboy- commented on a change in pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734#discussion_r530406723



##########
File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/snapshot/SnapshotService.java
##########
@@ -109,14 +109,15 @@ private void process(final Socket socket) throws IOException {
     private void dumpDirectly(final String path, final String jobName, final List<String> result) {
         for (String each : regCenter.getChildrenKeys(path)) {
             String zkPath = path + "/" + each;
-            String zkValue = regCenter.get(zkPath);
-            if (null == zkValue) {
-                zkValue = "";
-            }
+            String zkValue = Optional.ofNullable(regCenter.get(zkPath)).orElse("");
+            String cachePath = zkPath;
+            String cacheValue = zkValue;
             CuratorCache cache = (CuratorCache) regCenter.getRawCache("/" + jobName);
-            Optional<ChildData> cacheData = cache.get(zkPath);
-            String cachePath = cacheData.map(ChildData::getPath).orElse("");
-            String cacheValue = cacheData.map(childData -> new String(childData.getData())).orElse("");
+            if (null != cache) {

Review comment:
       If cache == null, that means cachePath equals zkPath. and will let the logic at line 121 always be true. it's not the same logic as before.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org