Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Only setting content-type to JSON if there actually is content #601

Merged
merged 9 commits into from

6 participants

@enyo

Since no content at all is not valid JSON.

@ahawkins

@enyo can you add a test?

@tomdale
Owner

:+1:, happy to pull after we get a test.

@enyo

I'll add a test soon.

@enyo

@twinturbo , @tomdale I wrote a test for it. I didn't find any stubbing library in the project (like http://sinonjs.org) so I just stubbed jQuery.ajax myself.
Hope the test is OK.

@timfpark

This pull request would solve issues with the bodyParser middleware commonly used on node.js where bodyParser fails the incoming GET requests with a Content-Type set with a HTTP 400 (Bad Request).

As a reference, backbone had the same issue. PR here: jashkenas/backbone#267

@tomdale
Owner

I am :thumbsup: on getting this one in. Would someone mind bringing this up-to-date so it can be merged?

@enyo

I'm on it.

@enyo

The tests on the current master branch aren't passing (and some tests that don't pass are actually related to the rest_adapter_test) so I won't push it until the other tests pass again.

@flecno

I am very happy about this PR.
@enyo I have rebase your branch to the current master and all tests will pass after some customization depending on this issue. Also I removed your tests, because there is already a suitable testcase.

Please checkout flecno/data@eeb4b6c

@enyo

Thanks @lord-spam . I merged your commits, and updated to the latest master! So it's good to merge! @tomdale do your magic.

@stefanpenner stefanpenner merged commit f8cb714 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  packages/ember-data/lib/adapters/rest_adapter.js
@@ -329,10 +329,10 @@ DS.RESTAdapter = DS.Adapter.extend({
hash.url = url;
hash.type = type;
hash.dataType = 'json';
- hash.contentType = 'application/json; charset=utf-8';
hash.context = this;
if (hash.data && type !== 'GET') {
+ hash.contentType = 'application/json; charset=utf-8';
hash.data = JSON.stringify(hash.data);
}
View
12 packages/ember-data/tests/unit/rest_adapter_test.js
@@ -1,6 +1,6 @@
var get = Ember.get, set = Ember.set;
-var adapter, store, serializer, ajaxUrl, ajaxType, ajaxHash;
+var adapter, store, serializer, ajaxUrl, ajaxType, ajaxHash, overwriteAjax;
var Person, person, people;
var Role, role, roles;
var Group, group;
@@ -10,13 +10,14 @@ module("the REST adapter", {
ajaxUrl = undefined;
ajaxType = undefined;
ajaxHash = undefined;
+ overwriteAjax = true;
var Adapter = DS.RESTAdapter.extend();
Adapter.configure('plurals', {
person: 'people'
});
- adapter = Adapter.create({
+ adapter = Adapter.createWithMixins({
ajax: function(url, type, hash) {
var success = hash.success, self = this;
@@ -124,21 +125,24 @@ test("Calling ajax() calls JQuery.ajax with json data", function() {
equal(ajaxHash.url, '/foo', 'Request URL is the given value');
equal(ajaxHash.type, 'GET', 'Request method is the given value');
equal(ajaxHash.dataType, 'json', 'Request data type is JSON');
- equal(ajaxHash.contentType, 'application/json; charset=utf-8', 'Request content type is JSON');
+ equal(ajaxHash.contentType, undefined, 'Request content type is undefined for GET');
equal(ajaxHash.context, adapter, 'Request context is the adapter');
equal(ajaxHash.extra, 'special', 'Extra options are passed through');
adapter.ajax('/foo', 'POST', {});
ok(!ajaxHash.data, 'Data not set when not provided');
+ equal(ajaxHash.contentType, undefined, 'Request content type is undefined when data is empty');
adapter.ajax('/foo', 'GET', {data: 'unsupported'});
equal(ajaxHash.data, 'unsupported', 'Data untouched for unsupported methods');
adapter.ajax('/foo', 'POST', {data: {id: 1, name: 'Bar'}});
equal(ajaxHash.data, JSON.stringify({id: 1, name: 'Bar'}), 'Data serialized for POST requests');
+ equal(ajaxHash.contentType, 'application/json; charset=utf-8', 'Request content type is JSON');
adapter.ajax('/foo', 'PUT', {data: {id: 1, name: 'Bar'}});
equal(ajaxHash.data, JSON.stringify({id: 1, name: 'Bar'}), 'Data serialized for PUT requests');
+ equal(ajaxHash.contentType, 'application/json; charset=utf-8', 'Request content type is JSON');
} finally {
// restore jQuery.ajax()
@@ -1067,3 +1071,5 @@ test("updating a record with a 500 error marks the record as error", function()
expectState('error');
});
+
+
Something went wrong with that request. Please try again.