Skip to content

Commit 41aa55a

Browse files
authored
Update logging flag to match recommended style and execution. (#2392)
* Update logging flag to match recommended style and execution. * Update logging flag to match recommended style and execution. * Updating statCollection name based on review feedback * Updating to Boolean and handling null edge case for collection method.. * fix formatting post merge
1 parent 4a4fbd2 commit 41aa55a

File tree

5 files changed

+73
-18
lines changed

5 files changed

+73
-18
lines changed

firebase-ml-modeldownloader/api.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ package com.google.firebase.ml.modeldownloader {
6262
method @NonNull public com.google.android.gms.tasks.Task<com.google.firebase.ml.modeldownloader.CustomModel> getModel(@NonNull String, @NonNull com.google.firebase.ml.modeldownloader.DownloadType, @Nullable com.google.firebase.ml.modeldownloader.CustomModelDownloadConditions);
6363
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Long> getModelDownloadId(@NonNull String, @Nullable com.google.android.gms.tasks.Task<com.google.firebase.ml.modeldownloader.CustomModel>);
6464
method @NonNull public com.google.android.gms.tasks.Task<java.util.Set<com.google.firebase.ml.modeldownloader.CustomModel>> listDownloadedModels();
65-
method public void setStatsCollectionEnabled(boolean);
65+
method public void setModelDownloaderCollectionEnabled(@Nullable Boolean);
6666
}
6767

6868
}

firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloader.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,9 +484,10 @@ private boolean deleteModelDetails(@NonNull String modelName) {
484484
*
485485
* <p>By default the logging matches the Firebase wide data collection switch.
486486
*
487-
* @param enabled - is statistics logging enabled
487+
* @param enabled - is logging enabled, set to null to use Firebase wide data collection switch
488+
* (default)
488489
*/
489-
public void setStatsCollectionEnabled(boolean enabled) {
490+
public void setModelDownloaderCollectionEnabled(@Nullable Boolean enabled) {
490491
sharedPreferencesUtil.setCustomModelStatsCollectionEnabled(enabled);
491492
}
492493

firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/SharedPreferencesUtil.java

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -253,25 +253,38 @@ public synchronized Set<CustomModel> listDownloadedModels() {
253253
/**
254254
* Should Firelog logging be enabled.
255255
*
256-
* @return whether or not firelog events should be logged. Default to true.
256+
* @return whether or not firelog events should be logged. If not specifically set, defaults to fi
257257
*/
258258
public synchronized boolean getCustomModelStatsCollectionFlag() {
259-
return getSharedPreferences()
260-
.getBoolean(
261-
String.format(EVENT_LOGGING_ENABLED_PATTERN, CUSTOM_MODEL_LIB, persistenceKey), true);
259+
if (getSharedPreferences()
260+
.contains(String.format(EVENT_LOGGING_ENABLED_PATTERN, CUSTOM_MODEL_LIB, persistenceKey))) {
261+
return getSharedPreferences()
262+
.getBoolean(
263+
String.format(EVENT_LOGGING_ENABLED_PATTERN, CUSTOM_MODEL_LIB, persistenceKey), true);
264+
}
265+
return firebaseApp.isDataCollectionDefaultEnabled();
262266
}
263267

264268
/**
265-
* Set whether firelog logging should be enabled. When not explicitly set, the default is true.
269+
* Set whether firelog logging should be enabled. When not explicitly set, uses the Firebase wide
270+
* data collection switch.
266271
*
267272
* @param enable - False to turn off logging. True to turn on logging.
268273
*/
269-
public synchronized void setCustomModelStatsCollectionEnabled(boolean enable) {
270-
getSharedPreferences()
271-
.edit()
272-
.putBoolean(
273-
String.format(EVENT_LOGGING_ENABLED_PATTERN, CUSTOM_MODEL_LIB, persistenceKey), enable)
274-
.apply();
274+
public synchronized void setCustomModelStatsCollectionEnabled(Boolean enable) {
275+
if (enable == null) {
276+
getSharedPreferences()
277+
.edit()
278+
.remove(String.format(EVENT_LOGGING_ENABLED_PATTERN, CUSTOM_MODEL_LIB, persistenceKey))
279+
.commit();
280+
} else {
281+
getSharedPreferences()
282+
.edit()
283+
.putBoolean(
284+
String.format(EVENT_LOGGING_ENABLED_PATTERN, CUSTOM_MODEL_LIB, persistenceKey),
285+
enable)
286+
.commit();
287+
}
275288
}
276289

277290
/**

firebase-ml-modeldownloader/src/test/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloaderTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -755,11 +755,14 @@ public void deleteDownloadedModel() throws Exception {
755755
@Test
756756
public void setStatsCollectionEnabled() {
757757
doNothing().when(mockPrefs).setCustomModelStatsCollectionEnabled(anyBoolean());
758-
firebaseModelDownloader.setStatsCollectionEnabled(true);
758+
firebaseModelDownloader.setModelDownloaderCollectionEnabled(true);
759759
verify(mockPrefs, times(1)).setCustomModelStatsCollectionEnabled(eq(true));
760760

761-
firebaseModelDownloader.setStatsCollectionEnabled(false);
761+
firebaseModelDownloader.setModelDownloaderCollectionEnabled(false);
762762
verify(mockPrefs, times(1)).setCustomModelStatsCollectionEnabled(eq(false));
763+
764+
firebaseModelDownloader.setModelDownloaderCollectionEnabled(null);
765+
verify(mockPrefs, times(1)).setCustomModelStatsCollectionEnabled(eq(null));
763766
}
764767

765768
@Test

firebase-ml-modeldownloader/src/test/java/com/google/firebase/ml/modeldownloader/internal/SharedPreferencesUtilTest.java

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,21 @@ public class SharedPreferencesUtilTest {
4646
private static final CustomModel CUSTOM_MODEL_DOWNLOADING =
4747
new CustomModel(MODEL_NAME, MODEL_HASH, 100, 986);
4848
private SharedPreferencesUtil sharedPreferencesUtil;
49+
private FirebaseApp app;
4950

5051
@Before
5152
public void setUp() {
5253
MockitoAnnotations.initMocks(this);
5354
FirebaseApp.clearInstancesForTest();
54-
FirebaseApp app =
55+
app =
5556
FirebaseApp.initializeApp(
5657
ApplicationProvider.getApplicationContext(),
5758
new FirebaseOptions.Builder()
5859
.setApplicationId("1:123456789:android:abcdef")
5960
.setProjectId(TEST_PROJECT_ID)
6061
.build());
6162

63+
app.setDataCollectionDefaultEnabled(Boolean.TRUE);
6264
// default sharedPreferenceUtil
6365
sharedPreferencesUtil = new SharedPreferencesUtil(app);
6466
assertNotNull(sharedPreferencesUtil);
@@ -172,10 +174,33 @@ public void listDownloadedModels_multipleModels() {
172174
}
173175

174176
@Test
175-
public void getCustomModelStatsCollectionFlag_defaultTrue() {
177+
public void getCustomModelStatsCollectionFlag_defaultFirebaseAppTrue() {
178+
assertEquals(
179+
sharedPreferencesUtil.getCustomModelStatsCollectionFlag(),
180+
app.isDataCollectionDefaultEnabled());
176181
assertTrue(sharedPreferencesUtil.getCustomModelStatsCollectionFlag());
177182
}
178183

184+
@Test
185+
public void getCustomModelStatsCollectionFlag_defaultFirebaseAppFalse() {
186+
app.setDataCollectionDefaultEnabled(Boolean.FALSE);
187+
// default sharedPreferenceUtil
188+
SharedPreferencesUtil disableLogUtil = new SharedPreferencesUtil(app);
189+
assertEquals(
190+
disableLogUtil.getCustomModelStatsCollectionFlag(), app.isDataCollectionDefaultEnabled());
191+
assertFalse(disableLogUtil.getCustomModelStatsCollectionFlag());
192+
}
193+
194+
@Test
195+
public void getCustomModelStatsCollectionFlag_overrideFirebaseAppFalse() {
196+
app.setDataCollectionDefaultEnabled(Boolean.FALSE);
197+
// default sharedPreferenceUtil
198+
SharedPreferencesUtil sharedPreferencesUtil2 = new SharedPreferencesUtil(app);
199+
sharedPreferencesUtil2.setCustomModelStatsCollectionEnabled(true);
200+
assertEquals(sharedPreferencesUtil2.getCustomModelStatsCollectionFlag(), true);
201+
assertTrue(sharedPreferencesUtil2.getCustomModelStatsCollectionFlag());
202+
}
203+
179204
@Test
180205
public void setCustomModelStatsCollectionFlag_updates() {
181206
sharedPreferencesUtil.setCustomModelStatsCollectionEnabled(false);
@@ -184,6 +209,19 @@ public void setCustomModelStatsCollectionFlag_updates() {
184209
assertTrue(sharedPreferencesUtil.getCustomModelStatsCollectionFlag());
185210
}
186211

212+
@Test
213+
public void setCustomModelStatsCollectionFlag_nullUpdates() {
214+
sharedPreferencesUtil.setCustomModelStatsCollectionEnabled(false);
215+
sharedPreferencesUtil.setCustomModelStatsCollectionEnabled(null);
216+
assertEquals(
217+
sharedPreferencesUtil.getCustomModelStatsCollectionFlag(),
218+
app.isDataCollectionDefaultEnabled());
219+
app.setDataCollectionDefaultEnabled(Boolean.FALSE);
220+
assertFalse(sharedPreferencesUtil.getCustomModelStatsCollectionFlag());
221+
app.setDataCollectionDefaultEnabled(Boolean.TRUE);
222+
assertTrue(sharedPreferencesUtil.getCustomModelStatsCollectionFlag());
223+
}
224+
187225
@Test
188226
public void getModelDownloadBeginTimeMs_default0() {
189227
assertEquals(sharedPreferencesUtil.getModelDownloadBeginTimeMs(CUSTOM_MODEL_DOWNLOADING), 0L);

0 commit comments

Comments
 (0)