From a85f0c32eca5d8cf677d621a8ff326623ad5266e Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 8 Jan 2020 12:01:52 -0800 Subject: [PATCH] fix: support Objects created with Object.create({}) (#842) --- dev/src/document.ts | 4 ++-- dev/src/serializer.ts | 18 +----------------- dev/src/transaction.ts | 3 +-- dev/src/util.ts | 19 +++++++++++++++++++ dev/src/validate.ts | 18 +----------------- dev/src/write-batch.ts | 4 ++-- dev/test/document.ts | 22 ++++++++++++++-------- dev/test/util.ts | 35 +++++++++++++++++++++++++++++++++++ 8 files changed, 75 insertions(+), 48 deletions(-) create mode 100644 dev/test/util.ts diff --git a/dev/src/document.ts b/dev/src/document.ts index 6cb6f1480..a13674f5f 100644 --- a/dev/src/document.ts +++ b/dev/src/document.ts @@ -23,10 +23,10 @@ import {google} from '../protos/firestore_v1_proto_api'; import {FieldTransform} from './field-value'; import {FieldPath, validateFieldPath} from './path'; import {DocumentReference} from './reference'; -import {isPlainObject, Serializer} from './serializer'; +import {Serializer} from './serializer'; import {Timestamp} from './timestamp'; import {ApiMapValue, DocumentData, UpdateMap} from './types'; -import {isEmpty, isObject} from './util'; +import {isEmpty, isObject, isPlainObject} from './util'; import api = google.firestore.v1; diff --git a/dev/src/serializer.ts b/dev/src/serializer.ts index 346161044..1b25b12bd 100644 --- a/dev/src/serializer.ts +++ b/dev/src/serializer.ts @@ -24,7 +24,7 @@ import {DocumentReference, Firestore} from './index'; import {FieldPath, QualifiedResourcePath} from './path'; import {Timestamp} from './timestamp'; import {ApiMapValue, DocumentData, ValidationOptions} from './types'; -import {isEmpty, isObject} from './util'; +import {isEmpty, isObject, isPlainObject} from './util'; import {customObjectMessage, invalidArgumentMessage} from './validate'; import api = proto.google.firestore.v1; @@ -256,22 +256,6 @@ export class Serializer { } } -/** - * Verifies that 'obj' is a plain JavaScript object that can be encoded as a - * 'Map' in Firestore. - * - * @private - * @param input The argument to verify. - * @returns 'true' if the input can be a treated as a plain object. - */ -export function isPlainObject(input: unknown): input is DocumentData { - return ( - isObject(input) && - (Object.getPrototypeOf(input) === Object.prototype || - Object.getPrototypeOf(input) === null) - ); -} - /** * Validates a JavaScript value for usage as a Firestore value. * diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 25b7cea24..7f20ec910 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -25,7 +25,6 @@ import { QuerySnapshot, validateDocumentReference, } from './reference'; -import {isPlainObject} from './serializer'; import { DocumentData, Precondition as PublicPrecondition, @@ -33,7 +32,7 @@ import { SetOptions, UpdateData, } from './types'; -import {isObject, requestTag} from './util'; +import {isObject, isPlainObject} from './util'; import { invalidArgumentMessage, RequiredArgumentOptions, diff --git a/dev/src/util.ts b/dev/src/util.ts index 95252e5a4..d7a690ce7 100644 --- a/dev/src/util.ts +++ b/dev/src/util.ts @@ -16,6 +16,8 @@ import {GoogleError, ServiceConfig, Status} from 'google-gax'; +import {DocumentData} from './types'; + /** * A Promise implementation that supports deferred resolution. * @private @@ -77,6 +79,23 @@ export function isObject(value: unknown): value is {[k: string]: unknown} { return Object.prototype.toString.call(value) === '[object Object]'; } +/** + * Verifies that 'obj' is a plain JavaScript object that can be encoded as a + * 'Map' in Firestore. + * + * @private + * @param input The argument to verify. + * @returns 'true' if the input can be a treated as a plain object. + */ +export function isPlainObject(input: unknown): input is DocumentData { + return ( + isObject(input) && + (Object.getPrototypeOf(input) === Object.prototype || + Object.getPrototypeOf(input) === null || + input.constructor.name === 'Object') + ); +} + /** * Returns whether `value` has no custom properties. * diff --git a/dev/src/validate.ts b/dev/src/validate.ts index b749e08f3..b820cca0f 100644 --- a/dev/src/validate.ts +++ b/dev/src/validate.ts @@ -37,22 +37,6 @@ export interface NumericRangeOptions { maxValue?: number; } -/** - * Returns the name of the base class (ignoring Object). - * - * @private - * @param value The object whose base class name to extract. - * @returns The name of the base class constructor or "Object" if value does not - * extend a custom class. - */ -function extractBaseClassName(value: object): string { - let constructorName = 'Object'; - while (Object.getPrototypeOf(value) !== Object.prototype) { - value = Object.getPrototypeOf(value); - constructorName = value.constructor.name; - } - return constructorName; -} /** * Generates an error message to use with custom objects that cannot be * serialized. @@ -73,7 +57,7 @@ export function customObjectMessage( // We use the base class name as the type name as the sentinel classes // returned by the public FieldValue API are subclasses of FieldValue. By // using the base name, we reduce the number of special cases below. - const typeName = extractBaseClassName(value); + const typeName = value.constructor.name; switch (typeName) { case 'DocumentReference': case 'FieldPath': diff --git a/dev/src/write-batch.ts b/dev/src/write-batch.ts index 9779c0ea3..19028f55c 100644 --- a/dev/src/write-batch.ts +++ b/dev/src/write-batch.ts @@ -28,7 +28,7 @@ import {Firestore} from './index'; import {logger} from './logger'; import {FieldPath, validateFieldPath} from './path'; import {DocumentReference, validateDocumentReference} from './reference'; -import {isPlainObject, Serializer, validateUserInput} from './serializer'; +import {Serializer, validateUserInput} from './serializer'; import {Timestamp} from './timestamp'; import { Precondition as PublicPrecondition, @@ -37,7 +37,7 @@ import { UpdateMap, } from './types'; import {DocumentData} from './types'; -import {isObject, requestTag} from './util'; +import {isObject, isPlainObject, requestTag} from './util'; import { customObjectMessage, invalidArgumentMessage, diff --git a/dev/test/document.ts b/dev/test/document.ts index dab80e153..6facb2029 100644 --- a/dev/test/document.ts +++ b/dev/test/document.ts @@ -166,18 +166,24 @@ describe('serialize document', () => { }).to.throw( 'Value for argument "data" is not a valid Firestore document. Couldn\'t serialize object of type "Foo". Firestore doesn\'t support JavaScript objects with custom prototypes (i.e. objects that were created via the "new" operator).' ); + + expect(() => { + class Foo {} + class Bar extends Foo {} + firestore + .doc('collectionId/documentId') + .set(new Bar() as InvalidApiUsage); + }).to.throw( + 'Value for argument "data" is not a valid Firestore document. Couldn\'t serialize object of type "Bar". Firestore doesn\'t support JavaScript objects with custom prototypes (i.e. objects that were created via the "new" operator).' + ); }); it('provides custom error for objects from different Firestore instance', () => { class FieldPath {} - class CustomFieldPath extends FieldPath {} - class VeryCustomFieldPath extends CustomFieldPath {} + class GeoPoint {} + class Timestamp {} - const customClasses = [ - new FieldPath(), - new CustomFieldPath(), - new VeryCustomFieldPath(), - ]; + const customClasses = [new FieldPath(), new GeoPoint(), new Timestamp()]; for (const customClass of customClasses) { expect(() => { @@ -186,7 +192,7 @@ describe('serialize document', () => { .set(customClass as InvalidApiUsage); }).to.throw( 'Value for argument "data" is not a valid Firestore document. ' + - 'Detected an object of type "FieldPath" that doesn\'t match the expected instance.' + `Detected an object of type "${customClass.constructor.name}" that doesn't match the expected instance.` ); } }); diff --git a/dev/test/util.ts b/dev/test/util.ts new file mode 100644 index 000000000..805d2bd8b --- /dev/null +++ b/dev/test/util.ts @@ -0,0 +1,35 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {expect} from 'chai'; +import {isPlainObject} from '../src/util'; + +describe('isPlainObject()', () => { + it('allows Object.create()', () => { + expect(isPlainObject(Object.create({}))).to.be.true; + expect(isPlainObject(Object.create(Object.prototype))).to.be.true; + expect(isPlainObject(Object.create(null))).to.be.true; + }); + + it(' allows plain types', () => { + expect(isPlainObject({foo: 'bar'})).to.be.true; + expect(isPlainObject({})).to.be.true; + }); + + it('rejects custom types', () => { + class Foo {} + expect(isPlainObject(new Foo())).to.be.false; + expect(isPlainObject(Object.create(new Foo()))).to.be.false; + }); +});