From 6c84d33b255e065efb78d732b825f2551cfe3157 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 15 Jun 2026 11:04:54 +0000 Subject: [PATCH] fix(cdn): defer lastUpdatedAt commit until manifest fetch succeeds When lastUpdateTime.json reports a newer timestamp but the manifest download fails, advancing _lastUpdatedAt before a successful fetch caused subsequent refresh checks to skip the download for the rest of the session. Evaluate freshness without mutating _lastUpdatedAt and commit the remote timestamp only after the manifest is parsed and cached. Co-authored-by: Sharjeel Yunus --- .../definition_providers/cdn_provider.dart | 76 ++++++++++++------- modules/ensemble/test/cdn_provider_test.dart | 71 +++++++++++++++++ 2 files changed, 118 insertions(+), 29 deletions(-) diff --git a/modules/ensemble/lib/framework/definition_providers/cdn_provider.dart b/modules/ensemble/lib/framework/definition_providers/cdn_provider.dart index c55536fb7..7de0ccd80 100644 --- a/modules/ensemble/lib/framework/definition_providers/cdn_provider.dart +++ b/modules/ensemble/lib/framework/definition_providers/cdn_provider.dart @@ -531,8 +531,8 @@ class CdnDefinitionProvider extends DefinitionProvider { Future _doRefreshIfStale() async { try { - final shouldFetch = await _shouldFetchManifest(); - if (!shouldFetch) { + final freshness = await _evaluateManifestFreshness(); + if (!freshness.shouldFetch) { return; } @@ -548,14 +548,14 @@ class CdnDefinitionProvider extends DefinitionProvider { _rebuildFromRoot(root); await _refreshTranslationsAtRuntime(); final savedEtag = newEtag ?? _etag; - final savedLastUpdatedAt = _lastUpdatedAt; _etag = savedEtag; + _commitRemoteLastUpdatedAt(freshness.remoteLastUpdatedAt); // Save to persistent cache (snapshot etag/timestamp with manifest body) await _saveCachedState( jsonString, etag: savedEtag, - lastUpdatedAt: savedLastUpdatedAt, + lastUpdatedAt: _lastUpdatedAt, ); await _applyStaleRefreshOutcome(); @@ -584,8 +584,8 @@ class CdnDefinitionProvider extends DefinitionProvider { } Future _loadManifest() async { - final shouldFetch = await _shouldFetchManifest(); - if (!shouldFetch) { + final freshness = await _evaluateManifestFreshness(); + if (!freshness.shouldFetch) { return; } @@ -596,57 +596,59 @@ class CdnDefinitionProvider extends DefinitionProvider { if (jsonString == null) return; final savedEtag = fetched['etag'] as String?; - final savedLastUpdatedAt = _lastUpdatedAt; _etag = savedEtag; final root = _decodeManifestRoot(jsonString); _rebuildFromRoot(root); + _commitRemoteLastUpdatedAt(freshness.remoteLastUpdatedAt); // Save to persistent cache (snapshot etag/timestamp with manifest body) await _saveCachedState( jsonString, etag: savedEtag, - lastUpdatedAt: savedLastUpdatedAt, + lastUpdatedAt: _lastUpdatedAt, ); } - Future _shouldFetchManifest() async { + /// Reads CDN lastUpdateTime and decides whether to download a new manifest. + /// Does not mutate [_lastUpdatedAt]; commit only after a successful fetch. + Future<({bool shouldFetch, int? remoteLastUpdatedAt})> + _evaluateManifestFreshness() async { final lastUpdateUri = Uri.parse('$baseUrl/$appId/lastUpdateTime.json'); try { final headers = await _cdnRequestHeaders(); final resp = await http.get(lastUpdateUri, headers: headers); if (resp.statusCode != 200) { - return true; + return (shouldFetch: true, remoteLastUpdatedAt: null); } final jsonString = _decodePossiblyBrotli(resp); if (jsonString == null || jsonString.isEmpty) { - return true; + return (shouldFetch: true, remoteLastUpdatedAt: null); } final lastUpdateData = jsonDecode(jsonString) as Map; final num? remoteLastUpdateNum = lastUpdateData['lastUpdatedAt'] as num?; final int? remoteLastUpdate = remoteLastUpdateNum?.toInt(); - if (remoteLastUpdate == null) { - return true; - } - - if (_lastUpdatedAt == null) { - _lastUpdatedAt = remoteLastUpdate; - return true; - } - - final shouldFetch = _isIncomingNewer(remoteLastUpdate, _lastUpdatedAt); - - _lastUpdatedAt = remoteLastUpdate; - - return shouldFetch; + return ( + shouldFetch: cdnShouldFetchManifest( + localLastUpdatedAt: _lastUpdatedAt, + remoteLastUpdatedAt: remoteLastUpdate, + ), + remoteLastUpdatedAt: remoteLastUpdate, + ); } catch (e) { debugPrint( 'CdnProvider: Error checking lastUpdateTime: $e, will fetch manifest'); - return true; + return (shouldFetch: true, remoteLastUpdatedAt: null); + } + } + + void _commitRemoteLastUpdatedAt(int? remoteLastUpdatedAt) { + if (remoteLastUpdatedAt != null) { + _lastUpdatedAt = remoteLastUpdatedAt; } } @@ -995,9 +997,6 @@ class CdnDefinitionProvider extends DefinitionProvider { return 'Ensemble/$version ($suffix)'; } - static bool _isIncomingNewer(int? incoming, int? current) => - incoming != null && (current == null || incoming > current); - /// SharedPreferences tuple for CDN cache; etag and timestamp must match /// [manifestJson] or cold-start If-None-Match can serve a mismatched body. @visibleForTesting @@ -1112,6 +1111,10 @@ class CdnDefinitionProvider extends DefinitionProvider { @visibleForTesting Future refreshIfStaleForTesting() => _refreshIfStale(); + @visibleForTesting + void commitRemoteLastUpdatedAtForTesting(int? remoteLastUpdatedAt) => + _commitRemoteLastUpdatedAt(remoteLastUpdatedAt); + Future _refreshTranslationsAtRuntime() async { try { final context = Utils.globalAppKey.currentContext; @@ -1131,3 +1134,18 @@ class CdnDefinitionProvider extends DefinitionProvider { } } } + +/// Whether the CDN manifest should be fetched based on lastUpdateTime values. +@visibleForTesting +bool cdnShouldFetchManifest({ + required int? localLastUpdatedAt, + required int? remoteLastUpdatedAt, +}) { + if (remoteLastUpdatedAt == null) return true; + if (localLastUpdatedAt == null) return true; + return cdnIsIncomingManifestNewer(remoteLastUpdatedAt, localLastUpdatedAt); +} + +@visibleForTesting +bool cdnIsIncomingManifestNewer(int? incoming, int? current) => + incoming != null && (current == null || incoming > current); diff --git a/modules/ensemble/test/cdn_provider_test.dart b/modules/ensemble/test/cdn_provider_test.dart index 3b855f166..570c5193e 100644 --- a/modules/ensemble/test/cdn_provider_test.dart +++ b/modules/ensemble/test/cdn_provider_test.dart @@ -11,6 +11,77 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:shared_preferences/shared_preferences.dart'; void main() { + group('cdnShouldFetchManifest', () { + test('fetches when remote timestamp is unknown or local is unset', () { + expect( + cdnShouldFetchManifest( + localLastUpdatedAt: 100, + remoteLastUpdatedAt: null, + ), + isTrue, + ); + expect( + cdnShouldFetchManifest( + localLastUpdatedAt: null, + remoteLastUpdatedAt: 200, + ), + isTrue, + ); + }); + + test('fetches only when remote timestamp is newer', () { + expect( + cdnShouldFetchManifest( + localLastUpdatedAt: 100, + remoteLastUpdatedAt: 200, + ), + isTrue, + ); + expect( + cdnShouldFetchManifest( + localLastUpdatedAt: 200, + remoteLastUpdatedAt: 200, + ), + isFalse, + ); + expect( + cdnShouldFetchManifest( + localLastUpdatedAt: 300, + remoteLastUpdatedAt: 200, + ), + isFalse, + ); + }); + }); + + group('CDN lastUpdatedAt commit', () { + test('does not advance local timestamp until manifest fetch succeeds', + () async { + final provider = CdnDefinitionProvider('test-app'); + expect(provider.lastUpdatedAtForTesting, isNull); + + expect( + cdnShouldFetchManifest( + localLastUpdatedAt: provider.lastUpdatedAtForTesting, + remoteLastUpdatedAt: 500, + ), + isTrue, + ); + expect(provider.lastUpdatedAtForTesting, isNull); + + provider.commitRemoteLastUpdatedAtForTesting(500); + expect(provider.lastUpdatedAtForTesting, 500); + + expect( + cdnShouldFetchManifest( + localLastUpdatedAt: provider.lastUpdatedAtForTesting, + remoteLastUpdatedAt: 500, + ), + isFalse, + ); + }); + }); + group('CDN User-Agent', () { test('cdnUserAgent formats Ensemble version, platform, and app name', () { expect(