Skip to content

Commit

Permalink
fix: support byte values in Bundles (#1395)
Browse files Browse the repository at this point in the history
  • Loading branch information
wu-hui committed Feb 4, 2021
1 parent fa6721f commit 8cf53a9
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 23 deletions.
7 changes: 6 additions & 1 deletion dev/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
} from './validate';

import api = google.firestore.v1;
import BundleElement = firestore.BundleElement;

const BUNDLE_VERSION = 1;

Expand Down Expand Up @@ -142,7 +143,11 @@ export class BundleBuilder {
private elementToLengthPrefixedBuffer(
bundleElement: firestore.IBundleElement
): Buffer {
const buffer = Buffer.from(JSON.stringify(bundleElement), 'utf-8');
// Convert to a valid proto message object then take its JSON representation.
// This take cares of stuff like converting internal byte array fields
// to Base64 encodings.
const message = BundleElement.fromObject(bundleElement).toJSON();
const buffer = Buffer.from(JSON.stringify(message), 'utf-8');
const lengthBuffer = Buffer.from(buffer.length.toString());
return Buffer.concat([lengthBuffer, buffer]);
}
Expand Down
4 changes: 2 additions & 2 deletions dev/src/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,8 @@ export class DocumentSnapshot<T = firestore.DocumentData>
toDocumentProto(): api.IDocument {
return {
name: this._ref.formattedName,
createTime: this.createTime,
updateTime: this.updateTime,
createTime: this.createTime?.toProto().timestampValue,
updateTime: this.updateTime?.toProto().timestampValue,
fields: this._fieldsProto,
};
}
Expand Down
2 changes: 1 addition & 1 deletion dev/src/timestamp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export class Timestamp implements firestore.Timestamp {
const timestamp: google.protobuf.ITimestamp = {};

if (this.seconds) {
timestamp.seconds = this.seconds;
timestamp.seconds = this.seconds.toString();
}

if (this.nanoseconds) {
Expand Down
16 changes: 12 additions & 4 deletions dev/system-test/firestore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ import {
postConverterMerge,
verifyInstance,
} from '../test/util/helpers';
import IBundleElement = firestore.IBundleElement;
import {BulkWriter} from '../src/bulk-writer';
import {Status} from 'google-gax';
import {QueryPartition} from '../src/query-partition';
import {CollectionGroup} from '../src/collection-group';

import IBundleElement = firestore.IBundleElement;

use(chaiAsPromised);

const version = require('../../package.json').version;
Expand Down Expand Up @@ -2761,7 +2762,7 @@ describe('Bundle building', () => {

it('succeeds when there are no results', async () => {
const bundle = firestore.bundle(TEST_BUNDLE_ID);
const query = testCol.where('sort', '==', 5);
const query = testCol.where('value', '==', '42');
const snap = await query.get();

bundle.add('query', snap);
Expand Down Expand Up @@ -2805,7 +2806,6 @@ describe('Bundle building', () => {
name: snap.toDocumentProto().name,
readTime: snap.readTime.toProto().timestampValue,
exists: false,
queries: [],
});
});

Expand Down Expand Up @@ -2880,6 +2880,14 @@ describe('Bundle building', () => {
});

const bundledDoc = (elements[4] as IBundleElement).document;
expect(bundledDoc).to.deep.equal(limitToLastSnap.docs[0].toDocumentProto());
// The `valueType` is auxiliary and does not exist in proto.
const expected = limitToLastSnap.docs[0].toDocumentProto();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete (expected.fields!.name as any).valueType;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete (expected.fields!.sort as any).valueType;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete (expected.fields!.value as any).valueType;
expect(bundledDoc).to.deep.equal(expected);
});
});
60 changes: 49 additions & 11 deletions dev/test/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
DATABASE_ROOT,
verifyInstance,
} from './util/helpers';

