Skip to content

Commit

Permalink
fix (client): fix immutable fields in setReplicationTransform (#1374)
Browse files Browse the repository at this point in the history
This PR modifies the immutable fields that are checked in
`setReplicationTransform` such that it checks for the outgoing FK
columns and the PK columns that are being pointed at. Also adds unit
tests to check that these are computed correctly and an error is thrown
if we try to modify them.
  • Loading branch information
kevin-dp authored Jun 17, 2024
1 parent bd42afb commit fbf8a4d
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/tasty-eels-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"electric-sql": patch
---

Modify setReplicationTransform to throw if a FK column is transformed.
20 changes: 18 additions & 2 deletions clients/typescript/src/client/model/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1618,8 +1618,24 @@ export class Table<
setReplicationTransform(i: ReplicatedRowTransformer<T>): void {
// forbid transforming relation keys to avoid breaking
// referential integrity
const relations = this._dbDescription.getRelations(this.tableName)
const immutableFields = relations.map((r) => r.relationField)

// the column could be the FK column when it is an outgoing FK
// or it could be a PK column when it is an incoming FK
const fkCols = this._dbDescription
.getOutgoingRelations(this.tableName)
.map((r) => r.fromField)

// Incoming relations don't have the `fromField` and `toField` filled in
// so we need to fetch the `toField` from the opposite relation
// which is effectively a column in this table to which the FK points
const pkCols = this._dbDescription
.getIncomingRelations(this.tableName)
.map((r) => r.getOppositeRelation(this._dbDescription).toField)

// Merge all columns that are part of a FK relation.
// Remove duplicate columns in case a column has both an outgoing FK and an incoming FK.
const immutableFields = Array.from(new Set(fkCols.concat(pkCols)))

this._replicationTransformManager.setTableTransform(
this._qualifiedTableName,
{
Expand Down
16 changes: 11 additions & 5 deletions clients/typescript/src/satellite/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
GoneBatchCallback,
DataGone,
DataChangeType,
DbRecord,
} from '../util/types'
import { ElectricConfig } from '../config/index'

Expand Down Expand Up @@ -228,6 +229,11 @@ export class MockSatelliteClient

private startReplicationDelayMs: number | null = null

private replicationTransforms: Map<
string,
ReplicatedRowTransformer<DbRecord>
> = new Map()

setStartReplicationDelayMs(delayMs: number | null) {
this.startReplicationDelayMs = delayMs
}
Expand Down Expand Up @@ -534,12 +540,12 @@ export class MockSatelliteClient
}

setReplicationTransform(
_tableName: QualifiedTablename,
_transform: ReplicatedRowTransformer<DataRecord>
tableName: QualifiedTablename,
transform: ReplicatedRowTransformer<DataRecord>
): void {
throw new Error('Method not implemented.')
this.replicationTransforms.set(tableName.tablename, transform)
}
clearReplicationTransform(_tableName: QualifiedTablename): void {
throw new Error('Method not implemented.')
clearReplicationTransform(tableName: QualifiedTablename): void {
this.replicationTransforms.delete(tableName.tablename)
}
}
98 changes: 98 additions & 0 deletions clients/typescript/test/client/model/table.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import {
_RECORD_NOT_FOUND_,
} from '../../../src/client/validation/errors/messages'
import { schema, Post } from '../generated'
import { makeContext } from '../../satellite/common'
import { globalRegistry } from '../../../src/satellite'
import { ElectricClient } from '../../../src/client/model'
import { InvalidRecordTransformationError } from '../../../src/client/validation/errors/invalidRecordTransformationError'

const db = new Database(':memory:')
const electric = await electrify(
Expand Down Expand Up @@ -1743,3 +1747,97 @@ test.serial(
t.is(res.tablenames[1].tablename, 'Post')
}
)

test('setReplicationTransform should validate transform does not modify outgoing FK column', async (t: any) => {
await makeContext(t, 'main')

const { adapter, notifier, satellite, client } = t.context

const electric = await ElectricClient.create(
'testDB',
schema,
adapter,
notifier,
satellite,
globalRegistry,
'SQLite'
)

const postTable = electric.db.Post

const modifyAuthorId = (post: any) => ({
...post,
authorId: 9, // this is a FK, should not be allowed to modify it
})

// postTable, userTable
postTable.setReplicationTransform({
transformInbound: modifyAuthorId,
transformOutbound: modifyAuthorId,
})

// Check outbound transform
t.throws(
() => client.replicationTransforms.get('Post').transformOutbound(post1),
{
instanceOf: InvalidRecordTransformationError,
message: 'Record transformation modified immutable fields: authorId',
}
)

// Also check inbound transform
t.throws(
() => client.replicationTransforms.get('Post').transformInbound(post1),
{
instanceOf: InvalidRecordTransformationError,
message: 'Record transformation modified immutable fields: authorId',
}
)
})

test('setReplicationTransform should validate transform does not modify incoming FK column', async (t: any) => {
await makeContext(t, 'main')

const { adapter, notifier, satellite, client } = t.context

const electric = await ElectricClient.create(
'testDB',
schema,
adapter,
notifier,
satellite,
globalRegistry,
'SQLite'
)

const userTable = electric.db.User

const modifyUserId = (user: any) => ({
...user,
id: 9, // this is the column pointed at by the FK, should not be allowed to modify it
})

// postTable, userTable
userTable.setReplicationTransform({
transformInbound: modifyUserId,
transformOutbound: modifyUserId,
})

// Check outbound transform
t.throws(
() => client.replicationTransforms.get('User').transformOutbound(author1),
{
instanceOf: InvalidRecordTransformationError,
message: 'Record transformation modified immutable fields: id',
}
)

// Also check inbound transform
t.throws(
() => client.replicationTransforms.get('User').transformInbound(author1),
{
instanceOf: InvalidRecordTransformationError,
message: 'Record transformation modified immutable fields: id',
}
)
})

0 comments on commit fbf8a4d

Please sign in to comment.