Allon Mureinik has uploaded a new change for review.
Change subject: engine: VdcActionUtils: Remove reflection
......................................................................
engine: VdcActionUtils: Remove reflection
Improved VdcActionUtils performance and readability by removing the
logic implemented by reflection and instead depend on proper Java
interfaces.
This patch includes:
1. A new interface, BusinessEntityWithStatus<?, ?> which could be
applied to any BusinessEntity<?> which has a notion of a status.
2. Applying the BusinessEntityWithStatus interface to VM, VmTemplate,
VDS and StorageDoamin, which are used by VdcActionUtils.
3. Applying the BusinessEntityWithStatus interface to StoragePool,
which is not used by VdcActionUtils, as a negative test case.
4. Updating the test case accordingly.
Benefits of this approach:
1. Less programmer errors - some of the validations are done in compile
time instead of runtime (i.e., you cannot pass any object to
VdcActionUtils.canExecute, only classes that implement
BusinessEntityWithStatus.
2. Better performance - no usage of reflection, which is known to have
a performance penalty.
3. The code is more straight forward, and easier to understand.
Change-Id: I4b1c3857e1237bef9fc1af96e96043677b2debd4
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/VdcActionUtils.java
A
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BusinessEntityWithStatus.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmTemplate.java
M
backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/VdcActionUtilsTest.java
M
frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
9 files changed, 42 insertions(+), 39 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/55/18855/1
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/VdcActionUtils.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/VdcActionUtils.java
index 8d9cafb..ca0c942 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/VdcActionUtils.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/VdcActionUtils.java
@@ -7,6 +7,7 @@
import java.util.Set;
import org.ovirt.engine.core.common.action.VdcActionType;
+import org.ovirt.engine.core.common.businessentities.BusinessEntityWithStatus;
import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
import org.ovirt.engine.core.common.businessentities.VDS;
@@ -15,7 +16,6 @@
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmTemplate;
import org.ovirt.engine.core.common.businessentities.VmTemplateStatus;
-import org.ovirt.engine.core.compat.NotImplementedException;
public final class VdcActionUtils {
@@ -294,41 +294,17 @@
_matrix.put(StorageDomain.class, storageDomainMatrix);
}
- public static boolean canExecute(List<?> entities, Class<?> type,
VdcActionType action) {
+ public static boolean canExecute(List<? extends
BusinessEntityWithStatus<?, ?>> entities,
+ Class type,
+ VdcActionType action) {
if (_matrix.containsKey(type)) {
- for (Object a : entities) {
- if (a.getClass() == type &&
_matrix.get(type).containsKey(getStatusProperty(a))
- &&
_matrix.get(type).get(getStatusProperty(a)).contains(action)) {
+ for (BusinessEntityWithStatus<?, ?> a : entities) {
+ if (a.getClass() == type &&
_matrix.get(type).containsKey(a.getStatus())
+ &&
_matrix.get(type).get(a.getStatus()).contains(action)) {
return false;
}
}
}
return true;
}
-
- private static Enum<?> getStatusProperty(Object entity) {
- if (entity.getClass().getName().endsWith("VDS")) {
- return entity instanceof VDS ?
- ((VDS) entity).getStatus() :
- null;
- }
- if (entity.getClass().getName().endsWith("VM")) {
- return entity instanceof VM ?
- ((VM) entity).getStatus() :
- null;
- }
- if (entity.getClass().getName().endsWith("VmTemplate")) {
- return entity instanceof VmTemplate ?
- ((VmTemplate) entity).getStatus() :
- null;
-
- }
- if (entity instanceof StorageDomain) {
- StorageDomainStatus status = ((StorageDomain) entity).getStatus();
- return status != null ? status : StorageDomainStatus.Uninitialized;
- }
-
- throw new NotImplementedException();
- }
-
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BusinessEntityWithStatus.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BusinessEntityWithStatus.java
new file mode 100644
index 0000000..6bbfe11
--- /dev/null
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BusinessEntityWithStatus.java
@@ -0,0 +1,12 @@
+package org.ovirt.engine.core.common.businessentities;
+
+import java.io.Serializable;
+
+/**
+ * A {@link }BusinessEntity} with a status enum.
+ */
+public interface BusinessEntityWithStatus<ID extends Serializable, Status
extends Enum<?>> extends BusinessEntity<ID> {
+ Status getStatus();
+
+ void setStatus(Status status);
+}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java
index 01d5078..f2e99c7 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java
@@ -7,7 +7,7 @@
import org.ovirt.engine.core.common.utils.ObjectUtils;
import org.ovirt.engine.core.compat.Guid;
-public class StorageDomain extends IVdcQueryable implements
BusinessEntity<Guid>, Nameable, Commented {
+public class StorageDomain extends IVdcQueryable implements
BusinessEntityWithStatus<Guid, StorageDomainStatus>, Nameable, Commented {
private static final long serialVersionUID = -6162192446628804305L;
public StorageDomain() {
@@ -196,10 +196,12 @@
}
}
+ @Override
public StorageDomainStatus getStatus() {
return getStoragePoolIsoMapData().getstatus();
}
+ @Override
public void setStatus(StorageDomainStatus status) {
getStoragePoolIsoMapData().setstatus(status);
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java
index 60709df..57d1179 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java
@@ -11,7 +11,7 @@
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.compat.Version;
-public class StoragePool extends IVdcQueryable implements
BusinessEntity<Guid>, Nameable, Commented {
+public class StoragePool extends IVdcQueryable implements
BusinessEntityWithStatus<Guid, StoragePoolStatus>, Nameable, Commented {
private static final long serialVersionUID = 6162262095329980112L;
private Guid id;
@@ -121,10 +121,12 @@
storagePoolFormatType = value;
}
+ @Override
public StoragePoolStatus getStatus() {
return status;
}
+ @Override
public void setStatus(StoragePoolStatus value) {
status = value;
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
index 585669d..b1d7a26 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
@@ -14,7 +14,7 @@
import org.ovirt.engine.core.compat.RpmVersion;
import org.ovirt.engine.core.compat.Version;
-public class VDS extends IVdcQueryable implements Serializable,
BusinessEntity<Guid>, HasStoragePool<Guid>, Commented, Nameable, Cloneable {
+public class VDS extends IVdcQueryable implements Serializable,
BusinessEntityWithStatus<Guid, VDSStatus>, HasStoragePool<Guid>, Commented,
Nameable, Cloneable {
private static final long serialVersionUID = -7893976203379789926L;
private VdsStatic mVdsStatic;
private VdsDynamic mVdsDynamic;
@@ -360,10 +360,12 @@
this.mVdsStatic.setVdsType(value);
}
+ @Override
public VDSStatus getStatus() {
return this.mVdsDynamic.getstatus();
}
+ @Override
public void setStatus(VDSStatus value) {
this.mVdsDynamic.setstatus(value);
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
index f48e275..e96c672 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
@@ -18,7 +18,7 @@
import org.ovirt.engine.core.compat.StringHelper;
import org.ovirt.engine.core.compat.Version;
-public class VM extends IVdcQueryable implements Serializable,
BusinessEntity<Guid>, HasStoragePool<Guid>, Nameable, Commented {
+public class VM extends IVdcQueryable implements Serializable,
BusinessEntityWithStatus<Guid, VMStatus>, HasStoragePool<Guid>, Nameable,
Commented {
private static final long serialVersionUID = -4078140531074414263L;
@Valid
private VmStatic vmStatic;
@@ -402,10 +402,12 @@
private Guid vmPoolId;
private String vmPoolName;
+ @Override
public VMStatus getStatus() {
return this.vmDynamic.getStatus();
}
+ @Override
public void setStatus(VMStatus value) {
this.vmDynamic.setStatus(value);
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmTemplate.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmTemplate.java
index 467c329..da366ab 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmTemplate.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmTemplate.java
@@ -19,7 +19,7 @@
import org.ovirt.engine.core.common.validation.group.UpdateEntity;
import org.ovirt.engine.core.compat.Guid;
-public class VmTemplate extends VmBase {
+public class VmTemplate extends VmBase implements
BusinessEntityWithStatus<Guid, VmTemplateStatus> {
private static final long serialVersionUID = -5238366659716600486L;
private List<VmNetworkInterface> interfaces;
@@ -120,10 +120,12 @@
this.childCount = value;
}
+ @Override
public VmTemplateStatus getStatus() {
return status;
}
+ @Override
public void setStatus(VmTemplateStatus value) {
status = value;
}
diff --git
a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/VdcActionUtilsTest.java
b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/VdcActionUtilsTest.java
index daca90b..929152d 100644
---
a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/VdcActionUtilsTest.java
+++
b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/VdcActionUtilsTest.java
@@ -11,6 +11,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.ovirt.engine.core.common.action.VdcActionType;
+import org.ovirt.engine.core.common.businessentities.BusinessEntityWithStatus;
import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
import org.ovirt.engine.core.common.businessentities.StoragePool;
@@ -56,14 +57,14 @@
});
}
- public VdcActionUtilsTest(Object toTest, VdcActionType action, boolean
result) {
+ public VdcActionUtilsTest(BusinessEntityWithStatus<?, ?> toTest,
VdcActionType action, boolean result) {
this.toTest = toTest;
this.action = action;
this.result = result;
}
/** The object to test. */
- private Object toTest;
+ private BusinessEntityWithStatus<?, ?> toTest;
/** The action to test */
private VdcActionType action;
@@ -75,7 +76,10 @@
@Test
public void canExecute() {
- assertEquals(result,
VdcActionUtils.canExecute(Collections.singletonList(toTest), toTest.getClass(),
action));
+ assertEquals(result,
+
VdcActionUtils.canExecute(Collections.<BusinessEntityWithStatus<?, ?>>
singletonList(toTest),
+ toTest.getClass(),
+ action));
}
}
diff --git
a/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
b/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
index 723f954..2aa3ccd 100644
---
a/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
+++
b/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
@@ -11,6 +11,7 @@
<!-- Business entities -->
<include name="common/businessentities/HasStoragePool.java" />
<include name="common/businessentities/BusinessEntity.java" />
+ <include
name="common/businessentities/BusinessEntityWithStatus.java" />
<include name="common/businessentities/LdapGroup.java" />
<include
name="common/businessentities/BusinessEntitiesDefinitions.java" />
<include name="common/businessentities/LdapUser.java" />
--
To view, visit http://gerrit.ovirt.org/18855
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4b1c3857e1237bef9fc1af96e96043677b2debd4
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
