main -> qa refactor

.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

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.
-

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"
   }
-]
+]

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
 }

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
+}