Make app robust to scsb api issues

Ensure that, if connecting to the SCSB API raises an error or responds
unexpectedly when querying for item statuses, we set offsite item statuses to
'Not available' and proceed with other work.

Includes light refactoring to improve readability

https://jira.nypl.org/browse/SCC-4050

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.