Lukasz Kalnik
08/23/2022, 1:56 PMsynchronized(lock)
doesn't seem to prevent the second call to try to refresh the token as well.
val okHttpClient = OkHttpClient.Builder()
.addInterceptor(AccessTokenInterceptor(oAuthRepository))
.authenticator(RefreshTokenAuthenticator(oAuthRepository))
.build()
val lock = Any()
private class AccessTokenInterceptor(
private val oAuthRepository: OAuthRepository
) : Interceptor {
override fun intercept(chain: Chain): Response {
synchronized(lock) {
val token = oAuthRepository.accessToken
val request = chain.request().newBuilder().apply {
if (token != null) header("Authorization", "Bearer $token")
}.build()
return chain.proceed(request)
}
}
}
private class RefreshTokenAuthenticator(
private val oAuthRepository: OAuthRepository
) : Authenticator {
override fun authenticate(route: Route?, response: Response): Request? {
if (response.responseCount >= 2) return null
synchronized(lock) {
return runBlocking {
oAuthRepository.refreshTokens().map {
oAuthRepository.accessToken
}
}.fold(
ifLeft = { null },
ifRight = {
it?.let { token ->
response.request.newBuilder()
.header("Authorization", "Bearer $token")
.build()
}
}
)
}
}
}
yschimke
08/23/2022, 4:17 PMLukasz Kalnik
08/23/2022, 5:57 PMsynchronized
block for the second (and subsequent) calls is not skipped - it's just delayed. That means every next call tries (unnecessarily) to refresh the token (which has just been refreshed). So after entering the synchronized block I should check if the token has changed, and in case it's the new token, skip the refresh and just use it to authenticate.
This means I have to add:
override fun authenticate(route: Route?, response: Response): Request? {
val oldToken = oAuthRepository.accessToken
synchronized(lock) {
val newToken = oAuthRepository.accessToken
// if the newToken is different, it means that it has been refreshed in the mean time by another call
// so we can just use it to authenticate and skip the refresh
if (newToken != oldToken) return response.request.newBuilder()
.header("Authorization", "Bearer $newToken")
.build()
// Otherwise this is the first call trying to refresh a token - so just refresh it
// Previous refresh flow
}
}
jessewilson
08/23/2022, 10:27 PMreturn chain.proceed(request)
unless you want to avoid all paralellismLukasz Kalnik
08/24/2022, 7:30 AMAccessTokenInterceptor
was actually unnecessary/harmful.Colton Idle
08/30/2022, 7:49 AMLukasz Kalnik
08/30/2022, 8:30 AMsynchronized
block. If it changed, that means it was already refreshed in another thread and I just skip the refresh and use the new token.