Skip to content

Commit

Permalink
Merge pull request #17 from cjcenizal/implement/storeStateInLocalstor…
Browse files Browse the repository at this point in the history
…age/refactor-storage-naming

Rename HashingStore interface methods for consistency and clarity.
  • Loading branch information
spalger authored Aug 24, 2016
2 parents 2099643 + 8d1ba17 commit 53da9e8
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 84 deletions.
30 changes: 14 additions & 16 deletions src/ui/public/state_management/__tests__/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 };
};
}));

Expand Down Expand Up @@ -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 });
Expand All @@ -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', () => {
Expand All @@ -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();
Expand Down
23 changes: 12 additions & 11 deletions src/ui/public/state_management/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand All @@ -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();
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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', () => {
Expand All @@ -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();
});

Expand Down Expand Up @@ -64,56 +64,45 @@ 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);
});
});

describe('#isHash', () => {
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);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
30 changes: 11 additions & 19 deletions src/ui/public/state_management/state_storage/hashing_store.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

/**
Expand All @@ -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) {
Expand All @@ -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

/**
Expand All @@ -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++) {
Expand All @@ -101,3 +89,7 @@ export default class HashingStore {
return short;
}
}

HashingStore.HASH_TAG = 'h@';

export default HashingStore;
Loading

0 comments on commit 53da9e8

Please sign in to comment.