Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add totalDocuments and totalBytes to bundle metadata. #1085

Merged
merged 4 commits into from
May 26, 2020
Merged

feat: Add totalDocuments and totalBytes to bundle metadata. #1085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be577dd
feat: Add totalDocuments and totalBytes to bundle metadata.
wu-hui May 19, 2020
095e12d
fix: Better comment
wu-hui May 19, 2020
38abf77
fix: Better testing.
wu-hui May 21, 2020
6905f59
fix: Improve metadata testing.
wu-hui May 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions dev/protos/firestore/bundle.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions dev/protos/firestore_v1_proto_api.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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
Expand Down
22 changes: 13 additions & 9 deletions dev/src/bundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -168,6 +159,19 @@ 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,
};
// Prepends the metadata element to the bundleBuffer: `bundleBuffer` is the second argument to `Buffer.concat`.
bundleBuffer = Buffer.concat([
this.elementToLengthPrefixedBuffer({metadata}),
bundleBuffer,
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand how this will work. I assume this was added to allow the progress handler to display their status. For that to work, would this not need to be the first message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the order of the arguments to Buffer.concat is different here. Also added a comment.

return bundleBuffer;
}
}
Expand Down
29 changes: 11 additions & 18 deletions dev/system-test/firestore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
WriteResult,
} from '../src';
import {autoId, Deferred} from '../src/util';
import {TEST_BUNDLE_ID, verifyMetadata} from '../test/bundle';
import {
bundleToElementArray,
Post,
Expand Down Expand Up @@ -2407,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();

Expand All @@ -2416,11 +2417,7 @@ describe('Bundle building', () => {
const elements = await bundleToElementArray(bundle.build());

const meta = (elements[0] as IBundleElement).metadata;
expect(meta).to.deep.equal({
id: 'test-bundle',
createTime: snap.readTime.toProto().timestampValue,
version: 1,
});
verifyMetadata(meta!, snap.readTime.toProto().timestampValue!, 0);

const namedQuery = (elements[1] as IBundleElement).namedQuery;
// Verify saved query.
Expand All @@ -2440,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);
Expand All @@ -2449,11 +2446,7 @@ describe('Bundle building', () => {
expect(elements.length).to.equal(2);

const meta = (elements[0] as IBundleElement).metadata;
expect(meta).to.deep.equal({
id: 'test-bundle',
createTime: snap.readTime.toProto().timestampValue,
version: 1,
});
verifyMetadata(meta!, snap.readTime.toProto().timestampValue!, 1);

const docMeta = (elements[1] as IBundleElement).documentMetadata;
expect(docMeta).to.deep.equal({
Expand All @@ -2464,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);
Expand All @@ -2476,11 +2469,11 @@ describe('Bundle building', () => {
const elements = await bundleToElementArray(await bundle.build());

const meta = (elements[0] as IBundleElement).metadata;
expect(meta).to.deep.equal({
id: 'test-bundle',
createTime: limitToLastSnap.readTime.toProto().timestampValue,
version: 1,
});
verifyMetadata(
meta!,
limitToLastSnap.readTime.toProto().timestampValue!,
1
);

let namedQuery1 = (elements[1] as IBundleElement).namedQuery;
let namedQuery2 = (elements[2] as IBundleElement).namedQuery;
Expand Down
82 changes: 56 additions & 26 deletions dev/test/bundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -24,6 +24,28 @@ import {
verifyInstance,
} 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,
createTime: ITimestamp,
totalDocuments: number,
expectEmptyContent = false
): void {
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', () => {
let firestore: Firestore;
Expand All @@ -49,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`,
Expand Down Expand Up @@ -80,12 +102,12 @@ describe('Bundle Buidler', () => {
expect(elements.length).to.equal(3);

const meta = (elements[0] as IBundleElement).metadata;
expect(meta).to.deep.equal({
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,
});
snap1.readTime.toProto().timestampValue!,
1
);

// Verify doc1Meta and doc1Snap
const docMeta = (elements[1] as IBundleElement).documentMetadata;
Expand All @@ -99,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`,
Expand Down Expand Up @@ -128,12 +150,12 @@ describe('Bundle Buidler', () => {
expect(elements.length).to.equal(4);

const meta = (elements[0] as IBundleElement).metadata;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind adding a helper for the metadata comparison?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

expect(meta).to.deep.equal({
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,
});
snap.readTime.toProto().timestampValue!,
1
);

// Verify named query
const namedQuery = (elements[1] as IBundleElement).namedQuery;
Expand Down Expand Up @@ -162,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`,
Expand All @@ -181,12 +203,12 @@ describe('Bundle Buidler', () => {
expect(elements.length).to.equal(3);

const meta = (elements[0] as IBundleElement).metadata;
expect(meta).to.deep.equal({
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,
});
snap1.readTime.toProto().timestampValue!,
1
);

// Verify doc1Meta and doc1Snap
const doc1Meta = (elements[1] as IBundleElement).documentMetadata;
Expand Down Expand Up @@ -215,7 +237,14 @@ 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;
verifyMetadata(
newMeta!,
// `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`.
snap1.readTime.toProto().timestampValue!,
2
);
expect(newElements.slice(1, 3)).to.deep.equal(elements.slice(1));

// Verify doc2Meta and doc2Snap
const doc2Meta = (newElements[3] as IBundleElement).documentMetadata;
Expand All @@ -229,17 +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,
});
verifyMetadata(
meta!,
new Timestamp(0, 0).toProto().timestampValue!,
0,
true
);
});
});