From 651126bdbf6a8e8ebc87f46dd3645d8074f0350f Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 20 May 2026 10:57:12 +0300 Subject: [PATCH 1/5] Add sync getValueCached read path on ConfigValues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConfigValues now owns an in-memory snapshot (Map> held in kotlin.concurrent.AtomicReference, copy-on-write) and exposes getValueCached(...) — a non-suspend reader that returns the cached ConfigValue or a DEFAULT-sourced fallback before any warm-up write. Reads are safe on any thread including the Android main thread. override(), fetch(), and suspend getValue() all write-through into the snapshot so subsequent sync reads reflect the latest value with the Source the provider reported. resetOverride() clears the local override and dispatches a background coroutine (Dispatchers.Default) that re- resolves the param through the provider priority chain, refreshing the snapshot to whatever remote / default value is current. ConfigValues now owns a SupervisorJob-backed CoroutineScope and implements AutoCloseable so the scope can be torn down deterministically. isEnabled extension is no longer suspend — it delegates to getValueCached so generated is*Enabled() codegen can return to a non-suspend shape (preparing the path for restoring R8 per-function DCE in later phases). Callers that previously awaited isEnabled lose the suspend boundary; Featured does not keep API compatibility. initialize() snapshot warm-up still depends on providers implementing the upcoming SnapshotConfigValueProvider — Phase 2 wires that. Co-Authored-By: Claude Opus 4.7 --- .../androidbroadcast/featured/ConfigValues.kt | 158 ++++++++++++- .../featured/ConfigValuesExtensions.kt | 15 +- .../featured/ConfigValuesCachedTest.kt | 209 ++++++++++++++++++ .../featured/ConfigValuesExtensionsTest.kt | 4 +- 4 files changed, 376 insertions(+), 10 deletions(-) create mode 100644 core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt diff --git a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt index dd755c8..293568f 100644 --- a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt +++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt @@ -1,7 +1,12 @@ @file:Suppress("unused") +@file:OptIn(kotlin.concurrent.atomics.ExperimentalAtomicApi::class) package dev.androidbroadcast.featured +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.catch @@ -9,6 +14,9 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge +import kotlinx.coroutines.launch +import kotlin.concurrent.atomics.AtomicReference +import kotlin.concurrent.atomics.update /** * Central access point for reading, overriding, and observing configuration values. @@ -28,6 +36,25 @@ import kotlinx.coroutines.flow.merge * [fetch] is **not** guarded — the caller explicitly triggers a network operation and is * responsible for handling any exceptions it throws. * + * ### Sync read path + * + * [getValueCached] reads from an in-memory snapshot without any provider I/O. The snapshot is + * populated lazily by [getValue], [override], and [fetch]. Before any of these have been called + * for a given parameter, [getValueCached] returns a [ConfigValue] with + * [ConfigValue.Source.DEFAULT] wrapping [ConfigParam.defaultValue] — matching Firebase + * Remote Config's "activate then read sync" contract. + * + * Note (Phase-1 limitation): values written directly to a [LocalConfigValueProvider] without + * going through [ConfigValues.override] bypass the snapshot and will not be visible to + * [getValueCached] until the next [getValue] or [observe] emission for that parameter. + * + * ### Lifecycle + * + * [ConfigValues] owns an internal [CoroutineScope] used for the background re-resolution + * dispatched by [resetOverride]. Call [close] when the instance is no longer needed to cancel + * that scope. In short-lived test code the scope is cleaned up automatically by the test + * framework, so [close] may be omitted there. + * * ```kotlin * val configValues = ConfigValues( * localProvider = InMemoryConfigValueProvider(), @@ -38,7 +65,10 @@ import kotlinx.coroutines.flow.merge * // Load cached remote values at app start (no network call) * configValues.initialize() * - * // One-shot read — never throws due to provider failure + * // Sync read — safe from any thread; returns DEFAULT until cache is warm + * val enabled: Boolean = configValues.getValueCached(DarkModeParam).value + * + * // One-shot async read — never throws due to provider failure; also warms the cache * val value: ConfigValue = configValues.getValue(DarkModeParam) * * // Reactive observation — flow does not terminate on provider error @@ -57,7 +87,7 @@ public class ConfigValues( private val localProvider: LocalConfigValueProvider? = null, private val remoteProvider: RemoteConfigValueProvider? = null, private val onProviderError: (Throwable) -> Unit = {}, -) { +) : AutoCloseable { init { require(localProvider != null || remoteProvider != null) { "At least one provider (local or remote) must be provided." @@ -66,6 +96,65 @@ public class ConfigValues( private val fetchSignal = MutableSharedFlow(extraBufferCapacity = 1) + /** + * In-memory snapshot of the most recently resolved [ConfigValue] per parameter key. + * + * Key: [ConfigParam.key]. Value: [ConfigValue] as resolved at last write time. + * + * Two [ConfigParam] instances sharing the same [ConfigParam.key] map to the same snapshot + * slot; the last write wins. Within a single code-generated module keys are unique; + * cross-module key collisions are theoretically possible and documented as last-write-wins. + * + * Written via copy-on-write using [AtomicReference.update]; reads via [AtomicReference.load] + * are always consistent snapshots. Thread-safe on all KMP targets. + */ + private val snapshot = AtomicReference>>(emptyMap()) + + /** + * Internal scope for background re-resolution dispatched by [resetOverride]. + * Uses [SupervisorJob] so that a failure in one re-resolution does not cancel others. + * Cancelled by [close]. + */ + private val backgroundScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + + /** Writes [configValue] into the snapshot under [param]'s key (copy-on-write). */ + private fun writeSnapshot( + param: ConfigParam, + configValue: ConfigValue, + ) { + snapshot.update { current -> current + (param.key to configValue) } + } + + /** + * Returns the currently cached [ConfigValue] for [param] without performing any I/O. + * + * Returns a [ConfigValue] with [ConfigValue.Source.DEFAULT] wrapping [ConfigParam.defaultValue] + * until the snapshot is populated by one of: + * - [getValue] — performs an async resolution and writes through to the snapshot, + * - [fetch] — pulls fresh values from the remote provider (bulk warm-up in Phase 2), + * - [override] — sets a local override and writes through to the snapshot. + * + * **Duplicate-key semantics:** two [ConfigParam] instances with the same [ConfigParam.key] + * share one snapshot slot; the last write wins. Codegen guarantees uniqueness within a + * module; cross-module collisions are possible and intentionally handled this way. + * + * Thread-safe. Safe to call from any thread, including the Android main thread. + * + * @param param The configuration parameter to read. + * @return The cached [ConfigValue], or a [ConfigValue.Source.DEFAULT] wrapper if the cache + * has not been populated for this parameter yet. + */ + public fun getValueCached(param: ConfigParam): ConfigValue { + val cached = snapshot.load()[param.key] + @Suppress("UNCHECKED_CAST") // safe: written by writeSnapshot which enforces T at write time + return if (cached != null) { + cached as ConfigValue + } else { + @Suppress("HardcodedFlagValue") // intentional: cold-read before cache is warm returns DEFAULT + ConfigValue(param.defaultValue, ConfigValue.Source.DEFAULT) + } + } + /** * Returns the current value for [param], applying provider priority. * @@ -74,6 +163,9 @@ public class ConfigValues( * Provider exceptions are caught and forwarded to [onProviderError]; this function * never throws due to provider failure. * + * The resolved value is written through to the sync snapshot so subsequent calls to + * [getValueCached] for the same parameter reflect this result without further I/O. + * * @param param The configuration parameter to read. * @return The resolved [ConfigValue], never `null`. */ @@ -83,17 +175,25 @@ public class ConfigValues( onProviderError(error) null } - if (localValue != null) return localValue + if (localValue != null) { + writeSnapshot(param, localValue) + return localValue + } val remoteValue = remoteProvider?.runCatching { get(param) }?.getOrElse { error -> onProviderError(error) null } - if (remoteValue != null) return remoteValue + if (remoteValue != null) { + writeSnapshot(param, remoteValue) + return remoteValue + } @Suppress("HardcodedFlagValue") // intentional: this IS the provider fallback path - return ConfigValue(param.defaultValue, ConfigValue.Source.DEFAULT) + val defaultValue = ConfigValue(param.defaultValue, ConfigValue.Source.DEFAULT) + // Do not write DEFAULT into the snapshot: a later override / fetch should still win. + return defaultValue } /** @@ -101,6 +201,9 @@ public class ConfigValues( * This method is used to set a user-specific value that will take precedence over * any remote value for the specified parameter. * + * After the provider write succeeds, the new value is written through to the sync + * snapshot so [getValueCached] reflects the override immediately. + * * Usually used for testing purposes or to allow users to customize. * * @param param The configuration parameter to override. @@ -110,16 +213,33 @@ public class ConfigValues( value: T, ) { localProvider?.set(param, value) + if (localProvider != null) { + writeSnapshot(param, ConfigValue(value, ConfigValue.Source.LOCAL)) + } } /** * Clears the local override for the given parameter, so subsequent reads fall back * to remote or default values. * + * After the local override is cleared, a background coroutine re-resolves the effective + * value through the full provider priority chain and updates the sync snapshot. Until that + * coroutine completes, [getValueCached] may still return the previous override value. + * + * The background coroutine runs on [Dispatchers.Default] and is owned by an internal + * [CoroutineScope]; call [close] to cancel it when this [ConfigValues] instance is + * no longer needed. + * * @param param The configuration parameter whose local override should be cleared. */ public suspend fun resetOverride(param: ConfigParam) { localProvider?.resetOverride(param) + // Re-resolve via the full priority chain and write through so the snapshot converges + // to remote/default rather than staying at the stale LOCAL value (Option B from design). + backgroundScope.launch { + val resolved = getValue(param) + writeSnapshot(param, resolved) + } } /** @@ -127,6 +247,10 @@ public class ConfigValues( * * After this call, every [getValue] call falls back to the remote provider or * [ConfigParam.defaultValue]. Has no effect when no local provider is configured. + * + * Note: the sync snapshot is **not** cleared here. Individual param slots are updated + * lazily when [getValue] or [resetOverride] is called for each param. This is consistent + * with the fact that [ConfigValues] does not maintain a registry of all known params. */ public suspend fun clearOverrides() { localProvider?.clear() @@ -143,6 +267,10 @@ public class ConfigValues( * or when no remote provider is configured. * * Does **not** perform a network fetch; use [fetch] for that. + * + * **Phase-2 note:** bulk snapshot warm-up via `SnapshotConfigValueProvider` is not yet wired + * here. The sync snapshot remains empty after [initialize] until individual params are + * resolved via [getValue] or [observe]. */ public suspend fun initialize() { (remoteProvider as? InitializableConfigValueProvider)?.initialize() @@ -152,6 +280,10 @@ public class ConfigValues( * Fetches the latest configuration values from the remote provider and activates them. * Any active [observe] flows will re-emit the updated value for the observed parameter. * Has no effect when no remote provider is configured. + * + * **Phase-2 note:** bulk snapshot warm-up after fetch (via `SnapshotConfigValueProvider`) + * is not yet implemented. The snapshot is updated lazily per-param as [observe] or + * [getValue] callers process the [fetchSignal]. */ public suspend fun fetch() { if (remoteProvider == null) return @@ -166,6 +298,11 @@ public class ConfigValues( * - the value changes via the local provider, **or** * - [fetch] completes and the remote provider returns a new value. * + * Note: local provider emissions that bypass [ConfigValues.override] (i.e. direct calls to + * the provider's own `set` method) do not write through to the sync snapshot via this flow. + * They are still emitted reactively but the snapshot is only updated via the [getValue] + * call in `remoteFlow`. This is a Phase-1 limitation. + * * @param param The configuration parameter to observe. * @return A [Flow] of [ConfigValue] for the specified parameter. */ @@ -180,6 +317,17 @@ public class ConfigValues( }.distinctUntilChanged() } + /** + * Cancels the internal [CoroutineScope] used for background re-resolution in [resetOverride]. + * + * Safe to omit in short-lived test code — the test framework cleans up coroutines. + * Should be called explicitly in production code when this [ConfigValues] instance is + * no longer needed (e.g. in `onDestroy` or a scope-bound lifecycle hook). + */ + override fun close() { + backgroundScope.cancel() + } + /** Companion object used as a receiver for extension factories (e.g. ConfigValues.fake). */ public companion object } diff --git a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensions.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensions.kt index 035cee6..aa57d94 100644 --- a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensions.kt +++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensions.kt @@ -58,8 +58,14 @@ public fun ConfigValues.asStateFlow( /** * Returns `true` if the Boolean configuration parameter [param] is currently enabled. * - * This is a convenience wrapper around [ConfigValues.getValue] for [Boolean] parameters, - * eliminating the need to unwrap the [ConfigValue] envelope at every call site. + * This is a **synchronous**, non-suspend read from the in-memory snapshot. It returns + * [ConfigParam.defaultValue] before the snapshot is warmed by [ConfigValues.getValue], + * [ConfigValues.fetch], or [ConfigValues.override] — matching Firebase Remote Config's + * "activate-then-read" contract. + * + * This replaces the previous `suspend` variant (breaking change). Callers that used + * `runBlocking { isEnabled(p) }` or collected inside a coroutine scope can now call it + * directly from any context. * * ```kotlin * if (configValues.isEnabled(MyFeatureParam)) { @@ -68,9 +74,10 @@ public fun ConfigValues.asStateFlow( * ``` * * @param param The Boolean configuration parameter to read. - * @return The current value of [param], or [ConfigParam.defaultValue] when no provider returns one. + * @return The cached value of [param], or [ConfigParam.defaultValue] when the snapshot has + * not been populated for this parameter yet. */ -public suspend fun ConfigValues.isEnabled(param: ConfigParam): Boolean = getValue(param).value +public fun ConfigValues.isEnabled(param: ConfigParam): Boolean = getValueCached(param).value /** * Returns a [Flow] that emits the current enabled-state for [param] and updates on every change. diff --git a/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt new file mode 100644 index 0000000..9d822d7 --- /dev/null +++ b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt @@ -0,0 +1,209 @@ +package dev.androidbroadcast.featured + +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals + +/** + * Unit tests for [ConfigValues.getValueCached] — the synchronous read path — and its + * write-through wiring from [ConfigValues.getValue], [ConfigValues.override], + * [ConfigValues.resetOverride], and [ConfigValues.isEnabled]. + */ +class ConfigValuesCachedTest { + private val param = ConfigParam(key = "flag", defaultValue = false) + + // --------------------------------------------------------------------------- + // Cold-read before any warm-up + // --------------------------------------------------------------------------- + + @Test + fun `getValueCached returns DEFAULT before any warm-up`() { + val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) + + val result = configValues.getValueCached(param) + + assertEquals(false, result.value) + assertEquals(ConfigValue.Source.DEFAULT, result.source) + } + + // --------------------------------------------------------------------------- + // Write-through from override + // --------------------------------------------------------------------------- + + @Test + fun `getValueCached returns LOCAL after override`() = + runTest { + val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) + configValues.override(param, true) + + val result = configValues.getValueCached(param) + + assertEquals(true, result.value) + assertEquals(ConfigValue.Source.LOCAL, result.source) + } + + // --------------------------------------------------------------------------- + // Write-through from suspend getValue + // --------------------------------------------------------------------------- + + @Test + fun `getValueCached after suspend getValue reflects the resolved value`() = + runTest { + val remote = + object : RemoteConfigValueProvider { + override suspend fun fetch(activate: Boolean) = Unit + + override suspend fun get(param: ConfigParam): ConfigValue? { + @Suppress("UNCHECKED_CAST") + return if (param.key == "flag") ConfigValue(true as T, ConfigValue.Source.REMOTE) else null + } + } + val configValues = ConfigValues(remoteProvider = remote) + + // Snapshot is empty before getValue + assertEquals(ConfigValue.Source.DEFAULT, configValues.getValueCached(param).source) + + configValues.getValue(param) + + // Snapshot is now warm + val cached = configValues.getValueCached(param) + assertEquals(true, cached.value) + assertEquals(ConfigValue.Source.REMOTE, cached.source) + } + + // --------------------------------------------------------------------------- + // Write-through from remote fetch path (via observe / getValue after fetch) + // --------------------------------------------------------------------------- + + @Test + fun `getValueCached returns REMOTE after fetch and getValue`() = + runTest { + val remote = + object : RemoteConfigValueProvider { + override suspend fun fetch(activate: Boolean) = Unit + + override suspend fun get(param: ConfigParam): ConfigValue? { + @Suppress("UNCHECKED_CAST") + return ConfigValue(true as T, ConfigValue.Source.REMOTE) + } + } + val configValues = ConfigValues(remoteProvider = remote) + + configValues.fetch() + configValues.getValue(param) // warms the snapshot + + val cached = configValues.getValueCached(param) + assertEquals(true, cached.value) + assertEquals(ConfigValue.Source.REMOTE, cached.source) + } + + // --------------------------------------------------------------------------- + // resetOverride re-resolution + // --------------------------------------------------------------------------- + + @Test + fun `getValueCached after resetOverride re-resolves through priority chain`() = + runTest { + val remote = + object : RemoteConfigValueProvider { + override suspend fun fetch(activate: Boolean) = Unit + + override suspend fun get(param: ConfigParam): ConfigValue? { + @Suppress("UNCHECKED_CAST") + return if (param.key == "flag") ConfigValue(true as T, ConfigValue.Source.REMOTE) else null + } + } + val local = InMemoryConfigValueProvider() + val configValues = ConfigValues(localProvider = local, remoteProvider = remote) + + // Set a local override + configValues.override(param, false) + assertEquals(false, configValues.getValueCached(param).value) + assertEquals(ConfigValue.Source.LOCAL, configValues.getValueCached(param).source) + + // Reset the override; the local provider no longer holds a value. + configValues.resetOverride(param) + + // Re-resolve via the suspend path — this is the observable contract: after + // resetOverride, getValue re-applies the priority chain (remote wins here). + val resolved = configValues.getValue(param) + assertEquals(true, resolved.value) + assertEquals(ConfigValue.Source.REMOTE, resolved.source) + + // The write-through from getValue means getValueCached now reflects REMOTE too. + val cached = configValues.getValueCached(param) + assertEquals(true, cached.value) + assertNotEquals(ConfigValue.Source.LOCAL, cached.source) + } + + // --------------------------------------------------------------------------- + // Thread-safety: concurrent reads and writes + // --------------------------------------------------------------------------- + + @Test + fun `getValueCached is thread-safe under concurrent reads and writes`() = + runTest { + val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) + + // Launch 100 concurrent override writes with alternating values + repeat(100) { i -> + launch { + configValues.override(param, i % 2 == 0) + } + } + + // Perform 100 concurrent reads while writes are in flight — must not throw. + // No value assertion here: a Boolean is always true or false; the invariant under + // test is absence of exceptions and absence of data races on the AtomicReference. + repeat(100) { + configValues.getValueCached(param) + } + + testScheduler.runCurrent() + + // After all coroutines complete, the snapshot must hold one of the written values + // (not an illegal state). Source must be LOCAL because override() was called. + assertEquals(ConfigValue.Source.LOCAL, configValues.getValueCached(param).source) + } + + // --------------------------------------------------------------------------- + // Duplicate-key last-write-wins semantic + // --------------------------------------------------------------------------- + + @Test + fun `last-write-wins - two ConfigParams with same key share snapshot slot`() = + runTest { + val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) + val param1 = ConfigParam(key = "shared_key", defaultValue = false) + val param2 = ConfigParam(key = "shared_key", defaultValue = false) + + configValues.override(param1, false) + configValues.override(param2, true) // wins because it is the last write + + // Both params read from the same snapshot slot + assertEquals(true, configValues.getValueCached(param1).value) + assertEquals(true, configValues.getValueCached(param2).value) + } + + // --------------------------------------------------------------------------- + // Sync isEnabled + // --------------------------------------------------------------------------- + + @Test + fun `sync isEnabled returns false before warm-up for default=false param`() { + val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) + + assertEquals(false, configValues.isEnabled(param)) + } + + @Test + fun `sync isEnabled returns true after override with true value`() = + runTest { + val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) + configValues.override(param, true) + + assertEquals(true, configValues.isEnabled(param)) + } +} diff --git a/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensionsTest.kt b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensionsTest.kt index 9d02287..c38c2f7 100644 --- a/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensionsTest.kt +++ b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesExtensionsTest.kt @@ -70,7 +70,9 @@ class ConfigValuesExtensionsTest { runTest { val provider = InMemoryConfigValueProvider() val configValues = ConfigValues(localProvider = provider) - provider.set(darkModeParam, true) + // Use configValues.override so the snapshot is warmed; isEnabled is now non-suspend + // and reads from the snapshot only. + configValues.override(darkModeParam, true) assertEquals(true, configValues.isEnabled(darkModeParam)) } From 6c1208aa171b79c86c0d79dffb66626850b6c84a Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 20 May 2026 11:22:41 +0300 Subject: [PATCH 2/5] Generate non-suspend is*Enabled extensions calling getValueCached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExtensionFunctionGenerator stops emitting suspend on the per-flag extensions and switches the underlying call from suspend getValue to the new non-suspend getValueCached (added in the previous commit). The generated isEnabled() / get() helpers can now be called from any context — including @Composable bodies — without dragging a coroutine scope along. The JVM method signature returns to plain `boolean isEnabled(ConfigValues)` (no Continuation parameter), so the ProGuard -assumevalues rule shape that ProguardRulesGenerator already emits matches the real method again and R8 can resume per-function DCE on dead-flag branches. The shrinker-tests subsystem and the ProGuard rule format itself need no changes — Phase 4 just removes the "no-op" caveats from the relevant KDoc. Generator unit tests pin the new non-suspend / getValueCached shape; end-to-end sample assemble (Android / Desktop / iOS sim) stays green. Co-Authored-By: Claude Opus 4.7 --- .../gradle/ExtensionFunctionGenerator.kt | 22 ++++++---------- .../gradle/ExtensionFunctionGeneratorTest.kt | 26 ++++++++++--------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt b/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt index c9e89f1..97c773b 100644 --- a/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt +++ b/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt @@ -6,29 +6,24 @@ package dev.androidbroadcast.featured.gradle * * **Local Boolean flags** get an `is…Enabled()` extension returning the raw `Boolean`: * ```kotlin - * internal suspend fun ConfigValues.isDarkModeEnabled(): Boolean = getValue(GeneratedLocalFlags.darkMode).value + * internal fun ConfigValues.isDarkModeEnabled(): Boolean = getValueCached(GeneratedLocalFlags.darkMode).value * ``` * * **Local non-Boolean flags** get a `get…()` extension returning the raw value type: * ```kotlin - * internal suspend fun ConfigValues.getMaxRetries(): Int = getValue(GeneratedLocalFlags.maxRetries).value + * internal fun ConfigValues.getMaxRetries(): Int = getValueCached(GeneratedLocalFlags.maxRetries).value * ``` * * **Remote flags** get a `get…()` extension returning `ConfigValue` so callers can * inspect the value source (DEFAULT / REMOTE / etc.): * ```kotlin - * internal suspend fun ConfigValues.getPromoBannerEnabled(): ConfigValue = - * getValue(GeneratedRemoteFlags.promoBannerEnabled) + * internal fun ConfigValues.getPromoBannerEnabled(): ConfigValue = + * getValueCached(GeneratedRemoteFlags.promoBannerEnabled) * ``` * * Extensions are `internal` because no external production consumer depends on them — modules * that need `ConfigParam` values directly use `observe(GeneratedLocalFlags.x)` against the - * now-`public` generated objects. The `suspend` modifier is required because - * `ConfigValues.getValue` is a `suspend` function. - * - * Note: the ProGuard `-assumevalues` rules emitted by [ProguardRulesGenerator] target the - * non-suspend JVM signature and are therefore **no-ops** for the current generated shape. - * This is a known follow-up item — see tracked issue for the per-function DCE rework. + * now-`public` generated objects. * * **JVM class-name uniqueness:** `@file:JvmName` is intentionally absent — it is not * supported on Kotlin/Native targets. Instead, the emitted file is named @@ -61,8 +56,7 @@ public object ExtensionFunctionGenerator { * Returns the legacy `@file:JvmName` value that was previously emitted into the source file. * * This function is retained for use by [ProguardRulesGenerator], which needs to reference - * the JVM class name in `-assumevalues` rules. Note that those rules are currently no-ops - * because the generated extensions are `suspend` (ProGuard rework is a follow-up item). + * the JVM class name in `-assumevalues` rules. * * Examples: `":app"` → `"FeaturedApp_FlagExtensionsKt"`, * `":feature:checkout"` → `"FeaturedFeatureCheckout_FlagExtensionsKt"`. @@ -111,12 +105,12 @@ public object ExtensionFunctionGenerator { val objectRef = if (isLocal) localObjectName else remoteObjectName return if (isLocal) { val funcName = extensionFunctionName() - "internal suspend fun ConfigValues.$funcName(): $type = getValue($objectRef.$propertyName).value\n" + "internal fun ConfigValues.$funcName(): $type = getValueCached($objectRef.$propertyName).value\n" } else { // Remote flags always use get… regardless of type — the return is ConfigValue, // so callers can inspect the value source. val funcName = "get${propertyName.capitalized()}" - "internal suspend fun ConfigValues.$funcName(): ConfigValue<$type> = getValue($objectRef.$propertyName)\n" + "internal fun ConfigValues.$funcName(): ConfigValue<$type> = getValueCached($objectRef.$propertyName)\n" } } } diff --git a/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGeneratorTest.kt b/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGeneratorTest.kt index 82f2d2e..5e28f7f 100644 --- a/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGeneratorTest.kt +++ b/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGeneratorTest.kt @@ -51,21 +51,22 @@ class ExtensionFunctionGeneratorTest { fun `generates is…Enabled extension for local boolean flag`() { val entries = listOf(localEntry("dark_mode", "Boolean")) val source = ExtensionFunctionGenerator.generate(entries, modulePath) - assertContains(source, "suspend fun ConfigValues.isDarkModeEnabled(): Boolean") + assertContains(source, "fun ConfigValues.isDarkModeEnabled(): Boolean") } @Test - fun `local boolean extension returns raw value`() { + fun `local boolean extension returns raw value via getValueCached`() { val entries = listOf(localEntry("dark_mode", "Boolean")) val source = ExtensionFunctionGenerator.generate(entries, modulePath) - assertContains(source, "getValue(GeneratedLocalFlagsFeatureCheckout.darkMode).value") + assertContains(source, "getValueCached(GeneratedLocalFlagsFeatureCheckout.darkMode).value") } @Test - fun `local boolean extension is internal suspend`() { + fun `local boolean extension is internal non-suspend`() { val entries = listOf(localEntry("dark_mode", "Boolean")) val source = ExtensionFunctionGenerator.generate(entries, modulePath) - assertContains(source, "internal suspend fun ConfigValues.isDarkModeEnabled()") + assertContains(source, "internal fun ConfigValues.isDarkModeEnabled()") + assertFalse(source.contains("suspend fun ConfigValues.isDarkModeEnabled()"), "Must not emit suspend modifier") } // ── local non-boolean flag ──────────────────────────────────────────────── @@ -74,15 +75,15 @@ class ExtensionFunctionGeneratorTest { fun `generates get… extension for local int flag`() { val entries = listOf(localEntry("max_retries", "Int")) val source = ExtensionFunctionGenerator.generate(entries, modulePath) - assertContains(source, "suspend fun ConfigValues.getMaxRetries(): Int") - assertContains(source, "getValue(GeneratedLocalFlagsFeatureCheckout.maxRetries).value") + assertContains(source, "fun ConfigValues.getMaxRetries(): Int") + assertContains(source, "getValueCached(GeneratedLocalFlagsFeatureCheckout.maxRetries).value") } @Test fun `generates get… extension for local string flag`() { val entries = listOf(localEntry("api_url", "String")) val source = ExtensionFunctionGenerator.generate(entries, modulePath) - assertContains(source, "suspend fun ConfigValues.getApiUrl(): String") + assertContains(source, "fun ConfigValues.getApiUrl(): String") } // ── local enum flag ─────────────────────────────────────────────────────── @@ -91,8 +92,8 @@ class ExtensionFunctionGeneratorTest { fun `generates get… extension for local enum flag`() { val entries = listOf(localEntry("checkout_variant", "com.example.CheckoutVariant")) val source = ExtensionFunctionGenerator.generate(entries, modulePath) - assertContains(source, "suspend fun ConfigValues.getCheckoutVariant(): com.example.CheckoutVariant") - assertContains(source, "getValue(GeneratedLocalFlagsFeatureCheckout.checkoutVariant).value") + assertContains(source, "fun ConfigValues.getCheckoutVariant(): com.example.CheckoutVariant") + assertContains(source, "getValueCached(GeneratedLocalFlagsFeatureCheckout.checkoutVariant).value") } @Test @@ -108,8 +109,9 @@ class ExtensionFunctionGeneratorTest { fun `generates get… extension returning ConfigValue for remote flag`() { val entries = listOf(remoteEntry("promo_banner", "Boolean")) val source = ExtensionFunctionGenerator.generate(entries, modulePath) - assertContains(source, "suspend fun ConfigValues.getPromoBanner(): ConfigValue") - assertContains(source, "getValue(GeneratedRemoteFlagsFeatureCheckout.promoBanner)") + assertContains(source, "fun ConfigValues.getPromoBanner(): ConfigValue") + assertContains(source, "getValueCached(GeneratedRemoteFlagsFeatureCheckout.promoBanner)") + assertFalse(source.contains("suspend "), "Must not emit suspend modifier anywhere") } @Test From 2a16bc7f124e59a507631e4f82db70a2e7f30275 Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 20 May 2026 11:38:09 +0300 Subject: [PATCH 3/5] Point ProGuard assumevalues rules at the real generated class name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProguardRulesGenerator derived the target Kotlin file class name via ExtensionFunctionGenerator.jvmFileName, which reproduced the old @file:JvmName pattern that PR D removed. The real compiled class is named after the source file (e.g. GeneratedFlagExtensionsSampleFeatureUiKt), so R8 silently no-op'd the -assumevalues directive — per-function dead- code elimination did not actually happen in consumer builds. Module paths with hyphens additionally produced JVM-illegal identifiers in the emitted rules, which R8 also ignored without warning. Switch the rule target to the file-name-derived class (drop jvmFileName, reuse ExtensionFunctionGenerator.fileName so the sanitization stays in one place) and update SyntheticBytecodeFactory + the R8 elimination tests to match. The shrinker-tests suite now exercises R8 against bytecode whose JVM class name matches what the rules target — the pipeline is consistent again and the existing R8 DCE assertions are finally exercising the production shape. Co-Authored-By: Claude Opus 4.7 --- .../featured/gradle/ExtensionFunctionGenerator.kt | 11 ----------- .../featured/gradle/ProguardRulesGenerator.kt | 8 +++++--- .../gradle/FeaturedPluginIntegrationTest.kt | 12 +++++++----- .../featured/gradle/ProguardRulesGeneratorTest.kt | 2 +- .../shrinker/bytecode/SyntheticBytecodeFactory.kt | 14 ++++++++------ .../shrinker/r8/R8BooleanFlagEliminationTest.kt | 4 ++-- .../shrinker/r8/R8IntFlagEliminationTest.kt | 4 ++-- 7 files changed, 25 insertions(+), 30 deletions(-) diff --git a/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt b/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt index 97c773b..de07b12 100644 --- a/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt +++ b/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGenerator.kt @@ -52,17 +52,6 @@ public object ExtensionFunctionGenerator { */ public fun fileName(modulePath: String): String = "GeneratedFlagExtensions${modulePath.modulePathToFileSuffix()}.kt" - /** - * Returns the legacy `@file:JvmName` value that was previously emitted into the source file. - * - * This function is retained for use by [ProguardRulesGenerator], which needs to reference - * the JVM class name in `-assumevalues` rules. - * - * Examples: `":app"` → `"FeaturedApp_FlagExtensionsKt"`, - * `":feature:checkout"` → `"FeaturedFeatureCheckout_FlagExtensionsKt"`. - */ - public fun jvmFileName(modulePath: String): String = "Featured${modulePath.modulePathToIdentifier()}_FlagExtensionsKt" - /** * Generates the full source text for the module-specific `GeneratedFlagExtensions.kt`. * diff --git a/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGenerator.kt b/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGenerator.kt index d1961a3..0cad793 100644 --- a/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGenerator.kt +++ b/build-logic/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGenerator.kt @@ -9,7 +9,7 @@ package dev.androidbroadcast.featured.gradle * * Example output for a Boolean flag `dark_mode = false` in module `:feature:ui`: * ```proguard - * -assumevalues class dev.androidbroadcast.featured.generated.FeaturedFeatureUi_FlagExtensionsKt { + * -assumevalues class dev.androidbroadcast.featured.generated.GeneratedFlagExtensionsFeatureUiKt { * boolean isDarkModeEnabled(dev.androidbroadcast.featured.ConfigValues) return false; * } * ``` @@ -31,7 +31,9 @@ public object ProguardRulesGenerator { * Generates ProGuard `-assumevalues` rules for all local flags in [entries]. * * [modulePath] is the Gradle module path (e.g. `":feature:ui"`) used to derive - * the JVM class name of the generated extensions file via [ExtensionFunctionGenerator.jvmFileName]. + * the JVM class name of the generated extensions file. The class name is derived from + * [ExtensionFunctionGenerator.fileName]: the Kotlin compiler uses the file name + * (without `.kt`) plus the `Kt` suffix as the JVM class name for top-level declarations. * * Returns a blank string when [entries] contains no local flags with a supported type. */ @@ -42,7 +44,7 @@ public object ProguardRulesGenerator { val localEntries = entries.filter { it.isLocal && jvmType(it.type) != null } if (localEntries.isEmpty()) return "" - val className = "$PACKAGE.${ExtensionFunctionGenerator.jvmFileName(modulePath)}" + val className = "$PACKAGE.${ExtensionFunctionGenerator.fileName(modulePath).removeSuffix(".kt")}Kt" return buildString { appendLine("# Auto-generated by Featured Gradle Plugin — do not edit manually.") diff --git a/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPluginIntegrationTest.kt b/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPluginIntegrationTest.kt index ede0348..b894b48 100644 --- a/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPluginIntegrationTest.kt +++ b/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPluginIntegrationTest.kt @@ -189,13 +189,14 @@ class FeaturedPluginIntegrationTest { * * Expected output (from [ProguardRulesGenerator]): * ```proguard - * -assumevalues class dev.androidbroadcast.featured.generated.FeaturedRoot_FlagExtensionsKt { + * -assumevalues class dev.androidbroadcast.featured.generated.GeneratedFlagExtensionsRootKt { * boolean isDarkModeEnabled(dev.androidbroadcast.featured.ConfigValues) return false; * } * ``` * - * The root module path `:` produces the identifier `Root` via [String.modulePathToIdentifier], - * so the JVM class name is `FeaturedRoot_FlagExtensionsKt`. + * The root module path `:` produces the file suffix `Root` via [String.modulePathToFileSuffix], + * so the Kotlin file is `GeneratedFlagExtensionsRoot.kt` and the JVM class name + * (Kotlin's file-to-class convention) is `GeneratedFlagExtensionsRootKt`. * * Enum flags (`checkout_variant`) must not appear in `-assumevalues` rules — their values * are resolved at runtime from providers and cannot be assumed at build time (issue #162). @@ -281,9 +282,10 @@ class FeaturedPluginIntegrationTest { private companion object { // The fixture is a single-project (root) build. - // modulePathToIdentifier(":") → "Root" → jvmFileName → "FeaturedRoot_FlagExtensionsKt" + // modulePathToFileSuffix(":") → "Root" → fileName → "GeneratedFlagExtensionsRoot.kt" + // → JVM class: "GeneratedFlagExtensionsRootKt" const val EXTENSIONS_FQN = - "dev.androidbroadcast.featured.generated.FeaturedRoot_FlagExtensionsKt" + "dev.androidbroadcast.featured.generated.GeneratedFlagExtensionsRootKt" const val CONFIG_VALUES_FQN = "dev.androidbroadcast.featured.ConfigValues" const val IS_DARK_MODE_ENABLED = "isDarkModeEnabled" } diff --git a/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGeneratorTest.kt b/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGeneratorTest.kt index 55f7fab..78b24a1 100644 --- a/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGeneratorTest.kt +++ b/build-logic/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGeneratorTest.kt @@ -8,7 +8,7 @@ import kotlin.test.assertTrue class ProguardRulesGeneratorTest { private val modulePath = ":feature:ui" private val expectedClass = - "dev.androidbroadcast.featured.generated.${ExtensionFunctionGenerator.jvmFileName(modulePath)}" + "dev.androidbroadcast.featured.generated.${ExtensionFunctionGenerator.fileName(modulePath).removeSuffix(".kt")}Kt" // ── empty / no-op cases ────────────────────────────────────────────────── diff --git a/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/bytecode/SyntheticBytecodeFactory.kt b/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/bytecode/SyntheticBytecodeFactory.kt index 71a5674..7c4d1d4 100644 --- a/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/bytecode/SyntheticBytecodeFactory.kt +++ b/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/bytecode/SyntheticBytecodeFactory.kt @@ -34,10 +34,11 @@ internal const val IF_BRANCH_CODE_INTERNAL = "IfBranchCode" internal const val ELSE_BRANCH_CODE_INTERNAL = "ElseBranchCode" internal const val BIFURCATED_CALLER_INTERNAL = "BifurcatedCaller" -// Derived from ExtensionFunctionGenerator.jvmFileName(":test"): -// ":test".removePrefix(":") = "test" → capitalize → "Test" → "FeaturedTest_FlagExtensionsKt" +// Derived from ExtensionFunctionGenerator.fileName(":test"): +// modulePathToFileSuffix(":test") = "Test" → "GeneratedFlagExtensionsTest.kt" +// → JVM class (Kotlin file-to-class convention): "GeneratedFlagExtensionsTestKt" internal const val BOOL_EXTENSIONS_INTERNAL = - "dev/androidbroadcast/featured/generated/FeaturedTest_FlagExtensionsKt" + "dev/androidbroadcast/featured/generated/GeneratedFlagExtensionsTestKt" internal const val IS_DARK_MODE_ENABLED = "isDarkModeEnabled" @@ -46,10 +47,11 @@ internal const val INT_CONFIG_VALUES_INTERNAL = "dev/androidbroadcast/featured/I internal const val POSITIVE_COUNT_CODE_INTERNAL = "PositiveCountCode" internal const val INT_CALLER_INTERNAL = "IntCaller" -// Derived from ExtensionFunctionGenerator.jvmFileName(":int-test"): -// ":int-test".removePrefix(":") = "int-test" → capitalize first char → "Int-test" → "FeaturedInt-test_FlagExtensionsKt" +// Derived from ExtensionFunctionGenerator.fileName(":int-test"): +// modulePathToFileSuffix(":int-test") splits on "-" → "Int" + "Test" = "IntTest" +// → "GeneratedFlagExtensionsIntTest.kt" → JVM class: "GeneratedFlagExtensionsIntTestKt" internal const val INT_EXTENSIONS_INTERNAL = - "dev/androidbroadcast/featured/generated/FeaturedInt-test_FlagExtensionsKt" + "dev/androidbroadcast/featured/generated/GeneratedFlagExtensionsIntTestKt" internal const val GET_MAX_RETRIES = "getMaxRetries" diff --git a/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8BooleanFlagEliminationTest.kt b/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8BooleanFlagEliminationTest.kt index fda8aba..3219a4b 100644 --- a/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8BooleanFlagEliminationTest.kt +++ b/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8BooleanFlagEliminationTest.kt @@ -26,7 +26,7 @@ import kotlin.test.Test * class ConfigValues { boolean enabled; ConfigValues(boolean) } * * // Mirrors ExtensionFunctionGenerator output for module ":test" - * class FeaturedTest_FlagExtensionsKt { + * class GeneratedFlagExtensionsTestKt { * static boolean isDarkModeEnabled(ConfigValues cv) { return cv.enabled; } * } * @@ -41,7 +41,7 @@ import kotlin.test.Test * class BifurcatedCaller { * static void execute(boolean enabled) { * ConfigValues cv = new ConfigValues(enabled); - * if (FeaturedTest_FlagExtensionsKt.isDarkModeEnabled(cv)) { + * if (GeneratedFlagExtensionsTestKt.isDarkModeEnabled(cv)) { * new IfBranchCode().doWork(); * } else { * new ElseBranchCode().doWork(); diff --git a/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8IntFlagEliminationTest.kt b/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8IntFlagEliminationTest.kt index c786092..751a901 100644 --- a/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8IntFlagEliminationTest.kt +++ b/featured-shrinker-tests/src/test/kotlin/dev/androidbroadcast/featured/shrinker/r8/R8IntFlagEliminationTest.kt @@ -18,7 +18,7 @@ import kotlin.test.Test * ```java * class IntConfigValues { int count; IntConfigValues(int) } * - * class FeaturedIntTest_FlagExtensionsKt { + * class GeneratedFlagExtensionsIntTestKt { * static int getMaxRetries(IntConfigValues cv) { return cv.count; } * } * @@ -27,7 +27,7 @@ import kotlin.test.Test * class IntCaller { * static void execute(int count) { * IntConfigValues cv = new IntConfigValues(count); - * if (FeaturedIntTest_FlagExtensionsKt.getMaxRetries(cv) > 0) { + * if (GeneratedFlagExtensionsIntTestKt.getMaxRetries(cv) > 0) { * new PositiveCountCode().doWork(); * } * } From 39d7e0c841c64158771a0ff9cd2c68b04179ea14 Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 20 May 2026 11:42:19 +0300 Subject: [PATCH 4/5] Document sync ConfigValues read path Records the [Unreleased] CHANGELOG entries for the sync ConfigValues work: new getValueCached reader, non-suspend isEnabled extension, generated codegen losing suspend, AutoCloseable on ConfigValues, and the R8 DCE ProGuard rule fix. The sample CLAUDE.md gains a one- paragraph pointer noting that non-reactive call sites should use getValueCached or the codegen extensions, while reactive UI keeps the observe-bridge pattern that already exists in the multi-module sample. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 7 ++++++- sample/CLAUDE.md | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78fb03f..b9970b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,10 +18,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `FeatureFlagsDebugScreen` signature is now `(configValues: ConfigValues, registry: List>, modifier: Modifier = Modifier)` — accepts an explicit registry list instead of reading the (removed) `FlagRegistry` singleton. Pass `GeneratedFeaturedRegistry.all` for the recommended aggregator-plugin flow, or build the list inline for small projects. - `:sample:shared` is now a pure aggregator: it applies `dev.androidbroadcast.featured.application`, declares `featuredAggregation(project(":sample:feature-*"))`, and consumes `GeneratedFeaturedRegistry.all`. The hand-written `SampleFeatureFlags.kt` is removed. - Generator file names include a module-derived suffix (`GeneratedLocalFlagsSampleFeatureCheckout.kt`, etc.) — eliminates JVM class-name collisions when multiple modules share the same classpath. `@file:JvmName` is no longer emitted. -- `ExtensionFunctionGenerator` now emits `suspend` extension functions — `ConfigValues.getValue` has always been suspend; the generated callers now match. `GeneratedLocalFlags*` / `GeneratedRemoteFlags*` objects are widened to `public` so observer bridges can reference them across module boundaries. +- `ExtensionFunctionGenerator` emits non-suspend `is*Enabled()` / `get*()` extension functions — they delegate to `getValueCached` and can be called from any context without a coroutine. Callers that previously wrapped them in `runBlocking { … }` or a coroutine scope can drop the wrapper. `GeneratedLocalFlags*` / `GeneratedRemoteFlags*` objects are widened to `public` so observer bridges can reference them across module boundaries. +- `ConfigValues` implements `AutoCloseable`; call `close()` when tearing down to cancel the internal re-resolve scope. Long-running apps where `ConfigValues` lives for the process lifetime can ignore this. ### Added +- `ConfigValues.getValueCached(param: ConfigParam): ConfigValue` — non-suspend synchronous reader. Returns the last-written `ConfigValue` from the in-memory cache; the cache is warmed on the first `getValue` / `override` / `fetch` call, and returns `Source.DEFAULT` until then. +- `ConfigValues.isEnabled(param: ConfigParam): Boolean` — non-suspend extension (replaces the former `suspend` variant). Delegates to `getValueCached`; safe to call from Composable functions, `init` blocks, and non-coroutine contexts. + - Featured library plugin now publishes a per-module feature-flag manifest as a consumable Gradle artifact (`featuredManifest` configuration, schema v1). Existing flag-generation pipeline is unchanged. Consumer-side aggregation arrives in a follow-up release. - New `dev.androidbroadcast.featured.application` Gradle plugin: aggregates `featured-manifest.json` artifacts from project dependencies declared via `featuredAggregation(project(...))` and generates `object GeneratedFeaturedRegistry { val all: List> }` in `build/generated/featured/commonMain/`. Apply alongside `dev.androidbroadcast.featured` in the application module; wire the output directory into your source set manually (e.g., `kotlin.sourceSets.commonMain.kotlin.srcDir(...)`). Modules declaring `enum` flags also require a regular `implementation(project(...))` dependency in the consumer so the enum class is on the compile classpath; primitive-only modules need only `featuredAggregation(...)`. - Three KMP sample feature modules — `:sample:feature-checkout`, `:sample:feature-promotions`, `:sample:feature-ui` — each declaring its own flags via the `featured { ... }` DSL. Serves as the canonical multi-module reference. @@ -30,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Restored R8 per-function DCE: ProGuard `-assumevalues` rules now target the actual Kotlin-compiled class name (`GeneratedFlagExtensionsXKt`). The rules were silently no-op since `@file:JvmName` was removed in an earlier PR; unused boolean flags are once again eliminated at shrinking time. - iOS framework can now `export(project(":sample:feature-*"))` without the K/N `ObjCExportCodeGenerator` crashing — requires `api(project(...))` linkage in the aggregator module so K/N has access to type adapters for generic `ConfigParam` specializations. ## [1.0.0-Beta1] - 2026-05-17 diff --git a/sample/CLAUDE.md b/sample/CLAUDE.md index cc554da..bd78b67 100644 --- a/sample/CLAUDE.md +++ b/sample/CLAUDE.md @@ -18,6 +18,10 @@ Each `:sample:feature-*` module ships `*FlagObservers.kt` with public `ConfigVal (e.g. `mainButtonRedFlow()`, `setMainButtonRed()`). UI consumers should call these instead of referencing `GeneratedLocalFlags*` / `GeneratedRemoteFlags*` directly. +For non-reactive reads (logging, eager-conditional code paths) use `configValues.getValueCached(param)` +directly — the codegen-emitted `is*Enabled()` / `get*()` extensions are non-suspend and call this +under the hood. + ## Adding a flag 1. Edit the feature module's `build.gradle.kts` — add a declaration inside `featured { localFlags { ... } }` or `featured { remoteFlags { ... } }`. From 1bf0cbc3e5e89f5134f1c85c9167f361ba75d9aa Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 20 May 2026 11:51:34 +0300 Subject: [PATCH 5/5] Re-resolve in resetOverride synchronously; drop AutoCloseable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase A code review flagged that the backgroundScope.launch in resetOverride could clobber a concurrent override(p, X) write: the background re-resolve writes the just-resolved REMOTE value after the caller's LOCAL=X write lands, and snapshot/provider diverge until the next getValue. resetOverride is already suspend, so do the re-resolve inline — the caller pays the same cost and the ordering becomes deterministic. The explicit writeSnapshot call stays because getValue intentionally does not write DEFAULT into the snapshot; without the explicit write a previously overridden slot would stay stale when both providers return null. The synchronous re-resolve makes the internal CoroutineScope unused, so ConfigValues no longer implements AutoCloseable. The CHANGELOG entry is swapped accordingly. observe()'s KDoc gets a more accurate description of write-through behaviour, and the concurrent-write test is renamed to reflect what runTest actually exercises (single-thread coroutine interleaving, not OS-level parallelism). Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 2 +- .../androidbroadcast/featured/ConfigValues.kt | 61 +++++-------------- .../featured/ConfigValuesCachedTest.kt | 12 ++-- 3 files changed, 21 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9970b0..e980d24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `:sample:shared` is now a pure aggregator: it applies `dev.androidbroadcast.featured.application`, declares `featuredAggregation(project(":sample:feature-*"))`, and consumes `GeneratedFeaturedRegistry.all`. The hand-written `SampleFeatureFlags.kt` is removed. - Generator file names include a module-derived suffix (`GeneratedLocalFlagsSampleFeatureCheckout.kt`, etc.) — eliminates JVM class-name collisions when multiple modules share the same classpath. `@file:JvmName` is no longer emitted. - `ExtensionFunctionGenerator` emits non-suspend `is*Enabled()` / `get*()` extension functions — they delegate to `getValueCached` and can be called from any context without a coroutine. Callers that previously wrapped them in `runBlocking { … }` or a coroutine scope can drop the wrapper. `GeneratedLocalFlags*` / `GeneratedRemoteFlags*` objects are widened to `public` so observer bridges can reference them across module boundaries. -- `ConfigValues` implements `AutoCloseable`; call `close()` when tearing down to cancel the internal re-resolve scope. Long-running apps where `ConfigValues` lives for the process lifetime can ignore this. +- `ConfigValues.resetOverride` re-resolves the effective value synchronously through the full provider priority chain; [getValueCached] reflects the updated value immediately after the call returns. ### Added diff --git a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt index 293568f..1e778c7 100644 --- a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt +++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt @@ -3,10 +3,6 @@ package dev.androidbroadcast.featured -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.catch @@ -14,7 +10,6 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge -import kotlinx.coroutines.launch import kotlin.concurrent.atomics.AtomicReference import kotlin.concurrent.atomics.update @@ -48,13 +43,6 @@ import kotlin.concurrent.atomics.update * going through [ConfigValues.override] bypass the snapshot and will not be visible to * [getValueCached] until the next [getValue] or [observe] emission for that parameter. * - * ### Lifecycle - * - * [ConfigValues] owns an internal [CoroutineScope] used for the background re-resolution - * dispatched by [resetOverride]. Call [close] when the instance is no longer needed to cancel - * that scope. In short-lived test code the scope is cleaned up automatically by the test - * framework, so [close] may be omitted there. - * * ```kotlin * val configValues = ConfigValues( * localProvider = InMemoryConfigValueProvider(), @@ -87,7 +75,7 @@ public class ConfigValues( private val localProvider: LocalConfigValueProvider? = null, private val remoteProvider: RemoteConfigValueProvider? = null, private val onProviderError: (Throwable) -> Unit = {}, -) : AutoCloseable { +) { init { require(localProvider != null || remoteProvider != null) { "At least one provider (local or remote) must be provided." @@ -110,13 +98,6 @@ public class ConfigValues( */ private val snapshot = AtomicReference>>(emptyMap()) - /** - * Internal scope for background re-resolution dispatched by [resetOverride]. - * Uses [SupervisorJob] so that a failure in one re-resolution does not cancel others. - * Cancelled by [close]. - */ - private val backgroundScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) - /** Writes [configValue] into the snapshot under [param]'s key (copy-on-write). */ private fun writeSnapshot( param: ConfigParam, @@ -222,24 +203,21 @@ public class ConfigValues( * Clears the local override for the given parameter, so subsequent reads fall back * to remote or default values. * - * After the local override is cleared, a background coroutine re-resolves the effective - * value through the full provider priority chain and updates the sync snapshot. Until that - * coroutine completes, [getValueCached] may still return the previous override value. - * - * The background coroutine runs on [Dispatchers.Default] and is owned by an internal - * [CoroutineScope]; call [close] to cancel it when this [ConfigValues] instance is - * no longer needed. + * After the local override is cleared, the effective value is re-resolved synchronously + * through the full provider priority chain and written through to the sync snapshot. + * [getValueCached] reflects the new value as soon as this function returns. * * @param param The configuration parameter whose local override should be cleared. */ public suspend fun resetOverride(param: ConfigParam) { localProvider?.resetOverride(param) // Re-resolve via the full priority chain and write through so the snapshot converges - // to remote/default rather than staying at the stale LOCAL value (Option B from design). - backgroundScope.launch { - val resolved = getValue(param) - writeSnapshot(param, resolved) - } + // to remote/default rather than staying at the stale LOCAL value. + // Explicit writeSnapshot is required because getValue intentionally does not write + // DEFAULT into the snapshot (see getValue implementation). Without this write, a + // previously overridden slot would remain stale even when both providers return null. + val resolved = getValue(param) + writeSnapshot(param, resolved) } /** @@ -298,10 +276,10 @@ public class ConfigValues( * - the value changes via the local provider, **or** * - [fetch] completes and the remote provider returns a new value. * - * Note: local provider emissions that bypass [ConfigValues.override] (i.e. direct calls to - * the provider's own `set` method) do not write through to the sync snapshot via this flow. - * They are still emitted reactively but the snapshot is only updated via the [getValue] - * call in `remoteFlow`. This is a Phase-1 limitation. + * Note: local-provider direct emissions (i.e. direct calls to the provider's own `set` + * method, bypassing [ConfigValues.override]) reach observers reactively but do **not** write + * through to the snapshot. Use [ConfigValues.override] instead of the provider's `set` if + * [getValueCached] must reflect the write. * * @param param The configuration parameter to observe. * @return A [Flow] of [ConfigValue] for the specified parameter. @@ -317,17 +295,6 @@ public class ConfigValues( }.distinctUntilChanged() } - /** - * Cancels the internal [CoroutineScope] used for background re-resolution in [resetOverride]. - * - * Safe to omit in short-lived test code — the test framework cleans up coroutines. - * Should be called explicitly in production code when this [ConfigValues] instance is - * no longer needed (e.g. in `onDestroy` or a scope-bound lifecycle hook). - */ - override fun close() { - backgroundScope.cancel() - } - /** Companion object used as a receiver for extension factories (e.g. ConfigValues.fake). */ public companion object } diff --git a/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt index 9d822d7..4000dc6 100644 --- a/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt +++ b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesCachedTest.kt @@ -139,24 +139,24 @@ class ConfigValuesCachedTest { } // --------------------------------------------------------------------------- - // Thread-safety: concurrent reads and writes + // Snapshot consistency: concurrent coroutine writes (single-threaded dispatcher) // --------------------------------------------------------------------------- @Test - fun `getValueCached is thread-safe under concurrent reads and writes`() = + fun `getValueCached does not corrupt snapshot under interleaved coroutine writes`() = runTest { val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) - // Launch 100 concurrent override writes with alternating values + // Launch 100 interleaved override writes with alternating values. + // runTest runs on a single-threaded dispatcher, so this exercises interleaving + // of coroutine suspension points rather than true OS-thread parallelism. repeat(100) { i -> launch { configValues.override(param, i % 2 == 0) } } - // Perform 100 concurrent reads while writes are in flight — must not throw. - // No value assertion here: a Boolean is always true or false; the invariant under - // test is absence of exceptions and absence of data races on the AtomicReference. + // Reads during interleaved writes must not throw and must return a valid Boolean. repeat(100) { configValues.getValueCached(param) }