Skip to content

Commit

Permalink
feat(multi_match): Remove old multi_match view
Browse files Browse the repository at this point in the history
BREAKING CHANGE: the old multi_match view is removed.

I don't remember writing it, but since
#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
#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.
  • Loading branch information
orangejulius committed Aug 26, 2020
1 parent cee0da3 commit ae426ae
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 233 deletions.
1 change: 0 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
1 change: 0 additions & 1 deletion test/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
8 changes: 6 additions & 2 deletions test/view/admin_multi_match.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,15 @@ 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');

var actual = admin_multi_match(vs);

var expected = {
'multi_match': {
'type': 'cross_fields',
'fields': [
'property1_field value^property1_boost value',
'property2_field value^property2_boost value'
Expand All @@ -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');
Expand All @@ -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'
Expand Down
169 changes: 0 additions & 169 deletions test/view/multi_match.js

This file was deleted.

21 changes: 8 additions & 13 deletions view/admin_multi_match.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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;
};
Expand Down
47 changes: 0 additions & 47 deletions view/multi_match.js

This file was deleted.

0 comments on commit ae426ae

Please sign in to comment.