From 6c205451c4f93fabe4964aff3ea6beedf65eb237 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Mon, 9 Dec 2024 13:56:37 -0500 Subject: [PATCH 1/9] start test --- lib/elasticsearch/elastic-query-builder.js | 43 ++++++++++++---------- test/elastic-query-builder.test.js | 10 +++++ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index db3a6697..849a218c 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -488,26 +488,9 @@ class ElasticQueryBuilder { } } - /** - * Examine request for user-filters. When found, add them to query. - */ - applyFilters () { - if (!this.request.params.filters) return - - let filterClausesWithPaths = [] - - // Add clauses for dateAfter / dateBefore filters, if used: - filterClausesWithPaths = filterClausesWithPaths.concat( - this.filterClausesForDateParams(this.request.params.filters) - ) - - // Collect those filters that use a simple term match - const simpleMatchFilters = Object.keys(this.request.params.filters) - .filter((k) => FILTER_CONFIG[k].operator === 'match') - - filterClausesWithPaths = filterClausesWithPaths.concat(simpleMatchFilters.map((prop) => { + buildSimpleMatchFilters (simpleMatchFilters) { + return simpleMatchFilters.map((prop) => { const config = FILTER_CONFIG[prop] - let value = this.request.params.filters[prop] // This builds a filter cause from the value: @@ -536,7 +519,27 @@ class ElasticQueryBuilder { const clause = (Array.isArray(value)) ? { bool: { [booleanOperator]: value.map(buildClause) } } : buildClause(value) return { path: config.path, clause } - })) + }) + } + + /** + * Examine request for user-filters. When found, add them to query. + */ + applyFilters () { + if (!this.request.params.filters) return + + let filterClausesWithPaths = [] + + // Add clauses for dateAfter / dateBefore filters, if used: + filterClausesWithPaths = filterClausesWithPaths.concat( + this.filterClausesForDateParams(this.request.params.filters) + ) + + // Collect those filters that use a simple term match + const simpleMatchFilters = Object.keys(this.request.params.filters) + .filter((k) => FILTER_CONFIG[k].operator === 'match') + + filterClausesWithPaths = filterClausesWithPaths.concat(this.buildSimpleMatchFilters(simpleMatchFilters)) // Gather root (not nested) filters: let filterClauses = filterClausesWithPaths diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index 8ed4bcd9..a4a9f625 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -4,6 +4,16 @@ const ElasticQueryBuilder = require('../lib/elasticsearch/elastic-query-builder' const ApiRequest = require('../lib/api-request') describe('ElasticQueryBuilder', () => { + describe.only('buildSimpleMatchFilters', () => { + it('can handle (multiple) single value, single match field filters', () => { + const request = new ApiRequest({ filters: { buildingLocation: 'toast', lanuage: 'spanish' } }) + const mockQueryBuilder = { + request, + buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters + } + console.log(mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'language'])) + }) + }) describe('search_scope all', () => { it('generates an "all" query', () => { const request = new ApiRequest({ q: 'toast' }) From 98b4c0b985ab31ae42405bda381b893468bd9531 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Mon, 9 Dec 2024 14:17:48 -0500 Subject: [PATCH 2/9] tests and pull out build clause --- lib/elasticsearch/elastic-query-builder.js | 46 ++++++----- test/elastic-query-builder.test.js | 96 +++++++++++++++++++++- 2 files changed, 116 insertions(+), 26 deletions(-) diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index 849a218c..8a40a882 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -488,35 +488,37 @@ class ElasticQueryBuilder { } } + // This builds a filter cause from the value: + buildClause (value, field) { + // If filtering on a packed field and value isn't a packed value: + if (value.indexOf('||') < 0 && field.match(/_packed$/)) { + // Figure out the base property (e.g. 'owner') + const baseField = field.replace(/_packed$/, '') + // Allow supplied val to match against either id or value: + return { + bool: { + should: [ + { term: { [`${baseField}.id`]: value } }, + { term: { [`${baseField}.label`]: value } } + ] + } + } + } else return { term: { [field]: value } } + } + buildSimpleMatchFilters (simpleMatchFilters) { return simpleMatchFilters.map((prop) => { const config = FILTER_CONFIG[prop] - let value = this.request.params.filters[prop] - - // This builds a filter cause from the value: - const buildClause = (value) => { - // If filtering on a packed field and value isn't a packed value: - if (config.operator === 'match' && value.indexOf('||') < 0 && config.field.match(/_packed$/)) { - // Figure out the base property (e.g. 'owner') - const baseField = config.field.replace(/_packed$/, '') - // Allow supplied val to match against either id or value: - return { - bool: { - should: [ - { term: { [`${baseField}.id`]: value } }, - { term: { [`${baseField}.label`]: value } } - ] - } - } - } else if (config.operator === 'match') return { term: { [config.field]: value } } - } + const value = this.request.params.filters[prop] // If multiple values given, let's join them with 'should', causing it to operate as a boolean OR // Note: using 'must' here makes it a boolean AND const booleanOperator = 'should' - // If only one value given, don't wrap it in a useless bool: - if (Array.isArray(value) && value.length === 1) value = value.shift() - const clause = (Array.isArray(value)) ? { bool: { [booleanOperator]: value.map(buildClause) } } : buildClause(value) + + let clause + const singleValueArray = (Array.isArray(value) && value.length === 1) + if (singleValueArray) clause = this.buildClause(value[0], config.field) + else clause = { bool: { [booleanOperator]: value.map((value) => this.buildClause(value, config.field)) } } return { path: config.path, clause } }) diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index a4a9f625..b61b08c2 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -5,13 +5,101 @@ const ApiRequest = require('../lib/api-request') describe('ElasticQueryBuilder', () => { describe.only('buildSimpleMatchFilters', () => { - it('can handle (multiple) single value, single match field filters', () => { - const request = new ApiRequest({ filters: { buildingLocation: 'toast', lanuage: 'spanish' } }) + it('can handle (multiple) single value, single match field filters, as arrays', () => { + const request = new ApiRequest({ filters: { buildingLocation: ['toast'], subjectLiteral: ['spaghetti'] } }) const mockQueryBuilder = { request, - buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters + buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, + buildClause: ElasticQueryBuilder.prototype.buildClause } - console.log(mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'language'])) + const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'subjectLiteral']) + expect(simpleMatchFilters).to.deep.equal([ + { + path: undefined, + clause: { term: { buildingLocationIds: 'toast' } } + }, + { + path: undefined, + clause: { term: { subjectLiteral_exploded: 'spaghetti' } } + } + ]) + }) + // it('can handle (multiple) single value, single match field filters, strings', () => { + // const request = new ApiRequest({ filters: { buildingLocation: 'toast', subjectLiteral: 'spaghetti' } }) + // const mockQueryBuilder = { + // request, + // buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, + // buildClause: ElasticQueryBuilder.prototype.buildClause + // } + // const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'subjectLiteral']) + // expect(simpleMatchFilters).to.deep.equal([ + // { + // path: undefined, + // clause: { term: { buildingLocationIds: 'toast' } } + // }, + // { + // path: undefined, + // clause: { term: { subjectLiteral_exploded: 'spaghetti' } } + // } + // ]) + // }) + it('can handle multiple values', () => { + const request = new ApiRequest({ filters: { subjectLiteral: ['spaghetti', 'meatballs'] } }) + const mockQueryBuilder = { + request, + buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, + buildClause: ElasticQueryBuilder.prototype.buildClause + } + const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['subjectLiteral']) + expect(simpleMatchFilters).to.deep.equal([ + { + path: undefined, + clause: { + bool: { + should: [ + { term: { subjectLiteral_exploded: 'spaghetti' } }, + { term: { subjectLiteral_exploded: 'meatballs' } } + ] + } + } + } + ]) + }) + it('can handle packed values', () => { + const request = new ApiRequest({ filters: { language: ['spanish', 'finnish'] } }) + const mockQueryBuilder = { + request, + buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, + buildClause: ElasticQueryBuilder.prototype.buildClause + } + const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['language']) + expect(simpleMatchFilters).to.deep.equal([ + { + path: undefined, + clause: { + bool: { + should: [ + { + bool: { + should: [ + { term: { 'language.id': 'spanish' } }, + { term: { 'language.label': 'spanish' } } + ] + } + }, + { + bool: { + should: [ + { term: { 'language.id': 'finnish' } }, + { term: { 'language.label': 'finnish' } } + ] + } + } + ] + } + } + } + ]) }) }) describe('search_scope all', () => { From f3657b47c1725a819e111328f7e6db9efc3761bc Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Mon, 9 Dec 2024 14:23:12 -0500 Subject: [PATCH 3/9] pull out buildpackedfields clause --- lib/elasticsearch/elastic-query-builder.js | 26 +++++++++++++--------- test/elastic-query-builder.test.js | 24 ++++++++------------ 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index 8a40a882..db85bfa9 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -488,21 +488,25 @@ class ElasticQueryBuilder { } } + buildPackedFieldClause (field, value) { + // Figure out the base property (e.g. 'owner') + const baseField = field.replace(/_packed$/, '') + // Allow supplied val to match against either id or value: + return { + bool: { + should: [ + { term: { [`${baseField}.id`]: value } }, + { term: { [`${baseField}.label`]: value } } + ] + } + } + } + // This builds a filter cause from the value: buildClause (value, field) { // If filtering on a packed field and value isn't a packed value: if (value.indexOf('||') < 0 && field.match(/_packed$/)) { - // Figure out the base property (e.g. 'owner') - const baseField = field.replace(/_packed$/, '') - // Allow supplied val to match against either id or value: - return { - bool: { - should: [ - { term: { [`${baseField}.id`]: value } }, - { term: { [`${baseField}.label`]: value } } - ] - } - } + return this.buildPackedFieldClause(field, value) } else return { term: { [field]: value } } } diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index b61b08c2..7e5984de 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -5,13 +5,15 @@ const ApiRequest = require('../lib/api-request') describe('ElasticQueryBuilder', () => { describe.only('buildSimpleMatchFilters', () => { + const mockQueryBuilderFactory = (request) => ({ + request, + buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, + buildClause: ElasticQueryBuilder.prototype.buildClause, + buildPackedFieldClause: ElasticQueryBuilder.prototype.buildPackedFieldClause + }) it('can handle (multiple) single value, single match field filters, as arrays', () => { const request = new ApiRequest({ filters: { buildingLocation: ['toast'], subjectLiteral: ['spaghetti'] } }) - const mockQueryBuilder = { - request, - buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, - buildClause: ElasticQueryBuilder.prototype.buildClause - } + const mockQueryBuilder = mockQueryBuilderFactory(request) const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'subjectLiteral']) expect(simpleMatchFilters).to.deep.equal([ { @@ -45,11 +47,7 @@ describe('ElasticQueryBuilder', () => { // }) it('can handle multiple values', () => { const request = new ApiRequest({ filters: { subjectLiteral: ['spaghetti', 'meatballs'] } }) - const mockQueryBuilder = { - request, - buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, - buildClause: ElasticQueryBuilder.prototype.buildClause - } + const mockQueryBuilder = mockQueryBuilderFactory(request) const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['subjectLiteral']) expect(simpleMatchFilters).to.deep.equal([ { @@ -67,11 +65,7 @@ describe('ElasticQueryBuilder', () => { }) it('can handle packed values', () => { const request = new ApiRequest({ filters: { language: ['spanish', 'finnish'] } }) - const mockQueryBuilder = { - request, - buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, - buildClause: ElasticQueryBuilder.prototype.buildClause - } + const mockQueryBuilder = mockQueryBuilderFactory(request) const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['language']) expect(simpleMatchFilters).to.deep.equal([ { From 338ef7b6ae2c4964367cb4b2e5865e24b4e62c4d Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Mon, 9 Dec 2024 15:41:06 -0500 Subject: [PATCH 4/9] filters building --- lib/elasticsearch/config.js | 28 ++++---- lib/elasticsearch/elastic-query-builder.js | 29 ++++++--- .../elastic-query-filter-builder.js | 0 test/elastic-query-builder.test.js | 65 +++++++++++++------ 4 files changed, 79 insertions(+), 43 deletions(-) create mode 100644 lib/elasticsearch/elastic-query-filter-builder.js diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index 0efe1a30..10fd164a 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -74,20 +74,20 @@ const SEARCH_SCOPES = { } const FILTER_CONFIG = { - recordType: { operator: 'match', field: 'recordTypeId', repeatable: true }, - owner: { operator: 'match', field: 'items.owner_packed', repeatable: true, path: 'items' }, - subjectLiteral: { operator: 'match', field: 'subjectLiteral_exploded', repeatable: true }, - holdingLocation: { operator: 'match', field: 'items.holdingLocation_packed', repeatable: true, path: 'items' }, - buildingLocation: { operator: 'match', field: 'buildingLocationIds', repeatable: true }, - language: { operator: 'match', field: 'language_packed', repeatable: true }, - materialType: { operator: 'match', field: 'materialType_packed', repeatable: true }, - mediaType: { operator: 'match', field: 'mediaType_packed', repeatable: true }, - carrierType: { operator: 'match', field: 'carrierType_packed', repeatable: true }, - publisher: { operator: 'match', field: 'publisherLiteral.raw', repeatable: true }, - contributorLiteral: { operator: 'match', field: 'contributorLiteral.raw', repeatable: true }, - creatorLiteral: { operator: 'match', field: 'creatorLiteral.raw', repeatable: true }, - issuance: { operator: 'match', field: 'issuance_packed', repeatable: true }, - createdYear: { operator: 'match', field: 'createdYear', repeatable: true }, + recordType: { operator: 'match', field: ['recordTypeId'], repeatable: true }, + owner: { operator: 'match', field: ['items.owner_packed'], repeatable: true, path: 'items' }, + subjectLiteral: { operator: 'match', field: ['subjectLiteral_exploded'], repeatable: true }, + holdingLocation: { operator: 'match', field: ['items.holdingLocation_packed'], repeatable: true, path: 'items' }, + buildingLocation: { operator: 'match', field: ['buildingLocationIds'], repeatable: true }, + language: { operator: 'match', field: ['language_packed'], repeatable: true }, + materialType: { operator: 'match', field: ['materialType_packed'], repeatable: true }, + mediaType: { operator: 'match', field: ['mediaType_packed'], repeatable: true }, + carrierType: { operator: 'match', field: ['carrierType_packed'], repeatable: true }, + publisher: { operator: 'match', field: ['publisherLiteral.raw'], repeatable: true }, + contributorLiteral: { operator: 'match', field: ['contributorLiteral.raw'], repeatable: true }, + creatorLiteral: { operator: 'match', field: ['creatorLiteral.raw'], repeatable: true }, + issuance: { operator: 'match', field: ['issuance_packed'], repeatable: true }, + createdYear: { operator: 'match', field: ['createdYear'], repeatable: true }, dateAfter: { operator: 'custom', type: 'int' diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index db85bfa9..802aa197 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -488,7 +488,7 @@ class ElasticQueryBuilder { } } - buildPackedFieldClause (field, value) { + buildPackedFieldClause (value, field) { // Figure out the base property (e.g. 'owner') const baseField = field.replace(/_packed$/, '') // Allow supplied val to match against either id or value: @@ -502,27 +502,38 @@ class ElasticQueryBuilder { } } + buildMultiFieldClause (value, fields) { + return { + bool: + { should: fields.map(field => ({ term: { [field]: value } })) } + } + } + // This builds a filter cause from the value: buildClause (value, field) { - // If filtering on a packed field and value isn't a packed value: - if (value.indexOf('||') < 0 && field.match(/_packed$/)) { - return this.buildPackedFieldClause(field, value) + const filterMatchesOnMoreThanOneField = field.length > 1 + if (filterMatchesOnMoreThanOneField) { + return this.buildMultiFieldClause(value, field) + } + field = field[0] + const valueIsNotPackedValue = value.indexOf('||') < 0 + const isPackedField = field.match(/_packed$/) + if (isPackedField && valueIsNotPackedValue) { + return this.buildPackedFieldClause(value, field) } else return { term: { [field]: value } } } buildSimpleMatchFilters (simpleMatchFilters) { return simpleMatchFilters.map((prop) => { const config = FILTER_CONFIG[prop] - const value = this.request.params.filters[prop] + let value = this.request.params.filters[prop] // If multiple values given, let's join them with 'should', causing it to operate as a boolean OR // Note: using 'must' here makes it a boolean AND const booleanOperator = 'should' - let clause - const singleValueArray = (Array.isArray(value) && value.length === 1) - if (singleValueArray) clause = this.buildClause(value[0], config.field) - else clause = { bool: { [booleanOperator]: value.map((value) => this.buildClause(value, config.field)) } } + if (Array.isArray(value) && value.length === 1) value = value.shift() + const clause = (Array.isArray(value)) ? { bool: { [booleanOperator]: value.map((value) => this.buildClause(config.field, value)) } } : this.buildClause(config.field, value) return { path: config.path, clause } }) diff --git a/lib/elasticsearch/elastic-query-filter-builder.js b/lib/elasticsearch/elastic-query-filter-builder.js new file mode 100644 index 00000000..e69de29b diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index 7e5984de..2f5ca138 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -4,9 +4,38 @@ const ElasticQueryBuilder = require('../lib/elasticsearch/elastic-query-builder' const ApiRequest = require('../lib/api-request') describe('ElasticQueryBuilder', () => { - describe.only('buildSimpleMatchFilters', () => { + describe('buildClause', () => { + it('can handle multiple fields', () => { + expect(ElasticQueryBuilder.prototype.buildClause('value', ['field', 'parallelField'])) + .to.deep.equal({ + bool: + { + should: [ + { term: { field: 'value' } }, + { term: { parallelField: 'value' } }] + } + }) + }) + it('can handle packed fields', () => { + expect(ElasticQueryBuilder.prototype.buildClause('not packed value', ['field_packed'])) + .to.deep.equal({ + bool: { + should: [ + { term: { 'field.id': 'not packed value' } }, + { term: { 'field.label': 'not packed value' } } + ] + } + }) + }) + it('can handle the simple case', () => { + expect(ElasticQueryBuilder.prototype.buildClause('value', ['field'])) + .to.deep.equal({ term: { field: 'value' } }) + }) + }) + describe('buildSimpleMatchFilters', () => { const mockQueryBuilderFactory = (request) => ({ request, + buildMultiFieldClause: ElasticQueryBuilder.prototype.buildMultiFieldClause, buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, buildClause: ElasticQueryBuilder.prototype.buildClause, buildPackedFieldClause: ElasticQueryBuilder.prototype.buildPackedFieldClause @@ -26,25 +55,21 @@ describe('ElasticQueryBuilder', () => { } ]) }) - // it('can handle (multiple) single value, single match field filters, strings', () => { - // const request = new ApiRequest({ filters: { buildingLocation: 'toast', subjectLiteral: 'spaghetti' } }) - // const mockQueryBuilder = { - // request, - // buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, - // buildClause: ElasticQueryBuilder.prototype.buildClause - // } - // const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'subjectLiteral']) - // expect(simpleMatchFilters).to.deep.equal([ - // { - // path: undefined, - // clause: { term: { buildingLocationIds: 'toast' } } - // }, - // { - // path: undefined, - // clause: { term: { subjectLiteral_exploded: 'spaghetti' } } - // } - // ]) - // }) + it('can handle (multiple) single value, single match field filters, strings', () => { + const request = new ApiRequest({ filters: { buildingLocation: 'toast', subjectLiteral: 'spaghetti' } }) + const mockQueryBuilder = mockQueryBuilderFactory(request) + const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'subjectLiteral']) + expect(simpleMatchFilters).to.deep.equal([ + { + path: undefined, + clause: { term: { buildingLocationIds: 'toast' } } + }, + { + path: undefined, + clause: { term: { subjectLiteral_exploded: 'spaghetti' } } + } + ]) + }) it('can handle multiple values', () => { const request = new ApiRequest({ filters: { subjectLiteral: ['spaghetti', 'meatballs'] } }) const mockQueryBuilder = mockQueryBuilderFactory(request) From 46afd1fc4201269808b0dd80f18b42a46723c385 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Mon, 9 Dec 2024 16:04:30 -0500 Subject: [PATCH 5/9] fix tests --- lib/elasticsearch/elastic-query-builder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index 802aa197..338b9ba4 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -533,7 +533,7 @@ class ElasticQueryBuilder { const booleanOperator = 'should' if (Array.isArray(value) && value.length === 1) value = value.shift() - const clause = (Array.isArray(value)) ? { bool: { [booleanOperator]: value.map((value) => this.buildClause(config.field, value)) } } : this.buildClause(config.field, value) + const clause = (Array.isArray(value)) ? { bool: { [booleanOperator]: value.map((value) => this.buildClause(value, config.field)) } } : this.buildClause(value, config.field) return { path: config.path, clause } }) From 547a271c9c084ead32ae4ddc522bef3443d03c23 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Mon, 9 Dec 2024 16:07:48 -0500 Subject: [PATCH 6/9] update creatorliteral fields --- lib/elasticsearch/config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index 10fd164a..d050ef62 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -85,7 +85,7 @@ const FILTER_CONFIG = { carrierType: { operator: 'match', field: ['carrierType_packed'], repeatable: true }, publisher: { operator: 'match', field: ['publisherLiteral.raw'], repeatable: true }, contributorLiteral: { operator: 'match', field: ['contributorLiteral.raw'], repeatable: true }, - creatorLiteral: { operator: 'match', field: ['creatorLiteral.raw'], repeatable: true }, + creatorLiteral: { operator: 'match', field: ['creatorLiteral.raw', 'parallelCreatorLiteral.raw'], repeatable: true }, issuance: { operator: 'match', field: ['issuance_packed'], repeatable: true }, createdYear: { operator: 'match', field: ['createdYear'], repeatable: true }, dateAfter: { From a53bfbb97e3b4827b4e9663ac1b71864f127af5f Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Tue, 10 Dec 2024 12:02:47 -0500 Subject: [PATCH 7/9] rm support for _packed fields --- lib/elasticsearch/config.js | 14 +++++----- lib/elasticsearch/elastic-query-builder.js | 25 ++++++++--------- test/elastic-query-builder.test.js | 31 +++++++--------------- 3 files changed, 28 insertions(+), 42 deletions(-) diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index d050ef62..84314ac2 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -75,18 +75,18 @@ const SEARCH_SCOPES = { const FILTER_CONFIG = { recordType: { operator: 'match', field: ['recordTypeId'], repeatable: true }, - owner: { operator: 'match', field: ['items.owner_packed'], repeatable: true, path: 'items' }, + owner: { operator: 'match', field: ['items.owner.id', 'items.owner.label'], repeatable: true, path: 'items' }, subjectLiteral: { operator: 'match', field: ['subjectLiteral_exploded'], repeatable: true }, - holdingLocation: { operator: 'match', field: ['items.holdingLocation_packed'], repeatable: true, path: 'items' }, + holdingLocation: { operator: 'match', field: ['items.holdingLocation.id', 'items.holdingLocation.label'], repeatable: true, path: 'items' }, buildingLocation: { operator: 'match', field: ['buildingLocationIds'], repeatable: true }, - language: { operator: 'match', field: ['language_packed'], repeatable: true }, - materialType: { operator: 'match', field: ['materialType_packed'], repeatable: true }, - mediaType: { operator: 'match', field: ['mediaType_packed'], repeatable: true }, - carrierType: { operator: 'match', field: ['carrierType_packed'], repeatable: true }, + language: { operator: 'match', field: ['language.id', 'language.label'], repeatable: true }, + materialType: { operator: 'match', field: ['materialType.id', 'materialType.label'], repeatable: true }, + mediaType: { operator: 'match', field: ['mediaType.id', 'mediaType.label'], repeatable: true }, + carrierType: { operator: 'match', field: ['carrierType.id', 'carrierType.label'], repeatable: true }, publisher: { operator: 'match', field: ['publisherLiteral.raw'], repeatable: true }, contributorLiteral: { operator: 'match', field: ['contributorLiteral.raw'], repeatable: true }, creatorLiteral: { operator: 'match', field: ['creatorLiteral.raw', 'parallelCreatorLiteral.raw'], repeatable: true }, - issuance: { operator: 'match', field: ['issuance_packed'], repeatable: true }, + issuance: { operator: 'match', field: ['issuance.id', 'issuance.label'], repeatable: true }, createdYear: { operator: 'match', field: ['createdYear'], repeatable: true }, dateAfter: { operator: 'custom', diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index 338b9ba4..e1788a46 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -510,21 +510,18 @@ class ElasticQueryBuilder { } // This builds a filter cause from the value: - buildClause (value, field) { - const filterMatchesOnMoreThanOneField = field.length > 1 + buildFilterClause (value, fieldsToMatchOn) { + const filterMatchesOnMoreThanOneField = fieldsToMatchOn.length > 1 if (filterMatchesOnMoreThanOneField) { - return this.buildMultiFieldClause(value, field) + return this.buildMultiFieldClause(value, fieldsToMatchOn) + } else { + const field = fieldsToMatchOn[0] + return { term: { [field]: value } } } - field = field[0] - const valueIsNotPackedValue = value.indexOf('||') < 0 - const isPackedField = field.match(/_packed$/) - if (isPackedField && valueIsNotPackedValue) { - return this.buildPackedFieldClause(value, field) - } else return { term: { [field]: value } } } - buildSimpleMatchFilters (simpleMatchFilters) { - return simpleMatchFilters.map((prop) => { + buildMatchOperatorFilterQueries (filtersWithMatchOperators) { + return filtersWithMatchOperators.map((prop) => { const config = FILTER_CONFIG[prop] let value = this.request.params.filters[prop] @@ -533,7 +530,7 @@ class ElasticQueryBuilder { const booleanOperator = 'should' if (Array.isArray(value) && value.length === 1) value = value.shift() - const clause = (Array.isArray(value)) ? { bool: { [booleanOperator]: value.map((value) => this.buildClause(value, config.field)) } } : this.buildClause(value, config.field) + const clause = (Array.isArray(value)) ? { bool: { [booleanOperator]: value.map((value) => this.buildFilterClause(value, config.field)) } } : this.buildFilterClause(value, config.field) return { path: config.path, clause } }) @@ -553,10 +550,10 @@ class ElasticQueryBuilder { ) // Collect those filters that use a simple term match - const simpleMatchFilters = Object.keys(this.request.params.filters) + const filtersWithMatchOperators = Object.keys(this.request.params.filters) .filter((k) => FILTER_CONFIG[k].operator === 'match') - filterClausesWithPaths = filterClausesWithPaths.concat(this.buildSimpleMatchFilters(simpleMatchFilters)) + filterClausesWithPaths = filterClausesWithPaths.concat(this.buildMatchOperatorFilterQueries(filtersWithMatchOperators)) // Gather root (not nested) filters: let filterClauses = filterClausesWithPaths diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index 2f5ca138..d8b42125 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -4,9 +4,9 @@ const ElasticQueryBuilder = require('../lib/elasticsearch/elastic-query-builder' const ApiRequest = require('../lib/api-request') describe('ElasticQueryBuilder', () => { - describe('buildClause', () => { + describe('buildFilterClause', () => { it('can handle multiple fields', () => { - expect(ElasticQueryBuilder.prototype.buildClause('value', ['field', 'parallelField'])) + expect(ElasticQueryBuilder.prototype.buildFilterClause('value', ['field', 'parallelField'])) .to.deep.equal({ bool: { @@ -16,34 +16,23 @@ describe('ElasticQueryBuilder', () => { } }) }) - it('can handle packed fields', () => { - expect(ElasticQueryBuilder.prototype.buildClause('not packed value', ['field_packed'])) - .to.deep.equal({ - bool: { - should: [ - { term: { 'field.id': 'not packed value' } }, - { term: { 'field.label': 'not packed value' } } - ] - } - }) - }) it('can handle the simple case', () => { - expect(ElasticQueryBuilder.prototype.buildClause('value', ['field'])) + expect(ElasticQueryBuilder.prototype.buildFilterClause('value', ['field'])) .to.deep.equal({ term: { field: 'value' } }) }) }) - describe('buildSimpleMatchFilters', () => { + describe('buildMatchOperatorFilterQueries', () => { const mockQueryBuilderFactory = (request) => ({ request, buildMultiFieldClause: ElasticQueryBuilder.prototype.buildMultiFieldClause, - buildSimpleMatchFilters: ElasticQueryBuilder.prototype.buildSimpleMatchFilters, - buildClause: ElasticQueryBuilder.prototype.buildClause, + buildMatchOperatorFilterQueries: ElasticQueryBuilder.prototype.buildMatchOperatorFilterQueries, + buildFilterClause: ElasticQueryBuilder.prototype.buildFilterClause, buildPackedFieldClause: ElasticQueryBuilder.prototype.buildPackedFieldClause }) it('can handle (multiple) single value, single match field filters, as arrays', () => { const request = new ApiRequest({ filters: { buildingLocation: ['toast'], subjectLiteral: ['spaghetti'] } }) const mockQueryBuilder = mockQueryBuilderFactory(request) - const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'subjectLiteral']) + const simpleMatchFilters = mockQueryBuilder.buildMatchOperatorFilterQueries(['buildingLocation', 'subjectLiteral']) expect(simpleMatchFilters).to.deep.equal([ { path: undefined, @@ -58,7 +47,7 @@ describe('ElasticQueryBuilder', () => { it('can handle (multiple) single value, single match field filters, strings', () => { const request = new ApiRequest({ filters: { buildingLocation: 'toast', subjectLiteral: 'spaghetti' } }) const mockQueryBuilder = mockQueryBuilderFactory(request) - const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['buildingLocation', 'subjectLiteral']) + const simpleMatchFilters = mockQueryBuilder.buildMatchOperatorFilterQueries(['buildingLocation', 'subjectLiteral']) expect(simpleMatchFilters).to.deep.equal([ { path: undefined, @@ -73,7 +62,7 @@ describe('ElasticQueryBuilder', () => { it('can handle multiple values', () => { const request = new ApiRequest({ filters: { subjectLiteral: ['spaghetti', 'meatballs'] } }) const mockQueryBuilder = mockQueryBuilderFactory(request) - const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['subjectLiteral']) + const simpleMatchFilters = mockQueryBuilder.buildMatchOperatorFilterQueries(['subjectLiteral']) expect(simpleMatchFilters).to.deep.equal([ { path: undefined, @@ -91,7 +80,7 @@ describe('ElasticQueryBuilder', () => { it('can handle packed values', () => { const request = new ApiRequest({ filters: { language: ['spanish', 'finnish'] } }) const mockQueryBuilder = mockQueryBuilderFactory(request) - const simpleMatchFilters = mockQueryBuilder.buildSimpleMatchFilters(['language']) + const simpleMatchFilters = mockQueryBuilder.buildMatchOperatorFilterQueries(['language']) expect(simpleMatchFilters).to.deep.equal([ { path: undefined, From c3d0b33e4fe70e40beadf9cf71e3857181d34b34 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Tue, 10 Dec 2024 13:33:52 -0500 Subject: [PATCH 8/9] add field and remove method --- lib/elasticsearch/config.js | 2 +- lib/elasticsearch/elastic-query-builder.js | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index 84314ac2..41b48a56 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -84,7 +84,7 @@ const FILTER_CONFIG = { mediaType: { operator: 'match', field: ['mediaType.id', 'mediaType.label'], repeatable: true }, carrierType: { operator: 'match', field: ['carrierType.id', 'carrierType.label'], repeatable: true }, publisher: { operator: 'match', field: ['publisherLiteral.raw'], repeatable: true }, - contributorLiteral: { operator: 'match', field: ['contributorLiteral.raw'], repeatable: true }, + contributorLiteral: { operator: 'match', field: ['contributorLiteral.raw', 'parallelContributor.raw'], repeatable: true }, creatorLiteral: { operator: 'match', field: ['creatorLiteral.raw', 'parallelCreatorLiteral.raw'], repeatable: true }, issuance: { operator: 'match', field: ['issuance.id', 'issuance.label'], repeatable: true }, createdYear: { operator: 'match', field: ['createdYear'], repeatable: true }, diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index e1788a46..84615112 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -488,20 +488,6 @@ class ElasticQueryBuilder { } } - buildPackedFieldClause (value, field) { - // Figure out the base property (e.g. 'owner') - const baseField = field.replace(/_packed$/, '') - // Allow supplied val to match against either id or value: - return { - bool: { - should: [ - { term: { [`${baseField}.id`]: value } }, - { term: { [`${baseField}.label`]: value } } - ] - } - } - } - buildMultiFieldClause (value, fields) { return { bool: From cab620e36b1aeed7fdc64d7b763f606bc07660a3 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Tue, 10 Dec 2024 14:50:47 -0500 Subject: [PATCH 9/9] rm mockmethod; --- test/elastic-query-builder.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index d8b42125..7c94654a 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -26,8 +26,7 @@ describe('ElasticQueryBuilder', () => { request, buildMultiFieldClause: ElasticQueryBuilder.prototype.buildMultiFieldClause, buildMatchOperatorFilterQueries: ElasticQueryBuilder.prototype.buildMatchOperatorFilterQueries, - buildFilterClause: ElasticQueryBuilder.prototype.buildFilterClause, - buildPackedFieldClause: ElasticQueryBuilder.prototype.buildPackedFieldClause + buildFilterClause: ElasticQueryBuilder.prototype.buildFilterClause }) it('can handle (multiple) single value, single match field filters, as arrays', () => { const request = new ApiRequest({ filters: { buildingLocation: ['toast'], subjectLiteral: ['spaghetti'] } })