From e553b4ccd564e25e6a38295e5d6feddfb082fd04 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Tue, 5 Apr 2022 15:18:52 +0800 Subject: [PATCH 1/4] refactor(mailclient): use di --- services/identity/MailClient.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/services/identity/MailClient.ts b/services/identity/MailClient.ts index 6b923246c..703733914 100644 --- a/services/identity/MailClient.ts +++ b/services/identity/MailClient.ts @@ -2,18 +2,13 @@ import axios from "axios" import logger from "@logger/logger" -const { POSTMAN_API_KEY } = process.env const POSTMAN_API_URL = "https://api.postman.gov.sg/v1" class MailClient { 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 + constructor(apiKey: string) { + this.POSTMAN_API_KEY = apiKey } async sendMail(recipient: string, body: string): Promise { @@ -29,7 +24,7 @@ class MailClient { try { await axios.post(endpoint, email, { headers: { - Authorization: `Bearer ${POSTMAN_API_KEY}`, + Authorization: `Bearer ${this.POSTMAN_API_KEY}`, }, }) } catch (err) { From 40da490a4cb14cef8e1b4f6ddd4372be4d6f3f42 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Tue, 5 Apr 2022 15:30:05 +0800 Subject: [PATCH 2/4] test(mailclient): update test for new api --- .../identity/__tests__/MailClient.spec.ts | 37 +------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/services/identity/__tests__/MailClient.spec.ts b/services/identity/__tests__/MailClient.spec.ts index e18c1ff08..b5357c619 100644 --- a/services/identity/__tests__/MailClient.spec.ts +++ b/services/identity/__tests__/MailClient.spec.ts @@ -10,7 +10,7 @@ import _MailClient from "../MailClient" const mockEndpoint = "https://api.postman.gov.sg/v1/transactional/email/send" -const MailClient = new _MailClient() +const MailClient = new _MailClient(process.env.POSTMAN_API_KEY!) const generateEmail = (recipient: string, body: string) => ({ subject: "One-Time Password (OTP) for IsomerCMS", @@ -21,20 +21,6 @@ const generateEmail = (recipient: string, body: string) => ({ }) 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 () => { @@ -53,27 +39,6 @@ describe("Mail Client", () => { ) }) - 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 - - // 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) From 54625cf4d33e461991edd0a7c0eaa49c46f75209 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Tue, 5 Apr 2022 15:30:22 +0800 Subject: [PATCH 3/4] refactor(identity): update mailclient init --- services/identity/index.ts | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/services/identity/index.ts b/services/identity/index.ts index 199cd9898..3271ec919 100644 --- a/services/identity/index.ts +++ b/services/identity/index.ts @@ -13,17 +13,24 @@ import TokenStore from "./TokenStore" import TotpGenerator from "./TotpGenerator" import UsersService from "./UsersService" -const IS_LOCAL_DEV = process.env.NODE_ENV === "LOCAL_DEV" -const { OTP_EXPIRY, OTP_SECRET } = process.env +const { + OTP_EXPIRY, + OTP_SECRET, + NODE_ENV, + LOCAL_SITE_ACCESS_TOKEN, + POSTMAN_API_KEY, +} = process.env + +const IS_LOCAL_DEV = NODE_ENV === "LOCAL_DEV" const tokenStore = IS_LOCAL_DEV ? (({ - getToken: (_apiTokenName: string) => process.env.LOCAL_SITE_ACCESS_TOKEN, + getToken: (_apiTokenName: string) => LOCAL_SITE_ACCESS_TOKEN, } as unknown) as TokenStore) : new TokenStore() if (!OTP_SECRET) { - logger.error( + throw new Error( "Please ensure that you have set OTP_SECRET in your env vars and that you have sourced them!" ) } @@ -33,10 +40,16 @@ const totpGenerator = new TotpGenerator({ expiry: parseInt(OTP_EXPIRY!, 10) ?? undefined, }) +if (!POSTMAN_API_KEY) { + throw new Error( + "Please ensure that you have set POSTMAN_API_KEY in your env vars and that you have sourced them!" + ) +} + const mockMailer = { sendMail: (_email: string, html: string) => logger.info(html), } as MailClient -const mailer = IS_LOCAL_DEV ? mockMailer : new MailClient() +const mailer = IS_LOCAL_DEV ? mockMailer : new MailClient(POSTMAN_API_KEY) const smsClient = IS_LOCAL_DEV ? ({ From d13ed96163f3a524aacfb347cf44d73078a6d945 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 6 Apr 2022 10:15:02 +0800 Subject: [PATCH 4/4] chore(identity): update error message for mailclient to show on non local dev --- services/identity/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/identity/index.ts b/services/identity/index.ts index 3271ec919..32c04126c 100644 --- a/services/identity/index.ts +++ b/services/identity/index.ts @@ -40,7 +40,7 @@ const totpGenerator = new TotpGenerator({ expiry: parseInt(OTP_EXPIRY!, 10) ?? undefined, }) -if (!POSTMAN_API_KEY) { +if (!POSTMAN_API_KEY && !IS_LOCAL_DEV) { throw new Error( "Please ensure that you have set POSTMAN_API_KEY in your env vars and that you have sourced them!" ) @@ -49,7 +49,7 @@ if (!POSTMAN_API_KEY) { const mockMailer = { sendMail: (_email: string, html: string) => logger.info(html), } as MailClient -const mailer = IS_LOCAL_DEV ? mockMailer : new MailClient(POSTMAN_API_KEY) +const mailer = IS_LOCAL_DEV ? mockMailer : new MailClient(POSTMAN_API_KEY!) const smsClient = IS_LOCAL_DEV ? ({