From c450183b4c9c417f3ac46a142db72522da76e8af Mon Sep 17 00:00:00 2001 From: wenincode Date: Tue, 18 Oct 2022 09:40:47 -0600 Subject: [PATCH] Use local-storage service to manage localStorage Use local-storage service, prototyped here https://github.com/LevelbossMike/local-storage-service, to manage local storage usage in an octane way. Does not write to local storage in tests by default and is easy to stub out. --- .../consul/node/agentless-notice/index.js | 19 +-- .../consul-ui/app/services/local-storage.js | 126 ++++++++++++++++++ ui/packages/consul-ui/app/storages/base.js | 14 ++ ui/packages/consul-ui/app/storages/notices.js | 24 ++++ ui/packages/consul-ui/package.json | 1 + .../consul/node/agentless-notice-test.js | 33 +---- ui/yarn.lock | 46 +++++-- 7 files changed, 214 insertions(+), 49 deletions(-) create mode 100644 ui/packages/consul-ui/app/services/local-storage.js create mode 100644 ui/packages/consul-ui/app/storages/base.js create mode 100644 ui/packages/consul-ui/app/storages/notices.js diff --git a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js index 6cc18594cf..ed9d311279 100644 --- a/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js +++ b/ui/packages/consul-ui/app/components/consul/node/agentless-notice/index.js @@ -1,34 +1,27 @@ import Component from '@glimmer/component'; import { action } from '@ember/object'; -import { tracked } from '@glimmer/tracking'; - -const DISMISSED_VALUE = 'true'; +import { storageFor } from '../../../../services/local-storage'; export default class AgentlessNotice extends Component { - storageKey = 'consul-nodes-agentless-notice-dismissed'; - @tracked hasDismissedNotice = false; + storageKey = 'nodes-agentless-dismissed'; + @storageFor('notices') notices; constructor(owner, args) { super(owner, args); if (this.args.postfix) { - this.storageKey = `consul-nodes-agentless-notice-dismissed-${this.args.postfix}`; - } - - if (window.localStorage.getItem(this.storageKey) === DISMISSED_VALUE) { - this.hasDismissedNotice = true; + this.storageKey = `nodes-agentless-dismissed-${this.args.postfix}`; } } get isVisible() { const { items, filteredItems } = this.args; - return !this.hasDismissedNotice && items.length > filteredItems.length; + return !this.notices.state.includes(this.storageKey) && items.length > filteredItems.length; } @action dismissAgentlessNotice() { - window.localStorage.setItem(this.storageKey, DISMISSED_VALUE); - this.hasDismissedNotice = true; + this.notices.add(this.storageKey); } } diff --git a/ui/packages/consul-ui/app/services/local-storage.js b/ui/packages/consul-ui/app/services/local-storage.js new file mode 100644 index 0000000000..b882855702 --- /dev/null +++ b/ui/packages/consul-ui/app/services/local-storage.js @@ -0,0 +1,126 @@ +import Service from '@ember/service'; +import { getOwner } from '@ember/application'; +import ENV from 'consul-ui/config/environment'; + +export function storageFor(key) { + return function () { + return { + get() { + const owner = getOwner(this); + + const localStorageService = owner.lookup('service:localStorage'); + + return localStorageService.getBucket(key); + }, + }; + }; +} + +/** + * An in-memory stub of window.localStorage. Ideally this would + * implement the [Storage](https://developer.mozilla.org/en-US/docs/Web/API/Storage)-interface that localStorage implements + * as well. + * + * We use this implementation during testing to not pollute `window.localStorage` + */ +class MemoryStorage { + constructor() { + this.data = new Map(); + } + + getItem(key) { + return this.data.get(key); + } + + setItem(key, value) { + return this.data.set(key, value.toString()); + } + + /** + * A function to seed data into MemoryStorage. This expects an object to be + * passed. The passed values will be persisted as a string - i.e. the values + * passed will call their `toString()`-method before writing to storage. You need + * to take this into account when you want to persist complex values, like arrays + * or objects: + * + * Example: + * + * ```js + * const storage = new MemoryStorage(); + * storage.seed({ notices: ['notice-a', 'notice-b']}); + * + * storage.getItem('notices') // => 'notice-a,notice-b' + * + * // won't work + * storage.seed({ + * user: { name: 'Tomster' } + * }) + * + * storage.getItem('user') // => '[object Object]' + * + * // this works + * storage.seed({ + * . user: JSON.stringify({name: 'Tomster'}) + * }) + * + * storage.getItem('user') // => '{ "name": "Tomster" }' + * ``` + * @param {object} data - the data to seed + */ + seed(data) { + const newData = new Map(); + + const keys = Object.keys(data); + + keys.forEach((key) => { + newData.set(key, data[key].toString()); + }); + + this.data = newData; + } +} + +/** + * There might be better ways to do this but this is good enough for now. + * During testing we want to use MemoryStorage not window.localStorage. + */ +function initStorage() { + if (ENV.environment === 'test') { + return new MemoryStorage(); + } else { + return window.localStorage; + } +} + +/** + * A service that wraps access to local-storage. We wrap + * local-storage to not pollute local-storage during testing. + */ +export default class LocalStorageService extends Service { + constructor() { + super(...arguments); + + this.storage = initStorage(); + this.buckets = new Map(); + } + + getBucket(key) { + const bucket = this.buckets.get(key); + + if (bucket) { + return bucket; + } else { + return this._setupBucket(key); + } + } + + _setupBucket(key) { + const owner = getOwner(this); + const Klass = owner.factoryFor(`storage:${key}`).class; + const storage = new Klass(key, this.storage); + + this.buckets.set(key, storage); + + return storage; + } +} diff --git a/ui/packages/consul-ui/app/storages/base.js b/ui/packages/consul-ui/app/storages/base.js new file mode 100644 index 0000000000..50a26fa669 --- /dev/null +++ b/ui/packages/consul-ui/app/storages/base.js @@ -0,0 +1,14 @@ +export default class Storage { + constructor(key, storage) { + this.key = key; + this.storage = storage; + + this.state = this.initState(this.key, this.storage); + } + + initState() { + const { key, storage } = this; + + return storage.getItem(key); + } +} diff --git a/ui/packages/consul-ui/app/storages/notices.js b/ui/packages/consul-ui/app/storages/notices.js new file mode 100644 index 0000000000..549219764c --- /dev/null +++ b/ui/packages/consul-ui/app/storages/notices.js @@ -0,0 +1,24 @@ +import { TrackedArray } from 'tracked-built-ins'; +import Storage from './base'; + +export default class Notices extends Storage { + initState() { + const { key, storage } = this; + + const persisted = storage.getItem(key); + + if (persisted) { + return new TrackedArray(persisted.split(',')); + } else { + return new TrackedArray(); + } + } + + add(value) { + const { key, storage, state } = this; + + state.push(value); + + storage.setItem(key, [...state]); + } +} diff --git a/ui/packages/consul-ui/package.json b/ui/packages/consul-ui/package.json index 146ff10df2..54bf1e27ab 100644 --- a/ui/packages/consul-ui/package.json +++ b/ui/packages/consul-ui/package.json @@ -191,6 +191,7 @@ "text-encoding": "^0.7.0", "tippy.js": "^6.2.7", "torii": "^0.10.1", + "tracked-built-ins": "^3.1.0", "unist-util-visit": "^2.0.3", "wayfarer": "^7.0.1", "webpack": "^5.74.0" diff --git a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js index 6260447efd..db05c8162f 100644 --- a/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js +++ b/ui/packages/consul-ui/tests/integration/components/consul/node/agentless-notice-test.js @@ -2,16 +2,9 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; import hbs from 'htmlbars-inline-precompile'; import { click, render } from '@ember/test-helpers'; -import sinon from 'sinon'; module('Integration | Component | consul node agentless-notice', function (hooks) { setupRenderingTest(hooks); - hooks.beforeEach(() => { - const localStore = {}; - - sinon.stub(window.localStorage, 'getItem').callsFake((key) => localStore[key]); - sinon.stub(window.localStorage, 'setItem').callsFake((key, value) => (localStore[key] = value)); - }); test('it does not display the notice if the filtered nodes are the same as the regular nodes', async function (assert) { this.set('nodes', [ @@ -33,7 +26,6 @@ module('Integration | Component | consul node agentless-notice', function (hooks await render( hbs`` ); - assert.true(window.localStorage.getItem.called); assert .dom('[data-test-node-agentless-notice]') .doesNotExist( @@ -66,10 +58,6 @@ module('Integration | Component | consul node agentless-notice', function (hooks assert .dom('[data-test-node-agentless-notice]') .doesNotExist('The agentless notice be dismissed'); - assert.true( - window.localStorage.setItem.calledOnceWith('consul-nodes-agentless-notice-dismissed', 'true'), - "Set the key in localstorage to 'true'" - ); }); test('it does not display if the localstorage key is already set to true', async function (assert) { @@ -81,30 +69,21 @@ module('Integration | Component | consul node agentless-notice', function (hooks }, ]); - this.set('filteredNodes', [ - { - Meta: { - 'synthetic-node': false, - }, - }, - ]); + this.set('filteredNodes', []); - window.localStorage.setItem('consul-nodes-agentless-notice-dismissed-partition', 'true'); + const localStorage = this.owner.lookup('service:local-storage'); + localStorage.storage.seed({ + notices: ['nodes-agentless-dismissed-partition'], + }); await render( hbs`` ); - assert.true( - window.localStorage.getItem.calledOnceWith( - 'consul-nodes-agentless-notice-dismissed-partition' - ) - ); - assert .dom('[data-test-node-agentless-notice]') .doesNotExist( - "The agentless notice should not display if the local storage key has already been set to 'true'" + 'The agentless notice should not display if the dismissal has already been stored in local storage' ); }); }); diff --git a/ui/yarn.lock b/ui/yarn.lock index a1b3fb4c01..88940cfa47 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -7862,7 +7862,7 @@ ember-auto-import@^2.2.3, ember-auto-import@^2.4.1, ember-auto-import@^2.4.2: typescript-memoize "^1.0.0-alpha.3" walk-sync "^3.0.0" -ember-basic-dropdown@3.0.21, ember-basic-dropdown@^3.0.16: +ember-basic-dropdown@^3.0.16: version "3.0.21" resolved "https://registry.yarnpkg.com/ember-basic-dropdown/-/ember-basic-dropdown-3.0.21.tgz#5711d071966919c9578d2d5ac2c6dcadbb5ea0e0" integrity sha512-Wu9hJWyqorKo+ZT2PMSIO1BxAeAdaiIC2IjSic0+HcKjmMU47botvG0xbxlprimOWaS9vM+nHat6Pt3xPvcB0A== @@ -7907,7 +7907,7 @@ ember-changeset-validations@~3.9.0: ember-get-config "^0.2.4" ember-validators "^2.0.0" -ember-changeset@3.10.1, ember-changeset@^3.9.1: +ember-changeset@^3.9.1: version "3.10.1" resolved "https://registry.yarnpkg.com/ember-changeset/-/ember-changeset-3.10.1.tgz#d6f06bc55f867a2c1ac7c5fd780776bd1e5a9b60" integrity sha512-4FoGKRcKxixSr+NBQ+ZoiwwbJE0/fuZRULUp9M1RIHejYhst+U8/ni47SsphrMhoRAcZCeyl+JqlBMlwR7v50g== @@ -7982,7 +7982,7 @@ ember-cli-babel@^6.0.0, ember-cli-babel@^6.0.0-beta.4, ember-cli-babel@^6.11.0, ember-cli-version-checker "^2.1.2" semver "^5.5.0" -ember-cli-babel@^7.12.0, ember-cli-babel@^7.13.2, ember-cli-babel@^7.26.1, ember-cli-babel@^7.26.11, ember-cli-babel@^7.26.3, ember-cli-babel@^7.26.5, ember-cli-babel@^7.4.0: +ember-cli-babel@^7.12.0, ember-cli-babel@^7.13.2, ember-cli-babel@^7.26.1, ember-cli-babel@^7.26.10, ember-cli-babel@^7.26.11, ember-cli-babel@^7.26.3, ember-cli-babel@^7.26.5, ember-cli-babel@^7.4.0: version "7.26.11" resolved "https://registry.yarnpkg.com/ember-cli-babel/-/ember-cli-babel-7.26.11.tgz#50da0fe4dcd99aada499843940fec75076249a9f" integrity sha512-JJYeYjiz/JTn34q7F5DSOjkkZqy8qwFOOxXfE6pe9yEJqWGu4qErKxlz8I22JoVEQ/aBUO+OcKTpmctvykM9YA== @@ -8448,6 +8448,22 @@ ember-cli-typescript@^5.0.0: stagehand "^1.0.0" walk-sync "^2.2.0" +ember-cli-typescript@^5.1.0: + version "5.1.1" + resolved "https://registry.yarnpkg.com/ember-cli-typescript/-/ember-cli-typescript-5.1.1.tgz#cf561026f3e7bd05312c1c212acffa1c30d5fa0c" + integrity sha512-DbzATYWY8nbXwSxXqtK8YlqGJTcyFyL+eg6IGCc2ur0AMnq/H+o6Z9np9eGoq1sI+HwX7vBkOVoD3k0WurAwXg== + dependencies: + ansi-to-html "^0.6.15" + broccoli-stew "^3.0.0" + debug "^4.0.0" + execa "^4.0.0" + fs-extra "^9.0.1" + resolve "^1.5.0" + rsvp "^4.8.1" + semver "^7.3.2" + stagehand "^1.0.0" + walk-sync "^2.2.0" + ember-cli-version-checker@^2.1.0, ember-cli-version-checker@^2.1.2: version "2.2.0" resolved "https://registry.yarnpkg.com/ember-cli-version-checker/-/ember-cli-version-checker-2.2.0.tgz#47771b731fe0962705e27c8199a9e3825709f3b3" @@ -9247,6 +9263,14 @@ ember-text-measurer@^0.6.0: ember-cli-babel "^7.19.0" ember-cli-htmlbars "^4.3.1" +ember-tracked-storage-polyfill@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/ember-tracked-storage-polyfill/-/ember-tracked-storage-polyfill-1.0.0.tgz#84d307a1e4badc5f84dca681db2cfea9bdee8a77" + integrity sha512-eL7lZat68E6P/D7b9UoTB5bB5Oh/0aju0Z7PCMi3aTwhaydRaxloE7TGrTRYU+NdJuyNVZXeGyxFxn2frvd3TA== + dependencies: + ember-cli-babel "^7.26.3" + ember-cli-htmlbars "^5.7.1" + "ember-truth-helpers@^2.1.0 || ^3.0.0", ember-truth-helpers@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/ember-truth-helpers/-/ember-truth-helpers-3.0.0.tgz#86766bdca4ac9b86bce3d262dff2aabc4a0ea384" @@ -16641,6 +16665,15 @@ tr46@~0.0.3: resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a" integrity sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw== +tracked-built-ins@^3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/tracked-built-ins/-/tracked-built-ins-3.1.0.tgz#827703e8e8857e45ac449dfc41e8706e0d6da309" + integrity sha512-yPEZV1aYaw7xFWdoEluvdwNxIJIA834HaBQaMATjNAYPwd1fRqIJ46YnuRo6+9mRRWu6nM6sJqrVVa5H6UhFuw== + dependencies: + ember-cli-babel "^7.26.10" + ember-cli-typescript "^5.1.0" + ember-tracked-storage-polyfill "^1.0.0" + tracked-maps-and-sets@^2.1.0: version "2.2.1" resolved "https://registry.yarnpkg.com/tracked-maps-and-sets/-/tracked-maps-and-sets-2.2.1.tgz#323dd40540c561e8b0ffdec8bf129c68ec5025f9" @@ -17177,7 +17210,7 @@ validate-peer-dependencies@^1.2.0: resolve-package-path "^3.1.0" semver "^7.3.2" -validated-changeset@0.10.0, validated-changeset@~0.10.0: +validated-changeset@~0.10.0: version "0.10.0" resolved "https://registry.yarnpkg.com/validated-changeset/-/validated-changeset-0.10.0.tgz#2e8188c089ab282c1b51fba3c289073f6bd14c8b" integrity sha512-n8NB3ol6Tbi0O7bnq1wz81m5Wd1gfHw0HUcH4MatOfqO3DyXzWZV+bUaNq6wThXn20rMFB82C8pTNFSWbgXJLA== @@ -17675,11 +17708,6 @@ xmlchars@^2.2.0: resolved "https://registry.yarnpkg.com/xmlchars/-/xmlchars-2.2.0.tgz#060fe1bcb7f9c76fe2a17db86a9bc3ab894210cb" integrity sha512-JZnDKK8B0RCDw84FNdDAIpZK+JuJw+s7Lz8nksI7SIuU3UXJJslUthsi+uWBUYOwPFwW7W7PRLRfUKpxjtjFCw== -xmlhttprequest-ssl@^1.6.3: - version "1.6.3" - resolved "https://registry.yarnpkg.com/xmlhttprequest-ssl/-/xmlhttprequest-ssl-1.6.3.tgz#03b713873b01659dfa2c1c5d056065b27ddc2de6" - integrity sha512-3XfeQE/wNkvrIktn2Kf0869fC0BN6UpydVasGIeSm2B1Llihf7/0UfZM+eCkOw3P7bP4+qPgqhm7ZoxuJtFU0Q== - xtend@^4.0.0, xtend@^4.0.1, xtend@^4.0.2, xtend@~4.0.1: version "4.0.2" resolved "https://registry.yarnpkg.com/xtend/-/xtend-4.0.2.tgz#bb72779f5fa465186b1f438f674fa347fdb5db54"