From be577dd3f95e0dce052d30e3adf4f61ffffab1a8 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Tue, 19 May 2020 11:55:05 -0400 Subject: [PATCH 1/4] feat: Add totalDocuments and totalBytes to bundle metadata. --- dev/protos/firestore/bundle.proto | 6 ++++++ dev/protos/firestore_v1_proto_api.d.ts | 12 ++++++++++++ dev/src/bundle.ts | 21 ++++++++++++--------- dev/system-test/firestore.ts | 9 +++++++++ dev/test/bundle.ts | 23 ++++++++++++++++++++++- 5 files changed, 61 insertions(+), 10 deletions(-) diff --git a/dev/protos/firestore/bundle.proto b/dev/protos/firestore/bundle.proto index 663fc9ae4..7b71db117 100644 --- a/dev/protos/firestore/bundle.proto +++ b/dev/protos/firestore/bundle.proto @@ -91,6 +91,12 @@ message BundleMetadata { // The schema version of the bundle. uint32 version = 3; + + // The number of documents in the bundle. + uint32 total_documents = 4; + + // The size of the bundle in bytes, excluding this `BundleMetadata`. + uint64 total_bytes = 5; } // A Firestore bundle is a length-prefixed stream of JSON representations of diff --git a/dev/protos/firestore_v1_proto_api.d.ts b/dev/protos/firestore_v1_proto_api.d.ts index 2427e8dc2..fba2bd333 100644 --- a/dev/protos/firestore_v1_proto_api.d.ts +++ b/dev/protos/firestore_v1_proto_api.d.ts @@ -198,6 +198,12 @@ export namespace firestore { /** BundleMetadata version */ version?: (number|null); + + /** BundleMetadata totalDocuments */ + totalDocuments?: (number|null); + + /** BundleMetadata totalBytes */ + totalBytes?: (number|string|null); } /** Represents a BundleMetadata. */ @@ -218,6 +224,12 @@ export namespace firestore { /** BundleMetadata version. */ public version: number; + /** BundleMetadata totalDocuments. */ + public totalDocuments: number; + + /** BundleMetadata totalBytes. */ + public totalBytes: (number|string); + /** * Creates a BundleMetadata message from a plain object. Also converts values to their respective internal types. * @param object Plain object diff --git a/dev/src/bundle.ts b/dev/src/bundle.ts index 439581845..478ce7113 100644 --- a/dev/src/bundle.ts +++ b/dev/src/bundle.ts @@ -134,15 +134,6 @@ export class BundleBuilder { build(): Buffer { let bundleBuffer = Buffer.alloc(0); - const metadata: firestore.IBundleMetadata = { - id: this.bundleId, - createTime: this.latestReadTime.toProto().timestampValue, - version: BUNDLE_VERSION, - }; - bundleBuffer = Buffer.concat([ - bundleBuffer, - this.elementToLengthPrefixedBuffer({metadata}), - ]); for (const namedQuery of this.namedQueries.values()) { bundleBuffer = Buffer.concat([ @@ -168,6 +159,18 @@ export class BundleBuilder { ]); } } + + const metadata: firestore.IBundleMetadata = { + id: this.bundleId, + createTime: this.latestReadTime.toProto().timestampValue, + version: BUNDLE_VERSION, + totalDocuments: this.documents.size, + totalBytes: bundleBuffer.length, + }; + bundleBuffer = Buffer.concat([ + this.elementToLengthPrefixedBuffer({metadata}), + bundleBuffer, + ]); return bundleBuffer; } } diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index eaa923a43..dae7e3afc 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2416,10 +2416,13 @@ describe('Bundle building', () => { const elements = await bundleToElementArray(bundle.build()); const meta = (elements[0] as IBundleElement).metadata; + expect(meta!.totalBytes).greaterThan(0); + delete meta!.totalBytes; expect(meta).to.deep.equal({ id: 'test-bundle', createTime: snap.readTime.toProto().timestampValue, version: 1, + totalDocuments: 0, }); const namedQuery = (elements[1] as IBundleElement).namedQuery; @@ -2449,10 +2452,13 @@ describe('Bundle building', () => { expect(elements.length).to.equal(2); const meta = (elements[0] as IBundleElement).metadata; + expect(meta!.totalBytes).greaterThan(0); + delete meta!.totalBytes; expect(meta).to.deep.equal({ id: 'test-bundle', createTime: snap.readTime.toProto().timestampValue, version: 1, + totalDocuments: 1, }); const docMeta = (elements[1] as IBundleElement).documentMetadata; @@ -2476,10 +2482,13 @@ describe('Bundle building', () => { const elements = await bundleToElementArray(await bundle.build()); const meta = (elements[0] as IBundleElement).metadata; + expect(meta!.totalBytes).greaterThan(0); + delete meta!.totalBytes; expect(meta).to.deep.equal({ id: 'test-bundle', createTime: limitToLastSnap.readTime.toProto().timestampValue, version: 1, + totalDocuments: 1, }); let namedQuery1 = (elements[1] as IBundleElement).namedQuery; diff --git a/dev/test/bundle.ts b/dev/test/bundle.ts index ad42ca8af..c9f591587 100644 --- a/dev/test/bundle.ts +++ b/dev/test/bundle.ts @@ -80,11 +80,14 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(3); const meta = (elements[0] as IBundleElement).metadata; + expect(meta!.totalBytes).greaterThan(0); + delete meta!.totalBytes; expect(meta).to.deep.equal({ id: 'test-bundle', // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. createTime: snap1.readTime.toProto().timestampValue, version: 1, + totalDocuments: 1, }); // Verify doc1Meta and doc1Snap @@ -128,11 +131,14 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(4); const meta = (elements[0] as IBundleElement).metadata; + expect(meta!.totalBytes).greaterThan(0); + delete meta!.totalBytes; expect(meta).to.deep.equal({ id: 'test-bundle', // `snap.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. createTime: snap.readTime.toProto().timestampValue, version: 1, + totalDocuments: 1, }); // Verify named query @@ -181,11 +187,14 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(3); const meta = (elements[0] as IBundleElement).metadata; + expect(meta!.totalBytes).greaterThan(0); + delete meta!.totalBytes; expect(meta).to.deep.equal({ id: 'test-bundle', // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. createTime: snap1.readTime.toProto().timestampValue, version: 1, + totalDocuments: 1, }); // Verify doc1Meta and doc1Snap @@ -215,7 +224,17 @@ describe('Bundle Buidler', () => { const newElements = await bundleToElementArray(bundle.build()); expect(newElements.length).to.equal(5); - expect(newElements.slice(0, 3)).to.deep.equal(elements); + const newMeta = (newElements[0] as IBundleElement).metadata; + expect(newMeta!.totalBytes).greaterThan(0); + delete newMeta!.totalBytes; + expect(newMeta).to.deep.equal({ + id: 'test-bundle', + // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. + createTime: snap1.readTime.toProto().timestampValue, + version: 1, + totalDocuments: 2, + }); + expect(newElements.slice(1, 3)).to.deep.equal(elements.slice(1)); // Verify doc2Meta and doc2Snap const doc2Meta = (newElements[3] as IBundleElement).documentMetadata; @@ -240,6 +259,8 @@ describe('Bundle Buidler', () => { id: 'test-bundle', createTime: new Timestamp(0, 0).toProto().timestampValue, version: 1, + totalDocuments: 0, + totalBytes: 0, }); }); }); From 095e12dc24b13ca60b98abbd387cd8130118565f Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Tue, 19 May 2020 12:41:48 -0400 Subject: [PATCH 2/4] fix: Better comment --- dev/src/bundle.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/src/bundle.ts b/dev/src/bundle.ts index 478ce7113..d22c088dd 100644 --- a/dev/src/bundle.ts +++ b/dev/src/bundle.ts @@ -167,6 +167,7 @@ export class BundleBuilder { totalDocuments: this.documents.size, totalBytes: bundleBuffer.length, }; + // Prepends the metadata element to the bundleBuffer, note `bundleBuffer` is the second argument to `Buffer.concat`. bundleBuffer = Buffer.concat([ this.elementToLengthPrefixedBuffer({metadata}), bundleBuffer, From 38abf77a9eabed65cd264452aef69b66f6b26234 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Thu, 21 May 2020 13:16:12 -0400 Subject: [PATCH 3/4] fix: Better testing. --- dev/src/bundle.ts | 2 +- dev/system-test/firestore.ts | 13 ++++--------- dev/test/bundle.ts | 26 ++++++++++++++------------ 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/dev/src/bundle.ts b/dev/src/bundle.ts index d22c088dd..ab7749924 100644 --- a/dev/src/bundle.ts +++ b/dev/src/bundle.ts @@ -167,7 +167,7 @@ export class BundleBuilder { totalDocuments: this.documents.size, totalBytes: bundleBuffer.length, }; - // Prepends the metadata element to the bundleBuffer, note `bundleBuffer` is the second argument to `Buffer.concat`. + // Prepends the metadata element to the bundleBuffer: `bundleBuffer` is the second argument to `Buffer.concat`. bundleBuffer = Buffer.concat([ this.elementToLengthPrefixedBuffer({metadata}), bundleBuffer, diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index dae7e3afc..db9a0622a 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -35,6 +35,7 @@ import { WriteResult, } from '../src'; import {autoId, Deferred} from '../src/util'; +import {verifyMetadata} from '../test/bundle'; import { bundleToElementArray, Post, @@ -2416,9 +2417,7 @@ describe('Bundle building', () => { const elements = await bundleToElementArray(bundle.build()); const meta = (elements[0] as IBundleElement).metadata; - expect(meta!.totalBytes).greaterThan(0); - delete meta!.totalBytes; - expect(meta).to.deep.equal({ + verifyMetadata(meta!, { id: 'test-bundle', createTime: snap.readTime.toProto().timestampValue, version: 1, @@ -2452,9 +2451,7 @@ describe('Bundle building', () => { expect(elements.length).to.equal(2); const meta = (elements[0] as IBundleElement).metadata; - expect(meta!.totalBytes).greaterThan(0); - delete meta!.totalBytes; - expect(meta).to.deep.equal({ + verifyMetadata(meta!, { id: 'test-bundle', createTime: snap.readTime.toProto().timestampValue, version: 1, @@ -2482,9 +2479,7 @@ describe('Bundle building', () => { const elements = await bundleToElementArray(await bundle.build()); const meta = (elements[0] as IBundleElement).metadata; - expect(meta!.totalBytes).greaterThan(0); - delete meta!.totalBytes; - expect(meta).to.deep.equal({ + verifyMetadata(meta!, { id: 'test-bundle', createTime: limitToLastSnap.readTime.toProto().timestampValue, version: 1, diff --git a/dev/test/bundle.ts b/dev/test/bundle.ts index c9f591587..eac1ee3d3 100644 --- a/dev/test/bundle.ts +++ b/dev/test/bundle.ts @@ -24,6 +24,16 @@ import { verifyInstance, } from './util/helpers'; import IBundleElement = firestore.IBundleElement; +import IBundleMetadata = firestore.IBundleMetadata; + +export function verifyMetadata( + meta: IBundleMetadata, + expected: IBundleMetadata +): void { + expect(meta!.totalBytes).greaterThan(0); + delete meta!.totalBytes; + expect(meta).to.deep.equal(expected); +} describe('Bundle Buidler', () => { let firestore: Firestore; @@ -80,9 +90,7 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(3); const meta = (elements[0] as IBundleElement).metadata; - expect(meta!.totalBytes).greaterThan(0); - delete meta!.totalBytes; - expect(meta).to.deep.equal({ + verifyMetadata(meta!, { id: 'test-bundle', // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. createTime: snap1.readTime.toProto().timestampValue, @@ -131,9 +139,7 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(4); const meta = (elements[0] as IBundleElement).metadata; - expect(meta!.totalBytes).greaterThan(0); - delete meta!.totalBytes; - expect(meta).to.deep.equal({ + verifyMetadata(meta!, { id: 'test-bundle', // `snap.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. createTime: snap.readTime.toProto().timestampValue, @@ -187,9 +193,7 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(3); const meta = (elements[0] as IBundleElement).metadata; - expect(meta!.totalBytes).greaterThan(0); - delete meta!.totalBytes; - expect(meta).to.deep.equal({ + verifyMetadata(meta!, { id: 'test-bundle', // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. createTime: snap1.readTime.toProto().timestampValue, @@ -225,9 +229,7 @@ describe('Bundle Buidler', () => { expect(newElements.length).to.equal(5); const newMeta = (newElements[0] as IBundleElement).metadata; - expect(newMeta!.totalBytes).greaterThan(0); - delete newMeta!.totalBytes; - expect(newMeta).to.deep.equal({ + verifyMetadata(newMeta!, { id: 'test-bundle', // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. createTime: snap1.readTime.toProto().timestampValue, From 6905f5931439e57dc12f67e6fb19f587d30e5043 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Mon, 25 May 2020 10:00:29 -0400 Subject: [PATCH 4/4] fix: Improve metadata testing. --- dev/system-test/firestore.ts | 33 +++++--------- dev/test/bundle.ts | 87 +++++++++++++++++++----------------- 2 files changed, 58 insertions(+), 62 deletions(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index db9a0622a..474bb6885 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -35,7 +35,7 @@ import { WriteResult, } from '../src'; import {autoId, Deferred} from '../src/util'; -import {verifyMetadata} from '../test/bundle'; +import {TEST_BUNDLE_ID, verifyMetadata} from '../test/bundle'; import { bundleToElementArray, Post, @@ -2408,7 +2408,7 @@ describe('Bundle building', () => { afterEach(() => verifyInstance(firestore)); it('succeeds when there are no results', async () => { - const bundle = firestore._bundle('test-bundle'); + const bundle = firestore._bundle(TEST_BUNDLE_ID); const query = testCol.where('sort', '==', 5); const snap = await query.get(); @@ -2417,12 +2417,7 @@ describe('Bundle building', () => { const elements = await bundleToElementArray(bundle.build()); const meta = (elements[0] as IBundleElement).metadata; - verifyMetadata(meta!, { - id: 'test-bundle', - createTime: snap.readTime.toProto().timestampValue, - version: 1, - totalDocuments: 0, - }); + verifyMetadata(meta!, snap.readTime.toProto().timestampValue!, 0); const namedQuery = (elements[1] as IBundleElement).namedQuery; // Verify saved query. @@ -2442,7 +2437,7 @@ describe('Bundle building', () => { }); it('succeeds when added document does not exist', async () => { - const bundle = firestore._bundle('test-bundle'); + const bundle = firestore._bundle(TEST_BUNDLE_ID); const snap = await testCol.doc('doc5-not-exist').get(); bundle.add(snap); @@ -2451,12 +2446,7 @@ describe('Bundle building', () => { expect(elements.length).to.equal(2); const meta = (elements[0] as IBundleElement).metadata; - verifyMetadata(meta!, { - id: 'test-bundle', - createTime: snap.readTime.toProto().timestampValue, - version: 1, - totalDocuments: 1, - }); + verifyMetadata(meta!, snap.readTime.toProto().timestampValue!, 1); const docMeta = (elements[1] as IBundleElement).documentMetadata; expect(docMeta).to.deep.equal({ @@ -2467,7 +2457,7 @@ describe('Bundle building', () => { }); it('succeeds to save limit and limitToLast queries', async () => { - const bundle = firestore._bundle('test-bundle'); + const bundle = firestore._bundle(TEST_BUNDLE_ID); const limitQuery = testCol.orderBy('sort', 'desc').limit(1); const limitSnap = await limitQuery.get(); const limitToLastQuery = testCol.orderBy('sort', 'asc').limitToLast(1); @@ -2479,12 +2469,11 @@ describe('Bundle building', () => { const elements = await bundleToElementArray(await bundle.build()); const meta = (elements[0] as IBundleElement).metadata; - verifyMetadata(meta!, { - id: 'test-bundle', - createTime: limitToLastSnap.readTime.toProto().timestampValue, - version: 1, - totalDocuments: 1, - }); + verifyMetadata( + meta!, + limitToLastSnap.readTime.toProto().timestampValue!, + 1 + ); let namedQuery1 = (elements[1] as IBundleElement).namedQuery; let namedQuery2 = (elements[2] as IBundleElement).namedQuery; diff --git a/dev/test/bundle.ts b/dev/test/bundle.ts index eac1ee3d3..d93221233 100644 --- a/dev/test/bundle.ts +++ b/dev/test/bundle.ts @@ -15,7 +15,7 @@ import {expect} from 'chai'; import * as extend from 'extend'; import {afterEach, beforeEach, describe, it} from 'mocha'; -import {firestore} from '../protos/firestore_v1_proto_api'; +import {firestore, google} from '../protos/firestore_v1_proto_api'; import {Firestore, QuerySnapshot, Timestamp} from '../src'; import { bundleToElementArray, @@ -25,14 +25,26 @@ import { } from './util/helpers'; import IBundleElement = firestore.IBundleElement; import IBundleMetadata = firestore.IBundleMetadata; +import ITimestamp = google.protobuf.ITimestamp; + +export const TEST_BUNDLE_ID = 'test-bundle'; +const TEST_BUNDLE_VERSION = 1; export function verifyMetadata( meta: IBundleMetadata, - expected: IBundleMetadata + createTime: ITimestamp, + totalDocuments: number, + expectEmptyContent = false ): void { - expect(meta!.totalBytes).greaterThan(0); - delete meta!.totalBytes; - expect(meta).to.deep.equal(expected); + if (!expectEmptyContent) { + expect(meta.totalBytes).greaterThan(0); + } else { + expect(meta.totalBytes).to.equal(0); + } + expect(meta.id).to.equal(TEST_BUNDLE_ID); + expect(meta.version).to.equal(TEST_BUNDLE_VERSION); + expect(meta.totalDocuments).to.equal(totalDocuments); + expect(meta.createTime).to.deep.equal(createTime); } describe('Bundle Buidler', () => { @@ -59,7 +71,7 @@ describe('Bundle Buidler', () => { }); it('succeeds with document snapshots', async () => { - const bundle = firestore._bundle('test-bundle'); + const bundle = firestore._bundle(TEST_BUNDLE_ID); const snap1 = firestore.snapshot_( { name: `${DATABASE_ROOT}/documents/collectionId/doc1`, @@ -90,13 +102,12 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(3); const meta = (elements[0] as IBundleElement).metadata; - verifyMetadata(meta!, { - id: 'test-bundle', + verifyMetadata( + meta!, // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. - createTime: snap1.readTime.toProto().timestampValue, - version: 1, - totalDocuments: 1, - }); + snap1.readTime.toProto().timestampValue!, + 1 + ); // Verify doc1Meta and doc1Snap const docMeta = (elements[1] as IBundleElement).documentMetadata; @@ -110,7 +121,7 @@ describe('Bundle Buidler', () => { }); it('succeeds with query snapshots', async () => { - const bundle = firestore._bundle('test-bundle'); + const bundle = firestore._bundle(TEST_BUNDLE_ID); const snap = firestore.snapshot_( { name: `${DATABASE_ROOT}/documents/collectionId/doc1`, @@ -139,13 +150,12 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(4); const meta = (elements[0] as IBundleElement).metadata; - verifyMetadata(meta!, { - id: 'test-bundle', + verifyMetadata( + meta!, // `snap.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. - createTime: snap.readTime.toProto().timestampValue, - version: 1, - totalDocuments: 1, - }); + snap.readTime.toProto().timestampValue!, + 1 + ); // Verify named query const namedQuery = (elements[1] as IBundleElement).namedQuery; @@ -174,7 +184,7 @@ describe('Bundle Buidler', () => { }); it('succeeds with multiple calls to build()', async () => { - const bundle = firestore._bundle('test-bundle'); + const bundle = firestore._bundle(TEST_BUNDLE_ID); const snap1 = firestore.snapshot_( { name: `${DATABASE_ROOT}/documents/collectionId/doc1`, @@ -193,13 +203,12 @@ describe('Bundle Buidler', () => { expect(elements.length).to.equal(3); const meta = (elements[0] as IBundleElement).metadata; - verifyMetadata(meta!, { - id: 'test-bundle', + verifyMetadata( + meta!, // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. - createTime: snap1.readTime.toProto().timestampValue, - version: 1, - totalDocuments: 1, - }); + snap1.readTime.toProto().timestampValue!, + 1 + ); // Verify doc1Meta and doc1Snap const doc1Meta = (elements[1] as IBundleElement).documentMetadata; @@ -229,13 +238,12 @@ describe('Bundle Buidler', () => { expect(newElements.length).to.equal(5); const newMeta = (newElements[0] as IBundleElement).metadata; - verifyMetadata(newMeta!, { - id: 'test-bundle', + verifyMetadata( + newMeta!, // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`. - createTime: snap1.readTime.toProto().timestampValue, - version: 1, - totalDocuments: 2, - }); + snap1.readTime.toProto().timestampValue!, + 2 + ); expect(newElements.slice(1, 3)).to.deep.equal(elements.slice(1)); // Verify doc2Meta and doc2Snap @@ -250,19 +258,18 @@ describe('Bundle Buidler', () => { }); it('succeeds when nothing is added', async () => { - const bundle = firestore._bundle('test-bundle'); + const bundle = firestore._bundle(TEST_BUNDLE_ID); // `elements` is expected to be [bundleMeta]. const elements = await bundleToElementArray(bundle.build()); expect(elements.length).to.equal(1); const meta = (elements[0] as IBundleElement).metadata; - expect(meta).to.deep.equal({ - id: 'test-bundle', - createTime: new Timestamp(0, 0).toProto().timestampValue, - version: 1, - totalDocuments: 0, - totalBytes: 0, - }); + verifyMetadata( + meta!, + new Timestamp(0, 0).toProto().timestampValue!, + 0, + true + ); }); });