Drop the Server and IOServer classes

[natebosch]

IOServer has some usage, unlike ServerHandler.

The usage that it does have is a little clunky. There is only 1 place I've found where the Server supertype is used, and there it isn't an important feature. https://github.com/flutter/flutter/blob/0a62694532340013db39b17cb1e50e21f31cd936/packages/flutter_tools/lib/src/test/flutter_web_platform.dart#L169

Most of the usage could be replaced by HttpServer from dart:io and inlining the serveRequests call in place of calling Server.bind.

There is some usage of one detail - we have some code to translate from a HttpServer.address to a Uri which is used in a few places. This isn't really a detail tied to shelf, but it's the one bit where client won't have a trivial migration path.

We might want to consider adding this utility separately to ease the migration.

extension ServerUri on HttpServer {
  Uri get url {
    if (address.isLoopback) {
      return Uri(scheme: 'http', host: 'localhost', port: port);
    }

    // IPv6 addresses in URLs need to be enclosed in square brackets to avoid
    // URL ambiguity with the ":" in the address.
    if (address.type == InternetAddressType.IPv6) {
      return Uri(
          scheme: 'http',
          host: '[${address.address}]',
          port: port);
    }

    return Uri(scheme: 'http', host: address.address, port: port);
  }
}

[natebosch]

One risk of adding this as a separate utility - it might bring a bigger expectation that it works... and our existing implementation is buggy... it assumes the scheme instead of checking for instance.

[natebosch]

For reference, here is the diff in pub if we drop IOServer.

diff --git a/test/descriptor_server.dart b/test/descriptor_server.dart
index a0235187..9e50eb17 100644
--- a/test/descriptor_server.dart
+++ b/test/descriptor_server.dart
@@ -5,6 +5,7 @@
 // @dart=2.10
 
 import 'dart:async';
+import 'dart:io';
 
 import 'package:path/path.dart' as p;
 import 'package:shelf/shelf.dart' as shelf;
@@ -42,10 +43,10 @@ Future serve([List<d.Descriptor> contents]) async {
 
 class DescriptorServer {
   /// The underlying server.
-  final shelf.Server _server;
+  final HttpServer _server;
 
   /// A future that will complete to the port used for the server.
-  int get port => _server.url.port;
+  int get port => _server.port;
 
   /// The list of paths that have been requested from this server.
   final requestedPaths = <String>[];
@@ -66,14 +67,14 @@ class DescriptorServer {
   /// This server exists only for the duration of the pub run. Subsequent calls
   /// to [serve] replace the previous server.
   static Future<DescriptorServer> start() async =>
-      DescriptorServer._(await shelf_io.IOServer.bind('localhost', 0));
+      DescriptorServer._(await HttpServer.bind('localhost', 0));
 
   /// Creates a server that reports an error if a request is ever received.
   static Future<DescriptorServer> errors() async =>
-      DescriptorServer._(await shelf_io.IOServer.bind('localhost', 0));
+      DescriptorServer._(await HttpServer.bind('localhost', 0));
 
   DescriptorServer._(this._server) : _baseDir = d.dir('serve-dir', []) {
-    _server.mount((request) async {
+    shelf_io.serveRequests(_server, (request) async {
       final pathWithInitialSlash = '/${request.url.path}';
       final key = extraHandlers.keys.firstWhere((pattern) {
         final match = pattern.matchAsPrefix(pathWithInitialSlash);
diff --git a/test/get/git/check_out_test.dart b/test/get/git/check_out_test.dart
index 194e9807..a778e8de 100644
--- a/test/get/git/check_out_test.dart
+++ b/test/get/git/check_out_test.dart
@@ -55,7 +55,7 @@ void main() {
         await _serveDirectory(p.join(descriptor.io.path, '.git'), funkyName);
 
     await d.appDir({
-      'foo': {'git': 'http://localhost:${server.url.port}/$funkyName'}
+      'foo': {'git': 'http://localhost:${server.port}/$funkyName'}
     }).create();
 
     await pubGet();
@@ -71,9 +71,9 @@ void main() {
   });
 }
 
-Future<shelf.Server> _serveDirectory(String dir, String prefix) async {
-  final server = await shelf_io.IOServer.bind('localhost', 0);
-  server.mount((request) async {
+Future<HttpServer> _serveDirectory(String dir, String prefix) async {
+  final server = await HttpServer.bind('localhost', 0);
+  shelf_io.serveRequests(server, (request) async {
     final path = request.url.path.substring(prefix.length + 1);
     try {
       return shelf.Response.ok(await File(p.join(dir, path)).readAsBytes());

[natebosch]

Pub didn't really use the url functionality, but flutter_tools does. However there we know we'd always hit the isLoopback conditional and it doesn't look too bad to inline the Uri construction.

diff --git a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart
index cd0a534cb2..b67169d354 100644
--- a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart
+++ b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart
@@ -5,6 +5,7 @@
 // @dart = 2.8
 
 import 'dart:async';
+import 'dart:io';
 import 'dart:typed_data';
 
 import 'package:async/async.dart';
@@ -40,7 +41,7 @@ import 'flutter_web_goldens.dart';
 import 'test_compiler.dart';
 
 class FlutterWebPlatform extends PlatformPlugin {
-  FlutterWebPlatform._(this._server, this._config, this._root, {
+  FlutterWebPlatform._(this._server, this.url, this._config, this._root, {
     FlutterProject flutterProject,
     String shellPath,
     this.updateGoldens,
@@ -73,7 +74,7 @@ class FlutterWebPlatform extends PlatformPlugin {
           serveFilesOutsidePath: true,
         ))
         .add(_packageFilesHandler);
-    _server.mount(cascade.handler);
+    shelf_io.serveRequests(_server, cascade.handler);
     _testGoldenComparator = TestGoldenComparator(
       shellPath,
       () => TestCompiler(buildInfo, flutterProject),
@@ -118,7 +119,7 @@ class FlutterWebPlatform extends PlatformPlugin {
     @required Artifacts artifacts,
     @required ProcessManager processManager,
   }) async {
-    final shelf_io.IOServer server = shelf_io.IOServer(await HttpMultiServer.loopback(0));
+    final HttpServer server = await HttpMultiServer.loopback(0);
     final PackageConfig packageConfig = await loadPackageConfigWithLogging(
       fileSystem.file(fileSystem.path.join(
         Cache.flutterRoot,
@@ -131,6 +132,7 @@ class FlutterWebPlatform extends PlatformPlugin {
     );
     return FlutterWebPlatform._(
       server,
+      Uri(scheme: 'http', host: 'localhost', port: server.port),
       Configuration.current.change(pauseAfterLoad: pauseAfterLoad),
       root,
       flutterProject: flutterProject,
@@ -166,8 +168,8 @@ class FlutterWebPlatform extends PlatformPlugin {
   }
 
   final Configuration _config;
-  final shelf.Server _server;
-  Uri get url => _server.url;
+  final HttpServer _server;
+  final Uri url;
 
   /// The ahem text file.
   File get _ahem => _fileSystem.file(_fileSystem.path.join(

[ykmnkmi]

Any updates? Should I rely upon this classes? Does an adapter should implement the Server class?