Skip to content

Commit

Permalink
ui: Fix up blocking reconciliation for multiple models (#11237)
Browse files Browse the repository at this point in the history
> In the future, this should all be moved to each individual repository now, which will mean we can finally get rid of this service.

This PR moves reconciliation to 'each individual repository'. I stopped short of getting rid of the service, but its so small now we pretty much don't need it. I'd rather wait until I look at the equivalent DataSink service and see if we can get rid of both equivalent services together (this also currently dependant on work soon to be merged)

Reconciliation of models (basically doing the extra work to clean up the ember-data store and bring our frontend 'truth' into line with the actual backend truth) when blocking/long-polling on different views/filters of data is slightly more complicated due to figuring out what should be cleaned up and what should be left in the store. This is especially apparent for KVs.

I built in a such a way to hopefully make sure it will all make sense for the future. I also checked that this all worked nicely with all our models, even KV which has never supported blocking queries. I left all that work in so that if we want to enable blocking queries/live updates for KV it now just involves deleting a couple of lines of code.

There is a tonne of old stuff that we can clean up here now (our 'fake headers' that we pass around) and I've added that to my list of thing for a 'Big Cleanup PR' that will remove lots of code that we no longer require.
  • Loading branch information
johncowen authored Oct 7, 2021
1 parent 2d05164 commit a9fe39e
Show file tree
Hide file tree
Showing 15 changed files with 144 additions and 128 deletions.
3 changes: 3 additions & 0 deletions .changelog/11237.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Ensure all types of data get reconciled with the backend data
```
4 changes: 3 additions & 1 deletion ui/packages/consul-ui/app/adapters/kv.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default class KvAdapter extends Adapter {
if (typeof id === 'undefined') {
throw new Error('You must specify an id');
}
return request`
const respond = await request`
GET /v1/kv/${keyToArray(id)}?${{ dc }}
${{
Expand All @@ -37,6 +37,8 @@ export default class KvAdapter extends Adapter {
index,
}}
`;
await respond((headers, body) => delete headers['x-consul-index']);
return respond;
}

// TODO: Should we replace text/plain here with x-www-form-encoded? See
Expand Down
3 changes: 1 addition & 2 deletions ui/packages/consul-ui/app/components/data-loader/index.hbs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
{{yield}}
<StateChart @src={{chart}} as |State Guard Action dispatch state|>

<Ref @target={{this}} @name="dispatch" @value={{dispatch}} />
<Guard @name="loaded" @cond={{action "isLoaded"}} />

{{did-update (fn dispatch "LOAD") src=src}}

