Hello Yair Zaslavsky,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/15570

to review the following change.

Change subject: core: Further cleanup for for end of command coordination
......................................................................

core: Further cleanup for for end of command coordination

The following patch introduces more cleanup to the code -
Renaming the classes to proper names (based on "Command")
Changing variables, method names, and fields

Change-Id: Ibc3c628b58510e500b24065f5d20253d5d7fba82
Signed-off-by: Yair Zaslavsky <yzasl...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
R 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java
R 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java
5 files changed, 50 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/70/15570/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
index 3393328..c472fe2 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java
@@ -67,7 +67,7 @@
             if (taskType == AsyncTaskType.unknown) {
                 result = new SPMAsyncTask(asyncTaskParams);
             } else {
-                result = new EntityAsyncTask(asyncTaskParams, duringInit);
+                result = new CommandAsyncTask(asyncTaskParams, duringInit);
             }
             return result;
         }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
index eb1f839..75a90e7 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
@@ -237,7 +237,7 @@
     }
 
     private boolean isCurrentTaskLookedFor(Guid id, SPMAsyncTask task) {
-        return (task instanceof EntityAsyncTask) && 
id.equals(task.getParameters().getEntityInfo().getId())
+        return (task instanceof CommandAsyncTask) && 
id.equals(task.getParameters().getEntityInfo().getId())
                 && (task.getState() != AsyncTaskState.Cleared)
                 && (task.getState() != AsyncTaskState.ClearFailed);
     }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java
similarity index 76%
rename from 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java
rename to 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java
index 72c6935..2a9cf2e 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java
@@ -23,28 +23,28 @@
 /**
  * Base class for all tasks regarding a specific command.
  */
