Skip to content

Rejecting custom types from the API surface #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 65 additions & 46 deletions src/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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.');
}
Expand All @@ -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;
Expand All @@ -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;
Expand Down
13 changes: 11 additions & 2 deletions src/field-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
*
Expand Down Expand Up @@ -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;
19 changes: 10 additions & 9 deletions src/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
'use strict';

const is = require('is');
const validate = require('./validate')();

/*!
* @see ResourcePath
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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 || {}
);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
34 changes: 18 additions & 16 deletions src/reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ let Firestore;
*/
let DocumentSnapshot;

/*!
* Injected.
*
* @see DocumentTransform
*/
let DocumentTransform;

/*!
* Injected.
*/
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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,
Expand All @@ -2134,6 +2135,7 @@ module.exports = FirestoreType => {
FieldPath: FieldPath.validateFieldPath,
FieldComparison: validateComparisonOperator,
FieldOrder: validateFieldOrder,
FieldValue: document.validateFieldValue,
Precondition: document.validatePrecondition,
ResourcePath: ResourcePath.validateResourcePath,
});
Expand Down
19 changes: 19 additions & 0 deletions src/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Loading