Lior Vernia has uploaded a new change for review. Change subject: engine: Moved more QoS validation to NetworkQosValidator ......................................................................
engine: Moved more QoS validation to NetworkQosValidator A few more methods will now be used elsewhere in the code, so they were also moved into this class. Change-Id: I6b5b374ff7da21de19851bdff6df4a2f53f0dd0e Signed-off-by: Lior Vernia <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/AddNetworkQoSCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/NetworkQoSCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/UpdateNetworkQoSCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkQosValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/NetworkQosValidatorTest.java 5 files changed, 105 insertions(+), 41 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/65/22765/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/AddNetworkQoSCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/AddNetworkQoSCommand.java index d9ba345..9981ee9 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/AddNetworkQoSCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/AddNetworkQoSCommand.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.errors.VdcBllMessages; @@ -14,9 +15,11 @@ @Override protected boolean canDoAction() { + NetworkQosValidator validator = new NetworkQosValidator(getNetworkQoS()); return validateParameters() && validateNameNotExistInDC() - && validateValues(); + && validate(validator.allValuesPresent()) + && validate(validator.peakConsistentWithAverage()); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/NetworkQoSCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/NetworkQoSCommandBase.java index 1e0eb49..bb393ba 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/NetworkQoSCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/NetworkQoSCommandBase.java @@ -58,39 +58,6 @@ return true; } - protected boolean validateValues() { - if(missingValues()) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_MISSING_VALUES); - } - - if (peakLowerThanAverage()) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_PEAK_LOWER_THAN_AVERAGE); - } - - return true; - } - - protected boolean peakLowerThanAverage() { - return (getNetworkQoS().getInboundPeak() != null - && getNetworkQoS().getInboundPeak() < getNetworkQoS().getInboundAverage()) - || (getNetworkQoS().getOutboundPeak() != null - && getNetworkQoS().getOutboundPeak() < getNetworkQoS().getOutboundAverage()); - } - - protected boolean missingValues() { - return missingValue(getNetworkQoS().getInboundAverage(), - getNetworkQoS().getInboundPeak(), - getNetworkQoS().getInboundBurst()) - || missingValue(getNetworkQoS().getOutboundAverage(), - getNetworkQoS().getOutboundPeak(), - getNetworkQoS().getOutboundBurst()); - } - - private boolean missingValue(Integer average, Integer peak, Integer burst) { - return (average != null || peak != null || burst != null) - && (average == null || peak == null || burst == null); - } - @Override public List<PermissionSubject> getPermissionCheckSubjects() { return Collections.singletonList(new PermissionSubject(getStoragePoolId(), 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 6b7f632..79c48b2 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 @@ -17,16 +17,15 @@ protected boolean canDoAction() { if (validateParameters()) { NetworkQosValidator validator = new NetworkQosValidator(getNetworkQoS()); - if (!validate(validator.qosExists()) || !validate(validator.consistentDataCenter())) { + if (!validate(validator.qosExists()) + || !validate(validator.consistentDataCenter()) + || !validate(validator.allValuesPresent()) + || !validate(validator.peakConsistentWithAverage())) { return false; } else { NetworkQoS oldNetworkQoS = getNetworkQoSDao().get(getNetworkQoS().getId()); - if (validateValues()) { - if (!oldNetworkQoS.getName().equals(getNetworkQoS().getName())) { - return validateNameNotExistInDC(); - } - } else { - return false; + 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 index ca56c03..8ad1ea5 100644 --- 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 @@ -39,4 +39,33 @@ : ValidationResult.VALID; } + /** + * Verify that if any inbound/outbound capping was specified, that all three parameters are present. + */ + public ValidationResult allValuesPresent() { + return (qos != null) + && (missingValue(qos.getInboundAverage(), qos.getInboundPeak(), qos.getInboundBurst()) + || missingValue(qos.getOutboundAverage(), qos.getOutboundPeak(), qos.getOutboundBurst())) + ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_MISSING_VALUES) + : ValidationResult.VALID; + } + + private boolean missingValue(Integer average, Integer peak, Integer burst) { + return (average != null || peak != null || burst != null) && (average == null || peak == null || burst == null); + } + + /** + * Verify that the specified peak value isn't lower than the specified average value. + */ + public ValidationResult peakConsistentWithAverage() { + return (qos != null) && (peakLowerThanAverage(qos.getInboundAverage(), qos.getInboundPeak()) + || peakLowerThanAverage(qos.getOutboundAverage(), qos.getOutboundPeak())) + ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_PEAK_LOWER_THAN_AVERAGE) + : ValidationResult.VALID; + } + + private boolean peakLowerThanAverage(Integer average, Integer peak) { + return peak != null && peak < average; + } + } 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 index 0401281..e33701d 100644 --- 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 @@ -18,6 +18,9 @@ private static final Guid DEFAULT_GUID = Guid.newGuid(); private static final Guid OTHER_GUID = Guid.newGuid(); + private static final Integer BANDWIDTH_LOW = 10; + private static final Integer BANDWIDTH_MEDIUM = 100; + private static final Integer BANDWIDTH_HIGH = 1000; private NetworkQoS qos; private NetworkQoS oldQos; @@ -83,4 +86,67 @@ oldQos.setStoragePoolId(OTHER_GUID); consistentDataCenterTest(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID)); } + + private void valuesPresentTest(Matcher<ValidationResult> matcher) { + assertThat(validator.allValuesPresent(), matcher); + } + + @Test + public void valuesPresentNullInput() { + assertThat(nullValidator.allValuesPresent(), isValid()); + } + + @Test + public void noValuePresent() { + valuesPresentTest(isValid()); + } + + @Test + public void allValuesPresent() { + qos.setInboundAverage(BANDWIDTH_MEDIUM); + qos.setInboundPeak(BANDWIDTH_MEDIUM); + qos.setInboundBurst(BANDWIDTH_MEDIUM); + valuesPresentTest(isValid()); + } + + @Test + public void onlyAveragePresent() { + qos.setInboundAverage(BANDWIDTH_MEDIUM); + valuesPresentTest(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_MISSING_VALUES)); + } + + @Test + public void burstMissing() { + qos.setInboundAverage(BANDWIDTH_MEDIUM); + qos.setInboundPeak(BANDWIDTH_MEDIUM); + valuesPresentTest(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_MISSING_VALUES)); + } + + private void peakConsistentWithAverageTest(Matcher<ValidationResult> matcher) { + qos.setInboundAverage(BANDWIDTH_MEDIUM); + assertThat(validator.peakConsistentWithAverage(), matcher); + } + + @Test + public void peakConsistentWithAverageNullInput() { + assertThat(nullValidator.peakConsistentWithAverage(), isValid()); + } + + @Test + public void peakHigherThanAverage() { + qos.setInboundPeak(BANDWIDTH_HIGH); + peakConsistentWithAverageTest(isValid()); + } + + @Test + public void peakEqualToAverage() { + qos.setInboundPeak(BANDWIDTH_MEDIUM); + peakConsistentWithAverageTest(isValid()); + } + + @Test + public void peakLowerThanAverage() { + qos.setInboundPeak(BANDWIDTH_LOW); + peakConsistentWithAverageTest(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_PEAK_LOWER_THAN_AVERAGE)); + } } -- To view, visit http://gerrit.ovirt.org/22765 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6b5b374ff7da21de19851bdff6df4a2f53f0dd0e 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
