85be463487
# Introduction
This PR introduces the slow instance commands pattern, that allow
migrating data in prior of the schema migration, that would fail if not.
Slow instance commands runs after the fast instance commands and before
the workspace commands.
On twenty instance that do not has any active or suspended workspace the
data migration part is skipped but the migration still runs, especially
for fresh installs
We were previously hacking through typeorm transaction system to gain
such granularity using save points:
```ts
export class AddPayloadToCommandMenuItem1775129635528
implements MigrationInterface
{
name = 'AddPayloadToCommandMenuItem1775129635528';
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "core"."commandMenuItem" ADD "payload" jsonb`,
);
const savepointName =
'sp_add_payload_check_constraint_to_command_menu_item';
try {
await queryRunner.query(`SAVEPOINT ${savepointName}`);
await addPayloadCheckConstraintToCommandMenuItem(queryRunner);
await queryRunner.query(`RELEASE SAVEPOINT ${savepointName}`);
} catch (e) {
try {
await queryRunner.query(`ROLLBACK TO SAVEPOINT ${savepointName}`);
await queryRunner.query(`RELEASE SAVEPOINT ${savepointName}`);
} catch (rollbackError) {
// oxlint-disable-next-line no-console
console.error(
'Failed to rollback to savepoint in AddPayloadToCommandMenuItem1775129635528',
rollbackError,
);
throw rollbackError;
}
// oxlint-disable-next-line no-console
console.error(
'Swallowing AddPayloadToCommandMenuItem1775129635528 error',
e,
);
}
}
```
It was afterwards re-applied within an workspace commands, it was hacky
and missleading for the self host having false positive in logs
## New pattern
Generate the slow instance command
```
npx nx database:migrate:generate twenty-server -- --name add-foo-bar-columns --type slow
```
```ts
import { DataSource, QueryRunner } from 'typeorm';
import { RegisteredInstanceCommand } from 'src/engine/core-modules/upgrade/decorators/registered-instance-command.decorator';
import { SlowInstanceCommand } from 'src/engine/core-modules/upgrade/interfaces/slow-instance-command.interface';
@RegisteredInstanceCommand('1.21.0', 1775640902366, { type: 'slow' })
export class AddPrastoinColToWorkspaceSlowInstanceCommand implements SlowInstanceCommand {
async runDataMigration(dataSource: DataSource): Promise<void> {
// TODO: implement data backfill before the DDL migration
}
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query('ALTER TABLE "core"."workspace" ADD "prastoin" character varying');
}
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query('ALTER TABLE "core"."workspace" DROP COLUMN "prastoin"');
}
}
```
### Why `run-instance-commands` and `upgrade` remain separate commands
These two commands serve fundamentally different purposes with
incompatible scoping semantics:
The run-instance-commands iterates over all the legacy typeorm and the
instance commands of all versions, used for database init and so on. we
could be centralizing both but the readability tradeoff isn't worth it
In the future thanks to the cross-version pattern we will be able to
centralize them but not right now
Please note that by default the `run-instance-commands` only run the
fast instance commands which is expected for our cloud prod CD
237 lines
6.5 KiB
TypeScript
237 lines
6.5 KiB
TypeScript
import { Test } from '@nestjs/testing';
|
|
import { getDataSourceToken } from '@nestjs/typeorm';
|
|
|
|
import { InstanceCommandGenerationService } from 'src/database/commands/instance-command-generation.service';
|
|
|
|
const FIXED_TIMESTAMP = 1775000000000;
|
|
|
|
const buildMockDataSource = (
|
|
upQueries: { query: string; parameters?: unknown[] }[],
|
|
downQueries: { query: string; parameters?: unknown[] }[],
|
|
) => ({
|
|
driver: {
|
|
createSchemaBuilder: () => ({
|
|
log: jest.fn().mockResolvedValue({ upQueries, downQueries }),
|
|
}),
|
|
},
|
|
});
|
|
|
|
describe('InstanceCommandGenerationService', () => {
|
|
const buildService = async (
|
|
upQueries: { query: string; parameters?: unknown[] }[] = [],
|
|
downQueries: { query: string; parameters?: unknown[] }[] = [],
|
|
) => {
|
|
const module = await Test.createTestingModule({
|
|
providers: [
|
|
InstanceCommandGenerationService,
|
|
{
|
|
provide: getDataSourceToken(),
|
|
useValue: buildMockDataSource(upQueries, downQueries),
|
|
},
|
|
],
|
|
}).compile();
|
|
|
|
return module.get(InstanceCommandGenerationService);
|
|
};
|
|
|
|
it('should return null when no schema changes are detected', async () => {
|
|
const service = await buildService();
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'no-changes',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toBeNull();
|
|
});
|
|
|
|
it('should generate a migration with a single up/down query', async () => {
|
|
const service = await buildService(
|
|
[{ query: 'ALTER TABLE "core"."user" ADD "foo" varchar' }],
|
|
[{ query: 'ALTER TABLE "core"."user" DROP COLUMN "foo"' }],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'add-foo-column',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should generate a migration with multiple queries', async () => {
|
|
const service = await buildService(
|
|
[
|
|
{
|
|
query:
|
|
'CREATE TABLE "core"."task" ("id" uuid NOT NULL DEFAULT uuid_generate_v4(), "name" varchar NOT NULL)',
|
|
},
|
|
{
|
|
query:
|
|
'ALTER TABLE "core"."task" ADD CONSTRAINT "PK_task" PRIMARY KEY ("id")',
|
|
},
|
|
],
|
|
[
|
|
{ query: 'ALTER TABLE "core"."task" DROP CONSTRAINT "PK_task"' },
|
|
{ query: 'DROP TABLE "core"."task"' },
|
|
],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'create-task-table',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should generate a migration with query parameters', async () => {
|
|
const service = await buildService(
|
|
[
|
|
{
|
|
query:
|
|
'INSERT INTO "core"."setting" ("key", "value") VALUES ($1, $2)',
|
|
parameters: ['theme', 'dark'],
|
|
},
|
|
],
|
|
[
|
|
{
|
|
query: 'DELETE FROM "core"."setting" WHERE "key" = $1',
|
|
parameters: ['theme'],
|
|
},
|
|
],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'seed-setting',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should escape single quotes in SQL queries', async () => {
|
|
const service = await buildService(
|
|
[{ query: 'UPDATE "core"."config" SET "value" = \'it\'\'s done\'' }],
|
|
[{ query: 'UPDATE "core"."config" SET "value" = \'original\'' }],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'update-config',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should escape backslashes in SQL queries', async () => {
|
|
const service = await buildService(
|
|
[
|
|
{
|
|
query: 'UPDATE "core"."config" SET "value" = E\'path\\\\to\\\\file\'',
|
|
},
|
|
],
|
|
[{ query: 'UPDATE "core"."config" SET "value" = NULL' }],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'update-path',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should use default migration name in class and file names', async () => {
|
|
const service = await buildService(
|
|
[{ query: 'ALTER TABLE "core"."user" ADD "bar" integer' }],
|
|
[{ query: 'ALTER TABLE "core"."user" DROP COLUMN "bar"' }],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'auto-generated',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should encode version correctly in file and class names', async () => {
|
|
const service = await buildService(
|
|
[{ query: 'SELECT 1' }],
|
|
[{ query: 'SELECT 1' }],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'test',
|
|
version: '1.20.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should return null for slow type when no schema changes are detected', async () => {
|
|
const service = await buildService();
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'no-changes',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
type: 'slow',
|
|
});
|
|
|
|
expect(result).toBeNull();
|
|
});
|
|
|
|
it('should generate a slow instance command with populated up/down', async () => {
|
|
const service = await buildService(
|
|
[
|
|
{
|
|
query: 'ALTER TABLE "core"."user" ALTER COLUMN "email" SET NOT NULL',
|
|
},
|
|
],
|
|
[
|
|
{
|
|
query: 'ALTER TABLE "core"."user" ALTER COLUMN "email" DROP NOT NULL',
|
|
},
|
|
],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'make-column-not-nullable',
|
|
version: '1.21.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
type: 'slow',
|
|
});
|
|
|
|
expect(result).toMatchSnapshot();
|
|
});
|
|
|
|
it('should use correct file naming for slow instance commands', async () => {
|
|
const service = await buildService(
|
|
[{ query: 'SELECT 1' }],
|
|
[{ query: 'SELECT 1' }],
|
|
);
|
|
|
|
const result = await service.generateInstanceCommand({
|
|
migrationName: 'backfill-data',
|
|
version: '1.20.0',
|
|
timestamp: FIXED_TIMESTAMP,
|
|
type: 'slow',
|
|
});
|
|
|
|
expect(result?.fileName).toBe(
|
|
'1-20-instance-command-slow-1775000000000-backfill-data.ts',
|
|
);
|
|
expect(result?.className).toBe('BackfillDataSlowInstanceCommand');
|
|
});
|
|
});
|