Skip to content
Open
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
104 commits
Select commit Hold shift + click to select a range
b1b4547
Merge pull request #491 from NYPL/main
charmingduchess Jun 2, 2025
7685722
Override agg-self-filtering behavior for open search
nonword Jul 10, 2025
472331e
Merge branch 'NOREF-filter-self-filtering-fix' into qa2
nonword Jul 10, 2025
579eec5
Fix query structure
danamansana Dec 1, 2025
ae28b72
Add more permissive key structure
danamansana Dec 4, 2025
8b6000f
Remove console logs and commented code
danamansana Dec 4, 2025
e5b61dc
Fix linter errors
danamansana Dec 4, 2025
b85f3fb
Move esRangeValue
danamansana Dec 9, 2025
a02a058
Move parseParams
danamansana Dec 9, 2025
0beec4c
Refactor nyplSourc/id calculation and move nyplSourceAndId to utils'
danamansana Dec 9, 2025
a749878
Add bodybuilder methods for findByUri
danamansana Dec 9, 2025
683445f
Add nyplSourceAndId call to annotatedMarc
danamansana Dec 9, 2025
5ac57ee
Move itemsByFilter to utils
danamansana Dec 9, 2025
d50ea45
Move buildElasticQuery/Body to bodybuilder
danamansana Dec 9, 2025
2f6c887
Factor out body for search
danamansana Dec 9, 2025
1075d65
Move buildElasticAggregationsBody
danamansana Dec 9, 2025
a0237b7
Move aggregationQueriesForParams to bodybuilder
danamansana Dec 9, 2025
3340f5e
Move mergeAggregationsResposes to utils
danamansana Dec 9, 2025
e072d93
Factor out body for aggregation
danamansana Dec 9, 2025
054d1e4
Move findByUri to async/await
danamansana Dec 9, 2025
2d60b15
Make annotatedMarc async
danamansana Dec 9, 2025
2ec818a
Make deliveryLocationsByBarcode async
danamansana Dec 9, 2025
c784247
Merge pull request #597 from NYPL/NOREF/bnf-test
danamansana Dec 10, 2025
ba7fcf5
Pull search from promise chain in search
danamansana Dec 11, 2025
9189c1b
Pull massaged response from promise chain in resources#search
danamansana Dec 11, 2025
3297b63
Pull ResourceResultsSerializer.serialize from promise chain in search
danamansana Dec 11, 2025
03d57dc
Remove nested promise
danamansana Dec 11, 2025
1c34469
Remove promise from search
danamansana Dec 11, 2025
6e8af8d
Factor out relevance report
danamansana Dec 11, 2025
cf7edcd
Make aggregation endpoint async
danamansana Dec 11, 2025
6ca7e33
Exclude parentheses in query term
danamansana Dec 18, 2025
39c0920
Make keyphrase/non_ws_key lowercase
danamansana Dec 18, 2025
26e0a15
Merge pull request #606 from NYPL/NOREF/bnf-exclude-query-term-parent…
danamansana Dec 18, 2025
6714e24
Change callNumber to callnumber to enable callnumber searches
danamansana Dec 18, 2025
9534528
Merge pull request #607 from NYPL/NOREF/bnf-exclude-query-term-parent…
danamansana Dec 18, 2025
cd4c24f
Add finding text by key for atomic queries
danamansana Jan 7, 2026
eefeb44
Reorganize addInnerHits
danamansana Jan 9, 2026
d26553b
Reorganize bodyForFindByUri except for innerHits
danamansana Jan 9, 2026
7429ad6
Remove addInnerHits from findByUri
danamansana Jan 9, 2026
b386ec1
Reorganize buildElasticBody
danamansana Jan 9, 2026
36d56c5
Fix linting
danamansana Jan 13, 2026
d8249ac
Remove adding source to body in body for search
danamansana Jan 13, 2026
9353887
Add innerHits options
danamansana Jan 15, 2026
15a5e5c
Remove dependence on addInnerHits
danamansana Jan 15, 2026
b5530eb
Add tests for bodyForSearch; remove tests for addInnerHits
danamansana Jan 16, 2026
6cfbd49
Remove remaining references to addInnerHits
danamansana Jan 16, 2026
1c769c9
Add bodybuilder tests
danamansana Jan 16, 2026
c379ba5
Fix merge conflicts
danamansana Jan 20, 2026
a5e190e
Remove options from elastic-query
danamansana Jan 23, 2026
58a0d9f
Merge branch 'main' into scc-5168
danamansana Jan 29, 2026
8b3e4e1
Add initial bnf
danamansana Jan 30, 2026
87c1e32
Update packages
danamansana Jan 30, 2026
a55444d
Add alternate grammars and comment for atomic
danamansana Jan 30, 2026
05dfb3f
Add reverseGrammar and related methods
danamansana Feb 5, 2026
f373383
Apparently working left associating cql
danamansana Feb 5, 2026
951a8e8
Clean up grammar file
danamansana Feb 6, 2026
3a03d81
Use parseWithRightCql in query builder and tests
danamansana Feb 6, 2026
1952828
Remove console log and commented code'
danamansana Feb 6, 2026
7d1612e
Merge pull request #638 from NYPL/main
charmingduchess Feb 6, 2026
5d3bf7d
fix gha dep
charmingduchess Feb 6, 2026
b2a79b8
Merge branch 'SC-5164/delivery-inegration-test' into qa2
charmingduchess Feb 6, 2026
ff32619
gha
charmingduchess Feb 6, 2026
ffe1b30
update gha invocation
charmingduchess Feb 9, 2026
c389071
config aws in integration test
charmingduchess Feb 9, 2026
3a83857
Fix some param passing and start adding query tests
danamansana Feb 9, 2026
d2109f5
Add tests for atomic queries and some small corrections
danamansana Feb 10, 2026
baf803c
Add initial boolean tests
danamansana Feb 10, 2026
8842a88
Add tests for negation
danamansana Feb 11, 2026
fd8ff37
Fix linting/tests/small errors
danamansana Feb 11, 2026
271c692
Add date queries
danamansana Feb 19, 2026
ffe585f
Add filters to cql query builder
danamansana Feb 19, 2026
5990165
Remove irrelevant code
danamansana Feb 19, 2026
bd098d5
Merge branch 'main' into scc-5050-2
danamansana Feb 19, 2026
020c75d
Remove spurious merge code
danamansana Feb 19, 2026
7aec0dd
Merge pull request #649 from NYPL/merge-test
danamansana Feb 19, 2026
1473e3d
Fix some small errors
danamansana Feb 19, 2026
ed4c14a
Add initial filter implementation for cql
danamansana Feb 20, 2026
74f97a4
Fix tests
danamansana Feb 20, 2026
d5cee7f
Add date and filter features to cql
danamansana Feb 20, 2026
e2e43d0
Add some more useful display of parsing and errors
danamansana Feb 20, 2026
93c042e
Fix linting
danamansana Feb 20, 2026
9c98372
Merge pull request #650 from NYPL/scc-5203
danamansana Feb 27, 2026
0ebe3bd
Add new strategy for handling keyword vs text fields
danamansana Mar 2, 2026
b2ebcab
Add exact match query
danamansana Mar 2, 2026
15ddf08
Merge branch 'NOREF/nyql-for-qa2' into merge-nyql
danamansana Mar 10, 2026
d99276f
Merge pull request #665 from NYPL/qa2-nyql
danamansana Mar 10, 2026
f9a0250
Merge main
danamansana Mar 10, 2026
691b974
Fix deploy yaml
danamansana Mar 10, 2026
fd33073
Merge pull request #666 from NYPL/qa2-nyql
danamansana Mar 10, 2026
36e6302
Fixes in response to PR comments
danamansana Mar 13, 2026
a7d78e7
Merge branch 'main' into scc-5050-2
yossariano Mar 17, 2026
d5ec597
Add check for whether query has fields before adding
danamansana Mar 19, 2026
4f26b20
Remove console log
danamansana Mar 19, 2026
94d0d61
Fix tests
danamansana Mar 19, 2026
ca8a4d1
Merge pull request #686 from NYPL/scc-5168
danamansana Mar 19, 2026
e0e8f6b
Add reversing strings in nested array
danamansana Mar 19, 2026
34a399e
Merge pull request #687 from NYPL/scc-5168
danamansana Mar 19, 2026
d65a3d3
Remove double reversing in display
danamansana Mar 19, 2026
753ca86
Merge branch 'scc-5168' into qa2
danamansana Mar 19, 2026
330e170
Fix double nesting of should array for dates
danamansana Mar 20, 2026
b0f7585
Fix date test fixtures
danamansana Mar 20, 2026
d4c655a
Merge branch 'scc-5168' into qa2
danamansana Mar 20, 2026
14d7134
Merge branch 'qa2' into scc-5050-2
danamansana Mar 26, 2026
c59f33e
fix merge
yossariano Mar 27, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion lib/elasticsearch/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,47 @@ const AGGREGATIONS_SPEC = {
collection: { terms: { field: 'collectionIds' } }
}

