Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[SB Refactor] Ensure the initial value is also logged to the table.

Browse Source 
Bug: 259454430
Bug: 238425913
Test: `adb shell dumpsys activity service
com.android.systemui/.SystemUIService WifiTableLog` -> the first logs
have all the columns with the Inactive network values
Test: atest PairwiseFlowTest
Test: atest TableLogBufferTest

Change-Id: Iae6b1b70aadc7ac0459b6b800191ff8f2103e244
This commit is contained in:
Caitlin Shkuratov
2022-11-28 20:45:54 +00:00
parent c886aa14e7
commit e3926eeb91
6 changed files with 226 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt
View File

@@ -30,10 +30,11 @@ import kotlinx.coroutines.flow.Flow
*/
interface Diffable<T> {
/**
* Finds the differences between [prevVal] and [this] and logs those diffs to [row].
* Finds the differences between [prevVal] and this object and logs those diffs to [row].
*
* Each implementer should determine which individual fields have changed between [prevVal] and
* [this], and only log the fields that have actually changed. This helps save buffer space.
* this object, and only log the fields that have actually changed. This helps save buffer
* space.
*
* For example, if:
* - prevVal = Object(val1=100, val2=200, val3=300)
@@ -42,6 +43,16 @@ interface Diffable<T> {
* Then only the val3 change should be logged.
*/
fun logDiffs(prevVal: T, row: TableRowLogger)
/**
* Logs all the relevant fields of this object to [row].
*
* As opposed to [logDiffs], this method should log *all* fields.
*
* Implementation is optional. This method will only be used with [logDiffsForTable] in order to
* fully log the initial value of the flow.
*/
fun logFull(row: TableRowLogger) {}
}
/**
@@ -57,7 +68,12 @@ fun <T : Diffable<T>> Flow<T>.logDiffsForTable(
columnPrefix: String,
initialValue: T,
): Flow<T> {
return this.pairwiseBy(initialValue) { prevVal, newVal ->
// Fully log the initial value to the table.
val getInitialValue = {
tableLogBuffer.logChange(columnPrefix) { row -> initialValue.logFull(row) }
initialValue
}
return this.pairwiseBy(getInitialValue) { prevVal: T, newVal: T ->
tableLogBuffer.logDiffs(columnPrefix, prevVal, newVal)
newVal
}

14
packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt
View File

@@ -113,6 +113,20 @@ class TableLogBuffer(
newVal.logDiffs(prevVal, row)
}
/**
* Logs change(s) to the buffer using [rowInitializer].
*
* @param rowInitializer a function that will be called immediately to store relevant data on
* the row.
*/
@Synchronized
fun logChange(columnPrefix: String, rowInitializer: (TableRowLogger) -> Unit) {
val row = tempRow
row.timestamp = systemClock.currentTimeMillis()
row.columnPrefix = columnPrefix
rowInitializer(row)
}
// Keep these individual [logChange] methods private (don't let clients give us their own
// timestamps.)

29
packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiNetworkModel.kt
View File

