diff --git a/src/shared/member-api/member-api.service.spec.ts b/src/shared/member-api/member-api.service.spec.ts index c14ff10..0a67b3b 100644 --- a/src/shared/member-api/member-api.service.spec.ts +++ b/src/shared/member-api/member-api.service.spec.ts @@ -1,12 +1,9 @@ import { Test, TestingModule } from '@nestjs/testing'; import { HttpService } from '@nestjs/axios'; import { ConfigService } from '@nestjs/config'; -import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { Cache } from 'cache-manager'; -import { HttpException, HttpStatus, Logger } from '@nestjs/common'; -import { of, throwError } from 'rxjs'; +import { HttpException, HttpStatus } from '@nestjs/common'; import { AxiosResponse } from 'axios'; - +import { of, throwError } from 'rxjs'; import { MemberApiService } from './member-api.service'; import { M2M_AUTH_CLIENT } from './member-api.constants'; import { MemberInfoDto } from '../../dto/member/member.dto'; @@ -15,59 +12,33 @@ describe('MemberApiService', () => { let service: MemberApiService; let httpService: HttpService; let configService: ConfigService; - let cacheManager: Cache; - let m2mAuthClient: any; + let m2mAuthClient: { getMachineToken: jest.Mock }; let module: TestingModule; - // Test data - const mockMemberInfo: MemberInfoDto[] = [ - { userId: 1, handle: 'john_doe', email: 'john@example.com' }, - { userId: 2, handle: 'jane_smith', email: 'jane@example.com' }, - ]; - - const mockToken = 'mock-jwt-token'; - const mockApiUrl = 'https://api.example.com/members'; - const mockClientId = 'test-client-id'; - const mockClientSecret = 'test-client-secret'; - - const mockConfigService = { - get: jest - .fn() - .mockImplementation((key: string, defaultValue?: T): T => { - const config = { - MEMBER_API_URL: mockApiUrl, - AUTH0_CLIENT_ID: mockClientId, - AUTH0_CLIENT_SECRET: mockClientSecret, - TOKEN_CACHE_TIME: 23 * 60 * 60, - }; + const mockApiUrl = 'https://api.example.com/v6/members'; + const mockClientId = 'client-id'; + const mockClientSecret = 'client-secret'; + const mockToken = 'machine-token'; + + const createMockConfigService = () => ({ + get: jest.fn((key: string, defaultValue?: unknown) => { + const values: Record = { + MEMBER_API_URL: mockApiUrl, + AUTH0_CLIENT_ID: mockClientId, + AUTH0_CLIENT_SECRET: mockClientSecret, + }; - return (config[key as keyof typeof config] || defaultValue) as T; - }), - }; + return key in values ? values[key] : defaultValue; + }), + }); beforeEach(async () => { const mockHttpService = { get: jest.fn(), }; - - const mockCacheManager = { - get: jest.fn(), - set: jest.fn(), - }; - - const mockM2MAuthClient = { - getMachineToken: jest.fn(), - }; - - // Create a proper Logger mock - const mockLogger = { - log: jest.fn(), - error: jest.fn(), - warn: jest.fn(), - debug: jest.fn(), - verbose: jest.fn(), - setContext: jest.fn(), - overrideLogger: jest.fn(), + const mockConfigService = createMockConfigService(); + const mockM2mAuthClient = { + getMachineToken: jest.fn().mockResolvedValue(mockToken), }; module = await Test.createTestingModule({ @@ -81,412 +52,141 @@ describe('MemberApiService', () => { provide: ConfigService, useValue: mockConfigService, }, - { - provide: CACHE_MANAGER, - useValue: mockCacheManager, - }, { provide: M2M_AUTH_CLIENT, - useValue: mockM2MAuthClient, - }, - { - provide: Logger, - useValue: mockLogger, + useValue: mockM2mAuthClient, }, ], - }) - .setLogger(mockLogger) // Use mock logger during tests - .compile(); + }).compile(); service = module.get(MemberApiService); httpService = module.get(HttpService); configService = module.get(ConfigService); - cacheManager = module.get(CACHE_MANAGER); m2mAuthClient = module.get(M2M_AUTH_CLIENT); }); afterEach(async () => { jest.clearAllMocks(); - if (module) { - await module.close(); - } - }); - - describe('Constructor', () => { - it('should initialize successfully with valid configuration', () => { - expect(service).toBeDefined(); - expect(mockConfigService.get).toHaveBeenCalledWith('MEMBER_API_URL'); - }); - }); - - describe('getM2mToken (private method)', () => { - it('should return cached token when available', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(mockToken); - - const result = await (service as any).getM2mToken(); - - expect(result).toBe(mockToken); - expect(cacheManager.get).toHaveBeenCalledWith('member_api_m2m_token'); - expect(m2mAuthClient.getMachineToken).not.toHaveBeenCalled(); - }); - - it('should fetch new token when cache is empty', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest.spyOn(cacheManager, 'set').mockResolvedValue(undefined); - jest.spyOn(m2mAuthClient, 'getMachineToken').mockResolvedValue(mockToken); - - const result = await (service as any).getM2mToken(); - - expect(result).toBe(mockToken); - expect(m2mAuthClient.getMachineToken).toHaveBeenCalledWith( - mockClientId, - mockClientSecret, - ); - expect(cacheManager.set).toHaveBeenCalledWith( - 'member_api_m2m_token', - mockToken, - expect.any(Number), - ); - }); - - it('should return null when clientId is missing', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest.spyOn(configService, 'get').mockImplementation((key: string) => { - if (key === 'AUTH0_CLIENT_ID') return undefined; - if (key === 'MEMBER_API_URL') return mockApiUrl; - return undefined; - }); - - const result = await (service as any).getM2mToken(); - - expect(result).toBeNull(); - expect(m2mAuthClient.getMachineToken).not.toHaveBeenCalled(); - }); - - it('should return null when clientSecret is missing', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest.spyOn(configService, 'get').mockImplementation((key: string) => { - if (key === 'AUTH0_CLIENT_SECRET') return undefined; - if (key === 'AUTH0_CLIENT_ID') return mockClientId; - if (key === 'MEMBER_API_URL') return mockApiUrl; - return undefined; - }); - - const result = await (service as any).getM2mToken(); - - expect(result).toBeNull(); - expect(m2mAuthClient.getMachineToken).not.toHaveBeenCalled(); - }); - - it('should return null when m2mAuthClient returns null token', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest.spyOn(m2mAuthClient, 'getMachineToken').mockResolvedValue(null); - - const result = await (service as any).getM2mToken(); - - expect(result).toBeNull(); - expect(cacheManager.set).not.toHaveBeenCalled(); - }); - - it('should return null when m2mAuthClient throws error', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest - .spyOn(m2mAuthClient, 'getMachineToken') - .mockRejectedValue(new Error('Auth service error')); - - const result = await (service as any).getM2mToken(); - - expect(result).toBeNull(); - expect(cacheManager.set).not.toHaveBeenCalled(); - }); - - it('should cache token with correct TTL', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest.spyOn(cacheManager, 'set').mockResolvedValue(undefined); - jest.spyOn(m2mAuthClient, 'getMachineToken').mockResolvedValue(mockToken); - jest - .spyOn(configService, 'get') - .mockImplementation((key: string, defaultValue?: any) => { - if (key === 'TOKEN_CACHE_TIME') return 3600; // 1 hour - if (key === 'MEMBER_API_URL') return mockApiUrl; - if (key === 'AUTH0_CLIENT_ID') return mockClientId; - if (key === 'AUTH0_CLIENT_SECRET') return mockClientSecret; - return defaultValue; - }); - - await (service as any).getM2mToken(); - - const expectedTtl = (3600 - 60) * 1000; // Cache 60 seconds less, in milliseconds - expect(cacheManager.set).toHaveBeenCalledWith( - 'member_api_m2m_token', - mockToken, - expectedTtl, - ); - }); - - it('should use minimum TTL of 60 seconds when calculated TTL is too low', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest.spyOn(cacheManager, 'set').mockResolvedValue(undefined); - jest.spyOn(m2mAuthClient, 'getMachineToken').mockResolvedValue(mockToken); - jest - .spyOn(configService, 'get') - .mockImplementation((key: string, defaultValue?: any) => { - if (key === 'TOKEN_CACHE_TIME') return 30; // 30 seconds - if (key === 'MEMBER_API_URL') return mockApiUrl; - if (key === 'AUTH0_CLIENT_ID') return mockClientId; - if (key === 'AUTH0_CLIENT_SECRET') return mockClientSecret; - return defaultValue; - }); - - await (service as any).getM2mToken(); - - const expectedTtl = 60 * 1000; // Minimum 60 seconds in milliseconds - expect(cacheManager.set).toHaveBeenCalledWith( - 'member_api_m2m_token', - mockToken, - expectedTtl, - ); - }); + await module.close(); }); describe('getUserInfoList', () => { - beforeEach(() => { - // Setup successful token retrieval by default - jest.spyOn(cacheManager, 'get').mockResolvedValue(mockToken); - }); - - it('should return empty array for empty input', async () => { - const getSpy = jest.spyOn(httpService, 'get'); - const result = await service.getUserInfoList([]); - expect(result).toEqual([]); - expect(getSpy).not.toHaveBeenCalled(); - }); - - it('should return empty array for null input', async () => { - const getSpy = jest.spyOn(httpService, 'get'); - const result = await service.getUserInfoList(null as any); - expect(result).toEqual([]); - expect(getSpy).not.toHaveBeenCalled(); + it('returns an empty array for empty input', async () => { + await expect(service.getUserInfoList([])).resolves.toEqual([]); + expect(httpService.get).not.toHaveBeenCalled(); }); - it('should throw HttpException when M2M token cannot be obtained', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest.spyOn(m2mAuthClient, 'getMachineToken').mockResolvedValue(null); + it('throws when the service cannot obtain an M2M token', async () => { + m2mAuthClient.getMachineToken.mockResolvedValueOnce(null); - await expect(service.getUserInfoList([1, 2])).rejects.toThrow( + await expect(service.getUserInfoList([1])).rejects.toThrow( new HttpException( 'Internal configuration error: Could not authenticate service.', HttpStatus.INTERNAL_SERVER_ERROR, ), ); + expect(httpService.get).not.toHaveBeenCalled(); }); - it('should successfully fetch user info for small list', async () => { - const userIds = [1, 2]; + it('requests only the identity fields needed for batched lookups', async () => { const mockResponse: AxiosResponse = { - data: mockMemberInfo, + data: [ + { userId: 1, handle: 'alpha', email: 'alpha@example.com' }, + { userId: 2, handle: 'beta', email: 'beta@example.com' }, + ], status: HttpStatus.OK, statusText: 'OK', headers: {}, - config: {} as any, + config: {} as AxiosResponse['config'], }; - const getSpy = jest - .spyOn(httpService, 'get') - .mockReturnValue(of(mockResponse)); + jest.spyOn(httpService, 'get').mockReturnValue(of(mockResponse)); - const result = await service.getUserInfoList(userIds); + const result = await service.getUserInfoList([1, 2]); - expect(result).toEqual(mockMemberInfo); - expect(getSpy).toHaveBeenCalledWith(`${mockApiUrl}?userIds=1&userIds=2`, { - headers: { - Authorization: `Bearer ${mockToken}`, - 'Content-Type': 'application/json', + expect(result).toEqual(mockResponse.data); + expect(httpService.get).toHaveBeenCalledWith( + `${mockApiUrl}?userIds=1&userIds=2&fields=userId%2Chandle%2Cemail&includeStats=false`, + { + headers: { + Authorization: `Bearer ${mockToken}`, + 'Content-Type': 'application/json', + }, + timeout: 10000, }, - }); + ); }); - it('should handle large lists by batching requests', async () => { - const userIds = Array.from({ length: 120 }, (_, i) => i + 1); // 120 users - const batchResponse1 = Array.from({ length: 50 }, (_, i) => ({ - userId: i + 1, - handle: `user${i + 1}`, - email: `user${i + 1}@example.com`, - })); - const batchResponse2 = Array.from({ length: 50 }, (_, i) => ({ - userId: i + 51, - handle: `user${i + 51}`, - email: `user${i + 51}@example.com`, - })); - const batchResponse3 = Array.from({ length: 20 }, (_, i) => ({ - userId: i + 101, - handle: `user${i + 101}`, - email: `user${i + 101}@example.com`, - })); - - const mockResponse1: AxiosResponse = { - data: batchResponse1, - status: HttpStatus.OK, - statusText: 'OK', - headers: {}, - config: {} as any, - }; - - const mockResponse2: AxiosResponse = { - data: batchResponse2, - status: HttpStatus.OK, - statusText: 'OK', - headers: {}, - config: {} as any, - }; + it('uses the singular userId parameter and configured timeout for single-user lookups', async () => { + jest + .spyOn(configService, 'get') + .mockImplementation((key: string, defaultValue?: unknown) => { + const values: Record = { + MEMBER_API_URL: mockApiUrl, + AUTH0_CLIENT_ID: mockClientId, + AUTH0_CLIENT_SECRET: mockClientSecret, + HTTP_TIMEOUT: '15000', + }; + + return key in values ? values[key] : defaultValue; + }); - const mockResponse3: AxiosResponse = { - data: batchResponse3, + const mockResponse: AxiosResponse = { + data: [{ userId: 7, handle: 'solo', email: 'solo@example.com' }], status: HttpStatus.OK, statusText: 'OK', headers: {}, - config: {} as any, + config: {} as AxiosResponse['config'], }; - const getSpy = jest - .spyOn(httpService, 'get') - .mockReturnValueOnce(of(mockResponse1)) - .mockReturnValueOnce(of(mockResponse2)) - .mockReturnValueOnce(of(mockResponse3)); + jest.spyOn(httpService, 'get').mockReturnValue(of(mockResponse)); - const result = await service.getUserInfoList(userIds); + await service.getUserInfoList([7]); - expect(getSpy).toHaveBeenCalledTimes(3); // 3 batches (50, 50, 20) - expect(result).toHaveLength(120); // All responses combined + expect(httpService.get).toHaveBeenCalledWith( + `${mockApiUrl}?userId=7&fields=userId%2Chandle%2Cemail&includeStats=false`, + expect.objectContaining({ + timeout: 15000, + }), + ); }); - it('should deduplicate user IDs', async () => { - const userIds = [1, 2, 1, 3, 2]; // Duplicates + it('deduplicates user IDs before batching', async () => { const mockResponse: AxiosResponse = { - data: mockMemberInfo, + data: [{ userId: 1, handle: 'alpha', email: 'alpha@example.com' }], status: HttpStatus.OK, statusText: 'OK', headers: {}, - config: {} as any, + config: {} as AxiosResponse['config'], }; - const getSpy = jest - .spyOn(httpService, 'get') - .mockReturnValue(of(mockResponse)); + jest.spyOn(httpService, 'get').mockReturnValue(of(mockResponse)); - await service.getUserInfoList(userIds); + await service.getUserInfoList([1, 2, 1, 3, 2]); - // Should only call with unique IDs: 1, 2, 3 - expect(getSpy).toHaveBeenCalledWith( - `${mockApiUrl}?userIds=1&userIds=2&userIds=3`, + expect(httpService.get).toHaveBeenCalledWith( + `${mockApiUrl}?userIds=1&userIds=2&userIds=3&fields=userId%2Chandle%2Cemail&includeStats=false`, expect.any(Object), ); }); - it('should throw HttpException when API call fails', async () => { - const userIds = [1, 2]; - const error = { - message: 'Network error', - response: { - status: 500, - data: { message: 'Internal server error' }, - }, - }; - - jest.spyOn(httpService, 'get').mockReturnValue(throwError(() => error)); - - await expect(service.getUserInfoList(userIds)).rejects.toThrow( - new HttpException( - 'Failed during Member API batch request 1/1: Internal server error', - 500, - ), + it('wraps upstream HTTP failures in an HttpException', async () => { + jest.spyOn(httpService, 'get').mockReturnValue( + throwError(() => ({ + message: 'timeout of 5000ms exceeded', + response: { + status: 504, + data: { message: 'Gateway Timeout' }, + }, + })), ); - }); - - it('should handle API error without response data', async () => { - const userIds = [1, 2]; - const error = { - message: 'Network timeout', - response: { - status: 408, - }, - }; - jest.spyOn(httpService, 'get').mockReturnValue(throwError(() => error)); - - await expect(service.getUserInfoList(userIds)).rejects.toThrow( + await expect(service.getUserInfoList([1, 2])).rejects.toThrow( new HttpException( - 'Failed during Member API batch request 1/1: Error fetching data batch from Member API (Status: 408)', - 408, + 'Failed during Member API batch request 1/1: Gateway Timeout', + 504, ), ); }); - - // it('should handle API error without response object', async () => { - // const userIds = [1, 2]; - // const error = new Error('Connection refused'); - - // jest.spyOn(httpService, 'get').mockReturnValue(throwError(() => error)); - - // await expect(service.getUserInfoList(userIds)).rejects.toThrow( - // new HttpException( - // expect.stringContaining('Failed during Member API batch request 1/1:'), - // HttpStatus.INTERNAL_SERVER_ERROR, - // ), - // ); - // }); - - it('should stop processing batches on first failure', async () => { - const userIds = Array.from({ length: 120 }, (_, i) => i + 1); // 120 users, 3 batches - const error = new Error('First batch failed'); - - const getSpy = jest - .spyOn(httpService, 'get') - .mockReturnValue(throwError(() => error)); - - await expect(service.getUserInfoList(userIds)).rejects.toThrow( - HttpException, - ); - - // Should only call once (first batch), then stop - expect(getSpy).toHaveBeenCalledTimes(1); - }); - - it('should properly encode user IDs in query string', async () => { - const userIds = [123, 456]; - const mockResponse: AxiosResponse = { - data: mockMemberInfo, - status: HttpStatus.OK, - statusText: 'OK', - headers: {}, - config: {} as any, - }; - - const getSpy = jest - .spyOn(httpService, 'get') - .mockReturnValue(of(mockResponse)); - - await service.getUserInfoList(userIds); - - expect(getSpy).toHaveBeenCalledWith( - `${mockApiUrl}?userIds=123&userIds=456`, - expect.any(Object), - ); - }); - }); - - describe('Error Handling Edge Cases', () => { - it('should handle cache set failure gracefully', async () => { - jest.spyOn(cacheManager, 'get').mockResolvedValue(null); - jest - .spyOn(cacheManager, 'set') - .mockRejectedValue(new Error('Cache write failed')); - jest.spyOn(m2mAuthClient, 'getMachineToken').mockResolvedValue(mockToken); - - const result = await (service as any).getM2mToken(); - - // Should still return token even if caching fails - expect(result).toBeNull(); - }); }); }); diff --git a/src/shared/member-api/member-api.service.ts b/src/shared/member-api/member-api.service.ts index 5e00e60..0583312 100644 --- a/src/shared/member-api/member-api.service.ts +++ b/src/shared/member-api/member-api.service.ts @@ -11,6 +11,9 @@ import { firstValueFrom } from 'rxjs'; import { MemberInfoDto } from '../../dto/member/member.dto'; import { M2M_AUTH_CLIENT } from './member-api.constants'; // Import M2M client token from constants +const MEMBER_LOOKUP_FIELDS = ['userId', 'handle', 'email'] as const; +const DEFAULT_MEMBER_API_TIMEOUT_MS = 10000; + // Define the interface for the M2M Auth client provided by the module // Based on the declaration file src/types/tc-core-library-js.d.ts interface M2MAuthClient { @@ -39,7 +42,7 @@ export class MemberApiService { } /** - * Gets a valid M2M token for Member API, using the injected M2M client and cache. + * Gets a valid M2M token for the Member API using the injected M2M client. */ private async getM2mToken(): Promise { const clientId = this.configService.get('AUTH0_CLIENT_ID'); @@ -78,6 +81,9 @@ export class MemberApiService { /** * Fetches member information for a list of user IDs from the Member API. * Handles large lists by batching requests and logs total execution time. + * The downstream lookup is intentionally restricted to `userId`, `handle`, + * and `email`, and disables stats expansion because that is all identity uses + * for role-subject responses. * * Batching is necessary because the Member API's GET /members endpoint is queried * using URL parameters (e.g., ?userIds=1&userIds=2...). Sending a large number @@ -109,6 +115,13 @@ export class MemberApiService { const batchSize = 50; // Reduced batch size to avoid 413/414 errors const allMemberInfo: MemberInfoDto[] = []; const totalBatches = Math.ceil(uniqueUserIds.length / batchSize); + const configuredTimeout = Number( + this.configService.get('HTTP_TIMEOUT') ?? + DEFAULT_MEMBER_API_TIMEOUT_MS, + ); + const requestTimeout = Number.isFinite(configuredTimeout) + ? configuredTimeout + : DEFAULT_MEMBER_API_TIMEOUT_MS; this.logger.log( `Fetching member info for ${uniqueUserIds.length} unique users in ${totalBatches} batches of ${batchSize}...`, @@ -118,12 +131,16 @@ export class MemberApiService { const currentBatchNum = Math.floor(i / batchSize) + 1; const batchIds = uniqueUserIds.slice(i, i + batchSize); - // Construct query string by repeating the key - const queryString = - batchIds.length > 1 - ? batchIds.map((id) => `userIds=${encodeURIComponent(id)}`).join('&') - : `userId=${encodeURIComponent(batchIds[0])}`; - const apiUrl = `${this.MEMBER_API_URL}?${queryString}`; + const queryParams = new URLSearchParams(); + if (batchIds.length > 1) { + batchIds.forEach((id) => queryParams.append('userIds', String(id))); + } else { + queryParams.append('userId', String(batchIds[0])); + } + queryParams.append('fields', MEMBER_LOOKUP_FIELDS.join(',')); + queryParams.append('includeStats', 'false'); + + const apiUrl = `${this.MEMBER_API_URL}?${queryParams.toString()}`; // Log base URL and count for the current batch this.logger.debug( @@ -137,8 +154,7 @@ export class MemberApiService { Authorization: `Bearer ${token}`, 'Content-Type': 'application/json', }, - // Consider adding a reasonable timeout per batch request - // timeout: this.configService.get('HTTP_TIMEOUT', 10000), + timeout: requestTimeout, }), );