Merge pull request #121 from NYPL-discovery/pb/improved-patron-servic…

…e-mocking

Remove dupe api-client instance. Improved mocking

.env.example

@@ -15,4 +15,4 @@ NYPL_API_BASE_URL=http://path-to-our-api-gateway.example.com/api/v0.1/
 NYPL_OAUTH_URL=https://url-to-our-oauth-server-including-slash.example.com/
 NYPL_OAUTH_ID=who-you-will-connect-to-the-api-as
 NYPL_OAUTH_SECRET=that-accounts-pw
-NYPL_CORE_VERSION=mapping-version
+NYPL_CORE_VERSION=v1.21

README.md

@@ -78,7 +78,7 @@ The following runs a series of tests against local fixtures:
 npm test
 ```
 
-Most tests rely on fixtures generated dynamically (using whatever elastic config is present in process.env or ./.env) via the following:
+Almost all HTTP dependencies are rerouted to fixtures (nypl-core mapping files are a known exception). All fixtures can be updated dynamically (using whatever elastic, scsb, & platform api config is present in `process.env` or `./.env`) via the following:
 
 ```
 UPDATE_FIXTURES=all npm test
@@ -90,6 +90,8 @@ Rebuilding fixtures tends to introduce trivial git diff noise, so one may use th
 UPDATE_FIXTURES=if-missing npm test
 ```
 
+The above command can be used to fill in missing fixtures as new tests are written.
+
 ## Deployment
 
 `npm run deploy-[development|qa|production]`

lib/available_delivery_location_types.js

@@ -1,4 +1,6 @@
 let logger = require('./logger')
+const { makeNyplDataApiClient } = require('./data-api-client')
+
 class AvailableDeliveryLocationTypes {
 
   static getByPatronId (patronID) {
@@ -15,7 +17,7 @@ class AvailableDeliveryLocationTypes {
     let __start = new Date()
 
     let apiURL = `patrons/${patronID}`
-    return client.get(apiURL).then((response) => {
+    return makeNyplDataApiClient().get(apiURL).then((response) => {
       logger.debug(`AvailableDeliveryLocationTypes: response from patron service ${apiURL}: ${JSON.stringify(response, null, 2)}`)
       let patronType = response.data.fixedFields['47']['value']
 
@@ -34,18 +36,8 @@ class AvailableDeliveryLocationTypes {
 
 }
 
-let NyplClient = require('@nypl/nypl-data-api-client')
-
-let client = new NyplClient({
-  base_url: process.env.NYPL_API_BASE_URL,
-  oauth_key: process.env.NYPL_OAUTH_ID,
-  oauth_secret: process.env.NYPL_OAUTH_SECRET,
-  oauth_url: process.env.NYPL_OAUTH_URL
-})
-
 let patronTypeMapping = require('@nypl/nypl-core-objects')('by-patron-type')
 
-AvailableDeliveryLocationTypes.client = client
 AvailableDeliveryLocationTypes.patronTypeMapping = patronTypeMapping
 
 module.exports = AvailableDeliveryLocationTypes

lib/data-api-client.js

@@ -0,0 +1,17 @@
+let nyplDataApiClient = null
+
+function makeNyplDataApiClient () {
+  if (!nyplDataApiClient) {
+    const NyplClient = require('@nypl/nypl-data-api-client')
+
+    nyplDataApiClient = new NyplClient({
+      base_url: process.env.NYPL_API_BASE_URL,
+      oauth_key: process.env.NYPL_OAUTH_ID,
+      oauth_secret: process.env.NYPL_OAUTH_SECRET,
+      oauth_url: process.env.NYPL_OAUTH_URL
+    })
+  }
+  return nyplDataApiClient
+}
+
+module.exports = { makeNyplDataApiClient }

lib/resources.js

@@ -5,6 +5,7 @@ var AggregationSerializer = require('./jsonld_serializers.js').AggregationSerial
 var ItemResultsSerializer = require('./jsonld_serializers.js').ItemResultsSerializer
 const LocationLabelUpdater = require('./location_label_updater')
 const AnnotatedMarcSerializer = require('./annotated-marc-serializer')
+const { makeNyplDataApiClient } = require('./data-api-client')
 
 const ResponseMassager = require('./response_massager.js')
 var DeliveryLocationsResolver = require('./delivery-locations-resolver')
@@ -118,22 +119,6 @@ var parseSearchParams = function (params) {
   })
 }
 
-let nyplDataApiClient = null
-
-function makeNyplDataApiClient () {
-  if (!nyplDataApiClient) {
-    const NyplClient = require('@nypl/nypl-data-api-client')
-
-    nyplDataApiClient = new NyplClient({
-      base_url: process.env.NYPL_API_BASE_URL,
-      oauth_key: process.env.NYPL_OAUTH_ID,
-      oauth_secret: process.env.NYPL_OAUTH_SECRET,
-      oauth_url: process.env.NYPL_OAUTH_URL
-    })
-  }
-  return nyplDataApiClient
-}
-
 // These are the handlers made available to the router:
 module.exports = function (app, _private = null) {
   app.resources = {}

test/aggregations.test.js

@@ -7,11 +7,11 @@ describe('Aggregations response', function () {
   this.timeout(10000)
 
   before(function () {
-    fixtures.enableFixtures()
+    fixtures.enableEsFixtures()
   })
 
   after(function () {
-    fixtures.disableFixtures()
+    fixtures.disableEsFixtures()
   })
 
   var requestPath = '/api/v0.1/discovery/resources/aggregations?q=hamilton&search_scope=all'

test/annotated-marc-rules.test.js

@@ -1,4 +1,5 @@
 const AnnotatedMarcSerializer = require('../lib/annotated-marc-serializer')
+const fixtures = require('./fixtures')
 
 const realMappingRules = AnnotatedMarcSerializer.mappingRules
 function overRideMappingRules (rules) {
@@ -641,4 +642,48 @@ describe('Annotated Marc Rules', function () {
       expect(serialized.bib.fields[0].values[2].content).to.equal('New York (N.Y.) -- 21st century -- Diaries.')
     })
   })
+
+  describe('Annotated marc endpoint', function () {
+    let app = {}
+
+    before(function () {
+      // Get a minimal instance of app (just controller code):
+      require('../lib/resources')(app)
+      app.logger = require('../lib/logger')
+
+      // Reroute this (and only this) api path to local fixture:
+      fixtures.enableDataApiFixtures({
+        'bibs/sierra-nypl/11055155': 'bib-11055155.json'
+      })
+    })
+
+    after(function () {
+      fixtures.disableDataApiFixtures()
+    })
+
+    it('transforms fetched marc-in-json document into "annotated-marc" format', function () {
+      return app.resources.annotatedMarc({ uri: 'b11055155' })
+        .then((resp) => {
+          expect(resp.bib.id).to.equal('11055155')
+          expect(resp.bib.nyplSource).to.equal('sierra-nypl')
+          expect(resp.bib.fields).to.be.an('array')
+
+          const lccn = resp.bib.fields
+            .filter((field) => field.label === 'LCCN')
+            .pop()
+          expect(lccn).to.be.a('object')
+          expect(lccn.values).to.be.a('array')
+          expect(lccn.values[0]).to.be.a('object')
+          expect(lccn.values[0].content).to.equal('   28025172')
+
+          const title = resp.bib.fields
+            .filter((field) => field.label === 'Title')
+            .pop()
+          expect(title).to.be.a('object')
+          expect(title.values).to.be.a('array')
+          expect(title.values[0]).to.be.a('object')
+          expect(title.values[0].content).to.equal('Topographical bibliography of ancient Egyptian hieroglyphic texts, reliefs, and paintings / by Bertha Porter and Rosalind L.B. Moss.')
+        })
+    })
+  })
 })

test/available_delivery_location_types.test.js

@@ -1,25 +1,18 @@
-const sinon = require('sinon')
-const fs = require('fs')
+const fixtures = require('./fixtures')
 
-// These are hard-wired to what I know about patron types 10 & 78
 describe('AvailableDeliveryLocationTypes', function () {
   let AvailableDeliveryLocationTypes = require('../lib/available_delivery_location_types.js')
 
   before(function () {
-    sinon.stub(AvailableDeliveryLocationTypes.client, 'get').callsFake(function (path) {
-      switch (path) {
-        case 'patrons/branch-patron-id':
-          return Promise.resolve(JSON.parse(fs.readFileSync('./test/fixtures/patron-research.json', 'utf8')))
-        case 'patrons/scholar-patron-id':
-          return Promise.resolve(JSON.parse(fs.readFileSync('./test/fixtures/patron-scholar.json', 'utf8')))
-        default:
-          return Promise.reject()
-      }
+    // Reroute these (and only these) api paths to local fixtures:
+    fixtures.enableDataApiFixtures({
+      'patrons/branch-patron-id': 'patron-research.json',
+      'patrons/scholar-patron-id': 'patron-scholar.json'
     })
   })
 
   after(function () {
-    AvailableDeliveryLocationTypes.client.get.restore()
+    fixtures.disableDataApiFixtures()
   })
 
   it('maps patron type 10 to [\'Research\']', function () {