Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: HTTPAdapter queryLeader refactor #6386

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ui-v2/app/adapters/acl.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ export default Adapter.extend({
function(adapter, request, serialized, unserialized) {
return adapter.requestForCloneRecord(request, serialized, unserialized);
},
function(serializer, response, serialized, unserialized) {
return serializer.respondForCreateRecord(response, serialized, unserialized);
function(serializer, respond, serialized, unserialized) {
return serializer.respondForCreateRecord(respond, serialized, unserialized);
},
snapshot,
type.modelName
Expand Down
4 changes: 2 additions & 2 deletions ui-v2/app/adapters/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ export default Adapter.extend({
.catch(function(e) {
return adapter.error(e);
})
.then(function(response) {
.then(function(respond) {
// TODO: When HTTPAdapter:responder changes, this will also need to change
return resp(serializer, response, serialized, unserialized);
return resp(serializer, respond, serialized, unserialized);
});
// TODO: Potentially add specific serializer errors here
// .catch(function(e) {
Expand Down
124 changes: 15 additions & 109 deletions ui-v2/app/adapters/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,115 +17,21 @@ export default Adapter.extend({
${{ index }}
`;
},
// keep the none queryLeader bits in here ready to be refactored
// they will all be noops until these are added to the refactor
urlForRequest: function({ type, snapshot, requestType }) {
switch (requestType) {
case 'queryLeader':
return this.urlForQueryLeader(snapshot, type.modelName);
}
return this._super(...arguments);
},
urlForQueryLeader: function(query, modelName) {
// https://www.consul.io/api/status.html#get-raft-leader
return this.appendURL('status/leader', [], this.cleanQuery(query));
},
isQueryLeader: function(url, method) {
return url.pathname === this.parseURL(this.urlForQueryLeader({})).pathname;
},
queryLeader: function(store, modelClass, id, snapshot) {
const params = {
store: store,
type: modelClass,
id: id,
snapshot: snapshot,
requestType: 'queryLeader',
};
// _requestFor is private... but these methods aren't, until they disappear..
const request = {
method: this.methodForRequest(params),
url: this.urlForRequest(params),
headers: this.headersForRequest(params),
data: this.dataForRequest(params),
};
// TODO: private..
return this._makeRequest(request);
},
urlForRequest: function({ type, snapshot, requestType }) {
switch (requestType) {
case 'queryLeader':
return this.urlForQueryLeader(snapshot, type.modelName);
}
return this._super(...arguments);
},
urlForQueryLeader: function(query, modelName) {
// https://www.consul.io/api/status.html#get-raft-leader
return this.appendURL('status/leader', [], this.cleanQuery(query));
},
isQueryLeader: function(url, method) {
return url.pathname === this.parseURL(this.urlForQueryLeader({})).pathname;
},
queryLeader: function(store, modelClass, id, snapshot) {
const params = {
store: store,
type: modelClass,
id: id,
snapshot: snapshot,
requestType: 'queryLeader',
};
// _requestFor is private... but these methods aren't, until they disappear..
const request = {
method: this.methodForRequest(params),
url: this.urlForRequest(params),
headers: this.headersForRequest(params),
data: this.dataForRequest(params),
};
// TODO: private..
return this._makeRequest(request);
},
handleBatchResponse: function(url, response, primary, slug) {
const dc = url.searchParams.get(API_DATACENTER_KEY) || '';
return response.map((item, i, arr) => {
// this can go in the serializer
item = fillSlug(item);
// this could be replaced by handleSingleResponse
// maybe perf test first although even polyfilled searchParams should be super fast
return {
...item,
...{
[DATACENTER_KEY]: dc,
[PRIMARY_KEY]: this.uidForURL(url, item[SLUG_KEY]),
},
};
});
requestForQueryLeader: function(request, { dc }) {
return request`
GET /v1/status/leader?${{ dc }}
`;
},
handleResponse: function(status, headers, payload, requestData) {
let response = payload;
const method = requestData.method;
if (status === HTTP_OK) {
const url = this.parseURL(requestData.url);
let temp, port, address;
switch (true) {
case this.isQueryLeader(url, method):
// This response is just an ip:port like `"10.0.0.1:8000"`
// split it and make it look like a `C`onsul.`R`esponse
// popping off the end for ports should cover us for IPv6 addresses
// as we should always get a `address:port` or `[a:dd:re:ss]:port` combo
temp = response.split(':');
port = temp.pop();
address = temp.join(':');
response = {
Address: address,
Port: port,
};
break;
case this.isQueryRecord(url, method):
response = this.handleSingleResponse(url, fillSlug(response), PRIMARY_KEY, SLUG_KEY);
break;
default:
response = this.handleBatchResponse(url, response, PRIMARY_KEY, SLUG_KEY);
}
}
return this._super(status, headers, response, requestData);
queryLeader: function(store, type, id, snapshot) {
return this.request(
function(adapter, request, serialized, unserialized) {
return adapter.requestForQueryLeader(request, serialized, unserialized);
},
function(serializer, respond, serialized, unserialized) {
return serializer.respondForQueryLeader(respond, serialized, unserialized);
},
snapshot,
type.modelName
);
},
});
8 changes: 4 additions & 4 deletions ui-v2/app/adapters/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ export default Adapter.extend({
function(adapter, request, serialized, unserialized) {
return adapter.requestForSelf(request, serialized, unserialized);
},
function(serializer, response, serialized, unserialized) {
return serializer.respondForQueryRecord(response, serialized, unserialized);
function(serializer, respond, serialized, unserialized) {
return serializer.respondForQueryRecord(respond, serialized, unserialized);
},
unserialized,
type.modelName
Expand All @@ -95,11 +95,11 @@ export default Adapter.extend({
function(adapter, request, serialized, unserialized) {
return adapter.requestForCloneRecord(request, serialized, unserialized);
},
function(serializer, response, serialized, unserialized) {
function(serializer, respond, serialized, unserialized) {
// here we just have to pass through the dc (like when querying)
// eventually the id is created with this dc value and the id talen from the
// json response of `acls/token/*/clone`
return serializer.respondForQueryRecord(response, {
return serializer.respondForQueryRecord(respond, {
[API_DATACENTER_KEY]: unserialized[SLUG_KEY],
});
},
Expand Down
18 changes: 18 additions & 0 deletions ui-v2/app/serializers/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,22 @@ export default Serializer.extend({
query
);
},
respondForQueryLeader: function(respond, query) {
// don't call super here we don't care about
// ids/fingerprinting
return respond(function(headers, body) {
// This response/body is just an ip:port like `"10.0.0.1:8500"`
// split it and make it look like a `C`onsul.`R`esponse
// popping off the end for ports should cover us for IPv6 addresses
// as we should always get a `address:port` or `[a:dd:re:ss]:port` combo

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be helpful to show example input and output here? I see the output but I’m not sure what input it’s operating on.

Copy link
Contributor Author

@johncowen johncowen Aug 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just incase, [Outdated] doesn't show in the GH ui in certain places as a code comment has been added for this a few lines under here to address this comment.

const temp = body.split(':');
const port = temp.pop();
const address = temp.join(':');
// The string input `10.0.0.1:8500` would be transformed to...
return {
Address: address,
Port: port,
};
});
},
});
12 changes: 5 additions & 7 deletions ui-v2/app/services/store.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import Store from 'ember-data/store';

// TODO: These only exist for ACLs, should probably make sure they fail
// nicely if you aren't on ACLs for good DX
export default Store.extend({
// TODO: These only exist for ACLs, should probably make sure they fail
// nicely if you aren't on ACLs for good DX
// cloning immediately refreshes the view
clone: function(modelName, id) {
// TODO: no normalization, type it properly for the moment
Expand All @@ -23,11 +23,9 @@ export default Store.extend({
const adapter = this.adapterFor(modelName);
return adapter.self(this, { modelName: modelName }, token.secret, token);
},
queryLeader: function(modelName, query) {
// TODO: no normalization, type it properly for the moment
const adapter = this.adapterFor(modelName);
return adapter.queryLeader(this, { modelName: modelName }, null, query);
},
//
// TODO: This one is only for nodes, should fail nicely if you call it
// for anything other than nodes for good DX
queryLeader: function(modelName, query) {
// TODO: no normalization, type it properly for the moment
const adapter = this.adapterFor(modelName);
Expand Down
4 changes: 3 additions & 1 deletion ui-v2/tests/acceptance/dc/services/instances/show.feature
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ Feature: dc / services / instances / show: Show Service Instance
ID: service-0-with-id
Tags: ['Tag1', 'Tag2']
Meta:
consul-dashboard-url: http://url.com
external-source: nomad
test-meta: test-meta-value
Node:
Node: another-node
Checks:
Expand Down Expand Up @@ -77,7 +79,7 @@ Feature: dc / services / instances / show: Show Service Instance

When I click metaData on the tabs
And I see metaDataIsSelected on the tabs
And I see 1 of the metaData object
And I see 3 of the metaData object

Scenario: A Service instance warns when deregistered whilst blocking
Given settings from yaml
Expand Down
3 changes: 3 additions & 0 deletions ui-v2/tests/helpers/set-cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ export default function(type, value) {
case 'instance':
key = 'CONSUL_NODE_COUNT';
break;
case 'proxy':
key = 'CONSUL_PROXY_COUNT';
break;
case 'kv':
key = 'CONSUL_KV_COUNT';
break;
Expand Down
9 changes: 9 additions & 0 deletions ui-v2/tests/integration/adapters/node-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,13 @@ module('Integration | Adapter | node', function(hooks) {
});
});
});
test('requestForQueryLeader returns the correct url', function(assert) {
const adapter = this.owner.lookup('adapter:node');
const client = this.owner.lookup('service:client/http');
const expected = `GET /v1/status/leader?dc=${dc}`;
const actual = adapter.requestForQueryLeader(client.url, {
dc: dc,
});
assert.equal(actual, expected);
});
});
24 changes: 24 additions & 0 deletions ui-v2/tests/integration/serializers/node-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,28 @@ module('Integration | Serializer | node', function(hooks) {
assert.deepEqual(actual, expected);
});
});
test('respondForQueryLeader returns the correct data', function(assert) {
const serializer = this.owner.lookup('serializer:node');
const dc = 'dc-1';
const request = {
url: `/v1/status/leader?dc=${dc}`,
};
return get(request.url).then(function(payload) {
const expected = {
Address: '211.245.86.75',
Port: '8500',
};
const actual = serializer.respondForQueryLeader(
function(cb) {
const headers = {};
const body = payload;
return cb(headers, body);
},
{
dc: dc,
}
);
assert.deepEqual(actual, expected);
});
});
});