diff --git a/backend/src/migration/interfaces/puzzle.interface.ts b/backend/src/migration/interfaces/puzzle.interface.ts index 7515e45f..413126b4 100644 --- a/backend/src/migration/interfaces/puzzle.interface.ts +++ b/backend/src/migration/interfaces/puzzle.interface.ts @@ -52,6 +52,11 @@ export interface MigrationResult { failedInserts: number; duplicatesSkipped: number; }; + /** + * True when the import was aborted mid-way and the whole batch was + * rolled back, so no partial data was persisted. + */ + rolledBack: boolean; errors: MigrationError[]; uploadInfo: { filename: string; diff --git a/backend/src/migration/services/migration.service.spec.ts b/backend/src/migration/services/migration.service.spec.ts new file mode 100644 index 00000000..6952d712 --- /dev/null +++ b/backend/src/migration/services/migration.service.spec.ts @@ -0,0 +1,142 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { DataSource } from 'typeorm'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { MigrationService } from './migration.service'; +import { Puzzle } from '../entities/puzzle.entity'; +import { PuzzleData } from '../interfaces/puzzle.interface'; + +function makePuzzle(title: string, category = 'math'): PuzzleData { + return { + title, + category, + difficulty: 'easy', + content: { + question: 'Question for ' + title, + answer: '42', + type: 'text', + }, + }; +} + +describe('MigrationService (atomic / resumable import)', () => { + let service: MigrationService; + let dataSource: { + transaction: jest.Mock; + }; + let puzzleRepository: { + findOne: jest.Mock; + create: jest.Mock; + save: jest.Mock; + }; + + // Shared manager object handed to the transaction callback. It returns + // the same fake puzzle repository regardless of which repository is + // requested, so state can be inspected after a run. + const buildManager = () => ({ + getRepository: jest.fn(() => puzzleRepository), + }); + + beforeEach(async () => { + dataSource = { + transaction: jest.fn(), + }; + puzzleRepository = { + findOne: jest.fn(), + create: jest.fn((data) => ({ id: 'new-id', ...data })), + save: jest.fn(async (data) => data), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + MigrationService, + { + provide: DataSource, + useValue: dataSource, + }, + { + provide: getRepositoryToken(Puzzle), + useValue: puzzleRepository, + }, + ], + }).compile(); + + service = module.get(MigrationService); + }); + + const uploadInfo = { filename: 'f.json', fileSize: 10, uploadedBy: 'admin' }; + + it('imports every row on a clean batch', async () => { + puzzleRepository.findOne.mockResolvedValue(undefined); + + // Run the provided transaction callback against the fake manager. + dataSource.transaction.mockImplementation(async (cb) => cb(buildManager())); + + const result = await service.migratePuzzles( + [makePuzzle('A'), makePuzzle('B')], + uploadInfo, + ); + + expect(result.success).toBe(true); + expect(result.rolledBack).toBe(false); + expect(result.summary.successfulInserts).toBe(2); + expect(result.summary.failedInserts).toBe(0); + expect(result.summary.duplicatesSkipped).toBe(0); + expect(puzzleRepository.save).toHaveBeenCalledTimes(2); + }); + + it('aborts and rolls back the whole batch when a row fails mid-import', async () => { + puzzleRepository.findOne.mockResolvedValue(undefined); + + // Simulate the transaction throwing when the callback is run. + let manager: ReturnType; + dataSource.transaction.mockImplementation(async (cb) => { + manager = buildManager(); + puzzleRepository.save.mockReset(); + puzzleRepository.save + .mockResolvedValueOnce({ id: 'a' }) + .mockImplementationOnce(async () => { + throw new Error('unique constraint violated on in-batch duplicate'); + }); + try { + return await cb(manager); + } catch (err) { + // The real DataSource rolls back and rethrows here. + throw err; + } + }); + + const result = await service.migratePuzzles( + [makePuzzle('A'), makePuzzle('A')], // in-batch duplicate -> unique index error + uploadInfo, + ); + + expect(result.success).toBe(false); + expect(result.rolledBack).toBe(true); + // Because the transaction rolled back, nothing counted as persisted. + expect(result.summary.successfulInserts).toBe(0); + expect(result.summary.failedInserts).toBe(1); + expect(result.errors[0].error).toContain('rolled back'); + expect(result.errors[0].index).toBe(1); + }); + + it('skips pre-existing rows so re-running the import is resumable', async () => { + // Row 'A' already exists in the DB; 'B' does not. + puzzleRepository.findOne.mockImplementation(async ({ where }) => { + return where.title === 'A' ? { id: 'existing' } : undefined; + }); + + dataSource.transaction.mockImplementation(async (cb) => cb(buildManager())); + + const result = await service.migratePuzzles( + [makePuzzle('A'), makePuzzle('B')], + uploadInfo, + ); + + expect(result.success).toBe(true); + expect(result.rolledBack).toBe(false); + expect(result.summary.duplicatesSkipped).toBe(1); + expect(result.summary.successfulInserts).toBe(1); + // Only the non-duplicate row is saved. + expect(puzzleRepository.save).toHaveBeenCalledTimes(1); + }); +}); diff --git a/backend/src/migration/services/migration.service.ts b/backend/src/migration/services/migration.service.ts index b451c3a9..07f3401c 100644 --- a/backend/src/migration/services/migration.service.ts +++ b/backend/src/migration/services/migration.service.ts @@ -1,20 +1,50 @@ import { Injectable, Logger } from '@nestjs/common'; -import type { Repository } from 'typeorm'; -import type { Puzzle } from '../entities/puzzle.entity'; +import { InjectRepository } from '@nestjs/typeorm'; +import { DataSource, EntityManager, Repository } from 'typeorm'; +import { Puzzle } from '../entities/puzzle.entity'; import type { PuzzleData, MigrationResult, MigrationError, } from '../interfaces/puzzle.interface'; +/** + * Thrown inside the migration transaction to abort the whole batch. Any + * error that escapes the transaction callback rolls back every row so a + * partial import can never leave categories/puzzles/dependencies/rewards + * in an inconsistent state. + */ +export class MigrationAbortError extends Error { + constructor( + readonly index: number, + readonly puzzle: Partial, + message: string, + ) { + super(message); + this.name = 'MigrationAbortError'; + } +} + @Injectable() export class MigrationService { private readonly logger = new Logger(MigrationService.name); - constructor(private readonly puzzleRepository: Repository) {} + constructor( + private readonly dataSource: DataSource, + @InjectRepository(Puzzle) + private readonly puzzleRepository: Repository, + ) {} /** - * Migrate puzzle data to database + * Migrate puzzle data to database. + * + * The whole batch runs inside a single DB transaction (atomic import): + * if any row fails to persist, the transaction rolls back and nothing is + * written. Rows whose `(title, category)` pair already exists in the DB + * are skipped so re-running the same upload is safe and resumable, but a + * genuine failure (e.g. a duplicate pair within the same incoming batch, + * which violates the unique index) aborts the entire import rather than + * leaving a partial write behind. */ async migratePuzzles( puzzleData: PuzzleData[], @@ -25,57 +55,90 @@ export class MigrationService { const errors: MigrationError[] = []; let successfulInserts = 0; let duplicatesSkipped = 0; - - for (let i = 0; i < puzzleData.length; i++) { - const puzzle = puzzleData[i]; - - try { - // Check for duplicates - const existingPuzzle = await this.puzzleRepository.findOne({ - where: { - title: puzzle.title, - category: puzzle.category, - }, - }); - - if (existingPuzzle) { - this.logger.warn( - `Duplicate puzzle found: ${puzzle.title} in category ${puzzle.category}`, - ); - duplicatesSkipped++; - continue; + let rolledBack = false; + + try { + await this.dataSource.transaction(async (manager: EntityManager) => { + const puzzleRepo = manager.getRepository(Puzzle); + + for (let i = 0; i < puzzleData.length; i++) { + const puzzle = puzzleData[i]; + + try { + // Check for duplicates against rows already in the DB. Pre-existing + // duplicates are skipped (idempotent/resumable), not failures. + const existingPuzzle = await puzzleRepo.findOne({ + where: { + title: puzzle.title, + category: puzzle.category, + }, + }); + + if (existingPuzzle) { + this.logger.warn( + `Duplicate puzzle found: ${puzzle.title} in category ${puzzle.category}`, + ); + duplicatesSkipped++; + continue; + } + + // Create new puzzle entity + const newPuzzle = puzzleRepo.create({ + title: puzzle.title, + description: puzzle.description, + difficulty: puzzle.difficulty, + category: puzzle.category, + content: puzzle.content, + metadata: puzzle.metadata, + tags: puzzle.tags, + isActive: puzzle.isActive, + }); + + await puzzleRepo.save(newPuzzle); + successfulInserts++; + + this.logger.debug(`Successfully inserted puzzle: ${puzzle.title}`); + } catch (error) { + // Wrap with row context, then rethrow so the surrounding + // transaction rolls back every already-inserted row. + throw new MigrationAbortError( + i, + puzzle, + error instanceof Error ? error.message : String(error), + ); + } } - - // Create new puzzle entity - const newPuzzle = this.puzzleRepository.create({ - title: puzzle.title, - description: puzzle.description, - difficulty: puzzle.difficulty, - category: puzzle.category, - content: puzzle.content, - metadata: puzzle.metadata, - tags: puzzle.tags, - isActive: puzzle.isActive, - }); - - await this.puzzleRepository.save(newPuzzle); - successfulInserts++; - - this.logger.debug(`Successfully inserted puzzle: ${puzzle.title}`); - } catch (error) { - this.logger.error( - `Failed to insert puzzle at index ${i}: ${error.message}`, - ); - errors.push({ - index: i, - puzzle, - error: error.message, - }); - } + }); + } catch (error) { + // Any error here aborts the transaction, rolling back every row that + // the batch may have inserted before failing. Record the abort so the + // caller can see the import did not partially apply. + const abortError = + error instanceof MigrationAbortError + ? error + : new MigrationAbortError( + -1, + {}, + error instanceof Error ? error.message : String(error), + ); + + rolledBack = true; + // The transaction rolled back, so nothing was actually persisted: + // report zero successful inserts and every row as not applied. + successfulInserts = 0; + errors.push({ + index: abortError.index, + puzzle: abortError.puzzle, + error: `Import aborted and rolled back: ${abortError.message}`, + }); + + this.logger.error( + `Migration aborted and rolled back (row ${abortError.index}): ${abortError.message}`, + ); } const result: MigrationResult = { - success: errors.length === 0, + success: !rolledBack && errors.length === 0, summary: { totalProcessed: puzzleData.length, successfulInserts, @@ -83,6 +146,7 @@ export class MigrationService { duplicatesSkipped, }, errors, + rolledBack, uploadInfo: { ...uploadInfo, uploadedAt: new Date(), @@ -90,7 +154,7 @@ export class MigrationService { }; this.logger.log( - `Migration completed: ${successfulInserts} inserted, ${duplicatesSkipped} duplicates skipped, ${errors.length} failed`, + `Migration completed: ${successfulInserts} inserted, ${duplicatesSkipped} duplicates skipped, ${errors.length} failed${rolledBack ? ' (rolled back)' : ''}`, ); return result;