diff --git a/.changeset/quick-needles-cheat.md b/.changeset/quick-needles-cheat.md new file mode 100644 index 000000000..e12d30edf --- /dev/null +++ b/.changeset/quick-needles-cheat.md @@ -0,0 +1,5 @@ +--- +'@powersync/common': minor +--- + +Added `setupContext` option to `CreateDiffTriggerOptions` and a lock context option to the cleanup function returned by `createDiffTrigger`, this allows you to create and dispose of a trigger inside of a lock context. diff --git a/packages/common/src/client/triggers/TriggerManager.ts b/packages/common/src/client/triggers/TriggerManager.ts index 6b73ce9c6..b5eaa671d 100644 --- a/packages/common/src/client/triggers/TriggerManager.ts +++ b/packages/common/src/client/triggers/TriggerManager.ts @@ -223,14 +223,27 @@ export interface CreateDiffTriggerOptions extends BaseCreateDiffTriggerOptions { * This table will be dropped once the trigger is removed. */ destination: string; + + /** + * Context to use for the setup operation. + * This is useful for when the setup operation needs to be executed in a specific context. + */ + setupContext?: LockContext; } /** * @experimental - * Callback to drop a trigger after it has been created. + * Options for {@link TriggerRemoveCallback}. */ -export type TriggerRemoveCallback = () => Promise; +export interface TriggerRemoveCallbackOptions { + context?: LockContext; +} +/** + * @experimental + * Callback to drop a trigger after it has been created. + */ +export type TriggerRemoveCallback = (options?: TriggerRemoveCallbackOptions) => Promise; /** * @experimental * Options for {@link TriggerDiffHandlerContext#withDiff}. diff --git a/packages/common/src/client/triggers/TriggerManagerImpl.ts b/packages/common/src/client/triggers/TriggerManagerImpl.ts index 6a6d7d00d..379fdd1fd 100644 --- a/packages/common/src/client/triggers/TriggerManagerImpl.ts +++ b/packages/common/src/client/triggers/TriggerManagerImpl.ts @@ -9,6 +9,7 @@ import { TriggerManager, TriggerManagerConfig, TriggerRemoveCallback, + TriggerRemoveCallbackOptions, WithDiffOptions } from './TriggerManager.js'; @@ -201,6 +202,7 @@ export class TriggerManagerImpl implements TriggerManager { columns, when, hooks, + setupContext, // Fall back to the provided default if not given on this level useStorage = this.defaultConfig.useStorageByDefault } = options; @@ -268,13 +270,19 @@ export class TriggerManagerImpl implements TriggerManager { * we need to ensure we can cleanup the created resources. * We unfortunately cannot rely on transaction rollback. */ - const cleanup = async () => { + const cleanup = async (options?: TriggerRemoveCallbackOptions) => { + const { context } = options ?? {}; disposeWarningListener(); - return this.db.writeLock(async (tx) => { + const doCleanup = async (tx: LockContext) => { await this.removeTriggers(tx, triggerIds); - await tx.execute(/* sql */ `DROP TABLE IF EXISTS ${destination};`); + await tx.execute(`DROP TABLE IF EXISTS ${destination};`); await releaseStorageClaim?.(); - }); + }; + if (context) { + await doCleanup(context); + } else { + await this.db.writeLock(doCleanup); + } }; const setup = async (tx: LockContext) => { @@ -360,11 +368,15 @@ export class TriggerManagerImpl implements TriggerManager { }; try { - await this.db.writeLock(setup); + if (setupContext) { + await setup(setupContext); + } else { + await this.db.writeLock(setup); + } return cleanup; } catch (error) { try { - await cleanup(); + await cleanup(setupContext ? { context: setupContext } : undefined); } catch (cleanupError) { throw new AggregateError([error, cleanupError], 'Error during operation and cleanup'); } diff --git a/packages/node/tests/trigger.test.ts b/packages/node/tests/trigger.test.ts index eca9c6c9e..5c677cefc 100644 --- a/packages/node/tests/trigger.test.ts +++ b/packages/node/tests/trigger.test.ts @@ -705,6 +705,49 @@ describe('Triggers', () => { ); }); + databaseTest('Recreating trigger with write lock on dispose and re-creation', async ({ database }) => { + const destination = 'temp_recreate'; + const columns: Array = ['content']; + const when = { [DiffTriggerOperation.INSERT]: 'TRUE' }; + + let dispose = await database.triggers.createDiffTrigger({ + source: 'todos', + destination, + columns, + when + }); + + // Dispose the trigger (drops destination table + triggers) + await database.writeLock(async (tx) => { + await dispose({ context: tx }); + + // Create new trigger in the same lock context — no nested lock needed + dispose = await database.triggers.createDiffTrigger({ + source: 'todos', + destination, + columns, + when, + setupContext: tx + }); + }); + + // Verify new trigger works after locked context recreation + await database.execute("INSERT INTO todos (id, content) VALUES (uuid(), 'after recreate');"); + + await vi.waitFor( + async () => { + const rows = await database.writeLock(async (tx) => + tx.getAll(`SELECT * FROM ${destination}`) + ); + expect(rows.length).toBe(1); + expect(JSON.parse(rows[0].value).content).toBe('after recreate'); + }, + { timeout: 1000 } + ); + + await dispose(); + }); + databaseTest('Should use persisted tables and triggers', async ({ database }) => { const table = 'temp_remote_lists'; diff --git a/packages/web/tests/triggers.test.ts b/packages/web/tests/triggers.test.ts index 7a356d6a8..585879f5b 100644 --- a/packages/web/tests/triggers.test.ts +++ b/packages/web/tests/triggers.test.ts @@ -55,7 +55,7 @@ describe('Triggers', () => { } }); - onTestFinished(disposeTrigger); + onTestFinished(() => disposeTrigger()); await db.execute("INSERT INTO customers (id, name) VALUES (uuid(), 'test')"); @@ -103,7 +103,7 @@ describe('Triggers', () => { } }); - onTestFinished(disposeTrigger); + onTestFinished(() => disposeTrigger()); await db.execute("INSERT INTO customers (id, name) VALUES (uuid(), 'test')"); @@ -221,7 +221,7 @@ describe('Triggers', () => { } }); - onTestFinished(disposeTrigger); + onTestFinished(() => disposeTrigger()); // Perform an insert from client B await dbB.execute("INSERT INTO customers (id, name) VALUES (uuid(), 'from-client-b')");