From fb2123974d5146b8f19f2f53591925d0de5136db Mon Sep 17 00:00:00 2001 From: Lucas Silva Date: Tue, 15 Nov 2022 23:37:53 -0500 Subject: Add ability to AND and OR conditions together In the dock setup flow, we have complex triggering which could be made simpler with the ability to OR conditions together. To cover the case where the value is undefined, this change introduces 3-valued AND/OR operators which can be used to construct new conditions from an existing condition. To keep the syntax readable, we add or() and and() methods to conditions, so the caller can simply say condition1.or(condition2) Bug: 255372044 Test: atest com.android.systemui.util.condition.ConditionTest Test: atest com.android.systemui.util.condition.ConditionMonitorTest Change-Id: Iac583492669cbb573c84807c9c0e78c35dfc02a0 --- .../systemui/util/condition/CombinedCondition.kt | 42 ++++++ .../android/systemui/util/condition/Condition.java | 39 ++++++ .../android/systemui/util/condition/Evaluator.kt | 92 ++++++++++++ .../android/systemui/util/condition/Monitor.java | 33 ++--- .../systemui/util/condition/ConditionTest.java | 154 +++++++++++++++++++++ .../systemui/util/condition/FakeCondition.java | 12 +- 6 files changed, 348 insertions(+), 24 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/util/condition/CombinedCondition.kt create mode 100644 packages/SystemUI/src/com/android/systemui/util/condition/Evaluator.kt diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/CombinedCondition.kt b/packages/SystemUI/src/com/android/systemui/util/condition/CombinedCondition.kt new file mode 100644 index 000000000000..da81d540f189 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/condition/CombinedCondition.kt @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.util.condition + +/** + * A higher order [Condition] which combines multiple conditions with a specified + * [Evaluator.ConditionOperand]. + */ +internal class CombinedCondition +constructor( + private val conditions: Collection, + @Evaluator.ConditionOperand private val operand: Int +) : Condition(null, false), Condition.Callback { + + override fun start() { + onConditionChanged(this) + conditions.forEach { it.addCallback(this) } + } + + override fun onConditionChanged(condition: Condition) { + Evaluator.evaluate(conditions, operand)?.also { value -> updateCondition(value) } + ?: clearCondition() + } + + override fun stop() { + conditions.forEach { it.removeCallback(this) } + } +} diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java b/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java index 2c317dd391c0..b39adefa238b 100644 --- a/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java +++ b/packages/SystemUI/src/com/android/systemui/util/condition/Condition.java @@ -24,7 +24,10 @@ import org.jetbrains.annotations.NotNull; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.Iterator; +import java.util.List; /** * Base class for a condition that needs to be fulfilled in order for {@link Monitor} to inform @@ -180,6 +183,42 @@ public abstract class Condition implements CallbackController others) { + final List conditions = new ArrayList<>(others); + conditions.add(this); + return new CombinedCondition(conditions, Evaluator.OP_AND); + } + + /** + * Creates a new condition which will only be true when both this condition and the provided + * condition is true. + */ + public Condition and(Condition other) { + return new CombinedCondition(Arrays.asList(this, other), Evaluator.OP_AND); + } + + /** + * Creates a new condition which will only be true when either this condition or any of the + * provided conditions are true. + */ + public Condition or(Collection others) { + final List conditions = new ArrayList<>(others); + conditions.add(this); + return new CombinedCondition(conditions, Evaluator.OP_OR); + } + + /** + * Creates a new condition which will only be true when either this condition or the provided + * condition is true. + */ + public Condition or(Condition other) { + return new CombinedCondition(Arrays.asList(this, other), Evaluator.OP_OR); + } + /** * Callback that receives updates about whether the condition has been fulfilled. */ diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/Evaluator.kt b/packages/SystemUI/src/com/android/systemui/util/condition/Evaluator.kt new file mode 100644 index 000000000000..cf44e84a563f --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/condition/Evaluator.kt @@ -0,0 +1,92 @@ +package com.android.systemui.util.condition + +import android.annotation.IntDef + +/** + * Helper for evaluating a collection of [Condition] objects with a given + * [Evaluator.ConditionOperand] + */ +internal object Evaluator { + /** Operands for combining multiple conditions together */ + @Retention(AnnotationRetention.SOURCE) + @IntDef(value = [OP_AND, OP_OR]) + annotation class ConditionOperand + + /** + * 3-valued logical AND operand, with handling for unknown values (represented as null) + * + * ``` + * +-----+----+---+---+ + * | AND | T | F | U | + * +-----+----+---+---+ + * | T | T | F | U | + * | F | F | F | F | + * | U | U | F | U | + * +-----+----+---+---+ + * ``` + */ + const val OP_AND = 0 + + /** + * 3-valued logical OR operand, with handling for unknown values (represented as null) + * + * ``` + * +-----+----+---+---+ + * | OR | T | F | U | + * +-----+----+---+---+ + * | T | T | T | T | + * | F | T | F | U | + * | U | T | U | U | + * +-----+----+---+---+ + * ``` + */ + const val OP_OR = 1 + + /** + * Evaluates a set of conditions with a given operand + * + * If overriding conditions are present, they take precedence over normal conditions if set. + * + * @param conditions The collection of conditions to evaluate. If empty, null is returned. + * @param operand The operand to use when evaluating. + * @return Either true or false if the value is known, or null if value is unknown + */ + fun evaluate(conditions: Collection, @ConditionOperand operand: Int): Boolean? { + if (conditions.isEmpty()) return null + // If there are overriding conditions with values set, they take precedence. + val targetConditions = + conditions + .filter { it.isConditionSet && it.isOverridingCondition } + .ifEmpty { conditions } + return when (operand) { + OP_AND -> + threeValuedAndOrOr(conditions = targetConditions, returnValueIfAnyMatches = false) + OP_OR -> + threeValuedAndOrOr(conditions = targetConditions, returnValueIfAnyMatches = true) + else -> null + } + } + + /** + * Helper for evaluating 3-valued logical AND/OR. + * + * @param returnValueIfAnyMatches AND returns false if any value is false. OR returns true if + * any value is true. + */ + private fun threeValuedAndOrOr( + conditions: Collection, + returnValueIfAnyMatches: Boolean + ): Boolean? { + var hasUnknown = false + for (condition in conditions) { + if (!condition.isConditionSet) { + hasUnknown = true + continue + } + if (condition.isConditionMet == returnValueIfAnyMatches) { + return returnValueIfAnyMatches + } + } + return if (hasUnknown) null else !returnValueIfAnyMatches + } +} diff --git a/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java b/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java index cb430ba454f0..24bc9078475f 100644 --- a/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java +++ b/packages/SystemUI/src/com/android/systemui/util/condition/Monitor.java @@ -24,12 +24,10 @@ import com.android.systemui.dagger.qualifiers.Main; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Set; import java.util.concurrent.Executor; -import java.util.stream.Collectors; import javax.inject.Inject; @@ -57,21 +55,10 @@ public class Monitor { } public void update() { - // Only consider set conditions. - final Collection setConditions = mSubscription.mConditions.stream() - .filter(Condition::isConditionSet).collect(Collectors.toSet()); - - // Overriding conditions do not override each other - final Collection overridingConditions = setConditions.stream() - .filter(Condition::isOverridingCondition).collect(Collectors.toSet()); - - final Collection targetCollection = overridingConditions.isEmpty() - ? setConditions : overridingConditions; - - final boolean newAllConditionsMet = targetCollection.isEmpty() ? true : targetCollection - .stream() - .map(Condition::isConditionMet) - .allMatch(conditionMet -> conditionMet); + final Boolean result = Evaluator.INSTANCE.evaluate(mSubscription.mConditions, + Evaluator.OP_AND); + // Consider unknown (null) as true + final boolean newAllConditionsMet = result == null || result; if (mAllConditionsMet != null && newAllConditionsMet == mAllConditionsMet) { return; @@ -109,6 +96,7 @@ public class Monitor { /** * Registers a callback and the set of conditions to trigger it. + * * @param subscription A {@link Subscription} detailing the desired conditions and callback. * @return A {@link Subscription.Token} that can be used to remove the subscription. */ @@ -139,6 +127,7 @@ public class Monitor { /** * Removes a subscription from participating in future callbacks. + * * @param token The {@link Subscription.Token} returned when the {@link Subscription} was * originally added. */ @@ -179,7 +168,9 @@ public class Monitor { private final Set mConditions; private final Callback mCallback; - /** */ + /** + * + */ public Subscription(Set conditions, Callback callback) { this.mConditions = Collections.unmodifiableSet(conditions); this.mCallback = callback; @@ -209,7 +200,6 @@ public class Monitor { /** * Default constructor specifying the {@link Callback} for the {@link Subscription}. - * @param callback */ public Builder(Callback callback) { mCallback = callback; @@ -218,7 +208,7 @@ public class Monitor { /** * Adds a {@link Condition} to be associated with the {@link Subscription}. - * @param condition + * * @return The updated {@link Builder}. */ public Builder addCondition(Condition condition) { @@ -228,7 +218,7 @@ public class Monitor { /** * Adds a set of {@link Condition} to be associated with the {@link Subscription}. - * @param condition + * * @return The updated {@link Builder}. */ public Builder addConditions(Set condition) { @@ -238,6 +228,7 @@ public class Monitor { /** * Builds the {@link Subscription}. + * * @return The resulting {@link Subscription}. */ public Subscription build() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java index 0b53133e9353..28788647dd58 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/util/condition/ConditionTest.java @@ -141,4 +141,158 @@ public class ConditionTest extends SysuiTestCase { mCondition.clearCondition(); assertThat(mCondition.isConditionSet()).isFalse(); } + + @Test + public void combineConditionsWithOr_allFalse_reportsNotMet() { + mCondition.fakeUpdateCondition(false); + + final Condition combinedCondition = mCondition.or( + new FakeCondition(/* initialValue= */ false)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isFalse(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithOr_allTrue_reportsMet() { + mCondition.fakeUpdateCondition(true); + + final Condition combinedCondition = mCondition.or( + new FakeCondition(/* initialValue= */ true)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isTrue(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithOr_singleTrue_reportsMet() { + mCondition.fakeUpdateCondition(false); + + final Condition combinedCondition = mCondition.or( + new FakeCondition(/* initialValue= */ true)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isTrue(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithOr_unknownAndTrue_reportsMet() { + mCondition.fakeUpdateCondition(true); + + // Combine with an unset condition. + final Condition combinedCondition = mCondition.or( + new FakeCondition(/* initialValue= */ null)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isTrue(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithOr_unknownAndFalse_reportsNotMet() { + mCondition.fakeUpdateCondition(false); + + // Combine with an unset condition. + final Condition combinedCondition = mCondition.or( + new FakeCondition(/* initialValue= */ null)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isFalse(); + assertThat(combinedCondition.isConditionMet()).isFalse(); + verify(callback, never()).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithAnd_allFalse_reportsNotMet() { + mCondition.fakeUpdateCondition(false); + + final Condition combinedCondition = mCondition.and( + new FakeCondition(/* initialValue= */ false)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isFalse(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithAnd_allTrue_reportsMet() { + mCondition.fakeUpdateCondition(true); + + final Condition combinedCondition = mCondition.and( + new FakeCondition(/* initialValue= */ true)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isTrue(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithAnd_singleTrue_reportsNotMet() { + mCondition.fakeUpdateCondition(true); + + final Condition combinedCondition = mCondition.and( + new FakeCondition(/* initialValue= */ false)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isFalse(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithAnd_unknownAndTrue_reportsNotMet() { + mCondition.fakeUpdateCondition(true); + + // Combine with an unset condition. + final Condition combinedCondition = mCondition.and( + new FakeCondition(/* initialValue= */ null)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isFalse(); + assertThat(combinedCondition.isConditionMet()).isFalse(); + verify(callback, never()).onConditionChanged(combinedCondition); + } + + @Test + public void combineConditionsWithAnd_unknownAndFalse_reportsMet() { + mCondition.fakeUpdateCondition(false); + + // Combine with an unset condition. + final Condition combinedCondition = mCondition.and( + new FakeCondition(/* initialValue= */ null)); + + final Condition.Callback callback = mock(Condition.Callback.class); + combinedCondition.addCallback(callback); + + assertThat(combinedCondition.isConditionSet()).isTrue(); + assertThat(combinedCondition.isConditionMet()).isFalse(); + verify(callback, times(1)).onConditionChanged(combinedCondition); + } } diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/util/condition/FakeCondition.java b/packages/SystemUI/tests/utils/src/com/android/systemui/util/condition/FakeCondition.java index 1353ad25d057..07ed1102e990 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/util/condition/FakeCondition.java +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/util/condition/FakeCondition.java @@ -25,15 +25,21 @@ public class FakeCondition extends Condition { super(); } - FakeCondition(Boolean initialValue, Boolean overriding) { + FakeCondition(Boolean initialValue) { + super(initialValue, false); + } + + FakeCondition(Boolean initialValue, boolean overriding) { super(initialValue, overriding); } @Override - public void start() {} + public void start() { + } @Override - public void stop() {} + public void stop() { + } public void fakeUpdateCondition(boolean isConditionMet) { updateCondition(isConditionMet); -- cgit v1.2.3-59-g8ed1b