Skip to content
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

fix(firebase-perf): Update device cache only if RC value is different from cached value #6431

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions firebase-perf/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@


# 21.0.2
* [fixed] Fixed a performance issue with shared preferences
visumickey marked this conversation as resolved.
Show resolved Hide resolved
calling `.apply()` every time a value is read from remote config (#6407)
exaby73 marked this conversation as resolved.
Show resolved Hide resolved
* [fixed] Fixed `IllegalStateException` that happened when starting a trace
before Firebase initializes.
* [changed] Updated protobuf dependency to `3.25.5` to fix
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ private boolean getIsSdkEnabled() {
// 2. If the value exists in device cache, return this value.
// 3. Otherwise, return default value.
SdkEnabled config = SdkEnabled.getInstance();
Optional<Boolean> deviceCacheValue = getDeviceCacheBoolean(config);

// 1. Reads value from Firebase Remote Config, saves this value in cache layer if fetch status
// is not failure.
Expand All @@ -230,13 +231,19 @@ private boolean getIsSdkEnabled() {
if (remoteConfigManager.isLastFetchFailed()) {
return false;
}
// b. Cache and return this value.
deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get());
return rcValue.get();

Boolean newValue = rcValue.get();
// b. Only cache and return this value if it is different from the current value.
if (deviceCacheValue == null
|| !deviceCacheValue.isAvailable()
|| deviceCacheValue.get() != newValue) {
deviceCacheManager.setValue(config.getDeviceCacheFlag(), newValue);
}

return newValue;
}

// 2. If the value exists in device cache, return this value.
Optional<Boolean> deviceCacheValue = getDeviceCacheBoolean(config);
if (deviceCacheValue.isAvailable()) {
return deviceCacheValue.get();
}
Expand All @@ -257,17 +264,23 @@ private boolean getIsSdkVersionDisabled() {
// 2. If the value exists in device cache, return this value.
// 3. Otherwise, return default value.
SdkDisabledVersions config = SdkDisabledVersions.getInstance();
Optional<String> deviceCacheValue = getDeviceCacheString(config);

// 1. Reads value from Firebase Remote Config, cache and return this value.
Optional<String> rcValue = getRemoteConfigString(config);
if (rcValue.isAvailable()) {
// Do not check FRC last fetch status because it is the most recent value device can get.
deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get());
return isFireperfSdkVersionInList(rcValue.get());
String newValue = rcValue.get();
// Only cache and return this value if it is different from the current value.
if (deviceCacheValue == null
|| !deviceCacheValue.isAvailable()
|| !deviceCacheValue.get().equals(newValue)) {
deviceCacheManager.setValue(config.getDeviceCacheFlag(), newValue);
}
return isFireperfSdkVersionInList(newValue);
}

// 2. If the value exists in device cache, return this value.
Optional<String> deviceCacheValue = getDeviceCacheString(config);
if (deviceCacheValue.isAvailable()) {
return isFireperfSdkVersionInList(deviceCacheValue.get());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,23 @@ public void getIsServiceCollectionEnabled_sdkDisabledVersionFlagNoFrc_returnDefa
verify(mockDeviceCacheManager, never()).setValue(any(), anyString());
}

@Test
public void getIsServiceCollectionEnabled_deviceCacheHasSameValueAsFrc_returnCacheValue() {
when(mockRemoteConfigManager.getBoolean(FIREBASE_PERFORMANCE_SDK_ENABLED_FRC_KEY))
.thenReturn(Optional.of(true));
when(mockDeviceCacheManager.getBoolean(FIREBASE_PERFORMANCE_SDK_ENABLED_CACHE_KEY))
.thenReturn(Optional.of(true));

when(mockDeviceCacheManager.getString(FIREBASE_PERFORMANCE_DISABLED_VERSIONS_CACHE_KEY))
.thenReturn(Optional.of(""));
when(mockRemoteConfigManager.getString(FIREBASE_PERFORMANCE_DISABLED_VERSIONS_FRC_KEY))
.thenReturn(Optional.of(""));

assertThat(testConfigResolver.getIsServiceCollectionEnabled()).isTrue();
verify(mockDeviceCacheManager, never()).setValue(any(), anyBoolean());
verify(mockDeviceCacheManager, never()).setValue(any(), anyString());
}

@Test
public void
getIsPerformanceCollectionConfigValueAvailable_noDeviceCacheNoRemoteConfig_returnsFalse() {
Expand Down
Loading