9

packages/dandi-contrib/data-pg/index.ts

@@ -1,7 +1,9 @@
-export * from './src/pg.db.module'
-export * from './src/pg.db.client'
-export * from './src/pg.db.config'
-export * from './src/pg.db.model.builder.options'
-export * from './src/pg.db.pool'
-export * from './src/pg.db.pool.client'
-export * from './src/pg.db.transaction.client'
+export * from './src/invalid-transaction-state-error'
+export * from './src/pg-db.module'
+export * from './src/pg-db-client'
+export * from './src/pg-db-config'
+export * from './src/pg-db-model-builder-options'
+export * from './src/pg-db-pool'
+export * from './src/pg-db-pool-client'
+export * from './src/pg-db-query-error'
+export * from './src/pg-db-transaction-client'

packages/dandi-contrib/data-pg/src/invalid-transaction-state-error.ts

@@ -0,0 +1,7 @@
+import { AppError } from '@dandi/common'
+
+export class InvalidTransactionStateError extends AppError {
+  constructor(message: string) {
+    super(message)
+  }
+}

packages/dandi-contrib/data-pg/src/pg.db.client.spec.ts → packages/dandi-contrib/data-pg/src/pg-db-client.spec.ts

@@ -1,30 +1,41 @@
+import { PgDbPoolClientFixture } from '@dandi-contrib/data-pg/testing'
 import { AppError } from '@dandi/common'
 import { stubHarness } from '@dandi/core/testing'
