Skip to content

Commit

Permalink
ui: HTTP Body testing (#7290)
Browse files Browse the repository at this point in the history
* 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])
  • Loading branch information
johncowen authored and kaxcode committed May 11, 2020
1 parent 619790c commit bfc80fe
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 55 deletions.
5 changes: 2 additions & 3 deletions ui-v2/app/adapters/application.js
Original file line number Diff line number Diff line change
@@ -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;
}
},
Expand Down
121 changes: 73 additions & 48 deletions ui-v2/app/services/client/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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'),
Expand All @@ -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',
Expand All @@ -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) {
Expand Down
8 changes: 8 additions & 0 deletions ui-v2/app/services/env.js
Original file line number Diff line number Diff line change
@@ -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);
},
});
25 changes: 25 additions & 0 deletions ui-v2/tests/helpers/get-nspace-runner.js
Original file line number Diff line number Diff line change
@@ -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);
};
}
12 changes: 12 additions & 0 deletions ui-v2/tests/integration/adapters/coordinate-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
23 changes: 23 additions & 0 deletions ui-v2/tests/integration/adapters/discovery-chain-test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
);
});
});
12 changes: 12 additions & 0 deletions ui-v2/tests/unit/services/env-test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
19 changes: 15 additions & 4 deletions ui-v2/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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==
Expand All @@ -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"
Expand Down

0 comments on commit bfc80fe

Please sign in to comment.