Skip to content

Refactor Filters #1894

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

Merged
merged 10 commits into from
Jun 21, 2019
Merged

Refactor Filters #1894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8ed122d
Rename RelationFilter to FieldFilter
wilhuff Jun 19, 2019
b64b21e
Make NullFilter and NanFilter extend FieldFilter
wilhuff Jun 19, 2019
0c41e9a
Rename RelationOp to Operator
wilhuff Jun 19, 2019
a80ada0
Extract KeyFieldFilter from FieldFilter
wilhuff Jun 19, 2019
3d65fda
Extract ArrayContainsFilter from FieldFilter
wilhuff Jun 19, 2019
cbf19e5
Extract InFilter from FieldFilter
wilhuff Jun 19, 2019
fab9aba
Extract ArrayContainsAnyFilter from FieldFilter
wilhuff Jun 19, 2019
c4e1f3b
Factor out ArrayValue.contains
wilhuff Jun 19, 2019
7efed96
Fix lint
wilhuff Jun 19, 2019
51382da
Review feedback
wilhuff Jun 20, 2019
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
41 changes: 19 additions & 22 deletions packages/firestore/src/api/database.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ import {
import {
Bound,
Direction,
FieldFilter,
Filter,
Operator,
OrderBy,
Query as InternalQuery,
RelationFilter,
RelationOp
Query as InternalQuery
} from '../core/query';
import { Transaction as InternalTransaction } from '../core/transaction';
import { ChangeType, ViewSnapshot } from '../core/view_snapshot';
Expand Down Expand Up @@ -1430,16 +1430,16 @@ export class Query implements firestore.Query {

let fieldValue;
const fieldPath = fieldPathFromArgument('Query.where', field);
const relationOp = RelationOp.fromString(opStr);
const operator = Operator.fromString(opStr);
if (fieldPath.isKeyField()) {
if (
relationOp === RelationOp.ARRAY_CONTAINS ||
relationOp === RelationOp.ARRAY_CONTAINS_ANY ||
relationOp === RelationOp.IN
operator === Operator.ARRAY_CONTAINS ||
operator === Operator.ARRAY_CONTAINS_ANY ||
operator === Operator.IN
) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid Query. You can't perform '${relationOp.toString()}' ` +
`Invalid Query. You can't perform '${operator.toString()}' ` +
'queries on FieldPath.documentId().'
);
}
Expand Down Expand Up @@ -1492,20 +1492,20 @@ export class Query implements firestore.Query {
}
} else {
if (
relationOp === RelationOp.IN ||
relationOp === RelationOp.ARRAY_CONTAINS_ANY
operator === Operator.IN ||
operator === Operator.ARRAY_CONTAINS_ANY
) {
if (!Array.isArray(value) || value.length === 0) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
'Invalid Query. A non-empty array is required for ' +
`'${relationOp.toString()}' filters.`
`'${operator.toString()}' filters.`
);
}
if (value.length > 10) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid Query. '${relationOp.toString()}' filters support a ` +
`Invalid Query. '${operator.toString()}' filters support a ` +
'maximum of 10 elements in the value array.'
);
}
Expand All @@ -1515,7 +1515,7 @@ export class Query implements firestore.Query {
value
);
}
const filter = Filter.create(fieldPath, relationOp, fieldValue);
const filter = Filter.create(fieldPath, operator, fieldValue);
this.validateNewFilter(filter);
return new Query(this._query.addFilter(filter), this.firestore);
}
Expand Down Expand Up @@ -1947,12 +1947,9 @@ export class Query implements firestore.Query {
}

private validateNewFilter(filter: Filter): void {
if (filter instanceof RelationFilter) {
const arrayOps = [
RelationOp.ARRAY_CONTAINS,
RelationOp.ARRAY_CONTAINS_ANY
];
const disjunctiveOps = [RelationOp.IN, RelationOp.ARRAY_CONTAINS_ANY];
if (filter instanceof FieldFilter) {
const arrayOps = [Operator.ARRAY_CONTAINS, Operator.ARRAY_CONTAINS_ANY];
const disjunctiveOps = [Operator.IN, Operator.ARRAY_CONTAINS_ANY];
const isArrayOp = arrayOps.indexOf(filter.op) >= 0;
const isDisjunctiveOp = disjunctiveOps.indexOf(filter.op) >= 0;

Expand All @@ -1978,12 +1975,12 @@ export class Query implements firestore.Query {
} else if (isDisjunctiveOp || isArrayOp) {
// You can have at most 1 disjunctive filter and 1 array filter. Check if
// the new filter conflicts with an existing one.
let conflictingOp: RelationOp | null = null;
let conflictingOp: Operator | null = null;
if (isDisjunctiveOp) {
conflictingOp = this._query.findRelationOpFilter(disjunctiveOps);
conflictingOp = this._query.findFilterOperator(disjunctiveOps);
}
if (conflictingOp === null && isArrayOp) {
conflictingOp = this._query.findRelationOpFilter(arrayOps);
conflictingOp = this._query.findFilterOperator(arrayOps);
}
if (conflictingOp != null) {
// We special case when it's a duplicate op to give a slightly clearer error message.
Expand Down
Loading