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

Commit 5cd0e659 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Reduce log spam when RankingMap is missing rankings for non-cancelled entries.

Fixes: 236140753
Test: dumpsysui NotifCollection
Test: atest NotifCollectionLoggerTest
Change-Id: I7308af63ec67aa9b1b3444bfe90a3953468024ad
parent 9647d7cf
Loading
Loading
Loading
Loading
+22 −1
Original line number Diff line number Diff line
@@ -89,6 +89,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.In
import com.android.systemui.statusbar.notification.collection.notifcollection.InternalNotifUpdater;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLogger;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLoggerKt;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifEvent;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;
@@ -110,6 +111,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import javax.inject.Inject;
@@ -157,6 +159,8 @@ public class NotifCollection implements Dumpable {
    private final List<NotifLifetimeExtender> mLifetimeExtenders = new ArrayList<>();
    private final List<NotifDismissInterceptor> mDismissInterceptors = new ArrayList<>();

    private Set<String> mNotificationsWithoutRankings = Collections.emptySet();

    private Queue<NotifEvent> mEventQueue = new ArrayDeque<>();

    private boolean mAttached = false;
@@ -560,6 +564,7 @@ public class NotifCollection implements Dumpable {
    }

    private void applyRanking(@NonNull RankingMap rankingMap) {
        ArrayMap<String, NotificationEntry> currentEntriesWithoutRankings = null;
        for (NotificationEntry entry : mNotificationSet.values()) {
            if (!isCanceled(entry)) {

@@ -581,10 +586,21 @@ public class NotifCollection implements Dumpable {
                        }
                    }
                } else {
                    mLogger.logRankingMissing(entry, rankingMap);
                    if (currentEntriesWithoutRankings == null) {
                        currentEntriesWithoutRankings = new ArrayMap<>();
                    }
                    currentEntriesWithoutRankings.put(entry.getKey(), entry);
                }
            }
        }
        NotifCollectionLoggerKt.maybeLogInconsistentRankings(
                mLogger,
                mNotificationsWithoutRankings,
                currentEntriesWithoutRankings,
                rankingMap
        );
        mNotificationsWithoutRankings = currentEntriesWithoutRankings == null
                ? Collections.emptySet() : currentEntriesWithoutRankings.keySet();
        mEventQueue.add(new RankingAppliedEvent());
    }

