Skip to content

Removing the isPerformanceMonitoringEnabled check in FirebasePerfProvider and moving the getSharedPreferences to a separate thread. #2458

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
Feb 23, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ public static void clearInstance() {
configResolver = null;
}

@VisibleForTesting
public void setDeviceCacheManager(DeviceCacheManager deviceCacheManager) {
this.deviceCacheManager = deviceCacheManager;
}

public void setContentProviderContext(Context context) {
setApplicationContext(context.getApplicationContext());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import com.google.firebase.perf.logging.AndroidLogger;
import com.google.firebase.perf.util.Constants;
import com.google.firebase.perf.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

/**
* Utilizes platform supported APIs for storing and retrieving Firebase Performance related
Expand All @@ -40,13 +42,19 @@ public class DeviceCacheManager {
// The key-value pairs are written to XML files that persist across user sessions, even if the
// app is killed.
// https://developer.android.com/guide/topics/data/data-storage.html#pref
private SharedPreferences sharedPref;
private volatile SharedPreferences sharedPref;

private DeviceCacheManager() {}
// Used for retrieving the shared preferences on a separate thread.
private ExecutorService serialExecutor;

@VisibleForTesting
public DeviceCacheManager(ExecutorService serialExecutor) {
this.serialExecutor = serialExecutor;
}

public static synchronized DeviceCacheManager getInstance() {
if (instance == null) {
instance = new DeviceCacheManager();
instance = new DeviceCacheManager(Executors.newSingleThreadExecutor());
}
return instance;
}
Expand All @@ -56,9 +64,19 @@ public static void clearInstance() {
instance = null;
}

/**
* Triggers a getSharedPreferences call in a separate thread if shared preferences is not set.
* This method returns immediately without waiting for the getSharedPreferences call to be
* complete.
*/
public synchronized void setContext(Context context) {
if (sharedPref == null && context != null) {
this.sharedPref = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE);
serialExecutor.execute(
() -> {
if (sharedPref == null && context != null) {
this.sharedPref = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE);
}
});
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,11 @@ public void attachInfo(Context context, ProviderInfo info) {
ConfigResolver configResolver = ConfigResolver.getInstance();
configResolver.setContentProviderContext(getContext());

if (configResolver.isPerformanceMonitoringEnabled()) {
AppStateMonitor.getInstance().registerActivityLifecycleCallbacks(getContext());
AppStartTrace appStartTrace = AppStartTrace.getInstance();
appStartTrace.registerActivityLifecycleCallbacks(getContext());
AppStateMonitor.getInstance().registerActivityLifecycleCallbacks(getContext());
AppStartTrace appStartTrace = AppStartTrace.getInstance();
appStartTrace.registerActivityLifecycleCallbacks(getContext());

mHandler.post(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace));
}
mHandler.post(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace));

