Skip to content

Commit 0c6acd0

Browse files
mikelehenmikelehen
authored
Collection Group Query Internals (#233)
Includes the new index and all internals for collection group queries, but the public API will not be exposed until backend support is ready.
1 parent 50f1277 commit 0c6acd0

28 files changed

+1989
-49
lines changed

‎firebase-firestore/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
- [fixed] Fixed calculation of SQLite database size on Android 9 Pie devices.
66
Previous method could be off by a few MBs on these devices, potentially
77
delaying garbage collection.
8+
- [changed] Prepared the persistence layer to support collection group queries.
9+
While this feature is not yet available, all schema changes are included
10+
in this release.
811

912
# 18.0.1
1013
- [fixed] Fixed an issue where Firestore would crash if handling write batches

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.querySnapshotToValues;
1919
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testCollection;
2020
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testCollectionWithDocs;
21+
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testFirestore;
2122
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor;
2223
import static com.google.firebase.firestore.testutil.TestUtil.map;
2324
import static java.util.Arrays.asList;
@@ -425,4 +426,119 @@ public void testQueriesCanUseArrayContainsFilters() {
425426
// NOTE: The backend doesn't currently support null, NaN, objects, or arrays, so there isn't
426427
// much of anything else interesting to test.
427428
}
429+
430+
@Test
431+
public void testCollectionGroupQueries() {
432+
FirebaseFirestore db = testFirestore();
433+
// Use .document() to get a random collection group name to use but ensure it starts with 'b'
434+
// for predictable ordering.
435+
String collectionGroup = "b" + db.collection("foo").document().getId();
436+
437+
String[] docPaths =
438+
new String[] {
439+
"abc/123/${collectionGroup}/cg-doc1",
440+
"abc/123/${collectionGroup}/cg-doc2",
441+
"${collectionGroup}/cg-doc3",
442+
"${collectionGroup}/cg-doc4",
443+
"def/456/${collectionGroup}/cg-doc5",
444+
"${collectionGroup}/virtual-doc/nested-coll/not-cg-doc",
445+
"x${collectionGroup}/not-cg-doc",
446+
"${collectionGroup}x/not-cg-doc",
447+
"abc/123/${collectionGroup}x/not-cg-doc",
448+
"abc/123/x${collectionGroup}/not-cg-doc",
449+
"abc/${collectionGroup}"
450+
};
451+
WriteBatch batch = db.batch();
452+
for (String path : docPaths) {
453+
batch.set(db.document(path.replace("${collectionGroup}", collectionGroup)), map("x", 1));
454+
}
455+
waitFor(batch.commit());
456+
457+
QuerySnapshot querySnapshot = waitFor(db.collectionGroup(collectionGroup).get());
458+
assertEquals(
459+
asList("cg-doc1", "cg-doc2", "cg-doc3", "cg-doc4", "cg-doc5"),
460+
querySnapshotToIds(querySnapshot));
461+
}
462+
463+
@Test
464+
public void testCollectionGroupQueriesWithStartAtEndAtWithArbitraryDocumentIds() {
465+
FirebaseFirestore db = testFirestore();
466+
// Use .document() to get a random collection group name to use but ensure it starts with 'b'
467+
// for predictable ordering.
468+
String collectionGroup = "b" + db.collection("foo").document().getId();
469+
470+
String[] docPaths =
471+
new String[] {
472+
"a/a/${collectionGroup}/cg-doc1",
473+
"a/b/a/b/${collectionGroup}/cg-doc2",
474+
"a/b/${collectionGroup}/cg-doc3",
475+
"a/b/c/d/${collectionGroup}/cg-doc4",
476+
"a/c/${collectionGroup}/cg-doc5",
477+
"${collectionGroup}/cg-doc6",
478+
"a/b/nope/nope"
479+
};
480+
WriteBatch batch = db.batch();
481+
for (String path : docPaths) {
482+
batch.set(db.document(path.replace("${collectionGroup}", collectionGroup)), map("x", 1));
483+
}
484+
waitFor(batch.commit());
485+
486+
QuerySnapshot querySnapshot =
487+
waitFor(
488+
db.collectionGroup(collectionGroup)
489+
.orderBy(FieldPath.documentId())
490+
.startAt("a/b")
491+
.endAt("a/b0")
492+
.get());
493+
assertEquals(asList("cg-doc2", "cg-doc3", "cg-doc4"), querySnapshotToIds(querySnapshot));
494+
495+
querySnapshot =
496+
waitFor(
497+
db.collectionGroup(collectionGroup)
498+
.orderBy(FieldPath.documentId())
499+
.startAfter("a/b")
500+
.endBefore("a/b/" + collectionGroup + "/cg-doc3")
501+
.get());
502+
assertEquals(asList("cg-doc2"), querySnapshotToIds(querySnapshot));
503+
}
504+
505+
@Test
506+
public void testCollectionGroupQueriesWithWhereFiltersOnArbitraryDocumentIds() {
507+
FirebaseFirestore db = testFirestore();
508+
// Use .document() to get a random collection group name to use but ensure it starts with 'b'
509+
// for predictable ordering.
510+
String collectionGroup = "b" + db.collection("foo").document().getId();
511+
512+
String[] docPaths =
513+
new String[] {
514+
"a/a/${collectionGroup}/cg-doc1",
515+
"a/b/a/b/${collectionGroup}/cg-doc2",
516+
"a/b/${collectionGroup}/cg-doc3",
517+
"a/b/c/d/${collectionGroup}/cg-doc4",
518+
"a/c/${collectionGroup}/cg-doc5",
519+
"${collectionGroup}/cg-doc6",
520+
"a/b/nope/nope"
521+
};
522+
WriteBatch batch = db.batch();
523+
for (String path : docPaths) {
524+
batch.set(db.document(path.replace("${collectionGroup}", collectionGroup)), map("x", 1));
525+
}
526+
waitFor(batch.commit());
527+
528+
QuerySnapshot querySnapshot =
529+
waitFor(
530+
db.collectionGroup(collectionGroup)
531+
.whereGreaterThanOrEqualTo(FieldPath.documentId(), "a/b")
532+
.whereLessThanOrEqualTo(FieldPath.documentId(), "a/b0")
533+
.get());
534+
assertEquals(asList("cg-doc2", "cg-doc3", "cg-doc4"), querySnapshotToIds(querySnapshot));
535+
536+
querySnapshot =
537+
waitFor(
538+
db.collectionGroup(collectionGroup)
539+
.whereGreaterThan(FieldPath.documentId(), "a/b")
540+
.whereLessThan(FieldPath.documentId(), "a/b/" + collectionGroup + "/cg-doc3")
541+
.get());
542+
assertEquals(asList("cg-doc2"), querySnapshotToIds(querySnapshot));
543+
}
428544
}

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -499,14 +499,21 @@ public void queriesMustNotHaveMoreComponentsThanOrderBy() {
499499

500500
@Test
501501
public void queryOrderByKeyBoundsMustBeStringsWithoutSlashes() {
502-
CollectionReference collection = testCollection();
503-
Query query = collection.orderBy(FieldPath.documentId());
502+
Query query = testFirestore().collection("collection").orderBy(FieldPath.documentId());
503+
Query cgQuery = testFirestore().collectionGroup("collection").orderBy(FieldPath.documentId());
504504
expectError(
505505
() -> query.startAt(1),
506506
"Invalid query. Expected a string for document ID in startAt(), but got 1.");
507507
expectError(
508508
() -> query.startAt("foo/bar"),
509-
"Invalid query. Document ID 'foo/bar' contains a slash in startAt().");
509+
"Invalid query. When querying a collection and ordering by "
510+
+ "FieldPath.documentId(), the value passed to startAt() must be a plain "
511+
+ "document ID, but 'foo/bar' contains a slash.");
512+
expectError(
513+
() -> cgQuery.startAt("foo"),
514+
"Invalid query. When querying a collection group and ordering by "
515+
+ "FieldPath.documentId(), the value passed to startAt() must result in a valid "
516+
+ "document path, but 'foo' is not because it contains an odd number of segments.");
510517
}
511518

