Do not set empty body. Fixes #129 (#130)

Author: f3ath

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';

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<String, String> headers) {
+    final mediaType = _mediaType(headers['content-type']);
+    final charset = mediaType.parameters['charset'];
+    return _encodingForCharset(charset);
+  }
+}

lib/src/client/persistent_handler.dart

@@ -1,55 +1,35 @@
 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
 /// call `close()` on it in the end pf the application lifecycle.
 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<HttpResponse> 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<String, String> 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<String, String> 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);
   }
 }

test/unit/http/encoding_test.dart renamed to 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,
       );
 

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);
+  });
+}