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

feat(identity): unit tests for services #369

Merged
merged 15 commits into from
Mar 17, 2022
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
15 changes: 15 additions & 0 deletions __mocks__/@aws-sdk/client-secrets-manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// This is a manual module mock for the AWS client.
// This is done to prevent our internal services from pinging the actual AWS ones
// and to ensure our tests are 1. consistent 2. reliable.

export const mockSend = jest.fn()

export const secretsManagerClient = {
send: mockSend,
}

export const SecretsManagerClient: jest.Mock<
typeof secretsManagerClient
> = jest.fn().mockImplementation(() => secretsManagerClient)

export const GetSecretValueCommand = jest.fn((secret) => ({ SecretId: secret }))
3 changes: 3 additions & 0 deletions __mocks__/axios.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import mockAxios from "jest-mock-axios"

export default mockAxios
6 changes: 6 additions & 0 deletions __mocks__/otplib.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// eslint-disable-next-line import/prefer-default-export
export const totp = {
clone: jest.fn().mockReturnThis(),
generate: jest.fn(),
verify: jest.fn(),
}
20 changes: 20 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/** @type {import('ts-jest/dist/types').InitialOptionsTsJest} */
module.exports = {
preset: "ts-jest",
testEnvironment: "node",
moduleNameMapper: {
"^@root/(.*)": "<rootDir>/$1",
"^@classes/(.*)": "<rootDir>/classes/$1",
"^@errors/(.*)": "<rootDir>/errors/$1",
"^@logger/(.*)": "<rootDir>/logger/$1",
"^@middleware/(.*)": "<rootDir>/middleware/$1",
"^@routes/(.*)": "<rootDir>/routes/$1",
"^@utils/(.*)": "<rootDir>/utils/$1",
"^@loaders/(.*)": "<rootDir>/loaders/$1",
"^@database/(.*)": "<rootDir>/database/$1",
"^@services/(.*)": "<rootDir>/services/$1",
"^@validators/(.*)": "<rootDir>/validators/$1",
"^@fixtures/(.*)": "<rootDir>/fixtures/$1",
"^@mocks/(.*)": "<rootDir>/__mocks__/$1",
},
}
1,536 changes: 847 additions & 689 deletions package-lock.json

Large diffs are not rendered by default.

