diff --git a/CHANGELOG.md b/CHANGELOG.md index b1bcffaa..b5be947b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,30 @@ # Changelog +## [2.6.0](https://github.com/Workvia/w_transport/compare/2.5.1...2.6.0) +_TBD_ + +- **Improvement:** the content-type for HTTP requests can now be set manually. + + ```dart + var request = new Request() + ..uri = Uri.parse('/example') + ..contentType = + new MediaType('application', 'x-custom', {'charset': UTF8.name}); + ``` + + - The content-type still has a default value based on the type of request + (`Request` - text/plain, `JsonRequest` - application/json, etc.). + + - The content-type's charset parameter will still be updated automatically + when you set the `encoding`, **but once you manually set `contentType`, this + behavior will stop.** In other words, we are assuming that if you set + `contentType` manually, you are intentionally overriding the defaults and + are taking responsibility of setting the `charset` parameter appropriately. + +- **Bug Fix:** the `StreamedRequest` now properly verifies that the request has + not been sent when setting `contentType`. It will now throw a `StateError` + like the rest of the request types. + ## [2.5.0](https://github.com/Workiva/w_transport/compare/2.4.0...2.5.0) _June 15, 2015_ diff --git a/lib/src/http/common/multipart_request.dart b/lib/src/http/common/multipart_request.dart index 9c978a67..6959ef7e 100644 --- a/lib/src/http/common/multipart_request.dart +++ b/lib/src/http/common/multipart_request.dart @@ -112,6 +112,12 @@ abstract class CommonMultipartRequest extends CommonRequest 'The content-length of a multipart request cannot be set manually.'); } + @override + set contentType(MediaType contentType) { + throw new UnsupportedError( + 'The content-type of a multipart request cannot be set manually.'); + } + @override MediaType get defaultContentType => new MediaType('multipart', 'form-data', {'boundary': boundary}); @@ -119,7 +125,8 @@ abstract class CommonMultipartRequest extends CommonRequest @override set encoding(Encoding encoding) { throw new UnsupportedError( - 'A multipart request has many individually-encoded parts. An encoding cannot be set for the entire request.'); + 'A multipart request has many individually-encoded parts. An encoding ' + 'cannot be set for the entire request.'); } Map get fields => diff --git a/lib/src/http/common/request.dart b/lib/src/http/common/request.dart index 1fd9d453..f2895c8a 100644 --- a/lib/src/http/common/request.dart +++ b/lib/src/http/common/request.dart @@ -107,6 +107,11 @@ abstract class CommonRequest extends Object /// Content-type of this request. MediaType _contentType; + /// Whether or not the content-type was set manually. If `false`, the + /// content-type will continue to be updated automatically when the [encoding] + /// is set/changed. If `true`, the content-type will be left alone. + bool _contentTypeSetManually = false; + /// Completer that should complete when the request has finished (successful /// or otherwise). Completer _done = new Completer(); @@ -155,12 +160,16 @@ abstract class CommonRequest extends Object return _contentType; } - /// By default, the content-type cannot be set manually because it's set - /// automatically based on the type of request. Streamed requests will be the - /// exception to this rule because the body is not known in advance. + /// Manually set the content-type for this request. + /// + /// NOTE: By default, the content-type will be set automatically based on the + /// request type and the [encoding]. Once you set the content-type manually, + /// we assume you are intentionally overriding this behavior and the + /// content-type will no longer be updated when [encoding] changes. set contentType(MediaType contentType) { - throw new UnsupportedError( - 'The content-type is set automatically when the request body and type is known in advance.'); + verifyUnsent(); + _contentTypeSetManually = true; + updateContentType(contentType); } /// Set the content-type of this request. Used to update the charset @@ -189,8 +198,10 @@ abstract class CommonRequest extends Object set encoding(Encoding encoding) { verifyUnsent(); _encoding = encoding; - updateContentType( - contentType.change(parameters: {'charset': encoding.name})); + if (!_contentTypeSetManually) { + updateContentType( + contentType.change(parameters: {'charset': encoding.name})); + } } /// Get the request headers to be sent with this HTTP request. diff --git a/lib/src/http/common/streamed_request.dart b/lib/src/http/common/streamed_request.dart index 29744dd1..2076b89c 100644 --- a/lib/src/http/common/streamed_request.dart +++ b/lib/src/http/common/streamed_request.dart @@ -49,11 +49,6 @@ abstract class CommonStreamedRequest extends CommonRequest _contentLength = value; } - @override - set contentType(MediaType contentType) { - updateContentType(contentType); - } - @override MediaType get defaultContentType => new MediaType('text', 'plain', {'charset': encoding.name}); diff --git a/test/integration/http/form_request/suite.dart b/test/integration/http/form_request/suite.dart index 875f5163..dc9f822c 100644 --- a/test/integration/http/form_request/suite.dart +++ b/test/integration/http/form_request/suite.dart @@ -59,6 +59,18 @@ void runFormRequestSuite() { expect(contentType.mimeType, equals('application/x-www-form-urlencoded')); }); + test('content-type should be overridable', () async { + var contentType = new MediaType('application', 'x-custom'); + FormRequest request = new FormRequest() + ..uri = IntegrationPaths.reflectEndpointUri + ..fields['field'] = 'value' + ..contentType = contentType; + Response response = await request.post(); + var reflectedContentType = new MediaType.parse( + response.body.asJson()['headers']['content-type']); + expect(reflectedContentType.mimeType, equals(contentType.mimeType)); + }); + test('UTF8', () async { FormRequest request = new FormRequest() ..uri = IntegrationPaths.echoEndpointUri diff --git a/test/integration/http/json_request/suite.dart b/test/integration/http/json_request/suite.dart index 5de7d9ae..f3a36694 100644 --- a/test/integration/http/json_request/suite.dart +++ b/test/integration/http/json_request/suite.dart @@ -54,6 +54,18 @@ void runJsonRequestSuite() { expect(contentType.mimeType, equals('application/json')); }); + test('content-type should be overridable', () async { + var contentType = new MediaType('application', 'x-custom'); + JsonRequest request = new JsonRequest() + ..uri = IntegrationPaths.reflectEndpointUri + ..body = {'field1': 'value1', 'field2': 'value2'} + ..contentType = contentType; + Response response = await request.post(); + var reflectedContentType = new MediaType.parse( + response.body.asJson()['headers']['content-type']); + expect(reflectedContentType.mimeType, equals(contentType.mimeType)); + }); + test('UTF8', () async { JsonRequest request = new JsonRequest() ..uri = IntegrationPaths.echoEndpointUri diff --git a/test/integration/http/plain_text_request/suite.dart b/test/integration/http/plain_text_request/suite.dart index fa679999..5942ec85 100644 --- a/test/integration/http/plain_text_request/suite.dart +++ b/test/integration/http/plain_text_request/suite.dart @@ -54,6 +54,18 @@ void runPlainTextRequestSuite() { expect(contentType.mimeType, equals('text/plain')); }); + test('content-type should be overridable', () async { + var contentType = new MediaType('application', 'x-custom'); + Request request = new Request() + ..uri = IntegrationPaths.reflectEndpointUri + ..body = 'data' + ..contentType = contentType; + Response response = await request.post(); + var reflectedContentType = new MediaType.parse( + response.body.asJson()['headers']['content-type']); + expect(reflectedContentType.mimeType, equals(contentType.mimeType)); + }); + test('UTF8', () async { Request request = new Request() ..uri = IntegrationPaths.echoEndpointUri diff --git a/test/integration/http/streamed_request/suite.dart b/test/integration/http/streamed_request/suite.dart index 89873423..c76661a3 100644 --- a/test/integration/http/streamed_request/suite.dart +++ b/test/integration/http/streamed_request/suite.dart @@ -69,6 +69,19 @@ void runStreamedRequestSuite() { expect(contentType.mimeType, equals('text/plain')); }); + test('content-type should be overridable', () async { + var contentType = new MediaType('application', 'x-custom'); + StreamedRequest request = new StreamedRequest() + ..uri = IntegrationPaths.reflectEndpointUri + ..body = new Stream.fromIterable([]) + ..contentLength = 0 + ..contentType = contentType; + Response response = await request.post(); + var reflectedContentType = new MediaType.parse( + response.body.asJson()['headers']['content-type']); + expect(reflectedContentType.mimeType, equals(contentType.mimeType)); + }); + test('UTF8', () async { StreamedRequest request = new StreamedRequest() ..uri = IntegrationPaths.echoEndpointUri diff --git a/test/unit/http/form_request_test.dart b/test/unit/http/form_request_test.dart index c81c780f..d9316062 100644 --- a/test/unit/http/form_request_test.dart +++ b/test/unit/http/form_request_test.dart @@ -35,11 +35,6 @@ void main() { configureWTransportForTest(); }); - test('content-type cannot be set manually', () { - FormRequest request = new FormRequest(); - expect(() => request.contentType = null, throwsUnsupportedError); - }); - test('setting fields defaults to empty map if null', () { FormRequest request = new FormRequest()..fields = null; expect(request.fields, equals({})); @@ -111,6 +106,33 @@ void main() { expect(request.contentType.parameters['charset'], equals(ASCII.name)); }); + test( + 'setting encoding should not update content-type if content-type has been set manually', + () { + FormRequest request = new FormRequest(); + expect(request.contentType.parameters['charset'], equals(UTF8.name)); + + // Manually override content-type. + request.contentType = + new MediaType('application', 'x-custom', {'charset': LATIN1.name}); + expect(request.contentType.mimeType, equals('application/x-custom')); + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + + // Changes to encoding should no longer update the content-type. + request.encoding = ASCII; + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + }); + + test('setting content-type should not be allowed once sent', () async { + Uri uri = Uri.parse('/test'); + MockTransports.http.expect('GET', uri); + FormRequest request = new FormRequest(); + await request.get(uri: uri); + expect(() { + request.contentType = new MediaType('application', 'x-custom'); + }, throwsStateError); + }); + test('setting encoding should not be allowed once sent', () async { Uri uri = Uri.parse('/test'); MockTransports.http.expect('GET', uri); diff --git a/test/unit/http/json_request_test.dart b/test/unit/http/json_request_test.dart index f9283269..cd4dbdf0 100644 --- a/test/unit/http/json_request_test.dart +++ b/test/unit/http/json_request_test.dart @@ -35,11 +35,6 @@ void main() { configureWTransportForTest(); }); - test('content-type cannot be set manually', () { - JsonRequest request = new JsonRequest(); - expect(() => request.contentType = null, throwsUnsupportedError); - }); - test('setting entire body (Map)', () { Map json = {'field': 'value'}; JsonRequest request = new JsonRequest()..body = json; @@ -137,6 +132,33 @@ void main() { expect(request.contentType.parameters['charset'], equals(ASCII.name)); }); + test( + 'setting encoding should not update content-type if content-type has been set manually', + () { + JsonRequest request = new JsonRequest(); + expect(request.contentType.parameters['charset'], equals(UTF8.name)); + + // Manually override content-type. + request.contentType = + new MediaType('application', 'x-custom', {'charset': LATIN1.name}); + expect(request.contentType.mimeType, equals('application/x-custom')); + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + + // Changes to encoding should no longer update the content-type. + request.encoding = ASCII; + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + }); + + test('setting content-type should not be allowed once sent', () async { + Uri uri = Uri.parse('/test'); + MockTransports.http.expect('GET', uri); + JsonRequest request = new JsonRequest(); + await request.get(uri: uri); + expect(() { + request.contentType = new MediaType('application', 'x-custom'); + }, throwsStateError); + }); + test('setting encoding should not be allowed once sent', () async { Uri uri = Uri.parse('/test'); MockTransports.http.expect('GET', uri); diff --git a/test/unit/http/plain_text_request_test.dart b/test/unit/http/plain_text_request_test.dart index c4ebd8c4..9404da92 100644 --- a/test/unit/http/plain_text_request_test.dart +++ b/test/unit/http/plain_text_request_test.dart @@ -35,11 +35,6 @@ void main() { configureWTransportForTest(); }); - test('content-type cannot be set manually', () { - Request request = new Request(); - expect(() => request.contentType = null, throwsUnsupportedError); - }); - test('setting body (string)', () { Request request = new Request(); @@ -133,6 +128,33 @@ void main() { expect(request.contentType.parameters['charset'], equals(ASCII.name)); }); + test( + 'setting encoding should not update content-type if content-type has been set manually', + () { + Request request = new Request(); + expect(request.contentType.parameters['charset'], equals(UTF8.name)); + + // Manually override content-type. + request.contentType = + new MediaType('application', 'x-custom', {'charset': LATIN1.name}); + expect(request.contentType.mimeType, equals('application/x-custom')); + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + + // Changes to encoding should no longer update the content-type. + request.encoding = ASCII; + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + }); + + test('setting content-type should not be allowed once sent', () async { + Uri uri = Uri.parse('/test'); + MockTransports.http.expect('GET', uri); + Request request = new Request(); + await request.get(uri: uri); + expect(() { + request.contentType = new MediaType('application', 'x-custom'); + }, throwsStateError); + }); + test('setting encoding should not be allowed once sent', () async { Uri uri = Uri.parse('/test'); MockTransports.http.expect('GET', uri); diff --git a/test/unit/http/streamed_request_test.dart b/test/unit/http/streamed_request_test.dart index 744cf857..6a451b6d 100644 --- a/test/unit/http/streamed_request_test.dart +++ b/test/unit/http/streamed_request_test.dart @@ -115,6 +115,33 @@ void main() { expect(request.contentType.parameters['charset'], equals(ASCII.name)); }); + test( + 'setting encoding should not update content-type if content-type has been set manually', + () { + StreamedRequest request = new StreamedRequest(); + expect(request.contentType.parameters['charset'], equals(UTF8.name)); + + // Manually override content-type. + request.contentType = + new MediaType('application', 'x-custom', {'charset': LATIN1.name}); + expect(request.contentType.mimeType, equals('application/x-custom')); + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + + // Changes to encoding should no longer update the content-type. + request.encoding = ASCII; + expect(request.contentType.parameters['charset'], equals(LATIN1.name)); + }); + + test('setting content-type should not be allowed once sent', () async { + Uri uri = Uri.parse('/test'); + MockTransports.http.expect('GET', uri); + StreamedRequest request = new StreamedRequest(); + await request.get(uri: uri); + expect(() { + request.contentType = new MediaType('application', 'x-custom'); + }, throwsStateError); + }); + test('setting encoding should not be allowed once sent', () async { Uri uri = Uri.parse('/test'); MockTransports.http.expect('GET', uri);