From bcc4ad64dcc80246ed8e7db902839566c5596d3b Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Wed, 24 Dec 2025 09:04:10 -0800 Subject: [PATCH] fixing unit tests --- src/routes/ai.routes.test.ts | 7 ++++++- src/routes/ai.routes.ts | 23 ++++++++++++++++++++--- src/routes/price.routes.test.ts | 9 ++++++--- src/utils/zodUtils.test.ts | 22 +++++++++++----------- src/utils/zodUtils.ts | 19 +++++++++++++------ 5 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/routes/ai.routes.test.ts b/src/routes/ai.routes.test.ts index 64804354..96de6940 100644 --- a/src/routes/ai.routes.test.ts +++ b/src/routes/ai.routes.test.ts @@ -353,10 +353,11 @@ describe('AI Routes (/api/ai)', () => { expect(response.status).toBe(400); }); - it('should return 409 Conflict if flyer checksum already exists', async () => { + it('should return 409 Conflict and delete the uploaded file if flyer checksum already exists', async () => { // Arrange const mockExistingFlyer = createMockFlyer({ flyer_id: 99 }); vi.mocked(mockedDb.flyerRepo.findFlyerByChecksum).mockResolvedValue(mockExistingFlyer); // Duplicate found + const unlinkSpy = vi.spyOn(fs.promises, 'unlink').mockResolvedValue(undefined); // Act const response = await supertest(app) @@ -368,6 +369,10 @@ describe('AI Routes (/api/ai)', () => { expect(response.status).toBe(409); expect(response.body.message).toBe('This flyer has already been processed.'); expect(mockedDb.createFlyerAndItems).not.toHaveBeenCalled(); + // Assert that the file was deleted + expect(unlinkSpy).toHaveBeenCalledTimes(1); + // The filename is predictable in the test environment because of the multer config in ai.routes.ts + expect(unlinkSpy).toHaveBeenCalledWith(expect.stringContaining('flyerImage-test-flyer-image.jpg')); }); it('should accept payload when extractedData.items is missing and save with empty items', async () => { diff --git a/src/routes/ai.routes.ts b/src/routes/ai.routes.ts index 22a95c48..aa04cc26 100644 --- a/src/routes/ai.routes.ts +++ b/src/routes/ai.routes.ts @@ -50,6 +50,15 @@ const errMsg = (e: unknown) => { return String(e || 'An unknown error occurred.'); }; +const cleanupUploadedFile = async (file?: Express.Multer.File) => { + if (!file) return; + try { + await fs.promises.unlink(file.path); + } catch (err) { + // Ignore cleanup errors (e.g. file already deleted) + } +}; + const cropAreaObjectSchema = z.object({ x: z.number(), y: z.number(), @@ -185,7 +194,7 @@ router.use((req: Request, res: Response, next: NextFunction) => { '[API /ai] Incoming request', ); } catch (e: unknown) { - logger.error({ error: e }, 'Failed to log incoming AI request headers'); + logger.error({ error: errMsg(e) }, 'Failed to log incoming AI request headers'); } next(); }); @@ -316,7 +325,7 @@ router.post( // Try several ways to obtain the payload so we are tolerant to client variations. let parsed: FlyerProcessPayload = {}; - let extractedData: Partial = {}; + let extractedData: Partial | null | undefined = {}; try { // If the client sent a top-level `data` field (stringified JSON), parse it. if (req.body && (req.body.data || req.body.extractedData)) { @@ -337,7 +346,7 @@ router.post( ) as FlyerProcessPayload; } // If parsed itself contains an `extractedData` field, use that, otherwise assume parsed is the extractedData - extractedData = parsed.extractedData ?? (parsed as Partial); + extractedData = 'extractedData' in parsed ? parsed.extractedData : (parsed as Partial); } else { // No explicit `data` field found. Attempt to interpret req.body as an object (Express may have parsed multipart fields differently). try { @@ -383,6 +392,12 @@ router.post( // Pull common metadata fields (checksum, originalFileName) from whichever shape we parsed. const checksum = parsed.checksum ?? parsed?.data?.checksum ?? ''; + + if (!checksum) { + await cleanupUploadedFile(req.file); + return res.status(400).json({ message: 'Checksum is required.' }); + } + const originalFileName = parsed.originalFileName ?? parsed?.data?.originalFileName ?? req.file.originalname; const userProfile = req.user as UserProfile | undefined; @@ -429,6 +444,7 @@ router.post( const existingFlyer = await db.flyerRepo.findFlyerByChecksum(checksum, req.log); if (existingFlyer) { logger.warn(`Duplicate flyer upload attempt blocked for checksum: ${checksum}`); + await cleanupUploadedFile(req.file); return res.status(409).json({ message: 'This flyer has already been processed.' }); } @@ -476,6 +492,7 @@ router.post( res.status(201).json({ message: 'Flyer processed and saved successfully.', flyer: newFlyer }); } catch (error) { + await cleanupUploadedFile(req.file); next(error); } }, diff --git a/src/routes/price.routes.test.ts b/src/routes/price.routes.test.ts index a578b848..4549d555 100644 --- a/src/routes/price.routes.test.ts +++ b/src/routes/price.routes.test.ts @@ -96,7 +96,8 @@ describe('Price Routes (/api/price-history)', () => { .send({ masterItemIds: 'not-an-array' }); expect(response.status).toBe(400); - expect(response.body.errors[0].message).toContain('Expected array, received string'); + // The actual message is "Invalid input: expected array, received string" + expect(response.body.errors[0].message).toBe('Invalid input: expected array, received string'); }); it('should return 400 if masterItemIds contains non-positive integers', async () => { @@ -112,7 +113,8 @@ describe('Price Routes (/api/price-history)', () => { const response = await supertest(app).post('/api/price-history').send({}); expect(response.status).toBe(400); - expect(response.body.errors[0].message).toBe('Required'); + // The actual message is "Invalid input: expected array, received undefined" + expect(response.body.errors[0].message).toBe('Invalid input: expected array, received undefined'); }); it('should return 400 for invalid limit and offset', async () => { @@ -122,7 +124,8 @@ describe('Price Routes (/api/price-history)', () => { expect(response.status).toBe(400); expect(response.body.errors).toHaveLength(2); - expect(response.body.errors[0].message).toBe('Number must be greater than 0'); + // The actual message is "Too small: expected number to be >0" + expect(response.body.errors[0].message).toBe('Too small: expected number to be >0'); expect(response.body.errors[1].message).toBe('Expected number, received string'); }); }); diff --git a/src/utils/zodUtils.test.ts b/src/utils/zodUtils.test.ts index d516b0f6..75ed0fd4 100644 --- a/src/utils/zodUtils.test.ts +++ b/src/utils/zodUtils.test.ts @@ -59,7 +59,7 @@ describe('Zod Utilities', () => { expect(result.success).toBe(false); if (!result.success) { // z.string() will throw its own error message before min(1) is checked. - expect(result.error.issues[0].message).toBe('Expected string, received number'); + expect(result.error.issues[0].message).toBe('Invalid input: expected string, received number'); } }); @@ -67,7 +67,7 @@ describe('Zod Utilities', () => { const result = schema.safeParse({ a: 1 }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error.issues[0].message).toBe('Expected string, received object'); + expect(result.error.issues[0].message).toBe('Invalid input: expected string, received object'); } }); }); @@ -95,7 +95,7 @@ describe('Zod Utilities', () => { const result = schema.safeParse({ params: { id: 'abc' } }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error.issues[0].message).toContain('Expected number, received nan'); + expect(result.error.issues[0].message).toBe('Invalid input: expected number, received NaN'); } }); @@ -103,7 +103,7 @@ describe('Zod Utilities', () => { const result = schema.safeParse({ params: { id: -1 } }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error.issues[0].message).toContain('Must be a number'); + expect(result.error.issues[0].message).toBe("Invalid ID for parameter 'id'. Must be a number."); } }); @@ -111,7 +111,7 @@ describe('Zod Utilities', () => { const result = schema.safeParse({ params: { id: 1.5 } }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error.issues[0].message).toContain('Must be a number'); + expect(result.error.issues[0].message).toBe("Invalid ID for parameter 'id'. Must be a number."); } }); @@ -119,7 +119,7 @@ describe('Zod Utilities', () => { const result = schema.safeParse({ params: { id: 0 } }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error.issues[0].message).toContain('Must be a number'); + expect(result.error.issues[0].message).toBe("Invalid ID for parameter 'id'. Must be a number."); } }); @@ -224,7 +224,7 @@ describe('Zod Utilities', () => { const floatResult = schema.safeParse('123.45'); expect(floatResult.success).toBe(false); if (!floatResult.success) { - expect(floatResult.error.issues[0].message).toBe('Expected integer, received float'); + expect(floatResult.error.issues[0].message).toBe('Invalid input: expected int, received number'); } }); @@ -234,7 +234,7 @@ describe('Zod Utilities', () => { const zeroResult = schema.safeParse('0'); expect(zeroResult.success).toBe(false); if (!zeroResult.success) { - expect(zeroResult.error.issues[0].message).toBe('Number must be greater than 0'); + expect(zeroResult.error.issues[0].message).toBe('Too small: expected number to be >0'); } }); @@ -244,7 +244,7 @@ describe('Zod Utilities', () => { const negativeResult = schema.safeParse('-1'); expect(negativeResult.success).toBe(false); if (!negativeResult.success) { - expect(negativeResult.error.issues[0].message).toBe('Number must be greater than or equal to 0'); + expect(negativeResult.error.issues[0].message).toBe('Too small: expected number to be >=0'); } }); @@ -254,12 +254,12 @@ describe('Zod Utilities', () => { const tooSmallResult = schema.safeParse('9'); expect(tooSmallResult.success).toBe(false); if (!tooSmallResult.success) { - expect(tooSmallResult.error.issues[0].message).toBe('Number must be greater than or equal to 10'); + expect(tooSmallResult.error.issues[0].message).toBe('Too small: expected number to be >=10'); } const tooLargeResult = schema.safeParse('21'); expect(tooLargeResult.success).toBe(false); if (!tooLargeResult.success) { - expect(tooLargeResult.error.issues[0].message).toBe('Number must be less than or equal to 20'); + expect(tooLargeResult.error.issues[0].message).toBe('Too big: expected number to be <=20'); } }); }); diff --git a/src/utils/zodUtils.ts b/src/utils/zodUtils.ts index 2f34bf3e..3f836149 100644 --- a/src/utils/zodUtils.ts +++ b/src/utils/zodUtils.ts @@ -59,15 +59,22 @@ export const optionalNumeric = ( nonnegative?: boolean; } = {}, ) => { - let schema = z.coerce.number(); + // Start with the base number schema and apply all number-specific constraints first. + let numberSchema = z.coerce.number(); - if (options.integer) schema = schema.int(); - if (options.positive) schema = schema.positive(); - else if (options.nonnegative) schema = schema.nonnegative(); + if (options.integer) numberSchema = numberSchema.int(); + if (options.positive) numberSchema = numberSchema.positive(); + else if (options.nonnegative) numberSchema = numberSchema.nonnegative(); - if (options.min !== undefined) schema = schema.min(options.min); - if (options.max !== undefined) schema = schema.max(options.max); + if (options.min !== undefined) numberSchema = numberSchema.min(options.min); + if (options.max !== undefined) numberSchema = numberSchema.max(options.max); + // Now, wrap the fully configured number schema with the preprocess step. + // This is crucial because z.coerce.number(null) results in 0, which bypasses + // the .optional() and .default() logic for null inputs. We want null to be + // treated as "not provided", just like undefined. + const schema = z.preprocess((val) => (val === null ? undefined : val), numberSchema); + if (options.default !== undefined) return schema.optional().default(options.default); return schema.optional();