{{#let (hash
data=data
Expand Down Expand Up @@ -79,4 +77,5 @@
</State>

{{/let}}
{{did-update (fn dispatch "LOAD") src=src}}
</StateChart>
4 changes: 0 additions & 4 deletions ui/packages/consul-ui/app/components/outlet/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ export default class Outlet extends Component {
}

setAppRoute(name) {
const nspace = 'nspace.';
if (name.startsWith(nspace)) {
name = name.substr(nspace.length);
}
if (name !== 'loading') {
const doc = this.element.ownerDocument.documentElement;
if (doc.classList.contains('ember-loading')) {
Expand Down
2 changes: 1 addition & 1 deletion ui/packages/consul-ui/app/components/route/index.hbs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{did-insert this.connect}}
{{will-destroy this.disconnect}}
{{yield (hash
model=this.model
model=(or this.model this._model)
params=this.params
currentName=this.router.currentRoute.name

Expand Down
16 changes: 15 additions & 1 deletion ui/packages/consul-ui/app/components/route/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,26 @@ export default class RouteComponent extends Component {
@service('routlet') routlet;
@service('router') router;

@tracked model;
@tracked _model;

get params() {
return this.routlet.paramsFor(this.args.name);
}

get model() {
if(this.args.name) {
const temp = this.args.name.split('.');
temp.pop();
const name = temp.join('.');
let model = this.routlet.modelFor(name);
if(Object.keys(model).length === 0) {
return null;
}
return model;
}
return null;
}

@action
connect() {
this.routlet.addRoute(this.args.name, this);
Expand Down
3 changes: 3 additions & 0 deletions ui/packages/consul-ui/app/models/kv.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ export default class Kv extends Model {
@attr('string') uid;
@attr('string') Key;

@attr('number') SyncTime;
@attr() meta; // {}

@attr('string') Datacenter;
@attr('string') Namespace;
@attr('string') Partition;
Expand Down
36 changes: 1 addition & 35 deletions ui/packages/consul-ui/app/services/data-source/protocols/http.js
Original file line number Diff line number Diff line change
@@ -1,47 +1,13 @@
import Service, { inject as service } from '@ember/service';
import { get } from '@ember/object';
import { getOwner } from '@ember/application';
import { match } from 'consul-ui/decorators/data-source';
import { singularize } from 'ember-inflector';

export default class HttpService extends Service {
@service('repository/dc') datacenters;
@service('repository/dc') datacenter;
@service('repository/kv') kvs;
@service('repository/kv') kv;
@service('repository/node') leader;
@service('repository/service') gateways;
@service('repository/service-instance') 'proxy-service-instance';
@service('repository/proxy') 'proxy-instance';
@service('repository/nspace') namespaces;
@service('repository/nspace') namespace;
@service('repository/metrics') metrics;
@service('repository/oidc-provider') oidc;
@service('ui-config') 'ui-config';
@service('ui-config') notfound;

@service('data-source/protocols/http/blocking') type;

source(src, configuration) {
const [, , , , model] = src.split('/');
const owner = getOwner(this);
const route = match(src);
const find = route.cb(route.params, owner);

const repo = this[model] || owner.lookup(`service:repository/${singularize(model)}`);
if (typeof repo.reconcile === 'function') {
configuration.createEvent = function(result = {}, configuration) {
const event = {
type: 'message',
data: result,
};
const meta = get(event, 'data.meta') || {};
if (typeof meta.range === 'undefined') {
repo.reconcile(meta);
}
return event;
};
}
const find = route.cb(route.params, getOwner(this));
return this.type.source(find, configuration);
}
}
79 changes: 55 additions & 24 deletions ui/packages/consul-ui/app/services/repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ACCESS_READ } from 'consul-ui/abilities/base';

export default class RepositoryService extends Service {
@service('store') store;
@service('env') env;
@service('repository/permission') permissions;

getModelName() {
Expand Down Expand Up @@ -66,30 +67,33 @@ export default class RepositoryService extends Service {
return item;
}

reconcile(meta = {}) {
shouldReconcile(item, params) {
const dc = get(item, 'Datacenter');
if (dc !== params.dc) {
return false;
}
if (this.env.var('CONSUL_NSPACES_ENABLED')) {
const nspace = get(item, 'Namespace');
if (typeof nspace !== 'undefined' && nspace !== params.ns) {
return false;
}
}
if (this.env.var('CONSUL_PARTITIONS_ENABLED')) {
const partition = get(item, 'Partition');
if (typeof partiton !== 'undefined' && partition !== params.partition) {
return false;
}
}
return true;
}

reconcile(meta = {}, params = {}, configuration = {}) {
// unload anything older than our current sync date/time
if (typeof meta.date !== 'undefined') {
const checkNspace = meta.nspace !== '';
const checkPartition = meta.partition !== '';
this.store.peekAll(this.getModelName()).forEach(item => {
const dc = get(item, 'Datacenter');
if (dc === meta.dc) {
if (checkNspace) {
const nspace = get(item, 'Namespace');
if (typeof nspace !== 'undefined' && nspace !== meta.nspace) {
return;
}
}
if (checkPartition) {
const partition = get(item, 'Partition');
if (typeof partiton !== 'undefined' && partition !== meta.partition) {
return;
}
}
const date = get(item, 'SyncTime');
if (!item.isDeleted && typeof date !== 'undefined' && date != meta.date) {
this.store.unloadRecord(item);
}
const date = get(item, 'SyncTime');
if (!item.isDeleted && typeof date !== 'undefined' && date != meta.date && this.shouldReconcile(item, params)) {
this.store.unloadRecord(item);
}
});
}
Expand All @@ -107,16 +111,43 @@ export default class RepositoryService extends Service {
}

// @deprecated
findAllByDatacenter(params, configuration = {}) {
async findAllByDatacenter(params, configuration = {}) {
return this.findAll(...arguments);
}

findAll(params = {}, configuration = {}) {
async findAll(params = {}, configuration = {}) {
if (typeof configuration.cursor !== 'undefined') {
params.index = configuration.cursor;
params.uri = configuration.uri;
}
return this.store.query(this.getModelName(), params);
return this.query(params);
}

async query(params = {}, configuration = {}) {
let error, meta, res;
try {
res = await this.store.query(this.getModelName(), params);
meta = res.meta;
} catch(e) {
switch(get(e, 'errors.firstObject.status')) {
case '404':
case '403':
meta = {
date: Number.POSITIVE_INFINITY
};
error = e;
break;
default:
throw e;
}
}
if(typeof meta !== 'undefined') {
this.reconcile(meta, params, configuration);
}
if(typeof error !== 'undefined') {
throw error;
}
return res;
}

async findBySlug(params, configuration = {}) {
Expand Down
32 changes: 10 additions & 22 deletions ui/packages/consul-ui/app/services/repository/kv.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ export default class KvService extends RepositoryService {
return PRIMARY_KEY;
}

shouldReconcile(item, params) {
return super.shouldReconcile(...arguments) && item.Key.startsWith(params.id);
}

// this one gives you the full object so key,values and meta
@dataSource('/:partition/:ns/:dc/kv/:id')
async findBySlug(params, configuration = {}) {
Expand Down Expand Up @@ -52,33 +56,17 @@ export default class KvService extends RepositoryService {
// https://www.consul.io/api/kv.html
@dataSource('/:partition/:ns/:dc/kvs/:id')
findAllBySlug(params, configuration = {}) {
params.separator = '/';
if (params.id === '/') {
params.id = '';
}
return this.authorizeBySlug(
async () => {
params.separator = '/';
if (typeof configuration.cursor !== 'undefined') {
params.index = configuration.cursor;
}
let items;
try {
items = await this.store.query(this.getModelName(), params);
} catch (e) {
if (get(e, 'errors.firstObject.status') === '404') {
// TODO: This very much shouldn't be here,
// needs to eventually use ember-datas generateId thing
// in the meantime at least our fingerprinter
// FIXME: Default/token partition
const uid = JSON.stringify([params.partition, params.ns, params.dc, params.id]);
const record = this.store.peekRecord(this.getModelName(), uid);
if (record) {
record.unloadRecord();
}
}
throw e;
}
return items.filter(item => params.id !== get(item, 'Key'));
let items = await this.findAll(...arguments);
const meta = items.meta;
items = items.filter(item => params.id !== get(item, 'Key'));
items.meta = meta;
return items;
},
ACCESS_LIST,
params
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,22 @@ export default class ServiceInstanceService extends RepositoryService {
return modelName;
}

shouldReconcile(item, params) {
return super.shouldReconcile(...arguments) && item.Service.Service === params.id;
}

@dataSource('/:partition/:ns/:dc/service-instances/for-service/:id')
async findByService(params, configuration = {}) {
if (typeof configuration.cursor !== 'undefined') {
params.index = configuration.cursor;
params.uri = configuration.uri;
}
return this.authorizeBySlug(
async () => this.store.query(this.getModelName(), params),
const instances = await this.authorizeBySlug(
async () => this.query(params),
ACCESS_READ,
params
);
return instances;
}

@dataSource('/:partition/:ns/:dc/service-instance/:serviceId/:node/:id')
Expand Down
2 changes: 1 addition & 1 deletion ui/packages/consul-ui/app/services/routlet.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export default class RoutletService extends Service {
const key = pos + 1;
const outlet = outlets.get(keys[key]);
if (typeof outlet !== 'undefined') {
route.model = outlet.model;
route._model = outlet.model;
// TODO: Try to avoid the double computation bug
schedule('afterRender', () => {
outlet.routeName = route.args.name;
Expand Down
Loading

0 comments on commit a9fe39e

Please sign in to comment.