From 7caecd36f785709525ee1be3e8fe211f1055c383 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Wed, 7 Mar 2018 17:17:29 -0800 Subject: [PATCH] Change anomalyType to ArraySet in AppInfo After this cl, in AppInfo we could store mutilple anomalyTypes, so AppInfo list will only contain one instance for each uid(however still keep all the anomaly data) In this way we could remove the duplicate items in app dialog. Bug: 74335346 Test: RunSettingsRoboTests Change-Id: I2ef7c218df2a956eea66aa6bdf03f5ddd19948e3 --- .../fuelgauge/batterytip/AppInfo.java | 21 ++++---- .../batterytip/BatteryDatabaseManager.java | 32 ++++++++---- .../fuelgauge/batterytip/BatteryTipUtils.java | 2 - .../fuelgauge/BatteryDatabaseManagerTest.java | 52 +++++++++++++------ .../fuelgauge/batterytip/AppInfoTest.java | 12 +++-- .../batterytip/tips/HighUsageTipTest.java | 3 +- .../batterytip/tips/RestrictAppTipTest.java | 11 ++-- 7 files changed, 87 insertions(+), 46 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batterytip/AppInfo.java b/src/com/android/settings/fuelgauge/batterytip/AppInfo.java index dc6ba2521b4..9e8aba354fb 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AppInfo.java +++ b/src/com/android/settings/fuelgauge/batterytip/AppInfo.java @@ -20,9 +20,12 @@ import android.os.Parcel; import android.os.Parcelable; import android.support.annotation.VisibleForTesting; import android.text.TextUtils; +import android.util.ArraySet; import com.android.settings.fuelgauge.anomaly.Anomaly; +import java.util.Objects; + /** * Model class stores app info(e.g. package name, type..) that used in battery tip */ @@ -32,13 +35,13 @@ public class AppInfo implements Comparable, Parcelable { * Anomaly type of the app * @see Anomaly.AnomalyType */ - public final int anomalyType; + public final ArraySet anomalyTypes; public final long screenOnTimeMs; public final int uid; private AppInfo(AppInfo.Builder builder) { packageName = builder.mPackageName; - anomalyType = builder.mAnomalyType; + anomalyTypes = builder.mAnomalyTypes; screenOnTimeMs = builder.mScreenOnTimeMs; uid = builder.mUid; } @@ -46,7 +49,7 @@ public class AppInfo implements Comparable, Parcelable { @VisibleForTesting AppInfo(Parcel in) { packageName = in.readString(); - anomalyType = in.readInt(); + anomalyTypes = (ArraySet) in.readArraySet(null /* loader */); screenOnTimeMs = in.readLong(); uid = in.readInt(); } @@ -64,14 +67,14 @@ public class AppInfo implements Comparable, Parcelable { @Override public void writeToParcel(Parcel dest, int flags) { dest.writeString(packageName); - dest.writeInt(anomalyType); + dest.writeArraySet(anomalyTypes); dest.writeLong(screenOnTimeMs); dest.writeInt(uid); } @Override public String toString() { - return "packageName=" + packageName + ",anomalyType=" + anomalyType + ",screenTime=" + return "packageName=" + packageName + ",anomalyTypes=" + anomalyTypes + ",screenTime=" + screenOnTimeMs; } @@ -85,7 +88,7 @@ public class AppInfo implements Comparable, Parcelable { } AppInfo other = (AppInfo) obj; - return anomalyType == other.anomalyType + return Objects.equals(anomalyTypes, other.anomalyTypes) && uid == other.uid && screenOnTimeMs == other.screenOnTimeMs && TextUtils.equals(packageName, other.packageName); @@ -102,13 +105,13 @@ public class AppInfo implements Comparable, Parcelable { }; public static final class Builder { - private int mAnomalyType; + private ArraySet mAnomalyTypes = new ArraySet<>(); private String mPackageName; private long mScreenOnTimeMs; private int mUid; - public Builder setAnomalyType(int type) { - mAnomalyType = type; + public Builder addAnomalyType(int type) { + mAnomalyTypes.add(type); return this; } diff --git a/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java b/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java index 798c8c692a0..58e9d03ef5c 100644 --- a/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java +++ b/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java @@ -32,10 +32,12 @@ import android.content.Context; import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; import android.text.TextUtils; +import android.util.ArrayMap; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; /** * Database manager for battery data. Now it only contains anomaly data stored in {@link AppInfo}. @@ -88,20 +90,30 @@ public class BatteryDatabaseManager { try (SQLiteDatabase db = mDatabaseHelper.getReadableDatabase()) { final String[] projection = {PACKAGE_NAME, ANOMALY_TYPE, UID}; final String orderBy = AnomalyDatabaseHelper.AnomalyColumns.TIME_STAMP_MS + " DESC"; + final Map mAppInfoBuilders = new ArrayMap<>(); + final String selection = TIME_STAMP_MS + " > ? AND " + ANOMALY_STATE + " = ? "; + final String[] selectionArgs = new String[]{String.valueOf(timestampMsAfter), + String.valueOf(state)}; - try (Cursor cursor = db.query(TABLE_ANOMALY, projection, - TIME_STAMP_MS + " > ? AND " + ANOMALY_STATE + " = ? ", - new String[]{String.valueOf(timestampMsAfter), String.valueOf(state)}, null, - null, orderBy)) { + try (Cursor cursor = db.query(TABLE_ANOMALY, projection, selection, selectionArgs, + null /* groupBy */, null /* having */, orderBy)) { while (cursor.moveToNext()) { - AppInfo appInfo = new AppInfo.Builder() - .setPackageName(cursor.getString(cursor.getColumnIndex(PACKAGE_NAME))) - .setAnomalyType(cursor.getInt(cursor.getColumnIndex(ANOMALY_TYPE))) - .setUid(cursor.getInt(cursor.getColumnIndex(UID))) - .build(); - appInfos.add(appInfo); + final int uid = cursor.getInt(cursor.getColumnIndex(UID)); + if (!mAppInfoBuilders.containsKey(uid)) { + final AppInfo.Builder builder = new AppInfo.Builder() + .setUid(uid) + .setPackageName( + cursor.getString(cursor.getColumnIndex(PACKAGE_NAME))); + mAppInfoBuilders.put(uid, builder); + } + mAppInfoBuilders.get(uid).addAnomalyType( + cursor.getInt(cursor.getColumnIndex(ANOMALY_TYPE))); } } + + for (Integer uid : mAppInfoBuilders.keySet()) { + appInfos.add(mAppInfoBuilders.get(uid).build()); + } } return appInfos; diff --git a/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java b/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java index 5c0147a466d..5ff5430aedb 100644 --- a/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java +++ b/src/com/android/settings/fuelgauge/batterytip/BatteryTipUtils.java @@ -16,8 +16,6 @@ package com.android.settings.fuelgauge.batterytip; -import android.app.Fragment; - import com.android.settings.SettingsActivity; import com.android.settings.core.InstrumentedPreferenceFragment; import com.android.settings.fuelgauge.batterytip.actions.BatterySaverAction; diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java index 636023211bc..30999cbcd00 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java @@ -40,7 +40,6 @@ import java.util.List; @RunWith(RobolectricTestRunner.class) public class BatteryDatabaseManagerTest { - private static String PACKAGE_NAME_NEW = "com.android.app1"; private static int UID_NEW = 345; private static int TYPE_NEW = 1; @@ -53,6 +52,9 @@ public class BatteryDatabaseManagerTest { private Context mContext; private BatteryDatabaseManager mBatteryDatabaseManager; + private AppInfo mNewAppInfo; + private AppInfo mOldAppInfo; + private AppInfo mCombinedAppInfo; @Before public void setUp() { @@ -60,6 +62,23 @@ public class BatteryDatabaseManagerTest { mContext = RuntimeEnvironment.application; mBatteryDatabaseManager = spy(BatteryDatabaseManager.getInstance(mContext)); + + mNewAppInfo = new AppInfo.Builder() + .setUid(UID_NEW) + .setPackageName(PACKAGE_NAME_NEW) + .addAnomalyType(TYPE_NEW) + .build(); + mOldAppInfo = new AppInfo.Builder() + .setUid(UID_OLD) + .setPackageName(PACKAGE_NAME_OLD) + .addAnomalyType(TYPE_OLD) + .build(); + mCombinedAppInfo = new AppInfo.Builder() + .setUid(UID_NEW) + .setPackageName(PACKAGE_NAME_NEW) + .addAnomalyType(TYPE_NEW) + .addAnomalyType(TYPE_OLD) + .build(); } @After @@ -77,23 +96,19 @@ public class BatteryDatabaseManagerTest { // In database, it contains two record List totalAppInfos = mBatteryDatabaseManager.queryAllAnomalies(0 /* timeMsAfter */, AnomalyDatabaseHelper.State.NEW); - assertThat(totalAppInfos).hasSize(2); - assertAppInfo(totalAppInfos.get(0), UID_NEW, PACKAGE_NAME_NEW, TYPE_NEW); - assertAppInfo(totalAppInfos.get(1), UID_OLD, PACKAGE_NAME_OLD, TYPE_OLD); + assertThat(totalAppInfos).containsExactly(mNewAppInfo, mOldAppInfo); // Only one record shows up if we query by timestamp List appInfos = mBatteryDatabaseManager.queryAllAnomalies(ONE_DAY_BEFORE, AnomalyDatabaseHelper.State.NEW); - assertThat(appInfos).hasSize(1); - assertAppInfo(appInfos.get(0), UID_NEW, PACKAGE_NAME_NEW, TYPE_NEW); + assertThat(appInfos).containsExactly(mNewAppInfo); mBatteryDatabaseManager.deleteAllAnomaliesBeforeTimeStamp(ONE_DAY_BEFORE); // The obsolete record is removed from database List appInfos1 = mBatteryDatabaseManager.queryAllAnomalies(0 /* timeMsAfter */, AnomalyDatabaseHelper.State.NEW); - assertThat(appInfos1).hasSize(1); - assertAppInfo(appInfos1.get(0), UID_NEW, PACKAGE_NAME_NEW, TYPE_NEW); + assertThat(appInfos1).containsExactly(mNewAppInfo); } @Test @@ -113,19 +128,24 @@ public class BatteryDatabaseManagerTest { // The state of PACKAGE_NAME_NEW is still new List newAppInfos = mBatteryDatabaseManager.queryAllAnomalies(ONE_DAY_BEFORE, AnomalyDatabaseHelper.State.NEW); - assertThat(newAppInfos).hasSize(1); - assertAppInfo(newAppInfos.get(0), UID_NEW, PACKAGE_NAME_NEW, TYPE_NEW); + assertThat(newAppInfos).containsExactly(mNewAppInfo); // The state of PACKAGE_NAME_OLD is changed to handled List handledAppInfos = mBatteryDatabaseManager.queryAllAnomalies(ONE_DAY_BEFORE, AnomalyDatabaseHelper.State.HANDLED); - assertThat(handledAppInfos).hasSize(1); - assertAppInfo(handledAppInfos.get(0), UID_OLD, PACKAGE_NAME_OLD, TYPE_OLD); + assertThat(handledAppInfos).containsExactly(mOldAppInfo); } - private void assertAppInfo(final AppInfo appInfo, int uid, String packageName, int type) { - assertThat(appInfo.packageName).isEqualTo(packageName); - assertThat(appInfo.anomalyType).isEqualTo(type); - assertThat(appInfo.uid).isEqualTo(uid); + @Test + public void testQueryAnomalies_removeDuplicateByUid() { + mBatteryDatabaseManager.insertAnomaly(UID_NEW, PACKAGE_NAME_NEW, TYPE_NEW, + AnomalyDatabaseHelper.State.NEW, NOW); + mBatteryDatabaseManager.insertAnomaly(UID_NEW, PACKAGE_NAME_NEW, TYPE_OLD, + AnomalyDatabaseHelper.State.NEW, NOW); + + // Only contain one AppInfo with multiple types + List newAppInfos = mBatteryDatabaseManager.queryAllAnomalies(ONE_DAY_BEFORE, + AnomalyDatabaseHelper.State.NEW); + assertThat(newAppInfos).containsExactly(mCombinedAppInfo); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AppInfoTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AppInfoTest.java index b140c4cb295..74e8fe63723 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AppInfoTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AppInfoTest.java @@ -36,7 +36,8 @@ import java.util.List; public class AppInfoTest { private static final String PACKAGE_NAME = "com.android.app"; - private static final int ANOMALY_TYPE = Anomaly.AnomalyType.WAKE_LOCK; + private static final int TYPE_WAKELOCK = Anomaly.AnomalyType.WAKE_LOCK; + private static final int TYPE_WAKEUP = Anomaly.AnomalyType.WAKEUP_ALARM; private static final long SCREEN_TIME_MS = DateUtils.HOUR_IN_MILLIS; private static final int UID = 3452; @@ -46,7 +47,8 @@ public class AppInfoTest { public void setUp() { mAppInfo = new AppInfo.Builder() .setPackageName(PACKAGE_NAME) - .setAnomalyType(ANOMALY_TYPE) + .addAnomalyType(TYPE_WAKELOCK) + .addAnomalyType(TYPE_WAKEUP) .setScreenOnTimeMs(SCREEN_TIME_MS) .setUid(UID) .build(); @@ -61,7 +63,7 @@ public class AppInfoTest { final AppInfo appInfo = new AppInfo(parcel); assertThat(appInfo.packageName).isEqualTo(PACKAGE_NAME); - assertThat(appInfo.anomalyType).isEqualTo(ANOMALY_TYPE); + assertThat(appInfo.anomalyTypes).containsExactly(TYPE_WAKELOCK, TYPE_WAKEUP); assertThat(appInfo.screenOnTimeMs).isEqualTo(SCREEN_TIME_MS); assertThat(appInfo.uid).isEqualTo(UID); } @@ -70,7 +72,7 @@ public class AppInfoTest { public void testCompareTo_hasCorrectOrder() { final AppInfo appInfo = new AppInfo.Builder() .setPackageName(PACKAGE_NAME) - .setAnomalyType(ANOMALY_TYPE) + .addAnomalyType(TYPE_WAKELOCK) .setScreenOnTimeMs(SCREEN_TIME_MS + 100) .build(); @@ -85,7 +87,7 @@ public class AppInfoTest { @Test public void testBuilder() { assertThat(mAppInfo.packageName).isEqualTo(PACKAGE_NAME); - assertThat(mAppInfo.anomalyType).isEqualTo(ANOMALY_TYPE); + assertThat(mAppInfo.anomalyTypes).containsExactly(TYPE_WAKELOCK, TYPE_WAKEUP); assertThat(mAppInfo.screenOnTimeMs).isEqualTo(SCREEN_TIME_MS); assertThat(mAppInfo.uid).isEqualTo(UID); } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/HighUsageTipTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/HighUsageTipTest.java index af32dc50335..85b1b029c48 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/HighUsageTipTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/HighUsageTipTest.java @@ -77,6 +77,7 @@ public class HighUsageTipTest { @Test public void toString_containsAppData() { assertThat(mBatteryTip.toString()).isEqualTo( - "type=2 state=0 { packageName=com.android.app,anomalyType=0,screenTime=1800000 }"); + "type=2 state=0 { packageName=com.android.app,anomalyTypes={},screenTime=1800000 " + + "}"); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTipTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTipTest.java index 3298ea84748..a2e91cb7beb 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTipTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/tips/RestrictAppTipTest.java @@ -39,9 +39,10 @@ import java.util.List; @RunWith(SettingsRobolectricTestRunner.class) public class RestrictAppTipTest { - private static final String PACKAGE_NAME = "com.android.app"; private static final String DISPLAY_NAME = "app"; + private static final int ANOMALY_WAKEUP = 0; + private static final int ANOMALY_WAKELOCK = 1; private Context mContext; private RestrictAppTip mNewBatteryTip; @@ -64,7 +65,11 @@ public class RestrictAppTipTest { doReturn(DISPLAY_NAME).when(mApplicationInfo).loadLabel(mPackageManager); mUsageAppList = new ArrayList<>(); - mUsageAppList.add(new AppInfo.Builder().setPackageName(PACKAGE_NAME).build()); + mUsageAppList.add(new AppInfo.Builder() + .setPackageName(PACKAGE_NAME) + .addAnomalyType(ANOMALY_WAKEUP) + .addAnomalyType(ANOMALY_WAKELOCK) + .build()); mNewBatteryTip = new RestrictAppTip(BatteryTip.StateType.NEW, mUsageAppList); mHandledBatteryTip = new RestrictAppTip(BatteryTip.StateType.HANDLED, mUsageAppList); mInvisibleBatteryTip = new RestrictAppTip(BatteryTip.StateType.INVISIBLE, mUsageAppList); @@ -125,6 +130,6 @@ public class RestrictAppTipTest { @Test public void toString_containsAppData() { assertThat(mNewBatteryTip.toString()).isEqualTo( - "type=1 state=0 { packageName=com.android.app,anomalyType=0,screenTime=0 }"); + "type=1 state=0 { packageName=com.android.app,anomalyTypes={0, 1},screenTime=0 }"); } }