diff --git a/CHANGELOG.md b/CHANGELOG.md index 78fb03f..e980d24 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.resetOverride` re-resolves the effective value synchronously through the full provider priority chain; [getValueCached] reflects the updated value immediately after the call returns. ### 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/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..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 @@ -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 @@ -57,18 +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. Note that those rules are currently no-ops - * because the generated extensions are `suspend` (ProGuard rework is a follow-up item). - * - * 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`. * @@ -111,12 +94,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/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/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 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/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt index dd755c8..1e778c7 100644 --- a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt +++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt @@ -1,4 +1,5 @@ @file:Suppress("unused") +@file:OptIn(kotlin.concurrent.atomics.ExperimentalAtomicApi::class) package dev.androidbroadcast.featured @@ -9,6 +10,8 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge +import kotlin.concurrent.atomics.AtomicReference +import kotlin.concurrent.atomics.update /** * Central access point for reading, overriding, and observing configuration values. @@ -28,6 +31,18 @@ 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. + * * ```kotlin * val configValues = ConfigValues( * localProvider = InMemoryConfigValueProvider(), @@ -38,7 +53,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 @@ -66,6 +84,58 @@ 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()) + + /** 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 +144,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 +156,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 +182,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 +194,30 @@ 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, 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. + // 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) } /** @@ -127,6 +225,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 +245,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 +258,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 +276,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 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. */ 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..4000dc6 --- /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) + } + + // --------------------------------------------------------------------------- + // Snapshot consistency: concurrent coroutine writes (single-threaded dispatcher) + // --------------------------------------------------------------------------- + + @Test + fun `getValueCached does not corrupt snapshot under interleaved coroutine writes`() = + runTest { + val configValues = ConfigValues(localProvider = InMemoryConfigValueProvider()) + + // 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) + } + } + + // Reads during interleaved writes must not throw and must return a valid Boolean. + 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)) } 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(); * } * } 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 { ... } }`.