Fix review issues: partial index, middleware scope, add timing tests
- Change idx_financing_plans_active to partial index (WHERE active = true) to avoid useless B-tree index on low-cardinality boolean column - Scope timingMiddleware to /api routes only (was globally registered, which caused noise from static asset requests) - Add unit tests for timing middleware (5 tests covering: next() call, timing log format, SLOW warning threshold, fast-request path, status code) Nightshift-Task: perf-regression Nightshift-Ref: https://github.com/marcus/nightshift
This commit is contained in:
@@ -4,4 +4,4 @@ CREATE INDEX IF NOT EXISTS idx_paycheck_bills_paycheck_id ON paycheck_bills(payc
|
||||
CREATE INDEX IF NOT EXISTS idx_actuals_paycheck_id ON actuals(paycheck_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_one_time_expenses_paycheck_id ON one_time_expenses(paycheck_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_financing_payments_plan_id ON financing_payments(plan_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_financing_plans_active ON financing_plans(active);
|
||||
CREATE INDEX IF NOT EXISTS idx_financing_plans_active ON financing_plans(active) WHERE active = true;
|
||||
|
||||
98
server/src/__tests__/timing.middleware.test.js
Normal file
98
server/src/__tests__/timing.middleware.test.js
Normal file
@@ -0,0 +1,98 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
const timingMiddleware = require('../middleware/timing');
|
||||
|
||||
function makeResMock() {
|
||||
const listeners = {};
|
||||
return {
|
||||
statusCode: 200,
|
||||
on(event, cb) {
|
||||
listeners[event] = cb;
|
||||
},
|
||||
emit(event) {
|
||||
if (listeners[event]) listeners[event]();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('timingMiddleware', () => {
|
||||
let consoleSpy;
|
||||
let warnSpy;
|
||||
|
||||
beforeEach(() => {
|
||||
consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
|
||||
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
consoleSpy.mockRestore();
|
||||
warnSpy.mockRestore();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('calls next()', () => {
|
||||
const req = { method: 'GET', path: '/api/health' };
|
||||
const res = makeResMock();
|
||||
const next = vi.fn();
|
||||
|
||||
timingMiddleware(req, res, next);
|
||||
expect(next).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('logs timing on response finish', () => {
|
||||
const req = { method: 'GET', path: '/api/health' };
|
||||
const res = makeResMock();
|
||||
|
||||
timingMiddleware(req, res, vi.fn());
|
||||
res.emit('finish');
|
||||
|
||||
expect(consoleSpy).toHaveBeenCalledOnce();
|
||||
const msg = consoleSpy.mock.calls[0][0];
|
||||
expect(msg).toMatch(/\[timing\] GET \/api\/health 200 \d+ms/);
|
||||
});
|
||||
|
||||
it('emits SLOW warning when duration exceeds 200ms threshold', () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const req = { method: 'POST', path: '/api/paychecks' };
|
||||
const res = makeResMock();
|
||||
|
||||
timingMiddleware(req, res, vi.fn());
|
||||
|
||||
// Advance time past the threshold
|
||||
vi.advanceTimersByTime(250);
|
||||
res.emit('finish');
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledOnce();
|
||||
const msg = warnSpy.mock.calls[0][0];
|
||||
expect(msg).toMatch(/\[SLOW\] POST \/api\/paychecks/);
|
||||
expect(consoleSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not emit SLOW warning for fast requests', () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const req = { method: 'GET', path: '/api/financing' };
|
||||
const res = makeResMock();
|
||||
|
||||
timingMiddleware(req, res, vi.fn());
|
||||
|
||||
vi.advanceTimersByTime(50);
|
||||
res.emit('finish');
|
||||
|
||||
expect(consoleSpy).toHaveBeenCalledOnce();
|
||||
expect(warnSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('includes status code in the log message', () => {
|
||||
const req = { method: 'GET', path: '/api/bills' };
|
||||
const res = makeResMock();
|
||||
res.statusCode = 404;
|
||||
|
||||
timingMiddleware(req, res, vi.fn());
|
||||
res.emit('finish');
|
||||
|
||||
const msg = consoleSpy.mock.calls[0][0];
|
||||
expect(msg).toContain('404');
|
||||
});
|
||||
});
|
||||
@@ -15,9 +15,8 @@ const app = express();
|
||||
|
||||
app.use(cors());
|
||||
app.use(express.json());
|
||||
app.use(timingMiddleware);
|
||||
|
||||
// API routes
|
||||
app.use('/api', timingMiddleware);
|
||||
app.use('/api', healthRouter);
|
||||
app.use('/api', configRouter);
|
||||
app.use('/api', billsRouter);
|
||||
|
||||
Reference in New Issue
Block a user