diff --git a/CHANGELOG.md b/CHANGELOG.md index d0b3bf7b..f03a1e4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## [13.4.0](https://github.com/Instabug/Instabug-Flutter/compare/v13.3.0...v13.4.0) (September 15, 2024) + +### Added + +- Add support for masking screen names captured by Instabug through the `Instabug.setScreenNameMaskingCallback` API ([#500](https://github.com/Instabug/Instabug-Flutter/pull/500)). + +### Changed + +- Bump Instabug Android SDK to v13.4.1 ([#509](https://github.com/Instabug/Instabug-Flutter/pull/509)). [See release notes](https://github.com/Instabug/Instabug-Android/releases/tag/v13.4.0). +- Bump Instabug iOS SDK to v13.4.1 ([#510](https://github.com/Instabug/Instabug-Flutter/pull/510)). See release notes for [13.4.0](https://github.com/Instabug/Instabug-iOS/releases/tag/13.4.0) and [13.4.1](https://github.com/Instabug/Instabug-iOS/releases/tag/13.4.1). + +### Fixed + +- Fixed an issue with empty screen names captured in `InstabugNavigatorObserver` and fallback to `N/A` when the screen name is empty ([#505](https://github.com/Instabug/Instabug-Flutter/pull/505)), closes [#504](https://github.com/Instabug/Instabug-Flutter/issues/504). + ## [13.3.0](https://github.com/Instabug/Instabug-Flutter/compare/v13.2.0...v13.3.0) (August 5, 2024) ### Added diff --git a/android/build.gradle b/android/build.gradle index f16d516e..92654b91 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -1,5 +1,5 @@ group 'com.instabug.flutter' -version '13.3.0' +version '13.4.0' buildscript { repositories { @@ -41,7 +41,7 @@ android { } dependencies { - api 'com.instabug.library:instabug:13.3.0' + api 'com.instabug.library:instabug:13.4.1' testImplementation 'junit:junit:4.13.2' testImplementation "org.mockito:mockito-inline:3.12.1" diff --git a/example/ios/InstabugTests/InstabugApiTests.m b/example/ios/InstabugTests/InstabugApiTests.m index 886e22ec..3e83aec1 100644 --- a/example/ios/InstabugTests/InstabugApiTests.m +++ b/example/ios/InstabugTests/InstabugApiTests.m @@ -4,7 +4,7 @@ #import "InstabugApi.h" #import "Instabug/Instabug.h" #import "Util/Instabug+Test.h" -#import "Util/IBGNetworkLogger+Test.h" +#import "IBGNetworkLogger+CP.h" #import "Flutter/Flutter.h" @interface InstabugTests : XCTestCase @@ -435,7 +435,11 @@ - (void)testNetworkLog { duration:duration.integerValue gqlQueryName:nil serverErrorMessage:nil - ]); + isW3cCaughted:nil + partialID:nil + timestamp:nil + generatedW3CTraceparent:nil + caughtedW3CTraceparent:nil]); } - (void)testWillRedirectToAppStore { diff --git a/example/ios/Podfile.lock b/example/ios/Podfile.lock index 17a3f845..34cc30d6 100644 --- a/example/ios/Podfile.lock +++ b/example/ios/Podfile.lock @@ -1,9 +1,9 @@ PODS: - Flutter (1.0.0) - - Instabug (13.3.0) - - instabug_flutter (13.3.0): + - Instabug (13.4.1) + - instabug_flutter (13.4.0): - Flutter - - Instabug (= 13.3.0) + - Instabug (= 13.4.1) - OCMock (3.6) DEPENDENCIES: @@ -24,8 +24,8 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: Flutter: e0871f40cf51350855a761d2e70bf5af5b9b5de7 - Instabug: 4f26295103a330ec0236918359eef7ccaa74e2fa - instabug_flutter: 6be22be13b3dda72b293151d2c009952bb20b940 + Instabug: 896a318fb64b282832e0eda6fce489dac74b5d48 + instabug_flutter: b9f4503c3788c7346649bfe3396d6ca6e9711e96 OCMock: 5ea90566be239f179ba766fd9fbae5885040b992 PODFILE CHECKSUM: 8f7552fd115ace1988c3db54a69e4a123c448f84 diff --git a/example/ios/Runner.xcodeproj/project.pbxproj b/example/ios/Runner.xcodeproj/project.pbxproj index f40f40e7..858ba01e 100644 --- a/example/ios/Runner.xcodeproj/project.pbxproj +++ b/example/ios/Runner.xcodeproj/project.pbxproj @@ -87,7 +87,6 @@ CC359DB82937720C0067A924 /* ApmApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ApmApiTests.m; sourceTree = ""; }; CC3D69E6293F47FC000DCE54 /* ArgsRegistryTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ArgsRegistryTests.m; sourceTree = ""; }; CC78720E293CA8EE008CB2A5 /* Instabug+Test.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "Instabug+Test.h"; sourceTree = ""; }; - CC787211293CAB28008CB2A5 /* IBGNetworkLogger+Test.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "IBGNetworkLogger+Test.h"; sourceTree = ""; }; CC9925D1293DEB0B001FD3EE /* CrashReportingApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CrashReportingApiTests.m; sourceTree = ""; }; CC9925D4293DF534001FD3EE /* FeatureRequestsApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FeatureRequestsApiTests.m; sourceTree = ""; }; CC9925D6293DFB03001FD3EE /* InstabugLogApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = InstabugLogApiTests.m; sourceTree = ""; }; @@ -216,7 +215,6 @@ children = ( BE26C80C2BD55575009FECCF /* IBGCrashReporting+CP.h */, CC78720E293CA8EE008CB2A5 /* Instabug+Test.h */, - CC787211293CAB28008CB2A5 /* IBGNetworkLogger+Test.h */, CC198C62293E2392007077C8 /* IBGSurvey+Test.h */, ); path = Util; diff --git a/example/lib/main.dart b/example/lib/main.dart index 7749ed02..cda7ff3e 100644 --- a/example/lib/main.dart +++ b/example/lib/main.dart @@ -2,10 +2,10 @@ import 'dart:async'; import 'dart:developer'; import 'dart:io'; import 'dart:convert'; -import 'package:http/http.dart' as http; import 'package:flutter/material.dart'; import 'package:instabug_flutter/instabug_flutter.dart'; +import 'package:instabug_http_client/instabug_http_client.dart'; import 'package:instabug_flutter_example/src/app_routes.dart'; import 'package:instabug_flutter_example/src/widget/nested_view.dart'; diff --git a/example/lib/src/components/network_content.dart b/example/lib/src/components/network_content.dart index c364ff89..f55af5c9 100644 --- a/example/lib/src/components/network_content.dart +++ b/example/lib/src/components/network_content.dart @@ -10,6 +10,8 @@ class NetworkContent extends StatefulWidget { } class _NetworkContentState extends State { + final http = InstabugHttpClient(); + final endpointUrlController = TextEditingController(); @override diff --git a/example/pubspec.lock b/example/pubspec.lock index 5b56a50f..31cb6f5d 100644 --- a/example/pubspec.lock +++ b/example/pubspec.lock @@ -115,7 +115,15 @@ packages: path: ".." relative: true source: path - version: "13.3.0" + version: "13.4.0" + instabug_http_client: + dependency: "direct main" + description: + name: instabug_http_client + sha256: "7d52803c0dd639f6dddbe07333418eb251ae02f3f9f4d30402517533ca692784" + url: "https://pub.dev" + source: hosted + version: "2.4.0" leak_tracker: dependency: transitive description: diff --git a/example/pubspec.yaml b/example/pubspec.yaml index 69fed929..7f3e9e62 100644 --- a/example/pubspec.yaml +++ b/example/pubspec.yaml @@ -26,6 +26,7 @@ dependencies: http: ^0.13.0 instabug_flutter: path: ../ + instabug_http_client: ^2.4.0 dev_dependencies: espresso: 0.2.0+5 @@ -35,6 +36,10 @@ dev_dependencies: sdk: flutter flutter_lints: 1.0.4 +dependency_overrides: + instabug_flutter: + path: ../ + # For information on the generic Dart part of this file, see the # following page: https://dart.dev/tools/pub/pubspec diff --git a/ios/Classes/Modules/InstabugApi.m b/ios/Classes/Modules/InstabugApi.m index 44f35668..11ea0935 100644 --- a/ios/Classes/Modules/InstabugApi.m +++ b/ios/Classes/Modules/InstabugApi.m @@ -263,10 +263,10 @@ - (void)networkLogData:(NSDictionary *)data error:(FlutterError NSString *method = data[@"method"]; NSString *requestBody = data[@"requestBody"]; NSString *responseBody = data[@"responseBody"]; - int32_t responseCode = [data[@"responseCode"] integerValue]; + int32_t responseCode = (int32_t) [data[@"responseCode"] integerValue]; int64_t requestBodySize = [data[@"requestBodySize"] integerValue]; int64_t responseBodySize = [data[@"responseBodySize"] integerValue]; - int32_t errorCode = [data[@"errorCode"] integerValue]; + int32_t errorCode = (int32_t) [data[@"errorCode"] integerValue]; NSString *errorDomain = data[@"errorDomain"]; NSDictionary *requestHeaders = data[@"requestHeaders"]; if ([requestHeaders count] == 0) { @@ -286,32 +286,27 @@ - (void)networkLogData:(NSDictionary *)data error:(FlutterError serverErrorMessage = data[@"serverErrorMessage"]; } - SEL networkLogSEL = NSSelectorFromString(@"addNetworkLogWithUrl:method:requestBody:requestBodySize:responseBody:responseBodySize:responseCode:requestHeaders:responseHeaders:contentType:errorDomain:errorCode:startTime:duration:gqlQueryName:serverErrorMessage:"); - - if ([[IBGNetworkLogger class] respondsToSelector:networkLogSEL]) { - NSInvocation *inv = [NSInvocation invocationWithMethodSignature:[[IBGNetworkLogger class] methodSignatureForSelector:networkLogSEL]]; - [inv setSelector:networkLogSEL]; - [inv setTarget:[IBGNetworkLogger class]]; - - [inv setArgument:&(url) atIndex:2]; - [inv setArgument:&(method) atIndex:3]; - [inv setArgument:&(requestBody) atIndex:4]; - [inv setArgument:&(requestBodySize) atIndex:5]; - [inv setArgument:&(responseBody) atIndex:6]; - [inv setArgument:&(responseBodySize) atIndex:7]; - [inv setArgument:&(responseCode) atIndex:8]; - [inv setArgument:&(requestHeaders) atIndex:9]; - [inv setArgument:&(responseHeaders) atIndex:10]; - [inv setArgument:&(contentType) atIndex:11]; - [inv setArgument:&(errorDomain) atIndex:12]; - [inv setArgument:&(errorCode) atIndex:13]; - [inv setArgument:&(startTime) atIndex:14]; - [inv setArgument:&(duration) atIndex:15]; - [inv setArgument:&(gqlQueryName) atIndex:16]; - [inv setArgument:&(serverErrorMessage) atIndex:17]; - - [inv invoke]; - } + [IBGNetworkLogger addNetworkLogWithUrl:url + method:method + requestBody:requestBody + requestBodySize:requestBodySize + responseBody:responseBody + responseBodySize:responseBodySize + responseCode:responseCode + requestHeaders:requestHeaders + responseHeaders:responseHeaders + contentType:contentType + errorDomain:errorDomain + errorCode:errorCode + startTime:startTime + duration:duration + gqlQueryName:gqlQueryName + serverErrorMessage:serverErrorMessage + isW3cCaughted:nil + partialID:nil + timestamp:nil + generatedW3CTraceparent:nil + caughtedW3CTraceparent:nil]; } - (void)willRedirectToStoreWithError:(FlutterError * _Nullable __autoreleasing *)error { diff --git a/ios/Classes/Util/IBGNetworkLogger+CP.h b/ios/Classes/Util/IBGNetworkLogger+CP.h index ae5d32d6..764524fb 100644 --- a/ios/Classes/Util/IBGNetworkLogger+CP.h +++ b/ios/Classes/Util/IBGNetworkLogger+CP.h @@ -6,6 +6,28 @@ NS_ASSUME_NONNULL_BEGIN + (void)disableAutomaticCapturingOfNetworkLogs; ++ (void)addNetworkLogWithUrl:(NSString *_Nonnull)url + method:(NSString *_Nonnull)method + requestBody:(NSString *_Nonnull)request + requestBodySize:(int64_t)requestBodySize + responseBody:(NSString *_Nonnull)response + responseBodySize:(int64_t)responseBodySize + responseCode:(int32_t)code + requestHeaders:(NSDictionary *_Nonnull)requestHeaders + responseHeaders:(NSDictionary *_Nonnull)responseHeaders + contentType:(NSString *_Nonnull)contentType + errorDomain:(NSString *_Nullable)errorDomain + errorCode:(int32_t)errorCode + startTime:(int64_t)startTime + duration:(int64_t) duration + gqlQueryName:(NSString * _Nullable)gqlQueryName + serverErrorMessage:(NSString * _Nullable)serverErrorMessage + isW3cCaughted:(NSNumber * _Nullable)isW3cCaughted + partialID:(NSNumber * _Nullable)partialID + timestamp:(NSNumber * _Nullable)timestamp + generatedW3CTraceparent:(NSString * _Nullable)generatedW3CTraceparent + caughtedW3CTraceparent:(NSString * _Nullable)caughtedW3CTraceparent; + @end NS_ASSUME_NONNULL_END diff --git a/ios/instabug_flutter.podspec b/ios/instabug_flutter.podspec index 31f7d878..e5755904 100644 --- a/ios/instabug_flutter.podspec +++ b/ios/instabug_flutter.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'instabug_flutter' - s.version = '13.3.0' + s.version = '13.4.0' s.summary = 'Flutter plugin for integrating the Instabug SDK.' s.author = 'Instabug' s.homepage = 'https://www.instabug.com/platforms/flutter' @@ -17,6 +17,6 @@ Pod::Spec.new do |s| s.pod_target_xcconfig = { 'OTHER_LDFLAGS' => '-framework "Flutter" -framework "Instabug"'} s.dependency 'Flutter' - s.dependency 'Instabug', '13.3.0' + s.dependency 'Instabug', '13.4.1' end diff --git a/lib/instabug_flutter.dart b/lib/instabug_flutter.dart index 783fb135..6dc949d1 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/models/instabug_route.dart b/lib/src/models/instabug_route.dart new file mode 100644 index 00000000..8c153942 --- /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/modules/instabug.dart b/lib/src/modules/instabug.dart index 9d578e03..bf457a4f 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/instabug_navigator_observer.dart b/lib/src/utils/instabug_navigator_observer.dart index 3cb39cc0..d9d6b02d 100644 --- a/lib/src/utils/instabug_navigator_observer.dart +++ b/lib/src/utils/instabug_navigator_observer.dart @@ -1,32 +1,44 @@ 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/repro_steps_constants.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 rawScreenName = newRoute.settings.name.toString().trim(); + final screenName = rawScreenName.isEmpty + ? ReproStepsConstants.emptyScreenFallback + : rawScreenName; + final maskedScreenName = ScreenNameMasker.I.mask(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, 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 - 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) { diff --git a/lib/src/utils/repro_steps_constants.dart b/lib/src/utils/repro_steps_constants.dart new file mode 100644 index 00000000..c26dc205 --- /dev/null +++ b/lib/src/utils/repro_steps_constants.dart @@ -0,0 +1,3 @@ +class ReproStepsConstants { + static const emptyScreenFallback = 'N/A'; +} diff --git a/lib/src/utils/screen_loading/screen_loading_manager.dart b/lib/src/utils/screen_loading/screen_loading_manager.dart index 2af57f09..816ffebe 100644 --- a/lib/src/utils/screen_loading/screen_loading_manager.dart +++ b/lib/src/utils/screen_loading/screen_loading_manager.dart @@ -110,24 +110,31 @@ class ScreenLoadingManager { @internal String sanitizeScreenName(String screenName) { const characterToBeRemoved = '/'; - final lastIndex = screenName.length - 1; var sanitizedScreenName = screenName; if (screenName == characterToBeRemoved) { return 'ROOT_PAGE'; } - if (screenName[0] == characterToBeRemoved) { + if (screenName.startsWith(characterToBeRemoved)) { sanitizedScreenName = sanitizedScreenName.substring(1); } - if (screenName[lastIndex] == characterToBeRemoved) { + if (screenName.endsWith(characterToBeRemoved)) { sanitizedScreenName = sanitizedScreenName.substring(0, sanitizedScreenName.length - 1); } 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 +158,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 +199,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 7fd03c9f..17ef4104 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: matchingScreenName ?? _matchingScreenName, 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/lib/src/utils/screen_name_masker.dart b/lib/src/utils/screen_name_masker.dart new file mode 100644 index 00000000..1c2c4a97 --- /dev/null +++ b/lib/src/utils/screen_name_masker.dart @@ -0,0 +1,44 @@ +import 'package:flutter/material.dart'; +import 'package:instabug_flutter/src/utils/repro_steps_constants.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; + + 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 mask(String screen) { + if (_screenNameMaskingCallback == null) { + return screen; + } + + final maskedScreen = _screenNameMaskingCallback!(screen).trim(); + + if (maskedScreen.isEmpty) { + return ReproStepsConstants.emptyScreenFallback; + } + + return maskedScreen; + } +} diff --git a/pubspec.yaml b/pubspec.yaml index 76c665ab..ba6dd0be 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: instabug_flutter -version: 13.3.0 +version: 13.4.0 description: >- Instabug empowers mobile teams to monitor, prioritize, and debug performance and stability issues throughout the app development lifecycle. @@ -15,6 +15,7 @@ dependencies: dev_dependencies: build_runner: ^2.0.3 + fake_async: '>=1.2.0 <1.4.0' flutter_test: sdk: flutter lint: ^1.0.0 diff --git a/test/instabug_test.dart b/test/instabug_test.dart index f525dc02..0e6f0f42 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(); diff --git a/test/utils/instabug_navigator_observer_test.dart b/test/utils/instabug_navigator_observer_test.dart new file mode 100644 index 00000000..ebf54113 --- /dev/null +++ b/test/utils/instabug_navigator_observer_test.dart @@ -0,0 +1,140 @@ +import 'package:fake_async/fake_async.dart'; +import 'package:flutter/material.dart'; +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'; + +import 'instabug_navigator_observer_test.mocks.dart'; + +@GenerateMocks([ + InstabugHostApi, + ScreenLoadingManager, +]) +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + WidgetsFlutterBinding.ensureInitialized(); + + final mHost = MockInstabugHostApi(); + final mScreenLoadingManager = MockScreenLoadingManager(); + + late InstabugNavigatorObserver observer; + const screen = '/screen'; + const previousScreen = '/previousScreen'; + late Route route; + late Route previousRoute; + + setUpAll(() { + Instabug.$setHostApi(mHost); + ScreenLoadingManager.setInstance(mScreenLoadingManager); + }); + + setUp(() { + observer = InstabugNavigatorObserver(); + route = createRoute(screen); + previousRoute = createRoute(previousScreen); + + ScreenNameMasker.I.setMaskingCallback(null); + }); + + test('should report screen change when a route is pushed', () { + fakeAsync((async) { + observer.didPush(route, previousRoute); + + async.elapse(const Duration(milliseconds: 1000)); + + verify( + mScreenLoadingManager.startUiTrace(screen, screen), + ).called(1); + + verify( + mHost.reportScreenChange(screen), + ).called(1); + }); + }); + + test( + 'should report screen change when a route is popped and previous is known', + () { + fakeAsync((async) { + observer.didPop(route, previousRoute); + + async.elapse(const Duration(milliseconds: 1000)); + + verify( + mScreenLoadingManager.startUiTrace(previousScreen, previousScreen), + ).called(1); + + verify( + mHost.reportScreenChange(previousScreen), + ).called(1); + }); + }); + + test( + 'should not report screen change when a route is popped and previous is not known', + () { + fakeAsync((async) { + observer.didPop(route, null); + + async.elapse(const Duration(milliseconds: 1000)); + + verifyNever( + mScreenLoadingManager.startUiTrace(any, any), + ); + + verifyNever( + mHost.reportScreenChange(any), + ); + }); + }); + + test('should fallback to "N/A" when the screen name is empty', () { + fakeAsync((async) { + final route = createRoute(''); + const fallback = 'N/A'; + + observer.didPush(route, previousRoute); + + async.elapse(const Duration(milliseconds: 1000)); + + verify( + mScreenLoadingManager.startUiTrace(fallback, fallback), + ).called(1); + + verify( + mHost.reportScreenChange(fallback), + ).called(1); + }); + }); + + 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) { + return MaterialPageRoute( + builder: (_) => Container(), + settings: RouteSettings(name: name), + ); +} diff --git a/test/utils/screen_loading/screen_loading_manager_test.dart b/test/utils/screen_loading/screen_loading_manager_test.dart index 14e33c66..c008b8bc 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); @@ -230,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', () { @@ -241,7 +286,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); @@ -409,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', () { @@ -421,7 +552,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 +850,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)); diff --git a/test/utils/screen_name_masker_test.dart b/test/utils/screen_name_masker_test.dart new file mode 100644 index 00000000..bfdd81c9 --- /dev/null +++ b/test/utils/screen_name_masker_test.dart @@ -0,0 +1,77 @@ +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)); + }); + + 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)); + }); +}