From 3a08210838be4b3e3ee15af8014472a1938923e2 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Jul 2021 14:24:29 -0600 Subject: [PATCH 1/3] Fix transaction interference in v9 --- .../src/core/view/EventRegistration.ts | 6 ++- .../database/test/exp/integration.test.ts | 37 ++++++++++++++++++- packages/database/test/transaction.test.ts | 30 +++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/database/src/core/view/EventRegistration.ts b/packages/database/src/core/view/EventRegistration.ts index cd9ff28985a..e9377dc843d 100644 --- a/packages/database/src/core/view/EventRegistration.ts +++ b/packages/database/src/core/view/EventRegistration.ts @@ -71,8 +71,10 @@ export class CallbackContext { matches(other: CallbackContext): boolean { return ( this.snapshotCallback === other.snapshotCallback || - (this.snapshotCallback.userCallback === - other.snapshotCallback.userCallback && + (this.snapshotCallback.userCallback !== undefined && + this.snapshotCallback.userCallback === + other.snapshotCallback.userCallback && + this.snapshotCallback.context !== undefined && this.snapshotCallback.context === other.snapshotCallback.context) ); } diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 9790c88d24c..823d03e0047 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -24,12 +24,15 @@ import { getDatabase, goOffline, goOnline, + push, ref, - refFromURL + refFromURL, + runTransaction } from '../../exp/index'; import { onValue, set } from '../../src/exp/Reference_impl'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; import { DATABASE_ADDRESS, DATABASE_URL } from '../helpers/util'; +import { Deferred } from '@firebase/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -150,4 +153,36 @@ describe('Database@exp Tests', () => { expect(() => ref(db)).to.throw('Cannot call ref on a deleted database.'); defaultApp = undefined; }); + + it('Can listen to transaction changes', async () => { + // Repro for https://github.com/firebase/firebase-js-sdk/issues/5195 + let latestValue = 0; + + let deferred = new Deferred(); + + const database = getDatabase(defaultApp); + const counterRef = push(ref(database, 'counter')); + + onValue(counterRef, snap => { + latestValue = snap.val(); + deferred.resolve(); + }); + + async function incrementViaTransaction() { + deferred = new Deferred(); + await runTransaction(counterRef, currentData => { + return currentData + 1; + }); + // Wait for the snapshot listener to fire. They are not invoked inline + // for transactions. + await deferred.promise; + } + + expect(latestValue).to.equal(0); + + await incrementViaTransaction(); + expect(latestValue).to.equal(1); + await incrementViaTransaction(); + expect(latestValue).to.equal(2); + }); }); diff --git a/packages/database/test/transaction.test.ts b/packages/database/test/transaction.test.ts index 530c41361b3..5bf5caabd2b 100644 --- a/packages/database/test/transaction.test.ts +++ b/packages/database/test/transaction.test.ts @@ -1500,4 +1500,34 @@ describe('Transaction Tests', () => { done(); }); }); + + it('Can listen to transaction changes', async () => { + // Repro for https://github.com/firebase/firebase-js-sdk/issues/5195 + let latestValue = 0; + + const ref = getRandomNode() as Reference; + + let deferred = new Deferred(); + ref.on('value', snap => { + latestValue = snap.val() as number; + deferred.resolve(); + }); + + async function incrementViaTransaction() { + deferred = new Deferred(); + await ref.transaction(currentData => { + return (currentData as number) + 1; + }); + // Wait for the snapshot listener to fire. They are not invoked inline + // for transactions. + await deferred.promise; + } + + expect(latestValue).to.equal(0); + + await incrementViaTransaction(); + expect(latestValue).to.equal(1); + await incrementViaTransaction(); + expect(latestValue).to.equal(2); + }); }); From e78e813e47a523e556a8d172fd5ea06273ade904 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Jul 2021 15:15:45 -0600 Subject: [PATCH 2/3] Lint --- packages/database/test/exp/integration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 823d03e0047..c2e276d8794 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -17,6 +17,7 @@ // eslint-disable-next-line import/no-extraneous-dependencies import { initializeApp, deleteApp } from '@firebase/app-exp'; +import { Deferred } from '@firebase/util'; import { expect } from 'chai'; import { @@ -32,7 +33,6 @@ import { import { onValue, set } from '../../src/exp/Reference_impl'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; import { DATABASE_ADDRESS, DATABASE_URL } from '../helpers/util'; -import { Deferred } from '@firebase/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); From 7ff906ddc799450ffed68c20d321c4a1f7ed0d0d Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Jul 2021 15:48:27 -0600 Subject: [PATCH 3/3] Fix tests --- packages/database/src/core/view/EventRegistration.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/database/src/core/view/EventRegistration.ts b/packages/database/src/core/view/EventRegistration.ts index e9377dc843d..1a814163e24 100644 --- a/packages/database/src/core/view/EventRegistration.ts +++ b/packages/database/src/core/view/EventRegistration.ts @@ -74,7 +74,6 @@ export class CallbackContext { (this.snapshotCallback.userCallback !== undefined && this.snapshotCallback.userCallback === other.snapshotCallback.userCallback && - this.snapshotCallback.context !== undefined && this.snapshotCallback.context === other.snapshotCallback.context) ); }