try facing the locking issue (#76)

andretietz · Sep 12, 2018 · 25e8e29 · 25e8e29
1 parent ea84cf9
commit 25e8e29
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 48 deletions.
diff --git a/demoandroid/src/main/java/com/andretietz/retroauth/demo/MainActivity.kt b/demoandroid/src/main/java/com/andretietz/retroauth/demo/MainActivity.kt
@@ -84,11 +84,9 @@ class MainActivity : AppCompatActivity() {
 
     buttonLogout.setOnClickListener {
       ownerManager.getActiveOwner(provider.ownerType)?.let { account ->
-        tokenStorage.getToken(account, provider.tokenType, object : Callback<AndroidToken> {
-          override fun onResult(result: AndroidToken?) {
-            result?.let {
-              tokenStorage.removeToken(account, provider.tokenType, it)
-            }
+        ownerManager.removeOwner(account, object : Callback<Boolean> {
+          override fun onResult(result: Boolean?) {
+            show("Logged out!")
           }
         })
       }

diff --git a/quality/detekt.yml b/quality/detekt.yml
@@ -11,6 +11,8 @@ complexity:
     active: false
   MethodOverloading:
     active: false
+  ComplexMethod:
+    active: false
 
 exceptions:
   TooGenericExceptionCaught:

diff --git a/retroauth-android/src/main/java/com/andretietz/retroauth/AndroidTokenStorage.kt b/retroauth-android/src/main/java/com/andretietz/retroauth/AndroidTokenStorage.kt
@@ -86,34 +86,26 @@ class AndroidTokenStorage constructor(
     private val activityManager = ActivityManager[application]
 
     override fun call(): AndroidToken {
-      var token: String?
       val future = accountManager.getAuthToken(
         owner,
         type.tokenType,
         null,
         activityManager.activity,
         null,
         null)
-      val result = future.result
-      token = result.getString(AccountManager.KEY_AUTHTOKEN)
-      if (token == null) {
-        token = accountManager.peekAuthToken(owner, type.tokenType)
+      return (future.result.getString(AccountManager.KEY_AUTHTOKEN)
+        ?: accountManager.peekAuthToken(owner, type.tokenType)
+        ?: throw AuthenticationCanceledException()).let { token ->
+        AndroidToken(
+          token,
+          type.dataKeys
+            ?.associateTo(HashMap()) {
+              it to accountManager.getUserData(owner, createDataKey(type, it))
+            }
+        ).apply {
+          callback?.onResult(this)
+        }
       }
-      if (token == null) {
-        throw IllegalStateException(
-          String.format("No token found! Make sure you store the token during login using %s#storeToken()",
-            AuthenticationActivity::class.java.simpleName)
-        )
-      }
-      val androidToken = AndroidToken(
-        token,
-        type.dataKeys
-          ?.associateTo(HashMap()) {
-            it to accountManager.getUserData(owner, createDataKey(type, it))
-          }
-      )
-      callback?.onResult(androidToken)
-      return androidToken
     }
   }
 }
diff --git a/retroauth/src/main/java/com/andretietz/retroauth/AuthenticationRequiredException.kt b/retroauth/src/main/java/com/andretietz/retroauth/AuthenticationRequiredException.kt
@@ -0,0 +1,5 @@
+package com.andretietz.retroauth
+
+import java.io.IOException
+
+class AuthenticationRequiredException : IOException()
diff --git a/retroauth/src/main/java/com/andretietz/retroauth/CredentialInterceptor.kt b/retroauth/src/main/java/com/andretietz/retroauth/CredentialInterceptor.kt
@@ -54,7 +54,7 @@ internal class CredentialInterceptor<out OWNER_TYPE : Any, OWNER : Any, TOKEN_TY
     var pending = false
     var refreshRequested = false
     var token: TOKEN
-    var owner: OWNER
+    var owner: OWNER?
     var tryCount = 0
     try {
       do {
@@ -64,28 +64,32 @@ internal class CredentialInterceptor<out OWNER_TYPE : Any, OWNER : Any, TOKEN_TY
 
           owner = ownerManager.getActiveOwner(authRequestType.ownerType)
             ?: ownerManager.openOwnerPicker(authRequestType.ownerType).get()
-            ?: ownerManager.createOwner(authRequestType.ownerType, authRequestType.tokenType).get()
-          // get the token of the owner
-          val localToken = tokenStorage.getToken(owner, authRequestType.tokenType).get()
-          // if the token is still valid and no refresh has been requested
-          if (authenticator.isTokenValid(localToken) && !refreshRequested) {
-            token = localToken
-          } else {
-            // otherwise remove the current token from the storage
-            tokenStorage.removeToken(owner, authRequestType.tokenType, localToken)
-            // try to refresh the token
-            val refreshedToken = authenticator.refreshToken(owner, authRequestType.tokenType, localToken)
-            if (refreshedToken != null) {
-              // if the token was refreshed, store it
-              tokenStorage.storeToken(owner, authRequestType.tokenType, refreshedToken)
-              token = refreshedToken
-            } else {
-              // otherwise use the "old" token
+          if (owner != null) {
+            // get the token of the owner
+            val localToken = tokenStorage.getToken(owner, authRequestType.tokenType).get()
+            // if the token is still valid and no refresh has been requested
+            if (authenticator.isTokenValid(localToken) && !refreshRequested) {
               token = localToken
+            } else {
+              // otherwise remove the current token from the storage
+              tokenStorage.removeToken(owner, authRequestType.tokenType, localToken)
+              // try to refresh the token
+              val refreshedToken = authenticator.refreshToken(owner, authRequestType.tokenType, localToken)
+              if (refreshedToken != null) {
+                // if the token was refreshed, store it
+                tokenStorage.storeToken(owner, authRequestType.tokenType, refreshedToken)
+                token = refreshedToken
+              } else {
+                // otherwise use the "old" token
+                token = localToken
+              }
             }
+            // authenticate the request using the token
+            request = authenticator.authenticateRequest(request, token)
+          } else {
+            ownerManager.createOwner(authRequestType.ownerType, authRequestType.tokenType)
+            throw AuthenticationRequiredException()
           }
-          // authenticate the request using the token
-          request = authenticator.authenticateRequest(request, token)
         } finally {
           // release type lock
           unlock(authRequestType, pending)
@@ -101,10 +105,22 @@ internal class CredentialInterceptor<out OWNER_TYPE : Any, OWNER : Any, TOKEN_TY
   }
 
   private fun storeAndThrowError(type: RequestType<OWNER_TYPE, TOKEN_TYPE>, exception: Exception) {
+    val unwrappedException = unwrapThrowable(exception)
     if (getLock(type).errorContainer.get() == null) {
-      getLock(type).errorContainer.set(exception)
+      getLock(type).errorContainer.set(unwrappedException)
+    }
+    throw unwrappedException
+  }
+
+  private fun unwrapThrowable(throwable: Throwable): Throwable {
+    if (
+      throwable is AuthenticationCanceledException ||
+        throwable is AuthenticationRequiredException
+    ) return throwable
+    throwable.cause?.let {
+      return unwrapThrowable(it)
     }
-    throw exception
+    return throwable
   }
 
   private fun getLock(type: RequestType<OWNER_TYPE, TOKEN_TYPE>): AccountTokenLock {
@@ -139,7 +155,7 @@ internal class CredentialInterceptor<out OWNER_TYPE : Any, OWNER : Any, TOKEN_TY
 
   internal data class AccountTokenLock(
     val lock: Lock = ReentrantLock(true),
-    val errorContainer: AtomicReference<Exception> = AtomicReference(),
+    val errorContainer: AtomicReference<Throwable> = AtomicReference(),
     val waitCounter: AtomicInteger = AtomicInteger()
   )
 }