diff --git a/src/api/forms/service/callSessionTransaction.js b/src/api/forms/service/callSessionTransaction.js new file mode 100644 index 00000000..0b1fb071 --- /dev/null +++ b/src/api/forms/service/callSessionTransaction.js @@ -0,0 +1,51 @@ +import * as formMetadata from '~/src/api/forms/repositories/form-metadata-repository.js' +import { logger, partialAuditFields } from '~/src/api/forms/service/shared.js' +import { client } from '~/src/mongo.js' + +/** + * Abstraction of a generic service method + * @template T + * @param {string} formId + * @param {(session: ClientSession) => Promise} asyncHandler + * @param {FormMetadataAuthor} author + * @param {{ start: string; end: string; fail: string }} logs + * @returns {Promise} + */ +export async function callSessionTransaction( + formId, + asyncHandler, + author, + logs +) { + logger.info(logs.start) + + const session = client.startSession() + + try { + const sessionReturn = await session.withTransaction(async () => { + const handlerReturn = await asyncHandler(session) + + // Update the form with the new draft state + await formMetadata.update( + formId, + { $set: partialAuditFields(new Date(), author) }, + session + ) + + return handlerReturn + }) + logger.info(logs.end) + + return sessionReturn + } catch (err) { + logger.error(err, logs.fail) + throw err + } finally { + await session.endSession() + } +} + +/** + * @import { FormMetadataAuthor } from '@defra/forms-model' + * @import { ClientSession } from 'mongodb' + */ diff --git a/src/api/forms/service/callSessionTransaction.test.js b/src/api/forms/service/callSessionTransaction.test.js new file mode 100644 index 00000000..04d7d484 --- /dev/null +++ b/src/api/forms/service/callSessionTransaction.test.js @@ -0,0 +1,80 @@ +import Boom from '@hapi/boom' +import { pino } from 'pino' + +import * as formMetadata from '~/src/api/forms/repositories/form-metadata-repository.js' +import { formMetadataDocument } from '~/src/api/forms/service/__stubs__/service.js' +import { callSessionTransaction } from '~/src/api/forms/service/callSessionTransaction.js' +import { getAuthor } from '~/src/helpers/get-author.js' +import { prepareDb } from '~/src/mongo.js' + +jest.mock('~/src/helpers/get-author.js') +jest.mock('~/src/api/forms/repositories/form-metadata-repository.js') +jest.mock('~/src/mongo.js') + +jest.useFakeTimers().setSystemTime(new Date('2020-01-01')) +describe('lists', () => { + beforeAll(async () => { + await prepareDb(pino()) + }) + + beforeEach(() => { + jest.mocked(formMetadata.get).mockResolvedValue(formMetadataDocument) + }) + + describe('callSessionTransaction', () => { + const id = '661e4ca5039739ef2902b214' + const author = getAuthor() + const dateUsedInFakeTime = new Date('2020-01-01') + const defaultAudit = { + 'draft.updatedAt': dateUsedInFakeTime, + 'draft.updatedBy': author, + updatedAt: dateUsedInFakeTime, + updatedBy: author + } + + beforeAll(async () => { + await prepareDb(pino()) + }) + + it('should update the component', async () => { + const transactionResolved = '265a71fd-f2c2-4028-94aa-7c1e2739730f' + const transactionHandler = jest + .fn() + .mockResolvedValue(transactionResolved) + + const dbMetadataSpy = jest.spyOn(formMetadata, 'update') + + const result = await callSessionTransaction( + id, + transactionHandler, + author, + { + start: 'started', + end: 'finished', + fail: 'failed' + } + ) + + expect(transactionHandler).toHaveBeenCalled() + expect(result).toBe(transactionResolved) + const [metaFormId, metaUpdateOperations] = dbMetadataSpy.mock.calls[0] + expect(metaFormId).toBe(id) + + expect(metaUpdateOperations.$set).toEqual(defaultAudit) + }) + + it('should correctly surface the error is the component is not found', async () => { + const transactionHandler = jest + .fn() + .mockRejectedValue(Boom.notFound('Not found')) + + await expect( + callSessionTransaction(id, transactionHandler, author, { + start: 'started', + end: 'finished', + fail: 'failed' + }) + ).rejects.toThrow(Boom.notFound('Not found')) + }) + }) +}) diff --git a/src/api/forms/service/index.test.js b/src/api/forms/service/index.test.js index bab369a4..42a8053d 100644 --- a/src/api/forms/service/index.test.js +++ b/src/api/forms/service/index.test.js @@ -2,6 +2,7 @@ import Boom from '@hapi/boom' import { MongoServerError, ObjectId } from 'mongodb' import { pino } from 'pino' +import { buildList } from '~/src/api/forms/__stubs__/definition.js' import { InvalidFormDefinitionError } from '~/src/api/forms/errors.js' import * as formDefinition from '~/src/api/forms/repositories/form-definition-repository.js' import * as formMetadata from '~/src/api/forms/repositories/form-metadata-repository.js' @@ -21,6 +22,7 @@ import { removeForm, updateFormMetadata } from '~/src/api/forms/service/index.js' +import { addListsToDraftFormDefinition } from '~/src/api/forms/service/lists.js' import * as formTemplates from '~/src/api/forms/templates.js' import { getAuthor } from '~/src/helpers/get-author.js' import { prepareDb } from '~/src/mongo.js' @@ -355,6 +357,42 @@ describe('Forms service', () => { ) }) }) + + describe('high level tests', () => { + const defaultAudit = { + 'draft.updatedAt': dateUsedInFakeTime, + 'draft.updatedBy': author, + updatedAt: dateUsedInFakeTime, + updatedBy: author + } + const dbMetadataSpy = jest.spyOn(formMetadata, 'update') + const expectMetadataUpdate = () => { + expect(dbMetadataSpy).toHaveBeenCalled() + const [formId, updateFilter] = dbMetadataSpy.mock.calls[0] + expect(formId).toBe(id) + expect(updateFilter.$set).toEqual(defaultAudit) + } + + describe('addListsToDraftFormDefinition', () => { + it('should add a list of lists to the form definition', async () => { + const expectedLists = [buildList()] + const addListsMock = jest + .mocked(formDefinition.addLists) + .mockResolvedValueOnce(expectedLists) + + const result = await addListsToDraftFormDefinition( + id, + expectedLists, + author + ) + const [expectedFormId, listToInsert] = addListsMock.mock.calls[0] + expect(expectedFormId).toBe(id) + expect(listToInsert).toEqual(expectedLists) + expect(result).toEqual(expectedLists) + expectMetadataUpdate() + }) + }) + }) }) /** diff --git a/src/api/forms/service/lists.js b/src/api/forms/service/lists.js index e547a844..61573e38 100644 --- a/src/api/forms/service/lists.js +++ b/src/api/forms/service/lists.js @@ -1,7 +1,5 @@ import * as formDefinition from '~/src/api/forms/repositories/form-definition-repository.js' -import * as formMetadata from '~/src/api/forms/repositories/form-metadata-repository.js' -import { logger, partialAuditFields } from '~/src/api/forms/service/shared.js' -import { client } from '~/src/mongo.js' +import { callSessionTransaction } from '~/src/api/forms/service/callSessionTransaction.js' /** * Add a list of new lists to the draft form definition @@ -10,48 +8,17 @@ import { client } from '~/src/mongo.js' * @param {FormMetadataAuthor} author */ export async function addListsToDraftFormDefinition(formId, lists, author) { - logger.info( - `Adding lists ${lists.map((list) => list.name).join(', ')} on Form Definition (draft) for form ID ${formId}` + const listStr = lists.map((list) => list.name).join(', ') + return callSessionTransaction( + formId, + (session) => formDefinition.addLists(formId, lists, session), + author, + { + start: `Adding lists ${listStr} on Form Definition (draft) for form ID ${formId}`, + end: `Added lists ${listStr} on Form Definition (draft) for form ID ${formId}`, + fail: `Failed to add lists ${listStr} on Form Definition (draft) for form ID ${formId}` + } ) - - const session = client.startSession() - - try { - const newForm = await session.withTransaction(async () => { - // Add the lists to the form definition - const returnedLists = await formDefinition.addLists( - formId, - lists, - session - ) - - const now = new Date() - await formMetadata.update( - formId, - { - $set: partialAuditFields(now, author) - }, - session - ) - - return returnedLists - }) - - logger.info( - `Added lists ${lists.map((list) => list.name).join(', ')} on Form Definition (draft) for form ID ${formId}` - ) - - return newForm - } catch (err) { - logger.error( - err, - `Failed to add lists ${lists.map((list) => list.id).join(', ')} on Form Definition (draft) for form ID ${formId}` - ) - - throw err - } finally { - await session.endSession() - } } /** @@ -67,49 +34,16 @@ export async function updateListOnDraftFormDefinition( list, author ) { - logger.info( - `Updating list ${listId} on Form Definition (draft) for form ID ${formId}` + return callSessionTransaction( + formId, + (session) => formDefinition.updateList(formId, listId, list, session), + author, + { + start: `Updating list ${listId} on Form Definition (draft) for form ID ${formId}`, + end: `Updated list ${listId} on Form Definition (draft) for form ID ${formId}`, + fail: `Failed to update list ${listId} on Form Definition (draft) for form ID ${formId}` + } ) - - const session = client.startSession() - - try { - const updatedList = await session.withTransaction(async () => { - // Update the list on the form definition - const returnedLists = await formDefinition.updateList( - formId, - listId, - list, - session - ) - - const now = new Date() - await formMetadata.update( - formId, - { - $set: partialAuditFields(now, author) - }, - session - ) - - return returnedLists - }) - - logger.info( - `Updated list ${listId} on Form Definition (draft) for form ID ${formId}` - ) - - return updatedList - } catch (err) { - logger.error( - err, - `Failed to update list ${listId} on Form Definition (draft) for form ID ${formId}` - ) - - throw err - } finally { - await session.endSession() - } } /** @@ -119,42 +53,19 @@ export async function updateListOnDraftFormDefinition( * @param {FormMetadataAuthor} author */ export async function removeListOnDraftFormDefinition(formId, listId, author) { - logger.info( - `Removing list ${listId} on Form Definition (draft) for form ID ${formId}` + await callSessionTransaction( + formId, + (session) => formDefinition.removeList(formId, listId, session), + author, + { + start: `Removing list ${listId} on Form Definition (draft) for form ID ${formId}`, + end: `Removed list ${listId} on Form Definition (draft) for form ID ${formId}`, + fail: `Failed to remove list ${listId} on Form Definition (draft) for form ID ${formId}` + } ) - - const session = client.startSession() - - try { - await session.withTransaction(async () => { - // Update the list on the form definition - await formDefinition.removeList(formId, listId, session) - - const now = new Date() - await formMetadata.update( - formId, - { - $set: partialAuditFields(now, author) - }, - session - ) - }) - - logger.info( - `Removed list ${listId} on Form Definition (draft) for form ID ${formId}` - ) - } catch (err) { - logger.error( - err, - `Failed to remove list ${listId} on Form Definition (draft) for form ID ${formId}` - ) - - throw err - } finally { - await session.endSession() - } } /** * @import { FormMetadataAuthor, List } from '@defra/forms-model' + * @import { ClientSession } from 'mongodb' */ diff --git a/src/api/forms/service/lists.test.js b/src/api/forms/service/lists.test.js index 4fbaba6d..f10c2b76 100644 --- a/src/api/forms/service/lists.test.js +++ b/src/api/forms/service/lists.test.js @@ -1,18 +1,14 @@ -import Boom from '@hapi/boom' -import { pino } from 'pino' - import { buildList } from '~/src/api/forms/__stubs__/definition.js' import * as formDefinition from '~/src/api/forms/repositories/form-definition-repository.js' -import * as formMetadata from '~/src/api/forms/repositories/form-metadata-repository.js' -import { formMetadataDocument } from '~/src/api/forms/service/__stubs__/service.js' +import { callSessionTransaction } from '~/src/api/forms/service/callSessionTransaction.js' import { addListsToDraftFormDefinition, removeListOnDraftFormDefinition, updateListOnDraftFormDefinition } from '~/src/api/forms/service/lists.js' import { getAuthor } from '~/src/helpers/get-author.js' -import { prepareDb } from '~/src/mongo.js' +jest.mock('~/src/api/forms/service/callSessionTransaction.js') jest.mock('~/src/helpers/get-author.js') jest.mock('~/src/api/forms/repositories/form-definition-repository.js') jest.mock('~/src/api/forms/repositories/form-metadata-repository.js') @@ -23,63 +19,58 @@ jest.useFakeTimers().setSystemTime(new Date('2020-01-01')) describe('lists', () => { const id = '661e4ca5039739ef2902b214' const author = getAuthor() - const dateUsedInFakeTime = new Date('2020-01-01') - const defaultAudit = { - 'draft.updatedAt': dateUsedInFakeTime, - 'draft.updatedBy': author, - updatedAt: dateUsedInFakeTime, - updatedBy: author - } - - const dbMetadataSpy = jest.spyOn(formMetadata, 'update') - - const expectMetadataUpdate = () => { - expect(dbMetadataSpy).toHaveBeenCalled() - const [formId, updateFilter] = dbMetadataSpy.mock.calls[0] - expect(formId).toBe(id) - expect(updateFilter.$set).toEqual(defaultAudit) - } - - beforeAll(async () => { - await prepareDb(pino()) - }) + const callSessionTransactionMock = jest.mocked(callSessionTransaction) + /** + * @type {any} + */ + const mockSession = author + const listId = '47cfaf57-6cda-44aa-9268-f37c674823d2' beforeEach(() => { - jest.mocked(formMetadata.get).mockResolvedValue(formMetadataDocument) + callSessionTransactionMock.mockClear() }) describe('addListsToDraftFormDefinition', () => { - it('should add a list of lists to the form definition', async () => { - const expectedLists = [buildList()] + it('should call callSessionTransaction with the correct parameters', async () => { + const expectedLists = [ + buildList({ + name: 'my-list' + }), + buildList({ + name: 'my-list-2' + }) + ] const addListsMock = jest .mocked(formDefinition.addLists) .mockResolvedValueOnce(expectedLists) + callSessionTransactionMock.mockResolvedValueOnce(expectedLists) const result = await addListsToDraftFormDefinition( id, expectedLists, author ) - const [expectedFormId, listToInsert] = addListsMock.mock.calls[0] - expect(expectedFormId).toBe(id) - expect(listToInsert).toEqual(expectedLists) + expect(callSessionTransactionMock).toHaveBeenCalledWith( + id, + expect.any(Function), + author, + { + start: `Adding lists my-list, my-list-2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214`, + end: `Added lists my-list, my-list-2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214`, + fail: `Failed to add lists my-list, my-list-2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214` + } + ) + const [, callback] = callSessionTransactionMock.mock.calls[0] + await callback(mockSession) + expect(addListsMock).toHaveBeenCalledWith(id, expectedLists, mockSession) expect(result).toEqual(expectedLists) - expectMetadataUpdate() - }) - it('should surface errors', async () => { - const boomInternal = Boom.internal('Something went wrong') - jest.mocked(formDefinition.addLists).mockRejectedValueOnce(boomInternal) - await expect( - addListsToDraftFormDefinition(id, [buildList()], author) - ).rejects.toThrow(boomInternal) }) }) describe('updateListOnDraftFormDefinition', () => { - const listToUpdate = buildList() - const listId = '47cfaf57-6cda-44aa-9268-f37c674823d2' - it('should update a list on the form definition', async () => { + const listToUpdate = buildList() + callSessionTransactionMock.mockResolvedValueOnce(listToUpdate) const updateListMock = jest .mocked(formDefinition.updateList) .mockResolvedValueOnce(listToUpdate) @@ -90,41 +81,50 @@ describe('lists', () => { listToUpdate, author ) - const [expectedFormId, expectedListId, expectedListToUpdate] = - updateListMock.mock.calls[0] - expect(expectedFormId).toBe(id) - expect(expectedListId).toBe(listId) - expect(expectedListToUpdate).toEqual(listToUpdate) + + expect(callSessionTransactionMock).toHaveBeenCalledWith( + id, + expect.any(Function), + author, + { + start: `Updating list 47cfaf57-6cda-44aa-9268-f37c674823d2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214`, + end: `Updated list 47cfaf57-6cda-44aa-9268-f37c674823d2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214`, + fail: `Failed to update list 47cfaf57-6cda-44aa-9268-f37c674823d2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214` + } + ) + + const [, callback] = callSessionTransactionMock.mock.calls[0] + await callback(mockSession) + expect(updateListMock).toHaveBeenCalledWith( + id, + listId, + listToUpdate, + mockSession + ) expect(result).toEqual(listToUpdate) - expectMetadataUpdate() - }) - it('should surface errors', async () => { - const boomInternal = Boom.internal('Something went wrong') - jest.mocked(formDefinition.updateList).mockRejectedValueOnce(boomInternal) - await expect( - updateListOnDraftFormDefinition(id, listId, listToUpdate, author) - ).rejects.toThrow(boomInternal) }) }) describe('removeListOnDraftFormDefinition', () => { - const listId = '47cfaf57-6cda-44aa-9268-f37c674823d2' - it('should remove a list on the form definition', async () => { await removeListOnDraftFormDefinition(id, listId, author) - const [expectedFormId, expectedListId] = jest.mocked( - formDefinition.removeList - ).mock.calls[0] - expect(expectedFormId).toBe(id) - expect(expectedListId).toBe(listId) - expectMetadataUpdate() - }) - it('should surface errors', async () => { - const boomInternal = Boom.internal('Something went wrong') - jest.mocked(formDefinition.removeList).mockRejectedValueOnce(boomInternal) - await expect( - removeListOnDraftFormDefinition(id, listId, author) - ).rejects.toThrow(boomInternal) + expect(callSessionTransactionMock).toHaveBeenCalledWith( + id, + expect.any(Function), + author, + { + start: `Removing list 47cfaf57-6cda-44aa-9268-f37c674823d2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214`, + end: `Removed list 47cfaf57-6cda-44aa-9268-f37c674823d2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214`, + fail: `Failed to remove list 47cfaf57-6cda-44aa-9268-f37c674823d2 on Form Definition (draft) for form ID 661e4ca5039739ef2902b214` + } + ) + const [, callback] = callSessionTransactionMock.mock.calls[0] + await callback(mockSession) + expect(jest.mocked(formDefinition.removeList)).toHaveBeenCalledWith( + id, + listId, + mockSession + ) }) }) }) diff --git a/src/api/forms/service/shared.js b/src/api/forms/service/shared.js index 26dfea73..314fda1f 100644 --- a/src/api/forms/service/shared.js +++ b/src/api/forms/service/shared.js @@ -89,10 +89,6 @@ export function getLastUpdated(document) { } } -/** - * @import { FormMetadataAuthor } from '@defra/forms-model' - * @import { PartialFormMetadataDocument } from '~/src/api/types.js' - */ /** * @param {Partial} document - form metadata document * @returns {{ createdAt: Date, createdBy: FormMetadataAuthor }} @@ -111,6 +107,7 @@ export function getCreated(document) { } /** - * @import { FormMetadataDocument, FormMetadata } from '@defra/forms-model' - * @import { WithId } from 'mongodb' + * @import { FormMetadataDocument, FormMetadata, FormMetadataAuthor } from '@defra/forms-model' + * @import { PartialFormMetadataDocument } from '~/src/api/types.js' + * @import { WithId, ClientSession } from 'mongodb' */