#7: Add more tests to OAuth component

app/build.gradle.kts

@@ -9,6 +9,7 @@ plugins {
   id("org.jlleitschuh.gradle.ktlint") version "8.1.0"
   id("com.gladed.androidgitversion") version "0.4.14"
   id("com.github.triplet.play") version "2.4.2"
+  id("de.mobilej.unmock")
   jacoco
 }
 
@@ -19,6 +20,12 @@ val props = Properties().apply {
   }
 }
 
+unMock {
+  keep = listOf("android.net.Uri")
+  //keepStartingWith("org.")
+  //keepStartingWith("libcore.")
+}
+
 jacoco {
   toolVersion = "0.8.7"
 }

app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt

@@ -33,15 +33,15 @@ interface OAuth {
 
   fun exchange(context: Activity, authorization: Intent, success: () -> Unit, error: () -> Unit)
 
-  fun init(hostname: String)
+  fun init(hostname: String): AuthState
 
-  fun register(callback: () -> Unit)
+  fun register(authState: AuthState? = null, callback: () -> Unit)
 
   fun authorize(context: Activity)
 
   fun isAuthorized(context: Context): Boolean
 
-  fun tryRefreshAccessToken(context: Context, overrideNeedsTokenRefresh: Boolean = false): Boolean
+  fun tryRefreshAccessToken(context: Context): Boolean
 
   fun tryState(): AuthState?
 
@@ -55,13 +55,20 @@ object OAuthFactory {
   private val oAuth: OAuth
 
   init {
-    oAuth = DefaultOAuth()
+    oAuth = DefaultOAuth(AuthorizationServiceFactory())
   }
 
   fun instance() = oAuth
 }
 
-class DefaultOAuth : OAuth {
+class AuthorizationServiceFactory {
+
+  fun create(context: Context): AuthorizationService {
+    return AuthorizationService(context)
+  }
+}
+
+class DefaultOAuth(private val authorizationServiceFactory: AuthorizationServiceFactory) : OAuth {
 
   companion object {
 
@@ -84,12 +91,13 @@ class DefaultOAuth : OAuth {
     }
   }
 
-  override fun state(): AuthState = tryState()!!
+  override fun state(): AuthState =
+    tryState() ?: throw IllegalStateException("Couldn't find saved state")
 
   override fun isAuthorized(context: Context): Boolean {
     val state = tryState()
     return if (state != null) {
-      state.isAuthorized || tryRefreshAccessToken(context)
+      state.isAuthorized || doTryRefreshAccessToken(state, context)
     } else {
       false
     }.also {
@@ -97,41 +105,53 @@ class DefaultOAuth : OAuth {
     }
   }
 
-  override fun tryRefreshAccessToken(
+  override fun tryRefreshAccessToken(context: Context): Boolean {
-    context: Context,
-    overrideNeedsTokenRefresh: Boolean
-  ): Boolean {
     tryState()?.let { state ->
-      val shouldRefreshAccessToken = overrideNeedsTokenRefresh || state.needsTokenRefresh
+      return doTryRefreshAccessToken(state, context)
-      if (shouldRefreshAccessToken && state.refreshToken != null) {
+    }
-        val refreshRequest = state.createTokenRefreshRequest()
+    return false
-        val auth = ClientSecretPost(state.clientSecret)
+  }
-        runBlocking {
+
-          service(context).performTokenRequest(refreshRequest, auth) { response, e ->
+  private fun doTryRefreshAccessToken(
-            state.apply {
+    state: AuthState,
-              update(response, e)
+    context: Context
-              save()
+  ): Boolean {
-            }
+    if (state.needsTokenRefresh && state.refreshToken != null) {
+      val refreshRequest = state.createTokenRefreshRequest()
+      val auth = ClientSecretPost(state.clientSecret)
+      runBlocking {
+        service(context).performTokenRequest(refreshRequest, auth) { response, e ->
+          state.apply {
+            update(response, e)
+            save()
           }
         }
       }
     }
-
+    return (state.isAuthorized)
-    return (tryState()?.isAuthorized ?: false)
       .also {
         it.log("tryRefreshAccessToken()")
       }
   }
 
-  override fun init(hostname: String) {
+  override fun init(hostname: String): AuthState {
-    AuthState(config(hostname)).save()
+    return AuthState(
+      AuthorizationServiceConfiguration(
+        Uri.parse("$hostname/authorize"),
+        Uri.parse("$hostname/api/v1/oauth/token/"),
+        Uri.parse("$hostname/api/v1/oauth/apps/")
+      )
+    )
+      .also {
+        it.save()
+      }
   }
 
-  override fun service(context: Context): AuthorizationService = AuthorizationService(context)
+  override fun service(context: Context): AuthorizationService =
-
+    authorizationServiceFactory.create(context)
-  override fun register(callback: () -> Unit) {
-    state().authorizationServiceConfiguration?.let { config ->
 
+  override fun register(authState: AuthState?, callback: () -> Unit) {
+    (authState ?: state()).authorizationServiceConfiguration?.let { config ->
       runBlocking {
         val (_, _, result) = Fuel.post(config.registrationEndpoint.toString())
           .header("Content-Type", "application/json")
@@ -214,12 +234,6 @@ class DefaultOAuth : OAuth {
     }
   }
 
-  private fun config(hostname: String) = AuthorizationServiceConfiguration(
-    Uri.parse("$hostname/authorize"),
-    Uri.parse("$hostname/api/v1/oauth/token/"),
-    Uri.parse("$hostname/api/v1/oauth/apps/")
-  )
-
   private fun registration() =
     state().authorizationServiceConfiguration?.let { config ->
       RegistrationRequest.Builder(config, listOf(REDIRECT_URI)).build()
@@ -238,3 +252,5 @@ class DefaultOAuth : OAuth {
     }
   }
 }
+
+

app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt

@@ -3,21 +3,20 @@ package audio.funkwhale.ffa.playback
 import android.content.Context
 import android.net.Uri
 import audio.funkwhale.ffa.utils.OAuth
-import audio.funkwhale.util.MockKJUnitRunner
 import com.google.android.exoplayer2.upstream.DataSpec
 import com.google.android.exoplayer2.upstream.HttpDataSource
 import com.google.android.exoplayer2.upstream.TransferListener
+import io.mockk.MockKAnnotations
 import io.mockk.every
 import io.mockk.impl.annotations.InjectMockKs
 import io.mockk.impl.annotations.MockK
 import io.mockk.mockk
 import io.mockk.verify
+import org.junit.Before
 import org.junit.Test
-import org.junit.runner.RunWith
 import strikt.api.expectThat
 import strikt.assertions.isEqualTo
 
-@RunWith(MockKJUnitRunner::class)
 class OAuthDatasourceTest {
 
   @InjectMockKs
@@ -34,10 +33,15 @@ class OAuthDatasourceTest {
 
   private var dataSpec: DataSpec = DataSpec(Uri.EMPTY)
 
+  @Before
+  fun setup(){
+    MockKAnnotations.init(this, relaxUnitFun = true)
+  }
+
   @Test
   fun `open() should set accessToken and delegate to http dataSource`() {
     every { http.open(any()) } returns 0
-    every { oAuth.tryRefreshAccessToken(any(), any()) } returns true
+    every { oAuth.tryRefreshAccessToken(any()) } returns true
     every { oAuth.state().accessToken } returns "accessToken"
 
     datasource.open(dataSpec)

app/src/test/java/audio/funkwhale/ffa/utils/DefaultOAuthTest.kt

@@ -0,0 +1,270 @@
+package audio.funkwhale.ffa.utils
+
+import android.content.Context
+import com.github.kittinunf.fuel.core.Client
+import com.github.kittinunf.fuel.core.FuelManager
+import com.github.kittinunf.fuel.core.Request
+import com.google.gson.Gson
+import com.google.gson.reflect.TypeToken
+import com.preference.PowerPreference
+import com.preference.Preference
+import io.mockk.MockKAnnotations
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.impl.annotations.InjectMockKs
+import io.mockk.impl.annotations.MockK
+import io.mockk.mockk
+import io.mockk.mockkStatic
+import io.mockk.slot
+import io.mockk.verify
+import net.openid.appauth.AuthState
+import net.openid.appauth.AuthorizationService
+import net.openid.appauth.AuthorizationServiceConfiguration
+import net.openid.appauth.ClientSecretPost
+import org.junit.Before
+import org.junit.Test
+import strikt.api.expectThat
+import strikt.api.expectThrows
+import strikt.assertions.isEqualTo
+import strikt.assertions.isFalse
+import strikt.assertions.isNotNull
+import strikt.assertions.isNull
+import strikt.assertions.isTrue
+
+
+class DefaultOAuthTest {
+
+  @InjectMockKs
+  private lateinit var oAuth: DefaultOAuth
+
+  @MockK
+  private lateinit var authServiceFactory: AuthorizationServiceFactory
+
+  @MockK
+  private lateinit var authService: AuthorizationService
+
+  @MockK
+  private lateinit var mockPreference: Preference
+
+  @MockK
+  private lateinit var context: Context
+
+  @Before
+  fun setup() {
+    MockKAnnotations.init(this, relaxUnitFun = true)
+  }
+
+  @Test
+  fun `tryState() should return null if saved state is missing`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns null
+    expectThat(oAuth.tryState()).isNull()
+  }
+
+  @Test
+  fun `tryState() should return null if saved state is empty`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns ""
+    expectThat(oAuth.tryState()).isNull()
+  }
+
+  @Test
+  fun `tryState() should return deserialized object if saved state is present`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns "{}"
+    expectThat(oAuth.tryState()).isNotNull()
+  }
+
+  @Test
+  fun `state() should return deserialized object if saved state is present`() {
+
+    mockkStatic(PowerPreference::class)
+    mockkStatic(AuthState::class)
+
+    val authState = AuthState()
+    every { AuthState.jsonDeserialize(any<String>()) } returns authState
+
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns "{}"
+
+    val result = oAuth.state()
+    expectThat(result).isEqualTo(authState)
+  }
+
+  @Test
+  fun `state() should throw error if saved state is missing`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns null
+
+    expectThrows<IllegalStateException> { oAuth.state() }
+  }
+
+  @Test
+  fun `isAuthorized() should return false if no state exists`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns null
+
+    expectThat(oAuth.isAuthorized(context)).isFalse()
+  }
+
+  @Test
+  fun `isAuthorized() should return false if existing state is not authorized and token is not refreshed`() {
+    mockkStatic(PowerPreference::class)
+    mockkStatic(AuthState::class)
+
+    val authState = mockk<AuthState>()
+    every { AuthState.jsonDeserialize(any<String>()) } returns authState
+    every { authState.isAuthorized } returns false
+    every { authState.needsTokenRefresh } returns false
+
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns "{}"
+
+    expectThat(oAuth.isAuthorized(context)).isFalse()
+  }
+
+  @Test
+  fun `isAuthorized() should return true if existing state is authorized`() {
+    mockkStatic(PowerPreference::class)
+    mockkStatic(AuthState::class)
+
+    val authState = mockk<AuthState>()
+    every { AuthState.jsonDeserialize(any<String>()) } returns authState
+    every { authState.isAuthorized } returns true
+
+    val mockPref = mockk<Preference>()
+    every { PowerPreference.getFileByName(any()) } returns mockPref
+    every { mockPref.getString(any()) } returns "{}"
+
+    expectThat(oAuth.isAuthorized(context)).isTrue()
+  }
+
+  @Test
+  fun `tryRefreshAccessToken() should perform token refresh request if accessToken needs refresh and refreshToken exists`() {
+    mockkStatic(PowerPreference::class)
+    mockkStatic(AuthState::class)
+
+    val authState = mockk<AuthState>()
+    every { AuthState.jsonDeserialize(any<String>()) } returns authState
+    every { authState.isAuthorized } returns false
+    every { authState.needsTokenRefresh } returns true
+    every { authState.refreshToken } returns "refreshToken"
+    every { authState.createTokenRefreshRequest() } returns mockk()
+    every { authState.clientSecret } returns "clientSecret"
+    every { authServiceFactory.create(any()) } returns authService
+    every { authService.performTokenRequest(any(), any<ClientSecretPost>(), any()) } returns mockk()
+
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns "{}"
+
+    oAuth.tryRefreshAccessToken(context)
+
+    verify { authService.performTokenRequest(any(), any(), any()) }
+  }
+
+  @Test
+  fun `tryRefreshAccessToken() should not perform token refresh request if accessToken doesn't need refresh`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns "{}"
+
+    mockkStatic(AuthState::class)
+
+    val authState = mockk<AuthState>()
+    every { AuthState.jsonDeserialize(any<String>()) } returns authState
+    every { authState.isAuthorized } returns false
+    every { authState.needsTokenRefresh } returns false
+
+    oAuth.tryRefreshAccessToken(context)
+
+    verify(exactly = 0) { authService.performTokenRequest(any(), any(), any()) }
+  }
+
+  @Test
+  fun `init() should setup correct endpoints`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.setString(any(), any()) } returns true
+
+    val result = oAuth.init("hostname")
+
+    expectThat(result.authorizationServiceConfiguration?.authorizationEndpoint.toString())
+      .isEqualTo("hostname/authorize")
+    expectThat(result.authorizationServiceConfiguration?.tokenEndpoint.toString())
+      .isEqualTo("hostname/api/v1/oauth/token/")
+    expectThat(result.authorizationServiceConfiguration?.registrationEndpoint.toString())
+      .isEqualTo("hostname/api/v1/oauth/apps/")
+  }
+
+  @Test
+  fun `register() should not initiate http request if configuration is missing`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns "{}"
+
+    mockkStatic(AuthState::class)
+    val authState = mockk<AuthState>()
+    every { AuthState.jsonDeserialize(any<String>()) } returns authState
+    every { authState.authorizationServiceConfiguration } returns null
+
+    val mockkClient = mockk<Client>()
+    FuelManager.instance.client = mockkClient
+
+    oAuth.register {}
+
+    verify(exactly = 0) { mockkClient.executeRequest(any()) }
+  }
+
+  @Test
+  fun `register() should initiate correct HTTP request to registration endpoint`() {
+    mockkStatic(PowerPreference::class)
+    every { PowerPreference.getFileByName(any()) } returns mockPreference
+    every { mockPreference.getString(any()) } returns "{}"
+    every { mockPreference.setString(any(), any()) } returns true
+
+    mockkStatic(AuthState::class)
+    val authState = mockk<AuthState>()
+    every { AuthState.jsonDeserialize(any<String>()) } returns authState
+    val mockConfig = mockk<AuthorizationServiceConfiguration>()
+    every { authState.authorizationServiceConfiguration } returns mockConfig
+
+    val mockkClient = mockk<Client>()
+
+    FuelManager.instance.client = mockkClient
+
+    val state = oAuth.init("https://example.com")
+    oAuth.register(state) { }
+
+    val requestSlot = slot<com.github.kittinunf.fuel.core.Request>()
+
+    coVerify { mockkClient.awaitRequest(capture(requestSlot)) }
+
+    val capturedRequest = requestSlot.captured
+    expectThat(capturedRequest.url.toString())
+      .isEqualTo("https://example.com/api/v1/oauth/apps/")
+
+    expectThat(deserializeJson<Map<String, String>>(capturedRequest)).isEqualTo(
+      mapOf(
+        "name" to "Funkwhale for Android (null)",
+        "redirect_uris" to "urn:/audio.funkwhale.funkwhale-android/oauth/callback",
+        "scopes" to "read write"
+      )
+    )
+  }
+
+  private fun <T> deserializeJson(
+    capturedRequest: Request
+  ): T {
+    return Gson().fromJson(
+      capturedRequest.body.asString("application/json"),
+      object : TypeToken<T>() {}.type
+    )
+  }
+
+}
+

app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt

@@ -1,58 +0,0 @@
-package audio.funkwhale.util
-
-import io.mockk.MockKAnnotations
-import io.mockk.clearAllMocks
-import org.junit.Test
-import org.junit.runner.Description
-import org.junit.runner.Runner
-import org.junit.runner.notification.Failure
-import org.junit.runner.notification.RunNotifier
-import java.lang.reflect.Method
-
-class MockKJUnitRunner(private val testClass: Class<*>) : Runner() {
-
-  private val methodDescriptions: MutableMap<Method, Description> = mutableMapOf()
-
-  init {
-    // Build method/descriptions map
-    testClass.methods
-      .map { method ->
-        val annotation: Annotation? = method.getAnnotation(Test::class.java)
-        method to annotation
-      }
-      .filter { (_, annotation) ->
-        annotation != null
-      }
-      .map { (method, annotation) ->
-        val desc = Description.createTestDescription(testClass, method.name, annotation)
-        method to desc
-      }
-      .forEach { (method, desc) -> methodDescriptions[method] = desc }
-  }
-
-  override fun getDescription(): Description {
-    val description = Description.createSuiteDescription(
-      testClass.name, *testClass.annotations
-    )
-    methodDescriptions.values.forEach { description.addChild(it) }
-    return description
-  }
-
-  override fun run(notifier: RunNotifier?) {
-    val testObject = testClass.newInstance()
-    MockKAnnotations.init(testObject, relaxUnitFun = true)
-
-    methodDescriptions
-      .onEach { (_, _) -> clearAllMocks() }
-      .onEach { (_, desc) -> notifier!!.fireTestStarted(desc) }
-      .forEach { (method, desc) ->
-        try {
-          method.invoke(testObject)
-        } catch (e: Throwable) {
-          notifier!!.fireTestFailure(Failure(desc, e.cause))
-        } finally {
-          notifier!!.fireTestFinished(desc)
-        }
-      }
-  }
-}

build.gradle.kts

@@ -1,12 +1,14 @@
 buildscript {
   repositories {
     google()
+    mavenCentral()
     jcenter()
   }
 
   dependencies {
     classpath("com.android.tools.build:gradle:4.2.2")
     classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.5.21")
+    classpath("com.github.bjoernq:unmockplugin:0.7.8")
   }
 }