From 7ece2ad9260871e4b30a099a66bf5331a260415a Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 20 May 2021 19:29:35 +0800 Subject: [PATCH 1/3] refactor: rename ObjectID to ObjectId --- src/app/modules/webhook/__tests__/webhook.service.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/modules/webhook/__tests__/webhook.service.spec.ts b/src/app/modules/webhook/__tests__/webhook.service.spec.ts index 62c73ca322..10edc4cf69 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -1,5 +1,5 @@ import axios, { AxiosError, AxiosRequestConfig, AxiosResponse } from 'axios' -import { ObjectID } from 'bson' +import { ObjectId } from 'bson' import mongoose from 'mongoose' import { mocked } from 'ts-jest/utils' @@ -96,7 +96,7 @@ describe('webhook.service', () => { jest.restoreAllMocks() // prepare for form creation workflow - const MOCK_ADMIN_OBJ_ID = new ObjectID() + const MOCK_ADMIN_OBJ_ID = new ObjectId() const MOCK_EPOCH = 1487076708000 const preloaded = await dbHandler.insertFormCollectionReqs({ userId: MOCK_ADMIN_OBJ_ID, @@ -183,7 +183,7 @@ describe('webhook.service', () => { // Act const actual = await saveWebhookRecord( - new ObjectID(), + new ObjectId(), mockWebhookResponse, ) From 68c7dc32244ffe11e667c41df728763e8d1d4829 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Wed, 5 May 2021 12:37:30 +0800 Subject: [PATCH 2/3] test: remove dependency on model layer --- .../webhook/__tests__/webhook.service.spec.ts | 145 +++++++----------- 1 file changed, 53 insertions(+), 92 deletions(-) diff --git a/src/app/modules/webhook/__tests__/webhook.service.spec.ts b/src/app/modules/webhook/__tests__/webhook.service.spec.ts index 10edc4cf69..3bc5f556b8 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -4,17 +4,11 @@ import mongoose from 'mongoose' import { mocked } from 'ts-jest/utils' import formsgSdk from 'src/app/config/formsg-sdk' -import getFormModel from 'src/app/models/form.server.model' import { getEncryptSubmissionModel } from 'src/app/models/submission.server.model' import { WebhookValidationError } from 'src/app/modules/webhook/webhook.errors' import * as WebhookValidationModule from 'src/app/modules/webhook/webhook.validation' import { transformMongoError } from 'src/app/utils/handle-mongo-error' -import { - IEncryptedSubmissionSchema, - IWebhookResponse, - ResponseMode, - WebhookView, -} from 'src/types' +import { IWebhookResponse, WebhookView } from 'src/types' import dbHandler from 'tests/unit/backend/helpers/jest-db' @@ -28,10 +22,13 @@ const MockAxios = mocked(axios, true) jest.mock('src/app/modules/webhook/webhook.validation') const MockWebhookValidationModule = mocked(WebhookValidationModule, true) -// define test constants -const FormModel = getFormModel(mongoose) +jest.mock('src/app/config/formsg-sdk') +const MockFormSgSdk = mocked(formsgSdk, true) + const EncryptSubmissionModel = getEncryptSubmissionModel(mongoose) +// define test constants + const MOCK_WEBHOOK_URL = 'https://form.gov.sg/endpoint' const DEFAULT_ERROR_MSG = 'a generic error has occurred' const AXIOS_ERROR_MSG = 'an axios error has occurred' @@ -80,6 +77,20 @@ const MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE: Pick< } describe('webhook.service', () => { + const MOCK_FORM_ID = new ObjectId().toHexString() + const MOCK_SUBMISSION_ID = new ObjectId().toHexString() + const MOCK_WEBHOOK_VIEW: WebhookView = { + data: { + created: new Date(), + encryptedContent: 'mockEncryptedContent', + formId: MOCK_FORM_ID, + submissionId: MOCK_SUBMISSION_ID, + verifiedContent: 'mockVerifiedContent', + version: 1, + }, + } + const MOCK_SIGNATURE = 'mockSignature' + beforeAll(async () => await dbHandler.connect()) afterEach(async () => { await dbHandler.clearDatabase() @@ -87,55 +98,23 @@ describe('webhook.service', () => { afterAll(async () => await dbHandler.closeDatabase()) // test variables - let testEncryptedSubmission: IEncryptedSubmissionSchema let testConfig: AxiosRequestConfig - let testSubmissionWebhookView: WebhookView | null - let testSignature: string beforeEach(async () => { jest.restoreAllMocks() - // prepare for form creation workflow - const MOCK_ADMIN_OBJ_ID = new ObjectId() const MOCK_EPOCH = 1487076708000 - const preloaded = await dbHandler.insertFormCollectionReqs({ - userId: MOCK_ADMIN_OBJ_ID, - }) - jest.spyOn(Date, 'now').mockImplementation(() => MOCK_EPOCH) - // instantiate new form and save - const testEncryptedForm = await FormModel.create({ - title: 'Test Form', - admin: preloaded.user._id, - responseMode: ResponseMode.Encrypt, - publicKey: 'fake-public-key', - }) - - // initialise encrypted submussion - testEncryptedSubmission = await EncryptSubmissionModel.create({ - form: testEncryptedForm._id, - authType: testEncryptedForm.authType, - myInfoFields: [], - encryptedContent: 'encrypted-content', - verifiedContent: 'verified-content', - version: 1, - webhookResponses: [], - }) - - // initialise webhook related variables - testSubmissionWebhookView = testEncryptedSubmission.getWebhookView() - - testSignature = formsgSdk.webhooks.generateSignature({ - uri: MOCK_WEBHOOK_URL, - submissionId: testEncryptedSubmission._id, - formId: testEncryptedForm._id, - epoch: MOCK_EPOCH, - }) + MockFormSgSdk.webhooks.generateSignature.mockReturnValueOnce(MOCK_SIGNATURE) + const mockWebhookHeader = `t=${MOCK_EPOCH},s=${MOCK_SUBMISSION_ID},f=${MOCK_FORM_ID},v1=${MOCK_SIGNATURE}` + MockFormSgSdk.webhooks.constructHeader.mockReturnValueOnce( + mockWebhookHeader, + ) testConfig = { headers: { - 'X-FormSG-Signature': `t=${MOCK_EPOCH},s=${testEncryptedSubmission._id},f=${testEncryptedForm._id},v1=${testSignature}`, + 'X-FormSG-Signature': mockWebhookHeader, }, maxRedirects: 0, timeout: 10000, @@ -147,7 +126,7 @@ describe('webhook.service', () => { // Arrange const mockWebhookResponse = { ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } as IWebhookResponse @@ -159,7 +138,7 @@ describe('webhook.service', () => { // Act const actual = await saveWebhookRecord( - testEncryptedSubmission._id, + MOCK_SUBMISSION_ID, mockWebhookResponse, ) @@ -167,7 +146,7 @@ describe('webhook.service', () => { const expectedError = transformMongoError(mockDBError) expect(addWebhookResponseSpy).toHaveBeenCalledWith( - testEncryptedSubmission._id, + MOCK_SUBMISSION_ID, mockWebhookResponse, ) expect(actual._unsafeUnwrapErr()).toEqual(expectedError) @@ -177,7 +156,7 @@ describe('webhook.service', () => { // Arrange const mockWebhookResponse = { ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } as IWebhookResponse @@ -198,15 +177,15 @@ describe('webhook.service', () => { it('should return updated submission with new webhook response if the record is successfully saved', async () => { // Arrange const mockWebhookResponse = { - _id: testEncryptedSubmission._id, - created: testEncryptedSubmission.created, + _id: MOCK_SUBMISSION_ID, + created: new Date(), ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } as IWebhookResponse const expectedSubmission = new EncryptSubmissionModel({ - ...testEncryptedSubmission, + _id: MOCK_SUBMISSION_ID, }) expectedSubmission.webhookResponses = [mockWebhookResponse] @@ -216,13 +195,13 @@ describe('webhook.service', () => { // Act const actual = await saveWebhookRecord( - testEncryptedSubmission._id, + MOCK_SUBMISSION_ID, mockWebhookResponse, ) // Assert expect(addWebhookResponseSpy).toHaveBeenCalledWith( - testEncryptedSubmission._id, + MOCK_SUBMISSION_ID, mockWebhookResponse, ) expect(actual._unsafeUnwrap()).toEqual(expectedSubmission) @@ -237,10 +216,7 @@ describe('webhook.service', () => { ) // Act - const actual = await sendWebhook( - testEncryptedSubmission.getWebhookView(), - MOCK_WEBHOOK_URL, - ) + const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) // Assert const expectedError = new WebhookValidationError(DEFAULT_ERROR_MSG) @@ -258,10 +234,7 @@ describe('webhook.service', () => { ) // Act - const actual = await sendWebhook( - testEncryptedSubmission.getWebhookView(), - MOCK_WEBHOOK_URL, - ) + const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) // Assert const expectedError = new WebhookValidationError( @@ -288,28 +261,25 @@ describe('webhook.service', () => { toJSON: () => jest.fn(), } - expect( - MockWebhookValidationModule.validateWebhookUrl, - ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) MockAxios.post.mockRejectedValue(MOCK_AXIOS_ERROR) MockAxios.isAxiosError.mockReturnValue(true) // Act - const actual = await sendWebhook( - testEncryptedSubmission.getWebhookView(), - MOCK_WEBHOOK_URL, - ) + const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) // Assert const expectedResult = { ...MOCK_WEBHOOK_FAILURE_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } + expect( + MockWebhookValidationModule.validateWebhookUrl, + ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) @@ -323,15 +293,12 @@ describe('webhook.service', () => { MockAxios.isAxiosError.mockReturnValue(false) // Act - const actual = await sendWebhook( - testEncryptedSubmission.getWebhookView(), - MOCK_WEBHOOK_URL, - ) + const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) // Assert const expectedResult = { ...MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } @@ -340,7 +307,7 @@ describe('webhook.service', () => { ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) @@ -356,15 +323,12 @@ describe('webhook.service', () => { MockAxios.isAxiosError.mockReturnValue(false) // Act - const actual = await sendWebhook( - testEncryptedSubmission.getWebhookView(), - MOCK_WEBHOOK_URL, - ) + const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) // Assert const expectedResult = { ...MOCK_WEBHOOK_DEFAULT_FORMAT_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } @@ -373,7 +337,7 @@ describe('webhook.service', () => { ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) @@ -386,15 +350,12 @@ describe('webhook.service', () => { MockAxios.post.mockResolvedValue(MOCK_AXIOS_SUCCESS_RESPONSE) // Act - const actual = await sendWebhook( - testEncryptedSubmission.getWebhookView(), - MOCK_WEBHOOK_URL, - ) + const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) // Assert const expectedResult = { ...MOCK_WEBHOOK_SUCCESS_RESPONSE, - signature: testSignature, + signature: MOCK_SIGNATURE, webhookUrl: MOCK_WEBHOOK_URL, } @@ -403,7 +364,7 @@ describe('webhook.service', () => { ).toHaveBeenCalledWith(MOCK_WEBHOOK_URL) expect(MockAxios.post).toHaveBeenCalledWith( MOCK_WEBHOOK_URL, - testSubmissionWebhookView, + MOCK_WEBHOOK_VIEW, testConfig, ) expect(actual._unsafeUnwrap()).toEqual(expectedResult) From 523bdd652ebe005042ec3f40029bf831dbd67ff3 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Fri, 28 May 2021 14:20:46 +0800 Subject: [PATCH 3/3] test: add tests for createInitialWebhookSender --- .../webhook/__tests__/webhook.service.spec.ts | 124 ++++++++++++++++-- 1 file changed, 113 insertions(+), 11 deletions(-) diff --git a/src/app/modules/webhook/__tests__/webhook.service.spec.ts b/src/app/modules/webhook/__tests__/webhook.service.spec.ts index 3bc5f556b8..bb21dcd9df 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -1,6 +1,7 @@ import axios, { AxiosError, AxiosRequestConfig, AxiosResponse } from 'axios' import { ObjectId } from 'bson' import mongoose from 'mongoose' +import { ok, okAsync } from 'neverthrow' import { mocked } from 'ts-jest/utils' import formsgSdk from 'src/app/config/formsg-sdk' @@ -8,12 +9,18 @@ import { getEncryptSubmissionModel } from 'src/app/models/submission.server.mode import { WebhookValidationError } from 'src/app/modules/webhook/webhook.errors' import * as WebhookValidationModule from 'src/app/modules/webhook/webhook.validation' import { transformMongoError } from 'src/app/utils/handle-mongo-error' -import { IWebhookResponse, WebhookView } from 'src/types' +import { + IEncryptedSubmissionSchema, + IWebhookResponse, + WebhookView, +} from 'src/types' import dbHandler from 'tests/unit/backend/helpers/jest-db' import { SubmissionNotFoundError } from '../../submission/submission.errors' -import { saveWebhookRecord, sendWebhook } from '../webhook.service' +import { WebhookQueueMessage } from '../webhook.message' +import { WebhookProducer } from '../webhook.producer' +import * as WebhookService from '../webhook.service' // define suite-wide mocks jest.mock('axios') @@ -25,6 +32,9 @@ const MockWebhookValidationModule = mocked(WebhookValidationModule, true) jest.mock('src/app/config/formsg-sdk') const MockFormSgSdk = mocked(formsgSdk, true) +jest.mock('../webhook.message.ts') +const MockWebhookQueueMessage = mocked(WebhookQueueMessage, true) + const EncryptSubmissionModel = getEncryptSubmissionModel(mongoose) // define test constants @@ -137,7 +147,7 @@ describe('webhook.service', () => { .mockRejectedValueOnce(mockDBError) // Act - const actual = await saveWebhookRecord( + const actual = await WebhookService.saveWebhookRecord( MOCK_SUBMISSION_ID, mockWebhookResponse, ) @@ -161,7 +171,7 @@ describe('webhook.service', () => { } as IWebhookResponse // Act - const actual = await saveWebhookRecord( + const actual = await WebhookService.saveWebhookRecord( new ObjectId(), mockWebhookResponse, ) @@ -194,7 +204,7 @@ describe('webhook.service', () => { .mockResolvedValue(expectedSubmission) // Act - const actual = await saveWebhookRecord( + const actual = await WebhookService.saveWebhookRecord( MOCK_SUBMISSION_ID, mockWebhookResponse, ) @@ -216,7 +226,10 @@ describe('webhook.service', () => { ) // Act - const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, + MOCK_WEBHOOK_URL, + ) // Assert const expectedError = new WebhookValidationError(DEFAULT_ERROR_MSG) @@ -234,7 +247,10 @@ describe('webhook.service', () => { ) // Act - const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, + MOCK_WEBHOOK_URL, + ) // Assert const expectedError = new WebhookValidationError( @@ -265,7 +281,10 @@ describe('webhook.service', () => { MockAxios.isAxiosError.mockReturnValue(true) // Act - const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, + MOCK_WEBHOOK_URL, + ) // Assert const expectedResult = { @@ -293,7 +312,10 @@ describe('webhook.service', () => { MockAxios.isAxiosError.mockReturnValue(false) // Act - const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, + MOCK_WEBHOOK_URL, + ) // Assert const expectedResult = { @@ -323,7 +345,10 @@ describe('webhook.service', () => { MockAxios.isAxiosError.mockReturnValue(false) // Act - const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, + MOCK_WEBHOOK_URL, + ) // Assert const expectedResult = { @@ -350,7 +375,10 @@ describe('webhook.service', () => { MockAxios.post.mockResolvedValue(MOCK_AXIOS_SUCCESS_RESPONSE) // Act - const actual = await sendWebhook(MOCK_WEBHOOK_VIEW, MOCK_WEBHOOK_URL) + const actual = await WebhookService.sendWebhook( + MOCK_WEBHOOK_VIEW, + MOCK_WEBHOOK_URL, + ) // Assert const expectedResult = { @@ -370,4 +398,78 @@ describe('webhook.service', () => { expect(actual._unsafeUnwrap()).toEqual(expectedResult) }) }) + + describe('createInitialWebhookSender', () => { + // This suite only checks for correct behaviour for webhook retries, + // since there are separate tests for sending webhooks and saving + // responses to the database. + let testSubmission: IEncryptedSubmissionSchema + const MOCK_PRODUCER = ({ + sendMessage: jest.fn().mockReturnValue(okAsync(true)), + } as unknown) as WebhookProducer + beforeEach(() => { + jest.clearAllMocks() + + testSubmission = new EncryptSubmissionModel({ + _id: MOCK_SUBMISSION_ID, + }) + jest + .spyOn(EncryptSubmissionModel, 'addWebhookResponse') + .mockResolvedValue(testSubmission) + MockWebhookValidationModule.validateWebhookUrl.mockResolvedValue() + }) + + it('should return true without retrying when webhook is successful and retries are enabled', async () => { + MockAxios.post.mockResolvedValue(MOCK_AXIOS_SUCCESS_RESPONSE) + + const result = await WebhookService.createInitialWebhookSender( + MOCK_PRODUCER, + )(testSubmission, MOCK_WEBHOOK_URL, /* isRetryEnabled= */ true) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).not.toHaveBeenCalled() + }) + + it('should return true without retrying when webhook fails but retries are not enabled globally', async () => { + MockAxios.post.mockResolvedValue(MOCK_AXIOS_SUCCESS_RESPONSE) + + const result = await WebhookService + .createInitialWebhookSender + // no producer passed to createInitialWebhookSender, so retries not enabled globally + ()(testSubmission, MOCK_WEBHOOK_URL, true) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).not.toHaveBeenCalled() + }) + + it('should return true without retrying when webhook fails and retries are not enabled for form', async () => { + MockAxios.post.mockResolvedValue(MOCK_AXIOS_FAILURE_RESPONSE) + + const result = await WebhookService + .createInitialWebhookSender + // no producer passed to createInitialWebhookSender, so retries not enabled globally + ()(testSubmission, MOCK_WEBHOOK_URL, /* isRetryEnabled= */ false) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).not.toHaveBeenCalled() + }) + + it('should return true and retry when webhook fails and retries are enabled', async () => { + const mockQueueMessage = ('mockQueueMessage' as unknown) as WebhookQueueMessage + MockWebhookQueueMessage.fromSubmissionId.mockReturnValueOnce( + ok(mockQueueMessage), + ) + MockAxios.post.mockResolvedValue(MOCK_AXIOS_FAILURE_RESPONSE) + + const result = await WebhookService.createInitialWebhookSender( + MOCK_PRODUCER, + )(testSubmission, MOCK_WEBHOOK_URL, /* isRetryEnabled= */ true) + + expect(result._unsafeUnwrap()).toBe(true) + expect(MockWebhookQueueMessage.fromSubmissionId).toHaveBeenCalledWith( + String(testSubmission._id), + ) + expect(MOCK_PRODUCER.sendMessage).toHaveBeenCalledWith(mockQueueMessage) + }) + }) })