From 6761e0105341cd0ab63742b5fd1d6423433d2ee6 Mon Sep 17 00:00:00 2001 From: Court Ewing Date: Tue, 15 Sep 2015 11:34:43 -0400 Subject: [PATCH 1/5] Handle infinite ranges in filters 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. --- .../filter_bar/lib/__tests__/mapMatchAll.js | 51 +++++++++++++++++++ src/ui/public/filter_bar/lib/mapFilter.js | 1 + src/ui/public/filter_bar/lib/mapMatchAll.js | 12 +++++ .../filter_manager/__tests__/lib/range.js | 48 +++++++++++++++++ src/ui/public/filter_manager/lib/range.js | 34 ++++++++++--- 5 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js create mode 100644 src/ui/public/filter_bar/lib/mapMatchAll.js diff --git a/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js b/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js new file mode 100644 index 0000000000000..b0c851b6774b0 --- /dev/null +++ b/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js @@ -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); + }); + }); + }); +}); diff --git a/src/ui/public/filter_bar/lib/mapFilter.js b/src/ui/public/filter_bar/lib/mapFilter.js index 8bf2a2d5cf430..ebf6bb31ae115 100644 --- a/src/ui/public/filter_bar/lib/mapFilter.js +++ b/src/ui/public/filter_bar/lib/mapFilter.js @@ -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')), diff --git a/src/ui/public/filter_bar/lib/mapMatchAll.js b/src/ui/public/filter_bar/lib/mapMatchAll.js new file mode 100644 index 0000000000000..c062fa69e0dc3 --- /dev/null +++ b/src/ui/public/filter_bar/lib/mapMatchAll.js @@ -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); + }; + }; +}); diff --git a/src/ui/public/filter_manager/__tests__/lib/range.js b/src/ui/public/filter_manager/__tests__/lib/range.js index 7de6608c22bb7..fb3432a6cab21 100644 --- a/src/ui/public/filter_manager/__tests__/lib/range.js +++ b/src/ui/public/filter_manager/__tests__/lib/range.js @@ -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'); + }); + }); + }); }); }); diff --git a/src/ui/public/filter_manager/lib/range.js b/src/ui/public/filter_manager/lib/range.js index 759518041e64a..4d374a51d3208 100644 --- a/src/ui/public/filter_manager/lib/range.js +++ b/src/ui/public/filter_manager/lib/range.js @@ -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; + + 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(' '); @@ -32,6 +49,7 @@ define(function (require) { filter.range = {}; filter.range[field.name] = params; } + return filter; }; }); From 7e13333353f99414a18ecaebea7c29ad029dbbda Mon Sep 17 00:00:00 2001 From: Court Ewing Date: Tue, 15 Sep 2015 14:31:29 -0400 Subject: [PATCH 2/5] Move filter_manager/lib tests into correct directory Test directories should live alongside the files they are testing. --- .../filter_manager/{__tests__/lib => lib/__tests__}/phrase.js | 0 .../filter_manager/{__tests__/lib => lib/__tests__}/query.js | 0 .../filter_manager/{__tests__/lib => lib/__tests__}/range.js | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename src/ui/public/filter_manager/{__tests__/lib => lib/__tests__}/phrase.js (100%) rename src/ui/public/filter_manager/{__tests__/lib => lib/__tests__}/query.js (100%) rename src/ui/public/filter_manager/{__tests__/lib => lib/__tests__}/range.js (100%) diff --git a/src/ui/public/filter_manager/__tests__/lib/phrase.js b/src/ui/public/filter_manager/lib/__tests__/phrase.js similarity index 100% rename from src/ui/public/filter_manager/__tests__/lib/phrase.js rename to src/ui/public/filter_manager/lib/__tests__/phrase.js diff --git a/src/ui/public/filter_manager/__tests__/lib/query.js b/src/ui/public/filter_manager/lib/__tests__/query.js similarity index 100% rename from src/ui/public/filter_manager/__tests__/lib/query.js rename to src/ui/public/filter_manager/lib/__tests__/query.js diff --git a/src/ui/public/filter_manager/__tests__/lib/range.js b/src/ui/public/filter_manager/lib/__tests__/range.js similarity index 100% rename from src/ui/public/filter_manager/__tests__/lib/range.js rename to src/ui/public/filter_manager/lib/__tests__/range.js From a53e0868555aeef3c0828ce25aaf0b3235331922 Mon Sep 17 00:00:00 2001 From: Court Ewing Date: Tue, 15 Sep 2015 14:39:17 -0400 Subject: [PATCH 3/5] Move filter_bar/lib tests into correct directory Test directories should live alongside the files they are testing. --- src/ui/public/filter_bar/{ => lib}/__tests__/changeTimeFilter.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/dedupFilters.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/extractTimeFilter.js | 0 .../filter_bar/{ => lib}/__tests__/filterAppliedAndUnwrap.js | 0 .../filter_bar/{ => lib}/__tests__/filterOutTimeBasedFilter.js | 0 .../public/filter_bar/{ => lib}/__tests__/generateMappingChain.js | 0 .../public/filter_bar/{ => lib}/__tests__/mapAndFlattenFilters.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapDefault.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapExists.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapFilter.js | 0 .../filter_bar/{ => lib}/__tests__/mapFlattenAndWrapFilters.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapGeoBoundingBox.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapMissing.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapQueryString.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapRange.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapScript.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/mapTerms.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/onlyDisabled.js | 0 src/ui/public/filter_bar/{ => lib}/__tests__/uniqFilters.js | 0 19 files changed, 0 insertions(+), 0 deletions(-) rename src/ui/public/filter_bar/{ => lib}/__tests__/changeTimeFilter.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/dedupFilters.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/extractTimeFilter.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/filterAppliedAndUnwrap.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/filterOutTimeBasedFilter.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/generateMappingChain.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapAndFlattenFilters.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapDefault.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapExists.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapFilter.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapFlattenAndWrapFilters.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapGeoBoundingBox.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapMissing.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapQueryString.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapRange.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapScript.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/mapTerms.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/onlyDisabled.js (100%) rename src/ui/public/filter_bar/{ => lib}/__tests__/uniqFilters.js (100%) diff --git a/src/ui/public/filter_bar/__tests__/changeTimeFilter.js b/src/ui/public/filter_bar/lib/__tests__/changeTimeFilter.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/changeTimeFilter.js rename to src/ui/public/filter_bar/lib/__tests__/changeTimeFilter.js diff --git a/src/ui/public/filter_bar/__tests__/dedupFilters.js b/src/ui/public/filter_bar/lib/__tests__/dedupFilters.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/dedupFilters.js rename to src/ui/public/filter_bar/lib/__tests__/dedupFilters.js diff --git a/src/ui/public/filter_bar/__tests__/extractTimeFilter.js b/src/ui/public/filter_bar/lib/__tests__/extractTimeFilter.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/extractTimeFilter.js rename to src/ui/public/filter_bar/lib/__tests__/extractTimeFilter.js diff --git a/src/ui/public/filter_bar/__tests__/filterAppliedAndUnwrap.js b/src/ui/public/filter_bar/lib/__tests__/filterAppliedAndUnwrap.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/filterAppliedAndUnwrap.js rename to src/ui/public/filter_bar/lib/__tests__/filterAppliedAndUnwrap.js diff --git a/src/ui/public/filter_bar/__tests__/filterOutTimeBasedFilter.js b/src/ui/public/filter_bar/lib/__tests__/filterOutTimeBasedFilter.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/filterOutTimeBasedFilter.js rename to src/ui/public/filter_bar/lib/__tests__/filterOutTimeBasedFilter.js diff --git a/src/ui/public/filter_bar/__tests__/generateMappingChain.js b/src/ui/public/filter_bar/lib/__tests__/generateMappingChain.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/generateMappingChain.js rename to src/ui/public/filter_bar/lib/__tests__/generateMappingChain.js diff --git a/src/ui/public/filter_bar/__tests__/mapAndFlattenFilters.js b/src/ui/public/filter_bar/lib/__tests__/mapAndFlattenFilters.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapAndFlattenFilters.js rename to src/ui/public/filter_bar/lib/__tests__/mapAndFlattenFilters.js diff --git a/src/ui/public/filter_bar/__tests__/mapDefault.js b/src/ui/public/filter_bar/lib/__tests__/mapDefault.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapDefault.js rename to src/ui/public/filter_bar/lib/__tests__/mapDefault.js diff --git a/src/ui/public/filter_bar/__tests__/mapExists.js b/src/ui/public/filter_bar/lib/__tests__/mapExists.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapExists.js rename to src/ui/public/filter_bar/lib/__tests__/mapExists.js diff --git a/src/ui/public/filter_bar/__tests__/mapFilter.js b/src/ui/public/filter_bar/lib/__tests__/mapFilter.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapFilter.js rename to src/ui/public/filter_bar/lib/__tests__/mapFilter.js diff --git a/src/ui/public/filter_bar/__tests__/mapFlattenAndWrapFilters.js b/src/ui/public/filter_bar/lib/__tests__/mapFlattenAndWrapFilters.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapFlattenAndWrapFilters.js rename to src/ui/public/filter_bar/lib/__tests__/mapFlattenAndWrapFilters.js diff --git a/src/ui/public/filter_bar/__tests__/mapGeoBoundingBox.js b/src/ui/public/filter_bar/lib/__tests__/mapGeoBoundingBox.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapGeoBoundingBox.js rename to src/ui/public/filter_bar/lib/__tests__/mapGeoBoundingBox.js diff --git a/src/ui/public/filter_bar/__tests__/mapMissing.js b/src/ui/public/filter_bar/lib/__tests__/mapMissing.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapMissing.js rename to src/ui/public/filter_bar/lib/__tests__/mapMissing.js diff --git a/src/ui/public/filter_bar/__tests__/mapQueryString.js b/src/ui/public/filter_bar/lib/__tests__/mapQueryString.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapQueryString.js rename to src/ui/public/filter_bar/lib/__tests__/mapQueryString.js diff --git a/src/ui/public/filter_bar/__tests__/mapRange.js b/src/ui/public/filter_bar/lib/__tests__/mapRange.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapRange.js rename to src/ui/public/filter_bar/lib/__tests__/mapRange.js diff --git a/src/ui/public/filter_bar/__tests__/mapScript.js b/src/ui/public/filter_bar/lib/__tests__/mapScript.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapScript.js rename to src/ui/public/filter_bar/lib/__tests__/mapScript.js diff --git a/src/ui/public/filter_bar/__tests__/mapTerms.js b/src/ui/public/filter_bar/lib/__tests__/mapTerms.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/mapTerms.js rename to src/ui/public/filter_bar/lib/__tests__/mapTerms.js diff --git a/src/ui/public/filter_bar/__tests__/onlyDisabled.js b/src/ui/public/filter_bar/lib/__tests__/onlyDisabled.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/onlyDisabled.js rename to src/ui/public/filter_bar/lib/__tests__/onlyDisabled.js diff --git a/src/ui/public/filter_bar/__tests__/uniqFilters.js b/src/ui/public/filter_bar/lib/__tests__/uniqFilters.js similarity index 100% rename from src/ui/public/filter_bar/__tests__/uniqFilters.js rename to src/ui/public/filter_bar/lib/__tests__/uniqFilters.js From c08f6723bec6f1fa29e2f61b688d4629f9301093 Mon Sep 17 00:00:00 2001 From: Court Ewing Date: Tue, 15 Sep 2015 16:53:32 -0400 Subject: [PATCH 4/5] Use describe() instead of context() in tests --- src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js | 4 ++-- src/ui/public/filter_manager/lib/__tests__/range.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js b/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js index b0c851b6774b0..05cce74b773dd 100644 --- a/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js +++ b/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js @@ -21,7 +21,7 @@ describe('ui/filter_bar/lib', function () { }; })); - context('when given a filter that is not match_all', function () { + describe('when given a filter that is not match_all', function () { it('filter is rejected', function (done) { delete filter.match_all; mapMatchAll(filter).catch(result => { @@ -32,7 +32,7 @@ describe('ui/filter_bar/lib', function () { }); }); - context('when given a match_all filter', function () { + describe('when given a match_all filter', function () { let result; beforeEach(function () { mapMatchAll(filter).then(r => result = r); diff --git a/src/ui/public/filter_manager/lib/__tests__/range.js b/src/ui/public/filter_manager/lib/__tests__/range.js index fb3432a6cab21..8145a9a3a0212 100644 --- a/src/ui/public/filter_manager/lib/__tests__/range.js +++ b/src/ui/public/filter_manager/lib/__tests__/range.js @@ -64,7 +64,7 @@ describe('Filter Manager', function () { }); }); - context('when given params where one side is infinite', function () { + describe('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); @@ -90,7 +90,7 @@ describe('Filter Manager', function () { }); }); - context('when given params where both sides are infinite', function () { + describe('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); From 36052d3bdb7078418aeb5854727d77fd3cc564de Mon Sep 17 00:00:00 2001 From: Court Ewing Date: Tue, 15 Sep 2015 17:12:22 -0400 Subject: [PATCH 5/5] Use $rootScope.$apply() directly in tests --- src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js b/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js index 05cce74b773dd..d274a7bd95dac 100644 --- a/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js +++ b/src/ui/public/filter_bar/lib/__tests__/mapMatchAll.js @@ -3,14 +3,14 @@ describe('ui/filter_bar/lib', function () { describe('mapMatchAll()', function () { const expect = require('expect.js'); const ngMock = require('ngMock'); - let resolvePromises; + let $rootScope; let mapMatchAll; let filter; beforeEach(ngMock.module('kibana')); - beforeEach(ngMock.inject(function (Private, $rootScope) { - resolvePromises = () => $rootScope.$apply(); + beforeEach(ngMock.inject(function (Private, _$rootScope_) { + $rootScope = _$rootScope_; mapMatchAll = Private(require('ui/filter_bar/lib/mapMatchAll')); filter = { match_all: {}, @@ -28,7 +28,7 @@ describe('ui/filter_bar/lib', function () { expect(result).to.be(filter); done(); }); - resolvePromises(); + $rootScope.$apply(); }); }); @@ -36,7 +36,7 @@ describe('ui/filter_bar/lib', function () { let result; beforeEach(function () { mapMatchAll(filter).then(r => result = r); - resolvePromises(); + $rootScope.$apply(); }); it('key is set to meta field', function () {