Skip to content

Update Client interface #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Mar 6, 2017
Merged

Update Client interface #56

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d274582
Remove extensions of request and response
donny-dont Feb 17, 2017
bb81377
Update client interface
donny-dont Feb 17, 2017
9764158
Update async to include collectBytes
donny-dont Feb 17, 2017
cf91544
Implement updated interface in BaseClient
donny-dont Feb 17, 2017
949bb56
Update helper functions
donny-dont Feb 17, 2017
6cf06ea
Use Future in http helpers
donny-dont Feb 17, 2017
6df8771
Modify Request to take dynamic for urls
donny-dont Feb 23, 2017
07dd399
Remove uses of FutureOr
donny-dont Feb 23, 2017
69df648
Update version
donny-dont Feb 27, 2017
35a765c
Use dev for version
donny-dont Mar 2, 2017
8d2e503
Move url to utils and add additional documentation
donny-dont Mar 2, 2017
362ad9c
Fix a typo.
nex3 Mar 6, 2017
4285911
Merge branch 'master' into feature/client-interface
nex3 Mar 6, 2017
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
30 changes: 15 additions & 15 deletions lib/http.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export 'src/streamed_response.dart';
/// the same server, you should use a single [Client] for all of those requests.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> head(url, {Map<String, String> headers}) =>
FutureOr<Response> head(url, {Map<String, String> headers}) =>
Copy link
Member

Choose a reason for hiding this comment

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

Uh...these are always async, right?

Why the move to FutureOr?

Copy link
Author

Choose a reason for hiding this comment

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

@kevmoo the intent is to move to middleware like shelf.

Copy link
Member

Choose a reason for hiding this comment

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

Okay...but do we imagine a future where these calls may be sync?

Copy link
Author

Choose a reason for hiding this comment

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

Are we just talking http.dart or throughout? With the http helpers no.

Copy link
Member

Choose a reason for hiding this comment

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

Exactly. So this method should always return Future. FutureOr implies that it may synchronously return Response.

_withClient((client) => client.head(url, headers: headers));

/// Sends an HTTP GET request with the given headers to the given URL, which can
Expand All @@ -43,7 +43,7 @@ Future<Response> head(url, {Map<String, String> headers}) =>
/// the same server, you should use a single [Client] for all of those requests.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> get(url, {Map<String, String> headers}) =>
FutureOr<Response> get(url, {Map<String, String> headers}) =>
_withClient((client) => client.get(url, headers: headers));

