From b9ad99b0f54ac3b37d23b92b7873becd4227b055 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Thu, 6 Feb 2025 21:41:08 +0100 Subject: [PATCH 01/31] refactor: create base http client class --- src/lib/clients/base-backend.test.ts | 33 ++++++++++++++++++++++++++++ src/lib/clients/base-backend.ts | 24 ++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 src/lib/clients/base-backend.test.ts create mode 100644 src/lib/clients/base-backend.ts diff --git a/src/lib/clients/base-backend.test.ts b/src/lib/clients/base-backend.test.ts new file mode 100644 index 0000000..8e05474 --- /dev/null +++ b/src/lib/clients/base-backend.test.ts @@ -0,0 +1,33 @@ +import { BaseHTTPClientWithAuth } from './base-backend'; +import { AuthClient } from './auth'; +import { describe, beforeEach, it, expect, vi } from 'vitest'; + +describe('BaseHTTPClientWithAuth', () => { + let authClient: AuthClient; + + beforeEach(() => { + authClient = { + getToken: vi.fn().mockResolvedValue('test-token'), + getHeaderName: vi.fn().mockReturnValue('Authorization'), + } as unknown as AuthClient; + }); + + it('should return an empty object if authClient is undefined', async () => { + const client = new BaseHTTPClientWithAuth(); + const headers = await client.buildAuthHeader(); + expect(headers).toEqual({}); + }); + + it('should return the correct auth header if authClient is defined', async () => { + const client = new BaseHTTPClientWithAuth(authClient); + const headers = await client.buildAuthHeader(); + expect(headers).toEqual({ Authorization: 'Bearer test-token' }); + }); + + it('should call getToken and getHeaderName on authClient', async () => { + const client = new BaseHTTPClientWithAuth(authClient); + await client.buildAuthHeader(); + expect(authClient.getToken).toHaveBeenCalled(); + expect(authClient.getHeaderName).toHaveBeenCalled(); + }); +}); diff --git a/src/lib/clients/base-backend.ts b/src/lib/clients/base-backend.ts new file mode 100644 index 0000000..5349f4d --- /dev/null +++ b/src/lib/clients/base-backend.ts @@ -0,0 +1,24 @@ +import { AuthClient } from './auth'; + +interface IBaseHTTPClientWithAuth { + buildAuthHeader: () => Promise>; +} + +export class BaseHTTPClientWithAuth implements IBaseHTTPClientWithAuth { + private authClient?: AuthClient | undefined; + + constructor(authClient?: AuthClient) { + this.authClient = authClient; + } + + async buildAuthHeader(): Promise> { + if (this.authClient === undefined) { + return {}; + } + + const authToken = await this.authClient.getToken(); + return { + [this.authClient.getHeaderName()]: `Bearer ${authToken}`, + }; + } +} From 4e341bef374706190a73c81dd831a6ae370bc6b0 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Thu, 6 Feb 2025 21:43:06 +0100 Subject: [PATCH 02/31] refactor: rename backend client to artists backend client and update references --- ...ackend.test.ts => artists-backend.test.ts} | 14 +++++------ .../{backend.ts => artists-backend.ts} | 24 +++++++------------ src/pages/api/artists/search.test.ts | 2 +- src/pages/api/artists/search.ts | 9 ++++--- 4 files changed, 22 insertions(+), 27 deletions(-) rename src/lib/clients/{backend.test.ts => artists-backend.test.ts} (85%) rename src/lib/clients/{backend.ts => artists-backend.ts} (74%) diff --git a/src/lib/clients/backend.test.ts b/src/lib/clients/artists-backend.test.ts similarity index 85% rename from src/lib/clients/backend.test.ts rename to src/lib/clients/artists-backend.test.ts index fbb0e28..93b94c0 100644 --- a/src/lib/clients/backend.test.ts +++ b/src/lib/clients/artists-backend.test.ts @@ -1,10 +1,10 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; -import { HTTPBackendClient } from './backend'; +import { ArtistsHTTPBackendClient } from './artists-backend'; import { FakeHttpClient, HttpResponse, Method } from './http'; import { FakeAuthClient } from './auth'; -describe('HTTPBackendClient', () => { +describe('ArtistsHTTPBackendClient', () => { let url: string; let token: string; let name: string; @@ -28,7 +28,7 @@ describe('HTTPBackendClient', () => { }); it('should call the client with the correct parameters', async () => { - const client = new HTTPBackendClient(url, httpClient); + const client = new ArtistsHTTPBackendClient(url, httpClient); vi.spyOn(httpClient, 'send'); await client.searchArtists(token, name, limit); @@ -45,7 +45,7 @@ describe('HTTPBackendClient', () => { const authHeader = 'Some-Header'; const authToken = 'some-token'; const authClient = new FakeAuthClient(authToken, authHeader); - const client = new HTTPBackendClient(url, httpClient, authClient); + const client = new ArtistsHTTPBackendClient(url, httpClient, authClient); vi.spyOn(httpClient, 'send'); await client.searchArtists(token, name, limit); @@ -62,7 +62,7 @@ describe('HTTPBackendClient', () => { }); it('should return the list of artists returned by the HTTP client', async () => { - const client = new HTTPBackendClient(url, httpClient); + const client = new ArtistsHTTPBackendClient(url, httpClient); const actual = await client.searchArtists(token, name, limit); @@ -76,7 +76,7 @@ describe('HTTPBackendClient', () => { it('should throw an error if the HTTP client fails', async () => { const errorMessage = 'Request failed'; httpClient.setSendErrorMessage(errorMessage); - const client = new HTTPBackendClient(url, httpClient); + const client = new ArtistsHTTPBackendClient(url, httpClient); await expect(client.searchArtists(token, name, limit)).rejects.toThrow( errorMessage @@ -87,7 +87,7 @@ describe('HTTPBackendClient', () => { const errorMessage = 'Auth request failed'; const authClient = new FakeAuthClient('some-token', 'Some-Header'); authClient.setGetTokenErrorMessage(errorMessage); - const client = new HTTPBackendClient(url, httpClient, authClient); + const client = new ArtistsHTTPBackendClient(url, httpClient, authClient); await expect(client.searchArtists(token, name, limit)).rejects.toThrow( errorMessage diff --git a/src/lib/clients/backend.ts b/src/lib/clients/artists-backend.ts similarity index 74% rename from src/lib/clients/backend.ts rename to src/lib/clients/artists-backend.ts index 385a221..b5c6cbf 100644 --- a/src/lib/clients/backend.ts +++ b/src/lib/clients/artists-backend.ts @@ -1,8 +1,9 @@ import { Artist } from '../artists'; import { AuthClient } from './auth'; +import { BaseHTTPClientWithAuth } from './base-backend'; import { HttpClient, Method } from './http'; -export interface BackendClient { +export interface IArtistsHTTPBackendClient { searchArtists( _token: string, _name: string, @@ -10,15 +11,17 @@ export interface BackendClient { ): Promise; } -export class HTTPBackendClient implements BackendClient { +export class ArtistsHTTPBackendClient + extends BaseHTTPClientWithAuth + implements IArtistsHTTPBackendClient +{ private url: string; - private authClient?: AuthClient | undefined; private httpClient: HttpClient; constructor(url: string, httpClient: HttpClient, authClient?: AuthClient) { + super(authClient); this.url = url; this.httpClient = httpClient; - this.authClient = authClient; } async searchArtists( @@ -41,20 +44,9 @@ export class HTTPBackendClient implements BackendClient { ) ); } - - private async buildAuthHeader(): Promise> { - if (this.authClient === undefined) { - return {}; - } - - const authToken = await this.authClient.getToken(); - return { - [this.authClient.getHeaderName()]: `Bearer ${authToken}`, - }; - } } -export class FakeBackendClient implements BackendClient { +export class FakeBackendClient implements IArtistsHTTPBackendClient { private searchArtistError: Error | undefined = undefined; private searchArtistResult: Artist[]; diff --git a/src/pages/api/artists/search.test.ts b/src/pages/api/artists/search.test.ts index 3fc4162..8d6f46d 100644 --- a/src/pages/api/artists/search.test.ts +++ b/src/pages/api/artists/search.test.ts @@ -1,7 +1,7 @@ import { describe, it, vi, expect } from 'vitest'; import { NextApiRequest, NextApiResponse } from 'next'; import { createSearchArtistHandler, SearchArtistHandlerParams } from './search'; -import { FakeBackendClient } from '@/lib/clients/backend'; +import { FakeBackendClient } from '@/lib/clients/artists-backend'; import { Artist } from '@/lib/artists'; describe('searchArtistHandler', () => { diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index 64bde0a..5571e69 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -2,10 +2,13 @@ import type { NextApiRequest, NextApiResponse } from 'next'; import { z } from 'zod'; import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; -import { BackendClient, HTTPBackendClient } from '@/lib/clients/backend'; +import { + IArtistsHTTPBackendClient, + ArtistsHTTPBackendClient, +} from '@/lib/clients/artists-backend'; export type SearchArtistHandlerParams = { - client: BackendClient; + client: IArtistsHTTPBackendClient; defaultLimit: number; maxLimit: number; }; @@ -90,7 +93,7 @@ const maxLimit: number = parseInt(process.env.SEARCH_ARTISTS_MAX_LIMIT || '10'); const serverHost: string = process.env.SERVER_HOST || 'http://localhost'; const serverPort: number = parseInt(process.env.SERVER_PORT || '8080'); const httpClient: HttpClient = new HttpBaseClient(); -const client: BackendClient = new HTTPBackendClient( +const client: IArtistsHTTPBackendClient = new ArtistsHTTPBackendClient( `${serverHost}:${serverPort}`, httpClient ); From 9b408c0e0355fcf2b8a1de0fc589b5b29e98c814 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Sat, 8 Feb 2025 18:07:16 +0100 Subject: [PATCH 03/31] refactor: rename interface --- src/lib/clients/artists-backend.ts | 6 +++--- src/pages/api/artists/search.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/clients/artists-backend.ts b/src/lib/clients/artists-backend.ts index b5c6cbf..99f3438 100644 --- a/src/lib/clients/artists-backend.ts +++ b/src/lib/clients/artists-backend.ts @@ -3,7 +3,7 @@ import { AuthClient } from './auth'; import { BaseHTTPClientWithAuth } from './base-backend'; import { HttpClient, Method } from './http'; -export interface IArtistsHTTPBackendClient { +export interface ArtistsClient { searchArtists( _token: string, _name: string, @@ -13,7 +13,7 @@ export interface IArtistsHTTPBackendClient { export class ArtistsHTTPBackendClient extends BaseHTTPClientWithAuth - implements IArtistsHTTPBackendClient + implements ArtistsClient { private url: string; private httpClient: HttpClient; @@ -46,7 +46,7 @@ export class ArtistsHTTPBackendClient } } -export class FakeBackendClient implements IArtistsHTTPBackendClient { +export class FakeBackendClient implements ArtistsClient { private searchArtistError: Error | undefined = undefined; private searchArtistResult: Artist[]; diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index 5571e69..32889f3 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -3,12 +3,12 @@ import { z } from 'zod'; import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; import { - IArtistsHTTPBackendClient, + ArtistsClient, ArtistsHTTPBackendClient, } from '@/lib/clients/artists-backend'; export type SearchArtistHandlerParams = { - client: IArtistsHTTPBackendClient; + client: ArtistsClient; defaultLimit: number; maxLimit: number; }; @@ -93,7 +93,7 @@ const maxLimit: number = parseInt(process.env.SEARCH_ARTISTS_MAX_LIMIT || '10'); const serverHost: string = process.env.SERVER_HOST || 'http://localhost'; const serverPort: number = parseInt(process.env.SERVER_PORT || '8080'); const httpClient: HttpClient = new HttpBaseClient(); -const client: IArtistsHTTPBackendClient = new ArtistsHTTPBackendClient( +const client: ArtistsClient = new ArtistsHTTPBackendClient( `${serverHost}:${serverPort}`, httpClient ); From 5b741192833572c8a90972c2d48c510e01676ce3 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Sat, 8 Feb 2025 18:12:17 +0100 Subject: [PATCH 04/31] refactor: rename file --- src/lib/clients/{artists-backend.test.ts => artists.test.ts} | 2 +- src/lib/clients/{artists-backend.ts => artists.ts} | 0 src/pages/api/artists/search.test.ts | 2 +- src/pages/api/artists/search.ts | 5 +---- 4 files changed, 3 insertions(+), 6 deletions(-) rename src/lib/clients/{artists-backend.test.ts => artists.test.ts} (97%) rename src/lib/clients/{artists-backend.ts => artists.ts} (100%) diff --git a/src/lib/clients/artists-backend.test.ts b/src/lib/clients/artists.test.ts similarity index 97% rename from src/lib/clients/artists-backend.test.ts rename to src/lib/clients/artists.test.ts index 93b94c0..665a769 100644 --- a/src/lib/clients/artists-backend.test.ts +++ b/src/lib/clients/artists.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; -import { ArtistsHTTPBackendClient } from './artists-backend'; +import { ArtistsHTTPBackendClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; import { FakeAuthClient } from './auth'; diff --git a/src/lib/clients/artists-backend.ts b/src/lib/clients/artists.ts similarity index 100% rename from src/lib/clients/artists-backend.ts rename to src/lib/clients/artists.ts diff --git a/src/pages/api/artists/search.test.ts b/src/pages/api/artists/search.test.ts index 8d6f46d..1751e0d 100644 --- a/src/pages/api/artists/search.test.ts +++ b/src/pages/api/artists/search.test.ts @@ -1,7 +1,7 @@ import { describe, it, vi, expect } from 'vitest'; import { NextApiRequest, NextApiResponse } from 'next'; import { createSearchArtistHandler, SearchArtistHandlerParams } from './search'; -import { FakeBackendClient } from '@/lib/clients/artists-backend'; +import { FakeBackendClient } from '@/lib/clients/artists'; import { Artist } from '@/lib/artists'; describe('searchArtistHandler', () => { diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index 32889f3..3543197 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -2,10 +2,7 @@ import type { NextApiRequest, NextApiResponse } from 'next'; import { z } from 'zod'; import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; -import { - ArtistsClient, - ArtistsHTTPBackendClient, -} from '@/lib/clients/artists-backend'; +import { ArtistsClient, ArtistsHTTPBackendClient } from '@/lib/clients/artists'; export type SearchArtistHandlerParams = { client: ArtistsClient; From ff4d25d521133dda5b388a7d985df769e4547eec Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Sat, 8 Feb 2025 18:33:49 +0100 Subject: [PATCH 05/31] refactor: rename interface --- src/lib/clients/base-backend.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/clients/base-backend.ts b/src/lib/clients/base-backend.ts index 5349f4d..1ce407e 100644 --- a/src/lib/clients/base-backend.ts +++ b/src/lib/clients/base-backend.ts @@ -1,10 +1,10 @@ import { AuthClient } from './auth'; -interface IBaseHTTPClientWithAuth { +interface BaseClientWithAuth { buildAuthHeader: () => Promise>; } -export class BaseHTTPClientWithAuth implements IBaseHTTPClientWithAuth { +export class BaseHTTPClientWithAuth implements BaseClientWithAuth { private authClient?: AuthClient | undefined; constructor(authClient?: AuthClient) { From f8510a4f2a348a7188dec933d8dd1de32d67cf8a Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Sun, 9 Feb 2025 10:37:56 +0100 Subject: [PATCH 06/31] refactor: build auth headers with the two tokens --- src/lib/clients/artists.ts | 5 ++--- src/lib/clients/base-backend.test.ts | 23 +++++++++++++++++++++-- src/lib/clients/base-backend.ts | 18 +++++++++++------- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index 99f3438..97ba073 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -29,14 +29,13 @@ export class ArtistsHTTPBackendClient name: string, limit: number ): Promise { - const authHeader = await this.buildAuthHeader(); - const backendAuthHeader = { Authorization: `Bearer ${token}` }; + const authHeader = await this.buildAuthHeader(token); return this.httpClient .send({ url: `${this.url}/artists/search`, method: Method.Get, params: { name, limit }, - headers: { ...authHeader, ...backendAuthHeader }, + headers: authHeader, }) .then((response) => response.data.map( diff --git a/src/lib/clients/base-backend.test.ts b/src/lib/clients/base-backend.test.ts index 8e05474..045502c 100644 --- a/src/lib/clients/base-backend.test.ts +++ b/src/lib/clients/base-backend.test.ts @@ -8,7 +8,7 @@ describe('BaseHTTPClientWithAuth', () => { beforeEach(() => { authClient = { getToken: vi.fn().mockResolvedValue('test-token'), - getHeaderName: vi.fn().mockReturnValue('Authorization'), + getHeaderName: vi.fn().mockReturnValue('X-Serverless-Authorization'), } as unknown as AuthClient; }); @@ -21,7 +21,9 @@ describe('BaseHTTPClientWithAuth', () => { it('should return the correct auth header if authClient is defined', async () => { const client = new BaseHTTPClientWithAuth(authClient); const headers = await client.buildAuthHeader(); - expect(headers).toEqual({ Authorization: 'Bearer test-token' }); + expect(headers).toEqual({ + 'X-Serverless-Authorization': 'Bearer test-token', + }); }); it('should call getToken and getHeaderName on authClient', async () => { @@ -30,4 +32,21 @@ describe('BaseHTTPClientWithAuth', () => { expect(authClient.getToken).toHaveBeenCalled(); expect(authClient.getHeaderName).toHaveBeenCalled(); }); + + it('should return the correct auth header if spotifyToken is provided', async () => { + const client = new BaseHTTPClientWithAuth(); + const spotifyToken = 'spotify-token'; + const headers = await client.buildAuthHeader(spotifyToken); + expect(headers).toEqual({ Authorization: 'Bearer spotify-token' }); + }); + + it('should return the correct auth header if authClient and spotifyToken are provided', async () => { + const client = new BaseHTTPClientWithAuth(authClient); + const spotifyToken = 'spotify-token'; + const headers = await client.buildAuthHeader(spotifyToken); + expect(headers).toEqual({ + Authorization: 'Bearer spotify-token', + 'X-Serverless-Authorization': 'Bearer test-token', + }); + }); }); diff --git a/src/lib/clients/base-backend.ts b/src/lib/clients/base-backend.ts index 1ce407e..22ea506 100644 --- a/src/lib/clients/base-backend.ts +++ b/src/lib/clients/base-backend.ts @@ -1,7 +1,7 @@ import { AuthClient } from './auth'; interface BaseClientWithAuth { - buildAuthHeader: () => Promise>; + buildAuthHeader: (_spotifyToken: string) => Promise>; } export class BaseHTTPClientWithAuth implements BaseClientWithAuth { @@ -11,14 +11,18 @@ export class BaseHTTPClientWithAuth implements BaseClientWithAuth { this.authClient = authClient; } - async buildAuthHeader(): Promise> { - if (this.authClient === undefined) { - return {}; - } + async buildAuthHeader( + spotifyToken?: string + ): Promise> { + const headers: Record = spotifyToken + ? { Authorization: `Bearer ${spotifyToken}` } + : {}; + + if (!this.authClient) return headers; - const authToken = await this.authClient.getToken(); return { - [this.authClient.getHeaderName()]: `Bearer ${authToken}`, + ...headers, + [this.authClient.getHeaderName()]: `Bearer ${await this.authClient.getToken()}`, }; } } From a8361a2420347b0db9ffd4fafa2a62e570bcf9d6 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Mon, 10 Feb 2025 20:02:33 +0100 Subject: [PATCH 07/31] refactor: remove token parameter from search artist handler and use getToken for authentication --- src/pages/api/artists/search.test.ts | 44 ++++++++++++---------------- src/pages/api/artists/search.ts | 19 ++++++++---- src/types/next-auth.d.ts | 6 ++++ 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/pages/api/artists/search.test.ts b/src/pages/api/artists/search.test.ts index 1751e0d..9480d39 100644 --- a/src/pages/api/artists/search.test.ts +++ b/src/pages/api/artists/search.test.ts @@ -1,13 +1,21 @@ -import { describe, it, vi, expect } from 'vitest'; +import { describe, it, vi, expect, beforeEach } from 'vitest'; import { NextApiRequest, NextApiResponse } from 'next'; import { createSearchArtistHandler, SearchArtistHandlerParams } from './search'; import { FakeBackendClient } from '@/lib/clients/artists'; import { Artist } from '@/lib/artists'; +import { getToken } from 'next-auth/jwt'; + +vi.mock('next-auth/jwt', () => ({ + getToken: vi.fn(), +})); describe('searchArtistHandler', () => { - function createMockRequest( - query: any = { name: 'Brutus', token: 'some token' } - ): NextApiRequest { + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(getToken).mockResolvedValue({ accessToken: 'mocked-token' }); + }); + + function createMockRequest(query: any = { name: 'Brutus' }): NextApiRequest { return { query } as unknown as NextApiRequest; } @@ -32,7 +40,7 @@ describe('searchArtistHandler', () => { { _title: 'not a number', limit: 'invalid' }, { _title: 'bigger than maximum', limit: '20' }, ])('should return 400 if limit is $_title', async ({ limit }) => { - const request = createMockRequest({ name: 'test', token: 'test', limit }); + const request = createMockRequest({ name: 'test', limit }); const response = createMockResponse(); createHandler()(request, response); @@ -48,28 +56,14 @@ describe('searchArtistHandler', () => { _title: 'name is missing', variable: 'name', name: null, - token: 'some token', }, { _title: 'name is not a string', variable: 'name', name: 42, - token: 'some token', - }, - { - _title: 'token is missing', - variable: 'token', - name: 'some name', - token: null, - }, - { - _title: 'token is not a string', - variable: 'token', - name: 'some name', - token: 10, }, - ])('should return 400 if $_title', async ({ variable, name, token }) => { - const request = createMockRequest({ name: name, token: token }); + ])('should return 400 if $_title', async ({ variable, name }) => { + const request = createMockRequest({ name: name }); const response = createMockResponse(); createHandler()(request, response); @@ -83,23 +77,23 @@ describe('searchArtistHandler', () => { it.each([{ limit: undefined }, { limit: '4' }])( 'should search with the provided arguments (limit: $limit)', async ({ limit }) => { - const args: { name: string; token: string } = { + const args: { name: string } = { name: 'name', - token: 'token', }; const request = createMockRequest({ ...args, limit: limit }); const client = new FakeBackendClient(); vi.spyOn(client, 'searchArtists'); const defaultLimit = 5; - createHandler({ + + await createHandler({ client: client, defaultLimit: defaultLimit, maxLimit: 10, })(request, createMockResponse()); expect(client.searchArtists).toBeCalledWith( - args.token, + 'mocked-token', args.name, limit ? parseInt(limit, 10) : defaultLimit ); diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index 3543197..a05bcdc 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; import { ArtistsClient, ArtistsHTTPBackendClient } from '@/lib/clients/artists'; +import { getToken } from 'next-auth/jwt'; export type SearchArtistHandlerParams = { client: ArtistsClient; @@ -26,9 +27,6 @@ function searchQuerySchema(defaultLimit: number, maxLimit: number) { name: z.string({ message: '"name" should be provided as a string', }), - token: z.string({ - message: '"token" should be provided as a string', - }), limit: z.coerce .number({ message: limitMessage, @@ -62,11 +60,20 @@ export function createSearchArtistHandler({ return; } + const token = await getToken({ req: request }); + + if (!token) { + response.status(401).json({ message: 'Unauthorized' }); + return; + } + + const { name, limit } = parsedArgs.data; + try { const searchResults = await client.searchArtists( - parsedArgs.data.token as string, - parsedArgs.data.name as string, - parsedArgs.data.limit + token.accessToken, + name, + limit ); response.status(200).json({ artists: searchResults.map((artist: Artist) => ({ diff --git a/src/types/next-auth.d.ts b/src/types/next-auth.d.ts index 4bdb56a..90f7bd2 100644 --- a/src/types/next-auth.d.ts +++ b/src/types/next-auth.d.ts @@ -8,3 +8,9 @@ declare module 'next-auth' { } & DefaultSession['user']; } } + +declare module 'next-auth/jwt' { + export interface JWT { + accessToken: string; + } +} From b9d38fc6cfeb07007095cd438aa62aed5082fc9b Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Mon, 10 Feb 2025 20:05:31 +0100 Subject: [PATCH 08/31] refactor: rename artist http client --- src/lib/clients/artists.test.ts | 12 ++++++------ src/lib/clients/artists.ts | 4 ++-- src/pages/api/artists/search.test.ts | 10 +++++----- src/pages/api/artists/search.ts | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index 665a769..e81b8e2 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; -import { ArtistsHTTPBackendClient } from './artists'; +import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; import { FakeAuthClient } from './auth'; @@ -28,7 +28,7 @@ describe('ArtistsHTTPBackendClient', () => { }); it('should call the client with the correct parameters', async () => { - const client = new ArtistsHTTPBackendClient(url, httpClient); + const client = new ArtistsHTTPClient(url, httpClient); vi.spyOn(httpClient, 'send'); await client.searchArtists(token, name, limit); @@ -45,7 +45,7 @@ describe('ArtistsHTTPBackendClient', () => { const authHeader = 'Some-Header'; const authToken = 'some-token'; const authClient = new FakeAuthClient(authToken, authHeader); - const client = new ArtistsHTTPBackendClient(url, httpClient, authClient); + const client = new ArtistsHTTPClient(url, httpClient, authClient); vi.spyOn(httpClient, 'send'); await client.searchArtists(token, name, limit); @@ -62,7 +62,7 @@ describe('ArtistsHTTPBackendClient', () => { }); it('should return the list of artists returned by the HTTP client', async () => { - const client = new ArtistsHTTPBackendClient(url, httpClient); + const client = new ArtistsHTTPClient(url, httpClient); const actual = await client.searchArtists(token, name, limit); @@ -76,7 +76,7 @@ describe('ArtistsHTTPBackendClient', () => { it('should throw an error if the HTTP client fails', async () => { const errorMessage = 'Request failed'; httpClient.setSendErrorMessage(errorMessage); - const client = new ArtistsHTTPBackendClient(url, httpClient); + const client = new ArtistsHTTPClient(url, httpClient); await expect(client.searchArtists(token, name, limit)).rejects.toThrow( errorMessage @@ -87,7 +87,7 @@ describe('ArtistsHTTPBackendClient', () => { const errorMessage = 'Auth request failed'; const authClient = new FakeAuthClient('some-token', 'Some-Header'); authClient.setGetTokenErrorMessage(errorMessage); - const client = new ArtistsHTTPBackendClient(url, httpClient, authClient); + const client = new ArtistsHTTPClient(url, httpClient, authClient); await expect(client.searchArtists(token, name, limit)).rejects.toThrow( errorMessage diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index 97ba073..a643253 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -11,7 +11,7 @@ export interface ArtistsClient { ): Promise; } -export class ArtistsHTTPBackendClient +export class ArtistsHTTPClient extends BaseHTTPClientWithAuth implements ArtistsClient { @@ -45,7 +45,7 @@ export class ArtistsHTTPBackendClient } } -export class FakeBackendClient implements ArtistsClient { +export class FakeArtistsHTTPClient implements ArtistsClient { private searchArtistError: Error | undefined = undefined; private searchArtistResult: Artist[]; diff --git a/src/pages/api/artists/search.test.ts b/src/pages/api/artists/search.test.ts index 9480d39..dafb469 100644 --- a/src/pages/api/artists/search.test.ts +++ b/src/pages/api/artists/search.test.ts @@ -1,7 +1,7 @@ import { describe, it, vi, expect, beforeEach } from 'vitest'; import { NextApiRequest, NextApiResponse } from 'next'; import { createSearchArtistHandler, SearchArtistHandlerParams } from './search'; -import { FakeBackendClient } from '@/lib/clients/artists'; +import { FakeArtistsHTTPClient } from '@/lib/clients/artists'; import { Artist } from '@/lib/artists'; import { getToken } from 'next-auth/jwt'; @@ -28,7 +28,7 @@ describe('searchArtistHandler', () => { function createHandler( { client, defaultLimit, maxLimit }: SearchArtistHandlerParams = { - client: new FakeBackendClient(), + client: new FakeArtistsHTTPClient(), defaultLimit: 5, maxLimit: 10, } @@ -81,7 +81,7 @@ describe('searchArtistHandler', () => { name: 'name', }; const request = createMockRequest({ ...args, limit: limit }); - const client = new FakeBackendClient(); + const client = new FakeArtistsHTTPClient(); vi.spyOn(client, 'searchArtists'); const defaultLimit = 5; @@ -105,7 +105,7 @@ describe('searchArtistHandler', () => { new Artist('Brutus'), new Artist('Brutus Daughters', 'http://some_url'), ]; - const client = new FakeBackendClient(retrievedArtist); + const client = new FakeArtistsHTTPClient(retrievedArtist); const response = createMockResponse(); const handler = createHandler({ @@ -128,7 +128,7 @@ describe('searchArtistHandler', () => { }); it('should return an error if search fails', async () => { - const client = new FakeBackendClient(); + const client = new FakeArtistsHTTPClient(); client.setSearchArtistError(new Error('test error')); const response = createMockResponse(); diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index a05bcdc..43e008a 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -2,7 +2,7 @@ import type { NextApiRequest, NextApiResponse } from 'next'; import { z } from 'zod'; import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; -import { ArtistsClient, ArtistsHTTPBackendClient } from '@/lib/clients/artists'; +import { ArtistsClient, ArtistsHTTPClient } from '@/lib/clients/artists'; import { getToken } from 'next-auth/jwt'; export type SearchArtistHandlerParams = { @@ -97,7 +97,7 @@ const maxLimit: number = parseInt(process.env.SEARCH_ARTISTS_MAX_LIMIT || '10'); const serverHost: string = process.env.SERVER_HOST || 'http://localhost'; const serverPort: number = parseInt(process.env.SERVER_PORT || '8080'); const httpClient: HttpClient = new HttpBaseClient(); -const client: ArtistsClient = new ArtistsHTTPBackendClient( +const client: ArtistsClient = new ArtistsHTTPClient( `${serverHost}:${serverPort}`, httpClient ); From 831c7bc563799c8cb99017c5ee90ede1b077bf93 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Tue, 11 Feb 2025 07:29:02 +0100 Subject: [PATCH 09/31] refactor: use fake auth client in base backend tests --- src/lib/clients/base-backend.test.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/lib/clients/base-backend.test.ts b/src/lib/clients/base-backend.test.ts index 045502c..5510897 100644 --- a/src/lib/clients/base-backend.test.ts +++ b/src/lib/clients/base-backend.test.ts @@ -1,16 +1,13 @@ import { BaseHTTPClientWithAuth } from './base-backend'; -import { AuthClient } from './auth'; -import { describe, beforeEach, it, expect, vi } from 'vitest'; +import { FakeAuthClient } from './auth'; +import { describe, it, expect, vi } from 'vitest'; describe('BaseHTTPClientWithAuth', () => { - let authClient: AuthClient; - - beforeEach(() => { - authClient = { - getToken: vi.fn().mockResolvedValue('test-token'), - getHeaderName: vi.fn().mockReturnValue('X-Serverless-Authorization'), - } as unknown as AuthClient; - }); + function createAuthClient() { + const authHeader = 'X-Serverless-Authorization'; + const authToken = 'test-token'; + return new FakeAuthClient(authToken, authHeader); + } it('should return an empty object if authClient is undefined', async () => { const client = new BaseHTTPClientWithAuth(); @@ -19,6 +16,7 @@ describe('BaseHTTPClientWithAuth', () => { }); it('should return the correct auth header if authClient is defined', async () => { + const authClient = createAuthClient(); const client = new BaseHTTPClientWithAuth(authClient); const headers = await client.buildAuthHeader(); expect(headers).toEqual({ @@ -27,6 +25,9 @@ describe('BaseHTTPClientWithAuth', () => { }); it('should call getToken and getHeaderName on authClient', async () => { + const authClient = createAuthClient(); + vi.spyOn(authClient, 'getToken'); + vi.spyOn(authClient, 'getHeaderName'); const client = new BaseHTTPClientWithAuth(authClient); await client.buildAuthHeader(); expect(authClient.getToken).toHaveBeenCalled(); @@ -41,6 +42,7 @@ describe('BaseHTTPClientWithAuth', () => { }); it('should return the correct auth header if authClient and spotifyToken are provided', async () => { + const authClient = createAuthClient(); const client = new BaseHTTPClientWithAuth(authClient); const spotifyToken = 'spotify-token'; const headers = await client.buildAuthHeader(spotifyToken); From 7ad28fd8f9fdc39b08d7a1c210a5359ad0f5c82b Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Tue, 11 Feb 2025 07:31:03 +0100 Subject: [PATCH 10/31] refactor: rename base backend file name --- src/lib/clients/artists.ts | 2 +- .../clients/{base-backend.test.ts => base-http-client.test.ts} | 2 +- src/lib/clients/{base-backend.ts => base-http-client.ts} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/lib/clients/{base-backend.test.ts => base-http-client.test.ts} (97%) rename src/lib/clients/{base-backend.ts => base-http-client.ts} (100%) diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index a643253..93f84c7 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -1,6 +1,6 @@ import { Artist } from '../artists'; import { AuthClient } from './auth'; -import { BaseHTTPClientWithAuth } from './base-backend'; +import { BaseHTTPClientWithAuth } from './base-http-client'; import { HttpClient, Method } from './http'; export interface ArtistsClient { diff --git a/src/lib/clients/base-backend.test.ts b/src/lib/clients/base-http-client.test.ts similarity index 97% rename from src/lib/clients/base-backend.test.ts rename to src/lib/clients/base-http-client.test.ts index 5510897..4a42313 100644 --- a/src/lib/clients/base-backend.test.ts +++ b/src/lib/clients/base-http-client.test.ts @@ -1,4 +1,4 @@ -import { BaseHTTPClientWithAuth } from './base-backend'; +import { BaseHTTPClientWithAuth } from './base-http-client'; import { FakeAuthClient } from './auth'; import { describe, it, expect, vi } from 'vitest'; diff --git a/src/lib/clients/base-backend.ts b/src/lib/clients/base-http-client.ts similarity index 100% rename from src/lib/clients/base-backend.ts rename to src/lib/clients/base-http-client.ts From 6373df9570d4528c96e0eef1ec7e6db0669b6d40 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Tue, 11 Feb 2025 08:20:49 +0100 Subject: [PATCH 11/31] refactor: remove BaseHTTPClientWithAuth and implement HTTPAuthHeaderBuilder for authentication --- src/lib/clients/artists.test.ts | 43 +++++--- src/lib/clients/artists.ts | 19 ++-- src/lib/clients/auth.test.ts | 132 +++++++++++++++++------ src/lib/clients/auth.ts | 52 ++++++++- src/lib/clients/base-http-client.test.ts | 54 ---------- src/lib/clients/base-http-client.ts | 28 ----- src/pages/api/artists/search.ts | 5 +- 7 files changed, 191 insertions(+), 142 deletions(-) delete mode 100644 src/lib/clients/base-http-client.test.ts delete mode 100644 src/lib/clients/base-http-client.ts diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index e81b8e2..bf2dfd1 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; -import { FakeAuthClient } from './auth'; +import { FakeHTTPAuthHeaderBuilder } from './auth'; describe('ArtistsHTTPBackendClient', () => { let url: string; @@ -28,7 +28,12 @@ describe('ArtistsHTTPBackendClient', () => { }); it('should call the client with the correct parameters', async () => { - const client = new ArtistsHTTPClient(url, httpClient); + const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder(); + const client = new ArtistsHTTPClient( + url, + httpClient, + httpAuthHeaderBuilder + ); vi.spyOn(httpClient, 'send'); await client.searchArtists(token, name, limit); @@ -44,8 +49,15 @@ describe('ArtistsHTTPBackendClient', () => { it('should call the client with an additional auth header if auth client is provided', async () => { const authHeader = 'Some-Header'; const authToken = 'some-token'; - const authClient = new FakeAuthClient(authToken, authHeader); - const client = new ArtistsHTTPClient(url, httpClient, authClient); + const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder( + authToken, + authHeader + ); + const client = new ArtistsHTTPClient( + url, + httpClient, + httpAuthHeaderBuilder + ); vi.spyOn(httpClient, 'send'); await client.searchArtists(token, name, limit); @@ -62,7 +74,12 @@ describe('ArtistsHTTPBackendClient', () => { }); it('should return the list of artists returned by the HTTP client', async () => { - const client = new ArtistsHTTPClient(url, httpClient); + const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder(); + const client = new ArtistsHTTPClient( + url, + httpClient, + httpAuthHeaderBuilder + ); const actual = await client.searchArtists(token, name, limit); @@ -76,18 +93,12 @@ describe('ArtistsHTTPBackendClient', () => { it('should throw an error if the HTTP client fails', async () => { const errorMessage = 'Request failed'; httpClient.setSendErrorMessage(errorMessage); - const client = new ArtistsHTTPClient(url, httpClient); - - await expect(client.searchArtists(token, name, limit)).rejects.toThrow( - errorMessage + const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder(); + const client = new ArtistsHTTPClient( + url, + httpClient, + httpAuthHeaderBuilder ); - }); - - it('should throw an error if the auth client fails', async () => { - const errorMessage = 'Auth request failed'; - const authClient = new FakeAuthClient('some-token', 'Some-Header'); - authClient.setGetTokenErrorMessage(errorMessage); - const client = new ArtistsHTTPClient(url, httpClient, authClient); await expect(client.searchArtists(token, name, limit)).rejects.toThrow( errorMessage diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index 93f84c7..c700037 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -1,6 +1,5 @@ import { Artist } from '../artists'; -import { AuthClient } from './auth'; -import { BaseHTTPClientWithAuth } from './base-http-client'; +import { AuthHeaderBuilder, HTTPAuthHeaderBuilder } from './auth'; import { HttpClient, Method } from './http'; export interface ArtistsClient { @@ -11,17 +10,19 @@ export interface ArtistsClient { ): Promise; } -export class ArtistsHTTPClient - extends BaseHTTPClientWithAuth - implements ArtistsClient -{ +export class ArtistsHTTPClient implements ArtistsClient { private url: string; private httpClient: HttpClient; + private httpAuthHeaderBuilder: AuthHeaderBuilder; - constructor(url: string, httpClient: HttpClient, authClient?: AuthClient) { - super(authClient); + constructor( + url: string, + httpClient: HttpClient, + httpAuthHeaderBuilder: HTTPAuthHeaderBuilder + ) { this.url = url; this.httpClient = httpClient; + this.httpAuthHeaderBuilder = httpAuthHeaderBuilder; } async searchArtists( @@ -29,7 +30,7 @@ export class ArtistsHTTPClient name: string, limit: number ): Promise { - const authHeader = await this.buildAuthHeader(token); + const authHeader = await this.httpAuthHeaderBuilder.buildHeader(token); return this.httpClient .send({ url: `${this.url}/artists/search`, diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index 47c10ec..f23e173 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -1,48 +1,116 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; -import { GCPHTTPAuthClient } from './auth'; +import { + FakeGCPAuthClient, + GCPHTTPAuthClient, + HTTPAuthHeaderBuilder, +} from './auth'; import { FakeHttpClient, HttpResponse, Method } from './http'; -describe('HTTPAuthClient', () => { - const audience = 'test-audience'; - let httpClient: FakeHttpClient; - let response: HttpResponse; +describe('AuthClient', () => { + describe('GCPHTTPAuthClient', () => { + const audience = 'test-audience'; + let httpClient: FakeHttpClient; + let response: HttpResponse; - beforeEach(() => { - response = { data: 'test-token', status: 200 }; - httpClient = new FakeHttpClient(response); - }); + beforeEach(() => { + response = { data: 'test-token', status: 200 }; + httpClient = new FakeHttpClient(response); + }); - it('should call the client with the correct parameters', async () => { - const authClient = new GCPHTTPAuthClient(httpClient, audience); - const baseUrl = 'http://some_url/auth'; - authClient.setBaseUrl(baseUrl); - vi.spyOn(httpClient, 'send'); + it('should call the client with the correct parameters', async () => { + const authClient = new GCPHTTPAuthClient(httpClient, audience); + const baseUrl = 'http://some_url/auth'; + authClient.setBaseUrl(baseUrl); + vi.spyOn(httpClient, 'send'); - await authClient.getToken(); + await authClient.getToken(); - expect(httpClient.send).toHaveBeenCalledWith({ - url: baseUrl, - method: Method.Get, - params: { audience: audience }, - headers: { 'Metadata-Flavor': 'Google' }, + expect(httpClient.send).toHaveBeenCalledWith({ + url: baseUrl, + method: Method.Get, + params: { audience: audience }, + headers: { 'Metadata-Flavor': 'Google' }, + }); }); - }); - it('should return token from the client', async () => { - const expected = 'some-token'; - httpClient.setResult({ data: expected, status: 200 }); - const authClient = new GCPHTTPAuthClient(httpClient, audience); + it('should return token from the client', async () => { + const expected = 'some-token'; + httpClient.setResult({ data: expected, status: 200 }); + const authClient = new GCPHTTPAuthClient(httpClient, audience); - const actual = await authClient.getToken(); + const actual = await authClient.getToken(); + + expect(actual).toBe(expected); + }); - expect(actual).toBe(expected); + it('should throw an error if the client fails', async () => { + const errorMessage = 'Test error'; + httpClient.setSendErrorMessage('Test error'); + const authClient = new GCPHTTPAuthClient(httpClient, audience); + + await expect(authClient.getToken()).rejects.toThrow(errorMessage); + }); }); - it('should throw an error if the client fails', async () => { - const errorMessage = 'Test error'; - httpClient.setSendErrorMessage('Test error'); - const authClient = new GCPHTTPAuthClient(httpClient, audience); + describe('HTTPAuthHeaderBuilder', () => { + function createAuthClient() { + const authHeader = 'X-Serverless-Authorization'; + const authToken = 'test-token'; + return new FakeGCPAuthClient(authToken, authHeader); + } + + it('should return an empty object if authClient is undefined', async () => { + const client = new HTTPAuthHeaderBuilder(); + const headers = await client.buildHeader(); + expect(headers).toEqual({}); + }); + + it('should return the correct auth header if authClient is defined', async () => { + const authClient = createAuthClient(); + const client = new HTTPAuthHeaderBuilder(authClient); + const headers = await client.buildHeader(); + expect(headers).toEqual({ + 'X-Serverless-Authorization': 'Bearer test-token', + }); + }); - await expect(authClient.getToken()).rejects.toThrow(errorMessage); + it('should call getToken and getHeaderName on authClient', async () => { + const authClient = createAuthClient(); + vi.spyOn(authClient, 'getToken'); + vi.spyOn(authClient, 'getHeaderName'); + const client = new HTTPAuthHeaderBuilder(authClient); + await client.buildHeader(); + expect(authClient.getToken).toHaveBeenCalled(); + expect(authClient.getHeaderName).toHaveBeenCalled(); + }); + + it('should return the correct auth header if spotifyToken is provided', async () => { + const client = new HTTPAuthHeaderBuilder(); + const spotifyToken = 'spotify-token'; + const headers = await client.buildHeader(spotifyToken); + expect(headers).toEqual({ Authorization: 'Bearer spotify-token' }); + }); + + it('should return the correct auth header if authClient and spotifyToken are provided', async () => { + const authClient = createAuthClient(); + const client = new HTTPAuthHeaderBuilder(authClient); + const spotifyToken = 'spotify-token'; + const headers = await client.buildHeader(spotifyToken); + expect(headers).toEqual({ + Authorization: 'Bearer spotify-token', + 'X-Serverless-Authorization': 'Bearer test-token', + }); + }); + + it('should throw an error if the auth client fails', async () => { + const errorMessage = 'Auth request failed'; + const authClient = createAuthClient(); + authClient.setGetTokenErrorMessage(errorMessage); + const client = new HTTPAuthHeaderBuilder(authClient); + const spotifyToken = 'spotify-token'; + await expect(client.buildHeader(spotifyToken)).rejects.toThrow( + errorMessage + ); + }); }); }); diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index 931979d..8627b9a 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -1,6 +1,6 @@ import { HttpClient, Method } from './http'; -export interface AuthClient { +export interface GCPAuthClient { getToken(): Promise; getHeaderName(): string; } @@ -36,7 +36,7 @@ export class GCPHTTPAuthClient { } } -export class FakeAuthClient { +export class FakeGCPAuthClient { private header: string; private token: string; private getTokenErrorMessage: string | undefined = undefined; @@ -65,3 +65,51 @@ export class FakeAuthClient { return this.header; } } + +export interface AuthHeaderBuilder { + buildHeader: (_spotifyToken: string) => Promise>; +} + +export class HTTPAuthHeaderBuilder implements AuthHeaderBuilder { + private gcpAuthClient?: GCPAuthClient | undefined; + + constructor(gcpAuthClient?: GCPAuthClient) { + this.gcpAuthClient = gcpAuthClient; + } + + async buildHeader(spotifyToken?: string): Promise> { + const headers: Record = spotifyToken + ? { Authorization: `Bearer ${spotifyToken}` } + : {}; + + if (!this.gcpAuthClient) return headers; + + return { + ...headers, + [this.gcpAuthClient.getHeaderName()]: `Bearer ${await this.gcpAuthClient.getToken()}`, + }; + } +} + +export class FakeHTTPAuthHeaderBuilder implements AuthHeaderBuilder { + private gcpAuthToken: string | undefined; + private gcpAuthHeaderName: string | undefined; + + constructor(gcpAuthToken?: string, gcpAuthHeaderName?: string) { + this.gcpAuthToken = gcpAuthToken; + this.gcpAuthHeaderName = gcpAuthHeaderName; + } + + async buildHeader(spotifyToken: string): Promise> { + const headers: Record = spotifyToken + ? { Authorization: `Bearer ${spotifyToken}` } + : {}; + + if (!this.gcpAuthToken || !this.gcpAuthHeaderName) return headers; + + return { + ...headers, + [this.gcpAuthHeaderName]: `Bearer ${this.gcpAuthToken}`, + }; + } +} diff --git a/src/lib/clients/base-http-client.test.ts b/src/lib/clients/base-http-client.test.ts deleted file mode 100644 index 4a42313..0000000 --- a/src/lib/clients/base-http-client.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { BaseHTTPClientWithAuth } from './base-http-client'; -import { FakeAuthClient } from './auth'; -import { describe, it, expect, vi } from 'vitest'; - -describe('BaseHTTPClientWithAuth', () => { - function createAuthClient() { - const authHeader = 'X-Serverless-Authorization'; - const authToken = 'test-token'; - return new FakeAuthClient(authToken, authHeader); - } - - it('should return an empty object if authClient is undefined', async () => { - const client = new BaseHTTPClientWithAuth(); - const headers = await client.buildAuthHeader(); - expect(headers).toEqual({}); - }); - - it('should return the correct auth header if authClient is defined', async () => { - const authClient = createAuthClient(); - const client = new BaseHTTPClientWithAuth(authClient); - const headers = await client.buildAuthHeader(); - expect(headers).toEqual({ - 'X-Serverless-Authorization': 'Bearer test-token', - }); - }); - - it('should call getToken and getHeaderName on authClient', async () => { - const authClient = createAuthClient(); - vi.spyOn(authClient, 'getToken'); - vi.spyOn(authClient, 'getHeaderName'); - const client = new BaseHTTPClientWithAuth(authClient); - await client.buildAuthHeader(); - expect(authClient.getToken).toHaveBeenCalled(); - expect(authClient.getHeaderName).toHaveBeenCalled(); - }); - - it('should return the correct auth header if spotifyToken is provided', async () => { - const client = new BaseHTTPClientWithAuth(); - const spotifyToken = 'spotify-token'; - const headers = await client.buildAuthHeader(spotifyToken); - expect(headers).toEqual({ Authorization: 'Bearer spotify-token' }); - }); - - it('should return the correct auth header if authClient and spotifyToken are provided', async () => { - const authClient = createAuthClient(); - const client = new BaseHTTPClientWithAuth(authClient); - const spotifyToken = 'spotify-token'; - const headers = await client.buildAuthHeader(spotifyToken); - expect(headers).toEqual({ - Authorization: 'Bearer spotify-token', - 'X-Serverless-Authorization': 'Bearer test-token', - }); - }); -}); diff --git a/src/lib/clients/base-http-client.ts b/src/lib/clients/base-http-client.ts deleted file mode 100644 index 22ea506..0000000 --- a/src/lib/clients/base-http-client.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { AuthClient } from './auth'; - -interface BaseClientWithAuth { - buildAuthHeader: (_spotifyToken: string) => Promise>; -} - -export class BaseHTTPClientWithAuth implements BaseClientWithAuth { - private authClient?: AuthClient | undefined; - - constructor(authClient?: AuthClient) { - this.authClient = authClient; - } - - async buildAuthHeader( - spotifyToken?: string - ): Promise> { - const headers: Record = spotifyToken - ? { Authorization: `Bearer ${spotifyToken}` } - : {}; - - if (!this.authClient) return headers; - - return { - ...headers, - [this.authClient.getHeaderName()]: `Bearer ${await this.authClient.getToken()}`, - }; - } -} diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index 43e008a..719ab70 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -4,6 +4,7 @@ import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; import { ArtistsClient, ArtistsHTTPClient } from '@/lib/clients/artists'; import { getToken } from 'next-auth/jwt'; +import { HTTPAuthHeaderBuilder } from '@/lib/clients/auth'; export type SearchArtistHandlerParams = { client: ArtistsClient; @@ -97,9 +98,11 @@ const maxLimit: number = parseInt(process.env.SEARCH_ARTISTS_MAX_LIMIT || '10'); const serverHost: string = process.env.SERVER_HOST || 'http://localhost'; const serverPort: number = parseInt(process.env.SERVER_PORT || '8080'); const httpClient: HttpClient = new HttpBaseClient(); +const httpAuthHeaderBuilder = new HTTPAuthHeaderBuilder(); const client: ArtistsClient = new ArtistsHTTPClient( `${serverHost}:${serverPort}`, - httpClient + httpClient, + httpAuthHeaderBuilder ); const searchArtistHandler: ( _request: NextApiRequest, From 2593cec3c0d2b9069f987843c52889eb662376b0 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Wed, 12 Feb 2025 08:16:06 +0100 Subject: [PATCH 12/31] fix: rename auth client class --- src/lib/clients/auth.test.ts | 16 ++++++---------- src/lib/clients/auth.ts | 10 +++++----- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index f23e173..cf66c24 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -1,13 +1,9 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; -import { - FakeGCPAuthClient, - GCPHTTPAuthClient, - HTTPAuthHeaderBuilder, -} from './auth'; +import { FakeAuthClient, HTTPAuthClient, HTTPAuthHeaderBuilder } from './auth'; import { FakeHttpClient, HttpResponse, Method } from './http'; describe('AuthClient', () => { - describe('GCPHTTPAuthClient', () => { + describe('HTTPAuthClient', () => { const audience = 'test-audience'; let httpClient: FakeHttpClient; let response: HttpResponse; @@ -18,7 +14,7 @@ describe('AuthClient', () => { }); it('should call the client with the correct parameters', async () => { - const authClient = new GCPHTTPAuthClient(httpClient, audience); + const authClient = new HTTPAuthClient(httpClient, audience); const baseUrl = 'http://some_url/auth'; authClient.setBaseUrl(baseUrl); vi.spyOn(httpClient, 'send'); @@ -36,7 +32,7 @@ describe('AuthClient', () => { it('should return token from the client', async () => { const expected = 'some-token'; httpClient.setResult({ data: expected, status: 200 }); - const authClient = new GCPHTTPAuthClient(httpClient, audience); + const authClient = new HTTPAuthClient(httpClient, audience); const actual = await authClient.getToken(); @@ -46,7 +42,7 @@ describe('AuthClient', () => { it('should throw an error if the client fails', async () => { const errorMessage = 'Test error'; httpClient.setSendErrorMessage('Test error'); - const authClient = new GCPHTTPAuthClient(httpClient, audience); + const authClient = new HTTPAuthClient(httpClient, audience); await expect(authClient.getToken()).rejects.toThrow(errorMessage); }); @@ -56,7 +52,7 @@ describe('AuthClient', () => { function createAuthClient() { const authHeader = 'X-Serverless-Authorization'; const authToken = 'test-token'; - return new FakeGCPAuthClient(authToken, authHeader); + return new FakeAuthClient(authToken, authHeader); } it('should return an empty object if authClient is undefined', async () => { diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index 8627b9a..5028b72 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -1,11 +1,11 @@ import { HttpClient, Method } from './http'; -export interface GCPAuthClient { +export interface AuthClient { getToken(): Promise; getHeaderName(): string; } -export class GCPHTTPAuthClient { +export class HTTPAuthClient { private httpClient: HttpClient; private baseUrl: string = 'http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/identity'; @@ -36,7 +36,7 @@ export class GCPHTTPAuthClient { } } -export class FakeGCPAuthClient { +export class FakeAuthClient { private header: string; private token: string; private getTokenErrorMessage: string | undefined = undefined; @@ -71,9 +71,9 @@ export interface AuthHeaderBuilder { } export class HTTPAuthHeaderBuilder implements AuthHeaderBuilder { - private gcpAuthClient?: GCPAuthClient | undefined; + private gcpAuthClient?: AuthClient | undefined; - constructor(gcpAuthClient?: GCPAuthClient) { + constructor(gcpAuthClient?: AuthClient) { this.gcpAuthClient = gcpAuthClient; } From 818502ce696fb5b7c6acaad045f48a23f9371388 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Wed, 12 Feb 2025 08:22:05 +0100 Subject: [PATCH 13/31] refactor: rename HTTPAuthHeaderBuilder class --- src/lib/clients/artists.test.ts | 10 +++++----- src/lib/clients/artists.ts | 6 +++--- src/lib/clients/auth.test.ts | 20 ++++++++++++-------- src/lib/clients/auth.ts | 6 +++--- src/pages/api/artists/search.ts | 4 ++-- 5 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index bf2dfd1..4570ba7 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; -import { FakeHTTPAuthHeaderBuilder } from './auth'; +import { FakeBaseHTTPAuthHeaderBuilder } from './auth'; describe('ArtistsHTTPBackendClient', () => { let url: string; @@ -28,7 +28,7 @@ describe('ArtistsHTTPBackendClient', () => { }); it('should call the client with the correct parameters', async () => { - const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder(); const client = new ArtistsHTTPClient( url, httpClient, @@ -49,7 +49,7 @@ describe('ArtistsHTTPBackendClient', () => { it('should call the client with an additional auth header if auth client is provided', async () => { const authHeader = 'Some-Header'; const authToken = 'some-token'; - const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder( + const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder( authToken, authHeader ); @@ -74,7 +74,7 @@ describe('ArtistsHTTPBackendClient', () => { }); it('should return the list of artists returned by the HTTP client', async () => { - const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder(); const client = new ArtistsHTTPClient( url, httpClient, @@ -93,7 +93,7 @@ describe('ArtistsHTTPBackendClient', () => { it('should throw an error if the HTTP client fails', async () => { const errorMessage = 'Request failed'; httpClient.setSendErrorMessage(errorMessage); - const httpAuthHeaderBuilder = new FakeHTTPAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder(); const client = new ArtistsHTTPClient( url, httpClient, diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index c700037..07d5806 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -1,5 +1,5 @@ import { Artist } from '../artists'; -import { AuthHeaderBuilder, HTTPAuthHeaderBuilder } from './auth'; +import { HTTPAuthHeaderBuilder, BaseHTTPAuthHeaderBuilder } from './auth'; import { HttpClient, Method } from './http'; export interface ArtistsClient { @@ -13,12 +13,12 @@ export interface ArtistsClient { export class ArtistsHTTPClient implements ArtistsClient { private url: string; private httpClient: HttpClient; - private httpAuthHeaderBuilder: AuthHeaderBuilder; + private httpAuthHeaderBuilder: HTTPAuthHeaderBuilder; constructor( url: string, httpClient: HttpClient, - httpAuthHeaderBuilder: HTTPAuthHeaderBuilder + httpAuthHeaderBuilder: BaseHTTPAuthHeaderBuilder ) { this.url = url; this.httpClient = httpClient; diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index cf66c24..869fc1c 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -1,5 +1,9 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; -import { FakeAuthClient, HTTPAuthClient, HTTPAuthHeaderBuilder } from './auth'; +import { + FakeAuthClient, + HTTPAuthClient, + BaseHTTPAuthHeaderBuilder, +} from './auth'; import { FakeHttpClient, HttpResponse, Method } from './http'; describe('AuthClient', () => { @@ -48,7 +52,7 @@ describe('AuthClient', () => { }); }); - describe('HTTPAuthHeaderBuilder', () => { + describe('BaseHTTPAuthHeaderBuilder', () => { function createAuthClient() { const authHeader = 'X-Serverless-Authorization'; const authToken = 'test-token'; @@ -56,14 +60,14 @@ describe('AuthClient', () => { } it('should return an empty object if authClient is undefined', async () => { - const client = new HTTPAuthHeaderBuilder(); + const client = new BaseHTTPAuthHeaderBuilder(); const headers = await client.buildHeader(); expect(headers).toEqual({}); }); it('should return the correct auth header if authClient is defined', async () => { const authClient = createAuthClient(); - const client = new HTTPAuthHeaderBuilder(authClient); + const client = new BaseHTTPAuthHeaderBuilder(authClient); const headers = await client.buildHeader(); expect(headers).toEqual({ 'X-Serverless-Authorization': 'Bearer test-token', @@ -74,14 +78,14 @@ describe('AuthClient', () => { const authClient = createAuthClient(); vi.spyOn(authClient, 'getToken'); vi.spyOn(authClient, 'getHeaderName'); - const client = new HTTPAuthHeaderBuilder(authClient); + const client = new BaseHTTPAuthHeaderBuilder(authClient); await client.buildHeader(); expect(authClient.getToken).toHaveBeenCalled(); expect(authClient.getHeaderName).toHaveBeenCalled(); }); it('should return the correct auth header if spotifyToken is provided', async () => { - const client = new HTTPAuthHeaderBuilder(); + const client = new BaseHTTPAuthHeaderBuilder(); const spotifyToken = 'spotify-token'; const headers = await client.buildHeader(spotifyToken); expect(headers).toEqual({ Authorization: 'Bearer spotify-token' }); @@ -89,7 +93,7 @@ describe('AuthClient', () => { it('should return the correct auth header if authClient and spotifyToken are provided', async () => { const authClient = createAuthClient(); - const client = new HTTPAuthHeaderBuilder(authClient); + const client = new BaseHTTPAuthHeaderBuilder(authClient); const spotifyToken = 'spotify-token'; const headers = await client.buildHeader(spotifyToken); expect(headers).toEqual({ @@ -102,7 +106,7 @@ describe('AuthClient', () => { const errorMessage = 'Auth request failed'; const authClient = createAuthClient(); authClient.setGetTokenErrorMessage(errorMessage); - const client = new HTTPAuthHeaderBuilder(authClient); + const client = new BaseHTTPAuthHeaderBuilder(authClient); const spotifyToken = 'spotify-token'; await expect(client.buildHeader(spotifyToken)).rejects.toThrow( errorMessage diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index 5028b72..f7bfcf7 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -66,11 +66,11 @@ export class FakeAuthClient { } } -export interface AuthHeaderBuilder { +export interface HTTPAuthHeaderBuilder { buildHeader: (_spotifyToken: string) => Promise>; } -export class HTTPAuthHeaderBuilder implements AuthHeaderBuilder { +export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { private gcpAuthClient?: AuthClient | undefined; constructor(gcpAuthClient?: AuthClient) { @@ -91,7 +91,7 @@ export class HTTPAuthHeaderBuilder implements AuthHeaderBuilder { } } -export class FakeHTTPAuthHeaderBuilder implements AuthHeaderBuilder { +export class FakeBaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { private gcpAuthToken: string | undefined; private gcpAuthHeaderName: string | undefined; diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index 719ab70..f3d0ac1 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -4,7 +4,7 @@ import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; import { ArtistsClient, ArtistsHTTPClient } from '@/lib/clients/artists'; import { getToken } from 'next-auth/jwt'; -import { HTTPAuthHeaderBuilder } from '@/lib/clients/auth'; +import { BaseHTTPAuthHeaderBuilder } from '@/lib/clients/auth'; export type SearchArtistHandlerParams = { client: ArtistsClient; @@ -98,7 +98,7 @@ const maxLimit: number = parseInt(process.env.SEARCH_ARTISTS_MAX_LIMIT || '10'); const serverHost: string = process.env.SERVER_HOST || 'http://localhost'; const serverPort: number = parseInt(process.env.SERVER_PORT || '8080'); const httpClient: HttpClient = new HttpBaseClient(); -const httpAuthHeaderBuilder = new HTTPAuthHeaderBuilder(); +const httpAuthHeaderBuilder = new BaseHTTPAuthHeaderBuilder(); const client: ArtistsClient = new ArtistsHTTPClient( `${serverHost}:${serverPort}`, httpClient, From 599ba0a0c2a4ecb2a96482bb2b1c78355c5f27cf Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Wed, 12 Feb 2025 08:23:51 +0100 Subject: [PATCH 14/31] refactor: rename spotifyToken --- src/lib/clients/auth.test.ts | 18 ++++++++---------- src/lib/clients/auth.ts | 14 +++++++------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index 869fc1c..c6ef4ab 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -84,18 +84,18 @@ describe('AuthClient', () => { expect(authClient.getHeaderName).toHaveBeenCalled(); }); - it('should return the correct auth header if spotifyToken is provided', async () => { + it('should return the correct auth header if token is provided', async () => { const client = new BaseHTTPAuthHeaderBuilder(); - const spotifyToken = 'spotify-token'; - const headers = await client.buildHeader(spotifyToken); + const token = 'spotify-token'; + const headers = await client.buildHeader(token); expect(headers).toEqual({ Authorization: 'Bearer spotify-token' }); }); - it('should return the correct auth header if authClient and spotifyToken are provided', async () => { + it('should return the correct auth header if authClient and token are provided', async () => { const authClient = createAuthClient(); const client = new BaseHTTPAuthHeaderBuilder(authClient); - const spotifyToken = 'spotify-token'; - const headers = await client.buildHeader(spotifyToken); + const token = 'spotify-token'; + const headers = await client.buildHeader(token); expect(headers).toEqual({ Authorization: 'Bearer spotify-token', 'X-Serverless-Authorization': 'Bearer test-token', @@ -107,10 +107,8 @@ describe('AuthClient', () => { const authClient = createAuthClient(); authClient.setGetTokenErrorMessage(errorMessage); const client = new BaseHTTPAuthHeaderBuilder(authClient); - const spotifyToken = 'spotify-token'; - await expect(client.buildHeader(spotifyToken)).rejects.toThrow( - errorMessage - ); + const token = 'spotify-token'; + await expect(client.buildHeader(token)).rejects.toThrow(errorMessage); }); }); }); diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index f7bfcf7..e644355 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -67,7 +67,7 @@ export class FakeAuthClient { } export interface HTTPAuthHeaderBuilder { - buildHeader: (_spotifyToken: string) => Promise>; + buildHeader: (_token: string) => Promise>; } export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { @@ -77,9 +77,9 @@ export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { this.gcpAuthClient = gcpAuthClient; } - async buildHeader(spotifyToken?: string): Promise> { - const headers: Record = spotifyToken - ? { Authorization: `Bearer ${spotifyToken}` } + async buildHeader(token?: string): Promise> { + const headers: Record = token + ? { Authorization: `Bearer ${token}` } : {}; if (!this.gcpAuthClient) return headers; @@ -100,9 +100,9 @@ export class FakeBaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { this.gcpAuthHeaderName = gcpAuthHeaderName; } - async buildHeader(spotifyToken: string): Promise> { - const headers: Record = spotifyToken - ? { Authorization: `Bearer ${spotifyToken}` } + async buildHeader(token: string): Promise> { + const headers: Record = token + ? { Authorization: `Bearer ${token}` } : {}; if (!this.gcpAuthToken || !this.gcpAuthHeaderName) return headers; From cc24fe11583dd4d39c3606c93ecfedb5ed27a55a Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Wed, 12 Feb 2025 08:28:32 +0100 Subject: [PATCH 15/31] refactor: put app token as required in buildHeader method params --- src/lib/clients/auth.test.ts | 28 +++++++--------------------- src/lib/clients/auth.ts | 8 ++++---- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index c6ef4ab..7bdd7ac 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -59,45 +59,31 @@ describe('AuthClient', () => { return new FakeAuthClient(authToken, authHeader); } - it('should return an empty object if authClient is undefined', async () => { - const client = new BaseHTTPAuthHeaderBuilder(); - const headers = await client.buildHeader(); - expect(headers).toEqual({}); - }); - - it('should return the correct auth header if authClient is defined', async () => { - const authClient = createAuthClient(); - const client = new BaseHTTPAuthHeaderBuilder(authClient); - const headers = await client.buildHeader(); - expect(headers).toEqual({ - 'X-Serverless-Authorization': 'Bearer test-token', - }); - }); - it('should call getToken and getHeaderName on authClient', async () => { const authClient = createAuthClient(); vi.spyOn(authClient, 'getToken'); vi.spyOn(authClient, 'getHeaderName'); const client = new BaseHTTPAuthHeaderBuilder(authClient); - await client.buildHeader(); + const token = 'app-token'; + await client.buildHeader(token); expect(authClient.getToken).toHaveBeenCalled(); expect(authClient.getHeaderName).toHaveBeenCalled(); }); it('should return the correct auth header if token is provided', async () => { const client = new BaseHTTPAuthHeaderBuilder(); - const token = 'spotify-token'; + const token = 'app-token'; const headers = await client.buildHeader(token); - expect(headers).toEqual({ Authorization: 'Bearer spotify-token' }); + expect(headers).toEqual({ Authorization: 'Bearer app-token' }); }); it('should return the correct auth header if authClient and token are provided', async () => { const authClient = createAuthClient(); const client = new BaseHTTPAuthHeaderBuilder(authClient); - const token = 'spotify-token'; + const token = 'app-token'; const headers = await client.buildHeader(token); expect(headers).toEqual({ - Authorization: 'Bearer spotify-token', + Authorization: 'Bearer app-token', 'X-Serverless-Authorization': 'Bearer test-token', }); }); @@ -107,7 +93,7 @@ describe('AuthClient', () => { const authClient = createAuthClient(); authClient.setGetTokenErrorMessage(errorMessage); const client = new BaseHTTPAuthHeaderBuilder(authClient); - const token = 'spotify-token'; + const token = 'app-token'; await expect(client.buildHeader(token)).rejects.toThrow(errorMessage); }); }); diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index e644355..4a9a6ac 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -77,10 +77,10 @@ export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { this.gcpAuthClient = gcpAuthClient; } - async buildHeader(token?: string): Promise> { - const headers: Record = token - ? { Authorization: `Bearer ${token}` } - : {}; + async buildHeader(token: string): Promise> { + const headers: Record = { + Authorization: `Bearer ${token}`, + }; if (!this.gcpAuthClient) return headers; From ab14005fa9b738058a845d0454e4ca6c7eaba22d Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Fri, 14 Feb 2025 08:04:07 +0100 Subject: [PATCH 16/31] refactor(tests): add spaces between given/when/then --- src/lib/clients/auth.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index 7bdd7ac..c4b59ec 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -64,24 +64,30 @@ describe('AuthClient', () => { vi.spyOn(authClient, 'getToken'); vi.spyOn(authClient, 'getHeaderName'); const client = new BaseHTTPAuthHeaderBuilder(authClient); + const token = 'app-token'; await client.buildHeader(token); + expect(authClient.getToken).toHaveBeenCalled(); expect(authClient.getHeaderName).toHaveBeenCalled(); }); it('should return the correct auth header if token is provided', async () => { const client = new BaseHTTPAuthHeaderBuilder(); + const token = 'app-token'; const headers = await client.buildHeader(token); + expect(headers).toEqual({ Authorization: 'Bearer app-token' }); }); it('should return the correct auth header if authClient and token are provided', async () => { const authClient = createAuthClient(); const client = new BaseHTTPAuthHeaderBuilder(authClient); + const token = 'app-token'; const headers = await client.buildHeader(token); + expect(headers).toEqual({ Authorization: 'Bearer app-token', 'X-Serverless-Authorization': 'Bearer test-token', @@ -93,6 +99,7 @@ describe('AuthClient', () => { const authClient = createAuthClient(); authClient.setGetTokenErrorMessage(errorMessage); const client = new BaseHTTPAuthHeaderBuilder(authClient); + const token = 'app-token'; await expect(client.buildHeader(token)).rejects.toThrow(errorMessage); }); From 6720455704c335b0486803c1d9bf0945ed3ca2d2 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Fri, 14 Feb 2025 08:13:36 +0100 Subject: [PATCH 17/31] test: add new test to check when the token is not provided properly --- src/pages/api/artists/search.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/pages/api/artists/search.test.ts b/src/pages/api/artists/search.test.ts index dafb469..c322887 100644 --- a/src/pages/api/artists/search.test.ts +++ b/src/pages/api/artists/search.test.ts @@ -144,4 +144,17 @@ describe('searchArtistHandler', () => { message: 'Unexpected error, cannot retrieve artists', }); }); + + it('should return 401 if token is not provided', async () => { + vi.mocked(getToken).mockResolvedValue(null); + const response = createMockResponse(); + + const handler = createHandler(); + await handler(createMockRequest(), response); + + expect(response.status).toBeCalledWith(401); + expect(response.json).toBeCalledWith({ + message: 'Unauthorized', + }); + }); }); From 423e39ebf079e3972a1b220e74b2c6ce3376b884 Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Sat, 15 Feb 2025 09:12:51 +0100 Subject: [PATCH 18/31] fix: unify describe with the class name --- src/lib/clients/artists.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index 4570ba7..4071ebb 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -4,7 +4,7 @@ import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; import { FakeBaseHTTPAuthHeaderBuilder } from './auth'; -describe('ArtistsHTTPBackendClient', () => { +describe('ArtistsHTTPClient', () => { let url: string; let token: string; let name: string; From cbd4f47b0fad1440ac0b5287669a1cbd1a32004e Mon Sep 17 00:00:00 2001 From: "alex.barro" Date: Sat, 15 Feb 2025 09:18:17 +0100 Subject: [PATCH 19/31] refactor: rename class --- src/lib/clients/auth.test.ts | 10 +++++----- src/lib/clients/auth.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index c4b59ec..c162fda 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -1,13 +1,13 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { FakeAuthClient, - HTTPAuthClient, + GCPAuthClient, BaseHTTPAuthHeaderBuilder, } from './auth'; import { FakeHttpClient, HttpResponse, Method } from './http'; describe('AuthClient', () => { - describe('HTTPAuthClient', () => { + describe('GCPAuthClient', () => { const audience = 'test-audience'; let httpClient: FakeHttpClient; let response: HttpResponse; @@ -18,7 +18,7 @@ describe('AuthClient', () => { }); it('should call the client with the correct parameters', async () => { - const authClient = new HTTPAuthClient(httpClient, audience); + const authClient = new GCPAuthClient(httpClient, audience); const baseUrl = 'http://some_url/auth'; authClient.setBaseUrl(baseUrl); vi.spyOn(httpClient, 'send'); @@ -36,7 +36,7 @@ describe('AuthClient', () => { it('should return token from the client', async () => { const expected = 'some-token'; httpClient.setResult({ data: expected, status: 200 }); - const authClient = new HTTPAuthClient(httpClient, audience); + const authClient = new GCPAuthClient(httpClient, audience); const actual = await authClient.getToken(); @@ -46,7 +46,7 @@ describe('AuthClient', () => { it('should throw an error if the client fails', async () => { const errorMessage = 'Test error'; httpClient.setSendErrorMessage('Test error'); - const authClient = new HTTPAuthClient(httpClient, audience); + const authClient = new GCPAuthClient(httpClient, audience); await expect(authClient.getToken()).rejects.toThrow(errorMessage); }); diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index 4a9a6ac..4acbce5 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -5,7 +5,7 @@ export interface AuthClient { getHeaderName(): string; } -export class HTTPAuthClient { +export class GCPAuthClient { private httpClient: HttpClient; private baseUrl: string = 'http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/identity'; From cff8fe2da6c0a91cb1d7a83fa02214621d43d24f Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 15:27:35 +0100 Subject: [PATCH 20/31] refactor: do not duplicate behavior in fake Make the mock class just be a stub. Moreover, I removed a couple of tests that were actually checking the test behavior, not the production class behavior. --- src/lib/clients/artists.test.ts | 38 ++++++--------------------------- src/lib/clients/auth.ts | 25 ++++++++-------------- 2 files changed, 15 insertions(+), 48 deletions(-) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index 4071ebb..2616a9d 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; -import { FakeBaseHTTPAuthHeaderBuilder } from './auth'; +import { FakeAuthHeaderBuilder } from './auth'; describe('ArtistsHTTPClient', () => { let url: string; @@ -28,7 +28,8 @@ describe('ArtistsHTTPClient', () => { }); it('should call the client with the correct parameters', async () => { - const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder(); + const headers = { something: 'value' }; + const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(headers); const client = new ArtistsHTTPClient( url, httpClient, @@ -42,39 +43,12 @@ describe('ArtistsHTTPClient', () => { url: `${url}/artists/search`, method: Method.Get, params: { name: name, limit: limit }, - headers: { Authorization: `Bearer ${token}` }, - }); - }); - - it('should call the client with an additional auth header if auth client is provided', async () => { - const authHeader = 'Some-Header'; - const authToken = 'some-token'; - const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder( - authToken, - authHeader - ); - const client = new ArtistsHTTPClient( - url, - httpClient, - httpAuthHeaderBuilder - ); - vi.spyOn(httpClient, 'send'); - - await client.searchArtists(token, name, limit); - - expect(httpClient.send).toHaveBeenCalledWith({ - url: `${url}/artists/search`, - method: Method.Get, - params: { name: name, limit: limit }, - headers: { - Authorization: `Bearer ${token}`, - [authHeader]: `Bearer ${authToken}`, - }, + headers: headers, }); }); it('should return the list of artists returned by the HTTP client', async () => { - const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(); const client = new ArtistsHTTPClient( url, httpClient, @@ -93,7 +67,7 @@ describe('ArtistsHTTPClient', () => { it('should throw an error if the HTTP client fails', async () => { const errorMessage = 'Request failed'; httpClient.setSendErrorMessage(errorMessage); - const httpAuthHeaderBuilder = new FakeBaseHTTPAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(); const client = new ArtistsHTTPClient( url, httpClient, diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index 4acbce5..878b482 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -91,25 +91,18 @@ export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { } } -export class FakeBaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { - private gcpAuthToken: string | undefined; - private gcpAuthHeaderName: string | undefined; +export class FakeAuthHeaderBuilder implements HTTPAuthHeaderBuilder { + private headers: Record; - constructor(gcpAuthToken?: string, gcpAuthHeaderName?: string) { - this.gcpAuthToken = gcpAuthToken; - this.gcpAuthHeaderName = gcpAuthHeaderName; + constructor(headers: Record = {}) { + this.headers = headers; } - async buildHeader(token: string): Promise> { - const headers: Record = token - ? { Authorization: `Bearer ${token}` } - : {}; - - if (!this.gcpAuthToken || !this.gcpAuthHeaderName) return headers; + setHeaders(headers: Record) { + this.headers = headers; + } - return { - ...headers, - [this.gcpAuthHeaderName]: `Bearer ${this.gcpAuthToken}`, - }; + async buildHeader(_token: string): Promise> { + return this.headers; } } From 283d41d8159a4f695f4c4f722e00ac22408e4846 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 15:29:52 +0100 Subject: [PATCH 21/31] chore(test): test artists client when header fails --- src/lib/clients/artists.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index 2616a9d..6f15186 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -78,4 +78,18 @@ describe('ArtistsHTTPClient', () => { errorMessage ); }); + + it('should throw an error if the header builder fails', async () => { + const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(); + vi.spyOn(httpAuthHeaderBuilder, 'buildHeader').mockImplementation(() => { + throw new Error('test Error'); + }); + const client = new ArtistsHTTPClient( + url, + httpClient, + httpAuthHeaderBuilder + ); + + await expect(client.searchArtists(token, name, limit)).rejects.toThrow(); + }); }); From 9fcfcab89c5fe490ee0701f19767c64507ba6c2a Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 15:32:59 +0100 Subject: [PATCH 22/31] refactor: use mock for raising errors Removing code that manually raises the error to prevent adding a lot of complexity in tests. --- src/lib/clients/artists.ts | 8 -------- src/pages/api/artists/search.test.ts | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index 07d5806..6ae4bb7 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -47,7 +47,6 @@ export class ArtistsHTTPClient implements ArtistsClient { } export class FakeArtistsHTTPClient implements ArtistsClient { - private searchArtistError: Error | undefined = undefined; private searchArtistResult: Artist[]; constructor(result: Artist[] = []) { @@ -58,14 +57,7 @@ export class FakeArtistsHTTPClient implements ArtistsClient { this.searchArtistResult = result; } - setSearchArtistError(error: Error) { - this.searchArtistError = error; - } - async searchArtists(..._: any[]): Promise { - if (this.searchArtistError !== undefined) { - throw this.searchArtistError; - } return this.searchArtistResult; } } diff --git a/src/pages/api/artists/search.test.ts b/src/pages/api/artists/search.test.ts index c322887..bf85136 100644 --- a/src/pages/api/artists/search.test.ts +++ b/src/pages/api/artists/search.test.ts @@ -129,7 +129,7 @@ describe('searchArtistHandler', () => { it('should return an error if search fails', async () => { const client = new FakeArtistsHTTPClient(); - client.setSearchArtistError(new Error('test error')); + vi.spyOn(client, 'searchArtists').mockImplementation(() => { throw new Error('test error') }); const response = createMockResponse(); const handler = createHandler({ From 46f9e4451c9dd739f80bcd9a13005fa3f4849c85 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 15:36:12 +0100 Subject: [PATCH 23/31] chore: rename stub classes for consistency Thoses classes are now simply returning a pre-defined object, so stub is more convenient. We are also removing any redundant information in the class name to avoid confusion (e.g. HTTP). --- src/lib/clients/artists.ts | 2 +- src/pages/api/artists/search.test.ts | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index 6ae4bb7..473836d 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -46,7 +46,7 @@ export class ArtistsHTTPClient implements ArtistsClient { } } -export class FakeArtistsHTTPClient implements ArtistsClient { +export class ArtistsClientStub implements ArtistsClient { private searchArtistResult: Artist[]; constructor(result: Artist[] = []) { diff --git a/src/pages/api/artists/search.test.ts b/src/pages/api/artists/search.test.ts index bf85136..2392cec 100644 --- a/src/pages/api/artists/search.test.ts +++ b/src/pages/api/artists/search.test.ts @@ -1,7 +1,7 @@ import { describe, it, vi, expect, beforeEach } from 'vitest'; import { NextApiRequest, NextApiResponse } from 'next'; import { createSearchArtistHandler, SearchArtistHandlerParams } from './search'; -import { FakeArtistsHTTPClient } from '@/lib/clients/artists'; +import { ArtistsClientStub } from '@/lib/clients/artists'; import { Artist } from '@/lib/artists'; import { getToken } from 'next-auth/jwt'; @@ -28,7 +28,7 @@ describe('searchArtistHandler', () => { function createHandler( { client, defaultLimit, maxLimit }: SearchArtistHandlerParams = { - client: new FakeArtistsHTTPClient(), + client: new ArtistsClientStub(), defaultLimit: 5, maxLimit: 10, } @@ -81,7 +81,7 @@ describe('searchArtistHandler', () => { name: 'name', }; const request = createMockRequest({ ...args, limit: limit }); - const client = new FakeArtistsHTTPClient(); + const client = new ArtistsClientStub(); vi.spyOn(client, 'searchArtists'); const defaultLimit = 5; @@ -105,7 +105,7 @@ describe('searchArtistHandler', () => { new Artist('Brutus'), new Artist('Brutus Daughters', 'http://some_url'), ]; - const client = new FakeArtistsHTTPClient(retrievedArtist); + const client = new ArtistsClientStub(retrievedArtist); const response = createMockResponse(); const handler = createHandler({ @@ -128,8 +128,10 @@ describe('searchArtistHandler', () => { }); it('should return an error if search fails', async () => { - const client = new FakeArtistsHTTPClient(); - vi.spyOn(client, 'searchArtists').mockImplementation(() => { throw new Error('test error') }); + const client = new ArtistsClientStub(); + vi.spyOn(client, 'searchArtists').mockImplementation(() => { + throw new Error('test error'); + }); const response = createMockResponse(); const handler = createHandler({ From c289b7c59464f89577215c44f4ae672ca281e02a Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 15:42:23 +0100 Subject: [PATCH 24/31] chore: convert fake into stub Since we are only using in a reduced part of the codebase, I would rather keep it as a stub than a more comples fake. --- src/lib/clients/artists.test.ts | 10 +++++----- src/lib/clients/auth.test.ts | 8 +++++--- src/lib/clients/auth.ts | 12 ++---------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index 6f15186..d427046 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; -import { FakeAuthHeaderBuilder } from './auth'; +import { AuthHeaderBuilderStub } from './auth'; describe('ArtistsHTTPClient', () => { let url: string; @@ -29,7 +29,7 @@ describe('ArtistsHTTPClient', () => { it('should call the client with the correct parameters', async () => { const headers = { something: 'value' }; - const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(headers); + const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(headers); const client = new ArtistsHTTPClient( url, httpClient, @@ -48,7 +48,7 @@ describe('ArtistsHTTPClient', () => { }); it('should return the list of artists returned by the HTTP client', async () => { - const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(); const client = new ArtistsHTTPClient( url, httpClient, @@ -67,7 +67,7 @@ describe('ArtistsHTTPClient', () => { it('should throw an error if the HTTP client fails', async () => { const errorMessage = 'Request failed'; httpClient.setSendErrorMessage(errorMessage); - const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(); const client = new ArtistsHTTPClient( url, httpClient, @@ -80,7 +80,7 @@ describe('ArtistsHTTPClient', () => { }); it('should throw an error if the header builder fails', async () => { - const httpAuthHeaderBuilder = new FakeAuthHeaderBuilder(); + const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(); vi.spyOn(httpAuthHeaderBuilder, 'buildHeader').mockImplementation(() => { throw new Error('test Error'); }); diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index c162fda..7b2b7e0 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { - FakeAuthClient, + AuthClientStub, GCPAuthClient, BaseHTTPAuthHeaderBuilder, } from './auth'; @@ -56,7 +56,7 @@ describe('AuthClient', () => { function createAuthClient() { const authHeader = 'X-Serverless-Authorization'; const authToken = 'test-token'; - return new FakeAuthClient(authToken, authHeader); + return new AuthClientStub(authToken, authHeader); } it('should call getToken and getHeaderName on authClient', async () => { @@ -97,7 +97,9 @@ describe('AuthClient', () => { it('should throw an error if the auth client fails', async () => { const errorMessage = 'Auth request failed'; const authClient = createAuthClient(); - authClient.setGetTokenErrorMessage(errorMessage); + vi.spyOn(authClient, 'getToken').mockImplementation(() => { + throw new Error(errorMessage); + }); const client = new BaseHTTPAuthHeaderBuilder(authClient); const token = 'app-token'; diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index 878b482..f333671 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -36,10 +36,9 @@ export class GCPAuthClient { } } -export class FakeAuthClient { +export class AuthClientStub implements AuthClient { private header: string; private token: string; - private getTokenErrorMessage: string | undefined = undefined; constructor(token: string, header: string) { this.token = token; @@ -50,14 +49,7 @@ export class FakeAuthClient { this.token = token; } - setGetTokenErrorMessage(message: string) { - this.getTokenErrorMessage = message; - } - async getToken(): Promise { - if (this.getTokenErrorMessage !== undefined) { - throw new Error(this.getTokenErrorMessage); - } return this.token; } @@ -91,7 +83,7 @@ export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { } } -export class FakeAuthHeaderBuilder implements HTTPAuthHeaderBuilder { +export class AuthHeaderBuilderStub implements HTTPAuthHeaderBuilder { private headers: Record; constructor(headers: Record = {}) { From 627eeef9ae1da6259afd85527f80a9bdb33b76bb Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 16:00:23 +0100 Subject: [PATCH 25/31] chore: simplify tests by reusing variable --- src/lib/clients/artists.test.ts | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index d427046..e999ba0 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; -import { AuthHeaderBuilderStub } from './auth'; +import { AuthHeaderBuilderStub, HTTPAuthHeaderBuilder } from './auth'; describe('ArtistsHTTPClient', () => { let url: string; @@ -11,6 +11,7 @@ describe('ArtistsHTTPClient', () => { let limit: number; let httpClient: FakeHttpClient; let response: HttpResponse; + let authHeaderBuilder: HTTPAuthHeaderBuilder; beforeEach(() => { url = 'http://some_url'; @@ -25,15 +26,15 @@ describe('ArtistsHTTPClient', () => { status: 200, }; httpClient = new FakeHttpClient(response); + authHeaderBuilder = new AuthHeaderBuilderStub(); }); it('should call the client with the correct parameters', async () => { const headers = { something: 'value' }; - const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(headers); const client = new ArtistsHTTPClient( url, httpClient, - httpAuthHeaderBuilder + new AuthHeaderBuilderStub(headers) ); vi.spyOn(httpClient, 'send'); @@ -48,12 +49,7 @@ describe('ArtistsHTTPClient', () => { }); it('should return the list of artists returned by the HTTP client', async () => { - const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(); - const client = new ArtistsHTTPClient( - url, - httpClient, - httpAuthHeaderBuilder - ); + const client = new ArtistsHTTPClient(url, httpClient, authHeaderBuilder); const actual = await client.searchArtists(token, name, limit); @@ -67,12 +63,7 @@ describe('ArtistsHTTPClient', () => { it('should throw an error if the HTTP client fails', async () => { const errorMessage = 'Request failed'; httpClient.setSendErrorMessage(errorMessage); - const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(); - const client = new ArtistsHTTPClient( - url, - httpClient, - httpAuthHeaderBuilder - ); + const client = new ArtistsHTTPClient(url, httpClient, authHeaderBuilder); await expect(client.searchArtists(token, name, limit)).rejects.toThrow( errorMessage @@ -80,15 +71,10 @@ describe('ArtistsHTTPClient', () => { }); it('should throw an error if the header builder fails', async () => { - const httpAuthHeaderBuilder = new AuthHeaderBuilderStub(); - vi.spyOn(httpAuthHeaderBuilder, 'buildHeader').mockImplementation(() => { + vi.spyOn(authHeaderBuilder, 'buildHeader').mockImplementation(() => { throw new Error('test Error'); }); - const client = new ArtistsHTTPClient( - url, - httpClient, - httpAuthHeaderBuilder - ); + const client = new ArtistsHTTPClient(url, httpClient, authHeaderBuilder); await expect(client.searchArtists(token, name, limit)).rejects.toThrow(); }); From 05c9be9e677cff5c66d10842b983fd127f6ccfc8 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 16:02:01 +0100 Subject: [PATCH 26/31] chore(auth): remove http prefix from header builder Headers can also belong to other protocops outside HTTP (e.g. TCP). --- src/lib/clients/artists.test.ts | 4 ++-- src/lib/clients/artists.ts | 6 +++--- src/lib/clients/auth.test.ts | 14 +++++--------- src/lib/clients/auth.ts | 6 +++--- src/pages/api/artists/search.ts | 4 ++-- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/lib/clients/artists.test.ts b/src/lib/clients/artists.test.ts index e999ba0..757f778 100644 --- a/src/lib/clients/artists.test.ts +++ b/src/lib/clients/artists.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { Artist } from '@/lib/artists'; import { ArtistsHTTPClient } from './artists'; import { FakeHttpClient, HttpResponse, Method } from './http'; -import { AuthHeaderBuilderStub, HTTPAuthHeaderBuilder } from './auth'; +import { AuthHeaderBuilderStub, AuthHeaderBuilder } from './auth'; describe('ArtistsHTTPClient', () => { let url: string; @@ -11,7 +11,7 @@ describe('ArtistsHTTPClient', () => { let limit: number; let httpClient: FakeHttpClient; let response: HttpResponse; - let authHeaderBuilder: HTTPAuthHeaderBuilder; + let authHeaderBuilder: AuthHeaderBuilder; beforeEach(() => { url = 'http://some_url'; diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index 473836d..ac45504 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -1,5 +1,5 @@ import { Artist } from '../artists'; -import { HTTPAuthHeaderBuilder, BaseHTTPAuthHeaderBuilder } from './auth'; +import { AuthHeaderBuilder, BaseAuthHeaderBuilder } from './auth'; import { HttpClient, Method } from './http'; export interface ArtistsClient { @@ -13,12 +13,12 @@ export interface ArtistsClient { export class ArtistsHTTPClient implements ArtistsClient { private url: string; private httpClient: HttpClient; - private httpAuthHeaderBuilder: HTTPAuthHeaderBuilder; + private httpAuthHeaderBuilder: AuthHeaderBuilder; constructor( url: string, httpClient: HttpClient, - httpAuthHeaderBuilder: BaseHTTPAuthHeaderBuilder + httpAuthHeaderBuilder: BaseAuthHeaderBuilder ) { this.url = url; this.httpClient = httpClient; diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index 7b2b7e0..c28e445 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -1,9 +1,5 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; -import { - AuthClientStub, - GCPAuthClient, - BaseHTTPAuthHeaderBuilder, -} from './auth'; +import { AuthClientStub, GCPAuthClient, BaseAuthHeaderBuilder } from './auth'; import { FakeHttpClient, HttpResponse, Method } from './http'; describe('AuthClient', () => { @@ -63,7 +59,7 @@ describe('AuthClient', () => { const authClient = createAuthClient(); vi.spyOn(authClient, 'getToken'); vi.spyOn(authClient, 'getHeaderName'); - const client = new BaseHTTPAuthHeaderBuilder(authClient); + const client = new BaseAuthHeaderBuilder(authClient); const token = 'app-token'; await client.buildHeader(token); @@ -73,7 +69,7 @@ describe('AuthClient', () => { }); it('should return the correct auth header if token is provided', async () => { - const client = new BaseHTTPAuthHeaderBuilder(); + const client = new BaseAuthHeaderBuilder(); const token = 'app-token'; const headers = await client.buildHeader(token); @@ -83,7 +79,7 @@ describe('AuthClient', () => { it('should return the correct auth header if authClient and token are provided', async () => { const authClient = createAuthClient(); - const client = new BaseHTTPAuthHeaderBuilder(authClient); + const client = new BaseAuthHeaderBuilder(authClient); const token = 'app-token'; const headers = await client.buildHeader(token); @@ -100,7 +96,7 @@ describe('AuthClient', () => { vi.spyOn(authClient, 'getToken').mockImplementation(() => { throw new Error(errorMessage); }); - const client = new BaseHTTPAuthHeaderBuilder(authClient); + const client = new BaseAuthHeaderBuilder(authClient); const token = 'app-token'; await expect(client.buildHeader(token)).rejects.toThrow(errorMessage); diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index f333671..02cdbb0 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -58,11 +58,11 @@ export class AuthClientStub implements AuthClient { } } -export interface HTTPAuthHeaderBuilder { +export interface AuthHeaderBuilder { buildHeader: (_token: string) => Promise>; } -export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { +export class BaseAuthHeaderBuilder implements AuthHeaderBuilder { private gcpAuthClient?: AuthClient | undefined; constructor(gcpAuthClient?: AuthClient) { @@ -83,7 +83,7 @@ export class BaseHTTPAuthHeaderBuilder implements HTTPAuthHeaderBuilder { } } -export class AuthHeaderBuilderStub implements HTTPAuthHeaderBuilder { +export class AuthHeaderBuilderStub implements AuthHeaderBuilder { private headers: Record; constructor(headers: Record = {}) { diff --git a/src/pages/api/artists/search.ts b/src/pages/api/artists/search.ts index f3d0ac1..e38b4d9 100644 --- a/src/pages/api/artists/search.ts +++ b/src/pages/api/artists/search.ts @@ -4,7 +4,7 @@ import { HttpClient, HttpBaseClient } from '@/lib/clients/http'; import { Artist } from '@/lib/artists'; import { ArtistsClient, ArtistsHTTPClient } from '@/lib/clients/artists'; import { getToken } from 'next-auth/jwt'; -import { BaseHTTPAuthHeaderBuilder } from '@/lib/clients/auth'; +import { BaseAuthHeaderBuilder } from '@/lib/clients/auth'; export type SearchArtistHandlerParams = { client: ArtistsClient; @@ -98,7 +98,7 @@ const maxLimit: number = parseInt(process.env.SEARCH_ARTISTS_MAX_LIMIT || '10'); const serverHost: string = process.env.SERVER_HOST || 'http://localhost'; const serverPort: number = parseInt(process.env.SERVER_PORT || '8080'); const httpClient: HttpClient = new HttpBaseClient(); -const httpAuthHeaderBuilder = new BaseHTTPAuthHeaderBuilder(); +const httpAuthHeaderBuilder = new BaseAuthHeaderBuilder(); const client: ArtistsClient = new ArtistsHTTPClient( `${serverHost}:${serverPort}`, httpClient, From 75951aadb18fe48c91be31566c8608373c6485fd Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 16:04:14 +0100 Subject: [PATCH 27/31] chore(tests): remove unnecessary indentation --- src/lib/clients/auth.test.ts | 150 +++++++++++++++++------------------ 1 file changed, 74 insertions(+), 76 deletions(-) diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index c28e445..8de440c 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -2,104 +2,102 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'; import { AuthClientStub, GCPAuthClient, BaseAuthHeaderBuilder } from './auth'; import { FakeHttpClient, HttpResponse, Method } from './http'; -describe('AuthClient', () => { - describe('GCPAuthClient', () => { - const audience = 'test-audience'; - let httpClient: FakeHttpClient; - let response: HttpResponse; - - beforeEach(() => { - response = { data: 'test-token', status: 200 }; - httpClient = new FakeHttpClient(response); - }); +describe('GCPAuthClient', () => { + const audience = 'test-audience'; + let httpClient: FakeHttpClient; + let response: HttpResponse; + + beforeEach(() => { + response = { data: 'test-token', status: 200 }; + httpClient = new FakeHttpClient(response); + }); - it('should call the client with the correct parameters', async () => { - const authClient = new GCPAuthClient(httpClient, audience); - const baseUrl = 'http://some_url/auth'; - authClient.setBaseUrl(baseUrl); - vi.spyOn(httpClient, 'send'); + it('should call the client with the correct parameters', async () => { + const authClient = new GCPAuthClient(httpClient, audience); + const baseUrl = 'http://some_url/auth'; + authClient.setBaseUrl(baseUrl); + vi.spyOn(httpClient, 'send'); - await authClient.getToken(); + await authClient.getToken(); - expect(httpClient.send).toHaveBeenCalledWith({ - url: baseUrl, - method: Method.Get, - params: { audience: audience }, - headers: { 'Metadata-Flavor': 'Google' }, - }); + expect(httpClient.send).toHaveBeenCalledWith({ + url: baseUrl, + method: Method.Get, + params: { audience: audience }, + headers: { 'Metadata-Flavor': 'Google' }, }); + }); - it('should return token from the client', async () => { - const expected = 'some-token'; - httpClient.setResult({ data: expected, status: 200 }); - const authClient = new GCPAuthClient(httpClient, audience); + it('should return token from the client', async () => { + const expected = 'some-token'; + httpClient.setResult({ data: expected, status: 200 }); + const authClient = new GCPAuthClient(httpClient, audience); - const actual = await authClient.getToken(); + const actual = await authClient.getToken(); - expect(actual).toBe(expected); - }); + expect(actual).toBe(expected); + }); - it('should throw an error if the client fails', async () => { - const errorMessage = 'Test error'; - httpClient.setSendErrorMessage('Test error'); - const authClient = new GCPAuthClient(httpClient, audience); + it('should throw an error if the client fails', async () => { + const errorMessage = 'Test error'; + httpClient.setSendErrorMessage('Test error'); + const authClient = new GCPAuthClient(httpClient, audience); - await expect(authClient.getToken()).rejects.toThrow(errorMessage); - }); + await expect(authClient.getToken()).rejects.toThrow(errorMessage); }); +}); - describe('BaseHTTPAuthHeaderBuilder', () => { - function createAuthClient() { - const authHeader = 'X-Serverless-Authorization'; - const authToken = 'test-token'; - return new AuthClientStub(authToken, authHeader); - } +describe('BaseHTTPAuthHeaderBuilder', () => { + function createAuthClient() { + const authHeader = 'X-Serverless-Authorization'; + const authToken = 'test-token'; + return new AuthClientStub(authToken, authHeader); + } - it('should call getToken and getHeaderName on authClient', async () => { - const authClient = createAuthClient(); - vi.spyOn(authClient, 'getToken'); - vi.spyOn(authClient, 'getHeaderName'); - const client = new BaseAuthHeaderBuilder(authClient); + it('should call getToken and getHeaderName on authClient', async () => { + const authClient = createAuthClient(); + vi.spyOn(authClient, 'getToken'); + vi.spyOn(authClient, 'getHeaderName'); + const client = new BaseAuthHeaderBuilder(authClient); - const token = 'app-token'; - await client.buildHeader(token); + const token = 'app-token'; + await client.buildHeader(token); - expect(authClient.getToken).toHaveBeenCalled(); - expect(authClient.getHeaderName).toHaveBeenCalled(); - }); + expect(authClient.getToken).toHaveBeenCalled(); + expect(authClient.getHeaderName).toHaveBeenCalled(); + }); - it('should return the correct auth header if token is provided', async () => { - const client = new BaseAuthHeaderBuilder(); + it('should return the correct auth header if token is provided', async () => { + const client = new BaseAuthHeaderBuilder(); - const token = 'app-token'; - const headers = await client.buildHeader(token); + const token = 'app-token'; + const headers = await client.buildHeader(token); - expect(headers).toEqual({ Authorization: 'Bearer app-token' }); - }); + expect(headers).toEqual({ Authorization: 'Bearer app-token' }); + }); - it('should return the correct auth header if authClient and token are provided', async () => { - const authClient = createAuthClient(); - const client = new BaseAuthHeaderBuilder(authClient); + it('should return the correct auth header if authClient and token are provided', async () => { + const authClient = createAuthClient(); + const client = new BaseAuthHeaderBuilder(authClient); - const token = 'app-token'; - const headers = await client.buildHeader(token); + const token = 'app-token'; + const headers = await client.buildHeader(token); - expect(headers).toEqual({ - Authorization: 'Bearer app-token', - 'X-Serverless-Authorization': 'Bearer test-token', - }); + expect(headers).toEqual({ + Authorization: 'Bearer app-token', + 'X-Serverless-Authorization': 'Bearer test-token', }); + }); - it('should throw an error if the auth client fails', async () => { - const errorMessage = 'Auth request failed'; - const authClient = createAuthClient(); - vi.spyOn(authClient, 'getToken').mockImplementation(() => { - throw new Error(errorMessage); - }); - const client = new BaseAuthHeaderBuilder(authClient); - - const token = 'app-token'; - await expect(client.buildHeader(token)).rejects.toThrow(errorMessage); + it('should throw an error if the auth client fails', async () => { + const errorMessage = 'Auth request failed'; + const authClient = createAuthClient(); + vi.spyOn(authClient, 'getToken').mockImplementation(() => { + throw new Error(errorMessage); }); + const client = new BaseAuthHeaderBuilder(authClient); + + const token = 'app-token'; + await expect(client.buildHeader(token)).rejects.toThrow(errorMessage); }); }); From 128d03ffee16f47c37cad5c48bcf707a0eff7183 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 15:07:41 +0100 Subject: [PATCH 28/31] fix: make interface implementation explicit --- src/lib/clients/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index 02cdbb0..c85ac0a 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -5,7 +5,7 @@ export interface AuthClient { getHeaderName(): string; } -export class GCPAuthClient { +export class GCPAuthClient implements AuthClient { private httpClient: HttpClient; private baseUrl: string = 'http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/identity'; From e84a2a8cd22630edd7a2b850fddf1e98cdc5b421 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 16:07:59 +0100 Subject: [PATCH 29/31] chore: remove unused test method --- src/lib/clients/artists.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/lib/clients/artists.ts b/src/lib/clients/artists.ts index ac45504..d466386 100644 --- a/src/lib/clients/artists.ts +++ b/src/lib/clients/artists.ts @@ -53,10 +53,6 @@ export class ArtistsClientStub implements ArtistsClient { this.searchArtistResult = result; } - setResult(result: Artist[]) { - this.searchArtistResult = result; - } - async searchArtists(..._: any[]): Promise { return this.searchArtistResult; } From 91fea34784298e07f79950d24b1de043e401c566 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 16:09:10 +0100 Subject: [PATCH 30/31] chore: make test naming consistent --- src/lib/clients/auth.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/clients/auth.test.ts b/src/lib/clients/auth.test.ts index 8de440c..d994c03 100644 --- a/src/lib/clients/auth.test.ts +++ b/src/lib/clients/auth.test.ts @@ -47,7 +47,7 @@ describe('GCPAuthClient', () => { }); }); -describe('BaseHTTPAuthHeaderBuilder', () => { +describe('BaseAuthHeaderBuilder', () => { function createAuthClient() { const authHeader = 'X-Serverless-Authorization'; const authToken = 'test-token'; From 0f4464827ad87b6c77a52baea21e59c85b18a676 Mon Sep 17 00:00:00 2001 From: Dani Date: Sat, 15 Feb 2025 16:10:58 +0100 Subject: [PATCH 31/31] chore: remove unused method --- src/lib/clients/auth.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/lib/clients/auth.ts b/src/lib/clients/auth.ts index c85ac0a..64fa9c2 100644 --- a/src/lib/clients/auth.ts +++ b/src/lib/clients/auth.ts @@ -90,10 +90,6 @@ export class AuthHeaderBuilderStub implements AuthHeaderBuilder { this.headers = headers; } - setHeaders(headers: Record) { - this.headers = headers; - } - async buildHeader(_token: string): Promise> { return this.headers; }