Hello Yair Zaslavsky,

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

    http://gerrit.ovirt.org/15567

to review the following change.

Change subject: core: SPMAsyncTask - change from associatedEntities + single 
type to map
......................................................................

core: SPMAsyncTask - change from associatedEntities + single type to map

Change-Id: I8826761f833944e8a4d9159bf91863381cfb1f79
Signed-off-by: Yair Zaslavsky <yzasl...@redhat.com>
---
M backend/manager/dbscripts/async_tasks_sp.sql
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskUtils.java
M 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAO.java
M 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
M 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AsyncTaskDAOTest.java
7 files changed, 132 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/67/15567/1

diff --git a/backend/manager/dbscripts/async_tasks_sp.sql 
b/backend/manager/dbscripts/async_tasks_sp.sql
index 410c841..27c3349 100644
--- a/backend/manager/dbscripts/async_tasks_sp.sql
+++ b/backend/manager/dbscripts/async_tasks_sp.sql
@@ -92,7 +92,22 @@
 LANGUAGE plpgsql;
 
 
-CREATE OR REPLACE FUNCTION GetAsyncTasksByEntityId(v_entity_id UUID)
+Create or replace FUNCTION InsertAsyncTaskEntities(
+        v_task_id UUID,
+        v_entity_id UUID,
+        v_entity_type varchar(128))
+
+RETURNS VOID
+   AS $procedure$
+BEGIN
+      IF NOT EXISTS (SELECT 1 from async_tasks_entities where async_task_id = 
v_task_id and entity_id = v_entity_id) THEN
+            INSERT INTO async_tasks_entities 
(async_task_id,entity_id,entity_type) VALUES (v_task_id, v_entity_id, 
v_entity_type);
+      END IF;
+END; $procedure$
+LANGUAGE plpgsql;
+
+
+CREATE OR REPLACE FUNCTION GetAsyncTasksIdsByEntityId(v_entity_id UUID)
 RETURNS SETOF idUuidType
    AS $procedure$
 BEGIN
@@ -163,7 +178,7 @@
 Create or replace FUNCTION GetAsyncTasksByEntityId(v_entity_id UUID) RETURNS 
SETOF async_tasks
    AS $procedure$
 BEGIN
