Skip to content

Commit

Permalink
Merge pull request #337 from adamint/dev/adamint/fix-incorrect-logic-…
Browse files Browse the repository at this point in the history
…has-scopes

Several bug fixes
  • Loading branch information
adamint authored Mar 27, 2024
2 parents 3c7b55d + d50cc64 commit 1b2c9f8
Show file tree
Hide file tree
Showing 23 changed files with 49 additions and 37 deletions.
9 changes: 7 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -46,7 +47,7 @@ version = libraryVersion

android {
namespace = "com.adamratzman.spotify"
compileSdk = 30
compileSdk = 31
compileOptions {
sourceCompatibility = JavaVersion.VERSION_17
targetCompatibility = JavaVersion.VERSION_17
Expand All @@ -56,7 +57,7 @@ android {
}
defaultConfig {
minSdk = 23
setCompileSdkVersion(30)
setCompileSdkVersion(31)
testInstrumentationRunner = "android.support.test.runner.AndroidJUnitRunner"
}

Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ private suspend fun buildSpotifyApiInternal(): GenericSpotifyApi? {

val optionsCreator: (SpotifyApiOptions.() -> Unit) = {
this.enableDebugMode = logHttp
retryOnInternalServerErrorTimes = 0
}

return when {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions src/commonMain/kotlin/com.adamratzman.spotify/models/Albums.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 }
}

/**
Expand Down Expand Up @@ -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,
Expand All @@ -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)

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions src/commonMain/kotlin/com.adamratzman.spotify/models/Player.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)!!
}

/**
Expand Down Expand Up @@ -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)!!
}

/**
Expand All @@ -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
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -150,7 +150,7 @@ public data class Playlist(
@Serializable
public data class PlaylistTrackInfo(
val href: String,
val total: Int
val total: Int? = null
)

@Serializable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal val nonstrictJson =
ignoreUnknownKeys = true
allowSpecialFloatingPointValues = true
useArrayPolymorphism = true
coerceInputValues = true
}

// Parse function that catches on parse exception
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
}
2 changes: 2 additions & 0 deletions src/commonMain/kotlin/com.adamratzman.spotify/utils/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/jvmTest/kotlin/com/adamratzman/spotify/PkceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class PkceTest {

println(
getSpotifyPkceAuthorizationUrl(
*SpotifyScope.values(),
*SpotifyScope.entries.toTypedArray(),
clientId = clientId,
redirectUri = serverRedirectUri,
codeChallenge = getSpotifyPkceCodeChallenge(pkceCodeVerifier),
Expand Down

0 comments on commit 1b2c9f8

Please sign in to comment.