-public class EntityAsyncTask extends SPMAsyncTask {
+public class CommandAsyncTask extends SPMAsyncTask {
     private static final Object _lockObject = new Object();
 
-    private static final Map<Guid, EntityMultiAsyncTasks> 
_multiTasksByCommandIds =
-            new HashMap<Guid, EntityMultiAsyncTasks>();
+    private static final Map<Guid, CommandMultiAsyncTasks> 
_multiTasksByCommandIds =
+            new HashMap<Guid, CommandMultiAsyncTasks>();
 
-    private EntityMultiAsyncTasks GetEntityMultiAsyncTasks() {
-        EntityMultiAsyncTasks entityInfo = null;
+    private CommandMultiAsyncTasks GetCommandMultiAsyncTasks() {
+        CommandMultiAsyncTasks entityInfo = null;
         synchronized (_lockObject) {
             entityInfo = _multiTasksByCommandIds.get(getCommandId());
         }
         return entityInfo;
     }
 
-    public EntityAsyncTask(AsyncTaskParameters parameters, boolean duringInit) 
{
+    public CommandAsyncTask(AsyncTaskParameters parameters, boolean 
duringInit) {
         super(parameters);
         boolean isNewCommandAdded = false;
         synchronized (_lockObject) {
             if (!_multiTasksByCommandIds.containsKey(getCommandId())) {
-                log.infoFormat("EntityAsyncTask::Adding EntityMultiAsyncTasks 
object for command '{0}'",
+                log.infoFormat("CommandAsyncTask::Adding 
CommandMultiAsyncTasks object for command '{0}'",
                         getCommandId());
-                _multiTasksByCommandIds.put(getCommandId(), new 
EntityMultiAsyncTasks(getCommandId()));
+                _multiTasksByCommandIds.put(getCommandId(), new 
CommandMultiAsyncTasks(getCommandId()));
                 isNewCommandAdded = true;
             }
         }
@@ -59,13 +59,13 @@
             }
         }
 
-        EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks();
+        CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks();
         entityInfo.AttachTask(this);
     }
 
     @Override
     protected void ConcreteStartPollingTask() {
-        EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks();
+        CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks();
         entityInfo.StartPollingTask(getVdsmTaskId());
     }
 
@@ -76,10 +76,10 @@
     }
 
     private void EndActionIfNecessary() {
-        EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks();
+        CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks();
         if (entityInfo == null) {
             log.warnFormat(
-                    "EntityAsyncTask::EndActionIfNecessary: No info is 
available for entity '{0}', current task ('{1}') was probably created while 
other tasks were in progress, clearing task.",
+                    "CommandAsyncTask::EndActionIfNecessary: No info is 
available for entity '{0}', current task ('{1}') was probably created while 
other tasks were in progress, clearing task.",
                     getCommandId(),
                     getVdsmTaskId());
 
@@ -88,11 +88,11 @@
 
         else if (entityInfo.ShouldEndAction()) {
             log.infoFormat(
-                    "EntityAsyncTask::EndActionIfNecessary: All tasks of 
entity '{0}' has ended -> executing 'EndAction'",
+                    "CommandAsyncTask::EndActionIfNecessary: All tasks of 
entity '{0}' has ended -> executing 'EndAction'",
                     getCommandId());
 
             log.infoFormat(
-                    "EntityAsyncTask::EndAction: Ending action for {0} tasks 
(command ID: '{1}'): calling EndAction '.",
+                    "CommandAsyncTask::EndAction: Ending action for {0} tasks 
(command ID: '{1}'): calling EndAction '.",
                     entityInfo.getTasksCountCurrentActionType(),
                     entityInfo.getCommandId());
 
@@ -108,7 +108,7 @@
     }
 
     private void EndCommandAction() {
-        EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks();
+        CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks();
         VdcReturnValueBase vdcReturnValue = null;
         ExecutionContext context = null;
 
@@ -128,7 +128,7 @@
         
dbAsyncTask.getActionParameters().setImagesParameters(imagesParameters);
 
         try {
-            log.infoFormat("EntityAsyncTask::EndCommandAction [within thread] 
context: Attempting to EndAction '{0}', executionIndex: '{1}'",
+            log.infoFormat("CommandAsyncTask::EndCommandAction [within thread] 
context: Attempting to EndAction '{0}', executionIndex: '{1}'",
                     dbAsyncTask.getActionParameters().getCommandType(),
                     dbAsyncTask.getActionParameters().getExecutionIndex());
 
@@ -156,7 +156,7 @@
 
         catch (RuntimeException Ex2) {
             log.error(
-                    "EntityAsyncTask::EndCommandAction [within thread]: An 
exception has been thrown (not related to 'EndAction' itself)",
+                    "CommandAsyncTask::EndCommandAction [within thread]: An 
exception has been thrown (not related to 'EndAction' itself)",
                     Ex2);
         }
 
@@ -179,18 +179,18 @@
                 
getParameters().getDbAsyncTask().getActionParameters().getCommandType());
     }
 
-    private void handleEndActionResult(EntityMultiAsyncTasks commandInfo, 
VdcReturnValueBase vdcReturnValue,
+    private void handleEndActionResult(CommandMultiAsyncTasks commandInfo, 
VdcReturnValueBase vdcReturnValue,
             ExecutionContext context,
             boolean isTaskGroupSuccess) {
         try {
             VdcActionType actionType = 
getParameters().getDbAsyncTask().getaction_type();
             log.infoFormat(
-                    "EntityAsyncTask::HandleEndActionResult [within thread]: 
EndAction for action type '{0}' completed, handling the result.",
+                    "CommandAsyncTask::HandleEndActionResult [within thread]: 
EndAction for action type '{0}' completed, handling the result.",
                     actionType);
 
                 if (vdcReturnValue == null || (!vdcReturnValue.getSucceeded() 
&& vdcReturnValue.getEndActionTryAgain())) {
                     log.infoFormat(
-                            "EntityAsyncTask::HandleEndActionResult [within 
thread]: EndAction for action type {0} hasn't succeeded, not clearing tasks, 
will attempt again next polling.",
+                            "CommandAsyncTask::HandleEndActionResult [within 
thread]: EndAction for action type {0} hasn't succeeded, not clearing tasks, 
will attempt again next polling.",
                         actionType);
 
                     commandInfo.Repoll();
@@ -198,7 +198,7 @@
 
                 else {
                     log.infoFormat(
-                            "EntityAsyncTask::HandleEndActionResult [within 
thread]: EndAction for action type {0} {1}succeeded, clearing tasks.",
+                            "CommandAsyncTask::HandleEndActionResult [within 
thread]: EndAction for action type {0} {1}succeeded, clearing tasks.",
                         actionType,
                             (vdcReturnValue.getSucceeded() ? "" : "hasn't "));
 
@@ -215,7 +215,7 @@
                     synchronized (_lockObject) {
                         if (commandInfo.getAllCleared()) {
                             log.infoFormat(
-                                    "EntityAsyncTask::HandleEndActionResult 
[within thread]: Removing EntityMultiAsyncTasks object for entity '{0}'",
+                                    "CommandAsyncTask::HandleEndActionResult 
[within thread]: Removing CommandMultiAsyncTasks object for entity '{0}'",
                                     commandInfo.getCommandId());
                             
_multiTasksByCommandIds.remove(commandInfo.getCommandId());
                         }
@@ -224,7 +224,7 @@
         }
 
         catch (RuntimeException ex) {
-            log.error("EntityAsyncTask::HandleEndActionResult [within thread]: 
an exception has been thrown", ex);
+            log.error("CommandAsyncTask::HandleEndActionResult [within 
thread]: an exception has been thrown", ex);
         }
     }
 
@@ -241,5 +241,5 @@
     }
 
 
-    private static final Log log = LogFactory.getLog(EntityAsyncTask.class);
+    private static final Log log = LogFactory.getLog(CommandAsyncTask.class);
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityMultiAsyncTasks.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java
similarity index 71%
rename from 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityMultiAsyncTasks.java
rename to 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java
index 91ba9c2..92fa92d 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityMultiAsyncTasks.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java
@@ -6,9 +6,9 @@
 import org.ovirt.engine.core.utils.log.Log;
 import org.ovirt.engine.core.utils.log.LogFactory;
 
-public class EntityMultiAsyncTasks {
+public class CommandMultiAsyncTasks {
 
-    private java.util.HashMap<Guid, EntityAsyncTask> _listTasks;
+    private java.util.HashMap<Guid, CommandAsyncTask> _listTasks;
     private Guid commandId;
 
     public Guid getCommandId() {
@@ -19,12 +19,12 @@
         commandId = value;
     }
 
-    public EntityMultiAsyncTasks(Guid commandId) {
-        _listTasks = new java.util.HashMap<Guid, EntityAsyncTask>();
+    public CommandMultiAsyncTasks(Guid commandId) {
+        _listTasks = new java.util.HashMap<Guid, CommandAsyncTask>();
         setCommandId(commandId);
     }
 
-    public void AttachTask(EntityAsyncTask asyncTask) {
+    public void AttachTask(CommandAsyncTask asyncTask) {
         synchronized (_listTasks) {
             if (!_listTasks.containsKey(asyncTask.getVdsmTaskId())) {
                 log.infoFormat("EntityMultiAsyncTasks::AttachTask: Attaching 
task '{0}' to command '{1}'.",
@@ -35,10 +35,10 @@
         }
     }
 
-    private java.util.ArrayList<EntityAsyncTask> getCurrentTasks() {
-        java.util.ArrayList<EntityAsyncTask> retValue = new 
java.util.ArrayList<EntityAsyncTask>();
+    private java.util.ArrayList<CommandAsyncTask> getCurrentTasks() {
+        java.util.ArrayList<CommandAsyncTask> retValue = new 
java.util.ArrayList<CommandAsyncTask>();
 
-        for (EntityAsyncTask task : _listTasks.values()) {
+        for (CommandAsyncTask task : _listTasks.values()) {
             if (task.getParameters() != null
                     && task.getParameters().getDbAsyncTask() != null
                     && (task.getState() == AsyncTaskState.Polling || 
task.getState() == AsyncTaskState.Ended || task
@@ -52,9 +52,9 @@
 
     public boolean ShouldEndAction() {
         synchronized (_listTasks) {
-            java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
+            java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
 
-            for (EntityAsyncTask task : CurrentActionTypeTasks) {
+            for (CommandAsyncTask task : CurrentActionTypeTasks) {
                 if (task.getState() != AsyncTaskState.Ended) {
                     return false;
                 }
@@ -66,9 +66,9 @@
 
     public void MarkAllWithAttemptingEndAction() {
         synchronized (_listTasks) {
-            java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
+            java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
 
-            for (EntityAsyncTask task : CurrentActionTypeTasks) {
+            for (CommandAsyncTask task : CurrentActionTypeTasks) {
                 task.setState(AsyncTaskState.AttemptingEndAction);
             }
         }
@@ -79,9 +79,9 @@
         java.util.ArrayList<EndedTaskInfo> endedTaskInfoList = new 
java.util.ArrayList<EndedTaskInfo>();
 
         synchronized (_listTasks) {
-            java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
+            java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
 
-            for (EntityAsyncTask task : CurrentActionTypeTasks) {
+            for (CommandAsyncTask task : CurrentActionTypeTasks) {
                 task.setLastStatusAccessTime();
                 EndedTaskInfo tempVar = new EndedTaskInfo();
                 tempVar.setTaskStatus(task.getLastTaskStatus());
@@ -98,7 +98,7 @@
     public int getTasksCountCurrentActionType() {
         int returnValue = 0;
         synchronized (_listTasks) {
-            java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
+            java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
             returnValue = CurrentActionTypeTasks.size();
         }
 
@@ -107,9 +107,9 @@
 
     public void Repoll() {
         synchronized (_listTasks) {
-            java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
+            java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
 
-            for (EntityAsyncTask task : CurrentActionTypeTasks) {
+            for (CommandAsyncTask task : CurrentActionTypeTasks) {
                 task.setState(AsyncTaskState.Ended);
             }
         }
@@ -117,9 +117,9 @@
 
     public void ClearTasks() {
         synchronized (_listTasks) {
-            java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
+            java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = 
getCurrentTasks();
 
-            for (EntityAsyncTask task : CurrentActionTypeTasks) {
+            for (CommandAsyncTask task : CurrentActionTypeTasks) {
                 task.clearAsyncTask();
             }
         }
@@ -129,7 +129,7 @@
         synchronized (_listTasks) {
             if (_listTasks.containsKey(TaskID) && 
_listTasks.get(TaskID).getParameters() != null
                     && _listTasks.get(TaskID).getParameters().getDbAsyncTask() 
!= null) {
-                for (EntityAsyncTask task : _listTasks.values()) {
+                for (CommandAsyncTask task : _listTasks.values()) {
                     if (task.getParameters() != null && 
task.getParameters().getDbAsyncTask() != null)
                         task.setState(AsyncTaskState.Polling);
                     }
@@ -144,7 +144,7 @@
 
     public boolean getAllCleared() {
         synchronized (_listTasks) {
-            for (EntityAsyncTask task : _listTasks.values()) {
+            for (CommandAsyncTask task : _listTasks.values()) {
                 if (!taskWasCleared(task)) {
                     return false;
                 }
@@ -159,10 +159,10 @@
      *            The task to check.
      * @return Whether the task is cleared (succeeded or failed) or not 
cleared.
      */
-    private boolean taskWasCleared(EntityAsyncTask task) {
+    private boolean taskWasCleared(CommandAsyncTask task) {
         AsyncTaskState taskState = task.getState();
         return taskState == AsyncTaskState.Cleared || taskState == 
AsyncTaskState.ClearFailed;
     }
 
-    private static Log log = LogFactory.getLog(EntityMultiAsyncTasks.class);
+    private static Log log = LogFactory.getLog(CommandMultiAsyncTasks.class);
 }
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java
index 501d52c..2cc6751 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java
@@ -163,7 +163,7 @@
         assertEquals("wrong task result", AsyncTaskResultEnum.success, 
spmAsyncTask.getLastTaskStatus().getResult());
         assertEquals("wrong task status", AsyncTaskStatusEnum.init, 
spmAsyncTask.getLastTaskStatus().getStatus());
         assertEquals("wrong task state", AsyncTaskState.Initializing, 
spmAsyncTask.getState());
-        assertTrue("wrong task type", spmAsyncTask instanceof EntityAsyncTask);
+        assertTrue("wrong task type", spmAsyncTask instanceof 
CommandAsyncTask);
     }
 
     /**


-- 
To view, visit http://gerrit.ovirt.org/15570
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc3c628b58510e500b24065f5d20253d5d7fba82
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to