From 5e5568452149d3ed321c5e02d29b84f3ed0220c9 Mon Sep 17 00:00:00 2001 From: emmanuelStack Date: Fri, 26 Jun 2026 04:17:11 +0100 Subject: [PATCH] feat(backend): optimize sql queries in Audit Logger --- backend/src/lib/audit.js | 9 +++- backend/src/lib/audit.test.js | 1 + backend/src/services/auditService.js | 57 ++++++++++++++--------- backend/src/services/auditService.test.js | 24 ++++++---- 4 files changed, 58 insertions(+), 33 deletions(-) diff --git a/backend/src/lib/audit.js b/backend/src/lib/audit.js index 19d530ea..0981ae80 100644 --- a/backend/src/lib/audit.js +++ b/backend/src/lib/audit.js @@ -10,7 +10,8 @@ * - `status` is stored as a suffix of the `action` field: 'login_success' | 'login_failure'. */ -import { pool, isRetryablePoolError } from "./db.js"; +import { isRetryablePoolError } from "./db.js"; +import { optimizedWrite } from "./db-pooler-optimized.js"; import { consumeAuditLogRateLimit, createAuditLogRateLimitKey, @@ -104,7 +105,7 @@ async function insertAuditLog({ payload, payloadHash, signature }) { for (let attempt = 0; attempt <= AUDIT_DB_RETRY_ATTEMPTS; attempt += 1) { try { - await pool.query( + await optimizedWrite( `INSERT INTO audit_logs (merchant_id, action, status, ip_address, user_agent, payload_hash, signature) VALUES ($1, $2, $3, $4, $5, $6, $7)`, [ @@ -116,6 +117,10 @@ async function insertAuditLog({ payload, payloadHash, signature }) { payloadHash, signature, ], + { + label: "insert_login_audit_log", + merchantId: payload.merchant_id, + } ); recordCircuitSuccess(); return { success: true }; diff --git a/backend/src/lib/audit.test.js b/backend/src/lib/audit.test.js index e495e184..62f2a705 100644 --- a/backend/src/lib/audit.test.js +++ b/backend/src/lib/audit.test.js @@ -24,6 +24,7 @@ const { mockQuery, mockIsRetryablePoolError, mockConsumeRateLimit, mockHashPaylo vi.mock("./db.js", () => ({ pool: { query: mockQuery }, isRetryablePoolError: mockIsRetryablePoolError, + queryWithRetry: mockQuery, })); vi.mock("./audit-security.js", () => ({ diff --git a/backend/src/services/auditService.js b/backend/src/services/auditService.js index 9452a68f..4c223e07 100644 --- a/backend/src/services/auditService.js +++ b/backend/src/services/auditService.js @@ -1,4 +1,5 @@ -import { pool, isRetryablePoolError } from "../lib/db.js"; +import { isRetryablePoolError } from "../lib/db.js"; +import { optimizedQuery, optimizedWrite } from "../lib/db-pooler-optimized.js"; import { consumeAuditLogRateLimit, createAuditLogRateLimitKey, @@ -87,7 +88,7 @@ async function insertAuditLog({ payload, payloadHash, signature }) { for (let attempt = 0; attempt <= AUDIT_DB_RETRY_ATTEMPTS; attempt += 1) { try { - await pool.query( + await optimizedWrite( `INSERT INTO audit_logs (merchant_id, action, field_changed, old_value, new_value, ip_address, user_agent, payload_hash, signature) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, [ @@ -101,6 +102,10 @@ async function insertAuditLog({ payload, payloadHash, signature }) { payloadHash, signature, ], + { + label: "insert_audit_log", + merchantId: payload.merchant_id, + } ); recordSvcSuccess(); return { success: true }; @@ -125,11 +130,9 @@ export const auditService = { /** * Retrieve paginated audit logs for a merchant. * - * Uses a single SQL query with a COUNT(*) OVER() window function so that - * the total row count and the page data are fetched in one round-trip to - * the database instead of two (issue #770). The composite index on - * (merchant_id, timestamp) created in migration 20260425000000 is used by - * the ORDER BY clause to avoid a sequential scan on large tables. + * Splits paginated queries into parallel count and row retrieval queries (issue #770) + * executed via optimizedQuery to allow cacheability and avoid full table scan + * materialization in Postgres. */ async getAuditLogs(merchantId, page = 1, limit = 50) { let p = parseInt(page, 10) || 1; @@ -141,21 +144,31 @@ export const auditService = { const offset = (p - 1) * l; - // Single query: window function returns the full-table count alongside - // each row, eliminating the separate COUNT(*) round-trip (issue #770). - const result = await pool.query( - `SELECT id, action, field_changed, old_value, new_value, ip_address, user_agent, timestamp, - COUNT(*) OVER() AS total_count - FROM audit_logs - WHERE merchant_id = $1 - ORDER BY timestamp DESC - LIMIT $2 OFFSET $3`, - [merchantId, l, offset], - ); - - const totalCount = result.rows.length > 0 ? parseInt(result.rows[0].total_count, 10) : 0; - // Strip the synthetic total_count column from each returned row - const logs = result.rows.map(({ total_count: _tc, ...row }) => row); + const [countResult, logsResult] = await Promise.all([ + optimizedQuery( + `SELECT COUNT(*)::integer AS total_count FROM audit_logs WHERE merchant_id = $1`, + [merchantId], + { + label: "count_audit_logs", + merchantId, + } + ), + optimizedQuery( + `SELECT id, action, field_changed, old_value, new_value, ip_address, user_agent, timestamp + FROM audit_logs + WHERE merchant_id = $1 + ORDER BY timestamp DESC + LIMIT $2 OFFSET $3`, + [merchantId, l, offset], + { + label: "get_audit_logs", + merchantId, + } + ) + ]); + + const totalCount = countResult.rows.length > 0 ? parseInt(countResult.rows[0].total_count, 10) : 0; + const logs = logsResult.rows; return { logs, diff --git a/backend/src/services/auditService.test.js b/backend/src/services/auditService.test.js index 4834514a..71f1f408 100644 --- a/backend/src/services/auditService.test.js +++ b/backend/src/services/auditService.test.js @@ -13,6 +13,7 @@ const { mockQuery, mockIsRetryablePoolError, mockConsumeRateLimit, mockHashPaylo vi.mock("../lib/db.js", () => ({ pool: { query: mockQuery }, isRetryablePoolError: mockIsRetryablePoolError, + queryWithRetry: mockQuery, })); vi.mock("../lib/audit-security.js", () => ({ @@ -127,26 +128,30 @@ describe("auditService", () => { // ── SQL optimization: getAuditLogs (issue #770) ─────────────────────────── - it("fetches logs and count in a single window-function query", async () => { + it("fetches logs and count using optimized parallel queries", async () => { + mockQuery.mockResolvedValueOnce({ + rows: [{ total_count: "3" }], + }); mockQuery.mockResolvedValueOnce({ rows: [ - { id: 1, action: "update", field_changed: "email", old_value: "a@b.com", new_value: "c@d.com", ip_address: "1.2.3.4", user_agent: "ua", timestamp: new Date(), total_count: "3" }, - { id: 2, action: "login", field_changed: null, old_value: null, new_value: null, ip_address: "1.2.3.4", user_agent: "ua", timestamp: new Date(), total_count: "3" }, + { id: 1, action: "update", field_changed: "email", old_value: "a@b.com", new_value: "c@d.com", ip_address: "1.2.3.4", user_agent: "ua", timestamp: new Date() }, + { id: 2, action: "login", field_changed: null, old_value: null, new_value: null, ip_address: "1.2.3.4", user_agent: "ua", timestamp: new Date() }, ], }); const result = await auditService.getAuditLogs("merchant-1", 1, 2); - expect(mockQuery).toHaveBeenCalledOnce(); - const [sql] = mockQuery.mock.calls[0]; - expect(sql).toMatch(/COUNT\(\*\) OVER\(\)/i); + expect(mockQuery).toHaveBeenCalledTimes(2); + const [countSql] = mockQuery.mock.calls[0]; + const [logsSql] = mockQuery.mock.calls[1]; + expect(countSql).toMatch(/COUNT\(\*\)/i); + expect(logsSql).not.toMatch(/COUNT\(\*\) OVER\(\)/i); expect(result.total_count).toBe(3); expect(result.logs).toHaveLength(2); - // Ensure the synthetic total_count column is stripped from returned rows - expect(result.logs[0]).not.toHaveProperty("total_count"); }); it("returns zero total_count when no rows match", async () => { + mockQuery.mockResolvedValueOnce({ rows: [{ total_count: 0 }] }); mockQuery.mockResolvedValueOnce({ rows: [] }); const result = await auditService.getAuditLogs("merchant-nobody", 1, 10); expect(result.total_count).toBe(0); @@ -154,9 +159,10 @@ describe("auditService", () => { }); it("clamps page and limit to valid ranges", async () => { + mockQuery.mockResolvedValueOnce({ rows: [{ total_count: 0 }] }); mockQuery.mockResolvedValueOnce({ rows: [] }); const result = await auditService.getAuditLogs("merchant-1", -5, 200); - const [, params] = mockQuery.mock.calls[0]; + const [, params] = mockQuery.mock.calls[1]; expect(params[1]).toBe(100); // limit clamped to 100 expect(params[2]).toBe(0); // offset for page 1 = 0 expect(result.page).toBe(1);