Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f63a61d2 authored by Fan Zhang's avatar Fan Zhang
Browse files

Remove arbitrary Pair from action() logging method. 

Caller should use generic action(int, int, int, String, int) instead.

Bug: 117860032
Test: robotests
Change-Id: I4db5b9e764478bcb36ba5c96e805a929d212b344
parent 733e1423

packages/SettingsLib/src/com/android/settingslib/core/instrumentation/EventLogWriter.java

+14 −14
Original line number Diff line number Diff line
@@ -44,7 +44,14 @@ public class EventLogWriter implements LogWriter { 


    @Override

    public void action(Context context, int category, Pair<Integer, Object>... taggedData) {

        action(context, category, "", taggedData);

        final LogMaker logMaker = new LogMaker(category)

                .setType(MetricsProto.MetricsEvent.TYPE_ACTION);

        if (taggedData != null) {

            for (Pair<Integer, Object> pair : taggedData) {

                logMaker.addTaggedData(pair.first, pair.second);

            }

        }

        MetricsLogger.action(logMaker);

    }



    @Override
@@ -58,20 +65,13 @@ public class EventLogWriter implements LogWriter { 
    }



    @Override

    public void action(Context context, int category, String pkg,

            Pair<Integer, Object>... taggedData) {

        if (taggedData == null || taggedData.length == 0) {

            MetricsLogger.action(context, category, pkg);

        } else {

    public void action(Context context, int category, String pkg) {

        final LogMaker logMaker = new LogMaker(category)

                .setType(MetricsProto.MetricsEvent.TYPE_ACTION)

                .setPackageName(pkg);

            for (Pair<Integer, Object> pair : taggedData) {

                logMaker.addTaggedData(pair.first, pair.second);

            }



        MetricsLogger.action(logMaker);

    }

    }



    @Override

    public void action(int attribution, int action, int pageId, String key, int value) {

packages/SettingsLib/src/com/android/settingslib/core/instrumentation/LogWriter.java

+1 −1
Original line number Diff line number Diff line
@@ -51,7 +51,7 @@ public interface LogWriter { 
    /**

     * Logs an user action.

     */

    void action(Context context, int category, String pkg, Pair<Integer, Object>... taggedData);

    void action(Context context, int category, String pkg);



    /**

     * Generically log action.

packages/SettingsLib/src/com/android/settingslib/core/instrumentation/MetricsFeatureProvider.java

+16 −8
Original line number Diff line number Diff line
@@ -79,6 +79,9 @@ public class MetricsFeatureProvider { 
        }

    }



    /**

     * Logs a simple action without page id or attribution

     */

    public void action(Context context, int category,  Pair<Integer, Object>... taggedData) {

        for (LogWriter writer : mLoggerWriters) {

            writer.action(context, category, taggedData);
@@ -88,10 +91,9 @@ public class MetricsFeatureProvider { 
    /**

     * Logs a generic Settings event.

     */

    public void action(Context context, int category, String pkg,

            Pair<Integer, Object>... taggedData) {

    public void action(Context context, int category, String pkg) {

        for (LogWriter writer : mLoggerWriters) {

            writer.action(context, category, pkg, taggedData);

            writer.action(context, category, pkg);

        }

    }
@@ -135,16 +137,22 @@ public class MetricsFeatureProvider { 
                // Not loggable

                return;

            }

            action(context, MetricsEvent.ACTION_SETTINGS_TILE_CLICK, action,

                    Pair.create(MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory));

            action(sourceMetricsCategory,

                    MetricsEvent.ACTION_SETTINGS_TILE_CLICK,

                    SettingsEnums.PAGE_UNKNOWN,

                    action,

                    0);

            return;

        } else if (TextUtils.equals(cn.getPackageName(), context.getPackageName())) {

            // Going to a Setting internal page, skip click logging in favor of page's own

            // visibility logging.

            return;

        }

        action(context, MetricsEvent.ACTION_SETTINGS_TILE_CLICK, cn.flattenToString(),

                Pair.create(MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory));

        action(sourceMetricsCategory,

                MetricsEvent.ACTION_SETTINGS_TILE_CLICK,

                SettingsEnums.PAGE_UNKNOWN,

                cn.flattenToString(),

                0);

    }



}

packages/SettingsLib/src/com/android/settingslib/core/instrumentation/SharedPreferencesLogger.java

+24 −29
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
package com.android.settingslib.core.instrumentation;



import android.annotation.Nullable;

import android.app.settings.SettingsEnums;

import android.content.ComponentName;

import android.content.Context;

import android.content.SharedPreferences;
@@ -22,12 +23,9 @@ import android.content.pm.PackageManager; 
import android.os.AsyncTask;

import android.text.TextUtils;

import android.util.Log;

import android.util.Pair;



import androidx.annotation.VisibleForTesting;



import com.android.internal.logging.nano.MetricsProto.MetricsEvent;



import java.util.Map;

import java.util.Set;

import java.util.concurrent.ConcurrentSkipListSet;
@@ -117,10 +115,9 @@ public class SharedPreferencesLogger implements SharedPreferences { 
            return;

        }



        final Pair<Integer, Object> valueData;

        final int intVal;

        if (value instanceof Long) {

            final Long longVal = (Long) value;

            final int intVal;

            if (longVal > Integer.MAX_VALUE) {

                intVal = Integer.MAX_VALUE;

            } else if (longVal < Integer.MIN_VALUE) {
@@ -128,47 +125,45 @@ public class SharedPreferencesLogger implements SharedPreferences { 
            } 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_INT_VALUE,

                    value);

            intVal = (int) value;

        } else if (value instanceof Boolean) {

            valueData = Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_INT_VALUE,

                    (Boolean) value ? 1 : 0);

            intVal = (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) {

            Log.d(LOG_TAG, "Tried to log string preference " + prefKey + " = " + value);

            valueData = null;

            final float floatValue = (float) value;

            if (floatValue > Integer.MAX_VALUE) {

                intVal = Integer.MAX_VALUE;

            } else if (floatValue < Integer.MIN_VALUE) {

                intVal = Integer.MIN_VALUE;

            } else {

                intVal = (int) floatValue;

            }

        } else {

            Log.w(LOG_TAG, "Tried to log unloggable object" + value);

            valueData = null;

            return;

        }

        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);

        }

        mMetricsFeature.action(SettingsEnums.PAGE_UNKNOWN,

                SettingsEnums.ACTION_SETTINGS_PREFERENCE_CHANGE,

                SettingsEnums.PAGE_UNKNOWN,

                prefKey,

                intVal);

    }



    @VisibleForTesting

    void logPackageName(String key, String value) {

        final String prefKey = mTag + "/" + key;

        mMetricsFeature.action(mContext, MetricsEvent.ACTION_SETTINGS_PREFERENCE_CHANGE, value,

                Pair.create(MetricsEvent.FIELD_SETTINGS_PREFERENCE_CHANGE_NAME, prefKey));

        mMetricsFeature.action(SettingsEnums.PAGE_UNKNOWN,

                SettingsEnums.ACTION_SETTINGS_PREFERENCE_CHANGE,

                SettingsEnums.PAGE_UNKNOWN,

                prefKey + ":" + value,

                0);

    }



    private void safeLogValue(String key, String value) {

        new AsyncPackageCheck().executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, key, value);

    }



    public static String buildCountName(String prefKey, Object value) {

        return prefKey + "|" + value;

    }



    public static String buildPrefKey(String tag, String key) {

        return tag + "/" + key;

    }

packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/instrumentation/MetricsFeatureProviderTest.java

+10 −11
Original line number Diff line number Diff line
@@ -17,8 +17,6 @@ package com.android.settingslib.core.instrumentation; 


import static com.google.common.truth.Truth.assertThat;



import static org.mockito.Matchers.anyString;

import static org.mockito.Matchers.eq;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -27,7 +25,6 @@ import android.app.settings.SettingsEnums; 
import android.content.ComponentName;

import android.content.Context;

import android.content.Intent;

import android.util.Pair;



import com.android.internal.logging.nano.MetricsProto.MetricsEvent;

import com.android.settingslib.SettingsLibRobolectricTestRunner;
@@ -77,10 +74,11 @@ public class MetricsFeatureProviderTest { 
        mProvider.logDashboardStartIntent(mContext, intent, MetricsEvent.SETTINGS_GESTURES);



        verify(mLogWriter).action(

                eq(mContext),

                eq(MetricsEvent.ACTION_SETTINGS_TILE_CLICK),

                anyString(),

                eq(Pair.create(MetricsEvent.FIELD_CONTEXT, MetricsEvent.SETTINGS_GESTURES)));

                MetricsEvent.SETTINGS_GESTURES,

                MetricsEvent.ACTION_SETTINGS_TILE_CLICK,

                SettingsEnums.PAGE_UNKNOWN,

                Intent.ACTION_ASSIST,

                0);

    }



    @Test
@@ -90,10 +88,11 @@ public class MetricsFeatureProviderTest { 
        mProvider.logDashboardStartIntent(mContext, intent, MetricsEvent.SETTINGS_GESTURES);



        verify(mLogWriter).action(

                eq(mContext),

                eq(MetricsEvent.ACTION_SETTINGS_TILE_CLICK),

                anyString(),

                eq(Pair.create(MetricsEvent.FIELD_CONTEXT, MetricsEvent.SETTINGS_GESTURES)));

                MetricsEvent.SETTINGS_GESTURES,

                MetricsEvent.ACTION_SETTINGS_TILE_CLICK,

                SettingsEnums.PAGE_UNKNOWN,

                "pkg/cls",

                0);

    }



    @Test