From 9cc82821465e67f5f986c8dfbcd2f8c71e6a444b Mon Sep 17 00:00:00 2001 From: Rashid Khan Date: Tue, 18 Nov 2014 16:13:51 -0700 Subject: [PATCH 1/5] Move validate_query and break out the parser and formatter --- .../components/validate_query/lib/fromUser.js | 27 +++++++++++++++ .../components/validate_query/lib/toUser.js | 17 ++++++++++ .../validate_query}/validate_query.js | 34 ++----------------- .../plugins/discover/controllers/discover.js | 2 +- test/unit/specs/directives/validate_query.js | 2 +- 5 files changed, 49 insertions(+), 33 deletions(-) create mode 100644 src/kibana/components/validate_query/lib/fromUser.js create mode 100644 src/kibana/components/validate_query/lib/toUser.js rename src/kibana/{directives => components/validate_query}/validate_query.js (77%) diff --git a/src/kibana/components/validate_query/lib/fromUser.js b/src/kibana/components/validate_query/lib/fromUser.js new file mode 100644 index 0000000000000..c508dcd7bbaae --- /dev/null +++ b/src/kibana/components/validate_query/lib/fromUser.js @@ -0,0 +1,27 @@ +define(function (require) { + var _ = require('lodash'); + + /** + * Take text from the user and make it into a query object + * @param {text} user's query input + * @returns {object} + */ + return function (text) { + // If we get an empty object, treat it as a * + if (_.isObject(text)) { + if (Object.keys(text).length) { + return text; + } else { + return {query_string: {query: '*'}}; + } + } + + // Nope, not an object. + text = (text || '').trim(); + try { + return JSON.parse(text); + } catch (e) { + return {query_string: {query: text || '*'}}; + } + }; +}); diff --git a/src/kibana/components/validate_query/lib/toUser.js b/src/kibana/components/validate_query/lib/toUser.js new file mode 100644 index 0000000000000..67c157a73d90d --- /dev/null +++ b/src/kibana/components/validate_query/lib/toUser.js @@ -0,0 +1,17 @@ +define(function (require) { + var _ = require('lodash'); + + /** + * Take text from the model and present it to the user as a string + * @param {text} model value + * @returns {string} + */ + return function (text) { + if (_.isString(text)) return text; + if (_.isObject(text)) { + if (text.query_string) return text.query_string.query; + return JSON.stringify(text); + } + return undefined; + }; +}); diff --git a/src/kibana/directives/validate_query.js b/src/kibana/components/validate_query/validate_query.js similarity index 77% rename from src/kibana/directives/validate_query.js rename to src/kibana/components/validate_query/validate_query.js index dd09c4a2352fd..44807d3a3848e 100644 --- a/src/kibana/directives/validate_query.js +++ b/src/kibana/components/validate_query/validate_query.js @@ -1,6 +1,9 @@ define(function (require) { var _ = require('lodash'); var $ = require('jquery'); + var fromUser = require('components/validate_query/lib/fromUser'); + var toUser = require('components/validate_query/lib/toUser'); + require('services/debounce'); require('modules') @@ -105,37 +108,6 @@ define(function (require) { trailing: true }); - // What should I make with the input from the user? - var fromUser = function (text) { - - // If we get an empty object, treat it as a * - if (_.isObject(text)) { - if (Object.keys(text).length) { - return text; - } else { - return {query_string: {query: '*'}}; - } - } - - // Nope, not an object. - text = (text || '').trim(); - try { - return JSON.parse(text); - } catch (e) { - return {query_string: {query: text || '*'}}; - } - }; - - // How should I present the data back to the user in the input field? - var toUser = function (text) { - if (_.isString(text)) return text; - if (_.isObject(text)) { - if (text.query_string) return text.query_string.query; - return JSON.stringify(text); - } - return undefined; - }; - ngModel.$parsers.push(fromUser); ngModel.$formatters.push(toUser); diff --git a/src/kibana/plugins/discover/controllers/discover.js b/src/kibana/plugins/discover/controllers/discover.js index 7e35d1b97abd4..36045846b16cb 100644 --- a/src/kibana/plugins/discover/controllers/discover.js +++ b/src/kibana/plugins/discover/controllers/discover.js @@ -13,7 +13,7 @@ define(function (require) { require('components/timepicker/timepicker'); require('directives/fixed_scroll'); require('directives/validate_json'); - require('directives/validate_query'); + require('components/validate_query/validate_query'); require('filters/moment'); require('components/courier/courier'); require('components/index_patterns/index_patterns'); diff --git a/test/unit/specs/directives/validate_query.js b/test/unit/specs/directives/validate_query.js index f1c15e8f22e62..416576bf5b68c 100644 --- a/test/unit/specs/directives/validate_query.js +++ b/test/unit/specs/directives/validate_query.js @@ -3,7 +3,7 @@ define(function (require) { var sinon = require('test_utils/auto_release_sinon'); // Load the kibana app dependencies. - require('directives/validate_query'); + require('components/validate_query/validate_query'); var $rootScope; var $timeout; From da0b1abe8deb6a6bb0ff63cb69cb2464413ec01c Mon Sep 17 00:00:00 2001 From: Rashid Khan Date: Tue, 18 Nov 2014 16:56:45 -0700 Subject: [PATCH 2/5] Fix validation of query strings containing only a number. Closes #1832 --- .../components/validate_query/lib/fromUser.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/kibana/components/validate_query/lib/fromUser.js b/src/kibana/components/validate_query/lib/fromUser.js index c508dcd7bbaae..b1982a35d205d 100644 --- a/src/kibana/components/validate_query/lib/fromUser.js +++ b/src/kibana/components/validate_query/lib/fromUser.js @@ -7,21 +7,29 @@ define(function (require) { * @returns {object} */ return function (text) { + var matchAll = {query_string: {query: '*'}}; + // If we get an empty object, treat it as a * if (_.isObject(text)) { if (Object.keys(text).length) { return text; } else { - return {query_string: {query: '*'}}; + return matchAll; } } // Nope, not an object. text = (text || '').trim(); - try { - return JSON.parse(text); - } catch (e) { - return {query_string: {query: text || '*'}}; + if (text.length === 0) return matchAll; + + if (text[0] === '{') { + try { + return JSON.parse(text); + } catch (e) { + return {query_string: {query: text}}; + } + } else { + return {query_string: {query: text}}; } }; }); From 0e13e876585240af19cc74e36fdb6b9baa2df8fc Mon Sep 17 00:00:00 2001 From: Rashid Khan Date: Tue, 18 Nov 2014 16:57:35 -0700 Subject: [PATCH 3/5] Coerce all non-strings & non-objects into strings --- src/kibana/components/validate_query/lib/toUser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kibana/components/validate_query/lib/toUser.js b/src/kibana/components/validate_query/lib/toUser.js index 67c157a73d90d..44d4c81ec15b9 100644 --- a/src/kibana/components/validate_query/lib/toUser.js +++ b/src/kibana/components/validate_query/lib/toUser.js @@ -12,6 +12,6 @@ define(function (require) { if (text.query_string) return text.query_string.query; return JSON.stringify(text); } - return undefined; + return text.toString(); }; }); From 19577eb10e4905f74b2c49deeaa22ece4f307d8d Mon Sep 17 00:00:00 2001 From: Rashid Khan Date: Tue, 18 Nov 2014 16:57:56 -0700 Subject: [PATCH 4/5] Tests for fromUser and toUser in query validator --- test/unit/specs/directives/validate_query.js | 43 ++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/unit/specs/directives/validate_query.js b/test/unit/specs/directives/validate_query.js index 416576bf5b68c..580f0e49b623a 100644 --- a/test/unit/specs/directives/validate_query.js +++ b/test/unit/specs/directives/validate_query.js @@ -16,6 +16,9 @@ define(function (require) { var cycleIndex = 0; var mockValidateQuery; var markup = ''; + var fromUser = require('components/validate_query/lib/fromUser'); + var toUser = require('components/validate_query/lib/toUser'); + var validEsResponse = function () { return Promise.resolve({ valid: true }); @@ -152,5 +155,45 @@ define(function (require) { checkClass('ng-valid'); }); }); + + describe('user input parser', function () { + it('should return the input if passed an object', function () { + expect(fromUser({foo: 'bar'})).to.eql({foo: 'bar'}); + }); + + it('unless the object is empty, that implies a *', function () { + expect(fromUser({})).to.eql({query_string: {query: '*'}}); + }); + + it('should treat an empty string as a *', function () { + expect(fromUser('')).to.eql({query_string: {query: '*'}}); + }); + + it('should treat input that does not start with { as a query string', function () { + expect(fromUser('foo')).to.eql({query_string: {query: 'foo'}}); + expect(fromUser('400')).to.eql({query_string: {query: '400'}}); + expect(fromUser('true')).to.eql({query_string: {query: 'true'}}); + }); + + it('should parse valid JSON', function () { + expect(fromUser('{}')).to.eql({}); + expect(fromUser('{a:b}')).to.eql({query_string: {query: '{a:b}'}}); + }); + }); + + describe('model presentation formatter', function () { + it('should present objects as strings', function () { + expect(toUser({foo: 'bar'})).to.be('{"foo":"bar"}'); + }); + + it('should present string as strings', function () { + expect(toUser('foo')).to.be('foo'); + }); + + it('should present numbers as strings', function () { + expect(toUser(400)).to.be('400'); + }); + }); + }); }); From 3964ee1bbcb0155ec8f5576eea42e29ebb561212 Mon Sep 17 00:00:00 2001 From: Rashid Khan Date: Tue, 18 Nov 2014 17:10:17 -0700 Subject: [PATCH 5/5] move from camel case to snake case --- .../validate_query/lib/{fromUser.js => from_user.js} | 0 .../components/validate_query/lib/{toUser.js => to_user.js} | 0 src/kibana/components/validate_query/validate_query.js | 4 ++-- test/unit/specs/directives/validate_query.js | 4 ++-- 4 files changed, 4 insertions(+), 4 deletions(-) rename src/kibana/components/validate_query/lib/{fromUser.js => from_user.js} (100%) rename src/kibana/components/validate_query/lib/{toUser.js => to_user.js} (100%) diff --git a/src/kibana/components/validate_query/lib/fromUser.js b/src/kibana/components/validate_query/lib/from_user.js similarity index 100% rename from src/kibana/components/validate_query/lib/fromUser.js rename to src/kibana/components/validate_query/lib/from_user.js diff --git a/src/kibana/components/validate_query/lib/toUser.js b/src/kibana/components/validate_query/lib/to_user.js similarity index 100% rename from src/kibana/components/validate_query/lib/toUser.js rename to src/kibana/components/validate_query/lib/to_user.js diff --git a/src/kibana/components/validate_query/validate_query.js b/src/kibana/components/validate_query/validate_query.js index 44807d3a3848e..ce4a329c743f6 100644 --- a/src/kibana/components/validate_query/validate_query.js +++ b/src/kibana/components/validate_query/validate_query.js @@ -1,8 +1,8 @@ define(function (require) { var _ = require('lodash'); var $ = require('jquery'); - var fromUser = require('components/validate_query/lib/fromUser'); - var toUser = require('components/validate_query/lib/toUser'); + var fromUser = require('components/validate_query/lib/from_user'); + var toUser = require('components/validate_query/lib/to_user'); require('services/debounce'); diff --git a/test/unit/specs/directives/validate_query.js b/test/unit/specs/directives/validate_query.js index 580f0e49b623a..c847b1cff5fae 100644 --- a/test/unit/specs/directives/validate_query.js +++ b/test/unit/specs/directives/validate_query.js @@ -16,8 +16,8 @@ define(function (require) { var cycleIndex = 0; var mockValidateQuery; var markup = ''; - var fromUser = require('components/validate_query/lib/fromUser'); - var toUser = require('components/validate_query/lib/toUser'); + var fromUser = require('components/validate_query/lib/from_user'); + var toUser = require('components/validate_query/lib/to_user'); var validEsResponse = function () {