Add support for callnumber= param

NYPL · Nov 18, 2024 · 68de1c2 · 68de1c2
1 parent ef2d69f
commit 68de1c2
Show file tree

Hide file tree

Showing 70 changed files with 57,863 additions and 55,445 deletions.
diff --git a/lib/api-request.js b/lib/api-request.js
@@ -1,13 +1,14 @@
 /**
 * This class wraps request params to ease interpretting the query
 **/
+const { SEARCH_SCOPES } = require('./elasticsearch/config')
 
 // Build regex pattern for matching a phrase fully enclosed in quotes (or smart quotes):
 const QUOTE_CHARS = '"\u201C\u201D\u201E\u201F\u2033\u2036'
 const IN_QUOTES_PATTERN = new RegExp(`^[${QUOTE_CHARS}][^${QUOTE_CHARS}]+[${QUOTE_CHARS}]$`)
 
 class ApiRequest {
-  static ADVANCED_SEARCH_PARAMS = ['title', 'subject', 'contributor']
+  static ADVANCED_SEARCH_PARAMS = ['title', 'subject', 'contributor', 'callnumber']
   static IDENTIFIER_NUMBER_PARAMS = ['isbn', 'issn', 'lccn', 'oclc']
 
   constructor (params) {
@@ -28,6 +29,7 @@ class ApiRequest {
   advancedSearchParamsThatAreAlsoScopes () {
     // Return search params that are also valid search_scope values
     return ApiRequest.ADVANCED_SEARCH_PARAMS
+      .filter((key) => SEARCH_SCOPES[key])
       .filter((key) => this.params[key])
   }
 

diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js
@@ -1,4 +1,5 @@
 const ElasticQuery = require('./elastic-query')
+const ApiRequest = require('../api-request')
 const { escapeQuery, namedQuery, prefixMatch, termMatch, phraseMatch } = require('./utils')
 const { regexEscape } = require('../util')
 
@@ -60,17 +61,6 @@ class ElasticQueryBuilder {
       this.requireMultiMatch(SEARCH_SCOPES.all.fields)
     }
 
-    // Coming from Adv Search? Add additional multi-match clauses for subject,
-    // contributor, title:
-    if (this.request.advancedSearchParamsThatAreAlsoScopes()) {
-      this.request
-        .advancedSearchParamsThatAreAlsoScopes()
-        .forEach((param) =>
-          // Require a match on subject, contributor, or title fields:
-          this.requireMultiMatch(SEARCH_SCOPES[param].fields, this.request.params[param])
-        )
-    }
-
     if (this.request.hasSearch()) {
       // Apply common boosting:
       this.boostNyplOwned()
@@ -90,6 +80,9 @@ class ElasticQueryBuilder {
       }
     }
 
+    // Look for query params coming from Adv Search:
+    this.applyAdvancedSearchParams()
+
     // Lastly, if any identifier-number params are present (lccn=, isbn=, etc),
     // add those clauses:
     if (this.request.hasIdentifierNumberParam()) {
@@ -464,6 +457,37 @@ class ElasticQueryBuilder {
     this.query.addMust(should.length === 1 ? should[0] : { bool: { should } })
   }
 
+  /**
+  * Handle use of subject=, contributor=, title=, & callnumber= Adv Search
+  * params.
+  * Note that the RC Adv Search page may also apply filters, which are
+  * handled by `applyFilters`.
+  **/
+  applyAdvancedSearchParams () {
+    // We're specifically interested in params that match supported search-
+    // scopes because we can build a whole ES query just using the existing
+    // logic for that search scope:
+    if (this.request.advancedSearchParamsThatAreAlsoScopes()) {
+      this.request
+        .advancedSearchParamsThatAreAlsoScopes()
+        .forEach((advSearchParam) => {
+          const advSearchValue = this.request.params[advSearchParam]
+          // Build a new ApiRequest object for this search-scope:
+          const request = ApiRequest.fromParams({
+            q: advSearchValue,
+            search_scope: advSearchParam
+          })
+
+          // Build the ES query for the search-scope and value:
+          const builder = ElasticQueryBuilder.forApiRequest(request)
+          const subquery = builder.query.toJson()
+
+          // Add the query to the greater ES query's must clauses:
+          this.query.addMust(subquery)
+        })
+    }
+  }
+
   /**
   * Examine request for user-filters. When found, add them to query.
   */

diff --git a/lib/resources.js b/lib/resources.js
@@ -87,6 +87,7 @@ const parseSearchParams = function (params) {
     filters: { type: 'hash', fields: FILTER_CONFIG },
     items_size: { type: 'int', default: 100, range: [0, 200] },
     items_from: { type: 'int', default: 0 },
+    callnumber: { type: 'string' },
     contributor: { type: 'string' },
     title: { type: 'string' },
     subject: { type: 'string' },

diff --git a/lib/util.js b/lib/util.js
@@ -177,12 +177,6 @@ exports.parseParam = function (val, spec) {
   return val
 }
 
-exports.arrayIntersection = (a1, a2) => {
-  return a1.filter(function (n) {
-    return a2.indexOf(n) !== -1
-  })
-}
-
 /**
  * Get array of key-value pairs for object
  *
@@ -193,19 +187,6 @@ exports.objectEntries = (obj) => {
     .map((key) => [key, obj[key]])
 }
 
-exports.gatherParams = function (req, acceptedParams) {
-  // If specific params configured, pass those to handler
-  // otherwise just pass `value` param (i.e. keyword search)
-  acceptedParams = (typeof acceptedParams === 'undefined') ? ['page', 'per_page', 'value', 'q', 'filters', 'contributor', 'subject', 'title', 'isbn', 'issn', 'lccn', 'oclc', 'merge_checkin_card_items', 'include_item_aggregations'] : acceptedParams
-
-  const params = {}
-  acceptedParams.forEach((k) => {
-    params[k] = req.query[k]
-  })
-  if (req.query.q) params.value = req.query.q
-  return params
-}
-
 /*
  * Expects array of strings, numbers
  */

diff --git a/routes/resources.js b/routes/resources.js
@@ -1,5 +1,3 @@
-const gatherParams = require('../lib/util').gatherParams
-
 const VER = '0.1'
 
 module.exports = function (app) {
@@ -11,29 +9,6 @@ module.exports = function (app) {
     next()
   })
 
-  const standardParams = ['page',
-    'per_page',
-    'q',
-    'filters',
-    'expandContext',
-    'ext',
-    'field',
-    'sort',
-    'sort_direction',
-    'search_scope',
-    'all_items',
-    'items_size',
-    'items_from',
-    'contributor',
-    'title',
-    'subject',
-    'isbn',
-    'issn',
-    'lccn',
-    'oclc',
-    'merge_checkin_card_items',
-    'include_item_aggregations']
-
   const respond = (res, _resp, params) => {
     let contentType = 'application/ld+json'
     if (params.ext === 'ntriples') contentType = 'text/plain'
@@ -65,23 +40,23 @@ module.exports = function (app) {
   }
 
   app.get(`/api/v${VER}/discovery/resources$`, function (req, res) {
-    const params = gatherParams(req, standardParams)
+    const params = req.query
 
     return app.resources.search(params, { baseUrl: app.baseUrl }, req)
       .then((resp) => respond(res, resp, params))
       .catch((error) => handleError(res, error, params))
   })
 
   app.get(`/api/v${VER}/discovery/resources/aggregations`, function (req, res) {
-    const params = gatherParams(req, standardParams)
+    const params = req.query
 
     return app.resources.aggregations(params, { baseUrl: app.baseUrl })
       .then((resp) => respond(res, resp, params))
       .catch((error) => handleError(res, error, params))
   })
 
   app.get(`/api/v${VER}/discovery/resources/aggregation/:field`, function (req, res) {
-    const params = Object.assign({}, gatherParams(req, standardParams), req.params)
+    const params = req.query
 
     return app.resources.aggregation(params, { baseUrl: app.baseUrl })
       .then((resp) => respond(res, resp, params))
@@ -95,11 +70,11 @@ module.exports = function (app) {
    *   /api/v${VER}/request/deliveryLocationsByBarcode?barcodes[]=12345&barcodes[]=45678&barcodes=[]=78910
    */
   app.get(`/api/v${VER}/request/deliveryLocationsByBarcode`, function (req, res) {
-    const params = gatherParams(req, ['barcodes', 'patronId'])
+    const params = req.query
 
     const handler = app.resources.deliveryLocationsByBarcode
 
-    return handler(params, { baseUrl: app.baseUrl })
+    return handler(req.query.params, { baseUrl: app.baseUrl })
       .then((resp) => respond(res, resp, params))
       .catch((error) => handleError(res, error, params))
   })
@@ -125,7 +100,7 @@ module.exports = function (app) {
    * e.g. discovery/resources/b1234
    */
   app.get(`/api/v${VER}/discovery/resources/:uri.:ext?`, function (req, res) {
-    const gatheredParams = gatherParams(req, ['uri', 'items_size', 'items_from', 'merge_checkin_card_items', 'include_item_aggregations', 'all_items'])
+    const gatheredParams = req.query
     const params = Object.assign({}, req.query, { uri: req.params.uri })
 
     if (Number.isInteger(parseInt(gatheredParams.items_size))) params.items_size = gatheredParams.items_size

diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js
@@ -138,4 +138,129 @@ describe('ElasticQueryBuilder', () => {
         .include({ 'bool.filter[0].term.buildingLocationIds': 'ma' })
     })
   })
+
+  describe('Advanced Search query params', () => {
+    describe('callnumber=', () => {
+      it('applies callnumber clauses to query', () => {
+        const request = new ApiRequest({ callnumber: 'toast' })
+        const inst = ElasticQueryBuilder.forApiRequest(request)
+
+        expect(inst.query.toJson()).to.nested
+          .include({
+            // Match on bib shelfmark:
+            'bool.must[0].bool.must[0].bool.should[0].prefix.shelfMark\\.keywordLowercased.value': 'toast',
+            // Match on item shelfmark:
+            'bool.must[0].bool.must[0].bool.should[1].nested.path': 'items',
+            'bool.must[0].bool.must[0].bool.should[1].nested.query.prefix.items\\.shelfMark\\.keywordLowercased.value': 'toast'
+          })
+      })
+    })
+
+    describe('title=', () => {
+      it('applies title clauses to query', () => {
+        const request = new ApiRequest({ title: 'toast' })
+        const inst = ElasticQueryBuilder.forApiRequest(request)
+
+        // console.log('ES: ', JSON.stringify(inst.query.toJson(), null, 2))
+        const query = inst.query.toJson()
+
+        // Assert there's a multi-match:
+        expect(query).to.nested
+          .include({
+            // Multi-match on common title fields:
+            'bool.must[0].bool.must[0].multi_match.fields[0]': 'title^5',
+            'bool.must[0].bool.must[0].multi_match.query': 'toast'
+          })
+        // Assert there's at least one of the title boosting clauses:
+        const titleShoulds = query.bool.must[0].bool.should
+        const prefixMatch = titleShoulds.find((should) => should.prefix)
+        expect(prefixMatch).to.deep.equal({
+          prefix: {
+            'title.keywordLowercasedStripped': {
+              value: 'toast',
+              boost: 50
+            }
+          }
+        })
+      })
+    })
+
+    describe('contributor=', () => {
+      it('applies contributor clauses to query', () => {
+        const request = new ApiRequest({ contributor: 'toast' })
+        const inst = ElasticQueryBuilder.forApiRequest(request)
+
+        const query = inst.query.toJson()
+
+        // Assert there's a multi-match:
+        expect(query).to.nested
+          .include({
+            // Multi-match on common creator/contrib fields:
+            'bool.must[0].bool.must[0].multi_match.fields[0]': 'creatorLiteral^4',
+            'bool.must[0].bool.must[0].multi_match.query': 'toast'
+          })
+        // Assert there's at least one of the creator boosting clauses:
+        const contributorShoulds = query.bool.must[0].bool.should
+        const prefixMatch = contributorShoulds.find((should) => should.prefix)
+        expect(prefixMatch).to.deep.equal({
+          prefix: {
+            'creatorLiteralNormalized.keywordLowercased': {
+              value: 'toast',
+              boost: 100
+            }
+          }
+        })
+      })
+    })
+
+    describe('multiple adv search params', () => {
+      it('applies contributor clauses to query', () => {
+        const request = new ApiRequest({
+          title: 'title value',
+          contributor: 'contributor value',
+          callnumber: 'callnumber value'
+        })
+        const inst = ElasticQueryBuilder.forApiRequest(request)
+
+        console.log('ES: ', JSON.stringify(inst.query.toJson(), null, 2))
+        const query = inst.query.toJson()
+
+        // Assert there's a multi-match:
+        expect(query).to.nested
+          .include({
+            // Multi-match on title fields:
+            'bool.must[0].bool.must[0].multi_match.fields[0]': 'title^5',
+            'bool.must[0].bool.must[0].multi_match.query': 'title value',
+
+            // Multi-match on creator/contrib fields:
+            'bool.must[1].bool.must[0].multi_match.fields[0]': 'creatorLiteral^4',
+            'bool.must[1].bool.must[0].multi_match.query': 'contributor value'
+          })
+
+        // Assert there's at least one of the title boosting clauses:
+        const titleShoulds = query.bool.must[0].bool.should
+        const prefixMatch = titleShoulds.find((should) => should.prefix)
+        expect(prefixMatch).to.deep.equal({
+          prefix: {
+            'title.keywordLowercasedStripped': {
+              value: 'title value',
+              boost: 50
+            }
+          }
+        })
+
+        // Assert there's at least one of the creator boosting clauses:
+        const creatorShoulds = query.bool.must[1].bool.should
+        const creatorPrefixMatch = creatorShoulds.find((should) => should.prefix)
+        expect(creatorPrefixMatch).to.deep.equal({
+          prefix: {
+            'creatorLiteralNormalized.keywordLowercased': {
+              value: 'contributor value',
+              boost: 100
+            }
+          }
+        })
+      })
+    })
+  })
 })