@@ -31,15 +31,19 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> {
if (prevVal is Inactive) {
return
}
row.logChange(COL_NETWORK_TYPE, TYPE_INACTIVE)
if (prevVal is CarrierMerged) {
// The only difference between CarrierMerged and Inactive is the type
row.logChange(COL_NETWORK_TYPE, TYPE_INACTIVE)
return
}
// When changing from Active to Inactive, we need to log diffs to all the fields.
logDiffsFromActiveToNotActive(prevVal as Active, row)
logFullNonActiveNetwork(TYPE_INACTIVE, row)
}
override fun logFull(row: TableRowLogger) {
logFullNonActiveNetwork(TYPE_INACTIVE, row)
}
}
@@ -56,15 +60,15 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> {
if (prevVal is CarrierMerged) {
return
}
row.logChange(COL_NETWORK_TYPE, TYPE_CARRIER_MERGED)
if (prevVal is Inactive) {
// The only difference between CarrierMerged and Inactive is the type.
row.logChange(COL_NETWORK_TYPE, TYPE_CARRIER_MERGED)
return
}
// When changing from Active to CarrierMerged, we need to log diffs to all the fields.
logDiffsFromActiveToNotActive(prevVal as Active, row)
logFullNonActiveNetwork(TYPE_CARRIER_MERGED, row)
}
}
@@ -147,7 +151,6 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> {
}
}
override fun toString(): String {
// Only include the passpoint-related values in the string if we have them. (Most
// networks won't have them so they'll be mostly clutter.)
@@ -174,21 +177,15 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> {
}
}
internal fun logDiffsFromActiveToNotActive(prevActive: Active, row: TableRowLogger) {
internal fun logFullNonActiveNetwork(type: String, row: TableRowLogger) {
row.logChange(COL_NETWORK_TYPE, type)
row.logChange(COL_NETWORK_ID, NETWORK_ID_DEFAULT)
row.logChange(COL_VALIDATED, false)
row.logChange(COL_LEVEL, LEVEL_DEFAULT)
row.logChange(COL_SSID, null)
if (prevActive.isPasspointAccessPoint) {
row.logChange(COL_PASSPOINT_ACCESS_POINT, false)
}
if (prevActive.isOnlineSignUpForPasspointAccessPoint) {
row.logChange(COL_ONLINE_SIGN_UP, false)
}
if (prevActive.passpointProviderFriendlyName != null) {
row.logChange(COL_PASSPOINT_NAME, null)
}
row.logChange(COL_PASSPOINT_ACCESS_POINT, false)
row.logChange(COL_ONLINE_SIGN_UP, false)
row.logChange(COL_PASSPOINT_NAME, null)
}
}

17
packages/SystemUI/src/com/android/systemui/util/kotlin/Flow.kt
View File

