diff --git a/backend/__tests__/integration/picpeakCrossEngine.test.js b/backend/__tests__/integration/picpeakCrossEngine.test.js new file mode 100644 index 00000000..ee8f34e7 --- /dev/null +++ b/backend/__tests__/integration/picpeakCrossEngine.test.js @@ -0,0 +1,197 @@ +'use strict'; + +/** + * Cross-engine .picpeak restore policy (#1041): a SQLite archive restored onto + * a PostgreSQL instance — the official small-install → full-stack upgrade + * path — now allowed by validateManifest's direction rule instead of the + * former CLI-only allowEngineSwitch flag. The coercion engine itself + * (typedColumnsFor / epochToIso / coerceForTargetEngine) landed with #1039; + * these tests pin the direction policy and the coercion's cross-engine + * value-correctness. + * + * Ungated: validateManifest direction rules and the pure coercion units. + * The reverse direction (pg backup onto a sqlite instance) staying blocked is + * pinned by picpeakRoundtrip.test.js, which runs on the real sqlite harness. + * + * Gated on PICPEAK_PG_TEST_URL (same contract as picpeakRestorePg.test.js): + * sqlite-shaped NDJSON rows land in real Postgres with correct stored VALUES, + * not just row counts, e.g. + * PICPEAK_PG_TEST_URL="postgres://picpeak:pw@127.0.0.1:7102/picpeak_xengine_test" \ + * npx jest __tests__/integration/picpeakCrossEngine.test.js + */ +const knexLib = require('knex'); + +describe('validateManifest cross-engine direction (pg target)', () => { + let validateManifest; + + beforeAll(() => { + jest.resetModules(); + jest.doMock('../../knexfile', () => ({ client: 'pg' })); + // validateManifest wraps its knex_migrations lookup in try/catch — a + // throwing stub simply skips the forward-only check, which is not under + // test here. + jest.doMock('../../src/database/db', () => ({ db: () => { throw new Error('stub'); } })); + ({ validateManifest } = require('../../src/services/picpeakImportService')); + }); + + afterAll(() => { + jest.dontMock('../../src/database/db'); + jest.dontMock('../../knexfile'); + jest.resetModules(); + }); + + it('allows a sqlite backup onto a pg instance (upgrade direction)', async () => { + const blockers = await validateManifest({ + kind: 'picpeak-backup', format: 1, database: { engine: 'sqlite' }, tables: {}, + }); + expect(blockers.filter((b) => /engine/i.test(b))).toHaveLength(0); + }); + + it('still allows same-engine pg → pg', async () => { + const blockers = await validateManifest({ + kind: 'picpeak-backup', format: 1, database: { engine: 'pg' }, tables: {}, + }); + expect(blockers.filter((b) => /engine/i.test(b))).toHaveLength(0); + }); +}); + +describe('epochToIso (landed with #1039)', () => { + let epochToIso; + + beforeAll(() => { + jest.resetModules(); + ({ epochToIso } = require('../../src/services/picpeakImportService')); + }); + + it('converts epoch milliseconds', () => { + expect(epochToIso(1723400000000)).toBe('2024-08-11T18:13:20.000Z'); + }); + + it('converts epoch SECONDS to the same instant, not January 1970', () => { + expect(epochToIso(1723400000)).toBe('2024-08-11T18:13:20.000Z'); + }); + + it('converts numeric strings', () => { + expect(epochToIso('1723400000000')).toBe('2024-08-11T18:13:20.000Z'); + }); + + it('passes non-numeric values through untouched', () => { + expect(epochToIso('2026-08-12 10:00:00')).toBe('2026-08-12 10:00:00'); + }); +}); + +describe('coerceForTargetEngine on sqlite-shaped rows', () => { + let coerceForTargetEngine; + + beforeAll(() => { + jest.resetModules(); + ({ coerceForTargetEngine } = require('../../src/services/picpeakImportService')); + }); + + const types = { timestamps: ['created_at', 'expires_at'], booleans: ['is_active'] }; + + it('coerces 0/1 booleans and epoch timestamps, leaves date strings alone', () => { + const [row] = coerceForTargetEngine( + [{ id: 1, is_active: 1, created_at: 1723400000000, expires_at: '2026-09-01 12:00:00' }], + types + ); + expect(row.is_active).toBe(true); + expect(row.created_at).toBe('2024-08-11T18:13:20.000Z'); + expect(row.expires_at).toBe('2026-09-01 12:00:00'); // pg parses this natively + }); + + it('coerces falsy variants and passes null/empty through', () => { + const [row] = coerceForTargetEngine( + [{ is_active: 0, created_at: null, expires_at: '' }], + types + ); + expect(row.is_active).toBe(false); + expect(row.created_at).toBeNull(); + expect(row.expires_at).toBe(''); + }); +}); + +// ── Real-Postgres integration (gated) ──────────────────────────────────────── +const PG_URL = process.env.PICPEAK_PG_TEST_URL; +const maybe = PG_URL ? describe : describe.skip; + +maybe('sqlite-shaped rows land correctly in real Postgres', () => { + let pgDb; + let svc; + + beforeAll(async () => { + pgDb = knexLib({ client: 'pg', connection: PG_URL }); + await pgDb.raw('DROP TABLE IF EXISTS xengine_events, xengine_settings CASCADE'); + await pgDb.schema.createTable('xengine_events', (t) => { + t.increments('id'); + t.string('slug'); + t.boolean('is_active').defaultTo(true); + t.boolean('allow_downloads').defaultTo(true); + t.timestamp('created_at'); + t.timestamp('expires_at'); + }); + await pgDb.schema.createTable('xengine_settings', (t) => { + t.increments('id'); + t.string('setting_key').notNullable().unique(); + t.jsonb('setting_value'); + }); + + jest.resetModules(); + jest.doMock('../../knexfile', () => ({ client: 'pg' })); + jest.doMock('../../src/database/db', () => ({ db: pgDb })); + svc = require('../../src/services/picpeakImportService'); + }); + + afterAll(async () => { + jest.dontMock('../../src/database/db'); + jest.dontMock('../../knexfile'); + if (pgDb) { + await pgDb.raw('DROP TABLE IF EXISTS xengine_events, xengine_settings CASCADE'); + await pgDb.destroy(); + } + }); + + it('typedColumnsFor classifies boolean and timestamp columns via columnInfo()', async () => { + const types = await svc.typedColumnsFor(pgDb, 'xengine_events'); + expect(types.booleans.sort()).toEqual(['allow_downloads', 'is_active']); + expect(types.timestamps.sort()).toEqual(['created_at', 'expires_at']); + }); + + it('inserts a sqlite archive row (0/1 booleans, epoch dates, json text) with correct stored values', async () => { + // Exactly what a sqlite-created .picpeak carries: integers for booleans, + // epoch numbers for #485-shape timestamps (ms here, seconds covered by the + // epochToIso unit), a "YYYY-MM-DD HH:MM:SS" string for clean ones, and + // json columns as TEXT (the crossEngine path skips serialiseJsonColumns — + // the text is already what pg wants). + const epoch = 1723400000000; + const eventRows = [ + { id: 1, slug: 'wedding', is_active: 1, allow_downloads: 0, created_at: epoch, expires_at: '2026-09-01 12:00:00' }, + ]; + const settingRows = [{ id: 1, setting_key: 'brand', setting_value: '{"name":"PicPeak","dark":true}' }]; + + await pgDb.transaction(async (trx) => { + const evTypes = await svc.typedColumnsFor(trx, 'xengine_events'); + await trx.batchInsert('xengine_events', svc.coerceForTargetEngine(eventRows, evTypes), 100); + const stTypes = await svc.typedColumnsFor(trx, 'xengine_settings'); + await trx.batchInsert('xengine_settings', svc.coerceForTargetEngine(settingRows, stTypes), 100); + }); + + const ev = await pgDb('xengine_events').where({ id: 1 }).first(); + expect(ev.is_active).toBe(true); // 1 → true, not backwards (#1028 class) + expect(ev.allow_downloads).toBe(false); // 0 → false + expect(new Date(ev.created_at).getTime()).toBe(epoch); + expect(new Date(ev.expires_at).toISOString().slice(0, 10)).toBe('2026-09-01'); + + const st = await pgDb('xengine_settings').where({ id: 1 }).first(); + // jsonb parsed back by the driver — value intact, no double encoding. + expect(st.setting_value).toEqual({ name: 'PicPeak', dark: true }); + }); + + it('id sequence works after explicit-id insert + resync (next natural insert)', async () => { + await svc.resyncSequences(['xengine_events']); + const [next] = await pgDb('xengine_events') + .insert({ slug: 'fresh', is_active: true }) + .returning('id'); + expect(Number(next.id || next)).toBe(2); + }); +}); diff --git a/backend/scripts/migrate-sqlite-to-postgres.js b/backend/scripts/migrate-sqlite-to-postgres.js index 765ab06d..b0a70ff7 100644 --- a/backend/scripts/migrate-sqlite-to-postgres.js +++ b/backend/scripts/migrate-sqlite-to-postgres.js @@ -201,9 +201,9 @@ async function phaseImport(archivePath) { const { importFromPicpeak } = require('../src/services/picpeakImportService'); // No currentAdminId: this is a CLI, there is no operator session to preserve. // The SQLite install's own admin accounts come across with everything else. - // allowEngineSwitch: moving between engines is the whole point here. The - // upload/restore UI keeps refusing it. - const summary = await importFromPicpeak({ picpeakPath: archivePath, allowEngineSwitch: true }); + // sqlite → pg is allowed by validateManifest's direction policy (#1041) — + // the same gate the upload/restore UI uses, no separate opt-in flag. + const summary = await importFromPicpeak({ picpeakPath: archivePath }); return JSON.stringify(summary || {}); } diff --git a/backend/src/routes/adminBackup.js b/backend/src/routes/adminBackup.js index 1dedbe4e..2954f8e8 100644 --- a/backend/src/routes/adminBackup.js +++ b/backend/src/routes/adminBackup.js @@ -248,6 +248,7 @@ router.post('/picpeak/import', adminAuth, requirePermission('backup.restore'), p tables: result.tables, filesRestored: result.filesRestored, usesExternalMedia: result.usesExternalMedia, + crossEngine: result.crossEngine, sessionInvalidated: true, }); } catch (error) { diff --git a/backend/src/services/picpeakImportService.js b/backend/src/services/picpeakImportService.js index 5d050583..dec5cd57 100644 --- a/backend/src/services/picpeakImportService.js +++ b/backend/src/services/picpeakImportService.js @@ -9,9 +9,10 @@ // email collides with the current account is overwritten with the current // account's credentials (so the operator's known password keeps working). // -// Same-engine only (pg↔pg / sqlite↔sqlite) and forward-only (an older backup -// restores onto a newer instance; a newer backup is refused). The target's own -// schema is used as-is — we never replay the backup's DDL. +// Same-engine (pg↔pg / sqlite↔sqlite) or the upgrade direction (sqlite → pg, +// #1041) — the reverse is refused. Forward-only (an older backup restores onto +// a newer instance; a newer backup is refused). The target's own schema is +// used as-is — we never replay the backup's DDL. const fs = require('fs'); const fsp = require('fs').promises; @@ -45,7 +46,7 @@ async function readManifestFromZip(picpeakPath) { } // Returns an array of human-readable blockers ([] = OK to restore). -async function validateManifest(manifest, { allowEngineSwitch = false } = {}) { +async function validateManifest(manifest) { const errors = []; if (!manifest || manifest.kind !== 'picpeak-backup') { return ['This file is not a PicPeak backup (.picpeak).']; @@ -54,14 +55,16 @@ async function validateManifest(manifest, { allowEngineSwitch = false } = {}) { errors.push('This backup was created by a newer version of PicPeak. Update this instance first.'); } const engine = isPostgres() ? 'pg' : 'sqlite'; - // Cross-engine loads are opt-in and CLI-only (#1038). The archive format is - // engine-neutral NDJSON, but this path had never been exercised, so the - // upload/restore surface keeps refusing it — only - // scripts/migrate-sqlite-to-postgres.js, which exists to move an install - // between engines, passes allowEngineSwitch. - if (!allowEngineSwitch - && manifest.database && manifest.database.engine && manifest.database.engine !== engine) { - errors.push(`Database engine mismatch: the backup is "${manifest.database.engine}" but this instance is "${engine}". Restore is only supported between matching engines.`); + const backupEngine = manifest.database && manifest.database.engine; + // Cross-engine restore is allowed in the UPGRADE direction only: a SQLite + // archive onto a Postgres instance (#1041) — the official small-install → + // full-stack migration path, same gate for the upload UI and + // scripts/migrate-sqlite-to-postgres.js. The reverse stays refused: pg + // archives carry ISO "T"/"Z" timestamps that SQLite would store as-is in + // text columns (the #1028/#1029 drift class), and engine downgrades are + // rarely intentional. + if (backupEngine && backupEngine !== engine && !(backupEngine === 'sqlite' && engine === 'pg')) { + errors.push(`Database engine mismatch: the backup is "${backupEngine}" but this instance is "${engine}". Cross-engine restore is only supported from a SQLite backup onto a PostgreSQL instance.`); } // Forward-only: the target schema must be at least as new as the backup's. let targetLatest = null; @@ -424,11 +427,11 @@ async function detectExternalMedia() { * @param {Object} opts * @param {string} opts.picpeakPath path to the uploaded/staged .picpeak * @param {number} [opts.currentAdminId] admin to preserve across the wipe - * @returns {Promise<{restored:boolean, tables:number, filesRestored:number, usesExternalMedia:boolean, manifest:object}>} + * @returns {Promise<{restored:boolean, tables:number, filesRestored:number, usesExternalMedia:boolean, crossEngine:boolean, manifest:object}>} */ -async function importFromPicpeak({ picpeakPath, currentAdminId, allowEngineSwitch = false }) { +async function importFromPicpeak({ picpeakPath, currentAdminId }) { const manifest = await readManifestFromZip(picpeakPath); - const blockers = await validateManifest(manifest, { allowEngineSwitch }); + const blockers = await validateManifest(manifest); if (blockers.length) { const err = new Error(blockers[0]); err.statusCode = 400; @@ -436,6 +439,16 @@ async function importFromPicpeak({ picpeakPath, currentAdminId, allowEngineSwitc throw err; } + // Archives predating the manifest engine field get the target's engine — + // i.e. the exact same-engine behavior. After validateManifest, a mismatch + // can only be sqlite → pg. + const targetEngine = isPostgres() ? 'pg' : 'sqlite'; + const sourceEngine = (manifest.database && manifest.database.engine) || targetEngine; + const crossEngine = sourceEngine !== targetEngine; + if (crossEngine) { + logger.info(`[picpeak-import] cross-engine restore: ${sourceEngine} backup onto ${targetEngine} instance`); + } + const currentAdmin = currentAdminId ? await db('admin_users').where({ id: currentAdminId }).first() : null; @@ -470,7 +483,7 @@ async function importFromPicpeak({ picpeakPath, currentAdminId, allowEngineSwitc logger.warn(`[picpeak-import] ignoring ${skipped.length} backup table(s) not present in this DB (or protected): ${skipped.join(', ')}`); } - await replaceAllTables(tables, dataDir, currentAdmin, roleSnapshot, { crossEngine: allowEngineSwitch }); + await replaceAllTables(tables, dataDir, currentAdmin, roleSnapshot, { crossEngine }); // Post-commit fixups (must NOT run inside the restore transaction): // - resync Postgres identity sequences left behind by the explicit-id @@ -484,9 +497,9 @@ async function importFromPicpeak({ picpeakPath, currentAdminId, allowEngineSwitc const usesExternalMedia = await detectExternalMedia(); logger.info( - `[picpeak-import] restored ${tables.length} tables, ${filesRestored} files (externalMedia=${usesExternalMedia})` + `[picpeak-import] restored ${tables.length} tables, ${filesRestored} files (externalMedia=${usesExternalMedia}, crossEngine=${crossEngine})` ); - return { restored: true, tables: tables.length, filesRestored, usesExternalMedia, manifest }; + return { restored: true, tables: tables.length, filesRestored, usesExternalMedia, crossEngine, manifest }; } finally { await fsp.rm(staging, { recursive: true, force: true }).catch(() => {}); } @@ -499,6 +512,7 @@ module.exports = { // exported for testing — the cross-engine coercion (#1038) epochToIso, coerceForTargetEngine, + typedColumnsFor, reinjectCurrentAdmin, captureOperatorRole, preserveOperatorRole, diff --git a/frontend/src/components/admin/PicpeakBackupCard.tsx b/frontend/src/components/admin/PicpeakBackupCard.tsx index 1c9f947f..d6bdc6a6 100644 --- a/frontend/src/components/admin/PicpeakBackupCard.tsx +++ b/frontend/src/components/admin/PicpeakBackupCard.tsx @@ -16,6 +16,7 @@ interface RestoreResult { tables: number; filesRestored: number; usesExternalMedia: boolean; + crossEngine?: boolean; sessionInvalidated?: boolean; } @@ -137,7 +138,7 @@ export const PicpeakRestoreCard: React.FC = () => { {t('backup.picpeak.restoreTitle', 'Restore from a .picpeak')}
- {t('backup.picpeak.restoreIntro', 'Upload a .picpeak taken from this or another instance. Same database engine only.')} + {t('backup.picpeak.restoreIntro', 'Upload a .picpeak taken from this or another instance. Restoring a SQLite backup onto a PostgreSQL instance is supported (the upgrade path); other engine combinations must match.')}