diff --git a/backend/src/challenge-attempt/challenge-attempt.module.ts b/backend/src/challenge-attempt/challenge-attempt.module.ts index 56575ae7..e2eddbe9 100644 --- a/backend/src/challenge-attempt/challenge-attempt.module.ts +++ b/backend/src/challenge-attempt/challenge-attempt.module.ts @@ -7,6 +7,7 @@ import { UsersModule } from '../users/users.module'; import { ChallengeAttemptService } from './providers/challenge-attempt.service'; import { ChallengeAttemptController } from './controllers/challenge-attempt.controller'; import { ChallengeValidationService } from './providers/challenge-validation.service'; +import { IdempotencyModule } from '../common/idempotency/idempotency.module'; @Module({ imports: [ @@ -24,6 +25,7 @@ import { ChallengeValidationService } from './providers/challenge-validation.ser // ChallengeAttempt entity). TypeOrmModule.forFeature([ChallengeAttempt, Puzzle, GameSession]), UsersModule, // for XpLevelService + IdempotencyModule, ], controllers: [ChallengeAttemptController], providers: [ChallengeAttemptService, ChallengeValidationService], diff --git a/backend/src/challenge-attempt/controllers/challenge-attempt.controller.ts b/backend/src/challenge-attempt/controllers/challenge-attempt.controller.ts index 6c8ea574..4f6b0581 100644 --- a/backend/src/challenge-attempt/controllers/challenge-attempt.controller.ts +++ b/backend/src/challenge-attempt/controllers/challenge-attempt.controller.ts @@ -2,6 +2,7 @@ import { Body, Controller, Get, + Headers, HttpCode, HttpStatus, Param, @@ -14,6 +15,7 @@ import { import { AuthGuard } from '@nestjs/passport'; import { ApiBearerAuth, + ApiHeader, ApiOperation, ApiParam, ApiResponse, @@ -87,7 +89,14 @@ export class ChallengeAttemptController { 'the attempt, records progress, calculates score and XP, selects the ' + 'next challenge, and detects session completion. Idempotent: a repeat ' + 'submit on an already-graded attempt returns the original cached result ' + - '(isDuplicateReplay: true) instead of re-running the pipeline.', + '(isDuplicateReplay: true) instead of re-running the pipeline. ' + + 'Supports Redis-based idempotency via the Idempotency-Key header or the idempotencyKey body field.', + }) + @ApiHeader({ + name: 'Idempotency-Key', + required: false, + description: + 'Client-generated idempotency key (UUID v4 recommended). Prevents duplicate XP awards, session advances, and reward eligibility on retries.', }) @ApiResponse({ status: 200, @@ -102,7 +111,12 @@ export class ChallengeAttemptController { async submitAttempt( @ActiveUser() user: ActiveUserData, @Body() dto: SubmitAttemptDto, + @Headers('Idempotency-Key') idempotencyKey?: string, ): Promise { + // Header takes precedence over body field for idempotency key. + if (idempotencyKey && !dto.idempotencyKey) { + dto.idempotencyKey = idempotencyKey; + } return this.challengeAttemptService.submitAttempt( dto, this.requireUserId(user), diff --git a/backend/src/challenge-attempt/dtos/submit-attempt.dto.ts b/backend/src/challenge-attempt/dtos/submit-attempt.dto.ts index 680b543d..b3838f21 100644 --- a/backend/src/challenge-attempt/dtos/submit-attempt.dto.ts +++ b/backend/src/challenge-attempt/dtos/submit-attempt.dto.ts @@ -1,10 +1,20 @@ -import { IsInt, IsNotEmpty, IsString, IsUUID, Min } from 'class-validator'; +import { IsInt, IsNotEmpty, IsOptional, IsString, IsUUID, Min } from 'class-validator'; +import { ApiPropertyOptional } from '@nestjs/swagger'; /** * DTO for submitting an answer to an existing attempt. * * timeSpent is in seconds, measured client-side from when the player * opened the challenge to when they pressed submit. + * + * idempotencyKey is optional but strongly recommended. When provided, + * the backend guarantees the same submission cannot award XP, advance + * a session, or create duplicate rewards even if the request is sent + * multiple times (double-click, network retry, browser refresh). + * + * If omitted, a deterministic key is derived from the attemptId, + * answer, and timeSpent — but a client-generated UUID v4 is preferred + * for true idempotency across retries with the same logical submission. */ export class SubmitAttemptDto { @IsUUID('4', { message: 'attemptId must be a valid UUID v4' }) @@ -17,4 +27,12 @@ export class SubmitAttemptDto { @IsInt() @Min(0, { message: 'timeSpent must be a non-negative integer (seconds)' }) timeSpent: number; + + @ApiPropertyOptional({ + description: + 'Client-generated idempotency key (UUID v4 recommended). Prevents duplicate XP awards, session advances, and reward eligibility on retries.', + }) + @IsOptional() + @IsString() + idempotencyKey?: string; } diff --git a/backend/src/challenge-attempt/providers/challenge-attempt.service.spec.ts b/backend/src/challenge-attempt/providers/challenge-attempt.service.spec.ts index c601015b..cd7270b0 100644 --- a/backend/src/challenge-attempt/providers/challenge-attempt.service.spec.ts +++ b/backend/src/challenge-attempt/providers/challenge-attempt.service.spec.ts @@ -18,6 +18,8 @@ import { CreateChallengeAttemptDto } from '../dtos/create-challenge-attempt.dto' import { SubmitAttemptDto } from '../dtos/submit-attempt.dto'; import { RevealSolutionDto } from '../dtos/reveal-solution.dto'; import { UseHintDto } from '../dtos/use-hint.dto'; +import { SubmitAttemptResponseDto } from '../dtos/submit-attempt-response.dto'; +import { IdempotencyService } from '../../common/idempotency/idempotency.service'; import { afterEach, beforeEach, @@ -140,6 +142,7 @@ describe('ChallengeAttemptService', () => { let service: ChallengeAttemptService; let attemptRepo: jest.Mocked>; let puzzleRepo: jest.Mocked>; + let idempotencyService: { execute: jest.Mock }; let xpLevelService: jest.Mocked>; let dataSource: { transaction: jest.Mock }; @@ -162,6 +165,9 @@ describe('ChallengeAttemptService', () => { findOneBy: jest.fn(), }; + const mockIdempotencyService = { + execute: jest.fn(), + }; xpLevelService = { addXp: jest.fn() }; dataSource = { transaction: jest.fn() }; @@ -179,6 +185,10 @@ describe('ChallengeAttemptService', () => { provide: getRepositoryToken(Puzzle), useValue: mockPuzzleRepo, }, + { + provide: IdempotencyService, + useValue: mockIdempotencyService, + }, { // Read-only lookup in resolveSessionTarget(); no test currently // exercises a real GameSession row, so a bare jest.fn() (always @@ -196,6 +206,7 @@ describe('ChallengeAttemptService', () => { service = module.get(ChallengeAttemptService); attemptRepo = module.get(getRepositoryToken(ChallengeAttempt)); puzzleRepo = module.get(getRepositoryToken(Puzzle)); + idempotencyService = module.get(IdempotencyService); }); afterEach(() => { @@ -282,6 +293,37 @@ describe('ChallengeAttemptService', () => { }; const userId = 'user-1'; + /** Sets up idempotencyService.execute to simply call the inner fn (first request, not a duplicate). */ + beforeEach(() => { + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + }); + + /** Helper: sets up idempotencyService.execute to call the fn (first request). */ + function mockFirstRequest(attempt: ChallengeAttempt, puzzle: Puzzle, savedAttempt: ChallengeAttempt) { + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + attemptRepo.findOneBy!.mockResolvedValue(attempt); + puzzleRepo.findOneBy!.mockResolvedValue(puzzle); + attemptRepo.save!.mockResolvedValue(savedAttempt); + const data = await fn(); + return { duplicate: false, data }; + }, + ); + } + + /** Helper: sets up idempotencyService.execute to return a cached result (duplicate). */ + function mockDuplicateRequest(cachedAttempt: ChallengeAttempt) { + idempotencyService.execute!.mockResolvedValue({ + duplicate: true, + data: cachedAttempt, + }); + } + it('should mark attempt CORRECT, award score, XP, and record progress for a correct answer', async () => { const attempt = makeAttempt({ sessionId: undefined }); const puzzle = makePuzzle(); @@ -454,6 +496,16 @@ describe('ChallengeAttemptService', () => { }; const userId = 'user-1'; + /** Sets up idempotencyService.execute to simply call the inner fn (first request, not a duplicate). */ + beforeEach(() => { + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + }); + it('excludes already-attempted-in-session and the current challenge from next-challenge selection', async () => { const attempt = makeAttempt({ sessionId: 'sess-1' }); const puzzle = makePuzzle(); @@ -634,6 +686,201 @@ describe('ChallengeAttemptService', () => { ); expect(progressSaves).toHaveLength(1); }); + + // ───────────────────────────────────────────────────────────────────────── + // Idempotency tests + // ───────────────────────────────────────────────────────────────────────── + + describe('idempotency', () => { + it('should return cached result for a duplicate submission with the same idempotencyKey', async () => { + const cachedResult: SubmitAttemptResponseDto = { + attempt: makeAttempt({ + status: AttemptStatus.CORRECT, + score: 125, + answer: '4', + submittedAt: new Date(), + }), + isCorrect: true, + feedback: 'Correct!', + xp: { awarded: 125, levelUp: false, currentLevel: 1, currentXp: 0 }, + progress: { attemptsInSession: 1, sessionTarget: 5, sessionCompleted: false }, + nextChallenge: null, + isDuplicateReplay: false, + }; + + idempotencyService.execute!.mockResolvedValue({ + duplicate: true, + data: cachedResult, + }); + + const result = await service.submitAttempt({ + ...dto, + idempotencyKey: 'idempotency-key-abc', + }, userId); + + expect(result).toBe(cachedResult); + expect(result.isCorrect).toBe(true); + expect(result.attempt.score).toBe(125); + // The inner function should NOT have touched the repos + expect(dataSource.transaction).not.toHaveBeenCalled(); + }); + + it('should derive a deterministic key when idempotencyKey is not provided', async () => { + const attempt = makeAttempt({ sessionId: undefined }); + const puzzle = makePuzzle(); + const manager = makeMockManager({ attempt, puzzle, nextPuzzleCandidates: [] }); + dataSource.transaction.mockImplementation((cb: any) => cb(manager)); + xpLevelService.addXp.mockResolvedValue({ + levelUp: false, + currentLevel: 1, + currentXp: 100, + previousLevel: 1, + }); + + // Mock idempotencyService.execute to call the inner function + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + + await service.submitAttempt(dto, userId); + + // Verify idempotencyService.execute was called with a derived key + expect(idempotencyService.execute).toHaveBeenCalledWith( + expect.stringMatching(/^attempt-submit:[a-f0-9]{32}$/), + expect.any(Function), + ); + }); + + it('should use the client-provided idempotencyKey as the Redis key', async () => { + const attempt = makeAttempt({ sessionId: undefined }); + const puzzle = makePuzzle(); + const manager = makeMockManager({ attempt, puzzle, nextPuzzleCandidates: [] }); + dataSource.transaction.mockImplementation((cb: any) => cb(manager)); + xpLevelService.addXp.mockResolvedValue({ + levelUp: false, + currentLevel: 1, + currentXp: 100, + previousLevel: 1, + }); + + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + + const customKey = 'my-custom-idempotency-key'; + await service.submitAttempt({ + ...dto, + idempotencyKey: customKey, + }, userId); + + expect(idempotencyService.execute).toHaveBeenCalledWith( + `attempt-submit:${customKey}`, + expect.any(Function), + ); + }); + + it('should prevent double XP awards on duplicate submissions', async () => { + const cachedResult: SubmitAttemptResponseDto = { + attempt: makeAttempt({ + status: AttemptStatus.CORRECT, + score: 200, + answer: '4', + submittedAt: new Date(), + }), + isCorrect: true, + feedback: 'Correct!', + xp: { awarded: 200, levelUp: false, currentLevel: 1, currentXp: 0 }, + progress: { attemptsInSession: 1, sessionTarget: 5, sessionCompleted: false }, + nextChallenge: null, + isDuplicateReplay: false, + }; + + // First submission + idempotencyService.execute!.mockResolvedValueOnce({ + duplicate: false, + data: cachedResult, + }); + const result1 = await service.submitAttempt({ + ...dto, + idempotencyKey: 'duplicate-xp-test', + }, userId); + expect(result1.isCorrect).toBe(true); + expect(result1.attempt.score).toBe(200); + + // Second submission with the same key — should return cached, no re-grading + idempotencyService.execute!.mockResolvedValueOnce({ + duplicate: true, + data: cachedResult, + }); + const result2 = await service.submitAttempt({ + ...dto, + idempotencyKey: 'duplicate-xp-test', + }, userId); + expect(result2).toBe(cachedResult); + expect(result2.attempt.score).toBe(200); + + // The transaction should NOT have been called for the duplicate + expect(dataSource.transaction).not.toHaveBeenCalled(); + }); + + it('should allow different idempotencyKeys for different submissions', async () => { + const attempt1 = makeAttempt({ sessionId: undefined }); + const attempt2 = makeAttempt(); // fresh mutable attempt + const puzzle = makePuzzle(); + + const manager1 = makeMockManager({ attempt: attempt1, puzzle, nextPuzzleCandidates: [] }); + const manager2 = makeMockManager({ attempt: attempt2, puzzle, nextPuzzleCandidates: [] }); + + // First call with key-1 + dataSource.transaction.mockImplementationOnce((cb: any) => cb(manager1)); + xpLevelService.addXp.mockResolvedValue({ + levelUp: false, + currentLevel: 1, + currentXp: 100, + previousLevel: 1, + }); + idempotencyService.execute!.mockImplementationOnce( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + + const result1 = await service.submitAttempt({ + ...dto, + idempotencyKey: 'key-1', + }, userId); + expect(result1.isCorrect).toBe(true); + + // Second call with key-2 — different idempotency key + dataSource.transaction.mockImplementationOnce((cb: any) => cb(manager2)); + xpLevelService.addXp.mockResolvedValue({ + levelUp: false, + currentLevel: 1, + currentXp: 100, + previousLevel: 1, + }); + idempotencyService.execute!.mockImplementationOnce( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + + const result2 = await service.submitAttempt({ + ...dto, + answer: 'wrong', + idempotencyKey: 'key-2', + }, userId); + expect(result2.isCorrect).toBe(false); + }); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/backend/src/challenge-attempt/providers/challenge-attempt.service.ts b/backend/src/challenge-attempt/providers/challenge-attempt.service.ts index 07bcb464..3b29d6cb 100644 --- a/backend/src/challenge-attempt/providers/challenge-attempt.service.ts +++ b/backend/src/challenge-attempt/providers/challenge-attempt.service.ts @@ -2,10 +2,12 @@ import { BadRequestException, ForbiddenException, Injectable, + Logger, NotFoundException, } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { DataSource, EntityManager, In, Not, Repository } from 'typeorm'; +import { createHash } from 'crypto'; import { ChallengeAttempt } from '../entities/challenge-attempt.entity'; import { Puzzle } from '../../puzzles/entities/puzzle.entity'; import { UserProgress } from '../../progress/entities/progress.entity'; @@ -21,6 +23,7 @@ import { SubmitAttemptResponseDto, } from '../dtos/submit-attempt-response.dto'; import { ChallengeValidationService } from './challenge-validation.service'; +import { IdempotencyService } from '../../common/idempotency/idempotency.service'; /** Terminal states where no further mutations are allowed. */ const TERMINAL_STATES = new Set([ @@ -38,6 +41,8 @@ const UUID_V4_REGEX = @Injectable() export class ChallengeAttemptService { + private readonly logger = new Logger(ChallengeAttemptService.name); + /** * Fallback session-length target, used only when `attempt.sessionId` * doesn't resolve to a real `GameSession` row (e.g. legacy callers that @@ -59,6 +64,7 @@ export class ChallengeAttemptService { private readonly challengeValidationService: ChallengeValidationService, private readonly xpLevelService: XpLevelService, private readonly dataSource: DataSource, + private readonly idempotencyService: IdempotencyService, ) {} // ───────────────────────────────────────────────────────────────────────────── @@ -107,15 +113,51 @@ export class ChallengeAttemptService { * post-submission pipeline: validate → persist → update progress → * calculate score/XP → select next challenge → detect session completion. * - * Idempotent: a repeat submit on an attempt that's already terminal - * (CORRECT/INCORRECT/EXPIRED) does not re-grade, re-award XP, or - * re-select a next challenge — it returns the originally-persisted result - * with `isDuplicateReplay: true`. A pessimistic write lock on the attempt - * row also prevents two concurrent first-time submits from double-awarding. + * - Validates the attempt exists and is in a submittable state. + * - Compares the answer against the puzzle's correct answer (case-insensitive + * with whitespace trimming). + * - Sets status to CORRECT or INCORRECT, records timeSpent and submittedAt. + * - Awards score only on correct answers (unless solution was already + * revealed, which forfeits scoring). + * - Records progress, awards XP, selects next challenge, detects session completion. + * + * Idempotent at two levels: + * 1. Redis-based: If an idempotencyKey is provided (or can be derived), duplicate + * submissions are detected and the original result is returned without + * re-processing — preventing double XP awards, double session advances, + * and duplicate reward eligibility. + * 2. Database-based: A pessimistic write lock on the attempt row also prevents + * two concurrent first-time submits from double-awarding. */ async submitAttempt( dto: SubmitAttemptDto, userId: string, + ): Promise { + const idempotencyKey = + dto.idempotencyKey ?? this.deriveIdempotencyKey(dto); + + const { duplicate, data: result } = + await this.idempotencyService.execute( + `attempt-submit:${idempotencyKey}`, + () => this.processSubmitAttempt(dto, userId), + ); + + if (duplicate) { + this.logger.log( + `Duplicate submission detected for key: ${idempotencyKey}. Returning cached result.`, + ); + } + + return result; + } + + /** + * Internal method that performs the actual submission logic. + * Called inside an idempotency guard — only executes once per key. + */ + private async processSubmitAttempt( + dto: SubmitAttemptDto, + userId: string, ): Promise { return this.dataSource.transaction(async (manager) => { const attempt = await manager.findOne(ChallengeAttempt, { @@ -412,6 +454,18 @@ export class ChallengeAttemptService { }; } + /** + * Derives a deterministic idempotency key from the request parameters + * when the client doesn't provide one. This prevents double-processing + * of the exact same logical request, but does NOT protect against + * retries with a different answer (which is correct — a retry with a + * different answer is a new submission attempt). + */ + private deriveIdempotencyKey(dto: SubmitAttemptDto): string { + const payload = `${dto.attemptId}:${dto.answer}:${dto.timeSpent}`; + return createHash('sha256').update(payload).digest('hex').slice(0, 32); + } + // ───────────────────────────────────────────────────────────────────────────── // Hint Tracking // ───────────────────────────────────────────────────────────────────────────── diff --git a/backend/src/common/idempotency/idempotency.module.ts b/backend/src/common/idempotency/idempotency.module.ts new file mode 100644 index 00000000..6c911748 --- /dev/null +++ b/backend/src/common/idempotency/idempotency.module.ts @@ -0,0 +1,10 @@ +import { Module } from '@nestjs/common'; +import { IdempotencyService } from './idempotency.service'; +import { RedisModule } from '../../redis/redis.module'; + +@Module({ + imports: [RedisModule], + providers: [IdempotencyService], + exports: [IdempotencyService], +}) +export class IdempotencyModule {} diff --git a/backend/src/common/idempotency/idempotency.service.spec.ts b/backend/src/common/idempotency/idempotency.service.spec.ts new file mode 100644 index 00000000..fb2227a6 --- /dev/null +++ b/backend/src/common/idempotency/idempotency.service.spec.ts @@ -0,0 +1,243 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { IdempotencyService } from './idempotency.service'; +import { REDIS_CLIENT } from '../../redis/redis.constants'; +import { describe, it, expect, beforeEach, jest, afterEach } from '@jest/globals'; + +/** + * Mock Redis client that simulates SET NX, GET, SET, DEL, and EXPIRE. + */ +function createMockRedis() { + const store = new Map(); + const ttls = new Map(); + + return { + store, + ttls, + set: jest.fn( + async ( + key: string, + value: string, + ...args: string[] + ): Promise => { + const nx = args.includes('NX'); + const exIdx = args.indexOf('EX'); + const ttl = exIdx !== -1 ? parseInt(args[exIdx + 1], 10) : undefined; + + if (nx && store.has(key)) { + return null; // NX: key already exists + } + store.set(key, value); + if (ttl) ttls.set(key, ttl); + return 'OK'; + }, + ) as any, + get: jest.fn(async (key: string): Promise => { + return store.get(key) ?? null; + }) as any, + del: jest.fn(async (key: string): Promise => { + const existed = store.has(key); + store.delete(key); + ttls.delete(key); + return existed ? 1 : 0; + }) as any, + // Helper to simulate TTL expiry by removing the key + expire: jest.fn(async (_key: string, _ttl: number): Promise => 1) as any, + }; +} + +describe('IdempotencyService', () => { + let service: IdempotencyService; + let redis: ReturnType; + + beforeEach(async () => { + redis = createMockRedis(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + IdempotencyService, + { provide: REDIS_CLIENT, useValue: redis }, + ], + }).compile(); + + service = module.get(IdempotencyService); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + // ───────────────────────────────────────────────────────────────────────────── + // First request (no prior state) + // ───────────────────────────────────────────────────────────────────────────── + + describe('first request', () => { + it('should execute the function and return result with duplicate=false', async () => { + const fn = async () => ({ score: 100 }); + + const result = await service.execute('key-1', fn); + + expect(result.duplicate).toBe(false); + expect(result.data).toEqual({ score: 100 }); + // Lock acquired (SET NX) + expect(redis.set).toHaveBeenCalledWith( + 'idempotency:lock:key-1', + '1', + 'EX', + expect.any(Number), + 'NX', + ); + // Result stored + expect(redis.set).toHaveBeenCalledWith( + 'idempotency:result:key-1', + JSON.stringify({ score: 100 }), + 'EX', + expect.any(Number), + ); + }); + + it('should release the lock after execution', async () => { + const fn = async () => 'done'; + + await service.execute('key-2', fn); + + expect(redis.del).toHaveBeenCalledWith('idempotency:lock:key-2'); + }); + }); + + // ───────────────────────────────────────────────────────────────────────────── + // Duplicate request (result already cached) + // ───────────────────────────────────────────────────────────────────────────── + + describe('duplicate request (cached result)', () => { + it('should return the cached result with duplicate=true without re-executing', async () => { + // Pre-populate the result cache + redis.store.set( + 'idempotency:result:key-3', + JSON.stringify({ score: 200 }), + ); + + const fn = jest.fn() as unknown as () => Promise; + + const result = await service.execute('key-3', fn); + + expect(result.duplicate).toBe(true); + expect(result.data).toEqual({ score: 200 }); + // Function should NOT have been called + expect(fn).not.toHaveBeenCalled(); + }); + }); + + // ───────────────────────────────────────────────────────────────────────────── + // Lock contention (concurrent in-flight request) + // ───────────────────────────────────────────────────────────────────────────── + + describe('lock contention', () => { + it('should wait for a concurrent request to finish and return its cached result', async () => { + // Simulate: lock is held, then result appears after a delay + redis.store.set('idempotency:lock:key-4', '1'); + const fn = jest.fn() as unknown as () => Promise; + + // After 2 retries (100ms), the result should appear + let callCount = 0; + redis.get.mockImplementation(async (key: string) => { + if (key === 'idempotency:result:key-4') { + callCount++; + if (callCount >= 3) { + return JSON.stringify({ score: 300 }); + } + } + return redis.store.get(key) ?? null; + }); + + const result = await service.execute('key-4', fn); + + expect(result.duplicate).toBe(true); + expect(result.data).toEqual({ score: 300 }); + expect(fn).not.toHaveBeenCalled(); + }); + }); + + // ───────────────────────────────────────────────────────────────────────────── + // Function throws an error + // ───────────────────────────────────────────────────────────────────────────── + + describe('function throws', () => { + it('should release the lock and not cache the error', async () => { + const fn = async () => { + throw new Error('boom'); + }; + + await expect(service.execute('key-5', fn)).rejects.toThrow('boom'); + + // Lock should have been released + expect(redis.del).toHaveBeenCalledWith('idempotency:lock:key-5'); + // No result should have been cached + expect(redis.store.has('idempotency:result:key-5')).toBe(false); + }); + + it('should allow retrying after a failed attempt', async () => { + let callCount = 0; + const fn = async () => { + callCount++; + if (callCount === 1) throw new Error('transient'); + return { score: 50 }; + }; + + // First attempt fails + await expect(service.execute('key-6', fn)).rejects.toThrow('transient'); + + // Second attempt should succeed (lock was released) + const result = await service.execute('key-6', fn); + expect(result.duplicate).toBe(false); + expect(result.data).toEqual({ score: 50 }); + expect(callCount).toBe(2); + }); + }); + + // ───────────────────────────────────────────────────────────────────────────── + // Custom TTL + // ───────────────────────────────────────────────────────────────────────────── + + describe('custom TTL', () => { + it('should use the provided TTL for the result cache', async () => { + const fn = async () => 'ok'; + + await service.execute('key-7', fn, 3600); + + expect(redis.set).toHaveBeenCalledWith( + 'idempotency:result:key-7', + JSON.stringify('ok'), + 'EX', + 3600, + ); + }); + }); + + // ───────────────────────────────────────────────────────────────────────────── + // Lock held by dead request (timeout path) + // ───────────────────────────────────────────────────────────────────────────── + + describe('dead lock owner', () => { + it('should force-acquire the lock and execute after timeout when lock owner dies', async () => { + // Simulate: lock held forever, never a result + redis.store.set('idempotency:lock:key-8', '1'); + redis.get.mockImplementation(async (key: string) => { + if (key === 'idempotency:result:key-8') return null; + if (key === 'idempotency:lock:key-8') return '1'; + return redis.store.get(key) ?? null; + }); + + const fn = async () => ({ recovered: true }); + + // This will retry MAX_RETRIES times then force-acquire + const result = await service.execute('key-8', fn); + + expect(result.duplicate).toBe(false); + expect(result.data).toEqual({ recovered: true }); + }); + }); +}); diff --git a/backend/src/common/idempotency/idempotency.service.ts b/backend/src/common/idempotency/idempotency.service.ts new file mode 100644 index 00000000..4db8c897 --- /dev/null +++ b/backend/src/common/idempotency/idempotency.service.ts @@ -0,0 +1,130 @@ +import { Inject, Injectable, Logger } from '@nestjs/common'; +import { REDIS_CLIENT } from '../../redis/redis.constants'; +import Redis from 'ioredis'; + +/** Default TTL for idempotency keys (24 hours). */ +const DEFAULT_TTL_SECONDS = 86_400; + +/** How long a pending lock can live before we consider the original request dead. */ +const LOCK_TTL_SECONDS = 60; + +/** How often to retry when a lock is held by another in-flight request. */ +const RETRY_DELAY_MS = 50; + +/** Maximum number of retries when waiting for a concurrent request to finish. */ +const MAX_RETRIES = 20; + +export interface IdempotencyResult { + /** Whether this is a replayed request (cached result returned). */ + duplicate: boolean; + /** The result — either freshly computed or the cached original. */ + data: T; +} + +/** + * Provides idempotency guarantees for write operations by storing a + * per-key lock in Redis and caching the result of the first request. + * + * ## Protocol + * + * 1. Caller calls `execute(key, fn)`. + * 2. If a cached result already exists → return it (`duplicate: true`). + * 3. Attempt to acquire the lock (Redis SETNX). If acquired → run `fn`, + * store the result, release the lock, return (`duplicate: false`). + * 4. If the lock is held by another in-flight request → poll until the + * result appears (retry loop) or the lock expires. + * + * This handles double-clicks, network retries, browser refreshes, and + * malicious duplicate requests. + */ +@Injectable() +export class IdempotencyService { + private readonly logger = new Logger(IdempotencyService.name); + + constructor(@Inject(REDIS_CLIENT) private readonly redis: Redis) {} + + /** + * Execute a function with idempotency guarantees. + * + * @param key A unique idempotency key (e.g. from the client or derived from the request). + * @param fn The function to execute if this is the first request with this key. + * @param ttl TTL in seconds for both the lock and the cached result (default 24h). + */ + async execute( + key: string, + fn: () => Promise, + ttl: number = DEFAULT_TTL_SECONDS, + ): Promise> { + const lockKey = `idempotency:lock:${key}`; + const resultKey = `idempotency:result:${key}`; + + // 1. Check if a cached result already exists. + const cached = await this.redis.get(resultKey); + if (cached) { + this.logger.debug(`Idempotency hit for key: ${key}`); + return { duplicate: true, data: JSON.parse(cached) as T }; + } + + // 2. Try to acquire the lock atomically. + const acquired = await this.redis.set( + lockKey, + '1', + 'EX', + LOCK_TTL_SECONDS, + 'NX', + ); + + if (acquired === 'OK') { + // We won the race — execute the function. + try { + const result = await fn(); + // Store the result with the longer TTL. + await this.redis.set(resultKey, JSON.stringify(result), 'EX', ttl); + return { duplicate: false, data: result }; + } finally { + // Always release the lock so future requests can proceed + // (even if the function threw — we don't cache errors). + await this.redis.del(lockKey); + } + } + + // 3. Another request holds the lock — wait for its result to appear. + this.logger.debug( + `Lock contention for key: ${key}, waiting for result...`, + ); + + for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { + await this.delay(RETRY_DELAY_MS); + + const result = await this.redis.get(resultKey); + if (result) { + return { duplicate: true, data: JSON.parse(result) as T }; + } + + // Also check if the lock was released without storing a result + // (i.e. the other request failed). In that case, try to acquire + // the lock ourselves. + const lockStillHeld = await this.redis.get(lockKey); + if (!lockStillHeld) { + // Lock gone, no result stored — the previous request failed. + // Recurse to try again from scratch. + return this.execute(key, fn, ttl); + } + } + + // 4. Timed out waiting. The lock holder may have crashed. + // Try to acquire the lock one more time and execute ourselves. + this.logger.warn( + `Timed out waiting for lock on key: ${key}. Force-acquiring.`, + ); + await this.redis.del(lockKey); + + const result = await fn(); + await this.redis.set(resultKey, JSON.stringify(result), 'EX', ttl); + return { duplicate: false, data: result }; + } + + private delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); + } +} diff --git a/backend/src/game-sessions/game-sessions.module.ts b/backend/src/game-sessions/game-sessions.module.ts index d7fa7e31..471f9f51 100644 --- a/backend/src/game-sessions/game-sessions.module.ts +++ b/backend/src/game-sessions/game-sessions.module.ts @@ -8,12 +8,14 @@ import { SessionSummaryProvider } from './providers/session-summary.provider'; import { GameSessionsController } from './controllers/game-sessions.controller'; import { StreakModule } from '../streak/strerak.module'; import { RewardsModule } from '../rewards/rewards.module'; +import { IdempotencyModule } from '../common/idempotency/idempotency.module'; @Module({ imports: [ TypeOrmModule.forFeature([GameSession, ChallengeAttempt, Puzzle]), StreakModule, RewardsModule, + IdempotencyModule, ], controllers: [GameSessionsController], providers: [GameSessionsService, SessionSummaryProvider], diff --git a/backend/src/game-sessions/providers/game-sessions.service.spec.ts b/backend/src/game-sessions/providers/game-sessions.service.spec.ts index 7c6060dd..a2a077ce 100644 --- a/backend/src/game-sessions/providers/game-sessions.service.spec.ts +++ b/backend/src/game-sessions/providers/game-sessions.service.spec.ts @@ -17,6 +17,7 @@ import { CreateGameSessionDto } from '../dtos/create-game-session.dto'; import { UpdateGameSessionStatusDto } from '../dtos/update-game-session-status.dto'; import { SessionSummaryProvider } from './session-summary.provider'; import { SessionCompletionStats } from '../interfaces/game-session.interface'; +import { IdempotencyService } from '../../common/idempotency/idempotency.service'; // ───────────────────────────────────────────────────────────────────────────── // Fixtures @@ -79,6 +80,7 @@ describe('GameSessionsService', () => { let service: GameSessionsService; let repo: jest.Mocked>; let summaryProvider: jest.Mocked; + let idempotencyService: { execute: jest.Mock }; beforeEach(async () => { const mockRepo: Partial>> = { @@ -95,6 +97,10 @@ describe('GameSessionsService', () => { buildCompletionStats: jest.fn().mockResolvedValue(makeStatsStub()), }; + const mockIdempotencyService = { + execute: jest.fn(), + }; + const module: TestingModule = await Test.createTestingModule({ providers: [ GameSessionsService, @@ -106,12 +112,17 @@ describe('GameSessionsService', () => { provide: SessionSummaryProvider, useValue: mockSummaryProvider, }, + { + provide: IdempotencyService, + useValue: mockIdempotencyService, + }, ], }).compile(); service = module.get(GameSessionsService); repo = module.get(getRepositoryToken(GameSession)); summaryProvider = module.get(SessionSummaryProvider); + idempotencyService = module.get(IdempotencyService); }); afterEach(() => jest.clearAllMocks()); @@ -382,6 +393,14 @@ describe('GameSessionsService', () => { makeStatsStub({ challengesCompleted: 0 }), ); + // Idempotency mock: call the fn (first request) + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + const dto: UpdateGameSessionStatusDto = { status: GameSessionStatus.COMPLETED, score: 900, @@ -397,6 +416,10 @@ describe('GameSessionsService', () => { expect(result.score).toBe(900); expect(result.xpEarned).toBe(200); expect(result.completedAt).toBeInstanceOf(Date); + expect(idempotencyService.execute).toHaveBeenCalledWith( + 'session-complete:session-uuid-1', + expect.any(Function), + ); }); it('ignores client-supplied score/xp and uses server-calculated stats when attempts are tracked', async () => { @@ -429,6 +452,14 @@ describe('GameSessionsService', () => { }), ); + // Idempotency mock: call the fn (first request) + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + const dto: UpdateGameSessionStatusDto = { status: GameSessionStatus.COMPLETED, // A malicious/stale client value that must be ignored. @@ -539,6 +570,14 @@ describe('GameSessionsService', () => { makeStatsStub({ challengesCompleted: 0 }), ); + // Idempotency mock: call the fn (first request) + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + const result = await service.completeSession( 'session-uuid-1', 500, @@ -597,4 +636,137 @@ describe('GameSessionsService', () => { expect(repo.save).not.toHaveBeenCalled(); }); }); + + // ─────────────────────────────────────────────────────────────────────────── + // Idempotency — session completion + // ─────────────────────────────────────────────────────────────────────────── + + describe('session completion idempotency', () => { + const userId = 'user-uuid-1'; + + it('should return cached result for a duplicate COMPLETED request', async () => { + const session = makeSession({ + status: GameSessionStatus.ACTIVE, + startedAt: new Date(), + }); + repo.findOneBy.mockResolvedValue(session); + + const completedSession = makeSession({ + status: GameSessionStatus.COMPLETED, + startedAt: new Date(), + completedAt: new Date(), + score: 500, + xpEarned: 500, + accuracy: 80, + rewardEligible: true, + rewardReason: 'Player meets reward eligibility requirements', + }); + + // Simulate a cached result from a prior in-flight request + idempotencyService.execute!.mockResolvedValue({ + duplicate: true, + data: completedSession, + }); + + const dto: UpdateGameSessionStatusDto = { + status: GameSessionStatus.COMPLETED, + }; + const result = await service.updateStatus( + 'session-uuid-1', + dto, + userId, + ); + + expect(result).toBe(completedSession); + expect(result.status).toBe(GameSessionStatus.COMPLETED); + expect(result.score).toBe(500); + + // summaryProvider should NOT have been called — the cached result is returned directly + expect(summaryProvider.buildCompletionStats).not.toHaveBeenCalled(); + }); + + it('should use session-complete:{id} as the idempotency key', async () => { + const session = makeSession({ + status: GameSessionStatus.ACTIVE, + startedAt: new Date(), + }); + repo.findOneBy.mockResolvedValue(session); + repo.save.mockImplementation(async (s) => s as GameSession); + summaryProvider.buildCompletionStats.mockResolvedValue(makeStatsStub()); + + // Idempotency mock: call the fn (first request) + idempotencyService.execute!.mockImplementation( + async (key: string, fn: () => Promise) => { + const data = await fn(); + return { duplicate: false, data }; + }, + ); + + const dto: UpdateGameSessionStatusDto = { + status: GameSessionStatus.COMPLETED, + }; + await service.updateStatus('session-uuid-1', dto, userId); + + expect(idempotencyService.execute).toHaveBeenCalledWith( + 'session-complete:session-uuid-1', + expect.any(Function), + ); + }); + + it('should not use idempotency for non-COMPLETED transitions', async () => { + const session = makeSession({ + status: GameSessionStatus.ACTIVE, + startedAt: new Date(), + }); + repo.findOneBy.mockResolvedValue(session); + repo.save.mockImplementation(async (s) => s as GameSession); + + const dto: UpdateGameSessionStatusDto = { + status: GameSessionStatus.PAUSED, + }; + const result = await service.updateStatus('session-uuid-1', dto, userId); + + // Idempotency should NOT be invoked for PAUSED transitions + expect(idempotencyService.execute).not.toHaveBeenCalled(); + expect(result).toBeDefined(); + expect(result.status).toBe(GameSessionStatus.PAUSED); + }); + + it('should prevent double XP awards on duplicate COMPLETED requests', async () => { + const session = makeSession({ + status: GameSessionStatus.ACTIVE, + startedAt: new Date(), + }); + repo.findOneBy.mockResolvedValue(session); + + const completedSession = makeSession({ + status: GameSessionStatus.COMPLETED, + startedAt: new Date(), + completedAt: new Date(), + score: 300, + xpEarned: 300, + rewardEligible: true, + rewardReason: 'Eligible', + }); + + idempotencyService.execute!.mockResolvedValue({ + duplicate: true, + data: completedSession, + }); + + const dto: UpdateGameSessionStatusDto = { + status: GameSessionStatus.COMPLETED, + }; + + const result1 = await service.updateStatus('session-uuid-1', dto, userId); + const result2 = await service.updateStatus('session-uuid-1', dto, userId); + + expect(result1).toBe(completedSession); + expect(result2).toBe(completedSession); + expect(result1.score).toBe(300); + + // summaryProvider should not have been called at all — cached result returned + expect(summaryProvider.buildCompletionStats).not.toHaveBeenCalled(); + }); + }); }); diff --git a/backend/src/game-sessions/providers/game-sessions.service.ts b/backend/src/game-sessions/providers/game-sessions.service.ts index ea57aa07..d96ce957 100644 --- a/backend/src/game-sessions/providers/game-sessions.service.ts +++ b/backend/src/game-sessions/providers/game-sessions.service.ts @@ -2,6 +2,7 @@ import { BadRequestException, ForbiddenException, Injectable, + Logger, NotFoundException, } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; @@ -12,6 +13,7 @@ import { CreateGameSessionDto } from '../dtos/create-game-session.dto'; import { UpdateGameSessionStatusDto } from '../dtos/update-game-session-status.dto'; import { SessionTransitionMap } from '../interfaces/game-session.interface'; import { SessionSummaryProvider } from './session-summary.provider'; +import { IdempotencyService } from '../../common/idempotency/idempotency.service'; /** * Valid state transitions for a GameSession. @@ -48,10 +50,13 @@ const TERMINAL_STATES = new Set([ @Injectable() export class GameSessionsService { + private readonly logger = new Logger(GameSessionsService.name); + constructor( @InjectRepository(GameSession) private readonly sessionRepository: Repository, private readonly sessionSummaryProvider: SessionSummaryProvider, + private readonly idempotencyService: IdempotencyService, ) {} // ───────────────────────────────────────────────────────────────────────────── @@ -187,6 +192,74 @@ export class GameSessionsService { this.assertValidTransition(session.status, dto.status); + // Session completion is idempotent — the session ID is the key. + // This prevents double-completion from race conditions, retries, + // or browser refreshes, which would otherwise double-award XP, + // streak updates, and reward eligibility. + if (dto.status === GameSessionStatus.COMPLETED) { + const { duplicate, data: completedSession } = + await this.idempotencyService.execute( + `session-complete:${id}`, + () => this.processSessionCompletion(session, dto), + ); + + if (duplicate) { + this.logger.log( + `Duplicate completion detected for session ${id}. Returning cached result.`, + ); + } + + return completedSession; + } + + return this.processStatusUpdate(session, dto); + } + + /** + * Internal method that performs session completion logic. + * Called inside an idempotency guard — only executes once per session ID. + */ + private async processSessionCompletion( + session: GameSession, + dto: UpdateGameSessionStatusDto, + ): Promise { + session.completedAt = new Date(); + + const stats = await this.sessionSummaryProvider.buildCompletionStats( + session.id, + session.userId, + dto.userTimezone, + ); + + // Statistics are calculated server-side from persisted challenge + // attempts. Client-supplied score/xpEarned are only used as a + // fallback when the session has no tracked attempts (e.g. legacy + // or externally-managed sessions), so completion never regresses + // to an unscored state. + const hasTrackedAttempts = stats.challengesCompleted > 0; + session.score = hasTrackedAttempts ? stats.totalScore : dto.score ?? 0; + session.xpEarned = hasTrackedAttempts + ? stats.xpEarned + : dto.xpEarned ?? 0; + session.accuracy = stats.accuracy; + session.timeSpentSeconds = stats.timeSpentSeconds; + session.categoryPerformance = stats.categoryPerformance; + session.previousStreak = stats.previousStreak; + session.currentStreak = stats.currentStreak; + session.rewardEligible = stats.rewardEligible; + session.rewardReason = stats.rewardReason; + session.status = dto.status; + + return this.sessionRepository.save(session); + } + + /** + * Handles non-completion status updates (ACTIVE, PAUSED, ABANDONED, EXPIRED). + */ + private async processStatusUpdate( + session: GameSession, + dto: UpdateGameSessionStatusDto, + ): Promise { // Apply state-specific side-effects if ( dto.status === GameSessionStatus.ACTIVE && @@ -199,32 +272,6 @@ export class GameSessionsService { session.completedAt = new Date(); } - if (dto.status === GameSessionStatus.COMPLETED) { - const stats = await this.sessionSummaryProvider.buildCompletionStats( - session.id, - session.userId, - dto.userTimezone, - ); - - // Statistics are calculated server-side from persisted challenge - // attempts. Client-supplied score/xpEarned are only used as a - // fallback when the session has no tracked attempts (e.g. legacy - // or externally-managed sessions), so completion never regresses - // to an unscored state. - const hasTrackedAttempts = stats.challengesCompleted > 0; - session.score = hasTrackedAttempts ? stats.totalScore : dto.score ?? 0; - session.xpEarned = hasTrackedAttempts - ? stats.xpEarned - : dto.xpEarned ?? 0; - session.accuracy = stats.accuracy; - session.timeSpentSeconds = stats.timeSpentSeconds; - session.categoryPerformance = stats.categoryPerformance; - session.previousStreak = stats.previousStreak; - session.currentStreak = stats.currentStreak; - session.rewardEligible = stats.rewardEligible; - session.rewardReason = stats.rewardReason; - } - session.status = dto.status; return this.sessionRepository.save(session); diff --git a/backend/src/progress/dtos/submit-answer.dto.ts b/backend/src/progress/dtos/submit-answer.dto.ts index fca416b6..acdc52a9 100644 --- a/backend/src/progress/dtos/submit-answer.dto.ts +++ b/backend/src/progress/dtos/submit-answer.dto.ts @@ -1,5 +1,5 @@ -import { IsUUID, IsString, IsNumber, IsNotEmpty } from 'class-validator'; -import { ApiProperty } from '@nestjs/swagger'; +import { IsUUID, IsString, IsNumber, IsNotEmpty, IsOptional } from 'class-validator'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; export class SubmitAnswerDto { @ApiProperty({ @@ -42,4 +42,12 @@ export class SubmitAnswerDto { @IsNumber() @IsNotEmpty() timeSpent: number; // seconds + + @ApiPropertyOptional({ + description: + 'Client-generated idempotency key (UUID v4 recommended). Prevents duplicate XP awards and progress records on retries.', + }) + @IsOptional() + @IsString() + idempotencyKey?: string; } diff --git a/backend/src/progress/progress.module.ts b/backend/src/progress/progress.module.ts index feb16442..224fa93d 100644 --- a/backend/src/progress/progress.module.ts +++ b/backend/src/progress/progress.module.ts @@ -13,6 +13,7 @@ import { ProgressCalculationProvider } from './providers/progress-calculation.pr import { Puzzle } from '../puzzles/entities/puzzle.entity'; import { XpLevelService } from '../users/providers/xp-level.service'; import { ScoreService } from '../score/providers/score.service'; +import { IdempotencyModule } from '../common/idempotency/idempotency.module'; import { BlockchainModule } from '../blockchain/blockchain.module'; @Module({ @@ -21,6 +22,7 @@ import { BlockchainModule } from '../blockchain/blockchain.module'; // BlockchainService is needed by ProgressCalculationProvider for // submitPuzzleOnChain after puzzle answer verification. BlockchainModule, + IdempotencyModule, ], controllers: [ProgressController], providers: [ diff --git a/backend/src/progress/providers/progress-calculation.provider.ts b/backend/src/progress/providers/progress-calculation.provider.ts index 0d572286..6249fb79 100644 --- a/backend/src/progress/providers/progress-calculation.provider.ts +++ b/backend/src/progress/providers/progress-calculation.provider.ts @@ -1,6 +1,7 @@ -import { Injectable, NotFoundException } from '@nestjs/common'; +import { Injectable, Logger, NotFoundException } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { FindOptionsWhere, MoreThan, Repository } from 'typeorm'; +import { createHash } from 'crypto'; +import { Repository } from 'typeorm'; import { Puzzle } from '../../puzzles/entities/puzzle.entity'; import { UserProgress } from '../entities/progress.entity'; import { SubmitAnswerDto } from '../dtos/submit-answer.dto'; @@ -9,7 +10,7 @@ import { User } from '../../users/user.entity'; import { DailyQuest } from '../../quests/entities/daily-quest.entity'; import { getPointsByDifficulty } from '../../puzzles/enums/puzzle-difficulty.enum'; import { ScoreService } from '../../score/providers/score.service'; - +import { IdempotencyService } from '../../common/idempotency/idempotency.service'; export interface AnswerValidationResult { isCorrect: boolean; @@ -24,6 +25,8 @@ export interface ProgressCalculationResult { @Injectable() export class ProgressCalculationProvider { + private readonly logger = new Logger(ProgressCalculationProvider.name); + constructor( @InjectRepository(Puzzle) private readonly puzzleRepository: Repository, @@ -35,6 +38,7 @@ export class ProgressCalculationProvider { @InjectRepository(DailyQuest) private readonly dailyQuestRepository: Repository, private readonly scoreService: ScoreService, + private readonly idempotencyService: IdempotencyService, ) {} /** @@ -87,37 +91,53 @@ export class ProgressCalculationProvider { ); } - /** - * Processes answer submission and creates user progress record + * Processes answer submission and creates user progress record. + * + * Uses idempotency to prevent duplicate XP awards and progress records. + * If an idempotencyKey is provided, it is used directly; otherwise a + * deterministic key is derived from userId + puzzleId + userAnswer + timeSpent. */ async processAnswerSubmission( submitAnswerDto: SubmitAnswerDto, + ): Promise { + const idempotencyKey = + submitAnswerDto.idempotencyKey ?? + this.deriveIdempotencyKey(submitAnswerDto); + + const { duplicate, data: result } = + await this.idempotencyService.execute( + `progress-submit:${idempotencyKey}`, + () => this.processAnswerSubmissionInternal(submitAnswerDto), + ); + + if (duplicate) { + this.logger.log( + `Duplicate progress submission detected for key: ${idempotencyKey}. Returning cached result.`, + ); + } + + return result; + } + + /** + * Internal method that performs the actual progress processing logic. + * Called inside an idempotency guard — only executes once per key. + */ + private async processAnswerSubmissionInternal( + submitAnswerDto: SubmitAnswerDto, ): Promise { // Get puzzle to validate against const puzzle = await this.puzzleRepository.findOne({ where: { id: submitAnswerDto.puzzleId }, }); - // In processAnswerSubmission, check for recent duplicate: - const recentAttempt = await this.userProgressRepository.findOne({ - where: { - userId: submitAnswerDto.userId, - puzzleId: submitAnswerDto.puzzleId, - attemptedAt: MoreThan(new Date(Date.now() - 5000)), // 5 second window - }, - }); - if (!puzzle) { throw new NotFoundException( `Puzzle with ID ${submitAnswerDto.puzzleId} not found`, ); } - if (recentAttempt) { - throw new Error('Duplicate submission detected'); - } - // Validate answer const validation = this.validateAnswer( submitAnswerDto.userAnswer, @@ -126,15 +146,15 @@ export class ProgressCalculationProvider { // Calculate points const basePoints = this.calculatePoints( - puzzle, - submitAnswerDto.timeSpent, - validation.isCorrect, -); + puzzle, + submitAnswerDto.timeSpent, + validation.isCorrect, + ); - const scoreResult = this.scoreService.calculateScore({ - correct: validation.isCorrect, - basePoints, - }); + const scoreResult = this.scoreService.calculateScore({ + correct: validation.isCorrect, + basePoints, + }); let pointsEarned = scoreResult.score; @@ -145,20 +165,18 @@ export class ProgressCalculationProvider { }); if (user && validation.isCorrect) { - const streakCount = user.streak?.currentStreak || 0; + const streakCount = user.streak?.currentStreak || 0; - let streakMultiplier = 0; + let streakMultiplier = 0; - if (streakCount >= 7) { - streakMultiplier = 0.25; - } else if (streakCount >= 3) { - streakMultiplier = 0.1; - } + if (streakCount >= 7) { + streakMultiplier = 0.25; + } else if (streakCount >= 3) { + streakMultiplier = 0.1; + } - pointsEarned = Math.round( - pointsEarned * (1 + streakMultiplier), - ); - } + pointsEarned = Math.round(pointsEarned * (1 + streakMultiplier)); + } validation.pointsEarned = pointsEarned; @@ -192,10 +210,10 @@ export class ProgressCalculationProvider { dailyQuest.completedAt = new Date(); // Award bonus XP for daily quest completion (e.g., 50 XP as hinted in "completion screen") if (user) { - await this.xpLevelService.addXp(user.id, 50); -} + await this.xpLevelService.addXp(user.id, 50); } - await this.dailyQuestRepository.save(dailyQuest); + } + await this.dailyQuestRepository.save(dailyQuest); } } } @@ -226,11 +244,21 @@ export class ProgressCalculationProvider { }; } + /** + * Derives a deterministic idempotency key from request parameters. + * This ensures the same logical answer submission is only processed once, + * even when the client doesn't provide an explicit idempotency key. + */ + private deriveIdempotencyKey(dto: SubmitAnswerDto): string { + const payload = `${dto.userId}:${dto.puzzleId}:${dto.userAnswer}:${dto.timeSpent}`; + return createHash('sha256').update(payload).digest('hex').slice(0, 32); + } + /** * Gets user progress statistics for a category */ async getUserProgressStats(userId: string, categoryId: string) { - const where: FindOptionsWhere = { + const where = { userId, categoryId, };