From 77e3c0bd9065b40bb013678bff2262f44aef26af Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Thu, 27 Jul 2023 14:16:46 -0700 Subject: [PATCH 1/6] Add extensionActivationState endpoint to the devtools server --- .../config_specific/server/_server_stub.dart | 2 +- .../config_specific/server/_server_web.dart | 14 +- packages/devtools_extensions/.metadata | 10 - .../devtools_shared/lib/src/devtools_api.dart | 50 +++-- .../src/extensions/extension_activation.dart | 173 ++++++++++++++++++ .../lib/src/extensions/extension_model.dart | 24 +++ .../lib/src/server/server_api.dart | 54 +++++- .../devtools_shared/lib/src/server/usage.dart | 2 +- packages/devtools_shared/pubspec.yaml | 3 + .../extensions/extension_activation_test.dart | 110 +++++++++++ 10 files changed, 402 insertions(+), 40 deletions(-) delete mode 100644 packages/devtools_extensions/.metadata create mode 100644 packages/devtools_shared/lib/src/extensions/extension_activation.dart create mode 100644 packages/devtools_shared/test/extensions/extension_activation_test.dart diff --git a/packages/devtools_app/lib/src/shared/config_specific/server/_server_stub.dart b/packages/devtools_app/lib/src/shared/config_specific/server/_server_stub.dart index a2737661bbc..4676d4934cd 100644 --- a/packages/devtools_app/lib/src/shared/config_specific/server/_server_stub.dart +++ b/packages/devtools_app/lib/src/shared/config_specific/server/_server_stub.dart @@ -78,7 +78,7 @@ Future requestTestAppSizeFile(String path) async { } Future> refreshAvailableExtensions( - String? rootPath, + String rootPath, ) async { return []; } diff --git a/packages/devtools_app/lib/src/shared/config_specific/server/_server_web.dart b/packages/devtools_app/lib/src/shared/config_specific/server/_server_web.dart index 50abddfbc5c..d1bebe841d1 100644 --- a/packages/devtools_app/lib/src/shared/config_specific/server/_server_web.dart +++ b/packages/devtools_app/lib/src/shared/config_specific/server/_server_web.dart @@ -339,26 +339,30 @@ DevToolsJsonFile _devToolsJsonFileFromResponse( ); } +/// Makes a request to the server to refresh the list of available extensions, +/// serve their assets on the server, and return the list of available +/// extensions here. Future> refreshAvailableExtensions( - String? rootPath, + String rootPath, ) async { if (isDevToolsServerAvailable) { final uri = Uri( - path: apiServeAvailableExtensions, - queryParameters: {extensionRootPathPropertyName: rootPath}, + path: ExtensionsApi.apiServeAvailableExtensions, + queryParameters: {ExtensionsApi.extensionRootPathPropertyName: rootPath}, ); final resp = await request(uri.toString()); if (resp?.status == HttpStatus.ok) { final parsedResult = json.decode(resp!.responseText!); final extensionsAsJson = - (parsedResult[extensionsResultPropertyName]! as List) + (parsedResult[ExtensionsApi.extensionsResultPropertyName]! + as List) .whereNotNull() .cast>(); return extensionsAsJson .map((p) => DevToolsExtensionConfig.parse(p)) .toList(); } else { - logWarning(resp, apiServeAvailableExtensions); + logWarning(resp, ExtensionsApi.apiServeAvailableExtensions); return []; } } diff --git a/packages/devtools_extensions/.metadata b/packages/devtools_extensions/.metadata deleted file mode 100644 index 316d5923f21..00000000000 --- a/packages/devtools_extensions/.metadata +++ /dev/null @@ -1,10 +0,0 @@ -# This file tracks properties of this Flutter project. -# Used by Flutter tool to assess capabilities and perform upgrades etc. -# -# This file should be version controlled and should not be manually edited. - -version: - revision: "f842ed916514879fe6898b2a5a4053c63c3308fe" - channel: "[user-branch]" - -project_type: package diff --git a/packages/devtools_shared/lib/src/devtools_api.dart b/packages/devtools_shared/lib/src/devtools_api.dart index aeb2ee66719..13314bb6c42 100644 --- a/packages/devtools_shared/lib/src/devtools_api.dart +++ b/packages/devtools_shared/lib/src/devtools_api.dart @@ -41,18 +41,15 @@ const surveyActionTakenPropertyName = 'surveyActionTaken'; const apiGetSurveyShownCount = '${apiPrefix}getSurveyShownCount'; /// Increments the surveyShownCount of the of the activeSurvey (apiSetActiveSurvey). -const apiIncrementSurveyShownCount = - '${apiPrefix}incrementSurveyShownCount'; +const apiIncrementSurveyShownCount = '${apiPrefix}incrementSurveyShownCount'; const lastReleaseNotesVersionPropertyName = 'lastReleaseNotesVersion'; /// Returns the last DevTools version for which we have shown release notes. -const apiGetLastReleaseNotesVersion = - '${apiPrefix}getLastReleaseNotesVersion'; +const apiGetLastReleaseNotesVersion = '${apiPrefix}getLastReleaseNotesVersion'; /// Sets the last DevTools version for which we have shown release notes. -const apiSetLastReleaseNotesVersion = - '${apiPrefix}setLastReleaseNotesVersion'; +const apiSetLastReleaseNotesVersion = '${apiPrefix}setLastReleaseNotesVersion'; /// Returns the base app size file, if present. const apiGetBaseAppSizeFile = '${apiPrefix}getBaseAppSizeFile'; @@ -65,15 +62,32 @@ const baseAppSizeFilePropertyName = 'appSizeBase'; const testAppSizeFilePropertyName = 'appSizeTest'; -/// Serves any available extensions and returns a list of their configurations -/// to DevTools. -const apiServeAvailableExtensions = - '${apiPrefix}serveAvailableExtensions'; - -/// The property name for the query parameter passed along with -/// [apiServeAvailableExtensions] requests to the server. -const extensionRootPathPropertyName = 'rootPath'; - -/// The property name for the response that the server sends back upon -/// receiving a [apiServeAvailableExtensions] request. -const extensionsResultPropertyName = 'extensions'; +abstract class ExtensionsApi { + /// Serves any available extensions and returns a list of their configurations + /// to DevTools. + static const apiServeAvailableExtensions = + '${apiPrefix}serveAvailableExtensions'; + + /// The property name for the query parameter passed along with + /// extension-related requests to the server that describes the package root + /// for the app whose extensions are being queried. + static const extensionRootPathPropertyName = 'rootPath'; + + /// The property name for the response that the server sends back upon + /// receiving a [apiServeAvailableExtensions] request. + static const extensionsResultPropertyName = 'extensions'; + + /// Returns and optionally sets the activation state for a DevTools extension. + static const apiExtensionActivationState = + '${apiPrefix}extensionActivationState'; + + /// The property name for the query parameter passed along with + /// [apiExtensionActivationState] requests to the server that describes the + /// name of the extension whose state is being queried. + static const extensionNamePropertyName = 'name'; + + /// The property name for the query parameter that is optionally passed along + /// with [apiExtensionActivationState] requests to the server to set the + /// activation state for the extension. + static const activationStatePropertyName = 'activate'; +} diff --git a/packages/devtools_shared/lib/src/extensions/extension_activation.dart b/packages/devtools_shared/lib/src/extensions/extension_activation.dart new file mode 100644 index 00000000000..f6be3d44334 --- /dev/null +++ b/packages/devtools_shared/lib/src/extensions/extension_activation.dart @@ -0,0 +1,173 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io'; + +import 'package:collection/collection.dart'; +import 'package:path/path.dart' as path; +import 'package:yaml/yaml.dart'; +import 'package:yaml_edit/yaml_edit.dart'; + +import 'extension_model.dart'; + +/// Manages the `devtools_options.yaml` file and allows read / write access. +class DevToolsOptions { + static const optionsFileName = 'devtools_options.yaml'; + + static const _extensionsKey = 'extensions'; + + static const _defaultOptions = ''' +$_extensionsKey: +'''; + + /// Returns the current activation state for [extensionName] in the + /// 'devtools_options.yaml' file at [rootUri]. + /// + /// If the 'devtools_options.yaml' file does not exist, it will be created + /// with an empty set of extensions. + ExtensionActivationState lookupExtensionActivationState({ + required Uri rootUri, + required String extensionName, + }) { + final options = _optionsAsMap(rootUri: rootUri); + if (options == null) return ExtensionActivationState.err; + + final extensions = + (options[_extensionsKey] as List?)?.cast>(); + if (extensions == null) return ExtensionActivationState.none; + + for (final e in extensions) { + // Each entry should only have one key / value pair (e.g. '- foo: true'). + assert(e.keys.length == 1); + + if (e.keys.first == extensionName) { + return _extensionStateForValue(e[extensionName]); + } + } + return ExtensionActivationState.none; + } + + /// Sets the activation state for [extensionName] in the + /// 'devtools_options.yaml' file at [rootUri]. + /// + /// If the 'devtools_options.yaml' file does not exist, it will be created. + ExtensionActivationState setExtensionActivationState({ + required Uri rootUri, + required String extensionName, + required bool activate, + }) { + final options = _optionsAsMap(rootUri: rootUri); + if (options == null) return ExtensionActivationState.err; + + var extensions = + (options[_extensionsKey] as List?)?.cast>(); + if (extensions == null) { + options[_extensionsKey] = >[]; + extensions = options[_extensionsKey] as List>; + } + + // Write the new activation state to the map. + final extension = extensions.firstWhereOrNull( + (e) => e.keys.first == extensionName, + )?[extensionName] = activate; + if (extension == null) { + extensions.add({extensionName: activate}); + } + + _writeToOptionsFile(rootUri: rootUri, options: options); + + // Lookup the activation state from the file we just wrote to to ensure that + // are not returning an out of sync result. + return lookupExtensionActivationState( + rootUri: rootUri, + extensionName: extensionName, + ); + } + + /// Returns the content of the `devtools_options.yaml` file at [rootUri] as a + /// Map. + Map? _optionsAsMap({required Uri rootUri}) { + final optionsFile = _lookupOptionsFile(rootUri); + if (optionsFile == null) return null; + final yamlMap = loadYaml(optionsFile.readAsStringSync()) as YamlMap; + return yamlMap.toDartMap(); + } + + /// Writes the `devtools_options.yaml` file at [rootUri] with the value of + /// [options] as YAML. + /// + /// Any existing content in `devtools_options.yaml` will be overwritten. + void _writeToOptionsFile({ + required Uri rootUri, + required Map options, + }) { + final yamlEditor = YamlEditor(''); + yamlEditor.update([], options); + _lookupOptionsFile(rootUri)?.writeAsStringSync(yamlEditor.toString()); + } + + /// Returns the `devtools_options.yaml` file in the [rootUri] directory. + /// + /// Returns null if the directory at [rootUri] does not exist. Otherwise, if + /// the `devtools_options.yaml` does not already exist, it will be created + /// and written with [_defaultOptions], and then returned. + File? _lookupOptionsFile(Uri rootUri) { + final rootDir = Directory.fromUri(rootUri); + if (!rootDir.existsSync()) { + print('Directory does not exist at path: ${rootUri.toString()}'); + return null; + } + + final optionsFile = File(path.join(rootDir.path, optionsFileName)); + if (!optionsFile.existsSync()) { + print('creating options file: ${optionsFile.path}'); + optionsFile + ..createSync() + ..writeAsStringSync(_defaultOptions); + } + return optionsFile; + } + + ExtensionActivationState _extensionStateForValue(Object? value) { + switch (value) { + case true: + return ExtensionActivationState.enabled; + case false: + return ExtensionActivationState.disabled; + default: + return ExtensionActivationState.none; + } + } +} + +extension YamlExtension on YamlMap { + Map toDartMap() { + final map = {}; + for (final entry in nodes.entries) { + map[entry.key.toString()] = entry.value.convertToDartType(); + } + return map; + } +} + +extension YamlListExtension on YamlList { + List toDartList() { + final list = []; + for (final e in nodes) { + final element = e.convertToDartType(); + if (element != null) list.add(element); + } + return list; + } +} + +extension YamlNodeExtension on YamlNode { + Object? convertToDartType() { + return switch (this) { + YamlMap() => (this as YamlMap).toDartMap(), + YamlList() => (this as YamlList).toDartList(), + _ => value, + }; + } +} diff --git a/packages/devtools_shared/lib/src/extensions/extension_model.dart b/packages/devtools_shared/lib/src/extensions/extension_model.dart index dd95b3e841c..7dc7bea9166 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_model.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_model.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:collection/collection.dart'; + /// Describes an extension that can be dynamically loaded into a custom screen /// in DevTools. class DevToolsExtensionConfig { @@ -109,3 +111,25 @@ class DevToolsExtensionConfig { materialIconCodePointKey: materialIconCodePoint, }; } + +enum ExtensionActivationState { + /// The extension has been enabled manually by the user. + enabled, + + /// The extension has been disabled manually by the user. + disabled, + + /// The extension has been neither enabled nor disabled by the user. + none, + + /// Something went wrong with reading or writing the activation state. + /// + /// We should ignore extensions with this activation state. + err; + + static ExtensionActivationState from(String? value) { + return ExtensionActivationState.values + .firstWhereOrNull((e) => e.name == value) ?? + ExtensionActivationState.none; + } +} diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index ef43e809bc5..4f88d95fef1 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -9,6 +9,7 @@ import 'dart:io'; import 'package:shelf/shelf.dart' as shelf; import '../devtools_api.dart'; +import '../extensions/extension_activation.dart'; import '../extensions/extension_manager.dart'; import 'file_system.dart'; import 'usage.dart'; @@ -207,12 +208,14 @@ class ServerApi { // ----- Extensions api. ----- - case apiServeAvailableExtensions: + case ExtensionsApi.apiServeAvailableExtensions: final queryParams = request.requestedUri.queryParameters; - final rootPath = queryParams[extensionRootPathPropertyName]; + final rootPath = + queryParams[ExtensionsApi.extensionRootPathPropertyName]; if (rootPath == null) { return api.badRequest( - '$extensionRootPathPropertyName query parameter required', + '${ExtensionsApi.extensionRootPathPropertyName} query parameter ' + 'required', ); } @@ -220,10 +223,47 @@ class ServerApi { final extensions = extensionsManager.devtoolsExtensions .map((p) => p.toJson()) .toList(); - final result = jsonEncode({extensionsResultPropertyName: extensions}); + final result = jsonEncode({ + ExtensionsApi.extensionsResultPropertyName: extensions, + }); return api!.getCompleted(result); }); + case ExtensionsApi.apiExtensionActivationState: + final queryParams = request.requestedUri.queryParameters; + final rootPath = + queryParams[ExtensionsApi.extensionRootPathPropertyName]; + if (rootPath == null) { + return api.badRequest( + '${ExtensionsApi.extensionRootPathPropertyName} query parameter ' + 'required', + ); + } + final rootUri = Uri.parse(rootPath); + + final extensionName = + queryParams[ExtensionsApi.extensionNamePropertyName]; + if (extensionName == null) { + return api.badRequest( + '${ExtensionsApi.extensionNamePropertyName} query parameter required', + ); + } + + final activate = queryParams[ExtensionsApi.activationStatePropertyName]; + if (activate != null) { + final newState = _devToolsOptions.setExtensionActivationState( + rootUri: rootUri, + extensionName: extensionName, + activate: bool.parse(activate), + ); + return api.getCompleted(json.encode(newState.name)); + } + final activationState = _devToolsOptions.lookupExtensionActivationState( + rootUri: rootUri, + extensionName: extensionName, + ); + return api.getCompleted(json.encode(activationState.name)); + default: return api.notImplemented(); } @@ -236,10 +276,14 @@ class ServerApi { FlutterUsage.doesStoreExist ? FlutterUsage() : null; // Accessing DevTools usage file e.g., ~/.flutter-devtools/.devtools - static final DevToolsUsage _devToolsUsage = DevToolsUsage(); + static final _devToolsUsage = DevToolsUsage(); static DevToolsUsage get devToolsPreferences => _devToolsUsage; + /// Provides read and write access to DevTools options files + /// (e.g. path/to/app/root/devtools_options.yaml). + static final _devToolsOptions = DevToolsOptions(); + /// Logs a page view in the DevTools server. /// /// In the open-source version of DevTools, Google Analytics handles this diff --git a/packages/devtools_shared/lib/src/server/usage.dart b/packages/devtools_shared/lib/src/server/usage.dart index 52b1dd2ee84..43dc59cfd56 100644 --- a/packages/devtools_shared/lib/src/server/usage.dart +++ b/packages/devtools_shared/lib/src/server/usage.dart @@ -32,7 +32,7 @@ class FlutterUsage { String get clientId => _analytics.clientId; } -// Access the DevTools on disk store (~/.devtools/.devtools). +// Access the DevTools on disk store (~/.flutter-devtools/.devtools). class DevToolsUsage { DevToolsUsage() { LocalFileSystem.maybeMoveLegacyDevToolsStore(); diff --git a/packages/devtools_shared/pubspec.yaml b/packages/devtools_shared/pubspec.yaml index 4a0d7c3ba76..e4c5de81efe 100644 --- a/packages/devtools_shared/pubspec.yaml +++ b/packages/devtools_shared/pubspec.yaml @@ -9,11 +9,14 @@ environment: sdk: '>=3.0.0 <4.0.0' dependencies: + collection: ^1.15.0 path: ^1.8.0 shelf: ^1.1.0 usage: ^4.0.0 vm_service: ">=10.1.0 <12.0.0" webkit_inspection_protocol: '>=0.5.0 <2.0.0' + yaml: ^3.1.2 + yaml_edit: ^2.1.1 dev_dependencies: test: ^1.21.2 diff --git a/packages/devtools_shared/test/extensions/extension_activation_test.dart b/packages/devtools_shared/test/extensions/extension_activation_test.dart new file mode 100644 index 00000000000..568cde3d7e9 --- /dev/null +++ b/packages/devtools_shared/test/extensions/extension_activation_test.dart @@ -0,0 +1,110 @@ +// Copyright 2023 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'package:devtools_shared/devtools_extensions.dart'; +import 'package:devtools_shared/src/extensions/extension_activation.dart'; +import 'package:test/test.dart'; + +void main() { + group('$DevToolsOptions', () { + late DevToolsOptions options; + late Directory tmpDir; + + const tmpPath = '_tmp'; + final tmpUri = Uri.parse(tmpPath); + + setUp(() { + options = DevToolsOptions(); + tmpDir = Directory(tmpPath)..createSync(); + }); + + tearDown(() { + options = DevToolsOptions(); + tmpDir.deleteSync(recursive: true); + }); + + File _optionsFileFromTmp() { + final tmpFiles = tmpDir.listSync(); + expect(tmpFiles, isNotEmpty); + final optionsFile = File('$tmpPath/${DevToolsOptions.optionsFileName}'); + expect(optionsFile.existsSync(), isTrue); + return optionsFile; + } + + test('extensionActivatedState creates options file when none exists', () { + expect(tmpDir.listSync(), isEmpty); + options.lookupExtensionActivationState( + rootUri: tmpUri, + extensionName: 'foo', + ); + final file = _optionsFileFromTmp(); + expect( + file.readAsStringSync(), + ''' +extensions: +''', + ); + }); + + test('can write to options file', () { + options.setExtensionActivationState( + rootUri: tmpUri, + extensionName: 'foo', + activate: true, + ); + final file = _optionsFileFromTmp(); + expect( + file.readAsStringSync(), + ''' +extensions: + - foo: true''', + ); + }); + + test('can read from options file', () { + options.setExtensionActivationState( + rootUri: tmpUri, + extensionName: 'foo', + activate: true, + ); + options.setExtensionActivationState( + rootUri: tmpUri, + extensionName: 'bar', + activate: false, + ); + final file = _optionsFileFromTmp(); + expect( + file.readAsStringSync(), + ''' +extensions: + - foo: true + - bar: false''', + ); + + expect( + options.lookupExtensionActivationState( + rootUri: tmpUri, + extensionName: 'foo', + ), + ExtensionActivationState.enabled, + ); + expect( + options.lookupExtensionActivationState( + rootUri: tmpUri, + extensionName: 'bar', + ), + ExtensionActivationState.disabled, + ); + expect( + options.lookupExtensionActivationState( + rootUri: tmpUri, + extensionName: 'baz', + ), + ExtensionActivationState.none, + ); + }); + }); +} From 13a62cd644e22211e22fe5aa055cc78c9207b2e5 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Thu, 27 Jul 2023 15:16:51 -0700 Subject: [PATCH 2/6] remove print --- .../devtools_shared/lib/src/extensions/extension_activation.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/devtools_shared/lib/src/extensions/extension_activation.dart b/packages/devtools_shared/lib/src/extensions/extension_activation.dart index f6be3d44334..f38047b5a93 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_activation.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_activation.dart @@ -121,7 +121,6 @@ $_extensionsKey: final optionsFile = File(path.join(rootDir.path, optionsFileName)); if (!optionsFile.existsSync()) { - print('creating options file: ${optionsFile.path}'); optionsFile ..createSync() ..writeAsStringSync(_defaultOptions); From b94f747a1c2737d9253635cc08cc665176251cca Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 28 Jul 2023 08:23:06 -0700 Subject: [PATCH 3/6] review comments --- .../src/extensions/extension_activation.dart | 8 +- .../lib/src/extensions/extension_model.dart | 2 +- .../lib/src/server/server_api.dart | 154 +++++++++++------- 3 files changed, 103 insertions(+), 61 deletions(-) diff --git a/packages/devtools_shared/lib/src/extensions/extension_activation.dart b/packages/devtools_shared/lib/src/extensions/extension_activation.dart index f38047b5a93..8bd20c7821b 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_activation.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_activation.dart @@ -31,7 +31,7 @@ $_extensionsKey: required String extensionName, }) { final options = _optionsAsMap(rootUri: rootUri); - if (options == null) return ExtensionActivationState.err; + if (options == null) return ExtensionActivationState.error; final extensions = (options[_extensionsKey] as List?)?.cast>(); @@ -58,7 +58,7 @@ $_extensionsKey: required bool activate, }) { final options = _optionsAsMap(rootUri: rootUri); - if (options == null) return ExtensionActivationState.err; + if (options == null) return ExtensionActivationState.error; var extensions = (options[_extensionsKey] as List?)?.cast>(); @@ -70,9 +70,11 @@ $_extensionsKey: // Write the new activation state to the map. final extension = extensions.firstWhereOrNull( (e) => e.keys.first == extensionName, - )?[extensionName] = activate; + ); if (extension == null) { extensions.add({extensionName: activate}); + } else { + extension[extensionName] = activate; } _writeToOptionsFile(rootUri: rootUri, options: options); diff --git a/packages/devtools_shared/lib/src/extensions/extension_model.dart b/packages/devtools_shared/lib/src/extensions/extension_model.dart index 7dc7bea9166..96e6861e590 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_model.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_model.dart @@ -125,7 +125,7 @@ enum ExtensionActivationState { /// Something went wrong with reading or writing the activation state. /// /// We should ignore extensions with this activation state. - err; + error; static ExtensionActivationState from(String? value) { return ExtensionActivationState.values diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index 4f88d95fef1..44380d71540 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// ignore_for_file: avoid_classes_with_only_static_members + import 'dart:async'; import 'dart:convert'; import 'dart:io'; @@ -34,6 +36,10 @@ class ServerApi { ServerApi? api, ]) { api ??= ServerApi(); + final queryParams = request.requestedUri.queryParameters; + // TODO(kenz): break this switch statement up so that it uses helper methods + // for each case. Also use [_checkRequiredParameters] and [_encodeResponse] + // helpers. switch (request.url.path) { // ----- Flutter Tool GA store. ----- case apiGetFlutterGAEnabled: @@ -69,7 +75,6 @@ class ServerApi { ); case apiSetDevToolsEnabled: // Enable or disable DevTools analytics collection. - final queryParams = request.requestedUri.queryParameters; if (queryParams.containsKey(devToolsEnabledPropertyName)) { _devToolsUsage.analyticsEnabled = json.decode(queryParams[devToolsEnabledPropertyName]!); @@ -87,7 +92,6 @@ class ServerApi { // Set the active survey used to store subsequent apiGetSurveyActionTaken, // apiSetSurveyActionTaken, apiGetSurveyShownCount, and // apiIncrementSurveyShownCount calls. - final queryParams = request.requestedUri.queryParameters; if (queryParams.keys.length == 1 && queryParams.containsKey(activeSurveyName)) { final String theSurveyName = queryParams[activeSurveyName]!; @@ -123,7 +127,6 @@ class ServerApi { } // Set the SurveyActionTaken. // Has the survey been taken or dismissed.. - final queryParams = request.requestedUri.queryParameters; if (queryParams.containsKey(surveyActionTakenPropertyName)) { _devToolsUsage.surveyActionTaken = json.decode(queryParams[surveyActionTakenPropertyName]!); @@ -164,7 +167,6 @@ class ServerApi { json.encode(_devToolsUsage.lastReleaseNotesVersion), ); case apiSetLastReleaseNotesVersion: - final queryParams = request.requestedUri.queryParameters; if (queryParams.containsKey(lastReleaseNotesVersionPropertyName)) { _devToolsUsage.lastReleaseNotesVersion = queryParams[lastReleaseNotesVersionPropertyName]!; @@ -176,7 +178,6 @@ class ServerApi { // ----- App size api. ----- case apiGetBaseAppSizeFile: - final queryParams = request.requestedUri.queryParameters; if (queryParams.containsKey(baseAppSizeFilePropertyName)) { final filePath = queryParams[baseAppSizeFilePropertyName]!; final fileJson = LocalFileSystem.devToolsFileAsJson(filePath); @@ -191,7 +192,6 @@ class ServerApi { '$baseAppSizeFilePropertyName', ); case apiGetTestAppSizeFile: - final queryParams = request.requestedUri.queryParameters; if (queryParams.containsKey(testAppSizeFilePropertyName)) { final filePath = queryParams[testAppSizeFilePropertyName]!; final fileJson = LocalFileSystem.devToolsFileAsJson(filePath); @@ -209,66 +209,45 @@ class ServerApi { // ----- Extensions api. ----- case ExtensionsApi.apiServeAvailableExtensions: - final queryParams = request.requestedUri.queryParameters; - final rootPath = - queryParams[ExtensionsApi.extensionRootPathPropertyName]; - if (rootPath == null) { - return api.badRequest( - '${ExtensionsApi.extensionRootPathPropertyName} query parameter ' - 'required', - ); - } - - return extensionsManager.serveAvailableExtensions(rootPath).then((_) { - final extensions = extensionsManager.devtoolsExtensions - .map((p) => p.toJson()) - .toList(); - final result = jsonEncode({ - ExtensionsApi.extensionsResultPropertyName: extensions, - }); - return api!.getCompleted(result); - }); + return _ExtensionsApiHandler.handleServeAvailableExtensions( + api, + queryParams, + extensionsManager, + ); case ExtensionsApi.apiExtensionActivationState: - final queryParams = request.requestedUri.queryParameters; - final rootPath = - queryParams[ExtensionsApi.extensionRootPathPropertyName]; - if (rootPath == null) { - return api.badRequest( - '${ExtensionsApi.extensionRootPathPropertyName} query parameter ' - 'required', - ); - } - final rootUri = Uri.parse(rootPath); - - final extensionName = - queryParams[ExtensionsApi.extensionNamePropertyName]; - if (extensionName == null) { - return api.badRequest( - '${ExtensionsApi.extensionNamePropertyName} query parameter required', - ); - } - - final activate = queryParams[ExtensionsApi.activationStatePropertyName]; - if (activate != null) { - final newState = _devToolsOptions.setExtensionActivationState( - rootUri: rootUri, - extensionName: extensionName, - activate: bool.parse(activate), - ); - return api.getCompleted(json.encode(newState.name)); - } - final activationState = _devToolsOptions.lookupExtensionActivationState( - rootUri: rootUri, - extensionName: extensionName, + return _ExtensionsApiHandler.handleExtensionActivationState( + api, + queryParams, ); - return api.getCompleted(json.encode(activationState.name)); default: return api.notImplemented(); } } + static FutureOr _encodeResponse( + Object? object, { + required ServerApi api, + }) { + return api.getCompleted(json.encode(object)); + } + + static FutureOr _checkRequiredParameters( + List expectedParams, { + required Map queryParams, + required ServerApi api, + }) { + final missing = expectedParams.where( + (param) => !queryParams.containsKey(param), + ); + return missing.isNotEmpty + ? api.badRequest( + 'Missing required query parameters: ${missing.toList()}', + ) + : null; + } + // Accessing Flutter usage file e.g., ~/.flutter. // NOTE: Only access the file if it exists otherwise Flutter Tool hasn't yet // been run. @@ -315,3 +294,64 @@ class ServerApi { FutureOr notImplemented() => shelf.Response(HttpStatus.noContent); } + +abstract class _ExtensionsApiHandler { + static FutureOr handleServeAvailableExtensions( + ServerApi api, + Map queryParams, + ExtensionsManager extensionsManager, + ) async { + final missingRequiredParams = await ServerApi._checkRequiredParameters( + [ExtensionsApi.extensionRootPathPropertyName], + queryParams: queryParams, + api: api, + ); + if (missingRequiredParams != null) return missingRequiredParams; + + final rootPath = queryParams[ExtensionsApi.extensionRootPathPropertyName]; + + return extensionsManager.serveAvailableExtensions(rootPath).then((_) { + final extensions = + extensionsManager.devtoolsExtensions.map((p) => p.toJson()).toList(); + final result = jsonEncode({ + ExtensionsApi.extensionsResultPropertyName: extensions, + }); + return ServerApi._encodeResponse(result, api: api); + }); + } + + static FutureOr handleExtensionActivationState( + ServerApi api, + Map queryParams, + ) async { + final missingRequiredParams = await ServerApi._checkRequiredParameters( + [ + ExtensionsApi.extensionRootPathPropertyName, + ExtensionsApi.extensionsResultPropertyName, + ], + queryParams: queryParams, + api: api, + ); + if (missingRequiredParams != null) return missingRequiredParams; + + final rootPath = queryParams[ExtensionsApi.extensionRootPathPropertyName]!; + final rootUri = Uri.parse(rootPath); + final extensionName = queryParams[ExtensionsApi.extensionNamePropertyName]!; + + final activate = queryParams[ExtensionsApi.activationStatePropertyName]; + if (activate != null) { + final newState = ServerApi._devToolsOptions.setExtensionActivationState( + rootUri: rootUri, + extensionName: extensionName, + activate: bool.parse(activate), + ); + return ServerApi._encodeResponse(newState.name, api: api); + } + final activationState = + ServerApi._devToolsOptions.lookupExtensionActivationState( + rootUri: rootUri, + extensionName: extensionName, + ); + return ServerApi._encodeResponse(activationState.name, api: api); + } +} From a761f703dfed6f3615ddd2b312ec52e5c34121ef Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 28 Jul 2023 08:29:32 -0700 Subject: [PATCH 4/6] fix test --- .../extensions/extension_activation_test.dart | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/devtools_shared/test/extensions/extension_activation_test.dart b/packages/devtools_shared/test/extensions/extension_activation_test.dart index 568cde3d7e9..dca080050d4 100644 --- a/packages/devtools_shared/test/extensions/extension_activation_test.dart +++ b/packages/devtools_shared/test/extensions/extension_activation_test.dart @@ -12,13 +12,12 @@ void main() { group('$DevToolsOptions', () { late DevToolsOptions options; late Directory tmpDir; - - const tmpPath = '_tmp'; - final tmpUri = Uri.parse(tmpPath); + late Uri tmpUri; setUp(() { options = DevToolsOptions(); - tmpDir = Directory(tmpPath)..createSync(); + tmpDir = Directory.current.createTempSync(); + tmpUri = Uri.parse(tmpDir.path); }); tearDown(() { @@ -29,7 +28,8 @@ void main() { File _optionsFileFromTmp() { final tmpFiles = tmpDir.listSync(); expect(tmpFiles, isNotEmpty); - final optionsFile = File('$tmpPath/${DevToolsOptions.optionsFileName}'); + final optionsFile = + File('${tmpDir.path}/${DevToolsOptions.optionsFileName}'); expect(optionsFile.existsSync(), isTrue); return optionsFile; } @@ -86,21 +86,21 @@ extensions: expect( options.lookupExtensionActivationState( - rootUri: tmpUri, + rootUri: tmpUri, extensionName: 'foo', ), ExtensionActivationState.enabled, ); expect( options.lookupExtensionActivationState( - rootUri: tmpUri, + rootUri: tmpUri, extensionName: 'bar', ), ExtensionActivationState.disabled, ); expect( options.lookupExtensionActivationState( - rootUri: tmpUri, + rootUri: tmpUri, extensionName: 'baz', ), ExtensionActivationState.none, From 3d39fafc575ec8dadca10a00849e53b0efdcc2f5 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 28 Jul 2023 08:56:11 -0700 Subject: [PATCH 5/6] remove async --- .../lib/src/server/server_api.dart | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index 44380d71540..007f29bbf5d 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -226,14 +226,14 @@ class ServerApi { } } - static FutureOr _encodeResponse( + static shelf.Response _encodeResponse( Object? object, { required ServerApi api, }) { return api.getCompleted(json.encode(object)); } - static FutureOr _checkRequiredParameters( + static shelf.Response? _checkRequiredParameters( List expectedParams, { required Map queryParams, required ServerApi api, @@ -267,21 +267,19 @@ class ServerApi { /// /// In the open-source version of DevTools, Google Analytics handles this /// without any need to involve the server. - FutureOr logScreenView() => notImplemented(); + shelf.Response logScreenView() => notImplemented(); /// Return the value of the property. - FutureOr getCompleted(String value) => - shelf.Response.ok('$value'); + shelf.Response getCompleted(String value) => shelf.Response.ok('$value'); /// Return the value of the property after the property value has been set. - FutureOr setCompleted(String value) => - shelf.Response.ok('$value'); + shelf.Response setCompleted(String value) => shelf.Response.ok('$value'); /// A [shelf.Response] for API calls that encountered a request problem e.g., /// setActiveSurvey not called. /// /// This is a 400 Bad Request response. - FutureOr badRequest([String? logError]) { + shelf.Response badRequest([String? logError]) { if (logError != null) print(logError); return shelf.Response(HttpStatus.badRequest); } @@ -291,8 +289,7 @@ class ServerApi { /// /// This is a no-op 204 No Content response because returning 404 Not Found /// creates unnecessary noise in the console. - FutureOr notImplemented() => - shelf.Response(HttpStatus.noContent); + shelf.Response notImplemented() => shelf.Response(HttpStatus.noContent); } abstract class _ExtensionsApiHandler { @@ -300,8 +297,8 @@ abstract class _ExtensionsApiHandler { ServerApi api, Map queryParams, ExtensionsManager extensionsManager, - ) async { - final missingRequiredParams = await ServerApi._checkRequiredParameters( + ) { + final missingRequiredParams = ServerApi._checkRequiredParameters( [ExtensionsApi.extensionRootPathPropertyName], queryParams: queryParams, api: api, @@ -320,11 +317,11 @@ abstract class _ExtensionsApiHandler { }); } - static FutureOr handleExtensionActivationState( + static shelf.Response handleExtensionActivationState( ServerApi api, Map queryParams, - ) async { - final missingRequiredParams = await ServerApi._checkRequiredParameters( + ) { + final missingRequiredParams = ServerApi._checkRequiredParameters( [ ExtensionsApi.extensionRootPathPropertyName, ExtensionsApi.extensionsResultPropertyName, From 698bb8fe0ffb637b9108b6c31ed6ac5c7e7deb5c Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 28 Jul 2023 10:51:20 -0700 Subject: [PATCH 6/6] remove future or --- .../lib/src/server/server_api.dart | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart index 007f29bbf5d..e6de1762661 100644 --- a/packages/devtools_shared/lib/src/server/server_api.dart +++ b/packages/devtools_shared/lib/src/server/server_api.dart @@ -293,11 +293,11 @@ class ServerApi { } abstract class _ExtensionsApiHandler { - static FutureOr handleServeAvailableExtensions( + static Future handleServeAvailableExtensions( ServerApi api, Map queryParams, ExtensionsManager extensionsManager, - ) { + ) async { final missingRequiredParams = ServerApi._checkRequiredParameters( [ExtensionsApi.extensionRootPathPropertyName], queryParams: queryParams, @@ -307,14 +307,13 @@ abstract class _ExtensionsApiHandler { final rootPath = queryParams[ExtensionsApi.extensionRootPathPropertyName]; - return extensionsManager.serveAvailableExtensions(rootPath).then((_) { - final extensions = - extensionsManager.devtoolsExtensions.map((p) => p.toJson()).toList(); - final result = jsonEncode({ - ExtensionsApi.extensionsResultPropertyName: extensions, - }); - return ServerApi._encodeResponse(result, api: api); + await extensionsManager.serveAvailableExtensions(rootPath); + final extensions = + extensionsManager.devtoolsExtensions.map((p) => p.toJson()).toList(); + final result = jsonEncode({ + ExtensionsApi.extensionsResultPropertyName: extensions, }); + return ServerApi._encodeResponse(result, api: api); } static shelf.Response handleExtensionActivationState(