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

Commit 2887df96 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Small fixes to DumpManager 

1) Use a TreeMap instead of an ArrayMap for dumpables and buffers to ensure consistent and predictable dump order across bugreports.
2) Instead of selecting just the first dumpable to match the target string, select the one with the shortest name; this guaruntees that an exact match will always be returned.

Test: dumpsysui --list (observe: dumpables and buffers are in alphabetical order)
Test: dumpsysui Log (observe: dumps QSLog instead of WifiTableLog)
Test: atest DumpManagerTest
Fixes: 252783246
Change-Id: Ifbdf3428b9343e1146e39e20f65a5897f6821f90
parent 4542c10c

packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt

+30 −20
Original line number Diff line number Diff line
@@ -16,12 +16,12 @@ 


package com.android.systemui.dump



import android.util.ArrayMap

import com.android.systemui.Dumpable

import com.android.systemui.ProtoDumpable

import com.android.systemui.dump.nano.SystemUIProtoDump

import com.android.systemui.plugins.log.LogBuffer

import java.io.PrintWriter

import java.util.TreeMap

import javax.inject.Inject

import javax.inject.Singleton
@@ -36,8 +36,9 @@ import javax.inject.Singleton 
 */

@Singleton

open class DumpManager @Inject constructor() {

    private val dumpables: MutableMap<String, RegisteredDumpable<Dumpable>> = ArrayMap()

    private val buffers: MutableMap<String, RegisteredDumpable<LogBuffer>> = ArrayMap()

    // NOTE: Using TreeMap ensures that iteration is in a predictable & alphabetical order.

    private val dumpables: MutableMap<String, RegisteredDumpable<Dumpable>> = TreeMap()

    private val buffers: MutableMap<String, RegisteredDumpable<LogBuffer>> = TreeMap()



    /** See [registerCriticalDumpable]. */

