change android ktor client to okhttp, fix testTokenValidity

adamint · adamint · commit 2d29e600e208 · 2020-08-16T12:17:04.000-04:00
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -208,7 +208,7 @@ kotlin {
                     api("net.sourceforge.streamsupport:android-retrofuture:1.7.2")
                     api("org.jetbrains.kotlinx:kotlinx-coroutines-core:$coroutineVersion")
                     api("org.jetbrains.kotlinx:kotlinx-serialization-runtime:$serializationVersion")
-                    api("io.ktor:ktor-client-cio:$ktorVersion")
+                    api("io.ktor:ktor-client-okhttp:$ktorVersion")
                     api("io.coil-kt:coil:0.11.0")
                     implementation(kotlin("stdlib-jdk8"))
                 }
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/Builder.kt b/src/commonMain/kotlin/com.adamratzman.spotify/Builder.kt
@@ -237,7 +237,6 @@ fun spotifyAppApi(
  */
 fun spotifyAppApi(block: SpotifyAppApiBuilder.() -> Unit) = SpotifyAppApiBuilder().apply(block)
 
-
 // Client Api Builders
 /*
  ____________________________
@@ -468,11 +467,11 @@ fun spotifyClientPkceApi(
  * @return Configurable [SpotifyClientApiBuilder] that, when built, creates a new [SpotifyClientApi]
  */
 fun spotifyClientPkceApi(
-        clientId: String?,
-        redirectUri: String?,
-        authCode: String,
-        codeVerifier: String,
-        options: SpotifyApiOptionsBuilder? = null
+    clientId: String?,
+    redirectUri: String?,
+    authCode: String,
+    codeVerifier: String,
+    options: SpotifyApiOptionsBuilder? = null
 ) = SpotifyClientApiBuilder().apply {
     credentials {
         this.clientId = clientId
@@ -487,7 +486,6 @@ fun spotifyClientPkceApi(
     options?.let { this.options = it.build() }
 }
 
-
 /**
  *  Spotify API builder
  */
@@ -798,7 +796,7 @@ class SpotifyClientApiBuilder(
 
         // either application credentials, or a token is required
         require((clientId != null && clientSecret != null && redirectUri != null) || (clientId != null && redirectUri != null && authorization.pkceCodeVerifier != null) || authorization.token != null || authorization.tokenString != null) { "You need to specify a valid clientId, clientSecret, and redirectUri in the credentials block!" }
-        return when {
+        val api = when {
             authorization.authorizationCode != null && authorization.pkceCodeVerifier == null -> try {
                 require(clientId != null && clientSecret != null && redirectUri != null) { "You need to specify a valid clientId, clientSecret, and redirectUri in the credentials block!" }
 
@@ -844,7 +842,7 @@ class SpotifyClientApiBuilder(
                 throw SpotifyException.AuthenticationException("Invalid credentials provided in the login process (clientId=$clientId, clientSecret=$clientSecret, authCode=${authorization.authorizationCode})", e)
             }
             authorization.authorizationCode != null && authorization.pkceCodeVerifier != null -> try {
-                require(clientId != null && redirectUri != null) { "You need to specify a valid clientId, clientSecret, and redirectUri in the credentials block!" }
+                require(clientId != null && redirectUri != null) { "You need to specify a valid clientId and redirectUri in the credentials block!" }
 
                 val response = HttpConnection(
                         "https://accounts.spotify.com/api/token",
@@ -938,6 +936,10 @@ class SpotifyClientApiBuilder(
                             "to build a SpotifyClientApi object"
             )
         }
+
+        if (options.testTokenValidity) SpotifyApi.testTokenValidity(api)
+
+        return api
     }
 }
 
@@ -961,7 +963,8 @@ class SpotifyAppApiBuilder(
         val clientId = credentials.clientId
         val clientSecret = credentials.clientSecret
         require((clientId != null && clientSecret != null) || authorization.token != null || authorization.tokenString != null) { "You didn't specify a client id or client secret in the credentials block!" }
-        return when {
+
+        val api = when {
             authorization.token != null -> SpotifyAppApi(
                     clientId,
                     clientSecret,
@@ -1050,6 +1053,10 @@ class SpotifyAppApiBuilder(
                 throw SpotifyException.AuthenticationException("Invalid credentials provided in the login process (clientId=$clientId, clientSecret=$clientSecret)", e)
             }
         }
+
+        if (options.testTokenValidity) SpotifyApi.testTokenValidity(api)
+
+        return api
     }
 }
 
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/SpotifyApi.kt b/src/commonMain/kotlin/com.adamratzman.spotify/SpotifyApi.kt
@@ -68,22 +68,22 @@ internal const val base = "https://api.spotify.com/v1"
  *
  */
 sealed class SpotifyApi<T : SpotifyApi<T, B>, B : ISpotifyApiBuilder<T, B>>(
-        val clientId: String?,
-        val clientSecret: String?,
-        var token: Token,
-        useCache: Boolean,
-        var cacheLimit: Int?,
-        var automaticRefresh: Boolean,
-        var retryWhenRateLimited: Boolean,
-        enableLogger: Boolean,
-        testTokenValidity: Boolean,
-        var defaultLimit: Int,
-        var allowBulkRequests: Boolean,
-        var requestTimeoutMillis: Long?,
-        var json: Json,
-        var refreshTokenProducer: suspend (SpotifyApi<*, *>) -> Token,
-        var onTokenRefresh: (suspend (SpotifyApi<*, *>) -> Unit)?,
-        requiredScopes: List<SpotifyScope>?
+    val clientId: String?,
+    val clientSecret: String?,
+    var token: Token,
+    useCache: Boolean,
+    var cacheLimit: Int?,
+    var automaticRefresh: Boolean,
+    var retryWhenRateLimited: Boolean,
+    enableLogger: Boolean,
+    testTokenValidity: Boolean,
+    var defaultLimit: Int,
+    var allowBulkRequests: Boolean,
+    var requestTimeoutMillis: Long?,
+    var json: Json,
+    var refreshTokenProducer: suspend (SpotifyApi<*, *>) -> Token,
+    var onTokenRefresh: (suspend (SpotifyApi<*, *>) -> Unit)?,
+    requiredScopes: List<SpotifyScope>?
 ) {
     var useCache = useCache
         set(value) {
@@ -116,18 +116,6 @@ sealed class SpotifyApi<T : SpotifyApi<T, B>, B : ISpotifyApiBuilder<T, B>>(
                 )
             }
         }
-
-        if (testTokenValidity) {
-            if (!isTokenValid().isValid)
-                try {
-                    refreshToken()
-                } catch (e: BadRequestException) {
-                    throw SpotifyException.AuthenticationException(
-                            "Invalid token and refresh token supplied. Cannot refresh to a fresh token.",
-                            e
-                    )
-                }
-        }
     }
 
     /**
@@ -241,8 +229,8 @@ sealed class SpotifyApi<T : SpotifyApi<T, B>, B : ISpotifyApiBuilder<T, B>>(
 
     @JvmOverloads
     suspend fun suspendIsTokenValid(
-            makeTestRequest: Boolean = true,
-            context: CoroutineContext = Dispatchers.Default
+        makeTestRequest: Boolean = true,
+        context: CoroutineContext = Dispatchers.Default
     ): TokenValidityResponse {
         if (token.shouldRefresh()) return TokenValidityResponse(
                 false,
@@ -271,6 +259,19 @@ sealed class SpotifyApi<T : SpotifyApi<T, B>, B : ISpotifyApiBuilder<T, B>>(
     }
 
     companion object {
+        internal fun testTokenValidity(api: GenericSpotifyApi) {
+            if (!api.isTokenValid().isValid) {
+                try {
+                    api.refreshToken()
+                } catch (e: BadRequestException) {
+                    throw SpotifyException.AuthenticationException(
+                            "Invalid token and refresh token supplied. Cannot refresh to a fresh token.",
+                            e
+                    )
+                }
+            }
+        }
+
         /*
             Builder tools
          */
@@ -285,12 +286,12 @@ sealed class SpotifyApi<T : SpotifyApi<T, B>, B : ISpotifyApiBuilder<T, B>>(
          * @param state This provides protection against attacks such as cross-site request forgery.
          */
         fun getAuthUrlFull(
-                vararg scopes: SpotifyScope,
-                clientId: String,
-                redirectUri: String,
-                isImplicitGrantFlow: Boolean = false,
-                shouldShowDialog: Boolean = false,
-                state: String? = null
+            vararg scopes: SpotifyScope,
+            clientId: String,
+            redirectUri: String,
+            isImplicitGrantFlow: Boolean = false,
+            shouldShowDialog: Boolean = false,
+            state: String? = null
         ): String {
             return "https://accounts.spotify.com/authorize/?client_id=$clientId" +
                     "&response_type=${if (isImplicitGrantFlow) "token" else "code"}" +
@@ -312,11 +313,11 @@ sealed class SpotifyApi<T : SpotifyApi<T, B>, B : ISpotifyApiBuilder<T, B>>(
          * @param state This provides protection against attacks such as cross-site request forgery.
          */
         fun getPkceAuthUrlFull(
-                vararg scopes: SpotifyScope,
-                clientId: String,
-                redirectUri: String,
-                codeChallenge: String,
-                state: String? = null
+            vararg scopes: SpotifyScope,
+            clientId: String,
+            redirectUri: String,
+            codeChallenge: String,
+            state: String? = null
         ): String {
             return "https://accounts.spotify.com/authorize/?client_id=$clientId" +
                     "&response_type=code" +
@@ -361,22 +362,22 @@ sealed class SpotifyApi<T : SpotifyApi<T, B>, B : ISpotifyApiBuilder<T, B>>(
  * client authentication
  */
 class SpotifyAppApi internal constructor(
-        clientId: String?,
-        clientSecret: String?,
-        token: Token,
-        useCache: Boolean,
-        cacheLimit: Int?,
-        automaticRefresh: Boolean,
-        retryWhenRateLimited: Boolean,
-        enableLogger: Boolean,
-        testTokenValidity: Boolean,
-        defaultLimit: Int,
-        allowBulkRequests: Boolean,
-        requestTimeoutMillis: Long?,
-        json: Json,
-        refreshTokenProducer: (suspend (GenericSpotifyApi) -> Token)?,
-        onTokenRefresh: (suspend (GenericSpotifyApi) -> Unit)?,
-        requiredScopes: List<SpotifyScope>?
+    clientId: String?,
+    clientSecret: String?,
+    token: Token,
+    useCache: Boolean,
+    cacheLimit: Int?,
+    automaticRefresh: Boolean,
+    retryWhenRateLimited: Boolean,
+    enableLogger: Boolean,
+    testTokenValidity: Boolean,
+    defaultLimit: Int,
+    allowBulkRequests: Boolean,
+    requestTimeoutMillis: Long?,
+    json: Json,
+    refreshTokenProducer: (suspend (GenericSpotifyApi) -> Token)?,
+    onTokenRefresh: (suspend (GenericSpotifyApi) -> Unit)?,
+    requiredScopes: List<SpotifyScope>?
 ) : SpotifyApi<SpotifyAppApi, SpotifyAppApiBuilder>(
         clientId,
         clientSecret,
@@ -396,10 +397,10 @@ class SpotifyAppApi internal constructor(
         requiredScopes
 ) {
     constructor(
-            clientId: String,
-            clientSecret: String,
-            token: Token,
-            options: SpotifyApiOptions = SpotifyApiOptionsBuilder().build()
+        clientId: String,
+        clientSecret: String,
+        token: Token,
+        options: SpotifyApiOptions = SpotifyApiOptionsBuilder().build()
     ) : this(
             clientId,
             clientSecret,
@@ -481,24 +482,24 @@ class SpotifyAppApi internal constructor(
  * managed through the scopes exposed in [token]
  */
 open class SpotifyClientApi(
-        clientId: String?,
-        clientSecret: String?,
-        var redirectUri: String?,
-        token: Token,
-        useCache: Boolean,
-        cacheLimit: Int?,
-        automaticRefresh: Boolean,
-        retryWhenRateLimited: Boolean,
-        enableLogger: Boolean,
-        testTokenValidity: Boolean,
-        defaultLimit: Int,
-        allowBulkRequests: Boolean,
-        requestTimeoutMillis: Long?,
-        json: Json,
-        refreshTokenProducer: (suspend (GenericSpotifyApi) -> Token)?,
-        val usesPkceAuth: Boolean,
-        onTokenRefresh: (suspend (GenericSpotifyApi) -> Unit)?,
-        requiredScopes: List<SpotifyScope>?
+    clientId: String?,
+    clientSecret: String?,
+    var redirectUri: String?,
+    token: Token,
+    useCache: Boolean,
+    cacheLimit: Int?,
+    automaticRefresh: Boolean,
+    retryWhenRateLimited: Boolean,
+    enableLogger: Boolean,
+    testTokenValidity: Boolean,
+    defaultLimit: Int,
+    allowBulkRequests: Boolean,
+    requestTimeoutMillis: Long?,
+    json: Json,
+    refreshTokenProducer: (suspend (GenericSpotifyApi) -> Token)?,
+    val usesPkceAuth: Boolean,
+    onTokenRefresh: (suspend (GenericSpotifyApi) -> Unit)?,
+    requiredScopes: List<SpotifyScope>?
 ) : SpotifyApi<SpotifyClientApi, SpotifyClientApiBuilder>(
         clientId,
         clientSecret,
@@ -518,12 +519,12 @@ open class SpotifyClientApi(
         requiredScopes
 ) {
     constructor(
-            clientId: String,
-            clientSecret: String,
-            redirectUri: String,
-            token: Token,
-            usesPkceAuth: Boolean,
-            options: SpotifyApiOptions = SpotifyApiOptionsBuilder().build()
+        clientId: String,
+        clientSecret: String,
+        redirectUri: String,
+        token: Token,
+        usesPkceAuth: Boolean,
+        options: SpotifyApiOptions = SpotifyApiOptionsBuilder().build()
     ) : this(
             clientId,
             clientSecret,
@@ -699,6 +700,7 @@ open class SpotifyClientApi(
             api as SpotifyClientApi
 
             require(api.clientId != null) { "The client id is not set" }
+
             val response = if (!api.usesPkceAuth) {
                 require(api.clientSecret != null) { "The client secret is not set" }
                 executeTokenRequest(
@@ -753,20 +755,20 @@ open class SpotifyClientApi(
  * managed through the scopes exposed in [token]. [token] is not refreshable and is only accessible for limited time.
  */
 class SpotifyImplicitGrantApi(
-        clientId: String?,
-        clientSecret: String?,
-        redirectUri: String?,
-        token: Token,
-        useCache: Boolean,
-        cacheLimit: Int?,
-        retryWhenRateLimited: Boolean,
-        enableLogger: Boolean,
-        testTokenValidity: Boolean,
-        defaultLimit: Int,
-        allowBulkRequests: Boolean,
-        requestTimeoutMillis: Long?,
-        json: Json,
-        requiredScopes: List<SpotifyScope>?
+    clientId: String?,
+    clientSecret: String?,
+    redirectUri: String?,
+    token: Token,
+    useCache: Boolean,
+    cacheLimit: Int?,
+    retryWhenRateLimited: Boolean,
+    enableLogger: Boolean,
+    testTokenValidity: Boolean,
+    defaultLimit: Int,
+    allowBulkRequests: Boolean,
+    requestTimeoutMillis: Long?,
+    json: Json,
+    requiredScopes: List<SpotifyScope>?
 ) : SpotifyClientApi(
         clientId,
         clientSecret,
@@ -814,9 +816,9 @@ typealias GenericSpotifyApi = SpotifyApi<*, *>
 suspend fun getCredentialedToken(clientId: String, clientSecret: String, api: GenericSpotifyApi?, json: Json): Token = SpotifyApi.getCredentialedToken(clientId, clientSecret, api, json)
 
 internal suspend fun executeTokenRequest(
-        httpConnection: HttpConnection,
-        clientId: String,
-        clientSecret: String
+    httpConnection: HttpConnection,
+    clientId: String,
+    clientSecret: String
 ): HttpResponse {
     return httpConnection.execute(
             listOf(
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/http/Endpoints.kt b/src/commonMain/kotlin/com.adamratzman.spotify/http/Endpoints.kt
@@ -137,7 +137,7 @@ abstract class SpotifyEndpoint(val api: GenericSpotifyApi) {
             }
         }
 
-        if (document.responseCode / 200 != 1 /* Check if status is 2xx or 3xx */) {
+        if (document.responseCode / 200 != 1 /* Check if status is not 2xx or 3xx */) {
             val response = try {
                 document.body.toObject(ErrorResponse.serializer(), api, json)
             } catch (e: Exception) {
diff --git a/src/jvmTest/kotlin/com/adamratzman/spotify/PkceTest.kt b/src/jvmTest/kotlin/com/adamratzman/spotify/PkceTest.kt

Original file line number	Diff line number	Diff line change
`@@ -208,7 +208,7 @@ kotlin {`
`208`	`208`	`api("net.sourceforge.streamsupport:android-retrofuture:1.7.2")`
`209`	`209`	`api("org.jetbrains.kotlinx:kotlinx-coroutines-core:$coroutineVersion")`
`210`	`210`	`api("org.jetbrains.kotlinx:kotlinx-serialization-runtime:$serializationVersion")`
`211`		`- api("io.ktor:ktor-client-cio:$ktorVersion")`
	`211`	`+ api("io.ktor:ktor-client-okhttp:$ktorVersion")`
`212`	`212`	`api("io.coil-kt:coil:0.11.0")`
`213`	`213`	`implementation(kotlin("stdlib-jdk8"))`
`214`	`214`	`}`
Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ abstract class SpotifyEndpoint(val api: GenericSpotifyApi) {`
`137`	`137`	`}`
`138`	`138`	`}`
`139`	`139`
`140`		`- if (document.responseCode / 200 != 1 /* Check if status is 2xx or 3xx */) {`
	`140`	`+ if (document.responseCode / 200 != 1 /* Check if status is not 2xx or 3xx */) {`
`141`	`141`	`val response = try {`
`142`	`142`	`document.body.toObject(ErrorResponse.serializer(), api, json)`
`143`	`143`	`} catch (e: Exception) {`