@@ -836,6 +852,11 @@ public class NotifCollection implements Dumpable {
                        entries,
                        true,
                        "\t\t"));

        pw.println("\n\tmNotificationsWithoutRankings: " + mNotificationsWithoutRankings.size());
        for (String key : mNotificationsWithoutRankings) {
            pw.println("\t * : " + key);
        }
    }

    private final BatchableNotificationHandler mNotifHandler = new BatchableNotificationHandler() {
+48 −7
Original line number Diff line number Diff line
@@ -196,16 +196,32 @@ class NotifCollectionLogger @Inject constructor(
        })
    }

    fun logRankingMissing(entry: NotificationEntry, rankingMap: RankingMap) {
    fun logMissingRankings(
        newlyInconsistentEntries: List<NotificationEntry>,
        totalInconsistent: Int,
        rankingMap: RankingMap
    ) {
        buffer.log(TAG, WARNING, {
            str1 = entry.logKey
            int1 = totalInconsistent
            int2 = newlyInconsistentEntries.size
            str1 = newlyInconsistentEntries.joinToString { it.logKey ?: "null" }
        }, {
            "Ranking update is missing ranking for $str1"
            "Ranking update is missing ranking for $int1 entries ($int2 new): $str1"
        })
        buffer.log(TAG, DEBUG, {
            str1 = rankingMap.orderedKeys.map { logKey(it) ?: "null" }.toString()
        }, {
            "Ranking map contents: $str1"
        })
        buffer.log(TAG, DEBUG, {}, { "Ranking map contents:" })
        for (entry in rankingMap.orderedKeys) {
            buffer.log(TAG, DEBUG, { str1 = logKey(entry) }, { "  $str1" })
    }

    fun logRecoveredRankings(newlyConsistentKeys: List<String>) {
        buffer.log(TAG, INFO, {
            int1 = newlyConsistentKeys.size
            str1 = newlyConsistentKeys.joinToString { logKey(it) ?: "null" }
        }, {
            "Ranking update now contains rankings for $int1 previously inconsistent entries: $str1"
        })
    }

    fun logRemoteExceptionOnNotificationClear(entry: NotificationEntry, e: RemoteException) {
@@ -346,4 +362,29 @@ class NotifCollectionLogger @Inject constructor(
    }
}

fun maybeLogInconsistentRankings(
    logger: NotifCollectionLogger,
    oldKeysWithoutRankings: Set<String>,
    newEntriesWithoutRankings: Map<String, NotificationEntry>?,
    rankingMap: RankingMap
) {
    if (oldKeysWithoutRankings.isEmpty() && newEntriesWithoutRankings == null) return
    val newlyConsistent: List<String> = oldKeysWithoutRankings
        .mapNotNull { key ->
            key.takeIf { key !in (newEntriesWithoutRankings ?: emptyMap()) }
                .takeIf { key in rankingMap.orderedKeys }
        }.sorted()
    if (newlyConsistent.isNotEmpty()) {
        logger.logRecoveredRankings(newlyConsistent)
    }
    val newlyInconsistent: List<NotificationEntry> = newEntriesWithoutRankings
        ?.mapNotNull { (key, entry) ->
            entry.takeIf { key !in oldKeysWithoutRankings }
        }?.sortedBy { it.key } ?: emptyList()
    if (newlyInconsistent.isNotEmpty()) {
        val totalInconsistent: Int = newEntriesWithoutRankings?.size ?: 0
        logger.logMissingRankings(newlyInconsistent, totalInconsistent, rankingMap)
    }
}

private const val TAG = "NotifCollection"
+98 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2022 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.android.systemui.statusbar.notification.collection.notifcollection

import android.service.notification.NotificationListenerService.RankingMap
import android.testing.AndroidTestingRunner
import android.testing.TestableLooper.RunWithLooper
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.statusbar.notification.collection.NotificationEntry
import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder
import com.android.systemui.util.mockito.eq
import com.android.systemui.util.mockito.mock
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyNoMoreInteractions

@SmallTest
@RunWith(AndroidTestingRunner::class)
@RunWithLooper
class NotifCollectionLoggerTest : SysuiTestCase() {
    private val logger: NotifCollectionLogger = mock()
    private val entry1: NotificationEntry = NotificationEntryBuilder().setId(1).build()
    private val entry2: NotificationEntry = NotificationEntryBuilder().setId(2).build()

    private fun mapOfEntries(vararg entries: NotificationEntry): Map<String, NotificationEntry> =
        entries.associateBy { it.key }

    private fun rankingMapOf(vararg entries: NotificationEntry): RankingMap =
        RankingMap(entries.map { it.ranking }.toTypedArray())

    @Test
    fun testMaybeLogInconsistentRankings_logsNewlyInconsistentRanking() {
        val rankingMap = rankingMapOf(entry1)
        maybeLogInconsistentRankings(
            logger = logger,
            oldKeysWithoutRankings = emptySet(),
            newEntriesWithoutRankings = mapOfEntries(entry2),
            rankingMap = rankingMap
        )
        verify(logger).logMissingRankings(
            newlyInconsistentEntries = eq(listOf(entry2)),
            totalInconsistent = eq(1),
            rankingMap = eq(rankingMap),
        )
        verifyNoMoreInteractions(logger)
    }

    @Test
    fun testMaybeLogInconsistentRankings_doesNotLogAlreadyInconsistentRanking() {
        maybeLogInconsistentRankings(
            logger = logger,
            oldKeysWithoutRankings = setOf(entry2.key),
            newEntriesWithoutRankings = mapOfEntries(entry2),
            rankingMap = rankingMapOf(entry1)
        )
        verifyNoMoreInteractions(logger)
    }

    @Test
    fun testMaybeLogInconsistentRankings_logsWhenRankingIsAdded() {
        maybeLogInconsistentRankings(
            logger = logger,
            oldKeysWithoutRankings = setOf(entry2.key),
            newEntriesWithoutRankings = mapOfEntries(),
            rankingMap = rankingMapOf(entry1, entry2)
        )
        verify(logger).logRecoveredRankings(
            newlyConsistentKeys = eq(listOf(entry2.key)),
        )
        verifyNoMoreInteractions(logger)
    }

    @Test
    fun testMaybeLogInconsistentRankings_doesNotLogsWhenEntryIsRemoved() {
        maybeLogInconsistentRankings(
            logger = logger,
            oldKeysWithoutRankings = setOf(entry2.key),
            newEntriesWithoutRankings = mapOfEntries(),
            rankingMap = rankingMapOf(entry1)
        )
        verifyNoMoreInteractions(logger)
    }
}