Clean up unreferenced topic files after successful remote-config refresh#3439
Conversation
cce8354 to
b8f5223
Compare
1a10e10 to
9d39923
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3439 +/- ##
==========================================
- Coverage 79.63% 79.62% -0.01%
==========================================
Files 368 368
Lines 14829 14853 +24
Branches 2028 2037 +9
==========================================
+ Hits 11809 11827 +18
- Misses 2200 2202 +2
- Partials 820 824 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b8f5223 to
9cab8bc
Compare
9d39923 to
07ee1a7
Compare
07ee1a7 to
3a747f6
Compare
9cab8bc to
191c8ba
Compare
3a747f6 to
85a2bc8
Compare
191c8ba to
4e3d34d
Compare
85a2bc8 to
e2f648d
Compare
4e3d34d to
61a48d6
Compare
e2f648d to
88436cf
Compare
61a48d6 to
8c72036
Compare
📸 Snapshot Test591 unchanged
🛸 Powered by Emerge Tools |
8c72036 to
b3336d9
Compare
88436cf to
92529c2
Compare
b3336d9 to
bc65ad3
Compare
92529c2 to
6052810
Compare
3985e09 to
74eedab
Compare
9a1c9ca to
8f91f3d
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
74eedab to
e1dac4b
Compare
ajpallares
left a comment
There was a problem hiding this comment.
Looking good! I do have one concern that we may want to address before merging
ajpallares
left a comment
There was a problem hiding this comment.
Great progress!! A couple of nits, but no blockers. Good job!
| private fun deleteUnreferencedTopicFiles(referenced: Map<Topic, Set<String>>) { | ||
| val topicsRoot = File(applicationContext.noBackupFilesDir, TOPICS_ROOT) | ||
| if (!topicsRoot.exists()) return | ||
| Topic.values().forEach topics@{ topic -> |
There was a problem hiding this comment.
Seems like since Kotlin 1.9, entries is the recommended replacement for values()
| Topic.values().forEach topics@{ topic -> | |
| Topic.entries.forEach topics@{ topic -> |
There was a problem hiding this comment.
Indeed... but that requires opting in to an experimental flag because we are supporting older kotlin versions... So I think it's better to keep as is :)
| private fun deleteUnreferencedTopicFiles(referenced: Map<Topic, Set<String>>) { | ||
| val topicsRoot = File(applicationContext.noBackupFilesDir, TOPICS_ROOT) | ||
| if (!topicsRoot.exists()) return | ||
| Topic.values().forEach topics@{ topic -> |
There was a problem hiding this comment.
If we ever remove a case from the Topic enum, we would end up with an unused directory here. I guess that's fine for now
There was a problem hiding this comment.
That's a valid point... Could be a further cleanup we do but I won't block the PR because of this. Good catch though!
Merge activity
|
**This is an automatic release.** ## RevenueCat SDK ### 📦 Dependency Updates * [RENOVATE] Update dependency gradle to v8.14.5 (#3459) via RevenueCat Git Bot (@RCGitBot) ## RevenueCatUI SDK ### ✨ New Features * Pre-warm image cache for workflow step states (#3447) via Cesar de la Vega (@vegaro) ### Paywallv2 #### ✨ New Features * Add `close_workflow` button action (#3453) via Cesar de la Vega (@vegaro) #### 🐞 Bugfixes * Fix preload VideoComponent fallback override images (#3449) via Cesar de la Vega (@vegaro) ### 🔄 Other Changes * Select blob source by priority and weighted random (#3458) via Toni Rico (@tonidero) * [AUTOMATIC] Update golden test files for backend integration tests (#3473) via RevenueCat Git Bot (@RCGitBot) * Clean up unreferenced topic files after successful remote-config refresh (#3439) via Toni Rico (@tonidero) * Cache remote config response in memory with TTL and persist to disk (#3457) via Toni Rico (@tonidero) * build(deps): bump fastlane from 2.233.1 to 2.234.0 (#3463) via dependabot[bot] (@dependabot[bot]) * Update codelabs links (#3460) via Jaewoong Eum (@skydoves) * Add RemoteConfigManager and TopicFetcher (#3437) via Toni Rico (@tonidero) * Add exit offers support to workflows (#3452) via Cesar de la Vega (@vegaro) * Update baseline profiles (#3461) via RevenueCat Git Bot (@RCGitBot) * Add network scaffolding for remote config endpoint (#3435) via Toni Rico (@tonidero) * test: cover singleStepFallbackId == initialStepId edge case (#3445) via Facundo Menzella (@facumenzella) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: this is a release/versioning update (SNAPSHOT -> final) plus docs deployment path changes, with no functional code changes beyond version constants. > > **Overview** > Finalizes the `10.6.0` release by switching all version references from `10.6.0-SNAPSHOT` to `10.6.0` (root `.version`, `gradle.properties`, `Config.frameworkVersion`, and sample/test app `libs.versions.toml` files). > > Updates documentation publishing to point at the `10.6.0` docs path (CircleCI S3 sync target and `docs/index.html` redirect), and prepends the `10.6.0` section to `CHANGELOG.md`/`CHANGELOG.latest.md`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4da1697. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
### Motivation First step in landing remote-config support. The backend exposes `GET /v2/config`, which returns a manifest describing remote-controlled topics (e.g. product-entitlement mapping) plus the asset sources for downloading them. Wiring up the endpoint and its response models is a prerequisite for the orchestration and on-disk cache work in the rest of the stack. ### Description - Adds `Endpoint.GetRemoteConfig` to the sealed endpoint hierarchy and `Backend.getRemoteConfig(appUserID, appInBackground, onSuccess, onError)` to dispatch the call. Reuses the existing `BackgroundAwareCallbackCacheKey` / `RemoteConfigCallback` callback-coalescing pattern. - Adds the response models in a new feature package `com.revenuecat.purchases.common.remoteconfig`: - `RemoteConfigResponse(apiSources, blobSources, manifest)` - `ApiSource(id, url, priority, weight)` - `BlobSource(id, urlFormat, priority, weight)` - `Manifest(topics)` - `Topic` enum (`PRODUCT_ENTITLEMENT_MAPPING`) with `fromKey(key)` for wire-key lookups - `TopicEntry(blobRef)` - `TopicsMapSerializer` — custom kotlinx-serialization `KSerializer` that drops unknown topic wire keys at decode time and re-emits the enum's wire key on encode. - Test coverage for the serializer (unknown-key filtering, default decoding, round-trip), plus `BackendGetRemoteConfigTest` for the endpoint plumbing and error paths. - Adds a developer-only build-time override: setting `REMOTE_CONFIG_BASE_URL` in `local.properties` (e.g. `http://localhost:8080/`) redirects only `getRemoteConfig` to that host. Empty by default and gated to debug builds, so CI / release traffic is unaffected. Plumbed via a new `BuildConfig.REMOTE_CONFIG_BASE_URL` field, mirroring the existing `ENABLE_EXTRA_REQUEST_LOGGING` pattern. All new types are `internal`; no public API surface change. ### Stack - This PR (base): RevenueCat#3435 - ⬆ `Add RemoteConfigManager and TopicFetcher`: RevenueCat#3437 - ⬆ `Clean up unreferenced topic files after successful remote-config refresh`: RevenueCat#3439 ### Checklist - [x] Unit tests - [ ] Follow-up issues for `purchases-ios` / hybrids (deferred — feature is not yet wired to any consumer) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new backend networking path (`GET /v2/config`) and callback coalescing logic in `Backend`, plus a debug-only base URL override; issues here could affect request routing or parsing for this new call. > > **Overview** > Adds initial remote-config networking support by introducing `Endpoint.GetRemoteConfig` (`/v2/config`) and a new `Backend.getRemoteConfig` call that coalesces concurrent requests and parses a typed `RemoteConfigResponse`. > > Introduces internal remote-config response models (including a custom serializer that drops unknown topic keys) and adds unit tests covering endpoint properties, parsing/unknown-field behavior, error propagation, and request de-duping. > > Adds a debug-only `REMOTE_CONFIG_BASE_URL` `BuildConfig` field (documented in `local.properties.example`) to optionally route only the remote-config GET request to a local host. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a7f33ec. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
### Motivation Builds on RevenueCat#3435 by adding the orchestration layer that turns a remote-config response into a populated on-disk cache of topic blobs. With this PR, calling `RemoteConfigManager.updateRemoteConfigIfNeeded` will (a) hit the backend for the latest manifest and (b) download every referenced topic blob, verify its SHA-256, and persist it under `noBackupFilesDir`. Subsequent stack PRs build on this orchestration. ### Description Adds two classes in the `remoteconfig` package: - **`TopicFetcher`** — exposes `suspend fun fetchTopicIfNeeded(topic, variant, topicEntry, source): PurchasesError?`. Given the inputs, downloads the blob from `source.urlFormat` (with `{blob_ref}` substituted), writes to a temp file, verifies SHA-256 against `topicEntry.blobRef`, and atomically renames into `noBackupFilesDir/RevenueCat/topics/<topic.key>/<blobRef>`. The blocking I/O is wrapped in `withContext(Dispatchers.IO)`. Skips the download entirely when the target file already exists. Errors surface as `PurchasesError(NetworkError, ...)` for checksum mismatch or `IOException → toPurchasesError()` for transport failures. - **`RemoteConfigManager`** — keeps a callback boundary externally (`fun updateRemoteConfigIfNeeded(appInBackground, completion)`) since its caller (`PurchasesOrchestrator`) is callback-shaped. Internally launches a `private suspend fun refresh(...)` on a private `CoroutineScope(SupervisorJob() + dispatcher)` (`dispatcher: CoroutineDispatcher = Dispatchers.IO`, overridable for tests). The refresh: - Bridges the legacy callback-based `Backend.getRemoteConfig` via `suspendCancellableCoroutine` + `safeResume`/`safeResumeWithException(PurchasesException(error))`. The backend network call stays callback-based intentionally. - Fans the topic downloads out with `coroutineScope { tasks.map { async { topicFetcher.fetchTopicIfNeeded(...) } }.awaitAll().firstNotNullOfOrNull { it } }`. The first error wins; structured concurrency makes this much smaller than a hand-rolled `CompletionTracker`. Currently only the `DEFAULT` variant of each known topic is downloaded. ### Tests `runTest` + `UnconfinedTestDispatcher(testScheduler)` for both. `coEvery` / `coVerify` for the suspend `TopicFetcher`. Coverage: - **`TopicFetcherTest`** — cache hit, successful download + verify + persist, URL-format substitution, HTTP failure, IO failure, SHA mismatch, multi-variant filename isolation. - **`RemoteConfigManagerTest`** — empty-source / empty-topics / no-DEFAULT skip paths, first-error-wins fan-out, backend-error short-circuit, background flag forwarded, null completion. All new types are `internal`. No public API surface change. ### Stack - ⬇ `Add network scaffolding for remote config endpoint`: RevenueCat#3435 - This PR: RevenueCat#3437 - ⬆ `Clean up unreferenced topic files after successful remote-config refresh`: RevenueCat#3439 ### Checklist - [x] Unit tests - [ ] Follow-up issues for `purchases-ios` / hybrids (deferred) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new remote-config orchestration that performs concurrent network downloads and filesystem writes under `noBackupFilesDir`, including checksum verification and new error-handling paths. While internal-only, bugs here could impact startup/network behavior and caching correctness. > > **Overview** > Adds a new remote-config orchestration layer: `RemoteConfigManager.updateRemoteConfigIfNeeded` fetches the remote-config manifest from `Backend.getRemoteConfig` (bridged into coroutines) and concurrently downloads each topic’s `default` entry via a new `TopicFetcher`, returning the first download error (if any). > > Introduces `TopicFetcher` to cache topic blobs on disk under `noBackupFilesDir/RevenueCat/topics/<topic>/<blob_ref>`, with blob-ref input validation, limited parallel downloads, temp-file writes, and SHA-256 verification. > > Refactors `FontLoader` to reuse new `UrlConnectionFactory.downloadToFile` helpers, and extends `UrlConnectionFactory` with shared download + checksum-verification utilities. Updates/expands tests to cover the new manager/fetcher behavior and changes the manifest entry key from `DEFAULT` to `default` in `RemoteConfigResponseTest`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8cdae44. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Motivation
TopicFetcherwrites a new file every time the manifest'sblob_refrotates, but never deletes the previous one. Without cleanup, the on-disk cache atnoBackupFilesDir/RevenueCat/topics/...grows unboundedly across backend rotations. This PR adds a one-shot disk-hygiene pass that runs at the end of each fully successful remote-config refresh.Description
suspend fun TopicFetcher.cleanupUnreferencedTopics(referenced: Map<Topic, Set<String>>)— wraps the disk walk inwithContext(Dispatchers.IO), iteratesTopic.values(), lists each topic's directory, and deletes files whose name isn't in the keep-set for that topic. Skips files prefixed withrc_topic_(thecreateTempFileprefix, now extracted as aTEMP_PREFIXconstant) so a concurrent in-flight download isn't clobbered.RemoteConfigManager.refresh— builds the reference set as the union of every blob ref in the new manifest (across all variants, not justDEFAULT) and triggers cleanup at the right moment, all within the existing suspend coroutine flow:sourcesis empty or no DEFAULT-variant tasks remain → cleanup runs immediately (fetchError = nullshort-circuit).async/awaitAllfan-out, cleanup is awaited only if every fetch returnednull(no error). Partial-failure refreshes leave the cache untouched.Confirmed semantics:
Topicenum but absent from the new manifest get all their cached files wiped (reference-set = ∅).Tests
runTestfor both:TopicFetcherTestcases: delete unreferenced / wipe topic absent from set / no-op when root missing / preserverc_topic_*.tmp/ mixed keep+delete.RemoteConfigManagerTestcases (withcoEvery { topicFetcher.cleanupUnreferencedTopics(any()) } returns Unit+coVerify): cleanup with all-variant refs / cleanup on empty sources & empty topics / no cleanup on fetcher error / no cleanup on backend error.All changes are
internal; no public API surface change.Stack
Add network scaffolding for remote config endpoint: Add network scaffolding for remote config endpoint #3435Add RemoteConfigManager and TopicFetcher: Add RemoteConfigManager and TopicFetcher #3437Checklist
purchases-ios/ hybrids (deferred)Note
Medium Risk
Adds automatic deletion of on-disk topic files after successful remote-config refresh; mistakes in reference calculation or deletion logic could remove still-needed cached data, though it is guarded to run only on fully successful refreshes and has test coverage.
Overview
After a fully successful remote-config refresh, the SDK now performs a one-shot cleanup pass to delete unreferenced cached topic blob files under
noBackupFilesDir/RevenueCat/topics, preventing unbounded growth whenblob_refs rotate.RemoteConfigManagerbuilds a per-topic keep-set from all manifest entryIds’blob_refs and callsTopicFetcher.cleanupUnreferencedTopics()only when refresh completes with no errors (and caching conditions are met).TopicFetcherimplements the disk walk/deletion, skips in-flight temp files via a sharedrc_topic_prefix constant, and adds unit tests covering the new cleanup behavior and its failure/no-op cases.Reviewed by Cursor Bugbot for commit 9503c36. Bugbot is set up for automated code reviews on this repo. Configure here.