const ITEM_FILTER_AGGREGATIONS = {
item_location: { nested: { path: 'items' }, aggs: { _nested: { terms: { size: 100, field: 'items.holdingLocation_packed' } } } },
item_status: { nested: { path: 'items' }, aggs: { _nested: { terms: { size: 100, field: 'items.status_packed' } } } },
item_format: { nested: { path: 'items' }, aggs: { _nested: { terms: { size: 100, field: 'items.formatLiteral' } } } }
}

// Configure sort fields:
const SORT_FIELDS = {
title: {
initialDirection: 'asc',
field: 'title_sort'
},
date: {
initialDirection: 'desc',
field: 'dateStartYear'
},
creator: {
initialDirection: 'asc',
field: 'creator_sort'
},
relevance: {}
}

// The following fields can be excluded from ES responses because we don't pass them to client:
const EXCLUDE_FIELDS = [
'uris',
'*_packed',
'*_sort',
'items.*_packed',
'contentsTitle',
'suppressed',
// Hide contributor and creator transformed fields:
'*WithoutDates',
'*Normalized'
]

module.exports = {
SEARCH_SCOPES,
FILTER_CONFIG,
AGGREGATIONS_SPEC
AGGREGATIONS_SPEC,
ITEM_FILTER_AGGREGATIONS,
EXCLUDE_FIELDS,
SORT_FIELDS
}
203 changes: 203 additions & 0 deletions lib/elasticsearch/elastic-body-builder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
const { EXCLUDE_FIELDS, ITEM_FILTER_AGGREGATIONS, SORT_FIELDS, AGGREGATIONS_SPEC } = require('./config')
const { innerHits, itemsQueryContext, itemsFilterContext } = require('./elastic-query-filter-builder')
const ApiRequest = require('../api-request')
const ElasticQueryBuilder = require('../elasticsearch/elastic-query-builder')

