From ca3c06c788211869eff1de95f6b3fb30c7327f62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tao=20Bojl=C3=A9n?= Date: Mon, 22 Jun 2026 12:24:21 +0100 Subject: [PATCH] test: isolate server tests via transaction rollback (#732) * test: isolate DB-backed server tests via transaction rollback Introduce a transaction-rollback test harness so DB-backed tests get automatic per-test isolation with no hand-written cleanup, replacing the fixture-teardown (and hardcoded-id) patterns that leaked state across runs. Mechanism (server/test/harness/transactionalPgPool.ts): - A pg.Pool facade that pins one connection and rewrites the app's BEGIN/COMMIT/ROLLBACK to SAVEPOINTs, so an outer test transaction rolls everything back -- including the app's own makeKyselyTransactionWithRetry transactions. Also exposes pool.query for the express-session store. Adoption: - makeMockedServer is now transactional: real Postgres (mocked ClickHouse) wired through the rolled-back connection, exposing rollback()/shutdown(). getBottleContainerWithIOMocks gains an optional kyselyPool override for it. - makeTransactionalTestWithFixture drives rollback from the test lifecycle, so fixtures return only their data -- no cleanup. - Convert the makeMockedServer-tier tests and detectRulePassRateAnomalies to the harness, dropping all per-test cleanup. - Extract getPgConnectionParams so the harness reads the same DATABASE_* env vars as the app. Postgres-only: Scylla/ClickHouse/Redis writes are not rolled back; tests touching those keep uid-based isolation. Co-Authored-By: Claude Opus 4.8 * test: tighten harness query types and guard savepoint underflow Replace blanket no-explicit-any suppressions on the transactional pool's query path with proper pg types (string | QueryConfig, unknown[]), and guard COMMIT/ROLLBACK rewrites against savepoint-depth underflow by asserting depth > 0 and only decrementing after the SQL succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) * test: guarantee pinned-connection cleanup with try/finally Wrap each transactionalPgPool integ test body in try/finally so a mid-test throw can't leak the pinned Postgres connection; closing it in finally also aborts any still-open outer transaction. Co-Authored-By: Claude Opus 4.8 (1M context) * test: reject unsupported transaction-control statements in pg harness Strip trailing semicolons before matching transaction-control statements (so `COMMIT;` is rewritten too), and reject transaction-control commands the savepoint rewrite doesn't handle (END, ABORT, SAVEPOINT, RELEASE, COMMIT/ROLLBACK PREPARED, PREPARE TRANSACTION) with an explanatory error instead of silently forwarding them and defeating per-test isolation. Extract the savepoint open/close logic into helpers to cut duplication, and add a regression test covering the new rejection behaviour. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- server/graphql/datasources/OrgApi.test.ts | 112 +-- server/graphql/datasources/RuleApi.test.ts | 63 +- .../datasources/orgKyselyPersistence.test.ts | 45 +- .../datasources/userKyselyPersistence.test.ts | 314 +++---- server/iocContainer/index.ts | 18 +- server/routes/content/ContentRoutes.test.ts | 341 ++++--- server/routes/items/ItemRoutes.test.ts | 176 ++-- server/routes/policies/PoliciesRoutes.test.ts | 73 +- .../routes/reporting/ReportingRoutes.test.ts | 851 +++++++++--------- .../user_scores/UserScoresRoutes.test.ts | 64 +- .../detectRulePassRateAnomaliesJob.test.ts | 311 +++---- .../aggregation/AggregationSignal.test.ts | 36 +- .../fixtureHelpers/fixtureHelpers.test.ts | 126 +-- .../harness/transactionalPgPool.integ.test.ts | 157 ++++ server/test/harness/transactionalPgPool.ts | 176 ++++ server/test/harness/transactionalTest.ts | 39 + server/test/setupMockedServer.ts | 51 +- 17 files changed, 1530 insertions(+), 1423 deletions(-) create mode 100644 server/test/harness/transactionalPgPool.integ.test.ts create mode 100644 server/test/harness/transactionalPgPool.ts create mode 100644 server/test/harness/transactionalTest.ts diff --git a/server/graphql/datasources/OrgApi.test.ts b/server/graphql/datasources/OrgApi.test.ts index 3d043c0..51ed284 100644 --- a/server/graphql/datasources/OrgApi.test.ts +++ b/server/graphql/datasources/OrgApi.test.ts @@ -4,18 +4,13 @@ import { uid } from 'uid'; import { UserRole } from '../../services/userManagementService/index.js'; import createContentItemTypes from '../../test/fixtureHelpers/createContentItemTypes.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; import { CoopError } from '../../utils/errors.js'; -import { - kyselyUserDeleteById, - kyselyUserInsert, -} from './userKyselyPersistence.js'; +import { kyselyUserInsert } from './userKyselyPersistence.js'; describe('OrgAPI', () => { - const testWithFixture = makeTestWithFixture(async () => { - const { deps, shutdown } = await makeMockedServer(); - const { org, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -23,14 +18,7 @@ describe('OrgAPI', () => { }, uid(), ); - return { - deps, - org, - async cleanup() { - await orgCleanup(); - await shutdown(); - }, - }; + return { org }; }); describe('getGraphQLOrgFromId', () => { @@ -138,29 +126,25 @@ describe('OrgAPI', () => { testWithFixture( 'returns every item type for the org with the fields read by the ContentType resolver', async ({ deps, org }) => { - const { itemTypes, cleanup } = await createContentItemTypes({ + const { itemTypes } = await createContentItemTypes({ moderationConfigService: deps.ModerationConfigService, orgId: org.id, numItemTypes: 1, extra: {}, }); - try { - const result = await deps.OrgAPIDataSource.getContentTypesForOrg( - org.id, - ); - const actualIds = new Set(result.map((it) => it.id)); - for (const created of itemTypes) { - expect(actualIds.has(created.id)).toBe(true); - } - for (const it of result) { - expect(it.orgId).toBe(org.id); - expect(typeof it.id).toBe('string'); - expect(typeof it.name).toBe('string'); - expect(['CONTENT', 'USER', 'THREAD']).toContain(it.kind); - expect(Array.isArray(it.schema)).toBe(true); - } - } finally { - await cleanup(); + const result = await deps.OrgAPIDataSource.getContentTypesForOrg( + org.id, + ); + const actualIds = new Set(result.map((it) => it.id)); + for (const created of itemTypes) { + expect(actualIds.has(created.id)).toBe(true); + } + for (const it of result) { + expect(it.orgId).toBe(org.id); + expect(typeof it.id).toBe('string'); + expect(typeof it.name).toBe('string'); + expect(['CONTENT', 'USER', 'THREAD']).toContain(it.kind); + expect(Array.isArray(it.schema)).toBe(true); } }, ); @@ -179,7 +163,7 @@ describe('OrgAPI', () => { testWithFixture( 'does not leak item types across orgs', async ({ deps, org }) => { - const { org: otherOrg, cleanup: otherOrgCleanup } = await createOrg( + const { org: otherOrg } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -187,16 +171,12 @@ describe('OrgAPI', () => { }, uid(), ); - try { - const result = await deps.OrgAPIDataSource.getContentTypesForOrg( - org.id, - ); - for (const it of result) { - expect(it.orgId).toBe(org.id); - expect(it.orgId).not.toBe(otherOrg.id); - } - } finally { - await otherOrgCleanup(); + const result = await deps.OrgAPIDataSource.getContentTypesForOrg( + org.id, + ); + for (const it of result) { + expect(it.orgId).toBe(org.id); + expect(it.orgId).not.toBe(otherOrg.id); } }, ); @@ -230,22 +210,15 @@ describe('OrgAPI', () => { loginMethods: ['saml'], password: null, }); - try { - const users = await deps.OrgAPIDataSource.getOrgUsersForGraphQL( - org.id, - ); - const ids = users.map((u) => u.id).sort(); - expect(ids).toEqual([adminId, analystId].sort()); - const admin = users.find((u) => u.id === adminId)!; - const analyst = users.find((u) => u.id === analystId)!; - expect(admin.getPermissions()).toEqual( - expect.arrayContaining(['EDIT_MRT_QUEUES']), - ); - expect(analyst.getPermissions()).not.toContain('EDIT_MRT_QUEUES'); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, adminId); - await kyselyUserDeleteById(deps.KyselyPg, analystId); - } + const users = await deps.OrgAPIDataSource.getOrgUsersForGraphQL(org.id); + const ids = users.map((u) => u.id).sort(); + expect(ids).toEqual([adminId, analystId].sort()); + const admin = users.find((u) => u.id === adminId)!; + const analyst = users.find((u) => u.id === analystId)!; + expect(admin.getPermissions()).toEqual( + expect.arrayContaining(['EDIT_MRT_QUEUES']), + ); + expect(analyst.getPermissions()).not.toContain('EDIT_MRT_QUEUES'); }, ); @@ -262,7 +235,7 @@ describe('OrgAPI', () => { testWithFixture( 'does not leak users across orgs', async ({ deps, org }) => { - const { org: otherOrg, cleanup: otherOrgCleanup } = await createOrg( + const { org: otherOrg } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -282,15 +255,10 @@ describe('OrgAPI', () => { loginMethods: ['saml'], password: null, }); - try { - const result = await deps.OrgAPIDataSource.getOrgUsersForGraphQL( - org.id, - ); - expect(result.find((u) => u.id === otherUserId)).toBeUndefined(); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, otherUserId); - await otherOrgCleanup(); - } + const result = await deps.OrgAPIDataSource.getOrgUsersForGraphQL( + org.id, + ); + expect(result.find((u) => u.id === otherUserId)).toBeUndefined(); }, ); }); diff --git a/server/graphql/datasources/RuleApi.test.ts b/server/graphql/datasources/RuleApi.test.ts index 8907b7b..28da175 100644 --- a/server/graphql/datasources/RuleApi.test.ts +++ b/server/graphql/datasources/RuleApi.test.ts @@ -3,8 +3,7 @@ import { uid } from 'uid'; import createContentItemTypes from '../../test/fixtureHelpers/createContentItemTypes.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createUser from '../../test/fixtureHelpers/createUser.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; import { type GQLCreateContentRuleInput, type GQLCreateUserRuleInput, @@ -23,42 +22,28 @@ const minimalConditionSet = { }; describe('RuleAPI', () => { - const testWithRuleApiFixture = makeTestWithFixture(async () => { - const { deps, shutdown } = await makeMockedServer(); - const { ModerationConfigService } = deps; - const { org, cleanup: orgCleanup } = await createOrg( - { - KyselyPg: deps.KyselyPg, - ModerationConfigService, - ApiKeyService: deps.ApiKeyService, - }, - uid(), - ); - const { user, cleanup: userCleanup } = await createUser( - deps.KyselyPg, - org.id, - ); - const { itemTypes, cleanup: itemTypesCleanup } = - await createContentItemTypes({ + const testWithRuleApiFixture = makeTransactionalTestWithFixture( + async ({ deps }) => { + const { ModerationConfigService } = deps; + const { org } = await createOrg( + { + KyselyPg: deps.KyselyPg, + ModerationConfigService, + ApiKeyService: deps.ApiKeyService, + }, + uid(), + ); + const { user } = await createUser(deps.KyselyPg, org.id); + const { itemTypes } = await createContentItemTypes({ moderationConfigService: ModerationConfigService, orgId: org.id, includeCreator: true, extra: {}, }); - return { - deps, - org, - user, - itemTypes, - async cleanup() { - await itemTypesCleanup(); - await userCleanup(); - await orgCleanup(); - await shutdown(); - }, - }; - }); + return { org, user, itemTypes }; + }, + ); testWithRuleApiFixture( 'createContentRule + getGraphQLRuleFromId round-trip', @@ -86,8 +71,6 @@ describe('RuleAPI', () => { ); expect(fetched.id).toBe(rule.id); expect(fetched.name).toBe(name); - - await deps.RuleAPIDataSource.deleteRule({ id: rule.id, orgId: org.id }); }, ); @@ -116,8 +99,6 @@ describe('RuleAPI', () => { ); expect(fetched.id).toBe(rule.id); expect(fetched.name).toBe(name); - - await deps.RuleAPIDataSource.deleteRule({ id: rule.id, orgId: org.id }); }, ); @@ -156,8 +137,6 @@ describe('RuleAPI', () => { ); expect(plain).not.toBeNull(); expect(plain!.expirationTime?.getTime()).toBe(future.getTime()); - - await deps.RuleAPIDataSource.deleteRule({ id: rule.id, orgId: org.id }); }, ); @@ -199,8 +178,6 @@ describe('RuleAPI', () => { const expMs = plain!.expirationTime!.getTime(); expect(expMs).toBeGreaterThanOrEqual(before); expect(expMs).toBeLessThanOrEqual(after + 2000); - - await deps.RuleAPIDataSource.deleteRule({ id: rule.id, orgId: org.id }); }, ); @@ -208,7 +185,7 @@ describe('RuleAPI', () => { 'duplicate rule name yields RuleNameExistsError', async ({ deps, user, org }) => { const name = `Dup name ${uid()}`; - const first = await deps.RuleAPIDataSource.createUserRule( + await deps.RuleAPIDataSource.createUserRule( { name, description: null, @@ -239,8 +216,6 @@ describe('RuleAPI', () => { org.id, ), ).rejects.toMatchObject({ name: 'RuleNameExistsError' }); - - await deps.RuleAPIDataSource.deleteRule({ id: first.id, orgId: org.id }); }, ); @@ -286,8 +261,6 @@ describe('RuleAPI', () => { orgId: org.id, }), ).rejects.toMatchObject({ name: 'RuleHasRunningBacktestsError' }); - - await deps.RuleAPIDataSource.deleteRule({ id: rule.id, orgId: org.id }); }, ); diff --git a/server/graphql/datasources/orgKyselyPersistence.test.ts b/server/graphql/datasources/orgKyselyPersistence.test.ts index 2b990cb..b7ce06c 100644 --- a/server/graphql/datasources/orgKyselyPersistence.test.ts +++ b/server/graphql/datasources/orgKyselyPersistence.test.ts @@ -2,10 +2,8 @@ import { faker } from '@faker-js/faker'; import { uid } from 'uid'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; import { - kyselyOrgDeleteById, kyselyOrgFindByEmail, kyselyOrgFindById, kyselyOrgFindByName, @@ -14,9 +12,8 @@ import { } from './orgKyselyPersistence.js'; describe('orgKyselyPersistence', () => { - const testWithFixture = makeTestWithFixture(async () => { - const { deps, shutdown } = await makeMockedServer(); - const { org, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -24,14 +21,7 @@ describe('orgKyselyPersistence', () => { }, uid(), ); - return { - deps, - org, - async cleanup() { - await orgCleanup(); - await shutdown(); - }, - }; + return { org }; }); describe('kyselyOrgFindBy*', () => { @@ -99,19 +89,14 @@ describe('orgKyselyPersistence', () => { // apiKeyId intentionally omitted }); - try { - expect(inserted.id).toBe(id); - - const row = await deps.KyselyPg - .selectFrom('public.orgs') - .select(['api_key_id', 'on_call_alert_email']) - .where('id', '=', id) - .executeTakeFirstOrThrow(); - expect(row.api_key_id).toBeNull(); - expect(row.on_call_alert_email).toBeNull(); - } finally { - await kyselyOrgDeleteById(deps.KyselyPg, id); - } + expect(inserted.id).toBe(id); + + const row = await deps.KyselyPg.selectFrom('public.orgs') + .select(['api_key_id', 'on_call_alert_email']) + .where('id', '=', id) + .executeTakeFirstOrThrow(); + expect(row.api_key_id).toBeNull(); + expect(row.on_call_alert_email).toBeNull(); }, ); }); @@ -206,8 +191,7 @@ describe('orgKyselyPersistence', () => { const newWebsite = 'https://renamed.example.com'; // Read updated_at directly since it isn't part of GraphQLOrgParent. - const beforeRow = await deps.KyselyPg - .selectFrom('public.orgs') + const beforeRow = await deps.KyselyPg.selectFrom('public.orgs') .select(['updated_at']) .where('id', '=', org.id) .executeTakeFirstOrThrow(); @@ -224,8 +208,7 @@ describe('orgKyselyPersistence', () => { expect(updated!.name).toBe(newName); expect(updated!.websiteUrl).toBe(newWebsite); - const afterRow = await deps.KyselyPg - .selectFrom('public.orgs') + const afterRow = await deps.KyselyPg.selectFrom('public.orgs') .select(['updated_at']) .where('id', '=', org.id) .executeTakeFirstOrThrow(); diff --git a/server/graphql/datasources/userKyselyPersistence.test.ts b/server/graphql/datasources/userKyselyPersistence.test.ts index 665f3d5..6d5fa10 100644 --- a/server/graphql/datasources/userKyselyPersistence.test.ts +++ b/server/graphql/datasources/userKyselyPersistence.test.ts @@ -4,11 +4,9 @@ import { uid } from 'uid'; import { UserRole } from '../../services/userManagementService/index.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createRule from '../../test/fixtureHelpers/createRule.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; import { kyselyUserAddFavoriteRule, - kyselyUserDeleteById, kyselyUserFindByEmail, kyselyUserFindById, kyselyUserFindByIdAndOrg, @@ -41,9 +39,8 @@ const adminRoleSeed = { } as const; describe('userKyselyPersistence', () => { - const testWithFixture = makeTestWithFixture(async () => { - const { deps, shutdown } = await makeMockedServer(); - const { org, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -51,14 +48,7 @@ describe('userKyselyPersistence', () => { }, uid(), ); - return { - deps, - org, - async cleanup() { - await orgCleanup(); - await shutdown(); - }, - }; + return { org }; }); describe('kyselyUserInsert', () => { @@ -70,26 +60,22 @@ describe('userKyselyPersistence', () => { db: deps.KyselyPg, ...input, }); - try { - expect(inserted).toMatchObject({ - id: input.id, - orgId: org.id, - email: input.email, - firstName: input.firstName, - lastName: input.lastName, - role: UserRole.ADMIN, - loginMethods: ['saml'], - password: null, - approvedByAdmin: false, - rejectedByAdmin: false, - }); - // Fixture orgs lack `public.roles` rows, so this hits the fallback. - const permissions = inserted.getPermissions(); - expect(permissions).toContain('MANAGE_ORG'); - expect(permissions).toContain('MANAGE_ROLES'); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + expect(inserted).toMatchObject({ + id: input.id, + orgId: org.id, + email: input.email, + firstName: input.firstName, + lastName: input.lastName, + role: UserRole.ADMIN, + loginMethods: ['saml'], + password: null, + approvedByAdmin: false, + rejectedByAdmin: false, + }); + // Fixture orgs lack `public.roles` rows, so this hits the fallback. + const permissions = inserted.getPermissions(); + expect(permissions).toContain('MANAGE_ORG'); + expect(permissions).toContain('MANAGE_ROLES'); }, ); @@ -98,17 +84,13 @@ describe('userKyselyPersistence', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const updated = await kyselyUserUpdate(deps.KyselyPg, input.id, { - role: UserRole.ANALYST, - }); - expect(updated!.role).toBe(UserRole.ANALYST); - const permissions = updated!.getPermissions(); - expect(permissions).toContain('VIEW_INSIGHTS'); - expect(permissions).not.toContain('MANAGE_ORG'); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const updated = await kyselyUserUpdate(deps.KyselyPg, input.id, { + role: UserRole.ANALYST, + }); + expect(updated!.role).toBe(UserRole.ANALYST); + const permissions = updated!.getPermissions(); + expect(permissions).toContain('VIEW_INSIGHTS'); + expect(permissions).not.toContain('MANAGE_ORG'); }, ); @@ -129,11 +111,7 @@ describe('userKyselyPersistence', () => { db: deps.KyselyPg, ...input, }); - try { - expect(inserted.getPermissions()).toEqual(['MANAGE_ORG']); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + expect(inserted.getPermissions()).toEqual(['MANAGE_ORG']); }, ); @@ -150,11 +128,7 @@ describe('userKyselyPersistence', () => { db: deps.KyselyPg, ...input, }); - try { - expect(inserted.getPermissions()).toEqual([]); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + expect(inserted.getPermissions()).toEqual([]); }, ); @@ -173,12 +147,8 @@ describe('userKyselyPersistence', () => { loginMethods: ['password'], password: 'hashed-password-placeholder', }); - try { - expect(row.loginMethods).toEqual(['password']); - expect(row.password).toBe('hashed-password-placeholder'); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, id); - } + expect(row.loginMethods).toEqual(['password']); + expect(row.password).toBe('hashed-password-placeholder'); }, ); @@ -227,23 +197,16 @@ describe('userKyselyPersistence', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const byId = await kyselyUserFindById(deps.KyselyPg, input.id); - const byEmail = await kyselyUserFindByEmail( - deps.KyselyPg, - input.email, - ); - const byIdAndOrg = await kyselyUserFindByIdAndOrg(deps.KyselyPg, { - id: input.id, - orgId: org.id, - }); - - expect(byId).toMatchObject({ id: input.id, email: input.email }); - expect(byEmail).toMatchObject({ id: input.id }); - expect(byIdAndOrg).toMatchObject({ id: input.id, orgId: org.id }); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const byId = await kyselyUserFindById(deps.KyselyPg, input.id); + const byEmail = await kyselyUserFindByEmail(deps.KyselyPg, input.email); + const byIdAndOrg = await kyselyUserFindByIdAndOrg(deps.KyselyPg, { + id: input.id, + orgId: org.id, + }); + + expect(byId).toMatchObject({ id: input.id, email: input.email }); + expect(byEmail).toMatchObject({ id: input.id }); + expect(byIdAndOrg).toMatchObject({ id: input.id, orgId: org.id }); }, ); @@ -274,15 +237,11 @@ describe('userKyselyPersistence', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const result = await kyselyUserFindByIdAndOrg(deps.KyselyPg, { - id: input.id, - orgId: `different-org-${uid()}`, - }); - expect(result).toBeUndefined(); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const result = await kyselyUserFindByIdAndOrg(deps.KyselyPg, { + id: input.id, + orgId: `different-org-${uid()}`, + }); + expect(result).toBeUndefined(); }, ); @@ -293,16 +252,12 @@ describe('userKyselyPersistence', () => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const rows = await kyselyUserFindByIds(deps.KyselyPg, [ - input.id, - `missing-${uid()}`, - ]); - expect(rows).toHaveLength(1); - expect(rows[0].id).toBe(input.id); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const rows = await kyselyUserFindByIds(deps.KyselyPg, [ + input.id, + `missing-${uid()}`, + ]); + expect(rows).toHaveLength(1); + expect(rows[0].id).toBe(input.id); }, ); @@ -313,15 +268,10 @@ describe('userKyselyPersistence', () => { const b = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...a }); await kyselyUserInsert({ db: deps.KyselyPg, ...b }); - try { - const rows = await kyselyUserListByOrg(deps.KyselyPg, org.id); - const ids = rows.map((r) => r.id); - expect(ids).toEqual(expect.arrayContaining([a.id, b.id])); - expect(rows.every((r) => r.orgId === org.id)).toBe(true); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, a.id); - await kyselyUserDeleteById(deps.KyselyPg, b.id); - } + const rows = await kyselyUserListByOrg(deps.KyselyPg, org.id); + const ids = rows.map((r) => r.id); + expect(ids).toEqual(expect.arrayContaining([a.id, b.id])); + expect(rows.every((r) => r.orgId === org.id)).toBe(true); }, ); }); @@ -332,15 +282,11 @@ describe('userKyselyPersistence', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - await expect( - kyselyUserUpdate(deps.KyselyPg, input.id, { - email: 'not-an-email', - }), - ).rejects.toThrow(/kyselyUserUpdate invariant violated: email/); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + await expect( + kyselyUserUpdate(deps.KyselyPg, input.id, { + email: 'not-an-email', + }), + ).rejects.toThrow(/kyselyUserUpdate invariant violated: email/); }, ); @@ -365,22 +311,18 @@ describe('userKyselyPersistence', () => { password: 'placeholder', }; await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const skipped = await kyselyUserUpdate(deps.KyselyPg, input.id, { - firstName: null, - lastName: null, - email: null, - role: null, - }); - expect(skipped).toBeDefined(); - expect(skipped!.firstName).toBe(input.firstName); - expect(skipped!.lastName).toBe(input.lastName); - expect(skipped!.email).toBe(input.email); - expect(skipped!.role).toBe(input.role); - expect(skipped!.password).toBe('placeholder'); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const skipped = await kyselyUserUpdate(deps.KyselyPg, input.id, { + firstName: null, + lastName: null, + email: null, + role: null, + }); + expect(skipped).toBeDefined(); + expect(skipped!.firstName).toBe(input.firstName); + expect(skipped!.lastName).toBe(input.lastName); + expect(skipped!.email).toBe(input.email); + expect(skipped!.role).toBe(input.role); + expect(skipped!.password).toBe('placeholder'); }, ); @@ -391,14 +333,10 @@ describe('userKyselyPersistence', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const updated = await kyselyUserUpdate(deps.KyselyPg, input.id, { - password: null, - }); - expect(updated!.password).toBeNull(); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const updated = await kyselyUserUpdate(deps.KyselyPg, input.id, { + password: null, + }); + expect(updated!.password).toBeNull(); }, ); @@ -413,13 +351,9 @@ describe('userKyselyPersistence', () => { password: 'placeholder', }; await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - await expect( - kyselyUserUpdate(deps.KyselyPg, input.id, { password: null }), - ).rejects.toThrow(/password_null_when_not_present/); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + await expect( + kyselyUserUpdate(deps.KyselyPg, input.id, { password: null }), + ).rejects.toThrow(/password_null_when_not_present/); }, ); @@ -428,31 +362,27 @@ describe('userKyselyPersistence', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const beforeRow = await deps.KyselyPg.selectFrom('public.users') - .select(['updated_at']) - .where('id', '=', input.id) - .executeTakeFirstOrThrow(); - - await new Promise((resolve) => setTimeout(resolve, 5)); - - const updated = await kyselyUserUpdate(deps.KyselyPg, input.id, { - firstName: 'Updated', - approvedByAdmin: true, - }); - expect(updated!.firstName).toBe('Updated'); - expect(updated!.approvedByAdmin).toBe(true); - - const afterRow = await deps.KyselyPg.selectFrom('public.users') - .select(['updated_at']) - .where('id', '=', input.id) - .executeTakeFirstOrThrow(); - expect(afterRow.updated_at.getTime()).toBeGreaterThan( - beforeRow.updated_at.getTime(), - ); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const beforeRow = await deps.KyselyPg.selectFrom('public.users') + .select(['updated_at']) + .where('id', '=', input.id) + .executeTakeFirstOrThrow(); + + await new Promise((resolve) => setTimeout(resolve, 5)); + + const updated = await kyselyUserUpdate(deps.KyselyPg, input.id, { + firstName: 'Updated', + approvedByAdmin: true, + }); + expect(updated!.firstName).toBe('Updated'); + expect(updated!.approvedByAdmin).toBe(true); + + const afterRow = await deps.KyselyPg.selectFrom('public.users') + .select(['updated_at']) + .where('id', '=', input.id) + .executeTakeFirstOrThrow(); + expect(afterRow.updated_at.getTime()).toBeGreaterThan( + beforeRow.updated_at.getTime(), + ); }, ); }); @@ -468,32 +398,22 @@ describe('userKyselyPersistence', () => { const rule = await createRule(deps.KyselyPg, org.id, { creatorId: input.id, }); - try { - expect( - await kyselyUserListFavoriteRuleIds(deps.KyselyPg, input.id), - ).toEqual([]); - - await kyselyUserAddFavoriteRule(deps.KyselyPg, input.id, rule.id); - // Second add must be a no-op (matches Sequelize `addFavoriteRules`). - await kyselyUserAddFavoriteRule(deps.KyselyPg, input.id, rule.id); - - expect( - await kyselyUserListFavoriteRuleIds(deps.KyselyPg, input.id), - ).toEqual([rule.id]); - - await kyselyUserRemoveFavoriteRule(deps.KyselyPg, input.id, rule.id); - expect( - await kyselyUserListFavoriteRuleIds(deps.KyselyPg, input.id), - ).toEqual([]); - } finally { - await kyselyUserRemoveFavoriteRule( - deps.KyselyPg, - input.id, - rule.id, - ).catch(() => undefined); - await rule.destroy().catch(() => undefined); - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + expect( + await kyselyUserListFavoriteRuleIds(deps.KyselyPg, input.id), + ).toEqual([]); + + await kyselyUserAddFavoriteRule(deps.KyselyPg, input.id, rule.id); + // Second add must be a no-op (matches Sequelize `addFavoriteRules`). + await kyselyUserAddFavoriteRule(deps.KyselyPg, input.id, rule.id); + + expect( + await kyselyUserListFavoriteRuleIds(deps.KyselyPg, input.id), + ).toEqual([rule.id]); + + await kyselyUserRemoveFavoriteRule(deps.KyselyPg, input.id, rule.id); + expect( + await kyselyUserListFavoriteRuleIds(deps.KyselyPg, input.id), + ).toEqual([]); }, ); }); diff --git a/server/iocContainer/index.ts b/server/iocContainer/index.ts index 89c860f..bea4659 100644 --- a/server/iocContainer/index.ts +++ b/server/iocContainer/index.ts @@ -441,6 +441,17 @@ export interface Dependencies { // treatment that TS gives to classes with private fields; see https://stackoverflow.com/questions/55281162/can-i-force-the-typescript-compiler-to-use-nominal-typing) export type PublicInterface = { [K in keyof T]: T[K] }; +export function getPgConnectionParams(): pg.ClientConfig { + return { + user: process.env.DATABASE_USER ?? 'postgres', + database: process.env.DATABASE_NAME ?? 'development', + password: safeGetEnvVar('DATABASE_PASSWORD'), + port: parseInt(process.env.DATABASE_PORT ?? '5432'), + host: safeGetEnvVar('DATABASE_HOST'), + ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, + }; +} + /** * A function for creating our service container, configured for production. * Services can be rebound in other contexts (namely, tests) as needed. @@ -514,15 +525,10 @@ export default async function getBottle() { // that would defeat the ability of safeGetEnvVar to alert us in prod if a // worker that needs these vars is run without them. const getPgMasterConnectionInfo = () => ({ - user: process.env.DATABASE_USER ?? 'postgres', - database: process.env.DATABASE_NAME ?? 'development', - password: safeGetEnvVar('DATABASE_PASSWORD'), - port: parseInt(process.env.DATABASE_PORT ?? '5432'), - host: safeGetEnvVar('DATABASE_HOST'), + ...getPgConnectionParams(), max: parseInt(process.env.DATABASE_POOL_MAX ?? '30'), application_name: getEnvVarOrWarn('OTEL_SERVICE_NAME') ?? 'unknown-coop-service', - ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, ...getPgPoolTuning(), }); diff --git a/server/routes/content/ContentRoutes.test.ts b/server/routes/content/ContentRoutes.test.ts index a4f2efa..37eb150 100644 --- a/server/routes/content/ContentRoutes.test.ts +++ b/server/routes/content/ContentRoutes.test.ts @@ -1,51 +1,24 @@ import { faker } from '@faker-js/faker'; import _ from 'lodash'; -import { type ReadonlyDeep } from 'type-fest'; import { uid } from 'uid'; -import { type Dependencies } from '../../iocContainer/index.js'; import { serializeDerivedFieldSpec } from '../../services/derivedFieldsService/index.js'; -import { type ContentItemType } from '../../services/moderationConfigService/index.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createUser from '../../test/fixtureHelpers/createUser.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; const { omit } = _; describe('POST Content', () => { - const orgId = uid(), - userId = uid(); - let contentType1: ReadonlyDeep, - contentType2: ReadonlyDeep; - - let request: Awaited>['request'], - shutdown: Awaited>['shutdown'], - apiKey: Awaited>['apiKey'], - orgCleanup: Awaited>['cleanup'], - userCleanup: Awaited>['cleanup'], - ModerationConfigService: Dependencies['ModerationConfigService'], - ApiKeyService: Dependencies['ApiKeyService'], - analytics: Dependencies['DataWarehouseAnalytics'], - KyselyPg: Dependencies['KyselyPg']; - - beforeAll(async () => { - ({ - request, - shutdown, - deps: { - DataWarehouseAnalytics: analytics, - ModerationConfigService, - ApiKeyService, - KyselyPg, - }, - } = await makeMockedServer()); - - ({ apiKey, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { ModerationConfigService, ApiKeyService, KyselyPg } = deps; + const orgId = uid(); + const { apiKey } = await createOrg( { KyselyPg, ModerationConfigService, ApiKeyService }, orgId, - )); + ); - contentType1 = await ModerationConfigService.createContentType(orgId, { + await ModerationConfigService.createContentType(orgId, { name: 'test', description: faker.datatype.string(), schema: [ @@ -65,146 +38,135 @@ describe('POST Content', () => { schemaFieldRoles: {}, }); - contentType2 = await ModerationConfigService.createContentType(orgId, { - name: 'tes333t', - description: faker.datatype.string(), - schema: [ - { - name: 'video', - type: 'VIDEO', - required: false, - container: null, - }, - ], - schemaFieldRoles: {}, - }); - - ({ cleanup: userCleanup } = await createUser(KyselyPg, orgId, { - id: userId, - })); - }); - - afterAll(async () => { - await orgCleanup(); - await ModerationConfigService.deleteItemType({ + const contentType2 = await ModerationConfigService.createContentType( orgId, - itemTypeId: contentType1.id, - }); - await ModerationConfigService.deleteItemType({ - orgId, - itemTypeId: contentType2.id, - }); - await userCleanup(); - await shutdown(); - }); + { + name: 'tes333t', + description: faker.datatype.string(), + schema: [ + { + name: 'video', + type: 'VIDEO', + required: false, + container: null, + }, + ], + schemaFieldRoles: {}, + }, + ); - beforeEach(() => { - (analytics.bulkWrite as jest.Mock).mockClear(); + await createUser(KyselyPg, orgId, { id: uid() }); + + return { apiKey, contentType2, analytics: deps.DataWarehouseAnalytics }; }); - test('should return the expected response', async () => { - await request - .post('/api/v1/content') - .set('x-api-key', apiKey) - .send({ - contentId: uid(), - contentType: 'test', - userId: '32323', - content: { name: 'John Doe' }, - sync: true, - }) - .expect(200) - .expect(({ body }) => { - expect(body).toMatchInlineSnapshot(` + testWithFixture( + 'should return the expected response', + async ({ request, apiKey, analytics }) => { + await request + .post('/api/v1/content') + .set('x-api-key', apiKey) + .send({ + contentId: uid(), + contentType: 'test', + userId: '32323', + content: { name: 'John Doe' }, + sync: true, + }) + .expect(200) + .expect(({ body }) => { + expect(body).toMatchInlineSnapshot(` { "actionsTriggered": [], "derivedFields": {}, } `); - }); + }); - const bulkWrite = analytics.bulkWrite as jest.MockedFunction< - Dependencies['DataWarehouseAnalytics']['bulkWrite'] - >; - bulkWrite.mock.calls.forEach(([, , config]) => { - expect(config?.batchTimeout).toEqual(0); - }); - }); + analytics.bulkWrite.mock.calls.forEach(([, , config]) => { + expect(config?.batchTimeout).toEqual(0); + }); + }, + ); - it('should pass skipBatch param with sync requests', async () => { - await request - .post('/api/v1/content') - .set('x-api-key', apiKey) - .send({ - contentId: uid(), - contentType: 'test', - userId: '32323', - content: { name: 'John Doe' }, - sync: true, - }) - .expect(200) - .expect(({ body }) => { - expect(body).toMatchInlineSnapshot(` + testWithFixture( + 'should pass skipBatch param with sync requests', + async ({ request, apiKey, analytics }) => { + await request + .post('/api/v1/content') + .set('x-api-key', apiKey) + .send({ + contentId: uid(), + contentType: 'test', + userId: '32323', + content: { name: 'John Doe' }, + sync: true, + }) + .expect(200) + .expect(({ body }) => { + expect(body).toMatchInlineSnapshot(` { "actionsTriggered": [], "derivedFields": {}, } `); - }); + }); - const bulkWrite = analytics.bulkWrite as jest.MockedFunction< - Dependencies['DataWarehouseAnalytics']['bulkWrite'] - >; - bulkWrite.mock.calls.forEach(([, , config]) => { - expect(config?.batchTimeout).toEqual(0); - }); - }); + analytics.bulkWrite.mock.calls.forEach(([, , config]) => { + expect(config?.batchTimeout).toEqual(0); + }); + }, + ); - it('should return a 202 with async camelCase requests', async () => { - await request - .post('/api/v1/content') - .set('x-api-key', apiKey) - .send({ - contentId: uid(), - contentType: 'test', - userId: '32323', - content: { name: 'John Doe' }, - }) - .expect(202); - }); + testWithFixture( + 'should return a 202 with async camelCase requests', + async ({ request, apiKey }) => { + await request + .post('/api/v1/content') + .set('x-api-key', apiKey) + .send({ + contentId: uid(), + contentType: 'test', + userId: '32323', + content: { name: 'John Doe' }, + }) + .expect(202); + }, + ); // For now, we can't run this test routinely because we don't have mocking // set up (so it actually tries to contact Hive to transcribe the video). // But I ran it manually once and it works. - test.skip('should return the requested derived fields', async () => { - const seedOrgId = 'e7c89ce7729'; - const contentType = await ModerationConfigService.createContentType( - seedOrgId, - { - name: 'tes333t', - description: faker.datatype.string(), - schema: [ - { - name: 'video', - type: 'VIDEO', - required: false, - container: null, - }, - ], - schemaFieldRoles: {}, - }, - ); - const fieldId = serializeDerivedFieldSpec({ - source: { - type: 'CONTENT_FIELD', - name: 'video', - contentTypeId: contentType.id, - }, - derivationType: 'VIDEO_TRANSCRIPTION', - }); + testWithFixture.skip( + 'should return the requested derived fields', + async ({ deps, request }) => { + const seedOrgId = 'e7c89ce7729'; + const contentType = await deps.ModerationConfigService.createContentType( + seedOrgId, + { + name: 'tes333t', + description: faker.datatype.string(), + schema: [ + { + name: 'video', + type: 'VIDEO', + required: false, + container: null, + }, + ], + schemaFieldRoles: {}, + }, + ); + const fieldId = serializeDerivedFieldSpec({ + source: { + type: 'CONTENT_FIELD', + name: 'video', + contentTypeId: contentType.id, + }, + derivationType: 'VIDEO_TRANSCRIPTION', + }); - try { - return await request + await request .post(`/api/v1/content?includeDerivedField=${fieldId}`) .set('x-api-key', `fakeSecret.${seedOrgId}`) .send({ @@ -242,42 +204,40 @@ describe('POST Content', () => { console.log(e); throw e; }); - } finally { - await ModerationConfigService.deleteItemType({ - orgId: seedOrgId, - itemTypeId: contentType.id, - }); - } - }); + }, + ); - test('should return null for empty/missing derived fields', async () => { - const fieldId = serializeDerivedFieldSpec({ - source: { - type: 'CONTENT_FIELD', - name: 'video', - contentTypeId: contentType2.id, - }, - derivationType: 'VIDEO_TRANSCRIPTION', - }); + testWithFixture( + 'should return null for empty/missing derived fields', + async ({ request, apiKey, contentType2 }) => { + const fieldId = serializeDerivedFieldSpec({ + source: { + type: 'CONTENT_FIELD', + name: 'video', + contentTypeId: contentType2.id, + }, + derivationType: 'VIDEO_TRANSCRIPTION', + }); - return request - .post(`/api/v1/content?includeDerivedField=${fieldId}`) - .set('x-api-key', apiKey) - .send({ - contentId: uid(), - contentType: 'tes333t', - userId: '32323', - content: {}, // VIDEO field is missing! - sync: true, - }) - .expect(200) - .expect(({ body }) => { - expect(body.derivedFields[fieldId].field.source.contentTypeId).toBe( - contentType2.id, - ); + await request + .post(`/api/v1/content?includeDerivedField=${fieldId}`) + .set('x-api-key', apiKey) + .send({ + contentId: uid(), + contentType: 'tes333t', + userId: '32323', + content: {}, // VIDEO field is missing! + sync: true, + }) + .expect(200) + .expect(({ body }) => { + expect(body.derivedFields[fieldId].field.source.contentTypeId).toBe( + contentType2.id, + ); - expect(omit(body.derivedFields[fieldId], 'field.source.contentTypeId')) - .toMatchInlineSnapshot(` + expect( + omit(body.derivedFields[fieldId], 'field.source.contentTypeId'), + ).toMatchInlineSnapshot(` { "field": { "derivationType": "VIDEO_TRANSCRIPTION", @@ -289,10 +249,11 @@ describe('POST Content', () => { "value": null, } `); - }) - .catch((e) => { - console.log(e); - throw e; - }); - }); + }) + .catch((e) => { + console.log(e); + throw e; + }); + }, + ); }); diff --git a/server/routes/items/ItemRoutes.test.ts b/server/routes/items/ItemRoutes.test.ts index 4ee0a6e..f20d66d 100644 --- a/server/routes/items/ItemRoutes.test.ts +++ b/server/routes/items/ItemRoutes.test.ts @@ -1,46 +1,20 @@ import { faker } from '@faker-js/faker'; -import { type ReadonlyDeep } from 'type-fest'; import { uid } from 'uid'; -import { type Dependencies } from '../../iocContainer/index.js'; -import { type ContentItemType } from '../../services/moderationConfigService/index.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createUser from '../../test/fixtureHelpers/createUser.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; describe('POST Items', () => { - const orgId = uid(), - userId = uid(); - let contentType: ReadonlyDeep; - - let request: Awaited>['request'], - shutdown: Awaited>['shutdown'], - apiKey: Awaited>['apiKey'], - orgCleanup: Awaited>['cleanup'], - userCleanup: Awaited>['cleanup'], - ModerationConfigService: Dependencies['ModerationConfigService'], - ApiKeyService: Dependencies['ApiKeyService'], - analytics: Dependencies['DataWarehouseAnalytics'], - KyselyPg: Dependencies['KyselyPg']; - - beforeAll(async () => { - ({ - request, - shutdown, - deps: { - DataWarehouseAnalytics: analytics, - ModerationConfigService, - ApiKeyService, - KyselyPg, - }, - } = await makeMockedServer()); - - ({ apiKey, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { ModerationConfigService, ApiKeyService, KyselyPg } = deps; + const orgId = uid(); + const { apiKey } = await createOrg( { KyselyPg, ModerationConfigService, ApiKeyService }, orgId, - )); + ); - contentType = await ModerationConfigService.createContentType(orgId, { + const contentType = await ModerationConfigService.createContentType(orgId, { name: 'test', description: faker.datatype.string(), schema: [ @@ -60,79 +34,67 @@ describe('POST Items', () => { schemaFieldRoles: {}, }); - ({ cleanup: userCleanup } = await createUser(KyselyPg, orgId, { - id: userId, - })); - }); + await createUser(KyselyPg, orgId, { id: uid() }); - afterAll(async () => { - await orgCleanup(); - await ModerationConfigService.deleteItemType({ - orgId, - itemTypeId: contentType.id, - }); - await userCleanup(); - await shutdown(); + return { apiKey, contentType, analytics: deps.DataWarehouseAnalytics }; }); - beforeEach(() => { - (analytics.bulkWrite as jest.Mock).mockClear(); - }); + testWithFixture( + 'should return the expected response', + async ({ request, apiKey, contentType, analytics }) => { + await request + .post('/api/v1/items/async') + .set('x-api-key', apiKey) + .send({ + items: [ + { + id: uid(), + data: { name: 'John Doe' }, + typeId: contentType.id, + }, + ], + }) + .expect(202) + .expect(({ body }) => { + expect(body).toMatchInlineSnapshot(`{}`); + }); - test('should return the expected response', async () => { - await request - .post('/api/v1/items/async') - .set('x-api-key', apiKey) - .send({ - items: [ - { - id: uid(), - data: { name: 'John Doe' }, - typeId: contentType.id, - }, - ], - }) - .expect(202) - .expect(({ body }) => { - expect(body).toMatchInlineSnapshot(`{}`); + analytics.bulkWrite.mock.calls.forEach(([, , config]) => { + expect(config?.batchTimeout ?? undefined).toEqual(undefined); }); + }, + ); - const bulkWrite = analytics.bulkWrite as jest.MockedFunction< - Dependencies['DataWarehouseAnalytics']['bulkWrite'] - >; - bulkWrite.mock.calls.forEach(([, , config]) => { - expect(config?.batchTimeout ?? undefined).toEqual(undefined); - }); - }); - - test('should return errors for only items that failed to be validated', async () => { - const failingUid = uid(); - const failingUid2 = uid(); - await request - .post('/api/v1/items/async') - .set('x-api-key', apiKey) - .send({ - items: [ - { - id: uid(), - data: { name: 'John Doe' }, - typeId: contentType.id, - }, - { - id: failingUid, - data: { video: 'https://my-dummy-video.com/' }, - typeId: contentType.id, - }, - { - id: failingUid2, - data: { video: 'https://second-dummy-video.com/' }, - typeId: contentType.id, - }, - ], - }) - .expect(400) - .expect(({ body }) => { - expect(body).toMatchInlineSnapshot(` + testWithFixture( + 'should return errors for only items that failed to be validated', + async ({ request, apiKey, contentType, analytics }) => { + const failingUid = uid(); + const failingUid2 = uid(); + await request + .post('/api/v1/items/async') + .set('x-api-key', apiKey) + .send({ + items: [ + { + id: uid(), + data: { name: 'John Doe' }, + typeId: contentType.id, + }, + { + id: failingUid, + data: { video: 'https://my-dummy-video.com/' }, + typeId: contentType.id, + }, + { + id: failingUid2, + data: { video: 'https://second-dummy-video.com/' }, + typeId: contentType.id, + }, + ], + }) + .expect(400) + .expect(({ body }) => { + expect(body).toMatchInlineSnapshot(` { "errors": [ { @@ -158,13 +120,11 @@ describe('POST Items', () => { ], } `); - }); + }); - const bulkWrite = analytics.bulkWrite as jest.MockedFunction< - Dependencies['DataWarehouseAnalytics']['bulkWrite'] - >; - bulkWrite.mock.calls.forEach(([, , config]) => { - expect(config?.batchTimeout ?? undefined).toEqual(undefined); - }); - }); + analytics.bulkWrite.mock.calls.forEach(([, , config]) => { + expect(config?.batchTimeout ?? undefined).toEqual(undefined); + }); + }, + ); }); diff --git a/server/routes/policies/PoliciesRoutes.test.ts b/server/routes/policies/PoliciesRoutes.test.ts index b9c25c0..68dcee3 100644 --- a/server/routes/policies/PoliciesRoutes.test.ts +++ b/server/routes/policies/PoliciesRoutes.test.ts @@ -1,55 +1,37 @@ import { uid } from 'uid'; -import { type Dependencies } from '../../iocContainer/index.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createPolicy from '../../test/fixtureHelpers/createPolicy.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; describe('GET policies', () => { - const orgId = uid(); - - let request: Awaited>['request'], - shutdown: Awaited>['shutdown'], - apiKey: Awaited>['apiKey'], - orgCleanup: Awaited>['cleanup'], - ModerationConfigService: Dependencies['ModerationConfigService'], - ApiKeyService: Dependencies['ApiKeyService'], - KyselyPg: Dependencies['KyselyPg']; - - beforeAll(async () => { - ({ - request, - shutdown, - deps: { ModerationConfigService, ApiKeyService, KyselyPg }, - } = await makeMockedServer()); - - ({ apiKey, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { ModerationConfigService, ApiKeyService, KyselyPg } = deps; + const { org, apiKey } = await createOrg( { KyselyPg, ModerationConfigService, ApiKeyService }, - orgId, - )); - }); - - afterAll(async () => { - await orgCleanup(); - await shutdown(); + uid(), + ); + return { orgId: org.id, apiKey }; }); - test.skip('Should return expected response', async () => { - const policy1 = await createPolicy({ - moderationConfigService: ModerationConfigService, - orgId, - }); - const policy2 = await createPolicy({ - moderationConfigService: ModerationConfigService, - orgId, - }); - await request - .post('/api/v1/policies') - .set('x-api-key', apiKey) - .send() - .expect(200) - .expect(({ body }) => { - expect(body).toMatchInlineSnapshot(` + testWithFixture.skip( + 'Should return expected response', + async ({ deps, request, orgId, apiKey }) => { + const policy1 = await createPolicy({ + moderationConfigService: deps.ModerationConfigService, + orgId, + }); + const policy2 = await createPolicy({ + moderationConfigService: deps.ModerationConfigService, + orgId, + }); + await request + .post('/api/v1/policies') + .set('x-api-key', apiKey) + .send() + .expect(200) + .expect(({ body }) => { + expect(body).toMatchInlineSnapshot(` { policies: [ @@ -66,6 +48,7 @@ describe('GET policies', () => { ] } `); - }); - }); + }); + }, + ); }); diff --git a/server/routes/reporting/ReportingRoutes.test.ts b/server/routes/reporting/ReportingRoutes.test.ts index 805d214..26b5888 100644 --- a/server/routes/reporting/ReportingRoutes.test.ts +++ b/server/routes/reporting/ReportingRoutes.test.ts @@ -1,38 +1,22 @@ /* eslint-disable max-lines */ import { faker } from '@faker-js/faker'; +import { uid } from 'uid'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createUser from '../../test/fixtureHelpers/createUser.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; describe('POST Report', () => { - const orgId = '2a4634c81d5', - userId = '834f573a46d'; - - let contentTypeId: string; - let userTypeId: string; - let threadTypeId: string; - - let deps: Awaited>['deps'], - request: Awaited>['request'], - shutdown: Awaited>['shutdown'], - apiKey: Awaited>['apiKey'], - orgCleanup: Awaited>['cleanup'], - userCleanup: Awaited>['cleanup']; - - const getBulkWriteMock = () => deps.DataWarehouseAnalytics.bulkWrite; - - beforeAll(async () => { - ({ deps, request, shutdown } = await makeMockedServer()); - - ({ apiKey, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const orgId = uid(); + const { apiKey } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, ApiKeyService: deps.ApiKeyService, }, orgId, - )); + ); const userType = await deps.ModerationConfigService.createUserType(orgId, { name: 'test user type', description: faker.datatype.string(), @@ -99,238 +83,152 @@ describe('POST Report', () => { }, ); - contentTypeId = contentType.id; - - userTypeId = userType.id; + await createUser(deps.KyselyPg, orgId, { id: uid() }); - threadTypeId = threadType.id; - - ({ cleanup: userCleanup } = await createUser(deps.KyselyPg, orgId, { - id: userId, - })); - }); - - afterAll(async () => { - await orgCleanup(); - await Promise.all([ - deps.ModerationConfigService.deleteItemType({ - orgId, - itemTypeId: contentTypeId, - }), - deps.ModerationConfigService.deleteItemType({ - orgId, - itemTypeId: userTypeId, - }), - deps.ModerationConfigService.deleteItemType({ - orgId, - itemTypeId: threadTypeId, - }), - ]); - await userCleanup(); - await shutdown(); - }); - - beforeEach(() => { - // This is only safe while we're not running tests concurrently. - // Consider using the `makeTestWithFixture` helper instead to make - // a local copy of this state for each test. - getBulkWriteMock().mockClear(); + return { + apiKey, + orgId, + contentTypeId: contentType.id, + userTypeId: userType.id, + threadTypeId: threadType.id, + getBulkWriteMock: () => deps.DataWarehouseAnalytics.bulkWrite, + }; }); - test('Should return the expected response for user report and thread', async () => { - const payload = { - reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, - reportedAt: new Date().toISOString(), - reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, - reportedItem: { - id: '21342135', - typeId: userTypeId, - data: { name: 'Some name' }, - }, - reportedItemThread: [ - { + testWithFixture( + 'Should return the expected response for user report and thread', + async ({ + request, + apiKey, + orgId, + contentTypeId, + userTypeId, + getBulkWriteMock, + }) => { + const payload = { + reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, + reportedAt: new Date().toISOString(), + reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, + reportedItem: { id: '21342135', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - { - id: '12345123', - typeId: contentTypeId, + typeId: userTypeId, data: { name: 'Some name' }, }, - ], - }; - - await request - .post('/api/v1/report') - .set('x-api-key', apiKey) - .send(payload) - .expect(201); - - await new Promise((resolve) => setTimeout(resolve, 2000)); - - expect(getBulkWriteMock().mock.calls[0]).toMatchObject([ - 'REPORTING_SERVICE.REPORTS', - [ - { - ts: expect.any(Date), - org_id: orgId, - request_id: expect.any(String), - reporter_kind: 'user', - reported_at: expect.any(Date), - reported_item_id: '21342135', - reported_item_data: { name: 'Some name' }, - reported_item_type_id: userTypeId, - reported_item_type_kind: 'USER', - reported_item_type_schema: [ - { name: 'name', type: 'STRING', required: true, container: null }, - { name: 'video', type: 'VIDEO', required: false, container: null }, - ], - reported_item_type_schema_variant: 'original', - reported_item_type_version: expect.any(String), - reported_item_type_schema_field_roles: { - createdAt: undefined, - displayName: undefined, + reportedItemThread: [ + { + id: '21342135', + typeId: contentTypeId, + data: { name: 'Some name' }, }, - reporter_user_id: '5123521', - reporter_user_item_type_id: contentTypeId, - reported_item_thread: [ - { - id: '21342135', - typeIdentifier: { - id: contentTypeId, - version: expect.any(String), - schemaVariant: 'original', - }, - data: { name: 'Some name' }, - }, - { - id: '12345123', - typeIdentifier: { - id: contentTypeId, - version: expect.any(String), - schemaVariant: 'original', - }, - data: { name: 'Some name' }, - }, - ], - }, - ], - ]); - expect(getBulkWriteMock().mock.calls[1]).toMatchObject([ - 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', - [], - ]); - }); + { + id: '12345123', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + ], + }; - test('Should return the expected response for user report and additional items', async () => { - const payload = { - reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, - reportedAt: new Date().toISOString(), - reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, - reportedItem: { - id: '21342135', - typeId: userTypeId, - data: { name: 'Some name' }, - }, - additionalItems: [ - { - id: '21342135', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - { - id: '12345123', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - ], - }; + await request + .post('/api/v1/report') + .set('x-api-key', apiKey) + .send(payload) + .expect(201); - await request - .post('/api/v1/report') - .set('x-api-key', apiKey) - .send(payload) - .expect(201); + await new Promise((resolve) => setTimeout(resolve, 2000)); - await new Promise((resolve) => setTimeout(resolve, 2000)); - expect(getBulkWriteMock().mock.calls[0]).toMatchObject([ - 'REPORTING_SERVICE.REPORTS', - [ - { - ts: expect.any(Date), - org_id: orgId, - request_id: expect.any(String), - reporter_kind: 'user', - reported_at: expect.any(Date), - reported_item_id: '21342135', - reported_item_data: { name: 'Some name' }, - reported_item_type_id: userTypeId, - reported_item_type_kind: 'USER', - reported_item_type_schema: [ - { name: 'name', type: 'STRING', required: true, container: null }, - { name: 'video', type: 'VIDEO', required: false, container: null }, - ], - reported_item_type_schema_variant: 'original', - reported_item_type_version: expect.any(String), - reported_item_type_schema_field_roles: { - createdAt: undefined, - displayName: undefined, - }, - reporter_user_id: '5123521', - reporter_user_item_type_id: contentTypeId, - additional_items: [ - { - id: '21342135', - typeIdentifier: { - id: contentTypeId, - version: expect.any(String), - schemaVariant: 'original', + expect(getBulkWriteMock().mock.calls[0]).toMatchObject([ + 'REPORTING_SERVICE.REPORTS', + [ + { + ts: expect.any(Date), + org_id: orgId, + request_id: expect.any(String), + reporter_kind: 'user', + reported_at: expect.any(Date), + reported_item_id: '21342135', + reported_item_data: { name: 'Some name' }, + reported_item_type_id: userTypeId, + reported_item_type_kind: 'USER', + reported_item_type_schema: [ + { name: 'name', type: 'STRING', required: true, container: null }, + { + name: 'video', + type: 'VIDEO', + required: false, + container: null, }, - data: { name: 'Some name' }, + ], + reported_item_type_schema_variant: 'original', + reported_item_type_version: expect.any(String), + reported_item_type_schema_field_roles: { + createdAt: undefined, + displayName: undefined, }, - { - id: '12345123', - typeIdentifier: { - id: contentTypeId, - version: expect.any(String), - schemaVariant: 'original', + reporter_user_id: '5123521', + reporter_user_item_type_id: contentTypeId, + reported_item_thread: [ + { + id: '21342135', + typeIdentifier: { + id: contentTypeId, + version: expect.any(String), + schemaVariant: 'original', + }, + data: { name: 'Some name' }, }, - data: { name: 'Some name' }, - }, - ], - }, - ], - ]); - expect(getBulkWriteMock().mock.calls[1]).toMatchObject([ - 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', - [], - ]); - }); - - test('Should accept non-Content (user) items in additional items', async () => { - const payload = { - reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, - reportedAt: new Date().toISOString(), - reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, - reportedItem: { - id: '21342135', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - additionalItems: [ - { - id: '12345123', + { + id: '12345123', + typeIdentifier: { + id: contentTypeId, + version: expect.any(String), + schemaVariant: 'original', + }, + data: { name: 'Some name' }, + }, + ], + }, + ], + ]); + expect(getBulkWriteMock().mock.calls[1]).toMatchObject([ + 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', + [], + ]); + }, + ); + + testWithFixture( + 'Should return the expected response for user report and additional items', + async ({ + request, + apiKey, + orgId, + contentTypeId, + userTypeId, + getBulkWriteMock, + }) => { + const payload = { + reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, + reportedAt: new Date().toISOString(), + reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, + reportedItem: { + id: '21342135', typeId: userTypeId, data: { name: 'Some name' }, }, - ], - }; - - // Spy (calling through) so we can assert what gets forwarded to MRT. - const enqueueSpy = jest.spyOn(deps.ManualReviewToolService, 'enqueue'); + additionalItems: [ + { + id: '21342135', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + { + id: '12345123', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + ], + }; - try { await request .post('/api/v1/report') .set('x-api-key', apiKey) @@ -338,20 +236,50 @@ describe('POST Report', () => { .expect(201); await new Promise((resolve) => setTimeout(resolve, 2000)); - - // The user item is still indexed/recorded on the report row... expect(getBulkWriteMock().mock.calls[0]).toMatchObject([ 'REPORTING_SERVICE.REPORTS', [ { + ts: expect.any(Date), org_id: orgId, + request_id: expect.any(String), + reporter_kind: 'user', + reported_at: expect.any(Date), reported_item_id: '21342135', - reported_item_type_kind: 'CONTENT', + reported_item_data: { name: 'Some name' }, + reported_item_type_id: userTypeId, + reported_item_type_kind: 'USER', + reported_item_type_schema: [ + { name: 'name', type: 'STRING', required: true, container: null }, + { + name: 'video', + type: 'VIDEO', + required: false, + container: null, + }, + ], + reported_item_type_schema_variant: 'original', + reported_item_type_version: expect.any(String), + reported_item_type_schema_field_roles: { + createdAt: undefined, + displayName: undefined, + }, + reporter_user_id: '5123521', + reporter_user_item_type_id: contentTypeId, additional_items: [ + { + id: '21342135', + typeIdentifier: { + id: contentTypeId, + version: expect.any(String), + schemaVariant: 'original', + }, + data: { name: 'Some name' }, + }, { id: '12345123', typeIdentifier: { - id: userTypeId, + id: contentTypeId, version: expect.any(String), schemaVariant: 'original', }, @@ -361,144 +289,220 @@ describe('POST Report', () => { }, ], ]); - - // ...but it must NOT leak into MRT's Content-only `additionalContentItems`. - expect(enqueueSpy).toHaveBeenCalled(); - const enqueueArg = enqueueSpy.mock.calls[0]?.[0] as - | { - payload?: { - additionalContentItems?: ReadonlyArray<{ id: string }>; - }; - } - | undefined; - expect(enqueueArg?.payload?.additionalContentItems ?? []).toEqual([]); - } finally { - enqueueSpy.mockRestore(); - } - }); - - test('Should return the expected response for content report and additional items', async () => { - const payload = { - reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, - reportedAt: new Date().toISOString(), - reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, - reportedItem: { - id: '21342135', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - additionalItems: [ - { + expect(getBulkWriteMock().mock.calls[1]).toMatchObject([ + 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', + [], + ]); + }, + ); + + testWithFixture( + 'Should accept non-Content (user) items in additional items', + async ({ + request, + apiKey, + orgId, + contentTypeId, + userTypeId, + getBulkWriteMock, + deps, + }) => { + const payload = { + reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, + reportedAt: new Date().toISOString(), + reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, + reportedItem: { id: '21342135', typeId: contentTypeId, data: { name: 'Some name' }, }, - { - id: '12345123', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - ], - }; + additionalItems: [ + { + id: '12345123', + typeId: userTypeId, + data: { name: 'Some name' }, + }, + ], + }; - await request - .post('/api/v1/report') - .set('x-api-key', apiKey) - .send(payload) - .expect(201); + // Spy (calling through) so we can assert what gets forwarded to MRT. + const enqueueSpy = jest.spyOn(deps.ManualReviewToolService, 'enqueue'); - await new Promise((resolve) => setTimeout(resolve, 2000)); + try { + await request + .post('/api/v1/report') + .set('x-api-key', apiKey) + .send(payload) + .expect(201); - expect(getBulkWriteMock().mock.calls[0]).toMatchObject([ - 'REPORTING_SERVICE.REPORTS', - [ - { - ts: expect.any(Date), - org_id: orgId, - request_id: expect.any(String), - reporter_kind: 'user', - reported_at: expect.any(Date), - reported_item_id: '21342135', - reported_item_data: { name: 'Some name' }, - reported_item_type_id: contentTypeId, - reported_item_type_kind: 'CONTENT', - reported_item_type_schema: [ - { name: 'name', type: 'STRING', required: true, container: null }, + await new Promise((resolve) => setTimeout(resolve, 2000)); + + // The user item is still indexed/recorded on the report row... + expect(getBulkWriteMock().mock.calls[0]).toMatchObject([ + 'REPORTING_SERVICE.REPORTS', + [ { - name: 'video', - type: 'VIDEO', - required: false, - container: null, + org_id: orgId, + reported_item_id: '21342135', + reported_item_type_kind: 'CONTENT', + additional_items: [ + { + id: '12345123', + typeIdentifier: { + id: userTypeId, + version: expect.any(String), + schemaVariant: 'original', + }, + data: { name: 'Some name' }, + }, + ], }, ], - reported_item_type_schema_variant: 'original', - reported_item_type_version: expect.any(String), - reported_item_type_schema_field_roles: { - createdAt: undefined, - displayName: undefined, + ]); + + // ...but it must NOT leak into MRT's Content-only `additionalContentItems`. + expect(enqueueSpy).toHaveBeenCalled(); + const enqueueArg = enqueueSpy.mock.calls[0]?.[0] as + | { + payload?: { + additionalContentItems?: ReadonlyArray<{ id: string }>; + }; + } + | undefined; + expect(enqueueArg?.payload?.additionalContentItems ?? []).toEqual([]); + } finally { + enqueueSpy.mockRestore(); + } + }, + ); + + testWithFixture( + 'Should return the expected response for content report and additional items', + async ({ request, apiKey, orgId, contentTypeId, getBulkWriteMock }) => { + const payload = { + reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, + reportedAt: new Date().toISOString(), + reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, + reportedItem: { + id: '21342135', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + additionalItems: [ + { + id: '21342135', + typeId: contentTypeId, + data: { name: 'Some name' }, }, - reporter_user_id: '5123521', - reporter_user_item_type_id: contentTypeId, - additional_items: [ - { - id: '21342135', - typeIdentifier: { - id: contentTypeId, - version: expect.any(String), - schemaVariant: 'original', + { + id: '12345123', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + ], + }; + + await request + .post('/api/v1/report') + .set('x-api-key', apiKey) + .send(payload) + .expect(201); + + await new Promise((resolve) => setTimeout(resolve, 2000)); + + expect(getBulkWriteMock().mock.calls[0]).toMatchObject([ + 'REPORTING_SERVICE.REPORTS', + [ + { + ts: expect.any(Date), + org_id: orgId, + request_id: expect.any(String), + reporter_kind: 'user', + reported_at: expect.any(Date), + reported_item_id: '21342135', + reported_item_data: { name: 'Some name' }, + reported_item_type_id: contentTypeId, + reported_item_type_kind: 'CONTENT', + reported_item_type_schema: [ + { name: 'name', type: 'STRING', required: true, container: null }, + { + name: 'video', + type: 'VIDEO', + required: false, + container: null, }, - data: { name: 'Some name' }, + ], + reported_item_type_schema_variant: 'original', + reported_item_type_version: expect.any(String), + reported_item_type_schema_field_roles: { + createdAt: undefined, + displayName: undefined, }, - { - id: '12345123', - typeIdentifier: { - id: contentTypeId, - version: expect.any(String), - schemaVariant: 'original', + reporter_user_id: '5123521', + reporter_user_item_type_id: contentTypeId, + additional_items: [ + { + id: '21342135', + typeIdentifier: { + id: contentTypeId, + version: expect.any(String), + schemaVariant: 'original', + }, + data: { name: 'Some name' }, }, - data: { name: 'Some name' }, - }, - ], - }, - ], - ]); - expect(getBulkWriteMock().mock.calls[1]).toMatchObject([ - 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', - [], - ]); - }); - - test('Should fail thread report and additional items', async () => { - const payload = { - reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, - reportedAt: new Date().toISOString(), - reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, - reportedItem: { - id: '21342135', - typeId: threadTypeId, - data: { name: 'Some name' }, - }, - additionalItems: [ - { + { + id: '12345123', + typeIdentifier: { + id: contentTypeId, + version: expect.any(String), + schemaVariant: 'original', + }, + data: { name: 'Some name' }, + }, + ], + }, + ], + ]); + expect(getBulkWriteMock().mock.calls[1]).toMatchObject([ + 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', + [], + ]); + }, + ); + + testWithFixture( + 'Should fail thread report and additional items', + async ({ request, apiKey, contentTypeId, threadTypeId }) => { + const payload = { + reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, + reportedAt: new Date().toISOString(), + reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, + reportedItem: { id: '21342135', - typeId: contentTypeId, + typeId: threadTypeId, data: { name: 'Some name' }, }, - { - id: '12345123', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - ], - }; + additionalItems: [ + { + id: '21342135', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + { + id: '12345123', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + ], + }; - await request - .post('/api/v1/report') - .set('x-api-key', apiKey) - .send(payload) - .expect(400) - .expect(({ body }) => { - expect(body).toMatchInlineSnapshot(` + await request + .post('/api/v1/report') + .set('x-api-key', apiKey) + .send(payload) + .expect(400) + .expect(({ body }) => { + expect(body).toMatchInlineSnapshot(` { "errors": [ { @@ -511,71 +515,77 @@ describe('POST Report', () => { ], } `); - }); - }); - - test('Should pass thread report and item thread content items', async () => { - const payload = { - reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, - reportedAt: new Date().toISOString(), - reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, - reportedItem: { - id: '21342135', - typeId: threadTypeId, - data: { name: 'Some name' }, - }, - itemThreadContentItems: [ - { + }); + }, + ); + + testWithFixture( + 'Should pass thread report and item thread content items', + async ({ request, apiKey, contentTypeId, threadTypeId }) => { + const payload = { + reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, + reportedAt: new Date().toISOString(), + reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, + reportedItem: { id: '21342135', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - { - id: '12345123', - typeId: contentTypeId, + typeId: threadTypeId, data: { name: 'Some name' }, }, - ], - }; - - await request - .post('/api/v1/report') - .set('x-api-key', apiKey) - .send(payload) - .expect(201); - }); + itemThreadContentItems: [ + { + id: '21342135', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + { + id: '12345123', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + ], + }; - test('Should fail invalid reportedAt date', async () => { - const payload = { - reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, - reportedAt: 'invalid date', - reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, - reportedItem: { - id: '21342135', - typeId: userTypeId, - data: { name: 'Some name' }, - }, - additionalItems: [ - { + await request + .post('/api/v1/report') + .set('x-api-key', apiKey) + .send(payload) + .expect(201); + }, + ); + + testWithFixture( + 'Should fail invalid reportedAt date', + async ({ request, apiKey, contentTypeId, userTypeId }) => { + const payload = { + reporter: { kind: 'user', id: '5123521', typeId: contentTypeId }, + reportedAt: 'invalid date', + reportedForReason: { policyId: '1231241254', reason: 'Some Reason' }, + reportedItem: { id: '21342135', - typeId: contentTypeId, - data: { name: 'Some name' }, - }, - { - id: '12345123', - typeId: contentTypeId, + typeId: userTypeId, data: { name: 'Some name' }, }, - ], - }; + additionalItems: [ + { + id: '21342135', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + { + id: '12345123', + typeId: contentTypeId, + data: { name: 'Some name' }, + }, + ], + }; - await request - .post('/api/v1/report') - .set('x-api-key', apiKey) - .send(payload) - .expect(400) - .expect(({ body }) => { - expect(body).toMatchInlineSnapshot(` + await request + .post('/api/v1/report') + .set('x-api-key', apiKey) + .send(payload) + .expect(400) + .expect(({ body }) => { + expect(body).toMatchInlineSnapshot(` { "errors": [ { @@ -588,6 +598,7 @@ describe('POST Report', () => { ], } `); - }); - }); + }); + }, + ); }); diff --git a/server/routes/user_scores/UserScoresRoutes.test.ts b/server/routes/user_scores/UserScoresRoutes.test.ts index 96d0ed5..8d2aa74 100644 --- a/server/routes/user_scores/UserScoresRoutes.test.ts +++ b/server/routes/user_scores/UserScoresRoutes.test.ts @@ -2,54 +2,40 @@ import { uid } from 'uid'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createUserItemTypes from '../../test/fixtureHelpers/createUserItemTypes.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; describe('GET policies', () => { - test('Should return expected response', async () => { - const testUserScoresRoute = makeTestWithFixture(async () => { - const { - request, - shutdown, - deps: { ModerationConfigService, ApiKeyService, KyselyPg }, - } = await makeMockedServer(); + const testUserScoresRoute = makeTransactionalTestWithFixture( + async ({ deps }) => { + const { ModerationConfigService, ApiKeyService, KyselyPg } = deps; - const { org, apiKey, cleanup: orgCleanup } = await createOrg( + const { org, apiKey } = await createOrg( { KyselyPg, ModerationConfigService, ApiKeyService }, uid(), ); - const { itemTypes, cleanup } = await createUserItemTypes({ + const { itemTypes } = await createUserItemTypes({ moderationConfigService: ModerationConfigService, orgId: org.id, extra: {}, }); - return { - itemType: itemTypes[0], - apiKey, - request, - async cleanup() { - await cleanup(); - await orgCleanup(); - await shutdown(); - }, - }; - }); + return { itemType: itemTypes[0], apiKey }; + }, + ); - testUserScoresRoute( - 'Test that a random user gets a 5 returned back', - async ({ itemType, request, apiKey }) => { - await request - .get('/api/v1/user_scores') - .set('x-api-key', apiKey) - .query({ - id: 'any user id', - typeId: itemType.id, - }) - .expect(200) - .expect(({ body }) => { - expect(body).toBe(5); - }); - }, - ); - }); + testUserScoresRoute( + 'Test that a random user gets a 5 returned back', + async ({ itemType, request, apiKey }) => { + await request + .get('/api/v1/user_scores') + .set('x-api-key', apiKey) + .query({ + id: 'any user id', + typeId: itemType.id, + }) + .expect(200) + .expect(({ body }) => { + expect(body).toBe(5); + }); + }, + ); }); diff --git a/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.test.ts b/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.test.ts index d6ab826..f9d3187 100644 --- a/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.test.ts +++ b/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.test.ts @@ -1,4 +1,4 @@ -import getBottle, { +import { type Dependencies, type PublicInterface, } from '../../iocContainer/index.js'; @@ -7,6 +7,7 @@ import { type GetCurrentPeriodRuleAlarmStatuses } from '../../services/ruleAnoma import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createRule from '../../test/fixtureHelpers/createRule.js'; import createUser from '../../test/fixtureHelpers/createUser.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; import { type Mocked } from '../../test/mockHelpers/jestMocks.js'; import { RuleAlarmStatus } from '../moderationConfigService/index.js'; import DetectRulePassRateAnomaliesJob from './detectRulePassRateAnomaliesJob.js'; @@ -68,178 +69,166 @@ function makeMockKyselyForRules( describe('Detect Rule Anomalies', () => { describe('worker', () => { - let deleteMockData: () => Promise, - mockDummyRules: Array<{ - id: string; - orgId: string; - creatorId: string; - name: string; - alarmStatus: RuleAlarmStatus; - statusIfUnexpired: string; - }>, - mockKysely: ReturnType, - mockGetCurrentPeriodRuleAlarmStatuses: GetCurrentPeriodRuleAlarmStatuses, - mockNotificationsService: Mocked< - PublicInterface, - 'createNotifications' - >; + const testWithFixture = makeTransactionalTestWithFixture( + async ({ deps }) => { + const { ModerationConfigService, ApiKeyService, KyselyPg } = deps; - beforeAll(async () => { - /* eslint-disable functional/immutable-data */ - const { ModerationConfigService, ApiKeyService, KyselyPg } = ( - await getBottle() - ).container; - - // make some fake rules (w/ stable ids so we can match them in a snapshot) - // in different initial alarm statuses, to test all 9 combinations [i.e., - // starting and ending at one of (OK, ALARM, or INSUFFICENT_DATA), where - // the start and end states can be the same]. - const { org, cleanup: orgCleanup } = await createOrg({ - KyselyPg, - ModerationConfigService, - ApiKeyService, - }); - const { org: org2, cleanup: org2Cleanup } = await createOrg( - { + // Make some fake rules (w/ stable ids so we can match them in a snapshot) + // in different initial alarm statuses, to test all 9 combinations [i.e., + // starting and ending at one of (OK, ALARM, or INSUFFICENT_DATA), where + // the start and end states can be the same]. The rows are written inside + // the test transaction and rolled back afterward, so the stable ids never + // leak across runs. + const { org } = await createOrg({ KyselyPg, ModerationConfigService, ApiKeyService, - }, - undefined, - { onCallAlertEmail: 'test@gmail.com' }, - ); - const { user: ruleOwner, cleanup: ruleOwnerCleanup } = await createUser( - KyselyPg, - org.id, - { id: 'cb34377bcc3' }, - ); - const { user: ruleOwner2, cleanup: ruleOwner2Cleanup } = await createUser( - KyselyPg, - org.id, - { id: 'cb34377bcc4' }, - ); - const fakeRules = await Promise.all([ - createRule(KyselyPg, org.id, { - alarmStatus: RuleAlarmStatus.ALARM, - id: '9d237a650c1', - creator: ruleOwner, - }), - createRule(KyselyPg, org.id, { - alarmStatus: RuleAlarmStatus.ALARM, - id: '386da8abc3b', - creator: ruleOwner, - }), - createRule(KyselyPg, org.id, { - alarmStatus: RuleAlarmStatus.ALARM, - id: 'd237a650c13', - creator: ruleOwner, - }), + }); + const { org: org2 } = await createOrg( + { + KyselyPg, + ModerationConfigService, + ApiKeyService, + }, + undefined, + { onCallAlertEmail: 'test@gmail.com' }, + ); + const { user: ruleOwner } = await createUser(KyselyPg, org.id, { + id: 'cb34377bcc3', + }); + const { user: ruleOwner2 } = await createUser(KyselyPg, org.id, { + id: 'cb34377bcc4', + }); + const fakeRules = await Promise.all([ + createRule(KyselyPg, org.id, { + alarmStatus: RuleAlarmStatus.ALARM, + id: '9d237a650c1', + creator: ruleOwner, + }), + createRule(KyselyPg, org.id, { + alarmStatus: RuleAlarmStatus.ALARM, + id: '386da8abc3b', + creator: ruleOwner, + }), + createRule(KyselyPg, org.id, { + alarmStatus: RuleAlarmStatus.ALARM, + id: 'd237a650c13', + creator: ruleOwner, + }), - createRule(KyselyPg, org.id, { - alarmStatus: RuleAlarmStatus.OK, - id: '86da8abc3b6', - creator: ruleOwner, - }), - createRule(KyselyPg, org.id, { - alarmStatus: RuleAlarmStatus.OK, - id: 'fdb4ee86f93', - creator: ruleOwner, - }), - createRule(KyselyPg, org.id, { - alarmStatus: RuleAlarmStatus.OK, - id: '237a650c134', - creator: ruleOwner, - }), + createRule(KyselyPg, org.id, { + alarmStatus: RuleAlarmStatus.OK, + id: '86da8abc3b6', + creator: ruleOwner, + }), + createRule(KyselyPg, org.id, { + alarmStatus: RuleAlarmStatus.OK, + id: 'fdb4ee86f93', + creator: ruleOwner, + }), + createRule(KyselyPg, org.id, { + alarmStatus: RuleAlarmStatus.OK, + id: '237a650c134', + creator: ruleOwner, + }), - createRule(KyselyPg, org2.id, { - alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, - id: 'db4ee86f938', - creator: ruleOwner2, - }), - createRule(KyselyPg, org2.id, { - alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, - id: '37a650c1342', - creator: ruleOwner2, - }), - createRule(KyselyPg, org2.id, { - alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, - id: 'b4ee86f9386', - creator: ruleOwner2, - }), - ]); + createRule(KyselyPg, org2.id, { + alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, + id: 'db4ee86f938', + creator: ruleOwner2, + }), + createRule(KyselyPg, org2.id, { + alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, + id: '37a650c1342', + creator: ruleOwner2, + }), + createRule(KyselyPg, org2.id, { + alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, + id: 'b4ee86f9386', + creator: ruleOwner2, + }), + ]); - mockDummyRules = fakeRules.map((r) => ({ - id: r.id, - orgId: r.orgId, - creatorId: r.creatorId, - name: r.name, - alarmStatus: r.alarmStatus, - statusIfUnexpired: r.statusIfUnexpired, - })); + const mockDummyRules = fakeRules.map((r) => ({ + id: r.id, + orgId: r.orgId, + creatorId: r.creatorId, + name: r.name, + alarmStatus: r.alarmStatus, + statusIfUnexpired: r.statusIfUnexpired, + })); - mockGetCurrentPeriodRuleAlarmStatuses = async () => { - const newAlarmStatusByRule = - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - {} as Awaited>; + const mockGetCurrentPeriodRuleAlarmStatuses: GetCurrentPeriodRuleAlarmStatuses = + async () => { + const newAlarmStatusByRule = + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + {} as Awaited>; - fakeRules.forEach((rule, i) => { - newAlarmStatusByRule[rule.id] = { - status: - i % 3 === 0 - ? RuleAlarmStatus.ALARM - : i % 3 === 1 - ? RuleAlarmStatus.OK - : RuleAlarmStatus.INSUFFICIENT_DATA, - meta: { lastPeriodPassRate: 0.5, secondToLastPeriodPassRate: 0.4 }, + fakeRules.forEach((rule, i) => { + // eslint-disable-next-line functional/immutable-data + newAlarmStatusByRule[rule.id] = { + status: + i % 3 === 0 + ? RuleAlarmStatus.ALARM + : i % 3 === 1 + ? RuleAlarmStatus.OK + : RuleAlarmStatus.INSUFFICIENT_DATA, + meta: { + lastPeriodPassRate: 0.5, + secondToLastPeriodPassRate: 0.4, + }, + }; + }); + return newAlarmStatusByRule; }; - }); - return newAlarmStatusByRule; - }; - mockNotificationsService = { - createNotifications: jest.fn(), - getNotificationsForUser: jest.fn(), - } as unknown as Mocked< - PublicInterface, - 'createNotifications' - >; + const mockNotificationsService = { + createNotifications: jest.fn(), + getNotificationsForUser: jest.fn(), + } as unknown as Mocked< + PublicInterface, + 'createNotifications' + >; - mockKysely = makeMockKyselyForRules(mockDummyRules, [ - { id: org.id, on_call_alert_email: null }, - { id: org2.id, on_call_alert_email: 'test@gmail.com' }, - ]); - - deleteMockData = async () => { - await Promise.all(fakeRules.map(async (it) => it.destroy())); - await Promise.all([ruleOwnerCleanup(), ruleOwner2Cleanup()]); - await orgCleanup(); - await org2Cleanup(); - }; - /* eslint-enable functional/immutable-data */ - }); + const mockKysely = makeMockKyselyForRules(mockDummyRules, [ + { id: org.id, on_call_alert_email: null }, + { id: org2.id, on_call_alert_email: 'test@gmail.com' }, + ]); - afterAll(async () => { - return deleteMockData(); - }); + return { + mockKysely, + mockNotificationsService, + mockGetCurrentPeriodRuleAlarmStatuses, + mockDummyRules, + }; + }, + ); - test('should generate the proper notifications + update rules', async () => { - const worker = DetectRulePassRateAnomaliesJob( - mockKysely as unknown as Dependencies['KyselyPg'], + testWithFixture( + 'should generate the proper notifications + update rules', + async ({ + mockKysely, mockNotificationsService, mockGetCurrentPeriodRuleAlarmStatuses, - jest.fn<() => Promise>(), - ); - await worker.run(); + mockDummyRules, + }) => { + const worker = DetectRulePassRateAnomaliesJob( + mockKysely as unknown as Dependencies['KyselyPg'], + mockNotificationsService, + mockGetCurrentPeriodRuleAlarmStatuses, + jest.fn<() => Promise>(), + ); + await worker.run(); - const mockCreateNotifications = - mockNotificationsService.createNotifications; + const mockCreateNotifications = + mockNotificationsService.createNotifications; - expect(mockCreateNotifications).toHaveBeenCalledTimes(1); - expect( - mockCreateNotifications.mock.calls[0][0] - .slice(0) - .sort((a, b) => a.data.ruleId.localeCompare(b.data.ruleId)), - ).toMatchInlineSnapshot(` + expect(mockCreateNotifications).toHaveBeenCalledTimes(1); + expect( + mockCreateNotifications.mock.calls[0][0] + .slice(0) + .sort((a, b) => a.data.ruleId.localeCompare(b.data.ruleId)), + ).toMatchInlineSnapshot(` [ { "data": { @@ -312,11 +301,11 @@ describe('Detect Rule Anomalies', () => { ] `); - await mockGetCurrentPeriodRuleAlarmStatuses(); - const expectedUpdates = mockDummyRules.filter( - (_rule, i) => ![0, 4, 8].includes(i), - ).length; - expect(mockKysely.updateTable).toHaveBeenCalledTimes(expectedUpdates); - }); + const expectedUpdates = mockDummyRules.filter( + (_rule, i) => ![0, 4, 8].includes(i), + ).length; + expect(mockKysely.updateTable).toHaveBeenCalledTimes(expectedUpdates); + }, + ); }); }); diff --git a/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts b/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts index 66b631f..ba4878e 100644 --- a/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts +++ b/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts @@ -6,8 +6,7 @@ import createActions from '../../../../test/fixtureHelpers/createActions.js'; import createContentItemTypes from '../../../../test/fixtureHelpers/createContentItemTypes.js'; import createOrg from '../../../../test/fixtureHelpers/createOrg.js'; import createUser from '../../../../test/fixtureHelpers/createUser.js'; -import { makeMockedServer } from '../../../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../../../test/harness/transactionalTest.js'; import { toCorrelationId } from '../../../../utils/correlationIds.js'; import SafeTracer from '../../../../utils/SafeTracer.js'; import { @@ -17,9 +16,7 @@ import { } from '../../../itemProcessingService/index.js'; describe('AggregationSignal', () => { - const testWithFixture = makeTestWithFixture(async () => { - const { server, deps, shutdown } = await makeMockedServer(); - + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { const { ModerationConfigService, AggregationsService, @@ -29,22 +26,21 @@ describe('AggregationSignal', () => { KyselyPg, } = deps; - const { org, cleanup: orgCleanup } = await createOrg( + const { org } = await createOrg( { KyselyPg, ModerationConfigService, ApiKeyService }, uid(), ); - const { user, cleanup: userCleanup } = await createUser(KyselyPg, org.id); + const { user } = await createUser(KyselyPg, org.id); - const { itemTypes, cleanup: itemTypesCleanup } = - await createContentItemTypes({ - moderationConfigService: ModerationConfigService, - orgId: org.id, - includeCreator: true, - extra: {}, - }); + const { itemTypes } = await createContentItemTypes({ + moderationConfigService: ModerationConfigService, + orgId: org.id, + includeCreator: true, + extra: {}, + }); - const { actions, cleanup: actionsCleanup } = await createActions({ + const { actions } = await createActions({ actionAPI: ActionAPIDataSource, itemTypeIds: [itemTypes[0].id], orgId: org.id, @@ -133,20 +129,10 @@ describe('AggregationSignal', () => { ); return { - server, - deps, rule, itemType: itemTypes[0], org, dateProvider, - async cleanup() { - await RuleAPIDataSource.deleteRule({ id: rule.id, orgId: org.id }); - await actionsCleanup(); - await itemTypesCleanup(); - await userCleanup(); - await orgCleanup(); - await shutdown(); - }, }; }); diff --git a/server/test/fixtureHelpers/fixtureHelpers.test.ts b/server/test/fixtureHelpers/fixtureHelpers.test.ts index b4526e8..00bc508 100644 --- a/server/test/fixtureHelpers/fixtureHelpers.test.ts +++ b/server/test/fixtureHelpers/fixtureHelpers.test.ts @@ -7,16 +7,14 @@ import { RuleType, } from '../../services/moderationConfigService/index.js'; import { UserRole } from '../../services/userManagementService/index.js'; -import { makeMockedServer } from '../setupMockedServer.js'; -import { makeTestWithFixture } from '../utils.js'; +import { makeTransactionalTestWithFixture } from '../harness/transactionalTest.js'; import createOrg from './createOrg.js'; import createRule from './createRule.js'; import createUser from './createUser.js'; describe('fixtureHelpers', () => { - const testWithOrg = makeTestWithFixture(async () => { - const { deps, shutdown } = await makeMockedServer(); - const { org, cleanup: orgCleanup } = await createOrg( + const testWithOrg = makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -24,14 +22,7 @@ describe('fixtureHelpers', () => { }, uid(), ); - return { - deps, - org, - async cleanup() { - await orgCleanup(); - await shutdown(); - }, - }; + return { org }; }); describe('createUser', () => { @@ -39,20 +30,16 @@ describe('fixtureHelpers', () => { 'defaults: SAML-only loginMethods, ADMIN role, null password, override id honored', async ({ deps, org }) => { const overrideId = uid(); - const { user, cleanup } = await createUser(deps.KyselyPg, org.id, { + const { user } = await createUser(deps.KyselyPg, org.id, { + id: overrideId, + }); + expect(user).toMatchObject({ id: overrideId, + orgId: org.id, + role: UserRole.ADMIN, + loginMethods: ['saml'], + password: null, }); - try { - expect(user).toMatchObject({ - id: overrideId, - orgId: org.id, - role: UserRole.ADMIN, - loginMethods: ['saml'], - password: null, - }); - } finally { - await cleanup(); - } }, ); @@ -74,25 +61,21 @@ describe('fixtureHelpers', () => { id: overrideId, name: overrideName, }); - try { - expect(rule).toMatchObject({ - id: overrideId, - orgId: org.id, - name: overrideName, - alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, - statusIfUnexpired: RuleStatus.LIVE, - }); + expect(rule).toMatchObject({ + id: overrideId, + orgId: org.id, + name: overrideName, + alarmStatus: RuleAlarmStatus.INSUFFICIENT_DATA, + statusIfUnexpired: RuleStatus.LIVE, + }); - const row = await deps.KyselyPg.selectFrom('public.rules') - .select(['rule_type', 'status_if_unexpired', 'alarm_status']) - .where('id', '=', overrideId) - .executeTakeFirstOrThrow(); - expect(row.rule_type).toBe(RuleType.CONTENT); - expect(row.status_if_unexpired).toBe(RuleStatus.LIVE); - expect(row.alarm_status).toBe(RuleAlarmStatus.INSUFFICIENT_DATA); - } finally { - await rule.destroy(); - } + const row = await deps.KyselyPg.selectFrom('public.rules') + .select(['rule_type', 'status_if_unexpired', 'alarm_status']) + .where('id', '=', overrideId) + .executeTakeFirstOrThrow(); + expect(row.rule_type).toBe(RuleType.CONTENT); + expect(row.status_if_unexpired).toBe(RuleStatus.LIVE); + expect(row.alarm_status).toBe(RuleAlarmStatus.INSUFFICIENT_DATA); }, ); @@ -102,16 +85,12 @@ describe('fixtureHelpers', () => { const rule = await createRule(deps.KyselyPg, org.id, { alarmStatus: RuleAlarmStatus.ALARM, }); - try { - expect(rule.alarmStatus).toBe(RuleAlarmStatus.ALARM); - const row = await deps.KyselyPg.selectFrom('public.rules') - .select('alarm_status') - .where('id', '=', rule.id) - .executeTakeFirstOrThrow(); - expect(row.alarm_status).toBe(RuleAlarmStatus.ALARM); - } finally { - await rule.destroy(); - } + expect(rule.alarmStatus).toBe(RuleAlarmStatus.ALARM); + const row = await deps.KyselyPg.selectFrom('public.rules') + .select('alarm_status') + .where('id', '=', rule.id) + .executeTakeFirstOrThrow(); + expect(row.alarm_status).toBe(RuleAlarmStatus.ALARM); }, ); @@ -121,23 +100,19 @@ describe('fixtureHelpers', () => { const rule = await createRule(deps.KyselyPg, org.id, { ruleType: RuleType.USER, }); - try { - const row = await deps.KyselyPg.selectFrom('public.rules') - .select('rule_type') - .where('id', '=', rule.id) - .executeTakeFirstOrThrow(); - expect(row.rule_type).toBe(RuleType.USER); + const row = await deps.KyselyPg.selectFrom('public.rules') + .select('rule_type') + .where('id', '=', rule.id) + .executeTakeFirstOrThrow(); + expect(row.rule_type).toBe(RuleType.USER); - const junctions = await deps.KyselyPg.selectFrom( - 'public.rules_and_item_types', - ) - .select('item_type_id') - .where('rule_id', '=', rule.id) - .execute(); - expect(junctions).toEqual([]); - } finally { - await rule.destroy(); - } + const junctions = await deps.KyselyPg.selectFrom( + 'public.rules_and_item_types', + ) + .select('item_type_id') + .where('rule_id', '=', rule.id) + .execute(); + expect(junctions).toEqual([]); }, ); @@ -162,16 +137,9 @@ describe('fixtureHelpers', () => { 'auto-creates a creator user when none is supplied', async ({ deps, org }) => { const rule = await createRule(deps.KyselyPg, org.id); - try { - const creator = await kyselyUserFindById( - deps.KyselyPg, - rule.creatorId, - ); - expect(creator).toBeDefined(); - expect(creator?.orgId).toBe(org.id); - } finally { - await rule.destroy(); - } + const creator = await kyselyUserFindById(deps.KyselyPg, rule.creatorId); + expect(creator).toBeDefined(); + expect(creator?.orgId).toBe(org.id); }, ); }); diff --git a/server/test/harness/transactionalPgPool.integ.test.ts b/server/test/harness/transactionalPgPool.integ.test.ts new file mode 100644 index 0000000..f15f4b0 --- /dev/null +++ b/server/test/harness/transactionalPgPool.integ.test.ts @@ -0,0 +1,157 @@ +/** + * Keystone test for the transaction-rollback test harness. + * + * Proves that `createTransactionalTestDb` lets us wrap a whole test in a single + * Postgres transaction that is rolled back at the end. + */ +import 'dotenv/config'; + +import { Kysely, PostgresDialect, sql } from 'kysely'; +import pg from 'pg'; + +import { getPgConnectionParams } from '../../iocContainer/index.js'; +import { makeKyselyTransactionWithRetry } from '../../utils/kyselyTransactionWithRetry.js'; +import { createTransactionalTestDb } from './transactionalPgPool.js'; + +const pgConfig = getPgConnectionParams(); + +describe('createTransactionalTestDb', () => { + it('rolls back every write made through the facade, isolating it from other connections', async () => { + const tdb = createTransactionalTestDb(pgConfig); + await tdb.begin(); + try { + const db = new Kysely>({ + dialect: new PostgresDialect({ pool: tdb.pool }), + }); + + await sql`create table rollback_probe (id int)`.execute(db); + await sql`insert into rollback_probe (id) values (1)`.execute(db); + const within = await sql<{ + count: number; + }>`select count(*)::int as count from rollback_probe`.execute(db); + expect(within.rows[0].count).toBe(1); + + await tdb.rollback(); + await db.destroy(); + + // A separate, real connection must see no trace of the rolled-back work. + const probe = new pg.Client(pgConfig); + await probe.connect(); + try { + const exists = await probe.query( + `select to_regclass('public.rollback_probe') as table_oid`, + ); + expect(exists.rows[0].table_oid).toBeNull(); + } finally { + await probe.end(); + } + } finally { + // Always close the pinned connection so a mid-test throw can't leak it + // (and closing aborts any still-open outer transaction). + await tdb.end(); + } + }); + + it('rewrites application transactions to savepoints: committed nested work survives, rolled-back nested work does not, outer stays alive', async () => { + const tdb = createTransactionalTestDb(pgConfig); + await tdb.begin(); + try { + const db = new Kysely>({ + dialect: new PostgresDialect({ pool: tdb.pool }), + }); + const transactionWithRetry = makeKyselyTransactionWithRetry(db); + + await sql`create table savepoint_probe (id int, tag text)`.execute(db); + + // A nested transaction that commits — its COMMIT must become RELEASE + // SAVEPOINT, so the row persists within the still-open outer transaction. + await transactionWithRetry(async (trx) => { + await sql`insert into savepoint_probe (id, tag) values (1, 'committed')`.execute( + trx, + ); + }); + + // A nested transaction that throws — its ROLLBACK must become ROLLBACK TO + // SAVEPOINT, discarding only its own write while leaving the outer + // transaction (and the committed row above) intact. + await expect( + transactionWithRetry(async (trx) => { + await sql`insert into savepoint_probe (id, tag) values (2, 'rolled-back')`.execute( + trx, + ); + throw new Error('boom'); + }), + ).rejects.toThrow('boom'); + + const rows = await sql<{ + id: number; + }>`select id from savepoint_probe order by id`.execute(db); + expect(rows.rows.map((r) => r.id)).toEqual([1]); + + await tdb.rollback(); + await db.destroy(); + } finally { + // Always close the pinned connection so a mid-test throw can't leak it + // (and closing aborts any still-open outer transaction). + await tdb.end(); + } + }); + + it('rejects unhandled transaction-control statements that would escape the outer transaction', async () => { + const tdb = createTransactionalTestDb(pgConfig); + await tdb.begin(); + try { + // `END`/`ABORT` are aliases for COMMIT/ROLLBACK: forwarded unrewritten + // they would act on the outer per-test transaction and defeat isolation, + // so the facade must reject them rather than pass them through. + await expect(tdb.pool.query('END')).rejects.toThrow( + /transactionalPgPool refused to run "END"/, + ); + await expect(tdb.pool.query('ABORT')).rejects.toThrow( + /transactionalPgPool refused to run "ABORT"/, + ); + + // A `PREPARE ` statement is an ordinary query (not transaction + // control) and must still pass straight through. + await tdb.pool.query('prepare harness_probe as select 1'); + const prepared = await tdb.pool.query('execute harness_probe'); + expect(prepared.rows).toHaveLength(1); + + await tdb.rollback(); + } finally { + // Always close the pinned connection so a mid-test throw can't leak it + // (and closing aborts any still-open outer transaction). + await tdb.end(); + } + }); + + it('supports direct pool.query (e.g. for the session store), routed through the same rolled-back transaction', async () => { + const tdb = createTransactionalTestDb(pgConfig); + await tdb.begin(); + try { + await tdb.pool.query('create table direct_probe (id int)'); + await tdb.pool.query('insert into direct_probe (id) values ($1)', [7]); + const within = await tdb.pool.query( + 'select count(*)::int as count from direct_probe', + ); + expect(within.rows[0].count).toBe(1); + + await tdb.rollback(); + + const probe = new pg.Client(pgConfig); + await probe.connect(); + try { + const exists = await probe.query( + `select to_regclass('public.direct_probe') as table_oid`, + ); + expect(exists.rows[0].table_oid).toBeNull(); + } finally { + await probe.end(); + } + } finally { + // Always close the pinned connection so a mid-test throw can't leak it + // (and closing aborts any still-open outer transaction). + await tdb.end(); + } + }); +}); diff --git a/server/test/harness/transactionalPgPool.ts b/server/test/harness/transactionalPgPool.ts new file mode 100644 index 0000000..3197476 --- /dev/null +++ b/server/test/harness/transactionalPgPool.ts @@ -0,0 +1,176 @@ +import pg from 'pg'; + +/** + * Test-only Postgres harness that wraps an entire test in a single transaction + * and rolls it back at the end, giving perfect isolation without any per-test + * cleanup. + * + * It hands out a `pg.Pool`-shaped facade (`.pool`) to be injected wherever the + * app expects a Postgres pool (e.g. Kysely's `PostgresDialect`). The facade: + * + * - pins a SINGLE real connection and returns it for every `connect()`, so + * the app's reads see the app's own uncommitted writes (and so a read + * replica routed through the same facade stays consistent); + * - never truly releases that connection (`release()` is a no-op); + * - rewrites the application's own transaction-control statements to + * SAVEPOINTs: `BEGIN`/`START TRANSACTION` -> `SAVEPOINT`, `COMMIT` -> + * `RELEASE SAVEPOINT`, `ROLLBACK` -> `ROLLBACK TO SAVEPOINT`. This is what + * lets nested application transactions (e.g. via + * `makeKyselyTransactionWithRetry`) commit/roll back correctly relative to + * their own scope while the outer test transaction still discards + * everything on `rollback()`. + * + * The outer transaction itself is driven by `begin()`/`rollback()`, which issue + * the real `BEGIN`/`ROLLBACK` on the pinned connection (bypassing the savepoint + * rewrite). + * + * NOTE: a single pinned connection cannot run queries concurrently, and the + * savepoint stack assumes properly nested (LIFO) transactions. + */ +export type TransactionalTestDb = { + /** A `pg.Pool`-compatible facade to inject into the app under test. */ + pool: pg.Pool; + /** Connect and open the outer transaction. Call once before the test runs. */ + begin: () => Promise; + /** Roll back the outer transaction, discarding all writes made during it. */ + rollback: () => Promise; + /** Close the underlying connection. Call once after the test. */ + end: () => Promise; +}; + +// This harness doesn't yet support every single Postgres transaction control +// statement. The following are unsupported, and should raise an exception loudly +// rather than silently break the test harness. +const UNSUPPORTED_TXN_CONTROL_VERBS = [ + 'end', // alias for COMMIT + 'abort', // alias for ROLLBACK + 'commit', // COMMIT PREPARED + 'rollback', // ROLLBACK TO SAVEPOINT / ROLLBACK PREPARED + 'savepoint', + 'release', // RELEASE [SAVEPOINT] + 'prepare transaction', +]; + +function rejectUnsupportedTransactionControl( + normalized: string, + raw: string, +): void { + const isUnsupported = UNSUPPORTED_TXN_CONTROL_VERBS.some( + (verb) => normalized === verb || normalized.startsWith(`${verb} `), + ); + if (!isUnsupported) return; + + throw new Error( + [ + `transactionalPgPool refused to run "${raw}".`, + '', + 'This test harness isolates each test by wrapping it in one Postgres', + 'transaction and rewriting BEGIN/COMMIT/ROLLBACK into savepoints. The', + 'statement above is a different transaction-control command that would', + 'act on the outer per-test transaction instead — committing or aborting', + "the whole test's writes and breaking isolation for every later test.", + ].join('\n'), + ); +} + +export function createTransactionalTestDb( + config: pg.ClientConfig, +): TransactionalTestDb { + const client = new pg.Client(config); + let connected = false; + // Application transactions are strictly nested (LIFO), so the active + // savepoint is fully determined by the current depth — no stack needed. + let savepointDepth = 0; + const savepointName = (depth: number) => `coop_test_sp_${depth}`; + + const openSavepoint = async () => { + savepointDepth += 1; + return client.query(`SAVEPOINT ${savepointName(savepointDepth)}`); + }; + + const closeSavepoint = async (verb: 'commit' | 'rollback') => { + if (savepointDepth <= 0) { + throw new Error( + `${verb.toUpperCase()} without a matching application transaction`, + ); + } + const name = savepointName(savepointDepth); + if (verb === 'rollback') { + await client.query(`ROLLBACK TO SAVEPOINT ${name}`); + } + const result = await client.query(`RELEASE SAVEPOINT ${name}`); + // Drop the depth only after the SQL succeeds, so a failed call can't leave + // the savepoint stack out of sync. + savepointDepth -= 1; + return result; + }; + + // The single place transaction-control statements are rewritten to + // savepoints; everything else passes straight through to the pinned client. + // Shared by both the per-connection facade (`connect().query`) and the + // pool-level `query` (used by consumers like the express-session store). + const runQuery = async ( + textOrConfig: string | pg.QueryConfig, + values?: unknown[], + ): Promise => { + const text = + typeof textOrConfig === 'string' ? textOrConfig : textOrConfig.text; + const normalized = + typeof text === 'string' + ? text.trim().toLowerCase().replace(/;/g, '') + : ''; + + if (normalized === 'begin' || normalized.startsWith('start transaction')) { + return openSavepoint(); + } + if (normalized === 'commit') { + return closeSavepoint('commit'); + } + if (normalized === 'rollback') { + return closeSavepoint('rollback'); + } + + rejectUnsupportedTransactionControl(normalized, text); + + return values === undefined + ? client.query(textOrConfig) + : client.query(textOrConfig, values); + }; + + const facadeClient = { + query: runQuery, + // Keep the pinned connection alive across the whole test. + release() {}, + }; + + const pool = { + async connect() { + return facadeClient; + }, + async query(textOrConfig: string | pg.QueryConfig, values?: unknown[]) { + return runQuery(textOrConfig, values); + }, + async end() {}, + on() {}, + }; + + return { + pool: pool as unknown as pg.Pool, + async begin() { + await client.connect(); + connected = true; + await client.query('BEGIN'); + savepointDepth = 0; + }, + async rollback() { + await client.query('ROLLBACK'); + savepointDepth = 0; + }, + async end() { + if (connected) { + await client.end(); + connected = false; + } + }, + }; +} diff --git a/server/test/harness/transactionalTest.ts b/server/test/harness/transactionalTest.ts new file mode 100644 index 0000000..708a2ed --- /dev/null +++ b/server/test/harness/transactionalTest.ts @@ -0,0 +1,39 @@ +/** + * Like `makeTestWithFixture`, but each test gets a fresh `makeMockedServer` + * (real Postgres in a transaction) that's rolled back afterward, so fixtures + * need no cleanup. The setup callback receives `{ deps, request }` and returns + * its fixtures; the test gets those plus `deps` and `request`. + * + * ```ts + * const testWithOrg = makeTransactionalTestWithFixture(async ({ deps }) => { + * const { org } = await createOrg({ ... }, uid()); + * return { org }; + * }); + * testWithOrg('reads the org back', async ({ org }) => { ... }); + * ``` + */ +import { makeMockedServer, type MockedServer } from '../setupMockedServer.js'; +import { makeTestWithFixture } from '../utils.js'; + +type ServerVars = Pick; + +export function makeTransactionalTestWithFixture< + T extends Record, +>(makeFixtures: (server: ServerVars) => Promise | T) { + return makeTestWithFixture(async () => { + const server = await makeMockedServer(); + const fixtures = await makeFixtures({ + deps: server.deps, + request: server.request, + }); + return { + deps: server.deps, + request: server.request, + ...fixtures, + async cleanup() { + await server.rollback(); + await server.shutdown(); + }, + }; + }); +} diff --git a/server/test/setupMockedServer.ts b/server/test/setupMockedServer.ts index 644c359..b75f75a 100644 --- a/server/test/setupMockedServer.ts +++ b/server/test/setupMockedServer.ts @@ -3,13 +3,18 @@ // relying on here). import otel from '@opentelemetry/api'; +import type pg from 'pg'; import * as superTest from 'supertest'; -import getBottle, { type Dependencies } from '../iocContainer/index.js'; +import getBottle, { + getPgConnectionParams, + type Dependencies, +} from '../iocContainer/index.js'; import makeServer from '../server.js'; import { type IDataWarehouse } from '../storage/dataWarehouse/IDataWarehouse.js'; import type { IDataWarehouseAnalytics } from '../storage/dataWarehouse/IDataWarehouseAnalytics.js'; import SafeTracer from '../utils/SafeTracer.js'; +import { createTransactionalTestDb } from './harness/transactionalPgPool.js'; /** * Occassionally, we make a request that's supposed to error, so this function @@ -28,16 +33,52 @@ export function disableConsoleLogging() { /* eslint-enable functional/immutable-data */ } +/** + * Boots the Express app against real Postgres (ClickHouse/analytics mocked), + * inside a transaction that `rollback()` discards so tests need no cleanup. + * Only Postgres is rolled back. Usually used via + * `makeTransactionalTestWithFixture`. + */ export async function makeMockedServer() { - const deps = await getBottleContainerWithIOMocks(); - const { app: server, shutdown } = await makeServer(deps); + const tdb = createTransactionalTestDb(getPgConnectionParams()); + await tdb.begin(); + + const deps = await getBottleContainerWithIOMocks({ kyselyPool: tdb.pool }); + const { app: server, shutdown: shutdownServer } = await makeServer(deps); const request = superTest.agent(server); - return { deps, server, shutdown, request }; + + return { + deps, + server, + request, + /** Roll back everything written to Postgres during the test. */ + rollback: tdb.rollback, + async shutdown() { + try { + await shutdownServer(); + } finally { + await tdb.end(); + } + }, + }; } -export async function getBottleContainerWithIOMocks() { +export type MockedServer = Awaited>; + +export async function getBottleContainerWithIOMocks( + opts: { kyselyPool?: pg.Pool } = {}, +) { const bottle = await getBottle(); + // Optional Postgres pool override (used by the transaction-rollback harness + // to route all PG access through a single rolled-back connection). Applied + // before the container resolves anything so every service is built on it. + if (opts.kyselyPool != null) { + const pool = opts.kyselyPool; + bottle.factory('KyselyPgPool', () => pool); + bottle.factory('KyselyPgReadReplica', (container) => container.KyselyPg); + } + // The mutation rule below is a false positive, as we're just doing // initial setup on this mock object before exposing it. -- 2.51.2