diff --git a/src/session-factory.ts b/src/session-factory.ts index ef2bbc7f0..45d8e3abb 100644 --- a/src/session-factory.ts +++ b/src/session-factory.ts @@ -48,8 +48,27 @@ export interface GetSessionCallback { * @interface SessionFactoryInterface */ export interface SessionFactoryInterface { + /** + * When called returns a session. + * + * @name SessionFactoryInterface#getSession + * @param {GetSessionCallback} callback The callback function. + */ getSession(callback: GetSessionCallback): void; + + /** + * When called returns the pool object. + * + * @name SessionFactoryInterface#getPool + */ getPool(): SessionPoolInterface; + + /** + * To be called when releasing a session. + * + * @name SessionFactoryInterface#release + * @param {Session} session The session to be released. + */ release(session: Session): void; } @@ -86,7 +105,7 @@ export class SessionFactory this.pool_.on('error', this.emit.bind(database, 'error')); this.pool_.open(); this.multiplexedSession_ = new MultiplexedSession(database); - // multiplexed session should only get created if the env variable is enabled + // Multiplexed sessions should only be created if its enabled. if ((this.multiplexedSession_ as MultiplexedSession).isMultiplexedEnabled) { this.multiplexedSession_.on('error', this.emit.bind(database, 'error')); this.multiplexedSession_.createSession(); @@ -94,12 +113,12 @@ export class SessionFactory } /** - * Retrieves the session, either the regular session or the multiplexed session based upon the environment varibale - * If the environment variable GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is set to `true` the method will attempt to - * retrieve the multiplexed session. Otherwise it will retrieve the session from the pool. + * Retrieves a session, either a regular session or a multiplexed session, based on the environment variable configuration. + * + * If the environment variable `GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS` is set to `true`, the method will attempt to + * retrieve a multiplexed session. Otherwise, it will retrieve a session from the regular pool. * - * The session is returned asynchronously via the provided callback, which will receive either an error or the session object. - * @param callback + * @param {GetSessionCallback} callback The callback function. */ getSession(callback: GetSessionCallback): void { @@ -112,11 +131,9 @@ export class SessionFactory } /** - * Returns the SessionPoolInterface used by the current instance, which provide access to the session pool - * for obtaining database sessions. + * Returns the regular session pool object. * * @returns {SessionPoolInterface} The session pool used by current instance. - * This object allows interaction with the pool for acquiring and managing sessions. */ getPool(): SessionPoolInterface { @@ -124,20 +141,20 @@ export class SessionFactory } /** - * Releases the session back to the pool. - * - * This method is used to return the session back to the pool after it is no longer needed. + * Releases a session back to the session pool. * - * It only performs the operation if the Multiplexed Session is disabled which is controlled via the - * env variable GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS. + * This method returns a session to the pool after it is no longer needed. + * It is a no-op for multiplexed sessions. * - * @param session The session to be released. This should be an instance of `Session` that was previously - * acquired from the session pool. + * @param {Session} session - The session to be released. This should be an instance of `Session` that was + * previously acquired from the session pool. * - * @throws {Error} Throws an error if the session is invalid or cannot be released. + * @throws {Error} If the session is invalid or cannot be released. */ release(session: Session): void { - if (!this.isMuxCreated) { + if ( + !(this.multiplexedSession_ as MultiplexedSession).isMultiplexedEnabled + ) { this.pool_.release(session); } } diff --git a/test/database.ts b/test/database.ts index 3cc71bf37..ecbfebfc7 100644 --- a/test/database.ts +++ b/test/database.ts @@ -137,7 +137,7 @@ export class FakeSessionFactory extends EventEmitter { this.calledWith_ = arguments; } getSession(): FakeSession | FakeMultiplexedSession { - if (process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS) { + if (process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS === 'false') { return new FakeSession(); } else { return new FakeMultiplexedSession(); @@ -327,6 +327,16 @@ describe('Database', () => { assert(database.formattedName_, formattedName); }); + it('should accept a custom Pool class', () => { + function FakePool() {} + const database = new Database( + INSTANCE, + NAME, + FakePool as {} as db.SessionPoolConstructor + ); + assert(database.pool_ instanceof FakeSessionPool); + }); + it('should re-emit SessionPool errors', done => { const error = new Error('err'); diff --git a/test/session-factory.ts b/test/session-factory.ts index b7097bd48..5e9c5b8b2 100644 --- a/test/session-factory.ts +++ b/test/session-factory.ts @@ -109,7 +109,7 @@ describe('SessionFactory', () => { assert.strictEqual(openStub.callCount, 1); }); - describe('when env GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is enabled', () => { + describe('when GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is enabled', () => { before(() => { process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'true'; }); @@ -120,7 +120,7 @@ describe('SessionFactory', () => { ); }); - it('should initiate the multiplexed session creation if the env is enabled', () => { + it('should initiate the multiplexed session creation if GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is enabled', () => { const createSessionStub = sandbox .stub(MultiplexedSession.prototype, 'createSession') .callsFake(() => {}); @@ -133,12 +133,12 @@ describe('SessionFactory', () => { }); describe('getSession', () => { - describe('for regular session', () => { + describe('when GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is disabled', () => { before(() => { process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'false'; }); - it('should return the regular session if GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS env is disabled', done => { + it('should retrieve a regular session from the pool', done => { ( sandbox.stub(sessionFactory.pool_, 'getSession') as sinon.SinonStub ).callsFake(callback => callback(null, fakeSession)); @@ -149,7 +149,7 @@ describe('SessionFactory', () => { }); }); - it('should return the error from getSession if GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS env is disabled and regular session creation get failed', done => { + it('should propagate errors when regular session retrieval fails', done => { const fakeError = new Error(); ( sandbox.stub(sessionFactory.pool_, 'getSession') as sinon.SinonStub @@ -162,12 +162,12 @@ describe('SessionFactory', () => { }); }); - describe('for multiplexed session', () => { + describe('when GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is enabled', () => { before(() => { process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'true'; }); - it('should return the multiplexed session if GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS env is enabled', done => { + it('should return the multiplexed session', done => { ( sandbox.stub( sessionFactory.multiplexedSession_, @@ -183,7 +183,7 @@ describe('SessionFactory', () => { }); }); - it('should return the error from getSession if GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS env is enabled and multiplexed session creation get failed', done => { + it('should propagate error when multiplexed session return fails', done => { const fakeError = new Error(); ( sandbox.stub( @@ -209,29 +209,44 @@ describe('SessionFactory', () => { }); describe('release', () => { - before(() => { - process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'false'; - }); + describe('when GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is enabled', () => { + before(() => { + process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'true'; + }); - it('should call the release method to release a regular session', () => { - const releaseStub = sandbox.stub(sessionFactory.pool_, 'release'); - const fakeSession = createSession(); - sessionFactory.release(fakeSession); - assert.strictEqual(releaseStub.callCount, 1); + it('should not call the release method', () => { + const releaseStub = sandbox.stub(sessionFactory.pool_, 'release'); + const fakeSession = createSession(); + sessionFactory.release(fakeSession); + assert.strictEqual(releaseStub.callCount, 0); + }); }); - it('should throw an error if encounters any error during release', () => { - const fakeSession = createSession(); - try { + describe('when GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS is disabled', () => { + before(() => { + process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'false'; + }); + + it('should call the release method to release a regular session', () => { + const releaseStub = sandbox.stub(sessionFactory.pool_, 'release'); + const fakeSession = createSession(); sessionFactory.release(fakeSession); - assert.fail('Expected error was not thrown'); - } catch (error) { - assert.strictEqual( - (error as ReleaseError).message, - 'Unable to release unknown resource.' - ); - assert.strictEqual((error as ReleaseError).resource, fakeSession); - } + assert.strictEqual(releaseStub.callCount, 1); + }); + + it('should propagate an error when release fails', () => { + const fakeSession = createSession(); + try { + sessionFactory.release(fakeSession); + assert.fail('Expected error was not thrown'); + } catch (error) { + assert.strictEqual( + (error as ReleaseError).message, + 'Unable to release unknown resource.' + ); + assert.strictEqual((error as ReleaseError).resource, fakeSession); + } + }); }); }); }); diff --git a/test/spanner.ts b/test/spanner.ts index 15fb5aaca..b6ee01024 100644 --- a/test/spanner.ts +++ b/test/spanner.ts @@ -5082,7 +5082,7 @@ describe('Spanner with mock server', () => { process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'false'; }); - it('should not throw error when enabling env GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS after client initialization', done => { + it('should not propagate any error when enabling GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS after client initialization', done => { const database = newTestDatabase(); // enable env after database creation process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'true';