Deprecate DTDConnectionInfo and replace with a class that can handle …

…separate private/exposed URIs (flutter#8291) This replaces `DTDConnectionInfo` with `DTDInfo` (unsure if a better name?) and marks the former as deprecated. The new class has two URIs: - `localUri` which is the URI used for backend services on the same machine as the DTD process (this is usually a `localhost` URI) - `exposedUri` which is the URI that can be used from the frontend where the user is (this may or may not be the same as localUri depending on whether they're in a remote/web IDE session) The DevTools web API returns the exposed URI (since it will be used by frontend tools) and other places that used the URI for connecting directly use the local one. On its own, this change doesn't do anything, the intention is to roll this into the SDK and update the DevTools server code in dartdev/DDS to accept an additional optional URI which will then provide these back to the server code here (for the web API handler and extension search code). This is work towards fixing some of the issues in Dart-Code/Dart-Code#5158.
Sarsilmaz02 · Sep 9, 2024 · 5457f57 · 5457f57
1 parent af29d79
commit 5457f57
Show file tree

Hide file tree

Showing 12 changed files with 80 additions and 41 deletions.
diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md
@@ -15,6 +15,7 @@
 * Deprecate `apiIncrementSurveyShownCount` in favor of `SurveyApi.incrementSurveyShownCount`.
 * Support Chrome's new headless mode in the integration test runner.
 * Add `PreferencesApi` to get and set preference values.
+* Deprecate `DTDConnectionInfo` in favor of `DtdInfo` which supports tracking two URIs for DTD to better support web/remote environments.
 
 # 10.0.2
 * Update dependency `web_socket_channel: '>=2.4.0 <4.0.0'`.

diff --git a/packages/devtools_shared/lib/src/common.dart b/packages/devtools_shared/lib/src/common.dart
@@ -3,4 +3,38 @@
 // found in the LICENSE file.
 
 /// Describes an instance of the Dart Tooling Daemon.
+@Deprecated('Use DtdInfo instead')
 typedef DTDConnectionInfo = ({String? uri, String? secret});
+
+/// Information about a Dart Tooling Daemon instance.
+class DtdInfo {
+  DtdInfo(
+    this.localUri, {
+    Uri? exposedUri,
+    this.secret,
+  }) : exposedUri = exposedUri ?? localUri;
+
+  /// The URI for connecting to DTD from the backend.
+  ///
+  /// This is usually a `http://localhost/` address that is accessible to tools
+  /// running in the same location as the DTD process. It may NOT be accessible
+  /// to frontends that run in another location - for example the DevTools
+  /// frontend running in a browser (or embedded in an IDE) in a remote/web IDE
+  /// session.
+  final Uri localUri;
+
+  /// The exposed URI for connecting to DTD from the frontend.
+  ///
+  /// In a remote session, this can be an address provided by the hosting
+  /// infrastructure/proxy that tunnels through to the backend running the DTD
+  /// process which might look like `https://foo-123.cloud-ide.foo/`.
+  ///
+  /// For a non-remote session, this will be the same value as [localUri].
+  final Uri exposedUri;
+
+  /// The secret token that allows calling privileged DTD APIs.
+  ///
+  /// This may not always be available if DTD was spawned by another process
+  /// and it should never be shared with external code.
+  final String? secret;
+}
diff --git a/packages/devtools_shared/lib/src/extensions/extension_manager.dart b/packages/devtools_shared/lib/src/extensions/extension_manager.dart
@@ -59,7 +59,7 @@ class ExtensionsManager {
   Future<void> serveAvailableExtensions(
     String? rootFileUriString,
     List<String> logs,
-    DTDConnectionInfo? dtd,
+    DtdInfo? dtd,
   ) async {
     logs.add(
       'ExtensionsManager.serveAvailableExtensions for '
@@ -86,11 +86,11 @@ class ExtensionsManager {
 
     // Find all static extensions for the project roots, which are derived from
     // the Dart Tooling Daemon, and add them to [devtoolsExtensions].
-    final dtdUri = dtd?.uri;
+    final dtdUri = dtd?.localUri;
     if (dtdUri != null) {
       DartToolingDaemon? dartToolingDaemon;
       try {
-        dartToolingDaemon = await DartToolingDaemon.connect(Uri.parse(dtdUri));
+        dartToolingDaemon = await DartToolingDaemon.connect(dtdUri);
         final projectRoots = await dartToolingDaemon.getProjectRoots(
           depth: staticExtensionsSearchDepth,
         );

diff --git a/packages/devtools_shared/lib/src/server/handlers/_devtools_extensions.dart b/packages/devtools_shared/lib/src/server/handlers/_devtools_extensions.dart
@@ -11,7 +11,7 @@ abstract class _ExtensionsApiHandler {
     ServerApi api,
     Map<String, String> queryParams,
     ExtensionsManager extensionsManager,
-    DTDConnectionInfo? dtd,
+    DtdInfo? dtd,
   ) async {
     final missingRequiredParams = ServerApi._checkRequiredParameters(
       [ExtensionsApi.packageRootUriPropertyName],

diff --git a/packages/devtools_shared/lib/src/server/handlers/_dtd.dart b/packages/devtools_shared/lib/src/server/handlers/_dtd.dart
@@ -9,10 +9,13 @@ part of '../server_api.dart';
 abstract class _DtdApiHandler {
   static shelf.Response handleGetDtdUri(
     ServerApi api,
-    DTDConnectionInfo? dtd,
+    DtdInfo? dtd,
   ) {
     return ServerApi._encodeResponse(
-      {DtdApi.uriPropertyName: dtd?.uri},
+      {
+        // Always provide the exposed URI to callers of the web API.
+        DtdApi.uriPropertyName: dtd?.exposedUri.toString(),
+      },
       api: api,
     );
   }

diff --git a/packages/devtools_shared/lib/src/server/handlers/_general.dart b/packages/devtools_shared/lib/src/server/handlers/_general.dart
@@ -22,7 +22,7 @@ abstract class Handler {
   static Future<shelf.Response> handleNotifyForVmServiceConnection(
     ServerApi api,
     Map<String, String> queryParams,
-    DTDConnectionInfo? dtd,
+    DtdInfo? dtd,
   ) async {
     final missingRequiredParams = ServerApi._checkRequiredParameters(
       const [apiParameterValueKey, apiParameterVmServiceConnected],
@@ -32,7 +32,7 @@ abstract class Handler {
     );
     if (missingRequiredParams != null) return missingRequiredParams;
 
-    final dtdUri = dtd?.uri;
+    final dtdUri = dtd?.localUri;
     final dtdSecret = dtd?.secret;
     if (dtdUri == null || dtdSecret == null) {
       // If DevTools server did not start DTD, there is nothing for us to do.
@@ -62,7 +62,7 @@ abstract class Handler {
 
     DartToolingDaemon? dartToolingDaemon;
     try {
-      dartToolingDaemon = await DartToolingDaemon.connect(Uri.parse(dtd!.uri!));
+      dartToolingDaemon = await DartToolingDaemon.connect(dtd!.localUri);
 
       final detectRootResponse = await detectRootPackageForVmService(
         vmServiceUriAsString: vmServiceUriAsString,
@@ -152,7 +152,7 @@ abstract class Handler {
   @visibleForTesting
   static Future<shelf.Response> updateDtdWorkspaceRoots(
     DartToolingDaemon dtd, {
-    required DTDConnectionInfo dtdConnectionInfo,
+    required DtdInfo dtdConnectionInfo,
     required Uri rootFromVmService,
     required bool connected,
     required ServerApi api,

diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart
@@ -58,7 +58,7 @@ class ServerApi {
     required ExtensionsManager extensionsManager,
     required DeeplinkManager deeplinkManager,
     ServerApi? api,
-    DTDConnectionInfo? dtd,
+    DtdInfo? dtd,
   }) {
     api ??= ServerApi();
     final queryParams = request.requestedUri.queryParameters;

diff --git a/packages/devtools_shared/test/helpers/helpers.dart b/packages/devtools_shared/test/helpers/helpers.dart
@@ -10,9 +10,8 @@ import 'package:devtools_shared/devtools_shared.dart';
 import 'package:path/path.dart' as path;
 
 typedef TestDtdConnectionInfo = ({
-  String? uri,
-  String? secret,
-  Process? dtdProcess,
+  DtdInfo? info,
+  Process? process,
 });
 
 /// Helper method to start DTD for the purpose of testing.
@@ -23,8 +22,7 @@ Future<TestDtdConnectionInfo> startDtd() async {
   Process? dtdProcess;
   StreamSubscription? dtdStoutSubscription;
 
-  TestDtdConnectionInfo onFailure() =>
-      (uri: null, secret: null, dtdProcess: dtdProcess);
+  TestDtdConnectionInfo onFailure() => (info: null, process: dtdProcess);
 
   try {
     dtdProcess = await Process.start(
@@ -44,7 +42,10 @@ Future<TestDtdConnectionInfo> startDtd() async {
               }
             }) {
           completer.complete(
-            (uri: uri, secret: secret, dtdProcess: dtdProcess),
+            (
+              info: DtdInfo(Uri.parse(uri), secret: secret),
+              process: dtdProcess
+            ),
           );
         } else {
           completer.complete(onFailure());

diff --git a/packages/devtools_shared/test/server/devtools_extensions_api_test.dart b/packages/devtools_shared/test/server/devtools_extensions_api_test.dart
@@ -28,14 +28,14 @@ void main() {
   setUp(() async {
     extensionsManager = ExtensionsManager();
     dtd = await startDtd();
-    expect(dtd!.uri, isNotNull, reason: 'Error starting DTD for test');
-    testDtdConnection = await DartToolingDaemon.connect(Uri.parse(dtd!.uri!));
+    expect(dtd!.info, isNotNull, reason: 'Error starting DTD for test');
+    testDtdConnection = await DartToolingDaemon.connect(dtd!.info!.localUri);
   });
 
   tearDown(() async {
     await testDtdConnection?.close();
-    dtd?.dtdProcess?.kill();
-    await dtd?.dtdProcess?.exitCode;
+    dtd?.process?.kill();
+    await dtd?.process?.exitCode;
     dtd = null;
 
     await extensionTestManager.reset();
@@ -50,7 +50,7 @@ void main() {
       includeBadExtension: includeBadExtension,
     );
     await testDtdConnection!.setIDEWorkspaceRoots(
-      dtd!.secret!,
+      dtd!.info!.secret!,
       [extensionTestManager.packagesRootUri],
     );
   }
@@ -75,7 +75,7 @@ void main() {
       request,
       extensionsManager: manager,
       deeplinkManager: FakeDeeplinkManager(),
-      dtd: (uri: dtd!.uri, secret: dtd!.secret),
+      dtd: dtd!.info,
     );
   }
 
@@ -252,7 +252,7 @@ class _TestExtensionsManager extends ExtensionsManager {
   Future<void> serveAvailableExtensions(
     String? rootFileUriString,
     List<String> logs,
-    DTDConnectionInfo? dtd,
+    DtdInfo? dtd,
   ) async {
     await super.serveAvailableExtensions(rootFileUriString, logs, dtd);
     throw Exception('Fake exception for test');

diff --git a/packages/devtools_shared/test/server/dtd_api_test.dart b/packages/devtools_shared/test/server/dtd_api_test.dart
@@ -16,7 +16,7 @@ import '../fakes.dart';
 void main() {
   group('$DtdApi', () {
     test('handle ${DtdApi.apiGetDtdUri} succeeds', () async {
-      const dtdUri = 'ws://dtd:uri';
+      final dtdUri = Uri.parse('ws://dtd/uri');
       final request = Request(
         'get',
         Uri(
@@ -29,12 +29,12 @@ void main() {
         request,
         extensionsManager: ExtensionsManager(),
         deeplinkManager: FakeDeeplinkManager(),
-        dtd: (uri: dtdUri, secret: null),
+        dtd: DtdInfo(dtdUri),
       );
       expect(response.statusCode, HttpStatus.ok);
       expect(
         await response.readAsString(),
-        jsonEncode({DtdApi.uriPropertyName: dtdUri}),
+        jsonEncode({DtdApi.uriPropertyName: dtdUri.toString()}),
       );
     });
   });

diff --git a/packages/devtools_shared/test/server/general_api_test.dart b/packages/devtools_shared/test/server/general_api_test.dart
@@ -20,7 +20,7 @@ void main() {
   group('General DevTools server API', () {
     group(apiNotifyForVmServiceConnection, () {
       Future<Response> sendNotifyRequest({
-        required DTDConnectionInfo dtd,
+        required DtdInfo? dtd,
         Map<String, Object?>? queryParameters,
         // ignore: avoid-redundant-async, returning FutureOr.
       }) async {
@@ -45,7 +45,7 @@ void main() {
         'succeeds when DTD is not available',
         () async {
           final response = await sendNotifyRequest(
-            dtd: (uri: null, secret: null),
+            dtd: null,
             queryParameters: {
               apiParameterValueKey: 'fake_uri',
               apiParameterVmServiceConnected: 'true',
@@ -60,7 +60,7 @@ void main() {
         'returns badRequest for invalid VM service argument',
         () async {
           final response = await sendNotifyRequest(
-            dtd: (uri: 'ws://dtd:uri', secret: 'fake_secret'),
+            dtd: DtdInfo(Uri.parse('ws://dtd/uri'), secret: 'fake_secret'),
             queryParameters: {
               apiParameterValueKey: 'fake_uri',
               apiParameterVmServiceConnected: 'true',
@@ -77,7 +77,7 @@ void main() {
         'returns badRequest for invalid $apiParameterVmServiceConnected argument',
         () async {
           final response = await sendNotifyRequest(
-            dtd: (uri: 'ws://dtd:uri', secret: 'fake_secret'),
+            dtd: DtdInfo(Uri.parse('ws://dtd/uri'), secret: 'fake_secret'),
             queryParameters: {
               apiParameterValueKey: 'ws://127.0.0.1:8181/LEpVqqD7E_Y=/ws',
               apiParameterVmServiceConnected: 'bad_arg',
@@ -98,15 +98,15 @@ void main() {
 
       setUp(() async {
         dtd = await startDtd();
-        expect(dtd!.uri, isNotNull, reason: 'Error starting DTD for test');
+        expect(dtd!.info, isNotNull, reason: 'Error starting DTD for test');
         testDtdConnection =
-            await DartToolingDaemon.connect(Uri.parse(dtd!.uri!));
+            await DartToolingDaemon.connect(dtd!.info!.localUri);
       });
 
       tearDown(() async {
         await testDtdConnection?.close();
-        dtd?.dtdProcess?.kill();
-        await dtd?.dtdProcess?.exitCode;
+        dtd?.process?.kill();
+        await dtd?.process?.exitCode;
         dtd = null;
       });
 
@@ -117,7 +117,7 @@ void main() {
         }) async {
           await server.Handler.updateDtdWorkspaceRoots(
             testDtdConnection!,
-            dtdConnectionInfo: (uri: dtd!.uri, secret: dtd!.secret),
+            dtdConnectionInfo: dtd!.info!,
             rootFromVmService: root,
             connected: connected,
             api: ServerApi(),

diff --git a/packages/devtools_shared/test/utils/file_utils_test.dart b/packages/devtools_shared/test/utils/file_utils_test.dart
@@ -39,21 +39,21 @@ void main() {
 
     setUp(() async {
       dtd = await startDtd();
-      expect(dtd!.uri, isNotNull, reason: 'Error starting DTD for test');
-      testDtdConnection = await DartToolingDaemon.connect(Uri.parse(dtd!.uri!));
+      expect(dtd!.info, isNotNull, reason: 'Error starting DTD for test');
+      testDtdConnection = await DartToolingDaemon.connect(dtd!.info!.localUri);
 
       _setupTestDirectoryStructure();
 
       await testDtdConnection!.setIDEWorkspaceRoots(
-        dtd!.secret!,
+        dtd!.info!.secret!,
         [Uri.parse(projectRoot)],
       );
     });
 
     tearDown(() async {
       await testDtdConnection?.close();
-      dtd?.dtdProcess?.kill();
-      await dtd?.dtdProcess?.exitCode;
+      dtd?.process?.kill();
+      await dtd?.process?.exitCode;
       dtd = null;
     });