diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 7111f7e5..23ed4fe1 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -43,3 +43,5 @@ jobs: env: ENV: ${{env.ENV}} run: node test/integration/delivery-locations-by-barcode.test.js + - name: Run NYQL API tests + run: npm run nyql-api-test diff --git a/README.md b/README.md index 48bc2217..577a3033 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ To start the container with AWS creds so that the app can decrypt config from `c ``` After making changes, rebuild the image: + ``` docker-compose build ``` @@ -61,9 +62,10 @@ This app uses a [PRs Target Main, Merge to Deployment Branches](https://github.c ### Deploying App deploys through [GH Actions](./.github/workflows/test-and-deploy.yml) to ECS when updates are made to deployment branches: - - `qa`: discovery-api-qa.nypl.org - - `qa2`: discovery-api-qa2.nypl.org - - `production`: discovery-api-production.nypl.org + +- `qa`: discovery-api-qa.nypl.org +- `qa2`: discovery-api-qa2.nypl.org +- `production`: discovery-api-production.nypl.org ## Testing @@ -73,6 +75,22 @@ Run all tests: npm test ``` +### API testing + +NYQL integration tests are a QA contract suite in `test/integration/nyql/`. They verify that NYQL queries return the expected results against live QA data and are not run by default during `npm test` to avoid adding latency to CI. They do, however, run after deploys to QA and Production. The call to run them has been added to integration-tests.yml file. + +To run them locally: + +```bash +npm run nyql-api-test +``` + +To override the QA URL (e.g., to test against your local environment): + +```bash +NYQL_TEST_BASE_URL=http://localhost:3000/api/v0.1 npm run nyql-api-test +``` + ### Adding fixtures Almost all HTTP dependencies are rerouted to fixtures (except for requesting nypl-core mapping files). All fixtures can be updated dynamically (using creds in `./config/production.env`) via the following: @@ -113,7 +131,7 @@ For example, you could `q` alone without filters to retrieve records matching wa > /resources?q=war dateStartYear:[1999 TO 2012] -.. Or get things created in 1999 *or later*: +.. Or get things created in 1999 _or later_: > /resources?q=dateStartYear:>1999 @@ -131,7 +149,7 @@ Filters are applied using a `filters` param that expects this syntax on the quer Where `property*` is one of: 'owner', 'subjectLiteral', 'holdingLocation', 'deliveryLocation', 'language', 'materialType', 'mediaType', 'carrierType', 'publisher', 'contributor', 'creator', 'issuance', 'createdYear', 'dateAfter', or 'dateBefore'. -The value given should be *exact*. Do not use quotes. +The value given should be _exact_. Do not use quotes. For example, to filter by English language: @@ -165,16 +183,16 @@ Note that dateStartYear and dateEndYear are often very broad, causing the above All search queries support: - - `page`: Integer. Page number to retrieve. (Default 1) - - `per_page`: Integer. Number of results to retrieve at a time. Default 50. Valid range 0-100. +- `page`: Integer. Page number to retrieve. (Default 1) +- `per_page`: Integer. Number of results to retrieve at a time. Default 50. Valid range 0-100. ### Sorting All search queries support `sort`ing on: - - `title`: Case insensitive sort on title. Default ascending. - - `date`: Sort on dateStartYear. Default descending. - - `creator`: Case insensitive sort on first creator. (Note the "first" creator may not be the best creator.) Default ascending. +- `title`: Case insensitive sort on title. Default ascending. +- `date`: Sort on dateStartYear. Default descending. +- `creator`: Case insensitive sort on first creator. (Note the "first" creator may not be the best creator.) Default ascending. To set a non-default direction use `sort_direction=(asc|desc)`. To sort by relevance (i.e. keyword query), omit the `sort` param. @@ -210,7 +228,6 @@ Note that `page=` is not supported for aggregations (ES doesn't seem to offer a There is currently one feature flag in this app, which is 'no-on-site-edd'. When it is set, all onsite items have an eddRequestable property of false. -NB: numAvailable and numItem*Parsed counts do not **exclude** the e-item, but these items are not indexed with statuses, volumes, or date ranges, and are therefore not actually included in this count. +NB: numAvailable and numItem\*Parsed counts do not **exclude** the e-item, but these items are not indexed with statuses, volumes, or date ranges, and are therefore not actually included in this count. NB: As the table above indicates, there is a mismatch between what the front end and API regard as "electronic items". As far as the API is concerned, there is only at most ONE electronic item, which can have many electronic locator values. `numElectronicResources` counts these locator values, but the other item count values treat all the electronic resources as a single item. - diff --git a/data/annotated-marc-rules.json b/data/annotated-marc-rules.json index 26a5eaff..2271bfc2 100644 --- a/data/annotated-marc-rules.json +++ b/data/annotated-marc-rules.json @@ -625,7 +625,7 @@ "marcIndicatorRegExp": "^340", "subfieldSpec": { "subfields": [ - "6", "2" + "6" ], "directive": "exclude" }, @@ -724,7 +724,6 @@ "marcIndicatorRegExp": "^386", "subfieldSpec": { "subfields": [ - "n", "2", "6" ], @@ -2860,4 +2859,4 @@ "label": "Research Call Number", "directive": "include" } -] +] \ No newline at end of file diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index 660af724..7c39c1de 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -163,8 +163,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 } diff --git a/lib/elasticsearch/elastic-body-builder.js b/lib/elasticsearch/elastic-body-builder.js new file mode 100644 index 00000000..8dbec5a1 --- /dev/null +++ b/lib/elasticsearch/elastic-body-builder.js @@ -0,0 +1,219 @@ +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 { CqlQuery } = require('./cql_query_builder') + +const { InvalidQuerySyntaxError } = require('../errors') + +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 queryFilter = { filter: !returnAllItems ? [innerHits(itemsOptions)] : [] } + + 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) + + if (params.search_scope === 'cql') { + const cqlQuery = new CqlQuery(params.q) + let parsed + try { + parsed = cqlQuery.displayParsed() + } catch (e) { + throw new InvalidQuerySyntaxError('Unknown parsing error. Error most likely near end of string') + } + if (parsed.error) { + throw new InvalidQuerySyntaxError(parsed.error) + } + if (!parsed.parsed) { + throw new InvalidQuerySyntaxError('Unknown parsing error. Error most likely near end of string') + } + return cqlQuery.buildEsQuery(request) + } + + 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' }] + } +} + +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 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 +} diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index 3d5c7639..85d8292d 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -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') @@ -11,7 +12,7 @@ const POPULARITY_BOOSTS = [ ] class ElasticQueryBuilder { - constructor (apiRequest) { + constructor (apiRequest, options = {}) { this.request = apiRequest this.query = new ElasticQuery() @@ -52,6 +53,9 @@ class ElasticQueryBuilder { // Add user filters: this.applyFilters() + if (options.items) { + this.query.addFilter(innerHits(options.items)) + } // if a list of ids is specified, return those ids this.applyMultipleIdMatch() @@ -781,8 +785,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) } } diff --git a/lib/elasticsearch/elastic-query-filter-builder.js b/lib/elasticsearch/elastic-query-filter-builder.js index e69de29b..5149cde8 100644 --- a/lib/elasticsearch/elastic-query-filter-builder.js +++ b/lib/elasticsearch/elastic-query-filter-builder.js @@ -0,0 +1,253 @@ +const { esRangeValue } = require('../utils/resource-helpers') + +/** + * Given an object containing filters, + * returns content of the ES query filter context + * + * @param {object} options - An object with keys,value pairs of the form [filter_name]:[filter_value] + * @returns {object} + */ +const itemsFilterContext = (options) => { + if (!options.query) return {} + + const filterHandlers = { + volume: (volumes) => { + return { + range: { + 'items.volumeRange': esRangeValue(volumes) + } + } + }, + date: (dates) => { + return { + range: { + 'items.dateRange': esRangeValue(dates) + } + } + }, + format: (formats) => { + return { + terms: { + 'items.formatLiteral': formats + } + } + }, + location: (locations) => { + return { + terms: { + 'items.holdingLocation.id': locations + } + } + }, + status: (statuses) => { + // Determine if all possible ReCAP statuses were selected: + const selectedRecapStatuses = recapStatuses(statuses) + + if (selectedRecapStatuses.length === 1 && + Array.isArray(options.unavailable_recap_barcodes) && + options.unavailable_recap_barcodes.length > 0) { + // There are known unavailable ReCAP items, so build a complicated + // filter clause with appropriate barcode overrides: + return itemStatusFilterWithUnavailableRecapItems(statuses, options.unavailable_recap_barcodes) + } else { + // If there are no known unavailable ReCAP items, just do a straight + // status match: + return { + terms: { + 'items.status.id': statuses + } + } + } + }, + itemUri: (uri) => { + return { term: { 'items.uri': uri } } + } + } + + const filters = Object.keys(options.query).map((filter) => { + const value = options.query[filter] + const handler = filterHandlers[filter] + return value && handler ? handler(value) : null + }).filter((x) => x) + + return filters.length + ? { filter: filters } + : {} +} + +/** + * Given an array of status ids (e.g. "status:a", "status:na") returns the + * subset of statuses that are relevant in ReCAP + */ +const recapStatuses = (statuses) => { + return statuses + .filter((status) => ['status:a', 'status:na'].includes(status)) +} + +/** + * Builds a big complicated ES filter to allow us to filter items by status, + * but override the indexed status for ReCAP items with statuses retrieved + * from SCSB. This corrects for the fact that ReCAP item statuses tend to be + * wrong in the ES index: + * - partner items are indexed as Available and remain thus forever + * - NYPL item statuses _should_ equal SCSB status, but the mechanism + * for keeping them synced isn't perfect and operates on a delay + * + * @param {string[]} statuses - An array of statuses to filter on + * @param {string[]} unavailableRecapBarcodes - An array of item barcodes + * known to be unavailble + * + * Returns an ES filter that matches the desired statuses, but also uses + * the known unavailable items to override indexed item statuses for ReCAP + * items (because ReCAP is the authority for status of off-site items). + * Essentially, the criteria is for matching an item is: + * + * - if on-site (non-ReCAP): + * - has a matching indexed status + * - if off-site: + * - if filtering on status:na + * - item barcode must be in unavailableRecapBarcodes + * - if filtering on status:a: + * - item barcode must NOT be in unavailableRecapBarcodes + */ +const itemStatusFilterWithUnavailableRecapItems = (statuses, unavailableRecapBarcodes) => { + // First, let's set up some common clauses: + + // Item is in ReCAP: + const itemIsRecapClause = { + regexp: { 'items.holdingLocation.id': 'loc:rc.*' } + } + // Item's indexed status matches one of the filtered statuses: + const itemHasIndexedStatusClause = { + terms: { 'items.status.id': statuses } + } + // Item is marked Unavailable in SCSB: + const itemIsUnavailableInRecapClause = { + script: { + script: { + inline: 'doc[\'items.idBarcode\'].value == null || ' + + 'params.unavailableRecapBarcodes.contains(doc[\'items.idBarcode\'][0])', + lang: 'painless', + params: { unavailableRecapBarcodes } + } + } + } + // This function is only called if `statuses` param contains a single + // ReCAP-relevant status (i.e. status:a or status:na), so determine which + // ReCAP status to use: + const selectedRecapStatus = recapStatuses(statuses).shift() + // Item's ReCAP status agrees with filter: + const itemRecapStatusAgreesWithFilterClause = + selectedRecapStatus === 'status:na' + ? itemIsUnavailableInRecapClause + : { bool: { must_not: itemIsUnavailableInRecapClause } } + + return { + bool: { + should: [ + // Either 1) item is on-site and has correctly indexed status: + { + bool: { + must: [ + // Item is on-site (i.e. not recap): + { bool: { must_not: itemIsRecapClause } }, + // Item indexed status matches filter: + itemHasIndexedStatusClause + ] + } + }, + // Or 2) item is off-site and has a scsb status that agrees with the + // filter (e.g. if filtering on status:na, scsb marks the barcode as + // 'Not Available') + { + bool: { + must: [ + // Item is off-site: + JSON.parse(JSON.stringify(itemIsRecapClause)), + // Item is not marked unavailable + itemRecapStatusAgreesWithFilterClause + ] + } + } + ] + } + } +} + +/** + * Given an object containing query options, + * returns content of the ES query context + * + * @param {object} options - An object with request options. `merge_checkin_card_items` is the only one + * that matters right now + * @returns {object} + */ +const itemsQueryContext = (options) => { + const excludeClauses = [] + + if (!options.merge_checkin_card_items) excludeClauses.push({ term: { 'items.type': 'nypl:CheckinCardItem' } }) + + return excludeClauses.length ? { must_not: excludeClauses } : { must: { match_all: {} } } +} + +const innerHits = (_options = {}) => { + const options = Object.assign({ + size: process.env.SEARCH_ITEMS_SIZE || 200, + from: 0, + merge_checkin_card_items: true + }, _options) + + // If there is any item query at all, run an additional inner_hits query + // to retrieve the total number of items without filtering: + const itemsQuery = { + bool: Object.assign( + itemsQueryContext(options), + itemsFilterContext(options) + ) + } + + const allItemsQuery = itemsQuery.bool.filter + ? [{ + nested: { + path: 'items', + query: { + bool: { + must_not: [{ exists: { field: 'items.electronicLocator' } }] + } + }, + inner_hits: { name: 'allItems' } + } + }] + : [] + + const wrappedItemsQuery = { + bool: { + should: [ + { + nested: { + path: 'items', + query: itemsQuery, + inner_hits: { + sort: [{ 'items.enumerationChronology_sort': 'desc' }], + size: options.size, + from: options.from, + name: 'items' + } + } + }, + // Add a catch-all to ensure we return the bib document even when + // numItems=0 or applied item filters exclude all items: + { match_all: {} }, + ...allItemsQuery + ] + } + } + + return wrappedItemsQuery +} + +module.exports = { + innerHits, + itemsQueryContext, + itemsFilterContext +} diff --git a/lib/resources.js b/lib/resources.js index f77d3f80..36b53f43 100644 --- a/lib/resources.js +++ b/lib/resources.js @@ -1,4 +1,3 @@ -const NyplSourceMapper = require('research-catalog-indexer/lib/utils/nypl-source-mapper') const scsbClient = require('./scsb-client') const ResourceResultsSerializer = require('./jsonld_serializers.js').ResourceResultsSerializer @@ -6,100 +5,38 @@ const ResourceSerializer = require('./jsonld_serializers.js').ResourceSerializer const AggregationsSerializer = require('./jsonld_serializers.js').AggregationsSerializer const AggregationSerializer = require('./jsonld_serializers.js').AggregationSerializer const ItemResultsSerializer = require('./jsonld_serializers.js').ItemResultsSerializer -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, InvalidQuerySyntaxError } = require('./errors') +const { IndexSearchError, IndexConnectionError } = require('./errors') const ResponseMassager = require('./response_massager.js') -const AvailableDeliveryLocationTypes = require('./available_delivery_location_types') -const { parseParams, deepValue } = require('../lib/util') +const { parseParams } = require('../lib/util') -const ApiRequest = require('./api-request') -const ElasticQueryBuilder = require('./elasticsearch/elastic-query-builder') -const { CqlQuery } = require('./elasticsearch/cql_query_builder') -const { FILTER_CONFIG, SEARCH_SCOPES, AGGREGATIONS_SPEC } = require('./elasticsearch/config') +const { AGGREGATIONS_SPEC } = require('./elasticsearch/config') const errors = require('./errors') const Item = require('./models/Item.js') +const { + esRangeValue, + parseSearchParams, + nyplSourceAndId, + itemsByFilter, + mergeAggregationsResponses, + lookupPatronType, + makeRelevanceReport +} = require('./utils/resource-helpers') + +const { + bodyForFindByUri, + bodyForSearch, + aggregationQueriesForParams, + bodyForAggregation +} = require('./elasticsearch/elastic-body-builder') const RESOURCES_INDEX = process.env.RESOURCES_INDEX -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' - }, - callnumber: { - initialDirection: 'asc', - field: 'shelfMark.keywordLowercased' - }, - 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' -] - -// Configure controller-wide parameter parsing: -const parseSearchParams = function (params, overrideParams = {}) { - return parseParams(params, { - q: { type: 'string' }, - page: { type: 'int', default: 1 }, - per_page: { type: 'int', default: 50, range: [0, 100] }, - field: { type: 'string', range: Object.keys(AGGREGATIONS_SPEC) }, - sort: { type: 'string', range: Object.keys(SORT_FIELDS), default: 'relevance' }, - sort_direction: { type: 'string', range: ['asc', 'desc'] }, - search_scope: { type: 'string', range: Object.keys(SEARCH_SCOPES), default: 'all' }, - filters: { type: 'hash', fields: FILTER_CONFIG }, - items_size: { type: 'int', default: 100, range: [0, 200] }, - items_from: { type: 'int', default: 0 }, - callnumber: { type: 'string' }, - standard_number: { type: 'string' }, - contributor: { type: 'string' }, - title: { type: 'string' }, - subject: { type: 'string' }, - series: { type: 'string' }, - genre: { type: 'string' }, - subject_prefix: { type: 'string' }, - isbn: { type: 'string' }, - issn: { type: 'string' }, - lccn: { type: 'string' }, - oclc: { type: 'string' }, - role: { type: 'string' }, - merge_checkin_card_items: { type: 'boolean', default: true }, - include_item_aggregations: { type: 'boolean', default: true }, - ids: { type: 'string-list' }, - ...overrideParams - }) -} - // These are the handlers made available to the router: module.exports = function (app, _private = null) { app.resources = {} @@ -123,11 +60,7 @@ module.exports = function (app, _private = null) { }) // Validate uri: - const nyplSourceMapper = await NyplSourceMapper.instance() - const { id, nyplSource } = nyplSourceMapper.splitIdentifier(params.uri) ?? {} - if (!id || !nyplSource) { - throw new errors.InvalidParameterError(`Invalid bnum: ${params.uri}`) - } + await nyplSourceAndId(params.uri) // If we need to return itemAggregations or filter on item_status, // then we need to pre-retrieve SCSB item statuses to incorporate them into @@ -136,177 +69,71 @@ module.exports = function (app, _private = null) { // We only need to retrieve scsb statuses if building item aggs or // filtering on status: const retrieveScsbStatuses = params.include_item_aggregations || params.item_status - const scsbStatusLookup = retrieveScsbStatuses - ? scsbClient.getBarcodesByStatusForBnum(params.uri) - .catch((e) => { - app.logger.error(`Error connecting to SCSB; Unable to lookup barcodes for bib ${params.uri}`, e) - return {} - }) - : Promise.resolve({}) - - return scsbStatusLookup - .then((recapBarcodesByStatus) => { - // Establish base query: - let body = { - _source: { - excludes: EXCLUDE_FIELDS - }, - size: 1, - query: { - bool: { - must: [ - { - term: { - uri: params.uri - } - } - ] - } - } - } - const paramsIncludesItemLevelFiltering = Object.keys(params) - .filter((param) => param.startsWith('item_')).length > 0 - const returnAllItems = params.all_items && !paramsIncludesItemLevelFiltering - if (returnAllItems) { - body._source.excludes = EXCLUDE_FIELDS.filter((field) => field !== '*_sort') - } else { - // No specific item requested, so add pagination and matching params: - 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'] - } - body = addInnerHits(body, itemsOptions) - body._source = { - excludes: EXCLUDE_FIELDS.concat(['items']) - } - } - if (params.include_item_aggregations) { - body.aggregations = ITEM_FILTER_AGGREGATIONS - } - app.logger.debug('Resources#findByUri', body) - return app.esClient.search(body) - .then((resp) => { - // Mindfully throw errors for known issues: - if (!resp || !resp.hits) { - throw new Error('Error connecting to index') - } else if (resp?.hits?.total?.value === 0) { - throw new errors.NotFoundError(`Record not found: ${params.uri}`) - } else { - const massagedResponse = new ResponseMassager(resp) - return massagedResponse.massagedResponse(request, { queryRecapCustomerCode: !!params.itemUri, recapBarcodesByStatus }) - .catch((e) => { - // If error hitting HTC, just return response un-modified: - return resp - }) - } - }).then((resp) => { - const hitsAndItemAggregations = resp.hits.hits[0]._source - hitsAndItemAggregations.itemAggregations = resp.aggregations - return ResourceSerializer.serialize(hitsAndItemAggregations, Object.assign(opts, { root: true })) - }) - }) + let recapBarcodesByStatus = {} + if (retrieveScsbStatuses) { + try { + recapBarcodesByStatus = await scsbClient.getBarcodesByStatusForBnum(params.uri) + } catch (e) { + app.logger.error(`Error connecting to SCSB; Unable to lookup barcodes for bib ${params.uri}`, e) + } + } + + const body = bodyForFindByUri(recapBarcodesByStatus, params) + app.logger.debug('Resources#findByUri', body) + let resp = await app.esClient.search(body) + // Mindfully throw errors for known issues: + if (!resp || !resp.hits) { + throw new Error('Error connecting to index') + } else if (resp?.hits?.total?.value === 0) { + throw new errors.NotFoundError(`Record not found: ${params.uri}`) + } else { + const massagedResponse = new ResponseMassager(resp) + try { + resp = await massagedResponse.massagedResponse(request, { queryRecapCustomerCode: !!params.itemUri, recapBarcodesByStatus }) + } catch (e) { + // If error hitting HTC, just return response un-modified: + } + const hitsAndItemAggregations = resp.hits.hits[0]._source + hitsAndItemAggregations.itemAggregations = resp.aggregations + return ResourceSerializer.serialize(hitsAndItemAggregations, Object.assign(opts, { root: true })) + } } // Get a single raw annotated-marc resource: app.resources.annotatedMarc = async function (params, opts) { // Convert discovery id to nyplSource and un-prefixed id: - const nyplSourceMapper = await NyplSourceMapper.instance() - const { id, nyplSource } = nyplSourceMapper.splitIdentifier(params.uri) ?? {} - if (!id || !nyplSource) { - throw new errors.InvalidParameterError(`Invalid bnum: ${params.uri}`) - } + const { id, nyplSource } = await nyplSourceAndId(params.uri) app.logger.debug('Resources#annotatedMarc', { id, nyplSource }) - return makeNyplDataApiClient().get(`bibs/${nyplSource}/${id}`) - .then((resp) => { - // need to check that the query actually found an entry - if (!resp.data) { - throw new errors.NotFoundError(`Record not found: bibs/${nyplSource}/${id}`) - } else { - return resp.data - } - }) - .then(AnnotatedMarcSerializer.serialize) + const resp = await makeNyplDataApiClient().get(`bibs/${nyplSource}/${id}`) + // need to check that the query actually found an entry + if (!resp.data) { + throw new errors.NotFoundError(`Record not found: bibs/${nyplSource}/${id}`) + } + + return AnnotatedMarcSerializer.serialize(resp.data) } // Get a single raw marc: app.resources.marc = async function (params, opts) { // Convert discovery id to nyplSource and un-prefixed id: - const nyplSourceMapper = await NyplSourceMapper.instance() - const { id, nyplSource } = nyplSourceMapper.splitIdentifier(params.uri) ?? {} - - if (!id || !nyplSource) { - throw new errors.InvalidParameterError(`Invalid bnum: ${params.uri}`) - } + const { id, nyplSource } = await nyplSourceAndId(params.uri) - app.logger.debug('Resources#marc', { id, nyplSource }) - - return makeNyplDataApiClient().get(`bibs/${nyplSource}/${id}`) - .then((resp) => { - // need to check that the query actually found an entry - if (!resp.data) { - throw new errors.NotFoundError(`Record not found: bibs/${nyplSource}/${id}`) - } else { - return resp.data - } - }) - .then(MarcSerializer.serialize) - } + app.logger.debug('Resources#annotatedMarc', { id, nyplSource }) - function itemsByFilter (filter, opts) { - opts = Object.assign({ - _source: null - }, opts) - - // Build ES query body: - const body = { - query: { - nested: { - path: 'items', - score_mode: 'avg', - query: { - constant_score: { - filter - } - } - } - } + const resp = await makeNyplDataApiClient().get(`bibs/${nyplSource}/${id}`) + // need to check that the query actually found an entry + if (!resp.data) { + throw new errors.NotFoundError(`Record not found: bibs/${nyplSource}/${id}`) } - if (opts._source) body._source = opts._source - - app.logger.debug('Resources#itemsByFilter', body) - return app.esClient.search(body) - .then((resp) => { - if (!resp || !resp.hits || resp.hits.total === 0) return Promise.reject(new Error('No matching items')) - resp = new LocationLabelUpdater(resp).responseWithUpdatedLabels() - // Convert this ES bibs response into an array of flattened items: - return resp.hits.hits - .map((doc) => doc._source) - // Reduce to a flat array of items - .reduce((a, bib) => { - return a.concat(bib.items) - // Let's affix that bnum into the item's identifiers so we know where it came from: - .map((i) => { - return Object.assign(i, { identifier: [`urn:bnum:${bib.uri}`].concat(i.identifier) }) - }) - }, []) - }) + + return MarcSerializer.serialize(resp.data) } // Get deliveryLocations for given resource(s) - app.resources.deliveryLocationsByBarcode = function (params, opts) { + app.resources.deliveryLocationsByBarcode = async function (params, opts) { params = parseParams(params, { barcodes: { type: 'string', repeatable: true }, patronId: { type: 'string' } @@ -315,489 +142,70 @@ module.exports = function (app, _private = null) { const identifierValues = barcodes.map((barcode) => `urn:barcode:${barcode}`) - // Create promise to resolve deliveryLocationTypes by patron type: - const lookupPatronType = AvailableDeliveryLocationTypes.getScholarRoomByPatronId(params.patronId) - .catch((e) => { - throw new errors.InvalidParameterError('Invalid patronId') - }) - // Create promise to resolve items: - const fetchItems = itemsByFilter( - { terms: { 'items.identifier': identifierValues } }, - { _source: ['uri', 'type', 'items.uri', 'items.type', 'items.identifier', 'items.holdingLocation', 'items.status', 'items.catalogItemType', 'items.accessMessage', 'items.recapCustomerCode', 'items.m2CustomerCode'] } - - // Filter out any items (multi item bib) that don't match one of the queriered barcodes: - ).then((items) => { - return items.filter((item) => { - return item.identifier.filter((i) => identifierValues.indexOf(i) >= 0).length > 0 - }) - }) + const fetchItems = itemsByFilter(identifierValues, app) // Run both item fetch and patron fetch in parallel: - return Promise.all([fetchItems, lookupPatronType]) - .then((resp) => { - // The resolved values of Promise.all are strictly ordered based on original array of promises - const items = resp[0] - const scholarRoom = resp[1] - // Use HTC API and nypl-core mappings to ammend ES response with deliveryLocations: - return Promise.all(items.map(async (item) => Item.withDeliveryLocationsByBarcode(item, scholarRoom))) - .catch((e) => { - // An error here is likely an HTC API outage - // Let's return items unmodified: - // - app.logger.info({ message: 'Caught (and ignoring) error mapping barcodes to recap customer codes', htcError: e.message }) - return items - }) - }) - .then((items) => ItemResultsSerializer.serialize(items, opts)) - } - - /** - * Given a ES search body, returns same object modified to include the - * additional query necessary to limit (and paginate through) items - * - * @param {object} body - An ES query object (suitable for POSTing to ES - * @param {object} options - An object optionally defining `size` and `from` - * for limiting and paginating through items - */ - const addInnerHits = (body, _options = {}) => { - const options = Object.assign({ - size: process.env.SEARCH_ITEMS_SIZE || 200, - from: 0, - merge_checkin_card_items: true - }, _options) - - // Make sure necessary structure exists: - if (!deepValue(body, 'query.bool') && !deepValue(body, 'query.function_score.query.bool')) { - body.query = { bool: {} } - } - - // The place to add the filter depends on the query built to this point: - const placeToAddFilter = (body.query.bool || body.query.function_score.query.bool) - // Initialize filter object if it doesn't already exist: - placeToAddFilter.filter = placeToAddFilter.filter || [] - // If filter object already exists, convert it to array: - if (!Array.isArray(placeToAddFilter.filter)) placeToAddFilter.filter = [placeToAddFilter.filter] - - const itemsQuery = { - bool: Object.assign( - itemsQueryContext(options), - itemsFilterContext(options) - ) - } - - const wrappedItemsQuery = { - bool: { - should: [ - { - nested: { - path: 'items', - query: itemsQuery, - inner_hits: { - sort: [{ 'items.enumerationChronology_sort': 'desc' }], - size: options.size, - from: options.from, - name: 'items' - } - } - }, - // Add a catch-all to ensure we return the bib document even when - // numItems=0 or applied item filters exclude all items: - { match_all: {} } - ] - } - } - placeToAddFilter.filter.push(wrappedItemsQuery) - - // If there is any item query at all, run an additional inner_hits query - // to retrieve the total number of items without filtering: - if (itemsQuery.bool.filter) { - wrappedItemsQuery.bool.should.push({ - nested: { - path: 'items', - query: { - bool: { - must_not: [{ exists: { field: 'items.electronicLocator' } }] - } - }, - inner_hits: { name: 'allItems' } - } - }) - } - - return body - } - - /** - * Given a range represented as an array, returns a corresponding ES range object - * - * @param {Array.} range - An array consisting of a single date or a pair of dates - * @returns {object} - */ - const esRangeValue = (range) => { - // the greater-than-equal value will always be the first value in the range array. - // depending on the number of values and their equality, we query using less-than-equal - // the second value, or just less-than the first value plus one - - // Treat case where range start equals range end same as case of single value: - if (range[0] === range[1]) range = range.slice(0, 1) - const rangeQuery = { - gte: range[0] - } - if (range.length === 2) { - // search on both range values - rangeQuery.lte = range[range.length - 1] - } else if (range.length === 1) { - // if there is just one range, query up until the next year - rangeQuery.lt = range[0] + 1 - } - return rangeQuery - } + const [resp] = Promise.all([fetchItems, lookupPatronType]) + // The resolved values of Promise.all are strictly ordered based on original array of promises + let items = resp[0] + const scholarRoom = resp[1] - /** - * Given an object containing filters, - * returns content of the ES query filter context - * - * @param {object} options - An object with keys,value pairs of the form [filter_name]:[filter_value] - * @returns {object} - */ - const itemsFilterContext = (options) => { - if (!options.query) return {} - - const filterHandlers = { - volume: (volumes) => { - return { - range: { - 'items.volumeRange': esRangeValue(volumes) - } - } - }, - date: (dates) => { - return { - range: { - 'items.dateRange': esRangeValue(dates) - } - } - }, - format: (formats) => { - return { - terms: { - 'items.formatLiteral': formats - } - } - }, - location: (locations) => { - return { - terms: { - 'items.holdingLocation.id': locations - } - } - }, - status: (statuses) => { - // Determine if all possible ReCAP statuses were selected: - const selectedRecapStatuses = recapStatuses(statuses) - - if (selectedRecapStatuses.length === 1 && - Array.isArray(options.unavailable_recap_barcodes) && - options.unavailable_recap_barcodes.length > 0) { - // There are known unavailable ReCAP items, so build a complicated - // filter clause with appropriate barcode overrides: - return itemStatusFilterWithUnavailableRecapItems(statuses, options.unavailable_recap_barcodes) - } else { - // If there are no known unavailable ReCAP items, just do a straight - // status match: - return { - terms: { - 'items.status.id': statuses - } - } - } - }, - itemUri: (uri) => { - return { term: { 'items.uri': uri } } - } - } - - const filters = Object.keys(options.query).map((filter) => { - const value = options.query[filter] - const handler = filterHandlers[filter] - return value && handler ? handler(value) : null - }).filter((x) => x) - - return filters.length - ? { filter: filters } - : {} - } - - /** - * Given an array of status ids (e.g. "status:a", "status:na") returns the - * subset of statuses that are relevant in ReCAP - */ - const recapStatuses = (statuses) => { - return statuses - .filter((status) => ['status:a', 'status:na'].includes(status)) - } - - /** - * Builds a big complicated ES filter to allow us to filter items by status, - * but override the indexed status for ReCAP items with statuses retrieved - * from SCSB. This corrects for the fact that ReCAP item statuses tend to be - * wrong in the ES index: - * - partner items are indexed as Available and remain thus forever - * - NYPL item statuses _should_ equal SCSB status, but the mechanism - * for keeping them synced isn't perfect and operates on a delay - * - * @param {string[]} statuses - An array of statuses to filter on - * @param {string[]} unavailableRecapBarcodes - An array of item barcodes - * known to be unavailble - * - * Returns an ES filter that matches the desired statuses, but also uses - * the known unavailable items to override indexed item statuses for ReCAP - * items (because ReCAP is the authority for status of off-site items). - * Essentially, the criteria is for matching an item is: - * - * - if on-site (non-ReCAP): - * - has a matching indexed status - * - if off-site: - * - if filtering on status:na - * - item barcode must be in unavailableRecapBarcodes - * - if filtering on status:a: - * - item barcode must NOT be in unavailableRecapBarcodes - */ - const itemStatusFilterWithUnavailableRecapItems = (statuses, unavailableRecapBarcodes) => { - // First, let's set up some common clauses: - - // Item is in ReCAP: - const itemIsRecapClause = { - regexp: { 'items.holdingLocation.id': 'loc:rc.*' } - } - // Item's indexed status matches one of the filtered statuses: - const itemHasIndexedStatusClause = { - terms: { 'items.status.id': statuses } - } - // Item is marked Unavailable in SCSB: - const itemIsUnavailableInRecapClause = { - script: { - script: { - inline: 'doc[\'items.idBarcode\'].value == null || ' + - 'params.unavailableRecapBarcodes.contains(doc[\'items.idBarcode\'][0])', - lang: 'painless', - params: { unavailableRecapBarcodes } - } - } - } - // This function is only called if `statuses` param contains a single - // ReCAP-relevant status (i.e. status:a or status:na), so determine which - // ReCAP status to use: - const selectedRecapStatus = recapStatuses(statuses).shift() - // Item's ReCAP status agrees with filter: - const itemRecapStatusAgreesWithFilterClause = - selectedRecapStatus === 'status:na' - ? itemIsUnavailableInRecapClause - : { bool: { must_not: itemIsUnavailableInRecapClause } } - - return { - bool: { - should: [ - // Either 1) item is on-site and has correctly indexed status: - { - bool: { - must: [ - // Item is on-site (i.e. not recap): - { bool: { must_not: itemIsRecapClause } }, - // Item indexed status matches filter: - itemHasIndexedStatusClause - ] - } - }, - // Or 2) item is off-site and has a scsb status that agrees with the - // filter (e.g. if filtering on status:na, scsb marks the barcode as - // 'Not Available') - { - bool: { - must: [ - // Item is off-site: - JSON.parse(JSON.stringify(itemIsRecapClause)), - // Item is not marked unavailable - itemRecapStatusAgreesWithFilterClause - ] - } - } - ] - } + // Use HTC API and nypl-core mappings to ammend ES response with deliveryLocations: + try { + items = await items.map(async (item) => Item.withDeliveryLocationsByBarcode(item, scholarRoom)) + } catch (e) { + // An error here is likely an HTC API outage + // Let's return items unmodified: + // + app.logger.info({ message: 'Caught (and ignoring) error mapping barcodes to recap customer codes', htcError: e.message }) + return items } - } - - /** - * Given an object containing query options, - * returns content of the ES query context - * - * @param {object} options - An object with request options. `merge_checkin_card_items` is the only one - * that matters right now - * @returns {object} - */ - const itemsQueryContext = (options) => { - const excludeClauses = [] - - if (!options.merge_checkin_card_items) excludeClauses.push({ term: { 'items.type': 'nypl:CheckinCardItem' } }) - - return excludeClauses.length ? { must_not: excludeClauses } : { must: { match_all: {} } } + items = await ItemResultsSerializer.serialize(items, opts) + return items } // Conduct a search across resources: - app.resources.search = function (params, opts, request) { + app.resources.search = async function (params, opts, request) { app.logger.debug('Unparsed params: ', params) params = parseSearchParams(params) app.logger.debug('Parsed params: ', params) - let parsed = {} - let cqlQuery = null - - if (params.search_scope === 'cql') { - cqlQuery = new CqlQuery(params.q) - try { - parsed = cqlQuery.displayParsed() - } catch (e) { - throw new InvalidQuerySyntaxError('Unknown parsing error. Error most likely near end of string') - } - if (parsed.error) { - throw new InvalidQuerySyntaxError(parsed.error) - } - if (!parsed.parsed) { - throw new InvalidQuerySyntaxError('Unknown parsing error. Error most likely near end of string') - } - } - - let body = buildElasticBody(params, cqlQuery) - - // Strip unnecessary _source fields - body._source = { - excludes: EXCLUDE_FIELDS.concat(['items']) - } - - body = addInnerHits(body, { merge_checkin_card_items: params.merge_checkin_card_items }) + const body = bodyForSearch(params) app.logger.debug('Resources#search', RESOURCES_INDEX, body) - return app.esClient.search(body) - .then((resp) => { - const massagedResponse = new ResponseMassager(resp) - return massagedResponse.massagedResponse(request) - .catch((e) => { - // If error hitting HTC, just return response un-modified: - return resp - }) - .then((updatedResponse) => ResourceResultsSerializer.serialize(updatedResponse, opts)) - .then((resp) => { - // Build relevance report (for debugging): - const relevanceReport = resp.itemListElement - .map((r, ind) => { - const out = [] - out.push(`${ind + 1}: ${r.searchResultScore} score > ${r.result.uri}:`) - if (params.search_scope === 'contributor') out.push(`(${r.result.creatorLiteral || r.result.contributorLiteral})`) - if (['standard_number', 'callnumber'].includes(params.search_scope)) out.push(`(${r.result.items && r.result.items[0]?.shelfMark})`) - out.push(`${r.result.title} (displayed as "${r.result.titleDisplay}")`) - if (r.matchedQueries) out.push(`\n ${r.matchedQueries.join(', ')}`) - return out.join(' ') - }) - app.logger.debug(`Relevances:\n ${relevanceReport.join('\n')}`) - - resp.debug = { - relevanceReport, - query: body, - ...parsed - } - return resp - }) - }) - .catch((e) => { - // Wrap ES client errors or any downstream error - 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}`) - }) - } + let resp - 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 + try { + resp = await app.esClient.search(body) + } catch (e) { + // Wrap ES client errors or any downstream error + if (e instanceof IndexSearchError || e instanceof IndexConnectionError) { + throw e // already a custom error } - return aggs - }, {}) + throw new IndexSearchError(`Error processing search: ${e.message || e}`) + } - const body = buildElasticBody(params) - body.size = 0 - body.aggregations = aggregations + try { + const massagedResponse = new ResponseMassager(resp) + resp = await massagedResponse.massagedResponse(request) + } catch (e) { - return body - } + } - /** - * 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]) - }) - ) - } + resp = await ResourceResultsSerializer.serialize(resp, opts) - /** - * Given an array of ES aggregations responses (such as that returned from msearch) - **/ - const mergeAggregationsResponses = (responses) => { - // Filter out errored responses: - responses = responses.filter((resp) => resp.aggregations) - if (responses.length === 0) { - return {} - } - return { - // Use `hits` of last element, somewhat arbitrarily: - hits: responses[responses.length - 1].hits, - aggregations: responses - .reduce((allAggs, resp) => { - const respAggs = Object.entries(resp.aggregations) - // Build hash of response aggs, squashing _nested aggs: - .reduce((a, [field, _a]) => { - // If it's nested, it will be in our special '_nested' prop: - a[field] = _a._nested || _a - return a - }, {}) - // Add response aggs to combined aggs: - return Object.assign(allAggs, respAggs) - }, {}) + const relevanceReport = resp.itemListElement + .map(makeRelevanceReport(params)) + app.logger.debug(`Relevances:\n ${relevanceReport.join('\n')}`) + + resp.debug = { + relevanceReport, + query: body } + return resp } // Get all aggregations: @@ -821,7 +229,7 @@ module.exports = function (app, _private = null) { } // Get a single aggregation: - app.resources.aggregation = (params, opts) => { + app.resources.aggregation = async (params, opts) => { params = parseSearchParams(params, { per_page: { type: 'int', default: 50, range: [0, 1000] } }) @@ -829,99 +237,27 @@ module.exports = function (app, _private = null) { return Promise.reject(new Error('Invalid aggregation field')) } - const body = buildElasticBody(params) - - // We're fetching aggs, so specify 0 resource results: - body.size = 0 - - body.aggregations = {} - body.aggregations[params.field] = AGGREGATIONS_SPEC[params.field] - - // If it's a terms agg, we can apply per_page: - if (body.aggregations[params.field].terms) { - body.aggregations[params.field].terms.size = params.per_page - } - const serializationOpts = Object.assign(opts, { // This tells the serializer what fields are "packed" fields, which should be split apart packed_fields: ['materialType', 'language', 'carrierType', 'mediaType', 'issuance', 'status', 'owner'], root: true }) + const body = bodyForAggregation(params) + app.logger.debug('Resources#aggregation:', body) - return app.esClient.search(body) - .then((resp) => { - // If it's nested, it will be in our special '_nested' prop: - resp = resp.aggregations[params.field]._nested || resp.aggregations[params.field] - resp.id = params.field - return AggregationSerializer.serialize(resp, serializationOpts) - }) + + let resp = await app.esClient.search(body) + resp = resp.aggregations[params.field]._nested || resp.aggregations[params.field] + resp.id = params.field + return AggregationSerializer.serialize(resp, serializationOpts) } // For unit testing, export private methods if second arg given: if (_private && typeof _private === 'object') { - _private.buildElasticBody = buildElasticBody - _private.buildElasticQuery = buildElasticQuery _private.parseSearchParams = parseSearchParams _private.esRangeValue = esRangeValue - _private.itemsFilterContext = itemsFilterContext - _private.itemsQueryContext = itemsQueryContext - _private.addInnerHits = addInnerHits _private.aggregationQueriesForParams = aggregationQueriesForParams _private.mergeAggregationsResponses = mergeAggregationsResponses } } - -/** - * 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, cqlQuery = null) { - const body = { - from: (params.per_page * (params.page - 1)), - size: params.per_page - } - - body.query = buildElasticQuery(params, cqlQuery) - - // 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 - } - body.sort = [{ [field]: direction }, { uri: 'asc' }] - - 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, cqlQuery = null) { - const request = ApiRequest.fromParams(params) - if (params.search_scope === 'cql') { - try { - const query = (cqlQuery || new CqlQuery(params.q)).buildEsQuery(request) - return query - } catch (e) { - throw new InvalidQuerySyntaxError(e.message) - } - } - - const builder = ElasticQueryBuilder.forApiRequest(request) - return builder.query.toJson() -} diff --git a/lib/utils/resource-helpers.js b/lib/utils/resource-helpers.js new file mode 100644 index 00000000..30d4ed25 --- /dev/null +++ b/lib/utils/resource-helpers.js @@ -0,0 +1,177 @@ +const { parseParams } = require('../util') +const { FILTER_CONFIG, SEARCH_SCOPES, AGGREGATIONS_SPEC, SORT_FIELDS } = require('../elasticsearch/config') +const NyplSourceMapper = require('research-catalog-indexer/lib/utils/nypl-source-mapper') +const errors = require('../errors') +const LocationLabelUpdater = require('../location_label_updater') +const AvailableDeliveryLocationTypes = require('../available_delivery_location_types') + +/** + * Given a range represented as an array, returns a corresponding ES range object + * + * @param {Array.} range - An array consisting of a single date or a pair of dates + * @returns {object} + */ +const esRangeValue = (range) => { + // the greater-than-equal value will always be the first value in the range array. + // depending on the number of values and their equality, we query using less-than-equal + // the second value, or just less-than the first value plus one + + // Treat case where range start equals range end same as case of single value: + if (range[0] === range[1]) range = range.slice(0, 1) + const rangeQuery = { + gte: range[0] + } + if (range.length === 2) { + // search on both range values + rangeQuery.lte = range[range.length - 1] + } else if (range.length === 1) { + // if there is just one range, query up until the next year + rangeQuery.lt = range[0] + 1 + } + return rangeQuery +} + +// Configure controller-wide parameter parsing: +const parseSearchParams = function (params, overrideParams = {}) { + return parseParams(params, { + q: { type: 'string' }, + page: { type: 'int', default: 1 }, + per_page: { type: 'int', default: 50, range: [0, 100] }, + field: { type: 'string', range: Object.keys(AGGREGATIONS_SPEC) }, + sort: { type: 'string', range: Object.keys(SORT_FIELDS), default: 'relevance' }, + sort_direction: { type: 'string', range: ['asc', 'desc'] }, + search_scope: { type: 'string', range: Object.keys(SEARCH_SCOPES), default: 'all' }, + filters: { type: 'hash', fields: FILTER_CONFIG }, + items_size: { type: 'int', default: 100, range: [0, 200] }, + items_from: { type: 'int', default: 0 }, + callnumber: { type: 'string' }, + standard_number: { type: 'string' }, + contributor: { type: 'string' }, + title: { type: 'string' }, + subject: { type: 'string' }, + series: { type: 'string' }, + genre: { type: 'string' }, + subject_prefix: { type: 'string' }, + isbn: { type: 'string' }, + issn: { type: 'string' }, + lccn: { type: 'string' }, + oclc: { type: 'string' }, + role: { type: 'string' }, + merge_checkin_card_items: { type: 'boolean', default: true }, + include_item_aggregations: { type: 'boolean', default: true }, + ids: { type: 'string-list' }, + ...overrideParams + }) +} + +const nyplSourceAndId = async function (uri) { + const nyplSourceMapper = await NyplSourceMapper.instance() + const { id, nyplSource } = nyplSourceMapper.splitIdentifier(uri) ?? {} + if (!id || !nyplSource) { + throw new errors.InvalidParameterError(`Invalid bnum: ${uri}`) + } + return { id, nyplSource } +} + +function itemsByFilter (identifierValues, app) { + const filter = { terms: { 'items.identifier': identifierValues } } + let opts = { _source: ['uri', 'type', 'items.uri', 'items.type', 'items.identifier', 'items.holdingLocation', 'items.status', 'items.catalogItemType', 'items.accessMessage', 'items.m2CustomerCode'] } + + opts = Object.assign({ + _source: null + }, opts) + + // Build ES query body: + const body = { + query: { + nested: { + path: 'items', + score_mode: 'avg', + query: { + constant_score: { + filter + } + } + } + } + } + if (opts._source) body._source = opts._source + + app.logger.debug('Resources#itemsByFilter', body) + return app.esClient.search(body) + .then((resp) => { + if (!resp || !resp.hits || resp.hits.total === 0) return Promise.reject(new Error('No matching items')) + resp = new LocationLabelUpdater(resp).responseWithUpdatedLabels() + // Convert this ES bibs response into an array of flattened items: + return resp.hits.hits + .map((doc) => doc._source) + // Reduce to a flat array of items + .reduce((a, bib) => { + return a.concat(bib.items) + // Let's affix that bnum into the item's identifiers so we know where it came from: + .map((i) => { + return Object.assign(i, { identifier: [`urn:bnum:${bib.uri}`].concat(i.identifier) }) + }) + }, []) + }).then((items) => { + return items.filter((item) => { + return item.identifier.filter((i) => identifierValues.indexOf(i) >= 0).length > 0 + }) + }) +} + +/** +* Given an array of ES aggregations responses (such as that returned from msearch) +**/ +const mergeAggregationsResponses = (responses) => { + // Filter out errored responses: + responses = responses.filter((resp) => resp.aggregations) + if (responses.length === 0) { + return {} + } + return { + // Use `hits` of last element, somewhat arbitrarily: + hits: responses[responses.length - 1].hits, + aggregations: responses + .reduce((allAggs, resp) => { + const respAggs = Object.entries(resp.aggregations) + // Build hash of response aggs, squashing _nested aggs: + .reduce((a, [field, _a]) => { + // If it's nested, it will be in our special '_nested' prop: + a[field] = _a._nested || _a + return a + }, {}) + // Add response aggs to combined aggs: + return Object.assign(allAggs, respAggs) + }, {}) + } +} + +// Create promise to resolve deliveryLocationTypes by patron type: +const lookupPatronType = async function (params) { + try { + await AvailableDeliveryLocationTypes.getScholarRoomByPatronId(params.patronId) + } catch (e) { + throw new errors.InvalidParameterError('Invalid patronId') + } +} + +const makeRelevanceReport = (params) => (r, ind) => { + const out = [] + out.push(`${ind + 1}: ${r.searchResultScore} score > ${r.result.uri}:`) + if (params.search_scope === 'contributor') out.push(`(${r.result.creatorLiteral || r.result.contributorLiteral})`) + if (['standard_number', 'callnumber'].includes(params.search_scope)) out.push(`(${r.result.items && r.result.items[0]?.shelfMark})`) + out.push(`${r.result.title} (displayed as "${r.result.titleDisplay}")`) + if (r.matchedQueries) out.push(`\n ${r.matchedQueries.join(', ')}`) + return out.join(' ') +} + +module.exports = { + esRangeValue, + parseSearchParams, + nyplSourceAndId, + itemsByFilter, + mergeAggregationsResponses, + lookupPatronType, + makeRelevanceReport +} diff --git a/package.json b/package.json index 70295c99..eadec80c 100644 --- a/package.json +++ b/package.json @@ -33,12 +33,13 @@ "yaml": "^2.6.0" }, "scripts": { - "test": "./node_modules/.bin/standard --env mocha && NODE_ENV=test ./node_modules/.bin/mocha test --exit", + "test": "./node_modules/.bin/standard --env mocha && NODE_ENV=test ./node_modules/.bin/mocha test --ignore 'test/integration/nyql/**' --exit", "start": "node server.js", "deploy-development": "git checkout development && git pull origin development && eb deploy discovery-api-dev --profile nypl-sandbox", "deploy-qa": "git checkout qa && git pull origin qa && eb deploy discovery-api-qa --profile nypl-digital-dev", "deploy-production": "git checkout production && git pull origin qa && eb deploy discovery-api-production --profile nypl-digital-dev", - "rebuild-annotated-marc-rules": "./scripts/update-annotated-marc-rules.js --refetch" + "rebuild-annotated-marc-rules": "./scripts/update-annotated-marc-rules.js --refetch", + "nyql-api-test": "npx mocha test/integration/nyql/*.test.js" }, "description": "Discovery API as an AWS Lambda.", "license": "MIT", diff --git a/scripts/build-mappings-document.js b/scripts/build-mappings-document.js new file mode 100644 index 00000000..12c2851f --- /dev/null +++ b/scripts/build-mappings-document.js @@ -0,0 +1,306 @@ +/** + * Script to build a rc-mappings json document with detailed information that + * connects Indexer mappings to DiscoveryAPI properties and RC labels. This + * document can be used to build staff/patron facing reports. + * + * Usage: + * node scripts/build-mappings-document.js [--outfile out.json] + */ + +const { parseArgs: _parseArgs } = require('node:util') +const fs = require('fs') +const { parse: csvParse } = require('csv-parse/sync') + +const elasticConfig = require('../lib/elasticsearch/config') +const cqlConfig = require('../lib/elasticsearch/cql/index-mapping') + +const fetchRciMappings = async () => { + console.info('Retrieving RCI mappings...') + + const ghBase = 'https://raw.githubusercontent.com/NYPL/research-catalog-indexer/refs/heads/main' + + const [bibMappings, itemMappings] = await Promise.all( + [ + `${ghBase}/lib/mappings/bib-mapping.json`, + `${ghBase}/lib/mappings/item-mapping.json` + ].map(async (url) => { + const resp = await fetch(url) + return resp.json() + }) + ) + + console.info(` Got ${Object.keys(bibMappings).length} bib mappings & ${Object.keys(itemMappings).length} item mappings`) + + return { + bibMappings, + itemMappings + } +} + +/** + * Fetches and inspects key RC files to extract patron-facing labels for discovery-api properties. + * + * Returns a hash relating discovery-api property names (e.g. creatorLiteral) + * to RC label (e.g. "Additional authors") + */ +const fetchRcLabels = async () => { + console.info('Retrieving RC property labels...') + + const resp = await fetch('https://raw.githubusercontent.com/NYPL/research-catalog/main/src/models/BibDetails.ts') + const lines = (await resp.text()).split('\n') + const labels = lines.reduce((h, line) => { + // Look for lines of form `{ field: "contributorLiteral", label: "Additional authors" },` + const reg = /field: "([^"]+)", label: "([^"]+)"/ + if (reg.test(line)) { + const [, property, label] = line.match(reg) + h[property] = label + } + return h + }, {}) + + console.info(` Got ${Object.keys(labels).length} RC labels`) + + return labels +} + +/** + * Fetch browse-terms mappings config and return a callback function that, for + * a marc tag, returns the appropriate set of subfields. + */ +const buildBrowseTermsSubfieldLookup = async () => { + console.info('Retrieving Browse-Term module subfields...') + + const mappingsReq = await fetch('https://raw.githubusercontent.com/NYPL/browse-term/main/src/data/mappings.json') + const mappings = await mappingsReq.json() + + const subjectsMappings = mappings.subjects.first.concat(mappings.subjects.last) + const portionMap = mappings.contributors + + const subfieldsByMarc = (marc) => { + if (marc >= '600' && marc <= '690') { + return subjectsMappings + } else { + const key = marc.slice(1) + return Object.values(portionMap[key] || portionMap['00']) + .flat(1) + .sort() + } + } + + console.info(` Got ${subjectsMappings.length} subjects mappings and ${Object.keys(portionMap).length} sets of mappings for contributors`) + + return subfieldsByMarc +} + +/** + * Returns an object defining: + * - properties: Hash of properties relating property (internal name) to an object that defines + * - label {string} - Preferred label (overrides RC derived label) + * - advSearchEquivalent {string} - Name of field in RC Adv Search providing similar search + * - notes {string} - Freeform descrption of field + */ +const fetchEditorial = async () => { + console.info('Retrieving editorial from Google Sheet') + + const resp = await fetch('https://docs.google.com/spreadsheets/d/e/2PACX-1vSfPy-FGTOct1QjjZ3K8LOl2WQBZH8zgBViwWp5jd_sfXHg18BIvqvPej7mF_9ScSmxv0nO0l2FfoRr/pub?output=csv', { redirect: 'follow' }) + const data = await resp.text() + const rows = csvParse(data) + + let category = null + const lookup = rows + .slice(1) + .filter((row) => row[0] || row[1]) + .reduce((h, row) => { + if (row[0]) category = row[0] + + const bucket = { + 'search scopes': 'searchScopes', + 'cql indexes': 'cqlIndexes', + properties: 'properties' + }[category.toLowerCase().trim()] + + const [, internalName, label, advSearchEquivalent, notes] = row + + if (!bucket) console.error('Invalid bucket? ', category) + const o = {} + if (label) o.label = label + if (notes) o.notes = notes + if (advSearchEquivalent) o.advSearchEquivalent = advSearchEquivalent + + h[bucket][internalName] = o + + return h + }, { properties: {}, searchScopes: {}, cqlIndexes: {} }) + + console.info(` Got ${Object.entries(lookup).map(([cat, lookup]) => `${Object.keys(lookup).length} ${cat} entries`).join(', ')}`) + + return lookup +} + +const propertyName = (field) => { + return field + .replace(/\.(folded.*|raw|keyword.*|clean|range|id|label(\.\w+)?)$/, '') + .replace(/\^.*/, '') +} + +/** + * Build array of objects representing properties. + * + * Each entry defines: + * - name {string} - Name of property + * - marc {object[]} - Array of marc mappings (such as those defined in bib-mappings.json) + * - notes {string} - The optional "notes" prop sometimes offered in lieu of a mapping in *-mappings.json + * - label {string} - RC label for property (possibly overriden by "editorial" doc) + * - advSearchEquivalent {string} - Name of field in RC Adv Search providing + * similar search. Retrieved from + * "editorial" document. + * + */ +const buildProperties = async (editorial) => { + const rcLabels = await fetchRcLabels() + const { bibMappings, itemMappings } = await fetchRciMappings() + + const basicMappings = Object.entries(bibMappings).map(([name, config]) => { + return { + name, + marc: config.paths, + notes: config.notes, + label: rcLabels[name], + ...(editorial[name] || {}) + } + }) + .concat(Object.entries(itemMappings).map(([name, config]) => { + name = `items.${name}` + return { + name, + marc: config.paths, + notes: config.notes, + ...(editorial[name] || {}) + } + })) + + const browseTermSubfieldsByMarc = await buildBrowseTermsSubfieldLookup() + const browseTermManagedMappings = [ + 'creatorLiteral', + 'contributorLiteral', + 'subjectLiteral', + 'seriesAddedEntry' + ] + return basicMappings.map((m) => { + if (browseTermManagedMappings.includes(m.name)) { + m.marc = m.marc.map((marc) => { + marc.subfields = browseTermSubfieldsByMarc(marc.marc) + // Disregard excludedSufields when found in RCI mappings, since those + // are overridden by the `subfields` mappings found in browse-term: + delete marc.excludedSubfields + return marc + }) + } + return m + }) +} + +/** + * Build array of objects representing search scopes. + * + * Each entry defines: + * - name {string} - Name of cql index + * - properties {string[]} - Array of property names + * - advSearchEquivalent {string} - Name of field in RC Adv Search providing + * similar search. Retrieved from + * "editorial" document. + * - notes {string} - Freeform descrption of field. Retrieved from + * "editorial" document. + */ +const buildSearchScopes = (editorial) => { + return Object.entries(elasticConfig.SEARCH_SCOPES).map(([name, config]) => { + let properties = null + if (config.fields) { + properties = [...new Set( + config.fields + .map((field) => field.field || field) + .map((field) => propertyName(field)) + .filter((field) => field.indexOf('parallel') !== 0) + )] + } + return { + name, + properties, + ...(editorial[name] || {}) + } + }) +} + +/** + * Build array of objects representing all CQL indexes. + * + * Each entry defines: + * - name {string} - Name of cql index + * - properties {string[]} - Array of property names + * - advSearchEquivalent {string} - Name of field in RC Adv Search providing + * similar search. Retrieved from + * "editorial" document. + * - notes {string} - Freeform descrption of field. Retrieved from + * "editorial" document. + */ +const buildCqlIndexes = (editorial) => { + return Object.entries(cqlConfig.indexMapping).map(([name, config]) => { + let properties = null + const fields = config.fields || config.term + if (fields) { + properties = [...new Set( + fields + .map((field) => field.field || field) + .map((field) => propertyName(field)) + .filter((field) => field.indexOf('parallel') !== 0) + )] + } + return { + name, + properties, + ...(editorial[name] || {}) + } + }) +} + +const parseArgs = () => { + return _parseArgs({ + args: process.args, + options: { + outfile: { + type: 'string' + } + } + }).values +} + +/** + * Main script function. Fetch and collate data. Write result to outfile. + */ +const run = async () => { + const args = parseArgs() + + const editorial = await fetchEditorial() + + const properties = await buildProperties(editorial.properties) + const searchScopes = buildSearchScopes(editorial.searchScopes) + const cqlIndexes = buildCqlIndexes(editorial.cqlIndexes) + + const doc = { + properties, + searchScopes, + cqlIndexes + } + + const outDoc = JSON.stringify(doc, null, 2) + if (args.outfile) { + console.info(`Writing unified doc to ${args.outfile}`) + fs.writeFileSync(args.outfile, outDoc, 'utf-8') + } else { + console.info(outDoc) + } + + console.info('Done') +} + +run() diff --git a/test/elastic-body-builder.test.js b/test/elastic-body-builder.test.js new file mode 100644 index 00000000..ccec82dd --- /dev/null +++ b/test/elastic-body-builder.test.js @@ -0,0 +1,376 @@ +const { expect } = require('chai') + +const { bodyForSearch, bodyForFindByUri } = require('../lib/elasticsearch/elastic-body-builder') + +describe('bodyForSearch', function () { + it('excludes checkin cards by default', function () { + expect(bodyForSearch({ sort: 'relevance' })) + .to.deep.equal( + { + query: { + bool: { + filter: [ + { + bool: { + should: [ + { + nested: { + path: 'items', + query: { + bool: { + must_not: [ + { + term: { + 'items.type': 'nypl:CheckinCardItem' + } + } + ] + } + }, + inner_hits: { + sort: [ + { + 'items.enumerationChronology_sort': 'desc' + } + ], + size: '3', + from: 0, + name: 'items' + } + } + }, + { + match_all: {} + } + ] + } + } + ] + } + }, + sort: [ + { + _score: 'desc' + }, + { + uri: 'asc' + } + ], + _source: { + excludes: [ + 'uris', + '*_packed', + '*_sort', + 'items.*_packed', + 'contentsTitle', + 'suppressed', + '*WithoutDates', + '*Normalized', + 'items' + ] + } + } + ) + }) + + it('includes checkin cards when present in params', function () { + expect(bodyForSearch({ sort: 'relevance', merge_checkin_card_items: true })) + .to.deep.equal( + { + query: { + bool: { + filter: [ + { + bool: { + should: [ + { + nested: { + path: 'items', + query: { + bool: { + must: { + match_all: {} + } + } + }, + inner_hits: { + sort: [ + { + 'items.enumerationChronology_sort': 'desc' + } + ], + size: '3', + from: 0, + name: 'items' + } + } + }, + { + match_all: {} + } + ] + } + } + ] + } + }, + sort: [ + { + _score: 'desc' + }, + { + uri: 'asc' + } + ], + _source: { + excludes: [ + 'uris', + '*_packed', + '*_sort', + 'items.*_packed', + 'contentsTitle', + 'suppressed', + '*WithoutDates', + '*Normalized', + 'items' + ] + } + } + ) + }) +}) + +describe('bodyForFindByUri', function () { + it('queries for uri', function () { + const expected = { + _source: { + excludes: [ + 'uris', + '*_packed', + '*_sort', + 'items.*_packed', + 'contentsTitle', + 'suppressed', + '*WithoutDates', + '*Normalized', + 'items' + ] + }, + size: 1, + query: { + bool: { + must: [ + { + term: { + uri: 'b15781267' + } + } + ], + filter: [ + { + bool: { + should: [ + { + nested: { + path: 'items', + query: { + bool: { + must: { + match_all: {} + } + } + }, + inner_hits: { + sort: [ + { + 'items.enumerationChronology_sort': 'desc' + } + ], + size: 100, + from: 0, + name: 'items' + } + } + }, + { + match_all: {} + } + ] + } + } + ] + } + }, + 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' + } + } + } + } + } + } + + const params = { + all_items: false, + uri: 'b15781267', + items_size: 100, + items_from: 0, + merge_checkin_card_items: true, + include_item_aggregations: true + } + const barcodes = {} + expect(bodyForFindByUri(barcodes, params)) + .to.deep.equal(expected) + }) + + it('accepts item params', function () { + const barcodes = { 'Not Available': ['1234'] } + const params = { + all_items: false, + uri: 'b15781267', + items_size: 10, + items_from: 10, + merge_checkin_card_items: true, + include_item_aggregations: true + } + + const expected = { + _source: { + excludes: [ + 'uris', + '*_packed', + '*_sort', + 'items.*_packed', + 'contentsTitle', + 'suppressed', + '*WithoutDates', + '*Normalized', + 'items' + ] + }, + size: 1, + query: { + bool: { + must: [ + { + term: { + uri: 'b15781267' + } + } + ], + filter: [ + { + bool: { + should: [ + { + nested: { + path: 'items', + query: { + bool: { + must: { + match_all: {} + } + } + }, + inner_hits: { + sort: [ + { + 'items.enumerationChronology_sort': 'desc' + } + ], + size: 10, + from: 10, + name: 'items' + } + } + }, + { + match_all: {} + } + ] + } + } + ] + } + }, + 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' + } + } + } + } + } + } + + expect(bodyForFindByUri(barcodes, params)) + .to.deep.equal(expected) + }) +}) diff --git a/test/integration/nyql/helpers.js b/test/integration/nyql/helpers.js new file mode 100644 index 00000000..bed83e43 --- /dev/null +++ b/test/integration/nyql/helpers.js @@ -0,0 +1,54 @@ +const request = require('supertest') + +const ENV = process.env.ENV || process.env.NODE_ENV +const DEFAULT_BASE_URL = ENV === 'production' + ? 'https://platform.nypl.org/api/v0.1' + : 'https://qa-platform.nypl.org/api/v0.1' +const baseUrl = process.env.NYQL_TEST_BASE_URL || DEFAULT_BASE_URL +const getId = (item) => item?.result?.['@id'] +const endpoint = '/discovery/resources' +const TIMEOUT = 30000 + +const search = (params) => + request(baseUrl) + .get(endpoint) + .query({ + search_scope: 'cql', + per_page: 100, + ...params + }) + .timeout(TIMEOUT) + .expect(200) + +const normalizeCallnumber = (value) => + String(value || '') + .replace(/^[*"]+|[*"]+$/g, '') + .trim() + .toLowerCase() + +const resultContainsCallnumber = (result, callnumber) => { + const target = normalizeCallnumber(callnumber) + const shelfMarks = [ + ...(Array.isArray(result?.shelfMark) ? result.shelfMark : []), + ...(Array.isArray(result?.items) + ? result.items.flatMap((item) => + Array.isArray(item?.shelfMark) ? item.shelfMark : [] + ) + : []) + ] + .filter(Boolean) + .map(normalizeCallnumber) + + return shelfMarks.some( + (mark) => mark.includes(target) || target.includes(mark) + ) +} + +module.exports = { + baseUrl, + getId, + normalizeCallnumber, + resultContainsCallnumber, + endpoint, + search +} diff --git a/test/integration/nyql/nyql-precision.test.js b/test/integration/nyql/nyql-precision.test.js new file mode 100644 index 00000000..0a860953 --- /dev/null +++ b/test/integration/nyql/nyql-precision.test.js @@ -0,0 +1,62 @@ +const { expect } = require('chai') +const { resultContainsCallnumber, search } = require('./helpers') + +// These are some tests to verify that certain NYQL queries are returning results with the expected precision, especially for fields like call numbers where we want to ensure that the query is matching the intended values and not over- or under-matching. + +describe('Discovery API - NYQL precision tests', function () { + this.timeout(30000) + + it('should return exactly one result for callnumber = "JFE 86-3252"', async () => { + const callnumber = 'JFE 86-3252' + + const res = await search({ + q: `callnumber = "${callnumber}"` + }) + + expect(res.body.itemListElement).to.be.an('array') + + // Assert exactly one result + expect(res.body.itemListElement.length).to.equal(1) + + // Assert the result contains the target call number + const result = res.body.itemListElement[0].result + expect(resultContainsCallnumber(result, callnumber)).to.equal(true) + }) + + it('all returned bibs should contain callnumber = "MGZMD"', async () => { + const callnumber = 'MGZMD' + + const res = await search({ + q: `callnumber = "${callnumber}"` + }) + + expect(res.body.itemListElement).to.be.an('array') + + // Assert at least one result + expect(res.body.itemListElement.length).to.be.greaterThan(0) + + // Assert all results contain the target call number + res.body.itemListElement.forEach((item, idx) => { + const result = item.result + + expect(resultContainsCallnumber(result, callnumber)).to.equal(true) + }) + }) + + it('should return exactly one result for identifier = "b10670401"', async () => { + const identifier = 'b10670401' + + const res = await search({ + q: `identifier = "${identifier}"` + }) + + expect(res.body.itemListElement).to.be.an('array') + + // Assert exactly one result + expect(res.body.itemListElement.length).to.equal(1) + + // Assert the result contains the target identifier + const result = res.body.itemListElement[0].result + expect(result['@id']).to.equal(`res:${identifier}`) + }) +}) diff --git a/test/resources.test.js b/test/resources.test.js index f7e86c9b..5818dcce 100644 --- a/test/resources.test.js +++ b/test/resources.test.js @@ -5,6 +5,12 @@ const scsbClient = require('../lib/scsb-client') const errors = require('../lib/errors') const { AGGREGATIONS_SPEC } = require('../lib/elasticsearch/config') const numAggregations = Object.keys(AGGREGATIONS_SPEC).length +const { + itemsFilterContext, + itemsQueryContext, + buildElasticQuery, + buildElasticBody +} = require('../lib/elasticsearch/elastic-body-builder') const fixtures = require('./fixtures') const { verifyFilterFields } = require('./utils') @@ -73,7 +79,7 @@ describe('Resources query', function () { describe('buildElasticQuery', function () { it('uses "query string query" if subjectLiteral: used', function () { const params = resourcesPrivMethods.parseSearchParams({ q: 'subjectLiteral:potatoes' }) - const body = resourcesPrivMethods.buildElasticQuery(params) + const body = buildElasticQuery(params) expect(body).to.be.a('object') expect(body.bool).to.be.a('object') expect(body.bool.must).to.be.a('array') @@ -84,7 +90,7 @@ describe('Resources query', function () { it('uses "query string query" if subjectLiteral: quoted phrase used', function () { const params = resourcesPrivMethods.parseSearchParams({ q: 'subjectLiteral:"hot potatoes"' }) - const body = resourcesPrivMethods.buildElasticQuery(params) + const body = buildElasticQuery(params) expect(body).to.be.a('object') expect(body.bool).to.be.a('object') expect(body.bool.must).to.be.a('array') @@ -95,7 +101,7 @@ describe('Resources query', function () { it('escapes colon if field not recognized', function () { const params = resourcesPrivMethods.parseSearchParams({ q: 'fladeedle:"hot potatoes"' }) - const body = resourcesPrivMethods.buildElasticQuery(params) + const body = buildElasticQuery(params) expect(body).to.be.a('object') expect(body.bool).to.be.a('object') expect(body.bool.must).to.be.a('array') @@ -106,7 +112,7 @@ describe('Resources query', function () { it('uses "query string query" if plain keyword query used', function () { const params = resourcesPrivMethods.parseSearchParams({ q: 'potatoes' }) - const body = resourcesPrivMethods.buildElasticQuery(params) + const body = buildElasticQuery(params) expect(body).to.be.a('object') expect(body.bool).to.be.a('object') expect(body.bool.must).to.be.a('array') @@ -117,7 +123,7 @@ describe('Resources query', function () { it('accepts advanced search parameters', function () { const params = resourcesPrivMethods.parseSearchParams({ contributor: 'Poe', title: 'Raven', subject: 'ravens' }) - const body = resourcesPrivMethods.buildElasticQuery(params) + const body = buildElasticQuery(params) expect(body).to.nested.include({ // Expect a title match on Raven: @@ -136,7 +142,7 @@ describe('Resources query', function () { describe('buildElasticBody', function () { it('uses property specified in filter config when given a subjectLiteral filter', function () { const params = resourcesPrivMethods.parseSearchParams({ q: '', filters: { subjectLiteral: 'United States -- History' } }) - const body = resourcesPrivMethods.buildElasticBody(params) + const body = buildElasticBody(params) expect(body).to.be.a('object') expect(body.query).to.be.a('object') expect(body.query.bool).to.be.a('object') @@ -151,7 +157,7 @@ describe('Resources query', function () { expect(process.env.HIDE_NYPL_SOURCE).to.be.a('undefined') const params = resourcesPrivMethods.parseSearchParams({ q: '' }) - const body = resourcesPrivMethods.buildElasticBody(params) + const body = buildElasticBody(params) expect(body).to.be.a('object') expect(body.query.filter).to.be.a('undefined') @@ -161,7 +167,7 @@ describe('Resources query', function () { process.env.HIDE_NYPL_SOURCE = 'recap-hl' const params = resourcesPrivMethods.parseSearchParams({ q: '' }) - const body = resourcesPrivMethods.buildElasticBody(params) + const body = buildElasticBody(params) // Expect query to resemble: {"from":0,"size":50,"query":{"bool":{"filter":[{"bool":{"must_not":{"terms":{"nyplSource":["recap-hl"]}}}}]}},"sort":["uri"]} expect(body).to.be.a('object') @@ -173,26 +179,26 @@ describe('Resources query', function () { it('processes isbn correctly', () => { const params = resourcesPrivMethods.parseSearchParams({ isbn: '0689844921' }) - const body = resourcesPrivMethods.buildElasticBody(params) + const body = buildElasticBody(params) expect(body).to.nested .include({ 'query.bool.must[0].term.idIsbn\\.clean': '0689844921' }) }) it('processes issn correctly', () => { const params = resourcesPrivMethods.parseSearchParams({ issn: '1234-5678' }) - const body = resourcesPrivMethods.buildElasticBody(params) + const body = buildElasticBody(params) expect(body).to.nested.include({ 'query.bool.must[0].term.idIssn\\.clean': '1234-5678' }) }) it('processes lccn correctly', () => { const params = resourcesPrivMethods.parseSearchParams({ lccn: '00068799' }) - const body = resourcesPrivMethods.buildElasticBody(params) + const body = buildElasticBody(params) expect(body).to.nested.include({ 'query.bool.must[0].regexp.idLccn.value': '[^\\d]*00068799[^\\d]*' }) }) it('processes oclc correctly', () => { const params = resourcesPrivMethods.parseSearchParams({ oclc: '1033548057' }) - const body = resourcesPrivMethods.buildElasticBody(params) + const body = buildElasticBody(params) expect(body).to.nested.include({ 'query.bool.must[0].term.idOclc': '1033548057' }) }) @@ -205,9 +211,9 @@ describe('Resources query', function () { }) const paramsSnapshot = JSON.stringify(params) - resourcesPrivMethods.buildElasticBody(params) - resourcesPrivMethods.buildElasticBody(params) - resourcesPrivMethods.buildElasticBody(params) + buildElasticBody(params) + buildElasticBody(params) + buildElasticBody(params) expect(JSON.stringify(params)).to.equal(paramsSnapshot) }) @@ -468,7 +474,8 @@ describe('Resources query', function () { size: 1, query: { bool: { - must: [{ term: { uri: 'b1234' } }] + must: [{ term: { uri: 'b1234' } }], + filter: [] } }, aggregations: { @@ -649,40 +656,40 @@ describe('Resources query', function () { describe('itemsFilterContext', () => { it('should return an empty object in case of no query', () => { - expect(resourcesPrivMethods.itemsFilterContext({})).to.deep.equal({}) + expect(itemsFilterContext({})).to.deep.equal({}) }) it('should return an empty object in case there are no filters', () => { - expect(resourcesPrivMethods.itemsFilterContext({ query: {} })).to.deep.equal({}) + expect(itemsFilterContext({ query: {} })).to.deep.equal({}) }) it('should return filters for volume in case there is a volume', () => { - expect(resourcesPrivMethods.itemsFilterContext({ query: { volume: [1, 2] } })) + expect(itemsFilterContext({ query: { volume: [1, 2] } })) .to.deep.equal({ filter: [{ range: { 'items.volumeRange': { gte: 1, lte: 2 } } }] }) }) it('should return filters for date in case there is a date', () => { - expect(resourcesPrivMethods.itemsFilterContext({ query: { date: [1, 2] } })) + expect(itemsFilterContext({ query: { date: [1, 2] } })) .to.deep.equal({ filter: [{ range: { 'items.dateRange': { gte: 1, lte: 2 } } }] }) }) it('should return filters for format in case there is a format', () => { - expect(resourcesPrivMethods.itemsFilterContext({ query: { format: ['text', 'microfilm', 'AV'] } })) + expect(itemsFilterContext({ query: { format: ['text', 'microfilm', 'AV'] } })) .to.deep.equal({ filter: [{ terms: { 'items.formatLiteral': ['text', 'microfilm', 'AV'] } }] }) }) it('should return filters for location in case there is a location', () => { - expect(resourcesPrivMethods.itemsFilterContext({ query: { location: ['SASB', 'LPA', 'Schomburg'] } })) + expect(itemsFilterContext({ query: { location: ['SASB', 'LPA', 'Schomburg'] } })) .to.deep.equal({ filter: [{ terms: { 'items.holdingLocation.id': ['SASB', 'LPA', 'Schomburg'] } }] }) }) it('should return filters for status in case there is a status', () => { - expect(resourcesPrivMethods.itemsFilterContext({ query: { status: ['Available', 'Unavailable', 'In Process'] } })) + expect(itemsFilterContext({ query: { status: ['Available', 'Unavailable', 'In Process'] } })) .to.deep.equal({ filter: [{ terms: { 'items.status.id': ['Available', 'Unavailable', 'In Process'] } }] }) }) it('should combine all filters in case of multiple filters', () => { - expect(resourcesPrivMethods.itemsFilterContext({ + expect(itemsFilterContext({ query: { volume: [1, 2], date: [3, 4], @@ -702,160 +709,28 @@ describe('Resources query', function () { }) it('should ignore all other parameters', () => { - expect(resourcesPrivMethods.itemsFilterContext({ query: { location: ['SASB', 'LPA', 'Schomburg'] }, something: 'else' })) + expect(itemsFilterContext({ query: { location: ['SASB', 'LPA', 'Schomburg'] }, something: 'else' })) .to.deep.equal({ filter: [{ terms: { 'items.holdingLocation.id': ['SASB', 'LPA', 'Schomburg'] } }] }) }) }) describe('itemsQueryContext', () => { it('should exclude check in card items when options.merge_checkin_card_items is not set', () => { - expect(resourcesPrivMethods.itemsQueryContext({})) + expect(itemsQueryContext({})) .to.deep.equal({ must_not: [{ term: { 'items.type': 'nypl:CheckinCardItem' } }] }) }) it('should exclude check in card items when merge_checkin_card_items is falsey', () => { - expect(resourcesPrivMethods.itemsQueryContext({ merge_checkin_card_items: false })) + expect(itemsQueryContext({ merge_checkin_card_items: false })) .to.deep.equal({ must_not: [{ term: { 'items.type': 'nypl:CheckinCardItem' } }] }) }) it('should use match_all for items when merge_checkin_card_items is truthy', () => { - expect(resourcesPrivMethods.itemsQueryContext({ merge_checkin_card_items: true })) + expect(itemsQueryContext({ merge_checkin_card_items: true })) .to.deep.equal({ must: { match_all: {} } }) }) }) - describe('addInnerHits', () => { - it('should include query for items', () => { - expect(resourcesPrivMethods.addInnerHits({ query: { bool: {} } }, { size: 1, from: 2 })) - .to.deep.equal({ - query: { - bool: { - filter: [ - { - bool: { - should: [ - { - nested: { - path: 'items', - query: { - bool: { - must: { - match_all: {} - } - } - }, - inner_hits: { - sort: [{ 'items.enumerationChronology_sort': 'desc' }], - size: 1, - from: 2, - name: 'items' - } - } - }, - { match_all: {} } - ] - } - } - ] - } - } - }) - }) - - it('should exclude check in card items if explicitly set', () => { - expect(resourcesPrivMethods.addInnerHits({ query: { bool: {} } }, { size: 1, from: 2, merge_checkin_card_items: false })) - .to.deep.equal({ - query: { - bool: { - filter: [ - { - bool: { - should: [ - { - nested: { - path: 'items', - query: { - bool: { - must_not: [ - { term: { 'items.type': 'nypl:CheckinCardItem' } } - ] - } - }, - inner_hits: { - sort: [{ 'items.enumerationChronology_sort': 'desc' }], - size: 1, - from: 2, - name: 'items' - } - } - }, - { match_all: {} } - ] - } - } - ] - } - } - }) - }) - - it('should include filters for items', () => { - expect(resourcesPrivMethods.addInnerHits( - { query: { bool: {} } }, - { size: 1, from: 2, query: { volume: [1, 2], location: ['SASB', 'LPA'], other: 'filter' } } - )).to.deep.equal({ - query: { - bool: { - filter: [ - { - bool: { - should: [ - { - nested: { - path: 'items', - query: { - bool: { - must: { - match_all: {} - }, - filter: [ - { range: { 'items.volumeRange': { gte: 1, lte: 2 } } }, - { terms: { 'items.holdingLocation.id': ['SASB', 'LPA'] } } - ] - } - }, - inner_hits: { - sort: [{ 'items.enumerationChronology_sort': 'desc' }], - size: 1, - from: 2, - name: 'items' - } - } - }, - { match_all: {} }, - { - nested: { - inner_hits: { name: 'allItems' }, - path: 'items', - query: { - bool: { - must_not: [ - { exists: { field: 'items.electronicLocator' } } - ] - } - } - } - } - ] - } - } - ] - } - } - }) - }) - }) - describe('search exception handling', () => { describe('lexical error', () => { before(() => {