Skip to content

Commit 1650591

Browse files
authored
[basicprofiles] Fix division-by-zero error in $DELTA_PERCENT state filter (openhab#18089)
* [basicprofiles] Fix division-by-zero error in $DELTA_PERCENT state filter * use Optional for acceptedState Signed-off-by: Jimmy Tanagra <[email protected]>
1 parent ea978fe commit 1650591

File tree

2 files changed

+42
-12
lines changed

2 files changed

+42
-12
lines changed

bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java

+38-10
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public class StateFilterProfile implements StateProfile {
103103
private @Nullable Item linkedItem = null;
104104

105105
private State newState = UnDefType.UNDEF;
106-
private State acceptedState = UnDefType.UNDEF;
106+
private Optional<State> acceptedState = Optional.empty();
107107
private LinkedList<State> previousStates = new LinkedList<>();
108108

109109
private final int windowSize;
@@ -230,7 +230,7 @@ private State checkCondition(State state) {
230230
}
231231

232232
if (conditions.stream().allMatch(c -> c.check(state))) {
233-
acceptedState = state;
233+
acceptedState = Optional.of(state);
234234
return state;
235235
} else {
236236
return configMismatchState;
@@ -339,8 +339,7 @@ public boolean check(State input) {
339339
if (rhsState == null) {
340340
rhsItem = getItemOrNull(rhsString);
341341
} else if (rhsState instanceof FunctionType rhsFunction) {
342-
if (acceptedState == UnDefType.UNDEF && (rhsFunction.getType() == FunctionType.Function.DELTA
343-
|| rhsFunction.getType() == FunctionType.Function.DELTA_PERCENT)) {
342+
if (rhsFunction.alwaysAccept()) {
344343
return true;
345344
}
346345
rhsItem = getLinkedItem();
@@ -378,8 +377,7 @@ public boolean check(State input) {
378377
}
379378

380379
if (lhsState instanceof FunctionType lhsFunction) {
381-
if (acceptedState == UnDefType.UNDEF && (lhsFunction.getType() == FunctionType.Function.DELTA
382-
|| lhsFunction.getType() == FunctionType.Function.DELTA_PERCENT)) {
380+
if (lhsFunction.alwaysAccept()) {
383381
return true;
384382
}
385383
lhsItem = getLinkedItem();
@@ -567,6 +565,30 @@ public FunctionType(Function type, Optional<Integer> windowSize) {
567565
};
568566
}
569567

568+
/**
569+
* If the profile uses the DELTA or DELTA_PERCENT functions, the new state value will always be accepted if the
570+
* 'acceptedState' (prior state) has not yet been initialised, or -- in the case of the DELTA_PERCENT function
571+
* only -- if 'acceptedState' has a zero value. This ensures that 'acceptedState' is always initialised. And it
572+
* also ensures that the DELTA_PERCENT function cannot cause a divide by zero error.
573+
*
574+
* @return true if the new state value shall be accepted
575+
*/
576+
public boolean alwaysAccept() {
577+
if ((type == Function.DELTA || type == Function.DELTA_PERCENT) && acceptedState.isEmpty()) {
578+
return true;
579+
}
580+
if (type == Function.DELTA_PERCENT) {
581+
// avoid division by zero
582+
if (acceptedState.get() instanceof QuantityType base) {
583+
return base.toBigDecimal().compareTo(BigDecimal.ZERO) == 0;
584+
}
585+
if (acceptedState.get() instanceof DecimalType base) {
586+
return base.toBigDecimal().compareTo(BigDecimal.ZERO) == 0;
587+
}
588+
}
589+
return false;
590+
}
591+
570592
@Override
571593
public <T extends State> @Nullable T as(@Nullable Class<T> target) {
572594
if (target == DecimalType.class || target == QuantityType.class) {
@@ -692,27 +714,33 @@ public String toString() {
692714
}
693715

694716
private @Nullable State calculateDelta() {
717+
if (acceptedState.isEmpty()) {
718+
return null;
719+
}
695720
if (newState instanceof QuantityType newStateQuantity) {
696-
QuantityType result = newStateQuantity.subtract((QuantityType) acceptedState);
721+
QuantityType result = newStateQuantity.subtract((QuantityType) acceptedState.get());
697722
return result.toBigDecimal().compareTo(BigDecimal.ZERO) < 0 ? result.negate() : result;
698723
}
699724
BigDecimal result = ((DecimalType) newState).toBigDecimal()
700-
.subtract(((DecimalType) acceptedState).toBigDecimal()) //
725+
.subtract(((DecimalType) acceptedState.get()).toBigDecimal()) //
701726
.abs();
702727
return new DecimalType(result);
703728
}
704729

705730
private @Nullable State calculateDeltaPercent() {
731+
if (acceptedState.isEmpty()) {
732+
return null;
733+
}
706734
State calculatedDelta = calculateDelta();
707735
BigDecimal bdDelta;
708736
BigDecimal bdBase;
709-
if (acceptedState instanceof QuantityType acceptedStateQuantity) {
737+
if (acceptedState.get() instanceof QuantityType acceptedStateQuantity) {
710738
// Assume that delta and base are in the same unit
711739
bdDelta = ((QuantityType) calculatedDelta).toBigDecimal();
712740
bdBase = acceptedStateQuantity.toBigDecimal();
713741
} else {
714742
bdDelta = ((DecimalType) calculatedDelta).toBigDecimal();
715-
bdBase = ((DecimalType) acceptedState).toBigDecimal();
743+
bdBase = ((DecimalType) acceptedState.get()).toBigDecimal();
716744
}
717745
bdBase = bdBase.abs();
718746
BigDecimal percent = bdDelta.multiply(BigDecimal.valueOf(100)).divide(bdBase, 2, RoundingMode.HALF_EVEN);

bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -722,15 +722,17 @@ public static Stream<Arguments> testFunctions() {
722722
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", decimals, DecimalType.valueOf("0.89"), false), //
723723

724724
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", negativeDecimals, DecimalType.valueOf("0"), false),
725-
//
726725
Arguments.of(decimalItem, "10 > $DELTA_PERCENT", negativeDecimals, DecimalType.valueOf("0"), false),
727-
//
728726

729727
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("1.09"), true), //
730728
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("1.11"), false), //
731729
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("0.91"), true), //
732730
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("0.89"), false), //
733731

732+
// Check against possible division-by-zero errors in $DELTA_PERCENT
733+
Arguments.of(decimalItem, "> 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), //
734+
Arguments.of(decimalItem, "< 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), //
735+
734736
// Contrast a simple comparison against a Percent QuantityType vs delta percent check
735737
Arguments.of(percentItem, "> 5%", percentQuantities, QuantityType.valueOf("5.1 %"), true), //
736738
Arguments.of(percentItem, "$DELTA_PERCENT > 5", percentQuantities, QuantityType.valueOf("5.1 %"),

0 commit comments

Comments
 (0)