From 1448950b814745a73a106f9cbe7f956820ac3d96 Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Tue, 30 Dec 2025 02:10:29 -0800 Subject: [PATCH] fix unit tests --- src/middleware/multer.middleware.test.ts | 19 +++--- src/middleware/multer.middleware.ts | 9 ++- src/services/aiService.server.test.ts | 4 +- src/services/aiService.server.ts | 77 +++++++++--------------- 4 files changed, 46 insertions(+), 63 deletions(-) diff --git a/src/middleware/multer.middleware.test.ts b/src/middleware/multer.middleware.test.ts index 08ba3a6d..da635ce3 100644 --- a/src/middleware/multer.middleware.test.ts +++ b/src/middleware/multer.middleware.test.ts @@ -4,6 +4,7 @@ import multer from 'multer'; import type { Request, Response, NextFunction } from 'express'; import { createUploadMiddleware, handleMulterError } from './multer.middleware'; import { createMockUserProfile } from '../tests/utils/mockFactories'; +import { ValidationError } from '../services/db/errors.db'; // 1. Hoist the mocks so they can be referenced inside vi.mock factories. const mocks = vi.hoisted(() => ({ @@ -217,9 +218,11 @@ describe('createUploadMiddleware', () => { const cb = vi.fn(); const mockTextFile = { mimetype: 'text/plain' } as Express.Multer.File; - multerOptions!.fileFilter!({} as Request, mockTextFile, cb); + multerOptions!.fileFilter!({} as Request, { ...mockTextFile, fieldname: 'test' }, cb); - expect(cb).toHaveBeenCalledWith(new Error('Only image files are allowed!')); + const error = (cb as Mock).mock.calls[0][0]; + expect(error).toBeInstanceOf(ValidationError); + expect(error.validationErrors[0].message).toBe('Only image files are allowed!'); }); }); }); @@ -248,13 +251,13 @@ describe('handleMulterError Middleware', () => { expect(mockNext).not.toHaveBeenCalled(); }); - it('should handle the custom image file filter error', () => { - // This test covers lines 59-61 - const err = new Error('Only image files are allowed!'); + it('should pass on a ValidationError to the next handler', () => { + const err = new ValidationError([], 'Only image files are allowed!'); handleMulterError(err, mockRequest as Request, mockResponse as Response, mockNext); - expect(mockResponse.status).toHaveBeenCalledWith(400); - expect(mockResponse.json).toHaveBeenCalledWith({ message: 'Only image files are allowed!' }); - expect(mockNext).not.toHaveBeenCalled(); + // It should now pass the error to the global error handler + expect(mockNext).toHaveBeenCalledWith(err); + expect(mockResponse.status).not.toHaveBeenCalled(); + expect(mockResponse.json).not.toHaveBeenCalled(); }); it('should pass on non-multer errors to the next error handler', () => { diff --git a/src/middleware/multer.middleware.ts b/src/middleware/multer.middleware.ts index cd10f800..eb2de4a9 100644 --- a/src/middleware/multer.middleware.ts +++ b/src/middleware/multer.middleware.ts @@ -5,6 +5,7 @@ import fs from 'node:fs/promises'; import { Request, Response, NextFunction } from 'express'; import { UserProfile } from '../types'; import { sanitizeFilename } from '../utils/stringUtils'; +import { ValidationError } from '../services/db/errors.db'; import { logger } from '../services/logger.server'; export const flyerStoragePath = @@ -69,8 +70,9 @@ const imageFileFilter = (req: Request, file: Express.Multer.File, cb: multer.Fil cb(null, true); } else { // Reject the file with a specific error that can be caught by a middleware. - const err = new Error('Only image files are allowed!'); - cb(err); + const validationIssue = { path: ['file', file.fieldname], message: 'Only image files are allowed!' }; + const err = new ValidationError([validationIssue]); + cb(err as Error); // Cast to Error to satisfy multer's type, though ValidationError extends Error. } }; @@ -114,9 +116,6 @@ export const handleMulterError = ( if (err instanceof multer.MulterError) { // A Multer error occurred when uploading (e.g., file too large). return res.status(400).json({ message: `File upload error: ${err.message}` }); - } else if (err && err.message === 'Only image files are allowed!') { - // A custom error from our fileFilter. - return res.status(400).json({ message: err.message }); } // If it's not a multer error, pass it on. next(err); diff --git a/src/services/aiService.server.test.ts b/src/services/aiService.server.test.ts index 2a5d98bc..190488ca 100644 --- a/src/services/aiService.server.test.ts +++ b/src/services/aiService.server.test.ts @@ -1029,8 +1029,8 @@ describe('AI Service (Server)', () => { // Verify that the error was caught and logged using errMsg logic expect(mockLoggerInstance.warn).toHaveBeenCalledWith( - expect.objectContaining({ error: expect.any(String) }), // errMsg converts Error to string message - expect.stringContaining('Failed to parse parsed.data'), + expect.objectContaining({ error: expect.any(String) }), + '[AIService] Failed to parse nested "data" property string.', ); }); diff --git a/src/services/aiService.server.ts b/src/services/aiService.server.ts index ae138a20..5ac36444 100644 --- a/src/services/aiService.server.ts +++ b/src/services/aiService.server.ts @@ -787,56 +787,37 @@ async enqueueFlyerProcessing( logger: Logger, ): { parsed: FlyerProcessPayload; extractedData: Partial | null | undefined } { let parsed: FlyerProcessPayload = {}; - let extractedData: Partial | null | undefined = {}; + try { - if (body && (body.data || body.extractedData)) { - const raw = body.data ?? body.extractedData; - try { - parsed = typeof raw === 'string' ? JSON.parse(raw) : raw; - } catch (err) { - logger.warn( - { error: errMsg(err) }, - '[AIService] Failed to JSON.parse raw extractedData; falling back to direct assign', - ); - parsed = ( - typeof raw === 'string' ? JSON.parse(String(raw).slice(0, 2000)) : raw - ) as FlyerProcessPayload; - } - extractedData = 'extractedData' in parsed ? parsed.extractedData : (parsed as Partial); - } else { - try { - parsed = typeof body === 'string' ? JSON.parse(body) : body; - } catch (err) { - logger.warn( - { error: errMsg(err) }, - '[AIService] Failed to JSON.parse req.body; using empty object', - ); - parsed = (body as FlyerProcessPayload) || {}; - } - if (parsed.data) { - try { - const inner = typeof parsed.data === 'string' ? JSON.parse(parsed.data) : parsed.data; - extractedData = inner.extractedData ?? inner; - } catch (err) { - logger.warn({ error: errMsg(err) }, '[AIService] Failed to parse parsed.data; falling back'); - extractedData = parsed.data as unknown as Partial; - } - } else if (parsed.extractedData) { - extractedData = parsed.extractedData; - } else { - if ('items' in parsed || 'store_name' in parsed || 'valid_from' in parsed) { - extractedData = parsed as Partial; - } else { - extractedData = {}; - } - } - } - } catch (err) { - logger.error({ error: err }, '[AIService] Unexpected error while parsing legacy request body'); - parsed = {}; - extractedData = {}; + parsed = typeof body === 'string' ? JSON.parse(body) : body || {}; + } catch (e) { + logger.warn({ error: errMsg(e) }, '[AIService] Failed to parse top-level request body string.'); + return { parsed: {}, extractedData: {} }; } - return { parsed, extractedData }; + + // If the real payload is nested inside a 'data' property (which could be a string), + // we parse it out but keep the original `parsed` object for top-level properties like checksum. + let potentialPayload: FlyerProcessPayload = parsed; + if (parsed.data) { + if (typeof parsed.data === 'string') { + try { + potentialPayload = JSON.parse(parsed.data); + } catch (e) { + logger.warn({ error: errMsg(e) }, '[AIService] Failed to parse nested "data" property string.'); + } + } else if (typeof parsed.data === 'object') { + potentialPayload = parsed.data; + } + } + + // The extracted data is either in an `extractedData` key or is the payload itself. + const extractedData = potentialPayload.extractedData ?? potentialPayload; + + // Merge for checksum lookup: properties in the outer `parsed` object (like a top-level checksum) + // take precedence over any same-named properties inside `potentialPayload`. + const finalParsed = { ...potentialPayload, ...parsed }; + + return { parsed: finalParsed, extractedData }; } async processLegacyFlyerUpload(