Skip to content

Commit

Permalink
ui: [Bugfix] Fix DC switching when blocking queries are enabled (#6555)
Browse files Browse the repository at this point in the history
* ui: [BUGFIX] Fixes datacenter switching when block queries are enabled

We recently added a way to reconcile the ember data store when data was
deleted in the backend and the data was kept hanging around in the
frontend, only this didn't take into account different datacenters!

This fix adds a 'fake' `x-consul-datacenter` header which then is
converted to a meta value which we can access whilst doing this
reconciliation. Now we can check the datacenter also before deciding
whether to delete the record from our local cache.

* ui: Tests proving that the previous fix works and to prevent regression

* ui: Add the meta['x-consul-datacenter'] meta data to integration tests

As a result of this fix we added an new property to the meta values
everywhere. This updates our integration response comparision assertions
to look for that also.
  • Loading branch information
johncowen authored Nov 12, 2019
1 parent db05beb commit 9bab89d
Show file tree
Hide file tree
Showing 20 changed files with 100 additions and 25 deletions.
10 changes: 9 additions & 1 deletion ui-v2/app/adapters/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { get } from '@ember/object';
import URL from 'url';
import createURL from 'consul-ui/utils/createURL';
import { FOREIGN_KEY as DATACENTER_KEY } from 'consul-ui/models/dc';
import { HEADERS_SYMBOL as HTTP_HEADERS_SYMBOL } from 'consul-ui/utils/http/consul';
import {
HEADERS_SYMBOL as HTTP_HEADERS_SYMBOL,
HEADERS_DATACENTER as HTTP_HEADERS_DATACENTER,
} from 'consul-ui/utils/http/consul';

export const REQUEST_CREATE = 'createRecord';
export const REQUEST_READ = 'queryRecord';
Expand Down Expand Up @@ -77,6 +80,11 @@ export default Adapter.extend({
Object.keys(headers).forEach(function(key) {
lower[key.toLowerCase()] = headers[key];
});
// Add a 'pretend' Datacenter header, its not a header that comes from the
// request but we add it here so we can use it later
lower[HTTP_HEADERS_DATACENTER] = this.parseURL(requestData.url).searchParams.get(
DATACENTER_QUERY_PARAM
);
response[HTTP_HEADERS_SYMBOL] = lower;
}
return this._super(status, headers, response, requestData);
Expand Down
2 changes: 2 additions & 0 deletions ui-v2/app/serializers/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { set } from '@ember/object';
import {
HEADERS_SYMBOL as HTTP_HEADERS_SYMBOL,
HEADERS_INDEX as HTTP_HEADERS_INDEX,
HEADERS_DATACENTER as HTTP_HEADERS_DATACENTER,
} from 'consul-ui/utils/http/consul';
export default Serializer.extend({
// this could get confusing if you tried to override
Expand Down Expand Up @@ -50,6 +51,7 @@ export default Serializer.extend({
normalizeMeta: function(store, primaryModelClass, headers, payload, id, requestType) {
const meta = {
cursor: headers[HTTP_HEADERS_INDEX],
dc: headers[HTTP_HEADERS_DATACENTER],
};
if (requestType === 'query') {
meta.date = this.timestamp();
Expand Down
9 changes: 6 additions & 3 deletions ui-v2/app/services/repository/type/event-source.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ const createProxy = function(repo, find, settings, cache, serialize = JSON.strin
if (typeof meta.date !== 'undefined') {
// unload anything older than our current sync date/time
store.peekAll(repo.getModelName()).forEach(function(item) {
const date = get(item, 'SyncTime');
if (typeof date !== 'undefined' && date != meta.date) {
store.unloadRecord(item);
const dc = get(item, 'Datacenter');
if (dc === meta.dc) {
const date = get(item, 'SyncTime');
if (typeof date !== 'undefined' && date != meta.date) {
store.unloadRecord(item);
}
}
});
}
Expand Down
1 change: 1 addition & 0 deletions ui-v2/app/utils/http/consul.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const HEADERS_SYMBOL = '__consul_ui_http_headers__';
export const HEADERS_INDEX = 'x-consul-index';
export const HEADERS_DATACENTER = 'x-consul-datacenter';
export const HEADERS_DIGEST = 'x-consul-contenthash';
27 changes: 27 additions & 0 deletions ui-v2/tests/acceptance/dc/services/dc-switch.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
@setupApplicationTest
Feature: dc / services / dc-switch : Switching Datacenters
Scenario: Seeing all services when switching datacenters
Given 2 datacenter models from yaml
---
- dc-1
- dc-2
---
And 6 service models
When I visit the services page for yaml
---
dc: dc-1
---
Then the url should be /dc-1/services
Then I see 6 service models
When I click dc on the navigation
And I click dcs.1.name
Then the url should be /dc-2/services
Then I see 6 service models
When I click dc on the navigation
And I click dcs.0.name
Then the url should be /dc-1/services
Then I see 6 service models
When I click dc on the navigation
And I click dcs.1.name
Then the url should be /dc-2/services
Then I see 6 service models
10 changes: 10 additions & 0 deletions ui-v2/tests/acceptance/steps/dc/services/dc-switch-steps.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import steps from '../../steps';

// step definitions that are shared between features should be moved to the
// tests/acceptance/steps/steps.js file

export default function(assert) {
return steps(assert).then('I should find a file', function() {
assert.ok(true, this.step);
});
}
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/acl/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
module('Integration | Adapter | acl | response', function(hooks) {
setupTest(hooks);
const dc = 'dc-1';
Expand Down Expand Up @@ -30,7 +30,9 @@ module('Integration | Adapter | acl | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload[0], {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
});
const actual = adapter.handleResponse(200, {}, payload, request);
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/intention/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
module('Integration | Adapter | intention | response', function(hooks) {
setupTest(hooks);
const dc = 'dc-1';
Expand Down Expand Up @@ -32,7 +32,9 @@ module('Integration | Adapter | intention | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload, {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
});
const actual = adapter.handleResponse(200, {}, payload, request);
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/kv/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
module('Integration | Adapter | kv | response', function(hooks) {
setupTest(hooks);
const dc = 'dc-1';
Expand Down Expand Up @@ -36,7 +36,9 @@ module('Integration | Adapter | kv | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload[0], {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
});
const actual = adapter.handleResponse(200, {}, payload, request);
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/node/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
module('Integration | Adapter | node | response', function(hooks) {
setupTest(hooks);
test('handleResponse returns the correct data for list endpoint', function(assert) {
Expand Down Expand Up @@ -31,7 +31,9 @@ module('Integration | Adapter | node | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload, {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
});
const actual = adapter.handleResponse(200, {}, payload, request);
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/policy/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
module('Integration | Adapter | policy | response', function(hooks) {
setupTest(hooks);
const dc = 'dc-1';
Expand Down Expand Up @@ -30,7 +30,9 @@ module('Integration | Adapter | policy | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload, {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
});
const actual = adapter.handleResponse(200, {}, payload, request);
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/role/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
import { createPolicies } from 'consul-ui/tests/helpers/normalizers';

module('Integration | Adapter | role | response', function(hooks) {
Expand Down Expand Up @@ -33,7 +33,9 @@ module('Integration | Adapter | role | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload, {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
Policies: createPolicies(payload),
});
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/service/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
module('Integration | Adapter | service | response', function(hooks) {
setupTest(hooks);
test('handleResponse returns the correct data for list endpoint', function(assert) {
Expand Down Expand Up @@ -31,7 +31,9 @@ module('Integration | Adapter | service | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
Nodes: payload,
};
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/session/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';
module('Integration | Adapter | session | response', function(hooks) {
setupTest(hooks);
const dc = 'dc-1';
Expand Down Expand Up @@ -31,7 +31,9 @@ module('Integration | Adapter | session | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload[0], {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
});
const actual = adapter.handleResponse(200, {}, payload, request);
Expand Down
6 changes: 4 additions & 2 deletions ui-v2/tests/integration/adapters/token/response-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { get } from 'consul-ui/tests/helpers/api';
import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul';
import { HEADERS_SYMBOL as META, HEADERS_DATACENTER as DC } from 'consul-ui/utils/http/consul';

import { createPolicies } from 'consul-ui/tests/helpers/normalizers';

Expand Down Expand Up @@ -34,7 +34,9 @@ module('Integration | Adapter | token | response', function(hooks) {
return get(request.url).then(function(payload) {
const expected = Object.assign({}, payload, {
Datacenter: dc,
[META]: {},
[META]: {
[DC]: dc,
},
uid: `["${dc}","${id}"]`,
Policies: createPolicies(payload),
});
Expand Down
1 change: 1 addition & 0 deletions ui-v2/tests/integration/services/repository/node-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ test('findBySlug returns the correct data for item endpoint', function(assert) {
uid: `["${dc}","${item.ID}"]`,
meta: {
cursor: undefined,
dc: dc,
},
});
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ test('findBySlug returns the correct data for item endpoint', function(assert) {
service.Tags = payload.Nodes[0].Service.Tags;
service.meta = {
cursor: undefined,
dc: dc,
};

return service;
Expand Down
4 changes: 3 additions & 1 deletion ui-v2/tests/pages/components/page.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { clickable } from 'ember-cli-page-object';
export default {
const page = {
navigation: ['services', 'nodes', 'kvs', 'acls', 'intentions', 'docs', 'settings'].reduce(
function(prev, item, i, arr) {
const key = item;
Expand All @@ -23,3 +23,5 @@ export default {
}
),
};
page.navigation.dc = clickable('[data-test-datacenter-selected]');
export default page;
4 changes: 3 additions & 1 deletion ui-v2/tests/pages/dc/services/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ export default function(visitable, clickable, attribute, collection, page, filte
service: clickable('a'),
externalSource: attribute('data-test-external-source', 'a span'),
}),
dcs: collection('[data-test-datacenter-picker]'),
dcs: collection('[data-test-datacenter-picker]', {
name: clickable('a'),
}),
navigation: page.navigation,
filter: filter,
};
Expand Down
2 changes: 1 addition & 1 deletion ui-v2/tests/steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default function(assert, library, pages, utils) {
if (isNaN(parseInt(prop))) {
return (obj = obj[prop]);
} else {
return (obj = obj.objectAt(prop));
return (obj = obj.objectAt(parseInt(prop)));
}
}) && obj
);
Expand Down

0 comments on commit 9bab89d

Please sign in to comment.