Skip to content

Commit 3034b0a

Browse files
author
Michael Lehenbauer
committed
CR Feedback.
1 parent 335d84f commit 3034b0a

File tree

6 files changed

+13
-8
lines changed

6 files changed

+13
-8
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,8 @@ public DocumentReference document(@NonNull String documentPath) {
236236
}
237237

238238
/**
239-
* Creates and returns a new Query that includes all documents in the database that are contained
240-
* in a collection or subcollection with the given collectionId.
239+
* Creates and returns a new @link{Query} that includes all documents in the database that are
240+
* contained in a collection or subcollection with the given @code{collectionId}.
241241
*
242242
* @param collectionId Identifies the collections to query over. Every collection or subcollection
243243
* with this ID as the last segment of its path will be included. Cannot contain a slash.
@@ -252,6 +252,7 @@ public Query collectionGroup(@NonNull String collectionId) {
252252
String.format(
253253
"Invalid collectionId '%s'. Collection IDs must not contain '/'.", collectionId));
254254
}
255+
255256
ensureClientConfigured();
256257
return new Query(
257258
new com.google.firebase.firestore.core.Query(ResourcePath.EMPTY, collectionId), this);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,14 @@ private Query whereHelper(@NonNull FieldPath fieldPath, Operator op, Object valu
329329
"Invalid query. When querying with FieldPath.documentId() you must provide a valid "
330330
+ "document ID, but it was an empty string.");
331331
}
332-
if (!this.query.isCollectionGroupQuery() && documentKey.contains("/")) {
332+
if (!query.isCollectionGroupQuery() && documentKey.contains("/")) {
333333
throw new IllegalArgumentException(
334334
"Invalid query. When querying a collection by FieldPath.documentId() you must "
335335
+ "provide a plain document ID, but '"
336336
+ documentKey
337337
+ "' contains a '/' character.");
338338
}
339-
ResourcePath path = this.query.getPath().append(ResourcePath.fromString(documentKey));
339+
ResourcePath path = query.getPath().append(ResourcePath.fromString(documentKey));
340340
if (!DocumentKey.isDocumentKey(path)) {
341341
throw new IllegalArgumentException(
342342
"Invalid query. When querying a collection group by FieldPath.documentId(), the "
@@ -663,7 +663,7 @@ private Bound boundFromFields(String methodName, Object[] values, boolean before
663663
+ ".");
664664
}
665665
String documentId = (String) rawValue;
666-
if (!this.query.isCollectionGroupQuery() && documentId.contains("/")) {
666+
if (!query.isCollectionGroupQuery() && documentId.contains("/")) {
667667
throw new IllegalArgumentException(
668668
"Invalid query. When querying a collection and ordering by FieldPath.documentId(), "
669669
+ "the value passed to "

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public boolean isDocumentQuery() {
111111

112112
/** Returns true if this is a collection group query. */
113113
public boolean isCollectionGroupQuery() {
114-
return this.collectionGroup != null;
114+
return collectionGroup != null;
115115
}
116116

117117
/** The filters on the documents returned by the query. */

firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexManager.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ public interface IndexManager {
2828
* Creates an index entry mapping the collectionId (last segment of the path) to the parent path
2929
* (either the containing document location or the empty path for root-level collections). Index
3030
* entries can be retrieved via getCollectionParents().
31+
*
32+
* <p>NOTE: Currently we don't remove index entries. If this ends up being an issue we can devise
33+
* some sort of GC strategy.
3134
*/
3235
void addToCollectionParentIndex(ResourcePath collectionPath);
3336

firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryIndexManager.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,15 @@ public List<ResourcePath> getCollectionParents(String collectionId) {
4444

4545
/**
4646
* Internal implementation of the collection-parent index. Also used for in-memory caching by
47-
* SQLiteIndexManager and initial index population in SQLiteSchema.java
47+
* SQLiteIndexManager and initial index population in SQLiteSchema.
4848
*/
4949
static class MemoryCollectionParentIndex {
5050
private final HashMap<String, HashSet<ResourcePath>> index = new HashMap<>();
5151

5252
// Returns false if the entry already existed.
5353
boolean add(ResourcePath collectionPath) {
5454
hardAssert(collectionPath.length() % 2 == 1, "Expected a collection path.");
55+
5556
String collectionId = collectionPath.getLastSegment();
5657
ResourcePath parentPath = collectionPath.popLast();
5758
HashSet<ResourcePath> existingParents = index.get(collectionId);

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ final class SQLiteIndexManager implements IndexManager {
4141
@Override
4242
public void addToCollectionParentIndex(ResourcePath collectionPath) {
4343
hardAssert(collectionPath.length() % 2 == 1, "Expected a collection path.");
44+
4445
if (collectionParentsCache.add(collectionPath)) {
45-
hardAssert(collectionPath.length() >= 1, "Invalid collection path.");
4646
String collectionId = collectionPath.getLastSegment();
4747
ResourcePath parentPath = collectionPath.popLast();
4848
db.execute(

0 commit comments

Comments
 (0)