Skip to content

Commit 08deb69

Browse files
tom-andersentom-andersen
authored
Add FirestoreClientProvider (#6050)
Introduce FirestoreClientProvider to manage life cycle and access to FirestoreClient. Calls to FirestoreClient are made through the reference held in FirestoreClientProvider, thereby preparing for swapping FirestoreClient in future PR. This requires rewiring calls, such that code does not hold a direct reference to FirestoreClient. I am open to a better name than FirestoeClientProvider...
1 parent 57aaf39 commit 08deb69

24 files changed

+304
-203
lines changed

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ public static FirebaseFirestore newFirebaseFirestore(
3333
String persistenceKey,
3434
CredentialsProvider<User> authProvider,
3535
CredentialsProvider<String> appCheckProvider,
36-
AsyncQueue asyncQueue,
3736
Function<FirebaseFirestoreSettings, ComponentProvider> componentProviderFactory,
3837
FirebaseApp firebaseApp,
3938
FirebaseFirestore.InstanceRegistry instanceRegistry) {
@@ -43,14 +42,13 @@ public static FirebaseFirestore newFirebaseFirestore(
4342
persistenceKey,
4443
authProvider,
4544
appCheckProvider,
46-
asyncQueue,
4745
componentProviderFactory,
4846
firebaseApp,
4947
instanceRegistry,
5048
null);
5149
}
5250

5351
public static AsyncQueue getAsyncQueue(FirebaseFirestore firestore) {
54-
return firestore.getAsyncQueue();
52+
return firestore.clientProvider.getAsyncQueue();
5553
}
5654
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ public void testMultipleInequalityReadFromCacheWhenOffline() {
742742
assertEquals(2L, snapshot1.size());
743743
assertFalse(snapshot1.getMetadata().isFromCache());
744744

745-
waitFor(collection.firestore.getClient().disableNetwork());
745+
waitFor(collection.firestore.disableNetwork());
746746

747747
QuerySnapshot snapshot2 = waitFor(query.get());
748748
assertEquals(2L, snapshot2.size());

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import com.google.firebase.firestore.FirebaseFirestoreException.Code;
5050
import com.google.firebase.firestore.Query.Direction;
5151
import com.google.firebase.firestore.auth.User;
52+
import com.google.firebase.firestore.core.FirestoreClient;
5253
import com.google.firebase.firestore.model.DatabaseId;
5354
import com.google.firebase.firestore.testutil.EventAccumulator;
5455
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
@@ -1110,7 +1111,7 @@ public void testRestartFirestoreLeadsToNewInstance() {
11101111
assertSame(instance, sameInstance);
11111112
waitFor(instance.document("abc/123").set(Collections.singletonMap("field", 100L)));
11121113

1113-
instance.terminate();
1114+
waitFor(instance.terminate());
11141115
FirebaseFirestore newInstance = FirebaseFirestore.getInstance(app);
11151116
newInstance.setFirestoreSettings(newTestSettings());
11161117

@@ -1132,7 +1133,7 @@ public void testAppDeleteLeadsToFirestoreTerminate() {
11321133

11331134
app.delete();
11341135

1135-
assertTrue(instance.getClient().isTerminated());
1136+
assertTrue(instance.callClient(FirestoreClient::isTerminated));
11361137
}
11371138

11381139
@Test

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,12 +536,12 @@ public void testQueriesFireFromCacheWhenOffline() {
536536
assertFalse(querySnapshot.getMetadata().isFromCache());
537537

538538
// offline event with fromCache=true
539-
waitFor(collection.firestore.getClient().disableNetwork());
539+
waitFor(collection.firestore.disableNetwork());
540540
querySnapshot = accum.await();
541541
assertTrue(querySnapshot.getMetadata().isFromCache());
542542

543543
// back online event with fromCache=false
544-
waitFor(collection.firestore.getClient().enableNetwork());
544+
waitFor(collection.firestore.enableNetwork());
545545
querySnapshot = accum.await();
546546
assertFalse(querySnapshot.getMetadata().isFromCache());
547547

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ public void testServerTimestampsCanReturnPreviousValueOfDifferentType() {
214214
@Test
215215
public void testServerTimestampsCanRetainPreviousValueThroughConsecutiveUpdates() {
216216
writeInitialData();
217-
waitFor(docRef.getFirestore().getClient().disableNetwork());
217+
waitFor(docRef.getFirestore().disableNetwork());
218218
accumulator.awaitRemoteEvent();
219219

220220
docRef.update("a", FieldValue.serverTimestamp());
@@ -226,7 +226,7 @@ public void testServerTimestampsCanRetainPreviousValueThroughConsecutiveUpdates(
226226
localSnapshot = accumulator.awaitLocalEvent();
227227
assertEquals(42L, localSnapshot.get("a", ServerTimestampBehavior.PREVIOUS));
228228

229-
waitFor(docRef.getFirestore().getClient().enableNetwork());
229+
waitFor(docRef.getFirestore().enableNetwork());
230230

231231
DocumentSnapshot remoteSnapshot = accumulator.awaitRemoteEvent();
232232
assertThat(remoteSnapshot.get("a")).isInstanceOf(Timestamp.class);
@@ -235,7 +235,7 @@ public void testServerTimestampsCanRetainPreviousValueThroughConsecutiveUpdates(
235235
@Test
236236
public void testServerTimestampsUsesPreviousValueFromLocalMutation() {
237237
writeInitialData();
238-
waitFor(docRef.getFirestore().getClient().disableNetwork());
238+
waitFor(docRef.getFirestore().disableNetwork());
239239
accumulator.awaitRemoteEvent();
240240

241241
docRef.update("a", FieldValue.serverTimestamp());
@@ -249,7 +249,7 @@ public void testServerTimestampsUsesPreviousValueFromLocalMutation() {
249249
localSnapshot = accumulator.awaitLocalEvent();
250250
assertEquals(1337L, localSnapshot.get("a", ServerTimestampBehavior.PREVIOUS));
251251

252-
waitFor(docRef.getFirestore().getClient().enableNetwork());
252+
waitFor(docRef.getFirestore().enableNetwork());
253253

254254
DocumentSnapshot remoteSnapshot = accumulator.awaitRemoteEvent();
255255
assertThat(remoteSnapshot.get("a")).isInstanceOf(Timestamp.class);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ public void testIncrementTransactionally() {
371371
AtomicInteger started = new AtomicInteger(0);
372372

373373
FirebaseFirestore firestore = testFirestore();
374-
firestore.getAsyncQueue().skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
374+
AccessHelper.getAsyncQueue(firestore).skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
375375
DocumentReference doc = firestore.collection("counters").document();
376376
waitFor(doc.set(map("count", 5.0)));
377377

@@ -437,7 +437,7 @@ public void testUpdateTransactionally() {
437437
AtomicInteger counter = new AtomicInteger(0);
438438

439439
FirebaseFirestore firestore = testFirestore();
440-
firestore.getAsyncQueue().skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
440+
AccessHelper.getAsyncQueue(firestore).skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
441441
DocumentReference doc = firestore.collection("counters").document();
442442
waitFor(doc.set(map("count", 5.0, "other", "yes")));
443443

@@ -532,7 +532,7 @@ public void testUpdatePOJOTransactionally() {
532532
AtomicInteger started = new AtomicInteger(0);
533533

534534
FirebaseFirestore firestore = testFirestore();
535-
firestore.getAsyncQueue().skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
535+
AccessHelper.getAsyncQueue(firestore).skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
536536
DocumentReference doc = firestore.collection("counters").document();
537537
waitFor(doc.set(new POJO(5.0, "no", "clean")));
538538

@@ -601,7 +601,7 @@ public void testRetriesWhenDocumentThatWasReadWithoutBeingWrittenChanges() {
601601
@Test
602602
public void testReadingADocTwiceWithDifferentVersions() {
603603
FirebaseFirestore firestore = testFirestore();
604-
firestore.getAsyncQueue().skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
604+
AccessHelper.getAsyncQueue(firestore).skipDelaysForTimerId(TimerId.RETRY_TRANSACTION);
605605
DocumentReference doc = firestore.collection("counters").document();
606606
waitFor(doc.set(map("count", 15.0)));
607607
AtomicInteger counter = new AtomicInteger(0);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ public void queriesCannotBeSortedByAnUncommittedServerTimestamp() {
453453
CollectionReference collection = testCollection();
454454

455455
// Ensure the server timestamp stays uncommitted for the first half of the test
456-
waitFor(collection.firestore.getClient().disableNetwork());
456+
waitFor(collection.firestore.disableNetwork());
457457

458458
TaskCompletionSource<Void> offlineCallbackDone = new TaskCompletionSource<>();
459459
TaskCompletionSource<Void> onlineCallbackDone = new TaskCompletionSource<>();
@@ -497,7 +497,7 @@ public void queriesCannotBeSortedByAnUncommittedServerTimestamp() {
497497
document.set(map("timestamp", FieldValue.serverTimestamp()));
498498
waitFor(offlineCallbackDone.getTask());
499499

500-
waitFor(collection.firestore.getClient().enableNetwork());
500+
waitFor(collection.firestore.enableNetwork());
501501
waitFor(onlineCallbackDone.getTask());
502502

503503
listenerRegistration.remove();

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,6 @@ public static FirebaseFirestore testFirestore(
320320
persistenceKey,
321321
MockCredentialsProvider.instance(),
322322
new EmptyAppCheckTokenProvider(),
323-
asyncQueue,
324323
ComponentProvider::defaultFactory,
325324
/* firebaseApp= */ null,
326325
/* instanceRegistry= */ (dbId) -> {});

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ public Task<AggregateQuerySnapshot> get(@NonNull AggregateSource source) {
6666
TaskCompletionSource<AggregateQuerySnapshot> tcs = new TaskCompletionSource<>();
6767
query
6868
.firestore
69-
.getClient()
70-
.runAggregateQuery(query.query, aggregateFieldList)
69+
.callClient(client -> client.runAggregateQuery(query.query, aggregateFieldList))
7170
.continueWith(
7271
Executors.DIRECT_EXECUTOR,
7372
(task) -> {

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

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import com.google.firebase.firestore.core.ActivityScope;
3030
import com.google.firebase.firestore.core.AsyncEventListener;
3131
import com.google.firebase.firestore.core.EventManager.ListenOptions;
32-
import com.google.firebase.firestore.core.ListenerRegistrationImpl;
3332
import com.google.firebase.firestore.core.QueryListener;
3433
import com.google.firebase.firestore.core.UserData.ParsedSetData;
3534
import com.google.firebase.firestore.core.UserData.ParsedUpdateData;
@@ -38,11 +37,12 @@
3837
import com.google.firebase.firestore.model.DocumentKey;
3938
import com.google.firebase.firestore.model.ResourcePath;
4039
import com.google.firebase.firestore.model.mutation.DeleteMutation;
40+
import com.google.firebase.firestore.model.mutation.Mutation;
4141
import com.google.firebase.firestore.model.mutation.Precondition;
4242
import com.google.firebase.firestore.util.Assert;
4343
import com.google.firebase.firestore.util.Executors;
4444
import com.google.firebase.firestore.util.Util;
45-
import java.util.Collections;
45+
import java.util.List;
4646
import java.util.Map;
4747
import java.util.concurrent.ExecutionException;
4848
import java.util.concurrent.Executor;
@@ -165,9 +165,9 @@ public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
165165
options.isMerge()
166166
? firestore.getUserDataReader().parseMergeData(data, options.getFieldMask())
167167
: firestore.getUserDataReader().parseSetData(data);
168+
List<Mutation> mutations = singletonList(parsed.toMutation(key, Precondition.NONE));
168169
return firestore
169-
.getClient()
170-
.write(Collections.singletonList(parsed.toMutation(key, Precondition.NONE)))
170+
.callClient(client -> client.write(mutations))
171171
.continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
172172
}
173173

@@ -229,9 +229,9 @@ public Task<Void> update(
229229
}
230230

231231
private Task<Void> update(@NonNull ParsedUpdateData parsedData) {
232+
List<Mutation> mutations = singletonList(parsedData.toMutation(key, Precondition.exists(true)));
232233
return firestore
233-
.getClient()
234-
.write(Collections.singletonList(parsedData.toMutation(key, Precondition.exists(true))))
234+
.callClient(client -> client.write(mutations))
235235
.continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
236236
}
237237

@@ -242,9 +242,9 @@ private Task<Void> update(@NonNull ParsedUpdateData parsedData) {
242242
*/
243243
@NonNull
244244
public Task<Void> delete() {
245+
List<Mutation> mutations = singletonList(new DeleteMutation(key, Precondition.NONE));
245246
return firestore
246-
.getClient()
247-
.write(singletonList(new DeleteMutation(key, Precondition.NONE)))
247+
.callClient(client -> client.write(mutations))
248248
.continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
249249
}
250250

@@ -274,8 +274,7 @@ public Task<DocumentSnapshot> get() {
274274
public Task<DocumentSnapshot> get(@NonNull Source source) {
275275
if (source == Source.CACHE) {
276276
return firestore
277-
.getClient()
278-
.getDocumentFromLocalCache(key)
277+
.callClient(client -> client.getDocumentFromLocalCache(key))
279278
.continueWith(
280279
Executors.DIRECT_EXECUTOR,
281280
(Task<Document> task) -> {
@@ -531,11 +530,17 @@ private ListenerRegistration addSnapshotListenerInternal(
531530
new AsyncEventListener<>(userExecutor, viewListener);
532531

533532
com.google.firebase.firestore.core.Query query = asQuery();
534-
QueryListener queryListener = firestore.getClient().listen(query, options, asyncListener);
535533

536-
return ActivityScope.bind(
537-
activity,
538-
new ListenerRegistrationImpl(firestore.getClient(), queryListener, asyncListener));
534+
return firestore.callClient(
535+
client -> {
536+
QueryListener queryListener = client.listen(query, options, asyncListener);
537+
return ActivityScope.bind(
538+
activity,
539+
() -> {
540+
asyncListener.mute();
541+
client.stopListening(queryListener);
542+
});
543+
});
539544
}
540545

541546
@Override

0 commit comments

Comments
 (0)