From 1df709b89ac1b900cdeeea3e5267315a95de6df1 Mon Sep 17 00:00:00 2001 From: Alexey Date: Thu, 29 Dec 2022 22:05:32 -0800 Subject: [PATCH] Do not set empty body. Fixes #129 (#130) --- lib/client.dart | 3 +- lib/src/client/message_converter.dart | 67 +++++++++++++++++++ lib/src/client/persistent_handler.dart | 52 +++++--------- test/unit/{http => client}/encoding_test.dart | 3 +- test/unit/client/message_converter_test.dart | 19 ++++++ 5 files changed, 106 insertions(+), 38 deletions(-) create mode 100644 lib/src/client/message_converter.dart rename test/unit/{http => client}/encoding_test.dart (93%) create mode 100644 test/unit/client/message_converter_test.dart diff --git a/lib/client.dart b/lib/client.dart index 4ab25dc..7455c69 100644 --- a/lib/client.dart +++ b/lib/client.dart @@ -2,7 +2,7 @@ /// /// There are two clients implementation provided by this library. /// -/// The firs one, [Client], is the most flexible but low level. It operates +/// The first one, [Client], is the most flexible but low level. It operates /// generic [Request] and [Response] objects and performs basic operations /// such as JSON conversion and error handling. It is agnostic to the document /// structure and accepts any target URIs. @@ -25,6 +25,7 @@ library client; export 'package:json_api/src/client/client.dart'; export 'package:json_api/src/client/disposable_handler.dart'; +export 'package:json_api/src/client/message_converter.dart'; export 'package:json_api/src/client/persistent_handler.dart'; export 'package:json_api/src/client/request.dart'; export 'package:json_api/src/client/response/collection_fetched.dart'; diff --git a/lib/src/client/message_converter.dart b/lib/src/client/message_converter.dart new file mode 100644 index 0000000..9e830b5 --- /dev/null +++ b/lib/src/client/message_converter.dart @@ -0,0 +1,67 @@ +import 'dart:convert'; + +import 'package:http/http.dart'; +import 'package:http_parser/http_parser.dart'; +import 'package:json_api/http.dart'; + +/// Converts HTTP messages to and from to ones used by the `http` package. +class MessageConverter { + /// Creates an instance of the converter. + /// + /// Pass [defaultResponseEncoding] to use in cases when the server does + /// not specify the encoding. + MessageConverter({Encoding defaultResponseEncoding = utf8}) + : _defaultResponseEncoding = defaultResponseEncoding; + + final Encoding _defaultResponseEncoding; + + /// Converts [HttpRequest] to [Request]. + Request request(HttpRequest request) { + final converted = Request(request.method, request.uri); + final hasBody = !(request.isGet || request.isOptions); + if (hasBody) { + // The Request would set the content-type header if the body is assigned + // a value (even an empty string). We want to avoid this extra header for + // GET and OPTIONS requests. + // See https://github.com/dart-lang/http/issues/841 + converted.body = request.body; + } + converted.headers.addAll(request.headers); + return converted; + } + + /// Converts [Response] to [HttpResponse]. + HttpResponse response(Response response) { + final encoding = _encodingForHeaders(response.headers); + final body = encoding.decode(response.bodyBytes); + return HttpResponse(response.statusCode, body: body) + ..headers.addAll(response.headers); + } + + /// Returns the [Encoding] that corresponds to [charset]. + /// + /// Returns [defaultResponseEncoding] if [charset] is null or if no [Encoding] + /// was found that corresponds to [charset]. + Encoding _encodingForCharset(String? charset) { + if (charset == null) return _defaultResponseEncoding; + return Encoding.getByName(charset) ?? _defaultResponseEncoding; + } + + /// Returns the [MediaType] object for the given content-type. + /// + /// Defaults to `application/octet-stream`. + MediaType _mediaType(String? contentType) { + if (contentType != null) return MediaType.parse(contentType); + return MediaType('application', 'octet-stream'); + } + + /// Returns the encoding to use for a response with the given headers. + /// + /// Defaults to [defaultEncoding] if the headers don't specify the charset + /// or if that charset is unknown. + Encoding _encodingForHeaders(Map headers) { + final mediaType = _mediaType(headers['content-type']); + final charset = mediaType.parameters['charset']; + return _encodingForCharset(charset); + } +} diff --git a/lib/src/client/persistent_handler.dart b/lib/src/client/persistent_handler.dart index 117b900..7eacb9b 100644 --- a/lib/src/client/persistent_handler.dart +++ b/lib/src/client/persistent_handler.dart @@ -1,8 +1,8 @@ import 'dart:convert'; import 'package:http/http.dart'; -import 'package:http_parser/http_parser.dart'; import 'package:json_api/http.dart'; +import 'package:json_api/src/client/message_converter.dart'; /// Handler which relies on the built-in Dart HTTP client. /// It is the developer's responsibility to instantiate the client and @@ -10,46 +10,26 @@ import 'package:json_api/http.dart'; class PersistentHandler implements HttpHandler { /// Creates a new instance of the handler. Do not forget to call `close()` on /// the [client] when it's not longer needed. - PersistentHandler(this.client, {this.defaultEncoding = utf8}); + /// + /// Use [messageConverter] to fine tune the HTTP request/response conversion. + PersistentHandler( + this.client, + {@Deprecated('Deprecated in favor of MessageConverter.' + ' To be removed in version 6.0.0') + this.defaultEncoding = utf8, + MessageConverter? messageConverter}) + : _converter = messageConverter ?? + MessageConverter(defaultResponseEncoding: defaultEncoding); final Client client; final Encoding defaultEncoding; + final MessageConverter _converter; @override Future handle(HttpRequest request) async { - final response = await Response.fromStream( - await client.send(Request(request.method, request.uri) - ..headers.addAll(request.headers) - ..body = request.body)); - final responseBody = - _encodingForHeaders(response.headers).decode(response.bodyBytes); - return HttpResponse(response.statusCode, body: responseBody) - ..headers.addAll(response.headers); - } - - /// Returns the encoding to use for a response with the given headers. - /// - /// Defaults to [defaultEncoding] if the headers don't specify a charset or if that - /// charset is unknown. - Encoding _encodingForHeaders(Map headers) => - _encodingForCharset( - _contentTypeForHeaders(headers).parameters['charset']); - - /// Returns the [Encoding] that corresponds to [charset]. - /// - /// Returns [defaultEncoding] if [charset] is null or if no [Encoding] was found that - /// corresponds to [charset]. - Encoding _encodingForCharset(String? charset) { - if (charset == null) return defaultEncoding; - return Encoding.getByName(charset) ?? defaultEncoding; - } - - /// Returns the [MediaType] object for the given headers's content-type. - /// - /// Defaults to `application/octet-stream`. - MediaType _contentTypeForHeaders(Map headers) { - final contentType = headers['content-type']; - if (contentType != null) return MediaType.parse(contentType); - return MediaType('application', 'octet-stream'); + final convertedRequest = _converter.request(request); + final streamedResponse = await client.send(convertedRequest); + final response = await Response.fromStream(streamedResponse); + return _converter.response(response); } } diff --git a/test/unit/http/encoding_test.dart b/test/unit/client/encoding_test.dart similarity index 93% rename from test/unit/http/encoding_test.dart rename to test/unit/client/encoding_test.dart index 6bae98e..67605d1 100644 --- a/test/unit/http/encoding_test.dart +++ b/test/unit/client/encoding_test.dart @@ -2,8 +2,8 @@ import 'dart:convert'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; +import 'package:json_api/client.dart'; import 'package:json_api/http.dart'; -import 'package:json_api/src/client/persistent_handler.dart'; import 'package:test/test.dart'; void main() { @@ -23,6 +23,7 @@ void main() { return http.Response.bytes(bytesBody, 200); }, ), + // ignore: deprecated_member_use_from_same_package defaultEncoding: encoding, ); diff --git a/test/unit/client/message_converter_test.dart b/test/unit/client/message_converter_test.dart new file mode 100644 index 0000000..966d718 --- /dev/null +++ b/test/unit/client/message_converter_test.dart @@ -0,0 +1,19 @@ +import 'package:json_api/client.dart'; +import 'package:json_api/http.dart'; +import 'package:test/expect.dart'; +import 'package:test/scaffolding.dart'; + +void main() { + final converter = MessageConverter(); + final uri = Uri.parse('https://example.com'); + + test('No headers are set for GET requests', () { + final r = converter.request(HttpRequest('GET', uri)); + expect(r.headers, isEmpty); + }); + + test('No headers are set for OPTIONS requests', () { + final r = converter.request(HttpRequest('OPTIONS', uri)); + expect(r.headers, isEmpty); + }); +}