512519
@Test
@@ -563,8 +570,8 @@ public void queriesFilteredByDocumentIDMustUseStringsOrDocumentReferences() {
563570
expectError(() -> collection.whereGreaterThanOrEqualTo(FieldPath.documentId(), ""), reason);
564571

565572
reason =
566-
"Invalid query. When querying with FieldPath.documentId() you must provide "
567-
+ "a valid document ID, but 'foo/bar/baz' contains a '/' character.";
573+
"Invalid query. When querying a collection by FieldPath.documentId() you must provide "
574+
+ "a plain document ID, but 'foo/bar/baz' contains a '/' character.";
568575
expectError(
569576
() -> collection.whereGreaterThanOrEqualTo(FieldPath.documentId(), "foo/bar/baz"), reason);
570577

@@ -573,6 +580,17 @@ public void queriesFilteredByDocumentIDMustUseStringsOrDocumentReferences() {
573580
+ "a valid String or DocumentReference, but it was of type: java.lang.Integer";
574581
expectError(() -> collection.whereGreaterThanOrEqualTo(FieldPath.documentId(), 1), reason);
575582

583+
reason =
584+
"Invalid query. When querying a collection group by FieldPath.documentId(), the value "
585+
+ "provided must result in a valid document path, but 'foo' is not because it has "
586+
+ "an odd number of segments (1).";
587+
expectError(
588+
() ->
589+
testFirestore()
590+
.collectionGroup("collection")
591+
.whereGreaterThanOrEqualTo(FieldPath.documentId(), "foo"),
592+
reason);
593+
576594
reason =
577595
"Invalid query. You can't perform array-contains queries on FieldPath.documentId() since "
578596
+ "document IDs are not arrays.";

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,30 @@ public DocumentReference document(@NonNull String documentPath) {
235235
return DocumentReference.forPath(ResourcePath.fromString(documentPath), this);
236236
}
237237

238+
// TODO(b/116617988): Expose API publicly once backend support is ready (and add to CHANGELOG.md).
239+
/**
240+
* Creates and returns a new @link{Query} that includes all documents in the database that are
241+
* contained in a collection or subcollection with the given @code{collectionId}.
242+
*
243+
* @param collectionId Identifies the collections to query over. Every collection or subcollection
244+
* with this ID as the last segment of its path will be included. Cannot contain a slash.
245+
* @return The created Query.
246+
*/
247+
@NonNull
248+
// @PublicApi
249+
/* public */ Query collectionGroup(@NonNull String collectionId) {
250+
checkNotNull(collectionId, "Provided collection ID must not be null.");
251+
if (collectionId.contains("/")) {
252+
throw new IllegalArgumentException(
253+
String.format(
254+
"Invalid collectionId '%s'. Collection IDs must not contain '/'.", collectionId));
255+
}
256+
257+
ensureClientConfigured();
258+
return new Query(
259+
new com.google.firebase.firestore.core.Query(ResourcePath.EMPTY, collectionId), this);
260+
}
261+
238262
/**
239263
* Executes the given updateFunction and then attempts to commit the changes applied within the
240264
* transaction. If any document read within the transaction has changed, the updateFunction will

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

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -324,20 +324,28 @@ private Query whereHelper(@NonNull FieldPath fieldPath, Operator op, Object valu
324324
}
325325
if (value instanceof String) {
326326
String documentKey = (String) value;
327-
if (documentKey.contains("/")) {
328-
// TODO: Allow slashes once ancestor queries are supported
327+
if (documentKey.isEmpty()) {
329328
throw new IllegalArgumentException(
330329
"Invalid query. When querying with FieldPath.documentId() you must provide a valid "
331-
+ "document ID, but '"
330+
+ "document ID, but it was an empty string.");
331+
}
332+
if (!query.isCollectionGroupQuery() && documentKey.contains("/")) {
333+
throw new IllegalArgumentException(
334+
"Invalid query. When querying a collection by FieldPath.documentId() you must "
335+
+ "provide a plain document ID, but '"
332336
+ documentKey
333337
+ "' contains a '/' character.");
334-
} else if (documentKey.isEmpty()) {
338+
}
339+
ResourcePath path = query.getPath().append(ResourcePath.fromString(documentKey));
340+
if (!DocumentKey.isDocumentKey(path)) {
335341
throw new IllegalArgumentException(
336-
"Invalid query. When querying with FieldPath.documentId() you must provide a valid "
337-
+ "document ID, but it was an empty string.");
342+
"Invalid query. When querying a collection group by FieldPath.documentId(), the "
343+
+ "value provided must result in a valid document path, but '"
344+
+ path
345+
+ "' is not because it has an odd number of segments ("
346+
+ path.length()
347+
+ ").");
338348
}
339-
ResourcePath path = this.query.getPath().append(documentKey);
340-
hardAssert(path.length() % 2 == 0, "Path should be a document key");
341349
fieldValue =
342350
ReferenceValue.valueOf(this.getFirestore().getDatabaseId(), DocumentKey.fromPath(path));
343351
} else if (value instanceof DocumentReference) {
@@ -655,15 +663,26 @@ private Bound boundFromFields(String methodName, Object[] values, boolean before
655663
+ ".");
656664
}
657665
String documentId = (String) rawValue;
658-
if (documentId.contains("/")) {
666+
if (!query.isCollectionGroupQuery() && documentId.contains("/")) {
659667
throw new IllegalArgumentException(
660-
"Invalid query. Document ID '"
668+
"Invalid query. When querying a collection and ordering by FieldPath.documentId(), "
669+
+ "the value passed to "
670+
+ methodName
671+
+ "() must be a plain document ID, but '"
661672
+ documentId
662-
+ "' contains a slash in "
673+
+ "' contains a slash.");
674+
}
675+
ResourcePath path = query.getPath().append(ResourcePath.fromString(documentId));
676+
if (!DocumentKey.isDocumentKey(path)) {
677+
throw new IllegalArgumentException(
678+
"Invalid query. When querying a collection group and ordering by "
679+
+ "FieldPath.documentId(), the value passed to "
663680
+ methodName
664-
+ "().");
681+
+ "() must result in a valid document path, but '"
682+
+ path
683+
+ "' is not because it contains an odd number of segments.");
665684
}
666-
DocumentKey key = DocumentKey.fromPath(query.getPath().append(documentId));
685+
DocumentKey key = DocumentKey.fromPath(path);
667686
components.add(ReferenceValue.valueOf(firestore.getDatabaseId(), key));
668687
} else {
669688
FieldValue wrapped = firestore.getDataConverter().parseQueryValue(rawValue);

0 commit comments

Comments
 (0)