From 2a203ae901e7697b4f7be8e5b52a93abd361cab9 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 23 Aug 2016 22:17:20 -0700 Subject: [PATCH] Simplify HashingStore interface. Extract createHash method into a createStorageHash module. --- .../state_management/__tests__/state.js | 11 ++++-- src/ui/public/state_management/state.js | 25 +++++++------ .../state_storage/_tests__/hashing_store.js | 36 +++++++++---------- .../state_storage/create_storage_hash.js | 5 +++ .../state_storage/hashing_store.js | 27 +++----------- .../state_management/state_storage/index.js | 4 +++ 6 files changed, 53 insertions(+), 55 deletions(-) create mode 100644 src/ui/public/state_management/state_storage/create_storage_hash.js diff --git a/src/ui/public/state_management/__tests__/state.js b/src/ui/public/state_management/__tests__/state.js index fd7e7ec4148a5..261ab27f48928 100644 --- a/src/ui/public/state_management/__tests__/state.js +++ b/src/ui/public/state_management/__tests__/state.js @@ -7,8 +7,13 @@ import { encode as encodeRison } from 'rison-node'; import 'ui/private'; import Notifier from 'ui/notify/notifier'; import StateManagementStateProvider from 'ui/state_management/state'; -import { unhashQueryString } from 'ui/state_management/state_hashing'; -import { HashingStore } from 'ui/state_management/state_storage'; +import { + unhashQueryString, +} from 'ui/state_management/state_hashing'; +import { + createStorageHash, + HashingStore, +} from 'ui/state_management/state_storage'; import StubBrowserStorage from 'test_utils/stub_browser_storage'; import EventsProvider from 'ui/events'; @@ -32,7 +37,7 @@ describe('State Management', function () { const { param, initial, storeInHash } = (opts || {}); sinon.stub(config, 'get').withArgs('state:storeInSessionStorage').returns(!!storeInHash); const store = new StubBrowserStorage(); - const hashingStore = new HashingStore({ store }); + const hashingStore = new HashingStore(createStorageHash, store); const state = new State(param, initial, { hashingStore, notifier }); const getUnhashedSearch = state => { diff --git a/src/ui/public/state_management/state.js b/src/ui/public/state_management/state.js index f9e267b13855d..cf05585431054 100644 --- a/src/ui/public/state_management/state.js +++ b/src/ui/public/state_management/state.js @@ -8,6 +8,7 @@ import Notifier from 'ui/notify/notifier'; import KbnUrlProvider from 'ui/url'; import { + createStorageHash, HashingStore, LazyLruStore, } from './state_storage'; @@ -21,34 +22,36 @@ export default function StateProvider(Private, $rootScope, $location, config) { function State(urlParam, defaults, { hashingStore, notifier } = {}) { State.Super.call(this); - let self = this; - self.setDefaults(defaults); - self._urlParam = urlParam || '_s'; + this.setDefaults(defaults); + this._urlParam = urlParam || '_s'; this._notifier = notifier || new Notifier(); - self._hashingStore = hashingStore || new HashingStore({ - store: new LazyLruStore({ + + this._hashingStore = hashingStore || (() => { + const lazyLruStore = new LazyLruStore({ id: `${this._urlParam}:state`, store: window.sessionStorage, maxItems: MAX_BROWSER_HISTORY - }) - }); + }); + + return new HashingStore(createStorageHash, lazyLruStore); + })(); // When the URL updates we need to fetch the values from the URL - self._cleanUpListeners = _.partial(_.callEach, [ + this._cleanUpListeners = _.partial(_.callEach, [ // partial route update, no app reload - $rootScope.$on('$routeUpdate', function () { + $rootScope.$on('$routeUpdate', () => { self.fetch(); }), // beginning of full route update, new app will be initialized before // $routeChangeSuccess or $routeChangeError - $rootScope.$on('$routeChangeStart', function () { + $rootScope.$on('$routeChangeStart', () => { if (!self._persistAcrossApps) { self.destroy(); } }), - $rootScope.$on('$routeChangeSuccess', function () { + $rootScope.$on('$routeChangeSuccess', () => { if (self._persistAcrossApps) { self.fetch(); } 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 58b89e5f7d22c..d80ac3d7dc14a 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 @@ -1,20 +1,22 @@ import expect from 'expect.js'; import sinon from 'sinon'; import { encode as encodeRison } from 'rison-node'; - import StubBrowserStorage from 'test_utils/stub_browser_storage'; -import { HashingStore } from 'ui/state_management/state_storage'; +import { + createStorageHash, + HashingStore, +} from 'ui/state_management/state_storage'; -const setup = ({ createHash } = {}) => { +const setup = createStorageHash => { const store = new StubBrowserStorage(); - const hashingStore = new HashingStore({ store, createHash }); + const hashingStore = new HashingStore(createStorageHash, store); return { store, hashingStore }; }; describe('Hashing Store', () => { describe('#hashAndSetItem', () => { it('adds a value to the store and returns its hash', () => { - const { hashingStore, store } = setup(); + const { hashingStore, store } = setup(createStorageHash); const val = { foo: 'bar' }; const hash = hashingStore.hashAndSetItem(val); expect(hash).to.be.a('string'); @@ -23,7 +25,7 @@ describe('Hashing Store', () => { }); it('json encodes the values it stores', () => { - const { hashingStore, store } = setup(); + const { hashingStore, store } = setup(createStorageHash); const val = { toJSON() { return 1; } }; const hash = hashingStore.hashAndSetItem(val); expect(hashingStore.getItemAtHash(hash)).to.eql(1); @@ -32,9 +34,7 @@ describe('Hashing Store', () => { it('addresses values with a short hash', () => { const val = { foo: 'bar' }; const longHash = 'longlonglonglonglonglonglonglonglonglonghash'; - const { hashingStore } = setup({ - createHash: () => longHash - }); + const { hashingStore } = setup(() => longHash); const hash = hashingStore.hashAndSetItem(val); expect(hash.length < longHash.length).to.be.ok(); @@ -57,11 +57,9 @@ describe('Hashing Store', () => { ]; const matchVal = json => f => JSON.stringify(f.val) === json; - const { hashingStore } = setup({ - createHash: val => { - const fixture = fixtures.find(matchVal(val)); - return fixture.hash; - } + const { hashingStore } = setup(val => { + const fixture = fixtures.find(matchVal(val)); + return fixture.hash; }); const hash1 = hashingStore.hashAndSetItem(fixtures[0].val); @@ -73,7 +71,7 @@ describe('Hashing Store', () => { }); it('bubbles up the error if the store fails to hashAndSetItem', () => { - const { store, hashingStore } = setup(); + const { store, hashingStore } = setup(createStorageHash); const err = new Error(); sinon.stub(store, 'setItem').throws(err); expect(() => { @@ -84,14 +82,14 @@ describe('Hashing Store', () => { describe('#getItemAtHash', () => { it('reads a value from the store by its hash', () => { - const { hashingStore } = setup(); + const { hashingStore } = setup(createStorageHash); const val = { foo: 'bar' }; 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 { hashingStore } = setup(createStorageHash); const val = { foo: 'bar' }; const hash = hashingStore.hashAndSetItem(val); expect(hashingStore.getItemAtHash(`${hash} break`)).to.be(null); @@ -100,7 +98,7 @@ describe('Hashing Store', () => { describe('#isHash', () => { it('can identify values that look like hashes', () => { - const { hashingStore } = setup(); + const { hashingStore } = setup(createStorageHash); const val = { foo: 'bar' }; const hash = hashingStore.hashAndSetItem(val); expect(hashingStore.isHash(hash)).to.be(true); @@ -118,7 +116,7 @@ describe('Hashing Store', () => { tests.forEach(([type, val]) => { it(`is not fooled by rison ${type} "${val}"`, () => { - const { hashingStore } = setup(); + const { hashingStore } = setup(createStorageHash); const rison = encodeRison(val); expect(hashingStore.isHash(rison)).to.be(false); }); diff --git a/src/ui/public/state_management/state_storage/create_storage_hash.js b/src/ui/public/state_management/state_storage/create_storage_hash.js new file mode 100644 index 0000000000000..b90cde31c0523 --- /dev/null +++ b/src/ui/public/state_management/state_storage/create_storage_hash.js @@ -0,0 +1,5 @@ +import { Sha256 } from 'ui/crypto'; + +export default function createStorageHash(json) { + return new Sha256().update(json, 'utf8').digest('hex'); +} 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 c444e4df32fe7..a1c194efeb10a 100644 --- a/src/ui/public/state_management/state_storage/hashing_store.js +++ b/src/ui/public/state_management/state_storage/hashing_store.js @@ -1,9 +1,4 @@ import angular from 'angular'; -import { sortBy } from 'lodash'; -import { Sha256 } from 'ui/crypto'; - -import StubBrowserStorage from 'test_utils/stub_browser_storage'; -import { LazyLruStore } from './lazy_lru_store'; /** * The HashingStore is a wrapper around a browser store object @@ -12,9 +7,9 @@ import { LazyLruStore } from './lazy_lru_store'; * at a later time. */ class HashingStore { - constructor({ store, createHash, maxItems } = {}) { - this._store = store || window.sessionStorage; - if (createHash) this._createHash = createHash; + constructor(createStorageHash, store) { + this._createStorageHash = createStorageHash; + this._store = store; } /** @@ -49,25 +44,13 @@ class HashingStore { * @return {string} the hash of the value */ hashAndSetItem(object) { + // The object may contain Angular $$ properties, so let's ignore them. const json = angular.toJson(object); const hash = this._getShortHash(json); this._store.setItem(hash, json); return hash; } - // private api - - /** - * calculate the full hash of a json object - * - * @private - * @param {string} json - * @return {string} hash - */ - _createHash(json) { - return new Sha256().update(json, 'utf8').digest('hex'); - } - /** * Calculate the full hash for a json blob and then shorten in until * it until it doesn't collide with other short hashes in the store @@ -77,7 +60,7 @@ class HashingStore { * @param {string} shortHash */ _getShortHash(json) { - const fullHash = `${HashingStore.HASH_TAG}${this._createHash(json)}`; + const fullHash = `${HashingStore.HASH_TAG}${this._createStorageHash(json)}`; let short; for (let i = 7; i < fullHash.length; i++) { diff --git a/src/ui/public/state_management/state_storage/index.js b/src/ui/public/state_management/state_storage/index.js index 5c80b0f471901..99db210733cae 100644 --- a/src/ui/public/state_management/state_storage/index.js +++ b/src/ui/public/state_management/state_storage/index.js @@ -1,3 +1,7 @@ +export { + default as createStorageHash, +} from './create_storage_hash'; + export { default as HashingStore, } from './hashing_store';