From a6291c92ee0bd633e7356dd75c61c3f182b18dd9 Mon Sep 17 00:00:00 2001 From: Daniel Emery Date: Sat, 25 Nov 2023 15:40:26 +1030 Subject: [PATCH] Improve test coverage & fix exposed issues (#80) * #79 Inject config into the AuthenticationService * #79 Add test coverage to AuthorisationService.requireUserRole * #79 Add tests to memory cache and fix exposed issues * #79 Add test coverage to UserService.loadUserDetailsAndUpdateIfNecessary * #79 Add docs to loadUserDetailsAndUpdateIfNecessary * #79 Use function overloading to properly type UserService.getUsers and fix issue where role was not included in filter * #79 Add unit test for getQuizDetails * #79 Add new user service function to load a single user * #79 Refactor and unit test createQuiz * #79 Ignore rest siblings eslint violation * #79 Throw explicit UnauthorisedError when unauthorised --- src/auth/authentication.service.ts | 10 +- src/auth/authorisation.errors.ts | 1 + src/auth/authorisation.service.test.ts | 23 ++++ src/auth/authorisation.service.ts | 3 +- src/quiz/quiz.errors.ts | 1 + src/quiz/quiz.gql.ts | 5 +- src/quiz/quiz.service.test.ts | 141 +++++++++++++++++++++- src/quiz/quiz.service.ts | 71 ++++++++--- src/service.locator.ts | 10 +- src/statistics/statistics.service.test.ts | 19 +-- src/statistics/statistics.service.ts | 16 +-- src/user/user.errors.ts | 1 + src/user/user.gql.ts | 14 ++- src/user/user.persistence.ts | 98 +++++++++++---- src/user/user.service.test.ts | 129 ++++++++++++++++++++ src/user/user.service.ts | 104 ++++++++++++---- src/util/cache.test.ts | 39 ++++++ src/util/cache.ts | 8 +- 18 files changed, 579 insertions(+), 114 deletions(-) create mode 100644 src/auth/authorisation.errors.ts create mode 100644 src/auth/authorisation.service.test.ts create mode 100644 src/quiz/quiz.errors.ts create mode 100644 src/user/user.errors.ts create mode 100644 src/user/user.service.test.ts create mode 100644 src/util/cache.test.ts diff --git a/src/auth/authentication.service.ts b/src/auth/authentication.service.ts index 12fba76..324694b 100644 --- a/src/auth/authentication.service.ts +++ b/src/auth/authentication.service.ts @@ -1,20 +1,18 @@ import jwt, { JwtHeader, SigningKeyCallback, VerifyOptions } from 'jsonwebtoken'; import jwksClient, { RsaSigningKey } from 'jwks-rsa'; -import config from '../config/config'; - export class AuthenticationService { #client: jwksClient.JwksClient; #options: VerifyOptions; - constructor() { + constructor(auth0Domain: string, auth0Audience: string) { this.#client = jwksClient({ - jwksUri: `https://${config.AUTH0_DOMAIN}/.well-known/jwks.json`, + jwksUri: `https://${auth0Domain}/.well-known/jwks.json`, }); this.#options = { algorithms: ['RS256'], - audience: config.AUTH0_AUDIENCE, - issuer: `https://${config.AUTH0_DOMAIN}/`, + audience: auth0Audience, + issuer: `https://${auth0Domain}/`, }; } diff --git a/src/auth/authorisation.errors.ts b/src/auth/authorisation.errors.ts new file mode 100644 index 0000000..58bd0d8 --- /dev/null +++ b/src/auth/authorisation.errors.ts @@ -0,0 +1 @@ +export class UnauthorisedError extends Error {} diff --git a/src/auth/authorisation.service.test.ts b/src/auth/authorisation.service.test.ts new file mode 100644 index 0000000..6d41cd1 --- /dev/null +++ b/src/auth/authorisation.service.test.ts @@ -0,0 +1,23 @@ +import { QuizlordContext } from '..'; +import { UnauthorisedError } from './authorisation.errors'; +import { AuthorisationService } from './authorisation.service'; + +describe('authorisation', () => { + describe('authorisation.service', () => { + const sut = new AuthorisationService(); + describe('requireUserRole', () => { + it('must do nothing if the user has the required role', () => { + const context = { + roles: ['USER', 'ADMIN'], + } as QuizlordContext; + sut.requireUserRole(context, 'ADMIN'); + }); + it('must throw an error if the user does not have the required role', () => { + const context = { + roles: ['USER'], + } as QuizlordContext; + expect(() => sut.requireUserRole(context, 'ADMIN')).toThrow(UnauthorisedError); + }); + }); + }); +}); diff --git a/src/auth/authorisation.service.ts b/src/auth/authorisation.service.ts index 0d777b5..9768c74 100644 --- a/src/auth/authorisation.service.ts +++ b/src/auth/authorisation.service.ts @@ -1,10 +1,11 @@ import { Role } from '@prisma/client'; import { QuizlordContext } from '..'; +import { UnauthorisedError } from './authorisation.errors'; export class AuthorisationService { requireUserRole(context: QuizlordContext, role: Role) { if (!context.roles.includes(role)) { - throw new Error('You are not authorised to perform this action'); + throw new UnauthorisedError('You are not authorised to perform this action'); } } } diff --git a/src/quiz/quiz.errors.ts b/src/quiz/quiz.errors.ts new file mode 100644 index 0000000..7d71cbf --- /dev/null +++ b/src/quiz/quiz.errors.ts @@ -0,0 +1 @@ +export class MustProvideAtLeastOneFileError extends Error {} diff --git a/src/quiz/quiz.gql.ts b/src/quiz/quiz.gql.ts index b18b911..1f663f5 100644 --- a/src/quiz/quiz.gql.ts +++ b/src/quiz/quiz.gql.ts @@ -37,13 +37,10 @@ async function createQuiz( context: QuizlordContext, ): Promise { authorisationService.requireUserRole(context, 'USER'); - return quizService.createQuiz({ + return quizService.createQuiz(context.userId, { type, date, files, - userId: context.userId, - email: context.email, - userName: context.userName, }); } diff --git a/src/quiz/quiz.service.test.ts b/src/quiz/quiz.service.test.ts index 801dead..84f2c82 100644 --- a/src/quiz/quiz.service.test.ts +++ b/src/quiz/quiz.service.test.ts @@ -1,21 +1,44 @@ +import { v4 as uuidv4 } from 'uuid'; + import { QuizService } from './quiz.service'; import { QuizPersistence } from './quiz.persistence'; import { S3FileService } from '../file/s3.service'; import { Decimal } from '@prisma/client/runtime/library'; +import { UserService } from '../user/user.service'; +import { MustProvideAtLeastOneFileError } from './quiz.errors'; + +jest.mock('uuid'); +const mockedUUIDv4 = jest.mocked(uuidv4); const mockPersistence = { - getQuizzesWithUserResults: jest.fn(), + createQuizWithImages: jest.fn(), getCompletionScoreWithQuizTypesForUser: jest.fn(), + getQuizzesWithUserResults: jest.fn(), + getQuizByIdWithResults: jest.fn(), +}; +const mockFileService = { + createKey: jest.fn(), + generateSignedUploadUrl: jest.fn(), +}; +const mockUserService = { + getUser: jest.fn(), }; -const mockFileService = {}; -const sut = new QuizService(mockPersistence as unknown as QuizPersistence, mockFileService as S3FileService); +const sut = new QuizService( + mockPersistence as unknown as QuizPersistence, + mockFileService as unknown as S3FileService, + mockUserService as unknown as UserService, +); describe('quiz', () => { describe('quiz.service', () => { beforeEach(() => { + jest.clearAllMocks(); jest.restoreAllMocks(); }); + afterEach(() => { + jest.useRealTimers(); + }); describe('getQuizzesWithUserResults', () => { it('must call getQuizzesWithUserResults on persistence with correct arguments and transform the result', async () => { const persistenceResult = [ @@ -180,5 +203,117 @@ describe('quiz', () => { }); }); }); + describe('getQuizDetails', () => { + it('must call getQuizByIdWithResults on persistence with correct arguments and transform the result', async () => { + mockPersistence.getQuizByIdWithResults.mockResolvedValueOnce({ + id: 'fake-quiz-id', + type: 'SHARK', + date: new Date('2023-01-01'), + uploadedAt: new Date('2023-01-02'), + completions: [], + images: [], + uploadedByUser: { + id: 'fake-user-id', + email: 'master@quizlord.net', + name: 'Master', + }, + }); + + const actual = await sut.getQuizDetails('fake-quiz-id'); + + expect(actual).toEqual({ + id: 'fake-quiz-id', + type: 'SHARK', + date: new Date('2023-01-01'), + uploadedAt: new Date('2023-01-02'), + completions: [], + images: [], + uploadedBy: { + id: 'fake-user-id', + email: 'master@quizlord.net', + name: 'Master', + }, + }); + }); + }); + describe('createQuiz', () => { + it('must throw if no files are provided', async () => { + await expect(() => + sut.createQuiz('fake-user', { type: 'SHARK', date: new Date('2022-01-01'), files: [] }), + ).rejects.toThrow(MustProvideAtLeastOneFileError); + }); + it('must create quiz with upload link for each image', async () => { + const fakeNow = new Date('2023-02-12'); + jest.useFakeTimers(); + jest.setSystemTime(fakeNow); + + mockedUUIDv4.mockReturnValueOnce('fake-quiz-id'); + mockUserService.getUser.mockResolvedValueOnce({ + email: 'quizmaster@quizlord.net', + }); + mockFileService.createKey.mockReturnValueOnce('key-one').mockReturnValueOnce('key-two'); + mockFileService.generateSignedUploadUrl + .mockResolvedValueOnce('https://upload-one.net') + .mockResolvedValueOnce('https://upload-two.net'); + + const actual = await sut.createQuiz('fake-user-id', { + date: new Date('2022-01-01'), + type: 'SHARK', + files: [ + { + fileName: 'questions.jpg', + type: 'QUESTION', + }, + { + fileName: 'answers.jpg', + type: 'ANSWER', + }, + ], + }); + + expect(mockedUUIDv4).toHaveBeenCalledTimes(1); + expect(mockFileService.createKey).toHaveBeenCalledTimes(2); + expect(mockFileService.createKey).toHaveBeenNthCalledWith(1, 'fake-quiz-id', 'questions.jpg'); + expect(mockFileService.createKey).toHaveBeenNthCalledWith(2, 'fake-quiz-id', 'answers.jpg'); + expect(mockFileService.generateSignedUploadUrl).toHaveBeenCalledTimes(2); + expect(mockFileService.generateSignedUploadUrl).toHaveBeenNthCalledWith(1, 'key-one'); + expect(mockFileService.generateSignedUploadUrl).toHaveBeenNthCalledWith(2, 'key-two'); + expect(mockPersistence.createQuizWithImages).toHaveBeenCalledTimes(1); + expect(mockPersistence.createQuizWithImages).toHaveBeenCalledWith( + { + date: new Date('2022-01-01'), + id: 'fake-quiz-id', + type: 'SHARK', + uploadedAt: new Date(fakeNow), + uploadedByUserId: 'fake-user-id', + }, + [ + { imageKey: 'key-one', state: 'PENDING_UPLOAD', type: 'QUESTION' }, + { imageKey: 'key-two', state: 'PENDING_UPLOAD', type: 'ANSWER' }, + ], + ); + + expect(actual).toEqual({ + quiz: { + myCompletions: [], + uploadedBy: { + email: 'quizmaster@quizlord.net', + id: 'fake-user-id', + name: undefined, + }, + }, + uploadLinks: [ + { + fileName: 'questions.jpg', + link: 'https://upload-one.net', + }, + { + fileName: 'answers.jpg', + link: 'https://upload-two.net', + }, + ], + }); + }); + }); }); }); diff --git a/src/quiz/quiz.service.ts b/src/quiz/quiz.service.ts index 40a6c33..6b4379e 100644 --- a/src/quiz/quiz.service.ts +++ b/src/quiz/quiz.service.ts @@ -12,16 +12,20 @@ import { import { Quiz, QuizCompletion, QuizFilters, QuizImage } from './quiz.dto'; import { S3FileService } from '../file/s3.service'; import { QuizPersistence } from './quiz.persistence'; +import { UserService } from '../user/user.service'; +import { MustProvideAtLeastOneFileError } from './quiz.errors'; const MAXIMUM_QUIZ_PAGE_SIZE = 100; export class QuizService { #persistence: QuizPersistence; #fileService: S3FileService; + #userService: UserService; - constructor(persistence: QuizPersistence, fileService: S3FileService) { + constructor(persistence: QuizPersistence, fileService: S3FileService, userService: UserService) { this.#persistence = persistence; this.#fileService = fileService; + this.#userService = userService; } /** @@ -47,11 +51,18 @@ export class QuizService { return { data: data.map((entry) => this.#quizPersistenceWithMyCompletionsToQuiz(entry)), hasMoreRows }; } + /** + * Get a quiz along with all its completions and images. + * @param id Id of the quiz to get details for. + * @returns The quiz and its completions. + */ async getQuizDetails(id: string) { const quiz = await this.#persistence.getQuizByIdWithResults({ id, }); - const { images, completions, uploadedByUser, ...quizFieldsThatDoNotRequireTransform } = quiz; + // TODO look into modifying the upstream https://eslint.org/docs/latest/rules/no-unused-vars#ignorerestsiblings linting rule + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { images, completions, uploadedByUser, uploadedByUserId, ...quizFieldsThatDoNotRequireTransform } = quiz; return { ...quizFieldsThatDoNotRequireTransform, completions: completions.map((entry) => this.#quizCompletionPersistenceToQuizCompletion(entry)), @@ -64,21 +75,43 @@ export class QuizService { }; } - async createQuiz({ - userId, - email, - userName, - type, - date, - files, - }: { - userId: string; - email: string; - userName?: string; - type: QuizType; - date: Date; - files: { fileName: string; type: QuizImageType }[]; - }) { + /** + * Create a quiz. + * The images are not provided directly here, but rather signed links + * are returned that can be used to upload. + * + * The quiz will only be marked as ready once the images have been uploaded. + * + * @param userId The id of the user creating the quiz. + * @param quizDetails The type, date and files for the quiz. + * @returns The created quiz and links that should be used for uploading the files. + */ + async createQuiz( + userId: string, + { + type, + date, + files, + }: { + /** The type of quiz being created. */ + type: QuizType; + /** The date the quiz was published. */ + date: Date; + /** Images for the quiz. */ + files: { + /** Name of the file. */ + fileName: string; + /** What the image is of. */ + type: QuizImageType; + }[]; + }, + ) { + if (files.length === 0) { + throw new MustProvideAtLeastOneFileError(); + } + + const user = await this.#userService.getUser(userId); + const uuid = uuidv4(); const filesWithKeys = files.map((file) => ({ ...file, @@ -107,8 +140,8 @@ export class QuizService { completions: [], uploadedByUser: { id: userId, - email: email, - name: userName ?? null, + email: user.email, + name: user.name ?? null, }, }), uploadLinks: uploadLinks.map((ul) => ({ fileName: ul.fileName, link: ul.uploadLink })), diff --git a/src/service.locator.ts b/src/service.locator.ts index ba6957b..f52e3db 100644 --- a/src/service.locator.ts +++ b/src/service.locator.ts @@ -15,7 +15,7 @@ import config from './config/config'; const memoryCache = new MemoryCache(); // auth -export const authenticationService = new AuthenticationService(); +export const authenticationService = new AuthenticationService(config.AUTH0_DOMAIN, config.AUTH0_AUDIENCE); export const authorisationService = new AuthorisationService(); // prisma @@ -24,14 +24,14 @@ export const prismaService = new PrismaService(); // file export const fileService = new S3FileService(config.AWS_REGION, config.AWS_BUCKET_NAME, config.FILE_ACCESS_BASE_URL); -// quiz -export const quizPersistence = new QuizPersistence(prismaService); -export const quizService = new QuizService(quizPersistence, fileService); - // user export const userPersistence = new UserPersistence(prismaService); export const userService = new UserService(userPersistence); +// quiz +export const quizPersistence = new QuizPersistence(prismaService); +export const quizService = new QuizService(quizPersistence, fileService, userService); + // queue export const queueService = new SQSQueueService(quizService); diff --git a/src/statistics/statistics.service.test.ts b/src/statistics/statistics.service.test.ts index da9df18..a75ece1 100644 --- a/src/statistics/statistics.service.test.ts +++ b/src/statistics/statistics.service.test.ts @@ -46,7 +46,7 @@ describe('statistics', () => { const actual = await sut.getIndividualUserStatistics(); expect(mockCache.getItem).toHaveBeenCalledTimes(1); - expect(mockCache.getItem).toHaveBeenCalledWith('invidual-user-statistics'); + expect(mockCache.getItem).toHaveBeenCalledWith('individual-user-statistics'); expect(mockSortIndividualUserStatistics).toHaveBeenCalledTimes(1); expect(mockSortIndividualUserStatistics).toHaveBeenCalledWith( @@ -130,18 +130,8 @@ describe('statistics', () => { const actual = await sut.getIndividualUserStatistics(); expect(mockUserService.getUsers).toHaveBeenCalledTimes(2); - expect(mockUserService.getUsers).toHaveBeenNthCalledWith(1, { - currentUserId: '1', - first: 100, - afterId: undefined, - sortedBy: 'EMAIL_ASC', - }); - expect(mockUserService.getUsers).toHaveBeenNthCalledWith(2, { - currentUserId: '1', - first: 100, - afterId: '75', - sortedBy: 'EMAIL_ASC', - }); + expect(mockUserService.getUsers).toHaveBeenNthCalledWith(1, 100, 'EMAIL_ASC', undefined); + expect(mockUserService.getUsers).toHaveBeenNthCalledWith(2, 100, 'EMAIL_ASC', '75'); expect(mockGetStatisticsForUser).toHaveBeenCalledTimes(3); expect(mockGetStatisticsForUser).toHaveBeenNthCalledWith(1, 'userOne@quizlord.net'); @@ -151,6 +141,9 @@ describe('statistics', () => { expect(mockSortIndividualUserStatistics).toHaveBeenCalledTimes(1); expect(mockSortIndividualUserStatistics).toHaveBeenCalledWith(expected, 'QUIZZES_COMPLETED_DESC'); + expect(mockCache.setItem).toHaveBeenCalledTimes(1); + expect(mockCache.setItem).toHaveBeenCalledWith('individual-user-statistics', expected, 24 * 60 * 60 * 1000); + expect(actual).toEqual(expected); }); }); diff --git a/src/statistics/statistics.service.ts b/src/statistics/statistics.service.ts index 265cb5f..ac591ed 100644 --- a/src/statistics/statistics.service.ts +++ b/src/statistics/statistics.service.ts @@ -1,10 +1,10 @@ import { QuizService } from '../quiz/quiz.service'; -import { UserService } from '../user/user.service'; +import { GetUsersResult, UserService } from '../user/user.service'; import { Cache } from '../util/cache'; import { IndividualUserStatistic, IndividualUserStatisticsSortOption } from './statistics.dto'; -const INDIVIDUAL_STATISTICS_CACHE_KEY = 'invidual-user-statistics'; -const INDIVIDUAL_STATISTICS_CACHE_TTL = 60 * 60 * 1000; // 24 hours +const INDIVIDUAL_STATISTICS_CACHE_KEY = 'individual-user-statistics'; +const INDIVIDUAL_STATISTICS_CACHE_TTL_MILLIS = 24 * 60 * 60 * 1000; // 24 hours export class StatisticsService { #userService: UserService; @@ -35,12 +35,8 @@ export class StatisticsService { let hasMoreRows = true; let cursor: string | undefined = undefined; while (hasMoreRows) { - const { data, hasMoreRows: moreRows } = await this.#userService.getUsers({ - currentUserId: '1', // Current user id isn't valid here and isn't used for sorting by EMAIL_ASC - first: 100, - afterId: cursor, - sortedBy: 'EMAIL_ASC', - }); + const userPage: GetUsersResult = await this.#userService.getUsers(100, 'EMAIL_ASC', cursor); + const { data, hasMoreRows: moreRows } = userPage; for (const user of data) { const { totalQuizCompletions, averageScorePercentage } = await this.getStatisticsForUser(user.email); @@ -56,7 +52,7 @@ export class StatisticsService { hasMoreRows = moreRows; } - await this.#cache.setItem(INDIVIDUAL_STATISTICS_CACHE_KEY, results, INDIVIDUAL_STATISTICS_CACHE_TTL); + await this.#cache.setItem(INDIVIDUAL_STATISTICS_CACHE_KEY, results, INDIVIDUAL_STATISTICS_CACHE_TTL_MILLIS); return this.sortIndividualUserStatistics(results, sortedBy); } diff --git a/src/user/user.errors.ts b/src/user/user.errors.ts new file mode 100644 index 0000000..38bf264 --- /dev/null +++ b/src/user/user.errors.ts @@ -0,0 +1 @@ +export class UserNotFoundError extends Error {} diff --git a/src/user/user.gql.ts b/src/user/user.gql.ts index d2d1b12..f69f769 100644 --- a/src/user/user.gql.ts +++ b/src/user/user.gql.ts @@ -2,6 +2,7 @@ import { QuizlordContext } from '..'; import { base64Decode, base64Encode, PagedResult } from '../util/paging-helpers'; import { authorisationService, userService } from '../service.locator'; import { User, UserDetails, UserSortOption } from './user.dto'; +import { GetUsersResult } from './user.service'; async function users( _: unknown, @@ -14,12 +15,13 @@ async function users( ): Promise> { authorisationService.requireUserRole(context, 'USER'); const afterId = after ? base64Decode(after) : undefined; - const { data, hasMoreRows } = await userService.getUsers({ - currentUserId: context.userId, - afterId, - first, - sortedBy, - }); + let serviceResult: GetUsersResult; + if (sortedBy === 'NUMBER_OF_QUIZZES_COMPLETED_WITH_DESC') { + serviceResult = await userService.getUsers(first, sortedBy, context.userId, afterId); + } else { + serviceResult = await userService.getUsers(first, sortedBy, afterId); + } + const { data, hasMoreRows } = serviceResult; const edges = data.map((user) => ({ node: user, cursor: base64Encode(user.id), diff --git a/src/user/user.persistence.ts b/src/user/user.persistence.ts index 2e5e764..1a48c6c 100644 --- a/src/user/user.persistence.ts +++ b/src/user/user.persistence.ts @@ -4,6 +4,15 @@ import { PrismaService } from '../database/prisma.service'; import { UserSortOption } from '../user/user.dto'; import { getPagedQuery, slicePagedResults } from '../util/paging-helpers'; +export interface GetUsersWithRoleResult { + data: { + id: string; + email: string; + name: string | null; + }[]; + hasMoreRows: boolean; +} + export class UserPersistence { #prisma: PrismaService; constructor(prisma: PrismaService) { @@ -21,6 +30,14 @@ export class UserPersistence { }); } + async getUserById(id: string) { + return this.#prisma.client().user.findFirst({ + where: { + id, + }, + }); + } + async createNewUser(id: string, email: string, name: string | undefined) { await this.#prisma.client().user.create({ data: { @@ -56,19 +73,30 @@ export class UserPersistence { return roles.map((r) => r.role); } + /** + * Get a list of users with the given role sorted by the provided option. + * @param arguments Query parameters. + * @returns A list of users with the given role and flag indicating if there are more. + */ async getUsersWithRole({ role, afterId, limit, sortedBy, - currentUserId, }: { + /** Only users with this role will be returned. */ role: Role; + /** If provided, will be used as a cursor. */ afterId?: string; + /** The number of users to load. */ limit: number; - sortedBy: UserSortOption; - currentUserId: string; - }) { + /** + * The sorting option to use. + * Note if you want to sort by NUMBER_OF_QUIZZES_COMPLETED_WITH_DESC please see + * getUsersWithRoleSortedByNumberOfQuizzesCompletedWith. + */ + sortedBy: Omit; + }): Promise { const pagedWhereQuery = { ...getPagedQuery(limit, afterId), where: { @@ -83,6 +111,7 @@ export class UserPersistence { let result; switch (sortedBy) { case 'EMAIL_ASC': + default: result = await this.#prisma.client().user.findMany({ ...pagedWhereQuery, orderBy: { @@ -98,26 +127,51 @@ export class UserPersistence { }, }); break; - case 'NUMBER_OF_QUIZZES_COMPLETED_WITH_DESC': - default: - /** - * Prisma does not yet support ordering by a relation with anything other than count. - * We need to do something quite a bit more complex. - */ - result = (await this.#prisma.client().$queryRaw` -select id, email, name from - ( - select "user".*, count(my_completion.user_id) as completions from "user" - left outer join quiz_completion_user as their_completion on "user".id = their_completion.user_id - left outer join quiz_completion on their_completion.quiz_completion_id = quiz_completion.id - left outer join quiz_completion_user as my_completion on my_completion.quiz_completion_id = quiz_completion.id and my_completion.user_id = ${currentUserId} - group by "user".id - ) as completions_with_current_user -order by completions_with_current_user.completions desc; - `) as User[]; - break; } return slicePagedResults(result, limit, afterId !== undefined); } + + /** + * Special variant of getUsersWithRole which sorts by the number of quizzes completed with the provided user. + * + * Note this was split out from getUsersWithRole because it requires dropping down to a raw query and has the + * additional requirement of the current user's id. + * + * @param arguments Query parameters. + * @returns List of users with the given role, sorted by the number of quizzes completed with provided user. + */ + async getUsersWithRoleSortedByNumberOfQuizzesCompletedWith({ + role, + afterId, + limit, + currentUserId, + }: { + /** Only users with this role will be returned. */ + role: Role; + /** Optional user id to use as a cursor. */ + afterId?: string; + /** The number of users to load. */ + limit: number; + /** The id of the user to use to sort. */ + currentUserId: string; + }): Promise { + /** + * Prisma does not yet support ordering by a relation with anything other than count. + * We need to do something quite a bit more complex so we drop down to a raw query. + */ + const result = (await this.#prisma.client().$queryRaw` + select id, email, name from + ( + select "user".*, count(my_completion.user_id) as completions from "user" + inner join user_role on "user".id = user_role.user_id and user_role.role::text = ${role} + left outer join quiz_completion_user as their_completion on "user".id = their_completion.user_id + left outer join quiz_completion on their_completion.quiz_completion_id = quiz_completion.id + left outer join quiz_completion_user as my_completion on my_completion.quiz_completion_id = quiz_completion.id and my_completion.user_id = ${currentUserId} + group by "user".id + ) as completions_with_current_user + order by completions_with_current_user.completions desc; + `) as User[]; + return slicePagedResults(result, limit, afterId !== undefined); + } } diff --git a/src/user/user.service.test.ts b/src/user/user.service.test.ts new file mode 100644 index 0000000..d61b26a --- /dev/null +++ b/src/user/user.service.test.ts @@ -0,0 +1,129 @@ +import { v4 as uuidv4 } from 'uuid'; + +import { UserPersistence } from './user.persistence'; +import { UserService } from './user.service'; +import { UserNotFoundError } from './user.errors'; + +jest.mock('uuid'); + +const mockedUUIDv4 = jest.mocked(uuidv4); +const fakeUserPersistence = { + createNewUser: jest.fn(), + getUserByEmail: jest.fn(), + getUserById: jest.fn(), + updateUserName: jest.fn(), +}; + +describe('user', () => { + describe('user.service', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.restoreAllMocks(); + }); + const sut = new UserService(fakeUserPersistence as unknown as UserPersistence); + describe('loadUserDetailsAndUpdateIfNecessary', () => { + it('must return existing user with roles if it exists already with correct name', async () => { + fakeUserPersistence.getUserByEmail.mockResolvedValueOnce({ + id: 'fake-id', + name: 'Joe Blogs', + roles: [{ role: 'ADMIN' }], + }); + + const actual = await sut.loadUserDetailsAndUpdateIfNecessary('joe@quizlord.net', 'Joe Blogs'); + + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledWith('joe@quizlord.net'); + + expect(fakeUserPersistence.createNewUser).not.toHaveBeenCalled(); + expect(fakeUserPersistence.updateUserName).not.toHaveBeenCalled(); + + expect(actual).toEqual({ + id: 'fake-id', + roles: ['ADMIN'], + }); + }); + it('must create the user if it does not exist', async () => { + fakeUserPersistence.getUserByEmail.mockResolvedValueOnce(undefined); + mockedUUIDv4.mockReturnValueOnce('fake-id'); + + const actual = await sut.loadUserDetailsAndUpdateIfNecessary('joe@quizlord.net', 'Joe Blogs'); + + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledWith('joe@quizlord.net'); + + expect(fakeUserPersistence.createNewUser).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.createNewUser).toHaveBeenCalledWith('fake-id', 'joe@quizlord.net', 'Joe Blogs'); + + expect(fakeUserPersistence.updateUserName).not.toHaveBeenCalled(); + + expect(actual).toEqual({ + id: 'fake-id', + roles: [], + }); + }); + it('must update the name if it is different', async () => { + fakeUserPersistence.getUserByEmail.mockResolvedValueOnce({ + id: 'fake-id', + name: 'Joe Biggs', + roles: [{ role: 'ADMIN' }], + }); + + const actual = await sut.loadUserDetailsAndUpdateIfNecessary('joe@quizlord.net', 'Joe Blogs'); + + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledWith('joe@quizlord.net'); + + expect(fakeUserPersistence.createNewUser).not.toHaveBeenCalled(); + + expect(fakeUserPersistence.updateUserName).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.updateUserName).toHaveBeenCalledWith('fake-id', 'Joe Blogs'); + + expect(actual).toEqual({ + id: 'fake-id', + roles: ['ADMIN'], + }); + }); + it("must set the name if it doesn't exist", async () => { + fakeUserPersistence.getUserByEmail.mockResolvedValueOnce({ + id: 'fake-id', + roles: [{ role: 'ADMIN' }], + }); + + const actual = await sut.loadUserDetailsAndUpdateIfNecessary('joe@quizlord.net', 'Joe Blogs'); + + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.getUserByEmail).toHaveBeenCalledWith('joe@quizlord.net'); + + expect(fakeUserPersistence.createNewUser).not.toHaveBeenCalled(); + + expect(fakeUserPersistence.updateUserName).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.updateUserName).toHaveBeenCalledWith('fake-id', 'Joe Blogs'); + + expect(actual).toEqual({ + id: 'fake-id', + roles: ['ADMIN'], + }); + }); + }); + describe('getUser', () => { + it("must throw if user doesn't exist", async () => { + fakeUserPersistence.getUserById.mockResolvedValueOnce(null); + + await expect(() => sut.getUser('fake-id')).rejects.toThrow(UserNotFoundError); + + expect(fakeUserPersistence.getUserById).toHaveBeenCalledTimes(1); + expect(fakeUserPersistence.getUserById).toHaveBeenCalledWith('fake-id'); + }); + it('must return transformed user', async () => { + fakeUserPersistence.getUserById.mockResolvedValueOnce({ id: 'fake-id', email: 'fake@quizlord.net' }); + + const actual = await sut.getUser('fake-id'); + + expect(actual).toEqual({ + id: 'fake-id', + email: 'fake@quizlord.net', + }); + }); + }); + }); +}); diff --git a/src/user/user.service.ts b/src/user/user.service.ts index 0649bc6..468e366 100644 --- a/src/user/user.service.ts +++ b/src/user/user.service.ts @@ -1,8 +1,14 @@ import { v4 as uuidv4 } from 'uuid'; -import { User as UserPersistenceModel } from '@prisma/client'; +import { User as UserPersistenceModel, Role as RolePersistenceModel } from '@prisma/client'; import { Role, User, UserSortOption } from './user.dto'; import { UserPersistence } from './user.persistence'; +import { UserNotFoundError } from './user.errors'; + +export interface GetUsersResult { + data: User[]; + hasMoreRows: boolean; +} export class UserService { #persistence: UserPersistence; @@ -10,6 +16,15 @@ export class UserService { this.#persistence = persistence; } + /** + * Load user details based on the provided email. + * If the user does not exist, create a new user. + * If the user's name has changed, it will be updated. + * + * @param email The email to load the user by. + * @param name Optionally the user's name. + * @returns The resulting user's roles and id. + */ async loadUserDetailsAndUpdateIfNecessary( email: string, name: string | undefined, @@ -34,32 +49,79 @@ export class UserService { return { roles: user.roles.map((r) => r.role), id: user.id }; } - // TODO overload this function to only require the currentUserId parameter when the sortedBy parameter is - // NUMBER_OF_QUIZZES_COMPLETED_WITH_DESC - async getUsers({ - currentUserId, - first, - afterId, - sortedBy, - }: { - currentUserId: string; - first: number; - afterId?: string; - sortedBy: UserSortOption; - }) { - const { data, hasMoreRows } = await this.#persistence.getUsersWithRole({ - role: 'USER', - afterId, - limit: first, - sortedBy, - currentUserId, - }); + /** + * Get a page of users sorted by the given option. + * @param first The number of users to return. + * @param sortedBy The sorting option to use. + * @param currentUserId User id to use to compute the number of quizzes completed with. + * @param afterId Optional cursor to return users after. + */ + async getUsers( + first: number, + sortedBy: 'NUMBER_OF_QUIZZES_COMPLETED_WITH_DESC', + currentUserId: string, + afterId?: string, + ): Promise; + /** + * Get a page of users sorted by the given option. + * @param first The number of users to return. + * @param sortedBy The sorting option to use. + * @param afterId Optional cursor to return users after. + */ + async getUsers( + first: number, + sortedBy: Omit, + afterId?: string, + ): Promise; + async getUsers( + first: number, + sortedBy: UserSortOption, + afterIdOrCurrentUserId?: string, + maybeAfterId?: string, + ): Promise { + let data: UserPersistenceModel[]; + let hasMoreRows: boolean; + + if (sortedBy === 'NUMBER_OF_QUIZZES_COMPLETED_WITH_DESC' && afterIdOrCurrentUserId) { + const result = await this.#persistence.getUsersWithRoleSortedByNumberOfQuizzesCompletedWith({ + role: RolePersistenceModel.USER, + afterId: maybeAfterId, + limit: first, + currentUserId: afterIdOrCurrentUserId, + }); + data = result.data; + hasMoreRows = result.hasMoreRows; + } else { + const result = await this.#persistence.getUsersWithRole({ + role: RolePersistenceModel.USER, + afterId: afterIdOrCurrentUserId, + limit: first, + sortedBy, + }); + data = result.data; + hasMoreRows = result.hasMoreRows; + } + return { data: data.map((user) => this.#userPersistenceToUser(user)), hasMoreRows, }; } + /** + * Get the user with the given id. + * @param userId The id to load the user for. + * @returns The user with the given id. + * @throws UserNotFoundError when no user exists with the given id. + */ + async getUser(userId: string) { + const persistenceUser = await this.#persistence.getUserById(userId); + if (persistenceUser === null) { + throw new UserNotFoundError(`No user found with id ${userId}`); + } + return this.#userPersistenceToUser(persistenceUser); + } + #userPersistenceToUser(user: UserPersistenceModel): User { return { id: user.id, diff --git a/src/util/cache.test.ts b/src/util/cache.test.ts new file mode 100644 index 0000000..cad559f --- /dev/null +++ b/src/util/cache.test.ts @@ -0,0 +1,39 @@ +import { MemoryCache } from './cache'; + +describe('util', () => { + beforeEach(() => { + jest.useRealTimers(); + }); + describe('cache', () => { + it('must return undefined if the key does not exist', async () => { + const sut = new MemoryCache(); + const actual = await sut.getItem('key'); + expect(actual).toBeUndefined(); + }); + it('must return the value if the key exists', async () => { + const sut = new MemoryCache(); + await sut.setItem('key', 'value', 1000); + + const actual = await sut.getItem('key'); + + expect(actual).toBe('value'); + }); + it('must return undefined if the key has expired', async () => { + jest.useFakeTimers(); + jest.setSystemTime(new Date('2020-01-01')); + const sut = new MemoryCache(); + + await sut.setItem('key', 'value', 1000); + expect(await sut.getItem('key')).toBe('value'); + + jest.advanceTimersByTime(1001); + expect(await sut.getItem('key')).toBeUndefined(); + }); + it('must return undefined if the key has been expired manually', async () => { + const sut = new MemoryCache(); + await sut.setItem('key', 'value', 1000); + await sut.expireItem('key'); + expect(await sut.getItem('key')).toBeUndefined(); + }); + }); +}); diff --git a/src/util/cache.ts b/src/util/cache.ts index ea9a54f..cce04fd 100644 --- a/src/util/cache.ts +++ b/src/util/cache.ts @@ -20,9 +20,9 @@ export interface Cache { * * @param key The key to set the item for. * @param value The value to set. - * @param expiresInSeconds The number of seconds until the item expires. + * @param expiresInMillis The number of milliseconds until the item expires. */ - setItem(key: string, value: T, expiresInSeconds: number): Promise; + setItem(key: string, value: T, expiresInMillis: number): Promise; /** * Manually expire an item in the cache. @@ -45,8 +45,8 @@ export class MemoryCache implements Cache { } return Promise.resolve(record.value); } - setItem(key: string, value: T, expiresInSeconds: number): Promise { - const expiresAt = new Date(new Date().getTime() + expiresInSeconds * 1000); + setItem(key: string, value: T, expiresInMillis: number): Promise { + const expiresAt = new Date(new Date().getTime() + expiresInMillis); this.#values.set(key, { value, expiresAt,