From ea8771a8f78daf35718c16dadaa2b08e1b36307b Mon Sep 17 00:00:00 2001 From: yevhencheshchevyi Date: Tue, 19 Sep 2023 16:37:29 +0300 Subject: [PATCH] feat: add renameColumn migration step --- CHANGELOG-Unreleased.md | 1 + src/Schema/migrations/getSyncChanges/index.js | 22 +++++++- src/Schema/migrations/getSyncChanges/test.js | 55 +++++++++++++++++-- src/Schema/migrations/index.d.ts | 24 +++++++- src/Schema/migrations/index.js | 40 +++++++++++++- src/Schema/migrations/test.js | 22 +++++++- src/adapters/sqlite/encodeSchema/index.js | 27 +++++++-- src/adapters/sqlite/index.js | 2 +- .../__tests__/synchronize-migration.test.js | 5 +- 9 files changed, 179 insertions(+), 19 deletions(-) diff --git a/CHANGELOG-Unreleased.md b/CHANGELOG-Unreleased.md index 0e183688c..eb6e9c1b7 100644 --- a/CHANGELOG-Unreleased.md +++ b/CHANGELOG-Unreleased.md @@ -46,6 +46,7 @@ Changes unlikely to cause issues: ### New features - New `@experimentalFailsafe` decorator you can apply before `@relation/@immutableRelation` so that if relation points to a record that does not exist, `.fetch()/.observe()` yield `undefined` instead of throwing an error +- Added renameColumn migration step. ### Fixes diff --git a/src/Schema/migrations/getSyncChanges/index.js b/src/Schema/migrations/getSyncChanges/index.js index 91ed64a5a..0c6e74321 100644 --- a/src/Schema/migrations/getSyncChanges/index.js +++ b/src/Schema/migrations/getSyncChanges/index.js @@ -15,6 +15,13 @@ export type MigrationSyncChanges = $Exact<{ table: TableName, columns: ColumnName[], }>[], + +renamedColumns: $Exact<{ + table: TableName, + columns: $Exact<{ + from: ColumnName, + to: ColumnName, + }>[], + }>[], }> | null export default function getSyncChanges( @@ -34,7 +41,7 @@ export default function getSyncChanges( steps.forEach((step) => { invariant( - ['create_table', 'add_columns', 'sql'].includes(step.type), + ['create_table', 'add_columns', 'sql', 'rename_column'].includes(step.type), `Unknown migration step type ${step.type}. Can not perform migration sync. This most likely means your migrations are defined incorrectly. It could also be a WatermelonDB bug.`, ) }) @@ -63,9 +70,22 @@ export default function getSyncChanges( columns: unique(columnDefs.map(({ name }) => name)), })) + const allRenamedColumns = steps.filter( + (step) => step.type === 'rename_column' && !createdTables.includes(step.table), + ) + + const renamedColumns = pipe( + groupBy(({ table }) => table), + toPairs, + )(allRenamedColumns).map(([table, columns]) => ({ + table: tableName(table), + columns: columns.map(({ from, to }) => ({ from, to })), + })) + return { from: fromVersion, tables: unique(createdTables), columns: addedColumns, + renamedColumns, } } diff --git a/src/Schema/migrations/getSyncChanges/test.js b/src/Schema/migrations/getSyncChanges/test.js index ce0e87da2..631b6f7d2 100644 --- a/src/Schema/migrations/getSyncChanges/test.js +++ b/src/Schema/migrations/getSyncChanges/test.js @@ -1,5 +1,5 @@ import getSyncChanges from './index' -import { schemaMigrations, createTable, addColumns, unsafeExecuteSql } from '../index' +import { schemaMigrations, createTable, addColumns, unsafeExecuteSql, renameColumn } from '../index' const createCommentsTable = createTable({ name: 'comments', @@ -18,10 +18,15 @@ describe('getSyncChanges', () => { expect(test([{ toVersion: 2, steps: [createCommentsTable] }], 2, 2)).toEqual(null) }) it('returns empty changes for empty steps', () => { - expect(testSteps([])).toEqual({ from: 1, tables: [], columns: [] }) + expect(testSteps([])).toEqual({ from: 1, tables: [], columns: [], renamedColumns: [] }) }) it('returns created tables', () => { - expect(testSteps([createCommentsTable])).toEqual({ from: 1, tables: ['comments'], columns: [] }) + expect(testSteps([createCommentsTable])).toEqual({ + from: 1, + tables: ['comments'], + columns: [], + renamedColumns: [], + }) }) it('returns added columns', () => { expect( @@ -38,6 +43,36 @@ describe('getSyncChanges', () => { from: 1, tables: [], columns: [{ table: 'posts', columns: ['subtitle', 'is_pinned'] }], + renamedColumns: [], + }) + }) + it('returns renamed columns', () => { + expect(testSteps([renameColumn({ table: 'posts', from: 'body', to: 'text' })])).toEqual({ + from: 1, + tables: [], + columns: [], + renamedColumns: [{ table: 'posts', columns: [{ from: 'body', to: 'text' }] }], + }) + }) + it('combines renamed columns from multiple migration steps', () => { + expect( + testSteps([ + renameColumn({ table: 'posts', from: 'body', to: 'text' }), + renameColumn({ table: 'posts', from: 'favorite', to: 'saved' }), + ]), + ).toEqual({ + from: 1, + tables: [], + columns: [], + renamedColumns: [ + { + table: 'posts', + columns: [ + { from: 'body', to: 'text' }, + { from: 'favorite', to: 'saved' }, + ], + }, + ], }) }) it('combines added columns from multiple migration steps', () => { @@ -60,6 +95,7 @@ describe('getSyncChanges', () => { from: 1, tables: [], columns: [{ table: 'posts', columns: ['subtitle', 'is_pinned', 'author_id'] }], + renamedColumns: [], }) }) it('skips added columns for a table if it is also added', () => { @@ -75,6 +111,7 @@ describe('getSyncChanges', () => { from: 1, tables: ['comments'], columns: [], + renamedColumns: [], }) }) it('skips duplicates', () => { @@ -97,6 +134,7 @@ describe('getSyncChanges', () => { from: 1, tables: ['comments'], columns: [{ table: 'posts', columns: ['subtitle'] }], + renamedColumns: [], }) }) const bigMigrations = [ @@ -198,6 +236,7 @@ describe('getSyncChanges', () => { { table: 'comments', columns: ['is_pinned', 'extra'] }, { table: 'workspaces', columns: ['plan_info', 'limits'] }, ], + renamedColumns: [], }) }) it(`returns only the necessary range of migrations`, () => { @@ -208,13 +247,20 @@ describe('getSyncChanges', () => { { table: 'workspaces', columns: ['plan_info', 'limits'] }, { table: 'attachment_versions', columns: ['reactions'] }, ], + renamedColumns: [], }) expect(test(bigMigrations, 8, 10)).toEqual({ from: 8, tables: [], columns: [{ table: 'attachment_versions', columns: ['reactions'] }], + renamedColumns: [], + }) + expect(test(bigMigrations, 9, 10)).toEqual({ + from: 9, + tables: [], + columns: [], + renamedColumns: [], }) - expect(test(bigMigrations, 9, 10)).toEqual({ from: 9, tables: [], columns: [] }) expect(test(bigMigrations, 10, 10)).toEqual(null) }) it(`fails on incorrect migrations`, () => { @@ -225,7 +271,6 @@ describe('getSyncChanges', () => { const possibleFutureTypes = [ 'broken', 'rename_table', - 'rename_column', 'add_column_index', 'make_column_optional', 'make_column_required', diff --git a/src/Schema/migrations/index.d.ts b/src/Schema/migrations/index.d.ts index 35f27f669..6e2024eec 100644 --- a/src/Schema/migrations/index.d.ts +++ b/src/Schema/migrations/index.d.ts @@ -1,4 +1,5 @@ import type { $RE, $Exact } from '../../types' +import { ColumnName } from '../index' import type { ColumnSchema, TableName, TableSchema, TableSchemaSpec, SchemaVersion } from '../index' export type CreateTableMigrationStep = $RE<{ @@ -18,7 +19,18 @@ export type SqlMigrationStep = $RE<{ sql: string }> -export type MigrationStep = CreateTableMigrationStep | AddColumnsMigrationStep | SqlMigrationStep +export type RenameColumnMigrationStep = $RE<{ + type: 'rename_column' + table: TableName + from: ColumnName + to: ColumnName +}> + +export type MigrationStep = + | CreateTableMigrationStep + | AddColumnsMigrationStep + | SqlMigrationStep + | RenameColumnMigrationStep type Migration = $RE<{ toVersion: SchemaVersion @@ -51,3 +63,13 @@ export function addColumns({ }>): AddColumnsMigrationStep export function unsafeExecuteSql(sql: string): SqlMigrationStep + +export function renameColumn({ + table, + from, + to, +}: $Exact<{ + table: TableName + from: string + to: string +}>): RenameColumnMigrationStep diff --git a/src/Schema/migrations/index.js b/src/Schema/migrations/index.js index d6b8cd8ac..331ef82f5 100644 --- a/src/Schema/migrations/index.js +++ b/src/Schema/migrations/index.js @@ -6,7 +6,14 @@ import invariant from '../../utils/common/invariant' import isObj from '../../utils/fp/isObj' import type { $RE } from '../../types' -import type { ColumnSchema, TableName, TableSchema, TableSchemaSpec, SchemaVersion } from '../index' +import type { + ColumnSchema, + TableName, + TableSchema, + TableSchemaSpec, + SchemaVersion, + ColumnName, +} from '../index' import { tableSchema, validateColumnSchema } from '../index' export type CreateTableMigrationStep = $RE<{ @@ -26,7 +33,18 @@ export type SqlMigrationStep = $RE<{ sql: string, }> -export type MigrationStep = CreateTableMigrationStep | AddColumnsMigrationStep | SqlMigrationStep +export type RenameColumnMigrationStep = $RE<{ + type: 'rename_column', + table: TableName, + from: ColumnName, + to: ColumnName, +}> + +export type MigrationStep = + | CreateTableMigrationStep + | AddColumnsMigrationStep + | SqlMigrationStep + | RenameColumnMigrationStep type Migration = $RE<{ toVersion: SchemaVersion, @@ -166,6 +184,23 @@ export function unsafeExecuteSql(sql: string): SqlMigrationStep { return { type: 'sql', sql } } +export function renameColumn({ + table, + from, + to, +}: $Exact<{ + table: TableName, + from: ColumnName, + to: ColumnName, +}>): RenameColumnMigrationStep { + if (process.env.NODE_ENV !== 'production') { + invariant(table, `Missing table name in renameColumn()`) + invariant(from, `Missing 'from' in renameColumn()`) + invariant(to, `Missing 'to' in renameColumn()`) + } + return { type: 'rename_column', table, from, to } +} + /* TODO: Those types of migrations are currently not implemented. If you need them, feel free to contribute! @@ -175,7 +210,6 @@ destroyTable('table_name') renameTable({ from: 'old_table_name', to: 'new_table_name' }) // column operations -renameColumn({ table: 'table_name', from: 'old_column_name', to: 'new_column_name' }) destroyColumn({ table: 'table_name', column: 'column_name' }) // indexing diff --git a/src/Schema/migrations/test.js b/src/Schema/migrations/test.js index b47d24b65..4eb174e7f 100644 --- a/src/Schema/migrations/test.js +++ b/src/Schema/migrations/test.js @@ -1,4 +1,4 @@ -import { createTable, addColumns, schemaMigrations } from './index' +import { createTable, addColumns, schemaMigrations, renameColumn } from './index' import { stepsForMigration } from './stepsForMigration' describe('schemaMigrations()', () => { @@ -30,7 +30,7 @@ describe('schemaMigrations()', () => { it('returns a complex schema migrations spec', () => { const migrations = schemaMigrations({ migrations: [ - { toVersion: 4, steps: [] }, + { toVersion: 4, steps: [renameColumn({ table: 'comments', from: 'body', to: 'text' })] }, { toVersion: 3, steps: [ @@ -103,7 +103,17 @@ describe('schemaMigrations()', () => { }, ], }, - { toVersion: 4, steps: [] }, + { + toVersion: 4, + steps: [ + { + type: 'rename_column', + table: 'comments', + from: 'body', + to: 'text', + }, + ], + }, ], }) }) @@ -181,6 +191,12 @@ describe('migration step functions', () => { 'type', ) }) + + it('throws if renameColumns() is malformed', () => { + expect(() => renameColumn({ from: 'text', to: 'body' })).toThrow('table') + expect(() => renameColumn({ table: 'foo', from: 'text' })).toThrow('to') + expect(() => renameColumn({ table: 'foo', to: 'body' })).toThrow('from') + }) }) describe('stepsForMigration', () => { diff --git a/src/adapters/sqlite/encodeSchema/index.js b/src/adapters/sqlite/encodeSchema/index.js index dec177712..3c503e26c 100644 --- a/src/adapters/sqlite/encodeSchema/index.js +++ b/src/adapters/sqlite/encodeSchema/index.js @@ -2,18 +2,23 @@ import type { TableSchema, AppSchema, ColumnSchema, TableName } from '../../../Schema' import { nullValue } from '../../../RawRecord' -import type { MigrationStep, AddColumnsMigrationStep } from '../../../Schema/migrations' +import type { + MigrationStep, + AddColumnsMigrationStep, + RenameColumnMigrationStep, +} from '../../../Schema/migrations' import type { SQL } from '../index' import encodeValue from '../encodeValue' -const standardColumns = `"id" primary key, "_changed", "_status"` +const standardColumns = ['id', '_changed', '_status'] +const standardColumnsSQL = standardColumns.map((column) => column === 'id' ? `"${column}" primary key` : `"${column}"`).join(', ') const commonSchema = 'create table "local_storage" ("key" varchar(16) primary key not null, "value" text not null);' + 'create index "local_storage_key_index" on "local_storage" ("key");' const encodeCreateTable = ({ name, columns }: TableSchema): SQL => { - const columnsSQL = [standardColumns] + const columnsSQL = [standardColumnsSQL] .concat(Object.keys(columns).map((column) => `"${column}"`)) .join(', ') return `create table "${name}" (${columnsSQL});` @@ -85,7 +90,19 @@ const encodeAddColumnsMigrationStep: (AddColumnsMigrationStep) => SQL = ({ }) .join('') -export const encodeMigrationSteps: (MigrationStep[]) => SQL = (steps) => +const encodeRenameColumnMigrationsStep: (RenameColumnMigrationStep, TableSchema) => SQL = ({ table, from, to }, tableSchema) => { + const newTempTable = { ...tableSchema, name: `${table}Temp` } + const newColumns = [...standardColumns, ...Object.keys(tableSchema.columns)] + const prevColumns = newColumns.map(c => c === to ? from : c) + return ` + ${encodeTable(newTempTable)} + INSERT INTO ${table}Temp(${newColumns.join(',')}) SELECT ${prevColumns.join(',')} FROM ${table}; + DROP TABLE ${table}; + ALTER TABLE ${table}Temp RENAME TO ${table}; + ` +} + +export const encodeMigrationSteps: (MigrationStep[], AppSchema) => SQL = (steps, schema) => steps .map((step) => { if (step.type === 'create_table') { @@ -94,6 +111,8 @@ export const encodeMigrationSteps: (MigrationStep[]) => SQL = (steps) => return encodeAddColumnsMigrationStep(step) } else if (step.type === 'sql') { return step.sql + } else if (step.type === 'rename_column') { + return encodeRenameColumnMigrationsStep(step, schema.tables[step.table]) } throw new Error(`Unsupported migration step ${step.type}`) diff --git a/src/adapters/sqlite/index.js b/src/adapters/sqlite/index.js index 72a5af9f5..d28da976e 100644 --- a/src/adapters/sqlite/index.js +++ b/src/adapters/sqlite/index.js @@ -169,7 +169,7 @@ export default class SQLiteAdapter implements DatabaseAdapter { 'setUpWithMigrations', [ this.dbName, - require('./encodeSchema').encodeMigrationSteps(migrationSteps), + require('./encodeSchema').encodeMigrationSteps(migrationSteps, this.schema), databaseVersion, this.schema.version, ], diff --git a/src/sync/impl/__tests__/synchronize-migration.test.js b/src/sync/impl/__tests__/synchronize-migration.test.js index a29fd8f61..05d7f7098 100644 --- a/src/sync/impl/__tests__/synchronize-migration.test.js +++ b/src/sync/impl/__tests__/synchronize-migration.test.js @@ -1,6 +1,6 @@ import { mockDatabase, testSchema } from '../../../__tests__/testModels' import { expectToRejectWithMessage } from '../../../__tests__/utils' -import { schemaMigrations, createTable, addColumns } from '../../../Schema/migrations' +import { schemaMigrations, createTable, addColumns, renameColumn } from '../../../Schema/migrations' import { emptyPull } from './helpers' import { synchronize } from '../../index' @@ -17,6 +17,7 @@ describe('synchronize - migration syncs', () => { table: 'attachment_versions', columns: [{ name: 'reactions', type: 'string' }], }), + renameColumn({ table: 'post', from: 'body', to: 'text' }), ], }, { @@ -113,6 +114,7 @@ describe('synchronize - migration syncs', () => { from: 9, tables: [], columns: [{ table: 'attachment_versions', columns: ['reactions'] }], + renamedColumns: [{ table: 'post', columns: [{ from: 'body', to: 'text' }] }], }, }) expect(await getLastPulledSchemaVersion(database)).toBe(10) @@ -135,6 +137,7 @@ describe('synchronize - migration syncs', () => { from: 8, tables: ['attachments'], columns: [{ table: 'attachment_versions', columns: ['reactions'] }], + renamedColumns: [{ table: 'post', columns: [{ from: 'body', to: 'text' }] }], }, }) expect(await getLastPulledSchemaVersion(database)).toBe(10)