Skip to content

Commit e823b63

Browse files
tom-andersentom-andersen
committed
Merge branch 'tomandersen/pipelines' into tomandersen/pipelines-evaluate
# Conflicts: # firebase-firestore/api.txt # firebase-firestore/src/main/java/com/google/firebase/firestore/Pipeline.kt # firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.kt # firebase-firestore/src/main/java/com/google/firebase/firestore/pipeline/expressions.kt # firebase-firestore/src/main/java/com/google/firebase/firestore/pipeline/stage.kt
2 parents 9ea166b + 9f9f95a commit e823b63

File tree

12 files changed

+229
-143
lines changed

12 files changed

+229
-143
lines changed

‎firebase-firestore/api.txt‎

Lines changed: 48 additions & 46 deletions
Large diffs are not rendered by default.

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
import com.google.firebase.firestore.pipeline.AggregateFunction;
4848
import com.google.firebase.firestore.pipeline.AggregateStage;
4949
import com.google.firebase.firestore.pipeline.Expr;
50-
import com.google.firebase.firestore.pipeline.GenericStage;
50+
import com.google.firebase.firestore.pipeline.Stage;
5151
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
5252
import java.util.Collections;
5353
import java.util.LinkedHashMap;
@@ -279,15 +279,15 @@ public void groupAndAccumulateResultsGeneric() {
279279
firestore
280280
.pipeline()
281281
.collection(randomCol)
282-
.genericStage(GenericStage.ofName("where").withArguments(lt(field("published"), 1984)))
283-
.genericStage(
284-
GenericStage.ofName("aggregate")
282+
.addStage(Stage.ofName("where").withArguments(lt(field("published"), 1984)))
283+
.addStage(
284+
Stage.ofName("aggregate")
285285
.withArguments(
286286
ImmutableMap.of("avgRating", AggregateFunction.avg("rating")),
287287
ImmutableMap.of("genre", field("genre"))))
288-
.genericStage(GenericStage.ofName("where").withArguments(gt("avgRating", 4.3)))
289-
.genericStage(
290-
GenericStage.ofName("sort").withArguments(field("avgRating").descending()))
288+
.addStage(Stage.ofName("where").withArguments(gt("avgRating", 4.3)))
289+
.addStage(
290+
Stage.ofName("sort").withArguments(field("avgRating").descending()))
291291
.execute();
292292
assertThat(waitFor(execute).getResults())
293293
.comparingElementsUsing(DATA_CORRESPONDENCE)

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

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ public void testQueriesCanUseNotEqualFilters() {
356356
expectedDocsMap.remove("i");
357357
expectedDocsMap.remove("j");
358358
snapshot =
359-
waitFor(db.pipeline().convertFrom(collection.whereEqualTo("zip", Double.NaN)).execute());
359+
waitFor(db.pipeline().convertFrom(collection.whereNotEqualTo("zip", Double.NaN)).execute());
360360
assertEquals(Lists.newArrayList(expectedDocsMap.values()), pipelineSnapshotToValues(snapshot));
361361
}
362362

@@ -613,53 +613,41 @@ public void testQueriesCanUseArrayContainsAnyFilters() {
613613
FirebaseFirestore db = collection.firestore;
614614

615615
// Search for "array" to contain [42, 43].
616-
PipelineSnapshot snapshot =
617-
waitFor(
618-
db.pipeline()
619-
.convertFrom(collection.whereArrayContainsAny("array", asList(42L, 43L)))
620-
.execute());
616+
Pipeline pipeline = db.pipeline()
617+
.convertFrom(collection.whereArrayContainsAny("array", asList(42L, 43L)));
618+
PipelineSnapshot snapshot = waitFor(pipeline.execute());
621619
assertEquals(asList(docA, docB, docD, docE), pipelineSnapshotToValues(snapshot));
622620

623621
// With objects.
624-
snapshot =
625-
waitFor(
626-
db.pipeline()
627-
.convertFrom(collection.whereArrayContainsAny("array", asList(map("a", 42L))))
628-
.execute());
622+
pipeline = db.pipeline()
623+
.convertFrom(collection.whereArrayContainsAny("array", asList(map("a", 42L))));
624+
snapshot = waitFor(pipeline.execute());
629625
assertEquals(asList(docF), pipelineSnapshotToValues(snapshot));
630626

631627
// With null.
632-
snapshot =
633-
waitFor(
634-
db.pipeline()
635-
.convertFrom(collection.whereArrayContainsAny("array", nullList()))
636-
.execute());
628+
pipeline = db.pipeline()
629+
.convertFrom(collection.whereArrayContainsAny("array", nullList()));
630+
snapshot = waitFor(pipeline.execute());
637631
assertEquals(new ArrayList<>(), pipelineSnapshotToValues(snapshot));
638632

639633
// With null and a value.
640634
List<Object> inputList = nullList();
641635
inputList.add(43L);
642-
snapshot =
643-
waitFor(
644-
db.pipeline()
645-
.convertFrom(collection.whereArrayContainsAny("array", inputList))
646-
.execute());
636+
pipeline = db.pipeline()
637+
.convertFrom(collection.whereArrayContainsAny("array", inputList));
638+
snapshot = waitFor(pipeline.execute());
647639
assertEquals(asList(docE), pipelineSnapshotToValues(snapshot));
648640

649641
// With NaN.
650-
snapshot =
651-
waitFor(
652-
db.pipeline()
653-
.convertFrom(collection.whereArrayContainsAny("array", asList(Double.NaN)))
654-
.execute());
642+
pipeline = db.pipeline()
643+
.convertFrom(collection.whereArrayContainsAny("array", asList(Double.NaN)));
644+
snapshot = waitFor(pipeline.execute());
655645
assertEquals(new ArrayList<>(), pipelineSnapshotToValues(snapshot));
656646

657647
// With NaN and a value.
658-
snapshot =
659-
waitFor(
660-
db.pipeline()
661-
.convertFrom(collection.whereArrayContainsAny("array", asList(Double.NaN, 43L)))
662-
.execute());
648+
pipeline = db.pipeline()
649+
.convertFrom(collection.whereArrayContainsAny("array", asList(Double.NaN, 43L)));
650+
snapshot = waitFor(pipeline.execute());
663651
assertEquals(asList(docE), pipelineSnapshotToValues(snapshot));
664652
}
665653

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -567,9 +567,18 @@ public static void checkOnlineAndOfflineResultsMatch(Query query, String... expe
567567
* @param expectedDocs Ordered list of document keys that are expected to match the query
568568
*/
569569
public static void checkQueryAndPipelineResultsMatch(Query query, String... expectedDocs) {
570-
QuerySnapshot docsFromQuery = waitFor(query.get(Source.SERVER));
571-
PipelineSnapshot docsFromPipeline =
572-
waitFor(query.getFirestore().pipeline().convertFrom(query).execute());
570+
QuerySnapshot docsFromQuery;
571+
try {
572+
docsFromQuery = waitFor(query.get(Source.SERVER));
573+
} catch (Exception e) {
574+
throw new RuntimeException("Classic Query FAILED", e);
575+
}
576+
PipelineSnapshot docsFromPipeline;
577+
try {
578+
docsFromPipeline = waitFor(query.getFirestore().pipeline().convertFrom(query).execute());
579+
} catch (Exception e) {
580+
throw new RuntimeException("Pipeline FAILED", e);
581+
}
573582

574583
assertEquals(querySnapshotToIds(docsFromQuery), pipelineSnapshotToIds(docsFromPipeline));
575584
List<String> expected = asList(expectedDocs);

‎firebase-firestore/src/main/java/com/google/firebase/firestore/Pipeline.kt‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ import com.google.firebase.firestore.pipeline.ExprWithAlias
3737
import com.google.firebase.firestore.pipeline.Field
3838
import com.google.firebase.firestore.pipeline.FindNearestStage
3939
import com.google.firebase.firestore.pipeline.FunctionExpr
40-
import com.google.firebase.firestore.pipeline.GenericStage
4140
import com.google.firebase.firestore.pipeline.InternalOptions
41+
import com.google.firebase.firestore.pipeline.Stage
4242
import com.google.firebase.firestore.pipeline.LimitStage
4343
import com.google.firebase.firestore.pipeline.OffsetStage
4444
import com.google.firebase.firestore.pipeline.Ordering
@@ -50,7 +50,7 @@ import com.google.firebase.firestore.pipeline.SampleStage
5050
import com.google.firebase.firestore.pipeline.SelectStage
5151
import com.google.firebase.firestore.pipeline.Selectable
5252
import com.google.firebase.firestore.pipeline.SortStage
53-
import com.google.firebase.firestore.pipeline.Stage
53+
import com.google.firebase.firestore.pipeline.BaseStage
5454
import com.google.firebase.firestore.pipeline.UnionStage
5555
import com.google.firebase.firestore.pipeline.UnnestStage
5656
import com.google.firebase.firestore.pipeline.WhereStage
@@ -62,7 +62,7 @@ open class AbstractPipeline
6262
internal constructor(
6363
internal val firestore: FirebaseFirestore,
6464
internal val userDataReader: UserDataReader,
65-
internal val stages: FluentIterable<Stage<*>>
65+
internal val stages: FluentIterable<BaseStage<*>>
6666
) {
6767
private fun toStructuredPipelineProto(options: InternalOptions?): StructuredPipeline {
6868
val builder = StructuredPipeline.newBuilder()
@@ -136,10 +136,10 @@ private constructor(
136136
internal constructor(
137137
firestore: FirebaseFirestore,
138138
userDataReader: UserDataReader,
139-
stage: Stage<*>
139+
stage: BaseStage<*>
140140
) : this(firestore, userDataReader, FluentIterable.of(stage))
141141

142-
private fun append(stage: Stage<*>): Pipeline {
142+
private fun append(stage: BaseStage<*>): Pipeline {
143143
return Pipeline(firestore, userDataReader, stages.append(stage))
144144
}
145145

@@ -159,10 +159,10 @@ private constructor(
159159
* This method provides a way to call stages that are supported by the Firestore backend but that
160160
* are not implemented in the SDK version being used.
161161
*
162-
* @param stage An [GenericStage] object that specifies stage name and parameters.
162+
* @param stage An [Stage] object that specifies stage name and parameters.
163163
* @return A new [Pipeline] object with this stage appended to the stage list.
164164
*/
165-
fun genericStage(stage: GenericStage): Pipeline = append(stage)
165+
fun addStage(stage: Stage): Pipeline = append(stage)
166166

167167
/**
168168
* Adds new fields to outputs from previous stages.

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

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,23 @@
1414

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

17+
import static com.google.firebase.firestore.model.Values.isNanValue;
1718
import static com.google.firebase.firestore.pipeline.Expr.and;
19+
import static com.google.firebase.firestore.pipeline.Expr.ifError;
1820
import static com.google.firebase.firestore.util.Assert.hardAssert;
19-
import static java.lang.Double.isNaN;
21+
22+
import androidx.annotation.NonNull;
2023

2124
import com.google.firebase.firestore.model.Document;
2225
import com.google.firebase.firestore.model.FieldPath;
2326
import com.google.firebase.firestore.model.Values;
2427
import com.google.firebase.firestore.pipeline.BooleanExpr;
28+
import com.google.firebase.firestore.pipeline.Expr;
2529
import com.google.firebase.firestore.pipeline.Field;
2630
import com.google.firebase.firestore.util.Assert;
2731
import com.google.firestore.v1.Value;
32+
33+
import java.util.ArrayList;
2834
import java.util.Arrays;
2935
import java.util.Collections;
3036
import java.util.List;
@@ -188,16 +194,18 @@ BooleanExpr toPipelineExpr() {
188194
case EQUAL:
189195
if (value.hasNullValue()) {
190196
return and(exists, x.isNull());
191-
} else if (value.hasDoubleValue() && isNaN(value.getDoubleValue())) {
192-
return and(exists, x.isNan());
197+
} else if (isNanValue(value)) {
198+
// The isNan will error on non-numeric values.
199+
return and(exists, ifError(x.isNan(), Expr.constant(false)));
193200
} else {
194201
return and(exists, x.eq(value));
195202
}
196203
case NOT_EQUAL:
197204
if (value.hasNullValue()) {
198205
return and(exists, x.isNotNull());
199-
} else if (value.hasDoubleValue() && isNaN(value.getDoubleValue())) {
200-
return and(exists, x.isNotNan());
206+
} else if (isNanValue(value)) {
207+
// The isNotNan will error on non-numeric values.
208+
return and(exists, ifError(x.isNotNan(), Expr.constant(true)));
201209
} else {
202210
return and(exists, x.neq(value));
203211
}
@@ -211,14 +219,39 @@ BooleanExpr toPipelineExpr() {
211219
return and(exists, x.arrayContainsAny(value.getArrayValue().getValuesList()));
212220
case IN:
213221
return and(exists, x.eqAny(value.getArrayValue().getValuesList()));
214-
case NOT_IN:
215-
return and(exists, x.notEqAny(value.getArrayValue().getValuesList()));
222+
case NOT_IN: {
223+
List<Value> list = value.getArrayValue().getValuesList();
224+
if (hasNaN(list)) {
225+
return and(exists, x.notEqAny(filterNaN(list)), ifError(x.isNotNan(), Expr.constant(true)));
226+
} else {
227+
return and(exists, x.notEqAny(list));
228+
}
229+
}
216230
default:
217231
// Handle OPERATOR_UNSPECIFIED and UNRECOGNIZED cases as needed
218232
throw new IllegalArgumentException("Unsupported operator: " + operator);
219233
}
220234
}
221235

236+
private static boolean hasNaN(List<Value> list) {
237+
for (Value v : list) {
238+
if (isNanValue(v)) {
239+
return true;
240+
}
241+
}
242+
return false;
243+
}
244+
245+
@NonNull
246+
private static List<Value> filterNaN(List<Value> list) {
247+
List<Value> listWithoutNan = new ArrayList<>(list.size() - 1);
248+
for (Value v : list) {
249+
if (isNanValue(v)) continue;
250+
listWithoutNan.add(v);
251+
}
252+
return listWithoutNan;
253+
}
254+
222255
@Override
223256
public String toString() {
224257
return getCanonicalId();

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

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@
3737
import com.google.firebase.firestore.pipeline.FunctionExpr;
3838
import com.google.firebase.firestore.pipeline.InternalOptions;
3939
import com.google.firebase.firestore.pipeline.Ordering;
40-
import com.google.firebase.firestore.pipeline.Stage;
40+
import com.google.firebase.firestore.pipeline.BaseStage;
41+
import com.google.firebase.firestore.util.BiFunction;
42+
import com.google.firebase.firestore.util.Function;
43+
import com.google.firebase.firestore.util.IntFunction;
4144
import com.google.firestore.v1.Value;
4245
import java.util.ArrayList;
4346
import java.util.Collections;
@@ -46,7 +49,6 @@
4649
import java.util.List;
4750
import java.util.SortedSet;
4851
import java.util.TreeSet;
49-
import java.util.function.BiFunction;
5052

5153
/**
5254
* Encapsulates all the query attributes we support in the SDK. It can be run against the
@@ -547,8 +549,7 @@ public Pipeline toPipeline(FirebaseFirestore firestore, UserDataReader userDataR
547549
if (fields.size() == 1) {
548550
p = p.where(fields.get(0).exists());
549551
} else {
550-
BooleanExpr[] conditions =
551-
fields.stream().skip(1).map(Expr.Companion::exists).toArray(BooleanExpr[]::new);
552+
BooleanExpr[] conditions = skipFirstToArray(fields, BooleanExpr[]::new, Expr.Companion::exists);
552553
p = p.where(and(fields.get(0).exists(), conditions));
553554
}
554555

@@ -564,23 +565,43 @@ public Pipeline toPipeline(FirebaseFirestore firestore, UserDataReader userDataR
564565
if (hasLimit()) {
565566
// TODO: Handle situation where user enters limit larger than integer.
566567
if (limitType == LimitType.LIMIT_TO_FIRST) {
567-
p = p.sort(orderings.get(0), orderings.stream().skip(1).toArray(Ordering[]::new));
568+
p = p.sort(orderings.get(0), skipFirstToArray(orderings, Ordering[]::new));
568569
p = p.limit((int) limit);
569570
} else {
570571
p =
571572
p.sort(
572573
orderings.get(0).reverse(),
573-
orderings.stream().skip(1).map(Ordering::reverse).toArray(Ordering[]::new));
574+
skipFirstToArray(orderings, Ordering[]::new, Ordering::reverse));
574575
p = p.limit((int) limit);
575-
p = p.sort(orderings.get(0), orderings.stream().skip(1).toArray(Ordering[]::new));
576+
p = p.sort(orderings.get(0), skipFirstToArray(orderings, Ordering[]::new));
576577
}
577578
} else {
578-
p = p.sort(orderings.get(0), orderings.stream().skip(1).toArray(Ordering[]::new));
579+
p = p.sort(orderings.get(0), skipFirstToArray(orderings, Ordering[]::new));
579580
}
580581

581582
return p;
582583
}
583584

585+
// Many Pipelines require first parameter to be separated out from rest.
586+
private static <T> T[] skipFirstToArray(List<T> list, IntFunction<T[]> generator) {
587+
int size = list.size();
588+
T[] result = generator.apply(size - 1);
589+
for (int i = 1; i < size; i++) {
590+
result[i-1] = list.get(i);
591+
}
592+
return result;
593+
}
594+
595+
// Many Pipelines require first parameter to be separated out from rest.
596+
private static <T, R> R[] skipFirstToArray(List<T> list, IntFunction<R[]> generator, Function<T, R> map) {
597+
int size = list.size();
598+
R[] result = generator.apply(size - 1);
599+
for (int i = 1; i < size; i++) {
600+
result[i-1] = map.apply(list.get(i));
601+
}
602+
return result;
603+
}
604+
584605
private static BooleanExpr whereConditionsFromCursor(
585606
Bound bound, List<Field> fields, BiFunction<Expr, Object, BooleanExpr> cmp) {
586607
List<Value> boundPosition = bound.getPosition();
@@ -600,7 +621,7 @@ private static BooleanExpr whereConditionsFromCursor(
600621
}
601622

602623
@NonNull
603-
private Stage<?> pipelineSource(FirebaseFirestore firestore) {
624+
private BaseStage<?> pipelineSource(FirebaseFirestore firestore) {
604625
if (isDocumentQuery()) {
605626
return new DocumentsSource(path.canonicalString());
606627
} else if (isCollectionGroupQuery()) {

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.kt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -663,14 +663,14 @@ internal object Values {
663663
)
664664
}
665665

666-
@JvmStatic
667-
fun encodeValue(value: Timestamp): Value = Value.newBuilder().setTimestampValue(value).build()
668-
669-
@JvmField val TRUE_VALUE: Value = Value.newBuilder().setBooleanValue(true).build()
666+
@JvmField
667+
val TRUE: Value = Value.newBuilder().setBooleanValue(true).build()
670668

671-
@JvmField val FALSE_VALUE: Value = Value.newBuilder().setBooleanValue(false).build()
669+
@JvmField
670+
val FALSE: Value = Value.newBuilder().setBooleanValue(false).build()
672671

673-
@JvmStatic fun encodeValue(value: Boolean): Value = if (value) TRUE_VALUE else FALSE_VALUE
672+
@JvmStatic
673+
fun encodeValue(value: Boolean): Value = if (value) TRUE else FALSE
674674

675675
@JvmStatic
676676
fun encodeValue(geoPoint: GeoPoint): Value =

0 commit comments

Comments
 (0)