Skip to content
This repository has been archived by the owner on Sep 14, 2021. It is now read-only.

Add helper to convert Observatory URI to WS URI #232

Merged
merged 3 commits into from
Apr 23, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions dart/lib/utils.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) 2019, 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.

/// Map the URI (which may already be Observatory web app) to a WebSocket URI
/// for the VM service.
///
/// If the URI is already a VM Service WebSocket URI it will not be modified.
Uri getVmWsUriFromObservatoryUri(Uri uri) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we may rename this to something like convertToWebSocketUrl({@required String serviceProtocolUrl}).

final isSecure = uri.isScheme('wss') || uri.isScheme('https');
final scheme = isSecure ? 'wss' : 'ws';

final path = uri.path.endsWith('/ws')
? uri.path
: (uri.path.endsWith('/') ? '${uri.path}ws' : '${uri.path}/ws');

return uri.replace(scheme: scheme, path: path);
}
66 changes: 66 additions & 0 deletions dart/test/utils_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) 2019, 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 'package:test/test.dart';
import 'package:vm_service_lib/utils.dart';

@TestOn('vm')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test should run everywhere, not just on the VM, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I've removed that.

void main() {
group('getVmWsUriFromObservatoryUri', () {
void check(String input, String expected) {
final inputUri = Uri.parse(input);
final actualUri = getVmWsUriFromObservatoryUri(inputUri);
expect(actualUri.toString(), equals(expected));
}

test('handles http URIs',
() => check('http://localhost:123/', 'ws://localhost:123/ws'));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this formatting is a bit odd looking. I would suggest using () {
check('http://localhost:123/', 'wss://localhost:123/ws');
}

No need to write this code but if you have a similar situation in the future it might make sense to have a
Map<String, String> of input to expected uris and then have a single test case that
iterates through all key-value pairs in the map expecting that the uris match.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started it that way, but wasn't sure if having only a single test name without descriptions was good. However, it did turn out a mess, so I've changed it to a Map<String, String>. PTAL!

test('handles https URIs',
() => check('https://localhost:123/', 'wss://localhost:123/ws'));
test('handles ws URIs',
() => check('ws://localhost:123/', 'ws://localhost:123/ws'));
test('handles wss URIs',
() => check('wss://localhost:123/', 'wss://localhost:123/ws'));
test(
'handles http URIs with tokens',
() => check(
'http://localhost:123/ABCDEF=/', 'ws://localhost:123/ABCDEF=/ws'));
test(
'handles https URIs with tokens',
() => check('https://localhost:123/ABCDEF=/',
'wss://localhost:123/ABCDEF=/ws'));
test(
'handles ws URIs with tokens',
() => check(
'ws://localhost:123/ABCDEF=/', 'ws://localhost:123/ABCDEF=/ws'));
test(
'handles wss URIs with tokens',
() => check(
'wss://localhost:123/ABCDEF=/', 'wss://localhost:123/ABCDEF=/ws'));
test('handles http URIs without trailing slashes',
() => check('http://localhost:123', 'ws://localhost:123/ws'));
test('handles https URIs without trailing slashes',
() => check('https://localhost:123', 'wss://localhost:123/ws'));
test('handles ws URIs without trailing slashes',
() => check('ws://localhost:123', 'ws://localhost:123/ws'));
test('handles wss URIs without trailing slashes',
() => check('wss://localhost:123', 'wss://localhost:123/ws'));
test(
'handles http URIs without trailing slashes with tokens',
() => check(
'http://localhost:123/ABCDEF=', 'ws://localhost:123/ABCDEF=/ws'));
test(
'handles https URIs without trailing slashes with tokens',
() => check(
'https://localhost:123/ABCDEF=', 'wss://localhost:123/ABCDEF=/ws'));
test(
'handles ws URIs without trailing slashes with tokens',
() => check(
'ws://localhost:123/ABCDEF=', 'ws://localhost:123/ABCDEF=/ws'));
test(
'handles wss URIs without trailing slashes with tokens',
() => check(
'wss://localhost:123/ABCDEF=', 'wss://localhost:123/ABCDEF=/ws'));
});
}