Lior Vernia has uploaded a new change for review. Change subject: engine: Extracted some QoS validation to NetworkQosValidator ......................................................................
engine: Extracted some QoS validation to NetworkQosValidator This to facilitate code reuse for similar validations as part of the Host QoS feature. Change-Id: I13dbb35bbd4cdbf4071e79d81eeb52667b734fcb Signed-off-by: Lior Vernia <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/RemoveNetworkQoSCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/UpdateNetworkQoSCommand.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkQosValidator.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkQosValidatorTest.java 4 files changed, 139 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/00/22600/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/RemoveNetworkQoSCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/RemoveNetworkQoSCommand.java index 6ae2e2e..70aef42 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/RemoveNetworkQoSCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/RemoveNetworkQoSCommand.java @@ -1,9 +1,8 @@ package org.ovirt.engine.core.bll.qos; - +import org.ovirt.engine.core.bll.validator.NetworkQosValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.NetworkQoSParametersBase; -import org.ovirt.engine.core.common.businessentities.network.NetworkQoS; import org.ovirt.engine.core.common.errors.VdcBllMessages; public class RemoveNetworkQoSCommand extends NetworkQoSCommandBase { @@ -15,12 +14,8 @@ @Override protected boolean canDoAction() { if (validateParameters()) { - NetworkQoS oldNetworkQoS = getNetworkQoSDao().get(getNetworkQoS().getId()); - if (oldNetworkQoS == null) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_NOT_FOUND); - } else if (!oldNetworkQoS.getStoragePoolId().equals(getNetworkQoS().getStoragePoolId())) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID); - } + NetworkQosValidator validator = new NetworkQosValidator(getNetworkQoS()); + return validate(validator.qosExists()) && validate(validator.consistentDataCenter()); } return true; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/UpdateNetworkQoSCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/UpdateNetworkQoSCommand.java index 3983d23..6b7f632 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/UpdateNetworkQoSCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/UpdateNetworkQoSCommand.java @@ -1,6 +1,7 @@ package org.ovirt.engine.core.bll.qos; +import org.ovirt.engine.core.bll.validator.NetworkQosValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.NetworkQoSParametersBase; import org.ovirt.engine.core.common.businessentities.network.NetworkQoS; @@ -15,12 +16,11 @@ @Override protected boolean canDoAction() { if (validateParameters()) { - NetworkQoS oldNetworkQoS = getNetworkQoSDao().get(getNetworkQoS().getId()); - if (oldNetworkQoS == null) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_NOT_FOUND); - } else if (!oldNetworkQoS.getStoragePoolId().equals(getNetworkQoS().getStoragePoolId())) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID); + NetworkQosValidator validator = new NetworkQosValidator(getNetworkQoS()); + if (!validate(validator.qosExists()) || !validate(validator.consistentDataCenter())) { + return false; } else { + NetworkQoS oldNetworkQoS = getNetworkQoSDao().get(getNetworkQoS().getId()); if (validateValues()) { if (!oldNetworkQoS.getName().equals(getNetworkQoS().getName())) { return validateNameNotExistInDC(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkQosValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkQosValidator.java new file mode 100644 index 0000000..4d297f0 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkQosValidator.java @@ -0,0 +1,45 @@ +package org.ovirt.engine.core.bll.validator; + +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.network.NetworkQoS; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; + +public class NetworkQosValidator { + + boolean oldQosRetrieved; + + private final NetworkQoS qos; + private NetworkQoS oldQos; + + public NetworkQosValidator(NetworkQoS qos) { + this.qos = qos; + } + + protected NetworkQoS getOldQos() { + if (!oldQosRetrieved) { + oldQos = DbFacade.getInstance().getQosDao().get(qos.getId()); + oldQosRetrieved = true; + } + return oldQos; + } + + /** + * Verify that the QoS entity had previously existed in the database. + */ + public ValidationResult qosExists() { + return (qos != null && getOldQos() == null) + ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_NOT_FOUND) + : ValidationResult.VALID; + } + + /** + * Verify that the QoS entity has the same DC ID as the one stored in the database. + */ + public ValidationResult consistentDataCenter() { + return (qos != null && (getOldQos() == null || !qos.getStoragePoolId().equals(getOldQos().getStoragePoolId()))) + ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID) + : ValidationResult.VALID; + } + +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkQosValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkQosValidatorTest.java new file mode 100644 index 0000000..0401281 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkQosValidatorTest.java @@ -0,0 +1,86 @@ +package org.ovirt.engine.core.bll.validator; + +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.failsWith; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.isValid; + +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Test; +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.network.NetworkQoS; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Guid; + +public class NetworkQosValidatorTest { + + private static final Guid DEFAULT_GUID = Guid.newGuid(); + private static final Guid OTHER_GUID = Guid.newGuid(); + + private NetworkQoS qos; + private NetworkQoS oldQos; + private NetworkQosValidator validator; + private NetworkQosValidator nullValidator; + + @Before + public void setup() { + qos = new NetworkQoS(); + oldQos = new NetworkQoS(); + + validator = spy(new NetworkQosValidator(qos)); + doReturn(oldQos).when(validator).getOldQos(); + + nullValidator = spy(new NetworkQosValidator(null)); + doReturn(oldQos).when(nullValidator).getOldQos(); + } + + private void qosExistsTest(Matcher<ValidationResult> matcher, NetworkQosValidator validator) { + assertThat(validator.qosExists(), matcher); + } + + @Test + public void qosExistsNullInput() { + oldQos.setId(DEFAULT_GUID); + qosExistsTest(isValid(), nullValidator); + } + + @Test + public void qosDoesNotExistNullInput() { + doReturn(null).when(nullValidator).getOldQos(); + qosExistsTest(isValid(), nullValidator); + } + + @Test + public void qosExists() { + qos.setId(DEFAULT_GUID); + oldQos.setId(DEFAULT_GUID); + qosExistsTest(isValid(), validator); + } + + @Test + public void qosDoesNotExist() { + qos.setId(DEFAULT_GUID); + doReturn(null).when(validator).getOldQos(); + qosExistsTest(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_NOT_FOUND), validator); + } + + private void consistentDataCenterTest(Matcher<ValidationResult> matcher) { + assertThat(validator.consistentDataCenter(), matcher); + } + + @Test + public void sameDataCenter() { + qos.setStoragePoolId(DEFAULT_GUID); + oldQos.setStoragePoolId(DEFAULT_GUID); + consistentDataCenterTest(isValid()); + } + + @Test + public void differentDataCenter() { + qos.setStoragePoolId(DEFAULT_GUID); + oldQos.setStoragePoolId(OTHER_GUID); + consistentDataCenterTest(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID)); + } +} -- To view, visit http://gerrit.ovirt.org/22600 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I13dbb35bbd4cdbf4071e79d81eeb52667b734fcb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
