Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move UUID generator to single place #415

Merged
merged 5 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions app/lib/general.lib.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,28 @@
* @module GeneralLib
*/

const { randomUUID } = require('crypto')

/**
* Generate a Universally Unique Identifier (UUID)
*
* The service uses these as the IDs for most records in the DB. Most tables will automatically generate them when
* the record is created but not all do. There are also times when it is either more performant, simpler, or both for
* us to generate the ID before inserting a new record. For example, we can pass the generated ID to child records to
* set the foreign key relationship.
*
* NOTE: We set `disableEntropyCache` to `false` as normally, for performance reasons node caches enough random data to
* generate up to 128 UUIDs. We disable this as we may need to generate more than this and the performance hit in
* disabling this cache is a rounding error in comparison to the rest of the process.
*
* https://nodejs.org/api/crypto.html#cryptorandomuuidoptions
*
* @returns {String} a randomly generated UUID
*/
function generateUUID () {
return randomUUID({ disableEntropyCache: true })
}

/**
* Returns the current date and time as an ISO string
*
Expand All @@ -21,5 +43,6 @@ function timestampForPostgres () {
}

module.exports = {
generateUUID,
timestampForPostgres
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @module GenerateBillingInvoiceLicenceService
*/

const { randomUUID } = require('crypto')
const { generateUUID } = require('../../../lib/general.lib.js')

/**
* Return a billing invoice licence object ready for persisting
Expand All @@ -19,7 +19,7 @@ const { randomUUID } = require('crypto')
function go (billingInvoiceId, licence) {
const billingInvoiceLicence = {
billingInvoiceId,
billingInvoiceLicenceId: randomUUID({ disableEntropyCache: true }),
billingInvoiceLicenceId: generateUUID(),
licenceRef: licence.licenceRef,
licenceId: licence.licenceId
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @module GenerateBillingInvoiceService
*/

const { randomUUID } = require('crypto')
const { generateUUID } = require('../../../lib/general.lib.js')

/**
* Return a billing invoice object ready for persisting
Expand All @@ -21,7 +21,7 @@ function go (invoiceAccount, billingBatchId, financialYearEnding) {
billingBatchId,
financialYearEnding,
invoiceAccountId: invoiceAccount.invoiceAccountId,
billingInvoiceId: randomUUID({ disableEntropyCache: true }),
billingInvoiceId: generateUUID(),
address: {}, // Address is set to an empty object for SROC billing invoices
invoiceAccountNumber: invoiceAccount.invoiceAccountNumber,
isCredit: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @module GenerateBillingTransactionsService
*/

const { randomUUID } = require('crypto')
const { generateUUID } = require('../../../lib/general.lib.js')

const CalculateAuthorisedAndBillableDaysServiceService = require('./calculate-authorised-and-billable-days.service.js')

Expand Down Expand Up @@ -46,7 +46,7 @@ function go (chargeElement, billingPeriod, chargePeriod, isNewLicence, isWaterUn
}

