From c84fc335ac3a80f251fda7a425558cfca4051f4f Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 11 Oct 2026 14:34:35 +0700 Subject: [PATCH] fix(ios): preserve folder grants and surface picker failures --- lib/screens/queue_tab.dart | 7 +- lib/screens/settings/files_settings_page.dart | 28 +-- lib/screens/setup_screen.dart | 6 +- lib/services/platform_bridge.dart | 26 ++- lib/utils/file_access.dart | 12 +- test/file_access_ios_test.dart | 29 +++ test/ios_download_folder_test.dart | 181 ++++++++++++++++++ test/platform_bridge_metadata_test.dart | 31 +++ 8 files changed, 292 insertions(+), 28 deletions(-) create mode 100644 test/ios_download_folder_test.dart diff --git a/lib/screens/queue_tab.dart b/lib/screens/queue_tab.dart index 1450188e..458a0093 100644 --- a/lib/screens/queue_tab.dart +++ b/lib/screens/queue_tab.dart @@ -1990,8 +1990,11 @@ class _QueueTabState extends ConsumerState { } return false; } - if (picked == null) return false; - final validation = validateIosPath(picked.path); + if (picked == null || !mounted) return false; + final validation = validateIosPath( + picked.path, + bookmark: picked.bookmark, + ); if (!validation.isValid) { if (mounted) { ScaffoldMessenger.of(context).showSnackBar( diff --git a/lib/screens/settings/files_settings_page.dart b/lib/screens/settings/files_settings_page.dart index 24ade8ae..a04897e7 100644 --- a/lib/screens/settings/files_settings_page.dart +++ b/lib/screens/settings/files_settings_page.dart @@ -1,4 +1,5 @@ import 'dart:io'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:spotiflac_android/widgets/app_alert_dialog.dart'; import 'package:spotiflac_android/widgets/app_switch.dart'; @@ -476,9 +477,9 @@ class _FilesSettingsPageState extends ConsumerState { } Future _pickDirectory(BuildContext context, WidgetRef ref) async { - if (Platform.isIOS) { + if (defaultTargetPlatform == TargetPlatform.iOS) { _showIOSDirectoryOptions(context, ref); - } else if (Platform.isAndroid) { + } else if (defaultTargetPlatform == TargetPlatform.android) { _showAndroidDirectoryOptions(context, ref); } } @@ -632,33 +633,36 @@ class _FilesSettingsPageState extends ConsumerState { try { picked = await PlatformBridge.pickIosDirectory(); } catch (e) { - if (ctx.mounted) { - ScaffoldMessenger.of(ctx).showSnackBar( + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( SnackBar( content: Text( - ctx.l10n.snackbarFolderPickerFailed( - ctx.friendlyError(e), + context.l10n.snackbarFolderPickerFailed( + context.friendlyError(e), ), ), - backgroundColor: Theme.of(ctx).colorScheme.error, + backgroundColor: Theme.of(context).colorScheme.error, duration: const Duration(seconds: 4), ), ); } return; } - if (picked == null) return; + if (picked == null || !context.mounted) return; - final validation = validateIosPath(picked.path); + final validation = validateIosPath( + picked.path, + bookmark: picked.bookmark, + ); if (!validation.isValid) { - if (ctx.mounted) { - ScaffoldMessenger.of(ctx).showSnackBar( + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( SnackBar( content: Text( validation.errorReason ?? context.l10n.setupIcloudNotSupported, ), - backgroundColor: Theme.of(ctx).colorScheme.error, + backgroundColor: Theme.of(context).colorScheme.error, duration: const Duration(seconds: 4), ), ); diff --git a/lib/screens/setup_screen.dart b/lib/screens/setup_screen.dart index f66f7d9e..7f3c9fc1 100644 --- a/lib/screens/setup_screen.dart +++ b/lib/screens/setup_screen.dart @@ -505,8 +505,10 @@ class _SetupScreenState extends ConsumerState return; } - // Validate the selected path is writable - final validation = validateIosPath(picked.path); + final validation = validateIosPath( + picked.path, + bookmark: picked.bookmark, + ); if (!validation.isValid) { if (mounted) { ScaffoldMessenger.of(context).showSnackBar( diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 7d8bcffc..46720bd2 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -2780,13 +2780,25 @@ class PlatformBridge { /// active; creating it later from a bare path loses the grant. /// Returns null when the user cancels; throws on picker/bookmark failure. static Future pickIosDirectory() async { - final result = await _channel.invokeMethod('pickIosDirectory'); - final map = _decodeNullableMapResult(result, 'pickIosDirectory'); - if (map == null) return null; - final path = map['path'] as String? ?? ''; - final bookmark = map['bookmark'] as String? ?? ''; - if (path.isEmpty || bookmark.isEmpty) return null; - return IosPickedDirectory(path: path, bookmark: bookmark); + _log.i('Opening iOS folder picker'); + try { + final result = await _channel.invokeMethod('pickIosDirectory'); + final map = _decodeNullableMapResult(result, 'pickIosDirectory'); + if (map == null) { + _log.i('iOS folder selection cancelled'); + return null; + } + final path = map['path'] as String? ?? ''; + final bookmark = map['bookmark'] as String? ?? ''; + if (!path.startsWith('/') || bookmark.trim().isEmpty) { + throw const FormatException('Invalid iOS folder selection'); + } + _log.i('iOS folder selected with an access bookmark'); + return IosPickedDirectory(path: path, bookmark: bookmark); + } catch (error, stack) { + _log.e('iOS folder selection failed', error, stack); + rethrow; + } } /// Resolve a base64-encoded iOS security-scoped bookmark and start accessing diff --git a/lib/utils/file_access.dart b/lib/utils/file_access.dart index 16bfb4ba..ba4e782a 100644 --- a/lib/utils/file_access.dart +++ b/lib/utils/file_access.dart @@ -174,11 +174,9 @@ class IosPathValidationResult { } /// Validates an iOS path and returns detailed information about the result. -IosPathValidationResult validateIosPath(String path) { - if (!Platform.isIOS) { - return const IosPathValidationResult(isValid: true); - } - +/// A bookmark supplied by the native picker grants access independently of +/// the folder's path shape. Actual access is checked when resolving that grant. +IosPathValidationResult validateIosPath(String path, {String bookmark = ''}) { if (path.isEmpty) { return const IosPathValidationResult( isValid: false, @@ -194,6 +192,10 @@ IosPathValidationResult validateIosPath(String path) { ); } + if (bookmark.trim().isNotEmpty) { + return const IosPathValidationResult(isValid: true); + } + if (_iosContainerRootPattern.hasMatch(path)) { return const IosPathValidationResult( isValid: false, diff --git a/test/file_access_ios_test.dart b/test/file_access_ios_test.dart index fb6a1cb7..60a414c7 100644 --- a/test/file_access_ios_test.dart +++ b/test/file_access_ios_test.dart @@ -2,6 +2,35 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:spotiflac_android/utils/file_access.dart'; void main() { + group('validateIosPath', () { + const cloud = + '/private/var/mobile/Library/Mobile Documents/com~apple~CloudDocs/Music'; + + test('a Files grant takes precedence over the legacy path restriction', () { + expect(validateIosPath(cloud).isValid, isFalse); + expect( + validateIosPath(cloud, bookmark: 'native-access-bookmark').isValid, + isTrue, + ); + expect(validateIosPath(cloud, bookmark: ' ').isValid, isFalse); + }); + + test('a bookmark still requires an absolute nonempty path', () { + for (final path in ['', 'relative/Music']) { + expect( + validateIosPath(path, bookmark: 'native-access-bookmark').isValid, + isFalse, + ); + } + }); + + test('unbookmarked local folders retain their path validation', () { + const container = '/private/var/mobile/Containers/Data/Application/ABC'; + expect(validateIosPath(container).isValid, isFalse); + expect(validateIosPath('$container/Documents/Music').isValid, isTrue); + }); + }); + group('shouldValidateIosOutputDir', () { test('validates a non-empty app-folder path on iOS', () { expect( diff --git a/test/ios_download_folder_test.dart b/test/ios_download_folder_test.dart new file mode 100644 index 00000000..533d1025 --- /dev/null +++ b/test/ios_download_folder_test.dart @@ -0,0 +1,181 @@ +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/l10n/l10n.dart'; +import 'package:spotiflac_android/models/settings.dart'; +import 'package:spotiflac_android/providers/settings_provider.dart'; +import 'package:spotiflac_android/screens/settings/files_settings_page.dart'; +import 'package:spotiflac_android/screens/setup_screen.dart'; +import 'package:spotiflac_android/theme/mornye_theme.dart'; + +class _SetupSettings extends SettingsNotifier { + @override + AppSettings build() => const AppSettings(); + + @override + Future ensureLoaded() async {} +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + const backend = MethodChannel('com.zarz.spotiflac/backend'); + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + late Completer?> picker; + late SharedPreferences prefs; + var pickerCalls = 0; + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + prefs = await SharedPreferences.getInstance(); + pickerCalls = 0; + messenger.setMockMethodCallHandler(backend, (call) async { + expect(call.method, 'pickIosDirectory'); + pickerCalls++; + return picker.future; + }); + }); + + tearDown(() { + messenger.setMockMethodCallHandler(backend, null); + }); + + Future openPicker( + WidgetTester tester, { + bool mornye = false, + }) async { + picker = Completer?>(); + tester.view.physicalSize = const Size(820, 1180); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.reset); + final container = ProviderContainer( + overrides: [ + initialSettingsProvider.overrideWithValue( + const AppSettings(downloadDirectory: '/old/Music'), + ), + ], + ); + addTearDown(container.dispose); + await tester.pumpWidget( + UncontrolledProviderScope( + container: container, + child: MaterialApp( + theme: mornye ? MornyeTheme.build(Brightness.light) : ThemeData(), + locale: const Locale('en'), + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: const FilesSettingsPage(), + ), + ), + ); + await tester.pumpAndSettle(); + await tester.tap(find.text('Download Directory')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Choose from Files')); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pumpAndSettle(); + // The native reply arrives after its originating sheet has been disposed. + expect(find.text('Choose from Files'), findsNothing); + expect(pickerCalls, 1); + return container; + } + + for (final mornye in [false, true]) { + testWidgets( + 'picker failure stays visible after the sheet closes ($mornye)', + (tester) async { + final container = await openPicker(tester, mornye: mornye); + picker.completeError( + PlatformException(code: 'BOOKMARK_FAILED', message: 'Access denied'), + ); + await tester.pumpAndSettle(); + expect( + find.textContaining('Failed to open folder picker:'), + findsOneWidget, + ); + expect( + container.read(settingsProvider).downloadDirectory, + '/old/Music', + ); + expect(tester.takeException(), isNull); + }, + variant: TargetPlatformVariant.only(TargetPlatform.iOS), + ); + } + + testWidgets('a granted folder is saved even with a cloud-shaped path', ( + tester, + ) async { + final container = await openPicker(tester); + const path = + '/private/var/mobile/Library/Mobile Documents/com~apple~CloudDocs/Music'; + picker.complete({'path': path, 'bookmark': 'native-access-bookmark'}); + await tester.pumpAndSettle(); + final settings = container.read(settingsProvider); + expect(settings.downloadDirectory, path); + expect(settings.downloadDirectoryBookmark, 'native-access-bookmark'); + expect(prefs.getString('app_settings'), contains('native-access-bookmark')); + expect(tester.takeException(), isNull); + }, variant: TargetPlatformVariant.only(TargetPlatform.iOS)); + + testWidgets('cancelling Files preserves the previous download folder', ( + tester, + ) async { + final container = await openPicker(tester); + picker.complete(null); + await tester.pumpAndSettle(); + expect(container.read(settingsProvider).downloadDirectory, '/old/Music'); + expect(find.textContaining('Failed to open folder picker:'), findsNothing); + expect(tester.takeException(), isNull); + }, variant: TargetPlatformVariant.only(TargetPlatform.iOS)); + + testWidgets('setup retains the chosen folder and its grant in the draft', ( + tester, + ) async { + picker = Completer?>(); + const permissions = MethodChannel( + 'flutter.baseflow.com/permissions/methods', + ); + messenger.setMockMethodCallHandler(permissions, (_) async => 1); + addTearDown(() => messenger.setMockMethodCallHandler(permissions, null)); + await prefs.setString('setup_progress_v1', '{"step":3}'); + tester.view.physicalSize = const Size(820, 1180); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.reset); + await tester.pumpWidget( + ProviderScope( + overrides: [settingsProvider.overrideWith(_SetupSettings.new)], + child: const MaterialApp( + locale: Locale('en'), + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: SetupScreen(), + ), + ), + ); + await tester.pumpAndSettle(); + expect(find.text('Select Folder'), findsOneWidget); + await tester.tap(find.text('Select Folder')); + // Setup keeps its loading indicator active while the options sheet is open. + await tester.pump(); + await tester.pump(const Duration(milliseconds: 500)); + await tester.tap(find.text('Choose from Files')); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pumpAndSettle(); + expect(pickerCalls, 1); + const path = + '/private/var/mobile/Library/Mobile Documents/com~apple~CloudDocs/Music'; + picker.complete({'path': path, 'bookmark': 'native-access-bookmark'}); + await tester.pumpAndSettle(); + final draft = jsonDecode(prefs.getString('setup_progress_v1')!) as Map; + expect(draft['directory'], path); + expect(draft['bookmark'], 'native-access-bookmark'); + expect(draft['step'], 3); + expect(tester.takeException(), isNull); + }, variant: TargetPlatformVariant.only(TargetPlatform.iOS)); +} diff --git a/test/platform_bridge_metadata_test.dart b/test/platform_bridge_metadata_test.dart index 242e39ee..cdef1203 100644 --- a/test/platform_bridge_metadata_test.dart +++ b/test/platform_bridge_metadata_test.dart @@ -13,6 +13,37 @@ void main() { .setMockMethodCallHandler(backendChannel, null); }); + test( + 'iOS folder picker distinguishes cancellation from invalid replies', + () async { + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler(backendChannel, (_) async => null); + expect(await PlatformBridge.pickIosDirectory(), isNull); + + for (final reply in [ + {}, + {'path': '/external/Music'}, + {'path': 'relative/Music', 'bookmark': 'grant'}, + {'path': '/external/Music', 'bookmark': ' '}, + ]) { + messenger.setMockMethodCallHandler(backendChannel, (_) async => reply); + await expectLater( + PlatformBridge.pickIosDirectory(), + throwsA(isA()), + ); + } + + messenger.setMockMethodCallHandler(backendChannel, (call) async { + expect(call.method, 'pickIosDirectory'); + return {'path': '/external/Music', 'bookmark': 'grant'}; + }); + final selected = await PlatformBridge.pickIosDirectory(); + expect(selected?.path, '/external/Music'); + expect(selected?.bookmark, 'grant'); + }, + ); + test('complete metadata forwards descriptor format hint', () async { TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(backendChannel, (call) async {