From 8d1ba1776680d66c65c64350522e48f2b5319222 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 23 Aug 2016 16:49:30 -0700 Subject: [PATCH] Rename HashingStore interface methods for consistency and clarity. Remove unused '#remove' method. --- .../state_management/__tests__/state.js | 30 ++++++------- src/ui/public/state_management/state.js | 23 +++++----- .../state_storage/_tests__/hashing_store.js | 45 +++++++------------ .../state_storage/_tests__/lazy_lru_store.js | 2 +- .../state_storage/hashing_store.js | 30 +++++-------- .../state_storage/lazy_lru_store.js | 18 ++++---- 6 files changed, 64 insertions(+), 84 deletions(-) diff --git a/src/ui/public/state_management/__tests__/state.js b/src/ui/public/state_management/__tests__/state.js index c9e25b07494ac..fd7e7ec4148a5 100644 --- a/src/ui/public/state_management/__tests__/state.js +++ b/src/ui/public/state_management/__tests__/state.js @@ -13,7 +13,7 @@ import StubBrowserStorage from 'test_utils/stub_browser_storage'; import EventsProvider from 'ui/events'; describe('State Management', function () { - const notify = new Notifier(); + const notifier = new Notifier(); let $rootScope; let $location; let State; @@ -33,13 +33,13 @@ describe('State Management', function () { sinon.stub(config, 'get').withArgs('state:storeInSessionStorage').returns(!!storeInHash); const store = new StubBrowserStorage(); const hashingStore = new HashingStore({ store }); - const state = new State(param, initial, { hashingStore, notify }); + const state = new State(param, initial, { hashingStore, notifier }); const getUnhashedSearch = state => { return unhashQueryString($location.search(), [ state ]); }; - return { notify, store, hashingStore, state, getUnhashedSearch }; + return { notifier, store, hashingStore, state, getUnhashedSearch }; }; })); @@ -185,7 +185,6 @@ describe('State Management', function () { }); }); - describe('Hashing', () => { it('stores state values in a hashingStore, writing the hash to the url', () => { const { state, hashingStore } = setup({ storeInHash: true }); @@ -194,7 +193,7 @@ describe('State Management', function () { const urlVal = $location.search()[state.getQueryParamName()]; expect(hashingStore.isHash(urlVal)).to.be(true); - expect(hashingStore.lookup(urlVal)).to.eql({ foo: 'bar' }); + expect(hashingStore.getItemAtHash(urlVal)).to.eql({ foo: 'bar' }); }); it('should replace rison in the URL with a hash', () => { @@ -208,29 +207,28 @@ describe('State Management', function () { const urlVal = $location.search()._s; expect(urlVal).to.not.be(rison); expect(hashingStore.isHash(urlVal)).to.be(true); - expect(hashingStore.lookup(urlVal)).to.eql(obj); + expect(hashingStore.getItemAtHash(urlVal)).to.eql(obj); }); context('error handling', () => { it('notifies the user when a hash value does not map to a stored value', () => { - const { state, hashingStore, notify } = setup({ storeInHash: true }); + const { state, hashingStore, notifier } = setup({ storeInHash: true }); const search = $location.search(); - const badHash = hashingStore.add({}); - hashingStore.remove(badHash); + const badHash = hashingStore._getShortHash('{"a": "b"}'); search[state.getQueryParamName()] = badHash; $location.search(search); - expect(notify._notifs).to.have.length(0); + expect(notifier._notifs).to.have.length(0); state.fetch(); - expect(notify._notifs).to.have.length(1); - expect(notify._notifs[0].content).to.match(/use the share functionality/i); + expect(notifier._notifs).to.have.length(1); + expect(notifier._notifs[0].content).to.match(/use the share functionality/i); }); - it('presents fatal error linking to github when hashingStore.add fails', () => { - const { state, hashingStore, notify } = setup({ storeInHash: true }); - const fatalStub = sinon.stub(notify, 'fatal').throws(); - sinon.stub(hashingStore, 'add').throws(); + it('presents fatal error linking to github when hashingStore.hashAndSetItem fails', () => { + const { state, hashingStore, notifier } = setup({ storeInHash: true }); + const fatalStub = sinon.stub(notifier, 'fatal').throws(); + sinon.stub(hashingStore, 'hashAndSetItem').throws(); expect(() => { state.toQueryParam(); diff --git a/src/ui/public/state_management/state.js b/src/ui/public/state_management/state.js index f06862010c738..f9e267b13855d 100644 --- a/src/ui/public/state_management/state.js +++ b/src/ui/public/state_management/state.js @@ -18,14 +18,14 @@ export default function StateProvider(Private, $rootScope, $location, config) { const Events = Private(EventsProvider); _.class(State).inherits(Events); - function State(urlParam, defaults, { hashingStore, notify } = {}) { + function State(urlParam, defaults, { hashingStore, notifier } = {}) { State.Super.call(this); let self = this; self.setDefaults(defaults); self._urlParam = urlParam || '_s'; - this._notify = notify || new Notifier(); - self._hasher = hashingStore || new HashingStore({ + this._notifier = notifier || new Notifier(); + self._hashingStore = hashingStore || new HashingStore({ store: new LazyLruStore({ id: `${this._urlParam}:state`, store: window.sessionStorage, @@ -67,7 +67,7 @@ export default function StateProvider(Private, $rootScope, $location, config) { return null; } - if (this._hasher.isHash(urlVal)) { + if (this._hashingStore.isHash(urlVal)) { return this._parseQueryParamValue(urlVal); } @@ -80,7 +80,7 @@ export default function StateProvider(Private, $rootScope, $location, config) { } if (unableToParse) { - this._notify.error('Unable to parse URL'); + this._notifier.error('Unable to parse URL'); search[this._urlParam] = this.toQueryParam(this._defaults); $location.search(search).replace(); } @@ -194,13 +194,13 @@ export default function StateProvider(Private, $rootScope, $location, config) { * @return {any} - the stored value, or null if hash does not resolve */ State.prototype._parseQueryParamValue = function (queryParam) { - if (!this._hasher.isHash(queryParam)) { + if (!this._hashingStore.isHash(queryParam)) { return rison.decode(queryParam); } - const stored = this._hasher.lookup(queryParam); + const stored = this._hashingStore.getItemAtHash(queryParam); if (stored === null) { - this._notify.error('Unable to completely restore the URL, be sure to use the share functionality.'); + this._notifier.error('Unable to completely restore the URL, be sure to use the share functionality.'); } return stored; @@ -228,10 +228,11 @@ export default function StateProvider(Private, $rootScope, $location, config) { } try { - return this._hasher.add(state); + const hash = this._hashingStore.hashAndSetItem(state); + return hash; } catch (err) { - this._notify.log('Unable to create hash of State due to error: ' + (state.stack || state.message)); - this._notify.fatal( + this._notifier.log('Unable to create hash of State due to error: ' + (state.stack || state.message)); + this._notifier.fatal( new Error( 'Kibana is unable to store history items in your session ' + 'because it is full and there don\'t seem to be items any items safe ' + diff --git a/src/ui/public/state_management/state_storage/_tests__/hashing_store.js b/src/ui/public/state_management/state_storage/_tests__/hashing_store.js index c8d42ac321cd5..58b89e5f7d22c 100644 --- a/src/ui/public/state_management/state_storage/_tests__/hashing_store.js +++ b/src/ui/public/state_management/state_storage/_tests__/hashing_store.js @@ -11,12 +11,12 @@ const setup = ({ createHash } = {}) => { return { store, hashingStore }; }; -describe('State Management Hashing Store', () => { - describe('#add', () => { +describe('Hashing Store', () => { + describe('#hashAndSetItem', () => { it('adds a value to the store and returns its hash', () => { const { hashingStore, store } = setup(); const val = { foo: 'bar' }; - const hash = hashingStore.add(val); + const hash = hashingStore.hashAndSetItem(val); expect(hash).to.be.a('string'); expect(hash).to.be.ok(); expect(store).to.have.length(1); @@ -25,8 +25,8 @@ describe('State Management Hashing Store', () => { it('json encodes the values it stores', () => { const { hashingStore, store } = setup(); const val = { toJSON() { return 1; } }; - const hash = hashingStore.add(val); - expect(hashingStore.lookup(hash)).to.eql(1); + const hash = hashingStore.hashAndSetItem(val); + expect(hashingStore.getItemAtHash(hash)).to.eql(1); }); it('addresses values with a short hash', () => { @@ -36,7 +36,7 @@ describe('State Management Hashing Store', () => { createHash: () => longHash }); - const hash = hashingStore.add(val); + const hash = hashingStore.hashAndSetItem(val); expect(hash.length < longHash.length).to.be.ok(); }); @@ -64,48 +64,37 @@ describe('State Management Hashing Store', () => { } }); - const hash1 = hashingStore.add(fixtures[0].val); - const hash2 = hashingStore.add(fixtures[1].val); - const hash3 = hashingStore.add(fixtures[2].val); + const hash1 = hashingStore.hashAndSetItem(fixtures[0].val); + const hash2 = hashingStore.hashAndSetItem(fixtures[1].val); + const hash3 = hashingStore.hashAndSetItem(fixtures[2].val); expect(hash3).to.have.length(hash2.length + 1); expect(hash2).to.have.length(hash1.length + 1); }); - it('bubbles up the error if the store fails to setItem', () => { + it('bubbles up the error if the store fails to hashAndSetItem', () => { const { store, hashingStore } = setup(); const err = new Error(); sinon.stub(store, 'setItem').throws(err); expect(() => { - hashingStore.add({}); + hashingStore.hashAndSetItem({}); }).to.throwError(e => expect(e).to.be(err)); }); }); - describe('#lookup', () => { + describe('#getItemAtHash', () => { it('reads a value from the store by its hash', () => { const { hashingStore } = setup(); const val = { foo: 'bar' }; - const hash = hashingStore.add(val); - expect(hashingStore.lookup(hash)).to.eql(val); + const hash = hashingStore.hashAndSetItem(val); + expect(hashingStore.getItemAtHash(hash)).to.eql(val); }); it('returns null when the value is not in the store', () => { const { hashingStore } = setup(); const val = { foo: 'bar' }; - const hash = hashingStore.add(val); - expect(hashingStore.lookup(`${hash} break`)).to.be(null); - }); - }); - - describe('#remove', () => { - it('removes the value by its hash', () => { - const { hashingStore } = setup(); - const val = { foo: 'bar' }; - const hash = hashingStore.add(val); - expect(hashingStore.lookup(hash)).to.eql(val); - hashingStore.remove(hash); - expect(hashingStore.lookup(hash)).to.be(null); + const hash = hashingStore.hashAndSetItem(val); + expect(hashingStore.getItemAtHash(`${hash} break`)).to.be(null); }); }); @@ -113,7 +102,7 @@ describe('State Management Hashing Store', () => { it('can identify values that look like hashes', () => { const { hashingStore } = setup(); const val = { foo: 'bar' }; - const hash = hashingStore.add(val); + const hash = hashingStore.hashAndSetItem(val); expect(hashingStore.isHash(hash)).to.be(true); }); diff --git a/src/ui/public/state_management/state_storage/_tests__/lazy_lru_store.js b/src/ui/public/state_management/state_storage/_tests__/lazy_lru_store.js index 2fa5f5cf335f8..9f1a0a4f0aec9 100644 --- a/src/ui/public/state_management/state_storage/_tests__/lazy_lru_store.js +++ b/src/ui/public/state_management/state_storage/_tests__/lazy_lru_store.js @@ -27,7 +27,7 @@ const setup = (opts = {}) => { return { lru, store }; }; -describe('State Management LazyLruStore', () => { +describe('LazyLruStore', () => { describe('#getItem()', () => { it('returns null when item not found', () => { const { lru } = setup(); diff --git a/src/ui/public/state_management/state_storage/hashing_store.js b/src/ui/public/state_management/state_storage/hashing_store.js index b968f29f087ce..c444e4df32fe7 100644 --- a/src/ui/public/state_management/state_storage/hashing_store.js +++ b/src/ui/public/state_management/state_storage/hashing_store.js @@ -5,15 +5,13 @@ import { Sha256 } from 'ui/crypto'; import StubBrowserStorage from 'test_utils/stub_browser_storage'; import { LazyLruStore } from './lazy_lru_store'; -const TAG = 'h@'; - /** * The HashingStore is a wrapper around a browser store object * that hashes the items added to it and stores them by their * hash. This hash is then returned so that the item can be received * at a later time. */ -export default class HashingStore { +class HashingStore { constructor({ store, createHash, maxItems } = {}) { this._store = store || window.sessionStorage; if (createHash) this._createHash = createHash; @@ -22,11 +20,11 @@ export default class HashingStore { /** * Determine if the passed value looks like a hash * - * @param {string} hash + * @param {string} str * @return {boolean} */ - isHash(hash) { - return String(hash).slice(0, TAG.length) === TAG; + isHash(str) { + return String(str).indexOf(HashingStore.HASH_TAG) === 0; } /** @@ -35,7 +33,7 @@ export default class HashingStore { * @param {string} hash * @return {any} */ - lookup(hash) { + getItemAtHash(hash) { try { return JSON.parse(this._store.getItem(hash)); } catch (err) { @@ -50,23 +48,13 @@ export default class HashingStore { * @param {any} the value to hash * @return {string} the hash of the value */ - add(object) { + hashAndSetItem(object) { const json = angular.toJson(object); const hash = this._getShortHash(json); this._store.setItem(hash, json); return hash; } - /** - * Remove a value identified by the hash from the store - * - * @param {string} hash - * @return {undefined} - */ - remove(hash) { - this._store.removeItem(hash); - } - // private api /** @@ -89,7 +77,7 @@ export default class HashingStore { * @param {string} shortHash */ _getShortHash(json) { - const fullHash = `${TAG}${this._createHash(json)}`; + const fullHash = `${HashingStore.HASH_TAG}${this._createHash(json)}`; let short; for (let i = 7; i < fullHash.length; i++) { @@ -101,3 +89,7 @@ export default class HashingStore { return short; } } + +HashingStore.HASH_TAG = 'h@'; + +export default HashingStore; diff --git a/src/ui/public/state_management/state_storage/lazy_lru_store.js b/src/ui/public/state_management/state_storage/lazy_lru_store.js index 14f015287796d..a25489b7ac8f4 100644 --- a/src/ui/public/state_management/state_storage/lazy_lru_store.js +++ b/src/ui/public/state_management/state_storage/lazy_lru_store.js @@ -36,7 +36,7 @@ export default class LazyLruStore { const { id, store, - notify = new Notifier(`LazyLruStore (re: probably history hashing)`), + notifier = new Notifier(`LazyLruStore (re: probably history hashing)`), maxItems = Infinity, maxSetAttempts = DEFAULT_MAX_SET_ATTEMPTS, idealClearRatio = DEFAULT_IDEAL_CLEAR_RATIO, @@ -54,7 +54,7 @@ export default class LazyLruStore { this._id = id; this._prefix = `lru:${this._id}:`; this._store = store; - this._notify = notify; + this._notifier = notifier; this._maxItems = maxItems; this._itemCountGuess = this._getItemCount(); this._maxSetAttempts = maxSetAttempts; @@ -135,7 +135,7 @@ export default class LazyLruStore { * then this function will call itself again, this time sending * attempt + 1 as the attempt number. If this loop continues * and attempt meets or exceeds the this._maxSetAttempts then a fatal - * error will be sent to notify, as the users session is no longer + * error will be sent to notifier, as the users session is no longer * usable. * * @private @@ -173,7 +173,7 @@ export default class LazyLruStore { */ _indexStoredItems() { const store = this._store; - const notify = this._notify; + const notifier = this._notifier; const items = []; let totalBytes = 0; @@ -228,8 +228,8 @@ export default class LazyLruStore { * @return {boolean} success */ _makeSpaceFor(key, chunk) { - const notify = this._notify; - return notify.event(`trying to make room in lru ${this._id}`, () => { + const notifier = this._notifier; + return notifier.event(`trying to make room in lru ${this._id}`, () => { const { totalBytes, itemsByOldestAccess } = this._indexStoredItems(); // pick how much space we are going to try to clear @@ -238,7 +238,7 @@ export default class LazyLruStore { const freeMin = key.length + chunk.length; const freeIdeal = freeMin * this._idealClearRatio; const toClear = Math.max(freeMin, Math.min(freeIdeal, totalBytes * this._maxIdealClearPercent)); - notify.log(`PLAN: min ${freeMin} bytes, target ${toClear} bytes`); + notifier.log(`PLAN: min ${freeMin} bytes, target ${toClear} bytes`); let remainingToClear = toClear; let removedItemCount = 0; @@ -253,7 +253,7 @@ export default class LazyLruStore { const label = success ? 'SUCCESS' : 'FAILURE'; const removedBytes = toClear - remainingToClear; - notify.log(`${label}: removed ${removedItemCount} items for ${removedBytes} bytes`); + notifier.log(`${label}: removed ${removedItemCount} items for ${removedBytes} bytes`); return success; }); } @@ -269,7 +269,7 @@ export default class LazyLruStore { */ _doItemAutoRemoval(item) { const timeString = new Date(item.time).toISOString(); - this._notify.log(`REMOVE: entry "${item.key}" from ${timeString}, freeing ${item.bytes} bytes`); + this._notifier.log(`REMOVE: entry "${item.key}" from ${timeString}, freeing ${item.bytes} bytes`); this._store.removeItem(item.key); this._itemCountGuess -= 1; }