From 9dbd807ea179ca38741061964e8302ac8b0eb351 Mon Sep 17 00:00:00 2001 From: Doris Ling Date: Tue, 8 Aug 2017 14:03:29 -0700 Subject: [PATCH] Log preference change as Integer instead of Long. Change-Id: I3dc93a4b8a5aa7f03bef05d39bd4504ed205334f Fix: 64485529 Test: make RunSettingsRoboTests --- .../SharedPreferencesLogger.java | 24 +++++++--- .../SharedPreferenceLoggerTest.java | 46 ++++++++++++++++--- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java index b4e61587ac2..dee40c043ce 100644 --- a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java +++ b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java @@ -120,19 +120,29 @@ public class SharedPreferencesLogger implements SharedPreferences { final Pair valueData; if (value instanceof Long) { - valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, - value); + final Long longVal = (Long) value; + final int intVal; + if (longVal > Integer.MAX_VALUE) { + intVal = Integer.MAX_VALUE; + } else if (longVal < Integer.MIN_VALUE) { + intVal = Integer.MIN_VALUE; + } else { + intVal = longVal.intValue(); + } + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, + intVal); } else if (value instanceof Integer) { - valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, - ((Integer) value).longValue()); + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, + value); } else if (value instanceof Boolean) { - valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, - (Boolean) value ? 1L : 0L); + valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, + (Boolean) value ? 1 : 0); } 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); + Log.d(LOG_TAG, "Tried to log string preference " + prefKey + " = " + value); + valueData = null; } else { Log.w(LOG_TAG, "Tried to log unloggable object" + value); valueData = null; 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 ec644a6cfde..bb41cf0d4f4 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java @@ -39,7 +39,7 @@ import static com.android.internal.logging.nano.MetricsProto.MetricsEvent 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; + .FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE; import static com.android.internal.logging.nano.MetricsProto.MetricsEvent .FIELD_SETTINGS_PREFERENCE_CHANGE_NAME; import static org.mockito.Matchers.any; @@ -88,7 +88,7 @@ public class SharedPreferenceLoggerTest { verify(mMetricsFeature, times(6)).action(any(Context.class), anyInt(), argThat(mNamePairMatcher), - argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, Long.class))); + argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, Integer.class))); } @Test @@ -103,10 +103,10 @@ public class SharedPreferenceLoggerTest { verify(mMetricsFeature).action(any(Context.class), anyInt(), argThat(mNamePairMatcher), - argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, true))); + argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, true))); verify(mMetricsFeature, times(3)).action(any(Context.class), anyInt(), argThat(mNamePairMatcher), - argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, false))); + argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, false))); } @Test @@ -120,7 +120,33 @@ public class SharedPreferenceLoggerTest { verify(mMetricsFeature, times(4)).action(any(Context.class), anyInt(), argThat(mNamePairMatcher), - argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_LONG_VALUE, Long.class))); + argThat(pairMatches(FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, Integer.class))); + } + + @Test + public void putLong_biggerThanIntMax_shouldLogIntMax() { + final SharedPreferences.Editor editor = mSharedPrefLogger.edit(); + final long veryBigNumber = 500L + Integer.MAX_VALUE; + editor.putLong(TEST_KEY, 1); + editor.putLong(TEST_KEY, veryBigNumber); + + verify(mMetricsFeature).action(any(Context.class), anyInt(), + argThat(mNamePairMatcher), + argThat(pairMatches( + FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, Integer.MAX_VALUE))); + } + + @Test + public void putLong_smallerThanIntMin_shouldLogIntMin() { + final SharedPreferences.Editor editor = mSharedPrefLogger.edit(); + final long veryNegativeNumber = -500L + Integer.MIN_VALUE; + editor.putLong(TEST_KEY, 1); + editor.putLong(TEST_KEY, veryNegativeNumber); + + verify(mMetricsFeature).action(any(Context.class), anyInt(), + argThat(mNamePairMatcher), + argThat(pairMatches( + FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE, Integer.MIN_VALUE))); } @Test @@ -152,7 +178,13 @@ public class SharedPreferenceLoggerTest { private ArgumentMatcher> pairMatches(int tag, boolean bool) { return pair -> pair.first == tag - && Platform.isInstanceOfType(pair.second, Long.class) - && pair.second.equals((bool ? 1L : 0L)); + && Platform.isInstanceOfType(pair.second, Integer.class) + && pair.second.equals((bool ? 1 : 0)); + } + + private ArgumentMatcher> pairMatches(int tag, int val) { + return pair -> pair.first == tag + && Platform.isInstanceOfType(pair.second, Integer.class) + && pair.second.equals(val); } }