Merge pull request #517 from tzugen/cleanupStreamHandling

Cleaner separation of API result handling.

core/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/GetStreamUrlTest.kt

@@ -10,7 +10,7 @@ import org.moire.ultrasonic.api.subsonic.interceptors.toHexBytes
 import org.moire.ultrasonic.api.subsonic.rules.MockWebServerRule
 
 /**
- * Integration test for [SubsonicAPIClient.getStreamUrl] method.
+ * Integration test for [getStreamUrl] method.
  */
 class GetStreamUrlTest {
     @JvmField @Rule val mockWebServerRule = MockWebServerRule()
@@ -30,7 +30,7 @@ class GetStreamUrlTest {
         )
         client = SubsonicAPIClient(config)
         val baseExpectedUrl = mockWebServerRule.mockWebServer.url("").toString()
-        expectedUrl = "$baseExpectedUrl/rest/stream.view?id=$id&u=$USERNAME" +
+        expectedUrl = "$baseExpectedUrl/rest/stream.view?id=$id&format=raw&u=$USERNAME" +
             "&c=$CLIENT_ID&f=json&v=${V1_6_0.restApiVersion}&p=enc:${PASSWORD.toHexBytes()}"
     }
 
@@ -38,7 +38,7 @@ class GetStreamUrlTest {
     fun `Should return valid stream url`() {
         mockWebServerRule.enqueueResponse("ping_ok.json")
 
-        val streamUrl = client.getStreamUrl(id)
+        val streamUrl = client.api.getStreamUrl(id)
 
         streamUrl `should be equal to` expectedUrl
     }
@@ -47,7 +47,7 @@ class GetStreamUrlTest {
     fun `Should still return stream url if connection failed`() {
         mockWebServerRule.mockWebServer.enqueue(MockResponse().setResponseCode(500))
 
-        val streamUrl = client.getStreamUrl(id)
+        val streamUrl = client.api.getStreamUrl(id)
 
         streamUrl `should be equal to` expectedUrl
     }

core/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicApiGetAvatarTest.kt

@@ -7,14 +7,14 @@ import org.amshove.kluent.`should not be`
 import org.junit.Test
 
 /**
- * Integration test for [SubsonicAPIClient.getAvatar] call.
+ * Integration test for [SubsonicAPIDefinition.getAvatar] call.
  */
 class SubsonicApiGetAvatarTest : SubsonicAPIClientTest() {
     @Test
     fun `Should handle api error response`() {
         mockWebServerRule.enqueueResponse("request_data_not_found_error_response.json")
 
-        val response = client.getAvatar("some")
+        val response = client.api.getAvatar("some-id").execute().toStreamResponse()
 
         with(response) {
             stream `should be` null
@@ -28,7 +28,7 @@ class SubsonicApiGetAvatarTest : SubsonicAPIClientTest() {
         val httpErrorCode = 500
         mockWebServerRule.mockWebServer.enqueue(MockResponse().setResponseCode(httpErrorCode))
 
-        val response = client.getAvatar("some")
+        val response = client.api.getAvatar("some-id").execute().toStreamResponse()
 
         with(response) {
             stream `should be equal to` null
@@ -44,7 +44,7 @@ class SubsonicApiGetAvatarTest : SubsonicAPIClientTest() {
                 .setBody(mockWebServerRule.loadJsonResponse("ping_ok.json"))
         )
 
-        val response = client.stream("some")
+        val response = client.api.stream("some-id").execute().toStreamResponse()
 
         with(response) {
             responseHttpCode `should be equal to` 200

core/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicApiGetCoverArtTest.kt

@@ -14,7 +14,7 @@ class SubsonicApiGetCoverArtTest : SubsonicAPIClientTest() {
     fun `Should handle api error response`() {
         mockWebServerRule.enqueueResponse("request_data_not_found_error_response.json")
 
-        val response = client.getCoverArt("some-id")
+        val response = client.api.getCoverArt("some-id").execute().toStreamResponse()
 
         with(response) {
             stream `should be` null
@@ -28,7 +28,7 @@ class SubsonicApiGetCoverArtTest : SubsonicAPIClientTest() {
         val httpErrorCode = 404
         mockWebServerRule.mockWebServer.enqueue(MockResponse().setResponseCode(httpErrorCode))
 
-        val response = client.getCoverArt("some-id")
+        val response = client.api.getCoverArt("some-id").execute().toStreamResponse()
 
         with(response) {
             stream `should be` null
@@ -44,7 +44,7 @@ class SubsonicApiGetCoverArtTest : SubsonicAPIClientTest() {
                 .setBody(mockWebServerRule.loadJsonResponse("ping_ok.json"))
         )
 
-        val response = client.getCoverArt("some-id")
+        val response = client.api.getCoverArt("some-id").execute().toStreamResponse()
 
         with(response) {
             responseHttpCode `should be equal to` 200

core/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicApiStreamTest.kt

@@ -14,7 +14,7 @@ class SubsonicApiStreamTest : SubsonicAPIClientTest() {
     fun `Should handle api error response`() {
         mockWebServerRule.enqueueResponse("request_data_not_found_error_response.json")
 
-        val response = client.stream("some-id")
+        val response = client.api.stream("some-id").execute().toStreamResponse()
 
         with(response) {
             stream `should be` null
@@ -28,7 +28,7 @@ class SubsonicApiStreamTest : SubsonicAPIClientTest() {
         val httpErrorCode = 404
         mockWebServerRule.mockWebServer.enqueue(MockResponse().setResponseCode(httpErrorCode))
 
-        val response = client.stream("some-id")
+        val response = client.api.stream("some-id").execute().toStreamResponse()
 
         with(response) {
             stream `should be` null
@@ -38,13 +38,13 @@ class SubsonicApiStreamTest : SubsonicAPIClientTest() {
     }
 
     @Test
-    fun `Should return successfull call stream`() {
+    fun `Should return successful call stream`() {
         mockWebServerRule.mockWebServer.enqueue(
             MockResponse()
                 .setBody(mockWebServerRule.loadJsonResponse("ping_ok.json"))
         )
 
-        val response = client.stream("some-id")
+        val response = client.api.stream("some-id").execute().toStreamResponse()
 
         with(response) {
             responseHttpCode `should be equal to` 200

core/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/ApiVersionCheckWrapper.kt

@@ -45,7 +45,8 @@ import retrofit2.Call
 @Suppress("TooManyFunctions")
 internal class ApiVersionCheckWrapper(
     val api: SubsonicAPIDefinition,
-    var currentApiVersion: SubsonicAPIVersions
+    var currentApiVersion: SubsonicAPIVersions,
+    var isRealProtocolVersion: Boolean = false
 ) : SubsonicAPIDefinition by api {
     override fun getArtists(musicFolderId: String?): Call<GetArtistsResponse> {
         checkVersion(V1_8_0)
@@ -325,10 +326,15 @@ internal class ApiVersionCheckWrapper(
     }
 
     private fun checkVersion(expectedVersion: SubsonicAPIVersions) {
-        if (currentApiVersion < expectedVersion) throw ApiNotSupportedException(currentApiVersion)
+        // If it is true, it is probably the first call with this server
+        if (!isRealProtocolVersion) return
+        if (currentApiVersion < expectedVersion)
+            throw ApiNotSupportedException(currentApiVersion)
     }
 
     private fun checkParamVersion(param: Any?, expectedVersion: SubsonicAPIVersions) {
+        // If it is true, it is probably the first call with this server
+        if (!isRealProtocolVersion) return
         if (param != null) {
             checkVersion(expectedVersion)
         }

core/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/Extensions.kt

@@ -0,0 +1,101 @@
+package org.moire.ultrasonic.api.subsonic
+
+import com.fasterxml.jackson.module.kotlin.readValue
+import java.io.IOException
+import okhttp3.ResponseBody
+import org.moire.ultrasonic.api.subsonic.response.StreamResponse
+import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse
+import retrofit2.Response
+
+/**
+ * Converts a Response to a StreamResponse
+ */
+fun Response<out ResponseBody>.toStreamResponse(): StreamResponse {
+    val response = this
+    return if (response.isSuccessful) {
+        val responseBody = response.body()
+        val contentType = responseBody?.contentType()
+        if (
+            contentType != null &&
+            contentType.type().equals("application", true) &&
+            contentType.subtype().equals("json", true)
+        ) {
+            val error = SubsonicAPIClient.jacksonMapper.readValue<SubsonicResponse>(
+                responseBody.byteStream()
+            )
+            StreamResponse(apiError = error.error, responseHttpCode = response.code())
+        } else {
+            StreamResponse(
+                stream = responseBody?.byteStream(),
+                responseHttpCode = response.code()
+            )
+        }
+    } else {
+        StreamResponse(responseHttpCode = response.code())
+    }
+}
+
+/**
+ * This extension checks API call results for errors, API version, etc
+ * It creates Exceptions from the results returned by the Subsonic API
+ */
+@Suppress("ThrowsCount")
+fun <T : SubsonicResponse> Response<out T>.throwOnFailure(): Response<out T> {
+    val response = this
+
+    if (response.isSuccessful && response.body()!!.status === SubsonicResponse.Status.OK) {
+        return this as Response<T>
+    }
+    if (!response.isSuccessful) {
+        throw IOException("Server error, code: " + response.code())
+    } else if (
+        response.body()!!.status === SubsonicResponse.Status.ERROR &&
+        response.body()!!.error != null
+    ) {
+        throw SubsonicRESTException(response.body()!!.error!!)
+    } else {
+        throw IOException("Failed to perform request: " + response.code())
+    }
+}
+
+/**
+ * This extension checks API call results for errors, API version, etc
+ * @return Boolean: True if everything was ok, false if an error was found
+ */
+fun Response<out SubsonicResponse>.falseOnFailure(): Boolean {
+    return (this.isSuccessful && this.body()!!.status === SubsonicResponse.Status.OK)
+}
+
+/**
+ * This call wraps Subsonic API calls so their results can be checked for errors, API version, etc
+ * It creates Exceptions from a StreamResponse
+ */
+fun StreamResponse.throwOnFailure(): StreamResponse {
+    val response = this
+    if (response.hasError() || response.stream == null) {
+        if (response.apiError != null) {
+            throw SubsonicRESTException(response.apiError)
+        } else {
+            throw IOException(
+                "Failed to make endpoint request, code: " + response.responseHttpCode
+            )
+        }
+    }
+    return this
+}
+
+/**
+ * Gets a stream url.
+ *
+ * Calling this method do actual connection to the backend, though not downloading all content.
+ *
+ * Consider do not use this method, but [SubsonicAPIDefinition.stream] call.
+ */
+fun SubsonicAPIDefinition.getStreamUrl(id: String): String {
+    val response = this.stream(id, format = "raw").execute()
+    val url = response.raw().request().url().toString()
+    if (response.isSuccessful) {
+        response.body()?.close()
+    }
+    return url
+}

core/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIClient.kt

@@ -3,7 +3,6 @@ package org.moire.ultrasonic.api.subsonic
 import com.fasterxml.jackson.databind.DeserializationFeature
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.module.kotlin.KotlinModule
-import com.fasterxml.jackson.module.kotlin.readValue
 import java.security.SecureRandom
 import java.security.cert.X509Certificate
 import java.util.concurrent.TimeUnit.MILLISECONDS
@@ -18,7 +17,6 @@ import org.moire.ultrasonic.api.subsonic.interceptors.ProxyPasswordInterceptor
 import org.moire.ultrasonic.api.subsonic.interceptors.RangeHeaderInterceptor
 import org.moire.ultrasonic.api.subsonic.interceptors.VersionInterceptor
 import org.moire.ultrasonic.api.subsonic.response.StreamResponse
-import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse
 import retrofit2.Response
 import retrofit2.Retrofit
 
@@ -48,15 +46,20 @@ class SubsonicAPIClient(
         config.enableLdapUserSupport
     )
 
+    var onProtocolChange: (SubsonicAPIVersions) -> Unit = {}
+
     /**
-     * Get currently used protocol version.
+     * The currently used protocol version.
+     * The setter also updates the interceptors and callback (if registered)
      */
     var protocolVersion = config.minimalProtocolVersion
         private set(value) {
             field = value
             proxyPasswordInterceptor.apiVersion = field
             wrappedApi.currentApiVersion = field
+            wrappedApi.isRealProtocolVersion = true
             versionInterceptor.protocolVersion = field
+            onProtocolChange(field)
         }
 
     private val okHttpClient = baseOkClient.newBuilder()
@@ -78,18 +81,19 @@ class SubsonicAPIClient(
         .apply { if (config.debug) addLogging() }
         .build()
 
-    private val jacksonMapper = ObjectMapper()
+    // Create the Retrofit instance, and register a special converter factory
-        .configure(DeserializationFeature.UNWRAP_ROOT_VALUE, true)
+    // It will update our protocol version to the correct version, once we made a successful call
-        .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
-        .configure(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY, true)
-        .registerModule(KotlinModule())
-
     private val retrofit = Retrofit.Builder()
         .baseUrl("${config.baseUrl}/rest/")
         .client(okHttpClient)
         .addConverterFactory(
             VersionAwareJacksonConverterFactory.create(
-                { protocolVersion = it },
+                {
+                    // Only trigger update on change, or if still using the default
+                    if (protocolVersion != it || !config.isRealProtocolVersion) {
+                        protocolVersion = it
+                    }
+                },
                 jacksonMapper
             )
         )
@@ -97,90 +101,12 @@ class SubsonicAPIClient(
 
     private val wrappedApi = ApiVersionCheckWrapper(
         retrofit.create(SubsonicAPIDefinition::class.java),
-        config.minimalProtocolVersion
+        config.minimalProtocolVersion,
+        config.isRealProtocolVersion
     )
 
     val api: SubsonicAPIDefinition get() = wrappedApi
 
-    /**
-     * TODO: Remove this in favour of handling the stream response inside RESTService
-     * Convenient method to get cover art from api using item [id] and optional maximum [size].
-     *
-     * It detects the response `Content-Type` and tries to parse subsonic error if there is one.
-     *
-     * Prefer this method over [SubsonicAPIDefinition.getCoverArt] as this handles error cases.
-     */
-    fun getCoverArt(id: String, size: Long? = null): StreamResponse = handleStreamResponse {
-        api.getCoverArt(id, size).execute()
-    }
-
-    /**
-     * TODO: Remove this in favour of handling the stream response inside RESTService
-     * Convenient method to get media stream from api using item [id] and optional [maxBitrate].
-     *
-     * Optionally also you can provide [offset] that stream should start from.
-     *
-     * It detects the response `Content-Type` and tries to parse subsonic error if there is one.
-     *
-     * Prefer this method over [SubsonicAPIDefinition.stream] as this handles error cases.
-     */
-    fun stream(id: String, maxBitrate: Int? = null, offset: Long? = null): StreamResponse =
-        handleStreamResponse {
-            api.stream(id, maxBitrate, offset = offset).execute()
-        }
-
-    /**
-     * TODO: Remove this in favour of handling the stream response inside RESTService
-     * Convenient method to get user avatar using [username].
-     *
-     * It detects the response `Content-Type` and tries to parse subsonic error if there is one.
-     *
-     * Prefer this method over [SubsonicAPIDefinition.getAvatar] as this handles error cases.
-     */
-    fun getAvatar(username: String): StreamResponse = handleStreamResponse {
-        api.getAvatar(username).execute()
-    }
-
-    // TODO: Move this to response checker
-    private inline fun handleStreamResponse(apiCall: () -> Response<ResponseBody>): StreamResponse {
-        val response = apiCall()
-        return if (response.isSuccessful) {
-            val responseBody = response.body()
-            val contentType = responseBody?.contentType()
-            if (
-                contentType != null &&
-                contentType.type().equals("application", true) &&
-                contentType.subtype().equals("json", true)
-            ) {
-                val error = jacksonMapper.readValue<SubsonicResponse>(responseBody.byteStream())
-                StreamResponse(apiError = error.error, responseHttpCode = response.code())
-            } else {
-                StreamResponse(
-                    stream = responseBody?.byteStream(),
-                    responseHttpCode = response.code()
-                )
-            }
-        } else {
-            StreamResponse(responseHttpCode = response.code())
-        }
-    }
-
-    /**
-     * Get stream url.
-     *
-     * Calling this method do actual connection to the backend, though not downloading all content.
-     *
-     * Consider do not use this method, but [stream] call.
-     */
-    fun getStreamUrl(id: String): String {
-        val request = api.stream(id).execute()
-        val url = request.raw().request().url().toString()
-        if (request.isSuccessful) {
-            request.body()?.close()
-        }
-        return url
-    }
-
     private fun OkHttpClient.Builder.addLogging() {
         val loggingInterceptor = HttpLoggingInterceptor(okLogger)
         loggingInterceptor.level = HttpLoggingInterceptor.Level.BODY
@@ -202,4 +128,19 @@ class SubsonicAPIClient(
 
         hostnameVerifier { _, _ -> true }
     }
+
+    /**
+     * This function is necessary because Mockito has problems with stubbing chained calls
+     */
+    fun toStreamResponse(call: Response<ResponseBody>): StreamResponse {
+        return call.toStreamResponse()
+    }
+
+    companion object {
+        val jacksonMapper: ObjectMapper = ObjectMapper()
+            .configure(DeserializationFeature.UNWRAP_ROOT_VALUE, true)
+            .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
+            .configure(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY, true)
+            .registerModule(KotlinModule())
+    }
 }

core/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicClientConfiguration.kt

@@ -11,5 +11,6 @@ data class SubsonicClientConfiguration(
     val clientID: String,
     val allowSelfSignedCertificate: Boolean = false,
     val enableLdapUserSupport: Boolean = false,
-    val debug: Boolean = false
+    val debug: Boolean = false,
+    val isRealProtocolVersion: Boolean = false
 )

core/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicRESTException.kt

@@ -1,6 +1,4 @@
-package org.moire.ultrasonic.service
+package org.moire.ultrasonic.api.subsonic
-
-import org.moire.ultrasonic.api.subsonic.SubsonicError
 
 /**
  * Exception returned by API with given `code`.

core/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/VersionAwareJacksonConverterFactory.kt

@@ -63,7 +63,6 @@ class VersionAwareJacksonConverterFactory(
         }
     }
 
-    @Suppress("SwallowedException")
     class VersionAwareResponseBodyConverter<T> (
         private val notifier: (SubsonicAPIVersions) -> Unit = {},
         private val adapter: ObjectReader
@@ -77,7 +76,7 @@ class VersionAwareJacksonConverterFactory(
                 if (response is SubsonicResponse) {
                     try {
                         notifier(response.version)
-                    } catch (e: IllegalArgumentException) {
+                    } catch (ignored: IllegalArgumentException) {
                         // no-op
                     }
                 }

core/subsonic-api/src/test/kotlin/org/moire/ultrasonic/api/subsonic/ApiVersionCheckWrapperTest.kt

@@ -14,7 +14,7 @@ import org.moire.ultrasonic.api.subsonic.models.AlbumListType.BY_GENRE
  */
 class ApiVersionCheckWrapperTest {
     private val apiMock = mock<SubsonicAPIDefinition>()
-    private val wrapper = ApiVersionCheckWrapper(apiMock, V1_1_0)
+    private val wrapper = ApiVersionCheckWrapper(apiMock, V1_1_0, isRealProtocolVersion = true)
 
     @Test
     fun `Should just call real api for ping`() {

ultrasonic/src/main/java/org/moire/ultrasonic/service/JukeboxMediaPlayer.java

@@ -30,6 +30,7 @@ import android.widget.Toast;
 import org.jetbrains.annotations.NotNull;
 import org.moire.ultrasonic.R;
 import org.moire.ultrasonic.api.subsonic.ApiNotSupportedException;
+import org.moire.ultrasonic.api.subsonic.SubsonicRESTException;
 import org.moire.ultrasonic.app.UApp;
 import org.moire.ultrasonic.data.ActiveServerProvider;
 import org.moire.ultrasonic.domain.JukeboxStatus;

ultrasonic/src/main/kotlin/org/moire/ultrasonic/data/ActiveServerProvider.kt

@@ -37,12 +37,15 @@ class ActiveServerProvider(
                     cachedServer = repository.findById(serverId)
                 }
                 Timber.d(
-                    "getActiveServer retrieved from DataBase, id: $serverId; " +
+                    "getActiveServer retrieved from DataBase, id: %s cachedServer: %s",
-                        "cachedServer: $cachedServer"
+                    serverId, cachedServer
                 )
             }
 
-            if (cachedServer != null) return cachedServer!!
+            if (cachedServer != null) {
+                return cachedServer!!
+            }
+
             setActiveServerId(0)
         }
 
@@ -105,7 +108,7 @@ class ActiveServerProvider(
      * @param method: The Rest resource to use
      * @return The Rest Url of the method on the server
      */
-    fun getRestUrl(method: String?): String? {
+    fun getRestUrl(method: String?): String {
         val builder = StringBuilder(8192)
         val activeServer = getActiveServer()
         val serverUrl: String = activeServer.url

ultrasonic/src/main/kotlin/org/moire/ultrasonic/di/MusicServiceModule.kt

@@ -14,7 +14,6 @@ import org.moire.ultrasonic.cache.PermanentFileStorage
 import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.imageloader.ImageLoader
 import org.moire.ultrasonic.log.TimberOkHttpLogger
-import org.moire.ultrasonic.service.ApiCallResponseChecker
 import org.moire.ultrasonic.service.CachedMusicService
 import org.moire.ultrasonic.service.MusicService
 import org.moire.ultrasonic.service.OfflineMusicService
@@ -50,28 +49,29 @@ val musicServiceModule = module {
     }
 
     single {
+        val server = get<ActiveServerProvider>().getActiveServer()
+
         return@single SubsonicClientConfiguration(
-            baseUrl = get<ActiveServerProvider>().getActiveServer().url,
+            baseUrl = server.url,
-            username = get<ActiveServerProvider>().getActiveServer().userName,
+            username = server.userName,
-            password = get<ActiveServerProvider>().getActiveServer().password,
+            password = server.password,
             minimalProtocolVersion = SubsonicAPIVersions.getClosestKnownClientApiVersion(
-                get<ActiveServerProvider>().getActiveServer().minimumApiVersion
+                server.minimumApiVersion
                     ?: Constants.REST_PROTOCOL_VERSION
             ),
             clientID = Constants.REST_CLIENT_ID,
-            allowSelfSignedCertificate = get<ActiveServerProvider>()
+            allowSelfSignedCertificate = server.allowSelfSignedCertificate,
-                .getActiveServer().allowSelfSignedCertificate,
+            enableLdapUserSupport = server.ldapSupport,
-            enableLdapUserSupport = get<ActiveServerProvider>().getActiveServer().ldapSupport,
+            debug = BuildConfig.DEBUG,
-            debug = BuildConfig.DEBUG
+            isRealProtocolVersion = server.minimumApiVersion != null
         )
     }
 
     single<HttpLoggingInterceptor.Logger> { TimberOkHttpLogger() }
     single { SubsonicAPIClient(get(), get()) }
-    single { ApiCallResponseChecker(get(), get()) }
 
     single<MusicService>(named(ONLINE_MUSIC_SERVICE)) {
-        CachedMusicService(RESTMusicService(get(), get(), get(), get()))
+        CachedMusicService(RESTMusicService(get(), get(), get()))
     }
 
     single<MusicService>(named(OFFLINE_MUSIC_SERVICE)) {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/EditServerFragment.kt

@@ -22,12 +22,13 @@ import org.moire.ultrasonic.R
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIVersions
 import org.moire.ultrasonic.api.subsonic.SubsonicClientConfiguration
+import org.moire.ultrasonic.api.subsonic.SubsonicRESTException
+import org.moire.ultrasonic.api.subsonic.falseOnFailure
 import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse
+import org.moire.ultrasonic.api.subsonic.throwOnFailure
 import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.data.ServerSetting
-import org.moire.ultrasonic.service.ApiCallResponseChecker
 import org.moire.ultrasonic.service.MusicServiceFactory
-import org.moire.ultrasonic.service.SubsonicRESTException
 import org.moire.ultrasonic.util.Constants
 import org.moire.ultrasonic.util.ErrorDialog
 import org.moire.ultrasonic.util.ModalBackgroundTask
@@ -360,7 +361,7 @@ class EditServerFragment : Fragment(), OnBackPressedHandler {
 
                 // Execute a ping to check the authentication, now using the correct API version.
                 pingResponse = subsonicApiClient.api.ping().execute()
-                ApiCallResponseChecker.checkResponseSuccessful(pingResponse)
+                pingResponse.throwOnFailure()
 
                 currentServerSetting!!.chatSupport = isServerFunctionAvailable {
                     subsonicApiClient.api.getChatMessages().execute()
@@ -387,7 +388,8 @@ class EditServerFragment : Fragment(), OnBackPressedHandler {
                 updateProgress(getProgress())
 
                 val licenseResponse = subsonicApiClient.api.getLicense().execute()
-                ApiCallResponseChecker.checkResponseSuccessful(licenseResponse)
+                licenseResponse.throwOnFailure()
+
                 if (!licenseResponse.body()!!.license.valid) {
                     return getProgress() + "\n" +
                         resources.getString(R.string.settings_testing_unlicensed)
@@ -438,9 +440,7 @@ class EditServerFragment : Fragment(), OnBackPressedHandler {
 
     private fun isServerFunctionAvailable(function: () -> Response<out SubsonicResponse>): Boolean {
         return try {
-            val response = function()
+            function().falseOnFailure()
-            ApiCallResponseChecker.checkResponseSuccessful(response)
-            true
         } catch (_: IOException) {
             false
         } catch (_: SubsonicRESTException) {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/GenericListModel.kt

@@ -10,8 +10,6 @@ import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.viewModelScope
 import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
-import java.net.ConnectException
-import java.net.UnknownHostException
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
@@ -89,10 +87,8 @@ open class GenericListModel(application: Application) :
 
             try {
                 load(isOffline, useId3Tags, musicService, refresh, bundle)
-            } catch (exception: ConnectException) {
+            } catch (all: Exception) {
-                handleException(exception, swipe.context)
+                handleException(all, swipe.context)
-            } catch (exception: UnknownHostException) {
-                handleException(exception, swipe.context)
             }
         }
 

ultrasonic/src/main/kotlin/org/moire/ultrasonic/imageloader/AvatarRequestHandler.kt

@@ -11,7 +11,7 @@ import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
  * Loads avatars from subsonic api.
  */
 class AvatarRequestHandler(
-    private val apiClient: SubsonicAPIClient
+    private val client: SubsonicAPIClient
 ) : RequestHandler() {
     override fun canHandleRequest(data: Request): Boolean {
         return with(data.uri) {
@@ -23,7 +23,9 @@ class AvatarRequestHandler(
         val username = request.uri.getQueryParameter(QUERY_USERNAME)
             ?: throw IllegalArgumentException("Nullable username")
 
-        val response = apiClient.getAvatar(username)
+        // Inverted call order, because Mockito has problems with chained calls.
+        val response = client.toStreamResponse(client.api.getAvatar(username).execute())
+
         if (response.hasError() || response.stream == null) {
             throw IOException("${response.apiError}")
         } else {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/imageloader/CoverArtRequestHandler.kt

@@ -7,13 +7,14 @@ import com.squareup.picasso.RequestHandler
 import java.io.IOException
 import okio.Okio
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
+import org.moire.ultrasonic.api.subsonic.toStreamResponse
 import org.moire.ultrasonic.util.FileUtil.SUFFIX_LARGE
 import org.moire.ultrasonic.util.FileUtil.SUFFIX_SMALL
 
 /**
  * Loads cover arts from subsonic api.
  */
-class CoverArtRequestHandler(private val apiClient: SubsonicAPIClient) : RequestHandler() {
+class CoverArtRequestHandler(private val client: SubsonicAPIClient) : RequestHandler() {
     override fun canHandleRequest(data: Request): Boolean {
         return with(data.uri) {
             scheme == SCHEME &&
@@ -38,7 +39,10 @@ class CoverArtRequestHandler(private val apiClient: SubsonicAPIClient) : Request
         }
 
         // Try to fetch the image from the API
-        val response = apiClient.getCoverArt(id, size)
+        // Inverted call order, because Mockito has problems with chained calls.
+        val response = client.toStreamResponse(client.api.getCoverArt(id, size).execute())
+
+        // Handle the response
         if (!response.hasError() && response.stream != null) {
             return Result(Okio.source(response.stream!!), NETWORK)
         }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/imageloader/ImageLoader.kt

@@ -13,8 +13,9 @@ import java.io.OutputStream
 import org.moire.ultrasonic.BuildConfig
 import org.moire.ultrasonic.R
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
+import org.moire.ultrasonic.api.subsonic.throwOnFailure
+import org.moire.ultrasonic.api.subsonic.toStreamResponse
 import org.moire.ultrasonic.domain.MusicDirectory
-import org.moire.ultrasonic.service.RESTMusicService
 import org.moire.ultrasonic.util.FileUtil
 import org.moire.ultrasonic.util.Util
 import timber.log.Timber
@@ -24,9 +25,12 @@ import timber.log.Timber
  */
 class ImageLoader(
     context: Context,
-    private val apiClient: SubsonicAPIClient,
+    apiClient: SubsonicAPIClient,
     private val config: ImageLoaderConfig
 ) {
+    // Shortcut
+    @Suppress("VariableNaming", "PropertyName")
+    val API = apiClient.api
 
     private val picasso = Picasso.Builder(context)
         .addRequestHandler(CoverArtRequestHandler(apiClient))
@@ -143,8 +147,8 @@ class ImageLoader(
 
             // Query the API
             Timber.d("Loading cover art for: %s", entry)
-            val response = apiClient.getCoverArt(id!!, size.toLong())
+            val response = API.getCoverArt(id!!, size.toLong()).execute().toStreamResponse()
-            RESTMusicService.checkStreamResponseError(response)
+            response.throwOnFailure()
 
             // Check for failure
             if (response.stream == null) return

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/ApiCallResponseChecker.kt

@@ -1,66 +0,0 @@
-package org.moire.ultrasonic.service
-
-import java.io.IOException
-import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
-import org.moire.ultrasonic.api.subsonic.SubsonicAPIDefinition
-import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse
-import org.moire.ultrasonic.data.ActiveServerProvider
-import retrofit2.Response
-import timber.log.Timber
-
-/**
- * This call wraps Subsonic API calls so their results can be checked for errors, API version, etc
- */
-class ApiCallResponseChecker(
-    private val subsonicAPIClient: SubsonicAPIClient,
-    private val activeServerProvider: ActiveServerProvider
-) {
-    /**
-     * Executes a Subsonic API call with response check
-     */
-    @Throws(SubsonicRESTException::class, IOException::class)
-    fun <T : Response<out SubsonicResponse>> callWithResponseCheck(
-        call: (SubsonicAPIDefinition) -> T
-    ): T {
-        // Check for API version when first contacting the server
-        if (activeServerProvider.getActiveServer().minimumApiVersion == null) {
-            try {
-                val response = subsonicAPIClient.api.ping().execute()
-                if (response.body() != null) {
-                    val restApiVersion = response.body()!!.version.restApiVersion
-                    Timber.i("Server minimum API version set to %s", restApiVersion)
-                    activeServerProvider.setMinimumApiVersion(restApiVersion)
-                }
-            } catch (ignored: Exception) {
-                // This Ping is only used to get the API Version, if it fails, that's no problem.
-            }
-        }
-
-        // This call will be now executed with the correct API Version, so it shouldn't fail
-        val result = call.invoke(subsonicAPIClient.api)
-        checkResponseSuccessful(result)
-        return result
-    }
-
-    /**
-     * Creates Exceptions from the results returned by the Subsonic API
-     */
-    companion object {
-        @Throws(SubsonicRESTException::class, IOException::class)
-        fun checkResponseSuccessful(response: Response<out SubsonicResponse>) {
-            if (response.isSuccessful && response.body()!!.status === SubsonicResponse.Status.OK) {
-                return
-            }
-            if (!response.isSuccessful) {
-                throw IOException("Server error, code: " + response.code())
-            } else if (
-                response.body()!!.status === SubsonicResponse.Status.ERROR &&
-                response.body()!!.error != null
-            ) {
-                throw SubsonicRESTException(response.body()!!.error!!)
-            } else {
-                throw IOException("Failed to perform request: " + response.code())
-            }
-        }
-    }
-}

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/CommunicationErrorHandler.kt

@@ -28,6 +28,7 @@ import java.security.cert.CertificateException
 import javax.net.ssl.SSLException
 import org.moire.ultrasonic.R
 import org.moire.ultrasonic.api.subsonic.ApiNotSupportedException
+import org.moire.ultrasonic.api.subsonic.SubsonicRESTException
 import org.moire.ultrasonic.subsonic.getLocalizedErrorMessage
 import org.moire.ultrasonic.util.Util
 import timber.log.Timber

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RESTMusicService.kt

@@ -13,11 +13,14 @@ import java.io.IOException
 import java.io.InputStream
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.TimeUnit
+import java.util.concurrent.TimeoutException
 import org.moire.ultrasonic.api.subsonic.ApiNotSupportedException
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
+import org.moire.ultrasonic.api.subsonic.getStreamUrl
 import org.moire.ultrasonic.api.subsonic.models.AlbumListType.Companion.fromName
 import org.moire.ultrasonic.api.subsonic.models.JukeboxAction
-import org.moire.ultrasonic.api.subsonic.response.StreamResponse
+import org.moire.ultrasonic.api.subsonic.throwOnFailure
+import org.moire.ultrasonic.api.subsonic.toStreamResponse
 import org.moire.ultrasonic.cache.PermanentFileStorage
 import org.moire.ultrasonic.cache.serializers.getIndexesSerializer
 import org.moire.ultrasonic.cache.serializers.getMusicFolderListSerializer
@@ -50,20 +53,23 @@ import timber.log.Timber
  */
 @Suppress("LargeClass")
 open class RESTMusicService(
-    private val subsonicAPIClient: SubsonicAPIClient,
+    subsonicAPIClient: SubsonicAPIClient,
     private val fileStorage: PermanentFileStorage,
-    private val activeServerProvider: ActiveServerProvider,
+    private val activeServerProvider: ActiveServerProvider
-    private val responseChecker: ApiCallResponseChecker
 ) : MusicService {
 
+    // Shortcut to the API
+    @Suppress("VariableNaming", "PropertyName")
+    val API = subsonicAPIClient.api
+
     @Throws(Exception::class)
     override fun ping() {
-        responseChecker.callWithResponseCheck { api -> api.ping().execute() }
+        API.ping().execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
     override fun isLicenseValid(): Boolean {
-        val response = responseChecker.callWithResponseCheck { api -> api.getLicense().execute() }
+        val response = API.getLicense().execute().throwOnFailure()
 
         return response.body()!!.license.valid
     }
@@ -78,9 +84,7 @@ open class RESTMusicService(
 
         if (cachedMusicFolders != null && !refresh) return cachedMusicFolders
 
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getMusicFolders().execute().throwOnFailure()
-            api.getMusicFolders().execute()
-        }
 
         val musicFolders = response.body()!!.musicFolders.toDomainEntityList()
         fileStorage.store(MUSIC_FOLDER_STORAGE_NAME, musicFolders, getMusicFolderListSerializer())
@@ -98,9 +102,7 @@ open class RESTMusicService(
         val cachedIndexes = fileStorage.load(indexName, getIndexesSerializer())
         if (cachedIndexes != null && !refresh) return cachedIndexes
 
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getIndexes(musicFolderId, null).execute().throwOnFailure()
-            api.getIndexes(musicFolderId, null).execute()
-        }
 
         val indexes = response.body()!!.indexes.toDomainEntity()
         fileStorage.store(indexName, indexes, getIndexesSerializer())
@@ -114,9 +116,7 @@ open class RESTMusicService(
         val cachedArtists = fileStorage.load(ARTISTS_STORAGE_NAME, getIndexesSerializer())
         if (cachedArtists != null && !refresh) return cachedArtists
 
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getArtists(null).execute().throwOnFailure()
-            api.getArtists(null).execute()
-        }
 
         val indexes = response.body()!!.indexes.toDomainEntity()
         fileStorage.store(ARTISTS_STORAGE_NAME, indexes, getIndexesSerializer())
@@ -129,7 +129,7 @@ open class RESTMusicService(
         albumId: String?,
         artistId: String?
     ) {
-        responseChecker.callWithResponseCheck { api -> api.star(id, albumId, artistId).execute() }
+        API.star(id, albumId, artistId).execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
@@ -138,7 +138,7 @@ open class RESTMusicService(
         albumId: String?,
         artistId: String?
     ) {
-        responseChecker.callWithResponseCheck { api -> api.unstar(id, albumId, artistId).execute() }
+        API.unstar(id, albumId, artistId).execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
@@ -146,7 +146,7 @@ open class RESTMusicService(
         id: String,
         rating: Int
     ) {
-        responseChecker.callWithResponseCheck { api -> api.setRating(id, rating).execute() }
+        API.setRating(id, rating).execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
@@ -155,9 +155,7 @@ open class RESTMusicService(
         name: String?,
         refresh: Boolean
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getMusicDirectory(id).execute().throwOnFailure()
-            api.getMusicDirectory(id).execute()
-        }
 
         return response.body()!!.musicDirectory.toDomainEntity()
     }
@@ -168,7 +166,7 @@ open class RESTMusicService(
         name: String?,
         refresh: Boolean
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api -> api.getArtist(id).execute() }
+        val response = API.getArtist(id).execute().throwOnFailure()
 
         return response.body()!!.artist.toMusicDirectoryDomainEntity()
     }
@@ -179,7 +177,7 @@ open class RESTMusicService(
         name: String?,
         refresh: Boolean
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api -> api.getAlbum(id).execute() }
+        val response = API.getAlbum(id).execute().throwOnFailure()
 
         return response.body()!!.album.toMusicDirectoryDomainEntity()
     }
@@ -207,10 +205,9 @@ open class RESTMusicService(
     private fun searchOld(
         criteria: SearchCriteria
     ): SearchResult {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response =
-            api.search(null, null, null, criteria.query, criteria.songCount, null, null)
+            API.search(null, null, null, criteria.query, criteria.songCount, null, null)
-                .execute()
+                .execute().throwOnFailure()
-        }
 
         return response.body()!!.searchResult.toDomainEntity()
     }
@@ -223,12 +220,10 @@ open class RESTMusicService(
         criteria: SearchCriteria
     ): SearchResult {
         requireNotNull(criteria.query) { "Query param is null" }
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.search2(
-            api.search2(
+            criteria.query, criteria.artistCount, null, criteria.albumCount, null,
-                criteria.query, criteria.artistCount, null, criteria.albumCount, null,
+            criteria.songCount, null
-                criteria.songCount, null
+        ).execute().throwOnFailure()
-            ).execute()
-        }
 
         return response.body()!!.searchResult.toDomainEntity()
     }
@@ -238,12 +233,10 @@ open class RESTMusicService(
         criteria: SearchCriteria
     ): SearchResult {
         requireNotNull(criteria.query) { "Query param is null" }
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.search3(
-            api.search3(
+            criteria.query, criteria.artistCount, null, criteria.albumCount, null,
-                criteria.query, criteria.artistCount, null, criteria.albumCount, null,
+            criteria.songCount, null
-                criteria.songCount, null
+        ).execute().throwOnFailure()
-            ).execute()
-        }
 
         return response.body()!!.searchResult.toDomainEntity()
     }
@@ -253,9 +246,7 @@ open class RESTMusicService(
         id: String,
         name: String
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getPlaylist(id).execute().throwOnFailure()
-            api.getPlaylist(id).execute()
-        }
 
         val playlist = response.body()!!.playlist.toMusicDirectoryDomainEntity()
         savePlaylist(name, playlist)
@@ -300,9 +291,7 @@ open class RESTMusicService(
     override fun getPlaylists(
         refresh: Boolean
     ): List<Playlist> {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getPlaylists(null).execute().throwOnFailure()
-            api.getPlaylists(null).execute()
-        }
 
         return response.body()!!.playlists.toDomainEntitiesList()
     }
@@ -318,16 +307,15 @@ open class RESTMusicService(
         for ((id1) in entries) {
             pSongIds.add(id1)
         }
-        responseChecker.callWithResponseCheck { api ->
+
-            api.createPlaylist(id, name, pSongIds.toList()).execute()
+        API.createPlaylist(id, name, pSongIds.toList()).execute().throwOnFailure()
-        }
     }
 
     @Throws(Exception::class)
     override fun deletePlaylist(
         id: String
     ) {
-        responseChecker.callWithResponseCheck { api -> api.deletePlaylist(id).execute() }
+        API.deletePlaylist(id).execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
@@ -337,19 +325,15 @@ open class RESTMusicService(
         comment: String?,
         pub: Boolean
     ) {
-        responseChecker.callWithResponseCheck { api ->
+        API.updatePlaylist(id, name, comment, pub, null, null)
-            api.updatePlaylist(id, name, comment, pub, null, null)
+            .execute().throwOnFailure()
-                .execute()
-        }
     }
 
     @Throws(Exception::class)
     override fun getPodcastsChannels(
         refresh: Boolean
     ): List<PodcastsChannel> {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getPodcasts(false, null).execute().throwOnFailure()
-            api.getPodcasts(false, null).execute()
-        }
 
         return response.body()!!.podcastChannels.toDomainEntitiesList()
     }
@@ -358,9 +342,7 @@ open class RESTMusicService(
     override fun getPodcastEpisodes(
         podcastChannelId: String?
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getPodcasts(true, podcastChannelId).execute().throwOnFailure()
-            api.getPodcasts(true, podcastChannelId).execute()
-        }
 
         val podcastEntries = response.body()!!.podcastChannels[0].episodeList
         val musicDirectory = MusicDirectory()
@@ -384,9 +366,7 @@ open class RESTMusicService(
         artist: String,
         title: String
     ): Lyrics {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getLyrics(artist, title).execute().throwOnFailure()
-            api.getLyrics(artist, title).execute()
-        }
 
         return response.body()!!.lyrics.toDomainEntity()
     }
@@ -396,9 +376,7 @@ open class RESTMusicService(
         id: String,
         submission: Boolean
     ) {
-        responseChecker.callWithResponseCheck { api ->
+        API.scrobble(id, null, submission).execute().throwOnFailure()
-            api.scrobble(id, null, submission).execute()
-        }
     }
 
     @Throws(Exception::class)
@@ -408,10 +386,15 @@ open class RESTMusicService(
         offset: Int,
         musicFolderId: String?
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getAlbumList(
-            api.getAlbumList(fromName(type), size, offset, null, null, null, musicFolderId)
+            fromName(type),
-                .execute()
+            size,
-        }
+            offset,
+            null,
+            null,
+            null,
+            musicFolderId
+        ).execute().throwOnFailure()
 
         val childList = response.body()!!.albumList.toDomainEntityList()
         val result = MusicDirectory()
@@ -427,17 +410,15 @@ open class RESTMusicService(
         offset: Int,
         musicFolderId: String?
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getAlbumList2(
-            api.getAlbumList2(
+            fromName(type),
-                fromName(type),
+            size,
-                size,
+            offset,
-                offset,
+            null,
-                null,
+            null,
-                null,
+            null,
-                null,
+            musicFolderId
-                musicFolderId
+        ).execute().throwOnFailure()
-            ).execute()
-        }
 
         val result = MusicDirectory()
         result.addAll(response.body()!!.albumList.toDomainEntityList())
@@ -449,15 +430,13 @@ open class RESTMusicService(
     override fun getRandomSongs(
         size: Int
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getRandomSongs(
-            api.getRandomSongs(
+            size,
-                size,
+            null,
-                null,
+            null,
-                null,
+            null,
-                null,
+            null
-                null
+        ).execute().throwOnFailure()
-            ).execute()
-        }
 
         val result = MusicDirectory()
         result.addAll(response.body()!!.songsList.toDomainEntityList())
@@ -467,18 +446,14 @@ open class RESTMusicService(
 
     @Throws(Exception::class)
     override fun getStarred(): SearchResult {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getStarred(null).execute().throwOnFailure()
-            api.getStarred(null).execute()
-        }
 
         return response.body()!!.starred.toDomainEntity()
     }
 
     @Throws(Exception::class)
     override fun getStarred2(): SearchResult {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getStarred2(null).execute().throwOnFailure()
-            api.getStarred2(null).execute()
-        }
 
         return response.body()!!.starred2.toDomainEntity()
     }
@@ -491,8 +466,10 @@ open class RESTMusicService(
     ): Pair<InputStream, Boolean> {
         val songOffset = if (offset < 0) 0 else offset
 
-        val response = subsonicAPIClient.stream(song.id, maxBitrate, songOffset)
+        val response = API.stream(song.id, maxBitrate, offset = songOffset)
-        checkStreamResponseError(response)
+            .execute().toStreamResponse()
+
+        response.throwOnFailure()
 
         if (response.stream == null) {
             throw IOException("Null stream response")
@@ -518,13 +495,18 @@ open class RESTMusicService(
 
         Thread(
             {
-                expectedResult[0] = subsonicAPIClient.getStreamUrl(id) + "&format=raw"
+                expectedResult[0] = API.getStreamUrl(id)
                 latch.countDown()
             },
             "Get-Video-Url"
         ).start()
 
-        latch.await(5, TimeUnit.SECONDS)
+        // Getting the stream can take a long time on some servers
+        latch.await(1, TimeUnit.MINUTES)
+
+        if (expectedResult[0] == null) {
+            throw TimeoutException("Server didn't respond in time")
+        }
 
         return expectedResult[0]!!
     }
@@ -533,10 +515,8 @@ open class RESTMusicService(
     override fun updateJukeboxPlaylist(
         ids: List<String>?
     ): JukeboxStatus {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.jukeboxControl(JukeboxAction.SET, null, null, ids, null)
-            api.jukeboxControl(JukeboxAction.SET, null, null, ids, null)
+            .execute().throwOnFailure()
-                .execute()
-        }
 
         return response.body()!!.jukebox.toDomainEntity()
     }
@@ -546,40 +526,32 @@ open class RESTMusicService(
         index: Int,
         offsetSeconds: Int
     ): JukeboxStatus {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.jukeboxControl(JukeboxAction.SKIP, index, offsetSeconds, null, null)
-            api.jukeboxControl(JukeboxAction.SKIP, index, offsetSeconds, null, null)
+            .execute().throwOnFailure()
-                .execute()
-        }
 
         return response.body()!!.jukebox.toDomainEntity()
     }
 
     @Throws(Exception::class)
     override fun stopJukebox(): JukeboxStatus {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.jukeboxControl(JukeboxAction.STOP, null, null, null, null)
-            api.jukeboxControl(JukeboxAction.STOP, null, null, null, null)
+            .execute().throwOnFailure()
-                .execute()
-        }
 
         return response.body()!!.jukebox.toDomainEntity()
     }
 
     @Throws(Exception::class)
     override fun startJukebox(): JukeboxStatus {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.jukeboxControl(JukeboxAction.START, null, null, null, null)
-            api.jukeboxControl(JukeboxAction.START, null, null, null, null)
+            .execute().throwOnFailure()
-                .execute()
-        }
 
         return response.body()!!.jukebox.toDomainEntity()
     }
 
     @Throws(Exception::class)
     override fun getJukeboxStatus(): JukeboxStatus {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.jukeboxControl(JukeboxAction.STATUS, null, null, null, null)
-            api.jukeboxControl(JukeboxAction.STATUS, null, null, null, null)
+            .execute().throwOnFailure()
-                .execute()
-        }
 
         return response.body()!!.jukebox.toDomainEntity()
     }
@@ -588,10 +560,8 @@ open class RESTMusicService(
     override fun setJukeboxGain(
         gain: Float
     ): JukeboxStatus {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.jukeboxControl(JukeboxAction.SET_GAIN, null, null, null, gain)
-            api.jukeboxControl(JukeboxAction.SET_GAIN, null, null, null, gain)
+            .execute().throwOnFailure()
-                .execute()
-        }
 
         return response.body()!!.jukebox.toDomainEntity()
     }
@@ -600,7 +570,7 @@ open class RESTMusicService(
     override fun getShares(
         refresh: Boolean
     ): List<Share> {
-        val response = responseChecker.callWithResponseCheck { api -> api.getShares().execute() }
+        val response = API.getShares().execute().throwOnFailure()
 
         return response.body()!!.shares.toDomainEntitiesList()
     }
@@ -609,7 +579,7 @@ open class RESTMusicService(
     override fun getGenres(
         refresh: Boolean
     ): List<Genre>? {
-        val response = responseChecker.callWithResponseCheck { api -> api.getGenres().execute() }
+        val response = API.getGenres().execute().throwOnFailure()
 
         return response.body()!!.genresList.toDomainEntityList()
     }
@@ -620,9 +590,7 @@ open class RESTMusicService(
         count: Int,
         offset: Int
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getSongsByGenre(genre, count, offset, null).execute().throwOnFailure()
-            api.getSongsByGenre(genre, count, offset, null).execute()
-        }
 
         val result = MusicDirectory()
         result.addAll(response.body()!!.songsList.toDomainEntityList())
@@ -634,9 +602,7 @@ open class RESTMusicService(
     override fun getUser(
         username: String
     ): UserInfo {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getUser(username).execute().throwOnFailure()
-            api.getUser(username).execute()
-        }
 
         return response.body()!!.user.toDomainEntity()
     }
@@ -645,9 +611,7 @@ open class RESTMusicService(
     override fun getChatMessages(
         since: Long?
     ): List<ChatMessage> {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.getChatMessages(since).execute().throwOnFailure()
-            api.getChatMessages(since).execute()
-        }
 
         return response.body()!!.chatMessages.toDomainEntitiesList()
     }
@@ -656,12 +620,12 @@ open class RESTMusicService(
     override fun addChatMessage(
         message: String
     ) {
-        responseChecker.callWithResponseCheck { api -> api.addChatMessage(message).execute() }
+        API.addChatMessage(message).execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
     override fun getBookmarks(): List<Bookmark> {
-        val response = responseChecker.callWithResponseCheck { api -> api.getBookmarks().execute() }
+        val response = API.getBookmarks().execute().throwOnFailure()
 
         return response.body()!!.bookmarkList.toDomainEntitiesList()
     }
@@ -671,23 +635,21 @@ open class RESTMusicService(
         id: String,
         position: Int
     ) {
-        responseChecker.callWithResponseCheck { api ->
+        API.createBookmark(id, position.toLong(), null).execute().throwOnFailure()
-            api.createBookmark(id, position.toLong(), null).execute()
-        }
     }
 
     @Throws(Exception::class)
     override fun deleteBookmark(
         id: String
     ) {
-        responseChecker.callWithResponseCheck { api -> api.deleteBookmark(id).execute() }
+        API.deleteBookmark(id).execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
     override fun getVideos(
         refresh: Boolean
     ): MusicDirectory {
-        val response = responseChecker.callWithResponseCheck { api -> api.getVideos().execute() }
+        val response = API.getVideos().execute().throwOnFailure()
 
         val musicDirectory = MusicDirectory()
         musicDirectory.addAll(response.body()!!.videosList.toDomainEntityList())
@@ -701,9 +663,7 @@ open class RESTMusicService(
         description: String?,
         expires: Long?
     ): List<Share> {
-        val response = responseChecker.callWithResponseCheck { api ->
+        val response = API.createShare(ids, description, expires).execute().throwOnFailure()
-            api.createShare(ids, description, expires).execute()
-        }
 
         return response.body()!!.shares.toDomainEntitiesList()
     }
@@ -712,7 +672,7 @@ open class RESTMusicService(
     override fun deleteShare(
         id: String
     ) {
-        responseChecker.callWithResponseCheck { api -> api.deleteShare(id).execute() }
+        API.deleteShare(id).execute().throwOnFailure()
     }
 
     @Throws(Exception::class)
@@ -726,8 +686,15 @@ open class RESTMusicService(
             expiresValue = null
         }
 
-        responseChecker.callWithResponseCheck { api ->
+        API.updateShare(id, description, expiresValue).execute().throwOnFailure()
-            api.updateShare(id, description, expiresValue).execute()
+    }
+
+    init {
+        // The client will notice if the minimum supported API version has changed
+        // By registering a callback we ensure this info is saved in the database as well
+        subsonicAPIClient.onProtocolChange = {
+            Timber.i("Server minimum API version set to %s", it)
+            activeServerProvider.setMinimumApiVersion(it.toString())
         }
     }
 
@@ -735,19 +702,5 @@ open class RESTMusicService(
         private const val MUSIC_FOLDER_STORAGE_NAME = "music_folder"
         private const val INDEXES_STORAGE_NAME = "indexes"
         private const val ARTISTS_STORAGE_NAME = "artists"
-
-        // TODO: Move to response checker
-        @Throws(SubsonicRESTException::class, IOException::class)
-        fun checkStreamResponseError(response: StreamResponse) {
-            if (response.hasError() || response.stream == null) {
-                if (response.apiError != null) {
-                    throw SubsonicRESTException(response.apiError!!)
-                } else {
-                    throw IOException(
-                        "Failed to make endpoint request, code: " + response.responseHttpCode
-                    )
-                }
-            }
-        }
     }
 }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/subsonic/RestErrorMapper.kt

@@ -12,7 +12,7 @@ import org.moire.ultrasonic.api.subsonic.SubsonicError.TokenAuthNotSupportedForL
 import org.moire.ultrasonic.api.subsonic.SubsonicError.TrialPeriodIsOver
 import org.moire.ultrasonic.api.subsonic.SubsonicError.UserNotAuthorizedForOperation
 import org.moire.ultrasonic.api.subsonic.SubsonicError.WrongUsernameOrPassword
-import org.moire.ultrasonic.service.SubsonicRESTException
+import org.moire.ultrasonic.api.subsonic.SubsonicRESTException
 
 /**
  * Extension for [SubsonicRESTException] that returns localized error string, that can used to
@@ -21,7 +21,7 @@ import org.moire.ultrasonic.service.SubsonicRESTException
 fun SubsonicRESTException.getLocalizedErrorMessage(context: Context): String =
     when (error) {
         is Generic -> {
-            val message = error.message
+            val message = (error as Generic).message
             val errorMessage = if (message == "") {
                 context.getString(R.string.api_subsonic_generic_no_message)
             } else {

ultrasonic/src/test/kotlin/org/moire/ultrasonic/imageloader/AvatarRequestHandlerTest.kt

@@ -9,18 +9,20 @@ import org.amshove.kluent.`should throw`
 import org.amshove.kluent.shouldBeEqualTo
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Answers
 import org.mockito.kotlin.any
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.whenever
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
 import org.moire.ultrasonic.api.subsonic.response.StreamResponse
+import org.moire.ultrasonic.api.subsonic.toStreamResponse
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
 
 @RunWith(RobolectricTestRunner::class)
 @Config(manifest = Config.NONE)
 class AvatarRequestHandlerTest {
-    private val mockApiClient: SubsonicAPIClient = mock()
+    private val mockApiClient: SubsonicAPIClient = mock(defaultAnswer = Answers.RETURNS_DEEP_STUBS)
     private val handler = AvatarRequestHandler(mockApiClient)
 
     @Test
@@ -59,8 +61,10 @@ class AvatarRequestHandlerTest {
             apiError = null,
             responseHttpCode = 200
         )
-        whenever(mockApiClient.getAvatar(any()))
+
-            .thenReturn(streamResponse)
+        whenever(
+            mockApiClient.toStreamResponse(any())
+        ).thenReturn(streamResponse)
 
         val response = handler.load(
             createLoadAvatarRequest("some-username").buildRequest(), 0

ultrasonic/src/test/kotlin/org/moire/ultrasonic/imageloader/CoverArtRequestHandlerTest.kt

@@ -10,8 +10,8 @@ import org.amshove.kluent.`should throw`
 import org.amshove.kluent.shouldBeEqualTo
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Answers
 import org.mockito.kotlin.any
-import org.mockito.kotlin.anyOrNull
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.whenever
 import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient
@@ -20,7 +20,7 @@ import org.robolectric.RobolectricTestRunner
 
 @RunWith(RobolectricTestRunner::class)
 class CoverArtRequestHandlerTest {
-    private val mockApiClient: SubsonicAPIClient = mock()
+    private val mockApiClient: SubsonicAPIClient = mock(defaultAnswer = Answers.RETURNS_DEEP_STUBS)
     private val handler = CoverArtRequestHandler(mockApiClient)
 
     @Test
@@ -56,7 +56,9 @@ class CoverArtRequestHandlerTest {
     fun `Should throw IOException when request to api failed`() {
         val streamResponse = StreamResponse(null, null, 500)
 
-        whenever(mockApiClient.getCoverArt(any(), anyOrNull())).thenReturn(streamResponse)
+        whenever(
+            mockApiClient.toStreamResponse(any())
+        ).thenReturn(streamResponse)
 
         val fail = {
             handler.load(createLoadCoverArtRequest("some").buildRequest(), 0)
@@ -73,7 +75,9 @@ class CoverArtRequestHandlerTest {
             responseHttpCode = 200
         )
 
-        whenever(mockApiClient.getCoverArt(any(), anyOrNull())).thenReturn(streamResponse)
+        whenever(
+            mockApiClient.toStreamResponse(any())
+        ).thenReturn(streamResponse)
 
         val response = handler.load(
             createLoadCoverArtRequest("some").buildRequest(), 0