diff --git a/.changeset/thirty-suits-switch.md b/.changeset/thirty-suits-switch.md new file mode 100644 index 00000000000..d19e0d17660 --- /dev/null +++ b/.changeset/thirty-suits-switch.md @@ -0,0 +1,6 @@ +--- +"firebase": patch +"@firebase/firestore": patch +--- + +Fixed an issue that prevented Timestamps from being used via `update()` when connected to the Emulator diff --git a/packages/firestore/src/exp/database.ts b/packages/firestore/src/exp/database.ts index 76df6a72e9f..97d73ac2fe3 100644 --- a/packages/firestore/src/exp/database.ts +++ b/packages/firestore/src/exp/database.ts @@ -15,12 +15,12 @@ * limitations under the License. */ +// eslint-disable-next-line import/no-extraneous-dependencies import { _getProvider, _removeServiceInstance, FirebaseApp, getApp - // eslint-disable-next-line import/no-extraneous-dependencies } from '@firebase/app-exp'; import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; import { Provider } from '@firebase/component'; diff --git a/packages/firestore/src/lite/database.ts b/packages/firestore/src/lite/database.ts index 43730105de5..1c9d1beb64d 100644 --- a/packages/firestore/src/lite/database.ts +++ b/packages/firestore/src/lite/database.ts @@ -15,12 +15,12 @@ * limitations under the License. */ +// eslint-disable-next-line import/no-extraneous-dependencies import { _getProvider, _removeServiceInstance, FirebaseApp, getApp - // eslint-disable-next-line import/no-extraneous-dependencies } from '@firebase/app-exp'; import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; import { Provider } from '@firebase/component'; diff --git a/packages/firestore/src/model/values.ts b/packages/firestore/src/model/values.ts index 720d65137d4..854655010fc 100644 --- a/packages/firestore/src/model/values.ts +++ b/packages/firestore/src/model/values.ts @@ -559,8 +559,11 @@ export function isMapValue( export function deepClone(source: Value): Value { if (source.geoPointValue) { return { geoPointValue: { ...source.geoPointValue } }; - } else if (source.timestampValue) { - return { timestampValue: { ...normalizeTimestamp(source.timestampValue) } }; + } else if ( + source.timestampValue && + typeof source.timestampValue === 'object' + ) { + return { timestampValue: { ...source.timestampValue } }; } else if (source.mapValue) { const target: Value = { mapValue: { fields: {} } }; forEach( diff --git a/packages/firestore/test/integration/api/type.test.ts b/packages/firestore/test/integration/api/type.test.ts index 9c48dd40c68..fe40cee33a4 100644 --- a/packages/firestore/test/integration/api/type.test.ts +++ b/packages/firestore/test/integration/api/type.test.ts @@ -33,49 +33,59 @@ apiDescribe('Firestore', (persistence: boolean) => { async function expectRoundtrip( db: firestore.FirebaseFirestore, data: {}, - validateSnapshots = true - ): Promise { + validateSnapshots = true, + expectedData?: {} + ): Promise { + expectedData = expectedData ?? data; + const collection = db.collection(db.collection('a').doc().id); const doc = collection.doc(); - await doc.set(data); + await doc.set(data); let docSnapshot = await doc.get(); - expect(docSnapshot.data()).to.deep.equal(data); + expect(docSnapshot.data()).to.deep.equal(expectedData); + + await doc.update(data); + docSnapshot = await doc.get(); + expect(docSnapshot.data()).to.deep.equal(expectedData); // Validate that the transaction API returns the same types await db.runTransaction(async transaction => { docSnapshot = await transaction.get(doc); - expect(docSnapshot.data()).to.deep.equal(data); + expect(docSnapshot.data()).to.deep.equal(expectedData); }); if (validateSnapshots) { let querySnapshot = await collection.get(); docSnapshot = querySnapshot.docs[0]; - expect(docSnapshot.data()).to.deep.equal(data); + expect(docSnapshot.data()).to.deep.equal(expectedData); - const eventsAccumulator = new EventsAccumulator(); + const eventsAccumulator = + new EventsAccumulator(); const unlisten = collection.onSnapshot(eventsAccumulator.storeEvent); querySnapshot = await eventsAccumulator.awaitEvent(); docSnapshot = querySnapshot.docs[0]; - expect(docSnapshot.data()).to.deep.equal(data); + expect(docSnapshot.data()).to.deep.equal(expectedData); unlisten(); } + + return docSnapshot; } it('can read and write null fields', () => { - return withTestDb(persistence, db => { - return expectRoundtrip(db, { a: 1, b: null }); + return withTestDb(persistence, async db => { + await expectRoundtrip(db, { a: 1, b: null }); }); }); it('can read and write number fields', () => { - return withTestDb(persistence, db => { + return withTestDb(persistence, async db => { // TODO(b/174486484): If we build ViewSnapshots from IndexedDb, this test // fails since we first store the backend proto in IndexedDb, which turns // -0.0 into 0.0. const validateSnapshots = !persistence; - return expectRoundtrip( + await expectRoundtrip( db, { a: 1, b: NaN, c: Infinity, d: -0.0 }, validateSnapshots @@ -84,91 +94,78 @@ apiDescribe('Firestore', (persistence: boolean) => { }); it('can read and write array fields', () => { - return withTestDb(persistence, db => { - return expectRoundtrip(db, { array: [1, 'foo', { deep: true }, null] }); + return withTestDb(persistence, async db => { + await expectRoundtrip(db, { array: [1, 'foo', { deep: true }, null] }); }); }); it('can read and write geo point fields', () => { - return withTestDoc(persistence, doc => { - return doc - .set({ - geopoint1: new GeoPoint(1.23, 4.56), - geopoint2: new GeoPoint(0, 0) - }) - .then(() => { - return doc.get(); - }) - .then(docSnapshot => { - const latLong = docSnapshot.data()!['geopoint1']; - expect(latLong instanceof GeoPoint).to.equal(true); - expect(latLong.latitude).to.equal(1.23); - expect(latLong.longitude).to.equal(4.56); - - const zeroLatLong = docSnapshot.data()!['geopoint2']; - expect(zeroLatLong instanceof GeoPoint).to.equal(true); - expect(zeroLatLong.latitude).to.equal(0); - expect(zeroLatLong.longitude).to.equal(0); - }); + return withTestDb(persistence, async db => { + const docSnapshot = await expectRoundtrip(db, { + geopoint1: new GeoPoint(1.23, 4.56), + geopoint2: new GeoPoint(0, 0) + }); + + const latLong = docSnapshot.data()!['geopoint1']; + expect(latLong instanceof GeoPoint).to.equal(true); + expect(latLong.latitude).to.equal(1.23); + expect(latLong.longitude).to.equal(4.56); + + const zeroLatLong = docSnapshot.data()!['geopoint2']; + expect(zeroLatLong instanceof GeoPoint).to.equal(true); + expect(zeroLatLong.latitude).to.equal(0); + expect(zeroLatLong.longitude).to.equal(0); }); }); it('can read and write bytes fields', () => { - return withTestDoc(persistence, doc => { - return doc - .set({ - bytes: Blob.fromUint8Array(new Uint8Array([0, 1, 255])) - }) - .then(() => { - return doc.get(); - }) - .then(docSnapshot => { - const blob = docSnapshot.data()!['bytes']; - // TODO(firestorexp): As part of the Compat migration, the SDK - // should re-wrap the firestore-exp types into the Compat API. - // Comment this change back in once this is complete (note that this - // check passes in the legacy API). - // expect(blob instanceof Blob).to.equal(true); - expect(blob.toUint8Array()).to.deep.equal( - new Uint8Array([0, 1, 255]) - ); - }); + return withTestDb(persistence, async db => { + const docSnapshot = await expectRoundtrip(db, { + bytes: Blob.fromUint8Array(new Uint8Array([0, 1, 255])) + }); + + const blob = docSnapshot.data()!['bytes']; + // TODO(firestorexp): As part of the Compat migration, the SDK + // should re-wrap the firestore-exp types into the Compat API. + // Comment this change back in once this is complete (note that this + // check passes in the legacy API). + // expect(blob instanceof Blob).to.equal(true); + expect(blob.toUint8Array()).to.deep.equal(new Uint8Array([0, 1, 255])); }); }); it('can read and write date fields', () => { - return withTestDb(persistence, db => { - const dateValue = new Date('2017-04-10T09:10:11.123Z'); - // Dates are returned as Timestamps, so expectRoundtrip can't be used - // here. - const doc = db.collection('rooms').doc(); - return doc - .set({ date: dateValue }) - .then(() => doc.get()) - .then(snapshot => { - expect(snapshot.data()).to.deep.equal({ - date: Timestamp.fromDate(dateValue) - }); - }); + return withTestDb(persistence, async db => { + const date = new Date('2017-04-10T09:10:11.123Z'); + // Dates are returned as Timestamps + const data = { date }; + const expectedData = { date: Timestamp.fromDate(date) }; + + await expectRoundtrip( + db, + data, + /* validateSnapshot= */ true, + expectedData + ); }); }); it('can read and write timestamp fields', () => { - return withTestDb(persistence, db => { + return withTestDb(persistence, async db => { const timestampValue = Timestamp.now(); - return expectRoundtrip(db, { timestamp: timestampValue }); + await expectRoundtrip(db, { timestamp: timestampValue }); }); }); it('can read and write document references', () => { - return withTestDoc(persistence, doc => { - return expectRoundtrip(doc.firestore, { a: 42, ref: doc }); + return withTestDoc(persistence, async doc => { + await expectRoundtrip(doc.firestore, { a: 42, ref: doc }); }); }); it('can read and write document references in an array', () => { - return withTestDoc(persistence, doc => { - return expectRoundtrip(doc.firestore, { a: 42, refs: [doc] }); + return withTestDoc(persistence, async doc => { + await expectRoundtrip(doc.firestore, { a: 42, refs: [doc] }); }); }); }); diff --git a/packages/firestore/test/unit/model/values.test.ts b/packages/firestore/test/unit/model/values.test.ts index c781c6e9507..894b538cbdf 100644 --- a/packages/firestore/test/unit/model/values.test.ts +++ b/packages/firestore/test/unit/model/values.test.ts @@ -25,7 +25,8 @@ import { valueCompare, valueEquals, estimateByteSize, - refValue + refValue, + deepClone } from '../../../src/model/values'; import * as api from '../../../src/protos/firestore_proto_api'; import { primitiveComparator } from '../../../src/util/misc'; @@ -405,4 +406,41 @@ describe('Values', () => { ) ).to.equal('{a:1,b:2,c:3}'); }); + + it('clones properties without normalization', () => { + const values = [ + { integerValue: '1' }, + { integerValue: 1 }, + { doubleValue: '2' }, + { doubleValue: 2 }, + { timestampValue: '2007-04-05T14:30:01Z' }, + { timestampValue: { seconds: 1175783401 } }, + { timestampValue: '2007-04-05T14:30:01.999Z' }, + { + timestampValue: { seconds: 1175783401, nanos: 999000000 } + }, + { timestampValue: '2007-04-05T14:30:02Z' }, + { timestampValue: { seconds: 1175783402 } }, + { timestampValue: '2007-04-05T14:30:02.100Z' }, + { + timestampValue: { seconds: 1175783402, nanos: 100000000 } + }, + { timestampValue: '2007-04-05T14:30:02.100001Z' }, + { + timestampValue: { seconds: 1175783402, nanos: 100001000 } + }, + { bytesValue: new Uint8Array([0, 1, 2]) }, + { bytesValue: 'AAEC' }, + { bytesValue: new Uint8Array([0, 1, 3]) }, + { bytesValue: 'AAED' } + ]; + + for (const value of values) { + expect(deepClone(value)).to.deep.equal(value); + const mapValue = { mapValue: { fields: { foo: value } } }; + expect(deepClone(mapValue)).to.deep.equal(mapValue); + const arrayValue = { arrayValue: { values: [value] } }; + expect(deepClone(arrayValue)).to.deep.equal(arrayValue); + } + }); }); diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index 93c2ccd4f38..626ac35ac1e 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -47,7 +47,7 @@ export class TestRestConnection extends RestConnection { body: Req ): Promise { this.lastUrl = url; - this.lastRequestBody = (body as unknown) as Indexable; + this.lastRequestBody = body as unknown as Indexable; this.lastHeaders = headers; const response = this.nextResponse; this.nextResponse = Promise.resolve({}); @@ -63,7 +63,8 @@ describe('RestConnection', () => { 'example.com', /*ssl=*/ false, /*forceLongPolling=*/ false, - /*autoDetectLongPolling=*/ false + /*autoDetectLongPolling=*/ false, + /*useFetchStreams=*/ false ); const connection = new TestRestConnection(testDatabaseInfo); diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 70ef67cbb43..785ca80de30 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -256,7 +256,8 @@ abstract class TestRunner { 'host', /*ssl=*/ false, /*forceLongPolling=*/ false, - /*autoDetectLongPolling=*/ false + /*autoDetectLongPolling=*/ false, + /*useFetchStreams=*/ false ); // TODO(mrschmidt): During client startup in `firestore_client`, we block @@ -297,11 +298,12 @@ abstract class TestRunner { const onlineComponentProvider = new MockOnlineComponentProvider( this.connection ); - const offlineComponentProvider = await this.initializeOfflineComponentProvider( - onlineComponentProvider, - configuration, - this.useGarbageCollection - ); + const offlineComponentProvider = + await this.initializeOfflineComponentProvider( + onlineComponentProvider, + configuration, + this.useGarbageCollection + ); await onlineComponentProvider.initialize( offlineComponentProvider, configuration @@ -912,9 +914,8 @@ abstract class TestRunner { this.expectedActiveLimboDocs = expectedState.activeLimboDocs!.map(key); } if ('enqueuedLimboDocs' in expectedState) { - this.expectedEnqueuedLimboDocs = expectedState.enqueuedLimboDocs!.map( - key - ); + this.expectedEnqueuedLimboDocs = + expectedState.enqueuedLimboDocs!.map(key); } if ('activeTargets' in expectedState) { this.expectedActiveTargets.clear();