diff --git a/package.json b/package.json index 93cddf313..032522eda 100644 --- a/package.json +++ b/package.json @@ -458,7 +458,7 @@ "fuzzy": false }, "fiori": { - "draft_admin_integrity": true + "draft_admin_integrity": false }, "websocket": { "kind": "socket.io", diff --git a/srv/lib/draft-integrity-preflight.js b/srv/lib/draft-integrity-preflight.js index 416f984d9..74e9bf59e 100644 --- a/srv/lib/draft-integrity-preflight.js +++ b/srv/lib/draft-integrity-preflight.js @@ -19,7 +19,12 @@ // Authoritative list of draft-enabled entity table names (lowercase). // Source: all entities with @odata.draft.enabled: true in AdminService, // mapped to their underlying HANA table name (dots → underscores + _drafts). -// If a new draft-enabled entity is added to AdminService, add its entry here. +// DEPRECATED — no longer used by checkDraftCascadeConstraints, which now +// discovers the real `ADMINSERVICE__DRAFTS` tables from SYS.TABLES at +// runtime. These `com_sap_developers_ims_*_drafts` names never matched any +// deployed table (the check reported them all "missing" and validated nothing). +// Retained only for the existing unit test's import; safe to delete once that +// test is updated. export const DRAFT_TABLES = [ 'com_sap_developers_ims_tutorials_drafts', 'com_sap_developers_ims_missions_drafts', @@ -60,29 +65,52 @@ export function isHanaKind(cds) { } /** - * Query SYS.REFERENTIAL_CONSTRAINTS for ON DELETE CASCADE FKs on draft tables. + * Query SYS.REFERENTIAL_CONSTRAINTS for ON DELETE CASCADE FKs on draft tables, + * comparing against the draft tables ACTUALLY present in the schema. + * + * The real draft shadow tables are named `ADMINSERVICE__DRAFTS` + * (service-prefixed, uppercased) — NOT the entity-DB names. An earlier + * hardcoded `com_sap_developers_ims_*_drafts` list never matched any real + * table, so the check silently reported 26/26 "missing" and validated nothing. + * We now discover the draft tables from SYS.TABLES at runtime so the list can + * never drift out of sync with the deployed schema. * * Pure: receives dbRun as a parameter so it can be tested without booting CAP. * * @param {(sql: string) => Promise} dbRun - * @returns {Promise<{ok: boolean, found: string[], missing: string[], error?: string}>} + * @returns {Promise<{ok: boolean, found: string[], missing: string[], total: number, error?: string}>} */ export async function checkDraftCascadeConstraints(dbRun) { try { - const rows = await dbRun( + // 1. Discover the draft tables that actually exist in this schema, and + // that carry the DraftAdministrativeData FK column (i.e. real draft + // shadow tables, not child comps without the FK). + const draftTableRows = await dbRun( + `SELECT c.TABLE_NAME FROM SYS.TABLE_COLUMNS c` + + ` JOIN SYS.TABLES t ON t.SCHEMA_NAME = c.SCHEMA_NAME AND t.TABLE_NAME = c.TABLE_NAME` + + ` WHERE c.SCHEMA_NAME = CURRENT_SCHEMA` + + ` AND c.TABLE_NAME LIKE '%\\_DRAFTS' ESCAPE '\\'` + + ` AND c.COLUMN_NAME = 'DRAFTADMINISTRATIVEDATA_DRAFTUUID'` + ); + const draftTables = (draftTableRows || []).map(r => String(r.TABLE_NAME).toUpperCase()); + + // 2. Which of those have an ON DELETE CASCADE FK on the DraftUUID column. + const cascadeRows = await dbRun( `SELECT TABLE_NAME FROM SYS.REFERENTIAL_CONSTRAINTS` + ` WHERE COLUMN_NAME = 'DRAFTADMINISTRATIVEDATA_DRAFTUUID'` + ` AND DELETE_RULE = 'CASCADE'` ); - const foundSet = new Set((rows || []).map(r => String(r.TABLE_NAME).toLowerCase())); - const found = DRAFT_TABLES.filter(t => foundSet.has(t)); - const missing = DRAFT_TABLES.filter(t => !foundSet.has(t)); - return { ok: missing.length === 0, found, missing }; + const cascadeSet = new Set((cascadeRows || []).map(r => String(r.TABLE_NAME).toUpperCase())); + + const found = draftTables.filter(t => cascadeSet.has(t)); + const missing = draftTables.filter(t => !cascadeSet.has(t)); + return { ok: missing.length === 0, found, missing, total: draftTables.length }; } catch (err) { return { ok: false, found: [], - missing: [...DRAFT_TABLES], + missing: [], + total: 0, error: err?.message ?? String(err), }; } @@ -109,11 +137,17 @@ export function wireDraftIntegrityPreflight(cds) { `[draft-integrity-preflight] DB error querying SYS.REFERENTIAL_CONSTRAINTS (fail-open):`, result.error ); + } else if (result.total === 0) { + log.warn( + `[draft-integrity-preflight] no draft shadow tables with a` + + ` DraftAdministrativeData FK were found in the schema — cannot verify` + + ` cascade constraints. If drafts are expected, check the schema name.` + ); } else if (!result.ok) { log.warn( `[draft-integrity-preflight] cds.fiori.draft_admin_integrity is ON but` + ` ON DELETE CASCADE constraints are missing on HANA for` + - ` ${result.missing.length}/${DRAFT_TABLES.length} draft tables.` + + ` ${result.missing.length}/${result.total} draft tables.` + ` Missing: ${result.missing.join(', ')}.` + ` Deploy the HDI constraints (.hdbconstraint) before drafts are created` + ` or activated on this environment, then restart.` @@ -121,7 +155,7 @@ export function wireDraftIntegrityPreflight(cds) { } else { log.info( `[draft-integrity-preflight] draft cascade constraints verified` + - ` (${result.found.length}/${DRAFT_TABLES.length}).` + ` (${result.found.length}/${result.total}).` ); } } catch (err) { diff --git a/test/unit/draft-integrity-preflight.test.js b/test/unit/draft-integrity-preflight.test.js index 4f6aa14f1..97105918e 100644 --- a/test/unit/draft-integrity-preflight.test.js +++ b/test/unit/draft-integrity-preflight.test.js @@ -1,6 +1,9 @@ // test/unit/draft-integrity-preflight.test.js // -// Unit tests for srv/lib/draft-integrity-preflight.js (#2968). +// Unit tests for srv/lib/draft-integrity-preflight.js (#2968, rev #2909-fix). +// checkDraftCascadeConstraints now DISCOVERS the real ADMINSERVICE_*_DRAFTS +// tables from SYS.TABLE_COLUMNS, then checks which have an ON DELETE CASCADE +// constraint. The mock dbRun below branches on the SQL to emulate both queries. // All tests use mock dbRun — no CAP boot required. import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { @@ -10,22 +13,29 @@ import { wireDraftIntegrityPreflight, } from '../../srv/lib/draft-integrity-preflight.js'; -describe('DRAFT_TABLES', () => { - it('is a non-empty array of lowercase strings', () => { +// Realistic sample of discovered draft tables (service-prefixed, uppercase). +const DISCOVERED = [ + 'ADMINSERVICE_MISSIONS_DRAFTS', + 'ADMINSERVICE_GROUPS_DRAFTS', + 'ADMINSERVICE_TUTORIALS_DRAFTS', +]; + +// Build a mock dbRun that answers the discovery query with `discovered` +// and the cascade-constraint query with `cascade`. +const makeDbRun = (discovered, cascade) => async (sql) => { + if (/TABLE_COLUMNS/.test(sql)) { + return discovered.map((t) => ({ TABLE_NAME: t })); + } + if (/REFERENTIAL_CONSTRAINTS/.test(sql)) { + return cascade.map((t) => ({ TABLE_NAME: t })); + } + return []; +}; + +describe('DRAFT_TABLES (deprecated export)', () => { + it('is still exported as a non-empty array (legacy import compatibility)', () => { expect(Array.isArray(DRAFT_TABLES)).toBe(true); expect(DRAFT_TABLES.length).toBeGreaterThan(0); - for (const t of DRAFT_TABLES) { - expect(typeof t).toBe('string'); - expect(t).toBe(t.toLowerCase()); - } - }); - - it('contains the known high-risk draft tables', () => { - // These are the tables mentioned in the runbook. If a new draft-enabled - // entity is added to AdminService and this test fails, add it to DRAFT_TABLES. - expect(DRAFT_TABLES).toContain('com_sap_developers_ims_missions_drafts'); - expect(DRAFT_TABLES).toContain('com_sap_developers_ims_groups_drafts'); - expect(DRAFT_TABLES).toContain('com_sap_developers_ims_tutorials_drafts'); }); }); @@ -33,15 +43,12 @@ describe('isHanaKind', () => { it('returns true when cds.db.options.kind is hana', () => { expect(isHanaKind({ db: { options: { kind: 'hana' } } })).toBe(true); }); - it('returns false for sqlite', () => { expect(isHanaKind({ db: { options: { kind: 'sqlite' } } })).toBe(false); }); - it('returns false when cds.db is undefined', () => { expect(isHanaKind({})).toBe(false); }); - it('returns false when cds itself is empty-ish', () => { expect(isHanaKind(null)).toBe(false); expect(isHanaKind(undefined)).toBe(false); @@ -49,51 +56,66 @@ describe('isHanaKind', () => { }); describe('checkDraftCascadeConstraints', () => { - it('returns ok:true when all DRAFT_TABLES have a cascade constraint', async () => { - const allRows = DRAFT_TABLES.map(t => ({ TABLE_NAME: t.toUpperCase() })); - const result = await checkDraftCascadeConstraints(async () => allRows); + it('ok:true when every discovered draft table has a cascade constraint', async () => { + const result = await checkDraftCascadeConstraints(makeDbRun(DISCOVERED, DISCOVERED)); expect(result.ok).toBe(true); + expect(result.total).toBe(DISCOVERED.length); + expect(result.found).toHaveLength(DISCOVERED.length); expect(result.missing).toHaveLength(0); - expect(result.found).toHaveLength(DRAFT_TABLES.length); expect(result.error).toBeUndefined(); }); - it('returns ok:false when no constraints exist', async () => { - const result = await checkDraftCascadeConstraints(async () => []); + it('ok:false and lists missing when some tables lack the constraint', async () => { + const result = await checkDraftCascadeConstraints( + makeDbRun(DISCOVERED, [DISCOVERED[0]]) + ); expect(result.ok).toBe(false); - expect(result.found).toHaveLength(0); - expect(result.missing).toHaveLength(DRAFT_TABLES.length); + expect(result.total).toBe(DISCOVERED.length); + expect(result.found).toEqual([DISCOVERED[0]]); + expect(result.missing).toEqual([DISCOVERED[1], DISCOVERED[2]]); }); - it('returns ok:false and lists only missing tables when partial', async () => { - const partial = [{ TABLE_NAME: DRAFT_TABLES[0].toUpperCase() }]; - const result = await checkDraftCascadeConstraints(async () => partial); + it('ok:false with all missing when no constraints exist', async () => { + const result = await checkDraftCascadeConstraints(makeDbRun(DISCOVERED, [])); expect(result.ok).toBe(false); - expect(result.found).toHaveLength(1); - expect(result.found[0]).toBe(DRAFT_TABLES[0]); - expect(result.missing).toHaveLength(DRAFT_TABLES.length - 1); - expect(result.missing).not.toContain(DRAFT_TABLES[0]); + expect(result.missing).toHaveLength(DISCOVERED.length); + expect(result.found).toHaveLength(0); }); - it('handles case-insensitive TABLE_NAME matching', async () => { - // HANA returns uppercase; we store lowercase — ensure the comparison works - const rows = DRAFT_TABLES.map(t => ({ TABLE_NAME: t.toUpperCase() })); - const result = await checkDraftCascadeConstraints(async () => rows); + it('total:0 and ok:true when no draft tables are discovered', async () => { + // Nothing to validate -> ok (the served handler warns on total===0 separately) + const result = await checkDraftCascadeConstraints(makeDbRun([], [])); + expect(result.total).toBe(0); expect(result.ok).toBe(true); + expect(result.missing).toHaveLength(0); }); - it('returns ok:false with error property when dbRun throws (fail-open shape)', async () => { - const result = await checkDraftCascadeConstraints(async () => { throw new Error('HANA gone'); }); + it('matches case-insensitively (HANA uppercase vs mixed)', async () => { + const run = async (sql) => { + if (/TABLE_COLUMNS/.test(sql)) return [{ TABLE_NAME: 'adminservice_missions_drafts' }]; + if (/REFERENTIAL_CONSTRAINTS/.test(sql)) return [{ TABLE_NAME: 'ADMINSERVICE_MISSIONS_DRAFTS' }]; + return []; + }; + const result = await checkDraftCascadeConstraints(run); + expect(result.ok).toBe(true); + expect(result.found).toEqual(['ADMINSERVICE_MISSIONS_DRAFTS']); + }); + + it('ok:false with error property when dbRun throws (fail-open shape)', async () => { + const result = await checkDraftCascadeConstraints(async () => { + throw new Error('HANA gone'); + }); expect(result.ok).toBe(false); expect(result.error).toBe('HANA gone'); expect(result.found).toHaveLength(0); - expect(result.missing).toHaveLength(DRAFT_TABLES.length); + expect(result.total).toBe(0); }); - it('treats null/undefined rows result as empty (no crash)', async () => { + it('treats null rows result as empty (no crash)', async () => { const result = await checkDraftCascadeConstraints(async () => null); - expect(result.ok).toBe(false); - expect(result.found).toHaveLength(0); + // discovery returned null -> no tables -> total 0, ok true, no throw + expect(result.total).toBe(0); + expect(result.ok).toBe(true); }); }); @@ -103,9 +125,14 @@ describe('wireDraftIntegrityPreflight sentinel', () => { const makeFakeCds = () => ({ db: { options: { kind: 'hana' }, - run: async () => { dbRunCalls++; return []; }, + run: async () => { + dbRunCalls++; + return []; + }, + }, + on: (event, fn) => { + if (event === 'served') void fn(); }, - on: (event, fn) => { if (event === 'served') void fn(); }, log: () => ({ warn: () => {}, info: () => {} }), }); @@ -121,16 +148,16 @@ describe('wireDraftIntegrityPreflight sentinel', () => { const fakeCds = makeFakeCds(); wireDraftIntegrityPreflight(fakeCds); wireDraftIntegrityPreflight(fakeCds); - // One microtask tick to let async handler settle - await new Promise(r => setTimeout(r, 0)); - expect(dbRunCalls).toBeLessThanOrEqual(1); + await new Promise((r) => setTimeout(r, 0)); + // One served handler -> at most the two discovery+constraint queries for ONE run + expect(dbRunCalls).toBeLessThanOrEqual(2); }); it('skips the DB query entirely when kind is not hana', async () => { const fakeCds = makeFakeCds(); fakeCds.db.options.kind = 'sqlite'; wireDraftIntegrityPreflight(fakeCds); - await new Promise(r => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); expect(dbRunCalls).toBe(0); }); });