Some clean up and refactoring for query parameter logic (flutter#8316)

Sarsilmaz02 · Sep 10, 2024 · a221e8b · a221e8b
1 parent f363ca0
commit a221e8b
Show file tree

Hide file tree

Showing 17 changed files with 135 additions and 74 deletions.
diff --git a/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart b/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart
@@ -14,6 +14,7 @@ import 'package:web/web.dart';
 import '../../shared/development_helpers.dart';
 import '../../shared/globals.dart';
 import '../../shared/primitives/utils.dart';
+import '../../shared/query_parameters.dart';
 import '../../shared/server/server.dart';
 import '../../shared/utils.dart';
 import 'controller.dart';
@@ -55,7 +56,7 @@ class EmbeddedExtensionControllerImpl extends EmbeddedExtensionController
       'index.html',
     );
     final queryParams = {
-      ...loadQueryParams(),
+      ...DevToolsQueryParams.load().params,
       ExtensionEventParameters.theme: isDarkThemeEnabled()
           ? ExtensionEventParameters.themeValueDark
           : ExtensionEventParameters.themeValueLight,

diff --git a/packages/devtools_app/lib/src/screens/app_size/app_size_screen.dart b/packages/devtools_app/lib/src/screens/app_size/app_size_screen.dart
@@ -6,7 +6,6 @@ import 'dart:async';
 
 import 'package:devtools_app_shared/ui.dart';
 import 'package:devtools_app_shared/utils.dart';
-import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter/material.dart';
 import 'package:vm_snapshot_analysis/precompiler_trace.dart';
 
@@ -18,6 +17,7 @@ import '../../shared/config_specific/drag_and_drop/drag_and_drop.dart';
 import '../../shared/file_import.dart';
 import '../../shared/globals.dart';
 import '../../shared/primitives/utils.dart';
+import '../../shared/query_parameters.dart';
 import '../../shared/screen.dart';
 import '../../shared/server/server.dart' as server;
 import '../../shared/ui/tab.dart';
@@ -103,14 +103,14 @@ class _AppSizeBodyState extends State<AppSizeBody>
   }
 
   Future<void> maybeLoadAppSizeFiles() async {
-    final queryParams = loadQueryParams();
-    final baseFilePath = queryParams[AppSizeApi.baseAppSizeFilePropertyName];
+    final queryParams = DevToolsQueryParams.load();
+    final baseFilePath = queryParams.appSizeBaseFilePath;
     if (baseFilePath != null) {
       // TODO(kenz): does this have to be in a setState()?
       _preLoadingData = true;
       final baseAppSizeFile = await server.requestBaseAppSizeFile(baseFilePath);
       DevToolsJsonFile? testAppSizeFile;
-      final testFilePath = queryParams[AppSizeApi.testAppSizeFilePropertyName];
+      final testFilePath = queryParams.appSizeTestFilePath;
       if (testFilePath != null) {
         testAppSizeFile = await server.requestTestAppSizeFile(testFilePath);
       }

diff --git a/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_controller.dart b/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_controller.dart
@@ -552,7 +552,7 @@ class DeepLinksController extends DisposableController
           false);
 
       if (linkdata.os.contains(PlatformOS.ios)) {
-        final List<String> iosPaths = iosDomainPaths[linkdata.domain] ?? [];
+        final iosPaths = iosDomainPaths[linkdata.domain] ?? <String>[];
 
         // If no path is provided, we will still show the domain just with domain errors.
         if (iosPaths.isEmpty) {

diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart
@@ -30,13 +30,12 @@ import '../../shared/diagnostics/inspector_service.dart';
 import '../../shared/diagnostics/primitives/instance_ref.dart';
 import '../../shared/globals.dart';
 import '../../shared/primitives/utils.dart';
+import '../../shared/query_parameters.dart';
 import 'inspector_screen.dart';
 import 'inspector_tree_controller.dart';
 
 final _log = Logger('inspector_controller');
 
-const inspectorRefQueryParam = 'inspectorRef';
-
 /// This class is based on the InspectorPanel class from the Flutter IntelliJ
 /// plugin with some refactors to make it more of a true controller than a view.
 class InspectorController extends DisposableController
@@ -415,11 +414,7 @@ class InspectorController extends DisposableController
       if (_disposed) return;
       // We need to start by querying the inspector service to find out the
       // current state of the UI.
-
-      final queryParams = loadQueryParams();
-      final inspectorRef = queryParams.containsKey(inspectorRefQueryParam)
-          ? queryParams[inspectorRefQueryParam]
-          : null;
+      final inspectorRef = DevToolsQueryParams.load().inspectorRef;
       await updateSelectionFromService(
         firstFrame: true,
         inspectorRef: inspectorRef,

diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart
@@ -30,14 +30,13 @@ import '../../shared/diagnostics/inspector_service.dart';
 import '../../shared/diagnostics/primitives/instance_ref.dart';
 import '../../shared/globals.dart';
 import '../../shared/primitives/utils.dart';
+import '../../shared/query_parameters.dart';
 import 'inspector_data_models.dart';
 import 'inspector_screen.dart';
 import 'inspector_tree_controller.dart';
 
 final _log = Logger('inspector_controller');
 
-const inspectorRefQueryParam = 'inspectorRef';
-
 /// Data pattern containing the properties and render properties for a widget
 /// tree node.
 typedef WidgetTreeNodeProperties = ({
@@ -401,11 +400,7 @@ class InspectorController extends DisposableController
       if (_disposed) return;
       // We need to start by querying the inspector service to find out the
       // current state of the UI.
-
-      final queryParams = loadQueryParams();
-      final inspectorRef = queryParams.containsKey(inspectorRefQueryParam)
-          ? queryParams[inspectorRefQueryParam]
-          : null;
+      final inspectorRef = DevToolsQueryParams.load().inspectorRef;
       await updateSelectionFromService(
         inspectorRef: inspectorRef,
       );

diff --git a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart
@@ -18,6 +18,7 @@ import 'package:web/web.dart';
 
 import '../dtd_manager_extensions.dart';
 import '../globals.dart';
+import '../query_parameters.dart';
 import '../server/server.dart' as server;
 import '../utils.dart';
 import 'analytics_common.dart';
@@ -767,12 +768,13 @@ void computeDevToolsCustomGTagsData() {
 void computeDevToolsQueryParams() {
   _ideLaunched = ideLaunchedCLI; // Default is Command Line launch.
 
-  final ideValue = ideFromUrl();
-  if (ideValue != null) {
-    _ideLaunched = ideValue;
+  final queryParams = DevToolsQueryParams.load();
+  final ide = queryParams.ide;
+  if (ide != null) {
+    _ideLaunched = ide;
   }
 
-  final ideFeature = lookupFromQueryParams('ideFeature');
+  final ideFeature = queryParams.ideFeature;
   if (ideFeature != null) {
     ideLaunchedFeature = ideFeature;
   }

diff --git a/packages/devtools_app/lib/src/shared/query_parameters.dart b/packages/devtools_app/lib/src/shared/query_parameters.dart
@@ -5,6 +5,7 @@
 import 'package:devtools_app_shared/shared.dart';
 import 'package:devtools_app_shared/ui.dart';
 import 'package:devtools_app_shared/utils.dart';
+import 'package:devtools_shared/devtools_shared.dart';
 
 extension type DevToolsQueryParams(Map<String, String?> params) {
   static DevToolsQueryParams empty() => DevToolsQueryParams({});
@@ -21,30 +22,54 @@ extension type DevToolsQueryParams(Map<String, String?> params) {
     return DevToolsQueryParams({...params, ...?updates});
   }
 
+  /// The URI for the VM Service that DevTools is connected to.
   String? get vmServiceUri => params[vmServiceUriKey];
 
-  EmbedMode get embedMode => ideThemeParams.embedMode;
-
+  /// The set of screens that are hidden based on the query parameters.
   Set<String> get hiddenScreens => {...?params[hideScreensKey]?.split(',')};
 
+  /// Whether DevTools extensions should be hidden.
   bool get hideExtensions => hiddenScreens.contains(hideExtensionsValue);
 
+  /// Whether all screens except DevTools extension screens should be hidden.
   bool get hideAllExceptExtensions =>
       hiddenScreens.contains(hideAllExceptExtensionsValue);
 
+  /// The screen that should be visible for viewing offline data.
   String? get offlineScreenId => params[offlineScreenIdKey];
 
+  /// The Inspector object reference that should be automatically selected when
+  /// opening the Flutter Inspector.
   String? get inspectorRef => params[inspectorRefKey];
 
-  // Keys for theming values that an IDE may pass in the embedded DevTools URI.
+  /// The file path for the base file to load on the App Size screen.
+  String? get appSizeBaseFilePath =>
+      params[AppSizeApi.baseAppSizeFilePropertyName];
+
+  /// The file path for the test file to load on the App Size screen.
+  String? get appSizeTestFilePath =>
+      params[AppSizeApi.testAppSizeFilePropertyName];
+
+  /// The IDE that DevTools is embedded in or was launched from.
+  String? get ide => params[ideKey];
+
+  /// The feature of the IDE that DevTools was opened from.
+  String? get ideFeature => params[ideFeatureKey];
+
+  /// Keys for theming values that an IDE may pass in the embedded DevTools URI.
   IdeThemeQueryParams get ideThemeParams => IdeThemeQueryParams(params);
 
+  /// The current [EmbedMode] of DevTools based on the query parameters.
+  EmbedMode get embedMode => ideThemeParams.embedMode;
+
   static const vmServiceUriKey = 'uri';
   static const hideScreensKey = 'hide';
   static const hideExtensionsValue = 'extensions';
   static const hideAllExceptExtensionsValue = 'all-except-extensions';
   static const offlineScreenIdKey = 'screen';
   static const inspectorRefKey = 'inspectorRef';
+  static const ideKey = 'ide';
+  static const ideFeatureKey = 'ideFeature';
 
   // TODO(kenz): remove legacy value in May of 2025 when all IDEs are not using
   // these and 12 months have passed to allow users ample upgrade time.

diff --git a/packages/devtools_app/lib/src/shared/utils.dart b/packages/devtools_app/lib/src/shared/utils.dart
@@ -26,6 +26,7 @@ import 'common_widgets.dart';
 import 'connected_app.dart';
 import 'globals.dart';
 import 'primitives/utils.dart';
+import 'query_parameters.dart';
 
 final _log = Logger('lib/src/shared/utils');
 
@@ -133,7 +134,7 @@ List<ConnectionDescription> generateDeviceDescription(
 
 /// This method should be public, because it is used by g3 specific code.
 List<String> issueLinkDetails() {
-  final ide = ideFromUrl();
+  final ide = DevToolsQueryParams.load().ide;
   final issueDescriptionItems = [
     '<-- Please describe your problem here. Be sure to include repro steps. -->',
     '___', // This will create a separator in the rendered markdown.
@@ -225,15 +226,6 @@ class ConnectionDescription {
   final List<Widget> actions;
 }
 
-String? ideFromUrl() {
-  return lookupFromQueryParams('ide');
-}
-
-String? lookupFromQueryParams(String key) {
-  final queryParameters = loadQueryParams();
-  return queryParameters[key];
-}
-
 const _google3PathSegment = 'google3';
 
 bool isGoogle3Path(List<String> pathParts) =>

diff --git a/packages/devtools_app/test/shared/query_parameters_test.dart b/packages/devtools_app/test/shared/query_parameters_test.dart
@@ -7,6 +7,8 @@ import 'dart:ui';
 import 'package:devtools_app/devtools_app.dart';
 import 'package:devtools_app/src/shared/query_parameters.dart';
 import 'package:devtools_app_shared/shared.dart';
+import 'package:devtools_app_shared/ui.dart';
+import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter_test/flutter_test.dart';
 
 void main() {
@@ -17,26 +19,36 @@ void main() {
         DevToolsQueryParams.hideScreensKey:
             'foo,bar,extensions,all-except-extensions',
         DevToolsQueryParams.offlineScreenIdKey: 'performance',
+        DevToolsQueryParams.inspectorRefKey: '12345',
+        AppSizeApi.baseAppSizeFilePropertyName: '/path/to/base/file.json',
+        AppSizeApi.testAppSizeFilePropertyName: '/path/to/test/file.json',
+        DevToolsQueryParams.ideKey: 'Android-Studio',
+        DevToolsQueryParams.ideFeatureKey: 'onDebugAutomatic',
         DevToolsQueryParams.legacyPageKey: 'memory',
         // IdeThemeQueryParams values
         'embedMode': 'one',
-        'backgroundColor': '#112233',
-        'foregroundColor': '#112244',
-        'fontSize': '8.0',
-        'theme': 'dark',
+        IdeThemeQueryParams.backgroundColorKey: '#112233',
+        IdeThemeQueryParams.foregroundColorKey: '#112244',
+        IdeThemeQueryParams.fontSizeKey: '8.0',
+        IdeThemeQueryParams.devToolsThemeKey: 'dark',
       });
 
       expect(params.vmServiceUri, 'some_uri');
-      expect(params.embedMode, EmbedMode.embedOne);
       expect(
         params.hiddenScreens,
         {'foo', 'bar', 'extensions', 'all-except-extensions'},
       );
       expect(params.hideExtensions, true);
       expect(params.hideAllExceptExtensions, true);
       expect(params.offlineScreenId, 'performance');
+      expect(params.inspectorRef, '12345');
+      expect(params.appSizeBaseFilePath, '/path/to/base/file.json');
+      expect(params.appSizeTestFilePath, '/path/to/test/file.json');
+      expect(params.ide, 'Android-Studio');
+      expect(params.ideFeature, 'onDebugAutomatic');
       expect(params.legacyPage, 'memory');
       expect(params.ideThemeParams.params, isNotEmpty);
+      expect(params.embedMode, EmbedMode.embedOne);
       expect(params.ideThemeParams.embedMode, EmbedMode.embedOne);
       expect(params.ideThemeParams.backgroundColor, const Color(0xFF112233));
       expect(params.ideThemeParams.foregroundColor, const Color(0xFF112244));
@@ -47,33 +59,50 @@ void main() {
     test('creates empty params', () {
       final params = DevToolsQueryParams.empty();
       expect(params.vmServiceUri, isNull);
-      expect(params.embedMode, EmbedMode.none);
       expect(params.hiddenScreens, isEmpty);
+      expect(params.hideExtensions, false);
+      expect(params.hideAllExceptExtensions, false);
       expect(params.offlineScreenId, isNull);
+      expect(params.inspectorRef, isNull);
+      expect(params.appSizeBaseFilePath, isNull);
+      expect(params.appSizeTestFilePath, isNull);
+      expect(params.ide, isNull);
+      expect(params.ideFeature, isNull);
       expect(params.legacyPage, isNull);
       expect(params.ideThemeParams.params, isEmpty);
+      expect(params.embedMode, EmbedMode.none);
     });
 
     test('creates params with updates', () {
       var params = DevToolsQueryParams({
         DevToolsQueryParams.vmServiceUriKey: 'some_uri',
         DevToolsQueryParams.hideScreensKey: 'foo,bar,baz',
         DevToolsQueryParams.offlineScreenIdKey: 'performance',
+        DevToolsQueryParams.inspectorRefKey: '12345',
+        AppSizeApi.baseAppSizeFilePropertyName: '/path/to/base/file.json',
+        AppSizeApi.testAppSizeFilePropertyName: '/path/to/test/file.json',
+        DevToolsQueryParams.ideKey: 'Android-Studio',
+        DevToolsQueryParams.ideFeatureKey: 'onDebugAutomatic',
         DevToolsQueryParams.legacyPageKey: 'memory',
         // IdeThemeQueryParams values
         'embedMode': 'one',
-        'backgroundColor': '#112233',
-        'foregroundColor': '#112244',
-        'fontSize': '8.0',
-        'theme': 'dark',
+        IdeThemeQueryParams.backgroundColorKey: '#112233',
+        IdeThemeQueryParams.foregroundColorKey: '#112244',
+        IdeThemeQueryParams.fontSizeKey: '8.0',
+        IdeThemeQueryParams.devToolsThemeKey: 'dark',
       });
 
       expect(params.vmServiceUri, 'some_uri');
-      expect(params.embedMode, EmbedMode.embedOne);
       expect(params.hiddenScreens, {'foo', 'bar', 'baz'});
       expect(params.offlineScreenId, 'performance');
+      expect(params.inspectorRef, '12345');
+      expect(params.appSizeBaseFilePath, '/path/to/base/file.json');
+      expect(params.appSizeTestFilePath, '/path/to/test/file.json');
+      expect(params.ide, 'Android-Studio');
+      expect(params.ideFeature, 'onDebugAutomatic');
       expect(params.legacyPage, 'memory');
       expect(params.ideThemeParams.params, isNotEmpty);
+      expect(params.embedMode, EmbedMode.embedOne);
       expect(params.ideThemeParams.embedMode, EmbedMode.embedOne);
       expect(params.ideThemeParams.backgroundColor, const Color(0xFF112233));
       expect(params.ideThemeParams.foregroundColor, const Color(0xFF112244));
@@ -83,18 +112,24 @@ void main() {
       params = params.withUpdates({
         DevToolsQueryParams.vmServiceUriKey: 'some_other_uri',
         DevToolsQueryParams.hideScreensKey: 'foo',
+        DevToolsQueryParams.inspectorRefKey: '23456',
         // Update some IdeThemeQueryParams values
         'embedMode': 'many',
         'fontSize': '10.0',
         'theme': 'light',
       });
 
       expect(params.vmServiceUri, 'some_other_uri');
-      expect(params.embedMode, EmbedMode.embedMany);
       expect(params.hiddenScreens, {'foo'});
       expect(params.offlineScreenId, 'performance');
+      expect(params.inspectorRef, '23456');
+      expect(params.appSizeBaseFilePath, '/path/to/base/file.json');
+      expect(params.appSizeTestFilePath, '/path/to/test/file.json');
+      expect(params.ide, 'Android-Studio');
+      expect(params.ideFeature, 'onDebugAutomatic');
       expect(params.legacyPage, 'memory');
       expect(params.ideThemeParams.params, isNotEmpty);
+      expect(params.embedMode, EmbedMode.embedMany);
       expect(params.ideThemeParams.embedMode, EmbedMode.embedMany);
       expect(params.ideThemeParams.backgroundColor, const Color(0xFF112233));
       expect(params.ideThemeParams.foregroundColor, const Color(0xFF112244));

diff --git a/packages/devtools_app_shared/CHANGELOG.md b/packages/devtools_app_shared/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 0.2.4
+* Add `updateQueryParameter` utility method.
+
 ## 0.2.3
 * Bump `web` dependency to `^1.0.0`
 * Bump `pointer_interceptor` dependency to `^0.10.1+1`

diff --git a/packages/devtools_app_shared/lib/src/utils/url/_url_stub.dart b/packages/devtools_app_shared/lib/src/utils/url/_url_stub.dart
@@ -15,3 +15,9 @@ String? getWebUrl() => null;
 // Unused parameter lint doesn't make sense for stub files.
 // ignore: avoid-unused-parameters
 void webRedirect(String url) {}
+
+/// Updates the query parameter with [key] to the new [value], and optionally
+/// reloads the page when [reload] is true.
+///
+/// No-op for non-web platforms.
+void updateQueryParameter(String key, String? value, {bool reload = false}) {}