diff --git a/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt b/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt index 348d941d22cf4..ccd406001253a 100644 --- a/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt +++ b/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt @@ -79,10 +79,10 @@ fun > Flow.logDiffsForTable( } } -/** - * Each time the boolean flow is updated with a new value that's different from the previous value, - * logs the new value to the given [tableLogBuffer]. - */ +// Here and below: Various Flow extension functions that are effectively equivalent to the +// above [logDiffsForTable] method. + +/** See [logDiffsForTable(TableLogBuffer, String, T)]. */ fun Flow.logDiffsForTable( tableLogBuffer: TableLogBuffer, columnPrefix: String, @@ -100,10 +100,8 @@ fun Flow.logDiffsForTable( newVal } } -/** - * Each time the Int flow is updated with a new value that's different from the previous value, logs - * the new value to the given [tableLogBuffer]. - */ + +/** See [logDiffsForTable(TableLogBuffer, String, T)]. */ fun Flow.logDiffsForTable( tableLogBuffer: TableLogBuffer, columnPrefix: String, @@ -122,10 +120,26 @@ fun Flow.logDiffsForTable( } } -/** - * Each time the String? flow is updated with a new value that's different from the previous value, - * logs the new value to the given [tableLogBuffer]. - */ +/** See [logDiffsForTable(TableLogBuffer, String, T)]. */ +fun Flow.logDiffsForTable( + tableLogBuffer: TableLogBuffer, + columnPrefix: String, + columnName: String, + initialValue: Int?, +): Flow { + val initialValueFun = { + tableLogBuffer.logChange(columnPrefix, columnName, initialValue) + initialValue + } + return this.pairwiseBy(initialValueFun) { prevVal, newVal: Int? -> + if (prevVal != newVal) { + tableLogBuffer.logChange(columnPrefix, columnName, newVal) + } + newVal + } +} + +/** See [logDiffsForTable(TableLogBuffer, String, T)]. */ fun Flow.logDiffsForTable( tableLogBuffer: TableLogBuffer, columnPrefix: String, @@ -143,3 +157,23 @@ fun Flow.logDiffsForTable( newVal } } + +/** See [logDiffsForTable(TableLogBuffer, String, T)]. */ +fun Flow>.logDiffsForTable( + tableLogBuffer: TableLogBuffer, + columnPrefix: String, + columnName: String, + initialValue: List, +): Flow> { + val initialValueFun = { + tableLogBuffer.logChange(columnPrefix, columnName, initialValue.toString()) + initialValue + } + return this.pairwiseBy(initialValueFun) { prevVal, newVal: List -> + if (prevVal != newVal) { + // TODO(b/267761156): Can we log list changes without using toString? + tableLogBuffer.logChange(columnPrefix, columnName, newVal.toString()) + } + newVal + } +} diff --git a/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt b/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt index 68c297f76ab78..4880f80e77168 100644 --- a/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt +++ b/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt @@ -27,7 +27,7 @@ data class TableChange( var columnName: String = "", var type: DataType = DataType.EMPTY, var bool: Boolean = false, - var int: Int = 0, + var int: Int? = null, var str: String? = null, ) { /** Resets to default values so that the object can be recycled. */ @@ -54,7 +54,7 @@ data class TableChange( } /** Sets this to store an int change. */ - fun set(value: Int) { + fun set(value: Int?) { type = DataType.INT int = value } diff --git a/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt b/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt index 2c299d67022d8..1712dab8aff9e 100644 --- a/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt +++ b/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt @@ -138,7 +138,7 @@ class TableLogBuffer( } /** Logs a Int change. */ - fun logChange(prefix: String, columnName: String, value: Int) { + fun logChange(prefix: String, columnName: String, value: Int?) { logChange(systemClock.currentTimeMillis(), prefix, columnName, value) } @@ -155,7 +155,7 @@ class TableLogBuffer( change.set(value) } - private fun logChange(timestamp: Long, prefix: String, columnName: String, value: Int) { + private fun logChange(timestamp: Long, prefix: String, columnName: String, value: Int?) { val change = obtain(timestamp, prefix, columnName) change.set(value) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/log/table/LogDiffsForTableTest.kt b/packages/SystemUI/tests/src/com/android/systemui/log/table/LogDiffsForTableTest.kt index 3b5e6b9c145cb..d1744c61de587 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/log/table/LogDiffsForTableTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/log/table/LogDiffsForTableTest.kt @@ -275,6 +275,52 @@ class LogDiffsForTableTest : SysuiTestCase() { job.cancel() } + @Test + fun intNullable_logsNull() = + testScope.runTest { + systemClock.setCurrentTimeMillis(100L) + val flow = flow { + for (int in listOf(null, 6, null, 8)) { + systemClock.advanceTime(100L) + emit(int) + } + } + + val flowWithLogging = + flow.logDiffsForTable( + tableLogBuffer, + COLUMN_PREFIX, + COLUMN_NAME, + initialValue = 1234, + ) + + val job = launch { flowWithLogging.collect() } + + val logs = dumpLog() + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(100L) + SEPARATOR + FULL_NAME + SEPARATOR + "1234" + ) + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(200L) + SEPARATOR + FULL_NAME + SEPARATOR + "null" + ) + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(300L) + SEPARATOR + FULL_NAME + SEPARATOR + "6" + ) + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(400L) + SEPARATOR + FULL_NAME + SEPARATOR + "null" + ) + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(500L) + SEPARATOR + FULL_NAME + SEPARATOR + "8" + ) + + job.cancel() + } + @Test fun int_logsUpdates() = testScope.runTest { @@ -1030,6 +1076,246 @@ class LogDiffsForTableTest : SysuiTestCase() { job.cancel() } + // ---- Flow> tests ---- + + @Test + fun list_doesNotLogWhenNotCollected() { + val flow = flowOf(listOf(5), listOf(6), listOf(7)) + + flow.logDiffsForTable( + tableLogBuffer, + COLUMN_PREFIX, + COLUMN_NAME, + initialValue = listOf(1234), + ) + + val logs = dumpLog() + assertThat(logs).doesNotContain(COLUMN_PREFIX) + assertThat(logs).doesNotContain(COLUMN_NAME) + assertThat(logs).doesNotContain("1234") + } + + @Test + fun list_logsInitialWhenCollected() = + testScope.runTest { + val flow = flowOf(listOf(5), listOf(6), listOf(7)) + + val flowWithLogging = + flow.logDiffsForTable( + tableLogBuffer, + COLUMN_PREFIX, + COLUMN_NAME, + initialValue = listOf(1234), + ) + + systemClock.setCurrentTimeMillis(3000L) + val job = launch { flowWithLogging.collect() } + + val logs = dumpLog() + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(3000L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf(1234).toString() + ) + + job.cancel() + } + + @Test + fun list_logsUpdates() = + testScope.runTest { + systemClock.setCurrentTimeMillis(100L) + + val listItems = + listOf(listOf("val1", "val2"), listOf("val3"), listOf("val4", "val5", "val6")) + val flow = flow { + for (list in listItems) { + systemClock.advanceTime(100L) + emit(list) + } + } + + val flowWithLogging = + flow.logDiffsForTable( + tableLogBuffer, + COLUMN_PREFIX, + COLUMN_NAME, + initialValue = listOf("val0", "val00"), + ) + + val job = launch { flowWithLogging.collect() } + + val logs = dumpLog() + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(100L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val0", "val00").toString() + ) + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(200L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val1", "val2").toString() + ) + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(300L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val3").toString() + ) + assertThat(logs) + .contains( + TABLE_LOG_DATE_FORMAT.format(400L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val4", "val5", "val6").toString() + ) + + job.cancel() + } + + @Test + fun list_doesNotLogIfSameValue() = + testScope.runTest { + systemClock.setCurrentTimeMillis(100L) + + val listItems = + listOf( + listOf("val0", "val00"), + listOf("val1"), + listOf("val1"), + listOf("val1", "val2"), + ) + val flow = flow { + for (bool in listItems) { + systemClock.advanceTime(100L) + emit(bool) + } + } + + val flowWithLogging = + flow.logDiffsForTable( + tableLogBuffer, + COLUMN_PREFIX, + COLUMN_NAME, + initialValue = listOf("val0", "val00"), + ) + + val job = launch { flowWithLogging.collect() } + + val logs = dumpLog() + + val expected1 = + TABLE_LOG_DATE_FORMAT.format(100L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val0", "val00").toString() + val expected3 = + TABLE_LOG_DATE_FORMAT.format(300L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val1").toString() + val expected5 = + TABLE_LOG_DATE_FORMAT.format(500L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val1", "val2").toString() + assertThat(logs).contains(expected1) + assertThat(logs).contains(expected3) + assertThat(logs).contains(expected5) + + val unexpected2 = + TABLE_LOG_DATE_FORMAT.format(200L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val0", "val00") + val unexpected4 = + TABLE_LOG_DATE_FORMAT.format(400L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf("val1") + assertThat(logs).doesNotContain(unexpected2) + assertThat(logs).doesNotContain(unexpected4) + job.cancel() + } + + @Test + fun list_worksForStateFlows() = + testScope.runTest { + val flow = MutableStateFlow(listOf(1111)) + + val flowWithLogging = + flow.logDiffsForTable( + tableLogBuffer, + COLUMN_PREFIX, + COLUMN_NAME, + initialValue = listOf(1111), + ) + + systemClock.setCurrentTimeMillis(50L) + val job = launch { flowWithLogging.collect() } + assertThat(dumpLog()) + .contains( + TABLE_LOG_DATE_FORMAT.format(50L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf(1111).toString() + ) + + systemClock.setCurrentTimeMillis(100L) + flow.emit(listOf(2222, 3333)) + assertThat(dumpLog()) + .contains( + TABLE_LOG_DATE_FORMAT.format(100L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf(2222, 3333).toString() + ) + + systemClock.setCurrentTimeMillis(200L) + flow.emit(listOf(3333, 4444)) + assertThat(dumpLog()) + .contains( + TABLE_LOG_DATE_FORMAT.format(200L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf(3333, 4444).toString() + ) + + // Doesn't log duplicates + systemClock.setCurrentTimeMillis(300L) + flow.emit(listOf(3333, 4444)) + assertThat(dumpLog()) + .doesNotContain( + TABLE_LOG_DATE_FORMAT.format(300L) + + SEPARATOR + + FULL_NAME + + SEPARATOR + + listOf(3333, 4444).toString() + ) + + job.cancel() + } + private fun dumpLog(): String { val outputWriter = StringWriter() tableLogBuffer.dump(PrintWriter(outputWriter), arrayOf()) diff --git a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt index 432764a7de8fc..c7f3fa0830cd0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt @@ -35,6 +35,17 @@ class TableChangeTest : SysuiTestCase() { assertThat(underTest.getVal()).isEqualTo("fakeValue") } + @Test + fun setString_null() { + val underTest = TableChange() + + underTest.reset(timestamp = 100, columnPrefix = "", columnName = "fakeName") + underTest.set(null as String?) + + assertThat(underTest.hasData()).isTrue() + assertThat(underTest.getVal()).isEqualTo("null") + } + @Test fun setBoolean_isBoolean() { val underTest = TableChange() @@ -57,6 +68,17 @@ class TableChangeTest : SysuiTestCase() { assertThat(underTest.getVal()).isEqualTo("8900") } + @Test + fun setInt_null() { + val underTest = TableChange() + + underTest.reset(timestamp = 100, columnPrefix = "", columnName = "fakeName") + underTest.set(null as Int?) + + assertThat(underTest.hasData()).isTrue() + assertThat(underTest.getVal()).isEqualTo("null") + } + @Test fun setThenReset_isEmpty() { val underTest = TableChange()