    fun registerCriticalDumpable(module: Dumpable) {
@@ -132,7 +133,8 @@ open class DumpManager @Inject constructor() { 
    }



    /**

     * Dumps the first dumpable or buffer whose registered name ends with [target]

     * Dumps the alphabetically first, shortest-named dumpable or buffer whose registered name ends

     * with [target].

     */

    @Synchronized

    fun dumpTarget(
@@ -141,19 +143,14 @@ open class DumpManager @Inject constructor() { 
        args: Array<String>,

        tailLength: Int,

    ) {

        for (dumpable in dumpables.values) {

            if (dumpable.name.endsWith(target)) {

                dumpDumpable(dumpable, pw, args)

                return

            }

        }



        for (buffer in buffers.values) {

            if (buffer.name.endsWith(target)) {

                dumpBuffer(buffer, pw, tailLength)

                return

        sequence {

            findBestTargetMatch(dumpables, target)?.let {

                yield(it.name to { dumpDumpable(it, pw, args) })

            }

            findBestTargetMatch(buffers, target)?.let {

                yield(it.name to { dumpBuffer(it, pw, tailLength) })

            }

        }.sortedBy { it.first }.minByOrNull { it.first.length }?.second?.invoke()

    }



    @Synchronized
@@ -162,11 +159,8 @@ open class DumpManager @Inject constructor() { 
        protoDump: SystemUIProtoDump,

        args: Array<String>

    ) {

        for (dumpable in dumpables.values) {

            if (dumpable.dumpable is ProtoDumpable && dumpable.name.endsWith(target)) {

                dumpProtoDumpable(dumpable.dumpable, protoDump, args)

                return

            }

        findBestProtoTargetMatch(dumpables, target)?.let {

            dumpProtoDumpable(it, protoDump, args)

        }

    }
@@ -303,6 +297,22 @@ open class DumpManager @Inject constructor() { 
        val existingDumpable = dumpables[name]?.dumpable ?: buffers[name]?.dumpable

        return existingDumpable == null || newDumpable == existingDumpable

    }



    private fun <V : Any> findBestTargetMatch(map: Map<String, V>, target: String): V? = map

        .asSequence()

        .filter { it.key.endsWith(target) }

        .minByOrNull { it.key.length }

        ?.value



    private fun findBestProtoTargetMatch(

        map: Map<String, RegisteredDumpable<Dumpable>>,

        target: String

    ): ProtoDumpable? = map

        .asSequence()

        .filter { it.key.endsWith(target) }

        .filter { it.value.dumpable is ProtoDumpable }

        .minByOrNull { it.key.length }

        ?.value?.dumpable as? ProtoDumpable

}



private data class RegisteredDumpable<T>(

packages/SystemUI/tests/src/com/android/systemui/dump/DumpManagerTest.kt

+139 −35
Original line number Diff line number Diff line
@@ -60,16 +60,14 @@ class DumpManagerTest : SysuiTestCase() { 


        // WHEN a dumpable is dumped explicitly

        val args = arrayOf<String>()

        dumpManager.dumpTarget("dumpable2", pw, arrayOf(), tailLength = 0)

        dumpManager.dumpTarget("dumpable2", pw, args, tailLength = 0)



        // THEN only the requested one has their dump() method called

        verify(dumpable1, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2).dump(pw, args)

        verify(dumpable3, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())

        verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())

        verify(dumpable3, never()).dump(any(), any())

        verify(buffer1, never()).dump(any(), anyInt())

        verify(buffer2, never()).dump(any(), anyInt())

    }



    @Test
@@ -82,17 +80,15 @@ class DumpManagerTest : SysuiTestCase() { 
        dumpManager.registerBuffer("buffer2", buffer2)



        // WHEN a buffer is dumped explicitly

        dumpManager.dumpTarget("buffer1", pw, arrayOf(), tailLength = 14)

        val args = arrayOf<String>()

        dumpManager.dumpTarget("buffer1", pw, args, tailLength = 14)



        // THEN only the requested one has their dump() method called

        verify(dumpable1, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable2, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable2, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2, never()).dump(any(), any())

        verify(dumpable3, never()).dump(any(), any())

        verify(buffer1).dump(pw, tailLength = 14)

        verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())

        verify(buffer2, never()).dump(any(), anyInt())

    }



    @Test
@@ -108,6 +104,122 @@ class DumpManagerTest : SysuiTestCase() { 
        verify(dumpable1).dump(pw, args)

    }



    @Test

    fun testDumpTarget_selectsShortestNamedDumpable() {

        // GIVEN a variety of registered dumpables and buffers

        dumpManager.registerCriticalDumpable("first-dumpable", dumpable1)

        dumpManager.registerCriticalDumpable("scnd-dumpable", dumpable2)

        dumpManager.registerCriticalDumpable("third-dumpable", dumpable3)



        // WHEN a dumpable is dumped by a suffix that matches multiple options

        val args = arrayOf<String>()

        dumpManager.dumpTarget("dumpable", pw, args, tailLength = 0)



        // THEN the matching dumpable with the shorter name is dumped

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2).dump(pw, args)

        verify(dumpable3, never()).dump(any(), any())

    }



    @Test

    fun testDumpTarget_selectsShortestNamedBuffer() {

        // GIVEN a variety of registered dumpables and buffers

        dumpManager.registerBuffer("first-buffer", buffer1)

        dumpManager.registerBuffer("scnd-buffer", buffer2)



        // WHEN a dumpable is dumped by a suffix that matches multiple options

        val args = arrayOf<String>()

        dumpManager.dumpTarget("buffer", pw, args, tailLength = 14)



        // THEN the matching buffer with the shorter name is dumped

        verify(buffer1, never()).dump(any(), anyInt())

        verify(buffer2).dump(pw, tailLength = 14)

    }



    @Test

    fun testDumpTarget_selectsShortestNamedMatch_dumpable() {

        // GIVEN a variety of registered dumpables and buffers

        dumpManager.registerCriticalDumpable("dumpable1", dumpable1)

        dumpManager.registerCriticalDumpable("dumpable2", dumpable2)

        dumpManager.registerCriticalDumpable("dumpable3", dumpable3)

        dumpManager.registerBuffer("big-buffer1", buffer1)

        dumpManager.registerBuffer("big-buffer2", buffer2)



        // WHEN a dumpable is dumped by a suffix that matches multiple options

        val args = arrayOf<String>()

        dumpManager.dumpTarget("2", pw, args, tailLength = 14)



        // THEN the matching dumpable with the shorter name is dumped

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2).dump(pw, args)

        verify(dumpable3, never()).dump(any(), any())

        verify(buffer1, never()).dump(any(), anyInt())

        verify(buffer2, never()).dump(any(), anyInt())

    }



    @Test

    fun testDumpTarget_selectsShortestNamedMatch_buffer() {

        // GIVEN a variety of registered dumpables and buffers

        dumpManager.registerCriticalDumpable("dumpable1", dumpable1)

        dumpManager.registerCriticalDumpable("dumpable2", dumpable2)

        dumpManager.registerCriticalDumpable("dumpable3", dumpable3)

        dumpManager.registerBuffer("buffer1", buffer1)

        dumpManager.registerBuffer("buffer2", buffer2)



        // WHEN a dumpable is dumped by a suffix that matches multiple options

        val args = arrayOf<String>()

        dumpManager.dumpTarget("2", pw, args, tailLength = 14)



        // THEN the matching buffer with the shorter name is dumped

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2, never()).dump(any(), any())

        verify(dumpable3, never()).dump(any(), any())

        verify(buffer1, never()).dump(any(), anyInt())

        verify(buffer2).dump(pw, tailLength = 14)

    }



