From 8f2a0a82478a1c4e6f61408351a6a95b349ea4dd Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Fri, 24 Sep 2021 16:19:18 +0900 Subject: [PATCH] decoupled services from settings init --- .../streams/SettingsChangeStreamHandler.kt | 2 +- lib/l10n/app_en.arb | 4 +- lib/l10n/app_ko.arb | 2 +- lib/model/settings/defaults.dart | 2 +- lib/model/settings/settings.dart | 36 ++++---- lib/services/report_service.dart | 21 +++-- lib/widgets/aves_app.dart | 49 +++++++---- .../common/behaviour/route_tracker.dart | 2 +- lib/widgets/home_page.dart | 4 +- lib/widgets/settings/privacy/privacy.dart | 8 +- lib/widgets/welcome_page.dart | 4 +- pubspec.yaml | 4 +- test/fake/report_service.dart | 3 + test_driver/app.dart | 84 ------------------- test_driver/driver_app.dart | 39 +++++++++ .../{app_test.dart => driver_app_test.dart} | 0 16 files changed, 120 insertions(+), 144 deletions(-) delete mode 100644 test_driver/app.dart create mode 100644 test_driver/driver_app.dart rename test_driver/{app_test.dart => driver_app_test.dart} (100%) diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/SettingsChangeStreamHandler.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/SettingsChangeStreamHandler.kt index aa8ce8ae5..a1961f265 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/SettingsChangeStreamHandler.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/SettingsChangeStreamHandler.kt @@ -83,6 +83,6 @@ class SettingsChangeStreamHandler(private val context: Context) : EventChannel.S companion object { private val LOG_TAG = LogUtils.createTag() - const val CHANNEL = "deckers.thibault/aves/settingschange" + const val CHANNEL = "deckers.thibault/aves/settings_change" } } \ No newline at end of file diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index f052c630c..9dc88b889 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -746,8 +746,8 @@ "settingsSectionPrivacy": "Privacy", "@settingsSectionPrivacy": {}, - "settingsEnableCrashReport": "Allow anonymous error reporting", - "@settingsEnableCrashReport": {}, + "settingsEnableErrorReporting": "Allow anonymous error reporting", + "@settingsEnableErrorReporting": {}, "settingsSaveSearchHistory": "Save search history", "@settingsSaveSearchHistory": {}, diff --git a/lib/l10n/app_ko.arb b/lib/l10n/app_ko.arb index 761025db0..fd397a490 100644 --- a/lib/l10n/app_ko.arb +++ b/lib/l10n/app_ko.arb @@ -364,7 +364,7 @@ "settingsSubtitleThemeTextAlignmentRight": "오른쪽", "settingsSectionPrivacy": "개인정보 보호", - "settingsEnableCrashReport": "오류 보고서 보내기", + "settingsEnableErrorReporting": "오류 보고서 보내기", "settingsSaveSearchHistory": "검색기록", "settingsHiddenFiltersTile": "숨겨진 필터", diff --git a/lib/model/settings/defaults.dart b/lib/model/settings/defaults.dart index 781061c9e..9de2ddf4c 100644 --- a/lib/model/settings/defaults.dart +++ b/lib/model/settings/defaults.dart @@ -13,7 +13,7 @@ import 'package:flutter/material.dart'; class SettingsDefaults { // app static const hasAcceptedTerms = false; - static const isCrashlyticsEnabled = false; + static const isErrorReportingEnabled = false; static const mustBackTwiceToExit = true; static const keepScreenOn = KeepScreenOn.viewerOnly; static const homePage = HomePageSetting.collection; diff --git a/lib/model/settings/settings.dart b/lib/model/settings/settings.dart index acb5c1a81..a09875411 100644 --- a/lib/model/settings/settings.dart +++ b/lib/model/settings/settings.dart @@ -9,7 +9,6 @@ import 'package:aves/model/filters/filters.dart'; import 'package:aves/model/settings/defaults.dart'; import 'package:aves/model/settings/enums.dart'; import 'package:aves/model/settings/map_style.dart'; -import 'package:aves/model/settings/screen_on.dart'; import 'package:aves/model/source/enums.dart'; import 'package:aves/services/common/services.dart'; import 'package:collection/collection.dart'; @@ -20,7 +19,10 @@ import 'package:shared_preferences/shared_preferences.dart'; final Settings settings = Settings._private(); class Settings extends ChangeNotifier { - final EventChannel _platformSettingsChangeChannel = const EventChannel('deckers.thibault/aves/settingschange'); + final EventChannel _platformSettingsChangeChannel = const EventChannel('deckers.thibault/aves/settings_change'); + final StreamController _updateStreamController = StreamController.broadcast(); + + Stream get updateStream => _updateStreamController.stream; static SharedPreferences? _prefs; @@ -38,7 +40,7 @@ class Settings extends ChangeNotifier { // app static const hasAcceptedTermsKey = 'has_accepted_terms'; - static const isCrashlyticsEnabledKey = 'is_crashlytics_enabled'; + static const isErrorReportingEnabledKey = 'is_crashlytics_enabled'; static const localeKey = 'locale'; static const mustBackTwiceToExitKey = 'must_back_twice_to_exit'; static const keepScreenOnKey = 'keep_screen_on'; @@ -105,9 +107,13 @@ class Settings extends ChangeNotifier { // version static const lastVersionCheckDateKey = 'last_version_check_date'; - Future init() async { + // platform settings + // cf Android `Settings.System.ACCELEROMETER_ROTATION` + static const platformAccelerometerRotationKey = 'accelerometer_rotation'; + + Future init({bool isRotationLocked = false}) async { _prefs = await SharedPreferences.getInstance(); - _isRotationLocked = await windowService.isRotationLocked(); + _isRotationLocked = isRotationLocked; } Future reset({required bool includeInternalKeys}) async { @@ -139,12 +145,9 @@ class Settings extends ChangeNotifier { set hasAcceptedTerms(bool newValue) => setAndNotify(hasAcceptedTermsKey, newValue); - bool get isCrashlyticsEnabled => getBoolOrDefault(isCrashlyticsEnabledKey, SettingsDefaults.isCrashlyticsEnabled); + bool get isErrorReportingEnabled => getBoolOrDefault(isErrorReportingEnabledKey, SettingsDefaults.isErrorReportingEnabled); - set isCrashlyticsEnabled(bool newValue) { - setAndNotify(isCrashlyticsEnabledKey, newValue); - unawaited(reportService.setCollectionEnabled(isCrashlyticsEnabled)); - } + set isErrorReportingEnabled(bool newValue) => setAndNotify(isErrorReportingEnabledKey, newValue); static const localeSeparator = '-'; @@ -180,10 +183,7 @@ class Settings extends ChangeNotifier { KeepScreenOn get keepScreenOn => getEnumOrDefault(keepScreenOnKey, SettingsDefaults.keepScreenOn, KeepScreenOn.values); - set keepScreenOn(KeepScreenOn newValue) { - setAndNotify(keepScreenOnKey, newValue.toString()); - newValue.apply(); - } + set keepScreenOn(KeepScreenOn newValue) => setAndNotify(keepScreenOnKey, newValue.toString()); HomePageSetting get homePage => getEnumOrDefault(homePageKey, SettingsDefaults.homePage, HomePageSetting.values); @@ -418,6 +418,7 @@ class Settings extends ChangeNotifier { _prefs!.setBool(key, newValue); } if (oldValue != newValue) { + _updateStreamController.add(key); notifyListeners(); } } @@ -427,8 +428,7 @@ class Settings extends ChangeNotifier { void _onPlatformSettingsChange(Map? fields) { fields?.forEach((key, value) { switch (key) { - // cf Android `Settings.System.ACCELEROMETER_ROTATION` - case 'accelerometer_rotation': + case platformAccelerometerRotationKey: if (value is int) { final newValue = value == 0; if (_isRotationLocked != newValue) { @@ -436,6 +436,7 @@ class Settings extends ChangeNotifier { if (!_isRotationLocked) { windowService.requestOrientation(); } + _updateStreamController.add(key); notifyListeners(); } } @@ -488,7 +489,7 @@ class Settings extends ChangeNotifier { debugPrint('failed to import key=$key, value=$value is not a double'); } break; - case isCrashlyticsEnabledKey: + case isErrorReportingEnabledKey: case mustBackTwiceToExitKey: case showThumbnailLocationKey: case showThumbnailMotionPhotoKey: @@ -545,6 +546,7 @@ class Settings extends ChangeNotifier { break; } } + _updateStreamController.add(key); }); notifyListeners(); } diff --git a/lib/services/report_service.dart b/lib/services/report_service.dart index 658d43667..ea024b743 100644 --- a/lib/services/report_service.dart +++ b/lib/services/report_service.dart @@ -6,6 +6,8 @@ import 'package:flutter/services.dart'; import 'package:stack_trace/stack_trace.dart'; abstract class ReportService { + Future init(); + bool get isCollectionEnabled; Future setCollectionEnabled(bool enabled); @@ -22,26 +24,29 @@ abstract class ReportService { } class CrashlyticsReportService extends ReportService { - FirebaseCrashlytics get instance => FirebaseCrashlytics.instance; + FirebaseCrashlytics get _instance => FirebaseCrashlytics.instance; @override - bool get isCollectionEnabled => instance.isCrashlyticsCollectionEnabled; + Future init() => Firebase.initializeApp(); + + @override + bool get isCollectionEnabled => _instance.isCrashlyticsCollectionEnabled; @override Future setCollectionEnabled(bool enabled) async { + debugPrint('${enabled ? 'enable' : 'disable'} Firebase & Crashlytics collection'); await Firebase.app().setAutomaticDataCollectionEnabled(enabled); - await instance.setCrashlyticsCollectionEnabled(enabled); + await _instance.setCrashlyticsCollectionEnabled(enabled); } @override - Future log(String message) => instance.log(message); + Future log(String message) => _instance.log(message); @override - Future setCustomKey(String key, Object value) => instance.setCustomKey(key, value); + Future setCustomKey(String key, Object value) => _instance.setCustomKey(key, value); @override Future setCustomKeys(Map map) { - final _instance = instance; return Future.forEach>(map.entries, (kv) => _instance.setCustomKey(kv.key, kv.value)); } @@ -60,11 +65,11 @@ class CrashlyticsReportService extends ReportService { ) .join('\n')); } - return instance.recordError(exception, stack); + return _instance.recordError(exception, stack); } @override Future recordFlutterError(FlutterErrorDetails flutterErrorDetails) { - return instance.recordFlutterError(flutterErrorDetails); + return _instance.recordFlutterError(flutterErrorDetails); } } diff --git a/lib/widgets/aves_app.dart b/lib/widgets/aves_app.dart index d3c4602c2..07f0d5399 100644 --- a/lib/widgets/aves_app.dart +++ b/lib/widgets/aves_app.dart @@ -1,6 +1,7 @@ import 'dart:ui'; import 'package:aves/app_mode.dart'; +import 'package:aves/model/settings/screen_on.dart'; import 'package:aves/model/settings/settings.dart'; import 'package:aves/model/source/collection_source.dart'; import 'package:aves/model/source/media_store_source.dart'; @@ -16,7 +17,6 @@ import 'package:aves/widgets/common/providers/highlight_info_provider.dart'; import 'package:aves/widgets/home_page.dart'; import 'package:aves/widgets/welcome_page.dart'; import 'package:equatable/equatable.dart'; -import 'package:firebase_core/firebase_core.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -123,25 +123,38 @@ class _AvesAppState extends State { } Future _setup() async { - await Firebase.initializeApp().then((app) async { - FlutterError.onError = reportService.recordFlutterError; - final now = DateTime.now(); - final hasPlayServices = await availability.hasPlayServices; - await reportService.setCustomKeys({ - 'build_mode': kReleaseMode - ? 'release' - : kProfileMode - ? 'profile' - : 'debug', - 'has_play_services': hasPlayServices, - 'locales': window.locales.join(', '), - 'time_zone': '${now.timeZoneName} (${now.timeZoneOffset})', - }); + await settings.init( + isRotationLocked: await windowService.isRotationLocked(), + ); + + // keep screen on + settings.updateStream.where((key) => key == Settings.keepScreenOnKey).listen( + (_) => settings.keepScreenOn.apply(), + ); + settings.keepScreenOn.apply(); + + // error reporting + await reportService.init(); + settings.updateStream.where((key) => key == Settings.isErrorReportingEnabledKey).listen( + (_) => reportService.setCollectionEnabled(settings.isErrorReportingEnabled), + ); + await reportService.setCollectionEnabled(settings.isErrorReportingEnabled); + + FlutterError.onError = reportService.recordFlutterError; + final now = DateTime.now(); + final hasPlayServices = await availability.hasPlayServices; + await reportService.setCustomKeys({ + 'build_mode': kReleaseMode + ? 'release' + : kProfileMode + ? 'profile' + : 'debug', + 'has_play_services': hasPlayServices, + 'locales': window.locales.join(', '), + 'time_zone': '${now.timeZoneName} (${now.timeZoneOffset})', }); - await settings.init(); - await reportService.setCollectionEnabled(settings.isCrashlyticsEnabled); _navigatorObservers = [ - CrashlyticsRouteTracker(), + ReportingRouteTracker(), ]; } diff --git a/lib/widgets/common/behaviour/route_tracker.dart b/lib/widgets/common/behaviour/route_tracker.dart index 98391587b..e86f45559 100644 --- a/lib/widgets/common/behaviour/route_tracker.dart +++ b/lib/widgets/common/behaviour/route_tracker.dart @@ -1,7 +1,7 @@ import 'package:aves/services/common/services.dart'; import 'package:flutter/material.dart'; -class CrashlyticsRouteTracker extends NavigatorObserver { +class ReportingRouteTracker extends NavigatorObserver { @override void didPush(Route route, Route? previousRoute) => reportService.log('Nav didPush to ${_name(route)}'); diff --git a/lib/widgets/home_page.dart b/lib/widgets/home_page.dart index 81d9eea44..0f079f8cb 100644 --- a/lib/widgets/home_page.dart +++ b/lib/widgets/home_page.dart @@ -4,12 +4,11 @@ import 'package:aves/app_mode.dart'; import 'package:aves/model/entry.dart'; import 'package:aves/model/filters/filters.dart'; import 'package:aves/model/settings/home_page.dart'; -import 'package:aves/model/settings/screen_on.dart'; import 'package:aves/model/settings/settings.dart'; import 'package:aves/model/source/collection_lens.dart'; import 'package:aves/model/source/collection_source.dart'; -import 'package:aves/services/global_search.dart'; import 'package:aves/services/common/services.dart'; +import 'package:aves/services/global_search.dart'; import 'package:aves/services/viewer_service.dart'; import 'package:aves/utils/android_file_utils.dart'; import 'package:aves/widgets/collection/collection_page.dart'; @@ -50,7 +49,6 @@ class _HomePageState extends State { super.initState(); _setup(); imageCache!.maximumSizeBytes = 512 * (1 << 20); - settings.keepScreenOn.apply(); } @override diff --git a/lib/widgets/settings/privacy/privacy.dart b/lib/widgets/settings/privacy/privacy.dart index c850a92bd..75a1a2523 100644 --- a/lib/widgets/settings/privacy/privacy.dart +++ b/lib/widgets/settings/privacy/privacy.dart @@ -20,7 +20,7 @@ class PrivacySection extends StatelessWidget { @override Widget build(BuildContext context) { - final currentIsCrashlyticsEnabled = context.select((s) => s.isCrashlyticsEnabled); + final currentIsErrorReportingEnabled = context.select((s) => s.isErrorReportingEnabled); final currentSaveSearchHistory = context.select((s) => s.saveSearchHistory); return AvesExpansionTile( @@ -33,9 +33,9 @@ class PrivacySection extends StatelessWidget { showHighlight: false, children: [ SwitchListTile( - value: currentIsCrashlyticsEnabled, - onChanged: (v) => settings.isCrashlyticsEnabled = v, - title: Text(context.l10n.settingsEnableCrashReport), + value: currentIsErrorReportingEnabled, + onChanged: (v) => settings.isErrorReportingEnabled = v, + title: Text(context.l10n.settingsEnableErrorReporting), ), SwitchListTile( value: currentSaveSearchHistory, diff --git a/lib/widgets/welcome_page.dart b/lib/widgets/welcome_page.dart index 1f4f45a6c..13365b2e0 100644 --- a/lib/widgets/welcome_page.dart +++ b/lib/widgets/welcome_page.dart @@ -102,9 +102,9 @@ class _WelcomePageState extends State { crossAxisAlignment: CrossAxisAlignment.start, children: [ LabeledCheckbox( - value: settings.isCrashlyticsEnabled, + value: settings.isErrorReportingEnabled, onChanged: (v) { - if (v != null) setState(() => settings.isCrashlyticsEnabled = v); + if (v != null) setState(() => settings.isErrorReportingEnabled = v); }, text: context.l10n.welcomeCrashReportToggle, ), diff --git a/pubspec.yaml b/pubspec.yaml index c0bedb4b4..0bd07a5df 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -117,7 +117,7 @@ flutter: # Test driver # run (any device): -# % flutter drive -t test_driver/app.dart --profile +# % flutter drive -t test_driver/driver_app.dart --profile # capture shaders in profile mode (real device only): -# % flutter drive -t test_driver/app.dart --profile --cache-sksl --write-sksl-on-exit shaders.sksl.json +# % flutter drive -t test_driver/driver_app.dart --profile --cache-sksl --write-sksl-on-exit shaders.sksl.json diff --git a/test/fake/report_service.dart b/test/fake/report_service.dart index a73e3b046..d05e61c8f 100644 --- a/test/fake/report_service.dart +++ b/test/fake/report_service.dart @@ -3,6 +3,9 @@ import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; class FakeReportService extends ReportService { + @override + Future init() => SynchronousFuture(null); + @override bool get isCollectionEnabled => false; diff --git a/test_driver/app.dart b/test_driver/app.dart deleted file mode 100644 index 1ab9d1504..000000000 --- a/test_driver/app.dart +++ /dev/null @@ -1,84 +0,0 @@ -import 'dart:ui'; - -import 'package:aves/main.dart' as app; -import 'package:aves/model/settings/enums.dart'; -import 'package:aves/model/settings/settings.dart'; -import 'package:aves/services/common/services.dart'; -import 'package:aves/services/media/media_store_service.dart'; -import 'package:aves/services/report_service.dart'; -import 'package:aves/services/window_service.dart'; -import 'package:flutter/foundation.dart'; -import 'package:flutter_driver/driver_extension.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:path/path.dart' as p; - -import 'constants.dart'; - -void main() { - enableFlutterDriverExtension(); - - // scan files copied from test assets - // we do it via the app instead of broadcasting via ADB - // because `MEDIA_SCANNER_SCAN_FILE` intent got deprecated in API 29 - PlatformMediaStoreService() - ..scanFile(p.join(targetPicturesDir, 'aves_logo.svg'), 'image/svg+xml') - ..scanFile(p.join(targetPicturesDir, 'ipse.jpg'), 'image/jpeg'); - - // something like `configure().then((_) => app.main());` does not behave as expected - // and starts the app without waiting for `configure` to complete - configureAndLaunch(); -} - -Future configureAndLaunch() async { - // TODO TLAD [test] decouple services from settings setters, so there is no need for fake services here - // set up fake services called during settings initialization - getIt - ..registerSingleton(DriverInitWindowService()) - ..registerSingleton(DriverInitReportService()); - - await settings.init(); - settings - ..keepScreenOn = KeepScreenOn.always - ..hasAcceptedTerms = false - ..isCrashlyticsEnabled = false - ..locale = const Locale('en') - ..homePage = HomePageSetting.collection - ..imageBackground = EntryBackground.checkered; - - // tear down fake services - await Future.delayed(const Duration(seconds: 1)); - await getIt.reset(); - - app.main(); -} - -class DriverInitWindowService extends Fake implements WindowService { - @override - Future keepScreenOn(bool on) => SynchronousFuture(null); - - @override - Future isRotationLocked() => SynchronousFuture(false); -} - -class DriverInitReportService extends Fake implements ReportService { - @override - bool get isCollectionEnabled => false; - - @override - Future setCollectionEnabled(bool enabled) => SynchronousFuture(null); - - @override - Future log(String message) => SynchronousFuture(null); - - @override - Future setCustomKey(String key, Object value) => SynchronousFuture(null); - - @override - Future setCustomKeys(Map map) => SynchronousFuture(null); - - @override - Future recordError(exception, StackTrace? stack) => SynchronousFuture(null); - - @override - Future recordFlutterError(FlutterErrorDetails flutterErrorDetails) => SynchronousFuture(null); -} diff --git a/test_driver/driver_app.dart b/test_driver/driver_app.dart new file mode 100644 index 000000000..3b2bf575c --- /dev/null +++ b/test_driver/driver_app.dart @@ -0,0 +1,39 @@ +import 'dart:ui'; + +import 'package:aves/main.dart' as app; +import 'package:aves/model/settings/enums.dart'; +import 'package:aves/model/settings/settings.dart'; +import 'package:aves/services/media/media_store_service.dart'; +import 'package:flutter_driver/driver_extension.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; + +import 'constants.dart'; + +void main() { + enableFlutterDriverExtension(); + + // scan files copied from test assets + // we do it via the app instead of broadcasting via ADB + // because `MEDIA_SCANNER_SCAN_FILE` intent got deprecated in API 29 + PlatformMediaStoreService() + ..scanFile(p.join(targetPicturesDir, 'aves_logo.svg'), 'image/svg+xml') + ..scanFile(p.join(targetPicturesDir, 'ipse.jpg'), 'image/jpeg'); + + // something like `configure().then((_) => app.main());` does not behave as expected + // and starts the app without waiting for `configure` to complete + configureAndLaunch(); +} + +Future configureAndLaunch() async { + await settings.init(); + settings + ..keepScreenOn = KeepScreenOn.always + ..hasAcceptedTerms = false + ..isErrorReportingEnabled = false + ..locale = const Locale('en') + ..homePage = HomePageSetting.collection + ..imageBackground = EntryBackground.checkered; + + app.main(); +} diff --git a/test_driver/app_test.dart b/test_driver/driver_app_test.dart similarity index 100% rename from test_driver/app_test.dart rename to test_driver/driver_app_test.dart