Skip to content

Commit

Permalink
Handle infinite ranges in filters
Browse files Browse the repository at this point in the history
When only one side of the range is infinite, we never add a condition to
the script filter for that half of the range, so elasticsearch treats it
as infinite.

When both sides of the range are infinite, we use a match_all filter
instead of a conditional script to define the range.
  • Loading branch information
epixa committed Sep 15, 2015
1 parent 89d0b16 commit 6761e01
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 8 deletions.
51 changes: 51 additions & 0 deletions src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@

describe('ui/filter_bar/lib', function () {
describe('mapMatchAll()', function () {
const expect = require('expect.js');
const ngMock = require('ngMock');
let resolvePromises;
let mapMatchAll;
let filter;


beforeEach(ngMock.module('kibana'));
beforeEach(ngMock.inject(function (Private, $rootScope) {
resolvePromises = () => $rootScope.$apply();
mapMatchAll = Private(require('ui/filter_bar/lib/mapMatchAll'));
filter = {
match_all: {},
meta: {
field: 'foo',
formattedValue: 'bar'
}
};
}));

context('when given a filter that is not match_all', function () {
it('filter is rejected', function (done) {
delete filter.match_all;
mapMatchAll(filter).catch(result => {
expect(result).to.be(filter);
done();
});
resolvePromises();
});
});

context('when given a match_all filter', function () {
let result;
beforeEach(function () {
mapMatchAll(filter).then(r => result = r);
resolvePromises();
});

it('key is set to meta field', function () {
expect(result).to.have.property('key', filter.meta.field);
});

it('value is set to meta formattedValue', function () {
expect(result).to.have.property('value', filter.meta.formattedValue);
});
});
});
});
1 change: 1 addition & 0 deletions src/ui/public/filter_bar/lib/mapFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ define(function (require) {
// that either handles the mapping operation or not
// and add it here. ProTip: These are executed in order listed
var mappers = [
Private(require('./mapMatchAll')),
Private(require('./mapTerms')),
Private(require('./mapRange')),
Private(require('./mapExists')),
Expand Down
12 changes: 12 additions & 0 deletions src/ui/public/filter_bar/lib/mapMatchAll.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
define(function (require) {
return function mapMatchAllProvider(Promise) {
return function (filter) {
if (filter.match_all) {
const key = filter.meta.field;
const value = filter.meta.formattedValue || 'all';
return Promise.resolve({ key, value });
}
return Promise.reject(filter);
};
};
});
48 changes: 48 additions & 0 deletions src/ui/public/filter_manager/__tests__/lib/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,53 @@ describe('Filter Manager', function () {

});
});

context('when given params where one side is infinite', function () {
let filter;
beforeEach(function () {
filter = fn(indexPattern.fields.byName['script number'], { gte: 0, lt: Infinity }, indexPattern);
});

describe('returned filter', function () {
it('is a script filter', function () {
expect(filter).to.have.property('script');
});

it('contain a param for the finite side', function () {
expect(filter.script.params).to.have.property('gte', 0);
});

it('does not contain a param for the infinite side', function () {
expect(filter.script.params).not.to.have.property('lt');
});

it('does not contain a script condition for the infinite side', function () {
const script = indexPattern.fields.byName['script number'].script;
expect(filter.script.script).to.equal(`(${script})>=gte`);
});
});
});

context('when given params where both sides are infinite', function () {
let filter;
beforeEach(function () {
filter = fn(indexPattern.fields.byName['script number'], { gte: -Infinity, lt: Infinity }, indexPattern);
});

describe('returned filter', function () {
it('is a match_all filter', function () {
expect(filter).not.to.have.property('script');
expect(filter).to.have.property('match_all');
});

it('does not contain params', function () {
expect(filter).not.to.have.property('params');
});

it('meta field is set to field name', function () {
expect(filter.meta.field).to.equal('script number');
});
});
});
});
});
34 changes: 26 additions & 8 deletions src/ui/public/filter_manager/lib/range.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,44 @@
define(function (require) {
var _ = require('lodash');
const _ = require('lodash');
const OPERANDS_IN_RANGE = 2;

return function buildRangeFilter(field, params, indexPattern, formattedValue) {
var filter = { meta: { index: indexPattern.id } };
const filter = { meta: { index: indexPattern.id } };
if (formattedValue) filter.meta.formattedValue = formattedValue;

params = _.clone(params);

if (params.gte && params.gt) throw new Error('gte and gt are mutually exclusive');
if (params.lte && params.lt) throw new Error('lte and lt are mutually exclusive');
if ('gte' in params && 'gt' in params) throw new Error('gte and gt are mutually exclusive');
if ('lte' in params && 'lt' in params) throw new Error('lte and lt are mutually exclusive');

const totalInfinite = ['gt', 'lt'].reduce((totalInfinite, op) => {
const key = op in params ? op : `${op}e`;
const isInfinite = Math.abs(params[key]) === Infinity;

This comment has been minimized.

Copy link
@spalger

spalger Sep 15, 2015

Is there a reason that isFinite() wouldn't work here?

This comment has been minimized.

Copy link
@epixa

epixa Sep 15, 2015

Author Owner

I don't think so, no. I just had no idea that it existed! I'll update it.

This comment has been minimized.

Copy link
@epixa

epixa Sep 15, 2015

Author Owner

Actually, isFinite() wouldn't have the same effect. For null or undefined, isFinite()returns false, which will incorrectly be evaluated as infinite if I were to just negate the result in the conditional. Math.abs() handles null or undefined by returning NaN, which will never be equivalent to Infinity, which is the behavior we want.


if (isInfinite) {
totalInfinite++;
delete params[key];
}

if (field.scripted) {
var operators = {
return totalInfinite;
}, 0);

if (totalInfinite === OPERANDS_IN_RANGE) {
filter.match_all = {};
filter.meta.field = field.name;
} else if (field.scripted) {
const operators = {
gt: '>',
gte: '>=',
lte: '<=',
lt: '<',
};

var script = _.map(params, function (val, key) {
const script = _.map(params, function (val, key) {
return '(' + field.script + ')' + operators[key] + key;
}).join(' && ');

var value = _.map(params, function (val, key) {
const value = _.map(params, function (val, key) {
return operators[key] + field.format.convert(val);
}).join(' ');

Expand All @@ -32,6 +49,7 @@ define(function (require) {
filter.range = {};
filter.range[field.name] = params;
}

return filter;
};
});

0 comments on commit 6761e01

Please sign in to comment.