Readability clean up for UserRepository
Use a sealed interface for events to eliminate unused properties.
Extract the event branching from the flow statement.
Improve logging and extract to a simple method.
Bug: NA
Test: atest IntentResolver-tests-unit
Flag: NA
Change-Id: I348558b3ca1506b7ece1c19295263e8824ac2575
diff --git a/java/src/com/android/intentresolver/v2/data/repository/UserRepository.kt b/java/src/com/android/intentresolver/v2/data/repository/UserRepository.kt
index 4c42e2c..4067224 100644
--- a/java/src/com/android/intentresolver/v2/data/repository/UserRepository.kt
+++ b/java/src/com/android/intentresolver/v2/data/repository/UserRepository.kt
@@ -28,6 +28,7 @@
import android.content.Intent.EXTRA_USER
import android.content.IntentFilter
import android.content.pm.UserInfo
+import android.os.Build
import android.os.UserHandle
import android.os.UserManager
import android.util.Log
@@ -36,7 +37,6 @@
import com.android.intentresolver.inject.Main
import com.android.intentresolver.inject.ProfileParent
import com.android.intentresolver.v2.data.broadcastFlow
-import com.android.intentresolver.v2.data.repository.UserRepositoryImpl.UserEvent
import com.android.intentresolver.v2.shared.model.User
import dagger.hilt.android.qualifiers.ApplicationContext
import javax.inject.Inject
@@ -73,7 +73,7 @@
* stopping a profile user (along with their many associated processes).
*
* If successful, the change will be applied after the call returns and can be observed using
- * [UserRepository.isAvailable] for the given user.
+ * [UserRepository.availability] for the given user.
*
* No actions are taken if the user is already in requested state.
*
@@ -84,9 +84,9 @@
private const val TAG = "UserRepository"
-private data class UserWithState(val user: User, val available: Boolean)
+internal data class UserWithState(val user: User, val available: Boolean)
-private typealias UserStates = List<UserWithState>
+internal typealias UserStates = List<UserWithState>
/** Tracks and publishes state for the parent user and associated profiles. */
class UserRepositoryImpl
@@ -114,13 +114,21 @@
background
)
- data class UserEvent(val action: String, val user: UserHandle, val quietMode: Boolean = false)
+ private fun debugLog(msg: () -> String) {
+ if (Build.IS_USERDEBUG || Build.IS_ENG) {
+ Log.d(TAG, msg())
+ }
+ }
+
+ private fun errorLog(msg: String, caught: Throwable? = null) {
+ Log.e(TAG, msg, caught)
+ }
/**
* An exception which indicates that an inconsistency exists between the user state map and the
* rest of the system.
*/
- internal class UserStateException(
+ private class UserStateException(
override val message: String,
val event: UserEvent,
override val cause: Throwable? = null
@@ -129,35 +137,34 @@
private val sharingScope = CoroutineScope(scope.coroutineContext + backgroundDispatcher)
private val usersWithState: Flow<UserStates> =
userEvents
- .onStart { emit(UserEvent(INITIALIZE, profileParent)) }
- .onEach { Log.i(TAG, "userEvent: $it") }
- .runningFold<UserEvent, UserStates>(emptyList()) { users, event ->
- try {
- // Handle an action by performing some operation, then returning a new map
- when (event.action) {
- INITIALIZE -> createNewUserStates(profileParent)
- ACTION_PROFILE_ADDED -> handleProfileAdded(event, users)
- ACTION_PROFILE_REMOVED -> handleProfileRemoved(event, users)
- ACTION_MANAGED_PROFILE_UNAVAILABLE,
- ACTION_MANAGED_PROFILE_AVAILABLE,
- ACTION_PROFILE_AVAILABLE,
- ACTION_PROFILE_UNAVAILABLE -> handleAvailability(event, users)
- else -> {
- Log.w(TAG, "Unhandled event: $event)")
- users
- }
- }
- } catch (e: UserStateException) {
- Log.e(TAG, "An error occurred handling an event: ${e.event}", e)
- Log.e(TAG, "Attempting to recover...")
- createNewUserStates(profileParent)
- }
- }
+ .onStart { emit(Initialize) }
+ .onEach { debugLog { "userEvent: $it" } }
+ .runningFold(emptyList(), ::handleEvent)
.distinctUntilChanged()
- .onEach { Log.i(TAG, "userStateList: $it") }
+ .onEach { debugLog { "userStateList: $it" } }
.stateIn(sharingScope, SharingStarted.Eagerly, emptyList())
.filterNot { it.isEmpty() }
+ private suspend fun handleEvent(users: UserStates, event: UserEvent): UserStates {
+ return try {
+ // Handle an action by performing some operation, then returning a new map
+ when (event) {
+ is Initialize -> createNewUserStates(profileParent)
+ is ProfileAdded -> handleProfileAdded(event, users)
+ is ProfileRemoved -> handleProfileRemoved(event, users)
+ is AvailabilityChange -> handleAvailability(event, users)
+ is UnknownEvent -> {
+ debugLog { "Unhandled event: $event)" }
+ users
+ }
+ }
+ } catch (e: UserStateException) {
+ errorLog("An error occurred handling an event: ${e.event}")
+ errorLog("Attempting to recover...", e)
+ createNewUserStates(profileParent)
+ }
+ }
+
override val users: Flow<List<User>> =
usersWithState.map { userStateMap -> userStateMap.map { it.user } }.distinctUntilChanged()
@@ -168,7 +175,7 @@
override suspend fun requestState(user: User, available: Boolean) {
return withContext(backgroundDispatcher) {
- Log.i(TAG, "requestQuietModeEnabled: ${!available} for user $user")
+ debugLog { "requestQuietModeEnabled: ${!available} for user $user" }
userManager.requestQuietModeEnabled(/* enableQuietMode = */ !available, user.handle)
}
}
@@ -176,28 +183,28 @@
private fun List<UserWithState>.update(handle: UserHandle, user: UserWithState) =
filter { it.user.id != handle.identifier } + user
- private fun handleAvailability(event: UserEvent, current: UserStates): UserStates {
+ private fun handleAvailability(event: AvailabilityChange, current: UserStates): UserStates {
val userEntry =
current.firstOrNull { it.user.id == event.user.identifier }
?: throw UserStateException("User was not present in the map", event)
return current.update(event.user, userEntry.copy(available = !event.quietMode))
}
- private fun handleProfileRemoved(event: UserEvent, current: UserStates): UserStates {
+ private fun handleProfileRemoved(event: ProfileRemoved, current: UserStates): UserStates {
if (!current.any { it.user.id == event.user.identifier }) {
throw UserStateException("User was not present in the map", event)
}
return current.filter { it.user.id != event.user.identifier }
}
- private suspend fun handleProfileAdded(event: UserEvent, current: UserStates): UserStates {
+ private suspend fun handleProfileAdded(event: ProfileAdded, current: UserStates): UserStates {
val user =
try {
requireNotNull(readUser(event.user))
} catch (e: Exception) {
throw UserStateException("Failed to read user from UserManager", event, e)
}
- return current + UserWithState(user, !event.quietMode)
+ return current + UserWithState(user, true)
}
private suspend fun createNewUserStates(user: UserHandle): UserStates {
@@ -224,29 +231,64 @@
}
}
+/** A Model representing changes to profiles and availability */
+sealed interface UserEvent
+
+/** Used as a an initial value to trigger a fetch of all profile data. */
+data object Initialize : UserEvent
+
+/** A profile was added to the profile group. */
+data class ProfileAdded(
+ /** The handle for the added profile. */
+ val user: UserHandle,
+) : UserEvent
+
+/** A profile was removed from the profile group. */
+data class ProfileRemoved(
+ /** The handle for the removed profile. */
+ val user: UserHandle,
+) : UserEvent
+
+/** A profile has changed availability. */
+data class AvailabilityChange(
+ /** THe handle for the profile with availability change. */
+ val user: UserHandle,
+ /** The new quietMode state. */
+ val quietMode: Boolean = false,
+) : UserEvent
+
+/** An unhandled event, logged and ignored. */
+data class UnknownEvent(
+ /** The broadcast intent action received */
+ val action: String?,
+) : UserEvent
+
/** Used with [broadcastFlow] to transform a UserManager broadcast action into a [UserEvent]. */
-private fun Intent.toUserEvent(): UserEvent? {
+private fun Intent.toUserEvent(): UserEvent {
val action = action
val user = extras?.getParcelable(EXTRA_USER, UserHandle::class.java)
- val quietMode = extras?.getBoolean(EXTRA_QUIET_MODE, false) ?: false
- return if (user == null || action == null) {
- null
- } else {
- UserEvent(action, user, quietMode)
+ val quietMode = extras?.getBoolean(EXTRA_QUIET_MODE, false)
+ return when (action) {
+ ACTION_PROFILE_ADDED -> ProfileAdded(requireNotNull(user))
+ ACTION_PROFILE_REMOVED -> ProfileRemoved(requireNotNull(user))
+ ACTION_MANAGED_PROFILE_UNAVAILABLE,
+ ACTION_MANAGED_PROFILE_AVAILABLE,
+ ACTION_PROFILE_AVAILABLE,
+ ACTION_PROFILE_UNAVAILABLE ->
+ AvailabilityChange(requireNotNull(user), requireNotNull(quietMode))
+ else -> UnknownEvent(action)
}
}
-const val INITIALIZE = "INITIALIZE"
-
private fun createFilter(actions: Iterable<String>): IntentFilter {
return IntentFilter().apply { actions.forEach(::addAction) }
}
-private fun UserInfo?.isAvailable(): Boolean {
+internal fun UserInfo?.isAvailable(): Boolean {
return this?.isQuietModeEnabled != true
}
-private fun userBroadcastFlow(context: Context, profileParent: UserHandle): Flow<UserEvent> {
+internal fun userBroadcastFlow(context: Context, profileParent: UserHandle): Flow<UserEvent> {
val userActions =
setOf(
ACTION_PROFILE_ADDED,
diff --git a/tests/shared/src/com/android/intentresolver/v2/platform/FakeUserManager.kt b/tests/shared/src/com/android/intentresolver/v2/platform/FakeUserManager.kt
index 370e5a0..d1b56d5 100644
--- a/tests/shared/src/com/android/intentresolver/v2/platform/FakeUserManager.kt
+++ b/tests/shared/src/com/android/intentresolver/v2/platform/FakeUserManager.kt
@@ -1,12 +1,6 @@
package com.android.intentresolver.v2.platform
import android.content.Context
-import android.content.Intent.ACTION_MANAGED_PROFILE_AVAILABLE
-import android.content.Intent.ACTION_MANAGED_PROFILE_UNAVAILABLE
-import android.content.Intent.ACTION_PROFILE_ADDED
-import android.content.Intent.ACTION_PROFILE_AVAILABLE
-import android.content.Intent.ACTION_PROFILE_REMOVED
-import android.content.Intent.ACTION_PROFILE_UNAVAILABLE
import android.content.pm.UserInfo
import android.content.pm.UserInfo.FLAG_FULL
import android.content.pm.UserInfo.FLAG_INITIALIZED
@@ -18,7 +12,10 @@
import androidx.annotation.NonNull
import com.android.intentresolver.THROWS_EXCEPTION
import com.android.intentresolver.mock
-import com.android.intentresolver.v2.data.repository.UserRepositoryImpl.UserEvent
+import com.android.intentresolver.v2.data.repository.AvailabilityChange
+import com.android.intentresolver.v2.data.repository.ProfileAdded
+import com.android.intentresolver.v2.data.repository.ProfileRemoved
+import com.android.intentresolver.v2.data.repository.UserEvent
import com.android.intentresolver.v2.platform.FakeUserManager.State
import com.android.intentresolver.whenever
import kotlin.random.Random
@@ -155,21 +152,7 @@
} else {
it.flags and UserInfo.FLAG_QUIET_MODE.inv()
}
- val actions = mutableListOf<String>()
- if (quietMode) {
- actions += ACTION_PROFILE_UNAVAILABLE
- if (it.isManagedProfile) {
- actions += ACTION_MANAGED_PROFILE_UNAVAILABLE
- }
- } else {
- actions += ACTION_PROFILE_AVAILABLE
- if (it.isManagedProfile) {
- actions += ACTION_MANAGED_PROFILE_AVAILABLE
- }
- }
- actions.forEach { action ->
- eventChannel.trySend(UserEvent(action, user, quietMode))
- }
+ eventChannel.trySend(AvailabilityChange(user, quietMode))
}
}
@@ -187,7 +170,7 @@
profileGroupId = parentUser.profileGroupId
}
userInfoMap[userInfo.userHandle] = userInfo
- eventChannel.trySend(UserEvent(ACTION_PROFILE_ADDED, userInfo.userHandle))
+ eventChannel.trySend(ProfileAdded(userInfo.userHandle))
return userInfo.userHandle
}
@@ -195,7 +178,7 @@
return userInfoMap[handle]?.let { user ->
require(user.isProfile) { "Only profiles can be removed" }
userInfoMap.remove(user.userHandle)
- eventChannel.trySend(UserEvent(ACTION_PROFILE_REMOVED, user.userHandle))
+ eventChannel.trySend(ProfileRemoved(user.userHandle))
return true
}
?: false
diff --git a/tests/unit/src/com/android/intentresolver/v2/data/repository/UserRepositoryImplTest.kt b/tests/unit/src/com/android/intentresolver/v2/data/repository/UserRepositoryImplTest.kt
index 8628bd1..3fcc4c8 100644
--- a/tests/unit/src/com/android/intentresolver/v2/data/repository/UserRepositoryImplTest.kt
+++ b/tests/unit/src/com/android/intentresolver/v2/data/repository/UserRepositoryImplTest.kt
@@ -1,6 +1,5 @@
package com.android.intentresolver.v2.data.repository
-import android.content.Intent
import android.content.pm.UserInfo
import android.os.UserHandle
import android.os.UserHandle.SYSTEM
@@ -122,13 +121,7 @@
fun recovers_from_invalid_profile_added_event() = runTest {
val userManager =
mockUserManager(validUser = USER_SYSTEM, invalidUser = UserHandle.USER_NULL)
- val events =
- flowOf(
- UserRepositoryImpl.UserEvent(
- Intent.ACTION_PROFILE_ADDED,
- UserHandle.of(UserHandle.USER_NULL)
- )
- )
+ val events = flowOf(ProfileAdded(UserHandle.of(UserHandle.USER_NULL)))
val repo =
UserRepositoryImpl(
profileParent = SYSTEM,
@@ -147,13 +140,7 @@
fun recovers_from_invalid_profile_removed_event() = runTest {
val userManager =
mockUserManager(validUser = USER_SYSTEM, invalidUser = UserHandle.USER_NULL)
- val events =
- flowOf(
- UserRepositoryImpl.UserEvent(
- Intent.ACTION_PROFILE_REMOVED,
- UserHandle.of(UserHandle.USER_NULL)
- )
- )
+ val events = flowOf(ProfileRemoved(UserHandle.of(UserHandle.USER_NULL)))
val repo =
UserRepositoryImpl(
profileParent = SYSTEM,
@@ -172,13 +159,7 @@
fun recovers_from_invalid_profile_available_event() = runTest {
val userManager =
mockUserManager(validUser = USER_SYSTEM, invalidUser = UserHandle.USER_NULL)
- val events =
- flowOf(
- UserRepositoryImpl.UserEvent(
- Intent.ACTION_PROFILE_AVAILABLE,
- UserHandle.of(UserHandle.USER_NULL)
- )
- )
+ val events = flowOf(AvailabilityChange(UserHandle.of(UserHandle.USER_NULL)))
val repo =
UserRepositoryImpl(SYSTEM, userManager, events, backgroundScope, Dispatchers.Unconfined)
val users by collectLastValue(repo.users)
@@ -191,10 +172,7 @@
fun recovers_from_unknown_event() = runTest {
val userManager =
mockUserManager(validUser = USER_SYSTEM, invalidUser = UserHandle.USER_NULL)
- val events =
- flowOf(
- UserRepositoryImpl.UserEvent("UNKNOWN_EVENT", UserHandle.of(UserHandle.USER_NULL))
- )
+ val events = flowOf(UnknownEvent("UNKNOWN_EVENT"))
val repo =
UserRepositoryImpl(
profileParent = SYSTEM,
