From fbf8a4d1c93a165f18eb6200a7e5f788d7b4f56c Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 17 Jun 2024 11:32:19 +0200 Subject: [PATCH] fix (client): fix immutable fields in setReplicationTransform (#1374) 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. --- .changeset/tasty-eels-trade.md | 5 + clients/typescript/src/client/model/table.ts | 20 +++- clients/typescript/src/satellite/mock.ts | 16 ++- .../test/client/model/table.test.ts | 98 +++++++++++++++++++ 4 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 .changeset/tasty-eels-trade.md diff --git a/.changeset/tasty-eels-trade.md b/.changeset/tasty-eels-trade.md new file mode 100644 index 00000000..fc79bba0 --- /dev/null +++ b/.changeset/tasty-eels-trade.md @@ -0,0 +1,5 @@ +--- +"electric-sql": patch +--- + +Modify setReplicationTransform to throw if a FK column is transformed. diff --git a/clients/typescript/src/client/model/table.ts b/clients/typescript/src/client/model/table.ts index 4d309781..ed522df7 100644 --- a/clients/typescript/src/client/model/table.ts +++ b/clients/typescript/src/client/model/table.ts @@ -1618,8 +1618,24 @@ export class Table< setReplicationTransform(i: ReplicatedRowTransformer): 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, { diff --git a/clients/typescript/src/satellite/mock.ts b/clients/typescript/src/satellite/mock.ts index 873ac810..68900a84 100644 --- a/clients/typescript/src/satellite/mock.ts +++ b/clients/typescript/src/satellite/mock.ts @@ -25,6 +25,7 @@ import { GoneBatchCallback, DataGone, DataChangeType, + DbRecord, } from '../util/types' import { ElectricConfig } from '../config/index' @@ -228,6 +229,11 @@ export class MockSatelliteClient private startReplicationDelayMs: number | null = null + private replicationTransforms: Map< + string, + ReplicatedRowTransformer + > = new Map() + setStartReplicationDelayMs(delayMs: number | null) { this.startReplicationDelayMs = delayMs } @@ -534,12 +540,12 @@ export class MockSatelliteClient } setReplicationTransform( - _tableName: QualifiedTablename, - _transform: ReplicatedRowTransformer + tableName: QualifiedTablename, + transform: ReplicatedRowTransformer ): 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) } } diff --git a/clients/typescript/test/client/model/table.test.ts b/clients/typescript/test/client/model/table.test.ts index 253f44b1..1a9a5ac4 100644 --- a/clients/typescript/test/client/model/table.test.ts +++ b/clients/typescript/test/client/model/table.test.ts @@ -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( @@ -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', + } + ) +})