Another approach to managing persisted states. #25

cjcenizal · 2016-08-28T03:52:30Z

This approach is distinct from the original in a few ways:

The logic for creating hashes in encapsulated in its own module.
The logic for storing items by their hash and for freeing up storage space is combined in one module, essentially combining HashingStore and LazyLruStore.
The resulting HashedItemStore is a singleton.
The HashedItemStore implementation ignores any browser history limit, and instead focuses solely on freeing up storage space when it's needed.
It also has no knowledge of the different "types" of states being stored, i.e. whether the states belong to _a or _g query params.
It also uses an in-memory index to keep track of which items are persisted in browser storage, instead of deriving this information JIT, a la _indexStoredItems.

spalger · 2016-08-29T04:47:10Z

src/ui/public/state_management/state_storage/hashed_item_store.js

+ *
+ * Consideration 2: We can't tell when we've hit the browser history limit
+ *
+ * Because some of our persisted history states may no longer be referenced by the browser history,


spalger · 2016-08-29T04:48:16Z

This is awesome, a very necessary simplification. I assume you're working on the tests?

cjcenizal · 2016-08-29T15:27:09Z

Thanks Spencer! I was going to move onto tests after passing this by you and getting your thoughts on this approach (pros/cons/assumptions).

spalger · 2016-08-30T00:22:32Z

src/ui/public/state_management/state_storage/hashed_item_store_singleton.js

@@ -0,0 +1,9 @@
+import HashedItemStore from './hashed_item_store';
+
+const instance = new HashedItemStore();


This is probably overkill. Throughout the rest of kibana modules just export the "singleton" instance

spalger · 2016-08-30T01:22:57Z

src/ui/public/state_management/state_storage/_tests__/hashed_item_store.js

+          return itemSize * itemCount;
+        }
+
+        function calculateIndexSize(itemCount) {


Maybe it would be a good idea to create a StubBrowserStorage#_getSize() method, then you could get the size by filling up a temporary instance of StubBrowserStorage and calling it's _getSize() method.

I think this is a great idea. Do you mind if we remove the underscore prefixes from the non-standard API methods? I'm so used to seeing them used to indicate "private" methods that it feels weird to consume them externally.

spalger · 2016-08-30T21:36:24Z

src/ui/public/state_management/state.js

    }
+
+    // If we ran out of space trying to persist the state, notify the user.
+    this._notifier.log('Unable to create hash of State due to error: ' + (state.stack || state.message));


Should probably just drop this line now that an error isn't being thrown.

- Replace with HashedItemStore, createStateHash, and isStateHash. - Refactor stubBrowserStorage.

spalger · 2016-08-31T19:25:34Z

LGTM

