Skip to content

Commit 10dd59c

Browse files
kenzieschmollCommit Queue
authored andcommitted
Add a DTD method to get all registered services.
This CL contains breaking changes for package:dtd and prepares both package:dart_service_protocol_shared and package:dtd for publish. This CL also fixes #60757 so that DTD-registered services are sent over the `Service` stream upon initial subscription like what is done for client-registered services. Change-Id: I619af816e64af01864c7ed9b98743c6691bf7e0b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429161 Reviewed-by: Ben Konyi <[email protected]> Commit-Queue: Kenzie Davisson <[email protected]>
1 parent f4c87f2 commit 10dd59c

24 files changed

+645
-72
lines changed

pkg/analysis_server/CONTRIBUTING.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ To run just the analysis server integration tests:
6464
./tools/test.py -mrelease pkg/analysis_server/test/integration/
6565
```
6666
67+
To run a single test:
68+
69+
```
70+
dart test pkg/analysis_server/test/some_test.dart
71+
```
72+
73+
> Note: `dart` may need to point to a Dart SDK built from source
74+
depending on the changes you are testing.
6775
6876
[building]: https://github.com/dart-lang/sdk/wiki/Building
6977
[contributing]: https://github.com/dart-lang/sdk/blob/master/CONTRIBUTING.md

pkg/analysis_server/lib/src/lsp/handlers/custom/handler_experimental_echo.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ class ExperimentalEchoHandler extends SharedMessageHandler<Object?, Object?> {
2929
MessageInfo message,
3030
CancellationToken token,
3131
) async {
32-
// The DTD client automatically converts `null` params to an empty map, but
33-
// (because of a previous bug) we want to test null results. So if the
34-
// params are an empty map, return null. This is tested by
35-
// `test_service_success_echo_nullResponse` in `SharedDtdTests`.
32+
// The DTD client may send an empty map as params. Return null in this case.
33+
// This is tested by
34+
//`test_service_success_echo_nullResponse_with_empty_params` in
35+
// `SharedDtdTests`.
3636
if (params is Map && params.isEmpty) {
3737
return success(null);
3838
}

pkg/analysis_server/lib/src/services/dart_tooling_daemon/dtd_services.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ class DtdServices {
179179
var message = IncomingMessage(
180180
jsonrpc: jsonRpcVersion,
181181
method: method,
182-
params: params.asMap,
182+
params: params.value,
183183
);
184184
var scheduler = _server.messageScheduler;
185185
var completer = Completer<Map<String, Object?>>();

pkg/analysis_server/test/shared/shared_dtd_tests.dart

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,10 @@ mixin SharedDtdTests
8888

8989
/// A list of service/methods that the test client has seen registered (and
9090
/// not yet unregistered) over the DTD connection.
91-
final availableMethods = <(String, Method)>[];
91+
///
92+
/// The service name is a nullable String because DTD-internal methods and
93+
/// services do not have a service name.
94+
final availableMethods = <(String?, Method)>[];
9295

9396
/// An invalid DTD URI used for testing connection failures.
9497
final invalidUri = Uri.parse('ws://invalid:345/invalid');
@@ -207,12 +210,12 @@ mixin SharedDtdTests
207210
switch (e.kind) {
208211
case 'ServiceRegistered':
209212
availableMethods.add((
210-
e.data['service'] as String,
213+
e.data['service'] as String?,
211214
Method(e.data['method'] as String),
212215
));
213216
case 'ServiceUnregistered':
214217
availableMethods.remove((
215-
e.data['service'] as String,
218+
e.data['service'] as String?,
216219
Method(e.data['method'] as String),
217220
));
218221
}
@@ -470,7 +473,24 @@ FutureOr<void>? a;
470473
expect(result, equals({'a': 'b'}));
471474
}
472475

473-
Future<void> test_service_success_echo_nullResponse() async {
476+
Future<void>
477+
test_service_success_echo_nullResponse_with_empty_params() async {
478+
await initializeServer();
479+
await sendConnectToDtdRequest(registerExperimentalHandlers: true);
480+
481+
var response = await dtd.connection.call(
482+
lspServiceName,
483+
CustomMethods.experimentalEcho.toString(),
484+
params: const <String, Object?>{},
485+
);
486+
487+
var result = response.result['result'] as Map<String, Object?>?;
488+
489+
expect(response.type, 'Null');
490+
expect(result, isNull);
491+
}
492+
493+
Future<void> test_service_success_echo_nullResponse_with_null_params() async {
474494
await initializeServer();
475495
await sendConnectToDtdRequest(registerExperimentalHandlers: true);
476496

@@ -512,7 +532,10 @@ void [!myFun^ction!]() {}
512532
await initializeServer();
513533
await sendConnectToDtdRequest();
514534

515-
expect(availableMethods, isNotEmpty);
535+
var lspMethods = availableMethods.where(
536+
(serviceMethod) => serviceMethod.$1 == 'Lsp',
537+
);
538+
expect(lspMethods, isNotEmpty);
516539

517540
// Send a request to the server to connect to DTD. This will only complete
518541
// once all services are registered, however there's no guarantee about the
@@ -521,9 +544,9 @@ void [!myFun^ction!]() {}
521544
await shutdownServer();
522545

523546
// Wait for the services to be unregistered.
524-
while (availableMethods.isNotEmpty) {
547+
while (lspMethods.isNotEmpty) {
525548
await pumpEventQueue(times: 5000);
526549
}
527-
expect(availableMethods, isEmpty);
550+
expect(lspMethods, isEmpty);
528551
}
529552
}

pkg/dart_service_protocol_shared/CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
## 0.0.3-wip
1+
## 0.0.3
22
- Update sdk constraint to '^3.5.0'
3+
- Added `ClientServiceInfo.fromJson` and `ClientServiceInfo.toJson` methods.
4+
- Added `ClientServiceMethodInfo.fromJson` and `ClientServiceMethodInfo.toJson`
5+
methods.
36

47
## 0.0.2
58
- Fixed an issue with streamNotify data type being too specific.

pkg/dart_service_protocol_shared/lib/src/client.dart

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,26 @@ class ClientServiceInfo {
4848
ClientServiceInfo(this.name, [Map<String, ClientServiceMethodInfo>? methods])
4949
: methods = methods ?? {};
5050

51+
/// Deserializes a [json] object to create a [ClientServiceInfo] object.
52+
static ClientServiceInfo fromJson(Map<String, Object?> json) {
53+
if (json case {_kName: final String name, _kMethods: final List methods}) {
54+
return ClientServiceInfo(
55+
name,
56+
<String, ClientServiceMethodInfo>{
57+
for (final method in methods
58+
.cast<Map<String, Object?>>()
59+
.map(ClientServiceMethodInfo.fromJson))
60+
method.name: method
61+
},
62+
);
63+
}
64+
throw ArgumentError('Unexpected JSON format: $json');
65+
}
66+
67+
static const _kName = 'name';
68+
69+
static const _kMethods = 'methods';
70+
5171
/// The name of the service.
5272
///
5373
/// A client can register multiple services each with multiple methods.
@@ -58,13 +78,35 @@ class ClientServiceInfo {
5878

5979
/// The service methods registered for this service.
6080
final Map<String, ClientServiceMethodInfo> methods;
81+
82+
/// Serializes this [ClientServiceInfo] object to JSON.
83+
Map<String, Object?> toJson() => {
84+
_kName: name,
85+
_kMethods: methods.values.map((m) => m.toJson()).toList(),
86+
};
6187
}
6288

6389
/// Information about an individual method of a service provided by a
6490
/// client.
6591
class ClientServiceMethodInfo {
6692
ClientServiceMethodInfo(this.name, [this.capabilities]);
6793

94+
/// Deserializes a [json] object to create a [ClientServiceMethodInfo] object.
95+
static ClientServiceMethodInfo fromJson(Map<String, Object?> json) {
96+
try {
97+
return ClientServiceMethodInfo(
98+
json[_kName] as String,
99+
json[_kCapabilities] as Map<String, Object?>?,
100+
);
101+
} catch (e) {
102+
throw ArgumentError('Unexpected JSON format: $json');
103+
}
104+
}
105+
106+
static const _kName = 'name';
107+
108+
static const _kCapabilities = 'capabilities';
109+
68110
/// The name of the method.
69111
///
70112
/// A client can register multiple methods for each service but can only use
@@ -74,6 +116,12 @@ class ClientServiceMethodInfo {
74116

75117
/// Optional capabilities of this service method provided by the client.
76118
final Map<String, Object?>? capabilities;
119+
120+
/// Serializes this [ClientServiceMethodInfo] object to JSON.
121+
Map<String, Object?> toJson() => {
122+
_kName: name,
123+
if (capabilities != null) _kCapabilities: capabilities,
124+
};
77125
}
78126

79127
/// Used for keeping track and managing clients that are connected to a given

pkg/dart_service_protocol_shared/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: dart_service_protocol_shared
22
description: A package that implements service extensions and stream managers.
33

4-
version: 0.0.3-wip
4+
version: 0.0.3
55
repository: https://github.com/dart-lang/sdk/tree/main/pkg/dart_service_protocol_shared
66

77
environment:

pkg/dart_service_protocol_shared/test/client_test.dart

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,82 @@ void main() {
128128
);
129129
});
130130
});
131+
132+
group('ClientServiceInfo', () {
133+
test('toJson and fromJson', () {
134+
final serviceInfo = ClientServiceInfo(
135+
'testService',
136+
{
137+
'method1': ClientServiceMethodInfo('method1', {'capability': true}),
138+
'method2': ClientServiceMethodInfo('method2'),
139+
},
140+
);
141+
142+
final json = serviceInfo.toJson();
143+
final deserialized = ClientServiceInfo.fromJson(json);
144+
145+
expect(deserialized.name, 'testService');
146+
expect(deserialized.methods.length, 2);
147+
expect(deserialized.methods['method1']?.name, 'method1');
148+
expect(
149+
deserialized.methods['method1']?.capabilities,
150+
{'capability': true},
151+
);
152+
expect(deserialized.methods['method2']?.name, 'method2');
153+
expect(deserialized.methods['method2']?.capabilities, isNull);
154+
});
155+
156+
test('fromJson throws with invalid json', () {
157+
expect(
158+
() => ClientServiceInfo.fromJson({
159+
'name': 'testService',
160+
'methods': [
161+
{'bad': 'format'}
162+
],
163+
}),
164+
throwsA(isA<ArgumentError>()),
165+
);
166+
expect(
167+
() => ClientServiceInfo.fromJson({
168+
'methods': [
169+
{'name': 'method1'}
170+
],
171+
}),
172+
throwsA(isA<ArgumentError>()),
173+
reason: 'Missing "name" field in top-level map',
174+
);
175+
});
176+
});
177+
178+
group('ClientServiceMethodInfo', () {
179+
test('toJson and parse', () {
180+
final methodInfo =
181+
ClientServiceMethodInfo('testMethod', {'capability': 123});
182+
183+
final json = methodInfo.toJson();
184+
final deserialized = ClientServiceMethodInfo.fromJson(json);
185+
186+
expect(deserialized.name, 'testMethod');
187+
expect(deserialized.capabilities, {'capability': 123});
188+
});
189+
190+
test('toJson and parse without capabilities', () {
191+
final methodInfo = ClientServiceMethodInfo('testMethod');
192+
193+
final json = methodInfo.toJson();
194+
final deserialized = ClientServiceMethodInfo.fromJson(json);
195+
196+
expect(deserialized.name, 'testMethod');
197+
expect(deserialized.capabilities, isNull);
198+
});
199+
});
200+
201+
test('ClientServiceMethodInfo throws with invalid json', () {
202+
expect(
203+
() => ClientServiceMethodInfo.fromJson({
204+
'bad': 'format',
205+
}),
206+
throwsA(isA<ArgumentError>()),
207+
);
208+
});
131209
}

pkg/dtd/CHANGELOG.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1-
## 2.6.0
1+
## 3.0.0
22

3-
- Add `ConnectedAppService` to store the connections to Dart and Flutter
3+
- Added `ConnectedAppService` to store the connections to Dart and Flutter
44
applications that DTD is aware of.
55
- Log exceptions from invalid `streamNotify` events.
6+
- Added `getRegisteredServices` API.
7+
- Added a new response type `RegisteredServicesResponse`.
8+
- **Breaking Change**: Changed the `serviceName` parameter for the
9+
`DartToolingDaemon.call` method to have type `String?` instead of `String.`
10+
- **Breaking Change**: When the `params` parameter for the
11+
`DartToolingDaemon.call` method is null, pass the null value along to the client
12+
peer request instead of sending an empty Map value.
613

714
## 2.5.1
815

pkg/dtd/example/dtd_service_example.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,13 @@ void main(List<String> args) async {
3434
clientB.onEvent('Service').listen((e) {
3535
switch (e.kind) {
3636
case 'ServiceRegistered':
37-
serviceRegisteredCompleted.complete();
37+
if (e.data['service'] == 'ExampleServer') {
38+
serviceRegisteredCompleted.complete();
39+
}
3840
case 'ServiceUnregistered':
39-
serviceUnregisteredCompleted.complete();
41+
if (e.data['service'] == 'ExampleServer') {
42+
serviceUnregisteredCompleted.complete();
43+
}
4044
}
4145
print(jsonEncode({'stream': e.stream, 'kind': e.kind, 'data': e.data}));
4246
});

0 commit comments

Comments
 (0)