From 2ec161a3d46f66a4c16ee13fb3fd89c0fdc4de1d Mon Sep 17 00:00:00 2001 From: kirillgroshkov Date: Thu, 18 Jan 2024 11:40:24 +0100 Subject: [PATCH] feat: CommonDBTransactionOptions, forbidTransactionReadAfterWrite --- .github/workflows/release.yml | 2 +- src/adapter/inmemory/inMemory.db.ts | 50 +++++++++++++++++++++++++++-- src/base.common.db.ts | 3 +- src/common.db.ts | 6 +++- src/db.model.ts | 8 +++++ 5 files changed, 63 insertions(+), 6 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 72fbef9..42da9d9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,7 +14,7 @@ jobs: - { uses: actions/setup-node@v4, with: { node-version: 'lts/*', cache: 'yarn' } } # Cache for npm/npx in ~/.npm - - uses: actions/cache@v3 + - uses: actions/cache@v4 with: path: ~/.npm key: npm-v1-${{ runner.os }} diff --git a/src/adapter/inmemory/inMemory.db.ts b/src/adapter/inmemory/inMemory.db.ts index 93a829e..0fcf4d2 100644 --- a/src/adapter/inmemory/inMemory.db.ts +++ b/src/adapter/inmemory/inMemory.db.ts @@ -32,6 +32,7 @@ import { import { CommonDB, commonDBFullSupport, + CommonDBTransactionOptions, CommonDBType, DBIncrement, DBOperation, @@ -58,6 +59,17 @@ export interface InMemoryDBCfg { */ tablesPrefix: string + /** + * Many DB implementations (e.g Datastore and Firestore) forbid doing + * read operations after a write/delete operation was done inside a Transaction. + * + * To help spot that type of bug - InMemoryDB by default has this setting to `true`, + * which will throw on such occasions. + * + * Defaults to true. + */ + forbidTransactionReadAfterWrite?: boolean + /** * @default false * @@ -96,6 +108,7 @@ export class InMemoryDB implements CommonDB { this.cfg = { // defaults tablesPrefix: '', + forbidTransactionReadAfterWrite: true, persistenceEnabled: false, persistZip: true, persistentStoragePath: './tmp/inmemorydb', @@ -273,8 +286,12 @@ export class InMemoryDB implements CommonDB { return Readable.from(queryInMemory(q, Object.values(this.data[table] || {}) as ROW[])) } - async runInTransaction(fn: DBTransactionFn): Promise { - const tx = new InMemoryDBTransaction(this) + async runInTransaction(fn: DBTransactionFn, opt: CommonDBTransactionOptions = {}): Promise { + const tx = new InMemoryDBTransaction(this, { + readOnly: false, + ...opt, + }) + try { await fn(tx) await tx.commit() @@ -361,15 +378,28 @@ export class InMemoryDB implements CommonDB { } export class InMemoryDBTransaction implements DBTransaction { - constructor(private db: InMemoryDB) {} + constructor( + private db: InMemoryDB, + private opt: Required, + ) {} ops: DBOperation[] = [] + // used to enforce forbidReadAfterWrite setting + writeOperationHappened = false + async getByIds( table: string, ids: string[], opt?: CommonDBOptions, ): Promise { + if (this.db.cfg.forbidTransactionReadAfterWrite) { + _assert( + !this.writeOperationHappened, + `InMemoryDBTransaction: read operation attempted after write operation`, + ) + } + return await this.db.getByIds(table, ids, opt) } @@ -378,6 +408,13 @@ export class InMemoryDBTransaction implements DBTransaction { rows: ROW[], opt?: CommonDBSaveOptions, ): Promise { + _assert( + !this.opt.readOnly, + `InMemoryDBTransaction: saveBatch(${table}) called in readOnly mode`, + ) + + this.writeOperationHappened = true + this.ops.push({ type: 'saveBatch', table, @@ -387,6 +424,13 @@ export class InMemoryDBTransaction implements DBTransaction { } async deleteByIds(table: string, ids: string[], opt?: CommonDBOptions): Promise { + _assert( + !this.opt.readOnly, + `InMemoryDBTransaction: deleteByIds(${table}) called in readOnly mode`, + ) + + this.writeOperationHappened = true + this.ops.push({ type: 'deleteByIds', table, diff --git a/src/base.common.db.ts b/src/base.common.db.ts index f839c78..d3bfe0c 100644 --- a/src/base.common.db.ts +++ b/src/base.common.db.ts @@ -4,6 +4,7 @@ import { CommonDB, CommonDBSupport, CommonDBType } from './common.db' import { CommonDBOptions, CommonDBSaveOptions, + CommonDBTransactionOptions, DBPatch, DBTransactionFn, RunQueryResult, @@ -83,7 +84,7 @@ export class BaseCommonDB implements CommonDB { throw new Error('deleteByIds is not implemented') } - async runInTransaction(fn: DBTransactionFn): Promise { + async runInTransaction(fn: DBTransactionFn, opt?: CommonDBTransactionOptions): Promise { const tx = new FakeDBTransaction(this) await fn(tx) // there's no try/catch and rollback, as there's nothing to rollback diff --git a/src/common.db.ts b/src/common.db.ts index 242bb37..0e515d0 100644 --- a/src/common.db.ts +++ b/src/common.db.ts @@ -5,6 +5,7 @@ import { CommonDBOptions, CommonDBSaveOptions, CommonDBStreamOptions, + CommonDBTransactionOptions, DBPatch, DBTransactionFn, RunQueryResult, @@ -163,8 +164,11 @@ export interface CommonDB { * Transaction is automatically committed if fn resolves normally. * Transaction is rolled back if fn throws, the error is re-thrown in that case. * Graceful rollback is allowed on tx.rollback() + * + * By default, transaction is read-write, + * unless specified as readOnly in CommonDBTransactionOptions. */ - runInTransaction: (fn: DBTransactionFn) => Promise + runInTransaction: (fn: DBTransactionFn, opt?: CommonDBTransactionOptions) => Promise } /** diff --git a/src/db.model.ts b/src/db.model.ts index 625ebe6..6fe7b37 100644 --- a/src/db.model.ts +++ b/src/db.model.ts @@ -34,6 +34,14 @@ export interface DBTransaction { rollback: () => Promise } +export interface CommonDBTransactionOptions { + /** + * Default is false. + * If set to true - Transaction is created as read-only. + */ + readOnly?: boolean +} + export interface CommonDBOptions { /** * If passed - the operation will be performed in the context of that DBTransaction.