From 2e37f0d62f89bf30dc1bc214e59facb03a9ca5dd Mon Sep 17 00:00:00 2001 From: Robert Monfera Date: Wed, 3 Apr 2019 00:57:03 +0200 Subject: [PATCH] [Canvas] Feat: make an empty pipeline expression legal (#28796) --- .../common/lib/ast.from_expression.test.js | 14 ++++++++----- .../kbn-interpreter/src/common/lib/grammar.js | 5 ++++- .../src/common/lib/grammar.peg | 7 +++++-- .../common/interpreter/interpret.js | 2 +- .../renderers/time_filter/index.js | 20 ++++++++++--------- .../canvas/public/components/debug/debug.js | 2 +- .../render_with_fn/render_with_fn.js | 2 +- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/packages/kbn-interpreter/src/common/lib/ast.from_expression.test.js b/packages/kbn-interpreter/src/common/lib/ast.from_expression.test.js index c144770f94c5..dc8c479b8ecc 100644 --- a/packages/kbn-interpreter/src/common/lib/ast.from_expression.test.js +++ b/packages/kbn-interpreter/src/common/lib/ast.from_expression.test.js @@ -22,17 +22,21 @@ import { getType } from './get_type'; describe('ast fromExpression', () => { describe('invalid expression', () => { - it('throws when empty', () => { - const check = () => fromExpression(''); - expect(check).toThrowError(/Unable to parse expression/i); - }); - it('throws with invalid expression', () => { const check = () => fromExpression('wat!'); expect(check).toThrowError(/Unable to parse expression/i); }); }); + describe('zero-item expression', () => { + it('yields a zero-length chain when empty', () => { + const expression = ''; + const astObject = fromExpression(expression); + expect(astObject).toHaveProperty('chain'); + expect(astObject.chain).toEqual([]); + }); + }); + describe('single item expression', () => { it('is a chain', () => { const expression = 'whatever'; diff --git a/packages/kbn-interpreter/src/common/lib/grammar.js b/packages/kbn-interpreter/src/common/lib/grammar.js index 2e87b52d68f2..3f473b1beea6 100644 --- a/packages/kbn-interpreter/src/common/lib/grammar.js +++ b/packages/kbn-interpreter/src/common/lib/grammar.js @@ -147,7 +147,7 @@ function peg$parse(input, options) { peg$c3 = function(first, rest) { return addMeta({ type: 'expression', - chain: [first].concat(rest) + chain: first ? [first].concat(rest) : [] }, text(), location()); }, peg$c4 = peg$otherExpectation("function"), @@ -364,6 +364,9 @@ function peg$parse(input, options) { } if (s1 !== peg$FAILED) { s2 = peg$parsefunction(); + if (s2 === peg$FAILED) { + s2 = null; + } if (s2 !== peg$FAILED) { s3 = []; s4 = peg$currPos; diff --git a/packages/kbn-interpreter/src/common/lib/grammar.peg b/packages/kbn-interpreter/src/common/lib/grammar.peg index fea9564b67a2..e8a2988d2b4f 100644 --- a/packages/kbn-interpreter/src/common/lib/grammar.peg +++ b/packages/kbn-interpreter/src/common/lib/grammar.peg @@ -5,6 +5,9 @@ * Yes, technically you can load this and build the parser in real time, but this makes it annoying * to share the grammar between the front end and back, so, you know, just generate the parser. * You shouldn't be futzing around in the grammar very often anyway. +* +* Instructions for generating `grammar.json`: https://github.com/elastic/kibana/issues/28776#issue-399489673 +* */ { @@ -20,10 +23,10 @@ start = expression expression - = space? first:function rest:('|' space? fn:function { return fn; })* { + = space? first:function? rest:('|' space? fn:function { return fn; })* { return addMeta({ type: 'expression', - chain: [first].concat(rest) + chain: first ? [first].concat(rest) : [] }, text(), location()); } diff --git a/src/legacy/core_plugins/interpreter/common/interpreter/interpret.js b/src/legacy/core_plugins/interpreter/common/interpreter/interpret.js index 0fedbe273bb3..68e1d251c950 100644 --- a/src/legacy/core_plugins/interpreter/common/interpreter/interpret.js +++ b/src/legacy/core_plugins/interpreter/common/interpreter/interpret.js @@ -145,7 +145,7 @@ export function interpreterProvider(config) { const argAstsWithDefaults = reduce( argDefs, (argAsts, argDef, argName) => { - if (typeof argAsts[argName] === 'undefined' && typeof argDef.default !== 'undefined') { + if (typeof argAsts[argName] === 'undefined' && typeof argDef.default !== 'undefined') { argAsts[argName] = [fromExpression(argDef.default, 'argument')]; } diff --git a/x-pack/plugins/canvas/canvas_plugin_src/renderers/time_filter/index.js b/x-pack/plugins/canvas/canvas_plugin_src/renderers/time_filter/index.js index 40ab7bb7c85a..74e62ed4e246 100644 --- a/x-pack/plugins/canvas/canvas_plugin_src/renderers/time_filter/index.js +++ b/x-pack/plugins/canvas/canvas_plugin_src/renderers/time_filter/index.js @@ -16,16 +16,18 @@ export const timeFilter = () => ({ help: 'Set a time window', reuseDomNode: true, // must be true, otherwise filters get reset when re-rendered render(domNode, config, handlers) { - const ast = fromExpression(handlers.getFilter()); - - // Check if the current column is what we expect it to be. If the user changes column this will be called again, - // but we don't want to run setFilter() unless we have to because it will cause a data refresh - const column = get(ast, 'chain[0].arguments.column[0]'); - if (column !== config.column) { - set(ast, 'chain[0].arguments.column[0]', config.column); - handlers.setFilter(toExpression(ast)); + const filterExpression = handlers.getFilter(); + const filterExists = filterExpression !== ''; + const ast = fromExpression(filterExpression); + if (filterExists) { + // Check if the current column is what we expect it to be. If the user changes column this will be called again, + // but we don't want to run setFilter() unless we have to because it will cause a data refresh + const column = get(ast, 'chain[0].arguments.column[0]'); + if (column !== config.column) { + set(ast, 'chain[0].arguments.column[0]', config.column); + handlers.setFilter(toExpression(ast)); + } } - ReactDOM.render( ( ); Debug.propTypes = { - payload: PropTypes.object.isRequired, + payload: PropTypes.object, }; diff --git a/x-pack/plugins/canvas/public/components/render_with_fn/render_with_fn.js b/x-pack/plugins/canvas/public/components/render_with_fn/render_with_fn.js index 3896e604362f..41e8461137f6 100644 --- a/x-pack/plugins/canvas/public/components/render_with_fn/render_with_fn.js +++ b/x-pack/plugins/canvas/public/components/render_with_fn/render_with_fn.js @@ -25,7 +25,7 @@ export class RenderWithFn extends React.Component { destroy: PropTypes.func.isRequired, onDestroy: PropTypes.func.isRequired, }), - config: PropTypes.object.isRequired, + config: PropTypes.object, size: PropTypes.object.isRequired, onError: PropTypes.func.isRequired, };