Log different type of values into different constants

Bug: 38258793
Test: make RunSettingsRoboTests
Change-Id: I3ed170613709be04ffb2c561e132094d64619dd4

diff --git a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java
index d1c7c7a..51102a6 100644 (file)
--- 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<Integer, Object> 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 763d6e3..3e24fcf 100644 (file)
--- 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<Pair<Integer, Object>> {
+
+        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<Integer, Object> pair = (Pair) arg;
+            boolean booleanMatch = mExpectedBoolean == null
+                    || mExpectedBoolean == pair.second;
+            return pair.first == mExpectedTag
+                    && Platform.isInstanceOfType(pair.second, mExpectedClass)
+                    && booleanMatch;
+        }
+    }
 }