diff --git a/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/CommonFunctions.kt b/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/CommonFunctions.kt index 09b47447..5c052f7f 100644 --- a/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/CommonFunctions.kt +++ b/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/CommonFunctions.kt @@ -25,11 +25,12 @@ val dateFormat by lazy(LazyThreadSafetyMode.NONE, { fun MockWebServerRule.enqueueResponse(resourceName: String) { this.mockWebServer.enqueue(MockResponse() - .setBody(loadJsonResponse(this, resourceName))) + .setBody(loadJsonResponse(resourceName)) + .setHeader("Content-Type", "application/json;charset=UTF-8")) } -private fun loadJsonResponse(rule: MockWebServerRule, name: String): String { - val source = Okio.buffer(Okio.source(rule.javaClass.classLoader.getResourceAsStream(name))) +fun MockWebServerRule.loadJsonResponse(name: String): String { + val source = Okio.buffer(Okio.source(javaClass.classLoader.getResourceAsStream(name))) return source.readString(Charset.forName("UTF-8")) } @@ -67,7 +68,7 @@ fun SubsonicResponse.assertBaseResponseOk() { fun MockWebServerRule.assertRequestParam(responseResourceName: String, expectedParam: String, - apiRequest: () -> Response) { + apiRequest: () -> Response) { this.enqueueResponse(responseResourceName) apiRequest() diff --git a/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicApiGetCoverArtTest.kt b/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicApiGetCoverArtTest.kt new file mode 100644 index 00000000..d0c6d647 --- /dev/null +++ b/subsonic-api/src/integrationTest/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicApiGetCoverArtTest.kt @@ -0,0 +1,74 @@ +package org.moire.ultrasonic.api.subsonic + +import okhttp3.mockwebserver.MockResponse +import org.amshove.kluent.`should be` +import org.amshove.kluent.`should equal to` +import org.amshove.kluent.`should equal` +import org.amshove.kluent.`should not be` +import org.junit.Test + +/** + * Integration test for [SubsonicAPIClient] for [SubsonicAPIDefinition.getCoverArt] call. + */ +class SubsonicApiGetCoverArtTest : SubsonicAPIClientTest() { + @Test + fun `Should handle api error response`() { + mockWebServerRule.enqueueResponse("generic_error_response.json") + + val response = client.getCoverArt("some-id") + + with(response) { + stream `should be` null + requestErrorCode `should be` null + apiError `should equal` SubsonicError.GENERIC + } + } + + @Test + fun `Should handle server error`() { + val httpErrorCode = 404 + mockWebServerRule.mockWebServer.enqueue(MockResponse().setResponseCode(httpErrorCode)) + + val response = client.getCoverArt("some-id") + + with(response) { + stream `should be` null + requestErrorCode `should equal` 404 + apiError `should be` null + } + } + + @Test + fun `Should return successful call stream`() { + mockWebServerRule.mockWebServer.enqueue(MockResponse() + .setBody(mockWebServerRule.loadJsonResponse("ping_ok.json"))) + + val response = client.getCoverArt("some-id") + + with(response) { + requestErrorCode `should be` null + apiError `should be` null + stream `should not be` null + val expectedContent = mockWebServerRule.loadJsonResponse("ping_ok.json") + stream!!.bufferedReader().readText() `should equal to` expectedContent + } + } + + @Test + fun `Should pass id as parameter`() { + val id = "ca123994" + + mockWebServerRule.assertRequestParam("ping_ok.json", id) { + client.api.getCoverArt(id).execute() + } + } + + @Test + fun `Should pass size as a parameter`() { + val size = 45600L + + mockWebServerRule.assertRequestParam("ping_ok.json", size.toString()) { + client.api.getCoverArt("some-id", size).execute() + } + } +} diff --git a/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIClient.kt b/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIClient.kt index dd89c8f4..11fc9041 100644 --- a/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIClient.kt +++ b/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIClient.kt @@ -3,9 +3,14 @@ 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 okhttp3.HttpUrl import okhttp3.OkHttpClient +import okhttp3.ResponseBody import okhttp3.logging.HttpLoggingInterceptor +import org.moire.ultrasonic.api.subsonic.response.StreamResponse +import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse +import retrofit2.Response import retrofit2.Retrofit import retrofit2.converter.jackson.JacksonConverterFactory import java.lang.IllegalStateException @@ -65,6 +70,35 @@ class SubsonicAPIClient(baseUrl: String, val api: SubsonicAPIDefinition = retrofit.create(SubsonicAPIDefinition::class.java) + /** + * 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() + } + + private inline fun handleStreamResponse(apiCall: () -> Response): 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(responseBody.byteStream()) + StreamResponse(apiError = error.error) + } else { + StreamResponse(stream = responseBody.byteStream()) + } + } else { + StreamResponse(requestErrorCode = response.code()) + } + } + private val salt: String by lazy { val secureRandom = SecureRandom() BigInteger(130, secureRandom).toString(32) diff --git a/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIDefinition.kt b/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIDefinition.kt index 28d1c952..35413f1a 100644 --- a/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIDefinition.kt +++ b/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/SubsonicAPIDefinition.kt @@ -1,5 +1,6 @@ package org.moire.ultrasonic.api.subsonic +import okhttp3.ResponseBody import org.moire.ultrasonic.api.subsonic.models.AlbumListType import org.moire.ultrasonic.api.subsonic.response.GetAlbumList2Response import org.moire.ultrasonic.api.subsonic.response.GetAlbumListResponse @@ -24,6 +25,7 @@ import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse import retrofit2.Call import retrofit2.http.GET import retrofit2.http.Query +import retrofit2.http.Streaming /** * Subsonic API calls. @@ -160,4 +162,9 @@ interface SubsonicAPIDefinition { @GET("getStarred2.view") fun getStarred2(@Query("musicFolderId") musicFolderId: Long? = null): Call + + @Streaming + @GET("getCoverArt.view") + fun getCoverArt(@Query("id") id: String, + @Query("size") size: Long? = null): Call } diff --git a/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/response/StreamResponse.kt b/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/response/StreamResponse.kt new file mode 100644 index 00000000..47292296 --- /dev/null +++ b/subsonic-api/src/main/kotlin/org/moire/ultrasonic/api/subsonic/response/StreamResponse.kt @@ -0,0 +1,19 @@ +package org.moire.ultrasonic.api.subsonic.response + +import org.moire.ultrasonic.api.subsonic.SubsonicError +import java.io.InputStream + +/** + * Special response that contains either [stream] of data from api, or [apiError], + * or [requestErrorCode]. + * + * [requestErrorCode] will be only if there problem on http level. + */ +class StreamResponse(val stream: InputStream? = null, + val apiError: SubsonicError? = null, + val requestErrorCode: Int? = null) { + /** + * Check if this response has error. + */ + fun hasError(): Boolean = apiError != null || requestErrorCode != null +} diff --git a/subsonic-api/src/test/kotlin/org/moire/ultrasonic/api/subsonic/response/StreamResponseTest.kt b/subsonic-api/src/test/kotlin/org/moire/ultrasonic/api/subsonic/response/StreamResponseTest.kt new file mode 100644 index 00000000..e8a2c694 --- /dev/null +++ b/subsonic-api/src/test/kotlin/org/moire/ultrasonic/api/subsonic/response/StreamResponseTest.kt @@ -0,0 +1,25 @@ +package org.moire.ultrasonic.api.subsonic.response + +import org.amshove.kluent.`should equal to` +import org.junit.Test +import org.moire.ultrasonic.api.subsonic.SubsonicError.GENERIC + +/** + * Unit test for [StreamResponse]. + */ +class StreamResponseTest { + @Test + fun `Should have error if subsonic error is not null`() { + StreamResponse(apiError = GENERIC).hasError() `should equal to` true + } + + @Test + fun `Should have error if http error is not null`() { + StreamResponse(requestErrorCode = 500).hasError() `should equal to` true + } + + @Test + fun `Should not have error if subsonic error and http error is null`() { + StreamResponse().hasError() `should equal to` false + } +} diff --git a/ultrasonic/src/main/java/org/moire/ultrasonic/service/RESTMusicService.java b/ultrasonic/src/main/java/org/moire/ultrasonic/service/RESTMusicService.java index 803eeed2..a24b1cc9 100644 --- a/ultrasonic/src/main/java/org/moire/ultrasonic/service/RESTMusicService.java +++ b/ultrasonic/src/main/java/org/moire/ultrasonic/service/RESTMusicService.java @@ -77,6 +77,7 @@ import org.moire.ultrasonic.api.subsonic.response.MusicFoldersResponse; import org.moire.ultrasonic.api.subsonic.response.SearchResponse; import org.moire.ultrasonic.api.subsonic.response.SearchThreeResponse; import org.moire.ultrasonic.api.subsonic.response.SearchTwoResponse; +import org.moire.ultrasonic.api.subsonic.response.StreamResponse; import org.moire.ultrasonic.api.subsonic.response.SubsonicResponse; import org.moire.ultrasonic.data.APIAlbumConverter; import org.moire.ultrasonic.data.APIArtistConverter; @@ -110,6 +111,7 @@ import org.moire.ultrasonic.service.parser.JukeboxStatusParser; import org.moire.ultrasonic.service.parser.MusicDirectoryParser; import org.moire.ultrasonic.service.parser.RandomSongsParser; import org.moire.ultrasonic.service.parser.ShareParser; +import org.moire.ultrasonic.service.parser.SubsonicRESTException; import org.moire.ultrasonic.service.parser.UserInfoParser; import org.moire.ultrasonic.service.ssl.SSLSocketFactory; import org.moire.ultrasonic.service.ssl.TrustSelfSignedStrategy; @@ -756,84 +758,73 @@ public class RESTMusicService implements MusicService return serverVersion == null || serverVersion.compareTo(requiredVersion) >= 0; } - @Override - public Bitmap getCoverArt(Context context, final MusicDirectory.Entry entry, int size, boolean saveToFile, boolean highQuality, ProgressListener progressListener) throws Exception - { - // Synchronize on the entry so that we don't download concurrently for - // the same song. - if (entry == null) - { - return null; - } + @Override + public Bitmap getCoverArt(Context context, + final MusicDirectory.Entry entry, + int size, + boolean saveToFile, + boolean highQuality, + ProgressListener progressListener) throws Exception { + // Synchronize on the entry so that we don't download concurrently for + // the same song. + if (entry == null) { + return null; + } - synchronized (entry) - { - // Use cached file, if existing. - Bitmap bitmap = FileUtil.getAlbumArtBitmap(context, entry, size, highQuality); - boolean serverScaling = Util.isServerScalingEnabled(context); + synchronized (entry) { + // Use cached file, if existing. + Bitmap bitmap = FileUtil.getAlbumArtBitmap(context, entry, size, highQuality); + boolean serverScaling = Util.isServerScalingEnabled(context); - if (bitmap == null) - { - String url = Util.getRestUrl(context, "getCoverArt"); + if (bitmap == null) { + Log.d(TAG, "Loading cover art for: " + entry); - InputStream in = null; - try - { - List parameterNames; - List parameterValues; + final String id = entry.getCoverArt(); + if (id == null) { + return null; // Can't load + } - if (serverScaling) - { - parameterNames = asList("id", "size"); - parameterValues = Arrays.asList(entry.getCoverArt(), size); - } - else - { - parameterNames = Collections.singletonList("id"); - parameterValues = Arrays.asList(entry.getCoverArt()); - } + StreamResponse response = subsonicAPIClient.getCoverArt(id, (long) size); + if (response.hasError() || response.getStream() == null) { + if (response.getApiError() != null) { + throw new SubsonicRESTException(response.getApiError().getCode(), "rest error"); + } else { + throw new IOException("Failed to make endpoint request, code: " + + response.getRequestErrorCode()); + } + } - HttpEntity entity = getEntityForURL(context, url, null, parameterNames, parameterValues, progressListener); - in = entity.getContent(); + if (response.getStream() == null) { + return null; // Failed to load + } - // If content type is XML, an error occurred. Get it. - String contentType = Util.getContentType(entity); - if (contentType != null && contentType.startsWith("text/xml")) - { - new ErrorParser(context).parse(new InputStreamReader(in, Constants.UTF_8)); - return null; // Never reached. - } + InputStream in = null; + try { + in = response.getStream(); + byte[] bytes = Util.toByteArray(in); - byte[] bytes = Util.toByteArray(in); + // If we aren't allowing server-side scaling, always save the file to disk because it will be unmodified + if (!serverScaling || saveToFile) { + OutputStream out = null; - // If we aren't allowing server-side scaling, always save the file to disk because it will be unmodified - if (!serverScaling || saveToFile) - { - OutputStream out = null; + try { + out = new FileOutputStream(FileUtil.getAlbumArtFile(context, entry)); + out.write(bytes); + } finally { + Util.close(out); + } + } - try - { - out = new FileOutputStream(FileUtil.getAlbumArtFile(context, entry)); - out.write(bytes); - } - finally - { - Util.close(out); - } - } + bitmap = FileUtil.getSampledBitmap(bytes, size, highQuality); + } finally { + Util.close(in); + } + } - bitmap = FileUtil.getSampledBitmap(bytes, size, highQuality); - } - finally - { - Util.close(in); - } - } - - // Return scaled bitmap - return Util.scaleBitmap(bitmap, size); - } - } + // Return scaled bitmap + return Util.scaleBitmap(bitmap, size); + } + } @Override public HttpResponse getDownloadInputStream(Context context, MusicDirectory.Entry song, long offset, int maxBitrate, CancellableTask task) throws Exception