    @Test

    fun testDumpTarget_selectsTheAlphabeticallyFirstShortestMatch_dumpable() {

        // GIVEN a variety of registered dumpables and buffers

        dumpManager.registerCriticalDumpable("d1x", dumpable1)

        dumpManager.registerCriticalDumpable("d2x", dumpable2)

        dumpManager.registerCriticalDumpable("a3x", dumpable3)

        dumpManager.registerBuffer("ab1x", buffer1)

        dumpManager.registerBuffer("b2x", buffer2)



        // WHEN a dumpable is dumped by a suffix that matches multiple options

        val args = arrayOf<String>()

        dumpManager.dumpTarget("x", pw, args, tailLength = 14)



        // THEN the alphabetically first dumpable/buffer (of the 3 letter names) is dumped

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2, never()).dump(any(), any())

        verify(dumpable3).dump(pw, args)

        verify(buffer1, never()).dump(any(), anyInt())

        verify(buffer2, never()).dump(any(), anyInt())

    }



    @Test

    fun testDumpTarget_selectsTheAlphabeticallyFirstShortestMatch_buffer() {

        // GIVEN a variety of registered dumpables and buffers

        dumpManager.registerCriticalDumpable("d1x", dumpable1)

        dumpManager.registerCriticalDumpable("d2x", dumpable2)

        dumpManager.registerCriticalDumpable("az1x", dumpable3)

        dumpManager.registerBuffer("b1x", buffer1)

        dumpManager.registerBuffer("b2x", buffer2)



        // WHEN a dumpable is dumped by a suffix that matches multiple options

        val args = arrayOf<String>()

        dumpManager.dumpTarget("x", pw, args, tailLength = 14)



        // THEN the alphabetically first dumpable/buffer (of the 3 letter names) is dumped

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2, never()).dump(any(), any())

        verify(dumpable3, never()).dump(any(), any())

        verify(buffer1).dump(pw, tailLength = 14)

        verify(buffer2, never()).dump(any(), anyInt())

    }



    @Test

    fun testDumpDumpables() {

        // GIVEN a variety of registered dumpables and buffers
@@ -125,8 +237,8 @@ class DumpManagerTest : SysuiTestCase() { 
        verify(dumpable1).dump(pw, args)

        verify(dumpable2).dump(pw, args)

        verify(dumpable3).dump(pw, args)

        verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())

        verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())

        verify(buffer1, never()).dump(any(), anyInt())

        verify(buffer2, never()).dump(any(), anyInt())

    }



    @Test
@@ -142,12 +254,9 @@ class DumpManagerTest : SysuiTestCase() { 
        dumpManager.dumpBuffers(pw, tailLength = 1)



        // THEN all buffers are dumped (and no dumpables)

        verify(dumpable1, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable2, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable3, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2, never()).dump(any(), any())

        verify(dumpable3, never()).dump(any(), any())

        verify(buffer1).dump(pw, tailLength = 1)

        verify(buffer2).dump(pw, tailLength = 1)

    }
@@ -168,10 +277,9 @@ class DumpManagerTest : SysuiTestCase() { 
        // THEN only critical modules are dumped (and no buffers)

        verify(dumpable1).dump(pw, args)

        verify(dumpable2).dump(pw, args)

        verify(dumpable3, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(buffer1, never()).dump(any(PrintWriter::class.java), anyInt())

        verify(buffer2, never()).dump(any(PrintWriter::class.java), anyInt())

        verify(dumpable3, never()).dump(any(), any())

        verify(buffer1, never()).dump(any(), anyInt())

        verify(buffer2, never()).dump(any(), anyInt())

    }



    @Test
@@ -188,10 +296,8 @@ class DumpManagerTest : SysuiTestCase() { 
        dumpManager.dumpNormal(pw, args, tailLength = 2)



        // THEN the normal module and all buffers are dumped

        verify(dumpable1, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable2, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable1, never()).dump(any(), any())

        verify(dumpable2, never()).dump(any(), any())

        verify(dumpable3).dump(pw, args)

        verify(buffer1).dump(pw, tailLength = 2)

        verify(buffer2).dump(pw, tailLength = 2)
@@ -213,9 +319,7 @@ class DumpManagerTest : SysuiTestCase() { 


        // THEN the unregistered dumpables (both normal and critical) are not dumped

        verify(dumpable1).dump(pw, args)

        verify(dumpable2, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable3, never())

            .dump(any(PrintWriter::class.java), any(Array<String>::class.java))

        verify(dumpable2, never()).dump(any(), any())

        verify(dumpable3, never()).dump(any(), any())

    }

}