Skip to content

Clean up unreferenced topic files after successful remote-config refresh#3439

Merged
tonidero merged 2 commits into
mainfrom
toniricodiez/add-remote-config-topic-cleanup
May 12, 2026
Merged

Clean up unreferenced topic files after successful remote-config refresh#3439
tonidero merged 2 commits into
mainfrom
toniricodiez/add-remote-config-topic-cleanup

Conversation

@tonidero

@tonidero tonidero commented May 6, 2026

Copy link
Copy Markdown
Contributor

Motivation

TopicFetcher writes a new file every time the manifest's blob_ref rotates, but never deletes the previous one. Without cleanup, the on-disk cache at noBackupFilesDir/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 in withContext(Dispatchers.IO), iterates Topic.values(), lists each topic's directory, and deletes files whose name isn't in the keep-set for that topic. Skips files prefixed with rc_topic_ (the createTempFile prefix, now extracted as a TEMP_PREFIX constant) 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 just DEFAULT) and triggers cleanup at the right moment, all within the existing suspend coroutine flow:
    • When sources is empty or no DEFAULT-variant tasks remain → cleanup runs immediately (fetchError = null short-circuit).
    • When downloads dispatch → after the async/awaitAll fan-out, cleanup is awaited only if every fetch returned null (no error). Partial-failure refreshes leave the cache untouched.

Confirmed semantics:

  • Reference set spans all variants in the new manifest (forward-compatible if non-DEFAULT variants ever start being cached).
  • Topics in the SDK's Topic enum but absent from the new manifest get all their cached files wiped (reference-set = ∅).
  • Cleanup runs only when every download in the refresh succeeded.

Tests

runTest for both:

  • 5 new TopicFetcherTest cases: delete unreferenced / wipe topic absent from set / no-op when root missing / preserve rc_topic_*.tmp / mixed keep+delete.
  • 5 new RemoteConfigManagerTest cases (with coEvery { 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

Checklist

  • Unit tests
  • Follow-up issues for 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 when blob_refs rotate.

RemoteConfigManager builds a per-topic keep-set from all manifest entryIds’ blob_refs and calls TopicFetcher.cleanupUnreferencedTopics() only when refresh completes with no errors (and caching conditions are met). TopicFetcher implements the disk walk/deletion, skips in-flight temp files via a shared rc_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.

@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from cce8354 to b8f5223 Compare May 6, 2026 13:33
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-manager-and-topic-fetcher branch from 1a10e10 to 9d39923 Compare May 6, 2026 13:33
@codecov

codecov Bot commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.62%. Comparing base (b83a198) to head (e1dac4b).

Files with missing lines Patch % Lines
...ecat/purchases/common/remoteconfig/TopicFetcher.kt 70.00% 2 Missing and 4 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from b8f5223 to 9cab8bc Compare May 7, 2026 12:29
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-manager-and-topic-fetcher branch from 9d39923 to 07ee1a7 Compare May 7, 2026 12:29
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-manager-and-topic-fetcher branch from 07ee1a7 to 3a747f6 Compare May 7, 2026 15:40
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from 9cab8bc to 191c8ba Compare May 7, 2026 15:40
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-manager-and-topic-fetcher branch from 3a747f6 to 85a2bc8 Compare May 8, 2026 08:36
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from 191c8ba to 4e3d34d Compare May 8, 2026 08:36
@tonidero tonidero marked this pull request as ready for review May 8, 2026 09:13
@tonidero tonidero requested a review from a team as a May 8, 2026 09:13
@tonidero tonidero changed the base branch from toniricodiez/add-remote-config-manager-and-topic-fetcher to graphite-base/3439 May 8, 2026 10:20
@tonidero tonidero force-pushed the graphite-base/3439 branch from 85a2bc8 to e2f648d Compare May 8, 2026 10:37
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from 4e3d34d to 61a48d6 Compare May 8, 2026 10:37
@tonidero tonidero changed the base branch from graphite-base/3439 to toniricodiez/cache-remote-config-response May 8, 2026 10:37
@tonidero tonidero force-pushed the toniricodiez/cache-remote-config-response branch from e2f648d to 88436cf Compare May 8, 2026 11:27
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from 61a48d6 to 8c72036 Compare May 8, 2026 11:27
@emerge-tools

emerge-tools Bot commented May 8, 2026

Copy link
Copy Markdown

📸 Snapshot Test

591 unchanged

Name Added Removed Modified Renamed Unchanged Errored Approval
TestPurchasesUIAndroidCompatibility
com.revenuecat.testpurchasesuiandroidcompatibility
0 0 0 0 334 0 N/A
TestPurchasesUIAndroidCompatibility Paparazzi
com.revenuecat.testpurchasesuiandroidcompatibility.paparazzi
0 0 0 0 257 0 N/A

🛸 Powered by Emerge Tools

@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from 8c72036 to b3336d9 Compare May 8, 2026 12:09
@tonidero tonidero force-pushed the toniricodiez/cache-remote-config-response branch from 88436cf to 92529c2 Compare May 8, 2026 12:09
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch from b3336d9 to bc65ad3 Compare May 8, 2026 13:48
@tonidero tonidero force-pushed the toniricodiez/cache-remote-config-response branch from 92529c2 to 6052810 Compare May 8, 2026 13:48
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch 2 times, most recently from 3985e09 to 74eedab Compare May 12, 2026 09:47
@tonidero tonidero force-pushed the toniricodiez/cache-remote-config-response branch from 9a1c9ca to 8f91f3d Compare May 12, 2026 09:47
Base automatically changed from toniricodiez/cache-remote-config-response to main May 12, 2026 11:28
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tonidero tonidero force-pushed the toniricodiez/add-remote-config-topic-cleanup branch 2 times, most recently from 74eedab to e1dac4b Compare May 12, 2026 11:56

@ajpallares ajpallares left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I do have one concern that we may want to address before merging

@ajpallares ajpallares left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ->

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like since Kotlin 1.9, entries is the recommended replacement for values()

Suggested change
Topic.values().forEach topics@{ topic ->
Topic.entries.forEach topics@{ topic ->

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

private fun deleteUnreferencedTopicFiles(referenced: Map<Topic, Set<String>>) {
val topicsRoot = File(applicationContext.noBackupFilesDir, TOPICS_ROOT)
if (!topicsRoot.exists()) return
Topic.values().forEach topics@{ topic ->

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

tonidero commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • May 12, 2:06 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 12, 2:07 PM UTC: Graphite couldn't merge this PR because it failed for an unknown reason (This repository has GitHub's merge queue enabled. Please configure the GitHub merge queue integration in Graphite settings.).

@tonidero tonidero added this pull request to the merge queue May 12, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 12, 2026
@tonidero tonidero added this pull request to the merge queue May 12, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 12, 2026
@tonidero tonidero added this pull request to the merge queue May 12, 2026
Merged via the queue into main with commit e9b0f74 May 12, 2026
45 of 46 checks passed
@tonidero tonidero deleted the toniricodiez/add-remote-config-topic-cleanup branch May 12, 2026 16:12
github-merge-queue Bot pushed a commit that referenced this pull request May 13, 2026
**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 -->
matteinn pushed a commit to matteinn/purchases-android that referenced this pull request Jun 5, 2026
### 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>
matteinn pushed a commit to matteinn/purchases-android that referenced this pull request Jun 5, 2026
### 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants