spotbugs: make map traversal more efficient (#1035)

* make map traversal more efficient

* make map traversal more efficient

modify according to review

* compile fail and delete space

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataSourceService.java

@@ -517,14 +517,12 @@ public class DataSourceService extends BaseService{
             parameterMap.put(Constants.PRINCIPAL,principal);
         }
         if (other != null && !"".equals(other)) {
-            Map map = JSONObject.parseObject(other, new TypeReference<LinkedHashMap<String, String>>() {
+            LinkedHashMap<String, String> map = JSONObject.parseObject(other, new TypeReference<LinkedHashMap<String, String>>() {
             });
             if (map.size() > 0) {
-                Set<String> keys = map.keySet();
                 StringBuilder otherSb = new StringBuilder();
-                for (String key : keys) {
+                for (Map.Entry<String, String> entry: map.entrySet()) {
-                    otherSb.append(String.format("%s=%s%s", key, map.get(key), separator));
+                    otherSb.append(String.format("%s=%s%s", entry.getKey(), entry.getValue(), separator));
-
                 }
                 otherSb.deleteCharAt(otherSb.length() - 1);
                 parameterMap.put(Constants.OTHER, otherSb);

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java

@@ -144,11 +144,11 @@ public class ResourcesService extends BaseService {
             resourcesMapper.insert(resource);
 
             putMsg(result, Status.SUCCESS);
-            Map dataMap = new BeanMap(resource);
+            Map<Object, Object> dataMap = new BeanMap(resource);
             Map<String, Object> resultMap = new HashMap<String, Object>();
-            for (Object key : dataMap.keySet()) {
+            for (Map.Entry<Object, Object> entry: dataMap.entrySet()) {
-                if (!"class".equalsIgnoreCase(key.toString())) {
+                if (!"class".equalsIgnoreCase(entry.getKey().toString())) {
-                    resultMap.put(key.toString(), dataMap.get(key));
+                    resultMap.put(entry.getKey().toString(), entry.getValue());
                 }
             }
             result.setData(resultMap);
@@ -238,11 +238,11 @@ public class ResourcesService extends BaseService {
             resourcesMapper.updateById(resource);
 
             putMsg(result, Status.SUCCESS);
-            Map dataMap = new BeanMap(resource);
+            Map<Object, Object> dataMap = new BeanMap(resource);
             Map<String, Object> resultMap = new HashMap<>(5);
-            for (Object key : dataMap.keySet()) {
+            for (Map.Entry<Object, Object> entry: dataMap.entrySet()) {
-                if (!Constants.CLASS.equalsIgnoreCase(key.toString())) {
+                if (!Constants.CLASS.equalsIgnoreCase(entry.getKey().toString())) {
-                    resultMap.put(key.toString(), dataMap.get(key));
+                    resultMap.put(entry.getKey().toString(), entry.getValue());
                 }
             }
             result.setData(resultMap);
@@ -595,11 +595,11 @@ public class ResourcesService extends BaseService {
         resourcesMapper.insert(resource);
 
         putMsg(result, Status.SUCCESS);
-        Map dataMap = new BeanMap(resource);
+        Map<Object, Object> dataMap = new BeanMap(resource);
         Map<String, Object> resultMap = new HashMap<>(5);
-        for (Object key : dataMap.keySet()) {
+        for (Map.Entry<Object, Object> entry: dataMap.entrySet()) {
-            if (!Constants.CLASS.equalsIgnoreCase(key.toString())) {
+            if (!Constants.CLASS.equalsIgnoreCase(entry.getKey().toString())) {
-                resultMap.put(key.toString(), dataMap.get(key));
+                resultMap.put(entry.getKey().toString(), entry.getValue());
             }
         }
         result.setData(resultMap);
@@ -894,4 +894,4 @@ public class ResourcesService extends BaseService {
         }
     }
 
-}
+}

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CollectionUtils.java

@@ -276,13 +276,11 @@ public class CollectionUtils {
         for (T instance : originList) {
             Map<String, Object> dataMap = new BeanMap(instance);
             instanceMap = new LinkedHashMap<>(16,0.75f,true);
-            for (Object key : dataMap.keySet()) {
+            for (Map.Entry<String, Object> entry: dataMap.entrySet()) {
-
+                if (exclusionSet.contains(entry.getKey())) {
-                if (exclusionSet.contains(key.toString())) {
                     continue;
                 }
-                instanceMap.put(key.toString(), dataMap.get(key));
+                instanceMap.put(entry.getKey(), entry.getValue());
-
             }
             instanceList.add(instanceMap);
         }

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/zk/AbstractZKClient.java

@@ -55,7 +55,7 @@ public abstract class AbstractZKClient {
 	 *  load configuration file
 	 */
 	protected static Configuration conf;
-	
+
 	protected  CuratorFramework zkClient = null;
 
 	/**
@@ -92,13 +92,13 @@ public abstract class AbstractZKClient {
 
 			zkClient.start();
 			initStateLister();
-			
+
 		}catch(Exception e){
 			logger.error("create zookeeper connect failed : " + e.getMessage(),e);
 			System.exit(-1);
 		}
     }
-	
+
 	/**
 	 *
 	 *  register status monitoring events for zookeeper clients
@@ -109,7 +109,7 @@ public abstract class AbstractZKClient {
 		}
 		// add ConnectionStateListener monitoring zookeeper  connection state
 		ConnectionStateListener csLister = new ConnectionStateListener() {
-			
+
 			@Override
 			public void stateChanged(CuratorFramework client, ConnectionState newState) {
 				logger.info("state changed , current state : " + newState.name());
@@ -122,7 +122,7 @@ public abstract class AbstractZKClient {
 				}
 			}
 		};
-		
+
 		zkClient.getConnectionStateListenable().addListener(csLister);
 	}
 
@@ -346,9 +346,9 @@ public abstract class AbstractZKClient {
 
 		List<MasterServer> masterServers = new ArrayList<>();
 		int i = 0;
-		for(String path : masterMap.keySet()){
+		for (Map.Entry<String, String> entry : masterMap.entrySet()) {
-			MasterServer masterServer = ResInfo.parseHeartbeatForZKInfo(masterMap.get(path));
+			MasterServer masterServer = ResInfo.parseHeartbeatForZKInfo(entry.getValue());
-			masterServer.setZkDirectory( parentPath + "/"+ path);
+			masterServer.setZkDirectory( parentPath + "/"+ entry.getKey());
 			masterServer.setId(i);
 			i ++;
 			masterServers.add(masterServer);

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/ProcessDao.java

@@ -580,9 +580,9 @@ public class ProcessDao extends AbstractBaseDao {
             //reset command parameter
             if(processInstance.getCommandParam() != null){
                 Map<String, String> processCmdParam = JSONUtils.toMap(processInstance.getCommandParam());
-                for(String key : processCmdParam.keySet()){
+                for(Map.Entry<String, String> entry: processCmdParam.entrySet()) {
-                    if(!cmdParam.containsKey(key)){
+                    if(!cmdParam.containsKey(entry.getKey())){
-                        cmdParam.put(key,processCmdParam.get(key));
+                        cmdParam.put(entry.getKey(), entry.getValue());
                     }
                 }
             }

dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/master/runner/MasterExecThread.java

@@ -550,11 +550,9 @@ public class MasterExecThread implements Runnable {
      */
     private List<TaskInstance> getCompleteTaskByState(ExecutionStatus state){
         List<TaskInstance> resultList = new ArrayList<>();
-        Set<String> taskList  = completeTaskList.keySet();
+        for (Map.Entry<String, TaskInstance> entry: completeTaskList.entrySet()) {
-        for(String taskName : taskList){
+            if(entry.getValue().getState() == state){
-            TaskInstance taskInstance = completeTaskList.get(taskName);
+                resultList.add(entry.getValue());
-            if(taskInstance.getState() == state){
-                resultList.add(taskInstance);
             }
         }
         return resultList;
@@ -766,10 +764,8 @@ public class MasterExecThread implements Runnable {
      * @return
      */
     private Boolean hasRetryTaskInStandBy(){
-        Set<String> taskNameSet = this.readyToSubmitTaskList.keySet();
+        for (Map.Entry<String, TaskInstance> entry: readyToSubmitTaskList.entrySet()) {
-        for(String taskName : taskNameSet){
+            if(entry.getValue().getState().typeIsFailure()){
-            TaskInstance task = this.readyToSubmitTaskList.get(taskName);
-            if(task.getState().typeIsFailure()){
                 return true;
             }
         }
@@ -791,16 +787,15 @@ public class MasterExecThread implements Runnable {
                         processDao.findProcessDefineById(processInstance.getProcessDefinitionId()));
                 sendTimeWarning = true;
             }
-            Set<MasterBaseTaskExecThread> keys = activeTaskNode.keySet();
+            for(Map.Entry<MasterBaseTaskExecThread,Future<Boolean>> entry: activeTaskNode.entrySet()) {
-            for (MasterBaseTaskExecThread taskExecThread : keys) {
+                Future<Boolean> future = entry.getValue();
-                Future<Boolean> future = activeTaskNode.get(taskExecThread);
+                TaskInstance task  = entry.getKey().getTaskInstance();
-                TaskInstance task  = taskExecThread.getTaskInstance();
 
                 if(!future.isDone()){
                     continue;
                 }
                 // node monitor thread complete
-                activeTaskNode.remove(taskExecThread);
+                activeTaskNode.remove(entry.getKey());
                 if(task == null){
                     this.taskFailedSubmit = true;
                     continue;
@@ -842,11 +837,11 @@ public class MasterExecThread implements Runnable {
             // failure priority is higher than pause
             // if a task fails, other suspended tasks need to be reset kill
             if(errorTaskList.size() > 0){
-                for(String taskName : completeTaskList.keySet()){
+                for(Map.Entry<String, TaskInstance> entry: completeTaskList.entrySet()) {
-                    TaskInstance completeTask = completeTaskList.get(taskName);
+                    TaskInstance completeTask = entry.getValue();
                     if(completeTask.getState()== ExecutionStatus.PAUSE){
                         completeTask.setState(ExecutionStatus.KILL);
-                        completeTaskList.put(taskName, completeTask);
+                        completeTaskList.put(entry.getKey(), completeTask);
                         processDao.updateTaskInstance(completeTask);
                     }
                 }
@@ -938,9 +933,8 @@ public class MasterExecThread implements Runnable {
      * handling the list of tasks to be submitted
      */
     private void submitStandByTask(){
-        Set<String> readySubmitTaskNames = readyToSubmitTaskList.keySet();
+        for(Map.Entry<String, TaskInstance> entry: readyToSubmitTaskList.entrySet()) {
-        for(String readySubmitTaskName : readySubmitTaskNames){
+            TaskInstance task = entry.getValue();
-            TaskInstance task = readyToSubmitTaskList.get(readySubmitTaskName);
             DependResult dependResult = getDependResultForTask(task);
             if(DependResult.SUCCESS == dependResult){
                 if(retryTaskIntervalOverTime(task)){
@@ -949,7 +943,7 @@ public class MasterExecThread implements Runnable {
                 }
             }else if(DependResult.FAILED == dependResult){
                 // if the dependency fails, the current node is not submitted and the state changes to failure.
-                dependFailedTask.put(readySubmitTaskName, task);
+                dependFailedTask.put(entry.getKey(), task);
                 removeTaskFromStandbyList(task);
                 logger.info("task {},id:{} depend result : {}",task.getName(), task.getId(), dependResult);
             }

dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/dependent/DependentTask.java

@@ -141,14 +141,12 @@ public class DependentTask extends AbstractTask {
     private boolean allDependentTaskFinish(){
         boolean finish = true;
         for(DependentExecute dependentExecute : dependentTaskList){
-            Map<String, DependResult> resultMap = dependentExecute.getDependResultMap();
+            for(Map.Entry<String, DependResult> entry: dependentExecute.getDependResultMap().entrySet()) {
-            Set<String> keySet = resultMap.keySet();
+                if(!dependResultMap.containsKey(entry.getKey())){
-            for(String key : keySet){
+                    dependResultMap.put(entry.getKey(), entry.getValue());
-                if(!dependResultMap.containsKey(key)){
-                    dependResultMap.put(key, resultMap.get(key));
                     //save depend result to log
                     logger.info("dependent item complete {} {},{}",
-                            DEPENDENT_SPLIT, key, resultMap.get(key).toString());
+                            DEPENDENT_SPLIT, entry.getKey(), entry.getValue().toString());
                 }
             }
             if(!dependentExecute.finish(dependentDate)){

dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java

@@ -355,10 +355,10 @@ public class SqlTask extends AbstractTask {
             stmt.setQueryTimeout(taskProps.getTaskTimeout());
         }
         Map<Integer, Property> params = sqlBinds.getParamsMap();
-        if(params != null){
+        if(params != null) {
-            for(Integer key : params.keySet()){
+            for (Map.Entry<Integer, Property> entry : params.entrySet()) {
-                Property prop = params.get(key);
+                Property prop = entry.getValue();
-                ParameterUtils.setInParameter(key,stmt,prop.getType(),prop.getValue());
+                ParameterUtils.setInParameter(entry.getKey(), stmt, prop.getType(), prop.getValue());
             }
         }
         logger.info("prepare statement replace sql : {} ",stmt.toString());