ui: Stop ember-data overwriting SyncTimes

[johncowen]

The majority of this PR is just prettier (sorry!), the important line is line 218.

https://github.com/hashicorp/consul/compare/ui/scratch/more-ember-data-fun#diff-e4c456fea821208520a230bc235b8e5c6692e338129127b7feabe472ed481b6fR218

Background:

• Consul UI uses blocking queries for its live updates this means that we are mostly kept completely up to date with the single source of truth on the backend.
• We use ember-data which additionally caches its own version of truth on the frontend, which is separate from the backend's source of truth (something I've never been onboard with). Therefore the source of truth is no longer 'single'.
• In order to keep ember-data's version of truth in sync with the actual truth on the backend we have to perform various cleanup and syncing tasks by either invalidating part of ember-data's cache or all of it. We barely use ember-data's cache for anything (if at all).
• I can't quite remember why off the top of my head, but we can't just clear the entire cache for a model every time we receive a full list of models from the backend (possibly misremembering but its mainly due to how ember-data works but also partly down to the fact that we sometimes get filtered/incomplete views of data)
• We have an extra abstraction over ember-data in that all data layer requests/responses must go through our Repositories, ember-data is almost completely hidden from the rest of the application. Therefore the change here affects all models.

Therefore:

• In order to keep things in sync, when we get a response from a list type of blocking query, we add a SyncTime property to every record in the list which is a timestamp of when the record was received.
• Then after we receive an entire list response, we check the SyncTime of every record in ember-data's version of truth. If a SyncTime is out of date, that therefore means that we didn't receive that record in the last response i.e. it doesn't exist in the actual version of truth on the backend. This means we can delete the record from ember-data's version of truth.
• This SyncTime based invalidation technique is far more performant than doing 'n something or other' checks/looping to see if anything doesn't exist anymore.

The bug:

• When ember-data saves a record, it makes a HTTP request and waits for the response
• Once the response is received, it merges the response in with the record that it currently holds locally and then readds the modified record back into its cache. This means that any properties we use locally only (such as SyncTime) are not changed and are overwritten again into the cache whenever a save response is received. i.e. if anything is changed between the time of the request and the time of the response, that change is clobbered.
• If we are listening on a blocking query and saving at the same time (pretty common) and the blocking query responds before the save does (not as common), and a new SyncTime is written on every record. Then if the save responses comes in last, ember-data will apply the old partially modified record back onto the store. As SyncTime doesn't get modified by the backend, ember-data is essentially overwriting the updated value of SyncTime with the old version of SyncTime, essentially invalidating the record.
• When we come check all the records to see which is out of date, as ember-data has re-applied the old SyncTime to the newly updated record. We consider it out-of-date/invalidated/deleted so we remove it from ember-data's cache. Boom.

Notes:

• This only happens if a save happens and we receive the resulting blocking query update before the save response. If we get the save response first, the blocking query will then respond and all the SyncTimes will get rewritten and everything will be fine.
• This can result a record that you have just that second saved disappearing from listings as you are redirected back to them (a browser refresh will bring them back)
• We've only recently noticed this happening in our Intentions listings. I'm guessing that this could be due to changes being made that ever so slightly change the timing of these things on the backend. Maybe saves can sometimes be slower now, or maybe blocking queries are sometimes quicker now. All in all this is how it manifests in the UI.

The fix:

• Delete the SyncTime from the model entirely when we save anything. As we only use this when a blocking query is received SyncTime will be added again following a blocking queries response, and as its deleted locally before ember-data reapplies the saved record to the cache, it won't overwrite a correct, recently updated value.

The bigger fix here is obviously not to use ember-data at all - something we are working towards.

Thanks @lkysow for the find! 🙇

[jgwhite]

A bonafide distributed systems bug! Time to bust out the lamport clocks (j/k)

[hc-github-team-consul-core]

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/580738.

[hc-github-team-consul-core]

🍒✅ Cherry pick of commit cf98691 onto release/1.11.x succeeded!