From 271050f942367bdd9efe199960d58c7899b880e8 Mon Sep 17 00:00:00 2001 From: Steven Ontong Date: Fri, 1 Nov 2024 10:22:49 +0200 Subject: [PATCH 1/2] fix: cached parsed sync rules --- .changeset/heavy-shirts-chew.md | 5 +++ .../storage/mongo/MongoSyncBucketStorage.ts | 16 ++++++-- .../test/src/data_storage.test.ts | 40 ++++++++++++++++++- packages/service-core/test/src/util.ts | 2 +- 4 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 .changeset/heavy-shirts-chew.md diff --git a/.changeset/heavy-shirts-chew.md b/.changeset/heavy-shirts-chew.md new file mode 100644 index 000000000..9bcb5e662 --- /dev/null +++ b/.changeset/heavy-shirts-chew.md @@ -0,0 +1,5 @@ +--- +'@powersync/service-core': patch +--- + +Improved sync rules storage cached parsed sync rules, accommodating different parsing options where necessary. diff --git a/packages/service-core/src/storage/mongo/MongoSyncBucketStorage.ts b/packages/service-core/src/storage/mongo/MongoSyncBucketStorage.ts index 93fb87728..72e007714 100644 --- a/packages/service-core/src/storage/mongo/MongoSyncBucketStorage.ts +++ b/packages/service-core/src/storage/mongo/MongoSyncBucketStorage.ts @@ -9,13 +9,13 @@ import * as util from '../../util/util-index.js'; import { BucketDataBatchOptions, BucketStorageBatch, - ReplicationCheckpoint, CompactOptions, DEFAULT_DOCUMENT_BATCH_LIMIT, DEFAULT_DOCUMENT_CHUNK_LIMIT_BYTES, FlushedResult, ParseSyncRulesOptions, PersistedSyncRulesContent, + ReplicationCheckpoint, ResolveTableOptions, ResolveTableResult, StartBatchOptions, @@ -53,7 +53,7 @@ export class MongoSyncBucketStorage } }); - private parsedSyncRulesCache: SqlSyncRules | undefined; + private parsedSyncRulesCache: {parsed: SqlSyncRules, options: ParseSyncRulesOptions} | undefined; private writeCheckpointAPI: WriteCheckpointAPI; constructor( @@ -104,8 +104,16 @@ export class MongoSyncBucketStorage } getParsedSyncRules(options: ParseSyncRulesOptions): SqlSyncRules { - this.parsedSyncRulesCache ??= this.sync_rules.parsed(options).sync_rules; - return this.parsedSyncRulesCache; + const {parsed, options: cachedOptions} = this.parsedSyncRulesCache ?? {}; + /** + * Check if the cached sync rules, if present, had the same options. + * Parse sync rules if the options are different or if there is no cached value. + */ + if (!parsed || options.defaultSchema != cachedOptions?.defaultSchema ) { + this.parsedSyncRulesCache = {parsed: this.sync_rules.parsed(options).sync_rules, options}; + } + + return this.parsedSyncRulesCache!.parsed; } async getCheckpoint(): Promise { diff --git a/packages/service-core/test/src/data_storage.test.ts b/packages/service-core/test/src/data_storage.test.ts index 776043b03..9fcefa1b2 100644 --- a/packages/service-core/test/src/data_storage.test.ts +++ b/packages/service-core/test/src/data_storage.test.ts @@ -1,4 +1,5 @@ import { BucketDataBatchOptions, SaveOperationTag } from '@/storage/BucketStorage.js'; +import { getUuidReplicaIdentityBson } from '@/util/util-index.js'; import { RequestParameters } from '@powersync/service-sync-rules'; import { describe, expect, test } from 'vitest'; import { fromAsync, oneFromAsync } from './stream_utils.js'; @@ -13,7 +14,6 @@ import { StorageFactory, testRules } from './util.js'; -import { getUuidReplicaIdentityBson } from '@/util/util-index.js'; const TEST_TABLE = makeTestTable('test', ['id']); @@ -1460,4 +1460,42 @@ bucket_definitions: replication_size_bytes: 0 }); }); + + test('invalidate cached parsed sync rules', async () => { + const WORKSPACE_TABLE = makeTestTable('workspace', ['id']); + + const sync_rules_content = testRules( + ` +bucket_definitions: + by_workspace: + parameters: + - SELECT id as workspace_id FROM workspace WHERE + workspace."userId" = token_parameters.user_id + data: [] + ` + ); + + const bucketStorageFactory = await factory(); + const syncBucketStorage = bucketStorageFactory.getInstance(sync_rules_content); + + const parsedSchema1 = syncBucketStorage.getParsedSyncRules({ + defaultSchema: 'public' + }); + + const parsedSchema2 = syncBucketStorage.getParsedSyncRules({ + defaultSchema: 'public' + }); + + // These should be cached, this will be the same instance + expect(parsedSchema2).equals(parsedSchema1); + expect(parsedSchema1.getSourceTables()[0].schema).equals('public'); + + const parsedSchema3 = syncBucketStorage.getParsedSyncRules({ + defaultSchema: 'databasename' + }); + + // The cache should not be used + expect(parsedSchema3).not.equals(parsedSchema2); + expect(parsedSchema3.getSourceTables()[0].schema).equals('databasename'); + }); } diff --git a/packages/service-core/test/src/util.ts b/packages/service-core/test/src/util.ts index 98cd613e2..421879d3c 100644 --- a/packages/service-core/test/src/util.ts +++ b/packages/service-core/test/src/util.ts @@ -62,7 +62,7 @@ export function testRules(content: string): PersistedSyncRulesContent { parsed(options) { return { id: 1, - sync_rules: SqlSyncRules.fromYaml(content, PARSE_OPTIONS), + sync_rules: SqlSyncRules.fromYaml(content, options), slot_name: 'test' }; }, From 81bc3cb43d61ee53987ec1678bf9a8bd3d9b5565 Mon Sep 17 00:00:00 2001 From: Steven Ontong Date: Fri, 1 Nov 2024 10:29:09 +0200 Subject: [PATCH 2/2] cleanup --- packages/service-core/test/src/data_storage.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/service-core/test/src/data_storage.test.ts b/packages/service-core/test/src/data_storage.test.ts index 9fcefa1b2..df8249b6a 100644 --- a/packages/service-core/test/src/data_storage.test.ts +++ b/packages/service-core/test/src/data_storage.test.ts @@ -1462,8 +1462,6 @@ bucket_definitions: }); test('invalidate cached parsed sync rules', async () => { - const WORKSPACE_TABLE = makeTestTable('workspace', ['id']); - const sync_rules_content = testRules( ` bucket_definitions: