From fe9a996ee141175beac2f1c2c3bc3c0b8ea288c1 Mon Sep 17 00:00:00 2001 From: Natalia Kislicyn Date: Fri, 8 Mar 2019 15:41:36 +0100 Subject: [PATCH 1/4] Changed min value of target percentage range to 1; Fixed NullPointerException in groups validation callback; Signed-off-by: Natalia Kislicyn --- .../ui/rollout/rollout/DefineGroupsLayout.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java index a58cdcf0de..a18f9a9143 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java @@ -340,10 +340,12 @@ private void setGroupsValidation(final RolloutGroupsValidation validation) { continue; } row.resetError(); - final Long count = groupsValidation.getTargetsPerGroup().get(i); - if (count != null && count > maxTargets) { - row.setError(i18n.getMessage(MESSAGE_ROLLOUT_MAX_GROUP_SIZE_EXCEEDED, maxTargets)); - setValidationStatus(ValidationStatus.INVALID); + if (groupsValidation != null) { + final Long count = groupsValidation.getTargetsPerGroup().get(i); + if (count != null && count > maxTargets) { + row.setError(i18n.getMessage(MESSAGE_ROLLOUT_MAX_GROUP_SIZE_EXCEEDED, maxTargets)); + setValidationStatus(ValidationStatus.INVALID); + } } } @@ -462,12 +464,12 @@ private void removeGroupRow(final GroupRow groupRow) { private void validateMandatoryPercentage(final Object value) { if (value != null) { - final String message = i18n.getMessage("message.rollout.field.value.range", 0, 100); + final String message = i18n.getMessage("message.rollout.field.value.range", 1, 100); if (value instanceof Float) { - new FloatRangeValidator(message, 0F, 100F).validate(value); + new FloatRangeValidator(message, 1F, 100F).validate(value); } if (value instanceof Integer) { - new IntegerRangeValidator(message, 0, 100).validate(value); + new IntegerRangeValidator(message, 1, 100).validate(value); } } else { throw new Validator.EmptyValueException(i18n.getMessage("message.enter.number")); From 7b7f22537aa2914e54d0d2dcefe9cd1a0866275e Mon Sep 17 00:00:00 2001 From: Natalia Kislicyn Date: Mon, 11 Mar 2019 15:49:24 +0100 Subject: [PATCH 2/4] Extracted single group validation into an own method Signed-off-by: Natalia Kislicyn --- .../ui/rollout/rollout/DefineGroupsLayout.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java index a18f9a9143..b8250ee6fb 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java @@ -331,24 +331,29 @@ private void setGroupsValidation(final RolloutGroupsValidation validation) { } // validate the single groups - final int maxTargets = quotaManagement.getMaxTargetsPerRolloutGroup(); final boolean hasRemainingTargetsError = validationStatus == ValidationStatus.INVALID; + singleGroupsValidation(validation, lastRow, hasRemainingTargetsError); + } + + private void singleGroupsValidation(final RolloutGroupsValidation validation, final GroupRow lastRow, + final boolean remainingError) { + final int maxTargets = quotaManagement.getMaxTargetsPerRolloutGroup(); + for (int i = 0; i < groupRows.size(); ++i) { final GroupRow row = groupRows.get(i); // do not mask the 'remaining targets' error - if (hasRemainingTargetsError && row.equals(lastRow)) { + if (remainingError && row.equals(lastRow)) { continue; } row.resetError(); - if (groupsValidation != null) { - final Long count = groupsValidation.getTargetsPerGroup().get(i); + if (validation != null) { + final Long count = validation.getTargetsPerGroup().get(i); if (count != null && count > maxTargets) { row.setError(i18n.getMessage(MESSAGE_ROLLOUT_MAX_GROUP_SIZE_EXCEEDED, maxTargets)); setValidationStatus(ValidationStatus.INVALID); } } } - } private List getGroupsFromRows() { From aed9d03d51a26441ee60a12c7a0b45c006d31963 Mon Sep 17 00:00:00 2001 From: Natalia Kislicyn Date: Fri, 26 Apr 2019 16:03:05 +0200 Subject: [PATCH 3/4] Exclude value 0 from target percentage range; Use current local on string format; Signed-off-by: Natalia Kislicyn --- .../rollout/rollout/DefineGroupsLayout.java | 19 ++++++++++++++----- .../src/main/resources/messages.properties | 1 + 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java index b8250ee6fb..67ed3a85b4 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java @@ -48,6 +48,7 @@ import com.vaadin.data.Validator; import com.vaadin.data.util.converter.StringToFloatConverter; import com.vaadin.data.util.converter.StringToIntegerConverter; +import com.vaadin.data.validator.RangeValidator; import com.vaadin.data.validator.FloatRangeValidator; import com.vaadin.data.validator.IntegerRangeValidator; import com.vaadin.server.FontAwesome; @@ -62,6 +63,8 @@ import com.vaadin.ui.TextField; import com.vaadin.ui.UI; +import static org.eclipse.hawkbit.ui.utils.HawkbitCommonUtil.getCurrentLocale; + /** * Define groups for a Rollout */ @@ -469,13 +472,19 @@ private void removeGroupRow(final GroupRow groupRow) { private void validateMandatoryPercentage(final Object value) { if (value != null) { - final String message = i18n.getMessage("message.rollout.field.value.range", 1, 100); + final String message = i18n.getMessage("message.rollout.field.value.range", 0, 100); + RangeValidator rangeValidator; if (value instanceof Float) { - new FloatRangeValidator(message, 1F, 100F).validate(value); + rangeValidator = new FloatRangeValidator(message, 0F, 100F); + } + else if (value instanceof Integer) { + rangeValidator = new IntegerRangeValidator(message, 0, 100); } - if (value instanceof Integer) { - new IntegerRangeValidator(message, 1, 100).validate(value); + else { + throw new Validator.InvalidValueException(i18n.getMessage("message.rollout.field.value.type")); } + rangeValidator.setMinValueIncluded(false); + rangeValidator.validate(value); } else { throw new Validator.EmptyValueException(i18n.getMessage("message.enter.number")); } @@ -622,7 +631,7 @@ public void populateByGroup(final RolloutGroup group) { targetFilterQuery.setValue(group.getTargetFilterQuery()); populateTargetFilterQuery(group); - targetPercentage.setValue(String.format("%.2f", group.getTargetPercentage())); + targetPercentage.setValue(String.format(getCurrentLocale(), "%.2f", group.getTargetPercentage())); triggerThreshold.setValue(group.getSuccessConditionExp()); errorThreshold.setValue(group.getErrorConditionExp()); diff --git a/hawkbit-ui/src/main/resources/messages.properties b/hawkbit-ui/src/main/resources/messages.properties index 940bf1e1a8..23c3ce173c 100644 --- a/hawkbit-ui/src/main/resources/messages.properties +++ b/hawkbit-ui/src/main/resources/messages.properties @@ -616,6 +616,7 @@ message.rollout.name.empty = Please enter a name for Rollout message.correct.invalid.value = Please correct invalid values message.enter.number = Please enter number message.rollout.field.value.range = Value should be in range {0} to {1} +message.rollout.field.value.type = Invalid type of value message.rollout.filter.target.exists = The selected target filter does not match any existing target message.rollout.max.group.size.exceeded = The maximum group size of {0} targets is exceeded. Please increase the number of groups or select a different target filter message.rollout.max.group.size.exceeded.advanced = The maximum size of {0} targets is exceeded for this group. Please re-distribute the targets across the groups and create further groups if needed From b736f41340d7f23a8417d5f87fdcf27af131a1e9 Mon Sep 17 00:00:00 2001 From: Natalia Kislicyn Date: Wed, 15 May 2019 13:59:03 +0200 Subject: [PATCH 4/4] Adopting the review comments Signed-off-by: Natalia Kislicyn --- .../rollout/rollout/DefineGroupsLayout.java | 21 ++++++++----------- .../src/main/resources/messages.properties | 1 - 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java index 67ed3a85b4..0941da26cf 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java @@ -334,23 +334,22 @@ private void setGroupsValidation(final RolloutGroupsValidation validation) { } // validate the single groups - final boolean hasRemainingTargetsError = validationStatus == ValidationStatus.INVALID; - singleGroupsValidation(validation, lastRow, hasRemainingTargetsError); + singleGroupsValidation(lastRow); } - private void singleGroupsValidation(final RolloutGroupsValidation validation, final GroupRow lastRow, - final boolean remainingError) { + private void singleGroupsValidation(final GroupRow lastRow) { + final boolean hasRemainingTargetsError = validationStatus == ValidationStatus.INVALID; final int maxTargets = quotaManagement.getMaxTargetsPerRolloutGroup(); for (int i = 0; i < groupRows.size(); ++i) { final GroupRow row = groupRows.get(i); // do not mask the 'remaining targets' error - if (remainingError && row.equals(lastRow)) { + if (hasRemainingTargetsError && row.equals(lastRow)) { continue; } row.resetError(); - if (validation != null) { - final Long count = validation.getTargetsPerGroup().get(i); + if (groupsValidation != null) { + final Long count = groupsValidation.getTargetsPerGroup().get(i); if (count != null && count > maxTargets) { row.setError(i18n.getMessage(MESSAGE_ROLLOUT_MAX_GROUP_SIZE_EXCEEDED, maxTargets)); setValidationStatus(ValidationStatus.INVALID); @@ -476,12 +475,10 @@ private void validateMandatoryPercentage(final Object value) { RangeValidator rangeValidator; if (value instanceof Float) { rangeValidator = new FloatRangeValidator(message, 0F, 100F); - } - else if (value instanceof Integer) { + } else if (value instanceof Integer) { rangeValidator = new IntegerRangeValidator(message, 0, 100); - } - else { - throw new Validator.InvalidValueException(i18n.getMessage("message.rollout.field.value.type")); + } else { + throw new Validator.InvalidValueException(i18n.getMessage("message.enter.number")); } rangeValidator.setMinValueIncluded(false); rangeValidator.validate(value); diff --git a/hawkbit-ui/src/main/resources/messages.properties b/hawkbit-ui/src/main/resources/messages.properties index 23c3ce173c..940bf1e1a8 100644 --- a/hawkbit-ui/src/main/resources/messages.properties +++ b/hawkbit-ui/src/main/resources/messages.properties @@ -616,7 +616,6 @@ message.rollout.name.empty = Please enter a name for Rollout message.correct.invalid.value = Please correct invalid values message.enter.number = Please enter number message.rollout.field.value.range = Value should be in range {0} to {1} -message.rollout.field.value.type = Invalid type of value message.rollout.filter.target.exists = The selected target filter does not match any existing target message.rollout.max.group.size.exceeded = The maximum group size of {0} targets is exceeded. Please increase the number of groups or select a different target filter message.rollout.max.group.size.exceeded.advanced = The maximum size of {0} targets is exceeded for this group. Please re-distribute the targets across the groups and create further groups if needed