// In the case of cold start, we create a session and start collecting gauges as early as
// possible.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.firebase.perf.util.Constants;
import com.google.firebase.perf.util.ImmutableBundle;
import com.google.firebase.remoteconfig.RemoteConfigComponent;
import com.google.testing.timing.FakeDirectExecutorService;
import java.util.Map;
import org.junit.After;
import org.junit.Assert;
Expand Down Expand Up @@ -68,6 +69,8 @@ public class FirebasePerformanceTest {

@Rule public MockitoRule initRule = MockitoJUnit.rule();

private FakeDirectExecutorService fakeDirectExecutorService;

@Before
public void setUp() throws NameNotFoundException {
FirebaseOptions options =
Expand Down Expand Up @@ -98,6 +101,7 @@ public void setUp() throws NameNotFoundException {
spyConfigResolver = spy(ConfigResolver.getInstance());

spyGaugeManager = spy(GaugeManager.getInstance());
fakeDirectExecutorService = new FakeDirectExecutorService();
}

@After
Expand Down Expand Up @@ -542,8 +546,9 @@ private FirebasePerformance initializeFirebasePerformancePreferences(
Provider<RemoteConfigComponent> firebaseRemoteConfigProvider,
Provider<TransportFactory> transportFactoryProvider) {
if (sharedPreferencesEnabledDisabledKey != null) {
DeviceCacheManager.getInstance()
.setValue(Constants.ENABLE_DISABLE, sharedPreferencesEnabledDisabledKey);
DeviceCacheManager deviceCacheManager = new DeviceCacheManager(fakeDirectExecutorService);
deviceCacheManager.setContext(ApplicationProvider.getApplicationContext());
deviceCacheManager.setValue(Constants.ENABLE_DISABLE, sharedPreferencesEnabledDisabledKey);
}

Bundle bundle = new Bundle();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.firebase.FirebaseApp;
import com.google.firebase.perf.FirebasePerformanceTestBase;
import com.google.testing.timing.FakeScheduledExecutorService;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -28,27 +29,32 @@
public final class DeviceCacheManagerTest extends FirebasePerformanceTestBase {

private DeviceCacheManager deviceCacheManager;
private FakeScheduledExecutorService fakeScheduledExecutorService;

@Before
public void setUp() {
deviceCacheManager = DeviceCacheManager.getInstance();
fakeScheduledExecutorService = new FakeScheduledExecutorService();
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
}

@Test
public void getBoolean_valueIsNotSet_returnsEmpty() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();

assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
}

@Test
public void getBoolean_contextAndValueNotSet_returnsEmpty() {
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
}

@Test
public void getBoolean_valueIsSet_returnsSetValue() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", true);

assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();
Expand All @@ -57,7 +63,7 @@ public void getBoolean_valueIsSet_returnsSetValue() {
@Test
public void clear_setBooleanThenCleared_returnsEmpty() {
deviceCacheManager.setContext(context);

fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", true);

assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();
Expand All @@ -70,14 +76,17 @@ public void clear_setBooleanThenCleared_returnsEmpty() {
public void getBoolean_firebaseAppNotExist_returnsEmpty() {
DeviceCacheManager.clearInstance();
FirebaseApp.clearInstancesForTest();
deviceCacheManager = DeviceCacheManager.getInstance();
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
deviceCacheManager.setValue("some_key", true);

assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
}

@Test
public void setValueBoolean_setTwice_canGetLatestValue() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", true);
assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();

Expand All @@ -86,10 +95,10 @@ public void setValueBoolean_setTwice_canGetLatestValue() {
}

@Test
public void setValueBoolean_contextNotSet_canGetValue() {
public void setValueBoolean_contextNotSet_returnsEmpty() {
deviceCacheManager.setValue("some_key", true);

assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();
assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
}

@Test
Expand All @@ -100,6 +109,7 @@ public void setValueBoolean_keyIsNull_returnsFalse() {
@Test
public void getString_valueIsNotSet_returnsEmpty() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();

assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
}
Expand All @@ -112,23 +122,26 @@ public void getString_contextAndValueNotSet_returnsEmpty() {
@Test
public void getString_valueIsSet_returnsSetValue() {
deviceCacheManager.setContext(context);
deviceCacheManager.setValue("some_key", "speicalValue");
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", "specialValue");

assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("speicalValue");
assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("specialValue");
}

@Test
public void getString_firebaseAppNotExist_returnsEmpty() {
DeviceCacheManager.clearInstance();
FirebaseApp.clearInstancesForTest();
deviceCacheManager = DeviceCacheManager.getInstance();
deviceCacheManager.setValue("some_key", "speicalValue");
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
deviceCacheManager.setValue("some_key", "specialValue");

assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
}

@Test
public void setValueString_setTwice_canGetLatestValue() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", "EarliestValue");
assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("EarliestValue");

Expand All @@ -137,40 +150,47 @@ public void setValueString_setTwice_canGetLatestValue() {
}

@Test
public void setValueString_contextNotSet_canGetValue() {
public void setValueString_contextNotSet_returnsEmpty() {
deviceCacheManager.setValue("some_key", "newValue");
assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("newValue");
assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
}

@Test
public void setValueString_setNullString_returnsEmpty() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", null);
assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
}

@Test
public void setValueString_keyIsNull_returnsFalse() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
assertThat(deviceCacheManager.setValue(null, "value")).isFalse();
}

@Test
public void getFloat_valueIsNotSet_returnsEmpty() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();

assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
}

@Test
public void getFloat_contextAndValueNotSet_returnsEmpty() {
DeviceCacheManager.clearInstance();
deviceCacheManager = DeviceCacheManager.getInstance();
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);

assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
}

@Test
public void getFloat_valueIsSet_returnsSetValue() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", 1.2f);

assertThat(deviceCacheManager.getFloat("some_key").get()).isEqualTo(1.2f);
Expand All @@ -180,14 +200,17 @@ public void getFloat_valueIsSet_returnsSetValue() {
public void getFloat_firebaseAppNotExist_returnsEmpty() {
DeviceCacheManager.clearInstance();
FirebaseApp.clearInstancesForTest();
deviceCacheManager = DeviceCacheManager.getInstance();
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
deviceCacheManager.setValue("some_key", 1.2f);

assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
}

@Test
public void setValueFloat_setTwice_canGetLatestValue() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", 1.01f);
assertThat(deviceCacheManager.getFloat("some_key").get()).isEqualTo(1.01f);

Expand All @@ -196,36 +219,42 @@ public void setValueFloat_setTwice_canGetLatestValue() {
}

@Test
public void setValueFloat_contextNotSet_canGetValue() {
public void setValueFloat_contextNotSet_returnsEmpty() {
deviceCacheManager.setValue("some_key", 100.0f);
assertThat(deviceCacheManager.getFloat("some_key").get()).isEqualTo(100.0f);
assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
}

@Test
public void setValueFloat_keyIsNull_returnsFalse() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
assertThat(deviceCacheManager.setValue(null, 10.0f)).isFalse();
}

@Test
public void getLong_valueIsNotSet_returnsEmpty() {
DeviceCacheManager.clearInstance();
deviceCacheManager = DeviceCacheManager.getInstance();
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();

assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
}

@Test
public void getLong_contextAndValueNotSet_returnsEmpty() {
DeviceCacheManager.clearInstance();
deviceCacheManager = DeviceCacheManager.getInstance();
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);

assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
}

@Test
public void getLong_valueIsSet_returnsSetValue() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", 1L);

assertThat(deviceCacheManager.getLong("some_key").get()).isEqualTo(1L);
Expand All @@ -235,14 +264,16 @@ public void getLong_valueIsSet_returnsSetValue() {
public void getLong_firebaseAppNotExist_returnsEmpty() {
DeviceCacheManager.clearInstance();
FirebaseApp.clearInstancesForTest();
deviceCacheManager = DeviceCacheManager.getInstance();
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
deviceCacheManager.setValue("some_key", 1L);

assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
}

@Test
public void setValueLong_setTwice_canGetLatestValue() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
deviceCacheManager.setValue("some_key", 2L);
assertThat(deviceCacheManager.getLong("some_key").get()).isEqualTo(2L);

Expand All @@ -251,13 +282,16 @@ public void setValueLong_setTwice_canGetLatestValue() {
}

@Test
public void setValueLong_contextNotSet_canGetValue() {
public void setValueLong_contextNotSet_returnsEmpty() {
deviceCacheManager.setValue("some_key", 100L);
assertThat(deviceCacheManager.getLong("some_key").get()).isEqualTo(100L);
// The key is not set if the shared preference is not fetched and available.
assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
}

@Test
public void setValueLong_keyIsNull_returnsFalse() {
deviceCacheManager.setContext(context);
fakeScheduledExecutorService.runAll();
assertThat(deviceCacheManager.setValue(null, 10.0f)).isFalse();
}
}
Loading