From 0a7d4c75860b522484f3432323572543e2219974 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 27 May 2020 13:02:08 -0700 Subject: [PATCH 1/2] fix: Removed unused FirebaseAppStore abstraction --- .../java/com/google/firebase/FirebaseApp.java | 29 ++----- .../firebase/internal/FirebaseAppStore.java | 78 ------------------- .../internal/FirebaseAppStoreTest.java | 11 --- .../firebase/testing/FirebaseAppRule.java | 2 - 4 files changed, 6 insertions(+), 114 deletions(-) delete mode 100644 src/main/java/com/google/firebase/internal/FirebaseAppStore.java diff --git a/src/main/java/com/google/firebase/FirebaseApp.java b/src/main/java/com/google/firebase/FirebaseApp.java index 28efa9d47..c77697237 100644 --- a/src/main/java/com/google/firebase/FirebaseApp.java +++ b/src/main/java/com/google/firebase/FirebaseApp.java @@ -35,7 +35,6 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.firebase.internal.FirebaseAppStore; import com.google.firebase.internal.FirebaseScheduledExecutor; import com.google.firebase.internal.FirebaseService; import com.google.firebase.internal.ListenableFuture2ApiFuture; @@ -47,10 +46,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; @@ -219,21 +216,16 @@ public static FirebaseApp initializeApp(FirebaseOptions options, String name) { static FirebaseApp initializeApp(FirebaseOptions options, String name, TokenRefresher.Factory tokenRefresherFactory) { - FirebaseAppStore appStore = FirebaseAppStore.initialize(); String normalizedName = normalize(name); - final FirebaseApp firebaseApp; synchronized (appsLock) { checkState( !instances.containsKey(normalizedName), "FirebaseApp name " + normalizedName + " already exists!"); - firebaseApp = new FirebaseApp(normalizedName, options, tokenRefresherFactory); + FirebaseApp firebaseApp = new FirebaseApp(normalizedName, options, tokenRefresherFactory); instances.put(normalizedName, firebaseApp); + return firebaseApp; } - - appStore.persistApp(firebaseApp); - - return firebaseApp; } @VisibleForTesting @@ -249,19 +241,15 @@ static void clearInstancesForTest() { } private static List getAllAppNames() { - Set allAppNames = new HashSet<>(); + List allAppNames = new ArrayList<>(); synchronized (appsLock) { for (FirebaseApp app : instances.values()) { allAppNames.add(app.getName()); } - FirebaseAppStore appStore = FirebaseAppStore.getInstance(); - if (appStore != null) { - allAppNames.addAll(appStore.getAllPersistedAppNames()); - } } - List sortedNameList = new ArrayList<>(allAppNames); - Collections.sort(sortedNameList); - return sortedNameList; + + Collections.sort(allAppNames); + return ImmutableList.copyOf(allAppNames); } /** Normalizes the app name. */ @@ -357,11 +345,6 @@ public void delete() { synchronized (appsLock) { instances.remove(name); } - - FirebaseAppStore appStore = FirebaseAppStore.getInstance(); - if (appStore != null) { - appStore.removeApp(name); - } } private void checkNotDeleted() { diff --git a/src/main/java/com/google/firebase/internal/FirebaseAppStore.java b/src/main/java/com/google/firebase/internal/FirebaseAppStore.java deleted file mode 100644 index 778295655..000000000 --- a/src/main/java/com/google/firebase/internal/FirebaseAppStore.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2017 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.firebase.internal; - -import com.google.common.annotations.VisibleForTesting; -import com.google.firebase.FirebaseApp; -import com.google.firebase.FirebaseOptions; - -import java.util.Collections; -import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; - -/** No-op base class of FirebaseAppStore. */ -public class FirebaseAppStore { - - private static final AtomicReference sInstance = new AtomicReference<>(); - - FirebaseAppStore() {} - - @Nullable - public static FirebaseAppStore getInstance() { - return sInstance.get(); - } - - // TODO: reenable persistence. See b/28158809. - public static FirebaseAppStore initialize() { - sInstance.compareAndSet(null /* expected */, new FirebaseAppStore()); - return sInstance.get(); - } - - /** - * @hide - */ - public static void setInstanceForTest(FirebaseAppStore firebaseAppStore) { - sInstance.set(firebaseAppStore); - } - - @VisibleForTesting - public static void clearInstanceForTest() { - FirebaseAppStore instance = sInstance.get(); - if (instance != null) { - instance.resetStore(); - } - sInstance.set(null); - } - - /** The returned set is mutable. */ - public Set getAllPersistedAppNames() { - return Collections.emptySet(); - } - - public void persistApp(@NonNull FirebaseApp app) {} - - public void removeApp(@NonNull String name) {} - - /** - * @return The restored {@link FirebaseOptions}, or null if it doesn't exist. - */ - public FirebaseOptions restoreAppOptions(@NonNull String name) { - return null; - } - - protected void resetStore() {} -} diff --git a/src/test/java/com/google/firebase/internal/FirebaseAppStoreTest.java b/src/test/java/com/google/firebase/internal/FirebaseAppStoreTest.java index c9170daa3..b6b502fdc 100644 --- a/src/test/java/com/google/firebase/internal/FirebaseAppStoreTest.java +++ b/src/test/java/com/google/firebase/internal/FirebaseAppStoreTest.java @@ -16,8 +16,6 @@ package com.google.firebase.internal; -import static org.junit.Assert.assertFalse; - import com.google.auth.oauth2.GoogleCredentials; import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseOptions; @@ -71,13 +69,4 @@ public void incompatibleDefaultAppInitializedDoesntThrow() throws IOException { .build(); FirebaseApp.initializeApp(options); } - - @Test - public void persistenceDisabled() { - String name = "myApp"; - FirebaseApp.initializeApp(ALL_VALUES_OPTIONS, name); - TestOnlyImplFirebaseTrampolines.clearInstancesForTest(); - FirebaseAppStore appStore = FirebaseAppStore.getInstance(); - assertFalse(appStore.getAllPersistedAppNames().contains(name)); - } } diff --git a/src/test/java/com/google/firebase/testing/FirebaseAppRule.java b/src/test/java/com/google/firebase/testing/FirebaseAppRule.java index 28123293f..1049a1f2f 100644 --- a/src/test/java/com/google/firebase/testing/FirebaseAppRule.java +++ b/src/test/java/com/google/firebase/testing/FirebaseAppRule.java @@ -17,7 +17,6 @@ package com.google.firebase.testing; import com.google.firebase.TestOnlyImplFirebaseTrampolines; -import com.google.firebase.internal.FirebaseAppStore; import org.junit.rules.TestRule; import org.junit.runner.Description; import org.junit.runners.model.Statement; @@ -42,6 +41,5 @@ public void evaluate() throws Throwable { private void resetState() { TestOnlyImplFirebaseTrampolines.clearInstancesForTest(); - FirebaseAppStore.clearInstanceForTest(); } } From cb77948868aedd0c7d68e57761ea5ff8b91a3459 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 27 May 2020 14:42:29 -0700 Subject: [PATCH 2/2] Using the keySet of App instances to populate the app names list --- src/main/java/com/google/firebase/FirebaseApp.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/firebase/FirebaseApp.java b/src/main/java/com/google/firebase/FirebaseApp.java index c77697237..38482541c 100644 --- a/src/main/java/com/google/firebase/FirebaseApp.java +++ b/src/main/java/com/google/firebase/FirebaseApp.java @@ -241,11 +241,9 @@ static void clearInstancesForTest() { } private static List getAllAppNames() { - List allAppNames = new ArrayList<>(); + List allAppNames; synchronized (appsLock) { - for (FirebaseApp app : instances.values()) { - allAppNames.add(app.getName()); - } + allAppNames = new ArrayList<>(instances.keySet()); } Collections.sort(allAppNames);