From 4d475bfa3b9873bfc6ceb50bd53df646091b2068 Mon Sep 17 00:00:00 2001 From: Darcy Ye Date: Sat, 6 May 2023 18:47:48 +0800 Subject: [PATCH] chore(test): update connector integration tests (#3765) --- .../src/__mocks__/connectors-mock.ts | 8 +- .../integration-tests/src/api/connector.ts | 33 +++--- .../src/helpers/connector.ts | 2 +- .../src/tests/api/admin-user.test.ts | 8 +- .../src/tests/api/connector.test.ts | 103 ++++++++++-------- 5 files changed, 91 insertions(+), 63 deletions(-) diff --git a/packages/integration-tests/src/__mocks__/connectors-mock.ts b/packages/integration-tests/src/__mocks__/connectors-mock.ts index 91bd29498..0bc30c0d4 100644 --- a/packages/integration-tests/src/__mocks__/connectors-mock.ts +++ b/packages/integration-tests/src/__mocks__/connectors-mock.ts @@ -160,8 +160,8 @@ export const mockEmailConnectorConfig = { ], }; -export const mockStandardEmailConnectorId = 'mock-email-service-alternative'; -export const mockStandardEmailConnectorConfig = { +export const mockAlternativeEmailConnectorId = 'mock-email-service-alternative'; +export const mockAlternativeEmailConnectorConfig = { apiKey: 'api-key-value', fromEmail: 'noreply@logto.test.io', fromName: 'from-name-value', @@ -205,3 +205,7 @@ export const mockSocialConnectorConfig = { clientId: 'client_id_value', clientSecret: 'client_secret_value', }; +export const mockSocialConnectorNewConfig = { + clientId: 'client_id_value_new', + clientSecret: 'client_secret_value_new', +}; diff --git a/packages/integration-tests/src/api/connector.ts b/packages/integration-tests/src/api/connector.ts index b0362af1d..84ae9714f 100644 --- a/packages/integration-tests/src/api/connector.ts +++ b/packages/integration-tests/src/api/connector.ts @@ -7,19 +7,26 @@ import type { import { authedAdminApi } from './api.js'; +/** + * We are using `id` and `connectorFactoryId` here: + * + * - `id` is used to identify connectors from the database. + * - `connectorFactoryId` is used to identify connectors - more specifically, connector factories - in packages/connectors + * that contain metadata (considered connectors' FIXED properties) and code implementation (which determines how connectors work). + */ + export const listConnectors = async () => authedAdminApi.get('connectors').json(); -export const getConnector = async (connectorId: string) => - authedAdminApi.get(`connectors/${connectorId}`).json(); +export const getConnector = async (id: string) => + authedAdminApi.get(`connectors/${id}`).json(); export const listConnectorFactories = async () => authedAdminApi.get('connector-factories').json(); -export const getConnectorFactory = async (connectorId: string) => - authedAdminApi.get(`connector-factories/${connectorId}`).json(); +export const getConnectorFactory = async (connectorFactoryId: string) => + authedAdminApi.get(`connector-factories/${connectorFactoryId}`).json(); -// FIXME @Darcy: correct use of `id` and `connectorId`. export const postConnector = async ( payload: Pick ) => @@ -34,36 +41,36 @@ export const deleteConnectorById = async (id: string) => authedAdminApi.delete({ url: `connectors/${id}` }).json(); export const updateConnectorConfig = async ( - connectorId: string, + id: string, config: Record, metadata?: Record ) => authedAdminApi .patch({ - url: `connectors/${connectorId}`, + url: `connectors/${id}`, json: { config, metadata }, }) .json(); export const sendSmsTestMessage = async ( - connectorId: string, + connectorFactoryId: string, phone: string, config: Record -) => sendTestMessage(connectorId, 'phone', phone, config); +) => sendTestMessage(connectorFactoryId, 'phone', phone, config); export const sendEmailTestMessage = async ( - connectorId: string, + connectorFactoryId: string, email: string, config: Record -) => sendTestMessage(connectorId, 'email', email, config); +) => sendTestMessage(connectorFactoryId, 'email', email, config); const sendTestMessage = async ( - connectorId: string, + connectorFactoryId: string, receiverType: 'phone' | 'email', receiver: string, config: Record ) => authedAdminApi.post({ - url: `connectors/${connectorId}/test`, + url: `connectors/${connectorFactoryId}/test`, json: { [receiverType]: receiver, config }, }); diff --git a/packages/integration-tests/src/helpers/connector.ts b/packages/integration-tests/src/helpers/connector.ts index c81ef94de..d9fac05df 100644 --- a/packages/integration-tests/src/helpers/connector.ts +++ b/packages/integration-tests/src/helpers/connector.ts @@ -16,7 +16,7 @@ export const clearConnectorsByTypes = async (types: ConnectorType[]) => { await Promise.all(targetConnectors.map(async (connector) => deleteConnectorById(connector.id))); }; -export const clearConnectorById = async (connectorId: string) => deleteConnectorById(connectorId); +export const clearConnectorById = async (id: string) => deleteConnectorById(id); export const setEmailConnector = async () => postConnector({ diff --git a/packages/integration-tests/src/tests/api/admin-user.test.ts b/packages/integration-tests/src/tests/api/admin-user.test.ts index 72360fdcb..b27fa719e 100644 --- a/packages/integration-tests/src/tests/api/admin-user.test.ts +++ b/packages/integration-tests/src/tests/api/admin-user.test.ts @@ -12,7 +12,6 @@ import { updateUserPassword, deleteUserIdentity, postConnector, - updateConnectorConfig, deleteConnectorById, } from '#src/api/index.js'; import { createResponseWithCode } from '#src/helpers/admin-tenant.js'; @@ -95,9 +94,10 @@ describe('admin console user management', () => { }); it('should delete user identities successfully', async () => { - // @darcy FIXME: merge post and update - const { id } = await postConnector({ connectorId: mockSocialConnectorId }); - await updateConnectorConfig(id, mockSocialConnectorConfig); + const { id } = await postConnector({ + connectorId: mockSocialConnectorId, + config: mockSocialConnectorConfig, + }); const createdUserId = await createNewSocialUserWithUsernameAndPassword(id); diff --git a/packages/integration-tests/src/tests/api/connector.test.ts b/packages/integration-tests/src/tests/api/connector.test.ts index a37b7d4cd..b790183ac 100644 --- a/packages/integration-tests/src/tests/api/connector.test.ts +++ b/packages/integration-tests/src/tests/api/connector.test.ts @@ -6,9 +6,10 @@ import { mockSmsConnectorConfig, mockSmsConnectorId, mockSocialConnectorConfig, + mockSocialConnectorNewConfig, mockSocialConnectorId, - mockStandardEmailConnectorConfig, - mockStandardEmailConnectorId, + mockAlternativeEmailConnectorConfig, + mockAlternativeEmailConnectorId, } from '#src/__mocks__/connectors-mock.js'; import { deleteConnectorById, @@ -70,11 +71,8 @@ test('connector set-up flow', async () => { */ await Promise.all( mockConnectorSetups.map(async ({ connectorId, config }) => { - // @darcy FIXME: should call post method directly - const { id } = await postConnector({ connectorId }); + const { id } = await postConnector({ connectorId, config }); connectorIdMap.set(connectorId, id); - const updatedConnector = await updateConnectorConfig(id, config); - expect(updatedConnector.config).toEqual(config); // The result of getting a connector should be same as the result of updating a connector above. const connector = await getConnector(id); @@ -93,53 +91,64 @@ test('connector set-up flow', async () => { // we check: the mock connector config should stay the same. const mockSocialConnector = await getConnector(connectorIdMap.get(mockSocialConnectorId)!); expect(mockSocialConnector.config).toEqual(mockSocialConnectorConfig); + const { config: updatedConfig } = await updateConnectorConfig( + connectorIdMap.get(mockSocialConnectorId)!, + mockSocialConnectorNewConfig + ); + expect(updatedConfig).toEqual(mockSocialConnectorNewConfig); /* * Change to another SMS/Email connector */ const { id } = await postConnector({ - connectorId: mockStandardEmailConnectorId, + connectorId: mockAlternativeEmailConnectorId, + config: mockAlternativeEmailConnectorConfig, }); - await updateConnectorConfig(id, mockStandardEmailConnectorConfig); - connectorIdMap.set(mockStandardEmailConnectorId, id); + connectorIdMap.set(mockAlternativeEmailConnectorId, id); const currentConnectors = await listConnectors(); + // Only one SMS/email connector should be added at a time, hence previous SMS/email connector will be deleted automatically. expect( currentConnectors.some((connector) => connector.connectorId === mockEmailConnectorId) ).toBeFalsy(); connectorIdMap.delete(mockEmailConnectorId); expect( - currentConnectors.some((connector) => connector.connectorId === mockStandardEmailConnectorId) - ).toBeTruthy(); - expect( - currentConnectors.find((connector) => connector.connectorId === mockStandardEmailConnectorId) + currentConnectors.find((connector) => connector.connectorId === mockAlternativeEmailConnectorId) ?.config - ).toEqual(mockStandardEmailConnectorConfig); + ).toEqual(mockAlternativeEmailConnectorConfig); /* * Delete (i.e. disable) a connector */ await expect( - deleteConnectorById(connectorIdMap.get(mockStandardEmailConnectorId)!) + deleteConnectorById(connectorIdMap.get(mockAlternativeEmailConnectorId)!) ).resolves.not.toThrow(); - connectorIdMap.delete(mockStandardEmailConnectorId); + await expect(getConnector(connectorIdMap.get(mockAlternativeEmailConnectorId)!)).rejects.toThrow( + HTTPError + ); + connectorIdMap.delete(mockAlternativeEmailConnectorId); /** * List connectors after manually setting up connectors. * The result of listing connectors should be same as the result of updating connectors above. */ - expect(await listConnectors()).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - id: connectorIdMap.get(mockSmsConnectorId), - connectorId: mockSmsConnectorId, - config: mockSmsConnectorConfig, - }), - expect.objectContaining({ - id: connectorIdMap.get(mockSocialConnectorId), - connectorId: mockSocialConnectorId, - config: mockSocialConnectorConfig, - }), - ]) + const allConnectorsAfterSetup = await listConnectors(); + expect( + allConnectorsAfterSetup.find(({ id }) => id === connectorIdMap.get(mockSmsConnectorId)) + ).toEqual( + expect.objectContaining({ + id: connectorIdMap.get(mockSmsConnectorId), + connectorId: mockSmsConnectorId, + config: mockSmsConnectorConfig, + }) + ); + expect( + allConnectorsAfterSetup.find(({ id }) => id === connectorIdMap.get(mockSocialConnectorId)) + ).toEqual( + expect.objectContaining({ + id: connectorIdMap.get(mockSocialConnectorId), + connectorId: mockSocialConnectorId, + config: mockSocialConnectorNewConfig, + }) ); }); @@ -174,14 +183,20 @@ test('override metadata for non-standard social connector', async () => { }); test('send SMS/email test message', async () => { - const connectors = await listConnectors(); - await Promise.all( - connectors.map(async ({ id }) => { - await deleteConnectorById(id); - }) - ); - connectorIdMap.clear(); + await cleanUpConnectorTable(); + const generatePhone = '8612345678901'; + const generateEmail = 'test@example.com'; + + // Can send test message without enabling passwordless connectors (test whether `config` works). + await expect( + sendSmsTestMessage(mockSmsConnectorId, generatePhone, mockSmsConnectorConfig) + ).resolves.not.toThrow(); + await expect( + sendEmailTestMessage(mockEmailConnectorId, generateEmail, mockEmailConnectorConfig) + ).resolves.not.toThrow(); + + // Set up passwordless connectors. await Promise.all( [{ connectorId: mockSmsConnectorId }, { connectorId: mockEmailConnectorId }].map( async ({ connectorId }) => { @@ -191,17 +206,19 @@ test('send SMS/email test message', async () => { ) ); - const phone = '8612345678901'; - const email = 'test@example.com'; - await expect( - sendSmsTestMessage(mockSmsConnectorId, phone, mockSmsConnectorConfig) + sendSmsTestMessage(mockSmsConnectorId, generatePhone, mockSmsConnectorConfig) ).resolves.not.toThrow(); await expect( - sendEmailTestMessage(mockEmailConnectorId, email, mockEmailConnectorConfig) + sendEmailTestMessage(mockEmailConnectorId, generateEmail, mockEmailConnectorConfig) ).resolves.not.toThrow(); - await expect(sendSmsTestMessage(mockSmsConnectorId, phone, {})).rejects.toThrow(HTTPError); - await expect(sendEmailTestMessage(mockEmailConnectorId, email, {})).rejects.toThrow(HTTPError); + // Throws due to invalid `config`s. + await expect(sendSmsTestMessage(mockSmsConnectorId, generatePhone, {})).rejects.toThrow( + HTTPError + ); + await expect(sendEmailTestMessage(mockEmailConnectorId, generateEmail, {})).rejects.toThrow( + HTTPError + ); for (const [_connectorId, id] of connectorIdMap.entries()) { // eslint-disable-next-line no-await-in-loop