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

…age/simplify-lazy-lru-store

Another approach to managing persisted states.

src/test_utils/__tests__/stub_browser_storage.js

@@ -49,10 +49,10 @@ describe('StubBrowserStorage', () => {
     });
   });
 
-  describe('size limiting', () => {
+  describe('#setStubbedSizeLimit', () => {
     it('allows limiting the storage size', () => {
       const store = new StubBrowserStorage();
-      store._setSizeLimit(10);
+      store.setStubbedSizeLimit(10);
       store.setItem('abc', 'def'); // store size is 6, key.length + val.length
       expect(() => {
         store.setItem('ghi', 'jkl');
@@ -61,25 +61,41 @@ describe('StubBrowserStorage', () => {
 
     it('allows defining the limit as infinity', () => {
       const store = new StubBrowserStorage();
-      store._setSizeLimit(Infinity);
+      store.setStubbedSizeLimit(Infinity);
       store.setItem('abc', 'def');
       store.setItem('ghi', 'jkl'); // unlike the previous test, this doesn't throw
     });
 
-    it('requires setting the limit before keys', () => {
+    it('throws an error if the limit is below the current size', () => {
       const store = new StubBrowserStorage();
       store.setItem('key', 'val');
       expect(() => {
-        store._setSizeLimit(10);
-      }).throwError(/before setting/);
+        store.setStubbedSizeLimit(5);
+      }).throwError(Error);
     });
 
     it('respects removed items', () => {
       const store = new StubBrowserStorage();
-      store._setSizeLimit(10);
+      store.setStubbedSizeLimit(10);
       store.setItem('abc', 'def');
       store.removeItem('abc');
       store.setItem('ghi', 'jkl'); // unlike the previous test, this doesn't throw
     });
   });
+
+  describe('#getStubbedSizeLimit', () => {
+    it('returns the size limit', () => {
+      const store = new StubBrowserStorage();
+      store.setStubbedSizeLimit(10);
+      expect(store.getStubbedSizeLimit()).to.equal(10);
+    });
+  });
+
+  describe('#getStubbedSize', () => {
+    it('returns the size', () => {
+      const store = new StubBrowserStorage();
+      store.setItem(1, 1);
+      expect(store.getStubbedSize()).to.equal(2);
+    });
+  });
 });

src/test_utils/stub_browser_storage.js

@@ -1,92 +1,109 @@
-const keys = Symbol('keys');
-const values = Symbol('values');
-const remainingSize = Symbol('remainingSize');
-
 export default class StubBrowserStorage {
   constructor() {
-    this[keys] = [];
-    this[values] = [];
-    this[remainingSize] = 5000000; // 5mb, minimum browser storage size
+    this._keys = [];
+    this._values = [];
+    this._size = 0;
+    this._sizeLimit = 5000000; // 5mb, minimum browser storage size
   }
 
+  // -----------------------------------------------------------------------------------------------
+  // Browser-specific methods.
+  // -----------------------------------------------------------------------------------------------
+
   get length() {
-    return this[keys].length;
+    return this._keys.length;
   }
 
   key(i) {
-    return this[keys][i];
+    return this._keys[i];
   }
 
   getItem(key) {
     key = String(key);
 
-    const i = this[keys].indexOf(key);
+    const i = this._keys.indexOf(key);
     if (i === -1) return null;
-    return this[values][i];
+    return this._values[i];
   }
 
   setItem(key, value) {
     key = String(key);
     value = String(value);
-    this._takeUpSpace(this._calcSizeOfAdd(key, value));
+    const sizeOfAddition = this._getSizeOfAddition(key, value);
+    this._updateSize(sizeOfAddition);
 
-    const i = this[keys].indexOf(key);
+    const i = this._keys.indexOf(key);
     if (i === -1) {
-      this[keys].push(key);
-      this[values].push(value);
+      this._keys.push(key);
+      this._values.push(value);
     } else {
-      this[values][i] = value;
+      this._values[i] = value;
     }
   }
 
   removeItem(key) {
     key = String(key);
-    this._takeUpSpace(this._calcSizeOfRemove(key));
+    const sizeOfRemoval = this._getSizeOfRemoval(key);
+    this._updateSize(sizeOfRemoval);
 
-    const i = this[keys].indexOf(key);
+    const i = this._keys.indexOf(key);
     if (i === -1) return;
-    this[keys].splice(i, 1);
-    this[values].splice(i, 1);
+    this._keys.splice(i, 1);
+    this._values.splice(i, 1);
   }
 
-  // non-standard api methods
-  _getKeys() {
-    return this[keys].slice();
+  // -----------------------------------------------------------------------------------------------
+  // Test-specific methods.
+  // -----------------------------------------------------------------------------------------------
+
+  getStubbedKeys() {
+    return this._keys.slice();
   }
 
-  _getValues() {
-    return this[values].slice();
+  getStubbedValues() {
+    return this._values.slice();
   }
 
-  _setSizeLimit(limit) {
-    if (this[keys].length) {
-      throw new Error('You must call _setSizeLimit() before setting any values');
+  setStubbedSizeLimit(sizeLimit) {
+    // We can't reconcile a size limit with the "stored" items, if the stored items size exceeds it.
+    if (sizeLimit < this._size) {
+      throw new Error(`You can't set a size limit smaller than the current size.`);
     }
 
-    this[remainingSize] = limit;
+    this._sizeLimit = sizeLimit;
+  }
+
+  getStubbedSizeLimit() {
+    return this._sizeLimit;
+  }
+
+  getStubbedSize() {
+    return this._size;
   }
 
-  _calcSizeOfAdd(key, value) {
-    const i = this[keys].indexOf(key);
+  _getSizeOfAddition(key, value) {
+    const i = this._keys.indexOf(key);
     if (i === -1) {
       return key.length + value.length;
     }
-    return value.length - this[values][i].length;
+    // Return difference of what's been stored, and what *will* be stored.
+    return value.length - this._values[i].length;
   }
 
-  _calcSizeOfRemove(key) {
-    const i = this[keys].indexOf(key);
+  _getSizeOfRemoval(key) {
+    const i = this._keys.indexOf(key);
     if (i === -1) {
       return 0;
     }
-    return 0 - (key.length + this[values][i].length);
+    // Return negative value.
+    return -(key.length + this._values[i].length);
   }
 
-  _takeUpSpace(delta) {
-    if (this[remainingSize] - delta < 0) {
+  _updateSize(delta) {
+    if (this._size + delta > this._sizeLimit) {
       throw new Error('something about quota exceeded, browsers are not consistent here');
     }
 
-    this[remainingSize] -= delta;
+    this._size += delta;
   }
 }

src/ui/public/chrome/api/__tests__/apps.js

@@ -152,8 +152,8 @@ describe('Chrome API :: apps', function () {
         expect(chrome.getLastUrlFor('app')).to.equal(null);
         chrome.setLastUrlFor('app', 'url');
         expect(chrome.getLastUrlFor('app')).to.equal('url');
-        expect(store._getKeys().length).to.equal(1);
-        expect(store._getValues().shift()).to.equal('url');
+        expect(store.getStubbedKeys().length).to.equal(1);
+        expect(store.getStubbedValues().shift()).to.equal('url');
       });
     });
   });

src/ui/public/state_management/__tests__/state.js

@@ -11,9 +11,10 @@ import {
   unhashQueryString,
 } from 'ui/state_management/state_hashing';
 import {
-  createStorageHash,
-  HashingStore,
+  createStateHash,
+  isStateHash,
 } from 'ui/state_management/state_storage';
+import HashedItemStore from 'ui/state_management/state_storage/hashed_item_store';
 import StubBrowserStorage from 'test_utils/stub_browser_storage';
 import EventsProvider from 'ui/events';
 
@@ -37,14 +38,14 @@ 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(createStorageHash, store);
-      const state = new State(param, initial, { hashingStore, notifier });
+      const hashedItemStore = new HashedItemStore(store);
+      const state = new State(param, initial, hashedItemStore, notifier);
 
       const getUnhashedSearch = state => {
         return unhashQueryString($location.search(), [ state ]);
       };
 
-      return { notifier, store, hashingStore, state, getUnhashedSearch };
+      return { notifier, store, hashedItemStore, state, getUnhashedSearch };
     };
   }));
 
@@ -191,18 +192,18 @@ 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 });
+    it('stores state values in a hashedItemStore, writing the hash to the url', () => {
+      const { state, hashedItemStore } = setup({ storeInHash: true });
       state.foo = 'bar';
       state.save();
       const urlVal = $location.search()[state.getQueryParamName()];
 
-      expect(hashingStore.isHash(urlVal)).to.be(true);
-      expect(hashingStore.getItemAtHash(urlVal)).to.eql({ foo: 'bar' });
+      expect(isStateHash(urlVal)).to.be(true);
+      expect(hashedItemStore.getItem(urlVal)).to.eql(JSON.stringify({ foo: 'bar' }));
     });
 
     it('should replace rison in the URL with a hash', () => {
-      const { state, hashingStore } = setup({ storeInHash: true });
+      const { state, hashedItemStore } = setup({ storeInHash: true });
       const obj = { foo: { bar: 'baz' } };
       const rison = encodeRison(obj);
 
@@ -211,15 +212,15 @@ describe('State Management', function () {
 
       const urlVal = $location.search()._s;
       expect(urlVal).to.not.be(rison);
-      expect(hashingStore.isHash(urlVal)).to.be(true);
-      expect(hashingStore.getItemAtHash(urlVal)).to.eql(obj);
+      expect(isStateHash(urlVal)).to.be(true);
+      expect(hashedItemStore.getItem(urlVal)).to.eql(JSON.stringify(obj));
     });
 
     context('error handling', () => {
       it('notifies the user when a hash value does not map to a stored value', () => {
-        const { state, hashingStore, notifier } = setup({ storeInHash: true });
+        const { state, hashedItemStore, notifier } = setup({ storeInHash: true });
         const search = $location.search();
-        const badHash = hashingStore._getShortHash('{"a": "b"}');
+        const badHash = createStateHash('{"a": "b"}', () => null);
 
         search[state.getQueryParamName()] = badHash;
         $location.search(search);
@@ -230,10 +231,10 @@ describe('State Management', function () {
         expect(notifier._notifs[0].content).to.match(/use the share functionality/i);
       });
 
-      it('presents fatal error linking to github when hashingStore.hashAndSetItem fails', () => {
-        const { state, hashingStore, notifier } = setup({ storeInHash: true });
+      it('presents fatal error linking to github when setting item fails', () => {
+        const { state, hashedItemStore, notifier } = setup({ storeInHash: true });
         const fatalStub = sinon.stub(notifier, 'fatal').throws();
-        sinon.stub(hashingStore, 'hashAndSetItem').throws();
+        sinon.stub(hashedItemStore, 'setItem').returns(false);
 
         expect(() => {
           state.toQueryParam();