23 changes: 5 additions & 18 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
},
"devDependencies": {
"@tsconfig/recommended": "^1.0.1",
"@types/jest": "^27.4.1",
"@typescript-eslint/eslint-plugin": "^5.12.1",
"@typescript-eslint/parser": "^5.12.1",
"auto-changelog": "^2.4.0",
Expand All @@ -72,10 +73,12 @@
"eslint-plugin-only-warn": "^1.0.2",
"eslint-plugin-prettier": "^3.4.0",
"husky": "^6.0.0",
"jest": "^27.2.2",
"jest": "^27.5.1",
"jest-mock-axios": "^4.5.0",
"lint-staged": "^11.1.2",
"prettier": "2.2.1",
"typescript": "^4.5.5"
"ts-jest": "^27.1.3",
"typescript": "^4.6.2"
},
"config": {
"commitizen": {
Expand All @@ -96,22 +99,6 @@
"@validators": "validators",
"@fixtures": "fixtures"
},
"jest": {
"moduleNameMapper": {
"^@root/(.*)": "<rootDir>/$1",
"^@classes/(.*)": "<rootDir>/classes/$1",
"^@errors/(.*)": "<rootDir>/errors/$1",
"^@logger/(.*)": "<rootDir>/logger/$1",
"^@middleware/(.*)": "<rootDir>/middleware/$1",
"^@routes/(.*)": "<rootDir>/routes/$1",
"^@utils/(.*)": "<rootDir>/utils/$1",
"^@loaders/(.*)": "<rootDir>/loaders/$1",
"^@database/(.*)": "<rootDir>/database/$1",
"^@services/(.*)": "<rootDir>/services/$1",
"^@validators/(.*)": "<rootDir>/validators/$1",
"^@fixtures/(.*)": "<rootDir>/fixtures/$1"
}
},
"lint-staged": {
"**/*.(js|jsx|ts|tsx)": [
"eslint --fix",
Expand Down
13 changes: 10 additions & 3 deletions services/identity/MailClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@ const { POSTMAN_API_KEY } = process.env
const POSTMAN_API_URL = "https://api.postman.gov.sg/v1"

class MailClient {
async sendMail(recipient: string, body: string): Promise<void> {
if (!POSTMAN_API_KEY)
POSTMAN_API_KEY: string

constructor() {
if (!POSTMAN_API_KEY) {
throw new Error("Postman.gov.sg API key cannot be empty.")
}

this.POSTMAN_API_KEY = POSTMAN_API_KEY
}

async sendMail(recipient: string, body: string): Promise<void> {
const endpoint = `${POSTMAN_API_URL}/transactional/email/send`
const email = {
subject: "One-Time Password (OTP) for IsomerCMS",
Expand All @@ -31,4 +38,4 @@ class MailClient {
}
}

module.exports = MailClient
export default MailClient
9 changes: 6 additions & 3 deletions services/identity/SmsClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import logger from "@logger/logger"

import { AxiosClient } from "@root/types"

const { POSTMAN_API_KEY, POSTMAN_SMS_CRED_NAME } = process.env
const { POSTMAN_SMS_CRED_NAME } = process.env

const POSTMAN_API_URL = "https://api.postman.gov.sg/v1"

class SmsClient {
axiosClient: AxiosClient
private readonly axiosClient: AxiosClient

constructor() {
const { POSTMAN_API_KEY } = process.env

if (!POSTMAN_API_KEY)
throw new Error("Postman.gov.sg API key cannot be empty.")

Expand Down Expand Up @@ -39,4 +42,4 @@ class SmsClient {
}
}

module.exports = SmsClient
export default SmsClient
9 changes: 5 additions & 4 deletions services/identity/TokenStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@ import {
} from "@aws-sdk/client-secrets-manager"

class TokenStore {
secretsClient: SecretsManagerClient
private readonly secretsClient: SecretsManagerClient

constructor() {
this.secretsClient = this.createClient()
}

createClient() {
private createClient() {
const { AWS_REGION, AWS_ENDPOINT } = process.env
const config: SecretsManagerClientConfig = {
region: AWS_REGION || "ap-southeast-1",
}

// Use an alternate AWS endpoint if provided. For testing with localstack
if (AWS_ENDPOINT) config.endpoint = AWS_ENDPOINT

Expand All @@ -24,7 +25,7 @@ class TokenStore {

// NOTE: This is currently stricter than required.
// We can relax the constraint so that it can be undefined in the future.
async getToken(apiTokenName: string) {
async getToken(apiTokenName: string): Promise<string | undefined> {
const command = new GetSecretValueCommand({
SecretId: apiTokenName,
})
Expand All @@ -33,4 +34,4 @@ class TokenStore {
}
}

module.exports = TokenStore
export default TokenStore
8 changes: 4 additions & 4 deletions services/identity/TotpGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ interface TotpGeneratorProps {
}

class TotpGenerator {
generator: typeof totp
private readonly generator: typeof totp

expiry: number
private readonly expiry: number

secret: string
private readonly secret: string

constructor({ secret, expiry }: TotpGeneratorProps) {
this.secret = secret
Expand Down Expand Up @@ -47,4 +47,4 @@ class TotpGenerator {
}
}

module.exports = TotpGenerator
export default TotpGenerator
99 changes: 99 additions & 0 deletions services/identity/__tests__/AuthService.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import mockAxios from "jest-mock-axios"

import { BadRequestError } from "@root/errors/BadRequestError"

import _AuthService from "../AuthService"

import {
mockAccessToken,
mockHeaders,
mockSiteName,
mockUserId,
} from "./constants"

const AuthService = new _AuthService({ axiosClient: mockAxios })
const mockEndpoint = `/${mockSiteName}/collaborators/${mockUserId}`

describe("Auth Service", () => {
afterEach(() => mockAxios.reset())

it("should call axios successfully and return true when the call is successful", async () => {
// Arrange
const expected = true
mockAxios.get.mockResolvedValueOnce({
response: { status: 200 },
})

// Act
const actual = await AuthService.hasAccessToSite(
mockSiteName,
mockUserId,
mockAccessToken
)

// Assert
expect(actual).toBe(expected)
expect(mockAxios.get).toHaveBeenCalledWith(mockEndpoint, mockHeaders)
})

it("should call axios successfully and return false when the call fails with 403", async () => {
// Arrange
const expected = false
mockAxios.get.mockRejectedValueOnce({
response: { status: 403 },
// NOTE: Axios uses this property to determine if it's an axios error
isAxiosError: true,
})

// Act
const actual = await AuthService.hasAccessToSite(
mockSiteName,
mockUserId,
mockAccessToken
)

// Assert
expect(actual).toBe(expected)
expect(mockAxios.get).toHaveBeenCalledWith(mockEndpoint, mockHeaders)
})

it("should call axios successfully and return false when the call fails with 404", async () => {
// Arrange
const expected = false
mockAxios.get.mockRejectedValueOnce({
response: { status: 404 },
// NOTE: Axios uses this property to determine if it's an axios error
isAxiosError: true,
})

// Act
const actual = await AuthService.hasAccessToSite(
mockSiteName,
mockUserId,
mockAccessToken
)

// Assert
expect(actual).toBe(expected)
expect(mockAxios.get).toHaveBeenCalledWith(mockEndpoint, mockHeaders)
})

it("should call axios successfully and bubble the error when the status is not 403 or 404", async () => {
// Arrange
const expected = {
response: { status: 400 },
}
mockAxios.get.mockRejectedValueOnce(new BadRequestError(expected))

// Act
const actual = AuthService.hasAccessToSite(
mockSiteName,
mockUserId,
mockAccessToken
)

// Assert
expect(actual).rejects.toThrowError(new BadRequestError(expected))
expect(mockAxios.get).toHaveBeenCalledWith(mockEndpoint, mockHeaders)
})
})
88 changes: 88 additions & 0 deletions services/identity/__tests__/MailClient.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import mockAxios from "jest-mock-axios"

import _MailClient from "../MailClient"

import { mockRecipient, mockBody, mockHeaders } from "./constants"

const mockEndpoint = "https://api.postman.gov.sg/v1/transactional/email/send"

const MailClient = new _MailClient()

const generateEmail = (recipient: string, body: string) => ({
subject: "One-Time Password (OTP) for IsomerCMS",
from: "IsomerCMS <[email protected]>",
body,
recipient,
})

describe("Mail Client", () => {
const OLD_ENV = process.env

beforeEach(() => {
// Clears the cache so imports in tests uses a fresh copy
jest.resetModules()
// Make a copy of existing environment
process.env = { ...OLD_ENV }
})

afterAll(() => {
// Restore old environment
process.env = OLD_ENV
})

afterEach(() => mockAxios.reset())

it("should return the result successfully when all parameters are valid", async () => {
// Arrange
mockAxios.post.mockResolvedValueOnce(200)

// Act
const actual = await MailClient.sendMail(mockRecipient, mockBody)

// Assert
expect(actual).toBeUndefined()
expect(mockAxios.post).toHaveBeenCalledWith(
mockEndpoint,
generateEmail(mockRecipient, mockBody),
mockHeaders
)
})

it("should throw an error on initialization when the env var is not set", async () => {
// Arrange
// Store the API key and set it later so that other tests are not affected
const curApiKey = process.env.POSTMAN_API_KEY
process.env.POSTMAN_API_KEY = ""
// NOTE: This is because of typescript transpiling down to raw js
// Export default compiles down to module.exports.default, which is also
// done by babel.
// Read more here: https://www.typescriptlang.org/tsconfig#allowSyntheticDefaultImports
const _MailClientWithoutKey = (await import("../MailClient")).default
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ok for now - let's treat this as a work-in-progress.

I believe the end state we should aim for is to write MailClient in a dependency injectable way, so that we can invoke it more easily in our tests. Could we write this as a TODO comment in the MailClient file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if by this you mean that the POSTMAN_API_KEY is given as a parameter to the constructor, i can do it here!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok, we can leave it for now and add that to the various third-party client constructors in a separate PR/issue


// Act
// NOTE: We require a new instance because the old one would already have the API key bound
const actual = () => new _MailClientWithoutKey()

// Assert
expect(actual).toThrowError("Postman.gov.sg API key cannot be empty")
process.env.POSTMAN_API_KEY = curApiKey
expect(process.env.POSTMAN_API_KEY).toBe(curApiKey)
})

it("should return an error when a network error occurs", async () => {
// Arrange
const generatedEmail = generateEmail(mockRecipient, mockBody)
mockAxios.post.mockRejectedValueOnce("some error")

// Act
const actual = MailClient.sendMail(mockRecipient, mockBody)

// Assert
expect(actual).rejects.toThrowError("Failed to send email")
expect(mockAxios.post).toHaveBeenCalledWith(
mockEndpoint,
generatedEmail,
mockHeaders
)
})
})
Loading