const bodyForFindByUri = function (recapBarcodesByStatus, params) {
const paramsIncludesItemLevelFiltering = Object.keys(params)
.filter((param) => param.startsWith('item_')).length > 0

const returnAllItems = params.all_items && !paramsIncludesItemLevelFiltering

const excludes = returnAllItems ? EXCLUDE_FIELDS.filter((field) => field !== '*_sort') : EXCLUDE_FIELDS.concat(['items'])

const aggregations = params.include_item_aggregations
? { aggregations: ITEM_FILTER_AGGREGATIONS }
: {}

const itemsOptions = {
size: params.items_size,
from: params.items_from,
merge_checkin_card_items: params.merge_checkin_card_items,
query: {
volume: params.item_volume,
date: params.item_date,
format: params.item_format,
location: params.item_location,
status: params.item_status,
itemUri: params.itemUri
},
unavailable_recap_barcodes: recapBarcodesByStatus['Not Available']
}

// const filter = returnAllItems ? {} : { filter: [] }
Comment thread
nonword marked this conversation as resolved.
Outdated

const queryFilter = { filter: !returnAllItems ? [innerHits(itemsOptions)] : [] }

// Establish base query:
Comment thread
nonword marked this conversation as resolved.
Outdated
const body = {
_source: {
excludes
},
size: 1,
query: {
bool: {
must: [
{
term: {
uri: params.uri
}
}
],
...queryFilter
}
},
...aggregations
}

return body
}

