Merge "[SB Refactor] Update TableLogBuffer based on ABT changes." into tm-qpr-dev

This commit is contained in:
TreeHugger Robot
2022-11-30 00:12:00 +00:00
committed by Android (Google) Code Review
4 changed files with 189 additions and 112 deletions

View File

@@ -16,10 +16,7 @@
package com.android.systemui.log.table
import androidx.annotation.VisibleForTesting
import com.android.systemui.Dumpable
import com.android.systemui.dump.DumpManager
import com.android.systemui.dump.DumpsysTableLogger
import com.android.systemui.plugins.util.RingBuffer
import com.android.systemui.util.time.SystemClock
import java.io.PrintWriter
@@ -83,7 +80,7 @@ class TableLogBuffer(
maxSize: Int,
private val name: String,
private val systemClock: SystemClock,
) {
) : Dumpable {
init {
if (maxSize <= 0) {
throw IllegalArgumentException("maxSize must be > 0")
@@ -104,6 +101,9 @@ class TableLogBuffer(
* @param columnPrefix a prefix that will be applied to every column name that gets logged. This
* ensures that all the columns related to the same state object will be grouped together in the
* table.
*
* @throws IllegalArgumentException if [columnPrefix] or column name contain "|". "|" is used as
* the separator token for parsing, so it can't be present in any part of the column name.
*/
@Synchronized
fun <T : Diffable<T>> logDiffs(columnPrefix: String, prevVal: T, newVal: T) {
@@ -135,32 +135,31 @@ class TableLogBuffer(
@Synchronized
private fun obtain(timestamp: Long, prefix: String, columnName: String): TableChange {
verifyValidName(prefix, columnName)
val tableChange = buffer.advance()
tableChange.reset(timestamp, prefix, columnName)
return tableChange
}
/**
* Registers this buffer as dumpables in [dumpManager]. Must be called for the table to be
* dumped.
*
* This will be automatically called in [TableLogBufferFactory.create].
*/
fun registerDumpables(dumpManager: DumpManager) {
dumpManager.registerNormalDumpable("$name-changes", changeDumpable)
dumpManager.registerNormalDumpable("$name-table", tableDumpable)
private fun verifyValidName(prefix: String, columnName: String) {
if (prefix.contains(SEPARATOR)) {
throw IllegalArgumentException("columnPrefix cannot contain $SEPARATOR but was $prefix")
}
if (columnName.contains(SEPARATOR)) {
throw IllegalArgumentException(
"columnName cannot contain $SEPARATOR but was $columnName"
)
}
}
private val changeDumpable = Dumpable { pw, _ -> dumpChanges(pw) }
private val tableDumpable = Dumpable { pw, _ -> dumpTable(pw) }
/** Dumps the list of [TableChange] objects. */
@Synchronized
@VisibleForTesting
fun dumpChanges(pw: PrintWriter) {
override fun dump(pw: PrintWriter, args: Array<out String>) {
pw.println(HEADER_PREFIX + name)
pw.println("version $VERSION")
for (i in 0 until buffer.size) {
buffer[i].dump(pw)
}
pw.println(FOOTER_PREFIX + name)
}
/** Dumps an individual [TableChange]. */
@@ -170,69 +169,13 @@ class TableLogBuffer(
}
val formattedTimestamp = TABLE_LOG_DATE_FORMAT.format(timestamp)
pw.print(formattedTimestamp)
pw.print(" ")
pw.print(SEPARATOR)
pw.print(this.getName())
pw.print("=")
pw.print(SEPARATOR)
pw.print(this.getVal())
pw.println()
}
/**
* Coalesces all the [TableChange] objects into a table of values of time and dumps the table.
*/
// TODO(b/259454430): Since this is an expensive process, it could cause the bug report dump to
// fail and/or not dump anything else. We should move this processing to ABT (Android Bug
// Tool), where we have unlimited time to process.
@Synchronized
@VisibleForTesting
fun dumpTable(pw: PrintWriter) {
val messages = buffer.iterator().asSequence().toList()
if (messages.isEmpty()) {
return
}
// Step 1: Create list of column headers
val headerSet = mutableSetOf<String>()
messages.forEach { headerSet.add(it.getName()) }
val headers: MutableList<String> = headerSet.toList().sorted().toMutableList()
headers.add(0, "timestamp")
// Step 2: Create a list with the current values for each column. Will be updated with each
// change.
val currentRow: MutableList<String> = MutableList(headers.size) { DEFAULT_COLUMN_VALUE }
// Step 3: For each message, make the correct update to [currentRow] and save it to [rows].
val columnIndices: Map<String, Int> =
headers.mapIndexed { index, headerName -> headerName to index }.toMap()
val allRows = mutableListOf<List<String>>()
messages.forEach {
if (!it.hasData()) {
return@forEach
}
val formattedTimestamp = TABLE_LOG_DATE_FORMAT.format(it.timestamp)
if (formattedTimestamp != currentRow[0]) {
// The timestamp has updated, so save the previous row and continue to the next row
allRows.add(currentRow.toList())
currentRow[0] = formattedTimestamp
}
val columnIndex = columnIndices[it.getName()]!!
currentRow[columnIndex] = it.getVal()
}
// Add the last row
allRows.add(currentRow.toList())
// Step 4: Dump the rows
DumpsysTableLogger(
name,
headers,
allRows,
)
.printTableData(pw)
}
/**
* A private implementation of [TableRowLogger].
*
@@ -261,4 +204,8 @@ class TableLogBuffer(
}
val TABLE_LOG_DATE_FORMAT = SimpleDateFormat("MM-dd HH:mm:ss.SSS", Locale.US)
private const val DEFAULT_COLUMN_VALUE = "UNKNOWN"
private const val HEADER_PREFIX = "SystemUI StateChangeTableSection START: "
private const val FOOTER_PREFIX = "SystemUI StateChangeTableSection END: "
private const val SEPARATOR = "|"
private const val VERSION = "1"

View File

@@ -34,7 +34,7 @@ constructor(
maxSize: Int,
): TableLogBuffer {
val tableBuffer = TableLogBuffer(adjustMaxSize(maxSize), name, systemClock)
tableBuffer.registerDumpables(dumpManager)
dumpManager.registerNormalDumpable(name, tableBuffer)
return tableBuffer
}
}

View File

@@ -121,7 +121,11 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> {
row.logChange(COL_VALIDATED, isValidated)
}
if (prevVal !is Active || prevVal.level != level) {
row.logChange(COL_LEVEL, level ?: LEVEL_DEFAULT)
if (level != null) {
row.logChange(COL_LEVEL, level)
} else {
row.logChange(COL_LEVEL, LEVEL_DEFAULT)
}
}
if (prevVal !is Active || prevVal.ssid != ssid) {
row.logChange(COL_SSID, ssid)
@@ -201,5 +205,5 @@ const val COL_PASSPOINT_ACCESS_POINT = "isPasspointAccessPoint"
const val COL_ONLINE_SIGN_UP = "isOnlineSignUpForPasspointAccessPoint"
const val COL_PASSPOINT_NAME = "passpointProviderFriendlyName"
const val LEVEL_DEFAULT = -1
const val NETWORK_ID_DEFAULT = -1
val LEVEL_DEFAULT: String? = null
val NETWORK_ID_DEFAULT: String? = null

View File

@@ -45,6 +45,93 @@ class TableLogBufferTest : SysuiTestCase() {
TableLogBuffer(maxSize = 0, "name", systemClock)
}
@Test
fun dumpChanges_hasHeader() {
val dumpedString = dumpChanges()
assertThat(logLines(dumpedString)[0]).isEqualTo(HEADER_PREFIX + NAME)
}
@Test
fun dumpChanges_hasVersion() {
val dumpedString = dumpChanges()
assertThat(logLines(dumpedString)[1]).isEqualTo("version $VERSION")
}
@Test
fun dumpChanges_hasFooter() {
val dumpedString = dumpChanges()
assertThat(logLines(dumpedString).last()).isEqualTo(FOOTER_PREFIX + NAME)
}
@Test(expected = IllegalArgumentException::class)
fun dumpChanges_str_separatorNotAllowedInPrefix() {
val next =
object : TestDiffable() {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {
row.logChange("columnName", "stringValue")
}
}
underTest.logDiffs("some${SEPARATOR}thing", TestDiffable(), next)
}
@Test(expected = IllegalArgumentException::class)
fun dumpChanges_bool_separatorNotAllowedInPrefix() {
val next =
object : TestDiffable() {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {
row.logChange("columnName", true)
}
}
underTest.logDiffs("some${SEPARATOR}thing", TestDiffable(), next)
}
@Test(expected = IllegalArgumentException::class)
fun dumpChanges_int_separatorNotAllowedInPrefix() {
val next =
object : TestDiffable() {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {
row.logChange("columnName", 567)
}
}
underTest.logDiffs("some${SEPARATOR}thing", TestDiffable(), next)
}
@Test(expected = IllegalArgumentException::class)
fun dumpChanges_str_separatorNotAllowedInColumnName() {
val next =
object : TestDiffable() {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {
row.logChange("column${SEPARATOR}Name", "stringValue")
}
}
underTest.logDiffs("prefix", TestDiffable(), next)
}
@Test(expected = IllegalArgumentException::class)
fun dumpChanges_bool_separatorNotAllowedInColumnName() {
val next =
object : TestDiffable() {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {
row.logChange("column${SEPARATOR}Name", true)
}
}
underTest.logDiffs("prefix", TestDiffable(), next)
}
@Test(expected = IllegalArgumentException::class)
fun dumpChanges_int_separatorNotAllowedInColumnName() {
val next =
object : TestDiffable() {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {
row.logChange("column${SEPARATOR}Name", 456)
}
}
underTest.logDiffs("prefix", TestDiffable(), next)
}
@Test
fun dumpChanges_strChange_logsFromNext() {
systemClock.setCurrentTimeMillis(100L)
@@ -66,11 +153,14 @@ class TableLogBufferTest : SysuiTestCase() {
val dumpedString = dumpChanges()
assertThat(dumpedString).contains("prefix")
assertThat(dumpedString).contains("stringValChange")
assertThat(dumpedString).contains("newStringVal")
val expected =
TABLE_LOG_DATE_FORMAT.format(100L) +
SEPARATOR +
"prefix.stringValChange" +
SEPARATOR +
"newStringVal"
assertThat(dumpedString).contains(expected)
assertThat(dumpedString).doesNotContain("prevStringVal")
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))
}
@Test
@@ -94,11 +184,14 @@ class TableLogBufferTest : SysuiTestCase() {
val dumpedString = dumpChanges()
assertThat(dumpedString).contains("prefix")
assertThat(dumpedString).contains("booleanValChange")
assertThat(dumpedString).contains("true")
val expected =
TABLE_LOG_DATE_FORMAT.format(100L) +
SEPARATOR +
"prefix.booleanValChange" +
SEPARATOR +
"true"
assertThat(dumpedString).contains(expected)
assertThat(dumpedString).doesNotContain("false")
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))
}
@Test
@@ -122,11 +215,14 @@ class TableLogBufferTest : SysuiTestCase() {
val dumpedString = dumpChanges()
assertThat(dumpedString).contains("prefix")
assertThat(dumpedString).contains("intValChange")
assertThat(dumpedString).contains("67890")
val expected =
TABLE_LOG_DATE_FORMAT.format(100L) +
SEPARATOR +
"prefix.intValChange" +
SEPARATOR +
"67890"
assertThat(dumpedString).contains(expected)
assertThat(dumpedString).doesNotContain("12345")
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))
}
@Test
@@ -152,9 +248,9 @@ class TableLogBufferTest : SysuiTestCase() {
val dumpedString = dumpChanges()
// THEN the dump still works
assertThat(dumpedString).contains("booleanValChange")
assertThat(dumpedString).contains("true")
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(100L))
val expected =
TABLE_LOG_DATE_FORMAT.format(100L) + SEPARATOR + "booleanValChange" + SEPARATOR + "true"
assertThat(dumpedString).contains(expected)
}
@Test
@@ -186,15 +282,34 @@ class TableLogBufferTest : SysuiTestCase() {
val dumpedString = dumpChanges()
assertThat(dumpedString).contains("valChange")
assertThat(dumpedString).contains("stateValue12")
assertThat(dumpedString).contains("stateValue20")
assertThat(dumpedString).contains("stateValue40")
assertThat(dumpedString).contains("stateValue45")
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(12000L))
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(20000L))
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(40000L))
assertThat(dumpedString).contains(TABLE_LOG_DATE_FORMAT.format(45000L))
val expected1 =
TABLE_LOG_DATE_FORMAT.format(12000L) +
SEPARATOR +
"valChange" +
SEPARATOR +
"stateValue12"
val expected2 =
TABLE_LOG_DATE_FORMAT.format(20000L) +
SEPARATOR +
"valChange" +
SEPARATOR +
"stateValue20"
val expected3 =
TABLE_LOG_DATE_FORMAT.format(40000L) +
SEPARATOR +
"valChange" +
SEPARATOR +
"stateValue40"
val expected4 =
TABLE_LOG_DATE_FORMAT.format(45000L) +
SEPARATOR +
"valChange" +
SEPARATOR +
"stateValue45"
assertThat(dumpedString).contains(expected1)
assertThat(dumpedString).contains(expected2)
assertThat(dumpedString).contains(expected3)
assertThat(dumpedString).contains(expected4)
}
@Test
@@ -214,10 +329,11 @@ class TableLogBufferTest : SysuiTestCase() {
val dumpedString = dumpChanges()
assertThat(dumpedString).contains("status")
assertThat(dumpedString).contains("in progress")
assertThat(dumpedString).contains("connected")
assertThat(dumpedString).contains("false")
val timestamp = TABLE_LOG_DATE_FORMAT.format(100L)
val expected1 = timestamp + SEPARATOR + "status" + SEPARATOR + "in progress"
val expected2 = timestamp + SEPARATOR + "connected" + SEPARATOR + "false"
assertThat(dumpedString).contains(expected1)
assertThat(dumpedString).contains(expected2)
}
@Test
@@ -247,14 +363,24 @@ class TableLogBufferTest : SysuiTestCase() {
}
private fun dumpChanges(): String {
underTest.dumpChanges(PrintWriter(outputWriter))
underTest.dump(PrintWriter(outputWriter), arrayOf())
return outputWriter.toString()
}
private abstract class TestDiffable : Diffable<TestDiffable> {
private fun logLines(string: String): List<String> {
return string.split("\n").filter { it.isNotBlank() }
}
private open class TestDiffable : Diffable<TestDiffable> {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {}
}
}
private const val NAME = "TestTableBuffer"
private const val MAX_SIZE = 10
// Copying these here from [TableLogBuffer] so that we catch any accidental versioning change
private const val HEADER_PREFIX = "SystemUI StateChangeTableSection START: "
private const val FOOTER_PREFIX = "SystemUI StateChangeTableSection END: "
private const val SEPARATOR = "|" // TBD
private const val VERSION = "1"