Rejecting custom types from the API surface (#137)

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'
-    );
+    throw validate.customObjectError(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'].indexOf(options.allowDeletes) !== -1,
+    "Expected 'none', 'root', or 'all' for 'options.allowDeletes'"
+  );
+  assert(
+    typeof options.allowServerTimestamps === 'boolean',
+    "Expected boolean for 'options.allowServerTimestamps'"
+  );
 
   if (!depth) {
     depth = 1;
@@ -1332,25 +1337,39 @@ function validateFieldValue(obj, options, depth) {
     );
   }
 
-  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}.'
-      );
+  if (is.array(obj)) {
+    for (let prop of obj) {
+      validateFieldValue(obj[prop], options, depth + 1);
     }
-  }
-
-  if (isPlainObject(obj)) {
+  } else if (isPlainObject(obj)) {
     for (let prop in obj) {
       if (obj.hasOwnProperty(prop)) {
         validateFieldValue(obj[prop], options, depth + 1);
       }
     }
-  }
-  if (is.array(obj)) {
-    for (let prop of obj) {
-      validateFieldValue(obj[prop], options, depth + 1);
+  } else if (obj === FieldValue.DELETE_SENTINEL) {
+    if (
+      (options.allowDeletes === 'root' && depth > 1) ||
+      options.allowDeletes === 'none'
+    ) {
+      throw new Error(
+        'FieldValue.delete() must appear at the top-level and can only be used in update() or set() with {merge:true}.'
+      );
+    }
+  } else if (obj === FieldValue.SERVER_TIMESTAMP_SENTINEL) {
+    if (!options.allowServerTimestamps) {
+      throw new Error(
+        'FieldValue.serverTimestamp() 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)) {
+    throw validate.customObjectError(obj);
   }
 
   return true;

src/field-value.js

@@ -20,13 +20,13 @@
  * Sentinel value for a field delete.
  *
  */
-const DELETE_SENTINEL = {};
+let DELETE_SENTINEL;
 
 /*!
  * Sentinel value for a server timestamp.
  *
  */
-const SERVER_TIMESTAMP_SENTINEL = {};
+let SERVER_TIMESTAMP_SENTINEL;
 
 /**
  * Sentinel values that can be used when writing documents with set() or
@@ -35,6 +35,12 @@ const SERVER_TIMESTAMP_SENTINEL = {};
  * @class
  */
 class FieldValue {
+  /**
+   * @private
+   * @hideconstructor
+   */
+  constructor() {}
+
   /**
    * Returns a sentinel used with update() to mark a field for deletion.
    *
@@ -76,6 +82,9 @@ class FieldValue {
   }
 }
 
+DELETE_SENTINEL = new FieldValue();
+SERVER_TIMESTAMP_SENTINEL = new FieldValue();
+
 module.exports = FieldValue;
 module.exports.DELETE_SENTINEL = DELETE_SENTINEL;
 module.exports.SERVER_TIMESTAMP_SENTINEL = SERVER_TIMESTAMP_SENTINEL;

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.'
-      );
+      throw validate.customObjectError(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

@@ -484,7 +484,7 @@ class FieldPath extends Path {
   static validateFieldPath(fieldPath) {
     if (!is.instanceof(fieldPath, FieldPath)) {
       if (!is.string(fieldPath)) {
-        throw new Error(`Paths must be strings or FieldPath objects.`);
+        throw validate.customObjectError(fieldPath);
       }
 
       if (fieldPath.indexOf('..') >= 0) {

src/reference.js

@@ -36,13 +36,6 @@ let Firestore;
  */
 let DocumentSnapshot;
 
-/*!
- * Injected.
- *
- * @see DocumentTransform
- */
-let DocumentTransform;
-
 /*!
  * Injected.
  */
@@ -1118,6 +1111,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(
@@ -1402,12 +1399,10 @@ class Query {
         fieldValue = this._convertReference(fieldValue);
       }
 
-      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));
     }
@@ -2040,7 +2035,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(() => {
@@ -2110,14 +2109,16 @@ function validateComparisonOperator(str, val) {
  * @returns 'true' is value is an instance of DocumentReference.
  */
 function validateDocumentReference(value) {
-  return is.instanceof(value, DocumentReference);
+  if (is.instanceof(value, DocumentReference)) {
+    return true;
+  }
+  throw validate.customObjectError(value);
 }
 
 module.exports = FirestoreType => {
   Firestore = FirestoreType;
   let document = require('./document')(DocumentReference);
   DocumentSnapshot = document.DocumentSnapshot;
-  DocumentTransform = document.DocumentTransform;
   Watch = require('./watch')(
     FirestoreType,
     DocumentChange,
@@ -2134,6 +2135,7 @@ module.exports = FirestoreType => {
     FieldPath: FieldPath.validateFieldPath,
     FieldComparison: validateComparisonOperator,
     FieldOrder: validateFieldOrder,
+    FieldValue: document.validateFieldValue,
     Precondition: document.validatePrecondition,
     ResourcePath: ResourcePath.validateResourcePath,
   });

src/validate.js

@@ -140,5 +140,24 @@ module.exports = validators => {
     return true;
   };
 
+  exports.customObjectError = val => {
+    let typeName = is.object(val) ? val.constructor.name : typeof val;
+
+    switch (typeName) {
+      case 'DocumentReference':
+      case 'FieldPath':
+      case 'FieldValue':
+      case 'GeoPoint':
+        return 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:
+        return new Error(
+          `Cannot use custom type "${typeName}" as a Firestore type.`
+        );
+    }
+  };
+
   return exports;
 };