From bfc80fe643020f1bf2e62acd55f8fddedafe089b Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 18 Feb 2020 16:43:52 +0000 Subject: [PATCH] ui: HTTP Body testing (#7290) * ui: Test Coverage Reporting (#7027) * Serve up the /coverage folder whilst developing * Upgrade ember-cli-api-double now it supports passthrough per url * ui: make env into a service and use it where we need to be injectable * Give http/client a body method so we can use it on its own for testing * Add test helper for testing with and without nspaces enabled * Add two tests, one thay needs nspaces and one that doesn't * Keep cleaning up client/http, whilst figuring out a immutable bug * Convert tests to new return format ([actual, leftovers]) --- ui-v2/app/adapters/application.js | 5 +- ui-v2/app/services/client/http.js | 121 +++++++++++------- ui-v2/app/services/env.js | 8 ++ ui-v2/tests/helpers/get-nspace-runner.js | 25 ++++ .../integration/adapters/coordinate-test.js | 12 ++ .../adapters/discovery-chain-test.js | 23 ++++ ui-v2/tests/unit/services/env-test.js | 12 ++ ui-v2/yarn.lock | 19 ++- 8 files changed, 170 insertions(+), 55 deletions(-) create mode 100644 ui-v2/app/services/env.js create mode 100644 ui-v2/tests/helpers/get-nspace-runner.js create mode 100644 ui-v2/tests/unit/services/env-test.js diff --git a/ui-v2/app/adapters/application.js b/ui-v2/app/adapters/application.js index 93079afed1ce..45cb2e7291e1 100644 --- a/ui-v2/app/adapters/application.js +++ b/ui-v2/app/adapters/application.js @@ -1,14 +1,13 @@ import Adapter from './http'; import { inject as service } from '@ember/service'; -import { env } from 'consul-ui/env'; export const DATACENTER_QUERY_PARAM = 'dc'; export const NSPACE_QUERY_PARAM = 'ns'; export default Adapter.extend({ - repo: service('settings'), client: service('client/http'), + env: service('env'), formatNspace: function(nspace) { - if (env('CONSUL_NSPACES_ENABLED')) { + if (this.env.env('CONSUL_NSPACES_ENABLED')) { return nspace !== '' ? { [NSPACE_QUERY_PARAM]: nspace } : undefined; } }, diff --git a/ui-v2/app/services/client/http.js b/ui-v2/app/services/client/http.js index 60ec3c7fa67b..57f0f5bed325 100644 --- a/ui-v2/app/services/client/http.js +++ b/ui-v2/app/services/client/http.js @@ -8,6 +8,21 @@ import getObjectPool from 'consul-ui/utils/get-object-pool'; import Request from 'consul-ui/utils/http/request'; import createURL from 'consul-ui/utils/createURL'; +// reopen EventSources if a user changes tab +export const restartWhenAvailable = function(client) { + return function(e) { + // setup the aborted connection restarting + // this should happen here to avoid cache deletion + const status = get(e, 'errors.firstObject.status'); + if (status === '0') { + // Any '0' errors (abort) should possibly try again, depending upon the circumstances + // whenAvailable returns a Promise that resolves when the client is available + // again + return client.whenAvailable(e); + } + throw e; + }; +}; class HTTPError extends Error { constructor(statusCode, message) { super(message); @@ -37,6 +52,15 @@ const dispose = function(request) { // right now createURL converts undefined to '' so we need to check thats not needed // anywhere (todo written here for visibility) const url = createURL(encodeURIComponent); +const createHeaders = function(lines) { + return lines.reduce(function(prev, item) { + const temp = item.split(':'); + if (temp.length > 1) { + prev[temp[0].trim()] = temp[1].trim(); + } + return prev; + }, {}); +}; export default Service.extend({ dom: service('dom'), settings: service('settings'), @@ -58,64 +82,65 @@ export default Service.extend({ url: function() { return url(...arguments); }, + body: function(strs, ...values) { + let body = {}; + const doubleBreak = strs.reduce(function(prev, item, i) { + if (item.indexOf('\n\n') !== -1) { + return i; + } + return prev; + }, -1); + if (doubleBreak !== -1) { + // This merges request bodies together, so you can specify multiple bodies + // in the request and it will merge them together. + // Turns out we never actually do this, so it might be worth removing as it complicates + // matters slightly as we assumed post bodies would be an object. + // This actually works as it just uses the value of the first object, if its an array + // it concats + body = values.splice(doubleBreak).reduce(function(prev, item, i) { + switch (true) { + case Array.isArray(item): + if (i === 0) { + prev = []; + } + return prev.concat(item); + case typeof item !== 'string': + return { + ...prev, + ...item, + }; + default: + return item; + } + }, body); + } + return [body, ...values]; + }, request: function(cb) { const client = this; return cb(function(strs, ...values) { - let body = {}; - const doubleBreak = strs.reduce(function(prev, item, i) { - if (item.indexOf('\n\n') !== -1) { - return i; - } - return prev; - }, -1); - if (doubleBreak !== -1) { - // This merges request bodies together, so you can specify multiple bodies - // in the request and it will merge them together. - // Turns out we never actually do this, so it might be worth removing as it complicates - // matters slightly as we assumed post bodies would be an object. - // This actually works as it just uses the value of the first object, if its an array - // it concats - body = values.splice(doubleBreak).reduce(function(prev, item, i) { - switch (true) { - case Array.isArray(item): - if (i === 0) { - prev = []; - } - return prev.concat(item); - case typeof item !== 'string': - return { - ...prev, - ...item, - }; - default: - return item; - } - }, body); - } - let temp = url(strs, ...values).split(' '); - const method = temp.shift(); - let rest = temp.join(' '); - temp = rest.split('\n'); - const path = temp.shift().trim(); - const createHeaders = function(lines) { - return lines.reduce(function(prev, item) { - const temp = item.split(':'); - if (temp.length > 1) { - prev[temp[0].trim()] = temp[1].trim(); - } - return prev; - }, {}); - }; + // first go to the end and remove/parse the http body + const [body, ...urlVars] = client.body(...arguments); + // with whats left get the method off the front + const [method, ...urlParts] = client.url(strs, ...urlVars).split(' '); + // with whats left use the rest of the line for the url + // 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(), - ...createHeaders(temp), + // but overwrite or add to those from anything in the specific request + ...createHeaders(headerParts), }; + return new Promise(function(resolve, reject) { const options = { - url: path, + url: url.trim(), method: method, contentType: headers['Content-Type'], // type: 'json', @@ -127,7 +152,7 @@ export default Service.extend({ const respond = function(cb) { return cb(headers, response); }; - //TODO: nextTick ? + // TODO: nextTick ? resolve(respond); }, error: function(xhr, textStatus, err) { diff --git a/ui-v2/app/services/env.js b/ui-v2/app/services/env.js new file mode 100644 index 000000000000..2449c5dfe5ab --- /dev/null +++ b/ui-v2/app/services/env.js @@ -0,0 +1,8 @@ +import Service from '@ember/service'; +import { env } from 'consul-ui/env'; + +export default Service.extend({ + env: function(key) { + return env(key); + }, +}); diff --git a/ui-v2/tests/helpers/get-nspace-runner.js b/ui-v2/tests/helpers/get-nspace-runner.js new file mode 100644 index 000000000000..3a6042ea60a7 --- /dev/null +++ b/ui-v2/tests/helpers/get-nspace-runner.js @@ -0,0 +1,25 @@ +import Service from '@ember/service'; +export default function(type) { + return function(cb, withNspaces, withoutNspaces, container, assert) { + let CONSUL_NSPACES_ENABLED = true; + container.owner.register( + 'service:env', + Service.extend({ + env: function() { + return CONSUL_NSPACES_ENABLED; + }, + }) + ); + const adapter = container.owner.lookup(`adapter:${type}`); + const serializer = container.owner.lookup(`serializer:${type}`); + const client = container.owner.lookup('service:client/http'); + let actual; + + actual = cb(adapter, serializer, client); + assert.deepEqual(actual[0], withNspaces); + + CONSUL_NSPACES_ENABLED = false; + actual = cb(adapter, serializer, client); + assert.deepEqual(actual[0], withoutNspaces); + }; +} diff --git a/ui-v2/tests/integration/adapters/coordinate-test.js b/ui-v2/tests/integration/adapters/coordinate-test.js index eb8ef724c20b..c9a7e6c81767 100644 --- a/ui-v2/tests/integration/adapters/coordinate-test.js +++ b/ui-v2/tests/integration/adapters/coordinate-test.js @@ -12,4 +12,16 @@ module('Integration | Adapter | coordinate', function(hooks) { }); assert.equal(actual, expected); }); + test('requestForQuery returns the correct body', function(assert) { + const adapter = this.owner.lookup('adapter:coordinate'); + const client = this.owner.lookup('service:client/http'); + const expected = { + index: 1, + }; + const [actual] = adapter.requestForQuery(client.body, { + dc: dc, + index: 1, + }); + assert.deepEqual(actual, expected); + }); }); diff --git a/ui-v2/tests/integration/adapters/discovery-chain-test.js b/ui-v2/tests/integration/adapters/discovery-chain-test.js index f8eff7fafd11..3ded7b1f20df 100644 --- a/ui-v2/tests/integration/adapters/discovery-chain-test.js +++ b/ui-v2/tests/integration/adapters/discovery-chain-test.js @@ -1,6 +1,8 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; +import getNspaceRunner from 'consul-ui/tests/helpers/get-nspace-runner'; +const nspaceRunner = getNspaceRunner('discovery-chain'); module('Integration | Adapter | discovery-chain', function(hooks) { setupTest(hooks); const dc = 'dc-1'; @@ -24,4 +26,25 @@ module('Integration | Adapter | discovery-chain', function(hooks) { }); }); }); + test('requestForQueryRecord returns the correct body', function(assert) { + return nspaceRunner( + (adapter, serializer, client) => { + return adapter.requestForQueryRecord(client.body, { + id: id, + dc: dc, + ns: 'team-1', + index: 1, + }); + }, + { + index: 1, + ns: 'team-1', + }, + { + index: 1, + }, + this, + assert + ); + }); }); diff --git a/ui-v2/tests/unit/services/env-test.js b/ui-v2/tests/unit/services/env-test.js new file mode 100644 index 000000000000..bc30ef2dfeb0 --- /dev/null +++ b/ui-v2/tests/unit/services/env-test.js @@ -0,0 +1,12 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Service | env', function(hooks) { + setupTest(hooks); + + // Replace this with your real tests. + test('it exists', function(assert) { + let service = this.owner.lookup('service:env'); + assert.ok(service); + }); +}); diff --git a/ui-v2/yarn.lock b/ui-v2/yarn.lock index 8abb05a5b563..448534d88de1 100644 --- a/ui-v2/yarn.lock +++ b/ui-v2/yarn.lock @@ -1154,9 +1154,9 @@ integrity sha512-8OcgesUjWQ8AjaXzbz3tGJQn1kM0sN6pLidGM7isNPUyYmIjIEXQzaeUQYzsfv0N2Ko9ZuOXYUsaBl8IK1KGow== "@hashicorp/ember-cli-api-double@^3.0.0": - version "3.0.2" - resolved "https://registry.yarnpkg.com/@hashicorp/ember-cli-api-double/-/ember-cli-api-double-3.0.2.tgz#4f66f22e4b54293c46fe16dc24568267526c8acd" - integrity sha512-NmcA+jBcBO8tzCfbqWzrQdHvTUeaj71Gdu9phxaULMtM9Zw7BZtHlvb2P1ivknGGw92w9Se50pDNjkB57ww22A== + version "3.0.0" + resolved "https://registry.yarnpkg.com/@hashicorp/ember-cli-api-double/-/ember-cli-api-double-3.0.0.tgz#2a9e8e475c8ac9780221f46584297640870f9f1c" + integrity sha512-3jpkkB0jsVWbpI3ySsBJ5jmSb1bPkJu8nZTh9YvIiQDhH76zqHY7AXi35oSV4M83f5qYtBLJjDSdwrf0zJ9Dlg== dependencies: "@hashicorp/api-double" "^1.6.1" array-range "^1.0.1" @@ -6871,7 +6871,7 @@ growly@^1.3.0: resolved "https://registry.yarnpkg.com/growly/-/growly-1.3.0.tgz#f10748cbe76af964b7c96c93c6bcc28af120c081" integrity sha1-8QdIy+dq+WS3yWyTxrzCivEgwIE= -handlebars@^4.0.11, handlebars@^4.3.1: +handlebars@^4.0.11: version "4.7.3" resolved "https://registry.yarnpkg.com/handlebars/-/handlebars-4.7.3.tgz#8ece2797826886cf8082d1726ff21d2a022550ee" integrity sha512-SRGwSYuNfx8DwHD/6InAPzD6RgeruWLT+B8e8a7gGs8FWgHzlExpTFMEq2IA6QpAfOClpKHy6+8IqTjeBCu6Kg== @@ -6893,6 +6893,17 @@ handlebars@^4.0.13, handlebars@^4.0.4, handlebars@^4.1.2: optionalDependencies: uglify-js "^3.1.4" +handlebars@^4.3.1: + version "4.7.2" + resolved "https://registry.yarnpkg.com/handlebars/-/handlebars-4.7.2.tgz#01127b3840156a0927058779482031afe0e730d7" + integrity sha512-4PwqDL2laXtTWZghzzCtunQUTLbo31pcCJrd/B/9JP8XbhVzpS5ZXuKqlOzsd1rtcaLo4KqAn8nl8mkknS4MHw== + dependencies: + neo-async "^2.6.0" + optimist "^0.6.1" + source-map "^0.6.1" + optionalDependencies: + uglify-js "^3.1.4" + handlebars@~4.1.2: version "4.1.2" resolved "https://registry.yarnpkg.com/handlebars/-/handlebars-4.1.2.tgz#b6b37c1ced0306b221e094fc7aca3ec23b131b67"