From dde79eeea2d8ae46cd72fd2bf380e1aecef70074 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:53:19 +0700 Subject: [PATCH] fix(home): protect fresh feeds from late cache restoration --- lib/providers/explore_provider.dart | 48 +++++-- test/explore_featured_section_test.dart | 163 ++++++++++++++++++++++++ 2 files changed, 202 insertions(+), 9 deletions(-) diff --git a/lib/providers/explore_provider.dart b/lib/providers/explore_provider.dart index 87be22fe..c3f959a1 100644 --- a/lib/providers/explore_provider.dart +++ b/lib/providers/explore_provider.dart @@ -274,9 +274,25 @@ List _buildExploreSectionsFromNormalizedPayload( } class ExploreNotifier extends Notifier { + ExploreNotifier({Future> Function(String)? decodeCache}) + : _decodeCache = + decodeCache ?? ((raw) => compute(_decodeExploreCache, raw)); + + final Future> Function(String) _decodeCache; static const _cacheKey = 'explore_home_feed_cache'; static const _cacheTsKey = 'explore_home_feed_ts'; int _homeFeedRequestId = 0; + int? _activeHomeFeedRequestId; + int _cacheRestoreGeneration = 0; + + bool _isRequestValid(int requestId) => + ref.mounted && requestId == _homeFeedRequestId; + + bool _isCacheRestoreValid(int generation) => + ref.mounted && + generation == _cacheRestoreGeneration && + ref.read(settingsProvider).homeFeedProvider != + AppSettings.homeFeedProviderOff; @override ExploreState build() { @@ -285,6 +301,7 @@ class ExploreNotifier extends Notifier { } Future _restoreFromCache() async { + final generation = _cacheRestoreGeneration; try { if (ref.read(settingsProvider).homeFeedProvider == AppSettings.homeFeedProviderOff) { @@ -293,11 +310,13 @@ class ExploreNotifier extends Notifier { } final prefs = await SharedPreferences.getInstance(); + if (!_isCacheRestoreValid(generation)) return; final cached = prefs.getString(_cacheKey); final cachedTs = prefs.getInt(_cacheTsKey); if (cached == null || cached.isEmpty) return; - final cachePayload = await compute(_decodeExploreCache, cached); + final cachePayload = await _decodeCache(cached); + if (!_isCacheRestoreValid(generation)) return; final providerId = cachePayload['provider_id']?.toString().trim(); final rawSections = cachePayload['sections']; var normalizedSections = rawSections is List @@ -326,16 +345,21 @@ class ExploreNotifier extends Notifier { : null; _log.i('Restored ${sections.length} cached explore sections'); + // Cached content can replace the initial spinner while the separate + // active request still owns refresh completion and deduplication. state = ExploreState( + error: state.error, greeting: _getLocalGreeting(), providerId: resolvedProviderId, sections: sections, lastFetched: lastFetched, ); } catch (e) { + if (!_isCacheRestoreValid(generation)) return; _log.w('Failed to restore explore cache: $e'); try { final prefs = await SharedPreferences.getInstance(); + if (!_isCacheRestoreValid(generation)) return; await prefs.remove(_cacheKey); await prefs.remove(_cacheTsKey); _log.d('Removed invalid explore cache'); @@ -383,9 +407,7 @@ class ExploreNotifier extends Notifier { Future fetchHomeFeed({bool forceRefresh = false}) async { if (ref.read(settingsProvider).homeFeedProvider == AppSettings.homeFeedProviderOff) { - _homeFeedRequestId++; - PlatformBridge.cancelExtensionHomeFeedRequests(); - state = const ExploreState(); + clear(); return; } @@ -397,12 +419,13 @@ class ExploreNotifier extends Notifier { return; } - if (state.isLoading && !forceRefresh) { + if (_activeHomeFeedRequestId != null && !forceRefresh) { _log.d('Home feed fetch already in progress'); return; } final requestId = ++_homeFeedRequestId; + _activeHomeFeedRequestId = requestId; final showLoading = !state.hasContent; state = state.copyWith(isLoading: showLoading, error: null); @@ -411,7 +434,7 @@ class ExploreNotifier extends Notifier { if (targetExt == null) { _log.w('No extension with homeFeed capability found'); - if (requestId != _homeFeedRequestId) return; + if (!_isRequestValid(requestId)) return; state = state.copyWith( isLoading: false, error: 'No extension with home feed support enabled', @@ -423,7 +446,7 @@ class ExploreNotifier extends Notifier { targetExt.id, cancelPrevious: forceRefresh, ); - if (requestId != _homeFeedRequestId) return; + if (!_isRequestValid(requestId)) return; if (result == null) { state = state.copyWith( @@ -449,7 +472,7 @@ class ExploreNotifier extends Notifier { normalizedSectionsWithoutProvider, targetExt.id, ); - if (requestId != _homeFeedRequestId) return; + if (!_isRequestValid(requestId)) return; final sections = _buildExploreSectionsFromNormalizedPayload( normalizedSections, ); @@ -460,6 +483,7 @@ class ExploreNotifier extends Notifier { 'sections=${sections.length}', ); + _cacheRestoreGeneration++; state = ExploreState( isLoading: false, greeting: localGreeting, @@ -471,13 +495,19 @@ class ExploreNotifier extends Notifier { _saveToCache(normalizedSections, targetExt.id); } catch (e, stack) { _log.e('Error fetching home feed: $e', e, stack); - if (requestId != _homeFeedRequestId) return; + if (!_isRequestValid(requestId)) return; state = state.copyWith(isLoading: false, error: e.toString()); + } finally { + if (_activeHomeFeedRequestId == requestId) { + _activeHomeFeedRequestId = null; + } } } void clear() { _homeFeedRequestId++; + _cacheRestoreGeneration++; + _activeHomeFeedRequestId = null; PlatformBridge.cancelExtensionHomeFeedRequests(); state = const ExploreState(); } diff --git a/test/explore_featured_section_test.dart b/test/explore_featured_section_test.dart index bd39ed05..b3382008 100644 --- a/test/explore_featured_section_test.dart +++ b/test/explore_featured_section_test.dart @@ -1,11 +1,138 @@ +import 'dart:async'; +import 'dart:convert'; + import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:spotiflac_android/models/settings.dart'; +import 'package:spotiflac_android/providers/extension_provider.dart'; import 'package:spotiflac_android/providers/explore_provider.dart'; +import 'package:spotiflac_android/providers/settings_provider.dart'; import 'package:spotiflac_android/widgets/explore_featured_section.dart'; import 'package:spotiflac_android/widgets/audio_quality_badges.dart'; import 'package:spotiflac_android/l10n/app_localizations.dart'; void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('home feed cache ownership', () { + const channel = MethodChannel('com.zarz.spotiflac/backend'); + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + tearDown(() => messenger.setMockMethodCallHandler(channel, null)); + + Future< + ({ + ProviderContainer container, + ExploreNotifier notifier, + Completer> cache, + }) + > + pendingRestore() async { + SharedPreferences.setMockInitialValues({ + 'explore_home_feed_cache': jsonEncode(_feed('Cached')), + 'explore_home_feed_ts': DateTime.now() + .subtract(const Duration(hours: 1)) + .millisecondsSinceEpoch, + }); + final cache = Completer>(); + final started = Completer(); + final container = ProviderContainer( + overrides: [ + settingsProvider.overrideWith(_FeedSettings.new), + extensionProvider.overrideWith(_FeedExtensions.new), + exploreProvider.overrideWith( + () => ExploreNotifier( + decodeCache: (_) { + started.complete(); + return cache.future; + }, + ), + ), + ], + ); + addTearDown(container.dispose); + final notifier = container.read(exploreProvider.notifier); + await started.future; + return (container: container, notifier: notifier, cache: cache); + } + + for (final disabled in [false, true]) { + test('late cache cannot undo ${disabled ? 'Off' : 'Clear'}', () async { + final restore = await pendingRestore(); + if (disabled) { + (restore.container.read(settingsProvider.notifier) as _FeedSettings) + .disableHomeFeed(); + await restore.notifier.fetchHomeFeed(); + } else { + restore.notifier.clear(); + } + restore.cache.complete(_feed('Cached')); + await Future.delayed(Duration.zero); + final state = restore.container.read(exploreProvider); + expect(state.hasContent, isFalse); + expect(state.isLoading, isFalse); + }); + } + + test('completed refresh wins over late cache', () async { + messenger.setMockMethodCallHandler(channel, (call) async { + expect(call.method, 'getExtensionHomeFeed'); + return jsonEncode({ + 'success': true, + 'sections': _feed('Fresh')['sections'], + }); + }); + final restore = await pendingRestore(); + await restore.notifier.refresh(); + expect( + restore.container.read(exploreProvider).sections.single.title, + 'Fresh', + ); + restore.cache.complete(_feed('Cached')); + await Future.delayed(Duration.zero); + expect( + restore.container.read(exploreProvider).sections.single.title, + 'Fresh', + ); + }); + + test( + 'stale cache stays visible without duplicating the active refresh', + () async { + final response = Completer(); + final started = Completer(); + var calls = 0; + messenger.setMockMethodCallHandler(channel, (call) async { + expect(call.method, 'getExtensionHomeFeed'); + calls++; + if (!started.isCompleted) started.complete(); + return response.future; + }); + final restore = await pendingRestore(); + final refresh = restore.notifier.refresh(); + await started.future; + restore.cache.complete(_feed('Cached')); + await Future.delayed(Duration.zero); + final cached = restore.container.read(exploreProvider); + expect(cached.sections.single.title, 'Cached'); + expect(cached.isLoading, isFalse); + await restore.notifier.fetchHomeFeed(); + expect(calls, 1); + response.complete( + jsonEncode({'success': true, 'sections': _feed('Fresh')['sections']}), + ); + await refresh; + expect( + restore.container.read(exploreProvider).sections.single.title, + 'Fresh', + ); + }, + ); + }); + test('featured layout and artwork survive cache serialization', () { final section = ExploreSection.fromJson({ 'uri': 'featured', @@ -109,3 +236,39 @@ void main() { }); } } + +Map _feed(String title) => { + 'provider_id': 'cache-race-provider', + 'sections': >[ + {'uri': 'example:feed', 'title': title, 'items': >[]}, + ], +}; + +class _FeedSettings extends SettingsNotifier { + @override + AppSettings build() => + const AppSettings(homeFeedProvider: 'cache-race-provider'); + + void disableHomeFeed() { + state = state.copyWith(homeFeedProvider: AppSettings.homeFeedProviderOff); + } +} + +class _FeedExtensions extends ExtensionNotifier { + @override + ExtensionState build() => const ExtensionState( + isInitialized: true, + extensions: [ + Extension( + id: 'cache-race-provider', + name: 'cache-race-provider', + displayName: 'Cache race provider', + version: '1.0.0', + description: '', + enabled: true, + status: 'loaded', + capabilities: {'homeFeed': true}, + ), + ], + ); +}