Verified Commit 7ea2b5e6 authored by aguestuser's avatar aguestuser

[347] split redeem & recycle logic into 2 functions in registrar

* extract a `phoneNumberRegistrar.redeem`, which may be called from
  dispatcher as soon as a redemption message is received. it:
  * deletes recycle request for redeemed channel
  * immediately notifies channel admins and instance maintainers that
    channel has been redeemed (no more waiting 24 hours! :))
* refactor `recycleRequestRepository.evaluateRecycleRequests`
  * rename to `recycleRequest.getMaturerecycleRequests`
  * only retrieve and return mature recycle requests
  * don't worry about filtering out redeemed channel phone numbers
    anymore, since this will be done atomically and directly by calls
    to `#redeem` above
* simplify `phoneNumberRegistrar.processRecycleRequests`:
  * don't worry about redeemed nubmers
  * just get the mature requests, recycle their channels, and notify
    the admins that this happened
parent dbbd5650
const { Op } = require('sequelize')
const app = require('../../../app')
const util = require('../../util')
const { map, partition } = require('lodash')
const { map } = require('lodash')
const {
job: { recycleGracePeriod },
} = require('../../config')
// (string) -> Promise<{ recycleRequest: RecycleRequest, wasCreated: boolean }>
const requestToRecycle = phoneNumber =>
const requestToRecycle = channelPhoneNumber =>
app.db.recycleRequest
.findOrCreate({ where: { phoneNumber } })
.findOrCreate({ where: { channelPhoneNumber } })
.then(([recycleRequest, wasCreated]) => ({
recycleRequest,
wasCreated,
}))
// (Array<string>) -> Promise<void>
const destroyMany = phoneNumbers =>
app.db.recycleRequest.destroy({
where: { phoneNumber: { [Op.in]: phoneNumbers } },
})
const destroy = channelPhoneNumber =>
app.db.recycleRequest.destroy({ where: { channelPhoneNumber } })
const evaluateRecycleRequests = async () => {
// channel admins have a 1 day grace period to redeem a channel slated for recycling
// by using it. calculate when that grace period started...
const gracePeriodStart = util.now().subtract(parseInt(recycleGracePeriod), 'ms')
// (Array<string>) -> Promise<void>
const destroyMany = channelPhoneNumbers =>
app.db.recycleRequest.destroy({ where: { channelPhoneNumber: { [Op.in]: channelPhoneNumbers } } })
// find all the requests issued before the start of the grace period, indicating
// channels which should be considered for recycling (b/c their grace period has passed)
// () => Promise<string>
const getMatureRecycleRequests = async () => {
// Admins have a 24hr grace period to redeem a channel slated for recycling by using it.
// Here, we find all the requests issued before the start of the grace period, and return their
// phone numbers to the caller for recycling. We may safely assume they can be recycled, becuase
// if redeemed (in dispatcher.dispatch)
const gracePeriodStart = util.now().subtract(parseInt(recycleGracePeriod), 'ms')
const matureRequests = await app.db.recycleRequest.findAll({
where: { createdAt: { [Op.lte]: gracePeriodStart } },
})
// make lists of redeemed and unredeemed channel phone numbers, where "redeemed" channels
// have been used since the start of the grace period, and thus should not be recycled
const [redeemed, toRecycle] = partition(
await app.db.messageCount.findAll({
where: { channelPhoneNumber: { [Op.in]: map(matureRequests, 'phoneNumber') } },
}),
messageCount => messageCount.updatedAt > gracePeriodStart,
)
// pluck the channel phone numbers and return them for processing!
return {
redeemed: map(redeemed, 'channelPhoneNumber'),
toRecycle: map(toRecycle, 'channelPhoneNumber'),
}
return map(matureRequests, 'channelPhoneNumber')
}
module.exports = { requestToRecycle, evaluateRecycleRequests, destroyMany }
module.exports = { requestToRecycle, getMatureRecycleRequests, destroy, destroyMany }
......@@ -4,7 +4,7 @@ const { errors } = require('./common')
const { destroy } = require('./destroy')
const { list } = require('./present')
const { provisionN } = require('./provision')
const { requestToRecycle, recycle, processRecycleRequests } = require('./recycle')
const { requestToRecycle, processRecycleRequests, recycle, redeem } = require('./recycle')
const { register, registerAllPurchased, registerAllUnregistered } = require('./register')
const { handleSms } = require('./sms')
const { purchase, purchaseN } = require('./purchase')
......@@ -25,6 +25,7 @@ module.exports = {
processRecycleRequests,
requestToRecycle,
recycle,
redeem,
register,
registerAllPurchased,
registerAllUnregistered,
......
......@@ -47,22 +47,15 @@ const requestToRecycle = async phoneNumbers => {
// () -> Promise<Array<string>>
const processRecycleRequests = async () => {
try {
const { redeemed, toRecycle } = await recycleRequestRepository.evaluateRecycleRequests()
const recycleResults = await Promise.all(toRecycle.map(recycle))
await recycleRequestRepository.destroyMany([...redeemed, ...toRecycle])
const redeemedChannels = await channelRepository.findManyDeep(redeemed)
const numProcessed = redeemed.length + toRecycle.length
const phoneNumbersToRecycle = await recycleRequestRepository.getMatureRecycleRequests()
const recycleResults = await Promise.all(phoneNumbersToRecycle.map(recycle))
await recycleRequestRepository.destroyMany(phoneNumbersToRecycle)
return Promise.all([
...redeemedChannels.map(channel =>
notifier.notifyAdmins(channel, notificationKeys.CHANNEL_REDEEMED),
),
numProcessed === 0
phoneNumbersToRecycle.length === 0
? Promise.resolve()
: notifier.notifyMaintainers(
`${redeemed.length + toRecycle.length} recycle requests processed:\n\n` +
`${redeemed.map(r => `${r} redeemed by admins.`).join('\n')}` +
'\n' +
`${phoneNumbersToRecycle.length} recycle requests processed:\n\n` +
`${map(recycleResults, 'message').join('\n')}`,
),
])
......@@ -71,6 +64,21 @@ const processRecycleRequests = async () => {
}
}
// (Channel) -> Promise<void>
const redeem = async channel => {
try {
await recycleRequestRepository.destroy(channel.phoneNumber)
await Promise.all([
notifier.notifyAdmins(channel, notificationKeys.CHANNEL_REDEEMED),
notifier.notifyMaintainers(
`${channel.phoneNumber} had been scheduled for recycling, but was just redeemed.`,
),
])
} catch (err) {
return notifier.notifyMaintainers(`Error redeeming ${channel.phoneNumber}: ${err}`)
}
}
// (string) -> SignalboostStatus
const recycle = async phoneNumber => {
const channel = await channelRepository.findDeep(phoneNumber)
......@@ -93,4 +101,4 @@ const recycle = async phoneNumber => {
}
}
module.exports = { requestToRecycle, processRecycleRequests, recycle }
module.exports = { requestToRecycle, processRecycleRequests, recycle, redeem }
......@@ -8,134 +8,156 @@ import testApp from '../../../support/testApp'
import dbService from '../../../../app/db'
import recycleRequestRepository from '../../../../app/db/repositories/recycleRequest'
import util from '../../../../app/util'
import { values } from 'lodash'
import { times, values } from 'lodash'
import { channelFactory } from '../../../support/factories/channel'
import { Op } from 'sequelize'
const {
job: { recycleGracePeriod },
} = require('../../../../app/config')
describe('recycleablePhoneNumber repository', () => {
const phoneNumber = genPhoneNumber()
const channelPhoneNumber = genPhoneNumber()
let db, recycleRequestCount
before(async () => (db = (await app.run({ ...testApp, db: dbService })).db))
beforeEach(async () => {
await db.channel.create(channelFactory({ phoneNumber: channelPhoneNumber }))
})
afterEach(async () => {
await app.db.recycleRequest.destroy({ where: {} })
await app.db.messageCount.destroy({ where: {} })
await app.db.channel.destroy({ where: {} })
sinon.restore()
})
after(async () => await app.stop())
describe('issuing a recycle request', () => {
describe('if recycle request does not yet exist', () => {
describe('#requestToRecycle', () => {
describe('if recycle request does not yet exist for given channel phone number', () => {
beforeEach(async () => (recycleRequestCount = await db.recycleRequest.count()))
it('creates new request and flags that it was created', async () => {
const { recycleRequest, wasCreated } = await recycleRequestRepository.requestToRecycle(
phoneNumber,
channelPhoneNumber,
)
expect(wasCreated).to.eql(true)
expect(recycleRequest.phoneNumber).to.eql(phoneNumber)
expect(recycleRequest.channelPhoneNumber).to.eql(channelPhoneNumber)
expect(await app.db.recycleRequest.count()).to.eql(recycleRequestCount + 1)
})
})
describe('if recycle request already exists for the phoneNumber', () => {
describe('if recycle request already exists for given channel phone number', () => {
beforeEach(async () => {
await app.db.recycleRequest.create({ phoneNumber })
await app.db.recycleRequest.create({ channelPhoneNumber })
recycleRequestCount = await db.recycleRequest.count()
})
it('does not create new request but returns existing request', async () => {
const { recycleRequest, wasCreated } = await recycleRequestRepository.requestToRecycle(
phoneNumber,
channelPhoneNumber,
)
expect(wasCreated).to.eql(false)
expect(recycleRequest.phoneNumber).to.eql(phoneNumber)
expect(recycleRequest.channelPhoneNumber).to.eql(channelPhoneNumber)
expect(await app.db.recycleRequest.count()).to.eql(recycleRequestCount)
})
})
})
describe('processing mature recycle requests', () => {
describe('#destroy', () => {
beforeEach(async () => {
await app.db.recycleRequest.create({ channelPhoneNumber })
recycleRequestCount = await db.recycleRequest.count()
})
describe('when given the phone number of an existing recycle request', () => {
it('destroys the recycle request', async () => {
expect(await recycleRequestRepository.destroy(channelPhoneNumber)).to.eql(1)
expect(await db.recycleRequest.count()).to.eql(recycleRequestCount - 1)
expect(await db.recycleRequest.findOne({ where: { channelPhoneNumber } })).to.be.null
})
})
describe('when given a random phone number', () => {
it('does nothing', async () => {
expect(await recycleRequestRepository.destroy(genPhoneNumber())).to.eql(0)
expect(await db.recycleRequest.count()).to.eql(recycleRequestCount)
expect(await db.recycleRequest.findOne({ where: { channelPhoneNumber } })).not.to.be.null
})
})
})
describe('#destroyMany', () => {
const toBeDeleted = times(3, genPhoneNumber)
const toBeIgnored = times(2, genPhoneNumber)
const allNumbers = [...toBeDeleted, ...toBeIgnored]
beforeEach(async () => {
await Promise.all(
allNumbers.map(channelPhoneNumber =>
db.channel.create(channelFactory({ phoneNumber: channelPhoneNumber })),
),
)
await Promise.all(
allNumbers.map(channelPhoneNumber => db.recycleRequest.create({ channelPhoneNumber })),
)
recycleRequestCount = await db.recycleRequest.count()
})
it('deletes all recycle requests with phone numbers in a given list', async () => {
expect(await recycleRequestRepository.destroyMany(toBeDeleted)).to.eql(toBeDeleted.length)
expect(await db.recycleRequest.count()).to.eql(recycleRequestCount - toBeDeleted.length)
expect(
await db.recycleRequest.findAll({ where: { channelPhoneNumber: { [Op.in]: toBeDeleted } } }),
).to.have.length(0)
expect(
await db.recycleRequest.findAll({ where: { channelPhoneNumber: { [Op.in]: toBeIgnored } } }),
).to.have.length(toBeIgnored.length)
})
})
describe('#getMatureRecycleRequests', () => {
const now = moment().clone()
const gracePeriodStart = now.clone().subtract(recycleGracePeriod, 'ms')
const phoneNumbers = {
redeemed: genPhoneNumber(),
const channelPhoneNumbers = {
toRecycle: genPhoneNumber(),
pending: genPhoneNumber(),
}
const recycleRequests = {
redeemed: {
phoneNumber: phoneNumbers.redeemed,
// mature (created before start of grace period)
createdAt: gracePeriodStart.clone().subtract(1, 'ms'),
},
toRecycle: {
phoneNumber: phoneNumbers.toRecycle,
channelPhoneNumber: channelPhoneNumbers.toRecycle,
// mature (created before start of grace period)
createdAt: gracePeriodStart.clone().subtract(1, 'ms'),
},
pending: {
phoneNumber: phoneNumbers.pending,
channelPhoneNumber: channelPhoneNumbers.pending,
// not mature (created after start of grace period)
createdAt: gracePeriodStart.clone().add(1, 'ms'),
},
}
const messageCounts = {
redeemed: {
channelPhoneNumber: phoneNumbers.redeemed,
// used during grace period
updatedAt: gracePeriodStart.clone().add(1, 'ms'),
},
toRecycle: {
channelPhoneNumber: phoneNumbers.toRecycle,
// not used during grace perdiod
updatedAt: gracePeriodStart.clone().subtract(1, 'ms'),
},
pending: {
channelPhoneNumber: phoneNumbers.pending,
// does not matter when last used (b/c not mature), but let's say during grace period
updatedAt: gracePeriodStart.clone().add(1, 'ms'),
},
}
beforeEach(async () => {
sinon.stub(util, 'now').returns(now.clone())
await Promise.all(
values(recycleRequests).map(({ channelPhoneNumber }) =>
app.db.channel.create(channelFactory({ phoneNumber: channelPhoneNumber })),
),
)
await Promise.all(
values(recycleRequests).map(recycleRequest =>
app.db.recycleRequest.create(recycleRequest).then(() =>
app.db.sequelize.query(`
update "recycleRequests"
set "createdAt" = '${recycleRequest.createdAt.toISOString()}'
where "phoneNumber" = '${recycleRequest.phoneNumber}';
where "channelPhoneNumber" = '${recycleRequest.channelPhoneNumber}';
`),
),
),
)
await Promise.all(
values(messageCounts).map(messageCount =>
app.db.messageCount.create(messageCount).then(() =>
app.db.sequelize.query(`
update "messageCounts"
set "updatedAt" = '${messageCount.updatedAt.toISOString()}'
where "channelPhoneNumber" = '${messageCount.channelPhoneNumber}';
`),
),
),
)
})
it('retrieves all mature recycle requests and classifies them as redeemed or toRecycle', async () => {
const res = await recycleRequestRepository.evaluateRecycleRequests(values(phoneNumbers))
expect(res).to.eql({
redeemed: [phoneNumbers.redeemed],
toRecycle: [phoneNumbers.toRecycle],
})
it('retrieves all mature recycle requests and returns their phone numbers', async () => {
const res = await recycleRequestRepository.getMatureRecycleRequests(
values(channelPhoneNumbers),
)
expect(res).to.eql([channelPhoneNumbers.toRecycle])
})
})
})
......@@ -16,6 +16,7 @@ import notifier, { notificationKeys } from '../../../../app/notifier'
import { times, map, flatten } from 'lodash'
import { genPhoneNumber, phoneNumberFactory } from '../../../support/factories/phoneNumber'
import { eventFactory } from '../../../support/factories/event'
import { redeem } from '../../../../app/registrar/phoneNumber/recycle'
describe('phone number registrar -- recycle module', () => {
const phoneNumber = genPhoneNumber()
......@@ -310,12 +311,8 @@ describe('phone number registrar -- recycle module', () => {
})
describe('#processRecycleRequests', () => {
const redeemed = times(2, genPhoneNumber)
const redeemedChannels = redeemed.map(channelPhoneNumber =>
deepChannelFactory({ channelPhoneNumber }),
)
const toRecycle = times(3, genPhoneNumber)
let evaluateRecycleRequestsStub, destroyRecycleRequestsStub
let getMatureRecycleRequestsStub, destroyRecycleRequestsStub
beforeEach(() => {
// recycle helpers that should always succeed
......@@ -327,13 +324,15 @@ describe('phone number registrar -- recycle module', () => {
// processRecycle helpers that should always succeed
destroyRecycleRequestsStub = sinon
.stub(recycleRequestRepository, 'destroyMany')
.returns(Promise.resolve())
sinon.stub(channelRepository, 'findManyDeep').returns(Promise.resolve(redeemedChannels))
.returns(Promise.resolve(toRecycle.length))
notifyMaintainersStub.returns(Promise.resolve(['42']))
notifyAdminsStub.returns(Promise.resolve(['42', '42']))
// if this fails, processRecycleRequests will fail
evaluateRecycleRequestsStub = sinon.stub(recycleRequestRepository, 'evaluateRecycleRequests')
getMatureRecycleRequestsStub = sinon.stub(
recycleRequestRepository,
'getMatureRecycleRequests',
)
})
describe('when processing succeeds', () => {
......@@ -347,30 +346,21 @@ describe('phone number registrar -- recycle module', () => {
.onCall(2)
.callsFake(() => Promise.reject('BOOM!'))
// overall job succeeds
evaluateRecycleRequestsStub.returns(Promise.resolve({ redeemed, toRecycle }))
getMatureRecycleRequestsStub.returns(Promise.resolve(toRecycle))
await processRecycleRequests()
})
it('recycles unredeemed channels', () => {
it('recycles channels with mature recycle requests', () => {
expect(flatten(map(destroyChannelStub.getCalls(), 'args'))).to.eql(toRecycle)
})
it('destroys all recycle requests that were just evaluated', () => {
expect(destroyRecycleRequestsStub.getCall(0).args).to.eql([[...redeemed, ...toRecycle]])
})
it('notifies admins of redeemed channels of redemption', () => {
expect(map(notifyAdminsStub.getCalls(), 'args')).to.eql([
[redeemedChannels[0], notificationKeys.CHANNEL_REDEEMED],
[redeemedChannels[1], notificationKeys.CHANNEL_REDEEMED],
])
it('destroys all recycle requests that were just processed', () => {
expect(destroyRecycleRequestsStub.getCall(0).args).to.eql([toRecycle])
})
it('notifies maintainers of results', () => {
expect(notifyMaintainersStub.getCall(0).args).to.eql([
'5 recycle requests processed:\n\n' +
`${redeemed[0]} redeemed by admins.\n` +
`${redeemed[1]} redeemed by admins.\n` +
'3 recycle requests processed:\n\n' +
`${toRecycle[0]} recycled.\n` +
`${toRecycle[1]} recycled.\n` +
`${toRecycle[2]} failed to be recycled. Error: BOOM!`,
......@@ -379,7 +369,7 @@ describe('phone number registrar -- recycle module', () => {
})
describe('when job fails', () => {
beforeEach(() => evaluateRecycleRequestsStub.callsFake(() => Promise.reject('BOOM!')))
beforeEach(() => getMatureRecycleRequestsStub.callsFake(() => Promise.reject('BOOM!')))
it('notifies maintainers of error', async () => {
await processRecycleRequests()
expect(notifyMaintainersStub.getCall(0).args).to.eql([
......@@ -388,4 +378,35 @@ describe('phone number registrar -- recycle module', () => {
})
})
})
describe('#redeem', () => {
const channelToRedeem = deepChannelFactory({ phoneNumber })
let destroyRecycleRequestStub
beforeEach(() => (destroyRecycleRequestStub = sinon.stub(recycleRequestRepository, 'destroy')))
describe('when all tasks succeed', () => {
beforeEach(async () => {
destroyRecycleRequestStub.returns(Promise.resolve(1))
notifyMaintainersStub.returns(Promise.resolve(['42']))
notifyAdminsStub.returns(Promise.resolve(['42', '42']))
await redeem(channelToRedeem)
})
it('deletes the recycle requests for redeemed channels', () => {
expect(destroyRecycleRequestStub.getCall(0).args).to.eql([phoneNumber])
})
it('notifies admins of redeemed channels of redemption', () => {
expect(notifyAdminsStub.getCall(0).args).to.eql([
channelToRedeem,
notificationKeys.CHANNEL_REDEEMED,
])
})
it('notifies maintainers of results', () => {
expect(notifyMaintainersStub.getCall(0).args).to.eql([
`${phoneNumber} had been scheduled for recycling, but was just redeemed.`,
])
})
})
})
})
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment