Rejecting custom types from the API surface

Author: schmidt-sebastian

src/document.js

@@ -709,19 +709,22 @@ class DocumentSnapshot {
     }
 
     if (is.array(val)) {
-      let encodedElements = [];
-      for (let i = 0; i < val.length; ++i) {
-        let enc = DocumentSnapshot.encodeValue(val[i]);
-        if (enc) {
-          encodedElements.push(enc);
-        }
-      }
-      return {
+      const array = {
         valueType: 'arrayValue',
-        arrayValue: {
-          values: encodedElements,
-        },
+        arrayValue: {},
       };
+
+      if (val.length > 0) {
+        array.arrayValue.values = [];
+        for (let i = 0; i < val.length; ++i) {
+          let enc = DocumentSnapshot.encodeValue(val[i]);
+          if (enc) {
+            array.arrayValue.values.push(enc);
+          }
+        }
+      }
+
+      return array;
     }
 
     if (is.nil(val)) {
@@ -755,9 +758,7 @@ class DocumentSnapshot {
     if (isPlainObject(val)) {
       const map = {
         valueType: 'mapValue',
-        mapValue: {
-          fields: {},
-        },
+        mapValue: {},
       };
 
       // If we encounter an empty object, we always need to send it to make sure
@@ -772,11 +773,7 @@ class DocumentSnapshot {
       return map;
     }
 
-    throw new Error(
-      'Cannot encode type (' +
-        Object.prototype.toString.call(val) +
-        ') to a Firestore Value'
-    );
+    validate.throwCustomObjectError(val);
   }
 }
 
@@ -1184,10 +1181,7 @@ class DocumentTransform {
    * @return {boolean} Whether we encountered a transform sentinel.
    */
   static isTransformSentinel(val) {
-    return (
-      val === FieldValue.SERVER_TIMESTAMP_SENTINEL ||
-      val === FieldValue.DELETE_SENTINEL
-    );
+    return is.instanceof(val, FieldValue);
   }
 
   /**
@@ -1277,16 +1271,20 @@ class Precondition {
  * Validates a JavaScript object for usage as a Firestore document.
  *
  * @param {Object} obj JavaScript object to validate.
- * @param {boolean=} options.allowDeletes Whether field deletes are supported
- * at the top level (e.g. for document updates).
- * @param {boolean=} options.allowNestedDeletes Whether field deletes are supported
- * at any level (e.g. for document merges).
- * @param {boolean=} options.allowEmpty Whether empty documents are support.
- * Defaults to true.
+  *@param {string} options.allowDeletes At what level field deletes are
+ * supported (acceptable values are 'none', 'root' or 'all').
+ * @param {boolean} options.allowServerTimestamps Whether server timestamps
+ * are supported.
+ * @param {boolean} options.allowEmpty Whether empty documents are supported.
  * @returns {boolean} 'true' when the object is valid.
  * @throws {Error} when the object is invalid.
  */
 function validateDocumentData(obj, options) {
+  assert(
+    typeof options.allowEmpty === 'boolean',
+    "Expected boolean for 'options.allowEmpty'"
+  );
+
   if (!isPlainObject(obj)) {
     throw new Error('Input is not a plain JavaScript object.');
   }
@@ -1313,16 +1311,23 @@ function validateDocumentData(obj, options) {
  * Validates a JavaScript value for usage as a Firestore value.
  *
  * @param {Object} obj JavaScript value to validate.
- * @param {boolean=} options.allowDeletes Whether field deletes are supported
- * at the top level (e.g. for document updates).
- * @param {boolean=} options.allowNestedDeletes Whether field deletes are supported
- * at any level (e.g. for document merges).
+ * @param {string} options.allowDeletes At what level field deletes are
+ * supported (acceptable values are 'none', 'root' or 'all').
+ * @param {boolean} options.allowServerTimestamps Whether server timestamps
+ * are supported.
  * @param {number=} depth The current depth of the traversal.
  * @returns {boolean} 'true' when the object is valid.
  * @throws {Error} when the object is invalid.
  */
 function validateFieldValue(obj, options, depth) {
-  options = options || {};
+  assert(
+    ['none', 'root', 'all'].includes(options.allowDeletes),
+    "Expected 'none', 'root', or 'all' for 'options.allowDeletes'"
+  );
+  assert(
+    typeof options.allowServerTimestamps === 'boolean',
+    "Expected boolean for 'options.allowServerTimestamps'"
+  );
 
   if (!depth) {
     depth = 1;
@@ -1332,15 +1337,7 @@ function validateFieldValue(obj, options, depth) {
     );
   }
 
-  if (DocumentTransform.isTransformSentinel(obj)) {
-    if (obj === FieldValue.DELETE_SENTINEL) {
-      if (!options.allowNestedDeletes && (!options.allowDeletes || depth > 1)) {
-        throw new Error(
-          'Deletes must appear at the top-level and can only be used in update() or set() with {merge:true}.'
-        );
-      }
-    }
-  } else if (is.array(obj)) {
+  if (is.array(obj)) {
     for (let prop of obj) {
       validateFieldValue(obj[prop], options, depth + 1);
     }
@@ -1350,21 +1347,29 @@ function validateFieldValue(obj, options, depth) {
         validateFieldValue(obj[prop], options, depth + 1);
       }
     }
-  } else if (is.object(obj)) {
-    if (is.instanceof(obj, DocumentReference)) {
-      return true;
-    } else if (is.instanceof(obj, FieldValue)) {
-      return true;
+  } else if (obj === FieldValue.DELETE_SENTINEL) {
+    if (
+      (options.allowDeletes === 'root' && depth > 1) ||
+      options.allowDeletes === 'none'
+    ) {
+      throw new Error(
+        'Deletes must appear at the top-level and can only be used in update() or set() with {merge:true}.'
+      );
     }
-
-    validate.throwVersionMismatchErrorIfType(obj, DocumentReference);
-    validate.throwVersionMismatchErrorIfType(obj, FieldValue);
-
-    throw new Error(
-      'Cannot use type (' +
-        Object.prototype.toString.call(obj) +
-        ') as a Firestore Value'
-    );
+  } else if (obj === FieldValue.SERVER_TIMESTAMP_SENTINEL) {
+    if (!options.allowServerTimestamps) {
+      throw new Error(
+        'ServerTimestamps can only be used in update(), set() and create().'
+      );
+    }
+  } else if (is.instanceof(obj, DocumentReference)) {
+    return true;
+  } else if (is.instanceof(obj, GeoPoint)) {
+    return true;
+  } else if (is.instanceof(obj, FieldPath)) {
+    throw new Error("Cannot use 'FieldPath' as a Firestore type.");
+  } else if (is.object(obj)) {
+    validate.throwCustomObjectError(obj);
   }
 
   return true;

src/field-value.js

@@ -35,7 +35,12 @@ let SERVER_TIMESTAMP_SENTINEL;
  * @class
  */
 class FieldValue {
+  /**
+   * @private
+   * @hideconstructor
+   */
   constructor() {}
+
   /**
    * Returns a sentinel used with update() to mark a field for deletion.
    *
@@ -77,17 +82,8 @@ class FieldValue {
   }
 }
 
-/*!
- * Sentinel value for a field delete.
- *
- */
- DELETE_SENTINEL = new FieldValue();
-
-/*!
- * Sentinel value for a server timestamp.
- *
- */
- SERVER_TIMESTAMP_SENTINEL = new FieldValue();
+DELETE_SENTINEL = new FieldValue();
+SERVER_TIMESTAMP_SENTINEL = new FieldValue();
 
 module.exports = FieldValue;
 module.exports.DELETE_SENTINEL = DELETE_SENTINEL;

src/index.js

@@ -1147,31 +1147,6 @@ Transaction = require('./transaction')(Firestore);
  * region_tag:firestore_quickstart
  * Full quickstart example:
  */
-
-const version = require('../package.json').version;
-
-if (!is.defined(global.__FIRESTORE_DATA)) {
-  global.__FIRESTORE_DATA = {
-    version: version,
-    path: __dirname,
-  };
-} else if (is.defined(global.__FIRESTORE_DATA)) {
-  if (global.__FIRESTORE_DATA.version !== version) {
-    console.warn(
-      `Importing multiple versions @google-cloud/firestore can cause unexpected ` +
-        `behavior. If you are using Firestore through a transitive ` +
-        `dependency (such as the Firebase Admin SDK), please ensure that your ` +
-        `dependency versions match (found ${
-          global.__FIRESTORE_DATA.version
-        } and ` +
-        `${version}). For more information, see http://goo.gl/...`
-    );
-  } else if (__dirname !== global.__FIRESTORE_DATA.path) {
-    module.exports = require(global.__FIRESTORE_DATA.path);
-    return;
-  }
-}
-
 module.exports = Firestore;
 module.exports.default = Firestore;
 module.exports.Firestore = Firestore;

src/order.js

@@ -17,6 +17,7 @@
 'use strict';
 
 const is = require('is');
+const validate = require('./validate')();
 
 /*!
  * @see ResourcePath
@@ -78,13 +79,7 @@ function typeOrder(val) {
       return types.OBJECT;
     }
     default: {
-      throw new Error(
-        'Cannot use type (' +
-          val +
-          ': ' +
-          JSON.stringify(val) +
-          ') as a Firestore value.'
-      );
+      validate.throwCustomObjectError(val);
     }
   }
 }
@@ -260,10 +255,16 @@ function compare(left, right) {
       return compareGeoPoints(left.geoPointValue, right.geoPointValue);
     }
     case types.ARRAY: {
-      return compareArrays(left.arrayValue.values, right.arrayValue.values);
+      return compareArrays(
+        left.arrayValue.values || [],
+        right.arrayValue.values || []
+      );
     }
     case types.OBJECT: {
-      return compareObjects(left.mapValue.fields, right.mapValue.fields);
+      return compareObjects(
+        left.mapValue.fields || {},
+        right.mapValue.fields || {}
+      );
     }
   }
 }

src/path.js

@@ -483,10 +483,8 @@ class FieldPath extends Path {
    */
   static validateFieldPath(fieldPath) {
     if (!is.instanceof(fieldPath, FieldPath)) {
-      validate.throwVersionMismatchErrorIfType(fieldPath, FieldPath);
-
       if (!is.string(fieldPath)) {
-        throw new Error(`Paths must be strings or FieldPath objects.`);
+        validate.throwCustomObjectError(fieldPath);
       }
 
       if (fieldPath.indexOf('..') >= 0) {

src/reference.js

@@ -1118,6 +1118,10 @@ class Query {
   where(fieldPath, opStr, value) {
     validate.isFieldPath('fieldPath', fieldPath);
     validate.isFieldComparison('opStr', opStr, value);
+    validate.isFieldValue('value', value, {
+      allowDeletes: 'none',
+      allowServerTimestamps: false,
+    });
 
     if (this._queryOptions.startAt || this._queryOptions.endAt) {
       throw new Error(
@@ -1422,12 +1426,10 @@ class Query {
         }
       }
 
-      if (DocumentTransform.isTransformSentinel(fieldValue)) {
-        throw new Error(
-          `Cannot use FieldValue.delete() or FieldValue.serverTimestamp() in ` +
-            `a query boundary. Found at index ${i}.`
-        );
-      }
+      validate.isFieldValue(i, fieldValue, {
+        allowDeletes: 'none',
+        allowServerTimestamps: false,
+      });
 
       options.values.push(DocumentSnapshot.encodeValue(fieldValue));
     }
@@ -2019,7 +2021,11 @@ class CollectionReference extends Query {
    * });
    */
   add(data) {
-    validate.isDocument('data', data);
+    validate.isDocument('data', data, {
+      allowEmpty: true,
+      allowDeletes: 'none',
+      allowServerTimestamps: true,
+    });
 
     let documentRef = this.doc();
     return documentRef.create(data).then(() => {
@@ -2092,8 +2098,7 @@ function validateDocumentReference(value) {
   if (is.instanceof(value, DocumentReference)) {
     return true;
   }
-  validate.verifyConstructorName(value, DocumentReference);
-  return false;
+  validate.throwCustomObjectError(value);
 }
 
 module.exports = FirestoreType => {
@@ -2117,6 +2122,7 @@ module.exports = FirestoreType => {
     FieldPath: FieldPath.validateFieldPath,
     FieldComparison: validateComparisonOperator,
     FieldOrder: validateFieldOrder,
+    FieldValue: document.validateFieldValue,
     Precondition: document.validatePrecondition,
     ResourcePath: ResourcePath.validateResourcePath,
   });

src/validate.js

@@ -140,13 +140,22 @@ module.exports = validators => {
     return true;
   };
 
-  exports.throwVersionMismatchErrorIfType = (obj, classType) => {
-    if (is.object(obj) && obj.constructor.name === classType.name) {
-      throw new Error(
-        `Detected an object of type '${
-          obj.constructor.name
-        }' that doesn't match the expected version for this Firestore instance. For more information, please visit: http://go...`
-      );
+  exports.throwCustomObjectError = val => {
+    let typeName = is.object(val) ? val.constructor.name : typeof val;
+
+    switch (typeName) {
+      case 'DocumentReference':
+      case 'FieldPath':
+      case 'FieldValue':
+      case 'GeoPoint':
+        throw new Error(
+          `Detected an object of type '${typeName}' that doesn't match the expected instance. Please ensure that ` +
+            'the Firestore types you are using are from the same NPM package.'
+        );
+      default:
+        throw new Error(
+          `Cannot use custom type '${typeName}' as a Firestore type.`
+        );
     }
   };