-
Notifications
You must be signed in to change notification settings - Fork 615
Prepare shutdown
to be a public api.
#589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
fa5e624
9ba6ff3
835ddc8
e7e3ea1
317fea7
34171e8
74d507c
fe02b17
f8b33ed
d3141b2
701fcef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,12 +18,16 @@ | |
import androidx.annotation.NonNull; | ||
import androidx.annotation.Nullable; | ||
import com.google.firebase.FirebaseApp; | ||
import com.google.firebase.FirebaseAppLifecycleListener; | ||
import com.google.firebase.FirebaseOptions; | ||
import com.google.firebase.auth.internal.InternalAuthProvider; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
/** Multi-resource container for Firestore. */ | ||
class FirestoreMultiDbComponent { | ||
class FirestoreMultiDbComponent | ||
implements FirebaseAppLifecycleListener, FirebaseFirestore.InstanceRegistry { | ||
|
||
/** | ||
* A static map from instance key to FirebaseFirestore instances. Instance keys are database | ||
* names. | ||
|
@@ -41,16 +45,38 @@ class FirestoreMultiDbComponent { | |
this.context = context; | ||
this.app = app; | ||
this.authProvider = authProvider; | ||
this.app.addLifecycleEventListener(this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a consideration: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. During the API discussion about this, the consensus emerged that While this hasn't been made public or documented on Android specifically, that's a localized problem. We want our ports to behave identically to the extent that we can arrange it. If this ever becomes a documented API we'll be ready. In other words: this will be the behavior on the other ports so might as well do the same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sgtm |
||
} | ||
|
||
/** Provides instances of Firestore for given database names. */ | ||
/** Provides instances of Firestore for given database IDs. */ | ||
@NonNull | ||
synchronized FirebaseFirestore get(@NonNull String databaseName) { | ||
FirebaseFirestore firestore = instances.get(databaseName); | ||
synchronized FirebaseFirestore get(@NonNull String databaseId) { | ||
wilhuff marked this conversation as resolved.
Show resolved
Hide resolved
|
||
FirebaseFirestore firestore = instances.get(databaseId); | ||
if (firestore == null) { | ||
firestore = FirebaseFirestore.newInstance(context, app, authProvider, databaseName); | ||
instances.put(databaseName, firestore); | ||
firestore = FirebaseFirestore.newInstance(context, app, authProvider, databaseId, this); | ||
instances.put(databaseId, firestore); | ||
} | ||
return firestore; | ||
} | ||
|
||
/** | ||
* Remove the instance of a given database ID from this component, such that if {@link | ||
* FirestoreMultiDbComponent#get(String)} is called again with the same name, a new instance of | ||
* {@link FirebaseFirestore} is created. | ||
* | ||
* <p>It is a no-op if there is no instance associated with the given database name. | ||
*/ | ||
@Override | ||
public synchronized void remove(@NonNull String databaseId) { | ||
instances.remove(databaseId); | ||
} | ||
|
||
@Override | ||
public void onDeleted(String firebaseAppName, FirebaseOptions options) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this method also synchronize its access to the map? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. Thanks. |
||
// Shuts down all database instances and remove them from registry map when App is deleted. | ||
for (Map.Entry<String, FirebaseFirestore> entry : instances.entrySet()) { | ||
entry.getValue().shutdownInternal(); | ||
instances.remove(entry.getKey()); | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.