ui: Add data-source component and related services

[johncowen]

This PR is the beginning of a series of PRs (see #6821) to migrate to (and use moving forwards) a componentized approach to data rather than Routes to retrieve data.

The base branch for the PR here is kept up to date with ui-staging until we can merge this into it, at which point we'll continue using feature/ui-data-source as a base branch for PRs related to this series of work.

This PR includes:

1. DataSource component
2. Repository manager for retrieving repositories based on URIs
3. Blocking data service for injection to the data-source component to
   support blocking query types of data sources
4. 'Once' promise based data service for injection for potential
   fallback to old style promise based data (would need to be injected via
   an initial runtime variable)
5. Several utility functions taken from elsewhere

• maybeCall - a replication of code from elsewhere for condition
  calling a function based on the result of a promise
• restartWhenAvailable - used for restarting blocking queries when a
  tab is brought to the front
• ifNotBlocking - to check if blocking is NOT enabled (yuk I know, but it reads nice when you read the code)

Some notes:

The utility functions have been mainly refactored out of this file:

consul/ui-v2/app/services/repository/type/event-source.js

Lines 34 to 63 in 593a587

	return newPromisedEventSource(
	function(configuration) {
	// take a copy of the original arguments
	let args = [..._args];
	if (configuration.settings.enabled) {
	// ...and only add our current cursor/configuration if we are blocking
	args = args.concat([configuration]);
	}
	// original find... with configuration now added
	return repo[find](...args)
	.then(res => {
	if (!configuration.settings.enabled) {
	// 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
	// whenAvailable returns a Promise that resolves when the client is available
	// again
	return client.whenAvailable(e);
	}
	throw e;
	});
	},

We do not want to replace those lines as we want to keep the current functionality exactly the same until we have finishing migrating. Once the entire catalog is migrated, we can remove most of the files concerned with our Route based approach to blocking queries.

We are not entirely happy with where we have moved these functions, but it's mostly makes sense and I can't think of anywhere else I'd be happier with right now. This might change in future.

The remaining files in this PR are relatively straightforwards and use the functionality it we already had, with just moved it elsewhere. One thing we have changed slightly here is how we are doing our caching - we've decided to use mnemonist for the data structures we use for caching (along with standard Map and Set). We chose to use mnemonist for the moment as there maybe other functionality contained there that we use can for other work. If we decide against this in future we can switch out for something else.

We use IntersectionObserver here so we can control whether the component loads data based on other things apart from JS, such as CSS. Please note we have not included a polyfill here for IE11.

There is a reasonable amount of in-code based documentation, plus JSDoc comments. We plan to expand on this a little more moving forwards. If anyone has any recommendations for producing documentation from these, please shout.

We are usually quite light in the Consul UI on integration tests. The fact we have a good coverage of acceptance tests and unit tests and most of our components are glorified partials generally means we prioritize acceptance and unit above integration. In this case we plan to use this component quite a lot so we've added a more detailed component integration test than we usually would.

We've included enough 'finding' functionality here to fill out the Services section of the Consul UI using this new approach, we plan to implement that, and then move onto Nodes, before using this approach to ensure the entire UI uses live updating/blocking queries.

[meirish]

A smattering of comments, but I don't think anything is blocking - nicely done 👍

[meirish]

Ha does the lint rule not know the difference between something called observer and something imported from ember-object/observer?

[johncowen]

Hehe, yeah I was just as surprised when that came up

[meirish]

Why null for one and undefined for the other?

[johncowen]

Hmm 🤔 good point, I'm not sure. Will have a look at this just incase, but I think I'll make them both null, I generally don't like settings things to undefined as it just sounds wrong (define something as undefined!?), so I'm wondering why I did it here - it could just be a remnant of a previous iteration/prototype thats stuck around - will have a proper look later 👍

P.S. Just to note I'm going to look at this in during a separate PR, I've added a FIXME to the code locally so I don't forget!

[meirish]

That bug fix for blocking queries (#6808) will go on top of this, right?

[johncowen]

Yep, that'll be the third time it gets fixed 🙈

[meirish]

nit, but I think you could tighten this up with let [, dc, model, ...temp] = src.split('/')

[meirish]

If you squint this feels really similar to ember-data's store abstraction so it's interesting that this is built atop of that when it's very similar. In the store you pass a named model in and get back a unified interface (findRecord, findAll, etc). Here it's pass a uri and get back an object with a find method. And then the repos are the ED adapter layer.

[johncowen]

Yes definitely! It's funny as I was thinking the exact same thing while I was doing this. I think the reasoning is a little like this:

1. Repositories are to hide away/smooth over any rough edges that the Consul UI has with ember-data APIs and make the API feel a little Consul-y (btw these might be rough edges in our app but maybe not in others). For instance:

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

   Lines 42 to 52 in 58db6b5

   	remove: function(obj) {
   	let item = obj;
   	if (typeof obj.destroyRecord === 'undefined') {
   	item = obj.get('data');
   	}
   	if (typeOf(item) === 'object') {
   	item = this.store.peekRecord(this.getModelName(), item[this.getPrimaryKey()]);
   	}
   	return item.destroyRecord().then(item => {
   	return this.store.unloadRecord(item);
   	});

   
   This gives us the freedom to not have to interact with ember-data APIs in our Controller/View layer (so Routes/Controllers/Templates/Components). It's a clear boundary and means we can switch out ember-data entirely if we ever want to, or completely refactor how we use ember-data, as we did in UI: HTTPAdapter #5637 without touching the majority of our app.
2. The URI abstraction is only so we can work with this in a HTML-like way, i.e. we need to refer to data using URIs/strings/an attribute, just like <img src="" /> tags. I suppose its also similar to SVG <path d="" /> stuff I've been doing lately. The value of the d attribute I would guess is only like it is (i.e. a string of characters) as you have to be able to define the points on the path using a simple string, you don't have a scripting language to help you to define the path.

So although it seems a little like we are going around in circles here (I did double think myself!), there is reason to it, plus I've also tried to make sure that you can inject a different manager that works with pure ember-data (so no repositories at all) if we ever wanted to.

[meirish]

Just out of curiosity - how does this relate to tab changes? Are you talking about 2 tabs open to the same ui, or a tab being backgrounded?

[johncowen]

Ah so this is a browser tab being backgrounded, not a HTML/JS/UI tab. It stops and restarts the blocking queries when you switch browser tabs.

Ah! I couldn't figure out why this is an addition to this changeset, just realized/remembered:

All of this functionality is taken from previous PRs and just copied into place where we want it - I haven't deleted the old stuff yet as we don't want to alter any of the old code at all until we are ready to start moving stuff over to use {{data-source}} (see here for the same code

consul/ui-v2/app/services/repository/type/event-source.js

Lines 51 to 62 in d896d49

	.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
	// whenAvailable returns a Promise that resolves when the client is available
	// again
	return client.whenAvailable(e);
	}
	throw e;
	});

)

[meirish]

👍 great tests here

[meirish]

Would you ever have multiple sources with the same key? I don't see the size/count exposed anywhere, what's the plan for that?

[johncowen]

Would you ever have multiple sources with the same key?

The sources themselves are the key, so if you have multiple sources they are all used as a unique key.

I don't see the size/count exposed anywhere, what's the plan for that?

So to go into this slightly, as you said this is all about counting usage. We count the usage using a MultiMap, The keys of the MultiMap are the sources, and the values are the things that are using the source (in this case the values are ember components). So we essentially maintain a record of the amount of things that are using the source. When the MultiMap no longer has a certain source/key (.has(source)) we know it is no longer being used by any component/value, in which case we can stop/close/destroy it.

We use a Set (rather than an array) for the container for the values of the MultiMap (see further up above) so theoretically you cannot have a component 'registered' more than once with the same source. You've actually got me questioning here why I didn't use a MultiSet instead 🤔 so will have a re-look at this....Actually I suppose we don't count as such, we just check if a key exists or not i.e. is count zero or not? Maybe thats why I chose a MultiMap(Set) and not a MultiSet, my head's not in this right now, but I think that might have been it.

A longer story/plan related to other PRs is, I previously had this as some hand-rolled counting code and chose to bring in mnemonist instead for this as I figured I might be using other mnemonist and related things (graphing) for later work. That's has not happened as yet (and I've chosen a different graph lib for the disco-chain work). But I think mnemonist is full of useful goodies and its nicely packaged so I think I'll probably keep it in here.

