adressing comment and bump version

packages/devtools_shared/CHANGELOG.md

@@ -1,6 +1,10 @@
-# 3.0.2
+# 4.0.0
 
 - Bump `package:extension_discovery` version to ^2.0.0
+- Adds a `DeeplinkApi.androidBuildVariants` endpoint to ServerAPi.
+- **BREAKING CHANGE**:
+  - ServerApi.handle extensionsManager, api parameter are converted to named parameters
+  - Adds a new required name parameter deeplinkManager to `ServerApi.handle`.
 
 # 3.0.1
 

packages/devtools_shared/lib/src/deeplink/deeplink_manager.dart

@@ -1,32 +1,40 @@
-// Copyright 2021 The Chromium Authors. All rights reserved.
+// 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:convert';
 import 'dart:io';
 
 import 'package:meta/meta.dart';
-import 'package:shelf/shelf.dart' as shelf;
-
-import '../server/server_api.dart';
 
 class DeeplinkManager {
+  /// A regex to retrieve the json part of the stdout.
+  ///
+  /// Example stdout:
+  ///
+  /// Running Gradle task 'printBuildVariants'...                        10.4s
+  /// ["debug","release","profile"]
   static final _buildVariantJsonRegex = RegExp(r'(\[.*\])');
+  static const kErrorField = 'error';
+  static const kOutputJsonField = 'json';
 
   @visibleForTesting
   Future<ProcessResult> runProcess(String executable, List<String> arguments) {
     return Process.run(executable, arguments);
   }
 
-  Future<shelf.Response> getBuildVariants({
+  Future<Map<String, Object?>> getBuildVariants({
     required String rootPath,
-    required ServerApi api,
   }) async {
     final ProcessResult result = await runProcess(
       'flutter',
       <String>['analyze', '--android', '--list-build-variants', rootPath],
     );
     if (result.exitCode != 0) {
-      return api.serverError(result.stderr);
+      return <String, String>{
+        kErrorField:
+            'Flutter command exit with non-zero error code ${result.exitCode}\n${result.stderr}',
+      };
     }
     final match = _buildVariantJsonRegex.firstMatch(result.stdout);
     final String outputJson;
@@ -35,6 +43,16 @@ class DeeplinkManager {
     } else {
       outputJson = match.group(1)!;
     }
-    return api.getCompleted(outputJson);
+    try {
+      jsonEncode(outputJson);
+    } on Error catch (e) {
+      return <String, String?>{
+        kErrorField: e.toString(),
+      };
+    }
+    return <String, String?>{
+      kErrorField: null,
+      kOutputJsonField: outputJson,
+    };
   }
 }

packages/devtools_shared/lib/src/server/server_api.dart

@@ -32,11 +32,11 @@ class ServerApi {
   ///
   /// To override an API call, pass in a subclass of [ServerApi].
   static FutureOr<shelf.Response> handle(
-    shelf.Request request,
-    ExtensionsManager extensionsManager,
-    DeeplinkManager deeplinkManager, [
+    shelf.Request request, {
+    required ExtensionsManager extensionsManager,
+    required DeeplinkManager deeplinkManager,
     ServerApi? api,
-  ]) {
+  }) {
     api ??= ServerApi();
     final queryParams = request.requestedUri.queryParameters;
     // TODO(kenz): break this switch statement up so that it uses helper methods
@@ -223,7 +223,7 @@ class ServerApi {
           queryParams,
         );
 
-      // ----- deeplinks api. -----
+      // ----- deeplink api. -----
 
       case DeeplinkApi.androidBuildVariants:
         return _DeeplinkApiHandler.handleAndroidBuildVariants(
@@ -391,7 +391,12 @@ abstract class _DeeplinkApiHandler {
     if (missingRequiredParams != null) return missingRequiredParams;
 
     final rootPath = queryParams[DeeplinkApi.deeplinkRootPathPropertyName]!;
-
-    return deeplinkManager.getBuildVariants(rootPath: rootPath, api: api);
+    final result = await deeplinkManager.getBuildVariants(rootPath: rootPath);
+    final error = result[DeeplinkManager.kErrorField] as String?;
+    if (error != null) {
+      api.serverError(error);
+    }
+    return api
+        .getCompleted(result[DeeplinkManager.kOutputJsonField]! as String);
   }
 }

packages/devtools_shared/pubspec.yaml

@@ -1,7 +1,7 @@
 name: devtools_shared
 description: Package of shared Dart structures between devtools_app, dds, and other tools.
 
-version: 3.0.2
+version: 4.0.0
 
 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_shared
 

packages/devtools_shared/test/deeplink/deeplink_manager_test.dart

@@ -1,6 +1,6 @@
-// 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.
+// 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';
 
@@ -39,17 +39,20 @@ void main() {
           result: ProcessResult(
             0,
             0,
-            '["release", "profile"]',
+            r'''
+Running Gradle task 'printBuildVariants'...                        10.4s
+["debug","release","profile"]
+            ''',
             '',
           ),
         ),
       );
       final response = await manager.getBuildVariants(
         rootPath: projectRoot,
-        api: ServerApi(),
       );
-      expect(response.statusCode, HttpStatus.ok);
-      expect(await response.readAsString(), '["release", "profile"]');
+      expect(response[DeeplinkManager.kErrorField], isNull);
+      expect(response[DeeplinkManager.kOutputJsonField],
+          '["debug","release","profile"]');
     });
 
     test('getBuildVariants return internal server error if command failed',
@@ -74,9 +77,8 @@ void main() {
       );
       final response = await manager.getBuildVariants(
         rootPath: projectRoot,
-        api: ServerApi(),
       );
-      expect(response.statusCode, HttpStatus.internalServerError);
+      expect(response[DeeplinkManager.kErrorField], contains('unknown error'));
     });
   });
 }

