Skip to content

Commit

Permalink
More: Cleanup: Enum validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@schmidt-sebastian
schmidt-sebastian committed Dec 28, 2018
1 parent f79db80 commit 75eb0de
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 63 deletions.
51 changes: 19 additions & 32 deletions dev/src/reference.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ import {Firestore} from './index';
*/
const directionOperators: {[k: string]: api.StructuredQuery.Direction} = {
asc: 'ASCENDING',
ASC: 'ASCENDING',
desc: 'DESCENDING',
DESC: 'DESCENDING',
};

/*!
Expand All @@ -60,7 +58,6 @@ const comparisonOperators:
{[k: string]: api.StructuredQuery.FieldFilter.Operator} = {
'<': 'LESS_THAN',
'<=': 'LESS_THAN_OR_EQUAL',
'=': 'EQUAL',
'==': 'EQUAL',
'>': 'GREATER_THAN',
'>=': 'GREATER_THAN_OR_EQUAL',
Expand Down Expand Up @@ -1014,7 +1011,8 @@ export class Query {
* });
* });
*/
where(fieldPath: string|FieldPath, opStr: string, value: UserInput): Query {
where(fieldPath: string|FieldPath, opStr: WhereFilterOp, value: unknown):
Query {
this._validator.isFieldPath('fieldPath', fieldPath);
this._validator.isQueryComparison('opStr', opStr, value);
this._validator.isQueryValue('value', value, {
Expand Down Expand Up @@ -1107,7 +1105,7 @@ export class Query {
* });
* });
*/
orderBy(fieldPath: string|FieldPath, directionStr?: string): Query {
orderBy(fieldPath: string|FieldPath, directionStr?: OrderByDirection): Query {
this._validator.isFieldPath('fieldPath', fieldPath);
this._validator.isOptionalFieldOrder('directionStr', directionStr);

Expand Down Expand Up @@ -1720,8 +1718,9 @@ export class Query {
(s1: QueryDocumentSnapshot, s2: QueryDocumentSnapshot) => number {
return (doc1, doc2) => {
// Add implicit sorting by name, using the last specified direction.
const lastDirection = this._fieldOrders.length === 0 ?
directionOperators.ASC :
const lastDirection: api.StructuredQuery.Direction =
this._fieldOrders.length === 0 ?
'ASCENDING' :
this._fieldOrders[this._fieldOrders.length - 1].direction;
const orderBys = this._fieldOrders.concat(
new FieldOrder(FieldPath.documentId(), lastDirection));
Expand All @@ -1743,8 +1742,7 @@ export class Query {
}

if (comp !== 0) {
const direction =
orderBy.direction === directionOperators.ASC ? 1 : -1;
const direction = orderBy.direction === 'ASCENDING' ? 1 : -1;
return direction * comp;
}
}
Expand Down Expand Up @@ -1960,12 +1958,8 @@ function createCollectionReference(firestore, path): CollectionReference {
* @param {string=} str Order direction to validate.
* @throws {Error} when the direction is invalid
*/
export function validateFieldOrder(str: string): boolean {
if (!is.string(str) || !is.defined(directionOperators[str])) {
throw new Error('Order must be one of "asc" or "desc".');
}

return true;
export function validateQueryOrder(arg: string|number, op: unknown): void {
validateEnumValue(arg, op, Object.keys(directionOperators), {optional: true});
}

/*!
Expand All @@ -1975,26 +1969,19 @@ export function validateFieldOrder(str: string): boolean {
* @param {*} val Value that is used in the filter.
* @throws {Error} when the comparison operation is invalid
*/
export function validateComparisonOperator(
str: string, val: UserInput): boolean {
if (is.string(str) && comparisonOperators[str]) {
const op = comparisonOperators[str];

if (typeof val === 'number' && isNaN(val) && op !== 'EQUAL') {
throw new Error(
'Invalid query. You can only perform equals comparisons on NaN.');
}

if (val === null && op !== 'EQUAL') {
throw new Error(
'Invalid query. You can only perform equals comparisons on Null.');
}
export function validateQueryOperator(
arg: string|number, op: unknown, fieldValue: unknown): void {
validateEnumValue(arg, op, Object.keys(comparisonOperators));

return true;
if (typeof fieldValue === 'number' && isNaN(fieldValue) && op !== '==') {
throw new Error(
'Invalid query. You can only perform equals comparisons on NaN.');
}

throw new Error(
'Operator must be one of "<", "<=", "==", ">", ">=" or "array-contains".');
if (fieldValue === null && op !== '==') {
throw new Error(
'Invalid query. You can only perform equals comparisons on Null.');
}
}

/*!
Expand Down
12 changes: 12 additions & 0 deletions dev/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,18 @@ export type UpdateData = {
[fieldPath: string]: UserInput
};

/**
* The direction of a `Query.orderBy()` clause is specified as 'desc' or 'asc'
* (descending or ascending).
*/
export type OrderByDirection = 'desc'|'asc';

/**
* Filter conditions in a `Query.where()` clause are specified using the
* strings '<', '<=', '==', '>=', '>', and 'array-contains'.
*/
export type WhereFilterOp = '<'|'<='|'=='|'>='|'>'|'array-contains';

/**
* An options object that configures conditional behavior of `update()` and
* `delete()` calls in `DocumentReference`, `WriteBatch`, and `Transaction`.
Expand Down
63 changes: 32 additions & 31 deletions dev/test/query.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,13 @@ describe('query interface', () => {
};

queryEquals(
[query.where('a', '=', '1'), query.where('a', '=', '1')],
[query.where('a', '=', 1)]);
[query.where('a', '==', '1'), query.where('a', '==', '1')],
[query.where('a', '==', 1)]);

queryEquals(
[
query.orderBy('__name__'),
query.orderBy('__name__', 'asc'),
query.orderBy('__name__', 'ASC'),
query.orderBy(Firestore.FieldPath.documentId()),
],
[
Expand Down Expand Up @@ -355,7 +354,7 @@ describe('query interface', () => {

return createInstance(overrides).then(firestore => {
let query: Query = firestore.collection('collectionId');
query = query.where('foo', '=', 'bar');
query = query.where('foo', '==', 'bar');
query = query.orderBy('foo');
query = query.limit(10);
return query.get().then(results => {
Expand Down Expand Up @@ -603,10 +602,10 @@ describe('where() interface', () => {
fieldFilters(
'fooSmaller', 'LESS_THAN', 'barSmaller', 'fooSmallerOrEquals',
'LESS_THAN_OR_EQUAL', 'barSmallerOrEquals', 'fooEquals',
'EQUAL', 'barEquals', 'fooEqualsLong', 'EQUAL', 'barEqualsLong',
'fooGreaterOrEquals', 'GREATER_THAN_OR_EQUAL',
'barGreaterOrEquals', 'fooGreater', 'GREATER_THAN',
'barGreater', 'fooContains', 'ARRAY_CONTAINS', 'barContains'));
'EQUAL', 'barEquals', 'fooGreaterOrEquals',
'GREATER_THAN_OR_EQUAL', 'barGreaterOrEquals', 'fooGreater',
'GREATER_THAN', 'barGreater', 'fooContains', 'ARRAY_CONTAINS',
'barContains'));

return stream();
}
Expand All @@ -616,8 +615,7 @@ describe('where() interface', () => {
let query: Query = firestore.collection('collectionId');
query = query.where('fooSmaller', '<', 'barSmaller');
query = query.where('fooSmallerOrEquals', '<=', 'barSmallerOrEquals');
query = query.where('fooEquals', '=', 'barEquals');
query = query.where('fooEqualsLong', '==', 'barEqualsLong');
query = query.where('fooEquals', '==', 'barEquals');
query = query.where('fooGreaterOrEquals', '>=', 'barGreaterOrEquals');
query = query.where('fooGreater', '>', 'barGreater');
query = query.where('fooContains', 'array-contains', 'barContains');
Expand All @@ -642,7 +640,7 @@ describe('where() interface', () => {

return createInstance(overrides).then(firestore => {
let query: Query = firestore.collection('collectionId');
query = query.where('foo', '=', {foo: 'bar'});
query = query.where('foo', '==', {foo: 'bar'});
return query.get();
});
});
Expand All @@ -660,8 +658,9 @@ describe('where() interface', () => {

return createInstance(overrides).then(firestore => {
let query: Query = firestore.collection('collectionId');
query = query.where('foo.bar', '=', 'foobar');
query = query.where(new Firestore.FieldPath('bar', 'foo'), '=', 'foobar');
query = query.where('foo.bar', '==', 'foobar');
query =
query.where(new Firestore.FieldPath('bar', 'foo'), '==', 'foobar');
return query.get();
});
});
Expand Down Expand Up @@ -689,7 +688,7 @@ describe('where() interface', () => {
it('rejects custom objects for field paths', () => {
expect(() => {
let query: Query = firestore.collection('collectionId');
query = query.where({} as InvalidApiUsage, '=', 'bar');
query = query.where({} as InvalidApiUsage, '==', 'bar');
return query.get();
})
.to.throw(
Expand All @@ -698,7 +697,7 @@ describe('where() interface', () => {
class FieldPath {}
expect(() => {
let query: Query = firestore.collection('collectionId');
query = query.where(new FieldPath() as InvalidApiUsage, '=', 'bar');
query = query.where(new FieldPath() as InvalidApiUsage, '==', 'bar');
return query.get();
})
.to.throw(
Expand All @@ -708,7 +707,7 @@ describe('where() interface', () => {
it('rejects field paths as value', () => {
expect(() => {
let query: Query = firestore.collection('collectionId');
query = query.where('foo', '=', new Firestore.FieldPath('bar'));
query = query.where('foo', '==', new Firestore.FieldPath('bar'));
return query.get();
})
.to.throw(
Expand All @@ -718,7 +717,7 @@ describe('where() interface', () => {
it('rejects field delete as value', () => {
expect(() => {
let query = firestore.collection('collectionId');
query = query.where('foo', '=', Firestore.FieldValue.delete());
query = query.where('foo', '==', Firestore.FieldValue.delete());
return query.get();
})
.to.throw(
Expand All @@ -735,31 +734,31 @@ describe('where() interface', () => {
const query = firestore.collection('collectionId');

expect(() => {
query.where('foo', '=', new Foo()).get();
query.where('foo', '==', new Foo()).get();
})
.to.throw(
'Argument "value" is not a valid QueryValue. 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(() => {
query.where('foo', '=', new FieldPath()).get();
query.where('foo', '==', new FieldPath()).get();
})
.to.throw(
'Detected an object of type "FieldPath" that doesn\'t match the expected instance.');

expect(() => {
query.where('foo', '=', new FieldValue()).get();
query.where('foo', '==', new FieldValue()).get();
})
.to.throw(
'Detected an object of type "FieldValue" that doesn\'t match the expected instance.');

expect(() => {
query.where('foo', '=', new DocumentReference()).get();
query.where('foo', '==', new DocumentReference()).get();
})
.to.throw(
'Detected an object of type "DocumentReference" that doesn\'t match the expected instance.');

expect(() => {
query.where('foo', '=', new GeoPoint()).get();
query.where('foo', '==', new GeoPoint()).get();
})
.to.throw(
'Detected an object of type "GeoPoint" that doesn\'t match the expected instance.');
Expand All @@ -777,7 +776,7 @@ describe('where() interface', () => {
return createInstance(overrides).then(firestore => {
let query: Query = firestore.collection('collectionId');
query = query.where('foo', '==', NaN);
query = query.where('bar', '=', null);
query = query.where('bar', '==', null);
return query.get();
});
});
Expand Down Expand Up @@ -805,7 +804,7 @@ describe('where() interface', () => {
it('verifies field path', () => {
let query: Query = firestore.collection('collectionId');
expect(() => {
query = query.where('foo.', '=', 'foobar');
query = query.where('foo.', '==', 'foobar');
})
.to.throw(
'Argument "fieldPath" is not a valid FieldPath. Paths must not start or end with ".".');
Expand All @@ -814,10 +813,10 @@ describe('where() interface', () => {
it('verifies operator', () => {
let query = firestore.collection('collectionId');
expect(() => {
query = query.where('foo', '@', 'foobar');
query = query.where('foo', '@' as InvalidApiUsage, 'foobar');
})
.to.throw(
'Operator must be one of "<", "<=", "==", ">", ">=" or "array-contains".');
'Invalid value for argument "opStr". Acceptable values are: <, <=, ==, >, >=, array-contains');
});
});

Expand Down Expand Up @@ -881,8 +880,10 @@ describe('orderBy() interface', () => {
it('verifies order', () => {
let query: Query = firestore.collection('collectionId');
expect(() => {
query = query.orderBy('foo', 'foo');
}).to.throw('Order must be one of "asc" or "desc".');
query = query.orderBy('foo', 'foo' as InvalidApiUsage);
})
.to.throw(
'Invalid value for argument "directionStr". Acceptable values are: asc, desc');
});

it('accepts field path', () => {
Expand Down Expand Up @@ -1347,10 +1348,10 @@ describe('startAt() interface', () => {
return createInstance(overrides).then(firestore => {
return snapshot('collectionId/doc', {c: 'c'}).then(doc => {
const query = firestore.collection('collectionId')
.where('a', '=', 'a')
.where('a', '==', 'a')
.where('b', 'array-contains', 'b')
.where('c', '>=', 'c')
.where('d', '=', 'd')
.where('d', '==', 'd')
.startAt(doc);
return query.get();
});
Expand All @@ -1374,7 +1375,7 @@ describe('startAt() interface', () => {
return createInstance(overrides).then(firestore => {
return snapshot('collectionId/doc', {foo: 'bar'}).then(doc => {
const query = firestore.collection('collectionId')
.where('foo', '=', 'bar')
.where('foo', '==', 'bar')
.startAt(doc);
return query.get();
});
Expand Down

0 comments on commit 75eb0de

Please sign in to comment.