Skip to content

Commit 9ac7870

Browse files
author
Brian Chen
authored
Refactor Filters (#570)
1 parent c11a7a4 commit 9ac7870

File tree

17 files changed

+406
-312
lines changed

17 files changed

+406
-312
lines changed

‎firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/testutil/TestUtil.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import com.google.firebase.firestore.DocumentReference;
3434
import com.google.firebase.firestore.TestAccessHelper;
3535
import com.google.firebase.firestore.UserDataConverter;
36-
import com.google.firebase.firestore.core.Filter;
36+
import com.google.firebase.firestore.core.FieldFilter;
3737
import com.google.firebase.firestore.core.Filter.Operator;
3838
import com.google.firebase.firestore.core.OrderBy;
3939
import com.google.firebase.firestore.core.OrderBy.Direction;
@@ -220,8 +220,8 @@ public static ImmutableSortedSet<DocumentKey> keySet(DocumentKey... keys) {
220220
return keySet;
221221
}
222222

223-
public static Filter filter(String key, String operator, Object value) {
224-
return Filter.create(field(key), operatorFromString(operator), wrap(value));
223+
public static FieldFilter filter(String key, String operator, Object value) {
224+
return FieldFilter.create(field(key), operatorFromString(operator), wrap(value));
225225
}
226226

227227
public static Operator operatorFromString(String s) {

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,10 +429,11 @@ public void testQueriesCanUseArrayContainsFilters() {
429429
// much of anything else interesting to test.
430430
}
431431

432-
// TODO(in-queries): Re-enable in prod once feature lands in backend.
433432
@Test
434433
public void testQueriesCanUseInFilters() {
434+
// TODO(in-queries): Re-enable in prod once feature lands in backend.
435435
Assume.assumeTrue(isRunningAgainstEmulator());
436+
436437
Map<String, Object> docA = map("zip", 98101L);
437438
Map<String, Object> docB = map("zip", 91102L);
438439
Map<String, Object> docC = map("zip", 98103L);
@@ -452,10 +453,32 @@ public void testQueriesCanUseInFilters() {
452453
assertEquals(asList(docF), querySnapshotToValues(snapshot));
453454
}
454455

455-
// TODO(in-queries): Re-enable in prod once feature lands in backend.
456+
@Test
457+
public void testQueriesCanUseInFiltersWithDocIds() {
458+
// TODO(in-queries): Re-enable in prod once feature lands in backend.
459+
Assume.assumeTrue(isRunningAgainstEmulator());
460+
461+
Map<String, String> docA = map("key", "aa");
462+
Map<String, String> docB = map("key", "ab");
463+
Map<String, String> docC = map("key", "ba");
464+
Map<String, String> docD = map("key", "bb");
465+
Map<String, Map<String, Object>> testDocs =
466+
map(
467+
"aa", docA,
468+
"ab", docB,
469+
"ba", docC,
470+
"bb", docD);
471+
CollectionReference collection = testCollectionWithDocs(testDocs);
472+
QuerySnapshot docs =
473+
waitFor(collection.whereIn(FieldPath.documentId(), asList("aa", "ab")).get());
474+
assertEquals(asList(docA, docB), querySnapshotToValues(docs));
475+
}
476+
456477
@Test
457478
public void testQueriesCanUseArrayContainsAnyFilters() {
479+
// TODO(in-queries): Re-enable in prod once feature lands in backend.
458480
Assume.assumeTrue(isRunningAgainstEmulator());
481+
459482
Map<String, Object> docA = map("array", asList(42L));
460483
Map<String, Object> docB = map("array", asList("a", 42L, "c"));
461484
Map<String, Object> docC = map("array", asList(41.999, "42", map("a", asList(42))));

‎firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@
2929
import com.google.firebase.firestore.core.AsyncEventListener;
3030
import com.google.firebase.firestore.core.Bound;
3131
import com.google.firebase.firestore.core.EventManager.ListenOptions;
32+
import com.google.firebase.firestore.core.FieldFilter;
3233
import com.google.firebase.firestore.core.Filter;
3334
import com.google.firebase.firestore.core.Filter.Operator;
3435
import com.google.firebase.firestore.core.ListenerRegistrationImpl;
3536
import com.google.firebase.firestore.core.OrderBy;
3637
import com.google.firebase.firestore.core.QueryListener;
37-
import com.google.firebase.firestore.core.RelationFilter;
3838
import com.google.firebase.firestore.core.ViewSnapshot;
3939
import com.google.firebase.firestore.model.Document;
4040
import com.google.firebase.firestore.model.DocumentKey;
@@ -368,7 +368,7 @@ private Query whereHelper(@NonNull FieldPath fieldPath, Operator op, Object valu
368368
}
369369
fieldValue = firestore.getDataConverter().parseQueryValue(value);
370370
}
371-
Filter filter = Filter.create(fieldPath.getInternalPath(), op, fieldValue);
371+
Filter filter = FieldFilter.create(fieldPath.getInternalPath(), op, fieldValue);
372372
validateNewFilter(filter);
373373
return new Query(query.filter(filter), firestore);
374374
}
@@ -465,15 +465,15 @@ private void validateOrderByFieldMatchesInequality(
465465
}
466466

467467
private void validateNewFilter(Filter filter) {
468-
if (filter instanceof RelationFilter) {
469-
Operator filterOp = ((RelationFilter) filter).getOperator();
468+
if (filter instanceof FieldFilter) {
469+
FieldFilter fieldFilter = (FieldFilter) filter;
470+
Operator filterOp = fieldFilter.getOperator();
470471
List<Operator> arrayOps = Arrays.asList(Operator.ARRAY_CONTAINS, Operator.ARRAY_CONTAINS_ANY);
471472
List<Operator> disjunctiveOps = Arrays.asList(Operator.ARRAY_CONTAINS_ANY, Operator.IN);
472473
boolean isArrayOp = arrayOps.contains(filterOp);
473474
boolean isDisjunctiveOp = disjunctiveOps.contains(filterOp);
474475

475-
RelationFilter relationFilter = (RelationFilter) filter;
476-
if (relationFilter.isInequality()) {
476+
if (fieldFilter.isInequality()) {
477477
com.google.firebase.firestore.model.FieldPath existingInequality = query.inequalityField();
478478
com.google.firebase.firestore.model.FieldPath newInequality = filter.getField();
479479

@@ -494,10 +494,10 @@ private void validateNewFilter(Filter filter) {
494494
// conflicts with an existing one.
495495
Operator conflictingOp = null;
496496
if (isDisjunctiveOp) {
497-
conflictingOp = this.query.findOperatorFilter(disjunctiveOps);
497+
conflictingOp = this.query.findFilterOperator(disjunctiveOps);
498498
}
499499
if (conflictingOp == null && isArrayOp) {
500-
conflictingOp = this.query.findOperatorFilter(arrayOps);
500+
conflictingOp = this.query.findFilterOperator(arrayOps);
501501
}
502502
if (conflictingOp != null) {
503503
// We special case when it's a duplicate op to give a slightly clearer error message.

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/ArrayContainsAnyFilter.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2019 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.firestore.core;
16+
17+
import com.google.firebase.firestore.model.Document;
18+
import com.google.firebase.firestore.model.FieldPath;
19+
import com.google.firebase.firestore.model.value.ArrayValue;
20+
import com.google.firebase.firestore.model.value.FieldValue;
21+
22+
/** A Filter that implements the array-contains-any operator. */
23+
public class ArrayContainsAnyFilter extends FieldFilter {
24+
ArrayContainsAnyFilter(FieldPath field, FieldValue value) {
25+
super(field, Operator.ARRAY_CONTAINS_ANY, value);
26+
}
27+
28+
@Override
29+
public boolean matches(Document doc) {
30+
ArrayValue arrayValue = (ArrayValue) getValue();
31+
FieldValue other = doc.getField(getField());
32+
if (!(other instanceof ArrayValue)) {
33+
return false;
34+
}
35+
for (FieldValue val : ((ArrayValue) other).getInternalValue()) {
36+
if (arrayValue.getInternalValue().contains(val)) {
37+
return true;
38+
}
39+
}
40+
return false;
41+
}
42+
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/ArrayContainsFilter.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2019 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.firestore.core;
16+
17+
import com.google.firebase.firestore.model.Document;
18+
import com.google.firebase.firestore.model.FieldPath;
19+
import com.google.firebase.firestore.model.value.ArrayValue;
20+
import com.google.firebase.firestore.model.value.FieldValue;
21+
22+
/** A Filter that implements the array-contains operator. */
23+
public class ArrayContainsFilter extends FieldFilter {
24+
ArrayContainsFilter(FieldPath field, FieldValue value) {
25+
super(field, Operator.ARRAY_CONTAINS, value);
26+
}
27+
28+
@Override
29+
public boolean matches(Document doc) {
30+
FieldValue other = doc.getField(getField());
31+
return other instanceof ArrayValue
32+
&& ((ArrayValue) other).getInternalValue().contains(getValue());
33+
}
34+
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/RelationFilter.java renamed to ‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/FieldFilter.java

Lines changed: 62 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,17 @@
1717
import static com.google.firebase.firestore.util.Assert.hardAssert;
1818

1919
import com.google.firebase.firestore.model.Document;
20-
import com.google.firebase.firestore.model.DocumentKey;
2120
import com.google.firebase.firestore.model.FieldPath;
2221
import com.google.firebase.firestore.model.value.ArrayValue;
22+
import com.google.firebase.firestore.model.value.DoubleValue;
2323
import com.google.firebase.firestore.model.value.FieldValue;
24+
import com.google.firebase.firestore.model.value.NullValue;
25+
import com.google.firebase.firestore.model.value.ReferenceValue;
2426
import com.google.firebase.firestore.util.Assert;
2527
import java.util.Arrays;
2628

2729
/** Represents a filter to be applied to query. */
28-
public class RelationFilter extends Filter {
30+
public class FieldFilter extends Filter {
2931
private final Operator operator;
3032

3133
private final FieldValue value;
@@ -36,7 +38,7 @@ public class RelationFilter extends Filter {
3638
* Creates a new filter that compares fields and values. Only intended to be called from
3739
* Filter.create().
3840
*/
39-
RelationFilter(FieldPath field, Operator operator, FieldValue value) {
41+
protected FieldFilter(FieldPath field, Operator operator, FieldValue value) {
4042
this.field = field;
4143
this.operator = operator;
4244
this.value = value;
@@ -55,50 +57,67 @@ public FieldValue getValue() {
5557
return value;
5658
}
5759

58-
@Override
59-
public boolean matches(Document doc) {
60-
if (this.field.isKeyField()) {
61-
Object refValue = value.value();
62-
hardAssert(
63-
refValue instanceof DocumentKey, "Comparing on key, but filter value not a DocumentKey");
64-
hardAssert(
65-
operator != Operator.ARRAY_CONTAINS
66-
&& operator != Operator.ARRAY_CONTAINS_ANY
67-
&& operator != Operator.IN,
68-
"'" + operator.toString() + "' queries don't make sense on document keys.");
69-
int comparison = DocumentKey.comparator().compare(doc.getKey(), (DocumentKey) refValue);
70-
return matchesComparison(comparison);
71-
} else {
72-
FieldValue value = doc.getField(field);
73-
return value != null && matchesValue(doc.getField(field));
74-
}
75-
}
76-
77-
private boolean matchesValue(FieldValue other) {
78-
if (operator == Operator.ARRAY_CONTAINS) {
79-
return other instanceof ArrayValue && ((ArrayValue) other).getInternalValue().contains(value);
60+
/**
61+
* Gets a Filter instance for the provided path, operator, and value.
62+
*
63+
* <p>Note that if the relation operator is EQUAL and the value is null or NaN, this will return
64+
* the appropriate NullFilter or NaNFilter class instead of a FieldFilter.
65+
*/
66+
public static FieldFilter create(FieldPath path, Operator operator, FieldValue value) {
67+
if (path.isKeyField()) {
68+
if (operator == Operator.IN) {
69+
hardAssert(
70+
value instanceof ArrayValue,
71+
"Comparing on key with IN, but an array value was not a RefValue");
72+
return new KeyFieldInFilter(path, (ArrayValue) value);
73+
} else {
74+
hardAssert(
75+
value instanceof ReferenceValue,
76+
"Comparing on key, but filter value not a ReferenceValue");
77+
hardAssert(
78+
operator != Operator.ARRAY_CONTAINS && operator != Operator.ARRAY_CONTAINS_ANY,
79+
operator.toString() + "queries don't make sense on document keys");
80+
return new KeyFieldFilter(path, operator, (ReferenceValue) value);
81+
}
82+
} else if (value.equals(NullValue.nullValue())) {
83+
if (operator != Filter.Operator.EQUAL) {
84+
throw new IllegalArgumentException(
85+
"Invalid Query. You can only perform equality comparisons on null (via "
86+
+ "whereEqualTo()).");
87+
}
88+
return new FieldFilter(path, operator, value);
89+
} else if (value.equals(DoubleValue.NaN)) {
90+
if (operator != Filter.Operator.EQUAL) {
91+
throw new IllegalArgumentException(
92+
"Invalid Query. You can only perform equality comparisons on NaN (via "
93+
+ "whereEqualTo()).");
94+
}
95+
return new FieldFilter(path, operator, value);
96+
} else if (operator == Operator.ARRAY_CONTAINS) {
97+
return new ArrayContainsFilter(path, value);
8098
} else if (operator == Operator.IN) {
81-
hardAssert(value instanceof ArrayValue, "'in' filter has invalid value: " + value);
82-
return ((ArrayValue) value).getInternalValue().contains(other);
99+
hardAssert(value instanceof ArrayValue, "IN filter has invalid value: " + value.toString());
100+
return new InFilter(path, (ArrayValue) value);
83101
} else if (operator == Operator.ARRAY_CONTAINS_ANY) {
84102
hardAssert(
85-
value instanceof ArrayValue, "'array_contains_any' filter has invalid value: " + value);
86-
if (other instanceof ArrayValue) {
87-
for (FieldValue val : ((ArrayValue) other).getInternalValue()) {
88-
if (((ArrayValue) value).getInternalValue().contains(val)) {
89-
return true;
90-
}
91-
}
92-
}
93-
return false;
103+
value instanceof ArrayValue,
104+
"ARRAY_CONTAINS_ANY filter has invalid value: " + value.toString());
105+
return new ArrayContainsAnyFilter(path, (ArrayValue) value);
94106
} else {
95-
// Only compare types with matching backend order (such as double and int).
96-
return value.typeOrder() == other.typeOrder()
97-
&& matchesComparison(other.compareTo(this.value));
107+
return new FieldFilter(path, operator, value);
98108
}
99109
}
100110

101-
private boolean matchesComparison(int comp) {
111+
@Override
112+
public boolean matches(Document doc) {
113+
FieldValue other = doc.getField(field);
114+
// Only compare types with matching backend order (such as double and int).
115+
return other != null
116+
&& value.typeOrder() == other.typeOrder()
117+
&& this.matchesComparison(other.compareTo(value));
118+
}
119+
120+
protected boolean matchesComparison(int comp) {
102121
switch (operator) {
103122
case LESS_THAN:
104123
return comp < 0;
@@ -111,7 +130,7 @@ private boolean matchesComparison(int comp) {
111130
case GREATER_THAN_OR_EQUAL:
112131
return comp >= 0;
113132
default:
114-
throw Assert.fail("Unknown operator: %s", operator);
133+
throw Assert.fail("Unknown FieldFilter operator: %s", operator);
115134
}
116135
}
117136

@@ -138,10 +157,10 @@ public String toString() {
138157

139158
@Override
140159
public boolean equals(Object o) {
141-
if (o == null || !(o instanceof RelationFilter)) {
160+
if (o == null || !(o instanceof FieldFilter)) {
142161
return false;
143162
}
144-
RelationFilter other = (RelationFilter) o;
163+
FieldFilter other = (FieldFilter) o;
145164
return operator == other.operator && field.equals(other.field) && value.equals(other.value);
146165
}
147166

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/Filter.java

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616

1717
import com.google.firebase.firestore.model.Document;
1818
import com.google.firebase.firestore.model.FieldPath;
19-
import com.google.firebase.firestore.model.value.DoubleValue;
20-
import com.google.firebase.firestore.model.value.FieldValue;
21-
import com.google.firebase.firestore.model.value.NullValue;
2219

2320
/** Interface used for all query filters. */
2421
public abstract class Filter {
@@ -44,32 +41,6 @@ public String toString() {
4441
}
4542
}
4643

47-
/**
48-
* Gets a Filter instance for the provided path, operator, and value.
49-
*
50-
* <p>Note that if the relation operator is EQUAL and the value is null or NaN, this will return
51-
* the appropriate NullFilter or NaNFilter class instead of a RelationFilter.
52-
*/
53-
public static Filter create(FieldPath path, Operator operator, FieldValue value) {
54-
if (value.equals(NullValue.nullValue())) {
55-
if (operator != Filter.Operator.EQUAL) {
56-
throw new IllegalArgumentException(
57-
"Invalid Query. You can only perform equality comparisons on null (via "
58-
+ "whereEqualTo()).");
59-
}
60-
return new NullFilter(path);
61-
} else if (value.equals(DoubleValue.NaN)) {
62-
if (operator != Filter.Operator.EQUAL) {
63-
throw new IllegalArgumentException(
64-
"Invalid Query. You can only perform equality comparisons on NaN (via "
65-
+ "whereEqualTo()).");
66-
}
67-
return new NaNFilter(path);
68-
} else {
69-
return new RelationFilter(path, operator, value);
70-
}
71-
}
72-
7344
/** Returns the field the Filter operates over. */
7445
public abstract FieldPath getField();
7546

0 commit comments

Comments
 (0)