diff --git a/lib/l10n/arb/app_en.arb b/lib/l10n/arb/app_en.arb index 4889b636..66a3c3f9 100644 --- a/lib/l10n/arb/app_en.arb +++ b/lib/l10n/arb/app_en.arb @@ -7064,6 +7064,8 @@ "weeklyReleasesUnavailable": "Some artists could not be checked. Their saved results are kept. Providers may not supply artist catalogs or exact release dates.", "weeklyReleasesLoadError": "Could not check new releases. Pull to refresh.", "motionArtworkDescription": "Loop video covers supplied by metadata providers. Turn off to use still artwork and reduce battery use.", + "libraryLoading": "Loading Library…", + "libraryLoadFailed": "Library could not be loaded. Retry to load the saved folders and track index.", "optionsEmbeddedCoverSizeOriginal": "Original resolution", "@optionsEmbeddedCoverSizeOriginal": { "description": "Option that preserves the provider artwork at its original resolution" diff --git a/lib/l10n/arb/app_id.arb b/lib/l10n/arb/app_id.arb index 8784b69d..a4fc8fc0 100644 --- a/lib/l10n/arb/app_id.arb +++ b/lib/l10n/arb/app_id.arb @@ -6979,5 +6979,7 @@ "weeklyReleasesEmpty": "Tidak ditemukan rilis dengan tanggal dalam periode ini. Tambahkan artis favorit atau coba periode lebih panjang.", "weeklyReleasesUnavailable": "Beberapa artis tidak dapat diperiksa. Hasil tersimpan tetap dipertahankan. Penyedia mungkin tidak memberikan katalog artis atau tanggal rilis lengkap.", "weeklyReleasesLoadError": "Rilis baru tidak dapat diperiksa. Tarik untuk memuat ulang.", - "motionArtworkDescription": "Putar berulang cover video dari penyedia metadata. Matikan untuk memakai cover diam dan menghemat baterai." + "motionArtworkDescription": "Putar berulang cover video dari penyedia metadata. Matikan untuk memakai cover diam dan menghemat baterai.", + "libraryLoading": "Memuat Library…", + "libraryLoadFailed": "Library tidak dapat dimuat. Coba lagi untuk memuat folder tersimpan dan indeks lagu." } diff --git a/lib/providers/local_library_provider.dart b/lib/providers/local_library_provider.dart index a9ec8a38..bdbb58a3 100644 --- a/lib/providers/local_library_provider.dart +++ b/lib/providers/local_library_provider.dart @@ -19,6 +19,8 @@ const _excludedDownloadedCountKey = 'local_library_excluded_downloaded_count'; final _prefs = SharedPreferences.getInstance(); class LocalLibraryState { + final bool isLoading; + final bool loadFailed; final bool isScanning; final bool scanIsFinalizing; final bool scanIsPaused; @@ -38,6 +40,8 @@ class LocalLibraryState { final Set _isrcSet; LocalLibraryState({ + this.isLoading = false, + this.loadFailed = false, this.isScanning = false, this.scanIsFinalizing = false, this.scanIsPaused = false, @@ -76,6 +80,8 @@ class LocalLibraryState { } LocalLibraryState copyWith({ + bool? isLoading, + bool? loadFailed, bool? isScanning, bool? scanIsFinalizing, bool? scanIsPaused, @@ -97,6 +103,8 @@ class LocalLibraryState { Set? isrcSet, }) { return LocalLibraryState( + isLoading: isLoading ?? this.isLoading, + loadFailed: loadFailed ?? this.loadFailed, isScanning: isScanning ?? this.isScanning, scanIsFinalizing: scanIsFinalizing ?? this.scanIsFinalizing, scanIsPaused: scanIsPaused ?? this.scanIsPaused, @@ -124,7 +132,10 @@ class LocalLibraryState { } class LocalLibraryNotifier extends Notifier { - final LibraryDatabase _db = LibraryDatabase.instance; + LocalLibraryNotifier({LibraryDatabase? database}) + : _db = database ?? LibraryDatabase.instance; + + final LibraryDatabase _db; final NotificationService _notificationService = NotificationService(); static const _progressPollingInterval = Duration(milliseconds: 350); static const _progressStreamBootstrapTimeout = Duration(milliseconds: 900); @@ -184,7 +195,7 @@ class LocalLibraryNotifier extends Notifier { } Future.microtask(_ensureLoadedFromDatabase); - return LocalLibraryState(); + return LocalLibraryState(isLoading: true); } Future _ensureLoadedFromDatabase() { @@ -200,17 +211,27 @@ class LocalLibraryNotifier extends Notifier { return _loadFuture ?? Future.value(); } _isLoaded = true; + state = state.copyWith(isLoading: true, loadFailed: false); try { + // Folder configuration is independent of the potentially expensive + // track index and storage probes. Publish it before either can fail. + final configuredSources = await _db.getSources(); + if (!ref.mounted) return; + state = state.copyWith(sources: configuredSources); + await ref.read(settingsProvider.notifier).ensureLoaded(); await _migrateLegacySource(); final reconnectedSources = await _refreshSourceAvailabilityInDatabase(); - final countFuture = _db.getCount(); - final indexFuture = _db.getLookupIndex(); - final sourcesFuture = _db.getSources(); + final sources = await _db.getSources(); + if (!ref.mounted) return; + state = state.copyWith(sources: sources); + final summary = await Future.wait([ + _db.getCount(), + _db.getLookupIndex(), + ]); final prefsFuture = _prefs; - final count = await countFuture; - final lookupIndex = await indexFuture; - final sources = await sourcesFuture; + final count = summary[0] as int; + final lookupIndex = summary[1] as LocalLibraryLookupIndex; DateTime? lastScannedAt; var excludedDownloadedCount = 0; @@ -223,6 +244,7 @@ class LocalLibraryNotifier extends Notifier { _log.w('Failed to load lastScannedAt: $e'); } + if (!ref.mounted) return; state = state.copyWith( totalCount: count, loadedIndexVersion: state.loadedIndexVersion + 1, @@ -246,13 +268,20 @@ class LocalLibraryNotifier extends Notifier { } } catch (e, stack) { _isLoaded = false; + if (ref.mounted) state = state.copyWith(loadFailed: true); _log.e('Failed to load library from database: $e', e, stack); } finally { _loadFuture = null; + if (ref.mounted) state = state.copyWith(isLoading: false); } } Future reloadFromStorage() async { + final loading = _loadFuture; + if (loading != null) { + await loading; + return; + } _isLoaded = false; _hasLoadedFromDatabase = false; _loadFuture = null; @@ -388,12 +417,16 @@ class LocalLibraryNotifier extends Notifier { DateTime? lastScannedAt, int? excludedDownloadedCount, }) async { - final countFuture = _db.getCount(); - final indexFuture = _db.getLookupIndex(); - final sourcesFuture = _db.getSources(); - final count = await countFuture; - final index = await indexFuture; - final sources = await sourcesFuture; + final sources = await _db.getSources(); + if (!ref.mounted) return; + state = state.copyWith(sources: sources); + final summary = await Future.wait([ + _db.getCount(), + _db.getLookupIndex(), + ]); + final count = summary[0] as int; + final index = summary[1] as LocalLibraryLookupIndex; + if (!ref.mounted) return; final latestSourceScan = sources .map((source) => source.lastScannedAt) .whereType() @@ -403,6 +436,7 @@ class LocalLibraryNotifier extends Notifier { latest == null || value.isAfter(latest) ? value : latest, ); state = state.copyWith( + loadFailed: false, totalCount: count, loadedIndexVersion: state.loadedIndexVersion + 1, lastScannedAt: lastScannedAt ?? latestSourceScan, @@ -478,19 +512,25 @@ class LocalLibraryNotifier extends Notifier { } Future _probePathAvailability(String path, {String? bookmark}) async { - if (Platform.isAndroid && path.startsWith('content://')) { - return PlatformBridge.probeSafTreeReadAccess(path); - } - if (Platform.isIOS && bookmark != null && bookmark.trim().isNotEmpty) { - final access = await PlatformBridge.startAccessingIosBookmark(bookmark); - if (access == null) return false; - try { - return await Directory(access.path).exists(); - } finally { - await PlatformBridge.stopAccessingIosBookmark(access); + try { + if (Platform.isAndroid && path.startsWith('content://')) { + return await PlatformBridge.probeSafTreeReadAccess(path); } + if (Platform.isIOS && bookmark != null && bookmark.trim().isNotEmpty) { + final access = await PlatformBridge.startAccessingIosBookmark(bookmark); + if (access == null) return false; + try { + return await Directory(access.path).exists(); + } finally { + await PlatformBridge.stopAccessingIosBookmark(access); + } + } + return await Directory(path).exists(); + } catch (error) { + // A busy provider or a filesystem exception cannot prove detachment. + _log.w('Library source access probe failed: $error'); + return null; } - return await Directory(path).exists(); } Future> _refreshSourceAvailabilityInDatabase() => diff --git a/lib/screens/settings/library_settings_page.dart b/lib/screens/settings/library_settings_page.dart index 265b55c2..d177e922 100644 --- a/lib/screens/settings/library_settings_page.dart +++ b/lib/screens/settings/library_settings_page.dart @@ -636,6 +636,43 @@ class _LibrarySettingsPageState extends ConsumerState { title: context.l10n.libraryScanSettings, ), ), + SliverToBoxAdapter( + child: Consumer( + builder: (context, ref, _) { + final status = ref.watch( + localLibraryProvider.select( + (state) => + (loading: state.isLoading, failed: state.loadFailed), + ), + ); + if (!status.loading && !status.failed) { + return const SizedBox.shrink(); + } + return ListTile( + leading: status.loading + ? const SizedBox( + width: 24, + height: 24, + child: CircularProgressIndicator(strokeWidth: 2), + ) + : Icon(Icons.error_outline, color: colorScheme.error), + title: Text( + status.loading + ? context.l10n.libraryLoading + : context.l10n.libraryLoadFailed, + ), + trailing: status.loading + ? null + : TextButton( + onPressed: () => ref + .read(localLibraryProvider.notifier) + .reloadFromStorage(), + child: Text(context.l10n.dialogRetry), + ), + ); + }, + ), + ), SliverToBoxAdapter( child: SettingsGroup( children: [ diff --git a/lib/services/library_database.dart b/lib/services/library_database.dart index bce5721f..1393587e 100644 --- a/lib/services/library_database.dart +++ b/lib/services/library_database.dart @@ -1282,20 +1282,19 @@ class LibraryDatabase { }) async { if (path.trim().isEmpty) return; final db = await database; - await db.update( - 'library_sources', - { - 'path': path, - 'display_name': displayName, - 'bookmark': bookmark, - 'volume_id': volumeId, - 'is_removable': isRemovable ? 1 : 0, - 'available': available ? 1 : 0, - 'last_scanned_at': lastScannedAt?.toIso8601String(), - 'last_seen_at': available ? DateTime.now().toIso8601String() : null, - }, - where: 'id = ?', - whereArgs: [legacySourceId], + await persistLegacyLibrarySource( + db, + LocalLibrarySource( + id: legacySourceId, + path: path, + displayName: displayName, + bookmark: bookmark, + volumeId: volumeId, + isRemovable: isRemovable, + available: available, + lastScannedAt: lastScannedAt, + lastSeenAt: available ? DateTime.now() : null, + ), ); } @@ -2277,3 +2276,36 @@ class LibraryDatabase { return hash; } } + +/// Migrates the pre-multiple-folder setting even when its placeholder row is +/// absent. Do not REPLACE an existing source or change the IDs of its tracks. +Future persistLegacyLibrarySource( + Database db, + LocalLibrarySource source, +) async { + final values = { + 'path': source.path, + 'display_name': source.displayName, + 'bookmark': source.bookmark, + 'volume_id': source.volumeId, + 'is_removable': source.isRemovable ? 1 : 0, + 'available': source.available ? 1 : 0, + 'last_scanned_at': source.lastScannedAt?.toIso8601String(), + 'last_seen_at': source.lastSeenAt?.toIso8601String(), + }; + await db.transaction((txn) async { + final updated = await txn.update( + 'library_sources', + values, + where: 'id = ?', + whereArgs: [source.id], + ); + if (updated == 0) { + await txn.insert('library_sources', { + 'id': source.id, + 'enabled': source.enabled ? 1 : 0, + ...values, + }); + } + }); +} diff --git a/test/library_loading_test.dart b/test/library_loading_test.dart new file mode 100644 index 00000000..67e526f6 --- /dev/null +++ b/test/library_loading_test.dart @@ -0,0 +1,243 @@ +import 'dart:async'; +import 'dart:io'; + +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/local_library_provider.dart'; +import 'package:spotiflac_android/providers/settings_provider.dart'; +import 'package:spotiflac_android/services/library_database.dart'; + +class _Settings extends SettingsNotifier { + _Settings({Future? ready, String path = ''}) + : _ready = ready, + _path = path; + + final Future? _ready; + final String _path; + + @override + AppSettings build() => const AppSettings(); + + @override + Future ensureLoaded() async { + await _ready; + state = state.copyWith(localLibraryPath: _path); + } +} + +class _LibraryDatabase implements LibraryDatabase { + _LibraryDatabase(this.sources); + + List sources; + Future Function() count = () async => 2; + Future Function() index = () async => + const LocalLibraryLookupIndex(matchKeys: {'song|artist'}); + bool failSources = false; + + @override + Future> getSources() async { + if (failSources) throw StateError('database is busy'); + return List.of(sources); + } + + @override + Future getCount() => count(); + + @override + Future getLookupIndex() => index(); + + @override + Future getSourceByPath(String path) async => + sources.where((source) => source.path == path).firstOrNull; + + @override + Future migrateLegacySource({ + required String path, + required String displayName, + String? bookmark, + String? volumeId, + bool isRemovable = false, + bool available = true, + DateTime? lastScannedAt, + }) async { + sources = [ + ...sources, + LocalLibrarySource( + id: LibraryDatabase.legacySourceId, + path: path, + displayName: displayName, + available: available, + ), + ]; + } + + @override + Future updateSourceState( + String sourceId, { + bool? enabled, + bool? available, + DateTime? lastScannedAt, + DateTime? lastSeenAt, + String? lastScanError, + bool clearLastScanError = false, + }) async { + sources = [ + for (final source in sources) + source.id == sourceId + ? source.copyWith(enabled: enabled, available: available) + : source, + ]; + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +Future _waitFor(bool Function() condition) async { + for (var i = 0; i < 50 && !condition(); i++) { + await Future.delayed(Duration.zero); + } + expect(condition(), isTrue); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + late Directory music; + late _LibraryDatabase database; + late ProviderContainer container; + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + music = await Directory.systemTemp.createTemp('library_loading_'); + database = _LibraryDatabase([ + LocalLibrarySource( + id: 'music', + path: music.path, + displayName: 'Music', + trackCount: 2, + ), + ]); + container = ProviderContainer( + overrides: [ + settingsProvider.overrideWith(_Settings.new), + localLibraryProvider.overrideWith( + () => LocalLibraryNotifier(database: database), + ), + ], + ); + }); + + tearDown(() async { + container.dispose(); + await music.delete(recursive: true); + }); + + test( + 'upgrade waits for saved settings before migrating the old folder', + () async { + final ready = Completer(); + container.dispose(); + database.sources = []; + container = ProviderContainer( + overrides: [ + settingsProvider.overrideWith( + () => _Settings(ready: ready.future, path: music.path), + ), + localLibraryProvider.overrideWith( + () => LocalLibraryNotifier(database: database), + ), + ], + ); + container.read(localLibraryProvider); + await Future.delayed(Duration.zero); + expect(container.read(localLibraryProvider).isLoading, isTrue); + ready.complete(); + await _waitFor(() => !container.read(localLibraryProvider).isLoading); + expect(container.read(localLibraryProvider).loadFailed, isFalse); + expect( + container.read(localLibraryProvider).sources.single.path, + music.path, + ); + expect( + container.read(localLibraryProvider).sources.single.id, + LibraryDatabase.legacySourceId, + ); + }, + ); + + test( + 'saved folders appear while a large track index is still loading', + () async { + final pending = Completer(); + database.index = () => pending.future; + container.read(localLibraryProvider); + await _waitFor( + () => container.read(localLibraryProvider).sources.isNotEmpty, + ); + final loading = container.read(localLibraryProvider); + expect(loading.sources.single.path, music.path); + expect(loading.isLoading, isTrue); + pending.complete(const LocalLibraryLookupIndex()); + await _waitFor(() => !container.read(localLibraryProvider).isLoading); + expect(container.read(localLibraryProvider).totalCount, 2); + }, + ); + + test( + 'index failure retains folders and retry recovers without reattaching', + () async { + database.index = () async => throw StateError('index read failed'); + container.read(localLibraryProvider); + await _waitFor(() => !container.read(localLibraryProvider).isLoading); + final failed = container.read(localLibraryProvider); + expect(failed.loadFailed, isTrue); + expect(failed.sources.single.path, music.path); + database.index = () async => const LocalLibraryLookupIndex(); + await container.read(localLibraryProvider.notifier).reloadFromStorage(); + expect(container.read(localLibraryProvider).loadFailed, isFalse); + expect(container.read(localLibraryProvider).totalCount, 2); + expect(container.read(localLibraryProvider).sources.single.id, 'music'); + }, + ); + + test( + 'failed reload keeps the previously loaded folders and song count', + () async { + container.read(localLibraryProvider); + await _waitFor(() => !container.read(localLibraryProvider).isLoading); + database.failSources = true; + await container.read(localLibraryProvider.notifier).reloadFromStorage(); + final failed = container.read(localLibraryProvider); + expect(failed.loadFailed, isTrue); + expect(failed.totalCount, 2); + expect(failed.sources.single.id, 'music'); + database.failSources = false; + await container.read(localLibraryProvider.notifier).reloadFromStorage(); + expect(container.read(localLibraryProvider).loadFailed, isFalse); + }, + ); + + test('an unavailable folder remains attached instead of vanishing', () async { + await music.delete(recursive: true); + database.sources = [ + LocalLibrarySource( + id: 'missing', + path: music.path, + displayName: 'Offline music', + isRemovable: true, + available: false, + ), + ]; + database.count = () async => 0; + container.read(localLibraryProvider); + await _waitFor(() => !container.read(localLibraryProvider).isLoading); + final loaded = container.read(localLibraryProvider); + expect(loaded.loadFailed, isFalse); + expect(loaded.sources.single.available, isFalse); + expect(loaded.sources.single.path, music.path); + // Keep teardown simple after deliberately removing the storage above. + await music.create(); + }); +} diff --git a/test/library_settings_cleanup_test.dart b/test/library_settings_cleanup_test.dart index ba079f84..5234cf2a 100644 --- a/test/library_settings_cleanup_test.dart +++ b/test/library_settings_cleanup_test.dart @@ -48,6 +48,32 @@ class _History extends DownloadHistoryNotifier { Future cleanupOrphanedDownloads() => _cleanup(); } +class _RetryLibrary extends _Library { + _RetryLibrary() + : super( + LocalLibraryState( + loadFailed: true, + sources: const [ + LocalLibrarySource( + id: 'sd-card', + path: 'content://storage/tree/sd-card', + displayName: 'Saved music', + available: false, + ), + ], + ), + () async => 0, + ); + + int _reloadCalls = 0; + + @override + Future reloadFromStorage() async { + _reloadCalls++; + state = state.copyWith(loadFailed: false, totalCount: 5); + } +} + Finder get _cleanupRow => find.byWidgetPredicate( (widget) => widget is SettingsItem && widget.title == 'Cleanup Missing Files', ); @@ -84,6 +110,31 @@ Future _mount( void main() { for (final mornye in [false, true]) { + testWidgets('failed load shows saved folder and offers retry ($mornye)', ( + tester, + ) async { + final library = _RetryLibrary(); + await _mount( + tester, + mornye: mornye, + library: library, + history: _History(0, () async => 0), + ); + await tester.scrollUntilVisible(find.text('Retry'), -300); + await tester.pumpAndSettle(); + expect( + find.textContaining('Library could not be loaded'), + findsOneWidget, + ); + expect(find.text('Saved music'), findsOneWidget); + await tester.tap(find.text('Retry')); + await tester.pumpAndSettle(); + expect(library._reloadCalls, 1); + expect(find.textContaining('Library could not be loaded'), findsNothing); + expect(find.text('Saved music'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + testWidgets('cleanup works for download-only Library ($mornye)', ( tester, ) async { diff --git a/test/library_source_migration_test.dart b/test/library_source_migration_test.dart new file mode 100644 index 00000000..948c3b5e --- /dev/null +++ b/test/library_source_migration_test.dart @@ -0,0 +1,98 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/library_database.dart'; +import 'package:sqflite/sqflite.dart'; + +class _SourcesDatabase implements Database, Transaction { + final rows = >{}; + int transactions = 0; + int inserts = 0; + + @override + Future transaction( + Future Function(Transaction txn) action, { + bool? exclusive, + }) async { + transactions++; + return action(this); + } + + @override + Future update( + String table, + Map values, { + String? where, + List? whereArgs, + ConflictAlgorithm? conflictAlgorithm, + }) async { + expect(table, 'library_sources'); + final row = rows[whereArgs!.single]; + if (row == null) return 0; + row.addAll(values); + return 1; + } + + @override + Future insert( + String table, + Map values, { + String? nullColumnHack, + ConflictAlgorithm? conflictAlgorithm, + }) async { + expect(table, 'library_sources'); + inserts++; + rows[values['id'] as String] = Map.of(values); + return 1; + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +void main() { + const source = LocalLibrarySource( + id: LibraryDatabase.legacySourceId, + path: 'content://example.documents/tree/card%3AMusic', + displayName: 'Music', + isRemovable: true, + available: false, + bookmark: 'saved-bookmark', + ); + + test( + 'upgrade restores the old folder when the placeholder is missing', + () async { + final database = _SourcesDatabase(); + await persistLegacyLibrarySource(database, source); + final restored = database.rows[LibraryDatabase.legacySourceId]!; + expect(restored['path'], source.path); + expect(restored['bookmark'], source.bookmark); + expect(restored['available'], 0); + expect(restored['enabled'], 1); + expect(restored['is_removable'], 1); + expect(database.transactions, 1); + await persistLegacyLibrarySource(database, source); + expect(database.rows.length, 1); + expect(database.inserts, 1); + }, + ); + + test( + 'migration preserves the existing source identity and disabled state', + () async { + final database = _SourcesDatabase(); + database.rows[LibraryDatabase.legacySourceId] = { + 'id': LibraryDatabase.legacySourceId, + 'path': 'legacy://local-library', + 'enabled': 0, + 'last_scan_error': 'previous scan error', + }; + await persistLegacyLibrarySource(database, source); + final restored = database.rows[LibraryDatabase.legacySourceId]!; + expect(restored['id'], LibraryDatabase.legacySourceId); + expect(restored['enabled'], 0); + expect(restored['last_scan_error'], 'previous scan error'); + expect(restored['path'], source.path); + expect(database.inserts, 0); + }, + ); +}