From 5f94af37dda0e09c0718628ba5767a8e544409bb Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Sun, 11 Aug 2024 15:10:36 +0300 Subject: [PATCH 01/12] chore: add screen name masker --- lib/instabug_flutter.dart | 1 + lib/src/modules/instabug.dart | 9 ++++++ lib/src/utils/screen_name_masker.dart | 45 +++++++++++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 lib/src/utils/screen_name_masker.dart diff --git a/lib/instabug_flutter.dart b/lib/instabug_flutter.dart index 783fb1350..6dc949d1b 100644 --- a/lib/instabug_flutter.dart +++ b/lib/instabug_flutter.dart @@ -19,3 +19,4 @@ export 'src/modules/surveys.dart'; export 'src/utils/instabug_navigator_observer.dart'; export 'src/utils/screen_loading/instabug_capture_screen_loading.dart'; export 'src/utils/screen_loading/route_matcher.dart'; +export 'src/utils/screen_name_masker.dart' show ScreenNameMaskingCallback; diff --git a/lib/src/modules/instabug.dart b/lib/src/modules/instabug.dart index 9d578e030..bf457a4fb 100644 --- a/lib/src/modules/instabug.dart +++ b/lib/src/modules/instabug.dart @@ -19,6 +19,7 @@ import 'package:instabug_flutter/src/generated/instabug.api.g.dart'; import 'package:instabug_flutter/src/utils/enum_converter.dart'; import 'package:instabug_flutter/src/utils/ibg_build_info.dart'; import 'package:instabug_flutter/src/utils/instabug_logger.dart'; +import 'package:instabug_flutter/src/utils/screen_name_masker.dart'; import 'package:meta/meta.dart'; enum InvocationEvent { @@ -191,6 +192,14 @@ class Instabug { ); } + /// Sets a [callback] to be called wehenever a screen name is captured to mask + /// sensitive information in the screen name. + static void setScreenNameMaskingCallback( + ScreenNameMaskingCallback? callback, + ) { + ScreenNameMasker.I.setMaskingCallback(callback); + } + /// Shows the welcome message in a specific mode. /// [welcomeMessageMode] is an enum to set the welcome message mode to live, or beta. static Future showWelcomeMessageWithMode( diff --git a/lib/src/utils/screen_name_masker.dart b/lib/src/utils/screen_name_masker.dart new file mode 100644 index 000000000..3b581ee1b --- /dev/null +++ b/lib/src/utils/screen_name_masker.dart @@ -0,0 +1,45 @@ +import 'package:flutter/material.dart'; + +typedef ScreenNameMaskingCallback = String Function(String screen); + +/// Mockable [ScreenNameMasker] responsible for masking screen names +/// before they are sent to the native SDKs. +class ScreenNameMasker { + ScreenNameMasker._(); + + static ScreenNameMasker _instance = ScreenNameMasker._(); + + static ScreenNameMasker get instance => _instance; + + /// Shorthand for [instance] + static ScreenNameMasker get I => instance; + + static const emptyScreenNameFallback = "N/A"; + + ScreenNameMaskingCallback? _screenNameMaskingCallback; + + @visibleForTesting + // ignore: use_setters_to_change_properties + static void setInstance(ScreenNameMasker instance) { + _instance = instance; + } + + // ignore: use_setters_to_change_properties + void setMaskingCallback(ScreenNameMaskingCallback? callback) { + _screenNameMaskingCallback = callback; + } + + String maskScreenName(String screen) { + if (_screenNameMaskingCallback == null) { + return screen; + } + + final maskedScreen = _screenNameMaskingCallback!(screen).trim(); + + if (maskedScreen.isEmpty) { + return emptyScreenNameFallback; + } + + return maskedScreen; + } +} From 25471e08bb63af91db24c748e351cb61bc453af6 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Sun, 11 Aug 2024 15:11:38 +0300 Subject: [PATCH 02/12] feat: mask screen name in repro steps --- lib/src/models/instabug_route.dart | 11 ++++++++ .../utils/instabug_navigator_observer.dart | 28 ++++++++++++------- 2 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 lib/src/models/instabug_route.dart diff --git a/lib/src/models/instabug_route.dart b/lib/src/models/instabug_route.dart new file mode 100644 index 000000000..8c1539426 --- /dev/null +++ b/lib/src/models/instabug_route.dart @@ -0,0 +1,11 @@ +import 'package:flutter/material.dart'; + +class InstabugRoute { + final Route route; + final String name; + + const InstabugRoute({ + required this.route, + required this.name, + }); +} diff --git a/lib/src/utils/instabug_navigator_observer.dart b/lib/src/utils/instabug_navigator_observer.dart index 3cb39cc05..0d01de2b4 100644 --- a/lib/src/utils/instabug_navigator_observer.dart +++ b/lib/src/utils/instabug_navigator_observer.dart @@ -1,32 +1,40 @@ import 'package:flutter/material.dart'; import 'package:instabug_flutter/instabug_flutter.dart'; +import 'package:instabug_flutter/src/models/instabug_route.dart'; import 'package:instabug_flutter/src/modules/instabug.dart'; import 'package:instabug_flutter/src/utils/instabug_logger.dart'; import 'package:instabug_flutter/src/utils/screen_loading/screen_loading_manager.dart'; +import 'package:instabug_flutter/src/utils/screen_name_masker.dart'; class InstabugNavigatorObserver extends NavigatorObserver { - final List _steps = []; + final List _steps = []; void screenChanged(Route newRoute) { try { final screenName = newRoute.settings.name.toString(); + final maskedScreenName = ScreenNameMasker.I.maskScreenName(screenName); + + final route = InstabugRoute( + route: newRoute, + name: maskedScreenName, + ); + // Starts a the new UI trace which is exclusive to screen loading - ScreenLoadingManager.I.startUiTrace(screenName); + ScreenLoadingManager.I.startUiTrace(maskedScreenName); // If there is a step that hasn't been pushed yet if (_steps.isNotEmpty) { // Report the last step and remove it from the list - Instabug.reportScreenChange( - _steps[_steps.length - 1].settings.name.toString(), - ); - _steps.remove(_steps[_steps.length - 1]); + Instabug.reportScreenChange(_steps.last.name); + _steps.removeLast(); } + // Add the new step to the list - _steps.add(newRoute); + _steps.add(route); Future.delayed(const Duration(milliseconds: 1000), () { // If this route is in the array, report it and remove it from the list - if (_steps.contains(newRoute)) { - Instabug.reportScreenChange(screenName); - _steps.remove(newRoute); + if (_steps.contains(route)) { + Instabug.reportScreenChange(route.name); + _steps.remove(route); } }); } catch (e) { From a6b48e461e8822d8638d38dad3ff6ffdb21ffdec Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Sun, 11 Aug 2024 15:12:09 +0300 Subject: [PATCH 03/12] chore: match screen loading traces with ui traces on original screen name --- .../utils/instabug_navigator_observer.dart | 2 +- .../screen_loading_manager.dart | 26 +++++++++++++---- lib/src/utils/screen_loading/ui_trace.dart | 28 ++++++++++++++++--- .../screen_loading_manager_test.dart | 15 +++++----- 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/lib/src/utils/instabug_navigator_observer.dart b/lib/src/utils/instabug_navigator_observer.dart index 0d01de2b4..a9fed9797 100644 --- a/lib/src/utils/instabug_navigator_observer.dart +++ b/lib/src/utils/instabug_navigator_observer.dart @@ -20,7 +20,7 @@ class InstabugNavigatorObserver extends NavigatorObserver { ); // Starts a the new UI trace which is exclusive to screen loading - ScreenLoadingManager.I.startUiTrace(maskedScreenName); + ScreenLoadingManager.I.startUiTrace(maskedScreenName, screenName); // If there is a step that hasn't been pushed yet if (_steps.isNotEmpty) { // Report the last step and remove it from the list diff --git a/lib/src/utils/screen_loading/screen_loading_manager.dart b/lib/src/utils/screen_loading/screen_loading_manager.dart index 2af57f09f..e95543ee9 100644 --- a/lib/src/utils/screen_loading/screen_loading_manager.dart +++ b/lib/src/utils/screen_loading/screen_loading_manager.dart @@ -126,8 +126,16 @@ class ScreenLoadingManager { return sanitizedScreenName; } + /// Starts a new UI trace with [screenName] as the public screen name and + /// [matchingScreenName] as the screen name used for matching the UI trace + /// with a Screen Loading trace. @internal - Future startUiTrace(String screenName) async { + Future startUiTrace( + String screenName, [ + String? matchingScreenName, + ]) async { + matchingScreenName ??= screenName; + try { resetDidStartScreenLoading(); @@ -151,10 +159,19 @@ class ScreenLoadingManager { } final sanitizedScreenName = sanitizeScreenName(screenName); + final sanitizedMatchingScreenName = + sanitizeScreenName(matchingScreenName); + final microTimeStamp = IBGDateTime.I.now().microsecondsSinceEpoch; final uiTraceId = IBGDateTime.I.now().millisecondsSinceEpoch; + APM.startCpUiTrace(sanitizedScreenName, microTimeStamp, uiTraceId); - currentUiTrace = UiTrace(sanitizedScreenName, traceId: uiTraceId); + + currentUiTrace = UiTrace( + screenName: sanitizedScreenName, + matchingScreenName: sanitizedMatchingScreenName, + traceId: uiTraceId, + ); } catch (error, stackTrace) { _logExceptionErrorAndStackTrace(error, stackTrace); } @@ -183,10 +200,7 @@ class ScreenLoadingManager { return; } - final isSameScreen = RouteMatcher.I.match( - routePath: trace.screenName, - actualPath: currentUiTrace?.screenName, - ); + final isSameScreen = currentUiTrace?.matches(trace.screenName) == true; final didStartLoading = currentUiTrace?.didStartScreenLoading == true; diff --git a/lib/src/utils/screen_loading/ui_trace.dart b/lib/src/utils/screen_loading/ui_trace.dart index 7fd03c9fd..b3f505640 100644 --- a/lib/src/utils/screen_loading/ui_trace.dart +++ b/lib/src/utils/screen_loading/ui_trace.dart @@ -1,25 +1,45 @@ +import 'package:instabug_flutter/src/utils/screen_loading/route_matcher.dart'; + class UiTrace { final String screenName; + + /// The screen name used while matching the UI trace with a Screen Loading + /// trace. + /// + /// For example, this is set to the original screen name before masking when + /// screen names masking is enabled. + final String _matchingScreenName; + final int traceId; bool didStartScreenLoading = false; bool didReportScreenLoading = false; bool didExtendScreenLoading = false; - UiTrace( - this.screenName, { + UiTrace({ + required this.screenName, required this.traceId, - }); + String? matchingScreenName, + }) : _matchingScreenName = matchingScreenName ?? screenName; UiTrace copyWith({ String? screenName, + String? matchingScreenName, int? traceId, }) { return UiTrace( - screenName ?? this.screenName, + screenName: screenName ?? this.screenName, + matchingScreenName: screenName ?? this.screenName, traceId: traceId ?? this.traceId, ); } + bool matches(String routePath) { + return RouteMatcher.I.match( + routePath: routePath, + actualPath: _matchingScreenName, + ); + } + @override String toString() { return 'UiTrace{screenName: $screenName, traceId: $traceId, isFirstScreenLoadingReported: $didReportScreenLoading, isFirstScreenLoading: $didStartScreenLoading}'; diff --git a/test/utils/screen_loading/screen_loading_manager_test.dart b/test/utils/screen_loading/screen_loading_manager_test.dart index 14e33c663..c6c7905eb 100644 --- a/test/utils/screen_loading/screen_loading_manager_test.dart +++ b/test/utils/screen_loading/screen_loading_manager_test.dart @@ -81,7 +81,7 @@ void main() { '[resetDidStartScreenLoading] should set _currentUITrace?.didStartScreenLoading to false', () async { const expected = false; - final uiTrace = UiTrace('screen1', traceId: 1); + final uiTrace = UiTrace(screenName: 'screen1', traceId: 1); uiTrace.didStartScreenLoading = true; mScreenLoadingManager.currentUiTrace = uiTrace; @@ -103,7 +103,7 @@ void main() { '[resetDidReportScreenLoading] should set _currentUITrace?.didReportScreenLoading to false', () async { const expected = false; - final uiTrace = UiTrace('screen1', traceId: 1); + final uiTrace = UiTrace(screenName: 'screen1', traceId: 1); uiTrace.didReportScreenLoading = true; mScreenLoadingManager.currentUiTrace = uiTrace; @@ -125,7 +125,7 @@ void main() { '[resetDidExtendScreenLoading] should set _currentUITrace?.didExtendScreenLoading to false', () async { const expected = false; - final uiTrace = UiTrace('screen1', traceId: 1); + final uiTrace = UiTrace(screenName: 'screen1', traceId: 1); mScreenLoadingManager.currentUiTrace = uiTrace; ScreenLoadingManager.I.resetDidExtendScreenLoading(); @@ -160,7 +160,8 @@ void main() { setUp(() { time = DateTime.now(); - uiTrace = UiTrace(screenName, traceId: time.millisecondsSinceEpoch); + uiTrace = + UiTrace(screenName: screenName, traceId: time.millisecondsSinceEpoch); ScreenLoadingManager.setInstance(ScreenLoadingManagerNoResets.init()); mScreenLoadingManager.currentUiTrace = uiTrace; when(mDateTime.now()).thenReturn(time); @@ -241,7 +242,7 @@ void main() { mScreenLoadingManager = ScreenLoadingManagerNoResets.init(); time = DateTime.now(); traceId = time.millisecondsSinceEpoch; - uiTrace = UiTrace(screenName, traceId: traceId); + uiTrace = UiTrace(screenName: screenName, traceId: traceId); mScreenLoadingManager.currentUiTrace = uiTrace; when(mDateTime.now()).thenReturn(time); @@ -421,7 +422,7 @@ void main() { setUp(() { time = DateTime.now(); traceId = time.millisecondsSinceEpoch; - uiTrace = UiTrace(screenName, traceId: traceId); + uiTrace = UiTrace(screenName: screenName, traceId: traceId); mScreenLoadingManager.currentUiTrace = uiTrace; when(mDateTime.now()).thenReturn(time); screenLoadingTrace = ScreenLoadingTrace( @@ -719,7 +720,7 @@ void main() { setUp(() { time = DateTime.now(); traceId = time.millisecondsSinceEpoch; - uiTrace = UiTrace(screenName, traceId: traceId); + uiTrace = UiTrace(screenName: screenName, traceId: traceId); duration = 1000; extendedMonotonic = 500; endTime = time.add(Duration(microseconds: duration ?? 0)); From 8d8eddcf415db2146e5b55ab94d12ab57ee0465a Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Sun, 11 Aug 2024 15:58:09 +0300 Subject: [PATCH 04/12] refactor: rename maskScreenName to mask --- lib/src/utils/instabug_navigator_observer.dart | 2 +- lib/src/utils/screen_name_masker.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/utils/instabug_navigator_observer.dart b/lib/src/utils/instabug_navigator_observer.dart index a9fed9797..57550765c 100644 --- a/lib/src/utils/instabug_navigator_observer.dart +++ b/lib/src/utils/instabug_navigator_observer.dart @@ -12,7 +12,7 @@ class InstabugNavigatorObserver extends NavigatorObserver { void screenChanged(Route newRoute) { try { final screenName = newRoute.settings.name.toString(); - final maskedScreenName = ScreenNameMasker.I.maskScreenName(screenName); + final maskedScreenName = ScreenNameMasker.I.mask(screenName); final route = InstabugRoute( route: newRoute, diff --git a/lib/src/utils/screen_name_masker.dart b/lib/src/utils/screen_name_masker.dart index 3b581ee1b..ef228697f 100644 --- a/lib/src/utils/screen_name_masker.dart +++ b/lib/src/utils/screen_name_masker.dart @@ -29,7 +29,7 @@ class ScreenNameMasker { _screenNameMaskingCallback = callback; } - String maskScreenName(String screen) { + String mask(String screen) { if (_screenNameMaskingCallback == null) { return screen; } From 13926d809fc08f1aa935adbd65f868ccd8ed2466 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Sun, 11 Aug 2024 16:07:34 +0300 Subject: [PATCH 05/12] test: add unit tests for screen name masker --- test/utils/screen_name_masker_test.dart | 40 +++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 test/utils/screen_name_masker_test.dart diff --git a/test/utils/screen_name_masker_test.dart b/test/utils/screen_name_masker_test.dart new file mode 100644 index 000000000..24f71dd99 --- /dev/null +++ b/test/utils/screen_name_masker_test.dart @@ -0,0 +1,40 @@ +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:instabug_flutter/src/utils/screen_name_masker.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + WidgetsFlutterBinding.ensureInitialized(); + + setUp(() { + ScreenNameMasker.I.setMaskingCallback(null); + }); + + test('[mask] should return same screen name when no masking callback is set', + () { + const screen = 'home'; + + final result = ScreenNameMasker.I.mask(screen); + + expect(result, equals(screen)); + }); + + test( + '[mask] should mask screen name when [setMaskingCallback] has set a callback', + () { + const screen = '/documents/314159265'; + const masked = '/documents/REDACTED'; + + ScreenNameMasker.I.setMaskingCallback((screen) { + if (screen.startsWith('/documents/')) { + return masked; + } + + return screen; + }); + + final result = ScreenNameMasker.I.mask(screen); + + expect(result, equals(masked)); + }); +} From 86e5c0f8b8ed6d6a1b9c3987aac4d5f35443f8e6 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Sun, 11 Aug 2024 16:12:03 +0300 Subject: [PATCH 06/12] test: add unit tests for setScreenNameMaskingCallback --- test/instabug_test.dart | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/instabug_test.dart b/test/instabug_test.dart index f525dc02c..0e6f0f421 100644 --- a/test/instabug_test.dart +++ b/test/instabug_test.dart @@ -6,6 +6,7 @@ import 'package:instabug_flutter/instabug_flutter.dart'; import 'package:instabug_flutter/src/generated/instabug.api.g.dart'; import 'package:instabug_flutter/src/utils/enum_converter.dart'; import 'package:instabug_flutter/src/utils/ibg_build_info.dart'; +import 'package:instabug_flutter/src/utils/screen_name_masker.dart'; import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; @@ -14,6 +15,7 @@ import 'instabug_test.mocks.dart'; @GenerateMocks([ InstabugHostApi, IBGBuildInfo, + ScreenNameMasker, ]) void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -21,10 +23,12 @@ void main() { final mHost = MockInstabugHostApi(); final mBuildInfo = MockIBGBuildInfo(); + final mScreenNameMasker = MockScreenNameMasker(); setUpAll(() { Instabug.$setHostApi(mHost); IBGBuildInfo.setInstance(mBuildInfo); + ScreenNameMasker.setInstance(mScreenNameMasker); }); test('[setEnabled] should call host method', () async { @@ -76,6 +80,16 @@ void main() { ).called(1); }); + test( + '[setScreenNameMaskingCallback] should set masking callback on screen name masker', + () async { + String callback(String screen) => 'REDACTED/$screen'; + + Instabug.setScreenNameMaskingCallback(callback); + + verify(mScreenNameMasker.setMaskingCallback(callback)).called(1); + }); + test('[show] should call host method', () async { await Instabug.show(); From dfdfc4f6d2f0a8b736ad0f1c7956cf5ce8c31935 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Sun, 11 Aug 2024 17:58:32 +0300 Subject: [PATCH 07/12] test: add unit test for masking repro steps --- .../instabug_navigator_observer_test.dart | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/test/utils/instabug_navigator_observer_test.dart b/test/utils/instabug_navigator_observer_test.dart index 0eca4ca2e..f7a6f6a7f 100644 --- a/test/utils/instabug_navigator_observer_test.dart +++ b/test/utils/instabug_navigator_observer_test.dart @@ -4,6 +4,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:instabug_flutter/instabug_flutter.dart'; import 'package:instabug_flutter/src/generated/instabug.api.g.dart'; import 'package:instabug_flutter/src/utils/screen_loading/screen_loading_manager.dart'; +import 'package:instabug_flutter/src/utils/screen_name_masker.dart'; import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; @@ -35,6 +36,8 @@ void main() { observer = InstabugNavigatorObserver(); route = createRoute(screen); previousRoute = createRoute(previousScreen); + + ScreenNameMasker.I.setMaskingCallback(null); }); test('should report screen change when a route is pushed', () { @@ -44,7 +47,7 @@ void main() { async.elapse(const Duration(milliseconds: 1000)); verify( - mScreenLoadingManager.startUiTrace(screen), + mScreenLoadingManager.startUiTrace(screen, screen), ).called(1); verify( @@ -62,7 +65,7 @@ void main() { async.elapse(const Duration(milliseconds: 1000)); verify( - mScreenLoadingManager.startUiTrace(previousScreen), + mScreenLoadingManager.startUiTrace(previousScreen, previousScreen), ).called(1); verify( @@ -80,7 +83,7 @@ void main() { async.elapse(const Duration(milliseconds: 1000)); verifyNever( - mScreenLoadingManager.startUiTrace(any), + mScreenLoadingManager.startUiTrace(any, any), ); verifyNever( @@ -88,6 +91,26 @@ void main() { ); }); }); + + test('should mask screen name when masking callback is set', () { + const maskedScreen = 'maskedScreen'; + + ScreenNameMasker.I.setMaskingCallback((_) => maskedScreen); + + fakeAsync((async) { + observer.didPush(route, previousRoute); + + async.elapse(const Duration(milliseconds: 1000)); + + verify( + mScreenLoadingManager.startUiTrace(maskedScreen, screen), + ).called(1); + + verify( + mHost.reportScreenChange(maskedScreen), + ).called(1); + }); + }); } Route createRoute(String? name) { From e298d067b87ad1348ae7f9dab8500ca2d7ffa331 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Mon, 12 Aug 2024 11:55:13 +0300 Subject: [PATCH 08/12] test: add test for matching screen in screen loading start ui trace --- .../screen_loading_manager_test.dart | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/utils/screen_loading/screen_loading_manager_test.dart b/test/utils/screen_loading/screen_loading_manager_test.dart index c6c7905eb..0e96dceb2 100644 --- a/test/utils/screen_loading/screen_loading_manager_test.dart +++ b/test/utils/screen_loading/screen_loading_manager_test.dart @@ -231,6 +231,50 @@ void main() { ), ).called(1); }); + + test( + '[startUiTrace] with APM enabled should create a UI trace with the matching screen name', + () async { + when(FlagsConfig.apm.isEnabled()).thenAnswer((_) async => true); + when(IBGBuildInfo.I.isIOS).thenReturn(false); + when( + RouteMatcher.I.match( + routePath: anyNamed('routePath'), + actualPath: anyNamed('actualPath'), + ), + ).thenReturn(false); + + const matchingScreenName = 'matching_screen_name'; + + await ScreenLoadingManager.I.startUiTrace(screenName, matchingScreenName); + + final actualUiTrace = ScreenLoadingManager.I.currentUiTrace!; + + actualUiTrace.matches(screenName); + + verify( + mRouteMatcher.match( + routePath: screenName, + actualPath: matchingScreenName, + ), + ).called(1); + + verifyNever( + mRouteMatcher.match( + routePath: anyNamed('routePath'), + actualPath: screenName, + ), + ); + + expect(actualUiTrace.screenName, screenName); + verify( + mApmHost.startCpUiTrace( + screenName, + any, + any, + ), + ).called(1); + }); }); group('startScreenLoadingTrace tests', () { From d99cba24cd8fafd4caaa538747cd5538094d8e91 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Mon, 12 Aug 2024 13:49:05 +0300 Subject: [PATCH 09/12] fix: pass correct parameters in `UiTrace.copyWith` --- lib/src/utils/screen_loading/ui_trace.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/utils/screen_loading/ui_trace.dart b/lib/src/utils/screen_loading/ui_trace.dart index b3f505640..17ef41046 100644 --- a/lib/src/utils/screen_loading/ui_trace.dart +++ b/lib/src/utils/screen_loading/ui_trace.dart @@ -28,7 +28,7 @@ class UiTrace { }) { return UiTrace( screenName: screenName ?? this.screenName, - matchingScreenName: screenName ?? this.screenName, + matchingScreenName: matchingScreenName ?? _matchingScreenName, traceId: traceId ?? this.traceId, ); } From ebfb74b89085039627f9844d1496209aed411c6e Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Mon, 12 Aug 2024 13:49:23 +0300 Subject: [PATCH 10/12] test: add tests for masking with screen loading --- .../screen_loading_manager_test.dart | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/test/utils/screen_loading/screen_loading_manager_test.dart b/test/utils/screen_loading/screen_loading_manager_test.dart index 0e96dceb2..c008b8bcc 100644 --- a/test/utils/screen_loading/screen_loading_manager_test.dart +++ b/test/utils/screen_loading/screen_loading_manager_test.dart @@ -454,6 +454,92 @@ void main() { ), ).called(1); }); + + test( + '[startScreenLoadingTrace] should start screen loading trace when screen loading trace matches UI trace matching screen name', + () async { + const isSameScreen = true; + const matchingScreenName = 'matching_screen_name'; + mScreenLoadingManager = ScreenLoadingManagerNoResets.init(); + when(FlagsConfig.screenLoading.isEnabled()).thenAnswer((_) async => true); + when(IBGBuildInfo.I.isIOS).thenReturn(false); + when(mDateTime.now()).thenReturn(time); + + // Match on matching screen name + when( + RouteMatcher.I.match( + routePath: screenName, + actualPath: matchingScreenName, + ), + ).thenReturn(isSameScreen); + + when( + RouteMatcher.I.match( + routePath: screenName, + actualPath: screenName, + ), + ).thenReturn(!isSameScreen); + + ScreenLoadingManager.I.currentUiTrace = uiTrace.copyWith( + matchingScreenName: matchingScreenName, + ); + + await ScreenLoadingManager.I.startScreenLoadingTrace(screenLoadingTrace); + + final actualUiTrace = ScreenLoadingManager.I.currentUiTrace!; + + expect( + ScreenLoadingManager.I.currentScreenLoadingTrace, + equals(screenLoadingTrace), + ); + expect( + actualUiTrace.didStartScreenLoading, + isTrue, + ); + }); + + test( + '[startScreenLoadingTrace] should not start screen loading trace when screen loading trace does not matches UI trace matching screen name', + () async { + const isSameScreen = false; + const matchingScreenName = 'matching_screen_name'; + mScreenLoadingManager = ScreenLoadingManagerNoResets.init(); + when(FlagsConfig.screenLoading.isEnabled()).thenAnswer((_) async => true); + when(IBGBuildInfo.I.isIOS).thenReturn(false); + when(mDateTime.now()).thenReturn(time); + + // Don't match on matching screen name + when( + RouteMatcher.I.match( + routePath: screenName, + actualPath: matchingScreenName, + ), + ).thenReturn(isSameScreen); + + when( + RouteMatcher.I.match( + routePath: screenName, + actualPath: screenName, + ), + ).thenReturn(!isSameScreen); + + ScreenLoadingManager.I.currentUiTrace = uiTrace.copyWith( + matchingScreenName: matchingScreenName, + ); + + await ScreenLoadingManager.I.startScreenLoadingTrace(screenLoadingTrace); + + final actualUiTrace = ScreenLoadingManager.I.currentUiTrace!; + + expect( + ScreenLoadingManager.I.currentScreenLoadingTrace, + isNull, + ); + expect( + actualUiTrace.didStartScreenLoading, + isFalse, + ); + }); }); group('reportScreenLoading tests', () { From 47ba6a2941d5ac6849349b4a01b9b1512a461aa7 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Mon, 12 Aug 2024 14:35:43 +0300 Subject: [PATCH 11/12] chore: add changelog item --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0b3bf7bc..34e09b698 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased](https://github.com/Instabug/Instabug-Flutter/compare/v13.3.0...dev) + +### Added + +- Add support for masking screen names captured by Instabug through the `Instabug.setScreenNameMaskingCallback` API ([#500](https://github.com/Instabug/Instabug-Flutter/pull/500)). + ## [13.3.0](https://github.com/Instabug/Instabug-Flutter/compare/v13.2.0...v13.3.0) (August 5, 2024) ### Added From 1914942c07cca31eb08728b34eff699ce82169d6 Mon Sep 17 00:00:00 2001 From: Ahmed Mahmoud Date: Mon, 12 Aug 2024 16:50:47 +0300 Subject: [PATCH 12/12] test: add tests for trimming & fallback --- test/utils/screen_name_masker_test.dart | 37 +++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/utils/screen_name_masker_test.dart b/test/utils/screen_name_masker_test.dart index 24f71dd99..bfdd81c98 100644 --- a/test/utils/screen_name_masker_test.dart +++ b/test/utils/screen_name_masker_test.dart @@ -37,4 +37,41 @@ void main() { expect(result, equals(masked)); }); + + test('[mask] should fallback to "N/A" when callback returns an empty string', + () { + const fallback = 'N/A'; + const screen = '/documents/314159265'; + const masked = ''; + + ScreenNameMasker.I.setMaskingCallback((screen) { + if (screen.startsWith('/documents/')) { + return masked; + } + + return screen; + }); + + final result = ScreenNameMasker.I.mask(screen); + + expect(result, equals(fallback)); + }); + + test('[mask] should trim masked screen name', () { + const screen = '/documents/314159265'; + const masked = ' /documents/REDACTED '; + const expected = '/documents/REDACTED'; + + ScreenNameMasker.I.setMaskingCallback((screen) { + if (screen.startsWith('/documents/')) { + return masked; + } + + return screen; + }); + + final result = ScreenNameMasker.I.mask(screen); + + expect(result, equals(expected)); + }); }