Skip to content

Commit b03b87c

Browse files
committed
fix(firebase-perf): Update device cache only if RC value is different from cached value
1 parent 442bab2 commit b03b87c

File tree

2 files changed

+33
-7
lines changed

2 files changed

+33
-7
lines changed

firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ private boolean getIsSdkEnabled() {
221221
// 2. If the value exists in device cache, return this value.
222222
// 3. Otherwise, return default value.
223223
SdkEnabled config = SdkEnabled.getInstance();
224+
Optional<Boolean> deviceCacheValue = getDeviceCacheBoolean(config);
224225

225226
// 1. Reads value from Firebase Remote Config, saves this value in cache layer if fetch status
226227
// is not failure.
@@ -230,13 +231,17 @@ private boolean getIsSdkEnabled() {
230231
if (remoteConfigManager.isLastFetchFailed()) {
231232
return false;
232233
}
233-
// b. Cache and return this value.
234-
deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get());
235-
return rcValue.get();
234+
235+
Boolean newValue = rcValue.get();
236+
// b. Only cache and return this value if it is different from the current value.
237+
if (!deviceCacheValue.isAvailable() || deviceCacheValue.get() != newValue) {
238+
deviceCacheManager.setValue(config.getDeviceCacheFlag(), newValue);
239+
}
240+
241+
return newValue;
236242
}
237243

238244
// 2. If the value exists in device cache, return this value.
239-
Optional<Boolean> deviceCacheValue = getDeviceCacheBoolean(config);
240245
if (deviceCacheValue.isAvailable()) {
241246
return deviceCacheValue.get();
242247
}
@@ -257,17 +262,21 @@ private boolean getIsSdkVersionDisabled() {
257262
// 2. If the value exists in device cache, return this value.
258263
// 3. Otherwise, return default value.
259264
SdkDisabledVersions config = SdkDisabledVersions.getInstance();
265+
Optional<String> deviceCacheValue = getDeviceCacheString(config);
260266

261267
// 1. Reads value from Firebase Remote Config, cache and return this value.
262268
Optional<String> rcValue = getRemoteConfigString(config);
263269
if (rcValue.isAvailable()) {
264270
// Do not check FRC last fetch status because it is the most recent value device can get.
265-
deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get());
266-
return isFireperfSdkVersionInList(rcValue.get());
271+
String newValue = rcValue.get();
272+
// Only cache and return this value if it is different from the current value.
273+
if (!deviceCacheValue.isAvailable() || !deviceCacheValue.get().equals(newValue)) {
274+
deviceCacheManager.setValue(config.getDeviceCacheFlag(), newValue);
275+
}
276+
return isFireperfSdkVersionInList(newValue);
267277
}
268278

269279
// 2. If the value exists in device cache, return this value.
270-
Optional<String> deviceCacheValue = getDeviceCacheString(config);
271280
if (deviceCacheValue.isAvailable()) {
272281
return isFireperfSdkVersionInList(deviceCacheValue.get());
273282
}

firebase-perf/src/test/java/com/google/firebase/perf/config/ConfigResolverTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,23 @@ public void getIsServiceCollectionEnabled_sdkDisabledVersionFlagNoFrc_returnDefa
280280
verify(mockDeviceCacheManager, never()).setValue(any(), anyString());
281281
}
282282

283+
@Test
284+
public void getIsServiceCollectionEnabled_deviceCacheHasSameValueAsFrc_returnCacheValue() {
285+
when(mockRemoteConfigManager.getBoolean(FIREBASE_PERFORMANCE_SDK_ENABLED_FRC_KEY))
286+
.thenReturn(Optional.of(true));
287+
when(mockDeviceCacheManager.getBoolean(FIREBASE_PERFORMANCE_SDK_ENABLED_CACHE_KEY))
288+
.thenReturn(Optional.of(true));
289+
290+
when(mockDeviceCacheManager.getString(FIREBASE_PERFORMANCE_DISABLED_VERSIONS_CACHE_KEY))
291+
.thenReturn(Optional.of(""));
292+
when(mockRemoteConfigManager.getString(FIREBASE_PERFORMANCE_DISABLED_VERSIONS_FRC_KEY))
293+
.thenReturn(Optional.of(""));
294+
295+
assertThat(testConfigResolver.getIsServiceCollectionEnabled()).isTrue();
296+
verify(mockDeviceCacheManager, never()).setValue(any(), anyBoolean());
297+
verify(mockDeviceCacheManager, never()).setValue(any(), anyString());
298+
}
299+
283300
@Test
284301
public void
285302
getIsPerformanceCollectionConfigValueAvailable_noDeviceCacheNoRemoteConfig_returnsFalse() {

0 commit comments

Comments
 (0)