Skip to content

Commit 5fd9a73

Browse files
tom-andersentom-andersen
authored
Refactor ComponentProvider (#6037)
The ComponentProvider is lightweight dependency injection for Firestore components. This PR expands the number of class that are instantiated as part of the ComponentProvider. This separation of concerns simplifies classes, and makes testing these classes in isolation possible. This PR includes some refactoring of tests, with respect to instantiating classes. This PR prepares for more thorough integration tests as part of cache clearing logic.
1 parent b4b096b commit 5fd9a73

26 files changed

+390
-296
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
import com.google.firebase.FirebaseApp;
1919
import com.google.firebase.firestore.auth.CredentialsProvider;
2020
import com.google.firebase.firestore.auth.User;
21+
import com.google.firebase.firestore.core.ComponentProvider;
2122
import com.google.firebase.firestore.model.DatabaseId;
2223
import com.google.firebase.firestore.util.AsyncQueue;
24+
import com.google.firebase.firestore.util.Function;
2325

2426
/** Gives access to package private methods in integration tests. */
2527
public final class AccessHelper {
@@ -32,6 +34,7 @@ public static FirebaseFirestore newFirebaseFirestore(
3234
CredentialsProvider<User> authProvider,
3335
CredentialsProvider<String> appCheckProvider,
3436
AsyncQueue asyncQueue,
37+
Function<FirebaseFirestoreSettings, ComponentProvider> componentProviderFactory,
3538
FirebaseApp firebaseApp,
3639
FirebaseFirestore.InstanceRegistry instanceRegistry) {
3740
return new FirebaseFirestore(
@@ -41,6 +44,7 @@ public static FirebaseFirestore newFirebaseFirestore(
4144
authProvider,
4245
appCheckProvider,
4346
asyncQueue,
47+
componentProviderFactory,
4448
firebaseApp,
4549
instanceRegistry,
4650
null);

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/RemoteStoreTest.java

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import androidx.test.ext.junit.runners.AndroidJUnit4;
2121
import com.google.firebase.database.collection.ImmutableSortedSet;
2222
import com.google.firebase.firestore.auth.User;
23+
import com.google.firebase.firestore.core.DatabaseInfo;
2324
import com.google.firebase.firestore.core.OnlineState;
2425
import com.google.firebase.firestore.local.LocalStore;
2526
import com.google.firebase.firestore.local.MemoryPersistence;
@@ -40,14 +41,8 @@ public class RemoteStoreTest {
4041
@Test
4142
public void testRemoteStoreStreamStopsWhenNetworkUnreachable() {
4243
AsyncQueue testQueue = new AsyncQueue();
43-
Datastore datastore =
44-
new Datastore(
45-
IntegrationTestUtil.testEnvDatabaseInfo(),
46-
testQueue,
47-
null,
48-
null,
49-
ApplicationProvider.getApplicationContext(),
50-
null);
44+
RemoteSerializer serializer = new RemoteSerializer(IntegrationTestUtil.testEnvDatabaseId());
45+
Datastore datastore = new Datastore(testQueue, serializer, null);
5146
Semaphore networkChangeSemaphore = new Semaphore(0);
5247
RemoteStore.RemoteStoreCallback callback =
5348
new RemoteStore.RemoteStoreCallback() {
@@ -75,12 +70,11 @@ public ImmutableSortedSet<DocumentKey> getRemoteKeysForTarget(int targetId) {
7570
};
7671

7772
FakeConnectivityMonitor connectivityMonitor = new FakeConnectivityMonitor();
78-
QueryEngine queryEngine = new QueryEngine();
7973
Persistence persistence = MemoryPersistence.createEagerGcMemoryPersistence();
8074
persistence.start();
81-
LocalStore localStore = new LocalStore(persistence, queryEngine, User.UNAUTHENTICATED);
75+
LocalStore localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
8276
RemoteStore remoteStore =
83-
new RemoteStore(callback, localStore, datastore, testQueue, connectivityMonitor);
77+
new RemoteStore(IntegrationTestUtil.testEnvDatabaseId(), callback, localStore, datastore, testQueue, connectivityMonitor);
8478

8579
waitFor(testQueue.enqueue(remoteStore::forceEnableNetwork));
8680
drain(testQueue);

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/StreamTest.java

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
import androidx.test.core.app.ApplicationProvider;
3030
import androidx.test.ext.junit.runners.AndroidJUnit4;
3131
import com.google.android.gms.tasks.Task;
32+
import com.google.firebase.firestore.auth.CredentialsProvider;
33+
import com.google.firebase.firestore.auth.User;
34+
import com.google.firebase.firestore.core.DatabaseInfo;
3235
import com.google.firebase.firestore.model.SnapshotVersion;
3336
import com.google.firebase.firestore.model.mutation.Mutation;
3437
import com.google.firebase.firestore.model.mutation.MutationResult;
@@ -109,14 +112,16 @@ public void onWriteResponse(
109112
/** Creates a WriteStream and gets it in a state that accepts mutations. */
110113
private WriteStream createAndOpenWriteStream(
111114
AsyncQueue testQueue, StreamStatusCallback callback) {
112-
Datastore datastore =
113-
new Datastore(
114-
IntegrationTestUtil.testEnvDatabaseInfo(),
115+
DatabaseInfo databaseInfo = IntegrationTestUtil.testEnvDatabaseInfo();
116+
RemoteSerializer remoteSerializer = new RemoteSerializer(databaseInfo.getDatabaseId());
117+
FirestoreChannel firestoreChannel = new FirestoreChannel(
115118
testQueue,
116-
new EmptyCredentialsProvider(),
117-
new EmptyAppCheckTokenProvider(),
118119
ApplicationProvider.getApplicationContext(),
119-
null);
120+
null == null ? new EmptyCredentialsProvider() : null,
121+
new EmptyAppCheckTokenProvider(),
122+
databaseInfo,
123+
(GrpcMetadataProvider) null);
124+
Datastore datastore = new Datastore(testQueue, remoteSerializer, firestoreChannel);
120125
final WriteStream writeStream = datastore.createWriteStream(callback);
121126
waitForWriteStreamOpen(testQueue, writeStream, callback);
122127
return writeStream;
@@ -135,14 +140,16 @@ private void waitForWriteStreamOpen(
135140
public void testWatchStreamStopBeforeHandshake() throws Exception {
136141
AsyncQueue testQueue = new AsyncQueue();
137142
GrpcMetadataProvider mockGrpcProvider = mock(GrpcMetadataProvider.class);
138-
Datastore datastore =
139-
new Datastore(
140-
IntegrationTestUtil.testEnvDatabaseInfo(),
143+
DatabaseInfo databaseInfo = IntegrationTestUtil.testEnvDatabaseInfo();
144+
RemoteSerializer remoteSerializer = new RemoteSerializer(databaseInfo.getDatabaseId());
145+
FirestoreChannel firestoreChannel = new FirestoreChannel(
141146
testQueue,
147+
ApplicationProvider.getApplicationContext(),
142148
new EmptyCredentialsProvider(),
143149
new EmptyAppCheckTokenProvider(),
144-
ApplicationProvider.getApplicationContext(),
150+
databaseInfo,
145151
mockGrpcProvider);
152+
Datastore datastore = new Datastore(testQueue, remoteSerializer, firestoreChannel);
146153
StreamStatusCallback streamCallback = new StreamStatusCallback() {};
147154
final WatchStream watchStream = datastore.createWatchStream(streamCallback);
148155

@@ -158,14 +165,16 @@ public void testWatchStreamStopBeforeHandshake() throws Exception {
158165
@Test
159166
public void testWriteStreamStopAfterHandshake() throws Exception {
160167
AsyncQueue testQueue = new AsyncQueue();
161-
Datastore datastore =
162-
new Datastore(
163-
IntegrationTestUtil.testEnvDatabaseInfo(),
168+
DatabaseInfo databaseInfo = IntegrationTestUtil.testEnvDatabaseInfo();
169+
RemoteSerializer remoteSerializer = new RemoteSerializer(databaseInfo.getDatabaseId());
170+
FirestoreChannel firestoreChannel = new FirestoreChannel(
164171
testQueue,
172+
ApplicationProvider.getApplicationContext(),
165173
new EmptyCredentialsProvider(),
166174
new EmptyAppCheckTokenProvider(),
167-
ApplicationProvider.getApplicationContext(),
168-
null);
175+
databaseInfo,
176+
(GrpcMetadataProvider) null);
177+
Datastore datastore = new Datastore(testQueue, remoteSerializer, firestoreChannel);
169178
final WriteStream[] writeStreamWrapper = new WriteStream[1];
170179
StreamStatusCallback streamCallback =
171180
new StreamStatusCallback() {
@@ -206,14 +215,16 @@ public void onWriteResponse(
206215
@Test
207216
public void testWriteStreamStopPartial() throws Exception {
208217
AsyncQueue testQueue = new AsyncQueue();
209-
Datastore datastore =
210-
new Datastore(
211-
IntegrationTestUtil.testEnvDatabaseInfo(),
218+
DatabaseInfo databaseInfo = IntegrationTestUtil.testEnvDatabaseInfo();
219+
RemoteSerializer remoteSerializer = new RemoteSerializer(databaseInfo.getDatabaseId());
220+
FirestoreChannel firestoreChannel = new FirestoreChannel(
212221
testQueue,
222+
ApplicationProvider.getApplicationContext(),
213223
new EmptyCredentialsProvider(),
214224
new EmptyAppCheckTokenProvider(),
215-
ApplicationProvider.getApplicationContext(),
216-
null);
225+
databaseInfo,
226+
(GrpcMetadataProvider) null);
227+
Datastore datastore = new Datastore(testQueue, remoteSerializer, firestoreChannel);
217228
StreamStatusCallback streamCallback = new StreamStatusCallback() {};
218229
final WriteStream writeStream = datastore.createWriteStream(streamCallback);
219230

@@ -287,14 +298,16 @@ public void testStreamStaysIdle() throws Exception {
287298
public void testStreamRefreshesTokenUponExpiration() throws Exception {
288299
AsyncQueue testQueue = new AsyncQueue();
289300
MockCredentialsProvider mockCredentialsProvider = new MockCredentialsProvider();
290-
Datastore datastore =
291-
new Datastore(
292-
IntegrationTestUtil.testEnvDatabaseInfo(),
301+
DatabaseInfo databaseInfo = IntegrationTestUtil.testEnvDatabaseInfo();
302+
RemoteSerializer remoteSerializer = new RemoteSerializer(databaseInfo.getDatabaseId());
303+
FirestoreChannel firestoreChannel = new FirestoreChannel(
293304
testQueue,
305+
ApplicationProvider.getApplicationContext(),
294306
mockCredentialsProvider,
295307
new EmptyAppCheckTokenProvider(),
296-
ApplicationProvider.getApplicationContext(),
297-
null);
308+
databaseInfo,
309+
(GrpcMetadataProvider) null);
310+
Datastore datastore = new Datastore(testQueue, remoteSerializer, firestoreChannel);
298311
StreamStatusCallback callback = new StreamStatusCallback();
299312
WriteStream writeStream = datastore.createWriteStream(callback);
300313
waitForWriteStreamOpen(testQueue, writeStream, callback);
@@ -317,14 +330,16 @@ public void testStreamRefreshesTokenUponExpiration() throws Exception {
317330
public void testTokenIsNotInvalidatedOnceStreamIsHealthy() throws Exception {
318331
AsyncQueue testQueue = new AsyncQueue();
319332
MockCredentialsProvider mockCredentialsProvider = new MockCredentialsProvider();
320-
Datastore datastore =
321-
new Datastore(
322-
IntegrationTestUtil.testEnvDatabaseInfo(),
333+
DatabaseInfo databaseInfo = IntegrationTestUtil.testEnvDatabaseInfo();
334+
RemoteSerializer remoteSerializer = new RemoteSerializer(databaseInfo.getDatabaseId());
335+
FirestoreChannel firestoreChannel = new FirestoreChannel(
323336
testQueue,
337+
ApplicationProvider.getApplicationContext(),
324338
mockCredentialsProvider,
325339
new EmptyAppCheckTokenProvider(),
326-
ApplicationProvider.getApplicationContext(),
327-
null);
340+
databaseInfo,
341+
(GrpcMetadataProvider) null);
342+
Datastore datastore = new Datastore(testQueue, remoteSerializer, firestoreChannel);
328343
StreamStatusCallback callback = new StreamStatusCallback();
329344
WriteStream writeStream = datastore.createWriteStream(callback);
330345
waitForWriteStreamOpen(testQueue, writeStream, callback);

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.google.firebase.firestore.Source;
4646
import com.google.firebase.firestore.WriteBatch;
4747
import com.google.firebase.firestore.auth.User;
48+
import com.google.firebase.firestore.core.ComponentProvider;
4849
import com.google.firebase.firestore.core.DatabaseInfo;
4950
import com.google.firebase.firestore.model.DatabaseId;
5051
import com.google.firebase.firestore.testutil.provider.FirestoreProvider;
@@ -173,14 +174,20 @@ public static TargetBackend getTargetBackend() {
173174
}
174175
}
175176

177+
@NonNull
176178
public static DatabaseInfo testEnvDatabaseInfo() {
177179
return new DatabaseInfo(
178-
DatabaseId.forProject(provider.projectId()),
180+
testEnvDatabaseId(),
179181
"test-persistenceKey",
180182
getFirestoreHost(),
181183
getSslEnabled());
182184
}
183185

186+
@NonNull
187+
public static DatabaseId testEnvDatabaseId() {
188+
return DatabaseId.forProject(provider.projectId());
189+
}
190+
184191
public static FirebaseFirestoreSettings newTestSettings() {
185192
Logger.debug("IntegrationTestUtil", "target backend is: %s", backend.name());
186193
FirebaseFirestoreSettings.Builder settings = new FirebaseFirestoreSettings.Builder();
@@ -294,6 +301,7 @@ public static FirebaseFirestore testFirestore(
294301
MockCredentialsProvider.instance(),
295302
new EmptyAppCheckTokenProvider(),
296303
asyncQueue,
304+
ComponentProvider::defaultFactory,
297305
/*firebaseApp=*/ null,
298306
/*instanceRegistry=*/ (dbId) -> {});
299307
firestore.setFirestoreSettings(settings);

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

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.google.firebase.firestore.auth.User;
4040
import com.google.firebase.firestore.core.ActivityScope;
4141
import com.google.firebase.firestore.core.AsyncEventListener;
42+
import com.google.firebase.firestore.core.ComponentProvider;
4243
import com.google.firebase.firestore.core.DatabaseInfo;
4344
import com.google.firebase.firestore.core.FirestoreClient;
4445
import com.google.firebase.firestore.local.SQLitePersistence;
@@ -75,6 +76,8 @@
7576
*/
7677
public class FirebaseFirestore {
7778

79+
private final Function<FirebaseFirestoreSettings, ComponentProvider> componentProviderFactory;
80+
7881
/**
7982
* Provides a registry management interface for {@code FirebaseFirestore} instances.
8083
*
@@ -205,18 +208,17 @@ static FirebaseFirestore newInstance(
205208
// so there is no need to include it in the persistence key.
206209
String persistenceKey = app.getName();
207210

208-
FirebaseFirestore firestore =
209-
new FirebaseFirestore(
210-
context,
211-
databaseId,
212-
persistenceKey,
213-
authProvider,
214-
appCheckProvider,
215-
queue,
216-
app,
217-
instanceRegistry,
218-
metadataProvider);
219-
return firestore;
211+
return new FirebaseFirestore(
212+
context,
213+
databaseId,
214+
persistenceKey,
215+
authProvider,
216+
appCheckProvider,
217+
queue,
218+
ComponentProvider::defaultFactory,
219+
app,
220+
instanceRegistry,
221+
metadataProvider);
220222
}
221223

222224
@VisibleForTesting
@@ -227,6 +229,7 @@ static FirebaseFirestore newInstance(
227229
CredentialsProvider<User> authProvider,
228230
CredentialsProvider<String> appCheckProvider,
229231
AsyncQueue asyncQueue,
232+
@NonNull Function<FirebaseFirestoreSettings, ComponentProvider> componentProviderFactory,
230233
@Nullable FirebaseApp firebaseApp,
231234
InstanceRegistry instanceRegistry,
232235
@Nullable GrpcMetadataProvider metadataProvider) {
@@ -237,6 +240,7 @@ static FirebaseFirestore newInstance(
237240
this.authProvider = checkNotNull(authProvider);
238241
this.appCheckProvider = checkNotNull(appCheckProvider);
239242
this.asyncQueue = checkNotNull(asyncQueue);
243+
this.componentProviderFactory = checkNotNull(componentProviderFactory);
240244
// NOTE: We allow firebaseApp to be null in tests only.
241245
this.firebaseApp = firebaseApp;
242246
this.instanceRegistry = instanceRegistry;
@@ -256,10 +260,9 @@ public FirebaseFirestoreSettings getFirestoreSettings() {
256260
* can only be called before calling any other methods on this object.
257261
*/
258262
public void setFirestoreSettings(@NonNull FirebaseFirestoreSettings settings) {
259-
settings = mergeEmulatorSettings(settings, this.emulatorSettings);
260-
263+
checkNotNull(settings, "Provided settings must not be null.");
261264
synchronized (databaseId) {
262-
checkNotNull(settings, "Provided settings must not be null.");
265+
settings = mergeEmulatorSettings(settings, emulatorSettings);
263266

264267
// As a special exception, don't throw if the same settings are passed repeatedly. This
265268
// should make it simpler to get a Firestore instance in an activity.
@@ -288,8 +291,8 @@ public void useEmulator(@NonNull String host, int port) {
288291
"Cannot call useEmulator() after instance has already been initialized.");
289292
}
290293

291-
this.emulatorSettings = new EmulatedServiceSettings(host, port);
292-
this.settings = mergeEmulatorSettings(this.settings, this.emulatorSettings);
294+
emulatorSettings = new EmulatedServiceSettings(host, port);
295+
settings = mergeEmulatorSettings(settings, emulatorSettings);
293296
}
294297

295298
private void ensureClientConfigured() {
@@ -312,7 +315,8 @@ private void ensureClientConfigured() {
312315
authProvider,
313316
appCheckProvider,
314317
asyncQueue,
315-
metadataProvider);
318+
metadataProvider,
319+
componentProviderFactory.apply(settings));
316320
}
317321
}
318322

0 commit comments

Comments
 (0)