diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.kt b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.kt index 125870d21c..1a940a5206 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.kt @@ -9,10 +9,9 @@ import org.signal.core.util.logging.Log import org.signal.core.util.mebiBytes import org.thoughtcrime.securesms.BuildConfig import org.thoughtcrime.securesms.dependencies.AppDependencies -import org.thoughtcrime.securesms.dependencies.AppDependencies.application -import org.thoughtcrime.securesms.dependencies.AppDependencies.jobManager import org.thoughtcrime.securesms.groups.SelectionLimits import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob +import org.thoughtcrime.securesms.jobs.Svr3MirrorJob import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.messageprocessingalarm.RoutineMessageFetchReceiver import java.io.IOException @@ -67,7 +66,7 @@ object FeatureFlags { if (timeSinceLastFetch < 0 || timeSinceLastFetch > FETCH_INTERVAL.inWholeMilliseconds) { Log.i(TAG, "Scheduling remote config refresh.") - jobManager.add(RemoteConfigRefreshJob()) + AppDependencies.jobManager.add(RemoteConfigRefreshJob()) } else { Log.i(TAG, "Skipping remote config refresh. Refreshed $timeSinceLastFetch ms ago.") } @@ -194,29 +193,17 @@ object FeatureFlags { @JvmStatic @VisibleForTesting - fun computeChanges(oldMap: Map, newMap: Map): Map { - val changes: MutableMap = mutableMapOf() - val allKeys: MutableSet = mutableSetOf() + fun computeChanges(oldMap: Map, newMap: Map): Map { + val allKeys: Set = oldMap.keys + newMap.keys - allKeys += oldMap.keys - allKeys += newMap.keys - - for (key in allKeys) { - val oldValue = oldMap[key] - val newValue = newMap[key] - - if (oldValue == null && newValue == null) { - throw AssertionError("Should not be possible.") - } else if (oldValue != null && newValue == null) { - changes[key] = Change.REMOVED - } else if (newValue !== oldValue && newValue is Boolean) { - changes[key] = if (newValue) Change.ENABLED else Change.DISABLED - } else if (oldValue != newValue) { - changes[key] = Change.CHANGED + return allKeys + .filter { oldMap[it] != newMap[it] } + .associateWith { key -> + ConfigChange( + oldValue = oldMap[key], + newValue = newMap[key] + ) } - } - - return changes } private fun parseStoredConfig(stored: String?): Map { @@ -256,7 +243,7 @@ object FeatureFlags { } } - private fun triggerFlagChangeListeners(changes: Map) { + private fun triggerFlagChangeListeners(changes: Map) { for ((key, value) in changes) { val listener = configsByKey[key]?.onChangeListener @@ -271,16 +258,14 @@ object FeatureFlags { class UpdateResult( val memory: Map, val disk: Map, - val memoryChanges: Map + val memoryChanges: Map ) + data class ConfigChange(val oldValue: Any?, val newValue: Any?) + @VisibleForTesting fun interface OnFlagChange { - fun onFlagChange(change: Change) - } - - enum class Change { - ENABLED, DISABLED, CHANGED, REMOVED + fun onFlagChange(change: ConfigChange) } // endregion @@ -1072,7 +1057,7 @@ object FeatureFlags { val backgroundMessageProcessInterval: Long by remoteValue( key = "android.messageProcessor.alarmIntervalMins", hotSwappable = true, - onChangeListener = { RoutineMessageFetchReceiver.startOrUpdateAlarm(application) } + onChangeListener = { RoutineMessageFetchReceiver.startOrUpdateAlarm(AppDependencies.application) } ) { value -> val inMinutes = value.asLong(6.hours.inWholeMinutes) inMinutes.minutes.inWholeMilliseconds @@ -1098,9 +1083,14 @@ object FeatureFlags { val svr3MigrationPhase: Int by remoteInt( key = "global.svr3.phase", defaultValue = 0, - hotSwappable = true + hotSwappable = true, + onChangeListener = { + if ((it.oldValue == null || it.oldValue == 0) && it.newValue == 1) { + Log.w(TAG, "Detected the SVR3 migration phase change to 1! Enqueuing a mirroring job.") + AppDependencies.jobManager.add(Svr3MirrorJob()) + } + } ) - // TODO [svr3] on change listener to enqueue migration // endregion } diff --git a/app/src/test/java/org/thoughtcrime/securesms/util/FeatureFlagsTest.java b/app/src/test/java/org/thoughtcrime/securesms/util/FeatureFlagsTest.java index 9d4114beb9..f3b156b057 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/util/FeatureFlagsTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/util/FeatureFlagsTest.java @@ -2,7 +2,7 @@ package org.thoughtcrime.securesms.util; import org.junit.Test; import org.thoughtcrime.securesms.BaseUnitTest; -import org.thoughtcrime.securesms.util.FeatureFlags.Change; +import org.thoughtcrime.securesms.util.FeatureFlags.ConfigChange; import org.thoughtcrime.securesms.util.FeatureFlags.UpdateResult; import java.util.Arrays; @@ -61,7 +61,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getDisk()); - assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(null, true), result.getMemoryChanges().get(A)); } @Test @@ -75,7 +75,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(A, 1), result.getMemory()); assertEquals(mapOf(A, 1), result.getDisk()); - assertEquals(Change.CHANGED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(null, 1), result.getMemoryChanges().get(A)); } @Test @@ -103,7 +103,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getDisk()); - assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(null, true), result.getMemoryChanges().get(A)); } @Test @@ -145,7 +145,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getDisk()); - assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(false, true), result.getMemoryChanges().get(A)); } @Test @@ -159,7 +159,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(A, 2), result.getMemory()); assertEquals(mapOf(A, 2), result.getDisk()); - assertEquals(Change.CHANGED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(1, 2), result.getMemoryChanges().get(A)); } @Test @@ -173,7 +173,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getDisk()); - assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(false, true), result.getMemoryChanges().get(A)); } @Test @@ -229,7 +229,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getDisk()); - assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(true, null), result.getMemoryChanges().get(A)); } @Test @@ -243,7 +243,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getDisk()); - assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(true, null), result.getMemoryChanges().get(A)); } @Test @@ -327,7 +327,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getDisk()); - assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(false, null), result.getMemoryChanges().get(A)); } @Test @@ -341,7 +341,7 @@ public class FeatureFlagsTest extends BaseUnitTest { assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getDisk()); - assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); + assertEquals(new ConfigChange(true, null), result.getMemoryChanges().get(A)); } @Test @@ -378,7 +378,7 @@ public class FeatureFlagsTest extends BaseUnitTest { @Test public void computeChanges_generic() { - Map oldMap = new HashMap() {{ + Map oldMap = new HashMap<>() {{ put("a", true); put("b", false); put("c", true); @@ -389,7 +389,7 @@ public class FeatureFlagsTest extends BaseUnitTest { put("j", "gwen"); }}; - Map newMap = new HashMap() {{ + Map newMap = new HashMap<>() {{ put("a", true); put("b", true); put("c", false); @@ -400,17 +400,17 @@ public class FeatureFlagsTest extends BaseUnitTest { put("j", "stacy"); }}; - Map changes = FeatureFlags.computeChanges(oldMap, newMap); + Map changes = FeatureFlags.computeChanges(oldMap, newMap); assertFalse(changes.containsKey("a")); - assertEquals(Change.ENABLED, changes.get("b")); - assertEquals(Change.DISABLED, changes.get("c")); - assertEquals(Change.REMOVED, changes.get("d")); - assertEquals(Change.ENABLED, changes.get("e")); - assertEquals(Change.DISABLED, changes.get("f")); - assertEquals(Change.CHANGED, changes.get("h")); + assertEquals(new ConfigChange(false, true), changes.get("b")); + assertEquals(new ConfigChange(true, false), changes.get("c")); + assertEquals(new ConfigChange(false, null), changes.get("d")); + assertEquals(new ConfigChange(null, true), changes.get("e")); + assertEquals(new ConfigChange(null, false), changes.get("f")); + assertEquals(new ConfigChange(5, 7), changes.get("h")); assertFalse(changes.containsKey("i")); - assertEquals(Change.CHANGED, changes.get("j")); + assertEquals(new ConfigChange("gwen", "stacy"), changes.get("j")); } private static Set setOf(V... values) {