Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Cleanup: Remove validator #499

Closed
wants to merge 1 commit into from
Closed

Cleanup: Remove validator #499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 4 additions & 6 deletions dev/src/document.ts
View file
Open in desktop
Expand Up @@ -90,7 +90,6 @@ export class DocumentSnapshot {
private _ref: DocumentReference;
private _fieldsProto: ApiMapValue|undefined;
private _serializer: Serializer;
private _validator: AnyDuringMigration;
private _readTime: Timestamp|undefined;
private _createTime: Timestamp|undefined;
private _updateTime: Timestamp|undefined;
Expand All @@ -115,7 +114,6 @@ export class DocumentSnapshot {
this._ref = ref;
this._fieldsProto = fieldsProto;
this._serializer = ref.firestore._serializer;
this._validator = ref.firestore._validator;
this._readTime = readTime;
this._createTime = createTime;
this._updateTime = updateTime;
Expand Down Expand Up @@ -383,8 +381,8 @@ export class DocumentSnapshot {
* console.log(`Retrieved field value: ${field}`);
* });
*/
get(field: string|FieldPath): UserInput {
this._validator.isFieldPath('field', field);
get(field: string|FieldPath): UserInput
validateFieldPath('field', field);

const protoField = this.protoField(field);

Expand Down Expand Up @@ -824,8 +822,8 @@ export class DocumentMask {
*/
export class DocumentTransform {
private readonly _ref: DocumentReference;
private readonly _validator: AnyDuringMigration;
private readonly _transforms: Map<FieldPath, FieldTransform>;

/**
* @private
* @hideconstructor
Expand All @@ -835,9 +833,9 @@ export class DocumentTransform {
*/
constructor(ref: DocumentReference, transforms) {
this._ref = ref;
this._validator = ref.firestore._validator;
this._transforms = transforms;
}

/**
* Generates a DocumentTransform from a JavaScript object.
*
Expand Down
44 changes: 25 additions & 19 deletions dev/src/field-value.ts
View file
Open in desktop
Expand Up @@ -172,7 +172,7 @@ export abstract class FieldTransform extends FieldValue {
abstract get methodName(): string;

/** Performs input validation on the values of this field transform. */
abstract validate(validator: AnyDuringMigration): boolean;
abstract validate(): void;

/***
* The proto representation for this field transform.
Expand Down Expand Up @@ -218,9 +218,7 @@ export class DeleteTransform extends FieldTransform {
return 'FieldValue.delete';
}

validate(): true {
return true;
}
validate(): void {}

toProto(serializer: Serializer, fieldPath: FieldPath): never {
throw new Error(
Expand Down Expand Up @@ -267,9 +265,7 @@ class ServerTimestampTransform extends FieldTransform {
return 'FieldValue.serverTimestamp';
}

validate(): true {
return true;
}
validate(): void {}

toProto(serializer: Serializer, fieldPath: FieldPath):
api.DocumentTransform.IFieldTransform {
Expand Down Expand Up @@ -308,13 +304,10 @@ class ArrayUnionTransform extends FieldTransform {
return 'FieldValue.arrayUnion';
}

validate(validator: AnyDuringMigration): boolean {
let valid = true;
for (let i = 0; valid && i < this.elements.length; ++i) {
valid = validator.isArrayElement(
i, this.elements[i], {allowDeletes: 'none', allowTransforms: false});
validate(): void {
for (let i = 0; i < this.elements.length; ++i) {
validateArrayElement(i, this.elements[i]);
}
return valid;
}

toProto(serializer: Serializer, fieldPath: FieldPath):
Expand Down Expand Up @@ -362,13 +355,10 @@ class ArrayRemoveTransform extends FieldTransform {
return 'FieldValue.arrayRemove';
}

validate(validator: AnyDuringMigration): boolean {
let valid = true;
for (let i = 0; valid && i < this.elements.length; ++i) {
valid = validator.isArrayElement(
i, this.elements[i], {allowDeletes: 'none', allowTransforms: false});
validate(): void {
for (let i = 0; i < this.elements.length; ++i) {
validateArrayElement(i, this.elements[i]);
}
return valid;
}

toProto(serializer: Serializer, fieldPath):
Expand All @@ -387,3 +377,19 @@ class ArrayRemoveTransform extends FieldTransform {
deepEqual(this.elements, other.elements, {strict: true})));
}
}

/**
* Validates that `value` can be used as an element inside of an array. Certain
* field values (such as ServerTimestamps) are rejected.
*
* @private
* @param arg The argument name or argument index (for varargs methods).
* @param value The value to validate.
*/
function validateArrayElement(arg: string|number, value: unknown): void {
validateUserInput(
arg, value, 'array element',
/*path=*/{allowDeletes: 'none', allowTransforms: false},
/*path=*/undefined,
/*level=*/0, /*inArray=*/true);
}
59 changes: 2 additions & 57 deletions dev/src/index.ts
View file
Open in desktop
Expand Up @@ -288,28 +288,6 @@ export class Firestore {
* use `Timestamp` now and opt-in to this new behavior as soon as you can.
*/
constructor(settings?: Settings) {
this._validator = new Validator({
ArrayElement: (name, value) => validateFieldValue(
name, value, /*path=*/undefined, /*level=*/0,
/*inArray=*/true),
DeletePrecondition: precondition =>
validatePrecondition(precondition, /* allowExists= */ true),
Document: validateDocumentData,
DocumentReference: validateDocumentReference,
FieldPath: FieldPath.validateFieldPath,
FieldValue: validateFieldValue,
FieldOrder: validateFieldOrder,
QueryComparison: validateComparisonOperator,
QueryValue: validateFieldValue,
ResourcePath: ResourcePath.validateResourcePath,
SetOptions: validateSetOptions,
ReadOptions: validateReadOptions,
UpdateMap: validateUpdateMap,
UpdatePrecondition: precondition =>
validatePrecondition(precondition, /* allowExists= */ false),
} as AnyDuringMigration);


const libraryHeader = {
libName: 'gccl',
libVersion,
Expand Down Expand Up @@ -412,7 +390,7 @@ export class Firestore {
* console.log(`Path of document is ${documentRef.path}`);
*/
doc(documentPath: string): DocumentReference {
this._validator.isResourcePath('documentPath', documentPath);
validateResourcePath('documentPath', documentPath);

const path = this._referencePath!.append(documentPath);
if (!path.isDocument) {
Expand Down Expand Up @@ -440,7 +418,7 @@ export class Firestore {
* });
*/
collection(collectionPath: string): CollectionReference {
this._validator.isResourcePath('collectionPath', collectionPath);
validateResourcePath('collectionPath', collectionPath);

const path = this._referencePath!.append(collectionPath);
if (!path.isCollection) {
Expand Down Expand Up @@ -1341,39 +1319,6 @@ function validateFieldValue(
return true;
}

/**
* Validates a JavaScript object for usage as a Firestore document.
*
* @private
* @param obj JavaScript object to validate.
* @param options Validation options
* @returns 'true' when the object is valid.
* @throws when the object is invalid.
*/
function validateDocumentData(
obj: DocumentData, options: ValidationOptions): boolean {
if (!isPlainObject(obj)) {
throw customObjectError(obj);
}

options = options || {};

let isEmpty = true;

for (const prop in obj) {
if (obj.hasOwnProperty(prop)) {
isEmpty = false;
validateFieldValue(obj[prop], options, new FieldPath(prop));
}
}

if (options.allowEmpty === false && isEmpty) {
throw new Error('At least one field must be updated.');
}

return true;
}

/**
* Validates the use of 'options' as ReadOptions and enforces that 'fieldMask'
* is an array of strings or field paths.
Expand Down
136 changes: 75 additions & 61 deletions dev/src/path.ts
View file
Open in desktop
Expand Up @@ -280,27 +280,6 @@ export class ResourcePath extends Path<ResourcePath> {
return null;
}

/**
* Returns true if the given string can be used as a relative or absolute
* resource path.
*
* @private
* @param {string} resourcePath The path to validate.
* @throws if the string can't be used as a resource path.
* @returns {boolean} 'true' when the path is valid.
*/
static validateResourcePath(resourcePath: string): boolean {
if (!is.string(resourcePath) || resourcePath === '') {
throw new Error(`Path must be a non-empty string.`);
}

if (resourcePath.indexOf('//') >= 0) {
throw new Error('Paths must not contain //.');
}

return true;
}

/**
* Creates a resource path from an absolute Firestore path.
*
Expand Down Expand Up @@ -415,6 +394,30 @@ export class ResourcePath extends Path<ResourcePath> {
}
}

/**
* Validates that the given string can be used as a relative or absolute
* resource path.
*
* @private
* @param arg The argument name or argument index (for varargs methods).
* @param resourcePath The path to validate.
* @throws if the string can't be used as a resource path.
*/
export function validateResourcePath(
arg: string|number, resourcePath: string): void {
if (typeof resourcePath !== 'string' || resourcePath === '') {
throw new Error(`${
invalidArgumentMessage(
arg, 'resource path')} Path must be a non-empty string.`);
}

if (resourcePath.indexOf('//') >= 0) {
throw new Error(`${
invalidArgumentMessage(
arg, 'resource path')} Paths must not contain //.`);
}
}

/**
* A dot-separated path for navigating sub-objects within a document.
*
Expand Down Expand Up @@ -471,46 +474,6 @@ export class FieldPath extends Path<FieldPath> {
return FieldPath._DOCUMENT_ID;
}

/**
* Returns true if the provided value can be used as a field path argument.
*
* @private
* @param fieldPath The value to verify.
* @throws if the string can't be used as a field path.
* @returns 'true' when the path is valid.
*/
static validateFieldPath(fieldPath: unknown): boolean {
if (!(fieldPath instanceof FieldPath)) {
if (fieldPath === undefined) {
throw new Error('Path cannot be omitted.');
}

if (is.object(fieldPath) &&
(fieldPath as object).constructor.name === 'FieldPath') {
throw customObjectError(fieldPath);
}

if (typeof fieldPath !== 'string') {
throw new Error(
'Paths can only be specified as strings or via a FieldPath object.');
}

if (fieldPath.indexOf('..') >= 0) {
throw new Error(`Paths must not contain ".." in them.`);
}

if (fieldPath.startsWith('.') || fieldPath.endsWith('.')) {
throw new Error(`Paths must not start or end with ".".`);
}

if (!FIELD_PATH_RE.test(fieldPath)) {
throw new Error(`Paths can't be empty and must not contain "*~/[]".`);
}
}

return true;
}

/**
* Turns a field path argument into a [FieldPath]{@link FieldPath}.
* Supports FieldPaths as input (which are passed through) and dot-separated
Expand Down Expand Up @@ -581,3 +544,54 @@ export class FieldPath extends Path<FieldPath> {
return super.isEqual(other);
}
}

/**
* Validates that the provided value can be used as a field path argument.
*
* @private
* @param arg The argument name or argument index (for varargs methods).
* @param fieldPath The value to verify.
* @throws if the string can't be used as a field path.
*/
export function validateFieldPath(
arg: string|number, fieldPath: unknown): void {
if (fieldPath instanceof FieldPath) {
return;
}

if (fieldPath === undefined) {
throw new Error(
invalidArgumentMessage(arg, 'field path') +
' The path cannot be omitted.');
}

if (isObject(fieldPath) && fieldPath.constructor.name === 'FieldPath') {
throw new Error(customObjectMessage(arg, fieldPath));
}

if (typeof fieldPath !== 'string') {
throw new Error(`${
invalidArgumentMessage(
arg,
'field path')} Paths can only be specified as strings or via a FieldPath object.`);
}

if (fieldPath.indexOf('..') >= 0) {
throw new Error(`${
invalidArgumentMessage(
arg, 'field path')} Paths must not contain ".." in them.`);
}

if (fieldPath.startsWith('.') || fieldPath.endsWith('.')) {
throw new Error(`${
invalidArgumentMessage(
arg, 'field path')} Paths must not start or end with ".".`);
}

if (!FIELD_PATH_RE.test(fieldPath)) {
throw new Error(`${
invalidArgumentMessage(
arg, 'field path')} Paths can't be empty and must not contain
"*~/[]".`);
}
}