From b2f84299caf87457586dec44b6eab6b13f12b3de Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 3 Mar 2020 14:55:49 +0000 Subject: [PATCH] ui: Remove settings.findHeaders now we can use promises in our adapters (#7375) Previously the API around setting headers wasn't within a Promised like code path, so we needed a little 'cheat' function to get a token from localStorage syncronously. Since we refactored our adapter layer, we now have a Promised codepath where we need to access this localStorage value and set our headers. This means we can remove our little 'cheat' function. --- ui-v2/app/services/client/http.js | 166 +++++++++++++++--------------- ui-v2/app/services/settings.js | 12 --- 2 files changed, 85 insertions(+), 93 deletions(-) diff --git a/ui-v2/app/services/client/http.js b/ui-v2/app/services/client/http.js index 93f2bb655..a7edca107 100644 --- a/ui-v2/app/services/client/http.js +++ b/ui-v2/app/services/client/http.js @@ -126,90 +126,94 @@ export default Service.extend({ // with whats left after the line, use for the headers const [url, ...headerParts] = urlParts.join(' ').split('\n'); - const headers = { - // default to application/json - ...{ - 'Content-Type': 'application/json; charset=utf-8', - }, - // add any application level headers - ...get(client, 'settings').findHeaders(), - // but overwrite or add to those from anything in the specific request - ...createHeaders(headerParts), - }; + return client.settings.findBySlug('token').then(function(token) { + const headers = { + // default to application/json + ...{ + 'Content-Type': 'application/json; charset=utf-8', + }, + // add any application level headers + ...{ + 'X-Consul-Token': typeof token.SecretID === 'undefined' ? '' : token.SecretID, + }, + // but overwrite or add to those from anything in the specific request + ...createHeaders(headerParts), + }; - return new Promise(function(resolve, reject) { - const options = { - url: url.trim(), - method: method, - contentType: headers['Content-Type'], - // type: 'json', - complete: function(xhr, textStatus) { - client.complete(this.id); - }, - success: function(response, status, xhr) { - const headers = createHeaders(xhr.getAllResponseHeaders().split('\n')); - const respond = function(cb) { - return cb(headers, response); - }; - // TODO: nextTick ? - resolve(respond); - }, - error: function(xhr, textStatus, err) { - let error; - if (err instanceof Error) { - error = err; - } else { - let status = xhr.status; - // TODO: Not sure if we actually need this, but ember-data checks it - if (textStatus === 'abort') { - status = 0; - } - if (textStatus === 'timeout') { - status = 408; - } - error = new HTTPError(status, xhr.responseText); - } - //TODO: nextTick ? - reject(error); - }, - converters: { - 'text json': function(response) { - try { - return $.parseJSON(response); - } catch (e) { - return response; - } + return new Promise(function(resolve, reject) { + const options = { + url: url.trim(), + method: method, + contentType: headers['Content-Type'], + // type: 'json', + complete: function(xhr, textStatus) { + client.complete(this.id); }, - }, - }; - if (typeof body !== 'undefined') { - // Only read add HTTP body if we aren't GET - // Right now we do this to avoid having to put data in the templates - // for write-like actions - // potentially we should change things so you _have_ to do that - // as doing it this way is a little magical - if (method !== 'GET' && headers['Content-Type'].indexOf('json') !== -1) { - options.data = JSON.stringify(body); - } else { - // TODO: Does this need urlencoding? Assuming jQuery does this - options.data = body; + success: function(response, status, xhr) { + const headers = createHeaders(xhr.getAllResponseHeaders().split('\n')); + const respond = function(cb) { + return cb(headers, response); + }; + // TODO: nextTick ? + resolve(respond); + }, + error: function(xhr, textStatus, err) { + let error; + if (err instanceof Error) { + error = err; + } else { + let status = xhr.status; + // TODO: Not sure if we actually need this, but ember-data checks it + if (textStatus === 'abort') { + status = 0; + } + if (textStatus === 'timeout') { + status = 408; + } + error = new HTTPError(status, xhr.responseText); + } + //TODO: nextTick ? + reject(error); + }, + converters: { + 'text json': function(response) { + try { + return $.parseJSON(response); + } catch (e) { + return response; + } + }, + }, + }; + if (typeof body !== 'undefined') { + // Only read add HTTP body if we aren't GET + // Right now we do this to avoid having to put data in the templates + // for write-like actions + // potentially we should change things so you _have_ to do that + // as doing it this way is a little magical + if (method !== 'GET' && headers['Content-Type'].indexOf('json') !== -1) { + options.data = JSON.stringify(body); + } else { + // TODO: Does this need urlencoding? Assuming jQuery does this + options.data = body; + } } - } - // temporarily reset the headers/content-type so it works the same - // as previously, should be able to remove this once the data layer - // rewrite is over and we can assert sending via form-encoded is fine - // also see adapters/kv content-types in requestForCreate/UpdateRecord - // also see https://github.com/hashicorp/consul/issues/3804 - options.contentType = 'application/json; charset=utf-8'; - headers['Content-Type'] = options.contentType; - // - options.beforeSend = function(xhr) { - if (headers) { - Object.keys(headers).forEach(key => xhr.setRequestHeader(key, headers[key])); - } - this.id = client.acquire(options, xhr); - }; - return $.ajax(options); + // temporarily reset the headers/content-type so it works the same + // as previously, should be able to remove this once the data layer + // rewrite is over and we can assert sending via form-encoded is fine + // also see adapters/kv content-types in requestForCreate/UpdateRecord + // also see https://github.com/hashicorp/consul/issues/3804 + options.contentType = 'application/json; charset=utf-8'; + headers['Content-Type'] = options.contentType; + // + options.beforeSend = function(xhr) { + if (headers) { + Object.keys(headers).forEach(key => xhr.setRequestHeader(key, headers[key])); + } + this.id = client.acquire(options, xhr); + }; + return $.ajax(options); + }); }); }); }, diff --git a/ui-v2/app/services/settings.js b/ui-v2/app/services/settings.js index a5b9c0332..8262400fe 100644 --- a/ui-v2/app/services/settings.js +++ b/ui-v2/app/services/settings.js @@ -4,18 +4,6 @@ const SCHEME = 'consul'; const storage = getStorage(SCHEME); export default Service.extend({ storage: storage, - findHeaders: function() { - // TODO: if possible this should be a promise - // TODO: Actually this has nothing to do with settings it should be in the adapter, - // which probably can't work with a promise based interface :( - const token = this.storage.getValue('token'); - // TODO: The old UI always sent ?token= - // replicate the old functionality here - // but remove this to be cleaner if its not necessary - return { - 'X-Consul-Token': typeof token.SecretID === 'undefined' ? '' : token.SecretID, - }; - }, findAll: function(key) { return Promise.resolve(this.storage.all()); },