[meirish]

Do you still need this? I don't see this service used elsewhere (but maybe there's a plan to use it elsewhere?)

[johncowen]

Ah, this is for if you switch off blocking queries either at build time or using our localStorage based feature flag at runtime. It's the equivalent of services/blocking, so if you have blocking queries on we use services/blocking if you have it turned off we will be injecting services/promised.

The injection etc isn't done in this PR yet, but thanks for noticing/asking. I'm aware of a few other things in the codebase that aren't used anymore and we need to start removing.

[meirish]

you can do array destructuring here (and above), not necessary though

[johncowen]

Thanks @meirish !

but I don't think anything is blocking

:badumtss: 😆

I'll be getting back onto this work soonish, and I have a further PR waiting to go ontop of this, so I'm going to re-look at the destructing stuff there I think. You know me, I don't usually immediately reach for certain ES6 constructs partly due to our (until recently) 2.18 ES5-y app and also due to my cautiousness of shiny things.

I do think in this case though (the destructuring you mention above), there is definitely a case to go 'shiny' (yes I watch Moana regularly 😆 ), plus we are also on a more ES6-y ember version now so it feels more inline with that.

(P.S. parent PR for all related work here is #6821)

[ghost]

Hey there,

This issue has been automatically locked because it is closed and there hasn't been any activity for at least 30 days.

If you are still experiencing problems, or still have questions, feel free to open a new one 👍.