[215] add recycle job, move all job-launching logic to jobs module

app/config/job.js

 const defaults = {
-  recycleInterval: 1000 * 60 * 60, // 1 hr
-  recycleGracePeriod: 1000 * 60 * 60 * 24, // 1 day
+  healthcheckInterval: 1000 * 60 * 15, // 15 min
+  hotlineMessageExpiryInMillis: 1000 * 60 * 60 * 24 * 28, // 4 weeks
   inviteDeletionInterval: 1000 * 60 * 60, // 1 hour
   inviteExpiryInMillis: 1000 * 60 * 60 * 24 * 14, // 2 weeks
-  hotlineMessageExpiryInMillis: 1000 * 60 * 60 * 24 * 28, // 4 weeks
+  recycleInterval: 1000 * 60 * 60, // 1 hr
+  recycleGracePeriod: 1000 * 60 * 60 * 24, // 1 day
+  signaldStartupTime: 3000 * 60, // 3 min
 }
 
+const testInterval = 50
+
 const development = {
   ...defaults,
+  healthcheckInterval: 1000 * 60, // 60 sec
   recycleInterval: 1000 * 5, // 5 secs
   recycleGracePeriod: 1000 * 30, // 30 sec
 }
 
 const test = {
   ...defaults,
-  inviteDeletionInterval: 100, // 100 millis
+  testInterval,
+  healthcheckInterval: testInterval, // millis
+  inviteDeletionInterval: testInterval,
+  recycleInterval: testInterval,
+  signaldStartupTime: 1, //  millis
   inviteExpiryInMillis: 200, // 200 millis
 }
 

app/config/signal.js

