Merge pull request #337 from adamint/dev/adamint/fix-incorrect-logic-has-scopes

adamint · web-flow · commit 1b2c9f802008 · 2024-03-27T00:13:23.000-04:00
Several bug fixes
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -2,6 +2,7 @@
 
 import com.fasterxml.jackson.databind.json.JsonMapper
 import org.jetbrains.dokka.gradle.DokkaTask
+import org.jetbrains.kotlin.gradle.ExperimentalKotlinGradlePluginApi
 import org.jetbrains.kotlin.gradle.plugin.KotlinJsCompilerType
 import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget
 import org.jetbrains.kotlin.gradle.targets.js.webpack.KotlinWebpackOutput.Target
@@ -46,7 +47,7 @@ version = libraryVersion
 
 android {
     namespace = "com.adamratzman.spotify"
-    compileSdk = 30
+    compileSdk = 31
     compileOptions {
         sourceCompatibility = JavaVersion.VERSION_17
         targetCompatibility = JavaVersion.VERSION_17
@@ -56,7 +57,7 @@ android {
     }
     defaultConfig {
         minSdk = 23
-        setCompileSdkVersion(30)
+        setCompileSdkVersion(31)
         testInstrumentationRunner = "android.support.test.runner.AndroidJUnitRunner"
     }
 
@@ -81,6 +82,10 @@ val dokkaJar: TaskProvider<Jar> by tasks.registering(Jar::class) {
 }
 
 kotlin {
+    @OptIn(ExperimentalKotlinGradlePluginApi::class)
+    compilerOptions { 
+        freeCompilerArgs.add("-Xexpect-actual-classes")
+    }
     explicitApiWarning()
     jvmToolchain(17)
 
diff --git a/gradle.properties b/gradle.properties
@@ -21,6 +21,6 @@ kotlinxCoroutinesVersion=1.7.3
 androidBuildToolsVersion=8.2.2
 androidSpotifyAuthVersion=2.1.1
 androidCryptoVersion=1.1.0-alpha06
-androidxCompatVersion=1.6.1
+androidxCompatVersion=1.7.0-alpha03
 
 sparkVersion=2.9.4
diff --git a/src/commonJvmLikeTest/kotlin/com/adamratzman/spotify/CommonImpl.kt b/src/commonJvmLikeTest/kotlin/com/adamratzman/spotify/CommonImpl.kt
@@ -38,6 +38,7 @@ private suspend fun buildSpotifyApiInternal(): GenericSpotifyApi? {
 
     val optionsCreator: (SpotifyApiOptions.() -> Unit) = {
         this.enableDebugMode = logHttp
+        retryOnInternalServerErrorTimes = 0
     }
 
     return when {
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/SpotifyApi.kt b/src/commonMain/kotlin/com.adamratzman.spotify/SpotifyApi.kt
@@ -644,7 +644,7 @@ public open class SpotifyClientApi(
         if (token.scopes == null) {
             null
         } else {
-            !isTokenValid(false).isValid &&
+            isTokenValid(false).isValid &&
                 token.scopes?.contains(scope) == true &&
                 scopes.all { token.scopes?.contains(it) == true }
         }
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/endpoints/pub/EpisodeApi.kt b/src/commonMain/kotlin/com.adamratzman.spotify/endpoints/pub/EpisodeApi.kt
@@ -3,6 +3,7 @@ package com.adamratzman.spotify.endpoints.pub
 
 import com.adamratzman.spotify.GenericSpotifyApi
 import com.adamratzman.spotify.SpotifyAppApi
+import com.adamratzman.spotify.SpotifyClientApi
 import com.adamratzman.spotify.SpotifyException.BadRequestException
 import com.adamratzman.spotify.SpotifyScope
 import com.adamratzman.spotify.http.SpotifyEndpoint
@@ -60,7 +61,7 @@ public open class EpisodeApi(api: GenericSpotifyApi) : SpotifyEndpoint(api) {
      * Users can view the country that is associated with their account in the account settings. Required for [SpotifyAppApi], but **you may use [Market.FROM_TOKEN] to get the user market**
      *
      * @return List of possibly-null [Episode] objects.
-     * @throws BadRequestException If any invalid show id is provided
+     * @throws BadRequestException If any invalid show id is provided, if this is a [SpotifyClientApi]
      */
     public suspend fun getEpisodes(vararg ids: String, market: Market): List<Episode?> {
         checkBulkRequesting(50, ids.size)
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/endpoints/pub/SearchApi.kt b/src/commonMain/kotlin/com.adamratzman.spotify/endpoints/pub/SearchApi.kt
@@ -338,7 +338,7 @@ public open class SearchApi(api: GenericSpotifyApi) : SpotifyEndpoint(api) {
         market: Market,
         language: Language? = null
     ): SpotifySearchResult =
-        search(query, filters = filters, searchTypes = SearchType.values(), limit = limit, offset = offset, market = market)
+        search(query, filters = filters, searchTypes = SearchType.entries.toTypedArray(), limit = limit, offset = offset, market = market, language = language)
 
     protected fun build(
         query: String,
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/Albums.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/Albums.kt
@@ -41,7 +41,7 @@ public data class SimpleAlbum(
     override val uri: SpotifyUri,
 
     val artists: List<SimpleArtist>,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     val name: String,
     val type: String,
     val restrictions: Restrictions? = null,
@@ -54,14 +54,14 @@ public data class SimpleAlbum(
 
     val albumType: AlbumResultType
         get() = albumTypeString.let { _ ->
-            AlbumResultType.values().first { it.id.equals(albumTypeString, true) }
+            AlbumResultType.entries.first { it.id.equals(albumTypeString, true) }
         }
 
     val releaseDate: ReleaseDate? get() = releaseDateString?.let { getReleaseDate(releaseDateString) }
 
     val albumGroup: AlbumResultType?
         get() = albumGroupString?.let { _ ->
-            AlbumResultType.values().find { it.id == albumGroupString }
+            AlbumResultType.entries.find { it.id == albumGroupString }
         }
 
     /**
@@ -142,7 +142,7 @@ public data class Album(
     val artists: List<SimpleArtist>,
     val copyrights: List<SpotifyCopyright>,
     val genres: List<String>,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     val label: String,
     val name: String,
     val popularity: Double,
@@ -157,7 +157,7 @@ public data class Album(
 
     val externalIds: List<ExternalId> get() = externalIdsString.map { ExternalId(it.key, it.value) }
 
-    val albumType: AlbumResultType get() = AlbumResultType.values().first { it.id == albumTypeString }
+    val albumType: AlbumResultType get() = AlbumResultType.entries.first { it.id == albumTypeString }
 
     val releaseDate: ReleaseDate get() = getReleaseDate(releaseDateString)
 
@@ -181,7 +181,7 @@ public data class SpotifyCopyright(
             .removePrefix("(C)")
             .trim()
 
-    val type: CopyrightType get() = CopyrightType.values().match(typeString)!!
+    val type: CopyrightType get() = CopyrightType.entries.toTypedArray().match(typeString)!!
 }
 
 @Serializable
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/Artists.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/Artists.kt
@@ -59,7 +59,7 @@ public data class Artist(
 
     val followers: Followers,
     val genres: List<String>,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     val name: String? = null,
     val popularity: Double,
     val type: String
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/Authentication.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/Authentication.kt
@@ -33,7 +33,7 @@ public data class Token(
     val expiresAt: Long get() = getCurrentTimeMs() + expiresIn * 1000
 
     val scopes: List<SpotifyScope>? get() = scopeString?.let { str ->
-        str.split(" ").mapNotNull { scope -> SpotifyScope.values().find { it.uri.equals(scope, true) } }
+        str.split(" ").mapNotNull { scope -> SpotifyScope.entries.find { it.uri.equals(scope, true) } }
     }
 
     public fun shouldRefresh(): Boolean = getCurrentTimeMs() > expiresAt
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/Episode.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/Episode.kt
@@ -96,7 +96,7 @@ public data class Episode(
     @SerialName("external_urls") override val externalUrlsString: Map<String, String>,
     override val href: String,
     override val id: String,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     @SerialName("is_externally_hosted") val isExternallyHosted: Boolean,
     @SerialName("is_playable") val isPlayable: Boolean,
     @Deprecated("This field is deprecated and might be removed in the future. Please use the languages field instead")
@@ -148,7 +148,7 @@ public data class SimpleEpisode(
     @SerialName("external_urls") override val externalUrlsString: Map<String, String>,
     override val href: String,
     override val id: String,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     @SerialName("is_externally_hosted") val isExternallyHosted: Boolean,
     @SerialName("is_playable") val isPlayable: Boolean,
     @Deprecated("This field is deprecated and might be removed in the future. Please use the languages field instead")
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/Player.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/Player.kt
@@ -24,7 +24,7 @@ public data class SpotifyContext(
     val uri: ContextUri,
     @SerialName("type") val typeString: String
 ) {
-    val type: SpotifyContextType get() = SpotifyContextType.values().match(typeString)!!
+    val type: SpotifyContextType get() = SpotifyContextType.entries.toTypedArray().match(typeString)!!
     val externalUrls: List<ExternalUrl> get() = getExternalUrls(externalUrlsString)
 }
 
@@ -64,7 +64,7 @@ public data class Device(
     @SerialName("type") val typeString: String,
     @SerialName("volume_percent") val volumePercent: Int
 ) : IdentifiableNullable() {
-    val type: DeviceType get() = DeviceType.values().first { it.identifier.equals(typeString, true) }
+    val type: DeviceType get() = DeviceType.entries.first { it.identifier.equals(typeString, true) }
 
     override val href: String? = null
 }
@@ -118,7 +118,7 @@ public data class CurrentlyPlayingContext(
     val context: SpotifyContext? = null
 ) {
     val repeatState: ClientPlayerApi.PlayerRepeatState
-        get() = ClientPlayerApi.PlayerRepeatState.values().match(repeatStateString)!!
+        get() = ClientPlayerApi.PlayerRepeatState.entries.toTypedArray().match(repeatStateString)!!
 }
 
 /**
@@ -146,7 +146,7 @@ public data class CurrentlyPlayingObject(
     val actions: PlaybackActions
 ) {
     val currentlyPlayingType: CurrentlyPlayingType
-        get() = CurrentlyPlayingType.values().match(currentlyPlayingTypeString)!!
+        get() = CurrentlyPlayingType.entries.toTypedArray().match(currentlyPlayingTypeString)!!
 }
 
 /**
@@ -162,7 +162,7 @@ public data class PlaybackActions(
     val disallows: List<DisallowablePlaybackAction>
         get() = disallowsString.map {
             DisallowablePlaybackAction(
-                PlaybackAction.values().match(it.key)!!,
+                PlaybackAction.entries.toTypedArray().match(it.key)!!,
                 it.value ?: false
             )
         }
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/Playlist.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/Playlist.kt
@@ -39,7 +39,7 @@ public data class SimplePlaylist(
     override val uri: SpotifyUri,
 
     val collaborative: Boolean,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     val name: String,
     val description: String? = null,
     val owner: SpotifyPublicUser,
@@ -126,7 +126,7 @@ public data class Playlist(
     val description: String? = null,
     val followers: Followers,
     @SerialName("primary_color") val primaryColor: String? = null,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     val name: String,
     val owner: SpotifyPublicUser,
     val public: Boolean? = null,
@@ -150,7 +150,7 @@ public data class Playlist(
 @Serializable
 public data class PlaylistTrackInfo(
     val href: String,
-    val total: Int
+    val total: Int? = null
 )
 
 @Serializable
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/SpotifyUris.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/SpotifyUris.kt
@@ -76,8 +76,8 @@ public interface ISpotifyUri {
  */
 @Serializable(with = SpotifyUriSerializer::class)
 public sealed class SpotifyUri(input: String, public val type: String, allowColon: Boolean = false) : ISpotifyUri {
-    public override val uri: String
-    public override val id: String
+    public final override val uri: String
+    public final override val id: String
 
     init {
         input.replace(" ", "").also {
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/Users.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/Users.kt
@@ -40,7 +40,7 @@ public data class SpotifyUserInformation(
     @SerialName("display_name") val displayName: String? = null,
     val email: String? = null,
     val followers: Followers,
-    val images: List<SpotifyImage>,
+    val images: List<SpotifyImage>? = null,
     val product: String? = null,
     @SerialName("explicit_content") val explicitContentSettings: ExplicitContentSettings? = null,
     val type: String
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/models/serialization/SerializationUtils.kt b/src/commonMain/kotlin/com.adamratzman.spotify/models/serialization/SerializationUtils.kt
@@ -23,6 +23,7 @@ internal val nonstrictJson =
         ignoreUnknownKeys = true
         allowSpecialFloatingPointValues = true
         useArrayPolymorphism = true
+        coerceInputValues = true
     }
 
 // Parse function that catches on parse exception
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/utils/Locale.kt b/src/commonMain/kotlin/com.adamratzman.spotify/utils/Locale.kt
@@ -806,7 +806,7 @@ public enum class Locale(public val language: Language, public val country: Mark
 
     public companion object {
         public fun from(language: Language, country: Market): Locale? {
-            return values().find { locale -> locale.language == language && locale.country == country }
+            return entries.find { locale -> locale.language == language && locale.country == country }
         }
     }
 }
diff --git a/src/commonMain/kotlin/com.adamratzman.spotify/utils/Utils.kt b/src/commonMain/kotlin/com.adamratzman.spotify/utils/Utils.kt
@@ -22,6 +22,8 @@ internal suspend inline fun <T> catch(catchInternalServerError: Boolean = false,
         function()
     } catch (e: SpotifyException.BadRequestException) {
         if (e.statusCode !in listOf(400, 404)) throw e
+        else if (e.statusCode in 500..599 && catchInternalServerError) throw e
+        
         // we should only ignore the exception if it's 400 or 404. Otherwise, it's a larger issue
         null
     }
diff --git a/src/commonTest/kotlin/com.adamratzman/spotify/AbstractTest.kt b/src/commonTest/kotlin/com.adamratzman/spotify/AbstractTest.kt
@@ -37,7 +37,7 @@ abstract class AbstractTest<T : GenericSpotifyApi> {
         return if (apiInitialized) {
             true
         } else {
-            println("Api is not initialized. buildSpotifyApi returns ${buildSpotifyApi(testClassQualifiedName, "n/a")}")
+            println("Api is not initialized or does not match the expected type. buildSpotifyApi returns ${buildSpotifyApi(testClassQualifiedName, "n/a")}")
             false
         }
     }
diff --git a/src/commonTest/kotlin/com.adamratzman/spotify/priv/ClientEpisodeApiTest.kt b/src/commonTest/kotlin/com.adamratzman/spotify/priv/ClientEpisodeApiTest.kt
@@ -30,8 +30,9 @@ class ClientEpisodeApiTest : AbstractTest<SpotifyClientApi>() {
         buildApi<SpotifyClientApi>(::testGetEpisodes.name)
         if (!isApiInitialized()) return@runTestOnDefaultDispatcher
 
-        assertEquals(listOf(null, null), api.episodes.getEpisodes("hi", "dad"))
-        assertEquals(null, api.episodes.getEpisodes("1cfOhXP4GQCd5ZFHoSF8gg", "j")[1])
+        assertFailsWith<BadRequestException> { api.episodes.getEpisodes("hi", "dad") }
+        assertFailsWith<BadRequestException> { api.episodes.getEpisodes("1cfOhXP4GQCd5ZFHoSF8gg", "j")[1] }
+
         assertEquals(
             listOf("The Great Inflation (Classic)"),
             api.episodes.getEpisodes("3lMZTE81Pbrp0U12WZe27l").map { it?.name }
diff --git a/src/commonTest/kotlin/com.adamratzman/spotify/priv/ClientPlaylistApiTest.kt b/src/commonTest/kotlin/com.adamratzman/spotify/priv/ClientPlaylistApiTest.kt
@@ -85,8 +85,8 @@ class ClientPlaylistApiTest : AbstractTest<SpotifyClientApi>() {
 
         api.spotifyApiOptions.allowBulkRequests = true
 
-        suspend fun calculatePlaylistSize() = api.playlists.getClientPlaylist(createdPlaylist!!.id)!!.tracks.total
-        val sizeBefore = calculatePlaylistSize()
+        suspend fun calculatePlaylistSize(): Int? = api.playlists.getClientPlaylist(createdPlaylist!!.id)!!.tracks.total
+        val sizeBefore = calculatePlaylistSize() ?: 0
         api.playlists.addPlayablesToClientPlaylist(createdPlaylist!!.id, playables = tracks.toTypedArray())
         assertEquals(sizeBefore + tracks.size, calculatePlaylistSize())
         api.playlists.removePlayablesFromClientPlaylist(createdPlaylist!!.id, playables = tracks.toTypedArray())
@@ -98,6 +98,7 @@ class ClientPlaylistApiTest : AbstractTest<SpotifyClientApi>() {
     }
 
     @Test
+    @Ignore // ignored because Spotify currently broke the ability to change `public` field
     fun testEditPlaylists(): TestResult = runTestOnDefaultDispatcher {
         buildApi<SpotifyClientApi>(::testEditPlaylists.name)
         if (!isApiInitialized()) return@runTestOnDefaultDispatcher
@@ -130,7 +131,7 @@ class ClientPlaylistApiTest : AbstractTest<SpotifyClientApi>() {
         assertTrue(updatedPlaylist.public == false)
         assertEquals("test playlist", updatedPlaylist.name)
         //assertEquals("description 2", fullPlaylist.description)  <-- spotify is flaky about actually having description set
-        assertTrue(updatedPlaylist.tracks.total == 2 && updatedPlaylist.images.isNotEmpty())
+        assertTrue(updatedPlaylist.tracks.total == 2 && updatedPlaylist.images?.isNotEmpty() == true)
 
         api.playlists.reorderClientPlaylistPlayables(updatedPlaylist.id, 1, insertionPoint = 0)
 
diff --git a/src/commonTest/kotlin/com.adamratzman/spotify/pub/SearchApiTest.kt b/src/commonTest/kotlin/com.adamratzman/spotify/pub/SearchApiTest.kt
@@ -23,7 +23,7 @@ class SearchApiTest : AbstractTest<GenericSpotifyApi>() {
     fun testSearchMultiple(): TestResult = runTestOnDefaultDispatcher {
         buildApi(::testSearchMultiple.name)
 
-        val query = api.search.search("lo", *SearchApi.SearchType.values(), market = Market.US)
+        val query = api.search.search("lo", *SearchApi.SearchType.entries.toTypedArray(), market = Market.US)
         assertTrue(
             query.albums?.items?.isNotEmpty() == true && query.tracks?.items?.isNotEmpty() == true && query.artists?.items?.isNotEmpty() == true &&
                 query.playlists?.items?.isNotEmpty() == true && query.shows?.items?.isNotEmpty() == true && query.episodes?.items?.isNotEmpty() == true
diff --git a/src/jsMain/kotlin/com/adamratzman/spotify/webplayer/WebPlaybackSdk.kt b/src/jsMain/kotlin/com/adamratzman/spotify/webplayer/WebPlaybackSdk.kt
@@ -13,7 +13,7 @@ import kotlin.js.Promise
 public external interface Album {
     public var uri: String
     public var name: String
-    public var images: Array<Image>
+    public var images: Array<Image>?
 }
 
 public external interface Artist {
diff --git a/src/jvmTest/kotlin/com/adamratzman/spotify/PkceTest.kt b/src/jvmTest/kotlin/com/adamratzman/spotify/PkceTest.kt
@@ -27,7 +27,7 @@ class PkceTest {
 
             println(
                 getSpotifyPkceAuthorizationUrl(
-                    *SpotifyScope.values(),
+                    *SpotifyScope.entries.toTypedArray(),
                     clientId = clientId,
                     redirectUri = serverRedirectUri,
                     codeChallenge = getSpotifyPkceCodeChallenge(pkceCodeVerifier),

Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,7 @@ private suspend fun buildSpotifyApiInternal(): GenericSpotifyApi? {`
`38`	`38`
`39`	`39`	`val optionsCreator: (SpotifyApiOptions.() -> Unit) = {`
`40`	`40`	`this.enableDebugMode = logHttp`
	`41`	`+ retryOnInternalServerErrorTimes = 0`
`41`	`42`	`}`
`42`	`43`
`43`	`44`	`return when {`
Original file line number	Diff line number	Diff line change
`@@ -644,7 +644,7 @@ public open class SpotifyClientApi(`
`644`	`644`	`if (token.scopes == null) {`
`645`	`645`	`null`
`646`	`646`	`} else {`
`647`		`- !isTokenValid(false).isValid &&`
	`647`	`+ isTokenValid(false).isValid &&`
`648`	`648`	`token.scopes?.contains(scope) == true &&`
`649`	`649`	`scopes.all { token.scopes?.contains(it) == true }`
`650`	`650`	`}`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ public data class Token(`
`33`	`33`	`val expiresAt: Long get() = getCurrentTimeMs() + expiresIn * 1000`
`34`	`34`
`35`	`35`	`val scopes: List<SpotifyScope>? get() = scopeString?.let { str ->`
`36`		`- str.split(" ").mapNotNull { scope -> SpotifyScope.values().find { it.uri.equals(scope, true) } }`
	`36`	`+ str.split(" ").mapNotNull { scope -> SpotifyScope.entries.find { it.uri.equals(scope, true) } }`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`public fun shouldRefresh(): Boolean = getCurrentTimeMs() > expiresAt`