Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added destroyColumn migration step #1671

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-Unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### New features

- Added `Database#experimentalIsVerbose` option
- Added destroyColumn migration step.

### Fixes

Expand Down
3 changes: 1 addition & 2 deletions src/RawRecord/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { ColumnName, ColumnSchema, TableSchema } from '../Schema'
import type { RecordId, SyncStatus } from '../Model'


// Raw object representing a model record, coming from an untrusted source
// (disk, sync, user data). Before it can be used to create a Model instance
// it must be sanitized (with `sanitizedRaw`) into a RawRecord
Expand All @@ -15,7 +14,7 @@ type _RawRecord = {
}

// Raw object representing a model record. A RawRecord is guaranteed by the type system
// to be safe to use (sanitied with `sanitizedRaw`):
// to be safe to use (sanitized with `sanitizedRaw`):
// - it has exactly the fields described by TableSchema (+ standard fields)
// - every field is exactly the type described by ColumnSchema (string, number, or boolean)
// - … and the same optionality (will not be null unless isOptional: true)
Expand Down
29 changes: 27 additions & 2 deletions src/Schema/migrations/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import type { $RE, $Exact } from '../../types'
import type { ColumnSchema, TableName, TableSchema, TableSchemaSpec, SchemaVersion } from '../index'
import type {
ColumnName,
ColumnSchema,
TableName,
TableSchema,
TableSchemaSpec,
SchemaVersion,
} from '../index'

