AB#292770 fix: give configuration networking a cache large enough to hold the tenant config - #147
Open
eligutovsky wants to merge 2 commits into
Open
AB#292770 fix: give configuration networking a cache large enough to hold the tenant config#147eligutovsky wants to merge 2 commits into
eligutovsky wants to merge 2 commits into
Conversation
…enant config The tenant configuration was downloaded in full on every launch instead of revalidating. URLSession's automatic caching refuses to store a response larger than roughly 5% of the cache capacity. URLCache.shared has a 10 MB disk capacity, i.e. a ~500 KB ceiling, and the tenant configuration decodes to more than that. It was therefore never stored, no validator was ever kept, and no conditional request was ever sent, so nothing could return 304. Measured against a local server with ETag and Cache-Control, using URLSession's own caching rather than storeCachedResponse: 10 MB cache, 944 KB response -> not stored, second fetch hits the network 20 MB cache, 944 KB response -> stored, second fetch served from cache 10 MB cache, 109 B response -> stored Configuration networking now gets its own URLCache with a 20 MB disk capacity. Scoped to that one factory, which has a single call site feeding only createRemoteConfigurationNetworking(), and deliberately not URLCache.shared, which belongs to the host app. It is static because the factory returns a new instance per call. The origin was verified to support revalidation correctly, so the fix is enough to produce 304s. GCS varies the ETag by representation, and conditional requests succeed when the client sends the ETag of the encoding it accepts: Accept-Encoding: none, If-None-Match: W/"..." -> 304 Accept-Encoding: gzip, If-None-Match: "..." -> 304 mismatched pairs -> 200 URLSession accepts gzip and would cache the gzip representation's strong ETag, so it lands in the working case.
There was a problem hiding this comment.
Pull request overview
This PR addresses repeated full downloads of the tenant configuration by ensuring URLSession’s automatic caching can actually store the (large) configuration response, enabling proper cache reuse/revalidation across launches.
Changes:
- Introduces a dedicated
URLCachefor configuration networking with increased disk capacity, and wires it into theURLSessionConfigurationused byNetworkClientImplinServiceLocator.networkingFactory(). - Adds unit tests to assert the configuration cache is large enough and is not
URLCache.shared.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| OptimoveSDK/Tests/Sources/Network/ConfigurationURLCacheTests.swift | Adds tests validating cache sizing and isolation from the host app’s shared cache. |
| OptimoveSDK/Sources/Classes/Services/ServiceLocator.swift | Creates and injects a dedicated configuration URLCache into the URL session used for remote configuration downloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+126
to
+130
| /// URLSession will not store a response larger than roughly 5% of its cache | ||
| /// capacity. `URLCache.shared` has a 10 MB disk capacity, i.e. a ~500 KB ceiling, | ||
| /// and the tenant configuration decodes to more than that. It was therefore never | ||
| /// cached, so no validator was ever kept, no conditional request was ever sent, | ||
| /// and the whole file was downloaded again on every launch. A dedicated cache with |
6.8.1 is already claimed by the pending stopDispatchTimer fix (PR #144), so this takes the next patch number rather than colliding with it in the changelog.
This was referenced Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Changes
The tenant configuration is downloaded in full on every launch instead of revalidating.
Roughly 500 KB–1 MB decoded, every time.
Root cause: URLSession's automatic caching refuses to store a response larger than
roughly 5% of the cache capacity.
URLCache.sharedhas a 10 MB disk capacity — a ~500 KBceiling — and the tenant configuration exceeds it. So it was never stored, no validator was
ever kept, and no conditional request was ever sent. Nothing could return 304, because
nothing ever asked.
Measured against a local server serving
ETag+Cache-Control: max-age=300, drivingURLSession's own caching rather than
storeCachedResponse:URLCache.shared)944 KB is 9.4% of 10 MB and 4.5% of 20 MB, consistent with the ~5% rule.
Fix: configuration networking gets its own
URLCachewith a 20 MB disk capacity, set onthe
URLSessionConfigurationpassed toNetworkClientImpl. Scoped toServiceLocator.networkingFactory(), which has a single call site feeding onlycreateRemoteConfigurationNetworking(), so no other networking changes behaviour.Deliberately not
URLCache.shared— that belongs to the host app.static, because thefactory returns a new instance per call and the cache has to outlive them.
The origin does support revalidation
Worth recording, since it was the main open question. GCS varies the ETag by representation,
and conditional requests succeed when the client sends the ETag for the encoding it accepts:
Accept-EncodingIf-None-MatchW/"fb65…"(weak, as served)"fb65…"(strong)W/"fb65…"(weak)"fb65…"(strong, as served)URLSession sends
Accept-Encoding: gzipand would cache the gzip representation togetherwith its strong ETag, which is the working row. So raising the capacity is sufficient — no
SDK-side ETag handling is needed.
(Measured on the global config,
sdk-cdn.optimove.net, which is publicly reachable. Thetenant config needs a token, so it could not be measured directly, but both are served by
the same bucket and the handoff's captured tenant headers match this shape.)
Verification
Full suite: 77 tests, 0 failures (75 before, plus the two added here).
The new tests were mutation-checked — reverting the factory to
URLCache.sharedfails both:Two things deliberately not changed
No explicit 304 handling in
NetworkClientImpl. A bare 304 does reach the completionhandler — confirmed with a server returning 304 unconditionally:
status=304 bodyBytes=0 error=nil, which falls to thedefault:case and becomes a success with an empty body, soJSONDecoderthrows. But it only arises if an origin answers 304 to a non-conditionalrequest; when there is a cache entry, URLSession replays the cached bytes and the client sees
200. And the consequence is already benign:
TenantConfigurationDownloaderlogs the errorand returns without calling
saveTenant, so the previously stored configuration is retained— which is what "not modified" should mean. A 304 also implies a prior successful fetch, so
there is always a stored config to fall back on. Adding handling would change the log line,
not the behaviour, so it is left out of this PR.
ServiceLocator.networking()andnetworkClient()untouched. They build their ownNetworkClientImplfor other purposes and keep the default shared cache.Breaking Changes
The SDK now keeps up to 20 MB of configuration cache on disk, in its own
com.optimove.configuration-cachedirectory rather than in the host app's shared cache.Release Checklist
Prepare:
pod lib lintpassesBump versions in:
Bumped to 6.8.2. 6.8.1 is already claimed by the pending
stopDispatchTimerfix(#144), so this takes the next patch number rather than colliding with it in the changelog.
If this merges before #144, 6.8.1 is simply skipped.
OptimoveCore.podspecOptimoveNotificationServiceExtension.podspecOptimoveSDK.podspecOptimoveCore/Sources/Classes/Constants/SDKVersion.swiftREADME.md— n/a, contains no version referenceCHANGELOG.mdIntegration tests
Unit tests pass (77, 0 failures). The end-to-end check worth doing on a real tenant: capture
traffic across two launches within the 5 minute
max-ageand confirm the tenant config isnot refetched, then again after it expires and confirm a 304 rather than a 200.
T&T Only
Mobile Only
Release: