From 0207dbf1d540175057f54fb3213e3ac3b1f99f94 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Fri, 15 Jan 2021 10:34:10 -0600 Subject: [PATCH 1/6] Add toJSON() support for Firestore classes --- .changeset/clean-meals-double.md | 8 ++++++ packages-exp/app-compat/src/firebaseApp.ts | 8 ++++++ packages/app-types/index.d.ts | 2 ++ packages/app/src/firebaseApp.ts | 8 ++++++ packages/firestore/src/remote/backoff.ts | 15 ++++++++--- .../firestore/src/util/async_queue_impl.ts | 9 +++++++ .../firestore/test/unit/api/database.test.ts | 25 ++++++++++++++++++ .../test/unit/util/async_queue.test.ts | 26 +++++++++++++++++++ 8 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 .changeset/clean-meals-double.md diff --git a/.changeset/clean-meals-double.md b/.changeset/clean-meals-double.md new file mode 100644 index 00000000000..78777de5c1f --- /dev/null +++ b/.changeset/clean-meals-double.md @@ -0,0 +1,8 @@ +--- +'@firebase/app-compat': patch +'@firebase/app': patch +'@firebase/app-types': patch +'@firebase/firestore': patch +--- + +Firestore classes like DocumentReference and Query can now be serialized to JSON (#4258) diff --git a/packages-exp/app-compat/src/firebaseApp.ts b/packages-exp/app-compat/src/firebaseApp.ts index 19fe04d0cd0..ade5cbc11e5 100644 --- a/packages-exp/app-compat/src/firebaseApp.ts +++ b/packages-exp/app-compat/src/firebaseApp.ts @@ -134,6 +134,14 @@ export class FirebaseAppImpl implements FirebaseApp { _addOrOverwriteComponent(component: Component): void { _addOrOverwriteComponent(this.app, component); } + + toJSON(): object { + return { + name: this.name, + automaticDataCollectionEnabled: this.automaticDataCollectionEnabled, + options: this.options + }; + } } // TODO: investigate why the following needs to be commented out diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts index 4aad68c7173..b7043531126 100644 --- a/packages/app-types/index.d.ts +++ b/packages/app-types/index.d.ts @@ -112,6 +112,8 @@ export interface FirebaseNamespace { // Sets log handler for all Firebase components. onLog(logCallback: LogCallback, options?: LogOptions): void; + toJSON(): object; + // The current SDK version. SDK_VERSION: string; } diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 64ad5e21716..81d27eea699 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -163,6 +163,14 @@ export class FirebaseAppImpl implements FirebaseApp { this.container.addOrOverwriteComponent(component); } + toJSON(): object { + return { + name: this.name, + automaticDataCollectionEnabled: this.automaticDataCollectionEnabled, + options: this.options + }; + } + /** * This function will throw an Error if the App has already been deleted - * use before performing API actions on the App. diff --git a/packages/firestore/src/remote/backoff.ts b/packages/firestore/src/remote/backoff.ts index 6f3ebf59861..fe995311f4c 100644 --- a/packages/firestore/src/remote/backoff.ts +++ b/packages/firestore/src/remote/backoff.ts @@ -24,12 +24,12 @@ const LOG_TAG = 'ExponentialBackoff'; * Initial backoff time in milliseconds after an error. * Set to 1s according to https://cloud.google.com/apis/design/errors. */ -const DEFAULT_BACKOFF_INITIAL_DELAY_MS = 1000; +export const DEFAULT_BACKOFF_INITIAL_DELAY_MS = 1000; -const DEFAULT_BACKOFF_FACTOR = 1.5; +export const DEFAULT_BACKOFF_FACTOR = 1.5; /** Maximum backoff time in milliseconds */ -const DEFAULT_BACKOFF_MAX_DELAY_MS = 60 * 1000; +export const DEFAULT_BACKOFF_MAX_DELAY_MS = 60 * 1000; /** * A helper for running delayed tasks following an exponential backoff curve @@ -163,6 +163,15 @@ export class ExponentialBackoff { } } + toJSON(): object { + return { + timerId: this.timerId, + initialDelayMs: this.initialDelayMs, + backoffFactor: this.backoffFactor, + maxDelayMs: this.maxDelayMs + }; + } + /** Returns a random value in the range [-currentBaseMs/2, currentBaseMs/2] */ private jitterDelayMs(): number { return (Math.random() - 0.5) * this.currentBaseMs; diff --git a/packages/firestore/src/util/async_queue_impl.ts b/packages/firestore/src/util/async_queue_impl.ts index d012df905fc..412f9bcc599 100644 --- a/packages/firestore/src/util/async_queue_impl.ts +++ b/packages/firestore/src/util/async_queue_impl.ts @@ -295,6 +295,15 @@ export class AsyncQueueImpl implements AsyncQueue { debugAssert(index >= 0, 'Delayed operation not found.'); this.delayedOperations.splice(index, 1); } + + toJSON(): object { + return { + retryableOperationsCount: this.retryableOps.length, + delayedOperationsCount: this.delayedOperations.length, + operationsInProgress: this.operationInProgress, + backoff: this.backoff + }; + } } export function newAsyncQueue(): AsyncQueue { diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index 0aff83ac970..ee8cfac8b52 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -32,6 +32,10 @@ describe('CollectionReference', () => { expectEqual(collectionReference('foo'), collectionReference('foo')); expectNotEqual(collectionReference('foo'), collectionReference('bar')); }); + + it('JSON.stringify() does not throw', () => { + JSON.stringify(collectionReference('foo')); + }); }); describe('DocumentReference', () => { @@ -42,6 +46,10 @@ describe('DocumentReference', () => { documentReference('rooms/bar') ); }); + + it('JSON.stringify() does not throw', () => { + JSON.stringify(documentReference('foo/bar')); + }); }); describe('DocumentSnapshot', () => { @@ -72,6 +80,13 @@ describe('DocumentSnapshot', () => { documentSnapshot('rooms/bar', { a: 1 }, false) ); }); + + it('JSON.stringify() does not throw', () => { + JSON.stringify(documentSnapshot('foo/bar', { a: 1 }, true)); + JSON.stringify(documentSnapshot('foo/bar', { a: 1 }, false)); + JSON.stringify(documentSnapshot('foo/bar', null, true)); + JSON.stringify(documentSnapshot('foo/bar', null, false)); + }); }); describe('Query', () => { @@ -79,6 +94,10 @@ describe('Query', () => { expectEqual(query('foo'), query('foo')); expectNotEqual(query('foo'), query('bar')); }); + + it('JSON.stringify() does not throw', () => { + JSON.stringify(query('foo')); + }); }); describe('QuerySnapshot', () => { @@ -123,6 +142,12 @@ describe('QuerySnapshot', () => { querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, true) ); }); + + it('JSON.stringify() does not throw', () => { + JSON.stringify( + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false) + ); + }); }); describe('SnapshotMetadata', () => { diff --git a/packages/firestore/test/unit/util/async_queue.test.ts b/packages/firestore/test/unit/util/async_queue.test.ts index 2cc65a2a170..8fb003d2af0 100644 --- a/packages/firestore/test/unit/util/async_queue.test.ts +++ b/packages/firestore/test/unit/util/async_queue.test.ts @@ -20,6 +20,11 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import { IndexedDbTransactionError } from '../../../src/local/simple_db'; +import { + DEFAULT_BACKOFF_FACTOR, + DEFAULT_BACKOFF_INITIAL_DELAY_MS, + DEFAULT_BACKOFF_MAX_DELAY_MS +} from '../../../src/remote/backoff'; import { fail } from '../../../src/util/assert'; import { TimerId } from '../../../src/util/async_queue'; import { @@ -417,6 +422,27 @@ describe('AsyncQueue', () => { await queue.drain(); expect(completedSteps).to.deep.equal([1, 2, 4]); }); + + it('serializes to JSON', async () => { + const queue = newAsyncQueue() as AsyncQueueImpl; + // eslint-disable-next-line @typescript-eslint/no-floating-promises + queue.enqueueAfterDelay(timerId1, 20000, () => Promise.resolve()); + // toJSON() does not recursively call itself when serializing elements. + // We use JSON.stringify() here in order to check that `backoff` is + // serialized properly. + expect(JSON.parse(JSON.stringify(queue.toJSON()))).to.deep.equal({ + delayedOperationsCount: 1, + operationsInProgress: false, + retryableOperationsCount: 0, + backoff: { + timerId: TimerId.AsyncQueueRetry, + initialDelayMs: DEFAULT_BACKOFF_INITIAL_DELAY_MS, + backoffFactor: DEFAULT_BACKOFF_FACTOR, + maxDelayMs: DEFAULT_BACKOFF_MAX_DELAY_MS + } + }); + await queue.drain(); + }); }); function defer(op: () => T): Promise { From 2f631051bb718b732aa9f425bd589cd30ee031a1 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Fri, 15 Jan 2021 14:57:50 -0600 Subject: [PATCH 2/6] Serialize FirebaseFirestore instead --- packages/firestore/src/exp/snapshot.ts | 12 ++++----- packages/firestore/src/lite/database.ts | 7 +++++ packages/firestore/src/lite/snapshot.ts | 6 ++--- packages/firestore/src/remote/backoff.ts | 15 +++-------- .../firestore/src/util/async_queue_impl.ts | 9 ------- .../test/integration/api/query.test.ts | 4 +-- .../test/integration/api/type.test.ts | 4 +-- .../firestore/test/unit/specs/spec_builder.ts | 6 ++--- .../test/unit/util/async_queue.test.ts | 26 ------------------- 9 files changed, 24 insertions(+), 65 deletions(-) diff --git a/packages/firestore/src/exp/snapshot.ts b/packages/firestore/src/exp/snapshot.ts index bda734a8ecb..414f0300ada 100644 --- a/packages/firestore/src/exp/snapshot.ts +++ b/packages/firestore/src/exp/snapshot.ts @@ -214,9 +214,9 @@ export interface DocumentChange { * access will return 'undefined'. You can use the `exists()` method to * explicitly verify a document's existence. */ -export class DocumentSnapshot extends LiteDocumentSnapshot< - T -> { +export class DocumentSnapshot< + T = DocumentData +> extends LiteDocumentSnapshot { private readonly _firestoreImpl: FirebaseFirestore; /** @@ -329,9 +329,9 @@ export class DocumentSnapshot extends LiteDocumentSnapshot< * `exists` property will always be true and `data()` will never return * 'undefined'. */ -export class QueryDocumentSnapshot extends DocumentSnapshot< - T -> { +export class QueryDocumentSnapshot< + T = DocumentData +> extends DocumentSnapshot { /** * Retrieves all fields in the document as an `Object`. * diff --git a/packages/firestore/src/lite/database.ts b/packages/firestore/src/lite/database.ts index 9d0d2906171..58a439d89bc 100644 --- a/packages/firestore/src/lite/database.ts +++ b/packages/firestore/src/lite/database.ts @@ -134,6 +134,13 @@ export class FirebaseFirestore implements FirestoreService { return this._terminateTask; } + toJSON(): object { + return { + databaseId: this._databaseId, + settings: this._settings + }; + } + /** * Terminates all components used by this client. Subclasses can override * this method to clean up their own dependencies, but must also call this diff --git a/packages/firestore/src/lite/snapshot.ts b/packages/firestore/src/lite/snapshot.ts index 5aeef50947c..350dd109b13 100644 --- a/packages/firestore/src/lite/snapshot.ts +++ b/packages/firestore/src/lite/snapshot.ts @@ -212,9 +212,9 @@ export class DocumentSnapshot { * `exists` property will always be true and `data()` will never return * 'undefined'. */ -export class QueryDocumentSnapshot extends DocumentSnapshot< - T -> { +export class QueryDocumentSnapshot< + T = DocumentData +> extends DocumentSnapshot { /** * Retrieves all fields in the document as an `Object`. * diff --git a/packages/firestore/src/remote/backoff.ts b/packages/firestore/src/remote/backoff.ts index fe995311f4c..6f3ebf59861 100644 --- a/packages/firestore/src/remote/backoff.ts +++ b/packages/firestore/src/remote/backoff.ts @@ -24,12 +24,12 @@ const LOG_TAG = 'ExponentialBackoff'; * Initial backoff time in milliseconds after an error. * Set to 1s according to https://cloud.google.com/apis/design/errors. */ -export const DEFAULT_BACKOFF_INITIAL_DELAY_MS = 1000; +const DEFAULT_BACKOFF_INITIAL_DELAY_MS = 1000; -export const DEFAULT_BACKOFF_FACTOR = 1.5; +const DEFAULT_BACKOFF_FACTOR = 1.5; /** Maximum backoff time in milliseconds */ -export const DEFAULT_BACKOFF_MAX_DELAY_MS = 60 * 1000; +const DEFAULT_BACKOFF_MAX_DELAY_MS = 60 * 1000; /** * A helper for running delayed tasks following an exponential backoff curve @@ -163,15 +163,6 @@ export class ExponentialBackoff { } } - toJSON(): object { - return { - timerId: this.timerId, - initialDelayMs: this.initialDelayMs, - backoffFactor: this.backoffFactor, - maxDelayMs: this.maxDelayMs - }; - } - /** Returns a random value in the range [-currentBaseMs/2, currentBaseMs/2] */ private jitterDelayMs(): number { return (Math.random() - 0.5) * this.currentBaseMs; diff --git a/packages/firestore/src/util/async_queue_impl.ts b/packages/firestore/src/util/async_queue_impl.ts index 412f9bcc599..d012df905fc 100644 --- a/packages/firestore/src/util/async_queue_impl.ts +++ b/packages/firestore/src/util/async_queue_impl.ts @@ -295,15 +295,6 @@ export class AsyncQueueImpl implements AsyncQueue { debugAssert(index >= 0, 'Delayed operation not found.'); this.delayedOperations.splice(index, 1); } - - toJSON(): object { - return { - retryableOperationsCount: this.retryableOps.length, - delayedOperationsCount: this.delayedOperations.length, - operationsInProgress: this.operationInProgress, - backoff: this.backoff - }; - } } export function newAsyncQueue(): AsyncQueue { diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 10bab8cc329..eab9995dd4a 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -155,9 +155,7 @@ apiDescribe('Queries', (persistence: boolean) => { .onSnapshot(storeLimitEvent.storeEvent); // Setup mirroring `limitToLast` query - const storeLimitToLastEvent = new EventsAccumulator< - firestore.QuerySnapshot - >(); + const storeLimitToLastEvent = new EventsAccumulator(); let limitToLastUnlisten = collection .orderBy('sort', 'desc') .limitToLast(2) diff --git a/packages/firestore/test/integration/api/type.test.ts b/packages/firestore/test/integration/api/type.test.ts index 310b1906054..9c48dd40c68 100644 --- a/packages/firestore/test/integration/api/type.test.ts +++ b/packages/firestore/test/integration/api/type.test.ts @@ -53,9 +53,7 @@ apiDescribe('Firestore', (persistence: boolean) => { docSnapshot = querySnapshot.docs[0]; expect(docSnapshot.data()).to.deep.equal(data); - const eventsAccumulator = new EventsAccumulator< - firestore.QuerySnapshot - >(); + const eventsAccumulator = new EventsAccumulator(); const unlisten = collection.onSnapshot(eventsAccumulator.storeEvent); querySnapshot = await eventsAccumulator.awaitEvent(); docSnapshot = querySnapshot.docs[0]; diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index 500a558fb6c..d5b82968fc0 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -1047,9 +1047,9 @@ export class SpecBuilder { return { key: SpecBuilder.keyToSpec(doc.key), version: doc.version.toMicroseconds(), - value: userDataWriter.convertValue(doc.toProto()) as JsonObject< - unknown - >, + value: userDataWriter.convertValue( + doc.toProto() + ) as JsonObject, options: { hasLocalMutations: doc.hasLocalMutations, hasCommittedMutations: doc.hasCommittedMutations diff --git a/packages/firestore/test/unit/util/async_queue.test.ts b/packages/firestore/test/unit/util/async_queue.test.ts index 8fb003d2af0..2cc65a2a170 100644 --- a/packages/firestore/test/unit/util/async_queue.test.ts +++ b/packages/firestore/test/unit/util/async_queue.test.ts @@ -20,11 +20,6 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import { IndexedDbTransactionError } from '../../../src/local/simple_db'; -import { - DEFAULT_BACKOFF_FACTOR, - DEFAULT_BACKOFF_INITIAL_DELAY_MS, - DEFAULT_BACKOFF_MAX_DELAY_MS -} from '../../../src/remote/backoff'; import { fail } from '../../../src/util/assert'; import { TimerId } from '../../../src/util/async_queue'; import { @@ -422,27 +417,6 @@ describe('AsyncQueue', () => { await queue.drain(); expect(completedSteps).to.deep.equal([1, 2, 4]); }); - - it('serializes to JSON', async () => { - const queue = newAsyncQueue() as AsyncQueueImpl; - // eslint-disable-next-line @typescript-eslint/no-floating-promises - queue.enqueueAfterDelay(timerId1, 20000, () => Promise.resolve()); - // toJSON() does not recursively call itself when serializing elements. - // We use JSON.stringify() here in order to check that `backoff` is - // serialized properly. - expect(JSON.parse(JSON.stringify(queue.toJSON()))).to.deep.equal({ - delayedOperationsCount: 1, - operationsInProgress: false, - retryableOperationsCount: 0, - backoff: { - timerId: TimerId.AsyncQueueRetry, - initialDelayMs: DEFAULT_BACKOFF_INITIAL_DELAY_MS, - backoffFactor: DEFAULT_BACKOFF_FACTOR, - maxDelayMs: DEFAULT_BACKOFF_MAX_DELAY_MS - } - }); - await queue.drain(); - }); }); function defer(op: () => T): Promise { From 7c31f91c76ca9510cc59e67f86c5f984af7bf30b Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Tue, 19 Jan 2021 15:22:16 -0600 Subject: [PATCH 3/6] Remove toJSON() from index files --- packages/app-types/index.d.ts | 2 -- packages/firestore/src/lite/database.ts | 1 + packages/firestore/test/unit/api/database.test.ts | 3 --- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts index b7043531126..4aad68c7173 100644 --- a/packages/app-types/index.d.ts +++ b/packages/app-types/index.d.ts @@ -112,8 +112,6 @@ export interface FirebaseNamespace { // Sets log handler for all Firebase components. onLog(logCallback: LogCallback, options?: LogOptions): void; - toJSON(): object; - // The current SDK version. SDK_VERSION: string; } diff --git a/packages/firestore/src/lite/database.ts b/packages/firestore/src/lite/database.ts index 58a439d89bc..6ecb3d50647 100644 --- a/packages/firestore/src/lite/database.ts +++ b/packages/firestore/src/lite/database.ts @@ -136,6 +136,7 @@ export class FirebaseFirestore implements FirestoreService { toJSON(): object { return { + app: this.app.name, databaseId: this._databaseId, settings: this._settings }; diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index ee8cfac8b52..f01c5d509eb 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -83,9 +83,6 @@ describe('DocumentSnapshot', () => { it('JSON.stringify() does not throw', () => { JSON.stringify(documentSnapshot('foo/bar', { a: 1 }, true)); - JSON.stringify(documentSnapshot('foo/bar', { a: 1 }, false)); - JSON.stringify(documentSnapshot('foo/bar', null, true)); - JSON.stringify(documentSnapshot('foo/bar', null, false)); }); }); From 0364a35dce916aa51355ba2e272c5ff6385da443 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Tue, 19 Jan 2021 16:45:09 -0600 Subject: [PATCH 4/6] use _app to handled undefined states --- packages/firestore/src/lite/database.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/lite/database.ts b/packages/firestore/src/lite/database.ts index 6ecb3d50647..84fd0a993a9 100644 --- a/packages/firestore/src/lite/database.ts +++ b/packages/firestore/src/lite/database.ts @@ -136,7 +136,7 @@ export class FirebaseFirestore implements FirestoreService { toJSON(): object { return { - app: this.app.name, + app: this._app, databaseId: this._databaseId, settings: this._settings }; From 85adcdb36aa3a70b40c4855bf08b672d2d174c7e Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Tue, 26 Jan 2021 14:00:05 -0600 Subject: [PATCH 5/6] add test and lite toJSON() --- packages/app/src/lite/firebaseAppLite.ts | 8 ++++++++ packages/app/test/firebaseApp.test.ts | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index bc1be941a97..14795bb104c 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -141,4 +141,12 @@ export class FirebaseAppLiteImpl implements FirebaseApp { throw ERROR_FACTORY.create(AppError.APP_DELETED, { appName: this.name_ }); } } + + toJSON(): object { + return { + name: this.name, + automaticDataCollectionEnabled: this.automaticDataCollectionEnabled, + options: this.options + }; + } } diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index cf465b04ab8..da16c6c8411 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -336,6 +336,11 @@ function firebaseAppTests( expect(() => firebase.app('toString')).throws(/'toString'.*created/i); }); + it('JSON.stringify() does not throw', () => { + const app = firebase.initializeApp({}, 'new-app'); + JSON.stringify(app); + }); + describe('Check for bad app names', () => { const tests = ['', 123, false, null]; for (const data of tests) { From 0a9f5bd3a85ba3a45d4692bbfe104277fa701904 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Tue, 26 Jan 2021 12:09:20 -0800 Subject: [PATCH 6/6] Update .changeset/clean-meals-double.md Co-authored-by: Feiyang --- .changeset/clean-meals-double.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/clean-meals-double.md b/.changeset/clean-meals-double.md index 78777de5c1f..dab4a1cd7ab 100644 --- a/.changeset/clean-meals-double.md +++ b/.changeset/clean-meals-double.md @@ -1,7 +1,6 @@ --- '@firebase/app-compat': patch '@firebase/app': patch -'@firebase/app-types': patch '@firebase/firestore': patch ---