Skip to content
17 changes: 15 additions & 2 deletions lib/elasticsearch/cql/index-mapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,22 @@ const indexMapping = {
language: { term: ['language.id', 'language.label'] },
date: { fields: ['dates.range'] },
series: {
term: ['series', 'parallelSeries']
fields: [
'series.folded',
'parallelSeries.folded',
'parallelSeriesUniformTitle.folded',
'parallelSeriesAddedEntry.folded'
],
exact_fields: [
'series',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be updated to *.keywordLowercasedStripped once the new es index is popuated. i can add it to my ticket to update discovery API

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK will leave this for now so as not to break the existing searches but once ready we can update to *keywordLowercasedStripped. Is it all the exact_fields for series?

'parallelSeries',
'seriesUniformTitle',
'parallelSeriesUniformTitle',
'seriesAddedEntry',
'parallelSeriesAddedEntry'
]
},
genre: { fields: ['genreForm'], exact_fields: ['genreForm.raw'] },
genre: { fields: ['genreForm', 'genreForm.folded'], exact_fields: ['genreForm.raw'] },
center: { term: ['buildingLocationIds'] },
division: { term: ['collectionIds'] },
format: { term: ['formatId'] }
Expand Down
68 changes: 50 additions & 18 deletions lib/elasticsearch/cql_query_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,18 +341,27 @@ function matchTermWithFields (fields, term, type) {
}
}

