Stable backport of #1043 (main: 8809564a).
sqlite → pg restore is allowed for anyone holding backup.restore, from the
upload UI and the CLI alike, gated by the manifest-direction rule in
validateManifest. pg → sqlite stays refused, with an error naming the
supported direction. allowEngineSwitch is removed rather than kept alongside:
one gate, no way to drive the refused direction.
Two resolutions were needed against stable rather than a clean cherry-pick,
both from known main/stable divergences:
- replaceAllTables has no roleSnapshot parameter on this branch, so the call
keeps stable's 4-arg signature while taking the derived { crossEngine }.
- resyncSequences was guarded by `if (allowEngineSwitch)`, which this change
removes — leaving an undefined reference. It now runs unconditionally,
matching main. That also closes a stable-only gap: a same-engine pg → pg
restore previously left identity sequences stale, so the next natural
insert collided on the primary key.
Also exports resyncSequences (the function already existed here, main already
exports it) so the cross-engine suite can drive the post-restore fixup.
Verified on this branch: all four picpeak suites green on SQLite, and 20/20
against a real Postgres 15 with the PICPEAK_PG_TEST_URL-gated cases executing.
Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
co-authored by
Paul Nothaft
parent
0a999795cc
commit
980378a17b
@@ -240,6 +240,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) {
|
||||
|
||||
@@ -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;
|
||||
@@ -44,7 +45,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).'];
|
||||
@@ -53,14 +54,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;
|
||||
@@ -361,11 +364,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;
|
||||
@@ -373,6 +376,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;
|
||||
@@ -404,21 +417,22 @@ 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, { crossEngine: allowEngineSwitch });
|
||||
await replaceAllTables(tables, dataDir, currentAdmin, { crossEngine });
|
||||
|
||||
// Cross-engine only (#1038): rows are inserted with explicit ids, which
|
||||
// leaves Postgres identity sequences at 1 and makes the next natural insert
|
||||
// collide on the primary key. Same-engine restores keep today's behaviour
|
||||
// untouched — this branch exists for scripts/migrate-sqlite-to-postgres.js.
|
||||
if (allowEngineSwitch) await resyncSequences(tables);
|
||||
// Post-commit fixup: rows are inserted with explicit ids, which leaves
|
||||
// Postgres identity sequences behind, so the next natural insert collides
|
||||
// on the primary key. Runs unconditionally, matching main — the guard used
|
||||
// to be `if (allowEngineSwitch)`, which this change removes, and which also
|
||||
// left a same-engine pg → pg restore with stale sequences.
|
||||
await resyncSequences(tables);
|
||||
|
||||
const filesRestored = await restoreFiles(staging);
|
||||
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(() => {});
|
||||
}
|
||||
@@ -431,5 +445,8 @@ module.exports = {
|
||||
// exported for testing — the cross-engine coercion (#1038)
|
||||
epochToIso,
|
||||
coerceForTargetEngine,
|
||||
typedColumnsFor,
|
||||
reinjectCurrentAdmin,
|
||||
// The cross-engine suite drives the post-restore sequence fixup directly.
|
||||
resyncSequences,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user