packages/devtools_shared/test/server/server_api_test.dart

@@ -1,4 +1,4 @@
-// Copyright 2022 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -26,15 +26,16 @@ void main() {
       ),
     );
     final fakeManager = FakeDeeplinkManager();
-    final expectedResponse = Response(HttpStatus.ok);
-    fakeManager.responseForGetBuildVariants = expectedResponse;
+    fakeManager.responseForGetBuildVariants = <String, String>{
+      DeeplinkManager.kOutputJsonField: '["debug", "release]',
+    };
     final response = await ServerApi.handle(
       request,
-      ExtensionsManager(buildDir: '/'),
-      fakeManager,
+      extensionsManager: ExtensionsManager(buildDir: '/'),
+      deeplinkManager: fakeManager,
     );
-    expect(response, expectedResponse);
-    expect(fakeManager.receivedPathFromGetBuildVariants, expectedRootPath);
+    expect(response.statusCode, HttpStatus.ok);
+    expect(await response.readAsString(), '["debug", "release]');
   });
 
   test('handle deeplink api returns bad request if no root path', () async {
@@ -48,21 +49,20 @@ void main() {
     );
     final response = await ServerApi.handle(
       request,
-      ExtensionsManager(buildDir: '/'),
-      FakeDeeplinkManager(),
+      extensionsManager: ExtensionsManager(buildDir: '/'),
+      deeplinkManager: FakeDeeplinkManager(),
     );
     expect(response.statusCode, HttpStatus.badRequest);
   });
 }
 
 class FakeDeeplinkManager extends DeeplinkManager {
   String? receivedPathFromGetBuildVariants;
-  late Response responseForGetBuildVariants;
+  late Map<String, String> responseForGetBuildVariants;
 
   @override
-  Future<Response> getBuildVariants({
+  Future<Map<String, String>> getBuildVariants({
     required String rootPath,
-    required ServerApi api,
   }) async {
     receivedPathFromGetBuildVariants = rootPath;
     return responseForGetBuildVariants;