TASK-038: Settings screen with opt-in reactive auth retry and favorite-push toggle
Two features gated behind a new Settings screen so neither silently changes existing behavior: - Reactive auth retry (opt-in, default off): a live 401 despite proactive refresh already judging the token fine now optionally retries once with a forcibly-refreshed token before falling back to re-login. ReactiveTokenProvider is a separate interface from the existing fun interface BearerTokenProvider (not an addition to it), so every existing test fake keeps compiling; a new shared callAuthenticated helper (used by all four Gitea API classes instead of four duplicated retry implementations) checks for it and falls back to the old single-attempt behavior otherwise. - Favorite push notifications (on/off, default on): TASK-036's background checker can now be turned off entirely, closing that task's own 'no in-app way to disable this' limitation. Every new constructor parameter defaults to preserving old behavior exactly, so no existing call site or test needed to change. Live-verified: Settings screen opens from a new button, both switches toggle and persist across navigation, no crash. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
75f887c497
commit
9a8f855e5e
@ -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>(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) {
|
||||
|
||||
@ -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,7 +37,23 @@ class RefreshingTokenProvider(
|
||||
val needsRefresh = expiresAt != null && expiresAt - REFRESH_SKEW_MILLIS <= now()
|
||||
if (!needsRefresh || refreshToken == null) return@withLock tokens.accessToken
|
||||
|
||||
try {
|
||||
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
|
||||
@ -50,8 +66,7 @@ class RefreshingTokenProvider(
|
||||
// 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
|
||||
}
|
||||
staleAccessToken
|
||||
}
|
||||
|
||||
private companion object {
|
||||
|
||||
@ -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()
|
||||
|
||||
|
||||
@ -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 }
|
||||
}
|
||||
}
|
||||
@ -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"
|
||||
}
|
||||
}
|
||||
@ -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,10 +99,9 @@ class GiteaHistoryApi(
|
||||
throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private suspend fun get(url: HttpUrl): HttpResponse {
|
||||
val token = tokenProvider.accessToken()
|
||||
if (token.isBlank()) throw ApiFailure.AuthenticationExpired()
|
||||
return try {
|
||||
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)
|
||||
|
||||
@ -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<NotificationSummary> {
|
||||
validatePage(page, pageSize)
|
||||
@ -90,10 +93,9 @@ class GiteaNotificationsApi(
|
||||
throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private suspend fun get(url: HttpUrl): HttpResponse {
|
||||
val token = tokenProvider.accessToken()
|
||||
if (token.isBlank()) throw ApiFailure.AuthenticationExpired()
|
||||
return try {
|
||||
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)
|
||||
|
||||
@ -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 <T> 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<RepositorySummary> {
|
||||
validatePage(page, pageSize)
|
||||
@ -214,10 +261,9 @@ 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 {
|
||||
private suspend fun authenticated(call: suspend (String) -> HttpResponse): HttpResponse =
|
||||
callAuthenticated(tokenProvider, reactiveAuthRetryEnabled) { token ->
|
||||
try {
|
||||
call(token).requireSuccess()
|
||||
} catch (failure: IOException) {
|
||||
throw ApiFailure.Network(failure)
|
||||
|
||||
@ -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<IssueSummary> {
|
||||
validatePage(page, pageSize)
|
||||
@ -121,10 +124,9 @@ 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 {
|
||||
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)
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -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 <T : ViewModel> create(modelClass: Class<T>): 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
|
||||
}
|
||||
}
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -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))
|
||||
}
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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)
|
||||
}
|
||||
}
|
||||
@ -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<SettingsUiState> = 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 <T : ViewModel> create(modelClass: Class<T>): T {
|
||||
require(modelClass.isAssignableFrom(SettingsViewModel::class.java))
|
||||
return SettingsViewModel(AppSettingsSession.store(application)) as T
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -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 <T : ViewModel> create(modelClass: Class<T>): 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
|
||||
}
|
||||
}
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -14,6 +14,7 @@
|
||||
<string name="repositories_server_error">Gitea 서버에서 오류가 발생했습니다.</string>
|
||||
<string name="repositories_invalid_callback">로그인 응답을 확인할 수 없습니다. 다시 시도해 주세요.</string>
|
||||
<string name="repositories_retry">다시 시도</string>
|
||||
<string name="repositories_settings">설정</string>
|
||||
<string name="repositories_logout">로그아웃</string>
|
||||
<string name="repositories_private">비공개</string>
|
||||
<string name="repositories_public">공개</string>
|
||||
|
||||
7
app/src/main/res/values/strings_settings.xml
Normal file
7
app/src/main/res/values/strings_settings.xml
Normal file
@ -0,0 +1,7 @@
|
||||
<resources>
|
||||
<string name="settings_title">설정</string>
|
||||
<string name="settings_reactive_auth_retry_title">인증 오류 시 자동 복구</string>
|
||||
<string name="settings_reactive_auth_retry_description">인증이 예기치 않게 만료되면, 재로그인 화면을 띄우기 전에 한 번 더 조용히 갱신을 시도합니다.</string>
|
||||
<string name="settings_favorite_push_notifications_title">즐겨찾기 저장소 Push 알림</string>
|
||||
<string name="settings_favorite_push_notifications_description">즐겨찾기한 저장소에 새로운 push가 있으면 알려드립니다.</string>
|
||||
</resources>
|
||||
@ -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 }
|
||||
|
||||
@ -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<String>()
|
||||
|
||||
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 <T> runSuspend(block: suspend () -> T): T {
|
||||
var outcome: Result<T>? = null
|
||||
block.startCoroutine(
|
||||
object : Continuation<T> {
|
||||
override val context = EmptyCoroutineContext
|
||||
|
||||
override fun resumeWith(result: Result<T>) {
|
||||
outcome = result
|
||||
}
|
||||
},
|
||||
)
|
||||
return checkNotNull(outcome).getOrThrow()
|
||||
}
|
||||
@ -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"))
|
||||
|
||||
@ -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.
|
||||
|
||||
159
tasks/038-settings-screen-reactive-auth-retry-push-toggle.md
Normal file
159
tasks/038-settings-screen-reactive-auth-retry-push-toggle.md
Normal file
@ -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.
|
||||
Loading…
x
Reference in New Issue
Block a user