From 2d6bf057f8cce1e1e29be031e38213dd0a4667b8 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Fri, 23 Sep 2016 17:26:41 -0700 Subject: [PATCH] Avoid over logging preference changes. When preference screen is created all prefence goes through SharedPreferenceLogger once. We don't want to log it as preference change because it's just initial display. Subsequent preference logger calls should result in logging. Bug: 30914916 Test: make RunSettingsRoboTests Change-Id: Icaee9137c55555f4db7839ff0381a13ccd420190 --- .../SharedPreferencesLogger.java | 13 ++++- .../SharedPreferenceLoggerTest.java | 53 ++++++++++++++----- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java index 6a67e45477d..101f1b588c4 100644 --- a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java +++ b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java @@ -21,6 +21,7 @@ import android.content.SharedPreferences; import android.content.pm.PackageManager; import android.os.AsyncTask; import android.text.TextUtils; +import android.util.ArraySet; import com.android.internal.logging.MetricsProto.MetricsEvent; import com.android.settings.overlay.FeatureFactory; @@ -33,11 +34,13 @@ public class SharedPreferencesLogger implements SharedPreferences { private final String mTag; private final Context mContext; private final MetricsFeatureProvider mMetricsFeature; + private final Set mPreferenceKeySet; public SharedPreferencesLogger(Context context, String tag) { mContext = context; mTag = tag; mMetricsFeature = FeatureFactory.getFactory(context).getMetricsFeatureProvider(); + mPreferenceKeySet = new ArraySet<>(); } @Override @@ -96,7 +99,15 @@ public class SharedPreferencesLogger implements SharedPreferences { } private void logValue(String key, String value) { - mMetricsFeature.count(mContext, mTag + "/" + key + "|" + value, 1); + final String prefKey = mTag + "/" + key; + if (!mPreferenceKeySet.contains(prefKey)) { + // Pref key doesn't exist in set, this is initial display so we skip metrics but + // keeps track of this key. + mPreferenceKeySet.add(prefKey); + return; + } + // Pref key exists in set, log it's change in metrics. + mMetricsFeature.count(mContext, prefKey + "|" + value, 1); } private void logPackageName(String key, String value) { diff --git a/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java b/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java index 11f2784f545..a3e0e45c7f1 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java @@ -16,6 +16,7 @@ package com.android.settings.core.instrumentation; import android.content.Context; +import android.content.SharedPreferences; import com.android.settings.TestConfig; import com.android.settings.overlay.FeatureFactory; @@ -32,7 +33,7 @@ import org.robolectric.shadows.ShadowApplication; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @RunWith(RobolectricTestRunner.class) @@ -62,27 +63,53 @@ public class SharedPreferenceLoggerTest { } @Test - public void putInt_shouldLogCount() { - mSharedPrefLogger.edit().putInt(TEST_KEY, 1); - verify(mLogWriter).count(any(Context.class), anyString(), anyInt()); + public void putInt_shouldNotLogInitialPut() { + final SharedPreferences.Editor editor = mSharedPrefLogger.edit(); + editor.putInt(TEST_KEY, 1); + editor.putInt(TEST_KEY, 1); + editor.putInt(TEST_KEY, 1); + editor.putInt(TEST_KEY, 2); + editor.putInt(TEST_KEY, 2); + editor.putInt(TEST_KEY, 2); + editor.putInt(TEST_KEY, 2); + + verify(mLogWriter, times(6)).count(any(Context.class), anyString(), anyInt()); } @Test - public void putBoolean_shouldLogCount() { - mSharedPrefLogger.edit().putBoolean(TEST_KEY, true); - verify(mLogWriter).count(any(Context.class), anyString(), anyInt()); + public void putBoolean_shouldNotLogInitialPut() { + final SharedPreferences.Editor editor = mSharedPrefLogger.edit(); + editor.putBoolean(TEST_KEY, true); + editor.putBoolean(TEST_KEY, true); + editor.putBoolean(TEST_KEY, false); + editor.putBoolean(TEST_KEY, false); + editor.putBoolean(TEST_KEY, false); + + verify(mLogWriter, times(4)).count(any(Context.class), anyString(), anyInt()); } @Test - public void putLong_shouldLogCount() { - mSharedPrefLogger.edit().putLong(TEST_KEY, 1); - verify(mLogWriter).count(any(Context.class), anyString(), anyInt()); + public void putLong_shouldNotLogInitialPut() { + final SharedPreferences.Editor editor = mSharedPrefLogger.edit(); + editor.putLong(TEST_KEY, 1); + editor.putLong(TEST_KEY, 1); + editor.putLong(TEST_KEY, 1); + editor.putLong(TEST_KEY, 1); + editor.putLong(TEST_KEY, 2); + + verify(mLogWriter, times(4)).count(any(Context.class), anyString(), anyInt()); } @Test - public void putFloat_shouldLogCount() { - mSharedPrefLogger.edit().putInt(TEST_KEY, 1); - verify(mLogWriter).count(any(Context.class), anyString(), anyInt()); + public void putFloat_shouldNotLogInitialPut() { + final SharedPreferences.Editor editor = mSharedPrefLogger.edit(); + editor.putFloat(TEST_KEY, 1); + editor.putFloat(TEST_KEY, 1); + editor.putFloat(TEST_KEY, 1); + editor.putFloat(TEST_KEY, 1); + editor.putFloat(TEST_KEY, 2); + + verify(mLogWriter, times(4)).count(any(Context.class), anyString(), anyInt()); } }