Skip to content

fix: Cleaning up FirebaseApp state management #476

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

Merged
merged 1 commit into from
Sep 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/main/java/com/google/firebase/FirebaseApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ public FirebaseOptions getOptions() {
*/
@Nullable
String getProjectId() {
checkNotDeleted();

// Try to get project ID from user-specified options.
String projectId = options.getProjectId();

Expand Down Expand Up @@ -314,8 +316,10 @@ public String toString() {
}

/**
* Deletes the {@link FirebaseApp} and all its data. All calls to this {@link FirebaseApp}
* instance will throw once it has been called.
* Deletes this {@link FirebaseApp} object, and releases any local state and managed resources
* associated with it. All calls to this {@link FirebaseApp} instance will throw once this method
* has been called. This also releases any managed resources allocated by other services
* attached to this object instance (e.g. {@code FirebaseAuth}).
*
* <p>A no-op if delete was called before.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.google.firebase.internal.NonNull;

import java.util.concurrent.Callable;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ThreadFactory;

Expand Down
48 changes: 0 additions & 48 deletions src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;

import com.google.api.client.json.JsonFactory;
import com.google.api.client.util.Clock;
Expand Down Expand Up @@ -164,7 +163,6 @@ public ApiFuture<String> createCustomTokenAsync(

private CallableOperation<String, FirebaseAuthException> createCustomTokenOp(
final String uid, final Map<String, Object> developerClaims) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(uid), "uid must not be null or empty");
final FirebaseTokenFactory tokenFactory = this.tokenFactory.get();
return new CallableOperation<String, FirebaseAuthException>() {
Expand Down Expand Up @@ -208,7 +206,6 @@ public ApiFuture<String> createSessionCookieAsync(

private CallableOperation<String, FirebaseAuthException> createSessionCookieOp(
final String idToken, final SessionCookieOptions options) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(idToken), "idToken must not be null or empty");
checkNotNull(options, "options must not be null");
final FirebaseUserManager userManager = getUserManager();
Expand Down Expand Up @@ -299,7 +296,6 @@ public ApiFuture<FirebaseToken> verifyIdTokenAsync(@NonNull String idToken) {

private CallableOperation<FirebaseToken, FirebaseAuthException> verifyIdTokenOp(
final String idToken, final boolean checkRevoked) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(idToken), "ID token must not be null or empty");
final FirebaseTokenVerifier verifier = getIdTokenVerifier(checkRevoked);
return new CallableOperation<FirebaseToken, FirebaseAuthException>() {
Expand Down Expand Up @@ -380,7 +376,6 @@ public ApiFuture<FirebaseToken> verifySessionCookieAsync(String cookie, boolean

private CallableOperation<FirebaseToken, FirebaseAuthException> verifySessionCookieOp(
final String cookie, final boolean checkRevoked) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(cookie), "Session cookie must not be null or empty");
final FirebaseTokenVerifier sessionCookieVerifier = getSessionCookieVerifier(checkRevoked);
return new CallableOperation<FirebaseToken, FirebaseAuthException>() {
Expand Down Expand Up @@ -434,7 +429,6 @@ public ApiFuture<Void> revokeRefreshTokensAsync(@NonNull String uid) {
}

private CallableOperation<Void, FirebaseAuthException> revokeRefreshTokensOp(final String uid) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(uid), "uid must not be null or empty");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<Void, FirebaseAuthException>() {
Expand Down Expand Up @@ -475,7 +469,6 @@ public ApiFuture<UserRecord> getUserAsync(@NonNull String uid) {
}

private CallableOperation<UserRecord, FirebaseAuthException> getUserOp(final String uid) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(uid), "uid must not be null or empty");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<UserRecord, FirebaseAuthException>() {
Expand Down Expand Up @@ -513,7 +506,6 @@ public ApiFuture<UserRecord> getUserByEmailAsync(@NonNull String email) {

private CallableOperation<UserRecord, FirebaseAuthException> getUserByEmailOp(
final String email) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(email), "email must not be null or empty");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<UserRecord, FirebaseAuthException>() {
Expand Down Expand Up @@ -551,7 +543,6 @@ public ApiFuture<UserRecord> getUserByPhoneNumberAsync(@NonNull String phoneNumb

private CallableOperation<UserRecord, FirebaseAuthException> getUserByPhoneNumberOp(
final String phoneNumber) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(phoneNumber), "phone number must not be null or empty");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<UserRecord, FirebaseAuthException>() {
Expand Down Expand Up @@ -620,7 +611,6 @@ public ApiFuture<ListUsersPage> listUsersAsync(@Nullable String pageToken, int m

private CallableOperation<ListUsersPage, FirebaseAuthException> listUsersOp(
@Nullable final String pageToken, final int maxResults) {
checkNotDestroyed();
final FirebaseUserManager userManager = getUserManager();
final DefaultUserSource source = new DefaultUserSource(userManager, jsonFactory);
final ListUsersPage.Factory factory = new ListUsersPage.Factory(source, maxResults, pageToken);
Expand Down Expand Up @@ -661,7 +651,6 @@ public ApiFuture<UserRecord> createUserAsync(@NonNull UserRecord.CreateRequest r

private CallableOperation<UserRecord, FirebaseAuthException> createUserOp(
final UserRecord.CreateRequest request) {
checkNotDestroyed();
checkNotNull(request, "create request must not be null");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<UserRecord, FirebaseAuthException>() {
Expand Down Expand Up @@ -701,7 +690,6 @@ public ApiFuture<UserRecord> updateUserAsync(@NonNull UserRecord.UpdateRequest r

private CallableOperation<UserRecord, FirebaseAuthException> updateUserOp(
final UserRecord.UpdateRequest request) {
checkNotDestroyed();
checkNotNull(request, "update request must not be null");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<UserRecord, FirebaseAuthException>() {
Expand Down Expand Up @@ -754,7 +742,6 @@ public ApiFuture<Void> setCustomUserClaimsAsync(

private CallableOperation<Void, FirebaseAuthException> setCustomUserClaimsOp(
final String uid, final Map<String, Object> claims) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(uid), "uid must not be null or empty");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<Void, FirebaseAuthException>() {
Expand Down Expand Up @@ -793,7 +780,6 @@ public ApiFuture<Void> deleteUserAsync(String uid) {
}

private CallableOperation<Void, FirebaseAuthException> deleteUserOp(final String uid) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(uid), "uid must not be null or empty");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<Void, FirebaseAuthException>() {
Expand Down Expand Up @@ -876,7 +862,6 @@ public ApiFuture<UserImportResult> importUsersAsync(

private CallableOperation<UserImportResult, FirebaseAuthException> importUsersOp(
final List<ImportUserRecord> users, final UserImportOptions options) {
checkNotDestroyed();
final UserImportRequest request = new UserImportRequest(users, options, jsonFactory);
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<UserImportResult, FirebaseAuthException>() {
Expand Down Expand Up @@ -931,7 +916,6 @@ public ApiFuture<GetUsersResult> getUsersAsync(@NonNull Collection<UserIdentifie

private CallableOperation<GetUsersResult, FirebaseAuthException> getUsersOp(
@NonNull final Collection<UserIdentifier> identifiers) {
checkNotDestroyed();
checkNotNull(identifiers, "identifiers must not be null");
checkArgument(identifiers.size() <= FirebaseUserManager.MAX_GET_ACCOUNTS_BATCH_SIZE,
"identifiers parameter must have <= " + FirebaseUserManager.MAX_GET_ACCOUNTS_BATCH_SIZE
Expand Down Expand Up @@ -1004,7 +988,6 @@ public ApiFuture<DeleteUsersResult> deleteUsersAsync(List<String> uids) {

private CallableOperation<DeleteUsersResult, FirebaseAuthException> deleteUsersOp(
final List<String> uids) {
checkNotDestroyed();
checkNotNull(uids, "uids must not be null");
for (String uid : uids) {
UserRecord.checkUid(uid);
Expand Down Expand Up @@ -1178,7 +1161,6 @@ public ApiFuture<String> generateSignInWithEmailLinkAsync(

private CallableOperation<String, FirebaseAuthException> generateEmailActionLinkOp(
final EmailLinkType type, final String email, final ActionCodeSettings settings) {
checkNotDestroyed();
checkArgument(!Strings.isNullOrEmpty(email), "email must not be null or empty");
if (type == EmailLinkType.EMAIL_SIGNIN) {
checkNotNull(settings, "ActionCodeSettings must not be null when generating sign-in links");
Expand Down Expand Up @@ -1227,7 +1209,6 @@ public ApiFuture<OidcProviderConfig> createOidcProviderConfigAsync(

private CallableOperation<OidcProviderConfig, FirebaseAuthException>
createOidcProviderConfigOp(final OidcProviderConfig.CreateRequest request) {
checkNotDestroyed();
checkNotNull(request, "Create request must not be null.");
OidcProviderConfig.checkOidcProviderId(request.getProviderId());
final FirebaseUserManager userManager = getUserManager();
Expand Down Expand Up @@ -1271,7 +1252,6 @@ public ApiFuture<OidcProviderConfig> updateOidcProviderConfigAsync(

private CallableOperation<OidcProviderConfig, FirebaseAuthException> updateOidcProviderConfigOp(
final OidcProviderConfig.UpdateRequest request) {
checkNotDestroyed();
checkNotNull(request, "Update request must not be null.");
checkArgument(!request.getProperties().isEmpty(),
"Update request must have at least one property set.");
Expand Down Expand Up @@ -1316,7 +1296,6 @@ public ApiFuture<OidcProviderConfig> getOidcProviderConfigAsync(@NonNull String

private CallableOperation<OidcProviderConfig, FirebaseAuthException>
getOidcProviderConfigOp(final String providerId) {
checkNotDestroyed();
OidcProviderConfig.checkOidcProviderId(providerId);
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<OidcProviderConfig, FirebaseAuthException>() {
Expand Down Expand Up @@ -1400,7 +1379,6 @@ public ApiFuture<ListProviderConfigsPage<OidcProviderConfig>> listOidcProviderCo

private CallableOperation<ListProviderConfigsPage<OidcProviderConfig>, FirebaseAuthException>
listOidcProviderConfigsOp(@Nullable final String pageToken, final int maxResults) {
checkNotDestroyed();
final FirebaseUserManager userManager = getUserManager();
final DefaultOidcProviderConfigSource source = new DefaultOidcProviderConfigSource(userManager);
final ListProviderConfigsPage.Factory<OidcProviderConfig> factory =
Expand Down Expand Up @@ -1443,7 +1421,6 @@ public ApiFuture<Void> deleteOidcProviderConfigAsync(String providerId) {

private CallableOperation<Void, FirebaseAuthException> deleteOidcProviderConfigOp(
final String providerId) {
checkNotDestroyed();
OidcProviderConfig.checkOidcProviderId(providerId);
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<Void, FirebaseAuthException>() {
Expand Down Expand Up @@ -1490,7 +1467,6 @@ public ApiFuture<SamlProviderConfig> createSamlProviderConfigAsync(

private CallableOperation<SamlProviderConfig, FirebaseAuthException>
createSamlProviderConfigOp(final SamlProviderConfig.CreateRequest request) {
checkNotDestroyed();
checkNotNull(request, "Create request must not be null.");
SamlProviderConfig.checkSamlProviderId(request.getProviderId());
final FirebaseUserManager userManager = getUserManager();
Expand Down Expand Up @@ -1534,7 +1510,6 @@ public ApiFuture<SamlProviderConfig> updateSamlProviderConfigAsync(

private CallableOperation<SamlProviderConfig, FirebaseAuthException> updateSamlProviderConfigOp(
final SamlProviderConfig.UpdateRequest request) {
checkNotDestroyed();
checkNotNull(request, "Update request must not be null.");
checkArgument(!request.getProperties().isEmpty(),
"Update request must have at least one property set.");
Expand Down Expand Up @@ -1579,7 +1554,6 @@ public ApiFuture<SamlProviderConfig> getSamlProviderConfigAsync(@NonNull String

private CallableOperation<SamlProviderConfig, FirebaseAuthException>
getSamlProviderConfigOp(final String providerId) {
checkNotDestroyed();
SamlProviderConfig.checkSamlProviderId(providerId);
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<SamlProviderConfig, FirebaseAuthException>() {
Expand Down Expand Up @@ -1663,7 +1637,6 @@ public ApiFuture<ListProviderConfigsPage<SamlProviderConfig>> listSamlProviderCo

private CallableOperation<ListProviderConfigsPage<SamlProviderConfig>, FirebaseAuthException>
listSamlProviderConfigsOp(@Nullable final String pageToken, final int maxResults) {
checkNotDestroyed();
final FirebaseUserManager userManager = getUserManager();
final DefaultSamlProviderConfigSource source = new DefaultSamlProviderConfigSource(userManager);
final ListProviderConfigsPage.Factory<SamlProviderConfig> factory =
Expand Down Expand Up @@ -1706,7 +1679,6 @@ public ApiFuture<Void> deleteSamlProviderConfigAsync(String providerId) {

private CallableOperation<Void, FirebaseAuthException> deleteSamlProviderConfigOp(
final String providerId) {
checkNotDestroyed();
SamlProviderConfig.checkSamlProviderId(providerId);
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<Void, FirebaseAuthException>() {
Expand All @@ -1729,32 +1701,12 @@ <T> Supplier<T> threadSafeMemoize(final Supplier<T> supplier) {
public T get() {
checkNotNull(supplier);
synchronized (lock) {
checkNotDestroyed();
return supplier.get();
}
}
});
}

private void checkNotDestroyed() {
synchronized (lock) {
checkState(
!destroyed.get(),
"FirebaseAuth instance is no longer alive. This happens when "
+ "the parent FirebaseApp instance has been deleted.");
}
}

final void destroy() {
synchronized (lock) {
doDestroy();
destroyed.set(true);
}
}

/** Performs any additional required clean up. */
protected abstract void doDestroy();

protected abstract static class Builder<T extends Builder<T>> {

private FirebaseApp firebaseApp;
Expand Down
8 changes: 0 additions & 8 deletions src/main/java/com/google/firebase/auth/FirebaseAuth.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ public static synchronized FirebaseAuth getInstance(FirebaseApp app) {
return service.getInstance();
}

@Override
protected void doDestroy() { }

private static FirebaseAuth fromApp(final FirebaseApp app) {
return populateBuilderFromApp(builder(), app, null)
.setTenantManager(new Supplier<TenantManager>() {
Expand All @@ -88,11 +85,6 @@ private static class FirebaseAuthService extends FirebaseService<FirebaseAuth> {
FirebaseAuthService(FirebaseApp app) {
super(SERVICE_ID, FirebaseAuth.fromApp(app));
}

@Override
public void destroy() {
instance.destroy();
}
}

static Builder builder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ public ApiFuture<String> apply(FirebaseToken input) {
}, MoreExecutors.directExecutor());
}

@Override
protected void doDestroy() {
// Nothing extra needs to be destroyed.
}

static TenantAwareFirebaseAuth fromApp(FirebaseApp app, String tenantId) {
return populateBuilderFromApp(builder(), app, tenantId)
.setTenantId(tenantId)
Expand Down
8 changes: 0 additions & 8 deletions src/main/java/com/google/firebase/cloud/StorageClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,5 @@ private static class StorageClientService extends FirebaseService<StorageClient>
StorageClientService(StorageClient client) {
super(SERVICE_ID, client);
}

@Override
public void destroy() {
// NOTE: We don't explicitly tear down anything here, but public methods of StorageClient
// will now fail because calls to getOptions() and getToken() will hit FirebaseApp,
// which will throw once the app is deleted.
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ static FirebaseDatabase createForTests(
return db;
}

/**
/**
* @return The version for this build of the Firebase Database client
*/
public static String getSdkVersion() {
Expand Down Expand Up @@ -358,6 +358,10 @@ DatabaseConfig getConfig() {
return this.config;
}

/**
* Tears down the WebSocket connections and background threads started by this {@code
* FirebaseDatabase} instance thus disconnecting from the remote database.
*/
void destroy() {
synchronized (lock) {
if (destroyed.get()) {
Expand Down
7 changes: 0 additions & 7 deletions src/main/java/com/google/firebase/iid/FirebaseInstanceId.java
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,5 @@ private static class FirebaseInstanceIdService extends FirebaseService<FirebaseI
FirebaseInstanceIdService(FirebaseApp app) {
super(SERVICE_ID, new FirebaseInstanceId(app));
}

@Override
public void destroy() {
// NOTE: We don't explicitly tear down anything here, but public methods of StorageClient
// will now fail because calls to getOptions() and getToken() will hit FirebaseApp,
// which will throw once the app is deleted.
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
*
* @param <T> Type of the service
*/
public abstract class FirebaseService<T> {
public class FirebaseService<T> {

private final String id;
protected final T instance;
Expand Down Expand Up @@ -62,5 +62,7 @@ public final T getInstance() {
* Tear down this FirebaseService instance and the service object wrapped in it, cleaning up
* any allocated resources in the process.
*/
public abstract void destroy();
public void destroy() {
// Child classes can override this method to implement any service-specific cleanup logic.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -403,13 +403,6 @@ private static class FirebaseMessagingService extends FirebaseService<FirebaseMe
FirebaseMessagingService(FirebaseApp app) {
super(SERVICE_ID, FirebaseMessaging.fromApp(app));
}

@Override
public void destroy() {
// NOTE: We don't explicitly tear down anything here, but public methods of FirebaseMessaging
// will now fail because calls to getOptions() and getToken() will hit FirebaseApp,
// which will throw once the app is deleted.
}
}

private static FirebaseMessaging fromApp(final FirebaseApp app) {
Expand Down
Loading