In `collect_paypal_past_due_invoice.js`, iterate over each page instead of gathering data from all pages at first (#18414)

* Create `getPaginatedEndpointIterator` to iterate each page

* Create `waitMs` util, it will replace `slowCallback`

* Make `handleAPIError` async

* Make `isAccountUsingPaypal` async

* Make `attemptInvoiceCollection` async

* Make `attemptInvoicesCollection` async

* Use `await` instead of `new Promise`

* Remove unused callbackified `attemptInvoiceCollection`

* Run `attemptInvoiceCollection` for each page instead of gathering all pages in the beginning

* Add test on fetching multiple pages of invoice

GitOrigin-RevId: 2674b18c6ca5732b873fb2bc71b515909006f93d

services/web/app/src/Features/Subscription/RecurlyWrapper.js

@@ -467,9 +467,32 @@ const promises = {
     }
   },
 
+  /**
+   * @typedef {{getNextPage: () => Promise<PageData>, items: any[]}} PageData
+   */
+
   async getPaginatedEndpoint(resource, queryParams) {
-    queryParams.per_page = queryParams.per_page || 200
     let allItems = []
+    let items
+
+    /** @type {() => Promise<PageData>} */
+    let getNextPage = promises.getPaginatedEndpointIterator(
+      resource,
+      queryParams
+    )
+    while (getNextPage) {
+      ;({ items, getNextPage } = await getNextPage())
+      allItems = allItems.concat(items)
+      logger.debug(`total now ${allItems.length}`)
+    }
+    return allItems
+  },
+
+  /**
+   * @returns {() => Promise<PageData>}
+   */
+  getPaginatedEndpointIterator(resource, queryParams) {
+    queryParams.per_page = queryParams.per_page || 200
     const getPage = async (cursor = null) => {
       const opts = {
         url: resource,
@@ -483,21 +506,16 @@ const promises = {
       const data = await RecurlyWrapper.promises._parseXml(body)
 
       const items = data[resource]
-      allItems = allItems.concat(items)
-      logger.debug(`got another ${items.length}, total now ${allItems.length}`)
+      logger.debug(`got ${items.length} items in this page`)
       const match = response.headers.link?.match(/cursor=([0-9.]+%3A[0-9.]+)&/)
-      cursor = match && match[1]
-      if (cursor) {
-        cursor = decodeURIComponent(cursor)
-        return getPage(cursor)
-      } else {
-        return allItems
+      const nextCursor = match && match[1]
+      return {
+        items,
+        getNextPage:
+          nextCursor && (() => getPage(decodeURIComponent(nextCursor))),
       }
     }
-
-    await getPage()
-
-    return allItems
+    return getPage
   },
 
   async getAccount(accountId) {
@@ -860,13 +878,6 @@ const RecurlyWrapper = {
   apiUrl: Settings.apis.recurly.url || 'https://api.recurly.com/v2',
   _buildXml,
   _parseXml: callbackify(promises._parseXml),
-  // This one needs to be callbackified manually because we need to transform {response, body} to (err, response, body)
-  attemptInvoiceCollection: (invoiceId, callback) => {
-    promises
-      .attemptInvoiceCollection(invoiceId)
-      .then(({ response, body }) => callback(null, response, body))
-      .catch(err => callback(err))
-  },
   createFixedAmmountCoupon: callbackify(promises.createFixedAmmountCoupon),
   getAccountActiveCoupons: callbackify(promises.getAccountActiveCoupons),
   getBillingInfo: callbackify(promises.getBillingInfo),

services/web/scripts/recurly/collect_paypal_past_due_invoice.js

@@ -1,20 +1,19 @@
 const RecurlyWrapper = require('../../app/src/Features/Subscription/RecurlyWrapper')
-const async = require('async')
 const minimist = require('minimist')
 const logger = require('@overleaf/logger')
 
-const slowCallback =
+const waitMs =
   require.main === module
-    ? (callback, error, data) => setTimeout(() => callback(error, data), 80)
-    : (callback, error, data) => callback(error, data)
+    ? timeout => new Promise(resolve => setTimeout(() => resolve(), timeout))
+    : () => Promise.resolve()
 
 // NOTE: Errors are not propagated to the caller
-const handleAPIError = (source, id, error, callback) => {
+const handleAPIError = async (source, id, error) => {
   logger.warn(`Errors in ${source} with id=${id}`, error)
   if (typeof error === 'string' && error.match(/429$/)) {
-    return setTimeout(callback, 1000 * 60 * 5)
+    return waitMs(1000 * 60 * 5)
   }
-  slowCallback(callback)
+  await waitMs(80)
 }
 
 /**
@@ -25,100 +24,95 @@ const handleAPIError = (source, id, error, callback) => {
  * }>}
  */
 const main = async () => {
-  const attemptInvoiceCollection = (invoice, callback) => {
-    isAccountUsingPaypal(invoice, (error, isPaypal) => {
-      if (error || !isPaypal) {
-        return callback(error)
-      }
-      const accountId = invoice.account.url.match(/accounts\/(.*)/)[1]
-      if (USERS_COLLECTED.indexOf(accountId) > -1) {
-        logger.warn(`Skipping duplicate user ${accountId}`)
-        return callback()
-      }
-      INVOICES_COLLECTED.push(invoice.invoice_number)
-      USERS_COLLECTED.push(accountId)
-      if (DRY_RUN) {
-        return callback()
-      }
-      RecurlyWrapper.attemptInvoiceCollection(
+  const attemptInvoiceCollection = async invoice => {
+    const isPaypal = await isAccountUsingPaypal(invoice)
+
+    if (!isPaypal) {
+      return
+    }
+    const accountId = invoice.account.url.match(/accounts\/(.*)/)[1]
+    if (USERS_COLLECTED.indexOf(accountId) > -1) {
+      logger.warn(`Skipping duplicate user ${accountId}`)
+      return
+    }
+    INVOICES_COLLECTED.push(invoice.invoice_number)
+    USERS_COLLECTED.push(accountId)
+    if (DRY_RUN) {
+      return
+    }
+    try {
+      await RecurlyWrapper.promises.attemptInvoiceCollection(
+        invoice.invoice_number
+      )
+      INVOICES_COLLECTED_SUCCESS.push(invoice.invoice_number)
+      await waitMs(80)
+    } catch (error) {
+      return handleAPIError(
+        'attemptInvoiceCollection',
         invoice.invoice_number,
-        (error, response) => {
-          if (error) {
-            return handleAPIError(
-              'attemptInvoiceCollection',
-              invoice.invoice_number,
-              error,
-              callback
-            )
-          }
-          INVOICES_COLLECTED_SUCCESS.push(invoice.invoice_number)
-          slowCallback(callback, null)
-        }
+        error
       )
-    })
+    }
   }
 
-  const isAccountUsingPaypal = (invoice, callback) => {
+  const isAccountUsingPaypal = async invoice => {
     const accountId = invoice.account.url.match(/accounts\/(.*)/)[1]
-    RecurlyWrapper.getBillingInfo(accountId, (error, response) => {
-      if (error) {
-        return handleAPIError('billing info', accountId, error, callback)
-      }
-      if (response.billing_info.paypal_billing_agreement_id) {
-        return slowCallback(callback, null, true)
-      }
-      slowCallback(callback, null, false)
-    })
+    try {
+      const response = await RecurlyWrapper.promises.getBillingInfo(accountId)
+      await waitMs(80)
+      return !!response.billing_info.paypal_billing_agreement_id
+    } catch (error) {
+      return handleAPIError('billing info', accountId, error)
+    }
   }
 
-  const attemptInvoicesCollection = callback => {
-    RecurlyWrapper.getPaginatedEndpoint(
+  const attemptInvoicesCollection = async () => {
+    let getPage = await RecurlyWrapper.promises.getPaginatedEndpointIterator(
       'invoices',
-      { state: 'past_due' },
-      (error, invoices) => {
-        logger.info('invoices', invoices?.length)
-        if (error) {
-          return callback(error)
-        }
-        async.eachSeries(invoices, attemptInvoiceCollection, callback)
-      }
+      { state: 'past_due' }
     )
+
+    while (getPage) {
+      const { items, getNextPage } = await getPage()
+      logger.info('invoices', items?.length)
+      for (const invoice of items) {
+        await attemptInvoiceCollection(invoice)
+      }
+      getPage = getNextPage
+    }
   }
+
   const argv = minimist(process.argv.slice(2))
   const DRY_RUN = argv.n !== undefined
   const INVOICES_COLLECTED = []
   const INVOICES_COLLECTED_SUCCESS = []
   const USERS_COLLECTED = []
 
-  return new Promise((resolve, reject) => {
-    attemptInvoicesCollection(error => {
-      logger.info(
-        `DONE (DRY_RUN=${DRY_RUN}). ${INVOICES_COLLECTED.length} invoices collection attempts for ${USERS_COLLECTED.length} users. ${INVOICES_COLLECTED_SUCCESS.length} successful collections`
-      )
-      console.dir(
-        {
-          INVOICES_COLLECTED,
-          INVOICES_COLLECTED_SUCCESS,
-          USERS_COLLECTED,
-        },
-        { maxArrayLength: null }
-      )
+  try {
+    await attemptInvoicesCollection()
 
-      if (error) {
-        reject(error)
-      }
+    if (INVOICES_COLLECTED_SUCCESS.length === 0) {
+      throw new Error('No invoices collected')
+    }
 
-      if (INVOICES_COLLECTED_SUCCESS.length === 0) {
-        reject(new Error('No invoices collected'))
-      }
-
-      resolve({
+    return {
+      INVOICES_COLLECTED,
+      INVOICES_COLLECTED_SUCCESS,
+      USERS_COLLECTED,
+    }
+  } finally {
+    logger.info(
+      `DONE (DRY_RUN=${DRY_RUN}). ${INVOICES_COLLECTED.length} invoices collection attempts for ${USERS_COLLECTED.length} users. ${INVOICES_COLLECTED_SUCCESS.length} successful collections`
+    )
+    console.dir(
+      {
         INVOICES_COLLECTED,
         INVOICES_COLLECTED_SUCCESS,
         USERS_COLLECTED,
-      })
-    })
-  })
+      },
+      { maxArrayLength: null }
+    )
+  }
 }
 
 if (require.main === module) {

services/web/test/acceptance/src/CollectPayPalPastDueInvoiceTest.js

@@ -155,34 +155,55 @@ const invoiceCollectXml = `
 </invoice>
 `
 
+const ITEMS_PER_PAGE = 3
+
+const getInvoicePage = fullInvoicesIds => queryOptions => {
+  const cursor = queryOptions.qs.cursor
+  const startEnd = cursor?.split(':').map(Number) || []
+  const start = startEnd[0] || 0
+  const end = startEnd[1] || ITEMS_PER_PAGE
+  const body = invoicesXml(fullInvoicesIds.slice(start, end))
+  const hasMore = end < fullInvoicesIds.length
+  const nextPageCursor = hasMore ? `${end}%3A${end + ITEMS_PER_PAGE}&v=2` : null
+  const response = {
+    statusCode: 200,
+    headers: {
+      link: hasMore
+        ? `https://fakerecurly.com/v2/invoices?cursor=${nextPageCursor}`
+        : undefined,
+    },
+  }
+
+  return { response, body }
+}
+
 describe('CollectPayPalPastDueInvoice', function () {
   let apiRequestStub
-  const fakeApiRequests = invoiceIdsAndReturnCode => {
+  const fakeApiRequests = invoiceIds => {
     apiRequestStub = sinon.stub(RecurlyWrapper.promises, 'apiRequest')
     apiRequestStub.callsFake(options => {
-      switch (options.url) {
-        case 'invoices':
-          return {
-            response: { statusCode: 200, headers: {} },
-            body: invoicesXml(invoiceIdsAndReturnCode),
-          }
-        case 'accounts/200/billing_info':
-        case 'accounts/404/billing_info':
-          return {
-            response: { statusCode: 200, headers: {} },
-            body: billingInfoXml,
-          }
-        case 'invoices/200/collect':
+      if (options.url === 'invoices') {
+        return getInvoicePage(invoiceIds)(options)
+      }
+
+      if (/accounts\/(\d+)\/billing_info/.test(options.url)) {
+        return {
+          response: { statusCode: 200, headers: {} },
+          body: billingInfoXml,
+        }
+      }
+
+      if (/invoices\/(\d+)\/collect/.test(options.url)) {
+        const invoiceId = options.url.match(/invoices\/(\d+)\/collect/)[1]
+        if (invoiceId < 400) {
           return {
             response: { statusCode: 200, headers: {} },
             body: invoiceCollectXml,
           }
-        case 'invoices/404/collect':
-          throw new OError(`Recurly API returned with status code: 404`, {
-            statusCode: 404,
-          })
-        default:
-          throw new Error(`Unexpected URL: ${options.url}`)
+        }
+        throw new OError(`Recurly API returned with status code: 404`, {
+          statusCode: 404,
+        })
       }
     })
   }
@@ -194,13 +215,43 @@ describe('CollectPayPalPastDueInvoice', function () {
   it('collects one valid invoice', async function () {
     fakeApiRequests([200])
     const r = await main()
-    await expect(r).to.eql({
+    expect(r).to.eql({
       INVOICES_COLLECTED: [200],
       INVOICES_COLLECTED_SUCCESS: [200],
       USERS_COLLECTED: ['200'],
     })
   })
 
+  it('collects several pages', async function () {
+    // 10 invoices, from 200 to 209
+    fakeApiRequests([...Array(10).keys()].map(i => i + 200))
+    const r = await main()
+
+    expect(r).to.eql({
+      INVOICES_COLLECTED: [200, 201, 202, 203, 204, 205, 206, 207, 208, 209],
+      INVOICES_COLLECTED_SUCCESS: [
+        200, 201, 202, 203, 204, 205, 206, 207, 208, 209,
+      ],
+      USERS_COLLECTED: [
+        '200',
+        '201',
+        '202',
+        '203',
+        '204',
+        '205',
+        '206',
+        '207',
+        '208',
+        '209',
+      ],
+    })
+
+    // 4 calls to get the invoices
+    // 10 calls to get the billing info
+    // 10 calls to collect the invoices
+    expect(apiRequestStub.callCount).to.eql(24)
+  })
+
   it('rejects with no invoices are processed because of errors', async function () {
     fakeApiRequests([404])
     await expect(main()).to.be.rejectedWith('No invoices collected')
@@ -214,7 +265,7 @@ describe('CollectPayPalPastDueInvoice', function () {
   it('resolves when some invoices are partially successful', async function () {
     fakeApiRequests([200, 404])
     const r = await main()
-    await expect(r).to.eql({
+    expect(r).to.eql({
       INVOICES_COLLECTED: [200, 404],
       INVOICES_COLLECTED_SUCCESS: [200],
       USERS_COLLECTED: ['200', '404'],