import IBundleElement = firestore.IBundleElement;
import IBundleMetadata = firestore.IBundleMetadata;
import ITimestamp = google.protobuf.ITimestamp;
Expand All @@ -37,17 +38,17 @@ export function verifyMetadata(
expectEmptyContent = false
): void {
if (!expectEmptyContent) {
expect(meta.totalBytes).greaterThan(0);
expect(parseInt(meta.totalBytes!.toString())).greaterThan(0);
} else {
expect(meta.totalBytes).to.equal(0);
expect(parseInt(meta.totalBytes!.toString())).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', () => {
describe('Bundle Builder', () => {
let firestore: Firestore;

beforeEach(() => {
Expand Down Expand Up @@ -75,7 +76,7 @@ describe('Bundle Buidler', () => {
const snap1 = firestore.snapshot_(
{
name: `${DATABASE_ROOT}/documents/collectionId/doc1`,
fields: {foo: {stringValue: 'value'}, bar: {integerValue: 42}},
fields: {foo: {stringValue: 'value'}, bar: {integerValue: '42'}},
createTime: '1970-01-01T00:00:01.002Z',
updateTime: '1970-01-01T00:00:03.000004Z',
},
Expand All @@ -87,7 +88,7 @@ describe('Bundle Buidler', () => {
const snap2 = firestore.snapshot_(
{
name: `${DATABASE_ROOT}/documents/collectionId/doc1`,
fields: {foo: {stringValue: 'value'}, bar: {integerValue: -42}},
fields: {foo: {stringValue: 'value'}, bar: {integerValue: '-42'}},
createTime: '1970-01-01T00:00:01.002Z',
updateTime: '1970-01-01T00:00:03.000004Z',
},
Expand Down Expand Up @@ -116,7 +117,6 @@ describe('Bundle Buidler', () => {
name: snap1.toDocumentProto().name,
readTime: snap1.readTime.toProto().timestampValue,
exists: true,
queries: [],
});
expect(docSnap).to.deep.equal(snap1.toDocumentProto());
});
Expand All @@ -126,7 +126,7 @@ describe('Bundle Buidler', () => {
const snap = firestore.snapshot_(
{
name: `${DATABASE_ROOT}/documents/collectionId/doc1`,
value: 'string',
fields: {foo: {stringValue: 'value'}},
createTime: '1970-01-01T00:00:01.002Z',
updateTime: '1970-01-01T00:00:03.000004Z',
},
Expand Down Expand Up @@ -217,7 +217,7 @@ describe('Bundle Buidler', () => {
const snap1 = firestore.snapshot_(
{
name: `${DATABASE_ROOT}/documents/collectionId/doc1`,
fields: {foo: {stringValue: 'value'}, bar: {integerValue: 42}},
fields: {foo: {stringValue: 'value'}, bar: {integerValue: '42'}},
createTime: '1970-01-01T00:00:01.002Z',
updateTime: '1970-01-01T00:00:03.000004Z',
},
Expand Down Expand Up @@ -246,15 +246,14 @@ describe('Bundle Buidler', () => {
name: snap1.toDocumentProto().name,
readTime: snap1.readTime.toProto().timestampValue,
exists: true,
queries: [],
});
expect(doc1Snap).to.deep.equal(snap1.toDocumentProto());

// Add another document
const snap2 = firestore.snapshot_(
{
name: `${DATABASE_ROOT}/documents/collectionId/doc2`,
fields: {foo: {stringValue: 'value'}, bar: {integerValue: -42}},
fields: {foo: {stringValue: 'value'}, bar: {integerValue: '-42'}},
createTime: '1970-01-01T00:00:01.002Z',
updateTime: '1970-01-01T00:00:03.000004Z',
},
Expand Down Expand Up @@ -283,7 +282,6 @@ describe('Bundle Buidler', () => {
name: snap2.toDocumentProto().name,
readTime: snap2.readTime.toProto().timestampValue,
exists: true,
queries: [],
});
expect(doc2Snap).to.deep.equal(snap2.toDocumentProto());
});
Expand All @@ -304,3 +302,43 @@ describe('Bundle Buidler', () => {
);
});
});

describe('Bundle Builder using BigInt', () => {
let firestore: Firestore;

beforeEach(() => {
return createInstance(undefined, {useBigInt: true}).then(
firestoreInstance => {
firestore = firestoreInstance;
}
);
});

it('succeeds with document snapshots with BigInt field', async () => {
const bundle = firestore.bundle(TEST_BUNDLE_ID);
const bigIntValue =
BigInt(Number.MAX_SAFE_INTEGER) + BigInt(Number.MAX_SAFE_INTEGER);
const snap = firestore.snapshot_(
{
name: `${DATABASE_ROOT}/documents/collectionId/doc1`,
fields: {foo: {integerValue: bigIntValue.toString()}},
createTime: '1970-01-01T00:00:01.002Z',
updateTime: '1970-01-01T00:00:03.000004Z',
},
// This should be the bundle read time.
'2020-01-01T00:00:05.000000006Z',
'json'
);
bundle.add(snap);

// Bundle is expected to be [bundleMeta, snapMeta, snap]
const elements = await bundleToElementArray(bundle.build());
// The point is to make sure BigInt gets encoded correctly into a string without losing
// precision.
expect(elements[2].document?.fields).to.deep.equal({
foo: {integerValue: bigIntValue.toString()},
});
});

afterEach(() => verifyInstance(firestore));
});
4 changes: 2 additions & 2 deletions dev/test/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ describe('delete document', () => {
remove('documentId', {
updateTime: {
nanos: 123000000,
seconds: 479978400,
seconds: '479978400',
},
})
);
Expand Down Expand Up @@ -1682,7 +1682,7 @@ describe('update document', () => {
precondition: {
updateTime: {
nanos: 123000000,
seconds: 479978400,
seconds: '479978400',
},
},
})
Expand Down
2 changes: 1 addition & 1 deletion dev/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ const allSupportedTypesProtobufJs = document(
{
timestampValue: {
nanos: 123000000,
seconds: 479978400,
seconds: '479978400',
},
},
'doubleValue',
Expand Down
2 changes: 1 addition & 1 deletion dev/test/util/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {grpc} from 'google-gax';
import {JSONStreamIterator} from 'length-prefixed-json-stream';
import {Duplex, PassThrough} from 'stream';
import * as through2 from 'through2';
import {firestore} from '../../protos/firestore_v1_proto_api';
import {firestore, google} from '../../protos/firestore_v1_proto_api';

import * as proto from '../../protos/firestore_v1_proto_api';
import * as v1 from '../../src/v1';
Expand Down

0 comments on commit 8cf53a9

Please sign in to comment.