-import { PgDbClient, PgDbPoolClient, TransactionAlreadyInProgressError } from '@dandi-contrib/data-pg'
+import { PgDbClient, PgDbTransactionClient } from '@dandi-contrib/data-pg'
+import { ModelBuilderFixture } from '@dandi/model-builder/testing'
 
 import { expect } from 'chai'
 import { SinonStub, stub } from 'sinon'
 
 describe('PgDbClient', function() {
 
   const harness = stubHarness(PgDbClient,
-    {
-      provide: PgDbPoolClient,
-      useFactory: () => ({
-        query: stub().returns({ rows: [] }),
-        release: stub(),
-      }),
-    },
+    PgDbPoolClientFixture.factory,
+    ModelBuilderFixture.factory,
+    PgDbTransactionClient,
   )
 
   beforeEach(async function () {
     this.dbClient = await harness.inject(PgDbClient)
   })
 
-  xdescribe('transaction', function() {
-    it('throws if a transaction is already in progress', async function() {
-      this.dbClient.transaction(async () => {})
-      await expect(this.dbClient.transaction(async () => {})).to.be.rejectedWith(TransactionAlreadyInProgressError)
+  describe('transaction', function() {
+    it('creates a transaction client and calls the specified function with it', async function() {
+
+      const fn = stub()
+      await this.dbClient.transaction(fn)
+
+      expect(fn).to.have.been.called
+      expect(fn.firstCall.lastArg).to.be.instanceof(PgDbTransactionClient)
+    })
+
+    it('can create multiple active transactions', async function() {
+      const first = stub()
+
+      this.dbClient.transaction(async () => {}).then(first)
+      expect(first).not.to.have.been.called // sanity check
+
+      await expect(this.dbClient.transaction(async () => {})).to.be.fulfilled
     })
 
     it('calls the transactionFn and then automatically disposes the transaction', async function() {
@@ -65,21 +76,25 @@ describe('PgDbClient', function() {
     })
   })
 
-  xdescribe('dispose', function() {
+  describe('dispose', function() {
     it('calls dispose() on the current transaction, if there is one', async function() {
-      let waiter
-      const transactionDisposePromise = new Promise<SinonStub>((resolve) => {
-        waiter = resolve
-      })
+
+      let resolveDispose: Function
+      const disposePromise = new Promise(resolve => resolveDispose = resolve)
+
       const transactionFn = stub().callsFake((transaction) => {
-        waiter(stub(transaction, 'dispose'))
+        resolveDispose(stub(transaction, 'dispose'))
+        return new Promise(() => {})
       })
 
-      this.dbClient.transaction(transactionFn)
-      const transactionDispose = await transactionDisposePromise
-      expect(transactionDispose).not.to.have.been.called
-      this.dbClient.dispose('')
-      expect(transactionDispose).to.have.been.called
+      const after = stub()
+      this.dbClient.transaction(transactionFn).then(after)
+      const dispose = await disposePromise
+
+      expect(dispose).not.to.have.been.called
+      expect(after).not.to.have.been.called
+      await this.dbClient.dispose('')
+      expect(dispose).to.have.been.called
     })
   })
 })

packages/dandi-contrib/data-pg/src/pg.db.client.ts → packages/dandi-contrib/data-pg/src/pg-db-client.ts

@@ -1,17 +1,11 @@
-import { AppError, Disposable } from '@dandi/common'
+import { Disposable } from '@dandi/common'
 import { Inject, Injectable, Logger, Optional, Injector } from '@dandi/core'
 import { DbClient, DbTransactionClient, TransactionFn } from '@dandi/data'
 import { ModelBuilder, ModelBuilderOptions } from '@dandi/model-builder'
 
-import { PgDbPool } from './pg.db.pool'
-import { PgDbQueryableBase } from './pg.db.queryable'
-import { PgDbModelBuilderOptions } from './pg.db.model.builder.options'
-
-export class TransactionAlreadyInProgressError extends AppError {
-  constructor() {
-    super('A transaction is already in progress for this client instance')
-  }
-}
+import { PgDbPool } from './pg-db-pool'
+import { PgDbQueryableBase } from './pg-db-queryable'
+import { PgDbModelBuilderOptions } from './pg-db-model-builder-options'
 
 @Injectable(DbClient)
 export class PgDbClient extends PgDbQueryableBase<PgDbPool> implements DbClient, Disposable {
@@ -30,18 +24,20 @@ export class PgDbClient extends PgDbQueryableBase<PgDbPool> implements DbClient,
   }
 
   public async transaction<T>(transactionFn: TransactionFn<T>): Promise<T> {
-    const transaction = (await this.injector.inject(DbTransactionClient)).singleValue
-    try {
+    return Disposable.useAsync(this.injector.inject(DbTransactionClient), async transactionResult => {
+      const transaction = transactionResult.singleValue
       this.activeTransactions.push(transaction)
 
-      return await Disposable.useAsync(transaction, async (transaction) => {
-        return await transactionFn(transaction)
-      })
-    } catch (err) {
-      throw err
-    } finally {
-      this.activeTransactions.splice(this.activeTransactions.indexOf(transaction), 1)
-    }
+      try {
+        // IMPORTANT! must await the useAsync so that errors are caught and the active transaction is not removed
+        // until the transaction is complete
+        return await Disposable.useAsync(transaction, transactionFn)
+      } catch (err) {
+        throw err
+      } finally {
+        this.activeTransactions.splice(this.activeTransactions.indexOf(transaction), 1)
+      }
+    })
   }
 
   public async dispose(reason: string): Promise<void> {

packages/dandi-contrib/data-pg/src/pg.db.model.builder.options.ts → packages/dandi-contrib/data-pg/src/pg-db-model-builder-options.ts

@@ -1,7 +1,7 @@
 import { InjectionToken } from '@dandi/core'
 import { ModelBuilderOptions } from '@dandi/model-builder'
 
-import { localOpinionatedToken } from './local.token'
+import { localOpinionatedToken } from './local-token'
 
 export const PgDbModelBuilderOptions: InjectionToken<ModelBuilderOptions> = localOpinionatedToken(
   'PgDbModelBuilderOptions',

packages/dandi-contrib/data-pg/src/pg.db.pool.client.ts → packages/dandi-contrib/data-pg/src/pg-db-pool-client.ts

@@ -2,9 +2,9 @@ import { Disposable } from '@dandi/common'
 import { InjectionToken, Provider } from '@dandi/core'
 import { PoolClient } from 'pg'
 
-import { localOpinionatedToken } from './local.token'
-import { PgDbPool } from './pg.db.pool'
-import { PgDbQueryableClient } from './pg.db.queryable'
+import { localOpinionatedToken } from './local-token'
+import { PgDbPool } from './pg-db-pool'
+import { PgDbQueryableClient } from './pg-db-queryable'
 
 export interface PgDbPoolClient extends Disposable, PgDbQueryableClient {}
 

packages/dandi-contrib/data-pg/src/pg.db.pool.ts → packages/dandi-contrib/data-pg/src/pg-db-pool.ts

@@ -2,8 +2,8 @@ import { Disposable } from '@dandi/common'
 import { Inject, Injectable, Injector, Singleton } from '@dandi/core'
 import { Pool, PoolClient, PoolConfig, QueryResult } from 'pg'
 
-import { PgDbConfig } from './pg.db.config'
-import { PgDbQueryableClient } from './pg.db.queryable'
+import { PgDbConfig } from './pg-db-config'
+import { PgDbQueryableClient } from './pg-db-queryable'
 
 @Injectable(Singleton)
 export class PgDbPool implements Disposable, PgDbQueryableClient {
@@ -19,7 +19,7 @@ export class PgDbPool implements Disposable, PgDbQueryableClient {
 
   public async dispose(): Promise<void> {
     await this.pool.end()
-    this.pool = null
+    this.pool = undefined
     Disposable.remapDisposed(this, 'disposed')
   }
 

packages/dandi-contrib/data-pg/src/pg-db-queryable.spec.ts

@@ -0,0 +1,32 @@
+import { PgDbPoolClient } from '@dandi-contrib/data-pg'
+import { PgDbPoolClientFixture, PgDbQueryableBase, PgDbQueryableClient } from '@dandi-contrib/data-pg/testing'
+import { stubHarness } from '@dandi/core/testing'
+import { ModelBuilder } from '@dandi/model-builder'
+import { ModelBuilderFixture } from '@dandi/model-builder/testing'
+
+import { expect } from 'chai'
+
+describe('PgDbQueryableBase', function() {
+
+  const harness = stubHarness(
+    PgDbPoolClientFixture.factory,
+    ModelBuilderFixture.factory,
+  )
+
+  beforeEach(async function() {
+    PgDbPoolClientFixture.result({ rows: [{ id: 'a' }, { id: 'b' }] })
+    this.client = await harness.injectStub(PgDbPoolClient)
+    this.queryable = new PgDbQueryableBase<PgDbQueryableClient>(this.client, await harness.inject(ModelBuilder))
+  })
+
+  describe('query', function() {
+    it('passes the cmd and args arguments to the client', async function() {
+      const cmd = 'SELECT foo FROM bar WHERE ix = $1'
+      const args = 'nay'
+
+      await this.queryable.query(cmd, args)
+
+      expect(this.client.query).to.have.been.calledWithExactly(cmd, [args])
+    })
+  })
+})

packages/dandi-contrib/data-pg/src/pg.db.queryable.ts → packages/dandi-contrib/data-pg/src/pg-db-queryable.ts

@@ -2,15 +2,19 @@ import { Constructor, Url, Uuid } from '@dandi/common'
 import { DbQueryable } from '@dandi/data'
 import { DataPropertyMetadata, ModelUtil } from '@dandi/model'
 import { ModelBuilder, ModelBuilderOptions } from '@dandi/model-builder'
+
 import { snakeCase } from 'change-case'
 import { QueryResult } from 'pg'
 
-import { PgDbMultipleResultsError, PgDbQueryError } from './pg.db.query.error'
+import { PgDbMultipleResultsError, PgDbQueryError } from './pg-db-query-error'
 
 export interface PgDbQueryableClient {
-  query(cmd: string, args: any[]): Promise<QueryResult>;
+  query(cmd: string, args: any[]): Promise<QueryResult>
 }
 
+/**
+ * @internal
+ */
 export class PgDbQueryableBase<TClient extends PgDbQueryableClient> implements DbQueryable {
   constructor(
     protected client: TClient,