Skip to content

Commit 69e74fa

Browse files
ehsannasehsannas
committed
Address comments.
1 parent b61853f commit 69e74fa

File tree

4 files changed

+110
-108
lines changed

4 files changed

+110
-108
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,20 @@ private Filter(@NonNull FieldPath field, FieldFilter.Operator operator, Object v
3232
this.value = value;
3333
}
3434

35+
/** @hide */
36+
@RestrictTo(RestrictTo.Scope.LIBRARY)
3537
public FieldPath getField() {
3638
return field;
3739
}
3840

41+
/** @hide */
42+
@RestrictTo(RestrictTo.Scope.LIBRARY)
3943
public FieldFilter.Operator getOperator() {
4044
return operator;
4145
}
4246

47+
/** @hide */
48+
@RestrictTo(RestrictTo.Scope.LIBRARY)
4349
public Object getValue() {
4450
return value;
4551
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,7 @@ private FieldFilter parseFieldFilter(@NonNull FieldPath fieldPath, Operator op,
430430
return filter;
431431
}
432432

433+
// TODO(ehsann): This method will become public API. Change visibility and add documentation.
433434
private Query where(Filter filter) {
434435
return new Query(
435436
query.filter(parseFieldFilter(filter.getField(), filter.getOperator(), filter.getValue())),

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@
1414

1515
package com.google.firebase.firestore.core;
1616

17-
import androidx.annotation.NonNull;
1817
import com.google.firebase.firestore.model.Document;
1918

2019
public abstract class Filter {
2120
/** Returns true if a document matches the filter. */
22-
public abstract boolean matches(@NonNull Document doc);
21+
public abstract boolean matches(Document doc);
2322

2423
/** A unique ID identifying the filter; used when serializing queries. */
2524
public abstract String getCanonicalId();

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

Lines changed: 102 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import androidx.annotation.Nullable;
2121
import com.google.firebase.firestore.model.DocumentKey;
2222
import com.google.firebase.firestore.model.FieldIndex;
23+
import com.google.firebase.firestore.model.FieldPath;
2324
import com.google.firebase.firestore.model.ResourcePath;
2425
import com.google.firebase.firestore.model.Values;
2526
import com.google.firestore.v1.ArrayValue;
@@ -115,6 +116,17 @@ public boolean hasLimit() {
115116
return endAt;
116117
}
117118

119+
/** Returns the field filters from the given list that target the given field path. */
120+
private List<FieldFilter> getFieldFiltersForPath(List<Filter> filters, FieldPath path) {
121+
List<FieldFilter> result = new ArrayList<>();
122+
for (Filter filter : filters) {
123+
if ((filter instanceof FieldFilter) && (((FieldFilter) filter).getField()).equals(path)) {
124+
result.add((FieldFilter) filter);
125+
}
126+
}
127+
return result;
128+
}
129+
118130
/**
119131
* Returns the values that are used in ARRAY_CONTAINS or ARRAY_CONTAINS_ANY filters. Returns
120132
* {@code null} if there are no such filters.
@@ -123,16 +135,12 @@ public boolean hasLimit() {
123135
@Nullable FieldIndex.Segment segment = fieldIndex.getArraySegment();
124136
if (segment == null) return null;
125137

126-
for (Filter filter : filters) {
127-
if ((filter instanceof FieldFilter)
128-
&& (((FieldFilter) filter).getField()).equals(segment.getFieldPath())) {
129-
FieldFilter fieldFilter = (FieldFilter) filter;
130-
switch (fieldFilter.getOperator()) {
131-
case ARRAY_CONTAINS_ANY:
132-
return fieldFilter.getValue().getArrayValue().getValuesList();
133-
case ARRAY_CONTAINS:
134-
return Collections.singletonList(fieldFilter.getValue());
135-
}
138+
for (FieldFilter fieldFilter : getFieldFiltersForPath(filters, segment.getFieldPath())) {
139+
switch (fieldFilter.getOperator()) {
140+
case ARRAY_CONTAINS_ANY:
141+
return fieldFilter.getValue().getArrayValue().getValuesList();
142+
case ARRAY_CONTAINS:
143+
return Collections.singletonList(fieldFilter.getValue());
136144
}
137145
}
138146

@@ -147,23 +155,19 @@ public boolean hasLimit() {
147155
List<Value> values = new ArrayList<>();
148156

149157
for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
150-
for (Filter filter : filters) {
151-
if ((filter instanceof FieldFilter)
152-
&& ((FieldFilter) filter).getField().equals(segment.getFieldPath())) {
153-
FieldFilter fieldFilter = (FieldFilter) filter;
154-
switch (fieldFilter.getOperator()) {
155-
case EQUAL:
156-
case IN:
157-
// Encode equality prefix, which is encoded in the index value before the inequality
158-
// (e.g. `a == 'a' && b != 'b' is encoded to 'value != ab').
159-
values.add(fieldFilter.getValue());
160-
break;
161-
case NOT_IN:
162-
case NOT_EQUAL:
163-
// NotIn/NotEqual is always a suffix
164-
values.add(fieldFilter.getValue());
165-
return values;
166-
}
158+
for (FieldFilter fieldFilter : getFieldFiltersForPath(filters, segment.getFieldPath())) {
159+
switch (fieldFilter.getOperator()) {
160+
case EQUAL:
161+
case IN:
162+
// Encode equality prefix, which is encoded in the index value before the inequality
163+
// (e.g. `a == 'a' && b != 'b' is encoded to 'value != ab').
164+
values.add(fieldFilter.getValue());
165+
break;
166+
case NOT_IN:
167+
case NOT_EQUAL:
168+
// NotIn/NotEqual is always a suffix
169+
values.add(fieldFilter.getValue());
170+
return values;
167171
}
168172
}
169173
}
@@ -186,47 +190,43 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
186190
boolean segmentInclusive = true;
187191

188192
// Process all filters to find a value for the current field segment
189-
for (Filter filter : filters) {
190-
if ((filter instanceof FieldFilter)
191-
&& ((FieldFilter) filter).getField().equals(segment.getFieldPath())) {
192-
FieldFilter fieldFilter = (FieldFilter) filter;
193-
Value filterValue = null;
194-
boolean filterInclusive = true;
195-
196-
switch (fieldFilter.getOperator()) {
197-
case LESS_THAN:
198-
case LESS_THAN_OR_EQUAL:
199-
filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
200-
break;
201-
case EQUAL:
202-
case IN:
203-
case GREATER_THAN_OR_EQUAL:
204-
filterValue = fieldFilter.getValue();
205-
break;
206-
case GREATER_THAN:
207-
filterValue = fieldFilter.getValue();
208-
filterInclusive = false;
209-
break;
210-
case NOT_EQUAL:
211-
filterValue = Values.MIN_VALUE;
212-
break;
213-
case NOT_IN:
214-
{
215-
ArrayValue.Builder arrayValue = ArrayValue.newBuilder();
216-
for (int i = 0; i < fieldFilter.getValue().getArrayValue().getValuesCount(); ++i) {
217-
arrayValue.addValues(Values.MIN_VALUE);
218-
}
219-
filterValue = Value.newBuilder().setArrayValue(arrayValue).build();
220-
break;
193+
for (FieldFilter fieldFilter : getFieldFiltersForPath(filters, segment.getFieldPath())) {
194+
Value filterValue = null;
195+
boolean filterInclusive = true;
196+
197+
switch (fieldFilter.getOperator()) {
198+
case LESS_THAN:
199+
case LESS_THAN_OR_EQUAL:
200+
filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
201+
break;
202+
case EQUAL:
203+
case IN:
204+
case GREATER_THAN_OR_EQUAL:
205+
filterValue = fieldFilter.getValue();
206+
break;
207+
case GREATER_THAN:
208+
filterValue = fieldFilter.getValue();
209+
filterInclusive = false;
210+
break;
211+
case NOT_EQUAL:
212+
filterValue = Values.MIN_VALUE;
213+
break;
214+
case NOT_IN:
215+
{
216+
ArrayValue.Builder arrayValue = ArrayValue.newBuilder();
217+
for (int i = 0; i < fieldFilter.getValue().getArrayValue().getValuesCount(); ++i) {
218+
arrayValue.addValues(Values.MIN_VALUE);
221219
}
222-
default:
223-
// Remaining filters cannot be used as lower bounds.
224-
}
220+
filterValue = Value.newBuilder().setArrayValue(arrayValue).build();
221+
break;
222+
}
223+
default:
224+
// Remaining filters cannot be used as lower bounds.
225+
}
225226

226-
if (max(segmentValue, filterValue) == filterValue) {
227-
segmentValue = filterValue;
228-
segmentInclusive = filterInclusive;
229-
}
227+
if (max(segmentValue, filterValue) == filterValue) {
228+
segmentValue = filterValue;
229+
segmentInclusive = filterInclusive;
230230
}
231231
}
232232

@@ -272,48 +272,44 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
272272
boolean segmentInclusive = true;
273273

274274
// Process all filters to find a value for the current field segment
275-
for (Filter filter : filters) {
276-
if ((filter instanceof FieldFilter)
277-
&& ((FieldFilter) filter).getField().equals(segment.getFieldPath())) {
278-
FieldFilter fieldFilter = (FieldFilter) filter;
279-
Value filterValue = null;
280-
boolean filterInclusive = true;
281-
282-
switch (fieldFilter.getOperator()) {
283-
case GREATER_THAN_OR_EQUAL:
284-
case GREATER_THAN:
285-
filterValue = Values.getUpperBound(fieldFilter.getValue().getValueTypeCase());
286-
filterInclusive = false;
287-
break;
288-
case EQUAL:
289-
case IN:
290-
case LESS_THAN_OR_EQUAL:
291-
filterValue = fieldFilter.getValue();
292-
break;
293-
case LESS_THAN:
294-
filterValue = fieldFilter.getValue();
295-
filterInclusive = false;
296-
break;
297-
case NOT_EQUAL:
298-
filterValue = Values.MAX_VALUE;
299-
break;
300-
case NOT_IN:
301-
{
302-
ArrayValue.Builder arrayValue = ArrayValue.newBuilder();
303-
for (int i = 0; i < fieldFilter.getValue().getArrayValue().getValuesCount(); ++i) {
304-
arrayValue.addValues(Values.MAX_VALUE);
305-
}
306-
filterValue = Value.newBuilder().setArrayValue(arrayValue).build();
307-
break;
275+
for (FieldFilter fieldFilter : getFieldFiltersForPath(filters, segment.getFieldPath())) {
276+
Value filterValue = null;
277+
boolean filterInclusive = true;
278+
279+
switch (fieldFilter.getOperator()) {
280+
case GREATER_THAN_OR_EQUAL:
281+
case GREATER_THAN:
282+
filterValue = Values.getUpperBound(fieldFilter.getValue().getValueTypeCase());
283+
filterInclusive = false;
284+
break;
285+
case EQUAL:
286+
case IN:
287+
case LESS_THAN_OR_EQUAL:
288+
filterValue = fieldFilter.getValue();
289+
break;
290+
case LESS_THAN:
291+
filterValue = fieldFilter.getValue();
292+
filterInclusive = false;
293+
break;
294+
case NOT_EQUAL:
295+
filterValue = Values.MAX_VALUE;
296+
break;
297+
case NOT_IN:
298+
{
299+
ArrayValue.Builder arrayValue = ArrayValue.newBuilder();
300+
for (int i = 0; i < fieldFilter.getValue().getArrayValue().getValuesCount(); ++i) {
301+
arrayValue.addValues(Values.MAX_VALUE);
308302
}
309-
default:
310-
// Remaining filters cannot be used as upper bounds.
311-
}
303+
filterValue = Value.newBuilder().setArrayValue(arrayValue).build();
304+
break;
305+
}
306+
default:
307+
// Remaining filters cannot be used as upper bounds.
308+
}
312309

313-
if (min(segmentValue, filterValue) == filterValue) {
314-
segmentValue = filterValue;
315-
segmentInclusive = filterInclusive;
316-
}
310+
if (min(segmentValue, filterValue) == filterValue) {
311+
segmentValue = filterValue;
312+
segmentInclusive = filterInclusive;
317313
}
318314
}
319315

0 commit comments

Comments
 (0)