@@ -20,7 +20,6 @@ import java.util.concurrent.atomic.AtomicReference
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.onStart
@@ -57,6 +56,22 @@ fun <T, R> Flow<T>.pairwiseBy(
): Flow<R> =
onStart { emit(initialValue) }.pairwiseBy(transform)
/**
* Returns a new [Flow] that combines the two most recent emissions from [this] using [transform].
*
*
* The output of [getInitialValue] will be used as the "old" value for the first emission. As
* opposed to the initial value in the above [pairwiseBy], [getInitialValue] can do some work before
* returning the initial value.
*
* Useful for code that needs to compare the current value to the previous value.
*/
fun <T, R> Flow<T>.pairwiseBy(
getInitialValue: suspend () -> T,
transform: suspend (previousValue: T, newValue: T) -> R,
): Flow<R> =
onStart { emit(getInitialValue()) }.pairwiseBy(transform)
/**
* Returns a new [Flow] that produces the two most recent emissions from [this]. Note that the new
* Flow will not start emitting until it has received two emissions from the upstream Flow.

62
packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt
View File

@@ -336,6 +336,68 @@ class TableLogBufferTest : SysuiTestCase() {
assertThat(dumpedString).contains(expected2)
}
@Test
fun logChange_rowInitializer_dumpsCorrectly() {
systemClock.setCurrentTimeMillis(100L)
underTest.logChange("") { row ->
row.logChange("column1", "val1")
row.logChange("column2", 2)
row.logChange("column3", true)
}
val dumpedString = dumpChanges()
val timestamp = TABLE_LOG_DATE_FORMAT.format(100L)
val expected1 = timestamp + SEPARATOR + "column1" + SEPARATOR + "val1"
val expected2 = timestamp + SEPARATOR + "column2" + SEPARATOR + "2"
val expected3 = timestamp + SEPARATOR + "column3" + SEPARATOR + "true"
assertThat(dumpedString).contains(expected1)
assertThat(dumpedString).contains(expected2)
assertThat(dumpedString).contains(expected3)
}
@Test
fun logChangeAndLogDiffs_bothLogged() {
systemClock.setCurrentTimeMillis(100L)
underTest.logChange("") { row ->
row.logChange("column1", "val1")
row.logChange("column2", 2)
row.logChange("column3", true)
}
systemClock.setCurrentTimeMillis(200L)
val prevDiffable = object : TestDiffable() {}
val nextDiffable =
object : TestDiffable() {
override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) {
row.logChange("column1", "newVal1")
row.logChange("column2", 222)
row.logChange("column3", false)
}
}
underTest.logDiffs(columnPrefix = "", prevDiffable, nextDiffable)
val dumpedString = dumpChanges()
val timestamp1 = TABLE_LOG_DATE_FORMAT.format(100L)
val expected1 = timestamp1 + SEPARATOR + "column1" + SEPARATOR + "val1"
val expected2 = timestamp1 + SEPARATOR + "column2" + SEPARATOR + "2"
val expected3 = timestamp1 + SEPARATOR + "column3" + SEPARATOR + "true"
val timestamp2 = TABLE_LOG_DATE_FORMAT.format(200L)
val expected4 = timestamp2 + SEPARATOR + "column1" + SEPARATOR + "newVal1"
val expected5 = timestamp2 + SEPARATOR + "column2" + SEPARATOR + "222"
val expected6 = timestamp2 + SEPARATOR + "column3" + SEPARATOR + "false"
assertThat(dumpedString).contains(expected1)
assertThat(dumpedString).contains(expected2)
assertThat(dumpedString).contains(expected3)
assertThat(dumpedString).contains(expected4)
assertThat(dumpedString).contains(expected5)
assertThat(dumpedString).contains(expected6)
}
@Test
fun dumpChanges_rotatesIfBufferIsFull() {
lateinit var valToDump: String

150
packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowUtilTests.kt
View File

@@ -51,15 +51,11 @@ class PairwiseFlowTest : SysuiTestCase() {
)
}
@Test
fun notEnough() = runBlocking {
assertThatFlow(flowOf(1).pairwise()).emitsNothing()
}
@Test fun notEnough() = runBlocking { assertThatFlow(flowOf(1).pairwise()).emitsNothing() }
@Test
fun withInit() = runBlocking {
assertThatFlow(flowOf(2).pairwise(initialValue = 1))
.emitsExactly(WithPrev(1, 2))
assertThatFlow(flowOf(2).pairwise(initialValue = 1)).emitsExactly(WithPrev(1, 2))
}
@Test
@@ -68,25 +64,78 @@ class PairwiseFlowTest : SysuiTestCase() {
}
@Test
fun withStateFlow() = runBlocking(Dispatchers.Main.immediate) {
val state = MutableStateFlow(1)
val stop = MutableSharedFlow<Unit>()
val stoppable = merge(state, stop)
.takeWhile { it is Int }
.filterIsInstance<Int>()
val job1 = launch {
assertThatFlow(stoppable.pairwise()).emitsExactly(WithPrev(1, 2))
}
state.value = 2
val job2 = launch { assertThatFlow(stoppable.pairwise()).emitsNothing() }
stop.emit(Unit)
assertThatJob(job1).isCompleted()
assertThatJob(job2).isCompleted()
fun withTransform() = runBlocking {
assertThatFlow(
flowOf("val1", "val2", "val3").pairwiseBy { prev: String, next: String ->
"$prev|$next"
}
)
.emitsExactly("val1|val2", "val2|val3")
}
@Test
fun withGetInit() = runBlocking {
var initRun = false
assertThatFlow(
flowOf("val1", "val2").pairwiseBy(
getInitialValue = {
initRun = true
"initial"
}
) { prev: String, next: String -> "$prev|$next" }
)
.emitsExactly("initial|val1", "val1|val2")
assertThat(initRun).isTrue()
}
@Test
fun notEnoughWithGetInit() = runBlocking {
var initRun = false
assertThatFlow(
emptyFlow<String>().pairwiseBy(
getInitialValue = {
initRun = true
"initial"
}
) { prev: String, next: String -> "$prev|$next" }
)
.emitsNothing()
// Even though the flow will not emit anything, the initial value function should still get
// run.
assertThat(initRun).isTrue()
}
@Test
fun getInitNotRunWhenFlowNotCollected() = runBlocking {
var initRun = false
flowOf("val1", "val2").pairwiseBy(
getInitialValue = {
initRun = true
"initial"
}
) { prev: String, next: String -> "$prev|$next" }
// Since the flow isn't collected, ensure [initialValueFun] isn't run.
assertThat(initRun).isFalse()
}
@Test
fun withStateFlow() =
runBlocking(Dispatchers.Main.immediate) {
val state = MutableStateFlow(1)
val stop = MutableSharedFlow<Unit>()
val stoppable = merge(state, stop).takeWhile { it is Int }.filterIsInstance<Int>()
val job1 = launch { assertThatFlow(stoppable.pairwise()).emitsExactly(WithPrev(1, 2)) }
state.value = 2
val job2 = launch { assertThatFlow(stoppable.pairwise()).emitsNothing() }
stop.emit(Unit)
assertThatJob(job1).isCompleted()
assertThatJob(job2).isCompleted()
}
}
@SmallTest
@@ -94,18 +143,17 @@ class PairwiseFlowTest : SysuiTestCase() {
class SetChangesFlowTest : SysuiTestCase() {
@Test
fun simple() = runBlocking {
assertThatFlow(
flowOf(setOf(1, 2, 3), setOf(2, 3, 4)).setChanges()
).emitsExactly(
SetChanges(
added = setOf(1, 2, 3),
removed = emptySet(),
),
SetChanges(
added = setOf(4),
removed = setOf(1),
),
)
assertThatFlow(flowOf(setOf(1, 2, 3), setOf(2, 3, 4)).setChanges())
.emitsExactly(
SetChanges(
added = setOf(1, 2, 3),
removed = emptySet(),
),
SetChanges(
added = setOf(4),
removed = setOf(1),
),
)
}
@Test
@@ -147,14 +195,19 @@ class SetChangesFlowTest : SysuiTestCase() {
class SampleFlowTest : SysuiTestCase() {
@Test
fun simple() = runBlocking {
assertThatFlow(flow { yield(); emit(1) }.sample(flowOf(2)) { a, b -> a to b })
assertThatFlow(
flow {
yield()
emit(1)
}
.sample(flowOf(2)) { a, b -> a to b }
)
.emitsExactly(1 to 2)
}
@Test
fun otherFlowNoValueYet() = runBlocking {
assertThatFlow(flowOf(1).sample(emptyFlow<Unit>()))
.emitsNothing()
assertThatFlow(flowOf(1).sample(emptyFlow<Unit>())).emitsNothing()
}
@Test
@@ -178,13 +231,14 @@ class SampleFlowTest : SysuiTestCase() {
}
}
private fun <T> assertThatFlow(flow: Flow<T>) = object {
suspend fun emitsExactly(vararg emissions: T) =
assertThat(flow.toList()).containsExactly(*emissions).inOrder()
suspend fun emitsNothing() =
assertThat(flow.toList()).isEmpty()
}
private fun <T> assertThatFlow(flow: Flow<T>) =
object {
suspend fun emitsExactly(vararg emissions: T) =
assertThat(flow.toList()).containsExactly(*emissions).inOrder()
suspend fun emitsNothing() = assertThat(flow.toList()).isEmpty()
}
private fun assertThatJob(job: Job) = object {
fun isCompleted() = assertThat(job.isCompleted).isTrue()
}
private fun assertThatJob(job: Job) =
object {
fun isCompleted() = assertThat(job.isCompleted).isTrue()
}