Revert "Revert "Always construct DocumentSnapshot for event.data.previous for Firestore functions""

spec/providers/firestore.spec.ts

@@ -145,7 +145,8 @@ describe('Firestore Functions', () => {
       let testFunction = firestore.document('path').onCreate((event) => {
         expect(event.data.data()).to.deep.equal({key1: true, key2: 123});
         expect(event.data.get('key1')).to.equal(true);
-        expect(event.data.previous).to.equal(null);
+        expect(event.data.previous).to.not.equal(null);
+        expect(event.data.previous.exists).to.be.false;
       });
       let data = constructEvent({}, createValue());
       return testFunction(data);
@@ -164,8 +165,7 @@ describe('Firestore Functions', () => {
 
     it('constructs appropriate fields and getters for event.data on "document.delete" events', () => {
       let testFunction = firestore.document('path').onDelete((event) => {
-        expect(event.data.data).to.throw(Error);
-        expect(() => {return event.data.get('key1');}).to.throw(Error);
+        expect(event.data.exists).to.equal(false);
         expect(event.data.previous.data()).to.deep.equal({key1: false, key2: 111});
         expect(event.data.previous.get('key1')).to.equal(false);
       });
@@ -330,7 +330,7 @@ describe('Firestore Functions', () => {
         let snapshot = firestore.dataConstructor({
           data: { value: raw },
         });
-        expect(snapshot.data()).to.deep.equal({'binaryVal': 'Zm9vYmFy'});
+        expect(snapshot.data()).to.deep.equal({'binaryVal': new Buffer('foobar')});
       });
     });
 
@@ -387,8 +387,6 @@ describe('Firestore Functions', () => {
         });
         expect(snapshot.exists).to.be.false;
         expect(snapshot.ref.path).to.equal('collection/123');
-        expect(snapshot.data).to.throw(Error);
-        expect(() => {return snapshot.get('key1');}).to.throw(Error);
       });
 
       it('constructs existent DocumentSnapshot with empty data when all fields of document deleted', () => {

src/providers/firestore.ts

@@ -84,73 +84,21 @@ function isDeltaDocumentSnapshot(data: any): data is DeltaDocumentSnapshot {
   return 'exists' in data;
 };
 
-function getValueProto(event) {
+function getValueProto(event, valueFieldName) {
   let data = event.data;
-  if (_.isEmpty(_.get(data, 'value'))) {
+  if (_.isEmpty(_.get(data, valueFieldName))) {
     // Firestore#snapshot_ takes resource string instead of proto for a non-existent snapshot
     return event.resource;
   }
   let proto = {
-    fields: convertToFieldsProto(_.get(data, 'value.fields', {})),
-    createTime: dateToTimestampProto(_.get(data, 'value.createTime')),
-    updateTime: dateToTimestampProto(_.get(data, 'value.updateTime')),
-    name: _.get(data, 'value.name', event.resource),
+    fields: _.get(data, [valueFieldName, 'fields'], {}),
+    createTime: dateToTimestampProto(_.get(data, [valueFieldName, 'createTime'])),
+    updateTime: dateToTimestampProto(_.get(data, [valueFieldName, 'updateTime'])),
+    name: _.get(data, [valueFieldName, 'name'], event.resource),
   };
   return proto;
 };
 
-function getOldValueProto(event) {
-  let data = event.data;
-  let proto = {
-    fields: convertToFieldsProto(_.get(data, 'oldValue.fields', {})),
-    createTime: dateToTimestampProto(_.get(data, 'oldValue.createTime')),
-    updateTime: dateToTimestampProto(_.get(data, 'oldValue.updateTime')),
-    name: _.get(data, 'oldValue.name', event.resource),
-  };
-  return proto;
-};
-
-function convertToFieldsProto(fields): object {
-  if (!fields) {
-    return {};
-  }
-  function convertHelper(data) {
-    let result;
-    _.forEach(data, (value: any, valueType: string) => {
-      let dataPart;
-      if (valueType === 'arrayValue') {
-        let array = _.get(value, 'values', []);
-        dataPart = {
-          arrayValue: {
-            values: _.map(array, (elem) => {
-              return convertHelper(elem);
-            }),
-          },
-        };
-      } else if (valueType === 'mapValue') {
-        let map = _.get(value, 'fields', {});
-        dataPart = {
-          mapValue: {
-            fields: _.mapValues(map, (val) => {
-              return convertHelper(val);
-            }),
-          },
-        };
-      } else if (valueType === 'timestampValue') {
-        dataPart = {timestampValue: dateToTimestampProto(value)};
-      } else {
-        dataPart = data;
-      }
-      result = _.merge({}, dataPart, {valueType: valueType});
-    });
-    return result;
-  }
-
-  return _.mapValues(fields, (data: object) => {
-    return convertHelper(data);
-  });
-};
-
 /** @internal */
 export function dataConstructor(raw: Event<any>) {
   if (isDeltaDocumentSnapshot(raw.data)) {
@@ -159,17 +107,14 @@ export function dataConstructor(raw: Event<any>) {
   if (!firestoreInstance) {
     firestoreInstance = firebase.firestore(apps().admin);
   }
-  let valueProto = getValueProto(raw);
+  let valueProto = getValueProto(raw, 'value');
   let readTime = dateToTimestampProto(_.get(raw.data, 'value.readTime'));
-  let snapshot = firestoreInstance.snapshot_(valueProto, readTime) as DeltaDocumentSnapshot;
+  let snapshot = firestoreInstance.snapshot_(valueProto, readTime, 'json') as DeltaDocumentSnapshot;
   Object.defineProperty(snapshot, 'previous', {
     get: () => {
-      if (_.isEmpty(_.get(raw, 'data.oldValue', {}))) {
-        return null;
-      }
-      let oldValueProto = getOldValueProto(raw);
-      let oldReadTime = dateToTimestampProto(_.get(raw.data, 'value.oldValue.readTime'));
-      return firestoreInstance.snapshot_(oldValueProto, oldReadTime) as DeltaDocumentSnapshot;
+      let oldValueProto = getValueProto(raw, 'oldValue');
+      let oldReadTime = dateToTimestampProto(_.get(raw.data, 'oldValue.readTime'));
+      return firestoreInstance.snapshot_(oldValueProto, oldReadTime, 'json') as DeltaDocumentSnapshot;
     },
   });
   return snapshot;