From f62e98844c67cb5239b7199c046695ee00f4eb61 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Tue, 11 Sep 2018 13:17:49 -0700 Subject: [PATCH 1/6] use config specific imports, stop exporting io_client.dart --- lib/browser_client.dart | 105 +---------------------------- lib/http.dart | 1 - lib/io_client.dart | 5 ++ lib/src/browser_client.dart | 110 +++++++++++++++++++++++++++++++ lib/src/client.dart | 12 ++-- lib/src/client_stub.dart | 8 +++ lib/src/io_client.dart | 2 + lib/src/multipart_file.dart | 16 ++--- lib/src/multipart_file_io.dart | 23 +++++++ lib/src/multipart_file_stub.dart | 13 ++++ test/io/client_test.dart | 3 +- 11 files changed, 176 insertions(+), 122 deletions(-) create mode 100644 lib/io_client.dart create mode 100644 lib/src/browser_client.dart create mode 100644 lib/src/client_stub.dart create mode 100644 lib/src/multipart_file_io.dart create mode 100644 lib/src/multipart_file_stub.dart diff --git a/lib/browser_client.dart b/lib/browser_client.dart index f767390741..2cd0e5c7a4 100644 --- a/lib/browser_client.dart +++ b/lib/browser_client.dart @@ -2,107 +2,4 @@ // 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 'dart:async'; -import 'dart:html'; -import 'dart:typed_data'; - -import 'src/base_client.dart'; -import 'src/base_request.dart'; -import 'src/byte_stream.dart'; -import 'src/exception.dart'; -import 'src/streamed_response.dart'; - -// TODO(nweiz): Move this under src/, re-export from lib/http.dart, and use this -// automatically from [new Client] once sdk#24581 is fixed. - -/// A `dart:html`-based HTTP client that runs in the browser and is backed by -/// XMLHttpRequests. -/// -/// This client inherits some of the limitations of XMLHttpRequest. It ignores -/// the [BaseRequest.contentLength], [BaseRequest.persistentConnection], -/// [BaseRequest.followRedirects], and [BaseRequest.maxRedirects] fields. It is -/// also unable to stream requests or responses; a request will only be sent and -/// a response will only be returned once all the data is available. -class BrowserClient extends BaseClient { - /// The currently active XHRs. - /// - /// These are aborted if the client is closed. - final _xhrs = new Set(); - - /// Creates a new HTTP client. - BrowserClient(); - - /// Whether to send credentials such as cookies or authorization headers for - /// cross-site requests. - /// - /// Defaults to `false`. - bool withCredentials = false; - - /// Sends an HTTP request and asynchronously returns the response. - Future send(BaseRequest request) async { - var bytes = await request.finalize().toBytes(); - var xhr = new HttpRequest(); - _xhrs.add(xhr); - _openHttpRequest(xhr, request.method, request.url.toString(), asynch: true); - xhr.responseType = 'blob'; - xhr.withCredentials = withCredentials; - request.headers.forEach(xhr.setRequestHeader); - - var completer = new Completer(); - xhr.onLoad.first.then((_) { - // TODO(nweiz): Set the response type to "arraybuffer" when issue 18542 - // is fixed. - var blob = xhr.response == null ? new Blob([]) : xhr.response; - var reader = new FileReader(); - - reader.onLoad.first.then((_) { - var body = reader.result as Uint8List; - completer.complete(new StreamedResponse( - new ByteStream.fromBytes(body), xhr.status, - contentLength: body.length, - request: request, - headers: xhr.responseHeaders, - reasonPhrase: xhr.statusText)); - }); - - reader.onError.first.then((error) { - completer.completeError( - new ClientException(error.toString(), request.url), - StackTrace.current); - }); - - reader.readAsArrayBuffer(blob); - }); - - xhr.onError.first.then((_) { - // Unfortunately, the underlying XMLHttpRequest API doesn't expose any - // specific information about the error itself. - completer.completeError( - new ClientException("XMLHttpRequest error.", request.url), - StackTrace.current); - }); - - xhr.send(bytes); - - try { - return await completer.future; - } finally { - _xhrs.remove(xhr); - } - } - - // TODO(nweiz): Remove this when sdk#24637 is fixed. - void _openHttpRequest(HttpRequest request, String method, String url, - {bool asynch, String user, String password}) { - request.open(method, url, async: asynch, user: user, password: password); - } - - /// Closes the client. - /// - /// This terminates all active requests. - void close() { - for (var xhr in _xhrs) { - xhr.abort(); - } - } -} +export 'src/browser_client.dart' show BrowserClient; diff --git a/lib/http.dart b/lib/http.dart index 3e16285829..e682ed1a6a 100644 --- a/lib/http.dart +++ b/lib/http.dart @@ -16,7 +16,6 @@ export 'src/base_response.dart'; export 'src/byte_stream.dart'; export 'src/client.dart'; export 'src/exception.dart'; -export 'src/io_client.dart'; export 'src/multipart_file.dart'; export 'src/multipart_request.dart'; export 'src/request.dart'; diff --git a/lib/io_client.dart b/lib/io_client.dart new file mode 100644 index 0000000000..4078ff63fd --- /dev/null +++ b/lib/io_client.dart @@ -0,0 +1,5 @@ +// Copyright (c) 2018, 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. + +export 'src/io_client.dart'; diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart new file mode 100644 index 0000000000..da38812d02 --- /dev/null +++ b/lib/src/browser_client.dart @@ -0,0 +1,110 @@ +// Copyright (c) 2018, 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 'dart:async'; +import 'dart:html'; +import 'dart:typed_data'; + +import 'base_client.dart'; +import 'base_request.dart'; +import 'byte_stream.dart'; +import 'exception.dart'; +import 'streamed_response.dart'; + +BaseClient createClient() => BrowserClient(); + +// TODO(nweiz): Move this under src/, re-export from lib/http.dart, and use this +// automatically from [new Client] once sdk#24581 is fixed. + +/// A `dart:html`-based HTTP client that runs in the browser and is backed by +/// XMLHttpRequests. +/// +/// This client inherits some of the limitations of XMLHttpRequest. It ignores +/// the [BaseRequest.contentLength], [BaseRequest.persistentConnection], +/// [BaseRequest.followRedirects], and [BaseRequest.maxRedirects] fields. It is +/// also unable to stream requests or responses; a request will only be sent and +/// a response will only be returned once all the data is available. +class BrowserClient extends BaseClient { + /// The currently active XHRs. + /// + /// These are aborted if the client is closed. + final _xhrs = new Set(); + + /// Creates a new HTTP client. + BrowserClient(); + + /// Whether to send credentials such as cookies or authorization headers for + /// cross-site requests. + /// + /// Defaults to `false`. + bool withCredentials = false; + + /// Sends an HTTP request and asynchronously returns the response. + Future send(BaseRequest request) async { + var bytes = await request.finalize().toBytes(); + var xhr = new HttpRequest(); + _xhrs.add(xhr); + _openHttpRequest(xhr, request.method, request.url.toString(), asynch: true); + xhr.responseType = 'blob'; + xhr.withCredentials = withCredentials; + request.headers.forEach(xhr.setRequestHeader); + + var completer = new Completer(); + xhr.onLoad.first.then((_) { + // TODO(nweiz): Set the response type to "arraybuffer" when issue 18542 + // is fixed. + var blob = xhr.response == null ? new Blob([]) : xhr.response; + var reader = new FileReader(); + + reader.onLoad.first.then((_) { + var body = reader.result as Uint8List; + completer.complete(new StreamedResponse( + new ByteStream.fromBytes(body), xhr.status, + contentLength: body.length, + request: request, + headers: xhr.responseHeaders, + reasonPhrase: xhr.statusText)); + }); + + reader.onError.first.then((error) { + completer.completeError( + new ClientException(error.toString(), request.url), + StackTrace.current); + }); + + reader.readAsArrayBuffer(blob); + }); + + xhr.onError.first.then((_) { + // Unfortunately, the underlying XMLHttpRequest API doesn't expose any + // specific information about the error itself. + completer.completeError( + new ClientException("XMLHttpRequest error.", request.url), + StackTrace.current); + }); + + xhr.send(bytes); + + try { + return await completer.future; + } finally { + _xhrs.remove(xhr); + } + } + + // TODO(nweiz): Remove this when sdk#24637 is fixed. + void _openHttpRequest(HttpRequest request, String method, String url, + {bool asynch, String user, String password}) { + request.open(method, url, async: asynch, user: user, password: password); + } + + /// Closes the client. + /// + /// This terminates all active requests. + void close() { + for (var xhr in _xhrs) { + xhr.abort(); + } + } +} diff --git a/lib/src/client.dart b/lib/src/client.dart index bb78314c44..aa0bf3e53c 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -8,7 +8,9 @@ import 'dart:typed_data'; import 'base_client.dart'; import 'base_request.dart'; -import 'io_client.dart'; +import 'client_stub.dart' + if (dart.library.html) 'browser_client.dart' + if (dart.library.io) 'io_client.dart'; import 'response.dart'; import 'streamed_response.dart'; @@ -24,10 +26,10 @@ import 'streamed_response.dart'; abstract class Client { /// Creates a new client. /// - /// Currently this will create an [IOClient] if `dart:io` is available and - /// throw an [UnsupportedError] otherwise. In the future, it will create a - /// [BrowserClient] if `dart:html` is available. - factory Client() => new IOClient(); + /// Currently this will create an `IOClient` if `dart:io` is available and + /// a `BrowserClient` if `dart:html` is available, otherwise it will throw + /// an unsupported error. + factory Client() => createClient(); /// Sends an HTTP HEAD request with the given headers to the given URL, which /// can be a [Uri] or a [String]. diff --git a/lib/src/client_stub.dart b/lib/src/client_stub.dart new file mode 100644 index 0000000000..0f30c154bb --- /dev/null +++ b/lib/src/client_stub.dart @@ -0,0 +1,8 @@ +// Copyright (c) 2018, 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 'base_client.dart'; + +BaseClient createClient() => throw UnsupportedError( + 'Cannot create a client without dart:html or dart:io.'); diff --git a/lib/src/io_client.dart b/lib/src/io_client.dart index 403fe00340..4d2b2c47bc 100644 --- a/lib/src/io_client.dart +++ b/lib/src/io_client.dart @@ -12,6 +12,8 @@ import 'base_request.dart'; import 'exception.dart'; import 'streamed_response.dart'; +BaseClient createClient() => IOClient(); + /// A `dart:io`-based HTTP client. /// /// This is the default client when running on the command line. diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index 32f2ada2eb..1e2b231eff 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -4,15 +4,14 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:io'; -import 'package:async/async.dart'; import 'package:http_parser/http_parser.dart'; -import 'package:path/path.dart' as path; import 'byte_stream.dart'; import 'utils.dart'; +import 'multipart_file_stub.dart' if (dart.library.io) 'multipart_file_io.dart'; + /// A file to be uploaded as part of a [MultipartRequest]. This doesn't need to /// correspond to a physical file. class MultipartFile { @@ -87,14 +86,9 @@ class MultipartFile { /// Throws an [UnsupportedError] if `dart:io` isn't supported in this /// environment. static Future fromPath(String field, String filePath, - {String filename, MediaType contentType}) async { - if (filename == null) filename = path.basename(filePath); - var file = new File(filePath); - var length = await file.length(); - var stream = new ByteStream(DelegatingStream.typed(file.openRead())); - return new MultipartFile(field, stream, length, - filename: filename, contentType: contentType); - } + {String filename, MediaType contentType}) => + multipartFileFromPath(field, filePath, + filename: filename, contentType: contentType); // Finalizes the file in preparation for it being sent as part of a // [MultipartRequest]. This returns a [ByteStream] that should emit the body diff --git a/lib/src/multipart_file_io.dart b/lib/src/multipart_file_io.dart new file mode 100644 index 0000000000..c49998a3f3 --- /dev/null +++ b/lib/src/multipart_file_io.dart @@ -0,0 +1,23 @@ +// Copyright (c) 2018, 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 'dart:async'; +import 'dart:io'; + +import 'package:async/async.dart'; +import 'package:http_parser/http_parser.dart'; +import 'package:path/path.dart' as p; + +import 'byte_stream.dart'; +import 'multipart_file.dart'; + +Future multipartFileFromPath(String field, String filePath, + {String filename, MediaType contentType}) async { + if (filename == null) filename = p.basename(filePath); + var file = new File(filePath); + var length = await file.length(); + var stream = new ByteStream(DelegatingStream.typed(file.openRead())); + return new MultipartFile(field, stream, length, + filename: filename, contentType: contentType); +} diff --git a/lib/src/multipart_file_stub.dart b/lib/src/multipart_file_stub.dart new file mode 100644 index 0000000000..78083bf745 --- /dev/null +++ b/lib/src/multipart_file_stub.dart @@ -0,0 +1,13 @@ +// Copyright (c) 2018, 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 'dart:async'; + +import 'package:http_parser/http_parser.dart'; + +import 'multipart_file.dart'; + +external Future multipartFileFromPath( + String field, String filePath, + {String filename, MediaType contentType}); diff --git a/test/io/client_test.dart b/test/io/client_test.dart index 410a87543c..04fca5ce1a 100644 --- a/test/io/client_test.dart +++ b/test/io/client_test.dart @@ -7,6 +7,7 @@ import 'dart:io'; import 'package:http/http.dart' as http; +import 'package:http/src/io_client.dart' as http_io; import 'package:test/test.dart'; import 'utils.dart'; @@ -56,7 +57,7 @@ void main() { expect( startServer().then((_) { var ioClient = new HttpClient(); - var client = new http.IOClient(ioClient); + var client = new http_io.IOClient(ioClient); var request = new http.StreamedRequest("POST", serverUrl); request.headers[HttpHeaders.contentTypeHeader] = 'application/json; charset=utf-8'; From 760c7b02188298dd28b5588ab2f8e207798bff56 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 12 Sep 2018 07:45:45 -0700 Subject: [PATCH 2/6] update changelog/pubspec --- CHANGELOG.md | 17 +++++++++++++++++ pubspec.yaml | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c1d1b02f4..d084ff77e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,20 @@ +## 0.12.0-dev + +### New Features + +* The regular `Client` factory constructor is now usable anywhere that `dart:io` + or `dart:html` are available, and will give you an `IoClient` or + `BrowserClient` respectively. +* The `package:http/http.dart` import is now safe to use on the web (or + anywhere that either `dart:io` or `dart:html` are available). + +### Breaking Changes + +* In order to use or reference the `IoClient` directly, you will need to import + the new `package:http/io_client.dart` import. This is typically only necessary + if you are passing a custom `HttpClient` instance to the constructor, in which + case you are already giving up support for web. + ## 0.11.3+17 * Use new Dart 2 constant names. This branch is only for allowing existing diff --git a/pubspec.yaml b/pubspec.yaml index 36be5ad1bb..f8387237af 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: http -version: 0.11.4-dev +version: 0.12.0-dev author: "Dart Team " homepage: https://github.com/dart-lang/http description: A composable, Future-based API for making HTTP requests. From 6880dca213e55c9ba09ed3125bf9c869f132ea6b Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 12 Sep 2018 08:28:53 -0700 Subject: [PATCH 3/6] only export IoClient from io_client.dart --- lib/io_client.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/io_client.dart b/lib/io_client.dart index 4078ff63fd..170acfba4e 100644 --- a/lib/io_client.dart +++ b/lib/io_client.dart @@ -2,4 +2,4 @@ // 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. -export 'src/io_client.dart'; +export 'src/io_client.dart' show IOClient; From 75a24ac8fb63d325a807f7a5bd1d3bd4269819f5 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 12 Sep 2018 08:34:03 -0700 Subject: [PATCH 4/6] always throw UnsupportedError from stubs, add uri_does_not_exists comments --- lib/src/client.dart | 3 +++ lib/src/multipart_file.dart | 5 ++++- lib/src/multipart_file_stub.dart | 7 ++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/src/client.dart b/lib/src/client.dart index aa0bf3e53c..78ea0a18a8 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -8,8 +8,11 @@ import 'dart:typed_data'; import 'base_client.dart'; import 'base_request.dart'; +// ignore: uri_does_not_exist import 'client_stub.dart' + // ignore: uri_does_not_exist if (dart.library.html) 'browser_client.dart' + // ignore: uri_does_not_exist if (dart.library.io) 'io_client.dart'; import 'response.dart'; import 'streamed_response.dart'; diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index 1e2b231eff..a173f67f68 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -10,7 +10,10 @@ import 'package:http_parser/http_parser.dart'; import 'byte_stream.dart'; import 'utils.dart'; -import 'multipart_file_stub.dart' if (dart.library.io) 'multipart_file_io.dart'; +// ignore: uri_does_not_exist +import 'multipart_file_stub.dart' + // ignore: uri_does_not_exist + if (dart.library.io) 'multipart_file_io.dart'; /// A file to be uploaded as part of a [MultipartRequest]. This doesn't need to /// correspond to a physical file. diff --git a/lib/src/multipart_file_stub.dart b/lib/src/multipart_file_stub.dart index 78083bf745..b2304415fe 100644 --- a/lib/src/multipart_file_stub.dart +++ b/lib/src/multipart_file_stub.dart @@ -8,6 +8,7 @@ import 'package:http_parser/http_parser.dart'; import 'multipart_file.dart'; -external Future multipartFileFromPath( - String field, String filePath, - {String filename, MediaType contentType}); +Future multipartFileFromPath(String field, String filePath, + {String filename, MediaType contentType}) => + throw UnsupportedError( + 'MultipartFile is only supported where dart:io is available.'); From 12fb01b7ddfdd8a916dff625e136523cf5d081df Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 12 Sep 2018 08:36:54 -0700 Subject: [PATCH 5/6] override package_resolver dependency --- pubspec.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pubspec.yaml b/pubspec.yaml index f8387237af..67730fef85 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,3 +14,6 @@ dependencies: dev_dependencies: test: ^1.3.0 + +dependency_overrides: + package_resolver: 1.0.4 From 0f1585cad2bc2c064ea78daab7a7256105646761 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 12 Sep 2018 11:08:41 -0700 Subject: [PATCH 6/6] code review update, add comments --- lib/src/browser_client.dart | 4 +--- lib/src/client_stub.dart | 1 + lib/src/io_client.dart | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart index da38812d02..4f49ec7671 100644 --- a/lib/src/browser_client.dart +++ b/lib/src/browser_client.dart @@ -12,11 +12,9 @@ import 'byte_stream.dart'; import 'exception.dart'; import 'streamed_response.dart'; +/// Used from conditional imports, matches the definition in `client_stub.dart`. BaseClient createClient() => BrowserClient(); -// TODO(nweiz): Move this under src/, re-export from lib/http.dart, and use this -// automatically from [new Client] once sdk#24581 is fixed. - /// A `dart:html`-based HTTP client that runs in the browser and is backed by /// XMLHttpRequests. /// diff --git a/lib/src/client_stub.dart b/lib/src/client_stub.dart index 0f30c154bb..1a34d50d7e 100644 --- a/lib/src/client_stub.dart +++ b/lib/src/client_stub.dart @@ -4,5 +4,6 @@ import 'base_client.dart'; +/// Implemented in `browser_client.dart` and `io_client.dart`. BaseClient createClient() => throw UnsupportedError( 'Cannot create a client without dart:html or dart:io.'); diff --git a/lib/src/io_client.dart b/lib/src/io_client.dart index 4d2b2c47bc..53054f8b63 100644 --- a/lib/src/io_client.dart +++ b/lib/src/io_client.dart @@ -12,6 +12,7 @@ import 'base_request.dart'; import 'exception.dart'; import 'streamed_response.dart'; +/// Used from conditional imports, matches the definition in `client_stub.dart`. BaseClient createClient() => IOClient(); /// A `dart:io`-based HTTP client.