Skip to content

Commit

Permalink
Allow content-type to be set manually.
Browse files Browse the repository at this point in the history
  • Loading branch information
evanweible-wf committed Jun 16, 2016
1 parent 9f0e2d5 commit e819788
Show file tree
Hide file tree
Showing 12 changed files with 208 additions and 28 deletions.
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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_
Expand Down
9 changes: 8 additions & 1 deletion lib/src/http/common/multipart_request.dart
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,21 @@ 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});

@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<String, String> get fields =>
Expand Down
25 changes: 18 additions & 7 deletions lib/src/http/common/request.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<Null> _done = new Completer();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 0 additions & 5 deletions lib/src/http/common/streamed_request.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down
12 changes: 12 additions & 0 deletions test/integration/http/form_request/suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions test/integration/http/json_request/suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions test/integration/http/plain_text_request/suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions test/integration/http/streamed_request/suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 27 additions & 5 deletions test/unit/http/form_request_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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({}));
Expand Down Expand Up @@ -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);
Expand Down
32 changes: 27 additions & 5 deletions test/unit/http/json_request_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
32 changes: 27 additions & 5 deletions test/unit/http/plain_text_request_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
27 changes: 27 additions & 0 deletions test/unit/http/streamed_request_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit e819788

Please sign in to comment.