From 29e01a9ff0456e918e34c431876b251db91b7a9c Mon Sep 17 00:00:00 2001 From: eteonninob-arch Date: Tue, 29 Sep 2026 14:31:34 -0700 Subject: [PATCH] test(aml): add regression suite for AMLRuleRepository error paths and boundary contracts --- src/aml/amlRuleRepository.test.ts | 640 +++++++++++++++++++++++------- 1 file changed, 506 insertions(+), 134 deletions(-) diff --git a/src/aml/amlRuleRepository.test.ts b/src/aml/amlRuleRepository.test.ts index f6907f3d..e73c6ada 100644 --- a/src/aml/amlRuleRepository.test.ts +++ b/src/aml/amlRuleRepository.test.ts @@ -2,36 +2,71 @@ * AML Rule Repository Tests * * Comprehensive test coverage for AML rule repository including - * CRUD operations, versioning, and rollback functionality. + * CRUD operations, versioning, rollback functionality, failure paths, + * and boundary conditions. */ import { AMLRuleRepository } from './amlRuleRepository'; -import { Pool } from 'pg'; +import { Pool, QueryResult } from 'pg'; import { CreateRuleInput, UpdateRuleInput, SemVer } from './types'; +interface QueryCall { + text: string; + values?: unknown[]; +} + +interface MockQueryResult { + rows: T[]; + command?: string; + rowCount?: number; + oid?: number; + fields?: unknown[]; +} + // Mock Pool class MockPool { - private client: any; + public client: MockClient; constructor() { this.client = new MockClient(); } - async connect() { + async connect(): Promise { return this.client; } - async query(text: string, values?: any[]) { - return this.client.query(text, values); + async query(text: string, values?: unknown[]): Promise> { + return this.client.query(text, values) as Promise>; } } class MockClient { - private queries: any[] = []; - private inTransaction = false; - - async query(text: string, values?: any[]) { + public queries: QueryCall[] = []; + public inTransaction = false; + public isReleased = false; + public simulatedError: Error | null = null; + public errorOnQueryText: string | null = null; + public nonexistentRuleIds: Set = new Set([ + 'nonexistent', + '', + ' ', + 'rule_not_found', + 'rule/404#special!', + 'rule@special#1', + '00000000-0000-0000-0000-000000000000', + ]); + public nonexistentVersions: Set = new Set(); + + async query(text: string, values?: unknown[]): Promise> { this.queries.push({ text, values }); + + if (this.simulatedError) { + throw this.simulatedError; + } + + if (this.errorOnQueryText && text.includes(this.errorOnQueryText)) { + throw new Error(`Simulated database failure on: ${this.errorOnQueryText}`); + } // Handle BEGIN/COMMIT/ROLLBACK if (text.includes('BEGIN')) { @@ -52,12 +87,12 @@ class MockClient { return { rows: [{ id: 'rule_test_123', - name: values?.[1] || 'Test Rule', - description: values?.[2] || 'Test description', - type: values?.[3] || 'velocity', + name: (values?.[1] as string) || 'Test Rule', + description: (values?.[2] as string) || 'Test description', + type: (values?.[3] as string) || 'velocity', version: values?.[4] || { major: 1, minor: 0, patch: 0 }, - severity: values?.[5] || 'high', - enabled: values?.[6] ?? true, + severity: (values?.[5] as string) || 'high', + enabled: (values?.[6] as boolean) ?? true, config: values?.[7] || {}, created_at: new Date(), updated_at: new Date(), @@ -67,12 +102,13 @@ class MockClient { // Handle SELECT by ID if (text.includes('WHERE id = $1')) { - if (values && values[0] === 'nonexistent') { + const ruleId = values ? String(values[0]) : ''; + if (this.nonexistentRuleIds.has(ruleId)) { return { rows: [] }; } return { rows: [{ - id: values?.[0] || 'rule_1', + id: ruleId || 'rule_1', name: 'Test Rule', description: 'Test description', type: 'velocity', @@ -87,7 +123,7 @@ class MockClient { } // Handle SELECT all - if (text.includes('SELECT * FROM aml_rules')) { + if (text.includes('SELECT * FROM aml_rules') && !text.includes('WHERE')) { return { rows: [{ id: 'rule_1', @@ -104,41 +140,51 @@ class MockClient { }; } - // Handle UPDATE - if (text.includes('UPDATE aml_rules')) { - return { - rows: [{ - id: values ? values[values.length - 1] : 'rule_1', - name: 'Updated Rule', - description: 'Updated description', - type: 'velocity', - version: { major: 1, minor: 1, patch: 0 }, - severity: 'high', - enabled: true, - config: { window_minutes: 120 }, - created_at: new Date(), - updated_at: new Date(), - }] - }; - } - // Handle version history if (text.includes('aml_rule_version_history')) { - // Check if looking for specific version that doesn't exist + // Check if looking for specific version if (text.includes('WHERE rule_id = $1 AND version = $2')) { - const targetVersion = values?.[1]; - if (targetVersion && targetVersion.includes('99')) { + const ruleId = values ? String(values[0]) : ''; + const targetVersionStr = values ? String(values[1]) : ''; + if ( + this.nonexistentRuleIds.has(ruleId) || + targetVersionStr.includes('99') || + targetVersionStr.includes('"major":0,"minor":0,"patch":0') || + targetVersionStr.includes('-1') || + this.nonexistentVersions.has(targetVersionStr) + ) { return { rows: [] }; // Version not found } + + let parsedVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + try { + parsedVersion = JSON.parse(targetVersionStr); + } catch { + // fallback + } + + return { + rows: [{ + id: 'history_1', + rule_id: ruleId || 'rule_1', + version: parsedVersion, + config: { window_minutes: 60, max_amount: 10000 }, + enabled: true, + changed_by: 'user_123', + change_reason: 'Initial rule creation', + created_at: new Date(), + }] + }; } - // Return empty for nonexistent rule - if (values?.[0] === 'nonexistent') { + + // Return empty for nonexistent rule in version history list + if (values && this.nonexistentRuleIds.has(String(values[0]))) { return { rows: [] }; } return { rows: [{ id: 'history_1', - rule_id: values?.[0] || 'rule_1', + rule_id: values ? String(values[0]) : 'rule_1', version: { major: 1, minor: 0, patch: 0 }, config: { window_minutes: 60 }, enabled: true, @@ -149,26 +195,53 @@ class MockClient { }; } - // Handle UPDATE for rollback - if (text.includes('UPDATE aml_rules') && text.includes('SET config = $1')) { - // This is a rollback operation - the repository increments patch - // Target version is passed in values[2], repository returns {major, minor, patch + 1} - const targetVersion = values?.[2] ? JSON.parse(values[2]) : { major: 1, minor: 0, patch: 0 }; - const rollbackVersion = { - major: targetVersion.major, - minor: targetVersion.minor, - patch: targetVersion.patch + 1 - }; + // Handle UPDATE for rollback (must precede generic UPDATE) + if (text.includes('UPDATE aml_rules') && text.replace(/\s+/g, ' ').includes('SET config = $1, enabled = $2, version = $3')) { + const rollbackVersion = values?.[2] + ? (typeof values[2] === 'string' ? JSON.parse(values[2]) : values[2]) + : { major: 1, minor: 0, patch: 1 }; + const config = values?.[0] + ? (typeof values[0] === 'string' ? JSON.parse(values[0]) : values[0]) + : {}; return { rows: [{ - id: values?.[3] || 'rule_1', + id: values?.[3] ? String(values[3]) : 'rule_1', name: 'Test Rule', description: 'Test description', type: 'velocity', version: rollbackVersion, severity: 'high', - enabled: values?.[1] || true, - config: values?.[0] ? JSON.parse(values[0]) : {}, + enabled: values?.[1] ?? true, + config, + created_at: new Date(), + updated_at: new Date(), + }] + }; + } + + // Handle normal UPDATE + if (text.includes('UPDATE aml_rules')) { + const ruleId = values ? String(values[values.length - 1]) : 'rule_1'; + let versionObj: SemVer = { major: 1, minor: 1, patch: 0 }; + for (const val of values || []) { + if (typeof val === 'string' && val.includes('"major"')) { + try { + versionObj = JSON.parse(val); + } catch { + // ignore + } + } + } + return { + rows: [{ + id: ruleId, + name: 'Updated Rule', + description: 'Updated description', + type: 'velocity', + version: versionObj, + severity: 'high', + enabled: true, + config: { window_minutes: 120 }, created_at: new Date(), updated_at: new Date(), }] @@ -178,18 +251,19 @@ class MockClient { return { rows: [] }; } - release() { + release(): void { this.inTransaction = false; + this.isReleased = true; } } describe('AMLRuleRepository', () => { let repository: AMLRuleRepository; - let mockPool: any; + let mockPool: MockPool; beforeEach(() => { mockPool = new MockPool(); - repository = new AMLRuleRepository(mockPool as Pool); + repository = new AMLRuleRepository(mockPool as unknown as Pool); }); describe('create', () => { @@ -212,6 +286,8 @@ describe('AMLRuleRepository', () => { expect(rule.name).toBe(input.name); expect(rule.version).toEqual({ major: 1, minor: 0, patch: 0 }); expect(rule.enabled).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.queries.some(q => q.text.includes('COMMIT'))).toBe(true); }); it('should record version history on creation', async () => { @@ -229,6 +305,25 @@ describe('AMLRuleRepository', () => { expect(history).toHaveLength(1); expect(history[0].changed_by).toBe('user_123'); expect(history[0].change_reason).toBe('Initial rule creation'); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should rollback transaction and release client if error occurs during creation', async () => { + mockPool.client.errorOnQueryText = 'INSERT INTO aml_rules'; + const input: CreateRuleInput = { + name: 'Failing Rule', + description: 'Fails to insert', + type: 'velocity', + severity: 'high', + config: {}, + }; + + await expect(repository.create(input, 'user_123')) + .rejects.toThrow('Simulated database failure on: INSERT INTO aml_rules'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.inTransaction).toBe(false); }); }); @@ -246,6 +341,48 @@ describe('AMLRuleRepository', () => { expect(rule).toBeNull(); }); + + it('should return null for empty string rule ID', async () => { + const rule = await repository.findById(''); + + expect(rule).toBeNull(); + }); + + it('should return null for whitespace rule ID', async () => { + const rule = await repository.findById(' '); + + expect(rule).toBeNull(); + }); + + it('should handle JSON stringified version and config correctly', async () => { + const customClient = { + query: jest.fn().mockResolvedValue({ + rows: [{ + id: 'rule_json_test', + name: 'Rule with stringified json', + description: 'Test description', + type: 'velocity', + version: JSON.stringify({ major: 2, minor: 1, patch: 0 }), + severity: 'high', + enabled: true, + config: JSON.stringify({ threshold: 500 }), + created_at: new Date(), + updated_at: new Date(), + }] + }), + }; + const customPool = { + connect: jest.fn().mockResolvedValue(customClient), + query: customClient.query, + }; + const repo = new AMLRuleRepository(customPool as unknown as Pool); + + const rule = await repo.findById('rule_json_test'); + + expect(rule).toBeDefined(); + expect(rule?.version).toEqual({ major: 2, minor: 1, patch: 0 }); + expect(rule?.config).toEqual({ threshold: 500 }); + }); }); describe('findEnabled', () => { @@ -268,66 +405,172 @@ describe('AMLRuleRepository', () => { }); describe('update', () => { - it('should update rule and increment version', async () => { - const input: UpdateRuleInput = { - name: 'Updated Rule', - description: 'Updated description', - enabled: true, - config: { window_minutes: 120 }, - change_reason: 'Updated threshold', - }; - - const rule = await repository.update('rule_1', input, 'user_123'); - - expect(rule).toBeDefined(); - expect(rule.name).toBe(input.name); - expect(rule.version.minor).toBeGreaterThan(0); - }); - - it('should increment minor version for config changes', async () => { - const input: UpdateRuleInput = { - config: { new_param: true }, - change_reason: 'Config change', - }; - - const rule = await repository.update('rule_1', input, 'user_123'); - - expect(rule.version.minor).toBe(1); - expect(rule.version.patch).toBe(0); - }); - - it('should increment patch version for metadata changes', async () => { - const input: UpdateRuleInput = { - enabled: false, - change_reason: 'Disable rule', - }; - - const rule = await repository.update('rule_1', input, 'user_123'); - - expect(rule.version.minor).toBe(1); // Mock returns this - expect(rule.version.patch).toBe(0); // Mock returns this + describe('normal paths', () => { + it('should update rule and increment version', async () => { + const input: UpdateRuleInput = { + name: 'Updated Rule', + description: 'Updated description', + enabled: true, + config: { window_minutes: 120 }, + change_reason: 'Updated threshold', + }; + + const rule = await repository.update('rule_1', input, 'user_123'); + + expect(rule).toBeDefined(); + expect(rule.name).toBe(input.name); + expect(rule.version.minor).toBeGreaterThan(0); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.queries.some(q => q.text.includes('COMMIT'))).toBe(true); + }); + + it('should increment minor version for config changes', async () => { + const input: UpdateRuleInput = { + config: { new_param: true }, + change_reason: 'Config change', + }; + + const rule = await repository.update('rule_1', input, 'user_123'); + + expect(rule.version.minor).toBe(1); + expect(rule.version.patch).toBe(0); + }); + + it('should increment patch version for metadata changes', async () => { + const input: UpdateRuleInput = { + enabled: false, + change_reason: 'Disable rule', + }; + + const rule = await repository.update('rule_1', input, 'user_123'); + + // When only metadata or enabled changes, minor is current.minor (0) and patch is current.patch + 1 (1) + expect(rule.version.minor).toBe(0); + expect(rule.version.patch).toBe(1); + }); + + it('should increment patch version when updating name or description only', async () => { + const input: UpdateRuleInput = { + name: 'New Name Only', + description: 'New Description Only', + change_reason: 'Update name and description only', + }; + + const rule = await repository.update('rule_1', input, 'user_123'); + + expect(rule.version.minor).toBe(0); + expect(rule.version.patch).toBe(1); + }); + + it('should increment patch version when input has no fields specified', async () => { + const input: UpdateRuleInput = { + change_reason: 'No changes except audit reason', + }; + + const rule = await repository.update('rule_1', input, 'user_123'); + + expect(rule.version.minor).toBe(0); + expect(rule.version.patch).toBe(1); + }); + + it('should record version history on update', async () => { + const input: UpdateRuleInput = { + name: 'Updated', + change_reason: 'Update reason', + }; + + await repository.update('rule_1', input, 'user_456'); + + const history = await repository.getVersionHistory('rule_1'); + expect(history.length).toBeGreaterThan(0); + expect(mockPool.client.isReleased).toBe(true); + }); }); - it('should record version history on update', async () => { - const input: UpdateRuleInput = { - name: 'Updated', - change_reason: 'Update', - }; - - await repository.update('rule_1', input, 'user_456'); - - const history = await repository.getVersionHistory('rule_1'); - expect(history.length).toBeGreaterThan(0); - }); - - it('should throw error for nonexistent rule', async () => { - const input: UpdateRuleInput = { - name: 'Updated', - change_reason: 'Update', - }; - - await expect(repository.update('nonexistent', input, 'user_123')) - .rejects.toThrow('Rule nonexistent not found'); + describe('failure paths and boundary inputs (evidence src/aml/amlRuleRepository.ts:130)', () => { + it('should throw explicit error and rollback when rule is not found', async () => { + const input: UpdateRuleInput = { + name: 'Updated', + change_reason: 'Update', + }; + + await expect(repository.update('nonexistent', input, 'user_123')) + .rejects.toThrow('Rule nonexistent not found'); + + // Assert deterministic error and transactional rollback + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.queries.some(q => q.text.includes('COMMIT'))).toBe(false); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.inTransaction).toBe(false); + }); + + it('should throw explicit error when ruleId is empty string', async () => { + const input: UpdateRuleInput = { + name: 'Updated', + change_reason: 'Update empty rule ID', + }; + + await expect(repository.update('', input, 'user_123')) + .rejects.toThrow('Rule not found'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when ruleId is whitespace only', async () => { + const input: UpdateRuleInput = { + name: 'Updated', + change_reason: 'Update whitespace rule ID', + }; + + await expect(repository.update(' ', input, 'user_123')) + .rejects.toThrow('Rule not found'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when ruleId contains special characters', async () => { + const input: UpdateRuleInput = { + name: 'Updated', + change_reason: 'Update special chars rule ID', + }; + + await expect(repository.update('rule/404#special!', input, 'user_123')) + .rejects.toThrow('Rule rule/404#special! not found'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when ruleId is a non-existent UUID format', async () => { + const nonExistentUuid = '00000000-0000-0000-0000-000000000000'; + const input: UpdateRuleInput = { + name: 'Updated', + change_reason: 'Update nonexistent UUID', + }; + + await expect(repository.update(nonExistentUuid, input, 'user_123')) + .rejects.toThrow(`Rule ${nonExistentUuid} not found`); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should rollback transaction and release client when query fails during update execution', async () => { + mockPool.client.errorOnQueryText = 'UPDATE aml_rules'; + const input: UpdateRuleInput = { + name: 'Should Fail During Update', + change_reason: 'Simulated failure during update', + }; + + await expect(repository.update('rule_1', input, 'user_123')) + .rejects.toThrow('Simulated database failure on: UPDATE aml_rules'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.inTransaction).toBe(false); + }); }); }); @@ -344,34 +587,163 @@ describe('AMLRuleRepository', () => { expect(history).toEqual([]); }); + + it('should correctly parse stringified JSON version and config in version history', async () => { + const customClient = { + query: jest.fn().mockResolvedValue({ + rows: [{ + id: 'history_json_test', + rule_id: 'rule_1', + version: JSON.stringify({ major: 1, minor: 2, patch: 3 }), + config: JSON.stringify({ custom_field: 'value' }), + enabled: true, + changed_by: 'user_test', + change_reason: 'Testing json parsing', + created_at: new Date(), + }] + }), + }; + const customPool = { + connect: jest.fn().mockResolvedValue(customClient), + query: customClient.query, + }; + const repo = new AMLRuleRepository(customPool as unknown as Pool); + + const history = await repo.getVersionHistory('rule_1'); + + expect(history).toHaveLength(1); + expect(history[0].version).toEqual({ major: 1, minor: 2, patch: 3 }); + expect(history[0].config).toEqual({ custom_field: 'value' }); + }); }); describe('rollbackToVersion', () => { - it('should rollback to specific version', async () => { - const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + describe('normal paths', () => { + it('should rollback to specific version and increment patch', async () => { + const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + + const rule = await repository.rollbackToVersion('rule_1', targetVersion, 'user_123'); + + expect(rule).toBeDefined(); + expect(rule.version.major).toBe(1); + expect(rule.version.minor).toBe(0); + expect(rule.version.patch).toBe(1); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.queries.some(q => q.text.includes('COMMIT'))).toBe(true); + }); + + it('should record rollback in history with explicit audit reason', async () => { + const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + + await repository.rollbackToVersion('rule_1', targetVersion, 'user_123'); + + const history = await repository.getVersionHistory('rule_1'); + expect(history.length).toBeGreaterThan(0); + // Verify query recorded rollback version history + const insertHistoryQuery = mockPool.client.queries.find(q => + q.text.includes('INSERT INTO aml_rule_version_history') + ); + expect(insertHistoryQuery).toBeDefined(); + expect(insertHistoryQuery?.values).toContain('Rollback to version {"major":1,"minor":0,"patch":0}'); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should increment patch when rolling back to version with existing non-zero patch', async () => { + const targetVersion: SemVer = { major: 1, minor: 2, patch: 3 }; + + const rule = await repository.rollbackToVersion('rule_1', targetVersion, 'user_123'); + + expect(rule).toBeDefined(); + expect(rule.version).toEqual({ major: 1, minor: 2, patch: 4 }); + expect(mockPool.client.isReleased).toBe(true); + }); + }); - const rule = await repository.rollbackToVersion('rule_1', targetVersion, 'user_123'); + describe('failure paths and boundary inputs (evidence src/aml/amlRuleRepository.ts:236)', () => { + it('should throw explicit error contract and rollback when version does not exist for rule', async () => { + const targetVersion: SemVer = { major: 99, minor: 99, patch: 99 }; - expect(rule).toBeDefined(); - expect(rule.version.major).toBe(1); - expect(rule.version.minor).toBe(1); // Mock UPDATE handler returns this - expect(rule.version.patch).toBe(0); - }); + await expect(repository.rollbackToVersion('rule_1', targetVersion, 'user_123')) + .rejects.toThrow('Version {"major":99,"minor":99,"patch":99} not found for rule rule_1'); - it('should record rollback in history', async () => { - const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + // Verify transaction was rolled back and client released + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.queries.some(q => q.text.includes('COMMIT'))).toBe(false); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.inTransaction).toBe(false); + }); - await repository.rollbackToVersion('rule_1', targetVersion, 'user_123'); + it('should throw explicit error when version is boundary 0.0.0 and not found in history', async () => { + const targetVersion: SemVer = { major: 0, minor: 0, patch: 0 }; - const history = await repository.getVersionHistory('rule_1'); - expect(history.length).toBeGreaterThan(0); - }); + await expect(repository.rollbackToVersion('rule_1', targetVersion, 'user_123')) + .rejects.toThrow('Version {"major":0,"minor":0,"patch":0} not found for rule rule_1'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when target version has negative values', async () => { + const targetVersion: SemVer = { major: 0, minor: -1, patch: 0 }; + + await expect(repository.rollbackToVersion('rule_1', targetVersion, 'user_123')) + .rejects.toThrow('Version {"major":0,"minor":-1,"patch":0} not found for rule rule_1'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when ruleId does not exist for rollback', async () => { + const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + + await expect(repository.rollbackToVersion('nonexistent', targetVersion, 'user_123')) + .rejects.toThrow('Version {"major":1,"minor":0,"patch":0} not found for rule nonexistent'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when ruleId is empty string', async () => { + const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + + await expect(repository.rollbackToVersion('', targetVersion, 'user_123')) + .rejects.toThrow('Version {"major":1,"minor":0,"patch":0} not found for rule '); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when ruleId contains whitespace only', async () => { + const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + + await expect(repository.rollbackToVersion(' ', targetVersion, 'user_123')) + .rejects.toThrow('Version {"major":1,"minor":0,"patch":0} not found for rule '); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should throw explicit error when ruleId contains special characters', async () => { + const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; + + await expect(repository.rollbackToVersion('rule@special#1', targetVersion, 'user_123')) + .rejects.toThrow('Version {"major":1,"minor":0,"patch":0} not found for rule rule@special#1'); + + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + }); + + it('should rollback transaction and release client when query fails during rollback execution', async () => { + mockPool.client.errorOnQueryText = 'UPDATE aml_rules'; + const targetVersion: SemVer = { major: 1, minor: 0, patch: 0 }; - it('should throw error for nonexistent version', async () => { - const targetVersion: SemVer = { major: 99, minor: 99, patch: 99 }; + await expect(repository.rollbackToVersion('rule_1', targetVersion, 'user_123')) + .rejects.toThrow('Simulated database failure on: UPDATE aml_rules'); - await expect(repository.rollbackToVersion('rule_1', targetVersion, 'user_123')) - .rejects.toThrow(); + expect(mockPool.client.queries.some(q => q.text.includes('ROLLBACK'))).toBe(true); + expect(mockPool.client.isReleased).toBe(true); + expect(mockPool.client.inTransaction).toBe(false); + }); }); }); });