Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[state] store actual state value in session storage #8022

Merged
merged 12 commits into from
Sep 7, 2016
Merged
23 changes: 14 additions & 9 deletions src/core_plugins/kibana/public/discover/controllers/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import _ from 'lodash';
import angular from 'angular';
import moment from 'moment';
import getSort from 'ui/doc_table/lib/get_sort';
import rison from 'rison-node';
import dateMath from '@elastic/datemath';
import 'ui/doc_table';
import 'ui/visualize';
Expand All @@ -26,8 +25,7 @@ import AggTypesBucketsIntervalOptionsProvider from 'ui/agg_types/buckets/_interv
import uiRoutes from 'ui/routes';
import uiModules from 'ui/modules';
import indexTemplate from 'plugins/kibana/discover/index.html';


import StateProvider from 'ui/state_management/state';

const app = uiModules.get('apps/discover', [
'kibana/notify',
Expand All @@ -43,18 +41,25 @@ uiRoutes
template: indexTemplate,
reloadOnSearch: false,
resolve: {
ip: function (Promise, courier, config, $location) {
ip: function (Promise, courier, config, $location, Private) {
const State = Private(StateProvider);
return courier.indexPatterns.getIds()
.then(function (list) {
const stateRison = $location.search()._a;

let state;
try { state = rison.decode(stateRison); }
catch (e) { state = {}; }
/**
* In making the indexPattern modifiable it was placed in appState. Unfortunately,
* the load order of AppState conflicts with the load order of many other things
* so in order to get the name of the index we should use, and to switch to the
* default if necessary, we parse the appState with a temporary State object and
* then destroy it immediatly after we're done
*
* @type {State}
*/
const state = new State('_a', {});

const specified = !!state.index;
const exists = _.contains(list, state.index);
const id = exists ? state.index : config.get('defaultIndex');
state.destroy();

return Promise.props({
list: list,
Expand Down
30 changes: 23 additions & 7 deletions src/test_utils/__tests__/stub_browser_storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
});
});
});
93 changes: 55 additions & 38 deletions src/test_utils/stub_browser_storage.js
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a constant? IIRC there were ways to adjust this in some browsers. There also doesn't seem to be a way to use JS to get at it :(

Copy link
Contributor Author

@spalger spalger Sep 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's basically just the minimum standard size browsers use. Since this is a stub it's not really used outside of the tests, and really it's only used to enforce a limit when necessary.

}

// -----------------------------------------------------------------------------------------------
// 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;
}
}
4 changes: 2 additions & 2 deletions src/ui/public/chrome/api/__tests__/apps.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
Expand Down
20 changes: 15 additions & 5 deletions src/ui/public/chrome/api/angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import modules from 'ui/modules';
import Notifier from 'ui/notify/notifier';
import { UrlOverflowServiceProvider } from '../../error_url_overflow';

const URL_LIMIT_WARN_WITHIN = 150;
const URL_LIMIT_WARN_WITHIN = 1000;

module.exports = function (chrome, internals) {

Expand Down Expand Up @@ -57,10 +57,20 @@ module.exports = function (chrome, internals) {

try {
if (urlOverflow.check($location.absUrl()) <= URL_LIMIT_WARN_WITHIN) {
notify.warning(`
The URL has gotten big and may cause Kibana
to stop working. Please simplify the data on screen.
`);
notify.directive({
template: `
<p>
The URL has gotten big and may cause Kibana
to stop working. Please either enable the
<code>state:storeInSessionStorage</code>
option in the <a href="#/management/kibana/settings">advanced
settings</a> or simplify the onscreen visuals.
</p>
`
}, {
type: 'error',
actions: [{ text: 'close' }]
});
}
} catch (e) {
const { host, path, search, protocol } = parseUrl(window.location.href);
Expand Down
12 changes: 9 additions & 3 deletions src/ui/public/chrome/directives/kbn_chrome.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { remove } from 'lodash';
import './kbn_chrome.less';
import UiModules from 'ui/modules';
import { isSystemApiRequest } from 'ui/system_api';
import {
getUnhashableStatesProvider,
unhashUrl,
} from 'ui/state_management/state_hashing';

export default function (chrome, internals) {

Expand All @@ -28,15 +32,17 @@ export default function (chrome, internals) {
},

controllerAs: 'chrome',
controller($scope, $rootScope, $location, $http) {
controller($scope, $rootScope, $location, $http, Private) {
const getUnhashableStates = Private(getUnhashableStatesProvider);

// are we showing the embedded version of the chrome?
internals.setVisibleDefault(!$location.search().embed);

// listen for route changes, propogate to tabs
const onRouteChange = function () {
let { href } = window.location;
internals.trackPossibleSubUrl(href);
const urlWithHashes = window.location.href;
const urlWithStates = unhashUrl(urlWithHashes, getUnhashableStates());
internals.trackPossibleSubUrl(urlWithStates);
};

$rootScope.$on('$routeChangeSuccess', onRouteChange);
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/crypto/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { Sha256 } from './sha256';
Copy link
Contributor

@thomasneirynck thomasneirynck Aug 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just remove this file. IMHO, it's an extra indirection, and really doesn't add any abstraction (e.g. if it were to have a default export, and then a bunch of named ones)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we talked about this and agreed that this file is fine.

Loading