@@ -4,7 +4,6 @@ const defaults = {
   broadcastSpacing: 100, // 100 millis
   defaultMessageExpiryTime: 60 * 60 * 24 * 7, // 1 week
   expiryUpdateDelay: 200, // 200 millis
-  healtcheckInterval: 1000 * 60 * 15, // 15 min
   healthcheckTimeout: 1000 * 60 * 15, // 15 min
   healtcheckSpacing: 100, // 100 millis
   intervalBetweenRegistrationBatches: 120000, // 2 minutes
@@ -18,7 +17,6 @@ const defaults = {
   signaldRequestTimeout: 1000 * 10, // 10 sec
   signaldVerifyTimeout: 1000 * 30, // 30 sec
   signaldSendTimeout: 1000 * 60 * 60, // 60 min
-  signaldStartupTime: 3000 * 60, // 3 min
   supportPhoneNumber: (process.env.SUPPORT_CHANNEL_NUMBER || '').replace(`"`, ''),
   diagnosticsPhoneNumber: (process.env.DIAGNOSTICS_CHANNEL_NUMBER || '').replace(`"`, ''),
   welcomeDelay: 3000, // 3 sec
@@ -29,7 +27,6 @@ const test = {
   broadcastBatchInterval: 10, // 10 millis
   broadcastBatchSize: 1,
   expiryUpdateDelay: 1, // millis
-  healthcheckInterval: 30, // millis
   healthcheckTimeout: 30, // millis
   intervalBetweenRegistrationBatches: 30, // millis
   intervalBetweenRegistrations: 5, // millis,
@@ -40,7 +37,6 @@ const test = {
   signaldSendTimeout: 40, // millis
   signaldRequestTimeout: 10, // millis
   signaldVerifyTimeout: 20, // millis
-  signaldStartupTime: 1, //  millis
   supportPhoneNumber: '+15555555555',
   welcomeDelay: 0.0001, // millis
   diagnosticsPhoneNumber: '+15554443333',
@@ -48,7 +44,6 @@ const test = {
 
 const development = {
   ...defaults,
-  healtcheckInterval: 1000 * 60, // 60 sec
   healthcheckTimeout: 1000 * 60, // 60 sec
 }
 

app/db/repositories/invite.js

@@ -2,11 +2,9 @@ const app = require('../../../app')
 const moment = require('moment')
 const { Op } = require('sequelize')
 const membershipRepository = require('./membership')
-const { repeatEvery, loggerOf } = require('../../util')
-const logger = loggerOf('db|inviteRepository')
 const {
   defaultLanguage,
-  job: { inviteExpiryInMillis, inviteDeletionInterval },
+  job: { inviteExpiryInMillis },
 } = require('../../config')
 
 // (string, string, string) -> Promise<boolean>
@@ -33,10 +31,6 @@ const accept = (channelPhoneNumber, inviteePhoneNumber, language = defaultLangua
 const decline = async (channelPhoneNumber, inviteePhoneNumber) =>
   app.db.invite.destroy({ where: { channelPhoneNumber, inviteePhoneNumber } })
 
-// (number) -> Promise<void>
-const launchInviteDeletionJob = () =>
-  repeatEvery(() => deleteExpired().catch(logger.error), inviteDeletionInterval)
-
 // () -> Promise<number>
 const deleteExpired = async () =>
   app.db.invite.destroy({
@@ -47,4 +41,4 @@ const deleteExpired = async () =>
     },
   })
 
-module.exports = { issue, count, accept, decline, deleteExpired, launchInviteDeletionJob }
+module.exports = { issue, count, accept, decline, deleteExpired }

app/diagnostics.js

@@ -6,7 +6,7 @@ const metrics = require('./metrics')
 const { zip } = require('lodash')
 const { sdMessageOf } = require('./signal/constants')
 const {
-  signal: { diagnosticsPhoneNumber, healtcheckInterval, healthcheckSpacing, signaldStartupTime },
+  signal: { diagnosticsPhoneNumber, healthcheckSpacing },
 } = require('./config')
 
 const logger = util.loggerOf('diagnostics')
@@ -51,14 +51,7 @@ const respondToHealthcheck = (channelPhoneNumber, healthcheckId) =>
     ),
   )
 
-// () => Promise<void>
-const launchHealthcheckJob = async () => {
-  await util.wait(signaldStartupTime)
-  return diagnosticsPhoneNumber && util.repeatEvery(sendHealthchecks, healtcheckInterval)
-}
-
 module.exports = {
   respondToHealthcheck,
   sendHealthchecks,
-  launchHealthcheckJob,
 }

app/jobs.js

@@ -4,10 +4,19 @@ const inviteRepository = require('./db/repositories/invite')
 const smsSenderRepository = require('./db/repositories/smsSender')
 const hotlineMessageRepository = require('./db/repositories/hotlineMessage')
 const diagnostics = require('./diagnostics')
+const util = require('./util')
+const {
+  job: { healthcheckInterval, inviteDeletionInterval, recycleInterval, signaldStartupTime },
+  signal: { diagnosticsPhoneNumber },
+} = require('./config')
 
 const run = async () => {
   logger.log('--- Running startup jobs...')
 
+  /******************
+   * ONE-OFF JOBS
+   *****************/
+
   if (process.env.REREGISTER_ON_STARTUP === '1') {
     logger.log('----- Registering phone numbers...')
     const regs = await phoneNumberRegistrar.registerAllUnregistered().catch(logger.error)
@@ -15,21 +24,39 @@ const run = async () => {
   }
 
   logger.log('----- Deleting expired sms sender records...')
-  // here we rely on fact of nightly backups to ensure this task runs once every 24 hr.
+  // rely on fact of nightly backups to ensure this task runs at least every 24 hr.
   const sendersDeleted = await smsSenderRepository.deleteExpired()
   logger.log(`----- Deleted ${sendersDeleted} expired sms sender records.`)
 
   logger.log('----- Deleting expired hotline message records...')
-  // here we rely on fact of nightly backups to ensure this task runs once every 24 hr.
+  // rely on fact of nightly backups to ensure this task runs at least every 24 hr.
   const messageIdsDeleted = await hotlineMessageRepository.deleteExpired()
-  logger.log(`----- Deleted ${messageIdsDeleted} expired sms sender records.`)
+  logger.log(`----- Deleted ${messageIdsDeleted} expired hotline records.`)
+
+  /******************
+   * REPEATING JOBS
+   *****************/
 
   logger.log('----- Launching invite scrubbing job...')
-  inviteRepository.launchInviteDeletionJob()
+  util.repeatEvery(
+    () => inviteRepository.deleteExpired().catch(logger.error),
+    inviteDeletionInterval,
+  )
   logger.log('----- Launched invite scrubbing job.')
-  
+
+  logger.log('---- Launching recycle request processing job...')
+  util.repeatEvery(
+    () => phoneNumberRegistrar.processRecycleRequests().catch(logger.error),
+    recycleInterval,
+  )
+  logger.log('---- Launched recycle request job...')
+
   logger.log('---- Launching healthcheck job...')
-  diagnostics.launchHealthcheckJob()
+  const launchHealthchecks = async () => {
+    await util.wait(signaldStartupTime)
+    util.repeatEvery(() => diagnostics.sendHealthchecks().catch(logger.error), healthcheckInterval)
+  }
+  if (diagnosticsPhoneNumber) launchHealthchecks()
   logger.log('---- Launched healthcheck job...')
 
   logger.log('--- Startup jobs complete!')

test/unit/db/repositories/invite.spec.js

@@ -184,25 +184,4 @@ describe('invite repository', () => {
       expect(await db.invite.count()).to.eql(inviteCount - 1)
     })
   })
-
-  describe('#launchInviteDeletionJob', () => {
-    beforeEach(async () => {
-      await db.invite.destroy({ where: {}, force: true })
-      await Promise.all([db.invite.create(inviteFactory()), db.invite.create(inviteFactory())])
-      inviteCount = await db.invite.count()
-      inviteRepository.launchInviteDeletionJob()
-    })
-
-    it('launches a job to delete expired invites at a specified interval', async () => {
-      // the test deletion interval is 1/2 the expiry length
-      // so in 4 intervals, we should observe 2 deletions
-      expect(await db.invite.count()).to.eql(inviteCount)
-
-      await wait(inviteDeletionInterval)
-      expect(await db.invite.count()).to.eql(inviteCount)
-
-      await wait(3 * inviteDeletionInterval)
-      expect(await db.invite.count()).to.eql(inviteCount - 2)
-    })
-  })
 })

test/unit/diagnostics.spec.js

@@ -10,7 +10,8 @@ import { channelFactory, deepChannelFactory } from '../support/factories/channel
 import { sdMessageOf } from '../../app/signal/constants'
 import { wait } from '../../app/util'
 const {
-  signal: { diagnosticsPhoneNumber, healthcheckInterval, signaldStartupTime },
+  job: { healthcheckInterval },
+  signal: { diagnosticsPhoneNumber, signaldStartupTime },
 } = require('../../app/config')
 
 describe('diagnostics module', () => {
@@ -81,16 +82,4 @@ describe('diagnostics module', () => {
       ])
     })
   })
