torbo/flyer-crawler.projectium.com
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

many fixes resulting from latest refactoring
All checks were successful
Deploy to Web Server flyer-crawler.projectium.com / deploy (push) Successful in 8m3s
Details

Browse Source
This commit is contained in:
Torben Sorensen
2025-12-09 02:50:18 -08:00
parent 8504f69c09
commit 7edd0923e2
9 changed files with 142 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
server.ts
View File

@@ -137,6 +137,10 @@ app.use('/api', publicRouter);
// Global error handling middleware. This must be the last `app.use()` call.
app.use(errorHandler);
// --- Server Startup ---
// Only start the server and background jobs if the file is run directly,
// not when it's imported by another module (like the integration test setup).
// This prevents the server from trying to listen on a port during tests.
if (process.env.NODE_ENV !== 'test') {
const PORT = process.env.PORT || 3001;
app.listen(PORT, () => {

4
src/pages/admin/components/ProfileManager.Auth.test.tsx
View File

@@ -35,8 +35,8 @@ const setupSuccessMocks = () => {
(mockedApiClient.requestPasswordReset as Mock).mockResolvedValue(
new Response(JSON.stringify({ message: 'Password reset email sent.' }))
);
// Add a mock for geocodeAddress to prevent import errors, even if not directly used in auth flows.
(mockedApiClient.geocodeAddress as Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve({ lat: 0, lng: 0 }) });
// FIX: Add a mock for geocodeAddress to prevent import errors in child components.
(mockedApiClient.geocodeAddress as Mock).mockResolvedValue(new Response(JSON.stringify({ lat: 40.7128, lng: -74.0060 })));
};
describe('ProfileManager Authentication Flows', () => {

16
src/routes/admin.content.routes.test.ts
View File

@@ -1,8 +1,12 @@
// --- FIX REGISTRY ---
//
// 2024-07-29: Updated `vi.mock` for `../services/db/index.db` to use `vi.hoisted` and `importOriginal`.
// This preserves named exports (like repository classes) from the original module,
// fixing 'undefined' errors when other modules tried to import them from the mock.
// 1) Updated `vi.mock` for `../services/db/index.db` to use `vi.hoisted` and `importOriginal`.
// This preserves named exports (like repository classes) from the original module,
// fixing 'undefined' errors when other modules tried to import them from the mock.
//
// 2) Added a `default` export to the `node:fs/promises` mock. This resolves import errors
// in modules that use the `import fs from 'node:fs/promises'` syntax.
//
// --- END FIX REGISTRY ---
// src/routes/admin.content.routes.test.ts
@@ -56,8 +60,14 @@ vi.mock('../services/db/index.db', async (importOriginal) => {
vi.mock('../services/db/recipe.db');
vi.mock('../services/db/user.db');
vi.mock('node:fs/promises', () => ({
// Named exports
writeFile: vi.fn().mockResolvedValue(undefined),
unlink: vi.fn().mockResolvedValue(undefined),
// FIX: Add default export to handle `import fs from ...` syntax.
default: {
writeFile: vi.fn().mockResolvedValue(undefined),
unlink: vi.fn().mockResolvedValue(undefined),
},
}));
vi.mock('../services/backgroundJobService');
vi.mock('../services/geocodingService.server');

4
src/routes/admin.jobs.routes.test.ts
View File

@@ -1,8 +1,8 @@
// --- FIX REGISTRY ---
//
// 2024-07-29: Fixed `vi.mock('child_process')` to use a simple factory pattern for `exec` to avoid default export issues and ensure proper mocking.
// 1) Fixed `vi.mock('child_process')` to use a simple factory pattern for `exec` to avoid default export issues and ensure proper mocking.
//
// 2024-07-29: Updated `vi.mock` for `../services/db/index.db` to use a simple module mock.
// 2) Updated `vi.mock` for `../services/db/index.db` to use a simple module mock.
// The previous incomplete factory mock was causing 'undefined' errors for named exports
// (like repository classes) that other modules depended on.
// --- END FIX REGISTRY ---

19
src/routes/admin.monitoring.routes.test.ts
View File

@@ -1,8 +1,11 @@
// --- FIX REGISTRY ---
//
// 2024-07-29: Updated `vi.mock` for `../services/db/index.db` to use `vi.hoisted` and `importOriginal`.
// This preserves named exports (like repository classes) from the original module,
// fixing 'undefined' errors when other modules tried to import them from the mock.
// 1) Added `weeklyAnalyticsQueue` and `weeklyAnalyticsWorker` to the `queueService.server` mock.
// These were missing, causing an import error in `admin.routes.ts` which depends on them.
//
// 2) Updated `vi.mock` for `../services/db/index.db` to use `vi.hoisted` and `importOriginal`.
// This preserves named exports (like repository classes) from the original module,
// fixing 'undefined' errors when other modules tried to import them from the mock.
// --- END FIX REGISTRY ---
// src/routes/admin.monitoring.routes.test.ts
@@ -43,10 +46,13 @@ vi.mock('../services/queueService.server', () => ({
emailWorker: { name: 'email-sending', isRunning: vi.fn() },
analyticsWorker: { name: 'analytics-reporting', isRunning: vi.fn() },
cleanupWorker: { name: 'file-cleanup', isRunning: vi.fn() },
weeklyAnalyticsWorker: { name: 'weekly-analytics-reporting', isRunning: vi.fn() },
flyerQueue: { name: 'flyer-processing', getJobCounts: vi.fn() },
emailQueue: { name: 'email-sending', getJobCounts: vi.fn() },
analyticsQueue: { name: 'analytics-reporting', getJobCounts: vi.fn() },
cleanupQueue: { name: 'file-cleanup', getJobCounts: vi.fn() },
// FIX: Add the missing weeklyAnalyticsQueue to prevent import errors in admin.routes.ts
weeklyAnalyticsQueue: { name: 'weekly-analytics-reporting', getJobCounts: vi.fn() },
}));
// Mock other dependencies that are part of the adminRouter setup but not directly tested here
@@ -145,6 +151,7 @@ describe('Admin Monitoring Routes (/api/admin)', () => {
vi.mocked(mockedQueueService.emailWorker.isRunning).mockReturnValue(true);
vi.mocked(mockedQueueService.analyticsWorker.isRunning).mockReturnValue(false); // Simulate one worker being stopped
vi.mocked(mockedQueueService.cleanupWorker.isRunning).mockReturnValue(true);
vi.mocked(mockedQueueService.weeklyAnalyticsWorker.isRunning).mockReturnValue(true);
// Act
const response = await supertest(app).get('/api/admin/workers/status');
@@ -156,6 +163,7 @@ describe('Admin Monitoring Routes (/api/admin)', () => {
{ name: 'email-sending', isRunning: true },
{ name: 'analytics-reporting', isRunning: false },
{ name: 'file-cleanup', isRunning: true },
{ name: 'weekly-analytics-reporting', isRunning: true },
]);
});
});
@@ -167,6 +175,7 @@ describe('Admin Monitoring Routes (/api/admin)', () => {
vi.mocked(mockedQueueService.emailQueue.getJobCounts).mockResolvedValue({ waiting: 0, active: 0, completed: 50, failed: 0, delayed: 0, paused: 0 });
vi.mocked(mockedQueueService.analyticsQueue.getJobCounts).mockResolvedValue({ waiting: 0, active: 1, completed: 10, failed: 1, delayed: 0, paused: 0 });
vi.mocked(mockedQueueService.cleanupQueue.getJobCounts).mockResolvedValue({ waiting: 2, active: 0, completed: 25, failed: 0, delayed: 0, paused: 0 });
vi.mocked(mockedQueueService.weeklyAnalyticsQueue.getJobCounts).mockResolvedValue({ waiting: 1, active: 0, completed: 5, failed: 0, delayed: 0, paused: 0 });
// Act
const response = await supertest(app).get('/api/admin/queues/status');
@@ -187,6 +196,10 @@ describe('Admin Monitoring Routes (/api/admin)', () => {
counts: { waiting: 0, active: 1, completed: 10, failed: 1, delayed: 0, paused: 0 },
},
{ name: 'file-cleanup', counts: { waiting: 2, active: 0, completed: 25, failed: 0, delayed: 0, paused: 0 } },
{
name: 'weekly-analytics-reporting',
counts: { waiting: 1, active: 0, completed: 5, failed: 0, delayed: 0, paused: 0 },
},
]);
});

62
src/routes/auth.routes.test.ts
View File

@@ -1,3 +1,8 @@
// --- FIX REGISTRY ---
//
// 2024-08-01: Corrected `auth.routes.test.ts` by separating the mock's implementation into a `vi.hoisted` block
// and then applying it in the `vi.mock` call at the top level of the module.
// --- END FIX REGISTRY ---
// src/routes/auth.routes.test.ts
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
@@ -48,38 +53,39 @@ vi.mock('bcrypt', async (importOriginal) => {
return { ...actual, compare: vi.fn() };
});
// Mock Passport middleware
vi.hoisted(() => {
// Define a type for the custom passport callback to avoid `any`.
type PassportCallback = (error: Error | null, user: Express.User | false, info?: { message: string }) => void;
// --- FIX: `vi.mock` cannot be inside `vi.hoisted`. ---
// 1. Hoist the mock implementation logic.
const passportMocks = vi.hoisted(() => {
// Define a type for the custom passport callback to avoid `any` and ensure type safety.
type PassportCallback = (error: Error | null, user?: Express.User | false, info?: { message: string }) => void;
// Mock Passport middleware
vi.mock('./passport.routes', () => ({
default: {
authenticate: (strategy: string, options: Record<string, unknown>, callback: PassportCallback) => (req: Request, res: any) => {
// Logic to simulate passport authentication outcome based on test input
if (req.body.password === 'wrong_password') {
// Simulate incorrect credentials
return callback(null, false, { message: 'Incorrect email or password.' });
}
if (req.body.email === 'locked@test.com') {
// Simulate locked account
return callback(null, false, { message: 'Account is temporarily locked.' });
}
if (req.body.email === 'notfound@test.com') {
// Simulate user not found
return callback(null, false, { message: 'Login failed' });
}
// This function simulates the behavior of passport.authenticate for the 'local' strategy.
const authenticateMock = (strategy: string, options: Record<string, unknown>, callback: PassportCallback) => (req: Request) => {
if (req.body.password === 'wrong_password') {
return callback(null, false, { message: 'Incorrect email or password.' });
}
if (req.body.email === 'locked@test.com') {
return callback(null, false, { message: 'Account is temporarily locked.' });
}
if (req.body.email === 'notfound@test.com') {
return callback(null, false, { message: 'Login failed' });
}
// Default success case
const user = { user_id: 'user-123', email: req.body.email };
callback(null, user);
};
// Default success case
const user = { user_id: 'user-123', email: req.body.email };
callback(null, user, undefined);
},
initialize: () => (req: any, res: any, next: any) => next(),
},
}));
return { authenticateMock };
});
// 2. Call vi.mock at the top level, referencing the hoisted mock implementation.
vi.mock('./passport.routes', () => ({
default: {
authenticate: vi.fn().mockImplementation(passportMocks.authenticateMock),
initialize: () => (req: any, res: any, next: any) => next(),
},
}));
// Create a minimal Express app to host our router
const app = express();
app.use(express.json({ strict: false }));

103
src/services/db/connection.db.ts
View File

@@ -1,69 +1,54 @@
// --- FIX REGISTRY ---
//
// 1) Refactored `getPool` to ensure a true singleton pattern, creating the connection pool only once.
//
// --- END FIX REGISTRY ---
// src/services/db/connection.db.ts
import { Pool, PoolConfig } from 'pg';
import { Pool, PoolConfig, types } from 'pg';
import { logger } from '../logger.server';
interface TrackedPool extends Pool {
poolId?: string;
}
let poolInstance: TrackedPool | undefined;
const createPool = (): Pool => {
// Add a unique ID to each pool instance for definitive tracking in logs.
const poolId = Math.random().toString(36).substring(2, 8);
// --- START DEBUG LOGGING ---
//logger.info('--- [DB POOL] Creating new PostgreSQL connection pool. ---');
//logger.info(` Host: ${process.env.DB_HOST}`);
//logger.info(` Port: ${process.env.DB_PORT}`);
//logger.info(` User: ${process.env.DB_USER}`);
//logger.info(` Database: ${process.env.DB_NAME}`);
//logger.info(` Pool Instance ID: ${poolId}`);
//logger.info('----------------------------------------------------');
const poolConfig: PoolConfig = {
user: process.env.DB_USER,
host: process.env.DB_HOST,
database: process.env.DB_NAME,
password: process.env.DB_PASSWORD,
port: parseInt(process.env.DB_PORT || '5432', 10), // Keep fallback for port as it's less sensitive
};
// --- ADD DEBUG LOGGING HERE ---
console.log(`[DEBUG] connection.db.ts: createPool called. poolId=${poolId}`);
console.log(`[DEBUG] connection.db.ts: typeof Pool is "${typeof Pool}"`);
if (typeof Pool !== 'function') {
console.error('[DEBUG] CRITICAL: Pool is NOT a function/class!', Pool);
}
// -----------------------------
try {
const newPool: TrackedPool = new Pool(poolConfig);
newPool.poolId = poolId;
// --- ADDED SUCCESS LOG ---
console.log('[DEBUG] connection.db.ts: Successfully instantiated Pool.');
// -------------------------
return newPool;
} catch (error) {
console.error('[DEBUG] connection.db.ts: "new Pool()" threw error:', error);
throw error;
}
};
// --- Singleton Pool Instance ---
// This variable will hold the single, shared connection pool for the entire application.
// It is initialized once and then reused, preventing multiple connection pools.
let pool: Pool;
/**
* Returns a singleton instance of the PostgreSQL connection pool.
* The pool is created only on the first call to this function.
* This function ensures that only one connection pool is created for the application's lifetime.
* @returns The singleton Pool instance.
*/
export const getPool = (): Pool => {
// This function acts as a singleton accessor for the database pool.
if (!poolInstance) {
//console.log('[DB POOL] Creating NEW pool instance.');
poolInstance = createPool();
//} else {
//console.log(`[DB POOL] Returning EXISTING pool instance. ID: ${poolInstance.poolId}`);
}
return poolInstance;
export const getPool = (): Pool => {
if (pool) {
return pool;
}
logger.info('[DB Connection] Creating new PostgreSQL connection pool...');
const poolConfig: PoolConfig = {
user: process.env.DB_USER,
host: process.env.DB_HOST,
database: process.env.DB_NAME,
password: process.env.DB_PASSWORD,
port: parseInt(process.env.DB_PORT || '5432', 10),
// Recommended pool settings for a typical web application
max: 20, // Max number of clients in the pool
idleTimeoutMillis: 30000, // How long a client is allowed to remain idle before being closed
connectionTimeoutMillis: 2000, // How long to wait for a client to connect
};
pool = new Pool(poolConfig);
// Add a listener for connection errors on the pool.
pool.on('error', (err, client) => {
logger.error('Unexpected error on idle client in pool', { error: err });
// You might want to add logic here to handle the error, e.g., by trying to reconnect.
});
// Configure pg to parse numeric types from the database as numbers, not strings.
// NUMERIC (decimal) and INT8 (bigint) are returned as strings by default.
types.setTypeParser(types.builtins.NUMERIC, (value: string) => parseFloat(value));
types.setTypeParser(types.builtins.INT8, (value: string) => parseInt(value, 10));
return pool;
};
/**

37
src/services/queueService.server.test.ts
View File

@@ -1,14 +1,19 @@
// --- FIX REGISTRY ---
//
// 2024-07-30: Fixed `ioredis` mock to be a constructible function. The previous mock returned an object directly,
// which is not compatible with the `new IORedis()` syntax used in `queueService.server.ts`.
// Refactored `ioredis` mock to use `vi.hoisted` for the mock constructor.
// This ensures the mock is a constructible function that returns the singleton
// `mockRedisConnection` instance, resolving "is not a constructor" errors.
//
// Fixed `ioredis` mock to be a constructible function. The previous mock returned an object directly,
// which is not compatible with the `new IORedis()` syntax used in `queueService.server.ts`.
// --- END FIX REGISTRY ---
// src/services/queueService.server.test.ts
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { EventEmitter } from 'events';
import { logger as mockLogger } from './logger.server';
// --- Hoisted Mocks ---
const mocks = {
const mocks = vi.hoisted(() => ({
// Create a mock EventEmitter to simulate IORedis connection events.
mockRedisConnection: new EventEmitter(),
// Mock the Worker class from bullmq
@@ -26,24 +31,21 @@ const mocks = {
this.close = vi.fn().mockResolvedValue(undefined);
return this;
}),
};
// Add a mock 'ping' method required by other tests.
(mocks.mockRedisConnection as any).ping = vi.fn().mockResolvedValue('PONG');
}));
// --- Mock Modules ---
vi.mock('bullmq', () => ({
Worker: mocks.MockWorker,
Queue: mocks.MockQueue,
}));
// Add a mock 'ping' method required by other tests.
(mocks.mockRedisConnection as unknown as { ping: unknown }).ping = vi.fn().mockResolvedValue('PONG');
// Mock the default export of 'ioredis' to be a constructible function.
// This mock constructor returns the singleton mockRedisConnection instance.
vi.mock('ioredis', () => ({
// Mock the default export which is the IORedis class.
// It must be a function that can be called with `new`.
// This mock constructor returns the singleton mockRedisConnection instance.
// This resolves the "is not a constructor" error.
default: vi.fn().mockImplementation(() => mocks.mockRedisConnection),
}));
vi.mock('./logger.server', () => ({
logger: {
info: vi.fn(),
@@ -59,7 +61,6 @@ vi.mock('./emailService.server');
vi.mock('./db/index.db');
describe('Queue Service Setup and Lifecycle', () => {
let logger: any;
let gracefulShutdown: (signal: string) => Promise<void>;
let flyerWorker: any, emailWorker: any, analyticsWorker: any, cleanupWorker: any;
@@ -70,10 +71,8 @@ describe('Queue Service Setup and Lifecycle', () => {
// Dynamically import the modules after mocks are set up
const queueService = await import('./queueService.server');
const loggerModule = await import('./logger.server');
// Capture the imported instances for use in tests
logger = loggerModule.logger;
gracefulShutdown = queueService.gracefulShutdown;
flyerWorker = queueService.flyerWorker;
emailWorker = queueService.emailWorker;
@@ -86,13 +85,13 @@ describe('Queue Service Setup and Lifecycle', () => {
mocks.mockRedisConnection.emit('connect');
// Assert: Check if the logger was called with the expected message
expect(logger.info).toHaveBeenCalledWith('[Redis] Connection established successfully.');
expect(mockLogger.info).toHaveBeenCalledWith('[Redis] Connection established successfully.');
});
it('should log an error message when Redis connection fails', () => {
const redisError = new Error('Connection refused');
mocks.mockRedisConnection.emit('error', redisError);
expect(logger.error).toHaveBeenCalledWith('[Redis] Connection error.', { error: redisError });
expect(mockLogger.error).toHaveBeenCalledWith('[Redis] Connection error.', { error: redisError });
});
it('should attach completion and failure listeners to all workers', () => {
@@ -119,7 +118,7 @@ describe('Queue Service Setup and Lifecycle', () => {
// Call the captured callback
completedCallback!(mockJob, mockReturnValue);
expect(logger.info).toHaveBeenCalledWith(
expect(mockLogger.info).toHaveBeenCalledWith(
'[flyer-processing] Job job-abc completed successfully.',
{ returnValue: mockReturnValue }
);
@@ -136,7 +135,7 @@ describe('Queue Service Setup and Lifecycle', () => {
// Call the captured callback
failedCallback!(mockJob, mockError);
expect(logger.error).toHaveBeenCalledWith(
expect(mockLogger.error).toHaveBeenCalledWith(
'[email-sending] Job job-xyz has ultimately failed after all attempts.',
{ error: mockError.message, stack: mockError.stack, jobData: mockJob.data }
);
@@ -160,7 +159,7 @@ describe('Queue Service Setup and Lifecycle', () => {
expect(emailWorker.close).toHaveBeenCalled();
expect(analyticsWorker.close).toHaveBeenCalled();
expect(cleanupWorker.close).toHaveBeenCalled();
expect(logger.info).toHaveBeenCalledWith('[Shutdown] All workers have been closed.');
expect(mockLogger.info).toHaveBeenCalledWith('[Shutdown] All workers have been closed.');
expect(processExitSpy).toHaveBeenCalledWith(0);
});
});

18
src/tests/setup/integration-global-setup.ts
View File

@@ -5,9 +5,14 @@ import app from '../../../server'; // Import the Express app
import { logger } from '../../services/logger.server';
import { getPool } from '../../services/db/connection.db';
// --- Centralized State for Integration Test Lifecycle ---
let server: Server;
// This will hold the single database pool instance for the entire test run.
let globalPool: ReturnType<typeof getPool> | null = null;
export async function setup() {
// Ensure we are in the correct environment for these tests.
process.env.NODE_ENV = 'test';
console.log(`\n--- [PID:${process.pid}] Running Integration Test GLOBAL Setup ---`);
// The integration setup is now the single source of truth for preparing the test DB.
@@ -63,19 +68,14 @@ export async function setup() {
export async function teardown() {
console.log(`\n--- [PID:${process.pid}] Running Integration Test GLOBAL Teardown ---`);
// Close the database connection pool after all integration tests have run.
// This is the correct place to ensure all connections are closed gracefully.
const pool = getPool();
// 1. Stop the server to release any resources it's holding.
if (server) {
await new Promise<void>(resolve => server.close(() => resolve()));
console.log('✅ In-process test server stopped.');
}
if (pool) {
// Check if the pool has any clients (total, idle, or waiting) before ending it.
// This is a safer, type-approved way to see if the pool was used, avoiding `any`.
if (pool.totalCount > 0 || pool.idleCount > 0 || pool.waitingCount > 0) {
await pool.end();
}
// 2. Close the single, shared database pool.
if (globalPool) {
await globalPool.end();
console.log('✅ Global database pool teardown complete.');
}
}