From f01c5d367cb61ab0aa0cba048a07b46df975ecf0 Mon Sep 17 00:00:00 2001 From: Damontecres Date: Sun, 11 Oct 2026 20:11:09 -0400 Subject: [PATCH] Updates to session restoration (#1905) ## Description This PR reworks some of the internal handling of changing users and auto sign in. 1. Most recently used server/user is migrated from `AppPreferences` to `MostRecentServerProvider` 2. Checking if a user profile is protected occurs at a lower level in `ServerRepository` so fewer services need to check for this 3. Now always defaults to the most recent server's user list in cases where the most recent user can't be switched to such as if the profile is protected, auto signin is disabled, or and error occurred The default implementation for `MostRecentServerProvider` uses Android's `SharedPreferences`. `SharedPreferences` was already was being used to track the most recent server for crash reporting (the crash reporting process cannot use the Hilt DI). This PR enhances this and adds a better API for saving/retrieving. This also removes the last bit of "user specific" data from `AppPreferences`. ### Related issues None specific, but there have been a few related to auto sign-in & intents that were recently fixed. This PR is a more detailed approach to those same problems. ### Testing Manually via the emulator. There are also unit tests for most (all?) of the different settings and state combinations. ## Screenshots N/A ## AI or LLM usage None --- .../damontecres/wholphin/MainActivity.kt | 221 ++++++++-------- .../wholphin/WholphinDreamService.kt | 9 +- .../wholphin/data/MostRecentServer.kt | 181 +++++++++++++ .../wholphin/data/ServerRepository.kt | 180 +++++++------ .../wholphin/services/AppUpgradeHandler.kt | 28 ++ .../wholphin/services/HomeSettingsService.kt | 4 +- .../wholphin/services/IntentService.kt | 43 ++-- .../wholphin/services/LatestNextUpWorker.kt | 12 +- .../wholphin/services/SuggestionsWorker.kt | 9 +- .../wholphin/services/hilt/AppModule.kt | 8 + .../services/tvprovider/TvProviderWorker.kt | 9 +- .../wholphin/ui/playback/PlaybackViewModel.kt | 5 +- .../wholphin/util/CrashReportSenderFactory.kt | 56 ++-- app/src/main/proto/WholphinDataStore.proto | 5 +- .../{test => }/TestMainActivityViewModel.kt | 243 ++++++++++-------- .../wholphin/services/IntentServiceTest.kt | 83 +++--- .../wholphin/services/ServerRepositoryTest.kt | 169 ++++++++---- .../services/SuggestionsWorkerTest.kt | 8 +- .../damontecres/wholphin/ui/TestModule.kt | 6 + 19 files changed, 786 insertions(+), 493 deletions(-) create mode 100644 app/src/main/java/com/github/damontecres/wholphin/data/MostRecentServer.kt rename app/src/test/java/com/github/damontecres/wholphin/{test => }/TestMainActivityViewModel.kt (61%) diff --git a/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt b/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt index 62f0e39b..c9eb61eb 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt @@ -9,6 +9,7 @@ import android.view.WindowManager import android.widget.Toast import androidx.activity.compose.setContent import androidx.activity.viewModels +import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AppCompatActivity import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box @@ -75,7 +76,6 @@ import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.delay import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch @@ -84,7 +84,6 @@ import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlinx.serialization.json.Json import okhttp3.OkHttpClient -import org.jellyfin.sdk.model.serializer.toUUIDOrNull import timber.log.Timber import javax.inject.Inject import kotlin.time.Duration.Companion.milliseconds @@ -436,120 +435,27 @@ class MainActivityViewModel viewModelScope.launchDefault { mutex.withLock { try { - val result = intent?.let { intentService.parseIntent(intent) } - when (result) { - is IntentResult.Error -> { - val current = serverRepository.current.value - val destination = - if (current != null) { - SetupDestination.UserList(current.server) - } else { - SetupDestination.ServerList - } - setupNavigationManager.navigateTo(destination) - Timber.e("Error parsing intent: %s", result.message) - showToast(context, result.message) - return@withLock - } + handleAppUpgrade() + } catch (ex: Exception) { + Timber.e(ex, "Error during app upgrade!") + showToast( + context, + "Error during app upgrade: ${ex.localizedMessage}", + ) + } - is IntentResult.Target -> { - val current = serverRepository.current.value - if (current != null) { - Timber.i("Received valid intent, switching to AppContent") - - if (result.addHomeToBackStack) { - navigationManager.reloadHome() - } else { - navigationManager.backStack.clear() - } - navigationManager.backStack.addAll(result.destinations) - - setupNavigationManager.navigateTo( - SetupDestination.AppContent(current), - ) - } else { - // This should never happen, but just reset app state if it does - setupNavigationManager.navigateTo(SetupDestination.ServerList) - Timber.e("Error parsing intent, no user is active") - showToast(context, "An error occurred parsing the intent") - } - return@withLock - } - - IntentResult.NoOp, - null, - -> { - // No-op, proceed below - } + try { + val result = handleIntent(intent) + if (result) { + Timber.i("appStart was handled by intent") + return@withLock } } catch (ex: Exception) { Timber.e(ex, "Error parsing intent") } try { - val needUpgrade = appUpgradeHandler.needUpgrade() - if (needUpgrade) { - showToast( - context, - context.getString( - R.string.updated_toast, - appUpgradeHandler.currentVersion.toString(), - ), - ) - appUpgradeHandler.run() - } - appUpgradeHandler.copySubfont(false) - val prefs = - preferences.data.firstOrNull() ?: AppPreferences.getDefaultInstance() - val profileProtected = - serverRepository.current.value - ?.user - ?.isProtected == true - if (prefs.signInAutomatically && !profileProtected) { - val current = - serverRepository.restoreSession( - prefs.currentServerId?.toUUIDOrNull(), - prefs.currentUserId?.toUUIDOrNull(), - ) - if (current != null) { - if (current.user.isProtected) { - setupNavigationManager.navigateTo( - SetupDestination.UserList( - current.server, - ), - ) - } else { - // Restored - setupNavigationManager.navigateTo( - SetupDestination.AppContent( - current, - ), - ) - } - } else { - // Did not restore - setupNavigationManager.navigateTo(SetupDestination.ServerList) - } - } else { - setupNavigationManager.navigateTo(SetupDestination.Loading) - backdropService.clearBackdrop() - val currentServerId = prefs.currentServerId?.toUUIDOrNull() - if (currentServerId != null) { - val currentServer = - serverRepository.serverDao.getServer(currentServerId)?.server - if (currentServer != null) { - setupNavigationManager.navigateTo( - SetupDestination.UserList( - currentServer, - ), - ) - } else { - setupNavigationManager.navigateTo(SetupDestination.ServerList) - } - } else { - setupNavigationManager.navigateTo(SetupDestination.ServerList) - } - } + handleAppStart() } catch (ex: Exception) { Timber.e(ex, "Error during appStart") setupNavigationManager.navigateTo(SetupDestination.ServerList) @@ -574,6 +480,103 @@ class MainActivityViewModel } } } + + /** + * Handle an [Intent], navigating if needed + * + * @return true if the intent was handled, false if normal app start should proceed + */ + @VisibleForTesting + internal suspend fun handleIntent(intent: Intent?): Boolean { + val result = intent?.let { intentService.parseIntent(intent) } + when (result) { + is IntentResult.Error -> { + val current = serverRepository.current.value + val destination = + if (current != null) { + SetupDestination.UserList(current.server) + } else { + SetupDestination.ServerList + } + setupNavigationManager.navigateTo(destination) + Timber.e("Error parsing intent: %s", result.message) + showToast(context, result.message) + return true + } + + is IntentResult.Target -> { + val current = serverRepository.current.value + if (current != null) { + Timber.i("Received valid intent, switching to AppContent") + + if (result.addHomeToBackStack) { + navigationManager.reloadHome() + } else { + navigationManager.backStack.clear() + } + navigationManager.backStack.addAll(result.destinations) + + setupNavigationManager.navigateTo( + SetupDestination.AppContent(current), + ) + } else { + // This should never happen, but just reset app state if it does + setupNavigationManager.navigateTo(SetupDestination.ServerList) + Timber.e("Error parsing intent, no user is active") + showToast(context, "An error occurred parsing the intent") + } + return true + } + + IntentResult.NoOp, + null, + -> { + // Not handled + return false + } + } + } + + private suspend fun handleAppUpgrade() { + val needUpgrade = appUpgradeHandler.needUpgrade() + if (needUpgrade) { + showToast( + context, + context.getString( + R.string.updated_toast, + appUpgradeHandler.currentVersion.toString(), + ), + ) + appUpgradeHandler.run() + } + appUpgradeHandler.copySubfont(false) + } + + /** + * Handle starting the app including auto sign in and navigating to the right starting page + */ + @VisibleForTesting + internal suspend fun handleAppStart() { + val signInAutomatically = preferences.data.first().signInAutomatically + val currentUser = serverRepository.current.value + if (signInAutomatically && currentUser != null && !currentUser.user.isProtected) { + // Hot reload + Timber.v("Hot reload restoring state") + setupNavigationManager.navigateTo( + SetupDestination.AppContent(currentUser), + ) + } else { + val restoredSession = + if (signInAutomatically) { + Timber.v("Sign in automatically") + serverRepository.restoreLastSession() + } else { + serverRepository.getMostRecentServer() + } + Timber.d("Restore last session result=%s", restoredSession) + setupNavigationManager.navigateTo(restoredSession.destination) + } + } } private val Destination?.isPlayback: Boolean diff --git a/app/src/main/java/com/github/damontecres/wholphin/WholphinDreamService.kt b/app/src/main/java/com/github/damontecres/wholphin/WholphinDreamService.kt index 57d4a447..3bc17f3c 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/WholphinDreamService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/WholphinDreamService.kt @@ -22,6 +22,7 @@ import androidx.savedstate.SavedStateRegistry import androidx.savedstate.SavedStateRegistryController import androidx.savedstate.SavedStateRegistryOwner import androidx.savedstate.setViewTreeSavedStateRegistryOwner +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.services.ScreensaverService @@ -33,9 +34,7 @@ import com.github.damontecres.wholphin.ui.theme.WholphinTheme import com.github.damontecres.wholphin.ui.util.ProvideLocalClock import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.first import okhttp3.OkHttpClient -import org.jellyfin.sdk.model.serializer.toUUIDOrNull import timber.log.Timber import javax.inject.Inject import kotlin.time.Duration.Companion.milliseconds @@ -75,8 +74,10 @@ class WholphinDreamService : lifecycleRegistry.currentState = Lifecycle.State.CREATED lifecycleScope.launchDefault { if (serverRepository.current.value == null) { - val prefs = preferencesDataStore.data.first() - serverRepository.restoreSession(prefs.currentServerId.toUUIDOrNull(), prefs.currentUserId.toUUIDOrNull()) + val result = serverRepository.restoreLastSession() + if (result !is RestoredSession.Success) { + Timber.w("Could not restore user session") + } } } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/MostRecentServer.kt b/app/src/main/java/com/github/damontecres/wholphin/data/MostRecentServer.kt new file mode 100644 index 00000000..b46a5f33 --- /dev/null +++ b/app/src/main/java/com/github/damontecres/wholphin/data/MostRecentServer.kt @@ -0,0 +1,181 @@ +package com.github.damontecres.wholphin.data + +import android.content.Context +import android.content.SharedPreferences +import androidx.core.content.edit +import com.github.damontecres.wholphin.data.MostRecentServer.None +import com.github.damontecres.wholphin.data.MostRecentServer.Server +import com.github.damontecres.wholphin.data.MostRecentServer.ServerAndUser +import com.github.damontecres.wholphin.data.model.JellyfinServer +import com.github.damontecres.wholphin.services.SetupDestination +import com.github.damontecres.wholphin.ui.isNotNullOrBlank +import com.github.damontecres.wholphin.ui.toServerString +import org.jellyfin.sdk.model.serializer.toUUIDOrNull +import java.util.UUID + +/** + * Represents the status of the most recently used server + * + * This can be read/written to shared preferences + */ +sealed interface MostRecentServer { + /** + * There is no recently used server + */ + data object None : MostRecentServer + + /** + * The most recently used server without a recent user + */ + data class Server( + val serverId: UUID, + val serverUrl: String, + ) : MostRecentServer + + /** + * Most recently used server and user + */ + data class ServerAndUser( + val serverId: UUID, + val serverUrl: String, + val userId: UUID, + val accessToken: String, + ) : MostRecentServer { + override fun toString(): String = "ServerAndUser(serverId=$serverId, serverUrl=$serverUrl, userId=$userId)" + } + + companion object { + fun from(current: CurrentUser?): MostRecentServer = + if (current != null && current.user.accessToken.isNotNullOrBlank()) { + ServerAndUser(current.server.id, current.server.url, current.user.id, current.user.accessToken) + } else if (current != null) { + Server(current.server.id, current.server.url) + } else { + None + } + } +} + +interface MostRecentServerProvider { + /** + * Get the [MostRecentServer] from the shared preferences in [context] + */ + fun get(): MostRecentServer + + /** + * Save the [CurrentUser] as the most recent server into the shared preferences in [context] + */ + suspend fun save(current: CurrentUser) + + /** + * Clear the most recent server and user + */ + suspend fun clear() + + /** + * CLear the most recent user, leaving the server + */ + suspend fun clearUser() +} + +/** + * A [MostRecentServerProvider] that stores in [SharedPreferences] + */ +class MostRecentServerSharedPreferences( + context: Context, +) : MostRecentServerProvider { + private val prefs = getServerSharedPreferences(context) + + /** + * Get the [MostRecentServer] from the shared preferences in [context] + */ + override fun get(): MostRecentServer { + val serverId = prefs.getString(SERVER_ID_KEY, null)?.toUUIDOrNull() + val serverUrl = prefs.getString(SERVER_URL_KEY, null) + val userId = prefs.getString(USER_ID_KEY, null)?.toUUIDOrNull() + val accessToken = prefs.getString(USER_TOKEN_KEY, null) + return if (serverId != null && serverUrl.isNotNullOrBlank() && userId != null && accessToken.isNotNullOrBlank()) { + ServerAndUser(serverId, serverUrl, userId, accessToken) + } else if (serverId != null && serverUrl.isNotNullOrBlank()) { + Server(serverId, serverUrl) + } else { + None + } + } + + /** + * Save the [CurrentUser] as the most recent server into the shared preferences in [context] + */ + override suspend fun save(current: CurrentUser) { + prefs.edit(true) { + putString(SERVER_ID_KEY, current.server.id.toServerString()) + putString(SERVER_URL_KEY, current.server.url) + putString(USER_ID_KEY, current.user.id.toServerString()) + putString(USER_TOKEN_KEY, current.user.accessToken) + } + } + + override suspend fun clear() { + prefs.edit(true) { + remove(SERVER_ID_KEY) + remove(SERVER_URL_KEY) + remove(USER_ID_KEY) + remove(USER_TOKEN_KEY) + } + } + + override suspend fun clearUser() { + prefs.edit(true) { + remove(USER_ID_KEY) + remove(USER_TOKEN_KEY) + } + } + + private fun getServerSharedPreferences(context: Context): SharedPreferences = + context.getSharedPreferences( + "${context.packageName}_server", + Context.MODE_PRIVATE, + ) + + companion object { + private const val SERVER_ID_KEY = "current.server.id" + private const val SERVER_URL_KEY = "current.server.url" + private const val USER_ID_KEY = "current.user.id" + private const val USER_TOKEN_KEY = "current.user.accessToken" + } +} + +/** + * The result of restoring a session + */ +sealed interface RestoredSession { + val destination: SetupDestination + + /** + * Session could not be restored + */ + data object None : RestoredSession { + override val destination: SetupDestination + get() = SetupDestination.ServerList + } + + /** + * Only the server should be restored, so a user still need to be chosen + */ + data class ServerOnly( + val server: JellyfinServer, + ) : RestoredSession { + override val destination: SetupDestination + get() = SetupDestination.UserList(server) + } + + /** + * Session was restored successfully + */ + data class Success( + val currentUser: CurrentUser, + ) : RestoredSession { + override val destination: SetupDestination + get() = SetupDestination.AppContent(currentUser) + } +} diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt b/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt index 5deba4c3..329b6786 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt @@ -1,15 +1,10 @@ package com.github.damontecres.wholphin.data import android.content.Context -import android.content.SharedPreferences -import androidx.core.content.edit -import androidx.datastore.core.DataStore import com.github.damontecres.wholphin.data.model.JellyfinServer import com.github.damontecres.wholphin.data.model.JellyfinUser import com.github.damontecres.wholphin.data.model.ServerUserConfig -import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.services.hilt.IoDispatcher -import com.github.damontecres.wholphin.ui.toServerString import com.github.damontecres.wholphin.util.WholphinDispatchers import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineDispatcher @@ -49,8 +44,8 @@ class ServerRepository val jellyfin: Jellyfin, val serverDao: JellyfinServerDao, val apiClient: ApiClient, - val userPreferencesDataStore: DataStore, @param:IoDispatcher private val ioDispatcher: CoroutineDispatcher, + private val mostRecentServerProvider: MostRecentServerProvider, ) { private var _current = MutableStateFlow(null) val current: StateFlow = _current @@ -88,10 +83,11 @@ class ServerRepository } apiClient.update(baseUrl = server.url, accessToken = null) _current.value = null + mostRecentServerProvider.clearUser() } /** - * Saves the server & User to the app database and updates the [ApiClient] to use this server & user + * Saves the server & user to the app database and updates the [ApiClient] to use this server & user */ suspend fun changeUser( server: JellyfinServer, @@ -119,70 +115,101 @@ class ServerRepository ) serverDao.addOrUpdateServer(updatedServer) updatedUser = serverDao.addOrUpdateUser(updatedUser) - userPreferencesDataStore.updateData { - it - .toBuilder() - .apply { - currentServerId = updatedServer.id.toServerString() - currentUserId = updatedUser.id.toServerString() - }.build() - } + val currentUser = CurrentUser(updatedServer, updatedUser) - withContext(WholphinDispatchers.Main) { - _current.value = currentUser - _currentUserDto.value = ServerUserConfig(userDto) - } - getServerSharedPreferences(context).edit(true) { - putString(SERVER_URL_KEY, updatedServer.url) - putString(ACCESS_TOKEN_KEY, updatedUser.accessToken) - } + mostRecentServerProvider.save(currentUser) + _current.value = currentUser + _currentUserDto.value = ServerUserConfig(userDto) return@withContext currentUser } /** - * Restores a session for the given server & user such as when the app reopens + * Try to restore the most recent session * - * If user has a PIN, this returns false + * Does not check if auto sign-in is enabled, but does check if the user profile is protected */ - suspend fun restoreSession( + suspend fun restoreLastSession(): RestoredSession { + val recentServer = mostRecentServerProvider.get() + Timber.d("restoreLastSession: recentServer=%s", recentServer) + val result = + when (recentServer) { + MostRecentServer.None -> { + RestoredSession.None + } + + is MostRecentServer.Server -> { + val server = serverDao.getServer(recentServer.serverId) + if (server != null) { + RestoredSession.ServerOnly(server.server) + } else { + RestoredSession.None + } + } + + is MostRecentServer.ServerAndUser -> { + val currentUser = tryChangeUser(recentServer.serverId, recentServer.userId) + if (currentUser != null) { + if (currentUser.user.isProtected) { + RestoredSession.ServerOnly(currentUser.server) + } else { + RestoredSession.Success(currentUser) + } + } else { + getMostRecentServerInternal(recentServer) + } + } + } + return result + } + + /** + * Get the most recently used server, if any + */ + suspend fun getMostRecentServer(): RestoredSession = getMostRecentServerInternal(mostRecentServerProvider.get()) + + suspend fun getMostRecentServerInternal(recentServer: MostRecentServer): RestoredSession { + val recentServer = recentServer ?: mostRecentServerProvider.get() + val serverId = + when (recentServer) { + MostRecentServer.None -> null + is MostRecentServer.Server -> recentServer.serverId + is MostRecentServer.ServerAndUser -> recentServer.serverId + } + val server = serverId?.let { serverDao.getServer(serverId)?.server } + return if (server != null) { + RestoredSession.ServerOnly(server) + } else { + RestoredSession.None + } + } + + /** + * Try to change to the given server & user. If the user's profile is protected, returns null + * + * @return the server/user or null if the session could not be restored + */ + suspend fun tryChangeUser( serverId: UUID?, userId: UUID?, - ): CurrentUser? { - if (serverId == null || userId == null) { - _current.value = null - return null - } - val serverAndUsers = - withContext(ioDispatcher) { - serverDao.getServer(serverId) - } - if (serverAndUsers != null) { - val current = _current.value - if (current != null && current.server.id == serverId && current.user.id == userId) { - Timber.v("Restoring session for current user, so shortcut") - apiClient.update( - baseUrl = current.server.url, - accessToken = current.user.accessToken, - ) - return current + ): CurrentUser? = + withContext(ioDispatcher) { + return@withContext if (serverId == null || userId == null) { + _current.value = null + null } else { - val user = serverAndUsers.users.firstOrNull { it.id == userId } - if (user != null) { - return changeUser(serverAndUsers.server, user) + val serverAndUsers = serverDao.getServer(serverId) + if (serverAndUsers != null) { + val user = serverAndUsers.users.firstOrNull { it.id == userId } + if (user != null && !user.isProtected) { + changeUser(serverAndUsers.server, user) + } else { + null + } + } else { + null } } } - return null - } - - suspend fun fetchLastUsedServer(serverId: UUID?): JellyfinServer? = - withContext(ioDispatcher) { - serverId?.let { serverDao.getServer(serverId)?.server } - } - - fun closeSession() { - _current.value = null - } /** * Given a successful [AuthenticationResult], switch to the user that just authenticated @@ -244,13 +271,7 @@ class ServerRepository withContext(WholphinDispatchers.Main) { _current.value = null } - userPreferencesDataStore.updateData { - it - .toBuilder() - .apply { - currentUserId = "" - }.build() - } + mostRecentServerProvider.clearUser() apiClient.update(accessToken = null) } withContext(ioDispatcher) { @@ -263,14 +284,7 @@ class ServerRepository withContext(WholphinDispatchers.Main) { _current.value = null } - userPreferencesDataStore.updateData { - it - .toBuilder() - .apply { - currentServerId = "" - currentUserId = "" - }.build() - } + mostRecentServerProvider.clear() apiClient.update(baseUrl = null, accessToken = null) } withContext(ioDispatcher) { @@ -279,14 +293,7 @@ class ServerRepository } suspend fun switchServerOrUser() { - userPreferencesDataStore.updateData { - it - .toBuilder() - .apply { - currentServerId = "" - currentUserId = "" - }.build() - } + mostRecentServerProvider.clear() } suspend fun updateUserAuth( @@ -335,17 +342,6 @@ class ServerRepository if (currentUser?.id == userDto.id) ServerUserConfig(userDto) else it } } - - companion object { - fun getServerSharedPreferences(context: Context): SharedPreferences = - context.getSharedPreferences( - "${context.packageName}_server", - Context.MODE_PRIVATE, - ) - - const val SERVER_URL_KEY = "current.server" - const val ACCESS_TOKEN_KEY = "current.accessToken" - } } @Serializable diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/AppUpgradeHandler.kt b/app/src/main/java/com/github/damontecres/wholphin/services/AppUpgradeHandler.kt index 4910110a..318ad9d1 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/AppUpgradeHandler.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/AppUpgradeHandler.kt @@ -6,7 +6,9 @@ import android.os.Build import androidx.core.content.edit import androidx.datastore.core.DataStore import androidx.preference.PreferenceManager +import com.github.damontecres.wholphin.data.CurrentUser import com.github.damontecres.wholphin.data.JellyfinServerDao +import com.github.damontecres.wholphin.data.MostRecentServerProvider import com.github.damontecres.wholphin.data.RememberedTabDao import com.github.damontecres.wholphin.data.SeerrServerDao import com.github.damontecres.wholphin.data.model.JellyfinUser @@ -36,6 +38,7 @@ import com.github.damontecres.wholphin.ui.setup.seerr.migrateSeerrUrl import com.github.damontecres.wholphin.util.Version import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.firstOrNull import org.jellyfin.sdk.model.api.BaseItemKind import org.jellyfin.sdk.model.serializer.toUUIDOrNull import timber.log.Timber @@ -56,6 +59,7 @@ class AppUpgradeHandler private val seerrServerDao: SeerrServerDao, private val serverDao: JellyfinServerDao, private val rememberedTabDao: RememberedTabDao, + private val mostRecentServerProvider: MostRecentServerProvider, ) { val pkgInfo: PackageInfo get() = context.packageManager.getPackageInfo(context.packageName, 0) val currentVersion: Version get() = Version.fromString(pkgInfo.versionName!!) @@ -442,5 +446,29 @@ class AppUpgradeHandler } } } + + if (current.isEqualOrBefore(Version.fromString("1.0.8-0-g0"))) { + try { + val prefs = appPreferences.data.firstOrNull() + val serverId = prefs?.currentServerId?.toUUIDOrNull() + val userId = prefs?.currentUserId?.toUUIDOrNull() + if (serverId != null && userId != null) { + val server = serverDao.getServer(serverId) + val user = server?.users?.firstOrNull { it.id == userId } + if (server != null && user != null) { + Timber.i("Migrating most recent server") + mostRecentServerProvider.save(CurrentUser(server.server, user)) + } + } + } catch (ex: Exception) { + Timber.e(ex, "Error during migration for most recent server") + } + appPreferences.updateData { + it.update { + currentServerId = "" + currentUserId = "" + } + } + } } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt index c070b37e..b5dd3b7f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt @@ -2,6 +2,7 @@ package com.github.damontecres.wholphin.services import android.content.Context import androidx.annotation.StringRes +import androidx.annotation.VisibleForTesting import com.github.damontecres.wholphin.R import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.data.model.BaseItem @@ -188,9 +189,8 @@ class HomeSettingsService /** * Decodes [HomePageSettings] from a [JsonElement] skipping any unknown/unparsable rows - * - * This is public only for testing */ + @VisibleForTesting fun decode(element: JsonElement): HomePageSettings { val version = element.jsonObject["version"]?.jsonPrimitive?.intOrNull if (version == null || version > SUPPORTED_HOME_PAGE_SETTINGS_VERSION) { diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/IntentService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/IntentService.kt index 2dd0cde5..4fda8260 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/IntentService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/IntentService.kt @@ -2,6 +2,8 @@ package com.github.damontecres.wholphin.services import android.app.SearchManager import android.content.Intent +import androidx.annotation.VisibleForTesting +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.data.model.BaseItem import com.github.damontecres.wholphin.ui.detail.series.SeasonEpisodeIds @@ -114,6 +116,7 @@ class IntentService return IntentResult.Target(destinations, true) } + @VisibleForTesting internal suspend fun prepare(intent: Intent): IntentResult? { val appPrefs = userPreferencesService.flow.first().appPreferences @@ -121,39 +124,33 @@ class IntentService val serverId = intent.getStringParam(INTENT_SERVER_ID)?.toUUIDOrNull() return if (userId != null && serverId != null) { Timber.v("Intent switches user") - val user = serverRepository.serverDao.getUser(serverId, userId) - if (user != null && !user.isProtected) { - serverRepository.restoreSession(serverId, userId) - ?: IntentResult.Error("Error restoring user") + val user = serverRepository.tryChangeUser(serverId, userId) + if (user != null) { null } else { - IntentResult.Error("Cannot switch to specified user") + IntentResult.Error("Error restoring user") } - } else { - val profileProtected = - serverRepository.current.value - ?.user - ?.isProtected == true - if (appPrefs.signInAutomatically && !profileProtected) { - Timber.v("No current user, so restoring last") - val userId = appPrefs.currentUserId?.toUUIDOrNull() - val serverId = appPrefs.currentServerId?.toUUIDOrNull() + } else if (appPrefs.signInAutomatically) { + Timber.v("No current user, so restoring last") + val result = serverRepository.restoreLastSession() + Timber.v("Restored result=%s", result) + when (result) { + RestoredSession.None, + is RestoredSession.ServerOnly, + -> { + IntentResult.Error("Could not auto sign-in, specify a server & user") + } - if (userId != null && serverId != null) { - val current = - serverRepository.restoreSession(serverId, userId) - ?: return IntentResult.Error("Error restoring user") - if (current.user.isProtected) { + is RestoredSession.Success -> { + if (result.currentUser.user.isProtected) { IntentResult.Error("Could not auto sign-in, user is protected") } else { null } - } else { - IntentResult.Error("Could not auto sign-in, specify a server & user") } - } else { - IntentResult.Error("Auto sign-in not enabled or user is protected") } + } else { + IntentResult.Error("Auto sign-in not enabled") } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/LatestNextUpWorker.kt b/app/src/main/java/com/github/damontecres/wholphin/services/LatestNextUpWorker.kt index 01ed9dd2..a60bf7c4 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/LatestNextUpWorker.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/LatestNextUpWorker.kt @@ -1,6 +1,7 @@ package com.github.damontecres.wholphin.services import android.content.Context +import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AppCompatActivity import androidx.hilt.work.HiltWorker import androidx.lifecycle.lifecycleScope @@ -13,6 +14,7 @@ import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkManager import androidx.work.WorkerParameters import androidx.work.workDataOf +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.ui.collectLatestIn import com.github.damontecres.wholphin.util.ExceptionHandler @@ -52,12 +54,8 @@ class LatestNextUpWorker try { if (api.baseUrl.isNullOrBlank() || api.accessToken.isNullOrBlank()) { // Not active - var currentUser = serverRepository.current.value - if (currentUser == null) { - serverRepository.restoreSession(serverId, userId) - currentUser = serverRepository.current.value - } - if (currentUser == null) { + val result = serverRepository.restoreLastSession() + if (result !is RestoredSession.Success) { Timber.w("No user found during run") return Result.failure() } @@ -91,7 +89,7 @@ class LatestNextUpSchedulerService "SuggestionsSchedulerService requires an AppCompatActivity context, but received: ${context::class.java.name}", ) - // Exposed for testing + @VisibleForTesting internal var dispatcher: CoroutineDispatcher = WholphinDispatchers.IO init { diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt index aa64058d..e0dd472f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/SuggestionsWorker.kt @@ -5,6 +5,7 @@ import androidx.datastore.core.DataStore import androidx.hilt.work.HiltWorker import androidx.work.CoroutineWorker import androidx.work.WorkerParameters +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.preferences.AppPreference import com.github.damontecres.wholphin.preferences.AppPreferences @@ -63,12 +64,8 @@ class SuggestionsWorker } if (api.baseUrl.isNullOrBlank() || api.accessToken.isNullOrBlank()) { - var currentUser = serverRepository.current.value - if (currentUser == null) { - serverRepository.restoreSession(serverId, userId) - currentUser = serverRepository.current.value - } - if (currentUser == null) { + val result = serverRepository.restoreLastSession() + if (result !is RestoredSession.Success) { Timber.w("No user found during run") return Result.failure() } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt b/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt index 06607cf9..3a114017 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt @@ -4,6 +4,8 @@ import android.content.Context import androidx.work.WorkManager import com.github.damontecres.wholphin.BuildConfig import com.github.damontecres.wholphin.R +import com.github.damontecres.wholphin.data.MostRecentServerProvider +import com.github.damontecres.wholphin.data.MostRecentServerSharedPreferences import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.services.SeerrApi import com.github.damontecres.wholphin.util.CoroutineContextApiClientFactory @@ -188,6 +190,12 @@ object AppModule { fun seerrApi( @StandardOkHttpClient okHttpClient: OkHttpClient, ) = SeerrApi(okHttpClient) + + @Provides + @Singleton + fun mostRecentServerProvider( + @ApplicationContext context: Context, + ): MostRecentServerProvider = MostRecentServerSharedPreferences(context) } @Module diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/tvprovider/TvProviderWorker.kt b/app/src/main/java/com/github/damontecres/wholphin/services/tvprovider/TvProviderWorker.kt index d3dbf4c7..a4d73caa 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/tvprovider/TvProviderWorker.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/tvprovider/TvProviderWorker.kt @@ -19,6 +19,7 @@ import androidx.work.CoroutineWorker import androidx.work.WorkerParameters import com.github.damontecres.wholphin.MainActivity import com.github.damontecres.wholphin.R +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.data.model.BaseItem import com.github.damontecres.wholphin.preferences.AppPreference @@ -70,12 +71,8 @@ class TvProviderWorker if (api.baseUrl.isNullOrBlank() || api.accessToken.isNullOrBlank()) { // Not active - var currentUser = serverRepository.current.value - if (currentUser == null) { - serverRepository.restoreSession(serverId, userId) - currentUser = serverRepository.current.value - } - if (currentUser == null) { + val result = serverRepository.restoreLastSession() + if (result !is RestoredSession.Success) { Timber.w("No user found during run") return Result.failure() } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt index 012038a8..6ee345f5 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt @@ -5,6 +5,7 @@ import android.media.MediaCodecList import android.os.Build import android.widget.Toast import androidx.annotation.OptIn +import androidx.annotation.VisibleForTesting import androidx.compose.ui.unit.Density import androidx.core.net.toUri import androidx.lifecycle.ViewModel @@ -198,7 +199,7 @@ class PlaybackViewModel val currentUserDto = serverRepository.currentUserDtoFlow - // Exposed for testing + @VisibleForTesting val initJob: Job init { @@ -1110,7 +1111,7 @@ class PlaybackViewModel } // Variables for tracking segment state - // Exposed for testing + @VisibleForTesting internal var segmentJob: Job? = null private val autoSkippedSegments = mutableSetOf() private val outroShownSegments = mutableSetOf() diff --git a/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt b/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt index c334967f..d83ae269 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt @@ -1,7 +1,7 @@ package com.github.damontecres.wholphin.util import android.content.Context -import com.github.damontecres.wholphin.data.ServerRepository +import com.github.damontecres.wholphin.data.MostRecentServer import com.github.damontecres.wholphin.services.hilt.AppModule import com.github.damontecres.wholphin.services.hilt.DeviceModule import com.google.auto.service.AutoService @@ -13,6 +13,7 @@ import org.acra.sender.ReportSender import org.acra.sender.ReportSenderException import org.acra.sender.ReportSenderFactory import org.jellyfin.sdk.Jellyfin +import org.jellyfin.sdk.api.client.ApiClient import org.jellyfin.sdk.api.client.extensions.clientLogApi import org.jellyfin.sdk.api.okhttp.OkHttpFactory import org.jellyfin.sdk.createJellyfin @@ -39,26 +40,10 @@ class CrashReportSender : ReportSender { errorContent: CrashReportData, ) { Timber.v("Attempting to send crash report") - val prefs = ServerRepository.getServerSharedPreferences(context) - val serverUrl = prefs.getString(ServerRepository.SERVER_URL_KEY, null) - val accessToken = prefs.getString(ServerRepository.ACCESS_TOKEN_KEY, null) - if (serverUrl != null && accessToken != null) { - try { - val okHttpClient = - OkHttpClient - .Builder() - .build() - val okHttpFactory = OkHttpFactory(okHttpClient) - val api = - createJellyfin { - this.context = context - clientInfo = AppModule.clientInfo(context) - deviceInfo = DeviceModule.deviceInfo(context) - apiClientFactory = okHttpFactory - socketConnectionFactory = okHttpFactory - minimumServerVersion = Jellyfin.minimumVersion - }.createApi(baseUrl = serverUrl, accessToken = accessToken) - + try { + val recentServer = AppModule.mostRecentServerProvider(context).get() + val api = recentServer.createApi(context) + if (api != null) { val obj = JSONObject() for ((key, value) in errorContent.toMap()) { obj.put(key, value) @@ -75,13 +60,32 @@ class CrashReportSender : ReportSender { """.trimIndent() + jsonStr, ).content.fileName - Timber.i("Sent report to $serverUrl, filename=$filename") + Timber.i("Sent report to %s, filename=%s", api.baseUrl, filename) } - } catch (ex: Exception) { - throw ReportSenderException("Exception while sending crash report", ex) + } else { + throw ReportSenderException("Could not find valid server and/or credentials to use") } - } else { - throw ReportSenderException("Could not find valid server and/or credentials to use") + } catch (ex: Exception) { + throw ReportSenderException("Exception while sending crash report", ex) } } } + +private fun MostRecentServer.createApi(context: Context): ApiClient? = + (this as? MostRecentServer.ServerAndUser)?.let { + val okHttpClient = + OkHttpClient + .Builder() + .build() + val okHttpFactory = OkHttpFactory(okHttpClient) + val api = + createJellyfin { + this.context = context + clientInfo = AppModule.clientInfo(context) + deviceInfo = DeviceModule.deviceInfo(context) + apiClientFactory = okHttpFactory + socketConnectionFactory = okHttpFactory + minimumServerVersion = Jellyfin.minimumVersion + }.createApi(baseUrl = serverUrl, accessToken = accessToken) + api + } diff --git a/app/src/main/proto/WholphinDataStore.proto b/app/src/main/proto/WholphinDataStore.proto index 38f6099c..7de4ba67 100644 --- a/app/src/main/proto/WholphinDataStore.proto +++ b/app/src/main/proto/WholphinDataStore.proto @@ -233,9 +233,8 @@ message ExperimentalPreferences { } message AppPreferences { - // The currently signed in server and user IDs, mostly for restoring a session - string current_server_id = 1; - string current_user_id = 2; + string current_server_id = 1 [deprecated = true]; + string current_user_id = 2 [deprecated = true]; PlaybackPreferences playback_preferences = 3; HomePagePreferences home_page_preferences = 4; diff --git a/app/src/test/java/com/github/damontecres/wholphin/test/TestMainActivityViewModel.kt b/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt similarity index 61% rename from app/src/test/java/com/github/damontecres/wholphin/test/TestMainActivityViewModel.kt rename to app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt index 556a2fc0..1de5becd 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/test/TestMainActivityViewModel.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt @@ -1,14 +1,12 @@ -package com.github.damontecres.wholphin.test +package com.github.damontecres.wholphin import android.content.Context import android.content.Intent import android.widget.Toast import androidx.datastore.core.DataStore -import com.github.damontecres.wholphin.MainActivityViewModel import com.github.damontecres.wholphin.data.CurrentUser -import com.github.damontecres.wholphin.data.JellyfinServerDao +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository -import com.github.damontecres.wholphin.data.model.JellyfinServerUsers import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.preferences.update import com.github.damontecres.wholphin.services.AppUpgradeHandler @@ -19,8 +17,10 @@ import com.github.damontecres.wholphin.services.IntentService import com.github.damontecres.wholphin.services.NavigationManager import com.github.damontecres.wholphin.services.SetupDestination import com.github.damontecres.wholphin.services.SetupNavigationManager +import com.github.damontecres.wholphin.test.currentUser +import com.github.damontecres.wholphin.test.server +import com.github.damontecres.wholphin.test.user import com.github.damontecres.wholphin.ui.nav.Destination -import com.github.damontecres.wholphin.ui.toServerString import com.github.damontecres.wholphin.util.WholphinDispatchers import com.github.damontecres.wholphin.util.configure import com.github.damontecres.wholphin.util.reset @@ -36,11 +36,11 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.test.StandardTestDispatcher -import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.jellyfin.sdk.model.UUID import org.junit.After -import org.junit.Assert.assertEquals +import org.junit.Assert +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -58,7 +58,6 @@ class TestMainActivityViewModel { private val backdropService: BackdropService = mockk(relaxed = true) private val appUpgradeHandler: AppUpgradeHandler = mockk() private val intentService: IntentService = mockk() - private val serverDao: JellyfinServerDao = mockk() val viewModel = MainActivityViewModel( @@ -74,6 +73,7 @@ class TestMainActivityViewModel { ) private val serverId = UUID.randomUUID() + private val server = server(serverId) private val userId = UUID.randomUUID() private val currentUser = currentUser(serverId, userId) @@ -88,13 +88,6 @@ class TestMainActivityViewModel { WholphinDispatchers.configure(testDispatcher) every { appUpgradeHandler.needUpgrade() } returns false every { appUpgradeHandler.copySubfont(any()) } returns Unit - every { serverRepository.serverDao } returns serverDao - coEvery { serverDao.getServer(serverId) } returns - JellyfinServerUsers( - currentUser.server, - // User list is unused - emptyList(), - ) mockkStatic(Toast::class) val mockToast = mockk() every { Toast.makeText(any(), any(), any()) } returns mockToast @@ -121,22 +114,26 @@ class TestMainActivityViewModel { runTest(testDispatcher) { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser +// coEvery { serverRepository.tryRestoreSession(serverId, userId) } returns currentUser +// coEvery { serverRepository.restoreLastSession() } returns +// RestoredSession.Success(currentUser) - viewModel.appStart(null) - advanceUntilIdle() + viewModel.handleAppStart() val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.AppContent) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.AppContent, + ) destination as SetupDestination.AppContent - assertEquals(currentUser, destination.current) + Assert.assertEquals(currentUser, destination.current) } + coVerify(exactly = 0) { serverRepository.restoreLastSession() } + coVerify(exactly = 0) { serverRepository.getMostRecentServer() } } @Test @@ -144,27 +141,27 @@ class TestMainActivityViewModel { runTest { setupPreferences { signInAutomatically = false - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) // coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser + coEvery { serverRepository.getMostRecentServer() } returns + RestoredSession.ServerOnly(server) - viewModel.appStart(null) - advanceUntilIdle() + viewModel.handleAppStart() - verify(exactly = 1) { serverDao.getServer(serverId) } - coVerify(exactly = 0) { serverRepository.restoreSession(any(), any()) } + coVerify(exactly = 0) { serverRepository.tryChangeUser(any(), any()) } + coVerify(exactly = 0) { serverRepository.restoreLastSession() } + coVerify(exactly = 1) { serverRepository.getMostRecentServer() } val args = mutableListOf() verify { setupNavigationManager.navigateTo(capture(args)) } - assertEquals(2, args.size) + Assert.assertEquals(1, args.size) args[0].let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.Loading) - } - args[1].let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.UserList) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.UserList, + ) destination as SetupDestination.UserList - assertEquals(currentUser.server, destination.server) + Assert.assertEquals(currentUser.server, destination.server) } } @@ -173,27 +170,30 @@ class TestMainActivityViewModel { runTest { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } - every { serverRepository.current } returns MutableStateFlow(protectedCurrentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns protectedCurrentUser + every { serverRepository.current } returns + MutableStateFlow( + protectedCurrentUser, + ) + coEvery { + serverRepository.tryChangeUser(serverId, userId) + } returns null + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.ServerOnly(server) - viewModel.appStart(null) - advanceUntilIdle() + viewModel.handleAppStart() - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } - verify { serverDao.getServer(serverId) } + coVerify(exactly = 0) { serverRepository.tryChangeUser(serverId, userId) } val args = mutableListOf() verify { setupNavigationManager.navigateTo(capture(args)) } - assertEquals(2, args.size) + Assert.assertEquals(1, args.size) args[0].let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.Loading) - } - args[1].let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.UserList) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.UserList, + ) destination as SetupDestination.UserList - assertEquals(currentUser.server, destination.server) + Assert.assertEquals(currentUser.server, destination.server) } } @@ -202,22 +202,26 @@ class TestMainActivityViewModel { runTest { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreSession(serverId, userId) } returns protectedCurrentUser + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.ServerOnly(server) + coEvery { + serverRepository.tryChangeUser(serverId, userId) + } returns null - viewModel.appStart(null) - advanceUntilIdle() + viewModel.handleAppStart() - coVerify(exactly = 1) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 1) { serverRepository.restoreLastSession() } val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.UserList) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.UserList, + ) destination as SetupDestination.UserList - assertEquals(currentUser.server, destination.server) + Assert.assertEquals(currentUser.server, destination.server) } } @@ -226,45 +230,49 @@ class TestMainActivityViewModel { runTest { setupPreferences { signInAutomatically = true - currentServerId = "" - currentUserId = "" } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreSession(null, null) } returns null + coEvery { serverRepository.restoreLastSession() } returns RestoredSession.None - viewModel.appStart(null) - advanceUntilIdle() + viewModel.handleAppStart() - coVerify(exactly = 1) { serverRepository.restoreSession(null, null) } + coVerify(exactly = 1) { serverRepository.restoreLastSession() } val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.ServerList) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.ServerList, + ) } } @Test - fun `Test intent result is noop`() = + fun `Test intent result is noop, hot reload`() = runTest(testDispatcher) { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.Success(currentUser) coEvery { intentService.parseIntent(any()) } returns IntentResult.NoOp - viewModel.appStart(Intent()) - advanceUntilIdle() + val result = viewModel.handleIntent(Intent()) + assertFalse(result) - val slot = slot() - verify { setupNavigationManager.navigateTo(capture(slot)) } - slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.AppContent) - destination as SetupDestination.AppContent - assertEquals(currentUser, destination.current) - } +// val slot = slot() +// verify { setupNavigationManager.navigateTo(capture(slot)) } +// slot.captured.let { destination -> +// Assert.assertTrue( +// "destination is ${destination::class}", +// destination is SetupDestination.AppContent, +// ) +// destination as SetupDestination.AppContent +// Assert.assertEquals(currentUser, destination.current) +// } + coVerify(exactly = 0) { serverRepository.restoreLastSession() } + coVerify(exactly = 0) { serverRepository.getMostRecentServer() } } @Test @@ -272,22 +280,22 @@ class TestMainActivityViewModel { runTest(testDispatcher) { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser coEvery { intentService.parseIntent(any()) } returns IntentResult.Error("Error") - viewModel.appStart(Intent()) - advanceUntilIdle() + val result = viewModel.handleIntent(Intent()) + assertTrue(result) val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.UserList) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.UserList, + ) destination as SetupDestination.UserList - assertEquals(currentUser.server, destination.server) + Assert.assertEquals(currentUser.server, destination.server) } } @@ -296,20 +304,20 @@ class TestMainActivityViewModel { runTest(testDispatcher) { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser coEvery { intentService.parseIntent(any()) } returns IntentResult.Error("Error") - viewModel.appStart(Intent()) - advanceUntilIdle() + val result = viewModel.handleIntent(Intent()) + assertTrue(result) val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.ServerList) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.ServerList, + ) } } @@ -318,11 +326,9 @@ class TestMainActivityViewModel { runTest(testDispatcher) { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser +// coEvery { serverRepository.tryRestoreSession(serverId, userId) } returns currentUser coEvery { intentService.parseIntent(any()) } returns IntentResult.Target( destinations = listOf(Destination.Favorites), @@ -331,21 +337,27 @@ class TestMainActivityViewModel { val backstack = mutableListOf() every { navigationManager.backStack } returns backstack - viewModel.appStart(Intent()) - advanceUntilIdle() + val result = viewModel.handleIntent(Intent()) + assertTrue(result) val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } verify { navigationManager.reloadHome() } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.AppContent) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.AppContent, + ) destination as SetupDestination.AppContent - assertEquals(currentUser, destination.current) + Assert.assertEquals(currentUser, destination.current) } // Note: navigation manager internally handles Home destination, so it won't be in this list - assertEquals(1, backstack.size) + Assert.assertEquals(1, backstack.size) backstack[0].let { destination -> - assertTrue("destination is ${destination::class}", destination is Destination.Favorites) + Assert.assertTrue( + "destination is ${destination::class}", + destination is Destination.Favorites, + ) } } @@ -354,11 +366,9 @@ class TestMainActivityViewModel { runTest(testDispatcher) { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser +// coEvery { serverRepository.tryRestoreSession(serverId, userId) } returns currentUser coEvery { intentService.parseIntent(any()) } returns IntentResult.Target( destinations = listOf(Destination.Favorites), @@ -367,20 +377,26 @@ class TestMainActivityViewModel { val backstack = mutableListOf() every { navigationManager.backStack } returns backstack - viewModel.appStart(Intent()) - advanceUntilIdle() + val result = viewModel.handleIntent(Intent()) + assertTrue(result) val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } verify(exactly = 0) { navigationManager.reloadHome() } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.AppContent) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.AppContent, + ) destination as SetupDestination.AppContent - assertEquals(currentUser, destination.current) + Assert.assertEquals(currentUser, destination.current) } - assertEquals(1, backstack.size) + Assert.assertEquals(1, backstack.size) backstack[0].let { destination -> - assertTrue("destination is ${destination::class}", destination is Destination.Favorites) + Assert.assertTrue( + "destination is ${destination::class}", + destination is Destination.Favorites, + ) } } @@ -389,11 +405,9 @@ class TestMainActivityViewModel { runTest(testDispatcher) { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser +// coEvery { serverRepository.tryRestoreSession(serverId, userId) } returns currentUser coEvery { intentService.parseIntent(any()) } returns IntentResult.Target( destinations = listOf(Destination.Favorites), @@ -402,14 +416,17 @@ class TestMainActivityViewModel { val backstack = mutableListOf() every { navigationManager.backStack } returns backstack - viewModel.appStart(Intent()) - advanceUntilIdle() + val result = viewModel.handleIntent(Intent()) + assertTrue(result) val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } verify(exactly = 0) { navigationManager.reloadHome() } slot.captured.let { destination -> - assertTrue("destination is ${destination::class}", destination is SetupDestination.ServerList) + Assert.assertTrue( + "destination is ${destination::class}", + destination is SetupDestination.ServerList, + ) } } } diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/IntentServiceTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/IntentServiceTest.kt index ee32c7dd..2bcc6ab7 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/services/IntentServiceTest.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/services/IntentServiceTest.kt @@ -3,7 +3,7 @@ package com.github.damontecres.wholphin.services import android.app.SearchManager import android.content.Intent import com.github.damontecres.wholphin.data.CurrentUser -import com.github.damontecres.wholphin.data.JellyfinServerDao +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.preferences.UserPreferences @@ -20,7 +20,6 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk -import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.test.runTest @@ -42,7 +41,6 @@ import org.robolectric.annotation.Config class IntentServiceTest { private val api: ApiClient = mockk() private val serverRepository: ServerRepository = mockk() - private val serverDao: JellyfinServerDao = mockk() private val userPreferencesService: UserPreferencesService = mockk() private val userLibraryApi: UserLibraryApi = mockk() @@ -64,7 +62,6 @@ class IntentServiceTest { intentService = IntentService(api, serverRepository, userPreferencesService) every { api.userLibraryApi } returns userLibraryApi coEvery { userLibraryApi.getItem(itemId) } returns successResponse(movie(itemId)) - every { serverRepository.serverDao } returns serverDao } private fun setupPreferences(block: AppPreferences.Builder.() -> Unit) { @@ -82,11 +79,11 @@ class IntentServiceTest { private fun setupAutoSignInUnprotected() { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.Success(currentUser) + coEvery { serverRepository.tryChangeUser(serverId, userId) } returns currentUser } @Test @@ -98,7 +95,7 @@ class IntentServiceTest { val result = intentService.prepare(intent) assertNull(result) - coVerify { serverRepository.restoreSession(serverId, userId) } + coVerify { serverRepository.restoreLastSession() } } @Test @@ -106,17 +103,17 @@ class IntentServiceTest { runTest { setupPreferences { signInAutomatically = false - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.Success(currentUser) + coEvery { serverRepository.tryChangeUser(serverId, userId) } returns currentUser val intent = Intent() val result = intentService.prepare(intent) assertTrue(result is IntentResult.Error) - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 0) { serverRepository.tryChangeUser(serverId, userId) } } @Test @@ -124,17 +121,17 @@ class IntentServiceTest { runTest { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(protectedCurrentUser) - coEvery { serverRepository.restoreSession(serverId, userId) } returns protectedCurrentUser + coEvery { serverRepository.tryChangeUser(serverId, userId) } returns null + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.ServerOnly(protectedCurrentUser.server) val intent = Intent() val result = intentService.prepare(intent) assertTrue(result is IntentResult.Error) - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 0) { serverRepository.tryChangeUser(serverId, userId) } } @Test @@ -142,17 +139,17 @@ class IntentServiceTest { runTest { setupPreferences { signInAutomatically = true - currentServerId = serverId.toServerString() - currentUserId = userId.toServerString() } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreSession(serverId, userId) } returns protectedCurrentUser + coEvery { serverRepository.tryChangeUser(serverId, userId) } returns null + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.ServerOnly(protectedCurrentUser.server) val intent = Intent() val result = intentService.prepare(intent) assertTrue(result is IntentResult.Error) - coVerify(exactly = 1) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 1) { serverRepository.restoreLastSession() } } @Test @@ -160,17 +157,16 @@ class IntentServiceTest { runTest { setupPreferences { signInAutomatically = true - currentServerId = "" - currentUserId = "" } every { serverRepository.current } returns MutableStateFlow(null) + coEvery { serverRepository.restoreLastSession() } returns RestoredSession.None // coEvery { serverRepository.restoreSession(serverId, userId) } returns protectedCurrentUser val intent = Intent() val result = intentService.prepare(intent) assertTrue(result is IntentResult.Error) - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 0) { serverRepository.tryChangeUser(serverId, userId) } } @Test @@ -178,12 +174,11 @@ class IntentServiceTest { runTest { setupPreferences { signInAutomatically = true - currentServerId = "" - currentUserId = "" } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.serverDao.getUser(serverId, userId) } returns currentUser.user - coEvery { serverRepository.restoreSession(serverId, userId) } returns currentUser + coEvery { serverRepository.tryChangeUser(serverId, userId) } returns currentUser + coEvery { serverRepository.restoreLastSession() } returns + RestoredSession.Success(currentUser) val intent = Intent().apply { @@ -193,7 +188,7 @@ class IntentServiceTest { val result = intentService.prepare(intent) assertNull(result) - coVerify(exactly = 1) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 1) { serverRepository.tryChangeUser(serverId, userId) } } @Test @@ -201,12 +196,11 @@ class IntentServiceTest { runTest { setupPreferences { signInAutomatically = true - currentServerId = "" - currentUserId = "" } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.serverDao.getUser(serverId, userId) } returns protectedCurrentUser.user - coEvery { serverRepository.restoreSession(serverId, userId) } returns protectedCurrentUser + coEvery { serverRepository.tryChangeUser(serverId, userId) } returns null +// coEvery { serverRepository.restoreLastSession() } returns +// RestoredSession.ServerOnly(protectedCurrentUser.server) val intent = Intent().apply { @@ -216,7 +210,7 @@ class IntentServiceTest { val result = intentService.prepare(intent) assertTrue(result is IntentResult.Error) - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 1) { serverRepository.tryChangeUser(serverId, userId) } } @Test @@ -224,12 +218,9 @@ class IntentServiceTest { runTest { setupPreferences { signInAutomatically = true - currentServerId = "" - currentUserId = "" } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.serverDao.getUser(serverId, userId) } returns null - coEvery { serverRepository.restoreSession(serverId, userId) } returns null + coEvery { serverRepository.tryChangeUser(serverId, userId) } returns null val intent = Intent().apply { @@ -239,7 +230,7 @@ class IntentServiceTest { val result = intentService.prepare(intent) assertTrue(result is IntentResult.Error) - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } + coVerify(exactly = 1) { serverRepository.tryChangeUser(serverId, userId) } } @Test @@ -296,12 +287,11 @@ class IntentServiceTest { every { serverRepository.current } returns MutableStateFlow(currentUser) coEvery { - serverRepository.restoreSession( + serverRepository.tryChangeUser( newServerId, newUserId, ) } returns newCurrentUser - every { serverDao.getUser(newServerId, newUserId) } returns newCurrentUser.user val intent = Intent().apply { @@ -316,9 +306,8 @@ class IntentServiceTest { assertEquals(0, destinations.size) assertTrue(result.addHomeToBackStack) - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } - verify { serverDao.getUser(newServerId, newUserId) } - coVerify { serverRepository.restoreSession(newServerId, newUserId) } + coVerify(exactly = 0) { serverRepository.tryChangeUser(serverId, userId) } + coVerify { serverRepository.tryChangeUser(newServerId, newUserId) } coVerify(exactly = 0) { userLibraryApi.getItem(itemId) } } @@ -333,12 +322,11 @@ class IntentServiceTest { every { serverRepository.current } returns MutableStateFlow(currentUser) coEvery { - serverRepository.restoreSession( + serverRepository.tryChangeUser( newServerId, newUserId, ) } returns newCurrentUser - every { serverDao.getUser(newServerId, newUserId) } returns newCurrentUser.user val intent = Intent().apply { @@ -358,9 +346,8 @@ class IntentServiceTest { assertEquals(itemId, first.itemId) - coVerify(exactly = 0) { serverRepository.restoreSession(serverId, userId) } - verify { serverDao.getUser(newServerId, newUserId) } - coVerify { serverRepository.restoreSession(newServerId, newUserId) } + coVerify(exactly = 0) { serverRepository.tryChangeUser(serverId, userId) } + coVerify { serverRepository.tryChangeUser(newServerId, newUserId) } coVerify { userLibraryApi.getItem(itemId) } } diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/ServerRepositoryTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/ServerRepositoryTest.kt index 499208d6..2c2fdffe 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/services/ServerRepositoryTest.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/services/ServerRepositoryTest.kt @@ -1,11 +1,13 @@ package com.github.damontecres.wholphin.services import android.content.Context -import android.content.SharedPreferences import androidx.datastore.core.DataStoreFactory import androidx.datastore.core.handlers.ReplaceFileCorruptionHandler import com.github.damontecres.wholphin.data.CurrentUser import com.github.damontecres.wholphin.data.JellyfinServerDao +import com.github.damontecres.wholphin.data.MostRecentServer +import com.github.damontecres.wholphin.data.MostRecentServerProvider +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.data.model.JellyfinServer import com.github.damontecres.wholphin.data.model.JellyfinServerUsers @@ -14,6 +16,7 @@ import com.github.damontecres.wholphin.data.model.ServerUserConfig import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.preferences.AppPreferencesSerializer import com.github.damontecres.wholphin.test.nonBlankString +import com.github.damontecres.wholphin.ui.isNotNullOrBlank import com.github.damontecres.wholphin.ui.successResponse import com.github.damontecres.wholphin.ui.toServerString import com.github.damontecres.wholphin.util.WholphinDispatchers @@ -21,6 +24,7 @@ import com.github.damontecres.wholphin.util.configure import com.github.damontecres.wholphin.util.reset import io.mockk.Runs import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.just import io.mockk.mockk @@ -29,7 +33,6 @@ import io.mockk.verify import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.update import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.runTest @@ -48,6 +51,8 @@ import org.jellyfin.sdk.model.api.PublicSystemInfo import org.jellyfin.sdk.model.api.UserDto import org.junit.After import org.junit.Assert +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test @@ -80,14 +85,13 @@ class ServerRepositoryTest { private val mockUserApi = mockk() private val mockSystemApi = mockk() - private val mockSharedPreferences = mockk(relaxed = true) + private val mockMostRecentServerProvider = mockk() @OptIn(ExperimentalCoroutinesApi::class) @Before fun setup() { WholphinDispatchers.configure(testDispatcher) - every { mockContext.getSharedPreferences(any(), any()) } returns mockSharedPreferences every { mockApiClient.userApi } returns mockUserApi every { mockApiClient.systemApi } returns mockSystemApi coEvery { mockUserApi.getCurrentUser() } returns successResponse(userDto) @@ -102,9 +106,15 @@ class ServerRepositoryTest { emptyMap(), ) coEvery { mockJellyfinServerDao.addOrUpdateUser(user) } returns user + coEvery { mockJellyfinServerDao.getServer(serverId) } returns + JellyfinServerUsers(server, listOf(user)) + coEvery { mockJellyfinServerDao.addOrUpdateServer(server) } just Runs every { mockApiClient.clientInfo } returns ClientInfo("Wholphin test", "0.0.1") every { mockApiClient.deviceInfo } returns DeviceInfo("Wholphin test ID", "Wholphin test device") every { mockApiClient.update(any(), any(), any(), any()) } just Runs + coEvery { mockMostRecentServerProvider.save(any()) } just Runs + coEvery { mockMostRecentServerProvider.clear() } just Runs + coEvery { mockMostRecentServerProvider.clearUser() } just Runs } @OptIn(ExperimentalCoroutinesApi::class) @@ -119,8 +129,8 @@ class ServerRepositoryTest { mockJellyfin, mockJellyfinServerDao, mockApiClient, - dataStore, testDispatcher, + mockMostRecentServerProvider, ) private val serverId = UUID.randomUUID() @@ -194,24 +204,24 @@ class ServerRepositoryTest { Assert.assertEquals(user, serverRepository.currentUser) Assert.assertEquals(serverUserConfig, serverRepository.currentUserDto) - val appPreferences = dataStore.data.first() - Assert.assertEquals(serverId.toServerString(), appPreferences.currentServerId) - Assert.assertEquals(userId.toServerString(), appPreferences.currentUserId) - - verify(exactly = 1) { mockSharedPreferences.edit() } + coVerify(exactly = 1) { mockMostRecentServerProvider.save(CurrentUser(server, user)) } verify(exactly = 1) { mockJellyfinServerDao.addOrUpdateUser(user) } } @Test fun `Test restoreUser via changeUser`() = runTest { + val user = user.copy(pin = null) every { mockJellyfinServerDao.addOrUpdateServer(any()) } just Runs every { mockJellyfinServerDao.addOrUpdateUser(user) } returns user every { mockJellyfinServerDao.getServer(serverId) } returns JellyfinServerUsers(server, listOf(user)) val serverRepository = create() Assert.assertNull(serverRepository.currentUser) - serverRepository.restoreSession(server.id, user.id) + val result = serverRepository.tryChangeUser(server.id, user.id) + assertTrue(result != null) + assertEquals(serverId, result?.server?.id) + assertEquals(user.id, result?.user?.id) verify(exactly = 1) { mockApiClient.update( @@ -226,34 +236,6 @@ class ServerRepositoryTest { Assert.assertEquals(serverUserConfig, serverRepository.currentUserDto) } - @Test - fun `Test restoreUser via shortcut`() = - runTest { - every { mockJellyfinServerDao.getServer(serverId) } returns JellyfinServerUsers(server, listOf(user)) - - val serverRepository = create() - setUpCurrentUser(serverRepository) - - serverRepository.restoreSession(server.id, user.id) - - verify(exactly = 1) { - mockApiClient.update( - baseUrl = server.url, - accessToken = user.accessToken, - clientInfo = any(), - deviceInfo = any(), - ) - } - verify(exactly = 0) { mockJellyfinServerDao.addOrUpdateServer(any()) } - verify(exactly = 0) { mockJellyfinServerDao.addOrUpdateUser(any()) } - - Assert.assertEquals(server, serverRepository.currentServer) - Assert.assertEquals(user, serverRepository.currentUser) - - // TODO fix this, need to check userDto too -// Assert.assertEquals(userDto, serverRepository.currentUserDto) - } - @Test fun `Test remove user`() = runTest { @@ -308,6 +290,7 @@ class ServerRepositoryTest { ) } verify(exactly = 1) { mockJellyfinServerDao.deleteUser(serverId, userId) } + coVerify { mockMostRecentServerProvider.clearUser() } } @Test @@ -351,6 +334,7 @@ class ServerRepositoryTest { ) } verify(exactly = 1) { mockJellyfinServerDao.deleteServer(serverId) } + coVerify { mockMostRecentServerProvider.clear() } } @Test @@ -387,11 +371,7 @@ class ServerRepositoryTest { Assert.assertEquals(user, serverRepository.currentUser) Assert.assertEquals(serverUserConfig, serverRepository.currentUserDto) - val appPreferences = dataStore.data.first() - Assert.assertEquals(serverId.toServerString(), appPreferences.currentServerId) - Assert.assertEquals(userId.toServerString(), appPreferences.currentUserId) - - verify(exactly = 1) { mockSharedPreferences.edit() } + coVerify(exactly = 1) { mockMostRecentServerProvider.save(CurrentUser(server, user)) } } @Test @@ -430,11 +410,7 @@ class ServerRepositoryTest { Assert.assertEquals(user, serverRepository.currentUser) Assert.assertEquals(serverUserConfig, serverRepository.currentUserDto) - val appPreferences = dataStore.data.first() - Assert.assertEquals(serverId.toServerString(), appPreferences.currentServerId) - Assert.assertEquals(userId.toServerString(), appPreferences.currentUserId) - - verify(exactly = 1) { mockSharedPreferences.edit() } + coVerify(exactly = 1) { mockMostRecentServerProvider.save(CurrentUser(server, user)) } } @Test @@ -484,4 +460,99 @@ class ServerRepositoryTest { } } } + + @Test + fun `Test restoreLastSession, success`() = + runTest { + val user = user.copy(pin = null) + coEvery { mockJellyfinServerDao.addOrUpdateUser(user) } returns user + coEvery { mockJellyfinServerDao.getServer(serverId) } returns + JellyfinServerUsers(server, listOf(user)) + every { mockMostRecentServerProvider.get() } returns + MostRecentServer.from(CurrentUser(server, user)) + + val result = create().restoreLastSession() + assertTrue(result is RestoredSession.Success) + val current = (result as RestoredSession.Success).currentUser + assertEquals(server, current.server) + assertEquals(user, current.user) + + verify(exactly = 1) { + mockApiClient.update( + baseUrl = server.url, + accessToken = user.accessToken, + clientInfo = any(), + deviceInfo = any(), + ) + } + coVerify { mockMostRecentServerProvider.save(any()) } + } + + @Test + fun `Test restoreLastSession, user protected`() = + runTest { + every { mockMostRecentServerProvider.get() } returns + MostRecentServer.ServerAndUser(serverId, server.url, userId, "token") + + val result = create().restoreLastSession() + assertTrue(result is RestoredSession.ServerOnly) + val current = (result as RestoredSession.ServerOnly) + assertEquals(server, current.server) + } + + @Test + fun `Test restoreLastSession, server only`() = + runTest { + every { mockMostRecentServerProvider.get() } returns + MostRecentServer.Server(server.id, server.url) + + val result = create().restoreLastSession() + assertTrue(result is RestoredSession.ServerOnly) + val current = (result as RestoredSession.ServerOnly) + assertEquals(server, current.server) + } + + @Test + fun `Test restoreLastSession, nothing to restore`() = + runTest { + every { mockMostRecentServerProvider.get() } returns MostRecentServer.None + + val result = create().restoreLastSession() + assertTrue(result is RestoredSession.None) + } +} + +class TestMostRecentServerProvider : MostRecentServerProvider { + var mostRecentServer: MostRecentServer = MostRecentServer.None + + override fun get(): MostRecentServer = mostRecentServer + + override suspend fun save(current: CurrentUser) { + mostRecentServer = + if (current.user.accessToken.isNotNullOrBlank()) { + MostRecentServer.ServerAndUser( + current.server.id, + current.server.url, + current.user.id, + current.user.accessToken, + ) + } else { + MostRecentServer.Server( + current.server.id, + current.server.url, + ) + } + } + + override suspend fun clear() { + mostRecentServer = MostRecentServer.None + } + + override suspend fun clearUser() { + when (val s = mostRecentServer) { + MostRecentServer.None -> Unit + is MostRecentServer.Server -> Unit + is MostRecentServer.ServerAndUser -> MostRecentServer.Server(s.serverId, s.serverUrl) + } + } } diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt index e066e837..0e5e0eba 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/services/SuggestionsWorkerTest.kt @@ -5,8 +5,10 @@ import androidx.work.Data import androidx.work.ListenableWorker import androidx.work.WorkerParameters import com.github.damontecres.wholphin.data.CurrentUser +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.preferences.AppPreferences +import com.github.damontecres.wholphin.test.currentUser import com.github.damontecres.wholphin.util.GetItemsRequestHandler import io.mockk.coEvery import io.mockk.coVerify @@ -113,16 +115,16 @@ class SuggestionsWorkerTest { val mockUser = mockk() var restored = false every { mockServerRepository.current } answers { MutableStateFlow(if (restored) mockUser else null) } - coEvery { mockServerRepository.restoreSession(testServerId, testUserId) } coAnswers { + coEvery { mockServerRepository.restoreLastSession() } coAnswers { restored = true - mockUser + RestoredSession.Success(currentUser()) } every { mockPreferences.data } returns flowOf(mockPrefs()) coEvery { mockUserViewsApi.getUserViews(userId = testUserId) } returns mockQueryResult() val result = createWorker().doWork() - coVerify { mockServerRepository.restoreSession(testServerId, testUserId) } + coVerify { mockServerRepository.restoreLastSession() } assertEquals(ListenableWorker.Result.success(), result) } diff --git a/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt b/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt index c4a54ec2..4a463578 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt @@ -17,6 +17,7 @@ import com.github.damontecres.wholphin.data.ItemPlaybackDao import com.github.damontecres.wholphin.data.JellyfinServerDao import com.github.damontecres.wholphin.data.LibraryDisplayInfoDao import com.github.damontecres.wholphin.data.Migrations +import com.github.damontecres.wholphin.data.MostRecentServerProvider import com.github.damontecres.wholphin.data.PlaybackEffectDao import com.github.damontecres.wholphin.data.PlaybackLanguageChoiceDao import com.github.damontecres.wholphin.data.RememberedTabDao @@ -26,6 +27,7 @@ import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.preferences.AppPreferencesSerializer import com.github.damontecres.wholphin.services.SeerrApi +import com.github.damontecres.wholphin.services.TestMostRecentServerProvider import com.github.damontecres.wholphin.services.hilt.AppModule import com.github.damontecres.wholphin.services.hilt.AuthOkHttpClient import com.github.damontecres.wholphin.services.hilt.DatabaseModule @@ -201,6 +203,10 @@ object TestModule { fun seerrApi( @StandardOkHttpClient okHttpClient: OkHttpClient, ): SeerrApi = mockk() + + @Provides + @Singleton + fun mostRecentServerProvider(): MostRecentServerProvider = TestMostRecentServerProvider() } @Module