diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index 0efe1a3..79d7524 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -1,3 +1,5 @@ +const util = require('../util') + // Configure search scopes: const SEARCH_SCOPES = { all: { @@ -74,20 +76,32 @@ 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.id', 'items.owner.label'], repeatable: true, path: 'items' }, + subjectLiteral: { + transform: (val, logger) => { + if (typeof val === 'string') { + return util.removeTrailingPeriod(val, logger) + } + if (Array.isArray(val)) { + return val.map((val) => util.removeTrailingPeriod(val, logger)) + } + }, + operator: 'match', + field: ['subjectLiteral_exploded'], + repeatable: true + }, + 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.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', '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 }, dateAfter: { operator: 'custom', type: 'int' diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index db3a669..8461511 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -488,6 +488,40 @@ class ElasticQueryBuilder { } } + buildMultiFieldClause (value, fields) { + return { + bool: + { should: fields.map(field => ({ term: { [field]: value } })) } + } + } + + // This builds a filter cause from the value: + buildFilterClause (value, fieldsToMatchOn) { + const filterMatchesOnMoreThanOneField = fieldsToMatchOn.length > 1 + if (filterMatchesOnMoreThanOneField) { + return this.buildMultiFieldClause(value, fieldsToMatchOn) + } else { + const field = fieldsToMatchOn[0] + return { term: { [field]: value } } + } + } + + buildMatchOperatorFilterQueries (filtersWithMatchOperators) { + return filtersWithMatchOperators.map((prop) => { + const config = FILTER_CONFIG[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' + + if (Array.isArray(value) && value.length === 1) value = value.shift() + 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 } + }) + } + /** * Examine request for user-filters. When found, add them to query. */ @@ -502,41 +536,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(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 } } - } - - // 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) - - return { path: config.path, clause } - })) + filterClausesWithPaths = filterClausesWithPaths.concat(this.buildMatchOperatorFilterQueries(filtersWithMatchOperators)) // Gather root (not nested) filters: let filterClauses = filterClausesWithPaths diff --git a/lib/elasticsearch/elastic-query-filter-builder.js b/lib/elasticsearch/elastic-query-filter-builder.js new file mode 100644 index 0000000..e69de29 diff --git a/lib/util.js b/lib/util.js index b559ad8..636d95b 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1,6 +1,13 @@ const logger = require('./logger') const { isItemNyplOwned } = require('./ownership_determination') +exports.removeTrailingPeriod = (x, logger) => { + if (x.slice(-1) === '.') { + logger.debug('Removing terminal period', JSON.stringify(x, null, 4)) + return x.slice(0, -1) + } else return x +} + exports.sortOnPropWithUndefinedLast = (property) => { return function (a, b) { // equal items sort equally @@ -120,22 +127,8 @@ exports.parseParams = function (params, spec) { // `fields`: Hash - When `type` is 'hash', this property provides field spec to validate internal fields against // `repeatable`: Boolean - If true, array of values may be returned. Otherwise will select last. Default false exports.parseParam = function (val, spec) { - if (spec.fields && - spec.fields.subjectLiteral && - spec.fields.subjectLiteral.field === 'subjectLiteral_exploded' && - val.subjectLiteral - ) { - if (typeof val.subjectLiteral === 'string' && val.subjectLiteral.slice(-1) === '.') { - val.subjectLiteral = val.subjectLiteral.slice(0, -1) - logger.debug('Removing terminal period', JSON.stringify(val, null, 4)) - } else if (Array.isArray(val.subjectLiteral)) { - val.subjectLiteral.forEach((sub, i) => { - if (sub.slice(-1) === '.') { - val.subjectLiteral[i] = sub.slice(0, -1) - logger.debug('Removing terminal period', JSON.stringify(sub, null, 4)) - } - }) - } + if (spec.transform) { + val = spec.transform(val, logger) } // Unless it's marked repeatable, convert arrays of values to just last value: diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index 8ed4bcd..7c94654 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -4,6 +4,111 @@ const ElasticQueryBuilder = require('../lib/elasticsearch/elastic-query-builder' const ApiRequest = require('../lib/api-request') describe('ElasticQueryBuilder', () => { + describe('buildFilterClause', () => { + it('can handle multiple fields', () => { + expect(ElasticQueryBuilder.prototype.buildFilterClause('value', ['field', 'parallelField'])) + .to.deep.equal({ + bool: + { + should: [ + { term: { field: 'value' } }, + { term: { parallelField: 'value' } }] + } + }) + }) + it('can handle the simple case', () => { + expect(ElasticQueryBuilder.prototype.buildFilterClause('value', ['field'])) + .to.deep.equal({ term: { field: 'value' } }) + }) + }) + describe('buildMatchOperatorFilterQueries', () => { + const mockQueryBuilderFactory = (request) => ({ + request, + buildMultiFieldClause: ElasticQueryBuilder.prototype.buildMultiFieldClause, + buildMatchOperatorFilterQueries: ElasticQueryBuilder.prototype.buildMatchOperatorFilterQueries, + 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'] } }) + const mockQueryBuilder = mockQueryBuilderFactory(request) + const simpleMatchFilters = mockQueryBuilder.buildMatchOperatorFilterQueries(['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.buildMatchOperatorFilterQueries(['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) + const simpleMatchFilters = mockQueryBuilder.buildMatchOperatorFilterQueries(['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 = mockQueryBuilderFactory(request) + const simpleMatchFilters = mockQueryBuilder.buildMatchOperatorFilterQueries(['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', () => { it('generates an "all" query', () => { const request = new ApiRequest({ q: 'toast' }) diff --git a/test/util.test.js b/test/util.test.js index bfdb244..15f340a 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -2,6 +2,7 @@ const { expect } = require('chai') const mangledEnumerationChronologyItems = require('./fixtures/mangled_enumerationChronology_items.json') const util = require('../lib/util') +const { FILTER_CONFIG } = require('../lib/elasticsearch/config') describe('Util', function () { describe('sortOnPropWithUndefinedLast', () => { @@ -64,19 +65,14 @@ describe('Util', function () { filters: { subjectLiteral: 'cats', contributorLiteral: ['Contrib 1', 'Contrib 2'], - date: '2012', + dateAfter: '2012', badNumeric: 'blah' } } const spec = { filters: { type: 'hash', - fields: { - subjectLiteral: { type: 'string' }, - contributorLiteral: { type: 'string' }, - date: { type: 'int' }, - badNumeric: { type: 'int' } - } + fields: { ...FILTER_CONFIG, badNumeric: { type: 'int' } } } } const outgoing = util.parseParams(incoming, spec) @@ -87,8 +83,8 @@ describe('Util', function () { expect(outgoing.filters.subjectLiteral).to.be.a('string') expect(outgoing.filters.subjectLiteral).to.equal('cats') - expect(outgoing.filters.date).to.be.a('number') - expect(outgoing.filters.date).to.equal(2012) + expect(outgoing.filters.dateAfter).to.be.a('number') + expect(outgoing.filters.dateAfter).to.equal(2012) expect(outgoing.filters.badNumeric).to.be.a('undefined') }) @@ -131,15 +127,7 @@ describe('Util', function () { } const spec = { - filters: { - type: 'hash', - fields: { - subjectLiteral: { - type: 'string', - field: 'subjectLiteral_exploded' - } - } - } + filters: { type: 'hash', fields: FILTER_CONFIG } } const outgoing = util.parseParams(incoming, spec) @@ -154,16 +142,7 @@ describe('Util', function () { } const spec = { - filters: { - type: 'hash', - fields: { - subjectLiteral: { - type: 'string', - field: 'subjectLiteral_exploded', - repeatable: true - } - } - } + filters: { type: 'hash', fields: FILTER_CONFIG } } const outgoing = util.parseParams(incoming, spec)