ui: Fix up blocking reconciliation for multiple models (#11237)

> 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.

.changelog/11237.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Ensure all types of data get reconciled with the backend data
+```

ui/packages/consul-ui/app/adapters/kv.js

@@ -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 }}
 
       ${{
@@ -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

ui/packages/consul-ui/app/components/data-loader/index.hbs

@@ -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
@@ -79,4 +77,5 @@
   </State>
 
 {{/let}}
+  {{did-update (fn dispatch "LOAD") src=src}}
 </StateChart>

ui/packages/consul-ui/app/components/outlet/index.js

@@ -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')) {

ui/packages/consul-ui/app/components/route/index.hbs

@@ -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
 

ui/packages/consul-ui/app/components/route/index.js

@@ -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);

ui/packages/consul-ui/app/models/kv.js

@@ -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;

ui/packages/consul-ui/app/services/data-source/protocols/http.js

@@ -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 find = route.cb(route.params, getOwner(this));
-
-    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;
-      };
-    }
     return this.type.source(find, configuration);
   }
 }

ui/packages/consul-ui/app/services/repository.js

@@ -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() {
@@ -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');
+        const date = get(item, 'SyncTime');
-        if (dc === meta.dc) {
+        if (!item.isDeleted && typeof date !== 'undefined' && date != meta.date && this.shouldReconcile(item, params)) {
-          if (checkNspace) {
+          this.store.unloadRecord(item);
-            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);
-          }
         }
       });
     }
@@ -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 = {}) {

ui/packages/consul-ui/app/services/repository/kv.js

@@ -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 = {}) {
@@ -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 = '/';
+        let items = await this.findAll(...arguments);
-        if (typeof configuration.cursor !== 'undefined') {
+        const meta = items.meta;
-          params.index = configuration.cursor;
+        items = items.filter(item => params.id !== get(item, 'Key'));
-        }
+        items.meta = meta;
-        let items;
+        return 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'));
       },
       ACCESS_LIST,
       params

ui/packages/consul-ui/app/services/repository/service-instance.js

@@ -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(
+    const instances = await this.authorizeBySlug(
-      async () => this.store.query(this.getModelName(), params),
+      async () => this.query(params),
       ACCESS_READ,
       params
     );
+    return instances;
   }
 
   @dataSource('/:partition/:ns/:dc/service-instance/:serviceId/:node/:id')

ui/packages/consul-ui/app/services/routlet.js

@@ -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;

ui/packages/consul-ui/app/templates/dc/kv/index.hbs

@@ -2,7 +2,7 @@
   @name={{routeName}}
 as |route|>
   <DataSource
-    @src={{uri '/${partition}/${nspace}/${dc}/kvs/${key}'
+    @src={{uri '/${partition}/${nspace}/${dc}/kv/${key}'
       (hash
         partition=route.params.partition
         nspace=route.params.nspace
@@ -10,11 +10,10 @@ as |route|>
         key=(or route.params.key '/')
       )
     }}
-    @onchange={{action (mut items) value="data"}}
+    @onchange={{action (mut parent) value="data"}}
   />
   <DataLoader
-    @src={{
+    @src={{uri '/${partition}/${nspace}/${dc}/kvs/${key}'
-      uri '/${partition}/${nspace}/${dc}/kv/${key}'
       (hash
         partition=route.params.partition
         nspace=route.params.nspace
@@ -29,6 +28,30 @@ as |route|>
         @login={{route.model.app.login.open}}
       />
     </BlockSlot>
+      <BlockSlot @name="disconnected" as |Notification|>
+        {{#if (eq loader.error.status "404")}}
+          <Notification @sticky={{true}}>
+            <p data-notification role="alert" class="warning notification-update">
+              <strong>Warning!</strong>
+              This KV or parent of this KV was deleted.
+            </p>
+          </Notification>
+        {{else if (eq loader.error.status "403")}}
+          <Notification @sticky={{true}}>
+            <p data-notification role="alert" class="error notification-update">
+              <strong>Error!</strong>
+              You no longer have access to this KV.
+            </p>
+          </Notification>
+        {{else}}
+          <Notification @sticky={{true}}>
+            <p data-notification role="alert" class="warning notification-update">
+              <strong>Warning!</strong>
+              An error was returned whilst loading this data, refresh to try again.
+            </p>
+          </Notification>
+        {{/if}}
+      </BlockSlot>
 
     <BlockSlot @name="loaded">
 {{#let
@@ -45,8 +68,8 @@ as |route|>
     )
   )
 
+  parent
   loader.data
-  items
 
 as |sort filters parent items|}}
     <AppView>

ui/packages/consul-ui/app/templates/dc/nodes/show.hbs

@@ -1,9 +1,7 @@
 <Route
   @name={{routeName}}
-  @title={{item.Node}}
 as |route|>
-  <DataSource @src={{
+  <DataSource @src={{uri '/${partition}/${nspace}/${dc}/coordinates/for-node/${name}'
-    uri '/${partition}/${nspace}/${dc}/coordinates/for-node/${name}'
     (hash
       partition=route.params.partition
       nspace=route.params.nspace
@@ -21,7 +19,6 @@ as |route|>
       )
     }}
   as |loader|>
-
     <BlockSlot @name="error">
       <AppError
         @error={{loader.error}}
@@ -53,7 +50,6 @@ as |route|>
         </Notification>
       {{/if}}
     </BlockSlot>
-
     <BlockSlot @name="loaded">
 {{#let
   loader.data

ui/packages/consul-ui/tests/integration/services/repository/kv-test.js

@@ -1,6 +1,7 @@
 import { moduleFor, test } from 'ember-qunit';
 import repo from 'consul-ui/tests/helpers/repo';
 import { env } from '../../../../env';
+import { get } from '@ember/object';
 
 const NAME = 'kv';
 moduleFor(`service:repository/${NAME}`, `Integration | Service | ${NAME}`, {
@@ -9,11 +10,15 @@ moduleFor(`service:repository/${NAME}`, `Integration | Service | ${NAME}`, {
 });
 const dc = 'dc-1';
 const id = 'key-name';
+const now = new Date().getTime();
 const undefinedNspace = 'default';
 const undefinedPartition = 'default';
 const partition = 'default';
 [undefinedNspace, 'team-1', undefined].forEach(nspace => {
   test(`findAllBySlug returns the correct data for list endpoint when nspace is ${nspace}`, function(assert) {
+    get(this.subject(), 'store').serializerFor(NAME).timestamp = function() {
+      return now;
+    };
     return repo(
       'Kv',
       'findAllBySlug',
@@ -43,20 +48,10 @@ const partition = 'default';
         const expectedPartition = env('CONSUL_PARTITIONS_ENABLED')
           ? partition || undefinedPartition
           : 'default';
-        assert.deepEqual(
+        actual.forEach(item => {
-          actual,
+          assert.equal(item.uid, `["${expectedPartition}","${expectedNspace}","${dc}","${item.Key}"]`);
-          expected(function(payload) {
+          assert.equal(item.Datacenter, dc);
-            return payload.map(item => {
+        });
-              return {
-                Datacenter: dc,
-                Namespace: expectedNspace,
-                Partition: expectedPartition,
-                uid: `["${expectedPartition}","${expectedNspace}","${dc}","${item}"]`,
-                Key: item,
-              };
-            });
-          })
-        );
       }
     );
   });
@@ -81,18 +76,13 @@ const partition = 'default';
         });
       },
       function(actual, expected) {
-        assert.deepEqual(
+        expected(
-          actual,
+          function(payload) {
-          expected(function(payload) {
             const item = payload[0];
-            return Object.assign({}, item, {
+            assert.equal(actual.uid, `["${item.Partition || undefinedPartition}","${item.Namespace ||
-              Datacenter: dc,
+                      undefinedNspace}","${dc}","${item.Key}"]`);
-              Namespace: item.Namespace || undefinedNspace,
+            assert.equal(actual.Datacenter, dc);
-              Partition: item.Partition || undefinedPartition,
+          }
-              uid: `["${item.Partition || undefinedPartition}","${item.Namespace ||
-                undefinedNspace}","${dc}","${item.Key}"]`,
-            });
-          })
         );
       }
     );