export type CreateTableMigrationStep = $RE<{
type: 'create_table'
Expand All @@ -13,12 +20,22 @@ export type AddColumnsMigrationStep = $RE<{
unsafeSql?: (_: string) => string
}>

export type DestroyColumnMigrationStep = $RE<{
type: 'destroy_column'
table: TableName<any>
column: ColumnName
}>

export type SqlMigrationStep = $RE<{
type: 'sql'
sql: string
}>

export type MigrationStep = CreateTableMigrationStep | AddColumnsMigrationStep | SqlMigrationStep
export type MigrationStep =
| CreateTableMigrationStep
| AddColumnsMigrationStep
| DestroyColumnMigrationStep
| SqlMigrationStep

type Migration = $RE<{
toVersion: SchemaVersion
Expand Down Expand Up @@ -50,4 +67,12 @@ export function addColumns({
unsafeSql?: (_: string) => string
}>): AddColumnsMigrationStep

export function destroyColumn({
table,
column,
}: $Exact<{
table: TableName<any>
column: ColumnName
}>): DestroyColumnMigrationStep

export function unsafeExecuteSql(sql: string): SqlMigrationStep
37 changes: 34 additions & 3 deletions src/Schema/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
ColumnName,
ColumnSchema,
TableName,
TableSchema,
TableSchemaSpec,
SchemaVersion,
} from '../index'
import { tableSchema, validateColumnSchema } from '../index'

export type CreateTableMigrationStep = $RE<{
Expand All @@ -21,12 +28,22 @@ export type AddColumnsMigrationStep = $RE<{
unsafeSql?: (string) => string,
}>

export type DestroyColumnMigrationStep = $RE<{
type: 'destroy_column',
table: TableName<any>,
column: ColumnName,
}>

export type SqlMigrationStep = $RE<{
type: 'sql',
sql: string,
}>

export type MigrationStep = CreateTableMigrationStep | AddColumnsMigrationStep | SqlMigrationStep
export type MigrationStep =
| CreateTableMigrationStep
| AddColumnsMigrationStep
| DestroyColumnMigrationStep
| SqlMigrationStep

type Migration = $RE<{
toVersion: SchemaVersion,
Expand Down Expand Up @@ -159,6 +176,21 @@ export function addColumns({
return { type: 'add_columns', table, columns, unsafeSql }
}

export function destroyColumn({
table,
column,
}: $Exact<{
table: TableName<any>,
column: ColumnName,
}>): DestroyColumnMigrationStep {
if (process.env.NODE_ENV !== 'production') {
invariant(table, `Missing table name in destroyColumn()`)
invariant(column, `Missing column in destroyColumn()`)
}

return { type: 'destroy_column', table, column }
}

export function unsafeExecuteSql(sql: string): SqlMigrationStep {
if (process.env.NODE_ENV !== 'production') {
invariant(typeof sql === 'string', `SQL passed to unsafeExecuteSql is not a string`)
Expand All @@ -176,7 +208,6 @@ 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
addColumnIndex({ table: 'table_name', column: 'column_name' })
Expand Down
36 changes: 33 additions & 3 deletions src/Schema/migrations/test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createTable, addColumns, schemaMigrations } from './index'
import { createTable, addColumns, destroyColumn, schemaMigrations } from './index'
import { stepsForMigration } from './stepsForMigration'

describe('schemaMigrations()', () => {
Expand Down Expand Up @@ -30,7 +30,19 @@ describe('schemaMigrations()', () => {
it('returns a complex schema migrations spec', () => {
const migrations = schemaMigrations({
migrations: [
{ toVersion: 4, steps: [] },
{
toVersion: 4,
steps: [
addColumns({
table: 'comments',
columns: [{ name: 'text', type: 'string' }],
}),
destroyColumn({
table: 'comments',
column: 'body',
}),
],
},
{
toVersion: 3,
steps: [
Expand Down Expand Up @@ -103,7 +115,21 @@ describe('schemaMigrations()', () => {
},
],
},
{ toVersion: 4, steps: [] },
{
toVersion: 4,
steps: [
{
type: 'add_columns',
table: 'comments',
columns: [{ name: 'text', type: 'string' }],
},
{
type: 'destroy_column',
table: 'comments',
column: 'body',
},
],
},
],
})
})
Expand Down Expand Up @@ -181,6 +207,10 @@ describe('migration step functions', () => {
'type',
)
})
it('throws if destroyColumn() is malformed', () => {
expect(() => destroyColumn({ column: 'foo' })).toThrow('table')
expect(() => destroyColumn({ table: 'foo' })).toThrow('column')
})
})

describe('stepsForMigration', () => {
Expand Down
12 changes: 12 additions & 0 deletions src/adapters/lokijs/worker/DatabaseDriver.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
SchemaMigrations,
CreateTableMigrationStep,
AddColumnsMigrationStep,
DestroyColumnMigrationStep,
MigrationStep,
} from '../../../Schema/migrations'
import type { SerializedQuery } from '../../../Query'
Expand Down Expand Up @@ -390,6 +391,8 @@ export default class DatabaseDriver {
this._executeCreateTableMigration(step)
} else if (step.type === 'add_columns') {
this._executeAddColumnsMigration(step)
} else if (step.type === 'destroy_column') {
this._executeDestroyColumnMigration(step)
} else if (step.type === 'sql') {
// ignore
} else {
Expand Down Expand Up @@ -425,6 +428,15 @@ export default class DatabaseDriver {
})
}

_executeDestroyColumnMigration({ table, column }: DestroyColumnMigrationStep): void {
const collection = this.loki.getCollection(table)

// update ALL records in the collection, removing a field
collection.findAndUpdate({}, (record) => {
delete record[column]
})
}

// Maps records to their IDs if the record is already cached on JS side
_compactQueryResults(records: DirtyRaw[], table: TableName<any>): CachedQueryResult {
const cache = this.getCache(table)
Expand Down
31 changes: 29 additions & 2 deletions src/adapters/sqlite/encodeSchema/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

import type { TableSchema, AppSchema, ColumnSchema, TableName } from '../../../Schema'
import { nullValue } from '../../../RawRecord'
import type { MigrationStep, AddColumnsMigrationStep } from '../../../Schema/migrations'
import type {
MigrationStep,
AddColumnsMigrationStep,
DestroyColumnMigrationStep,
} from '../../../Schema/migrations'
import type { SQL } from '../index'

import encodeValue from '../encodeValue'
Expand Down Expand Up @@ -85,13 +89,36 @@ const encodeAddColumnsMigrationStep: (AddColumnsMigrationStep) => SQL = ({
})
.join('')

export const encodeMigrationSteps: (MigrationStep[]) => SQL = (steps) =>
const encodeDestroyColumnMigrationStep: (DestroyColumnMigrationStep, TableSchema) => SQL = (
{ table, column },
tableSchema,
) => {
const newTempTable = { ...tableSchema, name: `${table}Temp` }
const newColumns = [
...standardColumns,
...Object.keys(tableSchema.columns)
.filter((c) => c !== column)
.map((c) => `"${c}`),
]
return `
${encodeTable(newTempTable)}
INSERT INTO ${table}Temp(${newColumns.join(',')}) SELECT ${newColumns.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') {
return encodeTable(step.schema)
} else if (step.type === 'add_columns') {
return encodeAddColumnsMigrationStep(step)
} else if (step.type === 'destroy_column') {
return encodeDestroyColumnMigrationStep(step, schema.tables[step.table])
} else if (step.type === 'sql') {
return step.sql
}
Expand Down
2 changes: 1 addition & 1 deletion src/adapters/sqlite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
Expand Down