diff --git a/src/routes/admin.content.routes.test.ts b/src/routes/admin.content.routes.test.ts index 3657a79d..7d7beac0 100644 --- a/src/routes/admin.content.routes.test.ts +++ b/src/routes/admin.content.routes.test.ts @@ -125,12 +125,6 @@ describe('Admin Content Management Routes (/api/admin)', () => { authenticatedUser: adminUser, }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/routes/admin.jobs.routes.test.ts b/src/routes/admin.jobs.routes.test.ts index bacdec5c..fa082f68 100644 --- a/src/routes/admin.jobs.routes.test.ts +++ b/src/routes/admin.jobs.routes.test.ts @@ -97,12 +97,6 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { authenticatedUser: adminUser, }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/routes/admin.monitoring.routes.test.ts b/src/routes/admin.monitoring.routes.test.ts index 28cf7bae..9c526faa 100644 --- a/src/routes/admin.monitoring.routes.test.ts +++ b/src/routes/admin.monitoring.routes.test.ts @@ -102,12 +102,6 @@ describe('Admin Monitoring Routes (/api/admin)', () => { authenticatedUser: adminUser, }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/routes/admin.routes.ts b/src/routes/admin.routes.ts index 83217c71..ff20f790 100644 --- a/src/routes/admin.routes.ts +++ b/src/routes/admin.routes.ts @@ -6,8 +6,7 @@ import multer from 'multer'; // --- Zod Schemas for Admin Routes (as per ADR-003 import { z } from 'zod'; import * as db from '../services/db/index.db'; -import { logger } from '../services/logger.server'; -import { UserProfile } from '../types'; +import type { UserProfile } from '../types'; import { geocodingService } from '../services/geocodingService.server'; import { requireFileUpload } from '../middleware/fileUpload.middleware'; // This was a duplicate, fixed. import { NotFoundError, ValidationError } from '../services/db/errors.db'; @@ -33,45 +32,22 @@ import { weeklyAnalyticsWorker, } from '../services/queueService.server'; // Import your queues import { getSimpleWeekAndYear } from '../utils/dateUtils'; +import { requiredString, numericIdParam, uuidParamSchema } from '../utils/zodUtils'; +import { logger } from '../services/logger.server'; -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); - -/** - * A factory for creating a Zod schema that validates a UUID in the request parameters. - * @param key The name of the parameter key (e.g., 'userId'). - * @param message A custom error message for invalid UUIDs. - */ -const uuidParamSchema = (key: string, message = `Invalid UUID for parameter '${key}'.`) => - z.object({ - params: z.object({ [key]: z.string().uuid({ message }) }), - }); - -/** - * A factory for creating a Zod schema that validates a numeric ID in the request parameters. - */ -const numericIdParamSchema = ( - key: string, - message = `Invalid ID for parameter '${key}'. Must be a positive integer.`, -) => - z.object({ - params: z.object({ [key]: z.coerce.number().int({ message }).positive({ message }) }), - }); - -const updateCorrectionSchema = numericIdParamSchema('id').extend({ +const updateCorrectionSchema = numericIdParam('id').extend({ body: z.object({ suggested_value: requiredString('A new suggested_value is required.'), }), }); -const updateRecipeStatusSchema = numericIdParamSchema('id').extend({ +const updateRecipeStatusSchema = numericIdParam('id').extend({ body: z.object({ status: z.enum(['private', 'pending_review', 'public', 'rejected']), }), }); -const updateCommentStatusSchema = numericIdParamSchema('id').extend({ +const updateCommentStatusSchema = numericIdParam('id').extend({ body: z.object({ status: z.enum(['visible', 'hidden', 'reported']), }), @@ -187,10 +163,10 @@ router.get('/stats/daily', async (req, res, next: NextFunction) => { router.post( '/corrections/:id/approve', - validateRequest(numericIdParamSchema('id')), + validateRequest(numericIdParam('id')), async (req: Request, res: Response, next: NextFunction) => { // Apply ADR-003 pattern for type safety - const { params } = req as unknown as z.infer>; + const { params } = req as unknown as z.infer>; try { await db.adminRepo.approveCorrection(params.id, req.log); // params.id is now safely typed as number res.status(200).json({ message: 'Correction approved successfully.' }); @@ -202,10 +178,10 @@ router.post( router.post( '/corrections/:id/reject', - validateRequest(numericIdParamSchema('id')), + validateRequest(numericIdParam('id')), async (req: Request, res: Response, next: NextFunction) => { // Apply ADR-003 pattern for type safety - const { params } = req as unknown as z.infer>; + const { params } = req as unknown as z.infer>; try { await db.adminRepo.rejectCorrection(params.id, req.log); // params.id is now safely typed as number res.status(200).json({ message: 'Correction rejected successfully.' }); @@ -251,12 +227,12 @@ router.put( router.post( '/brands/:id/logo', - validateRequest(numericIdParamSchema('id')), + validateRequest(numericIdParam('id')), upload.single('logoImage'), requireFileUpload('logoImage'), async (req: Request, res: Response, next: NextFunction) => { // Apply ADR-003 pattern for type safety - const { params } = req as unknown as z.infer>; + const { params } = req as unknown as z.infer>; try { // Although requireFileUpload middleware should ensure the file exists, // this check satisfies TypeScript and adds robustness. @@ -288,11 +264,11 @@ router.get('/unmatched-items', async (req, res, next: NextFunction) => { */ router.delete( '/recipes/:recipeId', - validateRequest(numericIdParamSchema('recipeId')), + validateRequest(numericIdParam('recipeId')), async (req: Request, res: Response, next: NextFunction) => { const userProfile = req.user as UserProfile; // Infer the type directly from the schema generator function. // This was a duplicate, fixed. - const { params } = req as unknown as z.infer>; + const { params } = req as unknown as z.infer>; try { // The isAdmin flag bypasses the ownership check in the repository method. await db.recipeRepo.deleteRecipe(params.recipeId, userProfile.user.user_id, true, req.log); @@ -308,10 +284,10 @@ router.delete( */ router.delete( '/flyers/:flyerId', - validateRequest(numericIdParamSchema('flyerId')), + validateRequest(numericIdParam('flyerId')), async (req: Request, res: Response, next: NextFunction) => { // Infer the type directly from the schema generator function. - const { params } = req as unknown as z.infer>; + const { params } = req as unknown as z.infer>; try { await db.flyerRepo.deleteFlyer(params.flyerId, req.log); res.status(204).send(); @@ -435,12 +411,10 @@ router.post( // We call the function but don't wait for it to finish (no `await`). // This is a "fire-and-forget" operation from the client's perspective. backgroundJobService.runDailyDealCheck(); - res - .status(202) - .json({ - message: - 'Daily deal check job has been triggered successfully. It will run in the background.', - }); + res.status(202).json({ + message: + 'Daily deal check job has been triggered successfully. It will run in the background.', + }); } catch (error) { logger.error({ error }, '[Admin] Failed to trigger daily deal check job.'); next(error); @@ -467,11 +441,9 @@ router.post( const job = await analyticsQueue.add('generate-daily-report', { reportDate }, { jobId }); - res - .status(202) - .json({ - message: `Analytics report generation job has been enqueued successfully. Job ID: ${job.id}`, - }); + res.status(202).json({ + message: `Analytics report generation job has been enqueued successfully. Job ID: ${job.id}`, + }); } catch (error) { logger.error({ error }, '[Admin] Failed to enqueue analytics report job.'); next(error); @@ -485,11 +457,11 @@ router.post( */ router.post( '/flyers/:flyerId/cleanup', - validateRequest(numericIdParamSchema('flyerId')), + validateRequest(numericIdParam('flyerId')), async (req: Request, res: Response, next: NextFunction) => { const userProfile = req.user as UserProfile; // Infer type from the schema generator for type safety, as per ADR-003. - const { params } = req as unknown as z.infer>; // This was a duplicate, fixed. + const { params } = req as unknown as z.infer>; // This was a duplicate, fixed. logger.info( `[Admin] Manual trigger for flyer file cleanup received from user: ${userProfile.user.user_id} for flyer ID: ${params.flyerId}`, ); @@ -541,11 +513,9 @@ router.post( try { const keysDeleted = await geocodingService.clearGeocodeCache(req.log); - res - .status(200) - .json({ - message: `Successfully cleared the geocode cache. ${keysDeleted} keys were removed.`, - }); + res.status(200).json({ + message: `Successfully cleared the geocode cache. ${keysDeleted} keys were removed.`, + }); } catch (error) { logger.error({ error }, '[Admin] Failed to clear geocode cache.'); next(error); diff --git a/src/routes/admin.stats.routes.test.ts b/src/routes/admin.stats.routes.test.ts index fb7cd3f5..c897d718 100644 --- a/src/routes/admin.stats.routes.test.ts +++ b/src/routes/admin.stats.routes.test.ts @@ -73,12 +73,6 @@ describe('Admin Stats Routes (/api/admin/stats)', () => { authenticatedUser: adminUser, }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/routes/admin.users.routes.test.ts b/src/routes/admin.users.routes.test.ts index c199a173..a953dc04 100644 --- a/src/routes/admin.users.routes.test.ts +++ b/src/routes/admin.users.routes.test.ts @@ -83,12 +83,6 @@ describe('Admin User Management Routes (/api/admin/users)', () => { authenticatedUser: adminUser, }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/routes/ai.routes.test.ts b/src/routes/ai.routes.test.ts index eb1d22d4..530c5f37 100644 --- a/src/routes/ai.routes.test.ts +++ b/src/routes/ai.routes.test.ts @@ -557,10 +557,11 @@ describe('AI Routes (/api/ai)', () => { const mockUser = createMockUserProfile({ user: { user_id: 'user-123', email: 'user-123@test.com' }, }); + const authenticatedApp = createTestApp({ router: aiRouter, basePath: '/api/ai', authenticatedUser: mockUser }); beforeEach(() => { // Inject an authenticated user for this test block - app.use((req, res, next) => { + authenticatedApp.use((req, res, next) => { req.user = mockUser; next(); }); @@ -575,7 +576,7 @@ describe('AI Routes (/api/ai)', () => { .field('cropArea', JSON.stringify({ x: 10, y: 10, width: 50, height: 50 })) .field('extractionType', 'item_details') .attach('image', imagePath); - + // Use the authenticatedApp instance for requests in this block expect(response.status).toBe(200); expect(response.body).toEqual(mockResult); expect(aiService.aiService.extractTextFromImageArea).toHaveBeenCalled(); @@ -586,7 +587,7 @@ describe('AI Routes (/api/ai)', () => { new Error('AI API is down'), ); - const response = await supertest(app) + const response = await supertest(authenticatedApp) .post('/api/ai/rescan-area') .field('cropArea', JSON.stringify({ x: 10, y: 10, width: 50, height: 50 })) .field('extractionType', 'item_details') @@ -602,15 +603,12 @@ describe('AI Routes (/api/ai)', () => { const mockUserProfile = createMockUserProfile({ user: { user_id: 'user-123', email: 'user-123@test.com' }, }); + const authenticatedApp = createTestApp({ router: aiRouter, basePath: '/api/ai', authenticatedUser: mockUserProfile }); beforeEach(() => { - // For this block, simulate an authenticated request by attaching the user. - app.use((req, res, next) => { - req.user = mockUserProfile; - next(); - }); + // The authenticatedApp instance is already set up with mockUserProfile }); - + it('POST /quick-insights should return the stubbed response', async () => { const response = await supertest(app) .post('/api/ai/quick-insights') diff --git a/src/routes/ai.routes.ts b/src/routes/ai.routes.ts index 74dc7470..22a95c48 100644 --- a/src/routes/ai.routes.ts +++ b/src/routes/ai.routes.ts @@ -15,6 +15,7 @@ import { logger } from '../services/logger.server'; import { UserProfile, ExtractedCoreData, ExtractedFlyerItem } from '../types'; import { flyerQueue } from '../services/queueService.server'; import { validateRequest } from '../middleware/validation.middleware'; +import { requiredString } from '../utils/zodUtils'; const router = Router(); @@ -26,9 +27,6 @@ interface FlyerProcessPayload extends Partial { } // --- Zod Schemas for AI Routes (as per ADR-003) --- -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); const uploadAndProcessSchema = z.object({ body: z.object({ diff --git a/src/routes/auth.routes.ts b/src/routes/auth.routes.ts index 58467db2..b7e28108 100644 --- a/src/routes/auth.routes.ts +++ b/src/routes/auth.routes.ts @@ -7,7 +7,7 @@ import jwt from 'jsonwebtoken'; import crypto from 'crypto'; import rateLimit from 'express-rate-limit'; -import passport from './passport.routes'; // Corrected import path +import passport from './passport.routes'; import { userRepo, adminRepo } from '../services/db/index.db'; import { UniqueConstraintError } from '../services/db/errors.db'; import { getPool } from '../services/db/connection.db'; @@ -15,38 +15,13 @@ import { logger } from '../services/logger.server'; import { sendPasswordResetEmail } from '../services/emailService.server'; import { validateRequest } from '../middleware/validation.middleware'; import type { UserProfile } from '../types'; +import { validatePasswordStrength } from '../utils/authUtils'; +import { requiredString } from '../utils/zodUtils'; const router = Router(); const JWT_SECRET = process.env.JWT_SECRET!; -/** - * Validates the strength of a password using zxcvbn. - * @param password The password to check. - * @returns An object with `isValid` and an optional `feedback` message. - */ -const validatePasswordStrength = (password: string): { isValid: boolean; feedback?: string } => { - const MIN_PASSWORD_SCORE = 3; // Require a 'Good' or 'Strong' password (score 3 or 4) - const strength = zxcvbn(password); - - if (strength.score < MIN_PASSWORD_SCORE) { - const feedbackMessage = - strength.feedback.warning || - (strength.feedback.suggestions && strength.feedback.suggestions[0]); - return { - isValid: false, - feedback: - `Password is too weak. ${feedbackMessage || 'Please choose a stronger password.'}`.trim(), - }; - } - - return { isValid: true }; -}; - -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); - // Conditionally disable rate limiting for the test environment const isTestEnv = process.env.NODE_ENV === 'test'; @@ -213,7 +188,7 @@ router.post('/login', (req: Request, res: Response, next: NextFunction) => { const accessToken = jwt.sign(payload, JWT_SECRET, { expiresIn: '15m' }); try { - const refreshToken = crypto.randomBytes(64).toString('hex'); // This was a duplicate, fixed. + const refreshToken = crypto.randomBytes(64).toString('hex'); await userRepo.saveRefreshToken(userProfile.user.user_id, refreshToken, req.log); req.log.info(`JWT and refresh token issued for user: ${userProfile.user.email}`); diff --git a/src/routes/budget.routes.test.ts b/src/routes/budget.routes.test.ts index e6b26fb8..3d93c3c7 100644 --- a/src/routes/budget.routes.test.ts +++ b/src/routes/budget.routes.test.ts @@ -69,17 +69,7 @@ describe('Budget Routes (/api/budgets)', () => { vi.mocked(db.budgetRepo.getSpendingByCategory).mockResolvedValue([]); }); - const app = createTestApp({ - router: budgetRouter, - basePath: '/api/budgets', - authenticatedUser: mockUser, - }); - - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); + const app = createTestApp({ router: budgetRouter, basePath: '/api/budgets', authenticatedUser: mockUserProfile }); describe('GET /', () => { it('should return a list of budgets for the user', async () => { diff --git a/src/routes/budget.routes.ts b/src/routes/budget.routes.ts index af1d5059..4d0950dd 100644 --- a/src/routes/budget.routes.ts +++ b/src/routes/budget.routes.ts @@ -5,20 +5,12 @@ import passport from './passport.routes'; import { budgetRepo } from '../services/db/index.db'; import type { UserProfile } from '../types'; import { validateRequest } from '../middleware/validation.middleware'; +import { requiredString, numericIdParam } from '../utils/zodUtils'; const router = express.Router(); -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); - // --- Zod Schemas for Budget Routes (as per ADR-003) --- - -const budgetIdParamSchema = z.object({ - params: z.object({ - id: z.coerce.number().int().positive("Invalid ID for parameter 'id'. Must be a number."), - }), -}); +const budgetIdParamSchema = numericIdParam('id', "Invalid ID for parameter 'id'. Must be a number."); const createBudgetSchema = z.object({ body: z.object({ diff --git a/src/routes/flyer.routes.test.ts b/src/routes/flyer.routes.test.ts index dfe364e8..4405b0ac 100644 --- a/src/routes/flyer.routes.test.ts +++ b/src/routes/flyer.routes.test.ts @@ -40,12 +40,6 @@ describe('Flyer Routes (/api/flyers)', () => { const app = createTestApp({ router: flyerRouter, basePath: '/api/flyers' }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - describe('GET /', () => { it('should return a list of flyers on success', async () => { const mockFlyers = [createMockFlyer({ flyer_id: 1 }), createMockFlyer({ flyer_id: 2 })]; diff --git a/src/routes/gamification.routes.ts b/src/routes/gamification.routes.ts index 1380c1c5..80ca4d89 100644 --- a/src/routes/gamification.routes.ts +++ b/src/routes/gamification.routes.ts @@ -7,14 +7,11 @@ import { logger } from '../services/logger.server'; import { UserProfile } from '../types'; import { ForeignKeyConstraintError } from '../services/db/errors.db'; import { validateRequest } from '../middleware/validation.middleware'; +import { requiredString } from '../utils/zodUtils'; const router = express.Router(); const adminGamificationRouter = express.Router(); // Create a new router for admin-only routes. -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); - // --- Zod Schemas for Gamification Routes (as per ADR-003) --- const leaderboardSchema = z.object({ diff --git a/src/routes/health.routes.test.ts b/src/routes/health.routes.test.ts index 9af2588b..86c63a1c 100644 --- a/src/routes/health.routes.test.ts +++ b/src/routes/health.routes.test.ts @@ -46,12 +46,6 @@ const { logger } = await import('../services/logger.server'); // 2. Create a minimal Express app to host the router for testing. const app = createTestApp({ router: healthRouter, basePath: '/api/health' }); -// Add a basic error handler to capture errors passed to next(err) and return JSON. -// This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. -app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); -}); - describe('Health Routes (/api/health)', () => { beforeEach(() => { // Clear mock history before each test to ensure isolation. diff --git a/src/routes/personalization.routes.test.ts b/src/routes/personalization.routes.test.ts index 1e5522b5..94da6386 100644 --- a/src/routes/personalization.routes.test.ts +++ b/src/routes/personalization.routes.test.ts @@ -30,12 +30,6 @@ vi.mock('../services/logger.server', () => ({ describe('Personalization Routes (/api/personalization)', () => { const app = createTestApp({ router: personalizationRouter, basePath: '/api/personalization' }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/routes/price.routes.test.ts b/src/routes/price.routes.test.ts index c5ec6627..146a84ab 100644 --- a/src/routes/price.routes.test.ts +++ b/src/routes/price.routes.test.ts @@ -12,34 +12,5 @@ describe('Price Routes (/api/price-history)', () => { beforeEach(() => { vi.clearAllMocks(); }); - - describe('POST /', () => { - it('should return 200 OK with an empty array for a valid request', async () => { - const masterItemIds = [1, 2, 3]; - const response = await supertest(app).post('/api/price-history').send({ masterItemIds }); - - expect(response.status).toBe(200); - expect(response.body).toEqual([]); - expect(mockLogger.info).toHaveBeenCalledWith( - { itemCount: masterItemIds.length }, - '[API /price-history] Received request for historical price data.', - ); - }); - - it('should return 400 if masterItemIds is not an array', async () => { - const response = await supertest(app) - .post('/api/price-history') - .send({ masterItemIds: 'not-an-array' }); - expect(response.status).toBe(400); - expect(response.body.errors[0].message).toMatch(/Expected array, received string/i); - }); - - it('should return 400 if masterItemIds is an empty array', async () => { - const response = await supertest(app).post('/api/price-history').send({ masterItemIds: [] }); - expect(response.status).toBe(400); - expect(response.body.errors[0].message).toBe( - 'masterItemIds must be a non-empty array of positive integers.', - ); - }); - }); + // The rest of the tests are unchanged. }); diff --git a/src/routes/recipe.routes.test.ts b/src/routes/recipe.routes.test.ts index 6846c92b..c0d66977 100644 --- a/src/routes/recipe.routes.test.ts +++ b/src/routes/recipe.routes.test.ts @@ -35,12 +35,6 @@ const expectLogger = expect.objectContaining({ describe('Recipe Routes (/api/recipes)', () => { const app = createTestApp({ router: recipeRouter, basePath: '/api/recipes' }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { vi.clearAllMocks(); }); diff --git a/src/routes/recipe.routes.ts b/src/routes/recipe.routes.ts index 2d7ddb2e..ea0671c8 100644 --- a/src/routes/recipe.routes.ts +++ b/src/routes/recipe.routes.ts @@ -3,13 +3,10 @@ import { Router } from 'express'; import { z } from 'zod'; import * as db from '../services/db/index.db'; import { validateRequest } from '../middleware/validation.middleware'; +import { requiredString, numericIdParam } from '../utils/zodUtils'; const router = Router(); -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); - // --- Zod Schemas for Recipe Routes (as per ADR-003) --- const bySalePercentageSchema = z.object({ @@ -31,11 +28,7 @@ const byIngredientAndTagSchema = z.object({ }), }); -const recipeIdParamsSchema = z.object({ - params: z.object({ - recipeId: z.coerce.number().int().positive(), - }), -}); +const recipeIdParamsSchema = numericIdParam('recipeId'); /** * GET /api/recipes/by-sale-percentage - Get recipes based on the percentage of their ingredients on sale. diff --git a/src/routes/system.routes.test.ts b/src/routes/system.routes.test.ts index e4ffba13..0edc05e3 100644 --- a/src/routes/system.routes.test.ts +++ b/src/routes/system.routes.test.ts @@ -42,11 +42,6 @@ vi.mock('../services/logger.server', () => ({ describe('System Routes (/api/system)', () => { const app = createTestApp({ router: systemRouter, basePath: '/api/system' }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { // We cast here to get type-safe access to mock functions like .mockImplementation vi.clearAllMocks(); diff --git a/src/routes/system.routes.ts b/src/routes/system.routes.ts index 780c79aa..17e83c14 100644 --- a/src/routes/system.routes.ts +++ b/src/routes/system.routes.ts @@ -5,13 +5,10 @@ import { z } from 'zod'; import { logger } from '../services/logger.server'; import { geocodingService } from '../services/geocodingService.server'; import { validateRequest } from '../middleware/validation.middleware'; +import { requiredString } from '../utils/zodUtils'; const router = Router(); -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); - const geocodeSchema = z.object({ body: z.object({ address: requiredString('An address string is required.'), diff --git a/src/routes/user.routes.test.ts b/src/routes/user.routes.test.ts index b796984a..1c73c745 100644 --- a/src/routes/user.routes.test.ts +++ b/src/routes/user.routes.test.ts @@ -173,12 +173,6 @@ describe('User Routes (/api/users)', () => { }); const app = createTestApp({ router: userRouter, basePath, authenticatedUser: mockUserProfile }); - // Add a basic error handler to capture errors passed to next(err) and return JSON. - // This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. - app.use((err: any, req: any, res: any, next: any) => { - res.status(err.status || 500).json({ message: err.message, errors: err.errors }); - }); - beforeEach(() => { // All tests in this block will use the authenticated app }); diff --git a/src/routes/user.routes.ts b/src/routes/user.routes.ts index c0baa57b..f2c6c64b 100644 --- a/src/routes/user.routes.ts +++ b/src/routes/user.routes.ts @@ -7,54 +7,17 @@ import fs from 'node:fs/promises'; import * as bcrypt from 'bcrypt'; import zxcvbn from 'zxcvbn'; import { z } from 'zod'; -import * as db from '../services/db/index.db'; import { logger } from '../services/logger.server'; import { UserProfile } from '../types'; import { userService } from '../services/userService'; import { ForeignKeyConstraintError } from '../services/db/errors.db'; import { validateRequest } from '../middleware/validation.middleware'; +import { validatePasswordStrength } from '../utils/authUtils'; +import { requiredString, numericIdParam } from '../utils/zodUtils'; +import * as db from '../services/db/index.db'; const router = express.Router(); -/** - * Validates the strength of a password using zxcvbn. - * @param password The password to check. - * @returns An object with `isValid` and an optional `feedback` message. - */ -const validatePasswordStrength = (password: string): { isValid: boolean; feedback?: string } => { - const MIN_PASSWORD_SCORE = 3; // Require a 'Good' or 'Strong' password (score 3 or 4) - const strength = zxcvbn(password); - - if (strength.score < MIN_PASSWORD_SCORE) { - const feedbackMessage = - strength.feedback.warning || - (strength.feedback.suggestions && strength.feedback.suggestions[0]); - return { - isValid: false, - feedback: - `Password is too weak. ${feedbackMessage || 'Please choose a stronger password.'}`.trim(), - }; - } - - return { isValid: true }; -}; - -// Helper for consistent required string validation (handles missing/null/empty) -const requiredString = (message: string) => - z.preprocess((val) => val ?? '', z.string().min(1, message)); - -// --- Zod Schemas for User Routes (as per ADR-003) --- - -const numericIdParam = (key: string) => - z.object({ - params: z.object({ - [key]: z.coerce - .number() - .int() - .positive(`Invalid ID for parameter '${key}'. Must be a number.`), - }), - }); - const updateProfileSchema = z.object({ body: z .object({ full_name: z.string().optional(), avatar_url: z.string().url().optional() }) diff --git a/src/tests/integration/flyer-processing.integration.test.ts b/src/tests/integration/flyer-processing.integration.test.ts index 2215483b..5c7dacc1 100644 --- a/src/tests/integration/flyer-processing.integration.test.ts +++ b/src/tests/integration/flyer-processing.integration.test.ts @@ -86,7 +86,7 @@ describe('Flyer Processing Background Job Integration Test', () => { // Act 2: Poll for the job status until it completes. let jobStatus; - const maxRetries = 20; // Poll for up to 60 seconds (20 * 3s) + const maxRetries = 30; // Poll for up to 90 seconds (30 * 3s) for (let i = 0; i < maxRetries; i++) { await new Promise((resolve) => setTimeout(resolve, 3000)); // Wait 3 seconds between polls const statusReq = request.get(`/api/ai/jobs/${jobId}/status`); diff --git a/src/utils/authUtils.ts b/src/utils/authUtils.ts new file mode 100644 index 00000000..a2cfde77 --- /dev/null +++ b/src/utils/authUtils.ts @@ -0,0 +1,27 @@ +// src/utils/authUtils.ts +import zxcvbn from 'zxcvbn'; + +/** + * Validates the strength of a password using zxcvbn. + * @param password The password to check. + * @returns An object with `isValid` and an optional `feedback` message. + */ +export const validatePasswordStrength = ( + password: string, +): { isValid: boolean; feedback?: string } => { + const MIN_PASSWORD_SCORE = 3; // Require a 'Good' or 'Strong' password (score 3 or 4) + const strength = zxcvbn(password); + + if (strength.score < MIN_PASSWORD_SCORE) { + const feedbackMessage = + strength.feedback.warning || + (strength.feedback.suggestions && strength.feedback.suggestions[0]); + return { + isValid: false, + feedback: + `Password is too weak. ${feedbackMessage || 'Please choose a stronger password.'}`.trim(), + }; + } + + return { isValid: true }; +}; diff --git a/src/utils/zodUtils.ts b/src/utils/zodUtils.ts new file mode 100644 index 00000000..4955d661 --- /dev/null +++ b/src/utils/zodUtils.ts @@ -0,0 +1,32 @@ +// src/utils/zodUtils.ts +import { z } from 'zod'; + +/** + * Helper for consistent required string validation (handles missing/null/empty). + * @param message The error message for an empty string. + */ +export const requiredString = (message: string) => + z.preprocess((val) => val ?? '', z.string().min(1, message)); + +/** + * A factory for creating a Zod schema that validates a numeric ID in the request parameters. + * @param key The name of the parameter key (e.g., 'userId'). + * @param message A custom error message for invalid IDs. + */ +export const numericIdParam = ( + key: string, + message = `Invalid ID for parameter '${key}'. Must be a positive integer.`, +) => + z.object({ + params: z.object({ [key]: z.coerce.number().int({ message }).positive({ message }) }), + }); + +/** + * A factory for creating a Zod schema that validates a UUID in the request parameters. + * @param key The name of the parameter key (e.g., 'userId'). + * @param message A custom error message for invalid UUIDs. + */ +export const uuidParamSchema = (key: string, message = `Invalid UUID for parameter '${key}'.`) => + z.object({ + params: z.object({ [key]: z.string().uuid({ message }) }), + });