/**
* Given GET params, returns a plainobject suitable for use in a ES query.
*
* @param {object} params - A hash of request params including `filters`,
* `search_scope`, `q`
*
* @return {object} ES query object suitable to be POST'd to ES endpoint
*/
const buildElasticQuery = function (params, options = {}) {
const request = ApiRequest.fromParams(params)

const builder = ElasticQueryBuilder.forApiRequest(request, options)
return builder.query.toJson()
}

/**
* Given GET params, returns a plainobject with `from`, `size`, `query`,
* `sort`, and any other params necessary to perform the ES query based
* on the GET params.
*
* @return {object} An object that can be posted directly to ES
*/
const buildElasticBody = function (params, options = {}) {
// Apply sort:
let direction
let field

if (params.sort === 'relevance') {
field = '_score'
direction = 'desc'
} else {
field = SORT_FIELDS[params.sort].field || params.sort
direction = params.sort_direction || SORT_FIELDS[params.sort].initialDirection
}

const from = params.per_page && params.page ? { from: params.per_page * (params.page - 1) } : {}
const size = params.per_page ? { size: params.per_page } : {}

return {
...from,
...size,
query: buildElasticQuery(params, options),
sort: [{ [field]: direction }, { uri: 'asc' }]
}
Comment thread
danamansana marked this conversation as resolved.
}

const bodyForSearch = function (params) {
const itemsOptions = { merge_checkin_card_items: params.merge_checkin_card_items }

const body = Object.assign(
buildElasticBody(params, { items: itemsOptions }),
{
_source: {
excludes: EXCLUDE_FIELDS.concat(['items'])
}
}
)

// return withInnerHits
Comment thread
nonword marked this conversation as resolved.
Outdated
return body
}

const buildElasticAggregationsBody = (params, aggregateProps) => {
// Add an `aggregations` entry to the ES body describing the aggretations
// we want. Set the `size` property to per_page (default 50) for each.
// https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-terms-aggregation.html#search-aggregations-bucket-terms-aggregation-size
const aggregations = aggregateProps.reduce((aggs, prop) => {
aggs[prop] = AGGREGATIONS_SPEC[prop]
// Only set size for terms aggs for now:
if (aggs[prop].terms) {
aggs[prop].terms.size = params.per_page
}
return aggs
}, {})

return Object.assign(
buildElasticBody(params),
{ size: 0, aggregations }
)
}

/**
* Given a params hash, returns an array of ES queries for fetching relevant aggregations.
*/
const aggregationQueriesForParams = (params) => {
// Build the complete set of distinct aggregation queries we need to run
// depending on active filters. We want:
// - one agg representing the counts for all properties _not_ used in filter
// - one agg each for each property that is used in a filter, but counts should exclude that filter

// Build the standard aggregation:
const unfilteredAggregationProps = Object.keys(AGGREGATIONS_SPEC)
// Aggregate on all properties that aren't involved in filters:
.filter((prop) => !Object.keys(params.filters || {}).includes(prop))
const queries = [buildElasticAggregationsBody(params, unfilteredAggregationProps)]

// Now append all property-specific aggregation queries (one for each
// distinct property used in a filter):
return queries.concat(
Object.entries(params.filters || {})
// Only consider filters that are also aggregations:
.filter(([prop, values]) => Object.keys(AGGREGATIONS_SPEC).includes(prop))
.map(([prop, values]) => {
const aggFilters = structuredClone(params.filters)
// For this aggregation, don't filter on namesake property:
delete aggFilters[prop]

// Build query for single aggregation:
const modifiedParams = Object.assign({}, params, { filters: aggFilters })
return buildElasticAggregationsBody(modifiedParams, [prop])
})
)
}

