From db50631498674285061e35f9b613a0dedf8ad1c2 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 13 May 2022 09:08:47 -0700 Subject: [PATCH 01/52] WIP --- packages/database/src/core/Repo.ts | 36 ++++++++++++++++----- packages/database/src/core/SyncTree.ts | 44 +++++++++++++++++--------- 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 9479f620529..2e6201d4800 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -48,10 +48,13 @@ import { statsManagerGetOrCreateReporter } from './stats/StatsManager'; import { StatsReporter, statsReporterIncludeStat } from './stats/StatsReporter'; +import { syncPointGetView, syncPointViewExistsForQuery, syncPointViewForQuery } from './SyncPoint'; import { + createNewTag, SyncTree, syncTreeAckUserWrite, syncTreeAddEventRegistration, + syncTreeAddToPath, syncTreeApplyServerMerge, syncTreeApplyServerOverwrite, syncTreeApplyTaggedQueryMerge, @@ -60,7 +63,9 @@ import { syncTreeApplyUserOverwrite, syncTreeCalcCompleteEventCache, syncTreeGetServerValue, - syncTreeRemoveEventRegistration + syncTreeMakeQueryKey_, + syncTreeRemoveEventRegistration, + syncTreeTagForQuery_ } from './SyncTree'; import { Indexable } from './util/misc'; import { @@ -466,15 +471,30 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { } return repo.server_.get(query).then( payload => { - const node = nodeFromJSON(payload as string).withIndex( + const node = nodeFromJSON(payload).withIndex( query._queryParams.getIndex() ); - const events = syncTreeApplyServerOverwrite( - repo.serverSyncTree_, - query._path, - node - ); - eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); + // if this is not a filtered query, then overwrite at path + if(query._queryParams.loadsAllData()) { + const events = syncTreeApplyServerOverwrite( + repo.serverSyncTree_, + query._path, + node + ); + eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); + } else { + // Otherwise, only overwrite for query + console.log('query') + syncTreeAddToPath(query, repo.serverSyncTree_); + const tag = syncTreeTagForQuery_(repo.serverSyncTree_, query); + const events = syncTreeApplyTaggedQueryOverwrite( + repo.serverSyncTree_, + query._path, + node, + tag + ); + eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); + } return Promise.resolve(node); }, err => { diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index a2d0888f03c..1b8884cacdd 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -482,16 +482,14 @@ export function syncTreeApplyTaggedQueryMerge( } } -/** - * Add an event callback for the specified query. - * - * @returns Events to raise. - */ -export function syncTreeAddEventRegistration( - syncTree: SyncTree, - query: QueryContext, - eventRegistration: EventRegistration -): Event[] { +export function createNewTag(syncTree: SyncTree, queryKey: string) { + const tag = syncTreeGetNextQueryTag_(); + syncTree.queryToTagMap.set(queryKey, tag); + syncTree.tagToQueryMap.set(tag, queryKey); + return tag; +} + +export function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { const path = query._path; let serverCache: Node | null = null; @@ -537,6 +535,7 @@ export function syncTreeAddEventRegistration( }); } + const viewAlreadyExists = syncPointViewExistsForQuery(syncPoint, query); if (!viewAlreadyExists && !query._queryParams.loadsAllData()) { // We need to track a tag for this query @@ -545,11 +544,26 @@ export function syncTreeAddEventRegistration( !syncTree.queryToTagMap.has(queryKey), 'View does not exist, but we have a tag' ); - const tag = syncTreeGetNextQueryTag_(); - syncTree.queryToTagMap.set(queryKey, tag); - syncTree.tagToQueryMap.set(tag, queryKey); + createNewTag(syncTree, queryKey); } const writesCache = writeTreeChildWrites(syncTree.pendingWriteTree_, path); + // TODO: break this down so you do the minimal amount + return { syncPoint, writesCache, serverCache, serverCacheComplete, foundAncestorDefaultView, viewAlreadyExists }; +} + +/** + * Add an event callback for the specified query. + * + * @returns Events to raise. + */ +export function syncTreeAddEventRegistration( + syncTree: SyncTree, + query: QueryContext, + eventRegistration: EventRegistration +): Event[] { + + const { syncPoint, serverCache, writesCache, serverCacheComplete, viewAlreadyExists, foundAncestorDefaultView } = syncTreeAddToPath(query, syncTree); + let events = syncPointAddEventRegistration( syncPoint, query, @@ -803,7 +817,7 @@ function syncTreeCreateListenerForView_( /** * Return the tag associated with the given query. */ -function syncTreeTagForQuery_( +export function syncTreeTagForQuery_( syncTree: SyncTree, query: QueryContext ): number | null { @@ -814,7 +828,7 @@ function syncTreeTagForQuery_( /** * Given a query, computes a "queryKey" suitable for use in our queryToTagMap_. */ -function syncTreeMakeQueryKey_(query: QueryContext): string { +export function syncTreeMakeQueryKey_(query: QueryContext): string { return query._path.toString() + '$' + query._queryIdentifier; } From 46cd3781b4e34fb5d4171c797152789dd220ce98 Mon Sep 17 00:00:00 2001 From: Jan Wyszynski Date: Fri, 13 May 2022 10:25:42 -0700 Subject: [PATCH 02/52] Simulate add/remove event registration in repoGetValue --- packages/database/src/core/Repo.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 2e6201d4800..dbb9657cda6 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -48,7 +48,7 @@ import { statsManagerGetOrCreateReporter } from './stats/StatsManager'; import { StatsReporter, statsReporterIncludeStat } from './stats/StatsReporter'; -import { syncPointGetView, syncPointViewExistsForQuery, syncPointViewForQuery } from './SyncPoint'; +import { syncPointGetView, syncPointIsEmpty, syncPointViewExistsForQuery, syncPointViewForQuery } from './SyncPoint'; import { createNewTag, SyncTree, @@ -483,9 +483,15 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { ); eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); } else { - // Otherwise, only overwrite for query - console.log('query') - syncTreeAddToPath(query, repo.serverSyncTree_); + // Simulate `syncTreeAddEventRegistration` without events/listener setup. + // TODO: We can probably extract this. + const { syncPoint, serverCache, writesCache, serverCacheComplete, viewAlreadyExists, foundAncestorDefaultView } = syncTreeAddToPath(query, repo.serverSyncTree_); + if (!viewAlreadyExists && !foundAncestorDefaultView) { + const view = syncPointGetView(syncPoint, query, writesCache, serverCache, serverCacheComplete); + if (!syncPoint.views.has(query._queryIdentifier)) { + syncPoint.views.set(query._queryIdentifier, view); + } + } const tag = syncTreeTagForQuery_(repo.serverSyncTree_, query); const events = syncTreeApplyTaggedQueryOverwrite( repo.serverSyncTree_, @@ -494,6 +500,9 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { tag ); eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); + // Call `syncTreeRemoveEventRegistration` with a null event registration, since there is none. + const cancels = syncTreeRemoveEventRegistration(repo.serverSyncTree_, query, null) + assert(cancels.length == 0, "unexpected cancel events in repoGetValue"); } return Promise.resolve(node); }, From c3341715e005ed228acf3ccc625223c7656f64ab Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Tue, 17 May 2022 08:25:17 -0700 Subject: [PATCH 03/52] Fixed duplicate onValue calls --- packages/database/src/core/PersistentConnection.ts | 6 ------ packages/database/src/core/Repo.ts | 10 ++++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/database/src/core/PersistentConnection.ts b/packages/database/src/core/PersistentConnection.ts index 62b27a37686..95bdd9d9041 100644 --- a/packages/database/src/core/PersistentConnection.ts +++ b/packages/database/src/core/PersistentConnection.ts @@ -206,12 +206,6 @@ export class PersistentConnection extends ServerActions { onComplete: (message: { [k: string]: unknown }) => { const payload = message['d'] as string; if (message['s'] === 'ok') { - this.onDataUpdate_( - request['p'], - payload, - /*isMerge*/ false, - /*tag*/ null - ); deferred.resolve(payload); } else { deferred.reject(payload); diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index dbb9657cda6..7b6b89e5907 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -485,12 +485,10 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { } else { // Simulate `syncTreeAddEventRegistration` without events/listener setup. // TODO: We can probably extract this. - const { syncPoint, serverCache, writesCache, serverCacheComplete, viewAlreadyExists, foundAncestorDefaultView } = syncTreeAddToPath(query, repo.serverSyncTree_); - if (!viewAlreadyExists && !foundAncestorDefaultView) { - const view = syncPointGetView(syncPoint, query, writesCache, serverCache, serverCacheComplete); - if (!syncPoint.views.has(query._queryIdentifier)) { - syncPoint.views.set(query._queryIdentifier, view); - } + const { syncPoint, serverCache, writesCache, serverCacheComplete } = syncTreeAddToPath(query, repo.serverSyncTree_); + const view = syncPointGetView(syncPoint, query, writesCache, serverCache, serverCacheComplete); + if (!syncPoint.views.has(query._queryIdentifier)) { + syncPoint.views.set(query._queryIdentifier, view); } const tag = syncTreeTagForQuery_(repo.serverSyncTree_, query); const events = syncTreeApplyTaggedQueryOverwrite( From f7298df417c721ac43fa880f75941d8affd5ca10 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 18 May 2022 07:48:06 -0700 Subject: [PATCH 04/52] Updated test file --- packages/database-compat/test/get.test.ts | 45 +++++++++++++++++++++++ packages/database/src/core/Repo.ts | 25 +++++++++---- packages/database/src/core/SyncTree.ts | 20 ++++++++-- 3 files changed, 78 insertions(+), 12 deletions(-) create mode 100644 packages/database-compat/test/get.test.ts diff --git a/packages/database-compat/test/get.test.ts b/packages/database-compat/test/get.test.ts new file mode 100644 index 00000000000..31d47a43be1 --- /dev/null +++ b/packages/database-compat/test/get.test.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright 2017 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; + +import { Reference } from '../src/api/Reference'; + +import { getRandomNode } from './helpers/util'; + +describe.only('get tests', () => { + // TODO: setup spy on console.warn + + const clearRef = getRandomNode() as Reference; + + it('get should only trigger query listeners', async () => { + const ref = getRandomNode() as Reference; + const initial = { + a: 1, + b: 2 + }; + ref.set(initial); + let valueCt = 0; + ref.on('value', () => { + valueCt++; + }); + ref.limitToFirst(1).get(); + // Note: This is a real timeout, and if there is some issue with network conditions, this may fail unexpectedly + await new Promise(resolve => setTimeout(resolve, 3000)); + expect(valueCt).to.equal(1); + }); +}); diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 7b6b89e5907..89c5043d85a 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -48,9 +48,8 @@ import { statsManagerGetOrCreateReporter } from './stats/StatsManager'; import { StatsReporter, statsReporterIncludeStat } from './stats/StatsReporter'; -import { syncPointGetView, syncPointIsEmpty, syncPointViewExistsForQuery, syncPointViewForQuery } from './SyncPoint'; +import { syncPointGetView } from './SyncPoint'; import { - createNewTag, SyncTree, syncTreeAckUserWrite, syncTreeAddEventRegistration, @@ -63,7 +62,6 @@ import { syncTreeApplyUserOverwrite, syncTreeCalcCompleteEventCache, syncTreeGetServerValue, - syncTreeMakeQueryKey_, syncTreeRemoveEventRegistration, syncTreeTagForQuery_ } from './SyncTree'; @@ -475,7 +473,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { query._queryParams.getIndex() ); // if this is not a filtered query, then overwrite at path - if(query._queryParams.loadsAllData()) { + if (query._queryParams.loadsAllData()) { const events = syncTreeApplyServerOverwrite( repo.serverSyncTree_, query._path, @@ -485,8 +483,15 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { } else { // Simulate `syncTreeAddEventRegistration` without events/listener setup. // TODO: We can probably extract this. - const { syncPoint, serverCache, writesCache, serverCacheComplete } = syncTreeAddToPath(query, repo.serverSyncTree_); - const view = syncPointGetView(syncPoint, query, writesCache, serverCache, serverCacheComplete); + const { syncPoint, serverCache, writesCache, serverCacheComplete } = + syncTreeAddToPath(query, repo.serverSyncTree_); + const view = syncPointGetView( + syncPoint, + query, + writesCache, + serverCache, + serverCacheComplete + ); if (!syncPoint.views.has(query._queryIdentifier)) { syncPoint.views.set(query._queryIdentifier, view); } @@ -499,8 +504,12 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { ); eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); // Call `syncTreeRemoveEventRegistration` with a null event registration, since there is none. - const cancels = syncTreeRemoveEventRegistration(repo.serverSyncTree_, query, null) - assert(cancels.length == 0, "unexpected cancel events in repoGetValue"); + const cancels = syncTreeRemoveEventRegistration( + repo.serverSyncTree_, + query, + null + ); + assert(cancels.length == 0, 'unexpected cancel events in repoGetValue'); } return Promise.resolve(node); }, diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 1b8884cacdd..4e9f7ed0f20 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -535,7 +535,6 @@ export function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { }); } - const viewAlreadyExists = syncPointViewExistsForQuery(syncPoint, query); if (!viewAlreadyExists && !query._queryParams.loadsAllData()) { // We need to track a tag for this query @@ -548,7 +547,14 @@ export function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { } const writesCache = writeTreeChildWrites(syncTree.pendingWriteTree_, path); // TODO: break this down so you do the minimal amount - return { syncPoint, writesCache, serverCache, serverCacheComplete, foundAncestorDefaultView, viewAlreadyExists }; + return { + syncPoint, + writesCache, + serverCache, + serverCacheComplete, + foundAncestorDefaultView, + viewAlreadyExists + }; } /** @@ -561,8 +567,14 @@ export function syncTreeAddEventRegistration( query: QueryContext, eventRegistration: EventRegistration ): Event[] { - - const { syncPoint, serverCache, writesCache, serverCacheComplete, viewAlreadyExists, foundAncestorDefaultView } = syncTreeAddToPath(query, syncTree); + const { + syncPoint, + serverCache, + writesCache, + serverCacheComplete, + viewAlreadyExists, + foundAncestorDefaultView + } = syncTreeAddToPath(query, syncTree); let events = syncPointAddEventRegistration( syncPoint, From 00a59071c808c84bbc966c59bade1003645fea6e Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 19 May 2022 15:03:50 -0700 Subject: [PATCH 05/52] Updated integration test to include get --- packages/database-compat/src/api/Reference.ts | 2 +- packages/database-compat/test/get.test.ts | 45 ------------------- packages/database/src/api/Reference_impl.ts | 1 + packages/database/src/core/Repo.ts | 2 +- .../database/test/exp/integration.test.ts | 22 ++++++++- packages/database/test/helpers/util.ts | 6 +++ 6 files changed, 29 insertions(+), 49 deletions(-) delete mode 100644 packages/database-compat/test/get.test.ts diff --git a/packages/database-compat/src/api/Reference.ts b/packages/database-compat/src/api/Reference.ts index 3f9bb63b8ff..43f417ad0fa 100644 --- a/packages/database-compat/src/api/Reference.ts +++ b/packages/database-compat/src/api/Reference.ts @@ -36,7 +36,7 @@ import { endAt, endBefore, equalTo, - get, +get, set, update, setWithPriority, diff --git a/packages/database-compat/test/get.test.ts b/packages/database-compat/test/get.test.ts deleted file mode 100644 index 31d47a43be1..00000000000 --- a/packages/database-compat/test/get.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -/** - * @license - * Copyright 2017 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { expect } from 'chai'; - -import { Reference } from '../src/api/Reference'; - -import { getRandomNode } from './helpers/util'; - -describe.only('get tests', () => { - // TODO: setup spy on console.warn - - const clearRef = getRandomNode() as Reference; - - it('get should only trigger query listeners', async () => { - const ref = getRandomNode() as Reference; - const initial = { - a: 1, - b: 2 - }; - ref.set(initial); - let valueCt = 0; - ref.on('value', () => { - valueCt++; - }); - ref.limitToFirst(1).get(); - // Note: This is a real timeout, and if there is some issue with network conditions, this may fail unexpectedly - await new Promise(resolve => setTimeout(resolve, 3000)); - expect(valueCt).to.equal(1); - }); -}); diff --git a/packages/database/src/api/Reference_impl.ts b/packages/database/src/api/Reference_impl.ts index 1b363f2bbe9..92d4d59ba30 100644 --- a/packages/database/src/api/Reference_impl.ts +++ b/packages/database/src/api/Reference_impl.ts @@ -808,6 +808,7 @@ export function update(ref: DatabaseReference, values: object): Promise { * server is unreachable and there is nothing cached). */ export function get(query: Query): Promise { + console.log('get'); query = getModularInstance(query) as QueryImpl; return repoGetValue(query._repo, query).then(node => { return new DataSnapshot( diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 89c5043d85a..d49c28fdbaa 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -509,7 +509,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { query, null ); - assert(cancels.length == 0, 'unexpected cancel events in repoGetValue'); + assert(cancels.length === 0, 'unexpected cancel events in repoGetValue'); } return Promise.resolve(node); }, diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 0d0858c28c8..b195f90577b 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -20,7 +20,7 @@ import { initializeApp, deleteApp } from '@firebase/app'; import { Deferred } from '@firebase/util'; import { expect } from 'chai'; -import { onValue, set } from '../../src/api/Reference_impl'; +import { limitToFirst, onValue, query, set } from '../../src/api/Reference_impl'; import { get, getDatabase, @@ -32,7 +32,7 @@ import { runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_ADDRESS, DATABASE_URL } from '../helpers/util'; +import { DATABASE_ADDRESS, DATABASE_URL, waitFor } from '../helpers/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -97,6 +97,24 @@ describe('Database@exp Tests', () => { expect(snap2).to.equal('b'); }); + // Tests to make sure onValue's data does not get mutated after calling get + it('calls onValue only once after get request', async () => { + const db = getDatabase(defaultApp); + const testRef = ref(db, 'foo'); + const initial = [ + { name: 'child1' }, + { name: 'child2' }, + ]; + await set(testRef, initial); + let count = 0; + onValue(testRef, (snapshot) => { + count++; + }); + await get(query(testRef, limitToFirst(1))); + await waitFor(4000); + expect(count).to.equal(1); + }); + it('Can use onlyOnce', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index d50d42bd311..32022cc1ede 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -70,3 +70,9 @@ export function shuffle(arr, randFn = Math.random) { arr[j] = tmp; } } + +// Waits for specific number of milliseconds before resolving +// Example: await waitFor(4000) will wait until 4 seconds to execute the next line of code. +export function waitFor(waitTimeInMS: number) { + return new Promise(resolve => setTimeout(resolve, waitTimeInMS)); +} From 3ea96047626b53b6d9684e704da29eed09bdce41 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 20 May 2022 07:54:35 -0700 Subject: [PATCH 06/52] Reduced timeout for tests --- packages/database-compat/src/api/Reference.ts | 2 +- packages/database/src/api/Reference_impl.ts | 1 - packages/database/src/core/Repo.ts | 5 ++++- packages/database/test/exp/integration.test.ts | 16 +++++++++------- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/database-compat/src/api/Reference.ts b/packages/database-compat/src/api/Reference.ts index 43f417ad0fa..3f9bb63b8ff 100644 --- a/packages/database-compat/src/api/Reference.ts +++ b/packages/database-compat/src/api/Reference.ts @@ -36,7 +36,7 @@ import { endAt, endBefore, equalTo, -get, + get, set, update, setWithPriority, diff --git a/packages/database/src/api/Reference_impl.ts b/packages/database/src/api/Reference_impl.ts index 92d4d59ba30..1b363f2bbe9 100644 --- a/packages/database/src/api/Reference_impl.ts +++ b/packages/database/src/api/Reference_impl.ts @@ -808,7 +808,6 @@ export function update(ref: DatabaseReference, values: object): Promise { * server is unreachable and there is nothing cached). */ export function get(query: Query): Promise { - console.log('get'); query = getModularInstance(query) as QueryImpl; return repoGetValue(query._repo, query).then(node => { return new DataSnapshot( diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index d49c28fdbaa..87d4551fc3a 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -509,7 +509,10 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { query, null ); - assert(cancels.length === 0, 'unexpected cancel events in repoGetValue'); + assert( + cancels.length === 0, + 'unexpected cancel events in repoGetValue' + ); } return Promise.resolve(node); }, diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index b195f90577b..14447b39de9 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -20,7 +20,12 @@ import { initializeApp, deleteApp } from '@firebase/app'; import { Deferred } from '@firebase/util'; import { expect } from 'chai'; -import { limitToFirst, onValue, query, set } from '../../src/api/Reference_impl'; +import { + limitToFirst, + onValue, + query, + set +} from '../../src/api/Reference_impl'; import { get, getDatabase, @@ -101,17 +106,14 @@ describe('Database@exp Tests', () => { it('calls onValue only once after get request', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); - const initial = [ - { name: 'child1' }, - { name: 'child2' }, - ]; + const initial = [{ name: 'child1' }, { name: 'child2' }]; await set(testRef, initial); let count = 0; - onValue(testRef, (snapshot) => { + onValue(testRef, snapshot => { count++; }); await get(query(testRef, limitToFirst(1))); - await waitFor(4000); + await waitFor(2000); expect(count).to.equal(1); }); From 9520427d8ee9301d3e43bfb4ffa021ecff76e53c Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 20 May 2022 08:16:58 -0700 Subject: [PATCH 07/52] Removed unnecessary imports --- packages/database/src/core/Repo.ts | 1 - packages/database/src/core/SyncTree.ts | 17 ++++++----------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 87d4551fc3a..331c5d698b9 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -482,7 +482,6 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); } else { // Simulate `syncTreeAddEventRegistration` without events/listener setup. - // TODO: We can probably extract this. const { syncPoint, serverCache, writesCache, serverCacheComplete } = syncTreeAddToPath(query, repo.serverSyncTree_); const view = syncPointGetView( diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 4e9f7ed0f20..0f03fe9203e 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -482,14 +482,7 @@ export function syncTreeApplyTaggedQueryMerge( } } -export function createNewTag(syncTree: SyncTree, queryKey: string) { - const tag = syncTreeGetNextQueryTag_(); - syncTree.queryToTagMap.set(queryKey, tag); - syncTree.tagToQueryMap.set(tag, queryKey); - return tag; -} - -export function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { +function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { const path = query._path; let serverCache: Node | null = null; @@ -543,7 +536,9 @@ export function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { !syncTree.queryToTagMap.has(queryKey), 'View does not exist, but we have a tag' ); - createNewTag(syncTree, queryKey); + const tag = syncTreeGetNextQueryTag_(); + syncTree.queryToTagMap.set(queryKey, tag); + syncTree.tagToQueryMap.set(tag, queryKey); } const writesCache = writeTreeChildWrites(syncTree.pendingWriteTree_, path); // TODO: break this down so you do the minimal amount @@ -829,7 +824,7 @@ function syncTreeCreateListenerForView_( /** * Return the tag associated with the given query. */ -export function syncTreeTagForQuery_( +function syncTreeTagForQuery_( syncTree: SyncTree, query: QueryContext ): number | null { @@ -840,7 +835,7 @@ export function syncTreeTagForQuery_( /** * Given a query, computes a "queryKey" suitable for use in our queryToTagMap_. */ -export function syncTreeMakeQueryKey_(query: QueryContext): string { +function syncTreeMakeQueryKey_(query: QueryContext): string { return query._path.toString() + '$' + query._queryIdentifier; } From e7dd6510600cc6d4df779907f370378d31827d7f Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 20 May 2022 08:19:33 -0700 Subject: [PATCH 08/52] Added back imports --- packages/database/src/core/SyncTree.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 0f03fe9203e..8ee5177a80a 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -482,7 +482,7 @@ export function syncTreeApplyTaggedQueryMerge( } } -function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { +export function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { const path = query._path; let serverCache: Node | null = null; @@ -824,7 +824,7 @@ function syncTreeCreateListenerForView_( /** * Return the tag associated with the given query. */ -function syncTreeTagForQuery_( +export function syncTreeTagForQuery_( syncTree: SyncTree, query: QueryContext ): number | null { From f7ed5761228a73f57662f901bd89a372f8c50e35 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 20 May 2022 08:28:00 -0700 Subject: [PATCH 09/52] Removed snapshot param --- 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 14447b39de9..0e10f15c7a3 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -109,7 +109,7 @@ describe('Database@exp Tests', () => { const initial = [{ name: 'child1' }, { name: 'child2' }]; await set(testRef, initial); let count = 0; - onValue(testRef, snapshot => { + onValue(testRef, () => { count++; }); await get(query(testRef, limitToFirst(1))); From 98c6e342e93e53b20b30052fd80f14bae4c2fc3d Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 23 May 2022 15:20:05 -0700 Subject: [PATCH 10/52] Addressed comments --- packages/database/src/core/Repo.ts | 29 ++++--------------- packages/database/src/core/SyncTree.ts | 27 +++++++++++++++-- packages/database/src/core/view/EventQueue.ts | 2 ++ .../database/test/exp/integration.test.ts | 22 ++++++++++---- 4 files changed, 49 insertions(+), 31 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 331c5d698b9..2694c5e8e58 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -53,7 +53,7 @@ import { SyncTree, syncTreeAckUserWrite, syncTreeAddEventRegistration, - syncTreeAddToPath, + syncTreeRegisterSyncPoint, syncTreeApplyServerMerge, syncTreeApplyServerOverwrite, syncTreeApplyTaggedQueryMerge, @@ -63,7 +63,8 @@ import { syncTreeCalcCompleteEventCache, syncTreeGetServerValue, syncTreeRemoveEventRegistration, - syncTreeTagForQuery_ + syncTreeTagForQuery_, + syncTreeRegisterQuery } from './SyncTree'; import { Indexable } from './util/misc'; import { @@ -467,6 +468,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cached != null) { return Promise.resolve(cached); } + let tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); return repo.server_.get(query).then( payload => { const node = nodeFromJSON(payload).withIndex( @@ -474,34 +476,15 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { ); // if this is not a filtered query, then overwrite at path if (query._queryParams.loadsAllData()) { - const events = syncTreeApplyServerOverwrite( - repo.serverSyncTree_, - query._path, - node - ); - eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); + syncTreeApplyServerOverwrite(repo.serverSyncTree_, query._path, node); } else { // Simulate `syncTreeAddEventRegistration` without events/listener setup. - const { syncPoint, serverCache, writesCache, serverCacheComplete } = - syncTreeAddToPath(query, repo.serverSyncTree_); - const view = syncPointGetView( - syncPoint, - query, - writesCache, - serverCache, - serverCacheComplete - ); - if (!syncPoint.views.has(query._queryIdentifier)) { - syncPoint.views.set(query._queryIdentifier, view); - } - const tag = syncTreeTagForQuery_(repo.serverSyncTree_, query); - const events = syncTreeApplyTaggedQueryOverwrite( + syncTreeApplyTaggedQueryOverwrite( repo.serverSyncTree_, query._path, node, tag ); - eventQueueRaiseEventsAtPath(repo.eventQueue_, query._path, events); // Call `syncTreeRemoveEventRegistration` with a null event registration, since there is none. const cancels = syncTreeRemoveEventRegistration( repo.serverSyncTree_, diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 8ee5177a80a..055fd9a1aaa 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -423,6 +423,25 @@ export function syncTreeRemoveEventRegistration( return cancelEvents; } +export function syncTreeRegisterQuery(syncTree: SyncTree, query: QueryContext) { + if (!query._queryParams.loadsAllData()) { + const { syncPoint, serverCache, writesCache, serverCacheComplete } = + syncTreeRegisterSyncPoint(query, syncTree); + const view = syncPointGetView( + syncPoint, + query, + writesCache, + serverCache, + serverCacheComplete + ); + if (!syncPoint.views.has(query._queryIdentifier)) { + syncPoint.views.set(query._queryIdentifier, view); + } + return syncTreeTagForQuery_(syncTree, query); + } + return null; +} + /** * Apply new server data for the specified tagged query. * @@ -482,7 +501,10 @@ export function syncTreeApplyTaggedQueryMerge( } } -export function syncTreeAddToPath(query: QueryContext, syncTree: SyncTree) { +export function syncTreeRegisterSyncPoint( + query: QueryContext, + syncTree: SyncTree +) { const path = query._path; let serverCache: Node | null = null; @@ -569,7 +591,7 @@ export function syncTreeAddEventRegistration( serverCacheComplete, viewAlreadyExists, foundAncestorDefaultView - } = syncTreeAddToPath(query, syncTree); + } = syncTreeRegisterSyncPoint(query, syncTree); let events = syncPointAddEventRegistration( syncPoint, @@ -958,7 +980,6 @@ function syncTreeSetupListener_( const path = query._path; const tag = syncTreeTagForQuery_(syncTree, query); const listener = syncTreeCreateListenerForView_(syncTree, view); - const events = syncTree.listenProvider_.startListening( syncTreeQueryForListening_(query), tag, diff --git a/packages/database/src/core/view/EventQueue.ts b/packages/database/src/core/view/EventQueue.ts index dc62b5d1c51..fabe221f65e 100644 --- a/packages/database/src/core/view/EventQueue.ts +++ b/packages/database/src/core/view/EventQueue.ts @@ -84,6 +84,7 @@ export function eventQueueRaiseEventsAtPath( path: Path, eventDataList: Event[] ) { + debugger; eventQueueQueueEvents(eventQueue, eventDataList); eventQueueRaiseQueuedEventsMatchingPredicate(eventQueue, eventPath => pathEquals(eventPath, path) @@ -104,6 +105,7 @@ export function eventQueueRaiseEventsForChangedPath( changedPath: Path, eventDataList: Event[] ) { + debugger; eventQueueQueueEvents(eventQueue, eventDataList); eventQueueRaiseQueuedEventsMatchingPredicate( eventQueue, diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 0e10f15c7a3..b97ca101051 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -19,6 +19,7 @@ import { initializeApp, deleteApp } from '@firebase/app'; import { Deferred } from '@firebase/util'; import { expect } from 'chai'; +import Sinon, { createSandbox } from 'sinon'; import { limitToFirst, @@ -26,6 +27,7 @@ import { query, set } from '../../src/api/Reference_impl'; +import * as EventQueue from '../../src/core/view/EventQueue'; import { get, getDatabase, @@ -43,14 +45,17 @@ export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } -describe('Database@exp Tests', () => { +describe.only('Database@exp Tests', () => { let defaultApp; + let mySandbox: Sinon.SinonSandbox; beforeEach(() => { defaultApp = createTestApp(); + mySandbox = createSandbox(); }); afterEach(async () => { + mySandbox.restore(); if (defaultApp) { return deleteApp(defaultApp); } @@ -103,16 +108,23 @@ describe('Database@exp Tests', () => { }); // Tests to make sure onValue's data does not get mutated after calling get - it('calls onValue only once after get request', async () => { + it.only('calls onValue only once after get request', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; - await set(testRef, initial); + let count = 0; - onValue(testRef, () => { + const events = []; + onValue(testRef, snapshot => { + // expect(snapshot.val()).to.eq(initial); + events.push(snapshot.val()); count++; }); - await get(query(testRef, limitToFirst(1))); + await set(testRef, initial); + const newValues = [{ name: 'child1' }, { name: 'child3' }]; + const setPromise = set(testRef, newValues); + const getPromise = get(query(testRef, limitToFirst(1))); + await Promise.all([getPromise, setPromise]); await waitFor(2000); expect(count).to.equal(1); }); From 43f1f8a2d85c4294d7aeb7259e22643625d76151 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 23 May 2022 15:26:29 -0700 Subject: [PATCH 11/52] Fixed test --- packages/database/src/core/Repo.ts | 5 +---- packages/database/src/core/view/EventQueue.ts | 2 -- packages/database/test/exp/integration.test.ts | 10 ++-------- 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 2694c5e8e58..4b4c20be162 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -48,12 +48,10 @@ import { statsManagerGetOrCreateReporter } from './stats/StatsManager'; import { StatsReporter, statsReporterIncludeStat } from './stats/StatsReporter'; -import { syncPointGetView } from './SyncPoint'; import { SyncTree, syncTreeAckUserWrite, syncTreeAddEventRegistration, - syncTreeRegisterSyncPoint, syncTreeApplyServerMerge, syncTreeApplyServerOverwrite, syncTreeApplyTaggedQueryMerge, @@ -63,7 +61,6 @@ import { syncTreeCalcCompleteEventCache, syncTreeGetServerValue, syncTreeRemoveEventRegistration, - syncTreeTagForQuery_, syncTreeRegisterQuery } from './SyncTree'; import { Indexable } from './util/misc'; @@ -468,7 +465,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cached != null) { return Promise.resolve(cached); } - let tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); + const tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); return repo.server_.get(query).then( payload => { const node = nodeFromJSON(payload).withIndex( diff --git a/packages/database/src/core/view/EventQueue.ts b/packages/database/src/core/view/EventQueue.ts index fabe221f65e..dc62b5d1c51 100644 --- a/packages/database/src/core/view/EventQueue.ts +++ b/packages/database/src/core/view/EventQueue.ts @@ -84,7 +84,6 @@ export function eventQueueRaiseEventsAtPath( path: Path, eventDataList: Event[] ) { - debugger; eventQueueQueueEvents(eventQueue, eventDataList); eventQueueRaiseQueuedEventsMatchingPredicate(eventQueue, eventPath => pathEquals(eventPath, path) @@ -105,7 +104,6 @@ export function eventQueueRaiseEventsForChangedPath( changedPath: Path, eventDataList: Event[] ) { - debugger; eventQueueQueueEvents(eventQueue, eventDataList); eventQueueRaiseQueuedEventsMatchingPredicate( eventQueue, diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index b97ca101051..2826ab00790 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -27,7 +27,6 @@ import { query, set } from '../../src/api/Reference_impl'; -import * as EventQueue from '../../src/core/view/EventQueue'; import { get, getDatabase, @@ -114,17 +113,12 @@ describe.only('Database@exp Tests', () => { const initial = [{ name: 'child1' }, { name: 'child2' }]; let count = 0; - const events = []; onValue(testRef, snapshot => { - // expect(snapshot.val()).to.eq(initial); - events.push(snapshot.val()); + expect(snapshot.val()).to.deep.eq(initial); count++; }); await set(testRef, initial); - const newValues = [{ name: 'child1' }, { name: 'child3' }]; - const setPromise = set(testRef, newValues); - const getPromise = get(query(testRef, limitToFirst(1))); - await Promise.all([getPromise, setPromise]); + await get(query(testRef, limitToFirst(1))); await waitFor(2000); expect(count).to.equal(1); }); From 9394170d45307f0f7877826e0239de3f65960044 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 23 May 2022 15:26:49 -0700 Subject: [PATCH 12/52] Removed only --- packages/database/test/exp/integration.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 2826ab00790..6f3152fb91e 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -44,7 +44,7 @@ export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } -describe.only('Database@exp Tests', () => { +describe('Database@exp Tests', () => { let defaultApp; let mySandbox: Sinon.SinonSandbox; @@ -107,7 +107,7 @@ describe.only('Database@exp Tests', () => { }); // Tests to make sure onValue's data does not get mutated after calling get - it.only('calls onValue only once after get request', async () => { + it('calls onValue only once after get request', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; From 2f820521e7373c690ca1632b9644818ad5576e61 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 23 May 2022 15:44:10 -0700 Subject: [PATCH 13/52] Removed unnecessary import --- packages/database/src/core/SyncTree.ts | 2 +- packages/database/test/exp/integration.test.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 055fd9a1aaa..88772eac420 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -846,7 +846,7 @@ function syncTreeCreateListenerForView_( /** * Return the tag associated with the given query. */ -export function syncTreeTagForQuery_( +function syncTreeTagForQuery_( syncTree: SyncTree, query: QueryContext ): number | null { diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 6f3152fb91e..cc4784bce82 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -// eslint-disable-next-line import/no-extraneous-dependencies import { initializeApp, deleteApp } from '@firebase/app'; import { Deferred } from '@firebase/util'; import { expect } from 'chai'; From c976987693bdab0757eafb77ac4c5768a38a559c Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 23 May 2022 15:53:30 -0700 Subject: [PATCH 14/52] Updated test --- 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 cc4784bce82..c740659b97a 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -112,11 +112,11 @@ describe('Database@exp Tests', () => { const initial = [{ name: 'child1' }, { name: 'child2' }]; let count = 0; + await set(testRef, initial); onValue(testRef, snapshot => { expect(snapshot.val()).to.deep.eq(initial); count++; }); - await set(testRef, initial); await get(query(testRef, limitToFirst(1))); await waitFor(2000); expect(count).to.equal(1); From fc4cf5c58485d74e904b475ffb634af21ac81a55 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Tue, 24 May 2022 15:47:18 -0700 Subject: [PATCH 15/52] Added extra test --- packages/database/src/core/util/Path.ts | 2 +- packages/database/test/exp/integration.test.ts | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/database/src/core/util/Path.ts b/packages/database/src/core/util/Path.ts index c0ddcf5061b..6aba15bb98f 100644 --- a/packages/database/src/core/util/Path.ts +++ b/packages/database/src/core/util/Path.ts @@ -230,7 +230,7 @@ export function pathEquals(path: Path, other: Path): boolean { } /** - * @returns True if this path is a parent (or the same as) other + * @returns True if this path is a parent of (or the same as) other */ export function pathContains(path: Path, other: Path): boolean { let i = path.pieceNum_; diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index c740659b97a..e50821719ad 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -106,7 +106,7 @@ describe('Database@exp Tests', () => { }); // Tests to make sure onValue's data does not get mutated after calling get - it('calls onValue only once after get request', async () => { + it('calls onValue only once after get request with a non-default query', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; @@ -122,6 +122,22 @@ describe('Database@exp Tests', () => { expect(count).to.equal(1); }); + it('calls onValue only once after get request with a default query', async() => { + const db = getDatabase(defaultApp); + const testRef = ref(db, 'foo'); + const initial = [{ name: 'child1' }, { name: 'child2' }]; + + let count = 0; + await set(testRef, initial); + onValue(testRef, snapshot => { + expect(snapshot.val()).to.deep.eq(initial); + count++; + }); + await get(query(testRef)); + await waitFor(2000); + expect(count).to.equal(1); + }); + it('Can use onlyOnce', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); From 3959cf36267b7f615631e6bbe8ec57f8490a6cb4 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 25 May 2022 09:25:01 -0700 Subject: [PATCH 16/52] Fixed formatting --- 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 e50821719ad..fe3f2341bc5 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -122,7 +122,7 @@ describe('Database@exp Tests', () => { expect(count).to.equal(1); }); - it('calls onValue only once after get request with a default query', async() => { + it('calls onValue only once after get request with a default query', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; From 536e1030f4303f6931d0e4ff57944877b1aa7a2b Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 25 May 2022 15:43:29 -0700 Subject: [PATCH 17/52] Create rotten-tables-brush.md --- .changeset/rotten-tables-brush.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rotten-tables-brush.md diff --git a/.changeset/rotten-tables-brush.md b/.changeset/rotten-tables-brush.md new file mode 100644 index 00000000000..23716047a56 --- /dev/null +++ b/.changeset/rotten-tables-brush.md @@ -0,0 +1,5 @@ +--- +"@firebase/database": patch +--- + +Fixed issue where `get()` updated incorrect cache. From 3812a33bcd69bee4c262cdeec3c4d0a2d6cfa758 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 25 May 2022 15:48:25 -0700 Subject: [PATCH 18/52] Added documentation --- packages/database/src/core/Repo.ts | 7 +++++-- packages/database/src/core/SyncTree.ts | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 4b4c20be162..2f803da45f0 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -471,11 +471,14 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { const node = nodeFromJSON(payload).withIndex( query._queryParams.getIndex() ); - // if this is not a filtered query, then overwrite at path + // if this is a filtered query, then overwrite at path if (query._queryParams.loadsAllData()) { syncTreeApplyServerOverwrite(repo.serverSyncTree_, query._path, node); } else { - // Simulate `syncTreeAddEventRegistration` without events/listener setup. + // Simulate `syncTreeAddEventRegistration` without events/listener setup. + // We do this (along with the syncTreeRemoveEventRegistration` below) so that + // `repoGetValue` results have the same cache effects as initial listeners + // updates. syncTreeApplyTaggedQueryOverwrite( repo.serverSyncTree_, query._path, diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 88772eac420..4f9cfdcc864 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -423,6 +423,19 @@ export function syncTreeRemoveEventRegistration( return cancelEvents; } +/** + * This function was added to support non-listener queries, + * specifically for use in repoGetValue. It sets up all the same + * local cache data-structures (SyncPoint + View) that are + * needed for listeners without installing an event registration. + * If `query` is not `loadsAllData`, it will also provision a tag for + * the query so that query results can be merged into the sync + * tree using existing logic for tagged listener queries. + * + * @param syncTree - Synctree to add the query to. + * @param query - Query to register + * @returns tag as a string if query is not a default query, null if query is not. + */ export function syncTreeRegisterQuery(syncTree: SyncTree, query: QueryContext) { if (!query._queryParams.loadsAllData()) { const { syncPoint, serverCache, writesCache, serverCacheComplete } = @@ -501,6 +514,11 @@ export function syncTreeApplyTaggedQueryMerge( } } +/** + * Creates a new syncpoint for a query and creates a tag if the view doesn't exist. + * Extracted from addEventRegistration to allow `repoGetValue` to properly set up the SyncTree + * without actually listening on a query. + */ export function syncTreeRegisterSyncPoint( query: QueryContext, syncTree: SyncTree @@ -563,7 +581,6 @@ export function syncTreeRegisterSyncPoint( syncTree.tagToQueryMap.set(tag, queryKey); } const writesCache = writeTreeChildWrites(syncTree.pendingWriteTree_, path); - // TODO: break this down so you do the minimal amount return { syncPoint, writesCache, From a8d8ea6be459059d43692a7d221d7fa772772fa0 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 25 May 2022 15:51:01 -0700 Subject: [PATCH 19/52] Passed formatting --- packages/database/src/core/Repo.ts | 6 +++--- packages/database/src/core/SyncTree.ts | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 2f803da45f0..2228d78c666 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -475,9 +475,9 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (query._queryParams.loadsAllData()) { syncTreeApplyServerOverwrite(repo.serverSyncTree_, query._path, node); } else { - // Simulate `syncTreeAddEventRegistration` without events/listener setup. - // We do this (along with the syncTreeRemoveEventRegistration` below) so that - // `repoGetValue` results have the same cache effects as initial listeners + // Simulate `syncTreeAddEventRegistration` without events/listener setup. + // We do this (along with the syncTreeRemoveEventRegistration` below) so that + // `repoGetValue` results have the same cache effects as initial listeners // updates. syncTreeApplyTaggedQueryOverwrite( repo.serverSyncTree_, diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 4f9cfdcc864..7204a9fd9b9 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -425,12 +425,12 @@ export function syncTreeRemoveEventRegistration( /** * This function was added to support non-listener queries, - * specifically for use in repoGetValue. It sets up all the same - * local cache data-structures (SyncPoint + View) that are - * needed for listeners without installing an event registration. - * If `query` is not `loadsAllData`, it will also provision a tag for - * the query so that query results can be merged into the sync - * tree using existing logic for tagged listener queries. + * specifically for use in repoGetValue. It sets up all the same + * local cache data-structures (SyncPoint + View) that are + * needed for listeners without installing an event registration. + * If `query` is not `loadsAllData`, it will also provision a tag for + * the query so that query results can be merged into the sync + * tree using existing logic for tagged listener queries. * * @param syncTree - Synctree to add the query to. * @param query - Query to register From 766f46d86b8efef1cb0a06ce07420fef82eff1a8 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 26 May 2022 14:56:26 -0700 Subject: [PATCH 20/52] Addressed comments --- .changeset/rotten-tables-brush.md | 2 +- packages/database/src/core/Repo.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.changeset/rotten-tables-brush.md b/.changeset/rotten-tables-brush.md index 23716047a56..6db6bcd8618 100644 --- a/.changeset/rotten-tables-brush.md +++ b/.changeset/rotten-tables-brush.md @@ -2,4 +2,4 @@ "@firebase/database": patch --- -Fixed issue where `get()` updated incorrect cache. +Fixed issue where `get()` saved results incorrectly for non-default queries. diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 2228d78c666..7f3c6e62bee 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -477,7 +477,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { } else { // Simulate `syncTreeAddEventRegistration` without events/listener setup. // We do this (along with the syncTreeRemoveEventRegistration` below) so that - // `repoGetValue` results have the same cache effects as initial listeners + // `repoGetValue` results have the same cache effects as initial listener(s) // updates. syncTreeApplyTaggedQueryOverwrite( repo.serverSyncTree_, @@ -486,15 +486,15 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { tag ); // Call `syncTreeRemoveEventRegistration` with a null event registration, since there is none. + // Note: The below code essentially unregisters the query and cleans up any views/syncpoints temporarily created above. const cancels = syncTreeRemoveEventRegistration( repo.serverSyncTree_, query, null ); - assert( - cancels.length === 0, - 'unexpected cancel events in repoGetValue' - ); + if (cancels.length > 0) { + repoLog(repo, 'unexpected cancel events in repoGetValue'); + } } return Promise.resolve(node); }, From 04356e418ed26ee7109dba157581ff6d8d0fbc9b Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Tue, 31 May 2022 10:28:41 -0700 Subject: [PATCH 21/52] WIP --- .vscode/launch.json | 6 +- .../database/test/exp/integration.test.ts | 77 ++++++++++++++++--- 2 files changed, 70 insertions(+), 13 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index c727a20b4e7..1f246c39be9 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -8,11 +8,11 @@ "type": "node", "request": "launch", "name": "RTDB Unit Tests (Node)", - "program": "${workspaceRoot}/packages/firebase/node_modules/.bin/_mocha", + "program": "${workspaceRoot}/node_modules/.bin/_mocha", "cwd": "${workspaceRoot}/packages/database", "args": [ "test/{,!(browser)/**/}*.test.ts", - "--file", "index.node.ts", + "--file", "src/index.node.ts", "--config", "../../config/mocharc.node.js" ], "env": { @@ -30,7 +30,7 @@ "cwd": "${workspaceRoot}/packages/firestore", "args": [ "--require", "babel-register.js", - "--require", "index.node.ts", + "--require", "src/index.node.ts", "--timeout", "5000", "test/{,!(browser|integration)/**/}*.test.ts", "--exit" diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index fe3f2341bc5..e5f2cb23135 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -21,7 +21,6 @@ import { expect } from 'chai'; import Sinon, { createSandbox } from 'sinon'; import { - limitToFirst, onValue, query, set @@ -43,7 +42,7 @@ export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } -describe('Database@exp Tests', () => { +describe.only('Database@exp Tests', () => { let defaultApp; let mySandbox: Sinon.SinonSandbox; @@ -106,7 +105,7 @@ describe('Database@exp Tests', () => { }); // Tests to make sure onValue's data does not get mutated after calling get - it('calls onValue only once after get request with a non-default query', async () => { + it.only('calls onValue only once after get request with a non-default query', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; @@ -117,12 +116,28 @@ describe('Database@exp Tests', () => { expect(snapshot.val()).to.deep.eq(initial); count++; }); - await get(query(testRef, limitToFirst(1))); + // await get(query(testRef, limitToFirst(1))); await waitFor(2000); expect(count).to.equal(1); }); - it('calls onValue only once after get request with a default query', async () => { + it.only('calls onValue and expects no issues with removing the listener', async () => { + const db = getDatabase(defaultApp); + const testRef = ref(db, 'foo'); + const initial = [{ name: 'child1' }, { name: 'child2' }]; + const eventFactory = EventAccumulatorFactory.waitsForCount(1); + await set(testRef, initial); + const unsubscribe = onValue(testRef, snapshot => { + eventFactory.addEvent(snapshot.val()); + }); + await get(query(testRef)); + const update = [{name: 'child1'}, { name: 'child20'}]; + unsubscribe(); + await set(testRef, update); + const [snap1] = await eventFactory.promise; + expect(snap1).to.deep.eq(initial); + }); + it.only('calls onValue only once after get request with a default query', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; @@ -137,8 +152,50 @@ describe('Database@exp Tests', () => { await waitFor(2000); expect(count).to.equal(1); }); + it.only('calls onValue only once after get request with a nested query', async () => { + const db = getDatabase(defaultApp); + const testRef = ref(db, 'foo'); + const initial = { + test: { + abc: 123 + } + }; + + let count = 0; + await set(testRef, initial); + onValue(testRef, snapshot => { + expect(snapshot.val()).to.deep.eq(initial); + count++; + }); + const nestedRef = ref(db, '/foo/test'); + const result = await get(query(nestedRef)); + await waitFor(2000); + expect(count).to.deep.equal(1); + expect(result.val()).to.deep.eq(initial.test); + }); + it.only('calls onValue only once after parent get request', async () => { + const db = getDatabase(defaultApp); + const testRef = ref(db, 'foo'); + const initial = { + test: { + abc: 123 + } + }; + + let count = 0; + await set(testRef, initial); + const nestedRef = ref(db, '/foo/test'); + onValue(nestedRef, snapshot => { + expect(snapshot.val()).to.deep.eq(initial.test); + count++; + }); + const result = await get(query(testRef)); + await waitFor(2000); + expect(count).to.equal(1); + expect(result.val()).to.deep.eq(initial); + }); - it('Can use onlyOnce', async () => { + it.only('Can use onlyOnce', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); @@ -158,7 +215,7 @@ describe('Database@exp Tests', () => { expect(snap1).to.equal('a'); }); - it('Can unsubscribe', async () => { + it.only('Can unsubscribe', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); @@ -175,7 +232,7 @@ describe('Database@exp Tests', () => { expect(snap1).to.equal('a'); }); - it('Can goOffline/goOnline', async () => { + it.only('Can goOffline/goOnline', async () => { const db = getDatabase(defaultApp); goOffline(db); try { @@ -188,14 +245,14 @@ describe('Database@exp Tests', () => { await get(ref(db, 'foo/bar')); }); - it('Can delete app', async () => { + it.only('Can delete app', async () => { const db = getDatabase(defaultApp); await deleteApp(defaultApp); expect(() => ref(db)).to.throw('Cannot call ref on a deleted database.'); defaultApp = undefined; }); - it('Can listen to transaction changes', async () => { + it.only('Can listen to transaction changes', async () => { // Repro for https://github.com/firebase/firebase-js-sdk/issues/5195 let latestValue = 0; From a46705549cdb60d1e55a76f2912c373384ec6174 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Tue, 31 May 2022 12:29:48 -0700 Subject: [PATCH 22/52] Used unique id instead of foo --- .../database/test/exp/integration.test.ts | 78 ++++++++++--------- packages/database/test/helpers/util.ts | 4 + 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index e5f2cb23135..0cc30ce6bbb 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -36,7 +36,7 @@ import { runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_ADDRESS, DATABASE_URL, waitFor } from '../helpers/util'; +import { DATABASE_ADDRESS, DATABASE_URL, getUniqueID, waitFor } from '../helpers/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -105,56 +105,59 @@ describe.only('Database@exp Tests', () => { }); // Tests to make sure onValue's data does not get mutated after calling get - it.only('calls onValue only once after get request with a non-default query', async () => { + it('calls onValue only once after get request with a non-default query', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); + const testRef = ref(db, getUniqueID()); const initial = [{ name: 'child1' }, { name: 'child2' }]; let count = 0; await set(testRef, initial); - onValue(testRef, snapshot => { + const unsubscribe = onValue(testRef, snapshot => { expect(snapshot.val()).to.deep.eq(initial); count++; }); // await get(query(testRef, limitToFirst(1))); await waitFor(2000); expect(count).to.equal(1); + unsubscribe(); }); - it.only('calls onValue and expects no issues with removing the listener', async () => { - const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); - const initial = [{ name: 'child1' }, { name: 'child2' }]; - const eventFactory = EventAccumulatorFactory.waitsForCount(1); - await set(testRef, initial); - const unsubscribe = onValue(testRef, snapshot => { - eventFactory.addEvent(snapshot.val()); - }); - await get(query(testRef)); - const update = [{name: 'child1'}, { name: 'child20'}]; - unsubscribe(); - await set(testRef, update); - const [snap1] = await eventFactory.promise; - expect(snap1).to.deep.eq(initial); + it('calls onValue and expects no issues with removing the listener', () => { + // const db = getDatabase(defaultApp); + // const testRef = ref(db, 'foo'); + // const initial = [{ name: 'child1' }, { name: 'child2' }]; + // const eventFactory = EventAccumulatorFactory.waitsForCount(1); + // await set(testRef, initial); + // const unsubscribe = onValue(testRef, snapshot => { + // eventFactory.addEvent(snapshot.val()); + // }); + // await get(query(testRef)); + // const update = [{name: 'child1'}, { name: 'child20'}]; + // unsubscribe(); + // await set(testRef, update); + // const [snap1] = await eventFactory.promise; + // expect(snap1).to.deep.eq(initial); }); - it.only('calls onValue only once after get request with a default query', async () => { + it('calls onValue only once after get request with a default query', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); + const testRef = ref(db, getUniqueID()); const initial = [{ name: 'child1' }, { name: 'child2' }]; let count = 0; await set(testRef, initial); - onValue(testRef, snapshot => { + const unsubscribe = onValue(testRef, snapshot => { expect(snapshot.val()).to.deep.eq(initial); count++; }); await get(query(testRef)); await waitFor(2000); expect(count).to.equal(1); + unsubscribe(); }); - it.only('calls onValue only once after get request with a nested query', async () => { + it('calls onValue only once after get request with a nested query', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); + const uniqueID = getUniqueID(); + const testRef = ref(db, uniqueID); const initial = { test: { abc: 123 @@ -163,19 +166,21 @@ describe.only('Database@exp Tests', () => { let count = 0; await set(testRef, initial); - onValue(testRef, snapshot => { + const unsubscribe = onValue(testRef, snapshot => { expect(snapshot.val()).to.deep.eq(initial); count++; }); - const nestedRef = ref(db, '/foo/test'); + const nestedRef = ref(db, uniqueID + '/test'); const result = await get(query(nestedRef)); await waitFor(2000); expect(count).to.deep.equal(1); expect(result.val()).to.deep.eq(initial.test); + unsubscribe(); }); - it.only('calls onValue only once after parent get request', async () => { + it('calls onValue only once after parent get request', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); + const uniqueID = getUniqueID(); + const testRef = ref(db, uniqueID); const initial = { test: { abc: 123 @@ -184,18 +189,19 @@ describe.only('Database@exp Tests', () => { let count = 0; await set(testRef, initial); - const nestedRef = ref(db, '/foo/test'); - onValue(nestedRef, snapshot => { - expect(snapshot.val()).to.deep.eq(initial.test); + const nestedRef = ref(db, uniqueID + '/test'); + const unsubscribe = onValue(nestedRef, snapshot => { + // expect(snapshot.val()).to.deep.eq(initial.test); count++; }); const result = await get(query(testRef)); await waitFor(2000); expect(count).to.equal(1); expect(result.val()).to.deep.eq(initial); + unsubscribe(); }); - it.only('Can use onlyOnce', async () => { + it('Can use onlyOnce', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); @@ -215,7 +221,7 @@ describe.only('Database@exp Tests', () => { expect(snap1).to.equal('a'); }); - it.only('Can unsubscribe', async () => { + it('Can unsubscribe', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); @@ -232,7 +238,7 @@ describe.only('Database@exp Tests', () => { expect(snap1).to.equal('a'); }); - it.only('Can goOffline/goOnline', async () => { + it('Can goOffline/goOnline', async () => { const db = getDatabase(defaultApp); goOffline(db); try { @@ -245,14 +251,14 @@ describe.only('Database@exp Tests', () => { await get(ref(db, 'foo/bar')); }); - it.only('Can delete app', async () => { + it('Can delete app', async () => { const db = getDatabase(defaultApp); await deleteApp(defaultApp); expect(() => ref(db)).to.throw('Cannot call ref on a deleted database.'); defaultApp = undefined; }); - it.only('Can listen to transaction changes', async () => { + it('Can listen to transaction changes', async () => { // Repro for https://github.com/firebase/firebase-js-sdk/issues/5195 let latestValue = 0; diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index 32022cc1ede..5b788b4a5c4 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -76,3 +76,7 @@ export function shuffle(arr, randFn = Math.random) { export function waitFor(waitTimeInMS: number) { return new Promise(resolve => setTimeout(resolve, waitTimeInMS)); } + +export function getUniqueID() { + return Date.now().toString(); +} From 6145b2b6e49847b60b55e926989dfef5ea3d36b7 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 1 Jun 2022 15:02:04 -0700 Subject: [PATCH 23/52] Used exactCount instead of Count for accumulator --- .../database/test/exp/integration.test.ts | 65 ++++++++++--------- .../database/test/helpers/EventAccumulator.ts | 2 +- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 0cc30ce6bbb..20d5a7f3133 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -21,6 +21,7 @@ import { expect } from 'chai'; import Sinon, { createSandbox } from 'sinon'; import { + limitToFirst, onValue, query, set @@ -36,7 +37,7 @@ import { runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_ADDRESS, DATABASE_URL, getUniqueID, waitFor } from '../helpers/util'; +import { DATABASE_ADDRESS, DATABASE_URL, waitFor } from '../helpers/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -91,7 +92,7 @@ describe.only('Database@exp Tests', () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); - const ea = EventAccumulatorFactory.waitsForCount(2); + const ea = EventAccumulatorFactory.waitsForExactCount(2); onValue(fooRef, snap => { ea.addEvent(snap.val()); }); @@ -107,40 +108,41 @@ describe.only('Database@exp Tests', () => { // Tests to make sure onValue's data does not get mutated after calling get it('calls onValue only once after get request with a non-default query', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, getUniqueID()); + const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; + const ec = EventAccumulatorFactory.waitsForExactCount(1); - let count = 0; await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { - expect(snapshot.val()).to.deep.eq(initial); - count++; + ec.addEvent(snapshot.val()); }); - // await get(query(testRef, limitToFirst(1))); + await get(query(testRef, limitToFirst(1))); await waitFor(2000); - expect(count).to.equal(1); + const events = await ec.promise; + expect(events.length).to.equal(1); + expect(events[0]).to.deep.equal(initial); unsubscribe(); }); - it('calls onValue and expects no issues with removing the listener', () => { - // const db = getDatabase(defaultApp); - // const testRef = ref(db, 'foo'); - // const initial = [{ name: 'child1' }, { name: 'child2' }]; - // const eventFactory = EventAccumulatorFactory.waitsForCount(1); - // await set(testRef, initial); - // const unsubscribe = onValue(testRef, snapshot => { - // eventFactory.addEvent(snapshot.val()); - // }); - // await get(query(testRef)); - // const update = [{name: 'child1'}, { name: 'child20'}]; - // unsubscribe(); - // await set(testRef, update); - // const [snap1] = await eventFactory.promise; - // expect(snap1).to.deep.eq(initial); + it('calls onValue and expects no issues with removing the listener', async () => { + const db = getDatabase(defaultApp); + const testRef = ref(db, 'foo'); + const initial = [{ name: 'child1' }, { name: 'child2' }]; + const eventFactory = EventAccumulatorFactory.waitsForExactCount(1); + await set(testRef, initial); + const unsubscribe = onValue(testRef, snapshot => { + eventFactory.addEvent(snapshot.val()); + }); + await get(query(testRef)); + const update = [{name: 'child1'}, { name: 'child20'}]; + unsubscribe(); + await set(testRef, update); + const [snap1] = await eventFactory.promise; + expect(snap1).to.deep.eq(initial); }); it('calls onValue only once after get request with a default query', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, getUniqueID()); + const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; let count = 0; @@ -156,7 +158,7 @@ describe.only('Database@exp Tests', () => { }); it('calls onValue only once after get request with a nested query', async () => { const db = getDatabase(defaultApp); - const uniqueID = getUniqueID(); + const uniqueID = 'foo'; const testRef = ref(db, uniqueID); const initial = { test: { @@ -179,7 +181,7 @@ describe.only('Database@exp Tests', () => { }); it('calls onValue only once after parent get request', async () => { const db = getDatabase(defaultApp); - const uniqueID = getUniqueID(); + const uniqueID = 'foo'; const testRef = ref(db, uniqueID); const initial = { test: { @@ -205,7 +207,7 @@ describe.only('Database@exp Tests', () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); - const ea = EventAccumulatorFactory.waitsForCount(1); + const ea = EventAccumulatorFactory.waitsForExactCount(1); onValue( fooRef, snap => { @@ -218,14 +220,14 @@ describe.only('Database@exp Tests', () => { await set(fooRef, 'b'); const [snap1] = await ea.promise; - expect(snap1).to.equal('a'); + expect(snap1).to.equal('a'); // This doesn't necessarily test that onValue was only triggered once }); it('Can unsubscribe', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); - const ea = EventAccumulatorFactory.waitsForCount(1); + const ea = EventAccumulatorFactory.waitsForExactCount(1); const unsubscribe = onValue(fooRef, snap => { ea.addEvent(snap.val()); }); @@ -234,8 +236,9 @@ describe.only('Database@exp Tests', () => { unsubscribe(); await set(fooRef, 'b'); - const [snap1] = await ea.promise; - expect(snap1).to.equal('a'); + const events = await ea.promise; + expect(events.length).to.equal(1); + expect(events[0]).to.equal('a'); }); it('Can goOffline/goOnline', async () => { diff --git a/packages/database/test/helpers/EventAccumulator.ts b/packages/database/test/helpers/EventAccumulator.ts index 798e31405d0..fb5b57af9fa 100644 --- a/packages/database/test/helpers/EventAccumulator.ts +++ b/packages/database/test/helpers/EventAccumulator.ts @@ -16,7 +16,7 @@ */ export const EventAccumulatorFactory = { - waitsForCount: maxCount => { + waitsForCount: maxCount => { // Note: This should be sparingly used as it can result in more events being raised than expected let count = 0; const condition = () => count >= maxCount; const ea = new EventAccumulator(condition); From 788aaa0a847a8b497cd81031633d4b98ea0560e9 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 1 Jun 2022 15:38:27 -0700 Subject: [PATCH 24/52] Removed only --- packages/database/test/exp/integration.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 20d5a7f3133..cedac831f2b 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -43,7 +43,7 @@ export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } -describe.only('Database@exp Tests', () => { +describe('Database@exp Tests', () => { let defaultApp; let mySandbox: Sinon.SinonSandbox; @@ -110,7 +110,7 @@ describe.only('Database@exp Tests', () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; - const ec = EventAccumulatorFactory.waitsForExactCount(1); + const ec = EventAccumulatorFactory.waitsForCount(1); await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { From 51d57a29d0a1f13a7b6c9d6d666a5421323b133f Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 1 Jun 2022 15:38:59 -0700 Subject: [PATCH 25/52] Reformatted --- packages/database/test/exp/integration.test.ts | 2 +- packages/database/test/helpers/EventAccumulator.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index cedac831f2b..4781b4bf730 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -134,7 +134,7 @@ describe('Database@exp Tests', () => { eventFactory.addEvent(snapshot.val()); }); await get(query(testRef)); - const update = [{name: 'child1'}, { name: 'child20'}]; + const update = [{ name: 'child1' }, { name: 'child20' }]; unsubscribe(); await set(testRef, update); const [snap1] = await eventFactory.promise; diff --git a/packages/database/test/helpers/EventAccumulator.ts b/packages/database/test/helpers/EventAccumulator.ts index fb5b57af9fa..8ce033fca15 100644 --- a/packages/database/test/helpers/EventAccumulator.ts +++ b/packages/database/test/helpers/EventAccumulator.ts @@ -16,7 +16,8 @@ */ export const EventAccumulatorFactory = { - waitsForCount: maxCount => { // Note: This should be sparingly used as it can result in more events being raised than expected + waitsForCount: maxCount => { + // Note: This should be sparingly used as it can result in more events being raised than expected let count = 0; const condition = () => count >= maxCount; const ea = new EventAccumulator(condition); From 41a73030cd00cbbc5d58f3cff8e4f58e3ac785a6 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 2 Jun 2022 09:09:11 -0700 Subject: [PATCH 26/52] Removed get wip --- .vscode/launch.json | 2 ++ packages/database/test/exp/integration.test.ts | 10 ++++++---- packages/database/test/helpers/EventAccumulator.ts | 4 ++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 1f246c39be9..43fbf91ddaa 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -11,11 +11,13 @@ "program": "${workspaceRoot}/node_modules/.bin/_mocha", "cwd": "${workspaceRoot}/packages/database", "args": [ + // TS_NODE_FILES=true TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha 'test/{,!(browser)/**/}*.test.ts' --file src/index.node.ts --config ../../config/mocharc.node.js "test/{,!(browser)/**/}*.test.ts", "--file", "src/index.node.ts", "--config", "../../config/mocharc.node.js" ], "env": { + "TS_NODE_FILES":true, "TS_NODE_CACHE": "NO", "TS_NODE_COMPILER_OPTIONS" : "{\"module\":\"commonjs\"}" }, diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 4781b4bf730..8d3e6207de0 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -43,7 +43,7 @@ export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } -describe('Database@exp Tests', () => { +describe.only('Database@exp Tests', () => { let defaultApp; let mySandbox: Sinon.SinonSandbox; @@ -124,7 +124,7 @@ describe('Database@exp Tests', () => { unsubscribe(); }); - it('calls onValue and expects no issues with removing the listener', async () => { + it.only('calls onValue and expects no issues with removing the listener', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; @@ -133,10 +133,12 @@ describe('Database@exp Tests', () => { const unsubscribe = onValue(testRef, snapshot => { eventFactory.addEvent(snapshot.val()); }); - await get(query(testRef)); + // await get(query(testRef)); const update = [{ name: 'child1' }, { name: 'child20' }]; unsubscribe(); await set(testRef, update); + await waitFor(2000); + console.log('waiting for promise'); const [snap1] = await eventFactory.promise; expect(snap1).to.deep.eq(initial); }); @@ -220,7 +222,7 @@ describe('Database@exp Tests', () => { await set(fooRef, 'b'); const [snap1] = await ea.promise; - expect(snap1).to.equal('a'); // This doesn't necessarily test that onValue was only triggered once + expect(snap1).to.equal('a'); // This doesn't test that onValue was only triggered once }); it('Can unsubscribe', async () => { diff --git a/packages/database/test/helpers/EventAccumulator.ts b/packages/database/test/helpers/EventAccumulator.ts index 8ce033fca15..c318f74fbaf 100644 --- a/packages/database/test/helpers/EventAccumulator.ts +++ b/packages/database/test/helpers/EventAccumulator.ts @@ -49,20 +49,24 @@ export const EventAccumulatorFactory = { }; export class EventAccumulator { + // TODO: make these typesafe eventData = []; promise; resolve; reject; private onResetFxn; private onEventFxn; + private tag: string; // eslint-disable-next-line @typescript-eslint/ban-types constructor(public condition: Function) { + this.tag = Date.now().toString(); this.promise = new Promise((resolve, reject) => { this.resolve = resolve; this.reject = reject; }); } addEvent(eventData?: any) { + console.log(this.tag + ': Event received', Date.now()); this.eventData = [...this.eventData, eventData]; if (typeof this.onEventFxn === 'function') { this.onEventFxn(); From a58bb3395843d3402432b97811699659d5b8a8ac Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 2 Jun 2022 13:31:22 -0700 Subject: [PATCH 27/52] Got minimal repro --- .../database/test/exp/integration.test.ts | 117 ++++++++++-------- .../database/test/helpers/EventAccumulator.ts | 2 +- 2 files changed, 63 insertions(+), 56 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 8d3e6207de0..1aa40e60b3d 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -21,23 +21,20 @@ import { expect } from 'chai'; import Sinon, { createSandbox } from 'sinon'; import { + get, limitToFirst, onValue, query, set } from '../../src/api/Reference_impl'; import { - get, getDatabase, - goOffline, - goOnline, push, ref, - refFromURL, runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_ADDRESS, DATABASE_URL, waitFor } from '../helpers/util'; +import { DATABASE_URL, waitFor } from '../helpers/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -50,6 +47,7 @@ describe.only('Database@exp Tests', () => { beforeEach(() => { defaultApp = createTestApp(); mySandbox = createSandbox(); + waitFor(2000); }); afterEach(async () => { @@ -76,24 +74,24 @@ describe.only('Database@exp Tests', () => { expect(db.app).to.equal(defaultApp); }); - it('Can set and get ref', async () => { - const db = getDatabase(defaultApp); - await set(ref(db, 'foo/bar'), 'foobar'); - const snap = await get(ref(db, 'foo/bar')); - expect(snap.val()).to.equal('foobar'); - }); + // it('Can set and get ref', async () => { + // const db = getDatabase(defaultApp); + // await set(ref(db, 'foo/bar'), 'foobar'); + // const snap = await get(ref(db, 'foo/bar')); + // expect(snap.val()).to.equal('foobar'); + // }); - it('Can get refFromUrl', async () => { - const db = getDatabase(defaultApp); - await get(refFromURL(db, `${DATABASE_ADDRESS}/foo/bar`)); - }); + // it('Can get refFromUrl', async () => { + // const db = getDatabase(defaultApp); + // await get(refFromURL(db, `${DATABASE_ADDRESS}/foo/bar`)); + // }); it('Can get updates', async () => { const db = getDatabase(defaultApp); const fooRef = ref(db, 'foo'); const ea = EventAccumulatorFactory.waitsForExactCount(2); - onValue(fooRef, snap => { + const unsubscribe = onValue(fooRef, snap => { ea.addEvent(snap.val()); }); @@ -103,64 +101,70 @@ describe.only('Database@exp Tests', () => { const [snap1, snap2] = await ea.promise; expect(snap1).to.equal('a'); expect(snap2).to.equal('b'); + unsubscribe(); }); // Tests to make sure onValue's data does not get mutated after calling get - it('calls onValue only once after get request with a non-default query', async () => { + it.only('calls onValue only once after get request with a non-default query', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; - const ec = EventAccumulatorFactory.waitsForCount(1); + const ec = EventAccumulatorFactory.waitsForExactCount(1); await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { ec.addEvent(snapshot.val()); }); - await get(query(testRef, limitToFirst(1))); + // await get(query(testRef, limitToFirst(1))); await waitFor(2000); const events = await ec.promise; expect(events.length).to.equal(1); expect(events[0]).to.deep.equal(initial); unsubscribe(); + await set(testRef, {test: 'abc'}); + await waitFor(2000); + const result = await get(query(testRef, limitToFirst(1))); + expect(result.val()).to.deep.equal({test: 'abc'}); }); it.only('calls onValue and expects no issues with removing the listener', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; - const eventFactory = EventAccumulatorFactory.waitsForExactCount(1); + const ea = EventAccumulatorFactory.waitsForExactCount(1); await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { - eventFactory.addEvent(snapshot.val()); + ea.addEvent(snapshot.val()); }); // await get(query(testRef)); + await waitFor(2000); const update = [{ name: 'child1' }, { name: 'child20' }]; unsubscribe(); await set(testRef, update); - await waitFor(2000); - console.log('waiting for promise'); - const [snap1] = await eventFactory.promise; + const [snap1] = await ea.promise; expect(snap1).to.deep.eq(initial); }); it('calls onValue only once after get request with a default query', async () => { const db = getDatabase(defaultApp); const testRef = ref(db, 'foo'); const initial = [{ name: 'child1' }, { name: 'child2' }]; + const ea = EventAccumulatorFactory.waitsForExactCount(1); - let count = 0; await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { + ea.addEvent(snapshot.val()); expect(snapshot.val()).to.deep.eq(initial); - count++; }); - await get(query(testRef)); + // await get(query(testRef)); await waitFor(2000); - expect(count).to.equal(1); + const events = await ea.promise; + expect(events.length).to.equal(1); unsubscribe(); }); - it('calls onValue only once after get request with a nested query', async () => { + it.only('calls onValue only once after get request with a nested query', async () => { const db = getDatabase(defaultApp); const uniqueID = 'foo'; + const ea = EventAccumulatorFactory.waitsForExactCount(1); const testRef = ref(db, uniqueID); const initial = { test: { @@ -168,40 +172,41 @@ describe.only('Database@exp Tests', () => { } }; - let count = 0; await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { - expect(snapshot.val()).to.deep.eq(initial); - count++; + ea.addEvent(snapshot.val()); }); const nestedRef = ref(db, uniqueID + '/test'); - const result = await get(query(nestedRef)); + // const result = await get(query(nestedRef)); await waitFor(2000); - expect(count).to.deep.equal(1); - expect(result.val()).to.deep.eq(initial.test); + const events = await ea.promise; + expect(events.length).to.equal(1); + expect(events[0]).to.deep.equal(initial); + // expect(result.val()).to.deep.eq(initial.test); unsubscribe(); }); it('calls onValue only once after parent get request', async () => { const db = getDatabase(defaultApp); const uniqueID = 'foo'; const testRef = ref(db, uniqueID); + const ea = EventAccumulatorFactory.waitsForExactCount(1); const initial = { test: { abc: 123 } }; - let count = 0; await set(testRef, initial); const nestedRef = ref(db, uniqueID + '/test'); const unsubscribe = onValue(nestedRef, snapshot => { - // expect(snapshot.val()).to.deep.eq(initial.test); - count++; + ea.addEvent(snapshot.val()); }); - const result = await get(query(testRef)); + // const result = await get(query(testRef)); // commented out to make sure the unsubscribe isn't due to a recent change. + const events = await ea.promise; await waitFor(2000); - expect(count).to.equal(1); - expect(result.val()).to.deep.eq(initial); + expect(events.length).to.equal(1); + expect(events[0]).to.deep.eq(initial.test); + // expect(result.val()).to.deep.equal(initial); unsubscribe(); }); @@ -210,7 +215,7 @@ describe.only('Database@exp Tests', () => { const fooRef = ref(db, 'foo'); const ea = EventAccumulatorFactory.waitsForExactCount(1); - onValue( + const unsubscribe = onValue( fooRef, snap => { ea.addEvent(snap.val()); @@ -223,6 +228,7 @@ describe.only('Database@exp Tests', () => { const [snap1] = await ea.promise; expect(snap1).to.equal('a'); // This doesn't test that onValue was only triggered once + unsubscribe(); }); it('Can unsubscribe', async () => { @@ -243,18 +249,18 @@ describe.only('Database@exp Tests', () => { expect(events[0]).to.equal('a'); }); - it('Can goOffline/goOnline', async () => { - const db = getDatabase(defaultApp); - goOffline(db); - try { - await get(ref(db, 'foo/bar')); - expect.fail('Should have failed since we are offline'); - } catch (e) { - expect(e.message).to.equal('Error: Client is offline.'); - } - goOnline(db); - await get(ref(db, 'foo/bar')); - }); + // it('Can goOffline/goOnline', async () => { + // const db = getDatabase(defaultApp); + // goOffline(db); + // try { + // await get(ref(db, 'foo/bar')); + // expect.fail('Should have failed since we are offline'); + // } catch (e) { + // expect(e.message).to.equal('Error: Client is offline.'); + // } + // goOnline(db); + // await get(ref(db, 'foo/bar')); + // }); it('Can delete app', async () => { const db = getDatabase(defaultApp); @@ -272,7 +278,7 @@ describe.only('Database@exp Tests', () => { const database = getDatabase(defaultApp); const counterRef = push(ref(database, 'counter')); - onValue(counterRef, snap => { + const unsubscribe = onValue(counterRef, snap => { latestValue = snap.val(); deferred.resolve(); }); @@ -293,5 +299,6 @@ describe.only('Database@exp Tests', () => { expect(latestValue).to.equal(1); await incrementViaTransaction(); expect(latestValue).to.equal(2); + unsubscribe(); }); }); diff --git a/packages/database/test/helpers/EventAccumulator.ts b/packages/database/test/helpers/EventAccumulator.ts index c318f74fbaf..837ed675783 100644 --- a/packages/database/test/helpers/EventAccumulator.ts +++ b/packages/database/test/helpers/EventAccumulator.ts @@ -33,6 +33,7 @@ export const EventAccumulatorFactory = { let count = 0; const condition = () => { if (count > maxCount) { + console.log(ea.eventData); throw new Error('Received more events than expected'); } return count === maxCount; @@ -66,7 +67,6 @@ export class EventAccumulator { }); } addEvent(eventData?: any) { - console.log(this.tag + ': Event received', Date.now()); this.eventData = [...this.eventData, eventData]; if (typeof this.onEventFxn === 'function') { this.onEventFxn(); From 5d41ce0274672481395674684ad63e516f17ebd7 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 2 Jun 2022 13:34:26 -0700 Subject: [PATCH 28/52] Got minimal repro --- packages/database/test/exp/integration.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 1aa40e60b3d..f038f03eb03 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -113,9 +113,9 @@ describe.only('Database@exp Tests', () => { await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { - ec.addEvent(snapshot.val()); + ec.addEvent(snapshot.val()); // This is called more than once }); - // await get(query(testRef, limitToFirst(1))); + await get(query(testRef, limitToFirst(1))); await waitFor(2000); const events = await ec.promise; expect(events.length).to.equal(1); From bdfd40bc2eb531571ce7952cf38ff690538ef88b Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 2 Jun 2022 15:41:59 -0700 Subject: [PATCH 29/52] Fixed race condition issue --- .vscode/launch.json | 2 +- packages/database/package.json | 3 +- .../database/test/exp/integration.test.ts | 84 +++++++++---------- packages/database/test/helpers/util.ts | 9 +- 4 files changed, 50 insertions(+), 48 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 43fbf91ddaa..6985fbd4d42 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -14,7 +14,7 @@ // TS_NODE_FILES=true TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha 'test/{,!(browser)/**/}*.test.ts' --file src/index.node.ts --config ../../config/mocharc.node.js "test/{,!(browser)/**/}*.test.ts", "--file", "src/index.node.ts", - "--config", "../../config/mocharc.node.js" + "--config", "../../config/mocharc.node.js", ], "env": { "TS_NODE_FILES":true, diff --git a/packages/database/package.json b/packages/database/package.json index b0842710fba..319a2e43e70 100644 --- a/packages/database/package.json +++ b/packages/database/package.json @@ -55,7 +55,8 @@ "@firebase/app": "0.7.24", "rollup": "2.72.1", "rollup-plugin-typescript2": "0.31.2", - "typescript": "4.2.2" + "typescript": "4.2.2", + "uuid": "^8.0.0" }, "repository": { "directory": "packages/database", diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index f038f03eb03..26ed27be454 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -25,16 +25,19 @@ import { limitToFirst, onValue, query, + refFromURL, set } from '../../src/api/Reference_impl'; import { getDatabase, + goOffline, + goOnline, push, ref, runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_URL, waitFor } from '../helpers/util'; +import { DATABASE_ADDRESS, DATABASE_URL, getUniqueRef, waitFor } from '../helpers/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -47,7 +50,6 @@ describe.only('Database@exp Tests', () => { beforeEach(() => { defaultApp = createTestApp(); mySandbox = createSandbox(); - waitFor(2000); }); afterEach(async () => { @@ -74,17 +76,17 @@ describe.only('Database@exp Tests', () => { expect(db.app).to.equal(defaultApp); }); - // it('Can set and get ref', async () => { - // const db = getDatabase(defaultApp); - // await set(ref(db, 'foo/bar'), 'foobar'); - // const snap = await get(ref(db, 'foo/bar')); - // expect(snap.val()).to.equal('foobar'); - // }); + it('Can set and get ref', async () => { + const db = getDatabase(defaultApp); + await set(ref(db, 'foo/bar'), 'foobar'); + const snap = await get(ref(db, 'foo/bar')); + expect(snap.val()).to.equal('foobar'); + }); - // it('Can get refFromUrl', async () => { - // const db = getDatabase(defaultApp); - // await get(refFromURL(db, `${DATABASE_ADDRESS}/foo/bar`)); - // }); + it('Can get refFromUrl', async () => { + const db = getDatabase(defaultApp); + await get(refFromURL(db, `${DATABASE_ADDRESS}/foo/bar`)); + }); it('Can get updates', async () => { const db = getDatabase(defaultApp); @@ -105,9 +107,9 @@ describe.only('Database@exp Tests', () => { }); // Tests to make sure onValue's data does not get mutated after calling get - it.only('calls onValue only once after get request with a non-default query', async () => { + it('calls onValue only once after get request with a non-default query', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); + const { ref: testRef } = getUniqueRef(db); const initial = [{ name: 'child1' }, { name: 'child2' }]; const ec = EventAccumulatorFactory.waitsForExactCount(1); @@ -121,22 +123,18 @@ describe.only('Database@exp Tests', () => { expect(events.length).to.equal(1); expect(events[0]).to.deep.equal(initial); unsubscribe(); - await set(testRef, {test: 'abc'}); - await waitFor(2000); - const result = await get(query(testRef, limitToFirst(1))); - expect(result.val()).to.deep.equal({test: 'abc'}); }); - it.only('calls onValue and expects no issues with removing the listener', async () => { + it('calls onValue and expects no issues with removing the listener', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); + const { ref: testRef } = getUniqueRef(db); const initial = [{ name: 'child1' }, { name: 'child2' }]; const ea = EventAccumulatorFactory.waitsForExactCount(1); await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { ea.addEvent(snapshot.val()); }); - // await get(query(testRef)); + await get(query(testRef)); await waitFor(2000); const update = [{ name: 'child1' }, { name: 'child20' }]; unsubscribe(); @@ -146,7 +144,7 @@ describe.only('Database@exp Tests', () => { }); it('calls onValue only once after get request with a default query', async () => { const db = getDatabase(defaultApp); - const testRef = ref(db, 'foo'); + const { ref: testRef } = getUniqueRef(db); const initial = [{ name: 'child1' }, { name: 'child2' }]; const ea = EventAccumulatorFactory.waitsForExactCount(1); @@ -155,17 +153,16 @@ describe.only('Database@exp Tests', () => { ea.addEvent(snapshot.val()); expect(snapshot.val()).to.deep.eq(initial); }); - // await get(query(testRef)); + await get(query(testRef)); await waitFor(2000); const events = await ea.promise; expect(events.length).to.equal(1); unsubscribe(); }); - it.only('calls onValue only once after get request with a nested query', async () => { + it('calls onValue only once after get request with a nested query', async () => { const db = getDatabase(defaultApp); - const uniqueID = 'foo'; const ea = EventAccumulatorFactory.waitsForExactCount(1); - const testRef = ref(db, uniqueID); + const { ref: testRef, path } = getUniqueRef(db); const initial = { test: { abc: 123 @@ -176,7 +173,7 @@ describe.only('Database@exp Tests', () => { const unsubscribe = onValue(testRef, snapshot => { ea.addEvent(snapshot.val()); }); - const nestedRef = ref(db, uniqueID + '/test'); + const nestedRef = ref(db, path + '/test'); // const result = await get(query(nestedRef)); await waitFor(2000); const events = await ea.promise; @@ -187,8 +184,7 @@ describe.only('Database@exp Tests', () => { }); it('calls onValue only once after parent get request', async () => { const db = getDatabase(defaultApp); - const uniqueID = 'foo'; - const testRef = ref(db, uniqueID); + const { ref: testRef, path } = getUniqueRef(db); const ea = EventAccumulatorFactory.waitsForExactCount(1); const initial = { test: { @@ -197,16 +193,16 @@ describe.only('Database@exp Tests', () => { }; await set(testRef, initial); - const nestedRef = ref(db, uniqueID + '/test'); + const nestedRef = ref(db, path + '/test'); const unsubscribe = onValue(nestedRef, snapshot => { ea.addEvent(snapshot.val()); }); - // const result = await get(query(testRef)); // commented out to make sure the unsubscribe isn't due to a recent change. + const result = await get(query(testRef)); // commented out to make sure the unsubscribe isn't due to a recent change. const events = await ea.promise; await waitFor(2000); expect(events.length).to.equal(1); expect(events[0]).to.deep.eq(initial.test); - // expect(result.val()).to.deep.equal(initial); + expect(result.val()).to.deep.equal(initial); unsubscribe(); }); @@ -249,18 +245,18 @@ describe.only('Database@exp Tests', () => { expect(events[0]).to.equal('a'); }); - // it('Can goOffline/goOnline', async () => { - // const db = getDatabase(defaultApp); - // goOffline(db); - // try { - // await get(ref(db, 'foo/bar')); - // expect.fail('Should have failed since we are offline'); - // } catch (e) { - // expect(e.message).to.equal('Error: Client is offline.'); - // } - // goOnline(db); - // await get(ref(db, 'foo/bar')); - // }); + it('Can goOffline/goOnline', async () => { + const db = getDatabase(defaultApp); + goOffline(db); + try { + await get(ref(db, 'foo/bar')); + expect.fail('Should have failed since we are offline'); + } catch (e) { + expect(e.message).to.equal('Error: Client is offline.'); + } + goOnline(db); + await get(ref(db, 'foo/bar')); + }); it('Can delete app', async () => { const db = getDatabase(defaultApp); diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index 5b788b4a5c4..32fc65433ea 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -15,6 +15,9 @@ * limitations under the License. */ +import uuid from 'uuid/v4'; + +import { Database, ref } from '../../src'; import { ConnectionTarget } from '../../src/api/test_access'; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -77,6 +80,8 @@ export function waitFor(waitTimeInMS: number) { return new Promise(resolve => setTimeout(resolve, waitTimeInMS)); } -export function getUniqueID() { - return Date.now().toString(); +// Creates a unique reference using uuid +export function getUniqueRef(db: Database) { + const path = uuid(); + return { ref: ref(db, path), path }; } From 4ca2289bcd356d734c22f8bfc26038b1337e9e28 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 2 Jun 2022 15:48:04 -0700 Subject: [PATCH 30/52] Updated tests --- .../database/test/exp/integration.test.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 26ed27be454..0da2c3fc83c 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -37,7 +37,12 @@ import { runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_ADDRESS, DATABASE_URL, getUniqueRef, waitFor } from '../helpers/util'; +import { + DATABASE_ADDRESS, + DATABASE_URL, + getUniqueRef, + waitFor +} from '../helpers/util'; export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -119,9 +124,8 @@ describe.only('Database@exp Tests', () => { }); await get(query(testRef, limitToFirst(1))); await waitFor(2000); - const events = await ec.promise; - expect(events.length).to.equal(1); - expect(events[0]).to.deep.equal(initial); + const [snap] = await ec.promise; + expect(snap).to.deep.equal(initial); unsubscribe(); }); @@ -155,8 +159,8 @@ describe.only('Database@exp Tests', () => { }); await get(query(testRef)); await waitFor(2000); - const events = await ea.promise; - expect(events.length).to.equal(1); + const [snap] = await ea.promise; + expect(snap).to.deep.equal(initial); unsubscribe(); }); it('calls onValue only once after get request with a nested query', async () => { @@ -174,12 +178,12 @@ describe.only('Database@exp Tests', () => { ea.addEvent(snapshot.val()); }); const nestedRef = ref(db, path + '/test'); - // const result = await get(query(nestedRef)); + const result = await get(query(nestedRef)); await waitFor(2000); const events = await ea.promise; expect(events.length).to.equal(1); expect(events[0]).to.deep.equal(initial); - // expect(result.val()).to.deep.eq(initial.test); + expect(result.val()).to.deep.eq(initial.test); unsubscribe(); }); it('calls onValue only once after parent get request', async () => { From b52f468b77391ce2bafde091001c4eccc25c5d1a Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 3 Jun 2022 08:27:42 -0700 Subject: [PATCH 31/52] Updated yarn lock file --- yarn.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index d0d7dfc9da8..2622b2f84e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17099,7 +17099,7 @@ uuid@^3.0.0, uuid@^3.3.2, uuid@^3.3.3: resolved "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz#b23e4358afa8a202fe7a100af1f5f883f02007ee" integrity sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A== -uuid@^8.3.2: +uuid@^8.0.0, uuid@^8.3.2: version "8.3.2" resolved "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz#80d5b5ced271bb9af6c445f21a1a04c606cefbe2" integrity sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg== From 27f9ea19b733ef9decde753f3eea311ce1fc12b9 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 3 Jun 2022 09:04:06 -0700 Subject: [PATCH 32/52] Updated module type --- packages/database/test/helpers/util.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index 32fc65433ea..8e9e45ed3ea 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import uuid from 'uuid/v4'; +import { v4 as uuidv4 } from 'uuid'; import { Database, ref } from '../../src'; import { ConnectionTarget } from '../../src/api/test_access'; @@ -82,6 +82,6 @@ export function waitFor(waitTimeInMS: number) { // Creates a unique reference using uuid export function getUniqueRef(db: Database) { - const path = uuid(); + const path = uuidv4(); return { ref: ref(db, path), path }; } From 065dab05d60f37da16d988c7f32dda24807b3614 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 3 Jun 2022 09:09:33 -0700 Subject: [PATCH 33/52] Added comment --- packages/database/test/exp/integration.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 0da2c3fc83c..8e05186b5cc 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -48,7 +48,8 @@ export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } -describe.only('Database@exp Tests', () => { +// Note: these run in parallel with the node environment. If you use the same paths in parallel, you may experience race conditions. +describe('Database@exp Tests', () => { let defaultApp; let mySandbox: Sinon.SinonSandbox; From 74fe5e8efc81198412e2fa5b36f96d3bce9b097e Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 3 Jun 2022 10:12:37 -0700 Subject: [PATCH 34/52] Removed unnecessary code --- packages/database/test/exp/integration.test.ts | 3 --- packages/database/test/helpers/EventAccumulator.ts | 5 +---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 8e05186b5cc..7eb3bc89d0f 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -51,15 +51,12 @@ export function createTestApp() { // Note: these run in parallel with the node environment. If you use the same paths in parallel, you may experience race conditions. describe('Database@exp Tests', () => { let defaultApp; - let mySandbox: Sinon.SinonSandbox; beforeEach(() => { defaultApp = createTestApp(); - mySandbox = createSandbox(); }); afterEach(async () => { - mySandbox.restore(); if (defaultApp) { return deleteApp(defaultApp); } diff --git a/packages/database/test/helpers/EventAccumulator.ts b/packages/database/test/helpers/EventAccumulator.ts index 837ed675783..fc293b28b05 100644 --- a/packages/database/test/helpers/EventAccumulator.ts +++ b/packages/database/test/helpers/EventAccumulator.ts @@ -17,7 +17,7 @@ export const EventAccumulatorFactory = { waitsForCount: maxCount => { - // Note: This should be sparingly used as it can result in more events being raised than expected + // Note: This should be used sparingly as it can result in more events being raised than expected let count = 0; const condition = () => count >= maxCount; const ea = new EventAccumulator(condition); @@ -33,7 +33,6 @@ export const EventAccumulatorFactory = { let count = 0; const condition = () => { if (count > maxCount) { - console.log(ea.eventData); throw new Error('Received more events than expected'); } return count === maxCount; @@ -57,10 +56,8 @@ export class EventAccumulator { reject; private onResetFxn; private onEventFxn; - private tag: string; // eslint-disable-next-line @typescript-eslint/ban-types constructor(public condition: Function) { - this.tag = Date.now().toString(); this.promise = new Promise((resolve, reject) => { this.resolve = resolve; this.reject = reject; From be4197921f40a941839812874c262e80631ebd00 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 6 Jun 2022 10:04:28 -0700 Subject: [PATCH 35/52] Removed unnecessary comment --- .vscode/launch.json | 1 - 1 file changed, 1 deletion(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 6985fbd4d42..b2d5d88b94d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -11,7 +11,6 @@ "program": "${workspaceRoot}/node_modules/.bin/_mocha", "cwd": "${workspaceRoot}/packages/database", "args": [ - // TS_NODE_FILES=true TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha 'test/{,!(browser)/**/}*.test.ts' --file src/index.node.ts --config ../../config/mocharc.node.js "test/{,!(browser)/**/}*.test.ts", "--file", "src/index.node.ts", "--config", "../../config/mocharc.node.js", From f320a93c30b87c3dd8498f69e933c6e8d1f8ad2f Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 6 Jun 2022 10:06:55 -0700 Subject: [PATCH 36/52] Removed unused imports --- packages/database/test/exp/integration.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 7eb3bc89d0f..e1a741266ba 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -18,7 +18,6 @@ import { initializeApp, deleteApp } from '@firebase/app'; import { Deferred } from '@firebase/util'; import { expect } from 'chai'; -import Sinon, { createSandbox } from 'sinon'; import { get, @@ -118,7 +117,7 @@ describe('Database@exp Tests', () => { await set(testRef, initial); const unsubscribe = onValue(testRef, snapshot => { - ec.addEvent(snapshot.val()); // This is called more than once + ec.addEvent(snapshot.val()); }); await get(query(testRef, limitToFirst(1))); await waitFor(2000); From c4f8ff70cdfe935a42d2173b74001582e1e70209 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Mon, 6 Jun 2022 10:14:24 -0700 Subject: [PATCH 37/52] Updated test to remove redundant assertion --- packages/database/test/exp/integration.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index e1a741266ba..2320032acde 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -177,9 +177,8 @@ describe('Database@exp Tests', () => { const nestedRef = ref(db, path + '/test'); const result = await get(query(nestedRef)); await waitFor(2000); - const events = await ea.promise; - expect(events.length).to.equal(1); - expect(events[0]).to.deep.equal(initial); + const [snap] = await ea.promise; + expect(snap).to.deep.equal(initial); expect(result.val()).to.deep.eq(initial.test); unsubscribe(); }); From 1dc798cda4e2da80fb691c15f6434f908a944dae Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 8 Jun 2022 07:06:29 -0700 Subject: [PATCH 38/52] Moved query check --- packages/database/src/core/SyncTree.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 7204a9fd9b9..bffb4aeb104 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -437,19 +437,19 @@ export function syncTreeRemoveEventRegistration( * @returns tag as a string if query is not a default query, null if query is not. */ export function syncTreeRegisterQuery(syncTree: SyncTree, query: QueryContext) { + const { syncPoint, serverCache, writesCache, serverCacheComplete } = + syncTreeRegisterSyncPoint(query, syncTree); + const view = syncPointGetView( + syncPoint, + query, + writesCache, + serverCache, + serverCacheComplete + ); + if (!syncPoint.views.has(query._queryIdentifier)) { + syncPoint.views.set(query._queryIdentifier, view); + } if (!query._queryParams.loadsAllData()) { - const { syncPoint, serverCache, writesCache, serverCacheComplete } = - syncTreeRegisterSyncPoint(query, syncTree); - const view = syncPointGetView( - syncPoint, - query, - writesCache, - serverCache, - serverCacheComplete - ); - if (!syncPoint.views.has(query._queryIdentifier)) { - syncPoint.views.set(query._queryIdentifier, view); - } return syncTreeTagForQuery_(syncTree, query); } return null; From 42e38f6f392290f8c4c1f71aad053de4e15b7025 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 8 Jun 2022 09:18:35 -0700 Subject: [PATCH 39/52] Updated tests to include example queries --- .../database/test/exp/integration.test.ts | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 2320032acde..18006e8d5f2 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -25,7 +25,8 @@ import { onValue, query, refFromURL, - set + set, + startAt } from '../../src/api/Reference_impl'; import { getDatabase, @@ -112,18 +113,26 @@ describe('Database@exp Tests', () => { it('calls onValue only once after get request with a non-default query', async () => { const db = getDatabase(defaultApp); const { ref: testRef } = getUniqueRef(db); - const initial = [{ name: 'child1' }, { name: 'child2' }]; - const ec = EventAccumulatorFactory.waitsForExactCount(1); - - await set(testRef, initial); - const unsubscribe = onValue(testRef, snapshot => { - ec.addEvent(snapshot.val()); - }); - await get(query(testRef, limitToFirst(1))); - await waitFor(2000); - const [snap] = await ec.promise; - expect(snap).to.deep.equal(initial); - unsubscribe(); + const queries = [ + query(testRef, limitToFirst(1)), + query(testRef, startAt('child1')), + query(testRef, startAt('child2')), + query(testRef, limitToFirst(2)) + ]; + Promise.all(queries.map(async q => { + const initial = [{ name: 'child1' }, { name: 'child2' }]; + const ec = EventAccumulatorFactory.waitsForExactCount(1); + + await set(testRef, initial); + const unsubscribe = onValue(testRef, snapshot => { + ec.addEvent(snapshot.val()); + }); + await get(q); + await waitFor(2000); + const [snap] = await ec.promise; + expect(snap).to.deep.equal(initial); + unsubscribe(); + })); }); it('calls onValue and expects no issues with removing the listener', async () => { From b4c7f28a54fb1be48e441f148870bd8db93d43fc Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 8 Jun 2022 09:31:43 -0700 Subject: [PATCH 40/52] Fixed formatting --- .../database/test/exp/integration.test.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 18006e8d5f2..f80f100cc64 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -119,20 +119,22 @@ describe('Database@exp Tests', () => { query(testRef, startAt('child2')), query(testRef, limitToFirst(2)) ]; - Promise.all(queries.map(async q => { - const initial = [{ name: 'child1' }, { name: 'child2' }]; - const ec = EventAccumulatorFactory.waitsForExactCount(1); - - await set(testRef, initial); - const unsubscribe = onValue(testRef, snapshot => { - ec.addEvent(snapshot.val()); - }); - await get(q); - await waitFor(2000); - const [snap] = await ec.promise; - expect(snap).to.deep.equal(initial); - unsubscribe(); - })); + await Promise.all( + queries.map(async q => { + const initial = [{ name: 'child1' }, { name: 'child2' }]; + const ec = EventAccumulatorFactory.waitsForExactCount(1); + + await set(testRef, initial); + const unsubscribe = onValue(testRef, snapshot => { + ec.addEvent(snapshot.val()); + }); + await get(q); + await waitFor(2000); + const [snap] = await ec.promise; + expect(snap).to.deep.equal(initial); + unsubscribe(); + }) + ); }); it('calls onValue and expects no issues with removing the listener', async () => { From 55599212f78878b235f6b1582192e25200ec9305 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 8 Jun 2022 14:16:03 -0700 Subject: [PATCH 41/52] Added suggestions --- packages/database/src/core/Repo.ts | 2 +- packages/database/src/core/SyncTree.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 7f3c6e62bee..824e0b52cae 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -465,7 +465,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cached != null) { return Promise.resolve(cached); } - const tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); + let tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); return repo.server_.get(query).then( payload => { const node = nodeFromJSON(payload).withIndex( diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index bffb4aeb104..57eb8481bc4 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -449,7 +449,7 @@ export function syncTreeRegisterQuery(syncTree: SyncTree, query: QueryContext) { if (!syncPoint.views.has(query._queryIdentifier)) { syncPoint.views.set(query._queryIdentifier, view); } - if (!query._queryParams.loadsAllData()) { + if(!query._queryParams.loadsAllData()) { return syncTreeTagForQuery_(syncTree, query); } return null; From 52fcf722c3ab71404e3e6ef5b5fee61c9404d5a5 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 8 Jun 2022 14:38:41 -0700 Subject: [PATCH 42/52] Fixed formatting --- packages/database/src/core/SyncTree.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database/src/core/SyncTree.ts b/packages/database/src/core/SyncTree.ts index 57eb8481bc4..bffb4aeb104 100644 --- a/packages/database/src/core/SyncTree.ts +++ b/packages/database/src/core/SyncTree.ts @@ -449,7 +449,7 @@ export function syncTreeRegisterQuery(syncTree: SyncTree, query: QueryContext) { if (!syncPoint.views.has(query._queryIdentifier)) { syncPoint.views.set(query._queryIdentifier, view); } - if(!query._queryParams.loadsAllData()) { + if (!query._queryParams.loadsAllData()) { return syncTreeTagForQuery_(syncTree, query); } return null; From c55340509f02417e92cfc3d9d3cf70b5b37cd112 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 8 Jun 2022 14:39:26 -0700 Subject: [PATCH 43/52] Replaced let with const --- packages/database/src/core/Repo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 824e0b52cae..7f3c6e62bee 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -465,7 +465,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cached != null) { return Promise.resolve(cached); } - let tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); + const tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); return repo.server_.get(query).then( payload => { const node = nodeFromJSON(payload).withIndex( From a7e58656d49a0134c2275c56545a712571fbc40b Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 9 Jun 2022 08:53:37 -0700 Subject: [PATCH 44/52] Fixed tests --- packages/database/src/core/Repo.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 7f3c6e62bee..7843482d9a3 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -465,7 +465,10 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cached != null) { return Promise.resolve(cached); } - const tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); + let tag: number; + if (!query._queryParams.loadsAllData()) { + tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); + } return repo.server_.get(query).then( payload => { const node = nodeFromJSON(payload).withIndex( From 98845d169650d18fc5ee9e2aed704f389de80c88 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 9 Jun 2022 09:42:33 -0700 Subject: [PATCH 45/52] Fixed test --- packages/database/src/core/Repo.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 7843482d9a3..e4143f01eaa 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -465,10 +465,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cached != null) { return Promise.resolve(cached); } - let tag: number; - if (!query._queryParams.loadsAllData()) { - tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); - } + const tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); return repo.server_.get(query).then( payload => { const node = nodeFromJSON(payload).withIndex( @@ -490,15 +487,16 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { ); // Call `syncTreeRemoveEventRegistration` with a null event registration, since there is none. // Note: The below code essentially unregisters the query and cleans up any views/syncpoints temporarily created above. - const cancels = syncTreeRemoveEventRegistration( + + } + const cancels = syncTreeRemoveEventRegistration( repo.serverSyncTree_, query, null ); if (cancels.length > 0) { repoLog(repo, 'unexpected cancel events in repoGetValue'); - } - } + } return Promise.resolve(node); }, err => { From 69feeb70715b43c23ecd71eef734a500140908a8 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 9 Jun 2022 09:43:22 -0700 Subject: [PATCH 46/52] Fixed formatting --- packages/database/src/core/Repo.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index e4143f01eaa..ee8843ab3f1 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -487,16 +487,15 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { ); // Call `syncTreeRemoveEventRegistration` with a null event registration, since there is none. // Note: The below code essentially unregisters the query and cleans up any views/syncpoints temporarily created above. - } const cancels = syncTreeRemoveEventRegistration( - repo.serverSyncTree_, - query, - null - ); - if (cancels.length > 0) { - repoLog(repo, 'unexpected cancel events in repoGetValue'); - } + repo.serverSyncTree_, + query, + null + ); + if (cancels.length > 0) { + repoLog(repo, 'unexpected cancel events in repoGetValue'); + } return Promise.resolve(node); }, err => { From 60f9da031805e4ba670df16d93919023cc43cc5e Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 08:44:58 -0700 Subject: [PATCH 47/52] Fixed tests --- packages/database/src/core/PersistentConnection.ts | 2 +- packages/database/src/core/Repo.ts | 4 ++-- packages/database/test/exp/integration.test.ts | 9 ++++++++- packages/database/test/transport.test.ts | 3 ++- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/database/src/core/PersistentConnection.ts b/packages/database/src/core/PersistentConnection.ts index 95bdd9d9041..beeaa4e4389 100644 --- a/packages/database/src/core/PersistentConnection.ts +++ b/packages/database/src/core/PersistentConnection.ts @@ -259,7 +259,7 @@ export class PersistentConnection extends ServerActions { ); assert( !this.listens.get(pathString)!.has(queryId), - 'listen() called twice for same path/queryId.' + `listen() called twice for same path/queryId.` ); const listenSpec: ListenSpec = { onComplete, diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index ee8843ab3f1..3c2a457ba59 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -465,7 +465,6 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cached != null) { return Promise.resolve(cached); } - const tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); return repo.server_.get(query).then( payload => { const node = nodeFromJSON(payload).withIndex( @@ -479,6 +478,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { // We do this (along with the syncTreeRemoveEventRegistration` below) so that // `repoGetValue` results have the same cache effects as initial listener(s) // updates. + const tag = syncTreeRegisterQuery(repo.serverSyncTree_, query); syncTreeApplyTaggedQueryOverwrite( repo.serverSyncTree_, query._path, @@ -496,7 +496,7 @@ export function repoGetValue(repo: Repo, query: QueryContext): Promise { if (cancels.length > 0) { repoLog(repo, 'unexpected cancel events in repoGetValue'); } - return Promise.resolve(node); + return node; }, err => { repoLog(repo, 'get for query ' + stringify(query) + ' failed: ' + err); diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index f80f100cc64..9028714459b 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -208,7 +208,14 @@ describe('Database@exp Tests', () => { const unsubscribe = onValue(nestedRef, snapshot => { ea.addEvent(snapshot.val()); }); - const result = await get(query(testRef)); // commented out to make sure the unsubscribe isn't due to a recent change. + /* + 1. Set the value. + 2. Create the addEventRegistration /test + a. listen on /test + 3. Get the value / + 4. removeEventRegistration for / and then + */ + const result = await get(query(testRef)); const events = await ea.promise; await waitFor(2000); expect(events.length).to.equal(1); diff --git a/packages/database/test/transport.test.ts b/packages/database/test/transport.test.ts index 4f4407bf3f2..56c49d9fb55 100644 --- a/packages/database/test/transport.test.ts +++ b/packages/database/test/transport.test.ts @@ -29,7 +29,7 @@ import { WebSocketConnection } from '../src/realtime/WebSocketConnection'; use(sinonChai); const transportInitError = 'Transport has already been initialized. Please call this function before calling ref or setting up a listener'; -describe('Force Transport', () => { +describe.only('Force Transport', () => { const oldNodeValue = CONSTANTS.NODE_CLIENT; let mySandbox: SinonSandbox; let spyWarn: SinonSpy; @@ -47,6 +47,7 @@ describe('Force Transport', () => { WebSocketConnection.forceDisallow_ = false; mySandbox.restore(); }); + // Fails without retry due to running tests in async it('should enable websockets and disable longPolling', () => { forceWebSockets(); expect(spyWarn.called).to.equal(false); From 6f39fef07e81991665e76e8927e0ba38d1051ec3 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 08:46:00 -0700 Subject: [PATCH 48/52] Removed comments --- packages/database/test/exp/integration.test.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 9028714459b..998fe2c8591 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -208,13 +208,6 @@ describe('Database@exp Tests', () => { const unsubscribe = onValue(nestedRef, snapshot => { ea.addEvent(snapshot.val()); }); - /* - 1. Set the value. - 2. Create the addEventRegistration /test - a. listen on /test - 3. Get the value / - 4. removeEventRegistration for / and then - */ const result = await get(query(testRef)); const events = await ea.promise; await waitFor(2000); From 57a8db593ab3d507dbdf52e64661e6ab7d406022 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 09:06:16 -0700 Subject: [PATCH 49/52] Removed version of uuid --- packages/database/package.json | 2 +- packages/database/test/transport.test.ts | 2 +- yarn.lock | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/database/package.json b/packages/database/package.json index a619afab0bb..745c8e69b75 100644 --- a/packages/database/package.json +++ b/packages/database/package.json @@ -57,7 +57,7 @@ "rollup": "2.72.1", "rollup-plugin-typescript2": "0.31.2", "typescript": "4.2.2", - "uuid": "^8.0.0" + "uuid": "^8.3.2" }, "repository": { "directory": "packages/database", diff --git a/packages/database/test/transport.test.ts b/packages/database/test/transport.test.ts index 56c49d9fb55..7daee52f5db 100644 --- a/packages/database/test/transport.test.ts +++ b/packages/database/test/transport.test.ts @@ -29,7 +29,7 @@ import { WebSocketConnection } from '../src/realtime/WebSocketConnection'; use(sinonChai); const transportInitError = 'Transport has already been initialized. Please call this function before calling ref or setting up a listener'; -describe.only('Force Transport', () => { +describe('Force Transport', () => { const oldNodeValue = CONSTANTS.NODE_CLIENT; let mySandbox: SinonSandbox; let spyWarn: SinonSpy; diff --git a/yarn.lock b/yarn.lock index 2622b2f84e8..d0d7dfc9da8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17099,7 +17099,7 @@ uuid@^3.0.0, uuid@^3.3.2, uuid@^3.3.3: resolved "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz#b23e4358afa8a202fe7a100af1f5f883f02007ee" integrity sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A== -uuid@^8.0.0, uuid@^8.3.2: +uuid@^8.3.2: version "8.3.2" resolved "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz#80d5b5ced271bb9af6c445f21a1a04c606cefbe2" integrity sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg== From 31a747e8f85a46bace789410969708c6766549a5 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 09:09:08 -0700 Subject: [PATCH 50/52] Added a new line --- packages/database/test/exp/integration.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 998fe2c8591..c695a3ee6be 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -154,6 +154,7 @@ describe('Database@exp Tests', () => { const [snap1] = await ea.promise; expect(snap1).to.deep.eq(initial); }); + it('calls onValue only once after get request with a default query', async () => { const db = getDatabase(defaultApp); const { ref: testRef } = getUniqueRef(db); From bd23960cb226a9836fc4371ca0ea1d5229b7d2c9 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 17:19:57 -0700 Subject: [PATCH 51/52] Addressed comments --- packages/database/test/helpers/util.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index 8e9e45ed3ea..177bec1b38e 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { v4 as uuidv4 } from 'uuid'; import { Database, ref } from '../../src'; import { ConnectionTarget } from '../../src/api/test_access'; @@ -80,6 +79,18 @@ export function waitFor(waitTimeInMS: number) { return new Promise(resolve => setTimeout(resolve, waitTimeInMS)); } +/** + * Copied from https://stackoverflow.com/a/2117523 + * TODO: extract this into @firebase/util + */ +export function uuidv4(): string { + return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, c => { + const r = (Math.random() * 16) | 0, + v = c === 'x' ? r : (r & 0x3) | 0x8; + return v.toString(16); + }); +} + // Creates a unique reference using uuid export function getUniqueRef(db: Database) { const path = uuidv4(); From 7e09d4b21cd5c2b2051be5f698ccfbd19f3b0352 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 17:27:32 -0700 Subject: [PATCH 52/52] Fixed formatting --- packages/database/test/helpers/util.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index 177bec1b38e..83ef94cdd43 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -15,7 +15,6 @@ * limitations under the License. */ - import { Database, ref } from '../../src'; import { ConnectionTarget } from '../../src/api/test_access';