Detect exact device ID matches quickly (#62070)
diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 1130db6..1723582 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart
@@ -104,23 +104,51 @@ bool get hasSpecifiedAllDevices => _specifiedDeviceId == 'all'; Future<List<Device>> getDevicesById(String deviceId) async { - final List<Device> devices = await getAllConnectedDevices(); - deviceId = deviceId.toLowerCase(); + final String lowerDeviceId = deviceId.toLowerCase(); bool exactlyMatchesDeviceId(Device device) => - device.id.toLowerCase() == deviceId || - device.name.toLowerCase() == deviceId; + device.id.toLowerCase() == lowerDeviceId || + device.name.toLowerCase() == lowerDeviceId; bool startsWithDeviceId(Device device) => - device.id.toLowerCase().startsWith(deviceId) || - device.name.toLowerCase().startsWith(deviceId); + device.id.toLowerCase().startsWith(lowerDeviceId) || + device.name.toLowerCase().startsWith(lowerDeviceId); - final Device exactMatch = devices.firstWhere( - exactlyMatchesDeviceId, orElse: () => null); - if (exactMatch != null) { - return <Device>[exactMatch]; + // Some discoverers have hard-coded device IDs and return quickly, and others + // shell out to other processes and can take longer. + // Process discoverers as they can return results, so if an exact match is + // found quickly, we don't wait for all the discoverers to complete. + final List<Device> prefixMatches = <Device>[]; + final Completer<Device> exactMatchCompleter = Completer<Device>(); + final List<Future<List<Device>>> futureDevices = <Future<List<Device>>>[ + for (final DeviceDiscovery discoverer in _platformDiscoverers) + discoverer + .devices + .then((List<Device> devices) { + for (final Device device in devices) { + if (exactlyMatchesDeviceId(device)) { + exactMatchCompleter.complete(device); + return null; + } + if (startsWithDeviceId(device)) { + prefixMatches.add(device); + } + } + return null; + }, onError: (dynamic error, StackTrace stackTrace) { + // Return matches from other discoverers even if one fails. + globals.printTrace('Ignored error discovering $deviceId: $error'); + }) + ]; + + // Wait for an exact match, or for all discoverers to return results. + await Future.any<dynamic>(<Future<dynamic>>[ + exactMatchCompleter.future, + Future.wait<List<Device>>(futureDevices), + ]); + + if (exactMatchCompleter.isCompleted) { + return <Device>[await exactMatchCompleter.future]; } - - // Match on a id or name starting with [deviceId]. - return devices.where(startsWithDeviceId).toList(); + return prefixMatches; } /// Returns the list of connected devices, filtered by any user-specified device id.
diff --git a/packages/flutter_tools/test/general.shard/device_test.dart b/packages/flutter_tools/test/general.shard/device_test.dart index 0fcb542..1c1b3af 100644 --- a/packages/flutter_tools/test/general.shard/device_test.dart +++ b/packages/flutter_tools/test/general.shard/device_test.dart
@@ -4,6 +4,7 @@ import 'dart:async'; +import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/build_info.dart'; @@ -22,9 +23,11 @@ void main() { MockCache cache; + BufferLogger logger; setUp(() { cache = MockCache(); + logger = BufferLogger.test(); when(cache.dyLdLibEntry).thenReturn(const MapEntry<String, String>('foo', 'bar')); }); @@ -41,25 +44,67 @@ Cache: () => cache, }); - testUsingContext('getDeviceById', () async { + testUsingContext('getDeviceById exact matcher', () async { final FakeDevice device1 = FakeDevice('Nexus 5', '0553790d0a4e726f'); final FakeDevice device2 = FakeDevice('Nexus 5X', '01abfc49119c410e'); final FakeDevice device3 = FakeDevice('iPod touch', '82564b38861a9a5'); final List<Device> devices = <Device>[device1, device2, device3]; - final DeviceManager deviceManager = TestDeviceManager(devices); + + // Include different device discoveries: + // 1. One that never completes to prove the first exact match is + // returned quickly. + // 2. One that throws, to prove matches can return when some succeed + // and others fail. + // 3. A device discoverer that succeeds. + final DeviceManager deviceManager = TestDeviceManager( + devices, + testLongPollingDeviceDiscovery: true, + testThrowingDeviceDiscovery: true, + ); Future<void> expectDevice(String id, List<Device> expected) async { expect(await deviceManager.getDevicesById(id), expected); } await expectDevice('01abfc49119c410e', <Device>[device2]); + expect(logger.traceText, contains('Ignored error discovering 01abfc49119c410e')); await expectDevice('Nexus 5X', <Device>[device2]); + expect(logger.traceText, contains('Ignored error discovering Nexus 5X')); await expectDevice('0553790d0a4e726f', <Device>[device1]); - await expectDevice('Nexus 5', <Device>[device1]); - await expectDevice('0553790', <Device>[device1]); - await expectDevice('Nexus', <Device>[device1, device2]); + expect(logger.traceText, contains('Ignored error discovering 0553790d0a4e726f')); }, overrides: <Type, Generator>{ Artifacts: () => Artifacts.test(), Cache: () => cache, + Logger: () => logger, + }); + + testUsingContext('getDeviceById prefix matcher', () async { + final FakeDevice device1 = FakeDevice('Nexus 5', '0553790d0a4e726f'); + final FakeDevice device2 = FakeDevice('Nexus 5X', '01abfc49119c410e'); + final FakeDevice device3 = FakeDevice('iPod touch', '82564b38861a9a5'); + final List<Device> devices = <Device>[device1, device2, device3]; + + // Include different device discoveries: + // 1. One that throws, to prove matches can return when some succeed + // and others fail. + // 2. A device discoverer that succeeds. + final DeviceManager deviceManager = TestDeviceManager( + devices, + testThrowingDeviceDiscovery: true + ); + + Future<void> expectDevice(String id, List<Device> expected) async { + expect(await deviceManager.getDevicesById(id), expected); + } + await expectDevice('Nexus 5', <Device>[device1]); + expect(logger.traceText, contains('Ignored error discovering Nexus 5')); + await expectDevice('0553790', <Device>[device1]); + expect(logger.traceText, contains('Ignored error discovering 0553790')); + await expectDevice('Nexus', <Device>[device1, device2]); + expect(logger.traceText, contains('Ignored error discovering Nexus')); + }, overrides: <Type, Generator>{ + Artifacts: () => Artifacts.test(), + Cache: () => cache, + Logger: () => logger, }); testUsingContext('getAllConnectedDevices caches', () async { @@ -374,16 +419,27 @@ } class TestDeviceManager extends DeviceManager { - TestDeviceManager(List<Device> allDevices) { - _deviceDiscoverer = FakePollingDeviceDiscovery(); + TestDeviceManager(List<Device> allDevices, { + bool testLongPollingDeviceDiscovery = false, + bool testThrowingDeviceDiscovery = false, + }) { + _fakeDeviceDiscoverer = FakePollingDeviceDiscovery(); + _deviceDiscoverers = <DeviceDiscovery>[ + if (testLongPollingDeviceDiscovery) + LongPollingDeviceDiscovery(), + if (testThrowingDeviceDiscovery) + ThrowingPollingDeviceDiscovery(), + _fakeDeviceDiscoverer, + ]; resetDevices(allDevices); } @override - List<DeviceDiscovery> get deviceDiscoverers => <DeviceDiscovery>[_deviceDiscoverer]; - FakePollingDeviceDiscovery _deviceDiscoverer; + List<DeviceDiscovery> get deviceDiscoverers => _deviceDiscoverers; + List<DeviceDiscovery> _deviceDiscoverers; + FakePollingDeviceDiscovery _fakeDeviceDiscoverer; void resetDevices(List<Device> allDevices) { - _deviceDiscoverer.setDevices(allDevices); + _fakeDeviceDiscoverer.setDevices(allDevices); } bool isAlwaysSupportedOverride;
diff --git a/packages/flutter_tools/test/src/mocks.dart b/packages/flutter_tools/test/src/mocks.dart index 66c5aa9..e00ac54 100644 --- a/packages/flutter_tools/test/src/mocks.dart +++ b/packages/flutter_tools/test/src/mocks.dart
@@ -526,6 +526,48 @@ Stream<Device> get onRemoved => _onRemovedController.stream; } +class LongPollingDeviceDiscovery extends PollingDeviceDiscovery { + LongPollingDeviceDiscovery() : super('forever'); + + final Completer<List<Device>> _completer = Completer<List<Device>>(); + + @override + Future<List<Device>> pollingGetDevices({ Duration timeout }) async { + return _completer.future; + } + + @override + Future<void> stopPolling() async { + _completer.complete(); + } + + @override + Future<void> dispose() async { + _completer.complete(); + } + + @override + bool get supportsPlatform => true; + + @override + bool get canListAnything => true; +} + +class ThrowingPollingDeviceDiscovery extends PollingDeviceDiscovery { + ThrowingPollingDeviceDiscovery() : super('throw'); + + @override + Future<List<Device>> pollingGetDevices({ Duration timeout }) async { + throw const ProcessException('fake-discovery', <String>[]); + } + + @override + bool get supportsPlatform => true; + + @override + bool get canListAnything => true; +} + class MockIosProject extends Mock implements IosProject { static const String bundleId = 'com.example.test'; static const String appBundleName = 'My Super Awesome App.app';