-   RETURN QUERY SELECT *
+   RETURN QUERY SELECT async_tasks.*
    FROM async_tasks JOIN async_tasks_entities ON async_task_id = task_id
    WHERE entity_id = v_entity_id;
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
index 5d80672..3f20ced 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
@@ -1386,9 +1386,8 @@
     protected Guid createTask(Guid taskId,
             AsyncTaskCreationInfo asyncTaskCreationInfo,
             VdcActionType parentCommand,
-            VdcObjectType entityType,
-            Guid... entityIds) {
-        return createTask(taskId, asyncTaskCreationInfo, parentCommand, null, 
entityType, entityIds);
+            Map<Guid, VdcObjectType> entitiesMap) {
+        return createTask(taskId, asyncTaskCreationInfo, parentCommand, null, 
entitiesMap);
     }
 
     /**
@@ -1426,7 +1425,30 @@
      */
     protected Guid createTask(Guid taskId, AsyncTaskCreationInfo 
asyncTaskCreationInfo,
             VdcActionType parentCommand) {
-        return createTask(taskId, asyncTaskCreationInfo, parentCommand, null, 
null, EMPTY_GUID_ARRAY);
+        return createTask(taskId,
+                asyncTaskCreationInfo,
+                parentCommand,
+                null,
+                Collections.<Guid, VdcObjectType> emptyMap());
+    }
+
+    protected Guid createTask(Guid taskId, AsyncTaskCreationInfo 
asyncTaskCreationInfo,
+            VdcActionType parentCommand,
+            VdcObjectType vdcObjectType, Guid... entityIds) {
+        return createTask(taskId,
+                asyncTaskCreationInfo,
+                parentCommand,
+                createEntitiesMapForSingleEntityType(vdcObjectType, 
entityIds));
+    }
+
+    protected Guid createTask(Guid taskId, AsyncTaskCreationInfo 
asyncTaskCreationInfo,
+            VdcActionType parentCommand,
+            String description, VdcObjectType entityType, Guid... entityIds) {
+        return createTask(taskId,
+                asyncTaskCreationInfo,
+                parentCommand,
+                description,
+                createEntitiesMapForSingleEntityType(entityType, entityIds));
     }
 
     /**
@@ -1439,20 +1461,16 @@
      *            VdcActionType of the command that its EndAction we want to 
invoke when tasks are finished.
      * @param description
      *            A message which describes the task
-     * @param entityType
-     *            type of entities that are associated with the task
-     * @param entityIds
-     *            Ids of entities to be associated with task
-     * @return Guid of the created task.
+     * @param entitiesMap - map of entities
      */
     protected Guid createTask(Guid taskId, AsyncTaskCreationInfo 
asyncTaskCreationInfo,
             VdcActionType parentCommand,
-            String description, VdcObjectType entityType, Guid... entityIds) {
+            String description, Map<Guid, VdcObjectType> entitiesMap) {
 
         Transaction transaction = TransactionSupport.suspend();
 
         try {
-            return createTaskImpl(taskId, asyncTaskCreationInfo, 
parentCommand, description, entityType, entityIds);
+            return createTaskImpl(taskId, asyncTaskCreationInfo, 
parentCommand, description, entitiesMap);
         } catch (RuntimeException ex) {
             log.errorFormat("Error during CreateTask for command: {0}. 
Exception {1}", getClass().getName(), ex);
         } finally {
@@ -1464,6 +1482,17 @@
 
     private Guid createTaskImpl(Guid taskId, AsyncTaskCreationInfo 
asyncTaskCreationInfo, VdcActionType parentCommand,
             String description, VdcObjectType entityType, Guid... entityIds) {
+        return createTaskImpl(taskId,
+                asyncTaskCreationInfo,
+                parentCommand,
+                description,
+                createEntitiesMapForSingleEntityType(entityType, entityIds));
+    }
+
+    private Guid createTaskImpl(Guid taskId,
+            AsyncTaskCreationInfo asyncTaskCreationInfo, VdcActionType 
parentCommand,
+            String description,
+            Map<Guid, VdcObjectType> entitiesMap) {
         Step taskStep =
                 ExecutionHandler.addTaskStep(getExecutionContext(),
                         
StepEnum.getStepNameByTaskType(asyncTaskCreationInfo.getTaskType()),
@@ -1472,13 +1501,21 @@
             asyncTaskCreationInfo.setStepId(taskStep.getId());
         }
         SPMAsyncTask task = concreteCreateTask(taskId, asyncTaskCreationInfo, 
parentCommand);
-        task.setEntityType(entityType);
-        task.setAssociatedEntities(entityIds);
+        task.setEntitiesMap(entitiesMap);
         AsyncTaskUtils.addOrUpdateTaskInDB(task);
         getAsyncTaskManager().lockAndAddTaskToManager(task);
         Guid vdsmTaskId = task.getVdsmTaskId();
         ExecutionHandler.updateStepExternalId(taskStep, vdsmTaskId, 
ExternalSystemType.VDSM);
         return vdsmTaskId;
+
+    }
+
+    private Map<Guid, VdcObjectType> 
createEntitiesMapForSingleEntityType(VdcObjectType entityType, Guid... 
entityIds) {
+        Map<Guid, VdcObjectType> entitiesMap = new HashMap<Guid, 
VdcObjectType>();
+        for (Guid entityId : entityIds) {
+            entitiesMap.put(entityId, entityType);
+        }
+        return entitiesMap;
     }
 
     /**
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
index 5b4866c..444a372 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
@@ -2,6 +2,8 @@
 
 import static 
org.ovirt.engine.core.common.config.ConfigValues.UknownTaskPrePollingLapse;
 
+import java.util.Map;
+
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.common.VdcObjectType;
 import org.ovirt.engine.core.common.asynctasks.AsyncTaskParameters;
@@ -25,25 +27,16 @@
     }
 
     private AsyncTaskParameters privateParameters;
-    private Guid[] associatedEntities;
-    private VdcObjectType entityType;
 
-    public void setAssociatedEntities(Guid[] associatedEntities) {
-        this.associatedEntities = associatedEntities;
+    private Map<Guid, VdcObjectType> entitiesMap;
+
+    public Map<Guid, VdcObjectType> getEntitiesMap() {
+        return entitiesMap;
     }
 
-    public Guid[] getAssociatedEntities() {
-        return associatedEntities;
+    public void setEntitiesMap(Map<Guid, VdcObjectType> entitiesMap) {
+        this.entitiesMap = entitiesMap;
     }
-
-    public VdcObjectType getEntityType() {
-        return entityType;
-    }
-
-    public void setEntityType(VdcObjectType entityType) {
-        this.entityType = entityType;
-    }
-
 
     public AsyncTaskParameters getParameters() {
         return privateParameters;
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskUtils.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskUtils.java
index 7aa4b61..4bd25b5 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskUtils.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskUtils.java
@@ -1,12 +1,19 @@
 package org.ovirt.engine.core.bll.tasks;
 
+import java.util.Map;
+import java.util.Map.Entry;
+
 import org.ovirt.engine.core.bll.SPMAsyncTask;
 import org.ovirt.engine.core.common.VdcObjectType;
 import org.ovirt.engine.core.common.businessentities.AsyncTasks;
 import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.compat.TransactionScopeOption;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
+import org.ovirt.engine.core.dao.AsyncTaskDAO;
 import org.ovirt.engine.core.utils.log.Log;
 import org.ovirt.engine.core.utils.log.LogFactory;
+import org.ovirt.engine.core.utils.transaction.TransactionMethod;
+import org.ovirt.engine.core.utils.transaction.TransactionSupport;
 
 /**
  * Helper class for async tasks handling
@@ -18,14 +25,22 @@
      * exists in DB
      * @param asyncTask task to be added or updated
      */
-    public static void addOrUpdateTaskInDB(SPMAsyncTask asyncTask) {
+    public static void addOrUpdateTaskInDB(final SPMAsyncTask asyncTask) {
         try {
             if (asyncTask.getParameters().getDbAsyncTask() != null) {
-                DbFacade.getInstance()
-                        .getAsyncTaskDao()
-                        
.saveOrUpdate(asyncTask.getParameters().getDbAsyncTask(),
-                                asyncTask.getEntityType(),
-                                asyncTask.getAssociatedEntities());
+                
TransactionSupport.executeInScope(TransactionScopeOption.Required, new 
TransactionMethod<Void>() {
+
+                    @Override
+                    public Void runInTransaction() {
+                        
addOrUpdateTaskInDB(asyncTask.getParameters().getDbAsyncTask());
+                        Map<Guid, VdcObjectType> entitiesMap = 
asyncTask.getEntitiesMap();
+                        for (Entry<Guid, VdcObjectType> entry : 
entitiesMap.entrySet()) {
+                            
getAsyncTaskDao().insertAsyncTaskEntity(asyncTask.getParameters()
+                                    .getDbAsyncTask().getTaskId(), 
entry.getKey(), entry.getValue());
+                        }
+                        return null;
+                    }
+                });
             }
         } catch (RuntimeException e) {
             log.error(String.format(
@@ -34,14 +49,17 @@
         }
     }
 
+    private static void addOrUpdateTaskInDB(AsyncTasks asyncTask) {
+        getAsyncTaskDao().saveOrUpdate(asyncTask, null, (Guid[]) null);
+    }
+
     public static void addOrUpdateTaskInDB(
             AsyncTasks task,
             VdcObjectType entityType,
             Guid... entityIds) {
         try {
             if (task != null) {
-                DbFacade.getInstance()
-                        .getAsyncTaskDao()
+                getAsyncTaskDao()
                         .saveOrUpdate(task,
                                 entityType,
                                 entityIds);
@@ -53,6 +71,11 @@
         }
     }
 
+    private static AsyncTaskDAO getAsyncTaskDao() {
+        return DbFacade.getInstance()
+                .getAsyncTaskDao();
+    }
+
     private static final Log log = LogFactory.getLog(AsyncTaskUtils.class);
 
 }
diff --git 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAO.java
 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAO.java
index 6898d3e..2dd9745 100644
--- 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAO.java
+++ 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAO.java
@@ -102,4 +102,6 @@
      * @return
      */
     List<AsyncTasks> getTasksByEntity(Guid entityId);
+
+    void insertAsyncTaskEntity(Guid taskId, Guid entityId, VdcObjectType 
value);
 }
diff --git 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
index 4edcfee..a608d65 100644
--- 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
+++ 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
@@ -193,7 +193,7 @@
     public List<Guid> getAsyncTaskIdsByEntity(Guid entityId) {
         MapSqlParameterSource parameterSource = 
getCustomMapSqlParameterSource();
         parameterSource.addValue("entity_id", entityId);
-        return getCallsHandler().executeReadList("GetAsyncTasksByEntityId", 
IdRowMapper.instance, parameterSource);
+        return getCallsHandler().executeReadList("GetAsyncTasksIdsByEntityId", 
IdRowMapper.instance, parameterSource);
     }
 
     @Override
@@ -211,4 +211,14 @@
                 AsyncTaskRowMapper.instance,
                 parameterSource);
     }
+
+    @Override
+    public void insertAsyncTaskEntity(Guid taskId, Guid entityId, 
VdcObjectType entityType) {
+        MapSqlParameterSource parameterSource = 
getCustomMapSqlParameterSource();
+        parameterSource.addValue("task_id", taskId).
+                addValue("entity_id", entityId).
+                addValue("entity_type", entityType);
+        getCallsHandler().executeModification("InsertAsyncTaskEntities", 
parameterSource);
+
+    }
 }
diff --git 
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AsyncTaskDAOTest.java
 
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AsyncTaskDAOTest.java
index 0a0686f..c7ef058 100644
--- 
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AsyncTaskDAOTest.java
+++ 
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/AsyncTaskDAOTest.java
@@ -264,4 +264,17 @@
         assertEquals(taskFromDb, newAsyncTask);
 
     }
+
+    @Test
+    public void testInsertAsyncTaskEntities() {
+        dao.save(newAsyncTask);
+        Guid newEntityId = Guid.NewGuid();
+        dao.insertAsyncTaskEntity(newAsyncTask.getTaskId(), newEntityId, 
VdcObjectType.VM);
+        List<Guid> taskIds = dao.getAsyncTaskIdsByEntity(newEntityId);
+        assertNotNull(taskIds);
+        assertEquals(1, taskIds.size());
+        Guid taskIdFromDb = taskIds.get(0);
+        assertEquals(newAsyncTask.getTaskId(), taskIdFromDb);
+
+    }
 }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8826761f833944e8a4d9159bf91863381cfb1f79
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