Ignore Firestore types in object traversal (#29)

src/document.js

@@ -646,7 +646,7 @@ class DocumentSnapshot {
       };
     }
 
-    if (is.object(val)) {
+    if (isPlainObject(val)) {
       return {
         valueType: 'mapValue',
         mapValue: {
@@ -690,7 +690,7 @@ class DocumentSnapshot {
           target[key] = {};
           merge(target[key], value, path, pos + 1);
         }
-      } else if (is.object(target[key])) {
+      } else if (isPlainObject(target[key])) {
         if (isLast) {
           // The existing object has deeper nesting that the value we are trying
           // to merge.
@@ -877,7 +877,7 @@ class DocumentMask {
             ? currentPath.append(childSegment)
             : childSegment;
           const value = currentData[key];
-          if (is.object(value)) {
+          if (isPlainObject(value)) {
             extractFieldPaths(value, childPath);
           } else if (value !== FieldValue.SERVER_TIMESTAMP_SENTINEL) {
             fieldPaths.push(childPath.formattedName);
@@ -972,7 +972,7 @@ class DocumentTransform {
           // We need to verify that no array value contains a document transform
           encode_(val[i], path.concat(i), false);
         }
-      } else if (is.object(val)) {
+      } else if (isPlainObject(val)) {
         for (let prop in val) {
           if (val.hasOwnProperty(prop)) {
             transforms = transforms.concat(
@@ -1088,13 +1088,13 @@ function validateDocumentData(obj, usesPaths, depth) {
     );
   }
 
-  if (!is.object(obj)) {
-    throw new Error('Input is not a JavaScript object.');
+  if (!isPlainObject(obj)) {
+    throw new Error('Input is not a plain JavaScript object.');
   }
 
   for (let prop in obj) {
     if (obj.hasOwnProperty(prop)) {
-      if (is.object(obj[prop])) {
+      if (isPlainObject(obj[prop])) {
         validateDocumentData(obj[prop], false, depth + 1);
       }
     }
@@ -1107,7 +1107,6 @@ function validateDocumentData(obj, usesPaths, depth) {
  * Validates the use of 'options' as a Precondition and enforces that 'exists'
  * and 'lastUpdateTime' use valid types.
  *
- *
  * @param {boolean=} options.exists - Whether the referenced document
  * should exist.
  * @param {string=} options.lastUpdateTime - The last update time
@@ -1146,7 +1145,6 @@ function validatePrecondition(options) {
  * Validates the use of 'options' as SetOptions and enforces that 'merge' is a
  * boolean.
  *
- *
  * @param {boolean=} options.merge - Whether set() should merge the provided
  * data into an existing document.
  * @returns {boolean} 'true' if the input is a valid SetOptions object.
@@ -1163,9 +1161,24 @@ function validateSetOptions(options) {
   return true;
 }
 
-module.exports = (FirestoreType, DocumentRefType) => {
+/*!
+ * Verifies that 'obj' is a plain JavaScript object that can be encoded as a
+ * 'Map' in Firestore.
+ *
+ * @param {*} input - The argument to verify.
+ * @returns {boolean} 'true' if the input can be a treated as a plain object.
+ */
+function isPlainObject(input) {
+  return (
+    typeof input === 'object' &&
+    input !== null &&
+    Object.getPrototypeOf(input) === Object.prototype
+  );
+}
+
+module.exports = DocumentRefType => {
   DocumentReference = DocumentRefType;
-  validate = require('./validate.js')({
+  validate = require('./validate')({
     FieldPath: FieldPath.validateFieldPath,
   });
   return {

src/index.js

@@ -1071,10 +1071,10 @@ Firestore.setLogFunction = function(logger) {
 let reference = require('./reference')(Firestore);
 CollectionReference = reference.CollectionReference;
 DocumentReference = reference.DocumentReference;
-let document = require('./document')(Firestore, DocumentReference);
+let document = require('./document')(DocumentReference);
 DocumentSnapshot = document.DocumentSnapshot;
 GeoPoint = document.GeoPoint;
-validate = require('./validate.js')({
+validate = require('./validate')({
   DocumentReference: reference.validateDocumentReference,
   ResourcePath: ResourcePath.validateResourcePath,
 });

src/reference.js

@@ -1889,15 +1889,15 @@ function validateDocumentReference(value) {
 
 module.exports = FirestoreType => {
   Firestore = FirestoreType;
-  let document = require('./document.js')(DocumentReference);
+  let document = require('./document')(DocumentReference);
   DocumentSnapshot = document.DocumentSnapshot;
-  Watch = require('./watch.js')(
+  Watch = require('./watch')(
     FirestoreType,
     DocumentChange,
     DocumentReference,
     DocumentSnapshot
   );
-  WriteBatch = require('./write-batch.js')(
+  WriteBatch = require('./write-batch')(
     FirestoreType,
     DocumentReference,
     validateDocumentReference

src/transaction.js

@@ -293,8 +293,8 @@ module.exports = FirestoreType => {
   let reference = require('./reference')(FirestoreType);
   DocumentReference = reference.DocumentReference;
   Query = reference.Query;
-  let document = require('./document.js')(FirestoreType, DocumentReference);
-  require('./validate.js')({
+  let document = require('./document')(DocumentReference);
+  require('./validate')({
     Document: document.validateDocumentData,
     DocumentReference: reference.validateDocumentReference,
     Precondition: document.validatePrecondition,

src/write-batch.js

@@ -509,7 +509,7 @@ module.exports = (
   DocumentReferenceType,
   validateDocumentReference
 ) => {
-  let document = require('./document.js')(Firestore, DocumentReferenceType);
+  let document = require('./document')(DocumentReferenceType);
   Firestore = FirestoreType;
   DocumentMask = document.DocumentMask;
   DocumentSnapshot = document.DocumentSnapshot;

test/document.js

@@ -388,22 +388,6 @@ describe('serialize document', function() {
     }, /Cannot encode type/);
   });
 
-  it("doesn't serialize inherited properties", function() {
-    firestore.api.Firestore._commit = function(request, options, callback) {
-      requestEquals(
-        request,
-        update(document('member', 'bar'), updateMask('member'))
-      );
-      callback(null, defaultWriteResult);
-    };
-
-    let base = {inherited: 'foo'};
-    let instance = Object.create(base);
-    instance.member = 'bar';
-
-    return firestore.doc('collectionId/documentId').update(instance);
-  });
-
   it('serializes date before 1970', function() {
     firestore.api.Firestore._commit = function(request, options, callback) {
       requestEquals(
@@ -565,7 +549,33 @@ describe('serialize document', function() {
 
     assert.throws(() => {
       firestore.doc('collectionId/documentId').update(obj);
-    }, new RegExp('Argument "dataOrField" is not a valid Document. Input ' + 'object is deeper than 20 levels or contains a cycle.'));
+    }, new RegExp('Argument "dataOrField" is not a valid Document. Input object is deeper than 20 levels or contains a cycle.'));
+  });
+
+  it('is able to write a document reference with cycles', function() {
+    firestore.api.Firestore._commit = function(request, options, callback) {
+      requestEquals(
+        request,
+        set(
+          document('ref', {
+            referenceValue:
+              'projects/test-project/databases/(default)/documents/collectionId/documentId',
+            valueType: 'referenceValue',
+          })
+        )
+      );
+
+      callback(null, defaultWriteResult);
+    };
+
+    // The Firestore Admin SDK adds a cyclic reference to the 'Firestore' member
+    // of 'DocumentReference'. We emulate this behavior in this test to verify
+    // that we can properly serialize DocumentReference instances, even if they
+    // have cyclic references (we shouldn't try to validate them beyond the
+    // instanceof check).
+    let ref = firestore.doc('collectionId/documentId');
+    ref.firestore.firestore = firestore;
+    return ref.set({ref});
   });
 });
 
@@ -1035,13 +1045,13 @@ describe('set document', function() {
   it('requires an object', function() {
     assert.throws(() => {
       firestore.doc('collectionId/documentId').set(null);
-    }, new RegExp('Argument "data" is not a valid Document. Input is not a ' + 'JavaScript object.'));
+    }, new RegExp('Argument "data" is not a valid Document. Input is not a plain JavaScript object.'));
   });
 
   it("doesn't accept arrays", function() {
     assert.throws(() => {
       firestore.doc('collectionId/documentId').set([42]);
-    }, new RegExp('Argument "data" is not a valid Document. Input is not a ' + 'JavaScript object.'));
+    }, new RegExp('Argument "data" is not a valid Document. Input is not a plain JavaScript object.'));
   });
 });
 
@@ -1116,13 +1126,13 @@ describe('create document', function() {
   it('requires an object', function() {
     assert.throws(() => {
       firestore.doc('collectionId/documentId').create(null);
-    }, new RegExp('Argument "data" is not a valid Document. Input is not a ' + 'JavaScript object.'));
+    }, new RegExp('Argument "data" is not a valid Document. Input is not a plain JavaScript object.'));
   });
 
   it("doesn't accept arrays", function() {
     assert.throws(() => {
       firestore.doc('collectionId/documentId').create([42]);
-    }, new RegExp('Argument "data" is not a valid Document. Input is not a ' + 'JavaScript object.'));
+    }, new RegExp('Argument "data" is not a valid Document. Input is not a plain JavaScript object.'));
   });
 });
 
@@ -1475,13 +1485,13 @@ describe('update document', function() {
   it('accepts an object', function() {
     assert.throws(() => {
       firestore.doc('collectionId/documentId').update(null);
-    }, new RegExp('Argument "dataOrField" is not a valid Document. Input is ' + 'not a JavaScript object.'));
+    }, new RegExp('Argument "dataOrField" is not a valid Document. Input is not a plain JavaScript object.'));
   });
 
   it("doesn't accept arrays", function() {
     assert.throws(() => {
       firestore.doc('collectionId/documentId').update([42]);
-    }, new RegExp('Argument "dataOrField" is not a valid Document. Input is ' + 'not a JavaScript object.'));
+    }, new RegExp('Argument "dataOrField" is not a valid Document. Input is not a plain JavaScript object.'));
   });
 
   it('with field delete', function() {

test/order.js

@@ -23,7 +23,7 @@ const grpc = require('grpc');
 const Firestore = require('../');
 const DocumentReference = require('../src/reference')(Firestore)
   .DocumentReference;
-const document = require('../src/document')(Firestore, DocumentReference);
+const document = require('../src/document')(DocumentReference);
 const DocumentSnapshot = document.DocumentSnapshot;
 const GeoPoint = document.GeoPoint;
 const order = require('../src/order');

test/query.js

@@ -25,10 +25,8 @@ const through = require('through2');
 const Firestore = require('../');
 const reference = require('../src/reference')(Firestore);
 const DocumentReference = reference.DocumentReference;
-const DocumentSnapshot = require('../src/document')(
-  Firestore,
-  DocumentReference
-).DocumentSnapshot;
+const DocumentSnapshot = require('../src/document')(DocumentReference)
+  .DocumentSnapshot;
 const ResourcePath = require('../src/path').ResourcePath;
 
 const DATABASE_ROOT = 'projects/test-project/databases/(default)';

test/watch.js

@@ -25,10 +25,8 @@ const through = require('through2');
 const Firestore = require('../');
 const reference = require('../src/reference')(Firestore);
 const DocumentReference = reference.DocumentReference;
-const DocumentSnapshot = require('../src/document')(
-  Firestore,
-  DocumentReference
-).DocumentSnapshot;
+const DocumentSnapshot = require('../src/document')(DocumentReference)
+  .DocumentSnapshot;
 
 // Change the argument to 'console.log' to enable debug output.
 Firestore.setLogFunction(() => {});

test/write-batch.js

@@ -50,7 +50,7 @@ describe('set() method', function() {
     assert.throws(function() {
       writeBatch.set(firestore.doc('sub/doc'));
     }, new RegExp(
-      'Argument "data" is not a valid Document. Input is not a ' +
+      'Argument "data" is not a valid Document. Input is not a plain ' +
         'JavaScript object.'
     ));
   });
@@ -100,7 +100,7 @@ describe('update() method', function() {
   it('requires object', function() {
     assert.throws(() => {
       writeBatch.update(firestore.doc('sub/doc'));
-    }, new RegExp('Argument "dataOrField" is not a valid Document. ' + 'Input is not a JavaScript object.'));
+    }, new RegExp('Argument "dataOrField" is not a valid Document. Input is not a plain JavaScript object.'));
   });
 
   it('accepts preconditions', function() {
@@ -131,7 +131,7 @@ describe('create() method', function() {
     assert.throws(function() {
       writeBatch.create(firestore.doc('sub/doc'));
     }, new RegExp(
-      'Argument "data" is not a valid Document. Input is not a ' +
+      'Argument "data" is not a valid Document. Input is not a plain ' +
         'JavaScript object.'
     ));
   });