From ae426aed5488359ab0d94a5fa18cae9902a4a370 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 26 Aug 2020 14:18:47 -0400 Subject: [PATCH] feat(multi_match): Remove old multi_match view BREAKING CHANGE: the old multi_match view is removed. I don't remember writing it, but since https://github.com/pelias/query/pull/14 in early 2016(!) we have had a general purpose view for [multi_match](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-multi-match-query.html) queries. This view doesn't fit our current pattern for keeping general purpose (i.e. corresponding to built in Elasticsearch query) views in the `view/leaf` directory, and it doesn't support as much functionality as the `multi_match` view @joxit made for us in https://github.com/pelias/query/pull/114 last year. This PR removes the older view, and in the process converts the `admin_multi_match` view over to using the new `multi_match` leaf query. While I don't think anything in the Pelias API uses the old `multi_match` query directly, this removal will require changes to the API tests because of minor differences in the final output. --- index.js | 1 - test/run.js | 1 - test/view/admin_multi_match.js | 8 +- test/view/multi_match.js | 169 --------------------------------- view/admin_multi_match.js | 21 ++-- view/multi_match.js | 47 --------- 6 files changed, 14 insertions(+), 233 deletions(-) delete mode 100644 test/view/multi_match.js delete mode 100644 view/multi_match.js diff --git a/index.js b/index.js index 061167f..e0652bf 100644 --- a/index.js +++ b/index.js @@ -28,7 +28,6 @@ module.exports.view = { address: require('./view/address'), admin: require('./view/admin'), admin_multi_match: require('./view/admin_multi_match'), - multi_match: require('./view/multi_match'), categories: require('./view/categories'), boundary_circle: require('./view/boundary_circle'), boundary_rect: require('./view/boundary_rect'), diff --git a/test/run.js b/test/run.js index 160161d..4ca0ebd 100644 --- a/test/run.js +++ b/test/run.js @@ -24,7 +24,6 @@ var tests = [ require('./view/focus.js'), require('./view/focus_only_function.js'), require('./view/layers.js'), - require('./view/multi_match.js'), require('./view/ngrams.js'), require('./view/phrase.js'), require('./view/popularity.js'), diff --git a/test/view/admin_multi_match.js b/test/view/admin_multi_match.js index 6577b9a..0fa5346 100644 --- a/test/view/admin_multi_match.js +++ b/test/view/admin_multi_match.js @@ -80,6 +80,7 @@ module.exports.tests.no_exceptions_conditions = function(test, common) { vs.var('input:property2', 'property2 value'); vs.var('admin:property2:field', 'property2_field value'); vs.var('admin:property2:boost', 'property2_boost value'); + vs.var('multi_match:type', 'cross_fields'); var admin_multi_match = require('../../view/admin_multi_match')(['property1', 'property2'], 'analyzer value'); @@ -87,6 +88,7 @@ module.exports.tests.no_exceptions_conditions = function(test, common) { var expected = { 'multi_match': { + 'type': 'cross_fields', 'fields': [ 'property1_field value^property1_boost value', 'property2_field value^property2_boost value' @@ -101,10 +103,11 @@ module.exports.tests.no_exceptions_conditions = function(test, common) { }); - test('boosts should default to 1 when not specified', function(t) { + test('boosts should default to unset when not specified', function(t) { var vs = new VariableStore(); vs.var('input:property1', 'property1 value'); vs.var('admin:property1:field', 'property1_field value'); + vs.var('multi_match:type', 'cross_fields'); // there is no boost var admin_multi_match = require('../../view/admin_multi_match')(['property1'], 'analyzer value'); @@ -113,8 +116,9 @@ module.exports.tests.no_exceptions_conditions = function(test, common) { var expected = { 'multi_match': { + 'type': 'cross_fields', 'fields': [ - 'property1_field value^1' + 'property1_field value' ], 'query': { $: 'property1 value' }, 'analyzer': 'analyzer value' diff --git a/test/view/multi_match.js b/test/view/multi_match.js deleted file mode 100644 index 0116353..0000000 --- a/test/view/multi_match.js +++ /dev/null @@ -1,169 +0,0 @@ -var multi_match = require('../../view/multi_match'); -var VariableStore = require('../../lib/VariableStore'); - -module.exports.tests = {}; - -module.exports.tests.interface = function(test, common) { - test('interface: contructor', function(t) { - t.equal(typeof multi_match, 'function', 'valid function'); - t.equal(multi_match.length, 4, 'takes 4 args'); - t.end(); - }); - -}; - -module.exports.tests.missing_variable_conditions = function(test, common) { - test('missing query_var in VariableStore should return null', function(t) { - var vs = new VariableStore(); - - var actual = multi_match(vs, [], 'analyzer value', 'query var'); - - t.equal(actual, null, 'should have returned null for unset query_var'); - t.end(); - - }); - -}; - -module.exports.tests.no_exceptions_conditions = function(test, common) { - test('all fields available should populate all fields', function(t) { - var vs = new VariableStore(); - vs.var('query var', 'query value'); - - var fields_with_boosts = [ - { field: 'field 1', boost: 'boost value 1'}, - { field: 'field 2'}, - { field: 'field 3', boost: 'boost value 3'} - ]; - - var actual = multi_match(vs, fields_with_boosts, 'analyzer value', 'query var'); - - var expected = { - multi_match: { - fields: [ - 'field 1^boost value 1', - 'field 2^1', - 'field 3^boost value 3' - ], - query: { $: 'query value' }, - analyzer: 'analyzer value' - } - }; - - t.deepEquals(actual, expected, 'should have returned object'); - t.end(); - - }); - -}; - -module.exports.tests.cutoff_frequency = function (test, common) { - test('optional cutoff_frequency', function (t) { - var vs = new VariableStore(); - vs.var('query var', 'query value'); - vs.var('multi_match:cutoff_frequency', 0.1); - - var fields_with_boosts = [ - { field: 'field 1' }, - { field: 'field 2' }, - { field: 'field 3' } - ]; - - var actual = multi_match(vs, fields_with_boosts, 'analyzer value', 'query var'); - - var expected = { - multi_match: { - fields: [ - 'field 1^1', - 'field 2^1', - 'field 3^1' - ], - query: { $: 'query value' }, - analyzer: 'analyzer value', - cutoff_frequency: { $: 0.1 }, - } - }; - - t.deepEquals(actual, expected, 'should have returned object'); - t.end(); - - }); - -}; - -module.exports.tests.type = function (test, common) { - test('optional type', function (t) { - var vs = new VariableStore(); - vs.var('query var', 'query value'); - vs.var('multi_match:type', 'cross_fields'); - - var fields_with_boosts = [ - { field: 'field 1' }, - { field: 'field 2' }, - { field: 'field 3' } - ]; - - var actual = multi_match(vs, fields_with_boosts, 'analyzer value', 'query var'); - - var expected = { - multi_match: { - fields: [ - 'field 1^1', - 'field 2^1', - 'field 3^1' - ], - query: { $: 'query value' }, - analyzer: 'analyzer value', - type: { $: 'cross_fields' }, - } - }; - - t.deepEquals(actual, expected, 'should have returned object'); - t.end(); - - }); - -}; - -module.exports.tests.operator = function (test, common) { - test('optional operator', function (t) { - var vs = new VariableStore(); - vs.var('query var', 'query value'); - vs.var('multi_match:operator', 'and'); - - var fields_with_boosts = [ - { field: 'field 1' }, - { field: 'field 2' }, - { field: 'field 3' } - ]; - - var actual = multi_match(vs, fields_with_boosts, 'analyzer value', 'query var'); - - var expected = { - multi_match: { - fields: [ - 'field 1^1', - 'field 2^1', - 'field 3^1' - ], - query: { $: 'query value' }, - analyzer: 'analyzer value', - operator: { $: 'and' }, - } - }; - - t.deepEquals(actual, expected, 'should have returned object'); - t.end(); - - }); - -}; - -module.exports.all = function (tape, common) { - function test(name, testFunction) { - return tape('multi_match ' + name, testFunction); - } - for( var testCase in module.exports.tests ){ - module.exports.tests[testCase](test, common); - } -}; diff --git a/view/admin_multi_match.js b/view/admin_multi_match.js index ff91529..de91a47 100644 --- a/view/admin_multi_match.js +++ b/view/admin_multi_match.js @@ -2,7 +2,7 @@ this view is wrapped in a function so it can be re-used **/ -var multi_match = require('./multi_match'); +var multi_match = require('../lib/leaf/multi_match'); /* * Match several admin fields against the same query @@ -22,18 +22,13 @@ module.exports = function( admin_properties, analyzer ){ return null; } - // from the input parameters, generate a list of fields with boosts to - // query against - var fields_with_boosts = valid_admin_properties.map(function(admin_property) { - var boost = 1; - if (vs.isset('admin:' + admin_property + ':boost')) { - boost = vs.var('admin:' + admin_property + ':boost'); + const fields = valid_admin_properties.map(function(admin_property) { + if (vs.isset(`admin:${admin_property}:boost`)) { + return vs.var(`admin:${admin_property}:field`) + '^' + + vs.var(`admin:${admin_property}:boost`); + } else { + return vs.var(`admin:${admin_property}:field`).get(); } - - return { - field: vs.var('admin:' + admin_property + ':field'), - boost: boost - }; }); // the actual query text is simply taken from the first valid admin field @@ -42,7 +37,7 @@ module.exports = function( admin_properties, analyzer ){ var queryVar = 'input:' + valid_admin_properties[0]; // send the parameters to the standard multi_match view - var view = multi_match(vs, fields_with_boosts, analyzer, queryVar); + var view = multi_match(vs.var('multi_match:type').get(), fields, vs.var(queryVar), { analyzer: analyzer }); return view; }; diff --git a/view/multi_match.js b/view/multi_match.js deleted file mode 100644 index f982b71..0000000 --- a/view/multi_match.js +++ /dev/null @@ -1,47 +0,0 @@ -/*** - * // simple view for an arbitrary multi_match query - * - * params: - * @fields_with_boosts: objects with a structure like - * { - * field: 'fieldName', - * boost: 5 //optional - * } - * @analyzer: a string for the analyzer name - * @query_var: the variable that stores the actual query - * - */ -module.exports = function( vs, fields_with_boosts, analyzer, query_var ){ - // base view - var view = { multi_match: {} }; - - if (!vs.isset(query_var) || !analyzer) { - return null; - } - - // construct the field string (which contains the boost) - // from the more friendly object representation - var fields = fields_with_boosts.map(function(data) { - var fieldString = data.field; - var boost = data.boost || 1; - return fieldString +'^' + boost; - }); - - view.multi_match.fields = fields; - view.multi_match.query = vs.var(query_var); - view.multi_match.analyzer = analyzer; - - if (vs.isset('multi_match:type')) { - view.multi_match.type = vs.var('multi_match:type'); - } - - if (vs.isset('multi_match:operator')) { - view.multi_match.operator = vs.var('multi_match:operator'); - } - - if (vs.isset('multi_match:cutoff_frequency')) { - view.multi_match.cutoff_frequency = vs.var('multi_match:cutoff_frequency'); - } - - return view; -};