Stop generating filters with old invalid match phrase query syntax (#48033)

We've migrated the old syntax at query time for a long time, but when creating a phrase filter we still used the old (now invalid) syntax. This could be confusing to users since the raw query DSL is viewable in the filter editor. Also, since map_phrase.ts only expected the old syntax, it wouldn't detect a match_phrase filter if the user entered the new syntax into the raw DSL editor.

This PR updates all of the filter generation code that I know of to use the new syntax, and it updates the map_phrase.ts file to accept both syntaxes.

packages/kbn-es-query/src/filters/__tests__/phrase.js

@@ -39,11 +39,8 @@ describe('Filter Manager', function () {
     it('should return a match query filter when passed a standard field', function () {
       const field = getField(indexPattern, 'bytes');
       expected.query = {
-        match: {
-          bytes: {
-            query: 5,
-            type: 'phrase'
-          }
+        match_phrase: {
+          bytes: 5
         }
       };
       expect(buildPhraseFilter(field, 5, indexPattern)).to.eql(expected);

packages/kbn-es-query/src/filters/lib/index.ts

@@ -25,7 +25,13 @@ import { CustomFilter } from './custom_filter';
 import { ExistsFilter, isExistsFilter } from './exists_filter';
 import { GeoBoundingBoxFilter, isGeoBoundingBoxFilter } from './geo_bounding_box_filter';
 import { GeoPolygonFilter, isGeoPolygonFilter } from './geo_polygon_filter';
-import { PhraseFilter, isPhraseFilter, isScriptedPhraseFilter } from './phrase_filter';
+import {
+  PhraseFilter,
+  isPhraseFilter,
+  isScriptedPhraseFilter,
+  getPhraseFilterField,
+  getPhraseFilterValue,
+} from './phrase_filter';
 import { PhrasesFilter, isPhrasesFilter } from './phrases_filter';
 import { QueryStringFilter, isQueryStringFilter } from './query_string_filter';
 import {
@@ -48,6 +54,8 @@ export {
   PhraseFilter,
   isPhraseFilter,
   isScriptedPhraseFilter,
+  getPhraseFilterField,
+  getPhraseFilterValue,
   PhrasesFilter,
   isPhrasesFilter,
   QueryStringFilter,

packages/kbn-es-query/src/filters/lib/phrase_filter.ts

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { get } from 'lodash';
+import { get, isPlainObject } from 'lodash';
 import { Filter, FilterMeta } from './meta_filter';
 
 export type PhraseFilterMeta = FilterMeta & {
@@ -36,8 +36,30 @@ export type PhraseFilter = Filter & {
   meta: PhraseFilterMeta;
 };
 
-export const isPhraseFilter = (filter: any): filter is PhraseFilter =>
-  filter && (filter.query && filter.query.match);
+type PhraseFilterValue = string | number | boolean;
+
+export const isPhraseFilter = (filter: any): filter is PhraseFilter => {
+  const isMatchPhraseQuery = filter && filter.query && filter.query.match_phrase;
+
+  const isDeprecatedMatchPhraseQuery =
+    filter &&
+    filter.query &&
+    filter.query.match &&
+    Object.values(filter.query.match).find((params: any) => params.type === 'phrase');
+
+  return !!(isMatchPhraseQuery || isDeprecatedMatchPhraseQuery);
+};
 
 export const isScriptedPhraseFilter = (filter: any): filter is PhraseFilter =>
   Boolean(get(filter, 'script.script.params.value'));
+
+export const getPhraseFilterField = (filter: PhraseFilter) => {
+  const queryConfig = filter.query.match_phrase || filter.query.match;
+  return Object.keys(queryConfig)[0];
+};
+
+export const getPhraseFilterValue = (filter: PhraseFilter): PhraseFilterValue => {
+  const queryConfig = filter.query.match_phrase || filter.query.match;
+  const queryValue = Object.values(queryConfig)[0] as any;
+  return isPlainObject(queryValue) ? queryValue.query : queryValue;
+};

packages/kbn-es-query/src/filters/phrase.js

@@ -26,11 +26,8 @@ export function buildPhraseFilter(field, value, indexPattern) {
     filter.script = getPhraseScript(field, value);
     filter.meta.field = field.name;
   } else {
-    filter.query = { match: {} };
-    filter.query.match[field.name] = {
-      query: convertedValue,
-      type: 'phrase'
-    };
+    filter.query = { match_phrase: {} };
+    filter.query.match_phrase[field.name] = convertedValue;
   }
   return filter;
 }

src/legacy/core_plugins/data/public/filter/filter_manager/lib/map_filter.test.ts

@@ -29,7 +29,7 @@ describe('filter manager utilities', () => {
     test('should map query filters', async () => {
       const before = {
         meta: { index: 'logstash-*' },
-        query: { match: { _type: { query: 'apache' } } },
+        query: { match: { _type: { query: 'apache', type: 'phrase' } } },
       };
       const after = mapFilter(before as Filter);
 

src/legacy/core_plugins/data/public/filter/filter_manager/lib/map_phrase.ts

@@ -24,6 +24,8 @@ import {
   FILTERS,
   isPhraseFilter,
   isScriptedPhraseFilter,
+  getPhraseFilterField,
+  getPhraseFilterValue,
   FilterValueFormatter,
 } from '@kbn/es-query';
 
@@ -39,8 +41,8 @@ const getFormattedValueFn = (value: any) => {
 const getParams = (filter: PhraseFilter) => {
   const scriptedPhraseValue = getScriptedPhraseValue(filter);
   const isScriptedFilter = Boolean(scriptedPhraseValue);
-  const key = isScriptedFilter ? filter.meta.field || '' : Object.keys(filter.query.match)[0];
-  const query = scriptedPhraseValue || get(filter, ['query', 'match', key, 'query']);
+  const key = isScriptedFilter ? filter.meta.field || '' : getPhraseFilterField(filter);
+  const query = scriptedPhraseValue || getPhraseFilterValue(filter);
   const params = { query };
 
   return {

src/legacy/core_plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.js

@@ -19,7 +19,13 @@
 
 import _ from 'lodash';
 import { FilterManager } from './filter_manager.js';
-import { buildPhraseFilter, buildPhrasesFilter } from '@kbn/es-query';
+import {
+  buildPhraseFilter,
+  buildPhrasesFilter,
+  getPhraseFilterField,
+  getPhraseFilterValue,
+  isPhraseFilter,
+} from '@kbn/es-query';
 
 export class PhraseFilterManager extends FilterManager {
   constructor(controlId, fieldName, indexPattern, queryFilter) {
@@ -101,8 +107,12 @@ export class PhraseFilterManager extends FilterManager {
     }
 
     // single phrase filter
-    if (_.has(kbnFilter, ['query', 'match', this.fieldName])) {
-      return _.get(kbnFilter, ['query', 'match', this.fieldName, 'query']);
+    if (isPhraseFilter(kbnFilter)) {
+      if (getPhraseFilterField(kbnFilter) !== this.fieldName) {
+        return;
+      }
+
+      return getPhraseFilterValue(kbnFilter);
     }
 
     // single phrase filter from bool filter

src/legacy/core_plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.test.js

@@ -54,7 +54,7 @@ describe('PhraseFilterManager', function () {
       expect(newFilter.meta.controlledBy).to.be(controlId);
       expect(newFilter.meta.key).to.be('field1');
       expect(newFilter).to.have.property('query');
-      expect(JSON.stringify(newFilter.query, null, '')).to.be('{"match":{"field1":{"query":"ios","type":"phrase"}}}');
+      expect(JSON.stringify(newFilter.query, null, '')).to.be('{"match_phrase":{"field1":"ios"}}');
     });
 
     test('should create bool filter from multiple values', function () {

src/legacy/core_plugins/kibana/public/discover/context/query_parameters/__tests__/action_add_filter.js

@@ -49,10 +49,10 @@ describe('context app', function () {
       const filterManagerAddStub = filterManagerStub.addFilters;
       //get the generated filter
       const generatedFilter = filterManagerAddStub.firstCall.args[0][0];
-      const queryKeys = Object.keys(generatedFilter.query.match);
+      const queryKeys = Object.keys(generatedFilter.query.match_phrase);
       expect(filterManagerAddStub.calledOnce).to.be(true);
       expect(queryKeys[0]).to.eql('FIELD_NAME');
-      expect(generatedFilter.query.match[queryKeys[0]].query).to.eql('FIELD_VALUE');
+      expect(generatedFilter.query.match_phrase[queryKeys[0]]).to.eql('FIELD_VALUE');
     });
 
     it('should pass the index pattern id to the filterManager', function () {

src/legacy/ui/public/agg_types/__tests__/buckets/_terms_other_bucket_helper.js

@@ -186,8 +186,8 @@ describe('Terms Agg Other bucket helper', () => {
                 filter: [{ exists: { field: 'machine.os.raw' } }],
                 should: [],
                 must_not: [
-                  { match_phrase: { 'machine.os.raw': { query: 'ios' } } },
-                  { match_phrase: { 'machine.os.raw': { query: 'win xp' } } }
+                  { match_phrase: { 'machine.os.raw': 'ios' } },
+                  { match_phrase: { 'machine.os.raw': 'win xp' } }
                 ]
               }
             },
@@ -210,13 +210,13 @@ describe('Terms Agg Other bucket helper', () => {
                 bool: {
                   must: [],
                   filter: [
-                    { match_phrase: { 'geo.src': { query: 'IN' } } },
+                    { match_phrase: { 'geo.src': 'IN' } },
                     { exists: { field: 'machine.os.raw' } }
                   ],
                   should: [],
                   must_not: [
-                    { match_phrase: { 'machine.os.raw': { query: 'ios' } } },
-                    { match_phrase: { 'machine.os.raw': { query: 'win xp' } } }
+                    { match_phrase: { 'machine.os.raw': 'ios' } },
+                    { match_phrase: { 'machine.os.raw': 'win xp' } }
                   ]
                 }
               },
@@ -224,13 +224,13 @@ describe('Terms Agg Other bucket helper', () => {
                 bool: {
                   must: [],
                   filter: [
-                    { match_phrase: { 'geo.src': { query: 'US' } } },
+                    { match_phrase: { 'geo.src': 'US' } },
                     { exists: { field: 'machine.os.raw' } }
                   ],
                   should: [],
                   must_not: [
-                    { match_phrase: { 'machine.os.raw': { query: 'ios' } } },
-                    { match_phrase: { 'machine.os.raw': { query: 'win xp' } } }
+                    { match_phrase: { 'machine.os.raw': 'ios' } },
+                    { match_phrase: { 'machine.os.raw': 'win xp' } }
                   ]
                 }
               },

src/legacy/ui/public/agg_types/__tests__/buckets/create_filter/terms.js

@@ -35,7 +35,7 @@ describe('AggConfig Filters', function () {
       indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider);
     }));
 
-    it('should return a match filter for terms', function () {
+    it('should return a match_phrase filter for terms', function () {
       const vis = new Vis(indexPattern, {
         type: 'histogram',
         aggs: [ { type: 'terms', schema: 'segment', params: { field: '_type' } } ]
@@ -43,10 +43,9 @@ describe('AggConfig Filters', function () {
       const aggConfig = vis.aggs.byName('terms')[0];
       const filter = createFilterTerms(aggConfig, 'apache');
       expect(filter).to.have.property('query');
-      expect(filter.query).to.have.property('match');
-      expect(filter.query.match).to.have.property('_type');
-      expect(filter.query.match._type).to.have.property('query', 'apache');
-      expect(filter.query.match._type).to.have.property('type', 'phrase');
+      expect(filter.query).to.have.property('match_phrase');
+      expect(filter.query.match_phrase).to.have.property('_type');
+      expect(filter.query.match_phrase._type).to.be('apache');
       expect(filter).to.have.property('meta');
       expect(filter.meta).to.have.property('index', indexPattern.id);
 
@@ -60,15 +59,15 @@ describe('AggConfig Filters', function () {
       const aggConfig = vis.aggs.byName('terms')[0];
       const filterFalse = createFilterTerms(aggConfig, 0);
       expect(filterFalse).to.have.property('query');
-      expect(filterFalse.query).to.have.property('match');
-      expect(filterFalse.query.match).to.have.property('ssl');
-      expect(filterFalse.query.match.ssl).to.have.property('query', false);
+      expect(filterFalse.query).to.have.property('match_phrase');
+      expect(filterFalse.query.match_phrase).to.have.property('ssl');
+      expect(filterFalse.query.match_phrase.ssl).to.be(false);
 
       const filterTrue = createFilterTerms(aggConfig, 1);
       expect(filterTrue).to.have.property('query');
-      expect(filterTrue.query).to.have.property('match');
-      expect(filterTrue.query.match).to.have.property('ssl');
-      expect(filterTrue.query.match.ssl).to.have.property('query', true);
+      expect(filterTrue.query).to.have.property('match_phrase');
+      expect(filterTrue.query.match_phrase).to.have.property('ssl');
+      expect(filterTrue.query.match_phrase.ssl).to.be(true);
     });
 
     it('should generate correct __missing__ filter', () => {

src/legacy/ui/public/agg_types/buckets/_terms_other_bucket_helper.js

@@ -113,7 +113,7 @@ const getAggConfigResultMissingBuckets = (responseAggs, aggId) => {
 const getOtherAggTerms = (requestAgg, key, otherAgg) => {
   return requestAgg['other-filter'].filters.filters[key].bool.must_not
     .filter(filter => filter.match_phrase && filter.match_phrase[otherAgg.params.field.name])
-    .map(filter => filter.match_phrase[otherAgg.params.field.name].query);
+    .map(filter => filter.match_phrase[otherAgg.params.field.name]);
 };
 
 export const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => {

src/legacy/ui/public/filter_manager/__tests__/filter_generator.js

@@ -76,7 +76,7 @@ describe('Filter Manager', function () {
     expect(queryFilter.addFilters.callCount).to.be(1);
     checkAddFilters(1, [{
       meta: { index: 'myIndex', negate: false },
-      query: { match: { myField: { query: 1, type: 'phrase' } } }
+      query: { match_phrase: { myField: 1 } }
     }]);
   });
 
@@ -85,13 +85,13 @@ describe('Filter Manager', function () {
     expect(queryFilter.addFilters.callCount).to.be(1);
     checkAddFilters(3, [{
       meta: { index: 'myIndex', negate: false },
-      query: { match: { myField: { query: 1, type: 'phrase' } } }
+      query: { match_phrase: { myField: 1 } }
     }, {
       meta: { index: 'myIndex', negate: false },
-      query: { match: { myField: { query: 2, type: 'phrase' } } }
+      query: { match_phrase: { myField: 2 } }
     }, {
       meta: { index: 'myIndex', negate: false },
-      query: { match: { myField: { query: 3, type: 'phrase' } } }
+      query: { match_phrase: { myField: 3 } }
     }]);
   });
 
@@ -107,7 +107,7 @@ describe('Filter Manager', function () {
     filterGen.add('myField', 1, '+', 'myIndex');
     checkAddFilters(1, [{
       meta: { index: 'myIndex', negate: false },
-      query: { match: { myField: { query: 1, type: 'phrase' } } }
+      query: { match_phrase: { myField: 1 } }
     }], 0);
     expect(appState.filters).to.have.length(1);
 
@@ -115,7 +115,7 @@ describe('Filter Manager', function () {
     filterGen.add('myField', 1, '-', 'myIndex');
     checkAddFilters(1, [{
       meta: { index: 'myIndex', negate: true, disabled: false },
-      query: { match: { myField: { query: 1, type: 'phrase' } } }
+      query: { match_phrase: { myField: 1 } }
     }], 1);
     expect(appState.filters).to.have.length(1);
 
@@ -152,7 +152,7 @@ describe('Filter Manager', function () {
   it('should enable matching filters being changed', function () {
     _.each([true, false], function (negate) {
       appState.filters = [{
-        query: { match: { myField: { query: 1 } } },
+        query: { match_phrase: { myField: 1 } },
         meta: { disabled: true, negate: negate }
       }];
       expect(appState.filters.length).to.be(1);

src/legacy/ui/public/filter_manager/filter_generator.js

@@ -18,7 +18,7 @@
  */
 
 import _ from 'lodash';
-import { getPhraseScript } from '@kbn/es-query';
+import { getPhraseFilterField, getPhraseFilterValue, getPhraseScript, isPhraseFilter } from '@kbn/es-query';
 
 // Adds a filter to a passed state
 export function getFilterGenerator(queryFilter) {
@@ -42,8 +42,8 @@ export function getFilterGenerator(queryFilter) {
           return filter.exists.field === value;
         }
 
-        if (_.has(filter, 'query.match')) {
-          return filter.query.match[fieldName] && filter.query.match[fieldName].query === value;
+        if (isPhraseFilter(filter)) {
+          return getPhraseFilterField(filter) === fieldName && getPhraseFilterValue(filter) === value;
         }
 
         if (filter.script) {
@@ -76,8 +76,8 @@ export function getFilterGenerator(queryFilter) {
               script: getPhraseScript(field, value)
             };
           } else {
-            filter = { meta: { negate, index }, query: { match: {} } };
-            filter.query.match[fieldName] = { query: value, type: 'phrase' };
+            filter = { meta: { negate, index }, query: { match_phrase: {} } };
+            filter.query.match_phrase[fieldName] = value;
           }
 
           break;