function convertSingleDateToRange (date) {
const dateReg = /^(\d{4})(?:[-/](\d{2}))?(?:[-/](\d{2}))?$/
const match = date.match(dateReg)
let rangeEnd
function nextDate (dateString) {
const dateRegex = /^(\d{4})(?:[-/](\d{2}))?(?:[-/](\d{2}))?$/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this method is responsible for validating dates. is there a specific case here that you're trying to account for? maybe you can pass the string into new Date() without regex crunching?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the about all of the ES date range shenaningans, but i think some comments would help to describe the transformations you're making

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how to do this without some regex, because it isn't just about getting the date but detecting how specific the date that the user entered is (i.e. whether they specified day, month, or only year). I've added some comments to hopefully clarify

const match = dateString.match(dateRegex)

const year = parseInt(match[1], 10)
const month = match[2] ? parseInt(match[2], 10) - 1 : 0
const day = match[3] ? parseInt(match[3], 10) : 1
const d = new Date(Date.UTC(year, month, day))

if (match[3]) {
rangeEnd = { lte: `${match[1]}-${match[2]}-${match[3]}T23:59:59` }
} else if (match[2] && match[2] !== '12') {
rangeEnd = { lt: `${match[1]}-${parseInt(match[2], 10) + 1}` }
d.setUTCDate(d.getUTCDate() + 1)
} else if (match[2]) {
d.setUTCMonth(d.getUTCMonth() + 1)
} else {
rangeEnd = { lt: `${parseInt(match[1], 10) + 1}` }
d.setUTCFullYear(d.getUTCFullYear() + 1)
}
return Object.assign({ gte: date, relation: 'within' }, rangeEnd)
return d.toISOString().split('T')[0]
}

function convertSingleDateToRange (date) {
return { gte: date, relation: 'within', lt: nextDate(date) }
}

function rangeQueryForDates ({ relation, queryTerms }) {
Expand All @@ -362,19 +371,19 @@ function rangeQueryForDates ({ relation, queryTerms }) {
range = { lt: queryTerms[0] }
break
case '>':
range = { gt: queryTerms[0] }
range = { gte: nextDate(queryTerms[0]) }
break
case '>=':
range = { gte: queryTerms[0] }
break
case '<=':
range = { lte: queryTerms[0] }
range = { lt: nextDate(queryTerms[0]) }
break
case 'encloses':
range = { gt: queryTerms[0], lt: queryTerms[1] }
range = { gte: queryTerms[0], lte: queryTerms[0], relation: 'contains' }
break
case 'within':
range = { gte: queryTerms[0], lte: queryTerms[1] }
range = { gte: queryTerms[0], lt: nextDate(queryTerms[1]) }
break
default:
range = convertSingleDateToRange(queryTerms[0])
Expand Down Expand Up @@ -428,6 +437,21 @@ function dateQueries ({ fields, relation, terms, term }) {
}
}

if (relation === 'encloses') {
query = {
bool: {
must: [
query,
{
terms: {
'dates.tag': ['c', 'd', 'i', 'k', 'm', 'q', 'u']
}
}
]
}
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • These are the original marc subfield tags? Why restrict to these? Is it not possible to do a encloses match on an e tagged date - or any other non-null date for that matter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per Sean encloses should only match bibs with date range types


return {
nested: {
path: 'dates',
Expand All @@ -447,12 +471,20 @@ function prefixQuery (field, term) {
function multiMatch (fields, term, type) {
if (!fields || !fields.length) return []

const query = {
query: term,
fields,
type
}

if (term.includes('*')) {
return [{
query_string: query
}]
}

return [{
multi_match: {
query: term,
fields,
type
}
multi_match: query
}]
}

Expand Down
13 changes: 12 additions & 1 deletion lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,15 @@ class IndexSearchError extends Error {
}
}

module.exports = { InvalidParameterError, NotFoundError, IndexConnectionError, IndexSearchError }
/**
* Thrown when the user submits a query that cannot be parsed
*/
class InvalidQuerySyntaxError extends Error {
constructor (message) {
super()
this.name = 'InvalidQuerySyntaxError'
this.message = message
}
}

module.exports = { InvalidParameterError, NotFoundError, IndexConnectionError, IndexSearchError, InvalidQuerySyntaxError }
1 change: 1 addition & 0 deletions lib/handle-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const handleError = (error, req, res, next, logger) => {

switch (error.name) {
case 'InvalidParameterError':
case 'InvalidQuerySyntaxError':
statusCode = 422
if (logger) logger.warn(`${urlInfo}: ${error.message}`)
break
Expand Down
10 changes: 5 additions & 5 deletions lib/resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const LocationLabelUpdater = require('./location_label_updater')
const AnnotatedMarcSerializer = require('./annotated-marc-serializer')
const MarcSerializer = require('./marc-serializer')
const { makeNyplDataApiClient } = require('./data-api-client')
const { IndexSearchError, IndexConnectionError } = require('./errors')
const { IndexSearchError, IndexConnectionError, InvalidQuerySyntaxError } = require('./errors')

const ResponseMassager = require('./response_massager.js')
const AvailableDeliveryLocationTypes = require('./available_delivery_location_types')
Expand Down Expand Up @@ -657,13 +657,13 @@ module.exports = function (app, _private = null) {
try {
parsed = cqlQuery.displayParsed()
} catch (e) {
throw new IndexSearchError('Unknown parsing error. Error most likely near end of string')
throw new InvalidQuerySyntaxError('Unknown parsing error. Error most likely near end of string')
}
if (parsed.error) {
throw new IndexSearchError(parsed.error)
throw new InvalidQuerySyntaxError(parsed.error)
}
if (!parsed.parsed) {
throw new IndexSearchError('Unknown parsing error. Error most likely near end of string')
throw new InvalidQuerySyntaxError('Unknown parsing error. Error most likely near end of string')
}
}

Expand Down Expand Up @@ -711,7 +711,7 @@ module.exports = function (app, _private = null) {
})
.catch((e) => {
// Wrap ES client errors or any downstream error
if (e instanceof IndexSearchError || e instanceof IndexConnectionError) {
if (e instanceof IndexSearchError || e instanceof IndexConnectionError || e instanceof InvalidQuerySyntaxError) {
throw e // already a custom error
}
throw new IndexSearchError(`Error processing search: ${e.message || e}`)
Expand Down
2 changes: 1 addition & 1 deletion test/cql_query_builder.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe('CQL Query Builder', function () {
})

it('Date encloses query', function () {
expect(new CqlQuery('date encloses "1990 2000"').buildEsQuery())
expect(new CqlQuery('date encloses "1990"').buildEsQuery())
.to.deep.equal(
dateEnclosesQuery
)
Expand Down
45 changes: 28 additions & 17 deletions test/cql_query_builder_dates.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ describe('cql_query_builder date queries', () => {
should: [
{
range: {
'dates.range': { gte: '1999', relation: 'within', lt: '2000' }
'dates.range': { gte: '1999', relation: 'within', lt: '2000-01-01' }
}
},
{
range: {
'dates.range': { gte: '2000', relation: 'within', lt: '2001' }
'dates.range': { gte: '2000', relation: 'within', lt: '2001-01-01' }
}
}
]
Expand Down Expand Up @@ -68,12 +68,12 @@ describe('cql_query_builder date queries', () => {
must: [
{
range: {
'dates.range': { gte: '1999', relation: 'within', lt: '2000' }
'dates.range': { gte: '1999', relation: 'within', lt: '2000-01-01' }
}
},
{
range: {
'dates.range': { gte: '2000', relation: 'within', lt: '2001' }
'dates.range': { gte: '2000', relation: 'within', lt: '2001-01-01' }
}
}
]
Expand Down Expand Up @@ -105,7 +105,7 @@ describe('cql_query_builder date queries', () => {
path: 'dates',
query: {
range: {
'dates.range': { gte: '1990', lte: '2000' }
'dates.range': { gte: '1990', lt: '2001-01-01' }
}
}
}
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('cql_query_builder date queries', () => {
path: 'dates',
query: {
range: {
'dates.range': { gt: '1999' }
'dates.range': { gte: '2000-01-01' }
}
}
}
Expand Down Expand Up @@ -189,28 +189,39 @@ describe('cql_query_builder date queries', () => {
path: 'dates',
query: {
range: {
'dates.range': { lte: '1999' }
'dates.range': { lt: '2000-01-01' }
}
}
}
})
})

it('builds range query for relation "encloses" connecting two bounds', () => {
it('builds range query for relation "encloses"', () => {
const query = buildAtomicMain({
scope: 'date',
relation: 'encloses',
terms: ['1990', '2000'],
term: '1990 2000',
terms: ['1990'],
term: '1990',
fields: dateFields
})

expect(query).to.deep.equal({
nested: {
path: 'dates',
query: {
range: {
'dates.range': { gt: '1990', lt: '2000' }
bool: {
must: [
{
range: {
'dates.range': { gte: '1990', lte: '1990', relation: 'contains' }
}
},
{
terms: {
'dates.tag': ['c', 'd', 'i', 'k', 'm', 'q', 'u']
}
}
]
}
}
}
Expand All @@ -234,7 +245,7 @@ describe('cql_query_builder date queries', () => {
must: [
{
range: {
'dates.range': { gte: '1999', relation: 'within', lt: '2000' }
'dates.range': { gte: '1999', relation: 'within', lt: '2000-01-01' }
}
},
{
Expand Down Expand Up @@ -266,7 +277,7 @@ describe('cql_query_builder date queries', () => {
must: [
{
range: {
'dates.range': { gte: '1999-10', relation: 'within', lt: '1999-11' }
'dates.range': { gte: '1999-10', relation: 'within', lt: '1999-11-01' }
}
},
{
Expand Down Expand Up @@ -298,7 +309,7 @@ describe('cql_query_builder date queries', () => {
must: [
{
range: {
'dates.range': { gte: '1999-10-15', relation: 'within', lte: '1999-10-15T23:59:59' }
'dates.range': { gte: '1999-10-15', relation: 'within', lt: '1999-10-16' }
}
},
{
Expand Down Expand Up @@ -330,7 +341,7 @@ describe('cql_query_builder date queries', () => {
must: [
{
range: {
'dates.range': { gte: '1999-12', relation: 'within', lt: '2000' }
'dates.range': { gte: '1999-12', relation: 'within', lt: '2000-01-01' }
}
},
{
Expand Down Expand Up @@ -365,7 +376,7 @@ describe('cql_query_builder date queries', () => {
'dates.range': {
gte: '2023-04-30',
relation: 'within',
lte: '2023-04-30T23:59:59'
lt: '2023-05-01'
}
}
},
Expand Down
Loading
Loading