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: EventSource project implementation to enable blocking queries for service and node listings #5267

Merged
merged 7 commits into from
Feb 15, 2019
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 3 additions & 0 deletions ui-v2/app/adapters/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ export default Adapter.extend({
if (typeof query.separator !== 'undefined') {
delete query.separator;
}
if (typeof query.index !== 'undefined') {
delete query.index;
}
delete _query[DATACENTER_QUERY_PARAM];
return query;
},
Expand Down
3 changes: 2 additions & 1 deletion ui-v2/app/controllers/dc/nodes/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import Controller from '@ember/controller';
import { computed } from '@ember/object';
import WithEventSource from 'consul-ui/mixins/with-event-source';
import WithHealthFiltering from 'consul-ui/mixins/with-health-filtering';
import WithSearching from 'consul-ui/mixins/with-searching';
import { get } from '@ember/object';
export default Controller.extend(WithSearching, WithHealthFiltering, {
export default Controller.extend(WithEventSource, WithSearching, WithHealthFiltering, {
init: function() {
this.searchParams = {
healthyNode: 's',
Expand Down
15 changes: 8 additions & 7 deletions ui-v2/app/controllers/dc/services/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Controller from '@ember/controller';
import { get, computed } from '@ember/object';
import { htmlSafe } from '@ember/string';
import WithEventSource from 'consul-ui/mixins/with-event-source';
import WithHealthFiltering from 'consul-ui/mixins/with-health-filtering';
import WithSearching from 'consul-ui/mixins/with-searching';
const max = function(arr, prop) {
Expand All @@ -25,7 +26,7 @@ const width = function(num) {
const widthDeclaration = function(num) {
return htmlSafe(`width: ${num}px`);
};
export default Controller.extend(WithSearching, WithHealthFiltering, {
export default Controller.extend(WithEventSource, WithSearching, WithHealthFiltering, {
init: function() {
this.searchParams = {
service: 's',
Expand All @@ -52,14 +53,14 @@ export default Controller.extend(WithSearching, WithHealthFiltering, {
remainingWidth: computed('maxWidth', function() {
return htmlSafe(`width: calc(50% - ${Math.round(get(this, 'maxWidth') / 2)}px)`);
}),
maxPassing: computed('items', function() {
return max(get(this, 'items'), 'ChecksPassing');
maxPassing: computed('filtered', function() {
return max(get(this, 'filtered'), 'ChecksPassing');
}),
maxWarning: computed('items', function() {
return max(get(this, 'items'), 'ChecksWarning');
maxWarning: computed('filtered', function() {
return max(get(this, 'filtered'), 'ChecksWarning');
}),
maxCritical: computed('items', function() {
return max(get(this, 'items'), 'ChecksCritical');
maxCritical: computed('filtered', function() {
return max(get(this, 'filtered'), 'ChecksCritical');
}),
passingWidth: computed('maxPassing', function() {
return widthDeclaration(width(get(this, 'maxPassing')));
Expand Down
61 changes: 61 additions & 0 deletions ui-v2/app/instance-initializers/event-source.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import config from '../config/environment';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below here we dynamically create the new EventSource returning repositories. Right now we just need to do this with nodes and services - other models will be added in here to enable blocking queries support for those models.


const enabled = 'CONSUL_UI_DISABLE_REALTIME';
export function initialize(container) {
if (config[enabled] || window.localStorage.getItem(enabled) !== null) {
return;
}
['node', 'service']
.map(function(item) {
// create repositories that return a promise resolving to an EventSource
return {
service: `repository/${item}/event-source`,
extend: 'repository/type/event-source',
// Inject our original respository that is used by this class
// within the callable of the EventSource
services: {
content: `repository/${item}`,
},
};
})
.concat([
// These are the routes where we overwrite the 'default'
// repo service. Default repos are repos that return a promise resovlving to
// an ember-data record or recordset
{
route: 'dc/nodes/index',
services: {
repo: 'repository/node/event-source',
},
},
{
route: 'dc/services/index',
services: {
repo: 'repository/service/event-source',
},
},
])
.forEach(function(definition) {
if (typeof definition.extend !== 'undefined') {
// Create the class instances that we need
container.register(
`service:${definition.service}`,
container.resolveRegistration(`service:${definition.extend}`).extend({})
);
}
Object.keys(definition.services).forEach(function(name) {
const servicePath = definition.services[name];
// inject its dependencies, this could probably detect the type
// but hardcode this for the moment
if (typeof definition.route !== 'undefined') {
container.inject(`route:${definition.route}`, name, `service:${servicePath}`);
} else {
container.inject(`service:${definition.service}`, name, `service:${servicePath}`);
}
});
});
}

export default {
initialize,
};
16 changes: 16 additions & 0 deletions ui-v2/app/mixins/with-event-source.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import Mixin from '@ember/object/mixin';

export default Mixin.create({
reset: function(exiting) {
if (exiting) {
Object.keys(this).forEach(prop => {
if (this[prop] && typeof this[prop].close === 'function') {
this[prop].close();
// ember doesn't delete on 'resetController' by default
delete this[prop];
}
});
}
return this._super(...arguments);
},
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above uses the controller-lifecycle PR (#5056) to automatically cleanup any 'closeable' things, in this case EventSources

2 changes: 1 addition & 1 deletion ui-v2/app/mixins/with-filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const toKeyValue = function(el) {
};
export default Mixin.create({
filters: {},
filtered: computed('items', 'filters', function() {
filtered: computed('items.[]', 'filters', function() {
const filters = get(this, 'filters');
return get(this, 'items').filter(item => {
return this.filter(item, filters);
Expand Down
2 changes: 1 addition & 1 deletion ui-v2/app/mixins/with-health-filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default Mixin.create(WithFiltering, {
as: 'filter',
},
},
healthFilters: computed('items', function() {
healthFilters: computed('items.[]', function() {
const items = get(this, 'items');
const objs = ['', 'passing', 'warning', 'critical'].map(function(item) {
const count = countStatus(items, item);
Expand Down
29 changes: 29 additions & 0 deletions ui-v2/app/services/lazy-proxy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import Service from '@ember/service';
import { get } from '@ember/object';

export default Service.extend({
shouldProxy: function(content, method) {
return false;
},
init: function() {
this._super(...arguments);
const content = get(this, 'content');
for (let prop in content) {
(prop => {

Choose a reason for hiding this comment

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

Why wrap this in an iife? Wouldn't removing the iife result in the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm yeah , can't remember exactly. I think it's left over from a different version of that that had some async stuff in there somewhere. I vaguely remember putting this in as I hit that for async thing where the variable has already 'moved on', but I'm guessing that was in a previous version of this at some point. Will remove, thanks for spotting!

if (typeof content[prop] === 'function') {
if (this.shouldProxy(content, prop)) {
this[prop] = function() {
return this.execute(content, prop).then(method => {
return method.apply(this, arguments);
});
};
} else if (typeof this[prop] !== 'function') {
this[prop] = function() {
return content[prop](...arguments);
};
}
}
})(prop);
}
},
});
83 changes: 83 additions & 0 deletions ui-v2/app/services/repository/type/event-source.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { inject as service } from '@ember/service';
import { get } from '@ember/object';

import LazyProxyService from 'consul-ui/services/lazy-proxy';

import { cache as createCache, BlockingEventSource } from 'consul-ui/utils/dom/event-source';

const createProxy = function(repo, find, settings, cache, serialize = JSON.stringify) {
// proxied find*..(id, dc)
const throttle = get(this, 'wait').execute;
return function() {
const key = `${repo.getModelName()}.${find}.${serialize([...arguments])}`;
const _args = arguments;
const newPromisedEventSource = cache;
return newPromisedEventSource(
function(configuration) {
// take a copy of the original arguments
// this means we don't have any configuration object on it
let args = [..._args];
if (settings.blocking) {
// ...and only add our current cursor/configuration if we are blocking
args = args.concat([configuration]);
}
// save a callback so we can conditionally throttle
const cb = () => {
// original find... with configuration now added
return repo[find](...args)
.then(res => {
if (!settings.blocking) {
// blocking isn't enabled, immediately close
this.close();
}
return res;
})
.catch(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
}
throw e;
});
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above cb here is essentially the content of our Callable/BlockingEventSource, this is the code that it called every 'tick'. It uses the original ember-dataset resolving Repository that we already had to do the actual requests.

The code around it gives us the opportunity to centrally deal with a post-initialization on/off setting, plus any other settings or global EventSource related errors. Please note, we're planning on removing the throttle here and using a rate limiter of some sort.

// if we have a cursor (which means its at least the second call)
// and we have a throttle setting, wait for so many ms
if (configuration.cursor !== 'undefined' && settings.throttle) {
return throttle(settings.throttle).then(cb);
}
return cb();
},
{
key: key,
type: BlockingEventSource,
}
);
};
};
let cache = null;
export default LazyProxyService.extend({
store: service('store'),
settings: service('settings'),
wait: service('timeout'),
init: function() {
this._super(...arguments);
if (cache === null) {
cache = createCache({});
}
},
willDestroy: function() {
cache = null;
},
shouldProxy: function(content, method) {
return method.indexOf('find') === 0;
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any find methods will be dealt with by this class (which adds a 'layer' over the original Repository class), any other methods will be dealt with by the original itself without the extra 'layer'.

execute: function(repo, find) {
return get(this, 'settings')
.findBySlug('client')
.then(settings => {
return createProxy.bind(this)(repo, find, settings, cache);
});
},
});
34 changes: 34 additions & 0 deletions ui-v2/tests/acceptance/dc/list-blocking.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
@setupApplicationTest
Feature: dc / list-blocking
In order to see updates without refreshing the page
As a user
I want to see changes if I change consul externally
Background:
Given 1 datacenter model with the value "dc-1"
And settings from yaml
---
consul:client:
blocking: 1
throttle: 200
---
Scenario:
And 3 [Model] models
And a network latency of 100
When I visit the [Page] page for yaml
---
dc: dc-1
---
Then the url should be /dc-1/[Url]
And pause until I see 3 [Model] models
And an external edit results in 5 [Model] models

Choose a reason for hiding this comment

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

This is neat.

And pause until I see 5 [Model] models
And an external edit results in 1 [Model] model
And pause until I see 1 [Model] model
And an external edit results in 0 [Model] models
And pause until I see 0 [Model] models
Where:
--------------------------------------------
| Page | Model | Url |
| services | service | services |
| nodes | node | nodes |
--------------------------------------------
4 changes: 2 additions & 2 deletions ui-v2/tests/acceptance/page-navigation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Feature: Page Navigation
| Item | Model | URL | Endpoint | Back |
| service | services | /dc-1/services/service-0 | /v1/health/service/service-0?dc=dc-1 | /dc-1/services |
| node | nodes | /dc-1/nodes/node-0 | /v1/session/node/node-0?dc=dc-1 | /dc-1/nodes |
| kv | kvs | /dc-1/kv/necessitatibus-0/edit | /v1/session/info/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=dc-1 | /dc-1/kv |
| kv | kvs | /dc-1/kv/0-key-value/edit | /v1/session/info/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=dc-1 | /dc-1/kv |
# | acl | acls | /dc-1/acls/anonymous | /v1/acl/info/anonymous?dc=dc-1 | /dc-1/acls |
| intention | intentions | /dc-1/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca | /v1/internal/ui/services?dc=dc-1 | /dc-1/intentions |
| token | tokens | /dc-1/acls/tokens/ee52203d-989f-4f7a-ab5a-2bef004164ca | /v1/acl/policies?dc=dc-1 | /dc-1/acls/tokens |
Expand Down Expand Up @@ -116,7 +116,7 @@ Feature: Page Navigation
Where:
--------------------------------------------------------------------------------------------------------
| Item | Model | URL | Back |
| kv | kvs | /dc-1/kv/necessitatibus-0/edit | /dc-1/kv |
| kv | kvs | /dc-1/kv/0-key-value/edit | /dc-1/kv |
# | acl | acls | /dc-1/acls/anonymous | /dc-1/acls |
| intention | intentions | /dc-1/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca | /dc-1/intentions |
--------------------------------------------------------------------------------------------------------
Expand Down
10 changes: 10 additions & 0 deletions ui-v2/tests/acceptance/steps/dc/list-blocking-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);
});
}
Loading