Allon Mureinik has uploaded a new change for review.
Change subject: core: CommandsBase: refactor task creation
......................................................................
core: CommandsBase: refactor task creation
In order to create async tasks, CommandBase expects inheriting classes
to override the ConcreteCreateTask method. For classes that do not
overwrite it, an UnsupportedOperationException is thrown.
However, most places that override this method contain the exact same
code, where the only difference is the type of the task created.
This patch suggests a new mechanism so that each inheriting class will
just need to specify what type of task it requires. For commands that do
not specify the type, an UnsupportedOperationException will still be
thrown.
This patch contains the refactoring in CommandBase, the first usecase
for this mechanism as an example (using CreateSnapshotCommand) and a
test case that proves that the behavior has not changed.
If this patch is approved, subsequent patches will apply the new
mechanism to further commands.
Change-Id: I5a4c77067d43ba5c2dbaf46ff855ac14d29de2ff
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
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/CreateSnapshotCommand.java
A
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java
3 files changed, 142 insertions(+), 20 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/92/7792/1
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 0aa6b18..7ed48f6 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
@@ -33,14 +33,19 @@
import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.action.VdcReturnValueBase;
import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo;
+import org.ovirt.engine.core.common.asynctasks.AsyncTaskParameters;
+import org.ovirt.engine.core.common.asynctasks.AsyncTaskType;
import org.ovirt.engine.core.common.businessentities.ActionGroup;
+import org.ovirt.engine.core.common.businessentities.AsyncTaskResultEnum;
import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus;
+import org.ovirt.engine.core.common.businessentities.AsyncTaskStatusEnum;
import org.ovirt.engine.core.common.businessentities.BusinessEntity;
import org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot;
import
org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot.EntityStatusSnapshot;
import
org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot.SnapshotType;
import org.ovirt.engine.core.common.businessentities.QuotaEnforcementTypeEnum;
import org.ovirt.engine.core.common.businessentities.action_version_map;
+import org.ovirt.engine.core.common.businessentities.async_tasks;
import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.businessentities.tags;
import org.ovirt.engine.core.common.errors.VdcBLLException;
@@ -1100,8 +1105,23 @@
* @return An {@link SPMAsyncTask} object representing the task to be run
*/
protected SPMAsyncTask ConcreteCreateTask
- (AsyncTaskCreationInfo asyncTaskCreationInfo,
- VdcActionType parentCommand) {
+ (AsyncTaskCreationInfo asyncTaskCreationInfo, VdcActionType
parentCommand) {
+
+ VdcActionParametersBase parametersForTask =
getParametersForTask(parentCommand, getParameters());
+ AsyncTaskParameters p =
+ new AsyncTaskParameters(asyncTaskCreationInfo, new
async_tasks(parentCommand,
+ AsyncTaskResultEnum.success,
+ AsyncTaskStatusEnum.running,
+ asyncTaskCreationInfo.getTaskID(),
+ parametersForTask,
+ asyncTaskCreationInfo.getStepId(),
+ getCommandId()));
+ p.setEntityId(getParameters().getEntityId());
+ return AsyncTaskManager.getInstance().CreateTask(getTaskType(), p);
+ }
+
+ /** @return The type of task that should be created for this command.
Commands that do not create async tasks should throw a {@link
UnsupportedOperationException} */
+ protected AsyncTaskType getTaskType() {
throw new UnsupportedOperationException();
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateSnapshotCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateSnapshotCommand.java
index c11839b..21a0ea4 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateSnapshotCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateSnapshotCommand.java
@@ -8,16 +8,10 @@
import org.ovirt.engine.core.common.action.ImagesActionsParametersBase;
import org.ovirt.engine.core.common.action.ImagesContainterParametersBase;
import org.ovirt.engine.core.common.action.VdcActionParametersBase;
-import org.ovirt.engine.core.common.action.VdcActionType;
-import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo;
-import org.ovirt.engine.core.common.asynctasks.AsyncTaskParameters;
import org.ovirt.engine.core.common.asynctasks.AsyncTaskType;
-import org.ovirt.engine.core.common.businessentities.AsyncTaskResultEnum;
-import org.ovirt.engine.core.common.businessentities.AsyncTaskStatusEnum;
import org.ovirt.engine.core.common.businessentities.DiskImage;
import org.ovirt.engine.core.common.businessentities.VolumeFormat;
import org.ovirt.engine.core.common.businessentities.VolumeType;
-import org.ovirt.engine.core.common.businessentities.async_tasks;
import org.ovirt.engine.core.common.errors.VdcBLLException;
import org.ovirt.engine.core.common.errors.VdcBllErrors;
import
org.ovirt.engine.core.common.vdscommands.CreateSnapshotVDSCommandParameters;
@@ -132,18 +126,8 @@
}
@Override
- protected SPMAsyncTask ConcreteCreateTask(AsyncTaskCreationInfo
asyncTaskCreationInfo, VdcActionType parentCommand) {
- VdcActionParametersBase parametersForTask =
getParametersForTask(parentCommand, getParameters());
- AsyncTaskParameters p =
- new AsyncTaskParameters(asyncTaskCreationInfo, new
async_tasks(parentCommand,
- AsyncTaskResultEnum.success,
- AsyncTaskStatusEnum.running,
- asyncTaskCreationInfo.getTaskID(),
- parametersForTask,
- asyncTaskCreationInfo.getStepId(),
- getCommandId()));
- p.setEntityId(getParameters().getEntityId());
- return
AsyncTaskManager.getInstance().CreateTask(AsyncTaskType.createVolume, p);
+ protected AsyncTaskType getTaskType() {
+ return AsyncTaskType.createVolume;
}
/**
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
new file mode 100644
index 0000000..edba288
--- /dev/null
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java
@@ -0,0 +1,118 @@
+package org.ovirt.engine.core.bll;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.theories.DataPoints;
+import org.junit.experimental.theories.Theories;
+import org.junit.experimental.theories.Theory;
+import org.junit.runner.RunWith;
+import org.ovirt.engine.core.common.action.ImagesActionsParametersBase;
+import org.ovirt.engine.core.common.action.PermissionsOperationsParametes;
+import org.ovirt.engine.core.common.action.VdcActionParametersBase;
+import org.ovirt.engine.core.common.action.VdcActionType;
+import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo;
+import org.ovirt.engine.core.common.asynctasks.AsyncTaskType;
+import org.ovirt.engine.core.common.businessentities.AsyncTaskResultEnum;
+import org.ovirt.engine.core.common.businessentities.AsyncTaskStatusEnum;
+import org.ovirt.engine.core.common.config.ConfigValues;
+import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.utils.MockConfigRule;
+import org.ovirt.engine.core.utils.RandomUtils;
+import org.ovirt.engine.core.utils.RandomUtilsSeedingRule;
+import org.ovirt.engine.core.utils.ejb.BeanProxyType;
+import org.ovirt.engine.core.utils.ejb.BeanType;
+import org.ovirt.engine.core.utils.ejb.EJBUtilsStrategy;
+import org.ovirt.engine.core.utils.ejb.EjbUtils;
+import org.ovirt.engine.core.utils.ejb.EngineEJBUtilsStrategy;
+import org.ovirt.engine.core.utils.timer.SchedulerUtil;
+
+/**
+ * A test for task creation in the various commands.
+ * It's mainly intended to observe that future refactoring of these commands
will not break current behavior.
+ */
+@RunWith(Theories.class)
+public class BackwardCompatibilityTaskCreationTest {
+
+ @Rule
+ public static final RandomUtilsSeedingRule rusr = new
RandomUtilsSeedingRule();
+
+ @Rule
+ public static final MockConfigRule mcr = new MockConfigRule(
+ mockConfig(ConfigValues.AsyncTaskPollingRate, 10),
+ mockConfig(ConfigValues.AsyncTaskStatusCacheRefreshRateInSeconds,
10),
+ mockConfig(ConfigValues.AsyncTaskStatusCachingTimeInMinutes, 10)
+ );
+
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ @DataPoints
+ public static CommandBase<? extends VdcActionParametersBase>[] data() {
+ return new CommandBase<?>[] {
+ new CreateSnapshotCommand(new ImagesActionsParametersBase())
+ };
+ }
+
+ @Before
+ public void setUp() {
+ EJBUtilsStrategy ejbStrategy = mock(EJBUtilsStrategy.class);
+ SchedulerUtil sched = mock(SchedulerUtil.class);
+ when(ejbStrategy.<SchedulerUtil> findBean(BeanType.SCHEDULER,
BeanProxyType.LOCAL)).thenReturn(sched);
+ EjbUtils.setStrategy(ejbStrategy);
+ }
+
+ @After
+ public void tearDown() {
+ EjbUtils.setStrategy(new EngineEJBUtilsStrategy());
+ }
+
+ @Theory
+ public void testConcreateCreateTaskBackwardsComaptibility(CommandBase<?
extends VdcActionParametersBase> cmd) {
+ VdcActionParametersBase params = cmd.getParameters();
+ params.setEntityId(Guid.NewGuid());
+
params.setParentCommand(RandomUtils.instance().nextEnum(VdcActionType.class));
+ params.setParentParemeters(params);
+
+ AsyncTaskCreationInfo info = nextAsyncTaskCreationInfo();
+
+ SPMAsyncTask spmAsyncTask = cmd.ConcreteCreateTask(info,
cmd.getParameters().getParentCommand());
+ assertEquals("wrong storage pool ID", info.getStoragePoolID(),
spmAsyncTask.getStoragePoolID());
+ assertEquals("wrong task ID", info.getTaskID(),
spmAsyncTask.getTaskID());
+ 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);
+ }
+
+ /**
+ * Tests that a purely engine command, with no async tasks throws the
+ * correct exception when
+ * {@link CommandBase#ConcreteCreateTask(AsyncTaskCreationInfo,
VdcActionType)}
+ * is called.
+ *
+ * Note: {@link AddPermissionCommand} is merely used as an example here.
+ */
+ @Test(expected = UnsupportedOperationException.class)
+ public void testExceptionForCommandWithNoTasks() {
+ PermissionsOperationsParametes params = new
PermissionsOperationsParametes();
+ AddPermissionCommand<PermissionsOperationsParametes> cmd =
+ new
AddPermissionCommand<PermissionsOperationsParametes>(params);
+ cmd.ConcreteCreateTask(nextAsyncTaskCreationInfo(),
VdcActionType.Unknown);
+ }
+
+ /** @return A randomly generated {@link AsyncTaskCreationInfo} object */
+ private static AsyncTaskCreationInfo nextAsyncTaskCreationInfo() {
+ AsyncTaskCreationInfo info = new AsyncTaskCreationInfo();
+ info.setStepId(Guid.NewGuid());
+ info.setStoragePoolID(Guid.NewGuid());
+ info.setTaskID(Guid.NewGuid());
+ info.setTaskType(RandomUtils.instance().nextEnum(AsyncTaskType.class));
+ return info;
+ }
+}
--
To view, visit http://gerrit.ovirt.org/7792
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5a4c77067d43ba5c2dbaf46ff855ac14d29de2ff
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
