Cache smartspace timer creation time (2ce526f3) · Commits · e / os / android_frameworks_base

packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt

+183 −2

Original line number	Diff line number	Diff line
		@@ -29,6 +29,7 @@ import com.android.systemui.coroutines.collectLastValue
		import com.android.systemui.kosmos.testScope
		import com.android.systemui.plugins.BcSmartspaceDataPlugin.SmartspaceTargetListener
		import com.android.systemui.testKosmos
		import com.android.systemui.util.time.fakeSystemClock
		import com.google.common.truth.Truth.assertThat
		import kotlinx.coroutines.ExperimentalCoroutinesApi
		import kotlinx.coroutines.test.runCurrent
		@@ -54,12 +55,18 @@ class CommunalSmartspaceRepositoryImplTest : SysuiTestCase() {

		private val smartspaceController = mock<CommunalSmartspaceController>()
		private val fakeExecutor = kosmos.fakeExecutor
		private val systemClock = kosmos.fakeSystemClock

		private lateinit var underTest: CommunalSmartspaceRepositoryImpl

		@Before
		fun setUp() {
		underTest = CommunalSmartspaceRepositoryImpl(smartspaceController, fakeExecutor)
		underTest =
		CommunalSmartspaceRepositoryImpl(
		smartspaceController,
		fakeExecutor,
		systemClock,
		)
		}

		@DisableFlags(FLAG_REMOTE_VIEWS)
		@@ -127,10 +134,184 @@ class CommunalSmartspaceRepositoryImplTest : SysuiTestCase() {
		runCurrent()

		// Verify that only the valid target is listed
		assertThat(communalTimers?.size).isEqualTo(1)
		assertThat(communalTimers).hasSize(1)
		assertThat(communalTimers?.first()?.smartspaceTargetId).isEqualTo("timer-1-started")
		}

		@EnableFlags(FLAG_REMOTE_VIEWS)
		@Test
		fun communalTimers_cacheCreationTime() =
		testScope.runTest {
		underTest.startListening()

		val communalTimers by collectLastValue(underTest.timers)
		runCurrent()
		fakeExecutor.runAllReady()

		val listener = captureSmartspaceTargetListener()
		listener.onSmartspaceTargetsUpdated(
		listOf(
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-0-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(1000)
		},
		)
		)
		runCurrent()

		// Verify that the creation time is the current time, not the creation time passed in
		// the target, because this value can be inaccurate (due to b/318535930).
		val currentTime = systemClock.currentTimeMillis()
		assertThat(communalTimers?.get(0)?.createdTimestampMillis).isEqualTo(currentTime)
		assertThat(communalTimers?.get(0)?.createdTimestampMillis).isNotEqualTo(1000)

		// A second timer is added.
		listener.onSmartspaceTargetsUpdated(
		listOf(
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-0-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(2000)
		},
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-1-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(3000)
		},
		)
		)
		runCurrent()

		// Verify that the created timestamp for the first time is consistent
		assertThat(communalTimers?.get(0)?.createdTimestampMillis).isEqualTo(currentTime)

		// Verify that the second timer has a new creation time
		assertThat(communalTimers?.get(1)?.createdTimestampMillis)
		.isEqualTo(systemClock.currentTimeMillis())
		}

		@EnableFlags(FLAG_REMOTE_VIEWS)
		@Test
		fun communalTimers_creationTimeRemovedFromCacheWhenTimerRemoved() =
		testScope.runTest {
		underTest.startListening()

		val communalTimers by collectLastValue(underTest.timers)
		runCurrent()
		fakeExecutor.runAllReady()

		// Start timer 0
		val listener = captureSmartspaceTargetListener()
		listener.onSmartspaceTargetsUpdated(
		listOf(
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-0-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(1000)
		},
		)
		)
		runCurrent()

		// Verify timer 0 creation time
		val expectedCreationTimeForTimer0 = systemClock.currentTimeMillis()
		assertThat(communalTimers?.first()?.createdTimestampMillis)
		.isEqualTo(expectedCreationTimeForTimer0)

		// Advance some time
		systemClock.advanceTime(1000)

		// Start timer 1
		listener.onSmartspaceTargetsUpdated(
		listOf(
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-0-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(1000)
		},
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-1-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(2000)
		},
		)
		)
		runCurrent()

		// Verify timer 1 creation time is new
		val expectedCreationTimeForTimer1 = expectedCreationTimeForTimer0 + 1000
		assertThat(communalTimers?.get(1)?.createdTimestampMillis)
		.isEqualTo(expectedCreationTimeForTimer1)

		// Removed timer 0
		listener.onSmartspaceTargetsUpdated(
		listOf(
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-1-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(2000)
		},
		)
		)
		runCurrent()

		// Verify timer 0 removed, and timer 1 creation time is correct
		assertThat(communalTimers).hasSize(1)
		assertThat(communalTimers?.first()?.createdTimestampMillis)
		.isEqualTo(expectedCreationTimeForTimer1)

		// Advance some time
		systemClock.advanceTime(1000)

		// Start timer 0 again. Technically this is a new timer, but timers can reused stable
		// ids.
		listener.onSmartspaceTargetsUpdated(
		listOf(
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-1-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(2000)
		},
		mock<SmartspaceTarget> {
		on { smartspaceTargetId }.doReturn("timer-0-started")
		on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
		on { remoteViews }.doReturn(mock())
		on { creationTimeMillis }.doReturn(3000)
		},
		)
		)
		runCurrent()

		// Verify new timer added, and timer 1 creation time is still correct
		assertThat(communalTimers).hasSize(2)
		assertThat(communalTimers?.get(0)?.createdTimestampMillis)
		.isEqualTo(expectedCreationTimeForTimer1)

		// Verify creation time for the new timer is new, meaning that cache for timer 0 was
		// removed when it was removed
		assertThat(communalTimers?.get(1)?.createdTimestampMillis)
		.isEqualTo(expectedCreationTimeForTimer1 + 1000)
		}

		@Test
		fun stableId() {
		assertThat(CommunalSmartspaceRepositoryImpl.stableId("timer-0-12345-started"))
		.isEqualTo("timer-0")
		assertThat(CommunalSmartspaceRepositoryImpl.stableId("timer-1-67890-paused"))
		.isEqualTo("timer-1")
		assertThat(CommunalSmartspaceRepositoryImpl.stableId("i_am_an_unexpected_id"))
		.isEqualTo("i_am_an_unexpected_id")
		}

		private fun captureSmartspaceTargetListener(): SmartspaceTargetListener {
		verify(smartspaceController).addListener(listenerCaptor.capture())
		return listenerCaptor.firstValue

packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt

+43 −9

Original line number	Diff line number	Diff line
		@@ -18,11 +18,13 @@ package com.android.systemui.communal.data.repository

		import android.app.smartspace.SmartspaceTarget
		import android.os.Parcelable
		import androidx.annotation.VisibleForTesting
		import com.android.systemui.communal.data.model.CommunalSmartspaceTimer
		import com.android.systemui.communal.smartspace.CommunalSmartspaceController
		import com.android.systemui.dagger.SysUISingleton
		import com.android.systemui.dagger.qualifiers.Main
		import com.android.systemui.plugins.BcSmartspaceDataPlugin
		import com.android.systemui.util.time.SystemClock
		import java.util.concurrent.Executor
		import javax.inject.Inject
		import kotlinx.coroutines.flow.Flow
		@@ -45,25 +47,41 @@ class CommunalSmartspaceRepositoryImpl
		constructor(
		private val communalSmartspaceController: CommunalSmartspaceController,
		@Main private val uiExecutor: Executor,
		private val systemClock: SystemClock,
		) : CommunalSmartspaceRepository, BcSmartspaceDataPlugin.SmartspaceTargetListener {

		private val _timers: MutableStateFlow<List<CommunalSmartspaceTimer>> =
		MutableStateFlow(emptyList())
		override val timers: Flow<List<CommunalSmartspaceTimer>> = _timers

		private var targetCreationTimes = emptyMap<String, Long>()

		override fun onSmartspaceTargetsUpdated(targetsNullable: MutableList<out Parcelable>?) {
		val targets = targetsNullable?.filterIsInstance<SmartspaceTarget>() ?: emptyList()

		_timers.value =
		val timerTargets =
		targets
		.filter { target ->
		target.featureType == SmartspaceTarget.FEATURE_TIMER &&
		target.remoteViews != null
		}
		.map { target ->
		.associateBy { stableId(it.smartspaceTargetId) }

		// The creation times from smartspace targets are unreliable (b/318535930). Therefore,
		// SystemUI uses the timestamp of which a timer first appears, and caches these values to
		// prevent timers from swapping positions in the hub.
		targetCreationTimes =
		timerTargets.mapValues { (stableId, _) ->
		targetCreationTimes[stableId] ?: systemClock.currentTimeMillis()
		}

		_timers.value =
		timerTargets.map { (stableId, target) ->
		CommunalSmartspaceTimer(
		// The view layer should have the instance based smartspaceTargetId instead of
		// stable id, so that when a new instance of the timer is created, for example,
		// when it is paused, the view should re-render its remote views.
		smartspaceTargetId = target.smartspaceTargetId,
		createdTimestampMillis = target.creationTimeMillis,
		createdTimestampMillis = targetCreationTimes[stableId]!!,
		remoteViews = target.remoteViews!!,
		)
		}
		@@ -86,4 +104,20 @@ constructor(
		)
		}
		}

		companion object {
		/**
		* The smartspace target id is instance-based, meaning a single timer (from the user's
		* perspective) can have multiple instances. For example, when a timer is paused, a new
		* instance is created. To address this, SystemUI manually removes the instance id to
		* maintain a consistent id across sessions.
		*
		* It is assumed that timer target ids follow this format: timer-${stableId}-${instanceId}.
		* This function returns timer-${stableId}, stripping out the instance id.
		*/
		@VisibleForTesting
		fun stableId(targetId: String): String {
		return targetId.split("-").take(2).joinToString("-")
		}
		}
		}