Merge pull request #384 from NYPL/scc-4050

Make app robust to scsb api issues

lib/availability_resolver.js

@@ -22,6 +22,22 @@ class AvailabilityResolver {
     this.barcodes = this._parseBarCodesFromESResponse()
   }
 
+  /**
+  * Given a map relating status strings to arrays of barcodes,
+  * returns a map relating each barcode to a status string.
+  */
+  static invertBarcodeByStatusMapping (barcodesByStatus) {
+    if (!barcodesByStatus || typeof barcodesByStatus !== 'object') {
+      return {}
+    }
+    return Object.keys(barcodesByStatus)
+      .reduce((h, status) => {
+        const barcodeToStatusPairs = barcodesByStatus[status]
+          .map((barcode) => ({ [barcode]: status }))
+        return Object.assign(h, ...barcodeToStatusPairs)
+      }, {})
+  }
+
   // returns an updated elasticSearchResponse with the newest availability info from SCSB
   responseWithUpdatedAvailability (options = {}) {
     // If this serialization is a result of a hold request initializing, we want
@@ -30,12 +46,21 @@ class AvailabilityResolver {
       ? () => this._checkScsbForRecapCustomerCode()
       : () => Promise.resolve()
 
+    // When options.recapBarcodesByStatus is set, we can use it in place of
+    // re-querying status by barcode:
+    const barcodeToStatusMap = async () => {
+      if (options.recapBarcodesByStatus) {
+        // Invert mapping to map barcodes to statuses:
+        return AvailabilityResolver.invertBarcodeByStatusMapping(options.recapBarcodesByStatus)
+      } else {
+        return this._createSCSBBarcodeAvailbilityMapping(this.barcodes)
+      }
+    }
+
     // Get 1) barcode-availability mapping and 2) customer code query in
     // parallel because they don't depend on each other:
     return Promise.all([
-      // TODO: When options.recapBarcodesByStatus is set, we should be able to
-      // use it in place of re-querying status by barcode:
-      this._createSCSBBarcodeAvailbilityMapping(this.barcodes),
+      barcodeToStatusMap(),
       updateRecapCustomerCodes()
     ])
       .then((barcodeMappingAndCustomerCodeResult) => {
@@ -181,22 +206,30 @@ class AvailabilityResolver {
   /**
    *  Given an array of barcodes, returns a hash mapping barcode to SCSB availability
    */
-  _createSCSBBarcodeAvailbilityMapping (barcodes) {
+  async _createSCSBBarcodeAvailbilityMapping (barcodes) {
     if (barcodes.length === 0) {
-      return Promise.resolve({})
+      return {}
     }
-    return scsbClient.getItemsAvailabilityForBarcodes(this.barcodes)
-      .then((itemsStatus) => {
-        if (!Array.isArray(itemsStatus)) {
-          logger.warn(`Got bad itemAvailabilityStatus response from SCSB for barcodes (${barcodes}): ${JSON.stringify(itemsStatus)}`)
-          return {}
-        }
-        const barcodesAndAvailability = {}
-        itemsStatus.forEach((statusEntry) => {
-          barcodesAndAvailability[statusEntry.itemBarcode] = statusEntry.itemAvailabilityStatus
-        })
-        return barcodesAndAvailability
-      })
+    let itemsStatus
+    try {
+      itemsStatus = await scsbClient.getItemsAvailabilityForBarcodes(this.barcodes)
+    } catch (e) {
+      logger.warn(`Error retrieving SCSB statuses for barcodes: ${e}`)
+      return {}
+    }
+
+    if (!Array.isArray(itemsStatus)) {
+      logger.warn(`Got bad itemAvailabilityStatus response from SCSB for barcodes (${barcodes}): ${JSON.stringify(itemsStatus)}`)
+      return {}
+    }
+
+    // Convert SCSB API response into barcode => status map:
+    return itemsStatus
+      // Verify the entries have the properties we expect:
+      .filter((entry) => entry.itemBarcode && entry.itemAvailabilityStatus)
+      .reduce((h, entry) => {
+        return Object.assign(h, { [entry.itemBarcode]: entry.itemAvailabilityStatus })
+      }, {})
   }
 
   _parseBarCodesFromESResponse () {

lib/delivery-locations-resolver.js

@@ -7,15 +7,20 @@ const onsiteEddCriteria = require('../data/onsite-edd-criteria.json')
 const { isItemNyplOwned } = require('./ownership_determination')
 
 class DeliveryLocationsResolver {
+  static nyplCoreLocation (locationCode) {
+    return sierraLocations[locationCode]
+  }
+
   static requestableBasedOnHoldingLocation (item) {
-    // Is this not requestable because of its holding location?
-    try {
-      const holdingLocationSierraCode = item.holdingLocation[0].id.split(':').pop()
-      return sierraLocations[holdingLocationSierraCode].requestable
-    } catch (e) {
-      logger.warn('There is an item in the index with missing or malformed holdingLocation', item)
+    const locationCode = this.extractLocationCode(item)
+
+    if (!DeliveryLocationsResolver.nyplCoreLocation(locationCode)) {
+      logger.warn(`DeliveryLocationsResolver: Unrecognized holdingLocation for ${item.uri}: ${locationCode}`)
       return false
     }
+
+    // Is this not requestable because of its holding location?
+    return DeliveryLocationsResolver.nyplCoreLocation(locationCode).requestable
   }
 
   // Currently, there is no physical delivery requests for onsite items through Discovery API
@@ -24,11 +29,11 @@ class DeliveryLocationsResolver {
     // If holdingLocation given, strip code from @id for lookup:
     const locationCode = holdingLocation && holdingLocation.id ? holdingLocation.id.replace(/^loc:/, '') : null
     // Is Sierra location code mapped?
-    if (sierraLocations[locationCode] && sierraLocations[locationCode].sierraDeliveryLocations) {
+    if (DeliveryLocationsResolver.nyplCoreLocation(locationCode)?.sierraDeliveryLocations) {
       // It's mapped, but the sierraDeliveryLocation entities only have `code` and `label`
       // Do a second lookup to populate `deliveryLocationTypes`
-      return sierraLocations[locationCode].sierraDeliveryLocations.map((deliveryLocation) => {
-        deliveryLocation.deliveryLocationTypes = sierraLocations[deliveryLocation.code].deliveryLocationTypes
+      return DeliveryLocationsResolver.nyplCoreLocation(locationCode).sierraDeliveryLocations.map((deliveryLocation) => {
+        deliveryLocation.deliveryLocationTypes = DeliveryLocationsResolver.nyplCoreLocation(deliveryLocation.code).deliveryLocationTypes
         return deliveryLocation
       })
       // Either holdingLocation is null or code not matched; Fall back on mocked data:
@@ -141,11 +146,12 @@ class DeliveryLocationsResolver {
   }
 
   static extractLocationCode (item) {
-    try {
-      return item.holdingLocation[0].id.split(':').pop()
-    } catch (e) {
-      logger.warn('There is an item in the index with missing or malformed holdingLocation', item)
+    if (!Array.isArray(item.holdingLocation)) {
+      logger.warn(`DeliveryLocationsResolver#extractLocationCode: Item missing holdingLocation: ${item.uri}`)
+      return false
     }
+
+    return item.holdingLocation[0]?.id?.split(':').pop()
   }
 
   static sortPosition (location) {
@@ -224,7 +230,7 @@ class DeliveryLocationsResolver {
       deliveryLocation: []
     }
     const holdingLocationCode = this.extractLocationCode(item)
-    const sierraData = sierraLocations[holdingLocationCode]
+    const sierraData = DeliveryLocationsResolver.nyplCoreLocation(holdingLocationCode)
     if (!sierraData) {
       // This case is mainly to satisfy a test which wants eddRequestable = false
       // for a made up location code.