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

Fix query validation of numbers. Closes #1832 #1969

Merged
merged 5 commits into from
Nov 19, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/kibana/components/validate_query/lib/from_user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
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) {
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 matchAll;
}
}

// Nope, not an object.
text = (text || '').trim();
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}};
}
};
});
17 changes: 17 additions & 0 deletions src/kibana/components/validate_query/lib/to_user.js
Original file line number Diff line number Diff line change
@@ -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 text.toString();
};
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
define(function (require) {
var _ = require('lodash');
var $ = require('jquery');
var fromUser = require('components/validate_query/lib/from_user');
var toUser = require('components/validate_query/lib/to_user');

require('services/debounce');

require('modules')
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/kibana/plugins/discover/controllers/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should move this require statement into the kibana controller so that it's always loaded, not just loaded when Discover is.

require('filters/moment');
require('components/courier/courier');
require('components/index_patterns/index_patterns');
Expand Down
45 changes: 44 additions & 1 deletion test/unit/specs/directives/validate_query.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -16,6 +16,9 @@ define(function (require) {
var cycleIndex = 0;
var mockValidateQuery;
var markup = '<input ng-model="mockModel" validate-query="mockQueryInput" input-focus type="text">';
var fromUser = require('components/validate_query/lib/from_user');
var toUser = require('components/validate_query/lib/to_user');


var validEsResponse = function () {
return Promise.resolve({ valid: true });
Expand Down Expand Up @@ -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');
});
});

});
});