From fdedc8923b67350913cf7f2fa26adc8c0eaf4922 Mon Sep 17 00:00:00 2001 From: Christiaan Landman Date: Wed, 18 Sep 2024 13:28:52 +0200 Subject: [PATCH 1/4] Deprecated rawTables --- .../src/client/AbstractPowerSyncDatabase.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/common/src/client/AbstractPowerSyncDatabase.ts b/packages/common/src/client/AbstractPowerSyncDatabase.ts index ba03c03c2..a7fda31e4 100644 --- a/packages/common/src/client/AbstractPowerSyncDatabase.ts +++ b/packages/common/src/client/AbstractPowerSyncDatabase.ts @@ -79,6 +79,8 @@ export interface SQLWatchOptions { /** The minimum interval between queries. */ throttleMs?: number; /** + * @deprecated Underlying tables of tables specified in {@link tables} will be watched automatically. + * * Allows for watching any SQL table * by not removing PowerSync table name prefixes */ @@ -889,7 +891,9 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver( + (resolvedOptions?.tables ?? []).flatMap((table) => [table, `ps_data__${table}`, `ps_data_local__${table}`]) + ); const changedTables = new Set(); const throttleMs = resolvedOptions.throttleMs ?? DEFAULT_WATCH_THROTTLE_MS; @@ -910,8 +914,7 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver { try { - const { rawTableNames } = resolvedOptions; - this.processTableUpdates(update, rawTableNames, changedTables); + this.processTableUpdates(update, changedTables); flushTableUpdates(); } catch (error) { onError?.(error); @@ -976,25 +979,19 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver ): void { const tables = isBatchedUpdateNotification(updateNotification) ? updateNotification.tables : [updateNotification.table]; - const filteredTables = rawTableNames ? tables : tables.filter((t) => !!t.match(POWERSYNC_TABLE_MATCH)); - if (!filteredTables.length) { - return; - } - - // Remove any PowerSync table prefixes if necessary - const mappedTableNames = rawTableNames - ? filteredTables - : filteredTables.map((t) => t.replace(POWERSYNC_TABLE_MATCH, '')); - - for (const table of mappedTableNames) { + for (const table of tables) { changedTables.add(table); + + // if an underlying table is a `ps_` table then add the mapped table name + if (table.match(POWERSYNC_TABLE_MATCH)) { + changedTables.add(table.replace(POWERSYNC_TABLE_MATCH, '')); + } } } From a5a1382c8e14db0c58ecb85896458b5fafa027f0 Mon Sep 17 00:00:00 2001 From: Christiaan Landman Date: Fri, 20 Sep 2024 14:14:32 +0200 Subject: [PATCH 2/4] Added changeset entry. --- .changeset/brown-radios-report.md | 5 +++++ packages/common/src/client/AbstractPowerSyncDatabase.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/brown-radios-report.md diff --git a/.changeset/brown-radios-report.md b/.changeset/brown-radios-report.md new file mode 100644 index 000000000..a6d0a4d46 --- /dev/null +++ b/.changeset/brown-radios-report.md @@ -0,0 +1,5 @@ +--- +'@powersync/common': minor +--- + +Deprecated `rawTableNames` field in `SQLWatchOptions`. All tables specified in `tables` will now be watched, including PowerSync tables with prefixes. diff --git a/packages/common/src/client/AbstractPowerSyncDatabase.ts b/packages/common/src/client/AbstractPowerSyncDatabase.ts index a7fda31e4..0decf28e0 100644 --- a/packages/common/src/client/AbstractPowerSyncDatabase.ts +++ b/packages/common/src/client/AbstractPowerSyncDatabase.ts @@ -79,7 +79,7 @@ export interface SQLWatchOptions { /** The minimum interval between queries. */ throttleMs?: number; /** - * @deprecated Underlying tables of tables specified in {@link tables} will be watched automatically. + * @deprecated All tables specified in {@link tables} will be watched, including PowerSync tables with prefixes. * * Allows for watching any SQL table * by not removing PowerSync table name prefixes From 1a68e1e4a8c042a9b3d7a49340afc423512838f9 Mon Sep 17 00:00:00 2001 From: Christiaan Landman Date: Wed, 25 Sep 2024 17:01:07 +0200 Subject: [PATCH 3/4] No longer need to map ps_ table back to table name. Added unit tests for onChange. --- .../src/client/AbstractPowerSyncDatabase.ts | 5 -- packages/web/tests/on_change.test.ts | 66 +++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 packages/web/tests/on_change.test.ts diff --git a/packages/common/src/client/AbstractPowerSyncDatabase.ts b/packages/common/src/client/AbstractPowerSyncDatabase.ts index 0decf28e0..86560f3de 100644 --- a/packages/common/src/client/AbstractPowerSyncDatabase.ts +++ b/packages/common/src/client/AbstractPowerSyncDatabase.ts @@ -987,11 +987,6 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver((resolve) => setTimeout(resolve, 10)); + * waits for 1 second instead of 10ms. + * Setting this to 1 second as a work around. + */ +const throttleDuration = 1000; + +describe('Watch Tests', () => { + let powersync: AbstractPowerSyncDatabase; + + beforeEach(async () => { + powersync = new PowerSyncDatabase({ + database: { dbFilename: 'test-watch.db' }, + schema: testSchema, + flags: { + enableMultiTabs: false + } + }); + }); + + afterEach(async () => { + await powersync.disconnectAndClear(); + await powersync.close(); + }); + + async function runOnChangeTest(tablesToWatch: string[], expectedChangedTables: string[]) { + const changedTables: string[] = []; + const abortController = new AbortController(); + const onChange = (event: WatchOnChangeEvent) => { + changedTables.push(...event.changedTables); + }; + + powersync.onChange( + { onChange }, + { tables: tablesToWatch, signal: abortController.signal, throttleMs: throttleDuration } + ); + + await powersync.execute('INSERT INTO assets(id, make, customer_id) VALUES (uuid(), ?, ?)', ['test', uuid()]); + await new Promise((resolve) => setTimeout(resolve, throttleDuration)); + + abortController.abort(); + expect(changedTables).toEqual(expectedChangedTables); + } + + it('basic onChange test', async () => { + await runOnChangeTest(['assets'], ['ps_data__assets']); + }); + + it('internal "ps_data" table onChange test', async () => { + await runOnChangeTest(['ps_data__assets'], ['ps_data__assets']); + }); + + it('internal "ps_oplog" table onChange test', async () => { + await runOnChangeTest(['ps_oplog'], ['ps_oplog']); + }); +}); From fe2c173c076fe0b46d4f9c20f67e5d421275e0d1 Mon Sep 17 00:00:00 2001 From: Christiaan Landman Date: Wed, 25 Sep 2024 17:42:51 +0200 Subject: [PATCH 4/4] Droppped timeout, using waitFor. --- packages/web/tests/on_change.test.ts | 32 +++++++++++----------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/packages/web/tests/on_change.test.ts b/packages/web/tests/on_change.test.ts index 92342d4c5..71ee7d92b 100644 --- a/packages/web/tests/on_change.test.ts +++ b/packages/web/tests/on_change.test.ts @@ -3,19 +3,10 @@ import { PowerSyncDatabase } from '@powersync/web'; import { v4 as uuid } from 'uuid'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { testSchema } from './utils/testDb'; -vi.useRealTimers(); -/** - * There seems to be an issue with Vitest browser mode's setTimeout and - * fake timer functionality. - * e.g. calling: - * await new Promise((resolve) => setTimeout(resolve, 10)); - * waits for 1 second instead of 10ms. - * Setting this to 1 second as a work around. - */ -const throttleDuration = 1000; +const UPLOAD_TIMEOUT_MS = 3000; -describe('Watch Tests', () => { +describe('OnChange Tests', () => { let powersync: AbstractPowerSyncDatabase; beforeEach(async () => { @@ -36,18 +27,21 @@ describe('Watch Tests', () => { async function runOnChangeTest(tablesToWatch: string[], expectedChangedTables: string[]) { const changedTables: string[] = []; const abortController = new AbortController(); - const onChange = (event: WatchOnChangeEvent) => { + const onChange = vi.fn((event: WatchOnChangeEvent) => { changedTables.push(...event.changedTables); - }; + }); - powersync.onChange( - { onChange }, - { tables: tablesToWatch, signal: abortController.signal, throttleMs: throttleDuration } + powersync.onChange({ onChange }, { tables: tablesToWatch, signal: abortController.signal }); + powersync.execute('INSERT INTO assets(id, make, customer_id) VALUES (uuid(), ?, ?)', ['test', uuid()]); + await vi.waitFor( + () => { + expect(onChange).toHaveBeenCalled(); + }, + { + timeout: UPLOAD_TIMEOUT_MS + } ); - await powersync.execute('INSERT INTO assets(id, make, customer_id) VALUES (uuid(), ?, ?)', ['test', uuid()]); - await new Promise((resolve) => setTimeout(resolve, throttleDuration)); - abortController.abort(); expect(changedTables).toEqual(expectedChangedTables); }