Merge pull request #18 from cjcenizal/implement/storeStateInLocalstor…

…age/simplify-hashing-store-interface Simplify HashingStore interface.
spalger · Aug 24, 2016 · 35cf9a2 · 35cf9a2
2 parents 53da9e8 + 2a203ae
commit 35cf9a2
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 55 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,3 +1,7 @@
+export {
+  default as createStorageHash,
+} from './create_storage_hash';
+
 export {
   default as HashingStore,
 } from './hashing_store';