-
-  describe('launching a healthcheck job', () => {
-    beforeEach(() => {
-      healthcheckStub.returns(Promise.resolve(42))
-    })
-
-    it('schedules healthchecks to be sent on an interval', async () => {
-      launchHealthcheckJob()
-      await wait(signaldStartupTime + 2 * healthcheckInterval)
-      expect(healthcheckStub.callCount).to.be.at.least(2)
-    })
-  })
 })

test/unit/jobs.spec.js

@@ -6,19 +6,52 @@ import inviteRepository from '../../app/db/repositories/invite'
 import smsSenderRepository from '../../app/db/repositories/smsSender'
 import hotlineMessageRepository from '../../app/db/repositories/hotlineMessage'
 import jobs from '../../app/jobs'
+import diagnostics from '../../app/diagnostics'
+import util from '../../app/util'
+const {
+  job: { testInterval },
+} = require('../../app/config')
 
 describe('jobs service', () => {
-  let registerAllStub, inviteDeletionStub, smsSenderDeletionStub, hotlineMessageDeletionStub
+  let registerAllStub,
+    deleteInvitesStub,
+    deleteSmsSendersStub,
+    deleteHotlineMessagesStub,
+    processRecycleRequestsStub,
+    sendHealthchecksStub
+
+  /****
+   * TODO(aguestuser|2020-09-10):
+   *
+   *  The fact that this suite kicks of long-running recurring jobs but never
+   *  cancels them causes other unit tests to fail non-determinitically, because they
+   *  asserting on the same functions that are called repeatedly in this suite.
+   *
+   *  If this gets annoying enough, we should likely figure out a `jobs.stop()` function
+   *  that cancels all the `repeatEvery` calls a
+   **/
 
   describe('running the service', () => {
     let originalReregisterValue = process.env.REREGISTER_ON_STARTUP
     before(async () => {
+      // one-off jobs
       registerAllStub = sinon
         .stub(phoneNumberRegistrar, 'registerAllUnregistered')
         .returns(Promise.resolve([]))
-      inviteDeletionStub = sinon.stub(inviteRepository, 'launchInviteDeletionJob')
-      smsSenderDeletionStub = sinon.stub(smsSenderRepository, 'deleteExpired')
-      hotlineMessageDeletionStub = sinon.stub(hotlineMessageRepository, 'deleteExpired')
+      deleteSmsSendersStub = sinon
+        .stub(smsSenderRepository, 'deleteExpired')
+        .returns(Promise.resolve(1))
+      deleteHotlineMessagesStub = sinon
+        .stub(hotlineMessageRepository, 'deleteExpired')
+        .returns(Promise.resolve(1))
+
+      // repeating jobs
+      deleteInvitesStub = sinon.stub(inviteRepository, 'deleteExpired').returns(Promise.resolve(1))
+      sendHealthchecksStub = sinon.stub(diagnostics, 'sendHealthchecks').returns(Promise.resolve())
+      processRecycleRequestsStub = sinon
+        .stub(phoneNumberRegistrar, 'processRecycleRequests')
+        .returns(Promise.resolve(['42', '43']))
+
       process.env.REREGISTER_ON_STARTUP = '1'
       await jobs.run()
     })
@@ -28,20 +61,31 @@ describe('jobs service', () => {
       sinon.restore()
     })
 
-    it('registers any unregistered phone numbers with signal', () => {
-      expect(registerAllStub.callCount).to.be.above(0)
-    })
+    describe('one-off jobs', () => {
+      it('registers any unregistered phone numbers with signal', () => {
+        expect(registerAllStub.callCount).to.be.above(0)
+      })
 
-    it('launches an invite deletion job', () => {
-      expect(inviteDeletionStub.callCount).to.be.above(0)
-    })
+      it('deletes all expired sms sender records', () => {
+        expect(deleteSmsSendersStub.callCount).to.be.above(0)
+      })
 
-    it('deletes all expired sms sender records', () => {
-      expect(smsSenderDeletionStub.callCount).to.be.above(0)
+      it('deletes all expired hotline message records', () => {
+        expect(deleteHotlineMessagesStub.callCount).to.be.above(0)
+      })
     })
 
-    it('deletes all expired hotline message records', () => {
-      expect(hotlineMessageDeletionStub.callCount).to.be.above(0)
+    describe('recurring jobs', () => {
+      before(async () => await util.wait(testInterval))
+      it('launches an invite deletion job', () => {
+        expect(deleteInvitesStub.callCount).to.be.at.least(2)
+      })
+      it('lauches a recycle request processing job', () => {
+        expect(processRecycleRequestsStub.callCount).at.least(2)
+      })
+      it('launches a healtcheck job', async () => {
+        expect(sendHealthchecksStub.callCount).to.be.at.least(2)
+      })
     })
   })
 })