mirror of
https://github.com/actualbudget/actual.git
synced 2026-07-10 12:31:32 -05:00
[AI] Enforce additive-only database migrations
Migrations that remove or change existing schema break older clients that sync against the same budget file (invalid-schema sync errors and the out-of-sync-migrations fatal error). This adds layered enforcement: - A schema guard test in loot-core that runs every migration on a fresh database and diffs the resulting schema (tables, columns, indexes) against a committed golden snapshot. Removals/changes always fail; additions ask for the snapshot to be regenerated with `yarn generate:schema-snapshot`, making schema changes reviewable. - An escape hatch for maintainer-approved breaking changes (SCHEMA_SNAPSHOT_ALLOW_BREAKING=true) that records removed names in retired-names.json; a companion test forbids ever reusing those names, since historical CRDT messages would repopulate them with stale data. - Extended CI migration checks: shipped migration files can no longer be edited or deleted (diffed against the merge base by blob hash), and new migrations containing DROP/RENAME statements get an advisory PR annotation. The existing timestamp-ordering check is preserved. - A contributing docs section describing the policy, the snapshot workflow, and the breaking-change ceremony.
This commit is contained in:
3
.github/workflows/check.yml
vendored
3
.github/workflows/check.yml
vendored
@@ -139,6 +139,9 @@ jobs:
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
persist-credentials: false
|
||||
# Full history so the migration check can diff against the merge
|
||||
# base with master
|
||||
fetch-depth: 0
|
||||
- name: Set up environment
|
||||
uses: ./.github/actions/setup
|
||||
with:
|
||||
|
||||
@@ -45,6 +45,7 @@
|
||||
"deploy:docs": "yarn workspace docs deploy",
|
||||
"generate:i18n": "yarn workspace @actual-app/web generate:i18n",
|
||||
"generate:release-notes": "node ./bin/release-note-generator.mts",
|
||||
"generate:schema-snapshot": "yarn workspace @actual-app/core run generate:schema-snapshot",
|
||||
"test": "lage test --continue",
|
||||
"test:debug": "lage test --no-cache --continue",
|
||||
"e2e": "yarn workspace @actual-app/web run e2e",
|
||||
|
||||
@@ -1,11 +1,34 @@
|
||||
// overview:
|
||||
// 1. Identify the migrations in packages/loot-core/migrations/* on `master` and HEAD
|
||||
// 2. Make sure that any new migrations on HEAD are dated after the latest migration on `master`.
|
||||
// 1. Identify the migrations in packages/loot-core/migrations/* on `master`,
|
||||
// on the merge base, and on HEAD.
|
||||
// 2. Make sure that any new migrations on HEAD are dated after the latest
|
||||
// migration on `master` (older dates trigger `out-of-sync-migrations` for
|
||||
// users who already applied the newer one).
|
||||
// 3. Make sure no migration that exists on the merge base was edited or
|
||||
// deleted (shipped migrations are append-only: existing installs never
|
||||
// re-run them, so edits fork the schema across the user base).
|
||||
// 4. Emit advisory warnings when a new migration contains statements that
|
||||
// look like they remove or rename schema. The authoritative gate for this
|
||||
// is the schema guard test in loot-core.
|
||||
//
|
||||
// See https://actualbudget.org/docs/contributing/project-details/migrations
|
||||
|
||||
import { spawnSync } from 'child_process';
|
||||
import { readFileSync } from 'fs';
|
||||
import path from 'path';
|
||||
import { fileURLToPath } from 'url';
|
||||
|
||||
import {
|
||||
findAddedMigrations,
|
||||
findMisdatedMigrations,
|
||||
findMutatedMigrations,
|
||||
findRiskyStatements,
|
||||
parseMigrationTree,
|
||||
} from '../src/migrations/check';
|
||||
|
||||
const POLICY_URL =
|
||||
'https://actualbudget.org/docs/contributing/project-details/migrations';
|
||||
|
||||
const migrationsDir = path.join(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
'..',
|
||||
@@ -16,45 +39,91 @@ const migrationsDir = path.join(
|
||||
'migrations',
|
||||
);
|
||||
|
||||
function git(args: string[]): string {
|
||||
const { stdout } = spawnSync('git', args);
|
||||
return stdout.toString();
|
||||
}
|
||||
|
||||
function readMigrations(ref: string) {
|
||||
const { stdout } = spawnSync('git', [
|
||||
'ls-tree',
|
||||
'--name-only',
|
||||
ref,
|
||||
migrationsDir + '/',
|
||||
]);
|
||||
const files = stdout.toString().split('\n').filter(Boolean);
|
||||
console.log(`Found ${files.length} migrations on ${ref}.`);
|
||||
return files
|
||||
.map(file => path.basename(file))
|
||||
.filter(file => !file.startsWith('.'))
|
||||
.map(name => ({
|
||||
date: parseInt(name.split('_')[0]),
|
||||
name: name.match(/^\d+_(.+?)(\.sql)?$/)?.[1] ?? '***' + name,
|
||||
}));
|
||||
const migrations = parseMigrationTree(
|
||||
git(['ls-tree', ref, migrationsDir + '/']),
|
||||
);
|
||||
console.log(`Found ${migrations.length} migrations on ${ref}.`);
|
||||
return migrations;
|
||||
}
|
||||
|
||||
spawnSync('git', ['fetch', 'origin', 'master']);
|
||||
|
||||
const mergeBase =
|
||||
git(['merge-base', 'origin/master', 'HEAD']).trim() || 'origin/master';
|
||||
|
||||
const masterMigrations = readMigrations('origin/master');
|
||||
const mergeBaseMigrations = readMigrations(mergeBase);
|
||||
const headMigrations = readMigrations('HEAD');
|
||||
|
||||
const problems: string[] = [];
|
||||
|
||||
// 1. New migrations must be dated after the latest migration on master.
|
||||
const latestMasterMigration =
|
||||
masterMigrations[masterMigrations.length - 1].date;
|
||||
const newMigrations = headMigrations.filter(
|
||||
migration => !masterMigrations.find(m => m.name === migration.name),
|
||||
);
|
||||
const badMigrations = newMigrations.filter(
|
||||
migration => migration.date <= latestMasterMigration,
|
||||
masterMigrations[masterMigrations.length - 1]?.id ?? 0;
|
||||
const newMigrations = findAddedMigrations(masterMigrations, headMigrations);
|
||||
const misdated = findMisdatedMigrations(newMigrations, latestMasterMigration);
|
||||
|
||||
for (const migration of misdated) {
|
||||
problems.push(
|
||||
`Migration ${migration.name} is dated before the latest migration on ` +
|
||||
`master. Rename it with a newer timestamp, otherwise users who ` +
|
||||
`already applied the newer migration will fail to load their budget.`,
|
||||
);
|
||||
}
|
||||
|
||||
// 2. Migrations that exist on the merge base must not be edited or deleted.
|
||||
const { modified, deleted } = findMutatedMigrations(
|
||||
mergeBaseMigrations,
|
||||
headMigrations,
|
||||
);
|
||||
|
||||
if (badMigrations.length) {
|
||||
console.error(
|
||||
`The following migrations are dated before the latest migration on master:`,
|
||||
for (const name of modified) {
|
||||
problems.push(
|
||||
`Migration ${name} was modified. Shipped migrations are append-only: ` +
|
||||
`existing installs will never re-run it, so editing it forks the ` +
|
||||
`database schema across the user base. Add a new migration instead.`,
|
||||
);
|
||||
badMigrations.forEach(migration => {
|
||||
console.error(` ${migration.name}`);
|
||||
}
|
||||
for (const name of deleted) {
|
||||
problems.push(
|
||||
`Migration ${name} was deleted. Shipped migrations are append-only: ` +
|
||||
`existing installs already ran it, so deleting it breaks the ` +
|
||||
`migration check for every existing budget file.`,
|
||||
);
|
||||
}
|
||||
|
||||
// 3. Advisory: warn on new migrations that look like they remove or rename
|
||||
// schema. Not fatal here — the schema guard test in loot-core is the
|
||||
// authoritative check.
|
||||
for (const migration of newMigrations) {
|
||||
let source = '';
|
||||
try {
|
||||
source = readFileSync(path.join(migrationsDir, migration.name), 'utf8');
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
for (const risk of findRiskyStatements(source)) {
|
||||
console.log(
|
||||
`::warning file=packages/loot-core/migrations/${migration.name},` +
|
||||
`title=Possibly breaking migration::This migration ${risk}. ` +
|
||||
`Migrations must be additive-only; removing or renaming schema ` +
|
||||
`breaks older clients syncing the same budget file. See ${POLICY_URL}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (problems.length) {
|
||||
console.error(`Migration policy violations found (see ${POLICY_URL}):`);
|
||||
problems.forEach(problem => {
|
||||
console.error(` - ${problem}`);
|
||||
});
|
||||
process.exit(1);
|
||||
} else {
|
||||
console.log(`All migrations are dated after the latest migration on master.`);
|
||||
console.log('All migration checks passed.');
|
||||
}
|
||||
|
||||
99
packages/ci-actions/src/migrations/check.test.ts
Normal file
99
packages/ci-actions/src/migrations/check.test.ts
Normal file
@@ -0,0 +1,99 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
findAddedMigrations,
|
||||
findMisdatedMigrations,
|
||||
findMutatedMigrations,
|
||||
findRiskyStatements,
|
||||
parseMigrationTree,
|
||||
} from './check';
|
||||
|
||||
const treeOutput = [
|
||||
'100644 blob aaa111\tpackages/loot-core/migrations/1000_first.sql',
|
||||
'100644 blob bbb222\tpackages/loot-core/migrations/2000_second.js',
|
||||
'100644 blob ccc333\tpackages/loot-core/migrations/.hidden',
|
||||
].join('\n');
|
||||
|
||||
describe('parseMigrationTree', () => {
|
||||
it('parses names, ids, and hashes, skipping hidden files', () => {
|
||||
expect(parseMigrationTree(treeOutput)).toEqual([
|
||||
{ name: '1000_first.sql', id: 1000, hash: 'aaa111' },
|
||||
{ name: '2000_second.js', id: 2000, hash: 'bbb222' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns an empty list for empty output', () => {
|
||||
expect(parseMigrationTree('')).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('findAddedMigrations', () => {
|
||||
it('returns migrations present on head but not on base', () => {
|
||||
const base = parseMigrationTree(treeOutput);
|
||||
const head = [
|
||||
...base,
|
||||
{ name: '3000_third.sql', id: 3000, hash: 'ddd444' },
|
||||
];
|
||||
expect(findAddedMigrations(base, head)).toEqual([
|
||||
{ name: '3000_third.sql', id: 3000, hash: 'ddd444' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('findMutatedMigrations', () => {
|
||||
it('detects modified and deleted migrations', () => {
|
||||
const base = parseMigrationTree(treeOutput);
|
||||
const head = [{ name: '1000_first.sql', id: 1000, hash: 'changed' }];
|
||||
expect(findMutatedMigrations(base, head)).toEqual({
|
||||
modified: ['1000_first.sql'],
|
||||
deleted: ['2000_second.js'],
|
||||
});
|
||||
});
|
||||
|
||||
it('reports nothing when head matches base', () => {
|
||||
const base = parseMigrationTree(treeOutput);
|
||||
expect(findMutatedMigrations(base, base)).toEqual({
|
||||
modified: [],
|
||||
deleted: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('findMisdatedMigrations', () => {
|
||||
it('flags migrations dated at or before the latest existing one', () => {
|
||||
const added = [
|
||||
{ name: '1500_late.sql', id: 1500, hash: 'a' },
|
||||
{ name: '2000_same.sql', id: 2000, hash: 'b' },
|
||||
{ name: '2500_ok.sql', id: 2500, hash: 'c' },
|
||||
];
|
||||
expect(findMisdatedMigrations(added, 2000).map(m => m.name)).toEqual([
|
||||
'1500_late.sql',
|
||||
'2000_same.sql',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('findRiskyStatements', () => {
|
||||
it('flags DROP TABLE, DROP COLUMN, and RENAME statements', () => {
|
||||
expect(findRiskyStatements('ALTER TABLE x DROP COLUMN y;')).toEqual([
|
||||
'drops a column',
|
||||
]);
|
||||
expect(findRiskyStatements('DROP TABLE x;')).toEqual(['drops a table']);
|
||||
expect(findRiskyStatements('ALTER TABLE x RENAME TO y;')).toEqual([
|
||||
'renames a table or column',
|
||||
]);
|
||||
expect(findRiskyStatements('ALTER TABLE x RENAME COLUMN a TO b;')).toEqual([
|
||||
'renames a table or column',
|
||||
]);
|
||||
});
|
||||
|
||||
it('ignores additive statements and comments', () => {
|
||||
expect(findRiskyStatements('ALTER TABLE x ADD COLUMN y TEXT;')).toEqual([]);
|
||||
expect(
|
||||
findRiskyStatements('-- do not DROP TABLE here\nCREATE TABLE y (id);'),
|
||||
).toEqual([]);
|
||||
expect(
|
||||
findRiskyStatements('/* DROP COLUMN in a block comment */ SELECT 1;'),
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
88
packages/ci-actions/src/migrations/check.ts
Normal file
88
packages/ci-actions/src/migrations/check.ts
Normal file
@@ -0,0 +1,88 @@
|
||||
// Pure logic for the migration checks that run in CI. Database migrations
|
||||
// are append-only: once a migration has shipped, editing or deleting it
|
||||
// forks the schema across the user base (clients that already ran the old
|
||||
// version will never re-run it), and a migration dated before an already
|
||||
// shipped one triggers the `out-of-sync-migrations` error for users who
|
||||
// applied the newer one first. New migrations that remove or change schema
|
||||
// are caught by the schema guard test in loot-core; this module also flags
|
||||
// suspicious statements early so contributors get a hint in the PR checks.
|
||||
// See https://actualbudget.org/docs/contributing/project-details/migrations
|
||||
|
||||
export type MigrationEntry = {
|
||||
name: string;
|
||||
id: number;
|
||||
hash: string;
|
||||
};
|
||||
|
||||
// Parses `git ls-tree <ref> -- <dir>/` output, where each line looks like
|
||||
// `100644 blob <hash>\t<path>`.
|
||||
export function parseMigrationTree(lsTreeOutput: string): MigrationEntry[] {
|
||||
return lsTreeOutput
|
||||
.split('\n')
|
||||
.filter(Boolean)
|
||||
.map(line => {
|
||||
const [meta, filePath] = line.split('\t');
|
||||
const hash = meta?.split(/\s+/)[2] ?? '';
|
||||
const name = filePath?.split('/').pop() ?? '';
|
||||
return { name, id: Number.parseInt(name.split('_')[0], 10), hash };
|
||||
})
|
||||
.filter(entry => entry.name !== '' && !entry.name.startsWith('.'))
|
||||
.filter(entry => Number.isFinite(entry.id));
|
||||
}
|
||||
|
||||
export function findAddedMigrations(
|
||||
base: MigrationEntry[],
|
||||
head: MigrationEntry[],
|
||||
): MigrationEntry[] {
|
||||
const baseNames = new Set(base.map(migration => migration.name));
|
||||
return head.filter(migration => !baseNames.has(migration.name));
|
||||
}
|
||||
|
||||
export function findMutatedMigrations(
|
||||
base: MigrationEntry[],
|
||||
head: MigrationEntry[],
|
||||
): { modified: string[]; deleted: string[] } {
|
||||
const headByName = new Map(
|
||||
head.map(migration => [migration.name, migration]),
|
||||
);
|
||||
|
||||
const modified: string[] = [];
|
||||
const deleted: string[] = [];
|
||||
for (const migration of base) {
|
||||
const headMigration = headByName.get(migration.name);
|
||||
if (!headMigration) {
|
||||
deleted.push(migration.name);
|
||||
} else if (headMigration.hash !== migration.hash) {
|
||||
modified.push(migration.name);
|
||||
}
|
||||
}
|
||||
return { modified, deleted };
|
||||
}
|
||||
|
||||
export function findMisdatedMigrations(
|
||||
added: MigrationEntry[],
|
||||
latestExistingId: number,
|
||||
): MigrationEntry[] {
|
||||
return added.filter(migration => migration.id <= latestExistingId);
|
||||
}
|
||||
|
||||
const RISKY_PATTERNS = [
|
||||
{ pattern: /\bDROP\s+TABLE\b/i, description: 'drops a table' },
|
||||
{ pattern: /\bDROP\s+COLUMN\b/i, description: 'drops a column' },
|
||||
{
|
||||
pattern: /\bRENAME\s+(COLUMN|TO)\b/i,
|
||||
description: 'renames a table or column',
|
||||
},
|
||||
];
|
||||
|
||||
// Advisory only: the authoritative gate is the schema guard test in
|
||||
// loot-core, which diffs the actual resulting schema. This just surfaces an
|
||||
// early hint on obviously suspicious SQL.
|
||||
export function findRiskyStatements(source: string): string[] {
|
||||
const withoutComments = source
|
||||
.replace(/--[^\n]*/g, '')
|
||||
.replace(/\/\*[\s\S]*?\*\//g, '');
|
||||
return RISKY_PATTERNS.filter(({ pattern }) =>
|
||||
pattern.test(withoutComments),
|
||||
).map(({ description }) => description);
|
||||
}
|
||||
@@ -10,6 +10,57 @@ There are some important considerations to make when adding a feature with a db
|
||||
|
||||
- The naming convention is as follows: `TIMESTAMP_name.sql`. for example. `1694438752000_add_goal_targets.sql`
|
||||
|
||||
- It is strongly discouraged to try to remove columns and tables. This makes reverting changes impossible and introduces unnecessary risk when we can simply stop using them in code.
|
||||
|
||||
- You should be very deliberate with your migration. When adding a feature, try to think about future scenarios and options that may be desired later, so we can minimize the number of migrations.
|
||||
|
||||
## The Additive-Only Policy
|
||||
|
||||
Migrations must only ever **add** schema. A migration may create a new table, add a new column, or create a new index. A migration must never:
|
||||
|
||||
- Drop a table, column, or index.
|
||||
- Rename a table or a column (a rename is a drop plus an add).
|
||||
- Change an existing column's type, `NOT NULL` constraint, or default value.
|
||||
- Edit or delete a migration file that has already shipped in a release.
|
||||
|
||||
If a table or column is no longer needed, simply stop using it in code and leave it in place. An unused column in SQLite costs almost nothing.
|
||||
|
||||
### Why This Policy Exists
|
||||
|
||||
Actual is local-first: every device keeps its own copy of the budget file and applies migrations with whatever app version it is running. Devices on different versions sync against the same budget file at the same time.
|
||||
|
||||
- Sync messages are column-level changes. If one client removes or renames a column that another client still writes to, applying those messages fails with an `invalid-schema` sync error.
|
||||
- When a device downloads the budget file, the applied migrations are compared with the migrations bundled in the app. Removed, edited, or reordered migrations trigger the `out-of-sync-migrations` error, and the budget cannot be opened until the app is updated.
|
||||
- Migration files are append-only for the same reason: a device that already ran a migration will never run it again, so editing a shipped migration silently forks the database schema between existing and new installs.
|
||||
|
||||
## The Schema Snapshot Workflow
|
||||
|
||||
The policy is enforced by a test, not just by review. The file `packages/loot-core/src/server/migrate/schema-snapshot.json` records the full database schema (tables, columns, and indexes) produced by running every migration on a fresh database. A test in `packages/loot-core/src/server/migrate/schema-guard.test.ts` rebuilds that schema and compares it with the snapshot:
|
||||
|
||||
- If your migration **removes or changes** existing schema, the test fails and cannot be silenced. Rework the migration to be additive.
|
||||
- If your migration **adds** schema, the test fails with a reminder to regenerate the snapshot. Run the following command from the repository root and commit the updated `schema-snapshot.json` together with your migration:
|
||||
|
||||
```bash
|
||||
yarn generate:schema-snapshot
|
||||
```
|
||||
|
||||
The snapshot diff makes your schema change easy to see in the pull request.
|
||||
|
||||
In addition, a CI job checks that new migration files are dated after the latest migration on `master`, and that no shipped migration file was edited or deleted.
|
||||
|
||||
## Approved Breaking Changes
|
||||
|
||||
In rare cases the maintainers may approve a genuinely breaking schema change. This is a deliberate ceremony:
|
||||
|
||||
1. Get explicit approval from the maintainers first.
|
||||
2. Regenerate the snapshot with the escape hatch:
|
||||
|
||||
```bash
|
||||
SCHEMA_SNAPSHOT_ALLOW_BREAKING=true yarn generate:schema-snapshot
|
||||
```
|
||||
|
||||
3. The removed names are recorded in `packages/loot-core/src/server/migrate/retired-names.json`.
|
||||
|
||||
Retired names must **never be reused** for new tables or columns. Historical sync messages referencing the old name still exist on other devices and on the sync server, and would repopulate a reintroduced name with stale data. A test enforces this as well.
|
||||
|
||||
:::warning
|
||||
A breaking migration locks out every client that has not yet updated to the app version containing it. Treat it as a last resort.
|
||||
:::
|
||||
|
||||
@@ -184,6 +184,7 @@
|
||||
"watch:node": "cross-env NODE_ENV=development vite build --config ./vite.desktop.config.mts --watch",
|
||||
"test": "npm-run-all -cp 'test:*'",
|
||||
"test:node": "ENV=node vitest --run",
|
||||
"generate:schema-snapshot": "cross-env SCHEMA_SNAPSHOT_UPDATE=true vitest --run src/server/migrate/schema-guard.test.ts && oxfmt src/server/migrate/schema-snapshot.json src/server/migrate/retired-names.json",
|
||||
"test:web": "ENV=web vitest --run -c vitest.web.config.ts",
|
||||
"typecheck": "tsgo -b && tsc-strict",
|
||||
"build": "tsgo -b",
|
||||
|
||||
4
packages/loot-core/src/server/migrate/retired-names.json
Normal file
4
packages/loot-core/src/server/migrate/retired-names.json
Normal file
@@ -0,0 +1,4 @@
|
||||
{
|
||||
"tables": [],
|
||||
"columns": []
|
||||
}
|
||||
160
packages/loot-core/src/server/migrate/schema-guard.test.ts
Normal file
160
packages/loot-core/src/server/migrate/schema-guard.test.ts
Normal file
@@ -0,0 +1,160 @@
|
||||
import * as nativeFs from 'fs';
|
||||
import * as path from 'path';
|
||||
|
||||
import * as db from '#server/db';
|
||||
|
||||
import { migrate, withMigrationsDir } from './migrations';
|
||||
import {
|
||||
computeRetiredNames,
|
||||
diffSchemaSnapshots,
|
||||
findRetiredNameViolations,
|
||||
getSchemaSnapshot,
|
||||
mergeRetiredNames,
|
||||
serializeRetiredNames,
|
||||
serializeSchemaSnapshot,
|
||||
} from './schema-guard';
|
||||
import type { RetiredNames, SchemaSnapshot } from './schema-guard';
|
||||
|
||||
const MIGRATIONS_DIR = path.join(__dirname, '..', '..', '..', 'migrations');
|
||||
const SNAPSHOT_PATH = path.join(__dirname, 'schema-snapshot.json');
|
||||
const RETIRED_NAMES_PATH = path.join(__dirname, 'retired-names.json');
|
||||
|
||||
const POLICY_URL =
|
||||
'https://actualbudget.org/docs/contributing/project-details/migrations';
|
||||
|
||||
const isUpdatingSnapshot = process.env.SCHEMA_SNAPSHOT_UPDATE === 'true';
|
||||
const isBreakingChangeAllowed =
|
||||
process.env.SCHEMA_SNAPSHOT_ALLOW_BREAKING === 'true';
|
||||
|
||||
beforeEach(global.emptyDatabase(true));
|
||||
|
||||
async function buildCurrentSnapshot(): Promise<SchemaSnapshot> {
|
||||
let snapshot: SchemaSnapshot | null = null;
|
||||
await withMigrationsDir(MIGRATIONS_DIR, async () => {
|
||||
const database = db.getDatabase();
|
||||
if (database == null) {
|
||||
throw new Error('Database is not initialized');
|
||||
}
|
||||
await migrate(database);
|
||||
snapshot = getSchemaSnapshot(database);
|
||||
});
|
||||
if (snapshot == null) {
|
||||
throw new Error('Failed to build a schema snapshot');
|
||||
}
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
function readBaselineSnapshot(): SchemaSnapshot | null {
|
||||
if (!nativeFs.existsSync(SNAPSHOT_PATH)) {
|
||||
return null;
|
||||
}
|
||||
return JSON.parse(
|
||||
nativeFs.readFileSync(SNAPSHOT_PATH, 'utf8'),
|
||||
) as SchemaSnapshot;
|
||||
}
|
||||
|
||||
function readRetiredNames(): RetiredNames {
|
||||
if (!nativeFs.existsSync(RETIRED_NAMES_PATH)) {
|
||||
return { tables: [], columns: [] };
|
||||
}
|
||||
return JSON.parse(
|
||||
nativeFs.readFileSync(RETIRED_NAMES_PATH, 'utf8'),
|
||||
) as RetiredNames;
|
||||
}
|
||||
|
||||
function formatList(items: string[]): string {
|
||||
return items.map(item => ` - ${item}`).join('\n');
|
||||
}
|
||||
|
||||
describe('Migration schema guard', () => {
|
||||
test('migrations only make additive schema changes', async () => {
|
||||
const current = await buildCurrentSnapshot();
|
||||
|
||||
if (isUpdatingSnapshot) {
|
||||
const baseline = readBaselineSnapshot();
|
||||
if (baseline) {
|
||||
const diff = diffSchemaSnapshots(baseline, current);
|
||||
if (diff.breakages.length > 0) {
|
||||
if (!isBreakingChangeAllowed) {
|
||||
throw new Error(
|
||||
'Refusing to update the schema snapshot: the current ' +
|
||||
'migrations remove or change existing schema, which breaks ' +
|
||||
'older clients syncing the same budget file:\n' +
|
||||
formatList(diff.breakages) +
|
||||
'\n\nMigrations must be additive-only. See ' +
|
||||
POLICY_URL +
|
||||
'\nIf this breaking change has been explicitly approved by ' +
|
||||
'the maintainers, re-run with ' +
|
||||
'SCHEMA_SNAPSHOT_ALLOW_BREAKING=true to record it.',
|
||||
);
|
||||
}
|
||||
|
||||
const retired = mergeRetiredNames(
|
||||
readRetiredNames(),
|
||||
computeRetiredNames(baseline, current),
|
||||
);
|
||||
nativeFs.writeFileSync(
|
||||
RETIRED_NAMES_PATH,
|
||||
serializeRetiredNames(retired),
|
||||
);
|
||||
console.log(
|
||||
'Recorded retired schema names in retired-names.json:\n' +
|
||||
formatList(diff.breakages),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
nativeFs.writeFileSync(SNAPSHOT_PATH, serializeSchemaSnapshot(current));
|
||||
console.log('Updated schema-snapshot.json');
|
||||
return;
|
||||
}
|
||||
|
||||
const baseline = readBaselineSnapshot();
|
||||
if (baseline == null) {
|
||||
throw new Error(
|
||||
'Missing schema-snapshot.json. Run `yarn generate:schema-snapshot` ' +
|
||||
'from the repository root and commit the result.',
|
||||
);
|
||||
}
|
||||
|
||||
const diff = diffSchemaSnapshots(baseline, current);
|
||||
|
||||
if (diff.breakages.length > 0) {
|
||||
throw new Error(
|
||||
'Migrations must be additive-only, but the following schema was ' +
|
||||
'removed or changed:\n' +
|
||||
formatList(diff.breakages) +
|
||||
'\n\nRemoving or changing existing tables, columns, or indexes ' +
|
||||
'breaks older clients that sync against the same budget file. ' +
|
||||
'See ' +
|
||||
POLICY_URL,
|
||||
);
|
||||
}
|
||||
|
||||
if (diff.additions.length > 0) {
|
||||
throw new Error(
|
||||
'The database schema changed (additively):\n' +
|
||||
formatList(diff.additions) +
|
||||
'\n\nRun `yarn generate:schema-snapshot` from the repository root ' +
|
||||
'and commit the updated schema-snapshot.json.',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('retired schema names are not reused', async () => {
|
||||
const current = await buildCurrentSnapshot();
|
||||
const violations = findRetiredNameViolations(readRetiredNames(), current);
|
||||
|
||||
if (violations.length > 0) {
|
||||
throw new Error(
|
||||
'Migrations reintroduce schema names that were retired by an ' +
|
||||
'earlier breaking change:\n' +
|
||||
formatList(violations) +
|
||||
'\n\nHistorical sync messages referencing these names still ' +
|
||||
'exist and would repopulate them with stale data. Pick a ' +
|
||||
'different name. See ' +
|
||||
POLICY_URL,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
293
packages/loot-core/src/server/migrate/schema-guard.ts
Normal file
293
packages/loot-core/src/server/migrate/schema-guard.ts
Normal file
@@ -0,0 +1,293 @@
|
||||
// The schema guard enforces the additive-only migration policy: once a
|
||||
// table, column, or index has shipped in a release, migrations may add new
|
||||
// schema but must never remove or change what already exists. Removing or
|
||||
// changing schema breaks older clients that sync against the same budget
|
||||
// file. See https://actualbudget.org/docs/contributing/project-details/migrations
|
||||
import type { Database } from '@jlongster/sql.js';
|
||||
|
||||
import * as sqlite from '#platform/server/sqlite';
|
||||
|
||||
export type ColumnSnapshot = {
|
||||
type: string;
|
||||
notNull: boolean;
|
||||
defaultValue: string | null;
|
||||
primaryKey: boolean;
|
||||
};
|
||||
|
||||
export type RelationSnapshot = {
|
||||
columns: Record<string, ColumnSnapshot>;
|
||||
};
|
||||
|
||||
export type IndexSnapshot = {
|
||||
table: string;
|
||||
unique: boolean;
|
||||
partial: boolean;
|
||||
columns: string[];
|
||||
};
|
||||
|
||||
export type SchemaSnapshot = {
|
||||
tables: Record<string, RelationSnapshot>;
|
||||
views: Record<string, RelationSnapshot>;
|
||||
indexes: Record<string, IndexSnapshot>;
|
||||
};
|
||||
|
||||
export type RetiredNames = {
|
||||
tables: string[];
|
||||
columns: string[];
|
||||
};
|
||||
|
||||
export type SchemaDiff = {
|
||||
additions: string[];
|
||||
breakages: string[];
|
||||
};
|
||||
|
||||
type SqliteMasterRow = {
|
||||
name: string;
|
||||
type: 'table' | 'view';
|
||||
};
|
||||
|
||||
type TableInfoRow = {
|
||||
cid: number;
|
||||
name: string;
|
||||
type: string;
|
||||
notnull: number;
|
||||
dflt_value: string | null;
|
||||
pk: number;
|
||||
};
|
||||
|
||||
type IndexListRow = {
|
||||
name: string;
|
||||
unique: number;
|
||||
origin: string;
|
||||
partial: number;
|
||||
};
|
||||
|
||||
type IndexInfoRow = {
|
||||
seqno: number;
|
||||
name: string | null;
|
||||
};
|
||||
|
||||
function getRelationSnapshot(db: Database, name: string): RelationSnapshot {
|
||||
const rows = sqlite.runQuery<TableInfoRow>(
|
||||
db,
|
||||
'SELECT * FROM pragma_table_info(?) ORDER BY cid',
|
||||
[name],
|
||||
true,
|
||||
);
|
||||
|
||||
const columns: Record<string, ColumnSnapshot> = {};
|
||||
for (const row of rows) {
|
||||
columns[row.name] = {
|
||||
type: row.type,
|
||||
notNull: row.notnull !== 0,
|
||||
defaultValue: row.dflt_value,
|
||||
primaryKey: row.pk !== 0,
|
||||
};
|
||||
}
|
||||
return { columns };
|
||||
}
|
||||
|
||||
function getIndexSnapshots(
|
||||
db: Database,
|
||||
table: string,
|
||||
): Record<string, IndexSnapshot> {
|
||||
const rows = sqlite.runQuery<IndexListRow>(
|
||||
db,
|
||||
'SELECT * FROM pragma_index_list(?)',
|
||||
[table],
|
||||
true,
|
||||
);
|
||||
|
||||
const indexes: Record<string, IndexSnapshot> = {};
|
||||
// Only track explicitly created indexes; the ones SQLite creates
|
||||
// internally for PRIMARY KEY/UNIQUE constraints are covered by the
|
||||
// column snapshots of the table itself.
|
||||
const created = rows
|
||||
.filter(row => row.origin === 'c')
|
||||
.sort((i1, i2) => (i1.name < i2.name ? -1 : i1.name > i2.name ? 1 : 0));
|
||||
|
||||
for (const row of created) {
|
||||
const columnRows = sqlite.runQuery<IndexInfoRow>(
|
||||
db,
|
||||
'SELECT * FROM pragma_index_info(?) ORDER BY seqno',
|
||||
[row.name],
|
||||
true,
|
||||
);
|
||||
indexes[row.name] = {
|
||||
table,
|
||||
unique: row.unique !== 0,
|
||||
partial: row.partial !== 0,
|
||||
// A null column name means the index entry is an expression (or
|
||||
// rowid); represent it with a stable placeholder.
|
||||
columns: columnRows.map(col => col.name ?? '<expression>'),
|
||||
};
|
||||
}
|
||||
return indexes;
|
||||
}
|
||||
|
||||
export function getSchemaSnapshot(db: Database): SchemaSnapshot {
|
||||
const relations = sqlite.runQuery<SqliteMasterRow>(
|
||||
db,
|
||||
`SELECT name, type FROM sqlite_master
|
||||
WHERE type IN ('table', 'view') AND name NOT LIKE 'sqlite_%'
|
||||
ORDER BY name`,
|
||||
[],
|
||||
true,
|
||||
);
|
||||
|
||||
const snapshot: SchemaSnapshot = { tables: {}, views: {}, indexes: {} };
|
||||
|
||||
for (const relation of relations) {
|
||||
const relationSnapshot = getRelationSnapshot(db, relation.name);
|
||||
if (relation.type === 'table') {
|
||||
snapshot.tables[relation.name] = relationSnapshot;
|
||||
Object.assign(snapshot.indexes, getIndexSnapshots(db, relation.name));
|
||||
} else {
|
||||
snapshot.views[relation.name] = relationSnapshot;
|
||||
}
|
||||
}
|
||||
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
function diffRelations(
|
||||
kind: 'table' | 'view',
|
||||
baseline: Record<string, RelationSnapshot>,
|
||||
current: Record<string, RelationSnapshot>,
|
||||
additions: string[],
|
||||
breakages: string[],
|
||||
): void {
|
||||
for (const [name, baseRelation] of Object.entries(baseline)) {
|
||||
const currentRelation = current[name];
|
||||
if (!currentRelation) {
|
||||
breakages.push(`${kind} "${name}" was removed`);
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const [columnName, baseColumn] of Object.entries(
|
||||
baseRelation.columns,
|
||||
)) {
|
||||
const currentColumn = currentRelation.columns[columnName];
|
||||
if (!currentColumn) {
|
||||
breakages.push(`column "${name}.${columnName}" was removed`);
|
||||
} else if (JSON.stringify(currentColumn) !== JSON.stringify(baseColumn)) {
|
||||
breakages.push(
|
||||
`column "${name}.${columnName}" changed from ` +
|
||||
`${JSON.stringify(baseColumn)} to ${JSON.stringify(currentColumn)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
for (const columnName of Object.keys(currentRelation.columns)) {
|
||||
if (!baseRelation.columns[columnName]) {
|
||||
additions.push(`column "${name}.${columnName}"`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (const name of Object.keys(current)) {
|
||||
if (!baseline[name]) {
|
||||
additions.push(`${kind} "${name}"`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function diffSchemaSnapshots(
|
||||
baseline: SchemaSnapshot,
|
||||
current: SchemaSnapshot,
|
||||
): SchemaDiff {
|
||||
const additions: string[] = [];
|
||||
const breakages: string[] = [];
|
||||
|
||||
diffRelations('table', baseline.tables, current.tables, additions, breakages);
|
||||
diffRelations('view', baseline.views, current.views, additions, breakages);
|
||||
|
||||
for (const [name, baseIndex] of Object.entries(baseline.indexes)) {
|
||||
const currentIndex = current.indexes[name];
|
||||
if (!currentIndex) {
|
||||
breakages.push(
|
||||
`index "${name}" (on table "${baseIndex.table}") was removed`,
|
||||
);
|
||||
} else if (JSON.stringify(currentIndex) !== JSON.stringify(baseIndex)) {
|
||||
breakages.push(
|
||||
`index "${name}" changed from ${JSON.stringify(baseIndex)} ` +
|
||||
`to ${JSON.stringify(currentIndex)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
for (const name of Object.keys(current.indexes)) {
|
||||
if (!baseline.indexes[name]) {
|
||||
additions.push(`index "${name}"`);
|
||||
}
|
||||
}
|
||||
|
||||
return { additions, breakages };
|
||||
}
|
||||
|
||||
// When a breaking change is deliberately approved, the removed names are
|
||||
// recorded so they can never be reused: historical CRDT sync messages
|
||||
// referencing them still exist and would repopulate a reintroduced name
|
||||
// with stale data.
|
||||
export function computeRetiredNames(
|
||||
baseline: SchemaSnapshot,
|
||||
current: SchemaSnapshot,
|
||||
): RetiredNames {
|
||||
const tables: string[] = [];
|
||||
const columns: string[] = [];
|
||||
|
||||
for (const [name, baseTable] of Object.entries(baseline.tables)) {
|
||||
const currentTable = current.tables[name];
|
||||
if (!currentTable) {
|
||||
tables.push(name);
|
||||
continue;
|
||||
}
|
||||
for (const columnName of Object.keys(baseTable.columns)) {
|
||||
if (!currentTable.columns[columnName]) {
|
||||
columns.push(`${name}.${columnName}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return { tables, columns };
|
||||
}
|
||||
|
||||
export function mergeRetiredNames(
|
||||
existing: RetiredNames,
|
||||
incoming: RetiredNames,
|
||||
): RetiredNames {
|
||||
return {
|
||||
tables: [...new Set([...existing.tables, ...incoming.tables])].sort(),
|
||||
columns: [...new Set([...existing.columns, ...incoming.columns])].sort(),
|
||||
};
|
||||
}
|
||||
|
||||
export function findRetiredNameViolations(
|
||||
retired: RetiredNames,
|
||||
current: SchemaSnapshot,
|
||||
): string[] {
|
||||
const violations: string[] = [];
|
||||
|
||||
for (const table of retired.tables) {
|
||||
if (current.tables[table]) {
|
||||
violations.push(`table "${table}" is retired and must not be reused`);
|
||||
}
|
||||
}
|
||||
|
||||
for (const column of retired.columns) {
|
||||
const [table, columnName] = column.split('.');
|
||||
if (current.tables[table]?.columns[columnName]) {
|
||||
violations.push(`column "${column}" is retired and must not be reused`);
|
||||
}
|
||||
}
|
||||
|
||||
return violations;
|
||||
}
|
||||
|
||||
export function serializeSchemaSnapshot(snapshot: SchemaSnapshot): string {
|
||||
return JSON.stringify(snapshot, null, 2) + '\n';
|
||||
}
|
||||
|
||||
export function serializeRetiredNames(retired: RetiredNames): string {
|
||||
return JSON.stringify(retired, null, 2) + '\n';
|
||||
}
|
||||
1389
packages/loot-core/src/server/migrate/schema-snapshot.json
Normal file
1389
packages/loot-core/src/server/migrate/schema-snapshot.json
Normal file
File diff suppressed because it is too large
Load Diff
6
upcoming-release-notes/enforce-additive-migrations.md
Normal file
6
upcoming-release-notes/enforce-additive-migrations.md
Normal file
@@ -0,0 +1,6 @@
|
||||
---
|
||||
category: Maintenance
|
||||
authors: [MatissJanis]
|
||||
---
|
||||
|
||||
Enforce the additive-only database migration policy with a schema snapshot test and CI checks
|
||||
Reference in New Issue
Block a user