/// Sends an HTTP POST request with the given headers and body to the given URL,
Expand All @@ -65,10 +65,10 @@ Future<Response> get(url, {Map<String, String> headers}) =>
///
/// For more fine-grained control over the request, use [Request] or
/// [StreamedRequest] instead.
Future<Response> post(url, {Map<String, String> headers, body,
FutureOr<Response> post(url, body, {Map<String, String> headers,
Encoding encoding}) =>
_withClient((client) => client.post(url,
headers: headers, body: body, encoding: encoding));
_withClient((client) => client.post(url, body,
headers: headers, encoding: encoding));

/// Sends an HTTP PUT request with the given headers and body to the given URL,
/// which can be a [Uri] or a [String].
Expand All @@ -89,10 +89,10 @@ Future<Response> post(url, {Map<String, String> headers, body,
///
/// For more fine-grained control over the request, use [Request] or
/// [StreamedRequest] instead.
Future<Response> put(url, {Map<String, String> headers, body,
FutureOr<Response> put(url, body, {Map<String, String> headers,
Encoding encoding}) =>
_withClient((client) => client.put(url,
headers: headers, body: body, encoding: encoding));
_withClient((client) => client.put(url, body,
headers: headers, encoding: encoding));

/// Sends an HTTP PATCH request with the given headers and body to the given
/// URL, which can be a [Uri] or a [String].
Expand All @@ -113,10 +113,10 @@ Future<Response> put(url, {Map<String, String> headers, body,
///
/// For more fine-grained control over the request, use [Request] or
/// [StreamedRequest] instead.
Future<Response> patch(url, {Map<String, String> headers, body,
FutureOr<Response> patch(url, body, {Map<String, String> headers,
Encoding encoding}) =>
_withClient((client) => client.patch(url,
headers: headers, body: body, encoding: encoding));
_withClient((client) => client.patch(url, body,
headers: headers, encoding: encoding));

/// Sends an HTTP DELETE request with the given headers to the given URL, which
/// can be a [Uri] or a [String].
Expand All @@ -126,7 +126,7 @@ Future<Response> patch(url, {Map<String, String> headers, body,
/// the same server, you should use a single [Client] for all of those requests.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> delete(url, {Map<String, String> headers}) =>
FutureOr<Response> delete(url, {Map<String, String> headers}) =>
_withClient((client) => client.delete(url, headers: headers));

/// Sends an HTTP GET request with the given headers to the given URL, which can
Expand All @@ -142,7 +142,7 @@ Future<Response> delete(url, {Map<String, String> headers}) =>
///
/// For more fine-grained control over the request and response, use [Request]
/// instead.
Future<String> read(url, {Map<String, String> headers}) =>
FutureOr<String> read(url, {Map<String, String> headers}) =>
_withClient((client) => client.read(url, headers: headers));

/// Sends an HTTP GET request with the given headers to the given URL, which can
Expand All @@ -158,10 +158,10 @@ Future<String> read(url, {Map<String, String> headers}) =>
///
/// For more fine-grained control over the request and response, use [Request]
/// instead.
Future<Uint8List> readBytes(url, {Map<String, String> headers}) =>
FutureOr<Uint8List> readBytes(url, {Map<String, String> headers}) =>
_withClient((client) => client.readBytes(url, headers: headers));

Future/*<T>*/ _withClient/*<T>*/(Future/*<T>*/ fn(Client client)) async {
FutureOr<T> _withClient<T>(FutureOr<T> fn(Client client)) async {
var client = new Client();
try {
return await fn(client);
Expand Down
87 changes: 37 additions & 50 deletions lib/src/base_client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ import 'dart:async';
import 'dart:convert';
import 'dart:typed_data';

import 'package:collection/collection.dart';
import 'package:async/async.dart';

import 'base_request.dart';
import 'client.dart';
import 'exception.dart';
import 'request.dart';
import 'response.dart';
import 'streamed_response.dart';

/// The abstract base class for an HTTP client. This is a mixin-style class;
/// subclasses only need to implement [send] and maybe [close], and then they
Expand All @@ -23,15 +21,15 @@ abstract class BaseClient implements Client {
/// can be a [Uri] or a [String].
///
/// For more fine-grained control over the request, use [send] instead.
Future<Response> head(url, {Map<String, String> headers}) =>
_sendUnstreamed("HEAD", url, headers);
FutureOr<Response> head(url, {Map<String, String> headers}) =>
Copy link
Member

Choose a reason for hiding this comment

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

Same question here: why FutureOr? This is never going to return a Response synchronously.

send(new Request.head(_uri(url), headers: headers));
Copy link
Member

Choose a reason for hiding this comment

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

The request constructors should take dynamic urls, not Uris.

Copy link
Author

Choose a reason for hiding this comment

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

How about the named constructors take dynamic but the default takes a Uri? If all of them take dynamic then there's a check on each call to change.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather have a cleaner public API and add a private constructor for change to call, I think.

Copy link
Author

Choose a reason for hiding this comment

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

K works for me 👍


/// Sends an HTTP GET request with the given headers to the given URL, which
/// can be a [Uri] or a [String].
///
/// For more fine-grained control over the request, use [send] instead.
Future<Response> get(url, {Map<String, String> headers}) =>
_sendUnstreamed("GET", url, headers);
FutureOr<Response> get(url, {Map<String, String> headers}) =>
send(new Request.get(_uri(url), headers: headers));

/// Sends an HTTP POST request with the given headers and body to the given
/// URL, which can be a [Uri] or a [String].
Expand All @@ -51,9 +49,10 @@ abstract class BaseClient implements Client {
/// [encoding] defaults to UTF-8.
///
/// For more fine-grained control over the request, use [send] instead.
Future<Response> post(url, {Map<String, String> headers, body,
FutureOr<Response> post(url, body, {Map<String, String> headers,
Encoding encoding}) =>
_sendUnstreamed("POST", url, headers, body, encoding);
send(new Request.post(_uri(url), body, headers: headers,
encoding: encoding));

/// Sends an HTTP PUT request with the given headers and body to the given
/// URL, which can be a [Uri] or a [String].
Expand All @@ -73,9 +72,10 @@ abstract class BaseClient implements Client {
/// [encoding] defaults to UTF-8.
///
/// For more fine-grained control over the request, use [send] instead.
Future<Response> put(url, {Map<String, String> headers, body,
FutureOr<Response> put(url, body, {Map<String, String> headers,
Encoding encoding}) =>
_sendUnstreamed("PUT", url, headers, body, encoding);
send(new Request.put(_uri(url), body, headers: headers,
encoding: encoding));

/// Sends an HTTP PATCH request with the given headers and body to the given
/// URL, which can be a [Uri] or a [String].
Expand All @@ -95,16 +95,17 @@ abstract class BaseClient implements Client {
/// [encoding] defaults to UTF-8.
///
/// For more fine-grained control over the request, use [send] instead.
Future<Response> patch(url, {Map<String, String> headers, body,
FutureOr<Response> patch(url, body, {Map<String, String> headers,
Encoding encoding}) =>
_sendUnstreamed("PATCH", url, headers, body, encoding);
send(new Request.patch(_uri(url), body, headers: headers,
encoding: encoding));

/// Sends an HTTP DELETE request with the given headers to the given URL,
/// which can be a [Uri] or a [String].
///
/// For more fine-grained control over the request, use [send] instead.
Future<Response> delete(url, {Map<String, String> headers}) =>
_sendUnstreamed("DELETE", url, headers);
FutureOr<Response> delete(url, {Map<String, String> headers}) =>
send(new Request.delete(_uri(url), headers: headers));

/// Sends an HTTP GET request with the given headers to the given URL, which
/// can be a [Uri] or a [String], and returns a Future that completes to the
Expand All @@ -115,11 +116,11 @@ abstract class BaseClient implements Client {
///
/// For more fine-grained control over the request and response, use [send] or
/// [get] instead.
Future<String> read(url, {Map<String, String> headers}) {
return get(url, headers: headers).then((response) {
_checkResponseSuccess(url, response);
return response.body;
});
FutureOr<String> read(url, {Map<String, String> headers}) async {
var response = await get(url, headers: headers);
_checkResponseSuccess(url, response);

return await response.readAsString();
}

/// Sends an HTTP GET request with the given headers to the given URL, which
Expand All @@ -131,11 +132,11 @@ abstract class BaseClient implements Client {
///
/// For more fine-grained control over the request and response, use [send] or
/// [get] instead.
Future<Uint8List> readBytes(url, {Map<String, String> headers}) {
return get(url, headers: headers).then((response) {
_checkResponseSuccess(url, response);
return response.bodyBytes;
});
FutureOr<Uint8List> readBytes(url, {Map<String, String> headers}) async {
var response = await get(url, headers: headers);
_checkResponseSuccess(url, response);

return await collectBytes(response.read());
}

/// Sends an HTTP request and asynchronously returns the response.
Expand All @@ -145,31 +146,7 @@ abstract class BaseClient implements Client {
/// state of the stream; it could have data written to it asynchronously at a
/// later point, or it could already be closed when it's returned. Any
/// internal HTTP errors should be wrapped as [ClientException]s.
Future<StreamedResponse> send(BaseRequest request);

/// Sends a non-streaming [Request] and returns a non-streaming [Response].
Future<Response> _sendUnstreamed(String method, url,
Map<String, String> headers, [body, Encoding encoding]) async {

if (url is String) url = Uri.parse(url);
var request = new Request(method, url);

if (headers != null) request.headers.addAll(headers);
if (encoding != null) request.encoding = encoding;
if (body != null) {
if (body is String) {
request.body = body;
} else if (body is List) {
request.bodyBytes = DelegatingList.typed(body);
} else if (body is Map) {
request.bodyFields = DelegatingMap.typed(body);
} else {
throw new ArgumentError('Invalid request body "$body".');
}
}

return Response.fromStream(await send(request));
}
FutureOr<Response> send(Request request);

/// Throws an error if [response] is not successful.
void _checkResponseSuccess(url, Response response) {
Expand All @@ -187,3 +164,13 @@ abstract class BaseClient implements Client {
/// can cause the Dart process to hang.
void close() {}
}

Uri _uri(url) {
if (url is Uri) {
return url;
} else if (url is String) {
return Uri.parse(url);
} else {
throw new ArgumentError.value(url, 'url', 'Not a Uri or String');
}
}
140 changes: 0 additions & 140 deletions lib/src/base_request.dart
View file
Open in desktop

This file was deleted.

Loading