const standardTransaction = _standardTransaction(
_generateUuid(),
generateUUID(),
authorisedDays,
billableDays,
chargeElement,
Expand All @@ -57,7 +57,7 @@ function go (chargeElement, billingPeriod, chargePeriod, isNewLicence, isWaterUn
billingTransactions.push(standardTransaction)

if (!isWaterUndertaker) {
const compensationTransaction = _compensationTransaction(_generateUuid(), standardTransaction)
const compensationTransaction = _compensationTransaction(generateUUID(), standardTransaction)
billingTransactions.push(compensationTransaction)
}

Expand Down Expand Up @@ -86,25 +86,6 @@ function _description (chargeElement) {
return `Two-part tariff basic water abstraction charge: ${chargeElement.description}`
}

/**
* Return a unique UUID to be used as an ID
*
* We only intend to persist the transaction and associated billing invoice and licence if there is something to bill!
* But we have to provide the charging module with the ID of our transaction so it can protect against duplicates.
*
* So, we generate our transaction ID's in the code and avoid having to send a DB insert just to get back an ID to use.
*
* @returns {string} a unique UUID
*/
function _generateUuid () {
// We set `disableEntropyCache` to `false` as normally, for performance reasons node caches enough random data to
// generate up to 128 UUIDs. We disable this as we may need to generate more than this and the performance hit in
// disabling this cache is a rounding error in comparison to the rest of the process.
//
// https://nodejs.org/api/crypto.html#cryptorandomuuidoptions
return randomUUID({ disableEntropyCache: true })
}

/**
* Returns a json representation of all charge purposes in a charge element
*/
Expand Down
5 changes: 3 additions & 2 deletions app/services/billing/supplementary/reissue-invoice.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
* Handles the reissuing of a single invoice
* @module ReissueInvoiceService
*/
const { randomUUID } = require('crypto')

const { generateUUID } = require('../../../lib/general.lib.js')

const ChargingModuleBillRunStatusService = require('../../charging-module/bill-run-status.service.js')
const ChargingModuleReissueInvoiceService = require('../../charging-module/reissue-invoice.service.js')
Expand Down Expand Up @@ -152,7 +153,7 @@ async function _pauseUntilNotPending (billingBatchExternalId) {
function _generateTransaction (chargingModuleReissueTransaction, sourceTransaction, billingInvoiceLicenceId) {
return {
...sourceTransaction,
billingTransactionId: randomUUID({ disableEntropyCache: true }),
billingTransactionId: generateUUID(),
externalId: chargingModuleReissueTransaction.id,
isCredit: chargingModuleReissueTransaction.credit,
netAmount: _determineSignOfNetAmount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @module ReverseBillingTransactionsService
*/

const { randomUUID } = require('crypto')
const { generateUUID } = require('../../../lib/general.lib.js')

/**
* Takes an array of transactions and returns an array of transactions which will reverse them.
Expand Down Expand Up @@ -44,7 +44,7 @@ function _reverseTransactions (transactions, billingInvoiceLicence) {

return {
...propertiesToKeep,
billingTransactionId: _generateUuid(),
billingTransactionId: generateUUID(),
billingInvoiceLicenceId: billingInvoiceLicence.billingInvoiceLicenceId,
isCredit: true,
status: 'candidate',
Expand All @@ -57,15 +57,6 @@ function _reverseTransactions (transactions, billingInvoiceLicence) {
})
}

function _generateUuid () {
// We set `disableEntropyCache` to `false` as normally, for performance reasons node caches enough random data to
// generate up to 128 UUIDs. We disable this as we may need to generate more than this and the performance hit in
// disabling this cache is a rounding error in comparison to the rest of the process.
//
// https://nodejs.org/api/crypto.html#cryptorandomuuidoptions
return randomUUID({ disableEntropyCache: true })
}

module.exports = {
go
}
5 changes: 3 additions & 2 deletions db/seeds/01-users.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use strict'

const bcrypt = require('bcryptjs')
const { randomUUID } = require('crypto')

const { generateUUID } = require('../../app/lib/general.lib.js')

const DatabaseConfig = require('../../config/database.config.js')

Expand Down Expand Up @@ -107,7 +108,7 @@ async function _insertUserGroupsWhereNotExists (knex) {
if (!existingUserGroup) {
await knex('idm.userGroups')
.insert({
userGroupId: randomUUID({ disableEntropyCache: true }),
userGroupId: generateUUID(),
userId: seedUser.userId,
groupId: seedUser.groupId
})
Expand Down
15 changes: 15 additions & 0 deletions test/lib/general.lib.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@ const { expect } = Code
const GeneralLib = require('../../app/lib/general.lib.js')

describe('RequestLib', () => {
describe('#generateUUID', () => {
// NOTE: generateUUID() only calls crypto.randomUUID(); it does nothing else. So, there is nothing really to test
// and certainly, testing the UUID is really unique is beyond the scope of this project! But this test at least
// serves as documentation and means no one will get confused by the lack of a test :-)
it('returns a Universally unique identifier (UUID)', () => {
const uuid1 = GeneralLib.generateUUID()
const uuid2 = GeneralLib.generateUUID()
const uuid3 = GeneralLib.generateUUID()

expect(uuid1).not.to.equal(uuid2)
expect(uuid1).not.to.equal(uuid3)
expect(uuid2).not.to.equal(uuid3)
})
})

describe('#timestampForPostgres', () => {
let clock
let testDate
Expand Down
33 changes: 16 additions & 17 deletions test/services/billing/supplementary/reissue-invoice.service.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ const Sinon = require('sinon')
const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script()
const { expect } = Code

const { randomUUID } = require('crypto')

// Test helpers
const BillingInvoiceHelper = require('../../../support/helpers/water/billing-invoice.helper.js')
const BillingInvoiceLicenceHelper = require('../../../support/helpers/water/billing-invoice-licence.helper.js')
const BillingTransactionHelper = require('../../../support/helpers/water/billing-transaction.helper.js')
const DatabaseHelper = require('../../../support/helpers/database.helper.js')
const { generateUUID } = require('../../../../app/lib/general.lib.js')

// Things we need to stub
const ChargingModuleBillRunStatusService = require('../../../../app/services/charging-module/bill-run-status.service.js')
Expand All @@ -24,15 +23,15 @@ const ChargingModuleViewInvoiceService = require('../../../../app/services/charg
// Thing under test
const ReissueInvoiceService = require('../../../../app/services/billing/supplementary/reissue-invoice.service.js')

const ORIGINAL_BILLING_BATCH_EXTERNAL_ID = randomUUID({ disableEntropyCache: true })
const INVOICE_EXTERNAL_ID = randomUUID({ disableEntropyCache: true })
const INVOICE_LICENCE_1_TRANSACTION_ID = randomUUID({ disableEntropyCache: true })
const INVOICE_LICENCE_2_TRANSACTION_ID = randomUUID({ disableEntropyCache: true })
const ORIGINAL_BILLING_BATCH_EXTERNAL_ID = generateUUID()
const INVOICE_EXTERNAL_ID = generateUUID()
const INVOICE_LICENCE_1_TRANSACTION_ID = generateUUID()
const INVOICE_LICENCE_2_TRANSACTION_ID = generateUUID()

const CHARGING_MODULE_REISSUE_INVOICE_RESPONSE = {
invoices: [
{ id: randomUUID({ disableEntropyCache: true }), rebilledType: 'C' },
{ id: randomUUID({ disableEntropyCache: true }), rebilledType: 'R' }
{ id: generateUUID(), rebilledType: 'C' },
{ id: generateUUID(), rebilledType: 'R' }
]
}

Expand All @@ -48,12 +47,12 @@ const CHARGING_MODULE_VIEW_INVOICE_CREDIT_RESPONSE = {
creditLineValue: 2000,
licences: [
{
id: randomUUID({ disableEntropyCache: true }),
id: generateUUID(),
licenceNumber: 'INVOICE_LICENCE_1',
transactions: [_generateCMTransaction(true, INVOICE_LICENCE_1_TRANSACTION_ID)]
},
{
id: randomUUID({ disableEntropyCache: true }),
id: generateUUID(),
licenceNumber: 'INVOICE_LICENCE_2',
transactions: [_generateCMTransaction(true, INVOICE_LICENCE_2_TRANSACTION_ID)]
}
Expand All @@ -73,12 +72,12 @@ const CHARGING_MODULE_VIEW_INVOICE_REISSUE_RESPONSE = {
creditLineValue: 0,
licences: [
{
id: randomUUID({ disableEntropyCache: true }),
id: generateUUID(),
licenceNumber: 'INVOICE_LICENCE_1',
transactions: [_generateCMTransaction(false, INVOICE_LICENCE_1_TRANSACTION_ID)]
},
{
id: randomUUID({ disableEntropyCache: true }),
id: generateUUID(),
licenceNumber: 'INVOICE_LICENCE_2',
transactions: [_generateCMTransaction(false, INVOICE_LICENCE_2_TRANSACTION_ID)]
}
Expand All @@ -93,7 +92,7 @@ describe('Reissue invoice service', () => {
beforeEach(async () => {
await DatabaseHelper.clean()

reissueBillingBatch = { externalId: randomUUID({ disableEntropyCache: true }) }
reissueBillingBatch = { externalId: generateUUID() }

Sinon.stub(ChargingModuleReissueInvoiceService, 'go')
.withArgs(reissueBillingBatch.externalId, INVOICE_EXTERNAL_ID)
Expand Down Expand Up @@ -131,12 +130,12 @@ describe('Reissue invoice service', () => {
const sourceInvoiceLicences = await Promise.all([
BillingInvoiceLicenceHelper.add({
billingInvoiceId: sourceInvoice.billingInvoiceId,
licenceId: randomUUID({ disableEntropyCache: true }),
licenceId: generateUUID(),
licenceRef: 'INVOICE_LICENCE_1'
}),
BillingInvoiceLicenceHelper.add({
billingInvoiceId: sourceInvoice.billingInvoiceId,
licenceId: randomUUID({ disableEntropyCache: true }),
licenceId: generateUUID(),
licenceRef: 'INVOICE_LICENCE_2'
})
])
Expand Down Expand Up @@ -194,7 +193,7 @@ describe('Reissue invoice service', () => {
})

it("to the existing value if it's populated", async () => {
const ORIGINAL_BILLING_INVOICE_ID = randomUUID({ disableEntropyCache: true })
const ORIGINAL_BILLING_INVOICE_ID = generateUUID()
await sourceInvoice.$query().patch({ originalBillingInvoiceId: ORIGINAL_BILLING_INVOICE_ID })

await ReissueInvoiceService.go(sourceInvoice, reissueBillingBatch)
Expand Down Expand Up @@ -328,7 +327,7 @@ describe('Reissue invoice service', () => {

function _generateCMTransaction (credit, rebilledTransactionId) {
return {
id: randomUUID({ disableEntropyCache: true }),
id: generateUUID(),
chargeValue: 1000,
credit,
rebilledTransactionId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ const Sinon = require('sinon')
const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script()
const { expect } = Code

const { randomUUID } = require('crypto')

// Test helpers
const BillingInvoiceHelper = require('../../../support/helpers/water/billing-invoice.helper.js')
const BillingInvoiceModel = require('../../../../app/models/water/billing-invoice.model.js')
Expand All @@ -18,6 +16,7 @@ const BillingInvoiceLicenceModel = require('../../../../app/models/water/billing
const BillingTransactionHelper = require('../../../support/helpers/water/billing-transaction.helper.js')
const BillingTransactionModel = require('../../../../app/models/water/billing-transaction.model.js')
const DatabaseHelper = require('../../../support/helpers/database.helper.js')
const { generateUUID } = require('../../../../app/lib/general.lib.js')

// Things we need to stub
const LegacyRequestLib = require('../../../../app/lib/legacy-request.lib.js')
Expand All @@ -29,7 +28,7 @@ const ReissueInvoicesService = require('../../../../app/services/billing/supplem

describe('Reissue invoices service', () => {
let notifierStub
const reissueBillingBatch = { regionId: randomUUID({ disableEntropyCache: true }) }
const reissueBillingBatch = { regionId: generateUUID() }

beforeEach(async () => {
await DatabaseHelper.clean()
Expand Down Expand Up @@ -65,9 +64,9 @@ describe('Reissue invoices service', () => {
beforeEach(async () => {
// Three dummy invoices to ensure we iterate 3x
Sinon.stub(FetchInvoicesToBeReissuedService, 'go').resolves([
{ id: randomUUID({ disableEntropyCache: true }) },
{ id: randomUUID({ disableEntropyCache: true }) },
{ id: randomUUID({ disableEntropyCache: true }) }
{ id: generateUUID() },
{ id: generateUUID() },
{ id: generateUUID() }
])

// This stub will result in one new invoice, invoice licence and transaction for each dummy invoice
Expand Down
5 changes: 2 additions & 3 deletions test/support/helpers/idm/group.helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
* @module GroupHelper
*/

const { randomUUID } = require('crypto')

const { generateUUID } = require('../../../../app/lib/general.lib.js')
const GroupModel = require('../../../../app/models/idm/group.model.js')

/**
Expand Down Expand Up @@ -40,7 +39,7 @@ function add (data = {}) {
function defaults (data = {}) {
const defaults = {
// We create a random uuid as the id is NOT generated by the db, unlike most other tables
groupId: randomUUID({ disableEntropyCache: true }),
groupId: generateUUID(),
application: 'water_admin',
group: 'wirs',
description: 'Waste Industry Regulatory Services'
Expand Down
Loading