const bodyForAggregation = (params) => {
const aggregations = {}
aggregations[params.field] = AGGREGATIONS_SPEC[params.field]

// If it's a terms agg, we can apply per_page:
if (aggregations[params.field].terms) {
aggregations[params.field].terms.size = params.per_page
}

return Object.assign(
buildElasticBody(params),
{
size: 0,
aggregations
}
)
}

module.exports = {
bodyForFindByUri,
itemsFilterContext,
itemsQueryContext,
buildElasticQuery,
buildElasticBody,
bodyForSearch,
buildElasticAggregationsBody,
aggregationQueriesForParams,
bodyForAggregation
}
11 changes: 8 additions & 3 deletions lib/elasticsearch/elastic-query-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const ElasticQuery = require('./elastic-query')
const ApiRequest = require('../api-request')
const { escapeQuery, namedQuery, prefixMatch, termMatch, phraseMatch } = require('./utils')
const { regexEscape } = require('../util')
const { innerHits } = require('./elastic-query-filter-builder')

const { FILTER_CONFIG, SEARCH_SCOPES } = require('./config')

Expand All @@ -11,7 +12,7 @@ const POPULARITY_BOOSTS = [
]

class ElasticQueryBuilder {
constructor (apiRequest) {
constructor (apiRequest, options = {}) {
this.request = apiRequest
this.query = new ElasticQuery()

Expand Down Expand Up @@ -44,6 +45,10 @@ class ElasticQueryBuilder {
// Add user filters:
this.applyFilters()

if (options.items) {

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 there is some redundancy here around the items. If the items flag is implemented as a filter, it feels like it should be handled by this.applyFilters(). I'm curious the reason why you implemented the items param as a totally separate option as well. Why not keep it in the params? It doesn't much logic is saved by moving a boolean from one object to another, though I do see that it's a different kind of param. I'm not sure that it's worth adding an extra argument to multiple constructors.

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.

The applyFilters function is intended to be the place where we handle "user" filters. If you're tracking down an issue with user ?filter[...], that should be the obvious place to look. Although we implement inner_hits as a filter, I'd argue that's incidental and not obvious to someone chasing down how we paginate items, so I support keeping it as a top level concern for visibility.

The point that we probably don't need an additional options param here just to communicate whether or not to apply the inner_hits block is good. It looks like options.items is a plainobject built upstream to translate item-specific params into a "item options" hash. We may want to move that parsing into the ApiRequest object so that this ElasticQueryBuilder object can determine how to build the inner_hits from that object, which it already has. I think then the only outstanding question is how to convey whether to insert the inner_hits block. By checking for the presence of this options.items object as written, we're essentially enabling the inner_hits block any time we've decided to build it upstream, which happens to be any time we're submitting a query for 1+ bib (i.e. any time we're not just fetching aggregations). I don't think we can determine from the query params alone whether or not we should insert the inner_hits block. One option: just insert the inner_hits block all the time (because there's never a reason not to use it when we're fetching 1+ bibs and in size:0 cases it likely has no effect).

Also agree we can start scrubbing support for overriding the default setting to merge_inner_hits for simplicity, although that's also probably not something to take on here.

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 had a long conversation with Vera offline about this, we agreed to remove the options.items logic in ElasticQuery, leaving it for now in ElasticQueryBuilder. It is not clear to me yet what the right approach should be. I think it is expedient for now to leave it in and hopefully the question will become clearer as the refactor progresses. Open to discuss further if you disagree with that approach.

this.query.addFilter(innerHits(options.items))
}

// Apply global clauses:
// Hide specific nypl-sources when configured to do so:
this.applyHiddenNyplSources()
Expand Down Expand Up @@ -706,8 +711,8 @@ class ElasticQueryBuilder {
/**
* Create a ElasticQueryBuilder for given ApiRequest instance
*/
static forApiRequest (request) {
return new ElasticQueryBuilder(request)
static forApiRequest (request, options = {}) {
return new ElasticQueryBuilder(request, options)
}
}

Expand Down
Loading