diff --git a/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt b/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt index 8cf2605..b0c2037 100644 --- a/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt +++ b/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt @@ -33,6 +33,8 @@ import com.ubisam.ubgitea.feature.notifications.NotificationsViewModel import com.ubisam.ubgitea.feature.repositories.ReferenceKind import com.ubisam.ubgitea.feature.repositories.RepositoryListScreen import com.ubisam.ubgitea.feature.repositories.RepositoryListViewModel +import com.ubisam.ubgitea.feature.settings.SettingsScreen +import com.ubisam.ubgitea.feature.settings.SettingsViewModel import com.ubisam.ubgitea.feature.workitems.WorkItemsScreen import com.ubisam.ubgitea.feature.workitems.WorkItemsTarget import com.ubisam.ubgitea.feature.workitems.WorkItemsViewModel @@ -56,6 +58,9 @@ class MainActivity : ComponentActivity() { private val notificationsViewModel: NotificationsViewModel by viewModels { NotificationsViewModel.Factory(application) } + private val settingsViewModel: SettingsViewModel by viewModels { + SettingsViewModel.Factory(application) + } // TASK-036: no-op if already granted or already permanently denied -- the OS itself decides // whether to actually show a prompt, this call is safe to make on every launch. Must be @@ -82,6 +87,7 @@ class MainActivity : ComponentActivity() { val historyState by historyViewModel.state.collectAsState() val workItemsState by workItemsViewModel.state.collectAsState() val notificationsState by notificationsViewModel.state.collectAsState() + val settingsState by settingsViewModel.state.collectAsState() var routeStack by rememberSaveable(stateSaver = RouteStackSaver) { mutableStateOf(listOf(Route.Repositories)) @@ -114,6 +120,7 @@ class MainActivity : ComponentActivity() { WorkItemsTarget(RepositoryCoordinates.of(route.owner, route.name)), ) Route.Notifications -> notificationsViewModel.open() + Route.Settings -> Unit // SettingsViewModel reads its (synchronous) store at construction. } } @@ -128,6 +135,12 @@ class MainActivity : ComponentActivity() { UBGiteaTheme { when (currentRoute) { + Route.Settings -> SettingsScreen( + state = settingsState, + onSetReactiveAuthRetryEnabled = settingsViewModel::setReactiveAuthRetryEnabled, + onSetFavoritePushNotificationsEnabled = settingsViewModel::setFavoritePushNotificationsEnabled, + onBack = { pop() }, + ) Route.Notifications -> NotificationsScreen( state = notificationsState, onShowUnreadOnly = notificationsViewModel::showUnreadOnly, @@ -225,6 +238,7 @@ class MainActivity : ComponentActivity() { }, onSelectOrganization = viewModel::selectOrganization, onViewNotifications = { push(Route.Notifications) }, + onViewSettings = { push(Route.Settings) }, ) } LaunchedEffect(viewModel) { diff --git a/app/src/main/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProvider.kt b/app/src/main/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProvider.kt index eaab31f..4b9173d 100644 --- a/app/src/main/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProvider.kt +++ b/app/src/main/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProvider.kt @@ -2,7 +2,7 @@ package com.ubisam.ubgitea.core.auth.android import com.ubisam.ubgitea.data.auth.AuthenticationGateway import com.ubisam.ubgitea.data.auth.TokenVault -import com.ubisam.ubgitea.data.repositories.BearerTokenProvider +import com.ubisam.ubgitea.data.repositories.ReactiveTokenProvider import kotlinx.coroutines.CancellationException import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -27,7 +27,7 @@ class RefreshingTokenProvider( private val tokenVault: TokenVault, private val gateway: AuthenticationGateway, private val now: () -> Long = System::currentTimeMillis, -) : BearerTokenProvider { +) : ReactiveTokenProvider { private val mutex = Mutex() override suspend fun accessToken(): String = mutex.withLock { @@ -37,21 +37,36 @@ class RefreshingTokenProvider( val needsRefresh = expiresAt != null && expiresAt - REFRESH_SKEW_MILLIS <= now() if (!needsRefresh || refreshToken == null) return@withLock tokens.accessToken - try { - val refreshed = gateway.refreshToken(refreshToken, now()) - tokenVault.store(refreshed) - refreshed.accessToken - } catch (failure: CancellationException) { - throw failure - } catch (_: Exception) { - // Refresh itself failed (revoked/expired refresh token, offline, server error) — fall - // back to the stale access token rather than blocking the caller on this failure. The - // 401 that stale token then draws from the actual API call drives the app's existing - // AUTHENTICATION_EXPIRED handling, which is the correct outcome once refreshing - // genuinely can't succeed — this provider does not itself decide "give up," it only - // ever tries to avoid needing to. - tokens.accessToken - } + refreshOrFallBack(tokens.accessToken, refreshToken) + } + + // TASK-038: an opt-in reactive fallback for when a live 401 happens despite accessToken()'s own + // proactive check having judged the token not yet due for refresh -- e.g. a server-side + // revocation, not ordinary expiry. Unlike accessToken(), this always attempts a refresh + // regardless of the stated expiry, since the caller already knows the current token just + // failed. Still mutex-guarded, so a reactive retry racing an in-flight proactive refresh (or + // another reactive retry) shares the same single attempt at the same rotating refresh token + // rather than each independently trying to consume it. + override suspend fun forceRefresh(): String = mutex.withLock { + val tokens = tokenVault.read() ?: return@withLock "" + val refreshToken = tokens.refreshToken ?: return@withLock tokens.accessToken + refreshOrFallBack(tokens.accessToken, refreshToken) + } + + private suspend fun refreshOrFallBack(staleAccessToken: String, refreshToken: String): String = try { + val refreshed = gateway.refreshToken(refreshToken, now()) + tokenVault.store(refreshed) + refreshed.accessToken + } catch (failure: CancellationException) { + throw failure + } catch (_: Exception) { + // Refresh itself failed (revoked/expired refresh token, offline, server error) — fall + // back to the stale access token rather than blocking the caller on this failure. The + // 401 that stale token then draws from the actual API call drives the app's existing + // AUTHENTICATION_EXPIRED handling, which is the correct outcome once refreshing + // genuinely can't succeed — this provider does not itself decide "give up," it only + // ever tries to avoid needing to. + staleAccessToken } private companion object { diff --git a/app/src/main/java/com/ubisam/ubgitea/core/notifications/FavoritePushCheckWorker.kt b/app/src/main/java/com/ubisam/ubgitea/core/notifications/FavoritePushCheckWorker.kt index 18f3934..b6b2429 100644 --- a/app/src/main/java/com/ubisam/ubgitea/core/notifications/FavoritePushCheckWorker.kt +++ b/app/src/main/java/com/ubisam/ubgitea/core/notifications/FavoritePushCheckWorker.kt @@ -7,6 +7,7 @@ import androidx.work.WorkerParameters import com.ubisam.ubgitea.core.auth.android.AuthSession import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults import com.ubisam.ubgitea.core.network.OkHttpTransport +import com.ubisam.ubgitea.core.settings.AppSettingsSession import com.ubisam.ubgitea.data.repositories.GiteaRepositoryBrowserApi import com.ubisam.ubgitea.domain.usecase.GetLatestPushEpochMillisUseCase import com.ubisam.ubgitea.domain.usecase.ListFavoritedRepositoriesUseCase @@ -15,9 +16,9 @@ import kotlinx.coroutines.CancellationException /** * Periodic (see [FavoritePushScheduler] for the actual interval) check for every favorited * repository's last-push time, per TASK-036. A no-op -- [Result.success], nothing posted -- when - * there's no stored session or no favorited repositories, so this safely does nothing before the - * human product owner has ever logged in or favorited anything, without needing the scheduler - * itself to track that state. + * there's no stored session, no favorited repositories, or the human product owner has turned this + * off in Settings (TASK-038), so this safely does nothing before the human product owner has ever + * logged in or favorited anything, without needing the scheduler itself to track any of that state. */ class FavoritePushCheckWorker( context: Context, @@ -25,6 +26,7 @@ class FavoritePushCheckWorker( ) : CoroutineWorker(context, params) { override suspend fun doWork(): Result { val application = applicationContext as Application + if (!AppSettingsSession.store(application).isFavoritePushNotificationsEnabled()) return Result.success() val tokenVault = AuthSession.tokenVault(application) if (tokenVault.read() == null) return Result.success() diff --git a/app/src/main/java/com/ubisam/ubgitea/core/settings/AppSettingsSession.kt b/app/src/main/java/com/ubisam/ubgitea/core/settings/AppSettingsSession.kt new file mode 100644 index 0000000..2275231 --- /dev/null +++ b/app/src/main/java/com/ubisam/ubgitea/core/settings/AppSettingsSession.kt @@ -0,0 +1,17 @@ +package com.ubisam.ubgitea.core.settings + +import android.app.Application + +/** One shared [AppSettingsStore] per process, mirroring [com.ubisam.ubgitea.core.auth.android.AuthSession]'s + * own shared-singleton pattern -- every feature's API client and the settings screen itself all + * read/write the same underlying `SharedPreferences`-backed store rather than each independently + * constructing its own disconnected instance. + */ +object AppSettingsSession { + @Volatile private var storeInstance: AppSettingsStore? = null + + fun store(application: Application): AppSettingsStore = + storeInstance ?: synchronized(this) { + storeInstance ?: SharedPreferencesAppSettingsStore(application).also { storeInstance = it } + } +} diff --git a/app/src/main/java/com/ubisam/ubgitea/core/settings/AppSettingsStore.kt b/app/src/main/java/com/ubisam/ubgitea/core/settings/AppSettingsStore.kt new file mode 100644 index 0000000..dcdcd01 --- /dev/null +++ b/app/src/main/java/com/ubisam/ubgitea/core/settings/AppSettingsStore.kt @@ -0,0 +1,44 @@ +package com.ubisam.ubgitea.core.settings + +import android.content.Context + +/** + * The two user-facing toggles TASK-038 introduced, both defaulted to preserve exactly what the app + * already did before this existed: + * - Reactive auth retry defaults OFF (opt-in) -- it changes how every API call behaves on a 401, so + * it stays off until the human product owner explicitly turns it on, rather than silently + * changing existing behavior for everyone. + * - Favorite push notifications defaults ON, since that's what TASK-036 already did unconditionally + * before this settings screen existed -- this toggle only adds an explicit way to turn *off* what + * was already always on, not a new default. + */ +interface AppSettingsStore { + fun isReactiveAuthRetryEnabled(): Boolean + fun setReactiveAuthRetryEnabled(enabled: Boolean) + fun isFavoritePushNotificationsEnabled(): Boolean + fun setFavoritePushNotificationsEnabled(enabled: Boolean) +} + +class SharedPreferencesAppSettingsStore(context: Context) : AppSettingsStore { + private val preferences = context.getSharedPreferences(PREFERENCES_NAME, Context.MODE_PRIVATE) + + override fun isReactiveAuthRetryEnabled(): Boolean = + preferences.getBoolean(KEY_REACTIVE_AUTH_RETRY, false) + + override fun setReactiveAuthRetryEnabled(enabled: Boolean) { + preferences.edit().putBoolean(KEY_REACTIVE_AUTH_RETRY, enabled).apply() + } + + override fun isFavoritePushNotificationsEnabled(): Boolean = + preferences.getBoolean(KEY_FAVORITE_PUSH_NOTIFICATIONS, true) + + override fun setFavoritePushNotificationsEnabled(enabled: Boolean) { + preferences.edit().putBoolean(KEY_FAVORITE_PUSH_NOTIFICATIONS, enabled).apply() + } + + private companion object { + const val PREFERENCES_NAME = "ubgitea_app_settings" + const val KEY_REACTIVE_AUTH_RETRY = "reactive_auth_retry_enabled" + const val KEY_FAVORITE_PUSH_NOTIFICATIONS = "favorite_push_notifications_enabled" + } +} diff --git a/app/src/main/java/com/ubisam/ubgitea/data/history/GiteaHistoryApi.kt b/app/src/main/java/com/ubisam/ubgitea/data/history/GiteaHistoryApi.kt index 1f7b044..e3695a2 100644 --- a/app/src/main/java/com/ubisam/ubgitea/data/history/GiteaHistoryApi.kt +++ b/app/src/main/java/com/ubisam/ubgitea/data/history/GiteaHistoryApi.kt @@ -9,6 +9,7 @@ import com.ubisam.ubgitea.core.network.HttpResponse import com.ubisam.ubgitea.core.network.HttpTransport import com.ubisam.ubgitea.core.network.requireSuccess import com.ubisam.ubgitea.data.repositories.BearerTokenProvider +import com.ubisam.ubgitea.data.repositories.callAuthenticated import com.ubisam.ubgitea.domain.history.BoundedPatch import com.ubisam.ubgitea.domain.history.CommitDetail import com.ubisam.ubgitea.domain.history.FileChange @@ -33,6 +34,8 @@ class GiteaHistoryApi( private val maxPatchBytes: Int = 200_000, private val maxPatchLines: Int = 4_000, private val gson: Gson = Gson(), + // Defaulted off, see GiteaRepositoryBrowserApi's identical parameter for why (TASK-038). + private val reactiveAuthRetryEnabled: () -> Boolean = { false }, ) : HistoryRepository { init { require(maxPatchBytes > 0) { "maxPatchBytes must be positive" } @@ -96,15 +99,14 @@ class GiteaHistoryApi( throw ApiFailure.InvalidResponse() } - private suspend fun get(url: HttpUrl): HttpResponse { - val token = tokenProvider.accessToken() - if (token.isBlank()) throw ApiFailure.AuthenticationExpired() - return try { - transport.get(url.toString(), token).requireSuccess() - } catch (failure: IOException) { - throw ApiFailure.Network(failure) + private suspend fun get(url: HttpUrl): HttpResponse = + callAuthenticated(tokenProvider, reactiveAuthRetryEnabled) { token -> + try { + transport.get(url.toString(), token).requireSuccess() + } catch (failure: IOException) { + throw ApiFailure.Network(failure) + } } - } private inline fun parseObject(body: String): T = try { gson.fromJson(body, T::class.java) ?: throw ApiFailure.InvalidResponse() diff --git a/app/src/main/java/com/ubisam/ubgitea/data/notifications/GiteaNotificationsApi.kt b/app/src/main/java/com/ubisam/ubgitea/data/notifications/GiteaNotificationsApi.kt index 01abaff..148c49c 100644 --- a/app/src/main/java/com/ubisam/ubgitea/data/notifications/GiteaNotificationsApi.kt +++ b/app/src/main/java/com/ubisam/ubgitea/data/notifications/GiteaNotificationsApi.kt @@ -10,6 +10,7 @@ import com.ubisam.ubgitea.core.network.HttpResponse import com.ubisam.ubgitea.core.network.HttpTransport import com.ubisam.ubgitea.core.network.requireSuccess import com.ubisam.ubgitea.data.repositories.BearerTokenProvider +import com.ubisam.ubgitea.data.repositories.callAuthenticated import com.ubisam.ubgitea.domain.model.Page import com.ubisam.ubgitea.domain.notifications.NotificationSubjectState import com.ubisam.ubgitea.domain.notifications.NotificationSubjectType @@ -33,6 +34,8 @@ class GiteaNotificationsApi( private val transport: HttpTransport, private val tokenProvider: BearerTokenProvider, private val gson: Gson = Gson(), + // Defaulted off, see GiteaRepositoryBrowserApi's identical parameter for why (TASK-038). + private val reactiveAuthRetryEnabled: () -> Boolean = { false }, ) : NotificationsRepository { override suspend fun listNotifications(page: Int, pageSize: Int, unreadOnly: Boolean): Page { validatePage(page, pageSize) @@ -90,15 +93,14 @@ class GiteaNotificationsApi( throw ApiFailure.InvalidResponse() } - private suspend fun get(url: HttpUrl): HttpResponse { - val token = tokenProvider.accessToken() - if (token.isBlank()) throw ApiFailure.AuthenticationExpired() - return try { - transport.get(url.toString(), token).requireSuccess() - } catch (failure: IOException) { - throw ApiFailure.Network(failure) + private suspend fun get(url: HttpUrl): HttpResponse = + callAuthenticated(tokenProvider, reactiveAuthRetryEnabled) { token -> + try { + transport.get(url.toString(), token).requireSuccess() + } catch (failure: IOException) { + throw ApiFailure.Network(failure) + } } - } private inline fun parseList(body: String): List = try { val type = object : TypeToken>() {}.type diff --git a/app/src/main/java/com/ubisam/ubgitea/data/repositories/GiteaRepositoryBrowserApi.kt b/app/src/main/java/com/ubisam/ubgitea/data/repositories/GiteaRepositoryBrowserApi.kt index d4d511a..99aac63 100644 --- a/app/src/main/java/com/ubisam/ubgitea/data/repositories/GiteaRepositoryBrowserApi.kt +++ b/app/src/main/java/com/ubisam/ubgitea/data/repositories/GiteaRepositoryBrowserApi.kt @@ -37,11 +37,58 @@ fun interface BearerTokenProvider { suspend fun accessToken(): String } +/** + * The narrow extra capability TASK-038's opt-in reactive-auth-retry needs: forcing a refresh right + * now, bypassing [BearerTokenProvider.accessToken]'s own proactive time check (which would just + * hand back the same still-time-valid-looking token again if the 401 that triggered this was + * something other than ordinary expiry, e.g. a server-side revocation). Kept as its own interface, + * not added to [BearerTokenProvider] itself, so that interface can stay a `fun interface` (a single + * abstract method) — every existing test fake written as a plain `BearerTokenProvider { "token" }` + * lambda keeps compiling unchanged; only [RefreshingTokenProvider] (the one production + * implementation) also implements this one, and call sites check `is ReactiveTokenProvider` before + * relying on it, falling back to the pre-TASK-038 behavior otherwise. + */ +interface ReactiveTokenProvider : BearerTokenProvider { + suspend fun forceRefresh(): String +} + +/** + * Shared by every Gitea API class's own `authenticated`/`get` wrapper (TASK-038) instead of each + * duplicating this exact retry orchestration: get a token, make the call, and -- only when + * [reactiveAuthRetryEnabled] says so *and* [tokenProvider] actually supports it + * ([ReactiveTokenProvider]; a plain test fake typed as [BearerTokenProvider] simply can't, and this + * falls back to the old single-attempt behavior rather than failing to compile or throwing -- + * retry exactly once with a forcibly-refreshed token before giving up. [call] is expected to throw + * [ApiFailure.AuthenticationExpired] on a 401 (as `requireSuccess()` already does) and any other + * `ApiFailure` for every other outcome; only that one specific exception triggers the retry. + */ +internal suspend fun callAuthenticated( + tokenProvider: BearerTokenProvider, + reactiveAuthRetryEnabled: () -> Boolean, + call: suspend (token: String) -> T, +): T { + val token = tokenProvider.accessToken() + if (token.isBlank()) throw ApiFailure.AuthenticationExpired() + return try { + call(token) + } catch (failure: ApiFailure.AuthenticationExpired) { + val reactiveProvider = tokenProvider as? ReactiveTokenProvider + if (reactiveProvider == null || !reactiveAuthRetryEnabled()) throw failure + val refreshedToken = reactiveProvider.forceRefresh() + if (refreshedToken.isBlank()) throw failure + call(refreshedToken) + } +} + class GiteaRepositoryBrowserApi( private val config: OAuthClientConfig, private val transport: HttpTransport, private val tokenProvider: BearerTokenProvider, private val gson: Gson = Gson(), + // Defaulted off so every existing call site/test keeps compiling and behaving identically + // (TASK-038 is opt-in, see AppSettingsStore); production wiring passes the human product + // owner's actual Settings choice instead. + private val reactiveAuthRetryEnabled: () -> Boolean = { false }, ) : GiteaRepositoryBrowser, RepositoryFavorites, OrganizationsRepository { override suspend fun listRepositories(page: Int, pageSize: Int): Page { validatePage(page, pageSize) @@ -214,15 +261,14 @@ class GiteaRepositoryBrowserApi( transport.delete(url.toString(), token) } - private suspend fun authenticated(call: suspend (String) -> HttpResponse): HttpResponse { - val token = tokenProvider.accessToken() - if (token.isBlank()) throw ApiFailure.AuthenticationExpired() - return try { - call(token).requireSuccess() - } catch (failure: IOException) { - throw ApiFailure.Network(failure) + private suspend fun authenticated(call: suspend (String) -> HttpResponse): HttpResponse = + callAuthenticated(tokenProvider, reactiveAuthRetryEnabled) { token -> + try { + call(token).requireSuccess() + } catch (failure: IOException) { + throw ApiFailure.Network(failure) + } } - } private fun mapRepository(dto: RepositoryDto): RepositorySummary { val owner = dto.owner?.login.required() diff --git a/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt b/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt index 6ca04ef..f97bcc8 100644 --- a/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt +++ b/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt @@ -10,6 +10,7 @@ import com.ubisam.ubgitea.core.network.HttpResponse import com.ubisam.ubgitea.core.network.HttpTransport import com.ubisam.ubgitea.core.network.requireSuccess import com.ubisam.ubgitea.data.repositories.BearerTokenProvider +import com.ubisam.ubgitea.data.repositories.callAuthenticated import com.ubisam.ubgitea.domain.model.Page import com.ubisam.ubgitea.domain.model.RepositoryCoordinates import com.ubisam.ubgitea.domain.workitems.CommentSummary @@ -42,6 +43,8 @@ class GiteaWorkItemsApi( private val transport: HttpTransport, private val tokenProvider: BearerTokenProvider, private val gson: Gson = Gson(), + // Defaulted off, see GiteaRepositoryBrowserApi's identical parameter for why (TASK-038). + private val reactiveAuthRetryEnabled: () -> Boolean = { false }, ) : WorkItemsRepository { override suspend fun listIssues(repository: RepositoryCoordinates, page: Int, pageSize: Int): Page { validatePage(page, pageSize) @@ -121,15 +124,14 @@ class GiteaWorkItemsApi( return response.toPage(items, page, pageSize) } - private suspend fun get(url: HttpUrl): HttpResponse { - val token = tokenProvider.accessToken() - if (token.isBlank()) throw ApiFailure.AuthenticationExpired() - return try { - transport.get(url.toString(), token).requireSuccess() - } catch (failure: IOException) { - throw ApiFailure.Network(failure) + private suspend fun get(url: HttpUrl): HttpResponse = + callAuthenticated(tokenProvider, reactiveAuthRetryEnabled) { token -> + try { + transport.get(url.toString(), token).requireSuccess() + } catch (failure: IOException) { + throw ApiFailure.Network(failure) + } } - } private fun mapAuthor(dto: UserDto?): WorkItemAuthor { val login = dto?.login?.takeIf(String::isNotBlank) ?: return WorkItemAuthor.Unavailable diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt index a6e9f12..b314855 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt @@ -8,6 +8,7 @@ import com.ubisam.ubgitea.core.auth.android.AuthSession import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults import com.ubisam.ubgitea.core.network.ApiFailure import com.ubisam.ubgitea.core.network.OkHttpTransport +import com.ubisam.ubgitea.core.settings.AppSettingsSession import com.ubisam.ubgitea.data.repositories.GiteaRepositoryBrowserApi import com.ubisam.ubgitea.domain.model.RepositoryCoordinates import com.ubisam.ubgitea.domain.model.RepositoryPath @@ -240,6 +241,7 @@ class CodeBrowserViewModel( UBGiteaOAuthDefaults.config, OkHttpTransport(), AuthSession.tokenProvider(application), + reactiveAuthRetryEnabled = { AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }, ), ) as T } diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/history/HistoryViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/history/HistoryViewModel.kt index 1c267fd..09dac81 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/history/HistoryViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/history/HistoryViewModel.kt @@ -8,6 +8,7 @@ import com.ubisam.ubgitea.core.auth.android.AuthSession import com.ubisam.ubgitea.core.network.ApiFailure import com.ubisam.ubgitea.core.network.OkHttpTransport import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults +import com.ubisam.ubgitea.core.settings.AppSettingsSession import com.ubisam.ubgitea.data.history.GiteaHistoryApi import com.ubisam.ubgitea.data.repositories.GiteaRepositoryBrowserApi import com.ubisam.ubgitea.domain.history.CommitDetail @@ -237,8 +238,19 @@ class HistoryViewModel( override fun create(modelClass: Class): T { require(modelClass.isAssignableFrom(HistoryViewModel::class.java)) val tokenProvider = AuthSession.tokenProvider(application) - val browser = GiteaRepositoryBrowserApi(UBGiteaOAuthDefaults.config, OkHttpTransport(), tokenProvider) - val history = GiteaHistoryApi(UBGiteaOAuthDefaults.config, OkHttpTransport(), tokenProvider) + val reactiveAuthRetryEnabled = { AppSettingsSession.store(application).isReactiveAuthRetryEnabled() } + val browser = GiteaRepositoryBrowserApi( + UBGiteaOAuthDefaults.config, + OkHttpTransport(), + tokenProvider, + reactiveAuthRetryEnabled = reactiveAuthRetryEnabled, + ) + val history = GiteaHistoryApi( + UBGiteaOAuthDefaults.config, + OkHttpTransport(), + tokenProvider, + reactiveAuthRetryEnabled = reactiveAuthRetryEnabled, + ) return HistoryViewModel(browser, history) as T } } diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/notifications/NotificationsViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/notifications/NotificationsViewModel.kt index 5143d34..fdb685b 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/notifications/NotificationsViewModel.kt @@ -8,6 +8,7 @@ import com.ubisam.ubgitea.core.auth.android.AuthSession import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults import com.ubisam.ubgitea.core.network.ApiFailure import com.ubisam.ubgitea.core.network.OkHttpTransport +import com.ubisam.ubgitea.core.settings.AppSettingsSession import com.ubisam.ubgitea.data.notifications.GiteaNotificationsApi import com.ubisam.ubgitea.domain.notifications.NotificationSummary import com.ubisam.ubgitea.domain.notifications.NotificationsRepository @@ -101,6 +102,7 @@ class NotificationsViewModel(private val repository: NotificationsRepository) : UBGiteaOAuthDefaults.config, OkHttpTransport(), AuthSession.tokenProvider(application), + reactiveAuthRetryEnabled = { AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }, ) return NotificationsViewModel(repository) as T } diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListScreen.kt b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListScreen.kt index 3db414b..19ab8ee 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListScreen.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListScreen.kt @@ -63,6 +63,7 @@ fun RepositoryListScreen( onViewWorkItems: (RepositoryRowUiModel) -> Unit, onSelectOrganization: (String?) -> Unit, onViewNotifications: () -> Unit, + onViewSettings: () -> Unit, modifier: Modifier = Modifier, ) { Scaffold(modifier = modifier.fillMaxSize()) { padding -> @@ -126,6 +127,7 @@ fun RepositoryListScreen( onShowFavorites, onSelectOrganization, onViewNotifications, + onViewSettings, onToggleFavorite, Modifier.padding(padding), ) @@ -210,6 +212,7 @@ private fun RepositoryContent( onShowFavorites: () -> Unit, onSelectOrganization: (String?) -> Unit, onViewNotifications: () -> Unit, + onViewSettings: () -> Unit, onToggleFavorite: (RepositoryRowUiModel) -> Unit, modifier: Modifier = Modifier, ) { @@ -231,7 +234,10 @@ private fun RepositoryContent( Text(stringResource(R.string.repositories_account, state.accountName)) } Spacer(Modifier.weight(1f)) - OutlinedButton(onClick = onViewNotifications) { Text(stringResource(R.string.repositories_notifications)) } + OutlinedButton(onClick = onViewSettings) { Text(stringResource(R.string.repositories_settings)) } + OutlinedButton(onClick = onViewNotifications, modifier = Modifier.padding(start = 8.dp)) { + Text(stringResource(R.string.repositories_notifications)) + } OutlinedButton(onClick = onLogout, modifier = Modifier.padding(start = 8.dp)) { Text(stringResource(R.string.repositories_logout)) } diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt index 5c73fb8..7b1f722 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt @@ -13,6 +13,7 @@ import com.ubisam.ubgitea.core.auth.protocol.OAuthCallbackValidator import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults import com.ubisam.ubgitea.core.network.ApiFailure import com.ubisam.ubgitea.core.network.OkHttpTransport +import com.ubisam.ubgitea.core.settings.AppSettingsSession import com.ubisam.ubgitea.data.auth.AuthApi import com.ubisam.ubgitea.data.auth.AuthenticationGateway import com.ubisam.ubgitea.data.auth.TokenVault @@ -715,6 +716,7 @@ class RepositoryListViewModel( UBGiteaOAuthDefaults.config, OkHttpTransport(), AuthSession.tokenProvider(application), + reactiveAuthRetryEnabled = { AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }, ) return RepositoryListViewModel( tokenVault = tokenVault, diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/settings/SettingsScreen.kt b/app/src/main/java/com/ubisam/ubgitea/feature/settings/SettingsScreen.kt new file mode 100644 index 0000000..0219920 --- /dev/null +++ b/app/src/main/java/com/ubisam/ubgitea/feature/settings/SettingsScreen.kt @@ -0,0 +1,72 @@ +package com.ubisam.ubgitea.feature.settings + +import androidx.activity.compose.BackHandler +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Scaffold +import androidx.compose.material3.Switch +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import com.ubisam.ubgitea.R + +@Composable +fun SettingsScreen( + state: SettingsUiState, + onSetReactiveAuthRetryEnabled: (Boolean) -> Unit, + onSetFavoritePushNotificationsEnabled: (Boolean) -> Unit, + onBack: () -> Unit, + modifier: Modifier = Modifier, +) { + BackHandler(onBack = onBack) + Scaffold(modifier = modifier.fillMaxSize()) { padding -> + Column(Modifier.fillMaxSize().padding(padding).padding(16.dp)) { + Text(stringResource(R.string.settings_title), style = MaterialTheme.typography.headlineSmall) + SettingRow( + title = stringResource(R.string.settings_reactive_auth_retry_title), + description = stringResource(R.string.settings_reactive_auth_retry_description), + checked = state.reactiveAuthRetryEnabled, + onCheckedChange = onSetReactiveAuthRetryEnabled, + ) + HorizontalDivider() + SettingRow( + title = stringResource(R.string.settings_favorite_push_notifications_title), + description = stringResource(R.string.settings_favorite_push_notifications_description), + checked = state.favoritePushNotificationsEnabled, + onCheckedChange = onSetFavoritePushNotificationsEnabled, + ) + HorizontalDivider() + } + } +} + +@Composable +private fun SettingRow( + title: String, + description: String, + checked: Boolean, + onCheckedChange: (Boolean) -> Unit, +) { + Row( + Modifier.fillMaxWidth().padding(vertical = 16.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Column(Modifier.weight(1f).padding(end = 12.dp)) { + Text(title, style = MaterialTheme.typography.titleMedium) + Text( + description, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + Switch(checked = checked, onCheckedChange = onCheckedChange) + } +} diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/settings/SettingsViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/settings/SettingsViewModel.kt new file mode 100644 index 0000000..8807a86 --- /dev/null +++ b/app/src/main/java/com/ubisam/ubgitea/feature/settings/SettingsViewModel.kt @@ -0,0 +1,48 @@ +package com.ubisam.ubgitea.feature.settings + +import android.app.Application +import androidx.lifecycle.ViewModel +import androidx.lifecycle.ViewModelProvider +import com.ubisam.ubgitea.core.settings.AppSettingsSession +import com.ubisam.ubgitea.core.settings.AppSettingsStore +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow + +data class SettingsUiState( + val reactiveAuthRetryEnabled: Boolean, + val favoritePushNotificationsEnabled: Boolean, +) + +/** + * TASK-038's two opt-in toggles. Reads straight from [AppSettingsStore]'s synchronous + * `SharedPreferences` at construction time -- no loading state needed, unlike every other + * ViewModel in this app that has to wait on a network round trip first. + */ +class SettingsViewModel(private val store: AppSettingsStore) : ViewModel() { + private val mutableState = MutableStateFlow( + SettingsUiState( + reactiveAuthRetryEnabled = store.isReactiveAuthRetryEnabled(), + favoritePushNotificationsEnabled = store.isFavoritePushNotificationsEnabled(), + ), + ) + val state: StateFlow = mutableState.asStateFlow() + + fun setReactiveAuthRetryEnabled(enabled: Boolean) { + store.setReactiveAuthRetryEnabled(enabled) + mutableState.value = mutableState.value.copy(reactiveAuthRetryEnabled = enabled) + } + + fun setFavoritePushNotificationsEnabled(enabled: Boolean) { + store.setFavoritePushNotificationsEnabled(enabled) + mutableState.value = mutableState.value.copy(favoritePushNotificationsEnabled = enabled) + } + + class Factory(private val application: Application) : ViewModelProvider.Factory { + @Suppress("UNCHECKED_CAST") + override fun create(modelClass: Class): T { + require(modelClass.isAssignableFrom(SettingsViewModel::class.java)) + return SettingsViewModel(AppSettingsSession.store(application)) as T + } + } +} diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt index 42dd782..b1f9206 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt @@ -8,6 +8,7 @@ import com.ubisam.ubgitea.core.auth.android.AuthSession import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults import com.ubisam.ubgitea.core.network.ApiFailure import com.ubisam.ubgitea.core.network.OkHttpTransport +import com.ubisam.ubgitea.core.settings.AppSettingsSession import com.ubisam.ubgitea.data.workitems.GiteaWorkItemsApi import com.ubisam.ubgitea.domain.model.Page import com.ubisam.ubgitea.domain.model.RepositoryCoordinates @@ -241,7 +242,12 @@ class WorkItemsViewModel(private val repository: WorkItemsRepository) : ViewMode override fun create(modelClass: Class): T { require(modelClass.isAssignableFrom(WorkItemsViewModel::class.java)) val tokenProvider = AuthSession.tokenProvider(application) - val repository = GiteaWorkItemsApi(UBGiteaOAuthDefaults.config, OkHttpTransport(), tokenProvider) + val repository = GiteaWorkItemsApi( + UBGiteaOAuthDefaults.config, + OkHttpTransport(), + tokenProvider, + reactiveAuthRetryEnabled = { AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }, + ) return WorkItemsViewModel(repository) as T } } diff --git a/app/src/main/java/com/ubisam/ubgitea/ui/navigation/Route.kt b/app/src/main/java/com/ubisam/ubgitea/ui/navigation/Route.kt index 8649baf..a180dcb 100644 --- a/app/src/main/java/com/ubisam/ubgitea/ui/navigation/Route.kt +++ b/app/src/main/java/com/ubisam/ubgitea/ui/navigation/Route.kt @@ -16,4 +16,5 @@ sealed interface Route { data class BranchDetail(val owner: String, val name: String, val refKind: String, val ref: String) : Route data class WorkItems(val owner: String, val name: String) : Route data object Notifications : Route + data object Settings : Route } diff --git a/app/src/main/java/com/ubisam/ubgitea/ui/navigation/RouteStackSaver.kt b/app/src/main/java/com/ubisam/ubgitea/ui/navigation/RouteStackSaver.kt index 4059058..64a184f 100644 --- a/app/src/main/java/com/ubisam/ubgitea/ui/navigation/RouteStackSaver.kt +++ b/app/src/main/java/com/ubisam/ubgitea/ui/navigation/RouteStackSaver.kt @@ -25,12 +25,14 @@ internal object RouteEncoding { private const val TYPE_BRANCH_DETAIL = "branchDetail" private const val TYPE_WORK_ITEMS = "workItems" private const val TYPE_NOTIFICATIONS = "notifications" + private const val TYPE_SETTINGS = "settings" fun encode(route: Route): String = when (route) { Route.Repositories -> TYPE_REPOSITORIES is Route.BranchDetail -> join(TYPE_BRANCH_DETAIL, route.owner, route.name, route.refKind, route.ref) is Route.WorkItems -> join(TYPE_WORK_ITEMS, route.owner, route.name) Route.Notifications -> TYPE_NOTIFICATIONS + Route.Settings -> TYPE_SETTINGS } fun decode(encoded: String): Route = decodeOrNull(encoded) ?: Route.Repositories @@ -45,6 +47,7 @@ internal object RouteEncoding { Route.WorkItems(owner = it[1], name = it[2]) } TYPE_NOTIFICATIONS -> Route.Notifications + TYPE_SETTINGS -> Route.Settings TYPE_REPOSITORIES -> Route.Repositories else -> null } diff --git a/app/src/main/res/values/strings_repositories.xml b/app/src/main/res/values/strings_repositories.xml index 3b87563..603de48 100644 --- a/app/src/main/res/values/strings_repositories.xml +++ b/app/src/main/res/values/strings_repositories.xml @@ -14,6 +14,7 @@ Gitea 서버에서 오류가 발생했습니다. 로그인 응답을 확인할 수 없습니다. 다시 시도해 주세요. 다시 시도 + 설정 로그아웃 비공개 공개 diff --git a/app/src/main/res/values/strings_settings.xml b/app/src/main/res/values/strings_settings.xml new file mode 100644 index 0000000..dd6e81c --- /dev/null +++ b/app/src/main/res/values/strings_settings.xml @@ -0,0 +1,7 @@ + + 설정 + 인증 오류 시 자동 복구 + 인증이 예기치 않게 만료되면, 재로그인 화면을 띄우기 전에 한 번 더 조용히 갱신을 시도합니다. + 즐겨찾기 저장소 Push 알림 + 즐겨찾기한 저장소에 새로운 push가 있으면 알려드립니다. + diff --git a/app/src/test/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProviderTest.kt b/app/src/test/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProviderTest.kt index 313102c..0b4c73c 100644 --- a/app/src/test/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProviderTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/core/auth/android/RefreshingTokenProviderTest.kt @@ -93,6 +93,54 @@ class RefreshingTokenProviderTest { assertEquals(0, gateway.refreshCalls) } + @Test + fun `forceRefresh refreshes and persists a new token even when not close to expiry`() = runTest { + // Deliberately far from any skew window -- forceRefresh is the TASK-038 reactive path, + // called after a live 401 already happened, so it must not defer to accessToken()'s own + // proactive time check (which would just hand back the same still-time-valid-looking token + // again and achieve nothing). + val vault = FakeVault(TokenSet("stale-access", "refresh", expiresAtEpochMillis = 999_999_999L)) + val gateway = FakeGateway(refreshed = TokenSet("fresh-access", "fresh-refresh", 2_000_000L)) + val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000L }) + + val token = provider.forceRefresh() + + assertEquals("fresh-access", token) + assertEquals(1, gateway.refreshCalls) + assertEquals("fresh-access", vault.stored?.accessToken) + } + + @Test + fun `forceRefresh falls back to the stale token when there is no refresh token to use`() = runTest { + val vault = FakeVault(TokenSet("stale-access", refreshToken = null, expiresAtEpochMillis = null)) + val gateway = FakeGateway() + val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000L }) + + val token = provider.forceRefresh() + + assertEquals("stale-access", token) + assertEquals(0, gateway.refreshCalls) + } + + @Test + fun `forceRefresh falls back to the stale token when the refresh attempt itself fails`() = runTest { + val vault = FakeVault(TokenSet("stale-access", "refresh", expiresAtEpochMillis = 999_999_999L)) + val gateway = FakeGateway(refreshFailure = RuntimeException("offline")) + val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000L }) + + val token = provider.forceRefresh() + + assertEquals("stale-access", token) + } + + @Test + fun `forceRefresh returns empty string when nothing is stored at all`() = runTest { + val vault = FakeVault(null) + val provider = RefreshingTokenProvider(vault, FakeGateway()) + + assertEquals("", provider.forceRefresh()) + } + private class FakeVault(initial: TokenSet?) : TokenVault { var stored: TokenSet? = initial override suspend fun store(tokens: TokenSet) { stored = tokens } diff --git a/app/src/test/java/com/ubisam/ubgitea/data/repositories/CallAuthenticatedTest.kt b/app/src/test/java/com/ubisam/ubgitea/data/repositories/CallAuthenticatedTest.kt new file mode 100644 index 0000000..6061ba8 --- /dev/null +++ b/app/src/test/java/com/ubisam/ubgitea/data/repositories/CallAuthenticatedTest.kt @@ -0,0 +1,135 @@ +package com.ubisam.ubgitea.data.repositories + +import com.ubisam.ubgitea.core.network.ApiFailure +import kotlin.coroutines.Continuation +import kotlin.coroutines.EmptyCoroutineContext +import kotlin.coroutines.startCoroutine +import org.junit.Assert.assertEquals +import org.junit.Assert.assertThrows +import org.junit.Test + +/** + * TASK-038's shared reactive-retry orchestration, exercised directly rather than only through one + * of its four call sites (GiteaRepositoryBrowserApi/GiteaHistoryApi/GiteaWorkItemsApi/ + * GiteaNotificationsApi's own `get`/`authenticated`), since it's the one place the actual retry + * decision is made. + */ +class CallAuthenticatedTest { + @Test + fun `does not retry when reactive auth retry is disabled`() = runSuspend { + val provider = FakeReactiveProvider(initialToken = "stale") + var calls = 0 + + assertThrows(ApiFailure.AuthenticationExpired::class.java) { + runSuspend { + callAuthenticated(provider, reactiveAuthRetryEnabled = { false }) { calls++; throw ApiFailure.AuthenticationExpired() } + } + } + + assertEquals(1, calls) + assertEquals(0, provider.forceRefreshCalls) + } + + @Test + fun `does not retry when the token provider does not support reactive refresh`() = runSuspend { + val provider = BearerTokenProvider { "stale" } + var calls = 0 + + assertThrows(ApiFailure.AuthenticationExpired::class.java) { + runSuspend { + callAuthenticated(provider, reactiveAuthRetryEnabled = { true }) { calls++; throw ApiFailure.AuthenticationExpired() } + } + } + + assertEquals(1, calls) + } + + @Test + fun `retries once with a forcibly-refreshed token when enabled and supported`() = runSuspend { + val provider = FakeReactiveProvider(initialToken = "stale", refreshedToken = "fresh") + val seenTokens = mutableListOf() + + val result = callAuthenticated(provider, reactiveAuthRetryEnabled = { true }) { token -> + seenTokens += token + if (token == "stale") throw ApiFailure.AuthenticationExpired() + "success with $token" + } + + assertEquals("success with fresh", result) + assertEquals(listOf("stale", "fresh"), seenTokens) + assertEquals(1, provider.forceRefreshCalls) + } + + @Test + fun `propagates the original failure when the retry also fails`() = runSuspend { + val provider = FakeReactiveProvider(initialToken = "stale", refreshedToken = "still-bad") + var calls = 0 + + assertThrows(ApiFailure.AuthenticationExpired::class.java) { + runSuspend { + callAuthenticated(provider, reactiveAuthRetryEnabled = { true }) { calls++; throw ApiFailure.AuthenticationExpired() } + } + } + + assertEquals(2, calls) + assertEquals(1, provider.forceRefreshCalls) + } + + @Test + fun `does not retry when the forced refresh itself yields a blank token`() = runSuspend { + val provider = FakeReactiveProvider(initialToken = "stale", refreshedToken = "") + var calls = 0 + + assertThrows(ApiFailure.AuthenticationExpired::class.java) { + runSuspend { + callAuthenticated(provider, reactiveAuthRetryEnabled = { true }) { calls++; throw ApiFailure.AuthenticationExpired() } + } + } + + assertEquals(1, calls) + assertEquals(1, provider.forceRefreshCalls) + } + + @Test + fun `a non-authentication failure is never retried`() = runSuspend { + val provider = FakeReactiveProvider(initialToken = "stale") + var calls = 0 + + assertThrows(ApiFailure.PermissionDenied::class.java) { + runSuspend { + callAuthenticated(provider, reactiveAuthRetryEnabled = { true }) { calls++; throw ApiFailure.PermissionDenied() } + } + } + + assertEquals(1, calls) + assertEquals(0, provider.forceRefreshCalls) + } + + private class FakeReactiveProvider( + private val initialToken: String, + private val refreshedToken: String = "", + ) : ReactiveTokenProvider { + var forceRefreshCalls = 0 + + override suspend fun accessToken(): String = initialToken + + override suspend fun forceRefresh(): String { + forceRefreshCalls++ + return refreshedToken + } + } +} + +private fun runSuspend(block: suspend () -> T): T { + var outcome: Result? = null + block.startCoroutine( + object : Continuation { + override val context = EmptyCoroutineContext + + override fun resumeWith(result: Result) { + outcome = result + } + }, + ) + return checkNotNull(outcome).getOrThrow() +} diff --git a/app/src/test/java/com/ubisam/ubgitea/ui/navigation/RouteEncodingTest.kt b/app/src/test/java/com/ubisam/ubgitea/ui/navigation/RouteEncodingTest.kt index 2c6e0b1..29be040 100644 --- a/app/src/test/java/com/ubisam/ubgitea/ui/navigation/RouteEncodingTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/ui/navigation/RouteEncodingTest.kt @@ -22,6 +22,11 @@ class RouteEncodingTest { assertEquals(route, RouteEncoding.decode(RouteEncoding.encode(route))) } + @Test + fun `settings route round-trips`() { + assertEquals(Route.Settings, RouteEncoding.decode(RouteEncoding.encode(Route.Settings))) + } + @Test fun `an unrecognized encoding falls back to Repositories instead of crashing`() { assertEquals(Route.Repositories, RouteEncoding.decode("not-a-known-route-type")) diff --git a/docs/development-log.md b/docs/development-log.md index 3c1721a..faa286b 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -672,3 +672,17 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Known limitations: same as the rest of TASK-028 — not yet seen live against a repository with enough real issues/PRs to populate the numbered UI, since no such repository was found this session. - Next recommended task: this was a small, contained polish item. Larger remaining candidates (e.g. a reactive 401-retry fallback at the network layer, per-repository notification settings) were considered but judged too large/risky for unrequested autonomous continuation without a dedicated task scoping pass — flagging them for the human product owner's direction rather than proceeding further unrequested into security-adjacent or larger-surface-area territory. - Next owner: Claude, continuing per the standing direction once the human product owner responds, or awaiting live confirmation of everything already built this session. + +## 2026-08-29 (continued) — TASK-038: Settings screen with opt-in auth-retry and a favorite-push toggle + +- Agent and role: Claude, direct continuation of the same live session. Presented two candidate next-steps (reactive 401 retry / per-toggle push notification settings) and their tradeoffs; the human product owner asked for both, with the reactive retry explicitly framed as something the Settings screen should gate ("설정화면에서 설정 해두면 동작하고 안해두면 동작 안하게"). +- Commit/diff: on `claude/task-015-notifications`; new `core/settings/` (`AppSettingsStore`, `SharedPreferencesAppSettingsStore`, `AppSettingsSession`), new `feature/settings/` (`SettingsViewModel`, `SettingsScreen`), `Route.Settings` + `RouteStackSaver`, a "설정" button on `RepositoryListScreen`'s top bar, new `ReactiveTokenProvider` interface + shared `callAuthenticated` helper in `GiteaRepositoryBrowserApi.kt`, `RefreshingTokenProvider.forceRefresh()`, all four Gitea API classes (`GiteaRepositoryBrowserApi`/`GiteaHistoryApi`/`GiteaWorkItemsApi`/`GiteaNotificationsApi`) gained a defaulted `reactiveAuthRetryEnabled` parameter, all five `ViewModel.Factory`s that construct one of them wired to the new setting, `FavoritePushCheckWorker` gated on the push-notification toggle, matching tests, `tasks/038-settings-screen-reactive-auth-retry-push-toggle.md`. +- Feature implemented: (A) an opt-in (default off) reactive fallback — a live 401 despite proactive refresh having judged the token fine now optionally retries once with a forcibly-refreshed token before falling back to the existing re-login flow; (B) TASK-036's background push checker can now be turned off entirely from a new Settings screen, closing that task's own documented limitation. +- How it was implemented: `ReactiveTokenProvider` is a separate interface from the existing `fun interface BearerTokenProvider` (not an addition to it), specifically so every existing test fake written as a plain `BearerTokenProvider { "token" }` lambda keeps compiling — only `RefreshingTokenProvider` also implements it, and a new shared `callAuthenticated` helper checks `tokenProvider as? ReactiveTokenProvider` before relying on it, falling back to the old single-attempt behavior otherwise. All four API classes' near-identical `authenticated`/`get` wrappers now delegate to this one shared helper instead of duplicating the retry orchestration four times. `forceRefresh()` always attempts a refresh regardless of stated expiry (unlike the existing proactive `accessToken()`, which would just hand back the same still-time-valid-looking token again), reusing the exact same mutex and refresh-or-fall-back logic (factored into a shared private `refreshOrFallBack`) so a reactive retry racing an in-flight proactive refresh still shares one attempt at the single-use rotating refresh token. Every new constructor parameter defaults to preserving old behavior exactly, so no existing call site or test needed to change. +- Files changed: see `tasks/038-settings-screen-reactive-auth-retry-push-toggle.md`'s Scope/Evidence sections for the full list. +- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug` BUILD SUCCESSFUL, every pre-existing test across all four API classes and `RefreshingTokenProvider` passing unmodified, plus new coverage (`forceRefresh()`'s behavior, `callAuthenticated`'s retry decision matrix, a `Route.Settings` round-trip), 0 lint errors. Installed on the human product owner's device over wireless `adb`. +- Live verification: opened Settings from the new button; both switches visibly toggled (screenshot-confirmed each state change, including exact tap-coordinate discovery via `uiautomator dump` after an initial miss); backed out and re-entered Settings, confirming both toggles persisted via `SharedPreferences`; restored both to their defaults afterward; `logcat` clean of any crash throughout. +- Review findings and resolutions: None from a separate review pass. +- Known limitations: reactive auth retry's actual live effect (recovering from a genuine 401 without forcing re-login) hasn't been observed end-to-end, since that requires a rare, hard-to-trigger real revocation event — covered instead by direct unit tests of the retry orchestration. Favorite push notifications remain one app-wide toggle, not per-repository (deliberate scope decision, not an oversight). +- Next recommended task: this closes the two explicitly-requested items from this "continue developing" stretch. Remaining work across the whole session is largely live-confirmation of already-built features that haven't been directly exercised yet (pinch-zoom/CSV/PDF viewers, Issues/PR numbered pagination with real data, the actual favorite-push notification firing end-to-end). +- Next owner: Claude, continuing per the standing direction. diff --git a/tasks/038-settings-screen-reactive-auth-retry-push-toggle.md b/tasks/038-settings-screen-reactive-auth-retry-push-toggle.md new file mode 100644 index 0000000..364dff8 --- /dev/null +++ b/tasks/038-settings-screen-reactive-auth-retry-push-toggle.md @@ -0,0 +1,159 @@ +# TASK-038: Settings screen with an opt-in auth-retry toggle and a favorite-push on/off toggle + +Status: implemented; all required checks pass; live-confirmed on the human product owner's device +Owner: Claude +Reviewer: Codex + +## Outcome + +Two features the human product owner explicitly asked for together, both gated behind a new +Settings screen so neither one silently changes existing behavior for everyone: + +- **A — reactive auth retry (opt-in, default off)**: when a live 401 happens despite proactive + refresh already having judged the token not yet due (e.g. a server-side revocation, not ordinary + expiry), the app now optionally retries once with a forcibly-refreshed token instead of going + straight to the re-login screen. Off by default; the human product owner explicitly asked for + this to be something "설정화면에서 설정 해두면 동작하고 안해두면 동작 안하게." +- **B — favorite push notifications (on/off, default on)**: TASK-036's background push checker can + now be turned off entirely from Settings, closing that task's own "no in-app way to disable this" + known limitation. + +## Scope + +- Included: new `core/settings/` package (`AppSettingsStore` + `SharedPreferencesAppSettingsStore`, + `AppSettingsSession`), new `feature/settings/` package (`SettingsViewModel`, `SettingsScreen`), + `Route.Settings` + `RouteStackSaver` entry, a "설정" button on the repository list's top bar, + `FavoritePushCheckWorker` gated on B, a new `ReactiveTokenProvider` interface + `RefreshingTokenProvider.forceRefresh()` + + shared `callAuthenticated` retry helper wired into all four Gitea API classes for A. +- Excluded: No per-repository granularity for B (a single app-wide toggle, not one per favorited + repository — see Design notes). No UI indication of *why* a request succeeded on retry (it's + silent by design — the whole point is the human product owner never sees anything when it works). + No change to the *proactive* refresh TASK-029 already does unconditionally — A only adds a + reactive fallback on top of it, opt-in. + +## Design notes + +- **`ReactiveTokenProvider` is a separate interface from `BearerTokenProvider`, not an addition to + it**: `BearerTokenProvider` is a `fun interface` (exactly one abstract method) specifically so + every existing test fake written as a plain `BearerTokenProvider { "token" }` lambda keeps + compiling — adding a second method to that interface would have broken every one of those across + the whole test suite. `ReactiveTokenProvider : BearerTokenProvider` adds the one extra capability + (`forceRefresh()`) reactive retry needs; only `RefreshingTokenProvider` (the one production + implementation) also implements it, and the shared retry helper checks + `tokenProvider as? ReactiveTokenProvider` before relying on it, falling back to the old + single-attempt behavior (a plain test fake, or the setting disabled) otherwise. +- **One shared `callAuthenticated` helper, not four separate copies of the same retry logic**: all + four Gitea API classes (`GiteaRepositoryBrowserApi`, `GiteaHistoryApi`, `GiteaWorkItemsApi`, + `GiteaNotificationsApi`) had near-identical `authenticated`/`get` wrappers already; each now + delegates the "get a token, call, retry once reactively if warranted" orchestration to one + function (`data/repositories/GiteaRepositoryBrowserApi.kt`, alongside `BearerTokenProvider` and + `ReactiveTokenProvider`, which already lived there and are already imported by the other three + API classes — not a new architectural pattern, just one more shared thing in that same spot). +- **`forceRefresh()` always attempts a refresh, unlike `accessToken()`'s proactive check**: the + whole reason the reactive path exists is that a 401 already happened despite the token looking + fine by the clock — calling `accessToken()` again reactively would just return the same + still-time-valid-looking token and retry identically. `forceRefresh()` skips that time check + entirely, but reuses the exact same mutex and refresh-or-fall-back-to-stale logic `accessToken()` + already had (factored into a shared private `refreshOrFallBack`), so a reactive retry racing an + in-flight proactive refresh (or another reactive retry) still shares one attempt at the single-use + rotating refresh token rather than each independently trying to consume it. +- **Every new constructor parameter defaults to preserving old behavior exactly**: `reactiveAuthRetryEnabled: () -> Boolean = { false }` + on all four API classes means every existing test/call site that doesn't pass it keeps compiling + and behaving identically; production wiring (each feature's `ViewModel.Factory`) passes + `{ AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }` instead. A lambda, not + the whole `AppSettingsStore` interface, so a test can flip retry on/off with a plain closure + without needing a fake store. +- **B is one app-wide toggle, not per-repository**: a per-repository toggle would need its own list + UI (every favorited repository, each with its own switch) and its own storage shape — real, + separable scope beyond what was asked for in this combined request. `FavoritePushCheckWorker` + checks the single toggle first, before touching the network at all, so turning it off also stops + the periodic check from doing any work, not just from posting. +- **Settings has no loading state**: `SettingsViewModel` reads `AppSettingsStore`'s synchronous + `SharedPreferences`-backed values directly at construction — unlike every other ViewModel in this + app, there's no network round trip to wait on, so the screen renders immediately. + +## Acceptance criteria + +- [x] A new "설정" button opens a Settings screen from the repository list's top bar. +- [x] Reactive auth retry defaults off; toggling it on/off persists across navigation and app + restarts (`SharedPreferences`). +- [x] Favorite push notifications defaults on (matching TASK-036's prior unconditional behavior); + toggling it off makes `FavoritePushCheckWorker` a no-op before any network call. +- [x] With reactive auth retry enabled and the token provider supporting it, a 401 triggers exactly + one retry with a forcibly-refreshed token; the original failure propagates if that retry also + fails, if the setting is off, or if the token provider doesn't support it. +- [x] Every existing API-class test and call site keeps compiling and passing unmodified (all new + constructor parameters are defaulted). +- [x] Build, unit tests, and lint pass. +- [x] Live-confirmed on the human product owner's device: the Settings screen opens, both switches + visibly toggle and correctly persist across back-navigation and re-entry, no crash. + +## Security and privacy + +- `forceRefresh()` performs the exact same refresh-token exchange TASK-029's proactive path already + does (same gateway call, same Keystore-backed `TokenVault`, same mutex) — this task adds a second + *trigger* for that existing, already-reviewed flow, not a new credential-handling code path. +- No new data is stored beyond two booleans in a new `SharedPreferences` file + (`ubgitea_app_settings`) — no tokens, no personal data. +- Reactive retry is opt-in and defaults off specifically because it changes how *every* API call in + the app behaves on a 401 — a security-adjacent code path was judged worth requiring an explicit, + visible choice rather than silently changing default behavior for everyone. + +## Required checks + +```text +./gradlew compileDebugKotlin compileDebugUnitTestKotlin +./gradlew testDebugUnitTest +./gradlew assembleDebug lintDebug +``` + +## Evidence + +- `core/settings/AppSettingsStore.kt` (+ `SharedPreferencesAppSettingsStore`), `AppSettingsSession.kt`: + the two-toggle store and its shared-singleton accessor, mirroring `AuthSession`'s own pattern. +- `data/repositories/GiteaRepositoryBrowserApi.kt`: new `ReactiveTokenProvider` interface, new + shared `internal suspend fun callAuthenticated(...)`. +- `core/auth/android/RefreshingTokenProvider.kt`: implements `ReactiveTokenProvider`; new + `forceRefresh()`; `accessToken()`'s refresh/fallback logic factored into a shared private + `refreshOrFallBack` both methods now call. +- `GiteaRepositoryBrowserApi.kt`/`GiteaHistoryApi.kt`/`GiteaWorkItemsApi.kt`/`GiteaNotificationsApi.kt`: + each gained a defaulted `reactiveAuthRetryEnabled: () -> Boolean = { false }` constructor + parameter; each class's `authenticated`/`get` now delegates to `callAuthenticated`. +- `RepositoryListViewModel.kt`/`HistoryViewModel.kt`/`WorkItemsViewModel.kt`/`NotificationsViewModel.kt`/`CodeBrowserViewModel.kt` + (all five `Factory`s that construct one of the four API classes): now pass + `reactiveAuthRetryEnabled = { AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }`. +- `core/notifications/FavoritePushCheckWorker.kt`: checks + `AppSettingsSession.store(application).isFavoritePushNotificationsEnabled()` first, before the + session check, before any network call. +- `feature/settings/SettingsViewModel.kt`, `SettingsScreen.kt`: the new screen and its ViewModel. +- `ui/navigation/Route.kt`, `RouteStackSaver.kt`: new `Route.Settings` + its encode/decode case. +- `feature/repositories/RepositoryListScreen.kt`, `MainActivity.kt`: the "설정" button and its + wiring into the route stack. +- New tests: `RefreshingTokenProviderTest.kt` gained `forceRefresh()` coverage (refreshes regardless + of stated expiry, falls back to stale on no-refresh-token/failed-refresh, empty when nothing + stored); new `CallAuthenticatedTest.kt` (disabled setting doesn't retry, an unsupporting token + provider doesn't retry, a successful retry uses the refreshed token, a failed retry propagates the + original failure, a blank forced-refresh result doesn't retry, a non-auth failure is never + retried); `RouteEncodingTest.kt` gained a `Route.Settings` round-trip case. +- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL. +- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (every pre-existing test + across all four API classes and `RefreshingTokenProvider` unmodified and still green). +- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors. +- Live device: installed over wireless `adb`. Opened Settings from the new button; both switches + visibly toggled (screenshot-confirmed each state change); backed out and re-entered Settings — + both toggles still reflected exactly what was set, confirming `SharedPreferences` persistence; + restored both to their defaults afterward; `logcat` clean of `FATAL EXCEPTION`/`AndroidRuntime` + throughout. + +## Known limitations + +- Reactive auth retry's actual effect (recovering from a live 401 without forcing re-login) hasn't + been observed end-to-end live — that requires a genuine 401 to occur with the setting on, which + is by nature a rare, hard-to-trigger event; covered instead by direct unit tests of the retry + orchestration (`CallAuthenticatedTest`) and of `forceRefresh()` itself. +- B remains one app-wide toggle, not per-repository (see Design notes) — deliberate, not an + oversight. + +## Review findings + +None yet — pending Codex/human review.