Minor typo fixes

) * add preview for creating scripted fields * Move previewed results into flyout. (#25) * Move previewed results into flyout. Execute script as soon as the tab is selected. * Format error in a danger callout. * add functional test for preview * move executeScript functionallity into lib so it can be used to validate on save * validate script on save * add functional test for script validation during save * fix jest tests * fix jest tests * only validate script when field is scripted * only add script callouts and help flyout to DOM for scripted fields * move scripting stuff to its own render method * right justify delete button to have same style as delete button for user management * use data-test-subj key to find invalid script error msg in functional tests * move help flyout link out of formRow help text and into its own row

* 💄 Hack to fix suggestion box * 🐛 Fix validation messages * 🐛 Relax operations check for managedReferences * Change completion params * 🏷️ Fix missing arg issue * ✨ Add more tinymath fns * 🐛 Improved validation around math operations + multiple named arguments * 🐛 Use new onError feature in math expression * ♻️ Refactor namedArguments validation * 🐛 Fix circular dependency issue in tests + minor fixes * Move formula into a tab * 🔥 Leftovers from previous merge * ✨ Move over namedArgs from previous function * ✅ Add tests for transferable scenarios * ✅ Fixed broken test * ✨ Use custom label for axis * Allow switching back and forth to formula tab * Add a section for the function reference * Add modal editor and markdown docs * Change the way math nodes are validated * Use custom portal to fix monaco positioning * Fix model sharing issues * Provide signature help * 🐛 Fix small test issue * 🐛 Mark pow arguments as required * 🐛 validate on first render only if a formula is present * 🔥 Remove log10 fn for now * ✨ Improved math validation + add tests for math functions * Fix mount/unmount issues with Monaco * [Lens] Fully unmount React when flyout closes * Fix bug with editor frame unmounting * Fix type * Add tests for monaco providers, add hover provider * Add test for last_value * Usability improvements * Add KQL and Lucene named parameters * Add kql, lucene completion and validation * Fix autocomplete on weird characters and properly connect KQL * Highlight functions that have additional requirements after validating * Fix type error and move help text to popover * Fix escape characters inside KQL * 🐛 Fix dataType issue when moving over to Formula * Automatically insert single quotes on every named param * Only insert single quotes when typing kql= or lucene= * Reorganize help popover * Fix merge issues * Update grammar for formulas * Fix bad merge * Rough fullscreen mode * Type updates * Pass through fullscreen state * Remove more chrome from full screen mode * Fix minor bugs in formula typing * 🐛 Decouple column order of references and output * 🔧 Fix tests and types * ✅ Add first functional test * Fix copying formulas and empty formula * Trigger suggestion prompt when hitting enter on function or typing kql= * 🐛 Prevent flyout from closing while interacting with monaco * refactoring * move main column generation into parse module * fix tests * refactor small formula styles and markup * documentation * adjustments in formula footer * Formula refactoring (#12) * refactoring * move main column generation into parse module * fix tests * more style and markup tweak for custom formula * Fix tests * [Expressions] Use table column ID instead of name when set * [Lens] Create managedReference type for formulas * Fix test failures * Fix i18n types * fix fullscreen flex issues * Delete managedReference when replacing * refactor css and markup; add button placeholders * [Lens] Formulas * Tests for formula Co-authored-by: Marco Liberati <marco.liberati@elastic.co> * added error count placeholder * Add tooltips * Refactoring from code review * Fix some editor issues * Update ID matching to match by name sometimes * Improve performance of Monaco, fix formulas with 0, update labels * Improve performance of full screen toggle * Fix formula tests * fix stuff * Add an extra case to prevent insertion of duplicate column * Simplify logic and add test for output ID * add telemetry for Lens formula (#15) * Respond to review comments * ✨ Improve the signatures with better documentation and examples * adjust border styles to account for docs collapse * refactor docs markup; restructure docs obj; styles * Fix formula auto reordering (#18) * fix formula auto reordering * add unit test * Fix and improve suggestion experience in Formula (#19) * ✨ Revisit documentation and suggestions * 👌 Integrated feedback * ✨ Add query validation for quotes * Usability updates & type fixes * add search to formula * fix form styles to match designs * fix text styles; revert to Markdown for control * 👌 Integrated more feedback * improve search * improve suggestions * improve suggestions even more * 🐛 Fix i18n issues (#22) * Persist formula on leave, fix fullscreen and popovers * Fix documentation tests * 🏷️ fix type issue * 🐛 Remove hidden operations from valid functions list * 🐛 Fix empty string query edge case * 🐛 Enable more suggestions + extends validation * Fix tests that depended on setState being called without function * Error state and text wrapping updates * ✨ Add new module to CodeEditor for brackets matching (#25) * Fix type * show warning * keep current quick function * ✨ Improve suggestions within kql query * 📷 Fix snapshot editor test * 🐛 Improved suggestion for single quote and refactored debounce * Fix lodash usage * Fix tests * Revert "keep current quick function" This reverts commit ed47705. * Improve performance of dispatch by using timeout * Improve memoization of datapanel * Fix escape characters * fix reduced suggestions * fix responsiveness * fix unit test * Fix autocomplete on nested math * Show errors and warnings on first render * fix transposing column crash * Update comment * 🐛 Fix field error message * fix test types * 📝 Fix i18n name * 💄 Manage wordwrap via react component * Fix selector for palettes that interferes with quick functions * Use word wrapping by default * Errors for managed references are handled at the top level * 🐛 Move the cursor just next to new inserted text * ⚗️ First pass for performance * 🐛 Fix unwanted change * ⚡ Memoize as many combobox props as possible * ⚡ More memoization * Show errors in hover * Use temporary invalid state when moving away from formula * Remove setActiveDimension and shouldClose, fixed by async setters * Fix test dependency * do not show quick functions tab * increase documentation popover width * fix functional test * Call setActiveDimension when updating visualization * Simplify handling of flyout with incomplete columns * Fix test issues * add description to formula telemetry * fix schema * Update from design feedback * More review comments * Hide callout border from v7 theme Co-authored-by: dej611 <dej611@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Joe Reuter <johannes.reuter@elastic.co> Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co> Co-authored-by: Joe Reuter <email@johannes-reuter.de> Co-authored-by: Marco Liberati <marco.liberati@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com> Co-authored-by: Wylie Conlon <william.conlon@elastic.co> Co-authored-by: dej611 <dej611@gmail.com> Co-authored-by: Joe Reuter <johannes.reuter@elastic.co> Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co> Co-authored-by: Joe Reuter <email@johannes-reuter.de> Co-authored-by: Marco Liberati <marco.liberati@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

* 💄 Hack to fix suggestion box * 🐛 Fix validation messages * 🐛 Relax operations check for managedReferences * Change completion params * 🏷️ Fix missing arg issue * ✨ Add more tinymath fns * 🐛 Improved validation around math operations + multiple named arguments * 🐛 Use new onError feature in math expression * ♻️ Refactor namedArguments validation * 🐛 Fix circular dependency issue in tests + minor fixes * Move formula into a tab * 🔥 Leftovers from previous merge * ✨ Move over namedArgs from previous function * ✅ Add tests for transferable scenarios * ✅ Fixed broken test * ✨ Use custom label for axis * Allow switching back and forth to formula tab * Add a section for the function reference * Add modal editor and markdown docs * Change the way math nodes are validated * Use custom portal to fix monaco positioning * Fix model sharing issues * Provide signature help * 🐛 Fix small test issue * 🐛 Mark pow arguments as required * 🐛 validate on first render only if a formula is present * 🔥 Remove log10 fn for now * ✨ Improved math validation + add tests for math functions * Fix mount/unmount issues with Monaco * [Lens] Fully unmount React when flyout closes * Fix bug with editor frame unmounting * Fix type * Add tests for monaco providers, add hover provider * Add test for last_value * Usability improvements * Add KQL and Lucene named parameters * Add kql, lucene completion and validation * Fix autocomplete on weird characters and properly connect KQL * Highlight functions that have additional requirements after validating * Fix type error and move help text to popover * Fix escape characters inside KQL * 🐛 Fix dataType issue when moving over to Formula * Automatically insert single quotes on every named param * Only insert single quotes when typing kql= or lucene= * Reorganize help popover * Fix merge issues * Update grammar for formulas * Fix bad merge * Rough fullscreen mode * Type updates * Pass through fullscreen state * Remove more chrome from full screen mode * Fix minor bugs in formula typing * 🐛 Decouple column order of references and output * 🔧 Fix tests and types * ✅ Add first functional test * Fix copying formulas and empty formula * Trigger suggestion prompt when hitting enter on function or typing kql= * 🐛 Prevent flyout from closing while interacting with monaco * refactoring * move main column generation into parse module * fix tests * refactor small formula styles and markup * documentation * adjustments in formula footer * Formula refactoring (#12) * refactoring * move main column generation into parse module * fix tests * more style and markup tweak for custom formula * Fix tests * [Expressions] Use table column ID instead of name when set * [Lens] Create managedReference type for formulas * Fix test failures * Fix i18n types * fix fullscreen flex issues * Delete managedReference when replacing * refactor css and markup; add button placeholders * [Lens] Formulas * Tests for formula Co-authored-by: Marco Liberati <marco.liberati@elastic.co> * added error count placeholder * Add tooltips * Refactoring from code review * Fix some editor issues * Update ID matching to match by name sometimes * Improve performance of Monaco, fix formulas with 0, update labels * Improve performance of full screen toggle * Fix formula tests * fix stuff * Add an extra case to prevent insertion of duplicate column * Simplify logic and add test for output ID * add telemetry for Lens formula (#15) * Respond to review comments * ✨ Improve the signatures with better documentation and examples * adjust border styles to account for docs collapse * refactor docs markup; restructure docs obj; styles * Fix formula auto reordering (#18) * fix formula auto reordering * add unit test * Fix and improve suggestion experience in Formula (#19) * ✨ Revisit documentation and suggestions * 👌 Integrated feedback * ✨ Add query validation for quotes * Usability updates & type fixes * add search to formula * fix form styles to match designs * fix text styles; revert to Markdown for control * 👌 Integrated more feedback * improve search * improve suggestions * improve suggestions even more * 🐛 Fix i18n issues (#22) * Persist formula on leave, fix fullscreen and popovers * Fix documentation tests * 🏷️ fix type issue * 🐛 Remove hidden operations from valid functions list * 🐛 Fix empty string query edge case * 🐛 Enable more suggestions + extends validation * Fix tests that depended on setState being called without function * Error state and text wrapping updates * ✨ Add new module to CodeEditor for brackets matching (#25) * Fix type * show warning * keep current quick function * ✨ Improve suggestions within kql query * 📷 Fix snapshot editor test * 🐛 Improved suggestion for single quote and refactored debounce * Fix lodash usage * Fix tests * Revert "keep current quick function" This reverts commit ed47705. * Improve performance of dispatch by using timeout * Improve memoization of datapanel * Fix escape characters * fix reduced suggestions * fix responsiveness * fix unit test * Fix autocomplete on nested math * Show errors and warnings on first render * fix transposing column crash * Update comment * 🐛 Fix field error message * fix test types * 📝 Fix i18n name * 💄 Manage wordwrap via react component * Fix selector for palettes that interferes with quick functions * Use word wrapping by default * Errors for managed references are handled at the top level * 🐛 Move the cursor just next to new inserted text * ⚗️ First pass for performance * 🐛 Fix unwanted change * ⚡ Memoize as many combobox props as possible * ⚡ More memoization * Show errors in hover * Use temporary invalid state when moving away from formula * Remove setActiveDimension and shouldClose, fixed by async setters * Fix test dependency * do not show quick functions tab * increase documentation popover width * fix functional test * Call setActiveDimension when updating visualization * Simplify handling of flyout with incomplete columns * Fix test issues * add description to formula telemetry * fix schema * Update from design feedback * More review comments * Hide callout border from v7 theme Co-authored-by: dej611 <dej611@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Joe Reuter <johannes.reuter@elastic.co> Co-authored-by: Michael Marcialis <michael.marcialis@elastic.co> Co-authored-by: Joe Reuter <email@johannes-reuter.de> Co-authored-by: Marco Liberati <marco.liberati@elastic.co> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

cjcenizal force-pushed the implement/storeStateInLocalstorage/simplify-lazy-lru-store branch 12 times, most recently from af95215 to 13b17d1 Compare August 28, 2016 16:36

spalger reviewed Aug 29, 2016
View reviewed changes

cjcenizal force-pushed the implement/storeStateInLocalstorage/simplify-lazy-lru-store branch from 13b17d1 to 40d1128 Compare August 29, 2016 16:50

cjcenizal changed the title ~~[WIP] Another approach to managing persisted states.~~ Another approach to managing persisted states. Aug 29, 2016

cjcenizal force-pushed the implement/storeStateInLocalstorage/simplify-lazy-lru-store branch from 40d1128 to a801b83 Compare August 29, 2016 23:49

spalger reviewed Aug 30, 2016
View reviewed changes

cjcenizal force-pushed the implement/storeStateInLocalstorage/simplify-lazy-lru-store branch from a801b83 to c273a8d Compare August 30, 2016 21:26

spalger reviewed Aug 30, 2016
View reviewed changes

cjcenizal force-pushed the implement/storeStateInLocalstorage/simplify-lazy-lru-store branch from c273a8d to 79f6cfe Compare August 31, 2016 17:44

Remove LazyLruStore, HashingStore, and createStorageHash.

5b4f208

- Replace with HashedItemStore, createStateHash, and isStateHash. - Refactor stubBrowserStorage.

cjcenizal force-pushed the implement/storeStateInLocalstorage/simplify-lazy-lru-store branch from 79f6cfe to 5b4f208 Compare August 31, 2016 19:25

spalger merged commit daf9eba into spalger:implement/storeStateInLocalstorage Aug 31, 2016

cjcenizal deleted the implement/storeStateInLocalstorage/simplify-lazy-lru-store branch August 31, 2016 19:25

spalger pushed a commit that referenced this pull request Sep 6, 2016

Merge pull request #25 from ycombinator/doc

da76f58

Minor typo fixes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Another approach to managing persisted states. #25

Another approach to managing persisted states. #25

cjcenizal commented Aug 28, 2016 •

edited

Loading

spalger Aug 29, 2016

spalger commented Aug 29, 2016

cjcenizal commented Aug 29, 2016

spalger Aug 30, 2016 •

edited

Loading

spalger Aug 30, 2016

cjcenizal Aug 30, 2016

spalger Aug 30, 2016

spalger commented Aug 31, 2016

		@@ -0,0 +1,9 @@
		import HashedItemStore from './hashed_item_store';

		const instance = new HashedItemStore();

Another approach to managing persisted states. #25

Another approach to managing persisted states. #25

Conversation

cjcenizal commented Aug 28, 2016 • edited Loading

spalger Aug 29, 2016

Choose a reason for hiding this comment

spalger commented Aug 29, 2016

cjcenizal commented Aug 29, 2016

spalger Aug 30, 2016 • edited Loading

Choose a reason for hiding this comment

spalger Aug 30, 2016

Choose a reason for hiding this comment

cjcenizal Aug 30, 2016

Choose a reason for hiding this comment

spalger Aug 30, 2016

Choose a reason for hiding this comment

spalger commented Aug 31, 2016

cjcenizal commented Aug 28, 2016 •

edited

Loading

spalger Aug 30, 2016 •

edited

Loading