From 38a1b0ff9c481d0378fa7ef1486b346400fe91c9 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Sat, 17 Aug 2019 09:04:54 -0700 Subject: [PATCH 1/5] [BUGFIX]: set Content-Type for non GET requests only --- packages/adapter/addon/rest.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/adapter/addon/rest.js b/packages/adapter/addon/rest.js index 763f56a8234..4606048e6b4 100644 --- a/packages/adapter/addon/rest.js +++ b/packages/adapter/addon/rest.js @@ -1093,14 +1093,21 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, { options.headers = {}; } - if (options.data && options.type !== 'GET') { - let contentType = options.contentType || 'application/json; charset=utf-8'; - options.headers['content-type'] = contentType; - } + let contentType = + options.contentType || + options.headers['Content-Type'] || + options.headers['content-type'] || + 'application/json; charset=utf-8'; if (get(this, 'useFetch')) { + if (options.data && options.type !== 'GET') { + options.headers['content-type'] = contentType; + } options = fetchOptions(options, this); } else { + if (options.data && options.type !== 'GET') { + options = assign(options, { contentType }); + } options = ajaxOptions(options, this); } @@ -1370,7 +1377,6 @@ function ajaxOptions(options, adapter) { if (options.data && options.type !== 'GET') { options.data = JSON.stringify(options.data); - options.contentType = 'application/json; charset=utf-8'; } options.beforeSend = function(xhr) { From 4bad9c0771a9b2fbebd4bade95fb1bec6515ead4 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Sat, 17 Aug 2019 10:17:02 -0700 Subject: [PATCH 2/5] Add tests --- .../json-api-adapter/ajax-options-test.js | 55 +++++++++++++++ .../rest-adapter/ajax-options-test.js | 70 +++++++++++++++++-- packages/adapter/addon/json-api.js | 5 +- packages/adapter/addon/rest.js | 12 ++-- 4 files changed, 130 insertions(+), 12 deletions(-) diff --git a/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js b/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js index febd895919b..f90cc8e9124 100644 --- a/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js +++ b/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js @@ -89,4 +89,59 @@ module('unit/adapters/json-api-adapter/ajax-options - building requests', functi 'headers assigned, Accept header not overwritten' ); }); + + test('ajaxOptions() headers are set POST', function(assert) { + adapter.headers = { 'Other-Key': 'Other Value', Accept: 'application/json' }; + let url = 'example.com'; + let type = 'POST'; + let ajaxOptions = adapter.ajaxOptions(url, type, { data: { type: 'post' } }); + let receivedHeaders = ajaxOptions.headers; + + assert.deepEqual( + receivedHeaders, + { + Accept: 'application/json', + 'content-type': 'application/vnd.api+json', + 'Other-Key': 'Other Value', + }, + 'headers assigned on POST' + ); + }); + + test('ajaxOptions() will not override with existing headers["Content-Type"] POST', function(assert) { + adapter.headers = { 'Content-Type': 'application/x-www-form-urlencoded' }; + let url = 'example.com'; + let type = 'POST'; + let ajaxOptions = adapter.ajaxOptions(url, type, { data: { type: 'post' } }); + let receivedHeaders = ajaxOptions.headers; + + assert.deepEqual( + receivedHeaders, + { + 'Content-Type': 'application/x-www-form-urlencoded', + Accept: 'application/vnd.api+json', + }, + 'content-type header not overwritten' + ); + }); + + test('ajaxOptions() can override with options.contentType POST', function(assert) { + adapter.headers = {}; + let url = 'example.com'; + let type = 'POST'; + let ajaxOptions = adapter.ajaxOptions(url, type, { + contentType: 'application/x-www-form-urlencoded', + data: { type: 'post' }, + }); + let receivedHeaders = ajaxOptions.headers; + + assert.deepEqual( + receivedHeaders, + { + 'content-type': 'application/x-www-form-urlencoded', + Accept: 'application/vnd.api+json', + }, + 'content-type header overwritten' + ); + }); }); diff --git a/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js b/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js index 9ebd157c783..f09cac083cc 100644 --- a/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js +++ b/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js @@ -8,7 +8,7 @@ import DS from 'ember-data'; var Person, Place, store, adapter, env; -module('unit/adapters/rest-adapter/ajax-options - building requests', function(hooks) { +module('unit/adapters/rest-adapter/ajax-options - building requests fetch', function(hooks) { hooks.beforeEach(function() { Person = { modelName: 'person' }; Place = { modelName: 'place' }; @@ -82,7 +82,6 @@ module('unit/adapters/rest-adapter/ajax-options - building requests', function(h let url = 'example.com'; let type = 'GET'; let ajaxOptions = adapter.ajaxOptions(url, type, { data: { key: 'value' } }); - delete ajaxOptions.beforeSend; assert.deepEqual(ajaxOptions, { credentials: 'same-origin', @@ -100,7 +99,6 @@ module('unit/adapters/rest-adapter/ajax-options - building requests', function(h let url = 'example.com'; let type = 'POST'; let ajaxOptions = adapter.ajaxOptions(url, type, { data: { key: 'value' } }); - delete ajaxOptions.beforeSend; assert.deepEqual(ajaxOptions, { credentials: 'same-origin', @@ -115,11 +113,32 @@ module('unit/adapters/rest-adapter/ajax-options - building requests', function(h }); }); + test('ajaxOptions() can provide own Content-Type', function(assert) { + let url = 'example.com'; + let type = 'POST'; + let ajaxOptions = adapter.ajaxOptions(url, type, { + contentType: 'application/x-www-form-urlencoded', + data: { key: 'value' }, + }); + + assert.deepEqual(ajaxOptions, { + contentType: 'application/x-www-form-urlencoded', + credentials: 'same-origin', + data: { key: 'value' }, + body: '{"key":"value"}', + type: 'POST', + method: 'POST', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + }, + url: 'example.com', + }); + }); + test('ajaxOptions() empty data', function(assert) { let url = 'example.com'; let type = 'POST'; let ajaxOptions = adapter.ajaxOptions(url, type, {}); - delete ajaxOptions.beforeSend; assert.deepEqual(ajaxOptions, { credentials: 'same-origin', @@ -145,4 +164,47 @@ module('unit/adapters/rest-adapter/ajax-options - building requests', function(h return fetchPlacePromise; }); }); + + module('ajax-options - ajax', function(hooks) { + hooks.beforeEach(function() { + adapter.set('useFetch', false); + }); + + hooks.afterEach(function() { + run(() => { + store.destroy(); + env.container.destroy(); + }); + }); + + test('ajaxOptions() Content-Type is not set with ajax GET', function(assert) { + adapter.headers = {}; + + let url = 'example.com'; + let type = 'GET'; + let ajaxOptions = adapter.ajaxOptions(url, type, {}); + + assert.notOk(ajaxOptions.contentType, 'contentType not set with GET'); + }); + + test('ajaxOptions() Content-Type is not set with ajax POST no data', function(assert) { + adapter.headers = {}; + + let url = 'example.com'; + let type = 'POST'; + let ajaxOptions = adapter.ajaxOptions(url, type, {}); + + assert.notOk(ajaxOptions.contentType, 'contentType not set with POST no data'); + }); + + test('ajaxOptions() Content-Type is set with ajax POST with data', function(assert) { + adapter.headers = {}; + + let url = 'example.com'; + let type = 'POST'; + let ajaxOptions = adapter.ajaxOptions(url, type, { data: { type: 'post' } }); + + assert.equal(ajaxOptions.contentType, 'application/json; charset=utf-8', 'contentType is set with POST'); + }); + }); }); diff --git a/packages/adapter/addon/json-api.js b/packages/adapter/addon/json-api.js index e473453a413..ff9de5bbbe2 100644 --- a/packages/adapter/addon/json-api.js +++ b/packages/adapter/addon/json-api.js @@ -143,6 +143,8 @@ import { pluralize } from 'ember-inflector'; const JSONAPIAdapter = RESTAdapter.extend({ defaultSerializer: '-json-api', + defaultContentType: 'application/vnd.api+json', + /** @method ajaxOptions @private @@ -152,9 +154,8 @@ const JSONAPIAdapter = RESTAdapter.extend({ @return {Object} */ ajaxOptions(url, type, options = {}) { - options.contentType = options.contentType || 'application/vnd.api+json'; - let hash = this._super(url, type, options); + hash.headers['Accept'] = hash.headers['Accept'] || 'application/vnd.api+json'; return hash; diff --git a/packages/adapter/addon/rest.js b/packages/adapter/addon/rest.js index 4606048e6b4..7583c3365ed 100644 --- a/packages/adapter/addon/rest.js +++ b/packages/adapter/addon/rest.js @@ -291,6 +291,8 @@ const hasNajax = typeof najax !== 'undefined'; const RESTAdapter = Adapter.extend(BuildURLMixin, { defaultSerializer: '-rest', + defaultContentType: 'application/json; charset=utf-8', + fastboot: computed(function() { return getOwner(this).lookup('service:fastboot'); }), @@ -1093,15 +1095,13 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, { options.headers = {}; } - let contentType = - options.contentType || - options.headers['Content-Type'] || - options.headers['content-type'] || - 'application/json; charset=utf-8'; + let contentType = options.contentType || this.defaultContentType; if (get(this, 'useFetch')) { if (options.data && options.type !== 'GET') { - options.headers['content-type'] = contentType; + if (!options.headers['Content-Type'] && !options.headers['content-type']) { + options.headers['content-type'] = contentType; + } } options = fetchOptions(options, this); } else { From 11786d03c3ec1844ae4c56e67d4db90e7a853f20 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Sat, 17 Aug 2019 10:52:37 -0700 Subject: [PATCH 3/5] slight update to test --- .../unit/adapters/json-api-adapter/ajax-options-test.js | 7 +++---- .../tests/unit/adapters/rest-adapter/ajax-options-test.js | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js b/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js index f90cc8e9124..d291c26843d 100644 --- a/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js +++ b/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js @@ -7,7 +7,7 @@ import DS from 'ember-data'; let Person, Place, store, adapter, env; -module('unit/adapters/json-api-adapter/ajax-options - building requests', function(hooks) { +module('unit/adapters/json-api-adapter/ajax-options - building requests with fetch', function(hooks) { hooks.beforeEach(function() { Person = { modelName: 'person' }; Place = { modelName: 'place' }; @@ -91,7 +91,7 @@ module('unit/adapters/json-api-adapter/ajax-options - building requests', functi }); test('ajaxOptions() headers are set POST', function(assert) { - adapter.headers = { 'Other-Key': 'Other Value', Accept: 'application/json' }; + adapter.headers = {}; let url = 'example.com'; let type = 'POST'; let ajaxOptions = adapter.ajaxOptions(url, type, { data: { type: 'post' } }); @@ -100,9 +100,8 @@ module('unit/adapters/json-api-adapter/ajax-options - building requests', functi assert.deepEqual( receivedHeaders, { - Accept: 'application/json', + Accept: 'application/vnd.api+json', 'content-type': 'application/vnd.api+json', - 'Other-Key': 'Other Value', }, 'headers assigned on POST' ); diff --git a/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js b/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js index f09cac083cc..ae4aa6a0aaa 100644 --- a/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js +++ b/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js @@ -8,7 +8,7 @@ import DS from 'ember-data'; var Person, Place, store, adapter, env; -module('unit/adapters/rest-adapter/ajax-options - building requests fetch', function(hooks) { +module('unit/adapters/rest-adapter/ajax-options - building requests with fetch', function(hooks) { hooks.beforeEach(function() { Person = { modelName: 'person' }; Place = { modelName: 'place' }; From 2e32b88c6f165a1093b382bc69a805dff7f29425 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Sat, 17 Aug 2019 10:56:30 -0700 Subject: [PATCH 4/5] update tests --- .../json-api-adapter/ajax-options-test.js | 2 +- .../rest-adapter/ajax-options-test.js | 25 ++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js b/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js index d291c26843d..8b010f2ec67 100644 --- a/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js +++ b/packages/-ember-data/tests/unit/adapters/json-api-adapter/ajax-options-test.js @@ -107,7 +107,7 @@ module('unit/adapters/json-api-adapter/ajax-options - building requests with fet ); }); - test('ajaxOptions() will not override with existing headers["Content-Type"] POST', function(assert) { + test('ajaxOptions() does not override with existing headers["Content-Type"] POST', function(assert) { adapter.headers = { 'Content-Type': 'application/x-www-form-urlencoded' }; let url = 'example.com'; let type = 'POST'; diff --git a/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js b/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js index ae4aa6a0aaa..86fae73662d 100644 --- a/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js +++ b/packages/-ember-data/tests/unit/adapters/rest-adapter/ajax-options-test.js @@ -113,7 +113,30 @@ module('unit/adapters/rest-adapter/ajax-options - building requests with fetch', }); }); - test('ajaxOptions() can provide own Content-Type', function(assert) { + test('ajaxOptions() can provide own headers["Content-Type"]', function(assert) { + let url = 'example.com'; + let type = 'POST'; + let ajaxOptions = adapter.ajaxOptions(url, type, { + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + data: { key: 'value' }, + }); + + assert.deepEqual(ajaxOptions, { + credentials: 'same-origin', + data: { key: 'value' }, + body: '{"key":"value"}', + type: 'POST', + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + url: 'example.com', + }); + }); + + test('ajaxOptions() can provide own contentType in options', function(assert) { let url = 'example.com'; let type = 'POST'; let ajaxOptions = adapter.ajaxOptions(url, type, { From f33643266350d4beeaaba27374eab5d81a1e20a8 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Thu, 22 Aug 2019 18:13:53 -0700 Subject: [PATCH 5/5] update based on feedback --- packages/adapter/addon/json-api.js | 2 +- packages/adapter/addon/rest.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/adapter/addon/json-api.js b/packages/adapter/addon/json-api.js index ff9de5bbbe2..d86da31278a 100644 --- a/packages/adapter/addon/json-api.js +++ b/packages/adapter/addon/json-api.js @@ -143,7 +143,7 @@ import { pluralize } from 'ember-inflector'; const JSONAPIAdapter = RESTAdapter.extend({ defaultSerializer: '-json-api', - defaultContentType: 'application/vnd.api+json', + _defaultContentType: 'application/vnd.api+json', /** @method ajaxOptions diff --git a/packages/adapter/addon/rest.js b/packages/adapter/addon/rest.js index 7583c3365ed..16d64b6faa7 100644 --- a/packages/adapter/addon/rest.js +++ b/packages/adapter/addon/rest.js @@ -291,7 +291,7 @@ const hasNajax = typeof najax !== 'undefined'; const RESTAdapter = Adapter.extend(BuildURLMixin, { defaultSerializer: '-rest', - defaultContentType: 'application/json; charset=utf-8', + _defaultContentType: 'application/json; charset=utf-8', fastboot: computed(function() { return getOwner(this).lookup('service:fastboot'); @@ -1095,7 +1095,7 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, { options.headers = {}; } - let contentType = options.contentType || this.defaultContentType; + let contentType = options.contentType || this._defaultContentType; if (get(this, 'useFetch')) { if (options.data && options.type !== 'GET') { @@ -1105,6 +1105,8 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, { } options = fetchOptions(options, this); } else { + // GET requests without a body should not have a content-type header + // and may be unexpected by a server if (options.data && options.type !== 'GET') { options = assign(options, { contentType }); }