Skip to content

Commit 165b40b

Browse files
schmidt-sebastianschmidt-sebastian
committed
(Mostly) test cleanup
1 parent 62e4f7c commit 165b40b

File tree

4 files changed

+114
-84
lines changed

4 files changed

+114
-84
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,12 @@ public boolean isCollectionGroupQuery() {
122122
*/
123123
public boolean matchesAllDocuments() {
124124
return filters.isEmpty()
125-
&& getExplicitOrderBy().isEmpty()
126125
&& limit == NO_LIMIT
127126
&& startAt == null
128-
&& endAt == null;
127+
&& endAt == null
128+
&& (getExplicitOrderBy().isEmpty()
129+
|| (getExplicitOrderBy().size() == 1
130+
&& getFirstOrderByField().isKeyField()));
129131
}
130132

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

‎firebase-firestore/src/test/java/com/google/firebase/firestore/core/QueryTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -535,12 +535,15 @@ public void testMatchesAllDocuments() {
535535
Query baseQuery = Query.atPath(ResourcePath.fromString("collection"));
536536
assertTrue(baseQuery.matchesAllDocuments());
537537

538-
Query query = baseQuery.filter(filter("foo", "==", "bar"));
539-
assertFalse(query.matchesAllDocuments());
538+
Query query = baseQuery.orderBy(orderBy("__name__"));
539+
assertTrue(query.matchesAllDocuments());
540540

541541
query = baseQuery.orderBy(orderBy("foo"));
542542
assertFalse(query.matchesAllDocuments());
543543

544+
query = baseQuery.filter(filter("foo", "==", "bar"));
545+
assertFalse(query.matchesAllDocuments());
546+
544547
query = baseQuery.limit(1);
545548
assertFalse(query.matchesAllDocuments());
546549

‎firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexFreeQueryEngineTest.java

Lines changed: 85 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static com.google.firebase.firestore.testutil.TestUtil.version;
2424
import static org.junit.Assert.assertEquals;
2525

26+
import com.google.android.gms.common.internal.Preconditions;
2627
import com.google.firebase.database.collection.ImmutableSortedMap;
2728
import com.google.firebase.database.collection.ImmutableSortedSet;
2829
import com.google.firebase.firestore.auth.User;
@@ -34,6 +35,8 @@
3435
import com.google.firebase.firestore.model.SnapshotVersion;
3536
import com.google.protobuf.ByteString;
3637
import java.util.Collections;
38+
import java.util.concurrent.Callable;
39+
import javax.annotation.Nullable;
3740
import org.junit.Before;
3841
import org.junit.Test;
3942
import org.junit.runner.RunWith;
@@ -68,11 +71,11 @@ public class IndexFreeQueryEngineTest {
6871
private QueryCache queryCache;
6972
private QueryEngine queryEngine;
7073

71-
private boolean expectIndexFreeExecution;
74+
private @Nullable Boolean expectIndexFreeExecution;
7275

7376
@Before
7477
public void setUp() {
75-
expectIndexFreeExecution = false;
78+
expectIndexFreeExecution = null;
7679

7780
persistence = MemoryPersistence.createEagerGcMemoryPersistence();
7881
queryCache = new MemoryQueryCache(persistence);
@@ -98,24 +101,52 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
98101
queryEngine.setLocalDocumentsView(localDocuments);
99102
}
100103

101-
/** Add a document to local cache and the remote key mapping. */
102-
private void addDocumentToRemoteResult(Document doc) {
104+
/** Adds the provided documents to the query target mapping. */
105+
private void persistQueryMapping(Document ... docs) {
103106
persistence.runTransaction(
104-
"addDocumentToRemoteResult",
107+
"persistQueryMapping",
105108
() -> {
106-
queryCache.addMatchingKeys(
107-
DocumentKey.emptyKeySet().insert(doc.getKey()), TEST_TARGET_ID);
108-
remoteDocumentCache.add(doc, doc.getVersion());
109+
ImmutableSortedSet<DocumentKey> remoteKeys = DocumentKey.emptyKeySet();
110+
for (Document doc: docs) {
111+
remoteKeys = remoteKeys.insert(doc.getKey());
112+
}
113+
queryCache.addMatchingKeys(remoteKeys, TEST_TARGET_ID);
114+
});
115+
}
116+
117+
/** Adds the provided documents to the remote document cache. */
118+
private void addDocument(Document ... docs) {
119+
persistence.runTransaction(
120+
"addDocument",
121+
() -> {
122+
for (Document doc : docs) {
123+
remoteDocumentCache.add(doc, doc.getVersion());
124+
}
109125
});
110126
}
111127

112-
/** Add a document to local cache but not the remote key mapping. */
113-
private void addDocumentToLocalResult(Document doc) {
114-
remoteDocumentCache.add(doc, doc.getVersion());
128+
private <T> T expectIndexFreeQuery(Callable<T> c) throws Exception {
129+
try {
130+
expectIndexFreeExecution = true;
131+
return c.call();
132+
} finally {
133+
expectIndexFreeExecution = null;
134+
}
115135
}
116136

117-
private DocumentSet runQuery(Query query, QueryData queryData, boolean expectIndexFree) {
118-
expectIndexFreeExecution = expectIndexFree;
137+
private <T> T expectFullCollectionQuery(Callable<T> c) throws Exception {
138+
try {
139+
expectIndexFreeExecution = false;
140+
return c.call();
141+
} finally {
142+
expectIndexFreeExecution = null;
143+
}
144+
}
145+
146+
private DocumentSet runQuery(Query query, QueryData queryData) {
147+
Preconditions.checkNotNull(
148+
expectIndexFreeExecution,
149+
"Encountered runQuery() call not wrapped in expectIndexFreeQuery()/expectFullCollectionQuery()");
119150
ImmutableSortedMap<DocumentKey, Document> docs =
120151
queryEngine.getDocumentsMatchingQuery(
121152
query, queryData, queryCache.getMatchingKeysForTargetId(TEST_TARGET_ID));
@@ -126,80 +157,85 @@ private DocumentSet runQuery(Query query, QueryData queryData, boolean expectInd
126157
}
127158

128159
@Test
129-
public void usesTargetMappingForInitialView() {
160+
public void usesTargetMappingForInitialView() throws Exception {
130161
Query query = query("coll").filter(filter("matches", "==", true));
131162
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
132163

133-
addDocumentToRemoteResult(MATCHING_DOC_A);
134-
addDocumentToRemoteResult(MATCHING_DOC_B);
164+
addDocument(MATCHING_DOC_A, MATCHING_DOC_B);
165+
persistQueryMapping(MATCHING_DOC_A, MATCHING_DOC_B);
135166

136-
DocumentSet docs = runQuery(query, queryData, /* expectIndexFree= */ true);
167+
DocumentSet docs = expectIndexFreeQuery(() -> runQuery(query, queryData));
137168
assertEquals(docSet(query.comparator(), MATCHING_DOC_A, MATCHING_DOC_B), docs);
138169
}
139170

140171
@Test
141-
public void filtersNonMatchingInitialResults() {
172+
public void filtersNonMatchingInitialResults() throws Exception {
142173
Query query = query("coll").filter(filter("matches", "==", true));
143174
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
144175

145-
addDocumentToRemoteResult(MATCHING_DOC_A);
146-
addDocumentToRemoteResult(MATCHING_DOC_B);
176+
addDocument(MATCHING_DOC_A, MATCHING_DOC_B);
177+
persistQueryMapping(MATCHING_DOC_A, MATCHING_DOC_B);
147178

148-
addDocumentToLocalResult(PENDING_NON_MATCHING_DOC_A);
179+
// Add a mutated document that is not yet part of query's set of remote keys.
180+
addDocument(PENDING_NON_MATCHING_DOC_A);
149181

150-
DocumentSet docs = runQuery(query, queryData, /* expectIndexFree= */ true);
182+
DocumentSet docs = expectIndexFreeQuery(() -> runQuery(query, queryData));
151183
assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs);
152184
}
153185

154186
@Test
155-
public void includesChangesSinceInitialResults() {
187+
public void includesChangesSinceInitialResults() throws Exception {
156188
Query query = query("coll").filter(filter("matches", "==", true));
157189
QueryData originalQueryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
158190

159-
addDocumentToRemoteResult(MATCHING_DOC_A);
160-
addDocumentToRemoteResult(NON_MATCHING_DOC_B);
191+
addDocument(MATCHING_DOC_A, MATCHING_DOC_B);
192+
persistQueryMapping(MATCHING_DOC_A, MATCHING_DOC_B);
161193

162-
DocumentSet docs = runQuery(query, originalQueryData, /* expectIndexFree= */ true);
163-
assertEquals(docSet(query.comparator(), MATCHING_DOC_A), docs);
194+
DocumentSet docs = expectIndexFreeQuery(() -> runQuery(query, originalQueryData));
195+
assertEquals(docSet(query.comparator(), MATCHING_DOC_A, MATCHING_DOC_B), docs);
164196

165-
addDocumentToLocalResult(UPDATED_MATCHING_DOC_B);
197+
addDocument(UPDATED_MATCHING_DOC_B);
166198

167-
docs = runQuery(query, originalQueryData, /* expectIndexFree= */ true);
199+
docs = expectIndexFreeQuery(() -> runQuery(query, originalQueryData));
168200
assertEquals(docSet(query.comparator(), MATCHING_DOC_A, UPDATED_MATCHING_DOC_B), docs);
169201
}
170202

171203
@Test
172-
public void doesNotUseInitialResultsWithoutLimboFreeSnapshotVersion() {
204+
public void doesNotUseInitialResultsWithoutLimboFreeSnapshotVersion() throws Exception {
173205
Query query = query("coll").filter(filter("matches", "==", true));
174206
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ false);
175207

176-
DocumentSet docs = runQuery(query, queryData, /* expectIndexFree= */ false);
208+
DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData));
177209
assertEquals(docSet(query.comparator()), docs);
178210
}
179211

180212
@Test
181-
public void doesNotUseInitialResultsForUnfilteredCollectionQuery() {
213+
public void doesNotUseInitialResultsForUnfilteredCollectionQuery() throws Exception {
182214
Query query = query("coll");
183215
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
184216

185-
DocumentSet docs = runQuery(query, queryData, /* expectIndexFree= */ false);
217+
DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData));
186218
assertEquals(docSet(query.comparator()), docs);
187219
}
188220

189221
@Test
190-
public void doesNotUseInitialResultsForLimitQueryWithDocumentRemoval() {
222+
public void doesNotUseInitialResultsForLimitQueryWithDocumentRemoval() throws Exception {
191223
Query query = query("coll").filter(filter("matches", "==", true)).limit(1);
192224

193-
addDocumentToRemoteResult(NON_MATCHING_DOC_A);
194-
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
195-
addDocumentToLocalResult(MATCHING_DOC_B);
225+
// While the backend would never add DocA to the set of remote keys, this allows us to easily
226+
// simulate what would happen when a document no longer matches due to an out-of-band update.
227+
addDocument(NON_MATCHING_DOC_A);
228+
persistQueryMapping(NON_MATCHING_DOC_A);
229+
230+
addDocument(MATCHING_DOC_B);
196231

197-
DocumentSet docs = runQuery(query, queryData, /* expectIndexFree= */ false);
232+
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
233+
DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData));
198234
assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs);
199235
}
200236

201237
@Test
202-
public void doesNotUseInitialResultsForLimitQueryWithPendingWrite() {
238+
public void doesNotUseInitialResultsForLimitQueryWithPendingWrite() throws Exception {
203239
Query query =
204240
query("coll")
205241
.filter(filter("matches", "==", true))
@@ -208,18 +244,20 @@ public void doesNotUseInitialResultsForLimitQueryWithPendingWrite() {
208244

209245
// Add a query mapping for a document that matches, but that sorts below another document due to
210246
// a pending write.
211-
addDocumentToRemoteResult(PENDING_MATCHING_DOC_A);
247+
addDocument(PENDING_MATCHING_DOC_A);
248+
persistQueryMapping(PENDING_MATCHING_DOC_A);
212249

213250
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
214251

215-
addDocumentToLocalResult(MATCHING_DOC_B);
252+
addDocument(MATCHING_DOC_B);
216253

217-
DocumentSet docs = runQuery(query, queryData, /* expectIndexFree= */ false);
254+
DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData));
218255
assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs);
219256
}
220257

221258
@Test
222-
public void doesNotUseInitialResultsForLimitQueryWithDocumentThatHasBeenUpdatedOutOfBand() {
259+
public void doesNotUseInitialResultsForLimitQueryWithDocumentThatHasBeenUpdatedOutOfBand()
260+
throws Exception {
223261
Query query =
224262
query("coll")
225263
.filter(filter("matches", "==", true))
@@ -228,13 +266,14 @@ public void doesNotUseInitialResultsForLimitQueryWithDocumentThatHasBeenUpdatedO
228266

229267
// Add a query mapping for a document that matches, but that sorts below another document based
230268
// due to an update that the SDK received after the query's snapshot was persisted.
231-
addDocumentToRemoteResult(UDPATED_DOC_A);
269+
addDocument(UDPATED_DOC_A);
270+
persistQueryMapping(UDPATED_DOC_A);
232271

233272
QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true);
234273

235-
addDocumentToLocalResult(MATCHING_DOC_B);
274+
addDocument(MATCHING_DOC_B);
236275

237-
DocumentSet docs = runQuery(query, queryData, /* expectIndexFree= */ false);
276+
DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData));
238277
assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs);
239278
}
240279

0 commit comments

Comments
 (0)