diff --git a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java index d1c7c7a33d1..51102a624a0 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.Log; import android.util.Pair; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; @@ -32,6 +33,8 @@ import java.util.concurrent.ConcurrentSkipListSet; public class SharedPreferencesLogger implements SharedPreferences { + private static final String LOG_TAG = "SharedPreferencesLogger"; + private final String mTag; private final Context mContext; private final MetricsFeatureProvider mMetricsFeature; @@ -99,11 +102,11 @@ public class SharedPreferencesLogger implements SharedPreferences { OnSharedPreferenceChangeListener listener) { } - private void logValue(String key, String value) { + private void logValue(String key, Object value) { logValue(key, value, false /* forceLog */); } - private void logValue(String key, String value, boolean forceLog) { + private void logValue(String key, Object value, boolean forceLog) { final String prefKey = mTag + "/" + key; if (!forceLog && !mPreferenceKeySet.contains(prefKey)) { // Pref key doesn't exist in set, this is initial display so we skip metrics but @@ -114,10 +117,32 @@ public class SharedPreferencesLogger implements SharedPreferences { // TODO: Remove count logging to save some resource. mMetricsFeature.count(mContext, prefKey + "|" + value, 1); - // Pref key exists in set, log it's change in metrics. - mMetricsFeature.action(mContext, MetricsEvent.ACTION_SETTINGS_PREFERENCE_CHANGE, - Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_NAME, prefKey), - Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_VALUE, value)); + final Pair valueData; + if (value instanceof Long) { + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, + value); + } else if (value instanceof Integer) { + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, + ((Integer) value).longValue()); + } else if (value instanceof Boolean) { + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, + (Boolean) value ? 1L : 0L); + } else if (value instanceof Float) { + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_FLOAT_VALUE, + value); + } else if (value instanceof String){ + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_VALUE, + value); + } else { + Log.w(LOG_TAG, "Tried to log unloggable object" + value); + valueData = null; + } + if (valueData != null) { + // Pref key exists in set, log it's change in metrics. + mMetricsFeature.action(mContext, MetricsEvent.ACTION_SETTINGS_PREFERENCE_CHANGE, + Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_NAME, prefKey), + valueData); + } } private void logPackageName(String key, String value) { @@ -173,25 +198,25 @@ public class SharedPreferencesLogger implements SharedPreferences { @Override public Editor putInt(String key, int value) { - logValue(key, String.valueOf(value)); + logValue(key, value); return this; } @Override public Editor putLong(String key, long value) { - logValue(key, String.valueOf(value)); + logValue(key, value); return this; } @Override public Editor putFloat(String key, float value) { - logValue(key, String.valueOf(value)); + logValue(key, value); return this; } @Override public Editor putBoolean(String key, boolean value) { - logValue(key, String.valueOf(value)); + logValue(key, value); return this; } 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 763d6e35120..3e24fcfca91 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java @@ -23,16 +23,26 @@ import com.android.settings.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; import com.android.settings.testutils.FakeFeatureFactory; +import com.google.common.truth.Platform; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Answers; +import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.annotation.Config; +import static com.android.internal.logging.nano.MetricsProto.MetricsEvent + .FIELD_SETTINGS_PREFERENCE_CHANGE_FLOAT_VALUE; +import static com.android.internal.logging.nano.MetricsProto.MetricsEvent + .FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE; +import static com.android.internal.logging.nano.MetricsProto.MetricsEvent + .FIELD_SETTINGS_PREFERENCE_CHANGE_NAME; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -46,6 +56,7 @@ public class SharedPreferenceLoggerTest { @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Context mContext; + private PairMatcher mNamePairMatcher; private FakeFeatureFactory mFactory; private MetricsFeatureProvider mMetricsFeature; private SharedPreferencesLogger mSharedPrefLogger; @@ -58,6 +69,7 @@ public class SharedPreferenceLoggerTest { mMetricsFeature = mFactory.metricsFeatureProvider; mSharedPrefLogger = new SharedPreferencesLogger(mContext, TEST_TAG); + mNamePairMatcher = new PairMatcher(FIELD_SETTINGS_PREFERENCE_CHANGE_NAME, String.class); } @Test @@ -71,8 +83,11 @@ public class SharedPreferenceLoggerTest { editor.putInt(TEST_KEY, 2); editor.putInt(TEST_KEY, 2); + final PairMatcher longMatcher = + new PairMatcher(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, Long.class); + verify(mMetricsFeature, times(6)).action(any(Context.class), anyInt(), - any(Pair.class), any(Pair.class)); + argThat(mNamePairMatcher), argThat(longMatcher)); } @Test @@ -84,8 +99,16 @@ public class SharedPreferenceLoggerTest { editor.putBoolean(TEST_KEY, false); editor.putBoolean(TEST_KEY, false); - verify(mMetricsFeature, times(4)).action(any(Context.class), anyInt(), - any(Pair.class), any(Pair.class)); + + final PairMatcher trueMatcher = + new PairMatcher(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, true); + final PairMatcher falseMatcher = + new PairMatcher(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, false); + + verify(mMetricsFeature).action(any(Context.class), anyInt(), + argThat(mNamePairMatcher), argThat(trueMatcher)); + verify(mMetricsFeature, times(3)).action(any(Context.class), anyInt(), + argThat(mNamePairMatcher), argThat(falseMatcher)); } @Test @@ -97,8 +120,11 @@ public class SharedPreferenceLoggerTest { editor.putLong(TEST_KEY, 1); editor.putLong(TEST_KEY, 2); + final PairMatcher longMatcher = + new PairMatcher(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, Long.class); + verify(mMetricsFeature, times(4)).action(any(Context.class), anyInt(), - any(Pair.class), any(Pair.class)); + argThat(mNamePairMatcher), argThat(longMatcher)); } @Test @@ -110,8 +136,40 @@ public class SharedPreferenceLoggerTest { editor.putFloat(TEST_KEY, 1); editor.putFloat(TEST_KEY, 2); + final PairMatcher floatMatcher = + new PairMatcher(FIELD_SETTINGS_PREFERENCE_CHANGE_FLOAT_VALUE, Float.class); + verify(mMetricsFeature, times(4)).action(any(Context.class), anyInt(), - any(Pair.class), any(Pair.class)); + argThat(mNamePairMatcher), argThat(floatMatcher)); } + private static class PairMatcher extends ArgumentMatcher> { + + private final int mExpectedTag; + private final Class mExpectedClass; + private final Long mExpectedBoolean; + + + public PairMatcher(int tag, Class clazz) { + mExpectedTag = tag; + mExpectedClass = clazz; + mExpectedBoolean = null; + } + + public PairMatcher(int tag, boolean bool) { + mExpectedTag = tag; + mExpectedClass = Long.class; + mExpectedBoolean = bool ? 1L : 0L; + } + + @Override + public boolean matches(Object arg) { + final Pair pair = (Pair) arg; + boolean booleanMatch = mExpectedBoolean == null + || mExpectedBoolean == pair.second; + return pair.first == mExpectedTag + && Platform.isInstanceOfType(pair.second, mExpectedClass) + && booleanMatch; + } + } }