From 7ae1550d1b35348b308eb3e4cdcc607614202540 Mon Sep 17 00:00:00 2001 From: Daniel THIRION Date: Sat, 20 Jun 2020 00:47:46 +0200 Subject: [PATCH 1/7] fix(covers): catch HTTP timeouts in cover loading Fixes #107 ("Cover loading times out very often") --- src/main/xerus/monstercat/api/Covers.kt | 5 ++++- src/main/xerus/monstercat/downloader/SongView.kt | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/xerus/monstercat/api/Covers.kt b/src/main/xerus/monstercat/api/Covers.kt index 64ca7fc..82374dc 100644 --- a/src/main/xerus/monstercat/api/Covers.kt +++ b/src/main/xerus/monstercat/api/Covers.kt @@ -21,8 +21,11 @@ object Covers { /** Returns an Image of the cover in the requested size using caching. * @param size the size of the Image that is returned - the image file will always be 64x64 */ - fun getThumbnailImage(coverUrl: String, size: Number = 64, invalidate: Boolean = false): Image = + fun getThumbnailImage(coverUrl: String, size: Number = 64, invalidate: Boolean = false) = try { getCover(coverUrl, 64, invalidate).use { createImage(it, size) } + } catch (e: Exception) { + null + } /** Returns a larger Image of the cover in the requested size using caching. * @param size the size of the Image that is returned - the image file will always be 1024x1024 */ diff --git a/src/main/xerus/monstercat/downloader/SongView.kt b/src/main/xerus/monstercat/downloader/SongView.kt index 8379136..3d2a092 100644 --- a/src/main/xerus/monstercat/downloader/SongView.kt +++ b/src/main/xerus/monstercat/downloader/SongView.kt @@ -219,10 +219,12 @@ class SongView(private val sorter: ObservableValue): } GlobalScope.launch(globalDispatcher) { var image = Covers.getThumbnailImage(release.coverUrl, 16) - image.onError { + fun invalidateImage() { image = Covers.getThumbnailImage(release.coverUrl, 16, true) - image.onError { logger.debug("Failed to load coverUrl ${release.coverUrl} for $release", it) } + image?.onError { logger.debug("Failed to load coverUrl ${release.coverUrl} for $release", it) } } + if (image == null) invalidateImage() + else image!!.onError { invalidateImage() } onFx { treeItem.graphic = ImageView(image) done++ From 159c2f5f2e6432eee56f3ca84d0253af3c7cea31 Mon Sep 17 00:00:00 2001 From: Daniel THIRION Date: Sat, 20 Jun 2020 00:49:31 +0200 Subject: [PATCH 2/7] fix(covers): UGLY - hardcode jpeg extension to cover cache files Other than grabbing the Content-Type header (which is cumbersome in the current scope), it is better for now to hardcode the file extension. Most file viewers today can recognize wrong extension and still read, for example, a png file disguised with a .jpg extension; At least now the cover cache files aren't a bunch of extension-less files anymore. --- src/main/xerus/monstercat/api/Covers.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/xerus/monstercat/api/Covers.kt b/src/main/xerus/monstercat/api/Covers.kt index 82374dc..bd1629f 100644 --- a/src/main/xerus/monstercat/api/Covers.kt +++ b/src/main/xerus/monstercat/api/Covers.kt @@ -16,7 +16,7 @@ object Covers { private fun coverCacheFile(coverUrl: String, size: Int): File { coverCacheDir.mkdirs() val newFile = coverCacheDir.resolve(coverUrl.substringBeforeLast('/').substringAfterLast('/').replaceIllegalFileChars()) - return coverCacheDir.resolve("${newFile.nameWithoutExtension}x$size.${newFile.extension}") + return coverCacheDir.resolve("${newFile.nameWithoutExtension}x$size.jpg") // FIXME : Do not hardcode extension } /** Returns an Image of the cover in the requested size using caching. From 5bf8e4b02945619cf377ca4f64e24633218881bc Mon Sep 17 00:00:00 2001 From: Daniel THIRION Date: Sun, 28 Jun 2020 22:42:48 +0200 Subject: [PATCH 3/7] refactor(covers): use elvis operator to avoid double-bang Co-authored-by: Janek <27jf@pm.me> --- src/main/xerus/monstercat/downloader/SongView.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/xerus/monstercat/downloader/SongView.kt b/src/main/xerus/monstercat/downloader/SongView.kt index 3d2a092..02daadf 100644 --- a/src/main/xerus/monstercat/downloader/SongView.kt +++ b/src/main/xerus/monstercat/downloader/SongView.kt @@ -223,8 +223,7 @@ class SongView(private val sorter: ObservableValue): image = Covers.getThumbnailImage(release.coverUrl, 16, true) image?.onError { logger.debug("Failed to load coverUrl ${release.coverUrl} for $release", it) } } - if (image == null) invalidateImage() - else image!!.onError { invalidateImage() } + image?.onError { invalidateImage() } ?: invalidateImage() onFx { treeItem.graphic = ImageView(image) done++ From 14d34f5ed31afc8f75c85274ebcc398b8246d276 Mon Sep 17 00:00:00 2001 From: Daniel THIRION Date: Thu, 19 Nov 2020 19:17:23 +0100 Subject: [PATCH 4/7] feat(covers): revert covers cache file extension hardcode --- src/main/xerus/monstercat/api/Covers.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/xerus/monstercat/api/Covers.kt b/src/main/xerus/monstercat/api/Covers.kt index bd1629f..9730eb8 100644 --- a/src/main/xerus/monstercat/api/Covers.kt +++ b/src/main/xerus/monstercat/api/Covers.kt @@ -16,7 +16,7 @@ object Covers { private fun coverCacheFile(coverUrl: String, size: Int): File { coverCacheDir.mkdirs() val newFile = coverCacheDir.resolve(coverUrl.substringBeforeLast('/').substringAfterLast('/').replaceIllegalFileChars()) - return coverCacheDir.resolve("${newFile.nameWithoutExtension}x$size.jpg") // FIXME : Do not hardcode extension + return coverCacheDir.resolve("${newFile.nameWithoutExtension}x$size") } /** Returns an Image of the cover in the requested size using caching. From 81f8555b103b1747f228d7f914925b37b2d69c4a Mon Sep 17 00:00:00 2001 From: Daniel THIRION Date: Thu, 19 Nov 2020 19:43:21 +0100 Subject: [PATCH 5/7] feat(covers): better exception handling with propagation and try catches --- src/main/xerus/monstercat/MonsterUtilities.kt | 12 ++++++-- .../xerus/monstercat/api/APIConnection.kt | 3 ++ src/main/xerus/monstercat/api/Covers.kt | 30 ++++++++++++------- src/main/xerus/monstercat/api/Player.kt | 13 +++++--- .../xerus/monstercat/downloader/SongView.kt | 7 ++--- 5 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/main/xerus/monstercat/MonsterUtilities.kt b/src/main/xerus/monstercat/MonsterUtilities.kt index f81910e..82ccd28 100644 --- a/src/main/xerus/monstercat/MonsterUtilities.kt +++ b/src/main/xerus/monstercat/MonsterUtilities.kt @@ -39,6 +39,7 @@ import xerus.monstercat.api.Player import xerus.monstercat.downloader.TabDownloader import xerus.monstercat.tabs.* import java.io.File +import java.io.IOException import java.net.URL import java.net.UnknownHostException import java.util.* @@ -362,7 +363,7 @@ class MonsterUtilities(checkForUpdate: Boolean): JFXMessageDisplay { pane.add(Label("Cover loading...")) pane.add(largeImage) - App.stage.createStage(title, pane).apply { + val stage = App.stage.createStage(title, pane).apply { height = windowSize width = windowSize this.isResizable = isResizable @@ -396,7 +397,14 @@ class MonsterUtilities(checkForUpdate: Boolean): JFXMessageDisplay { } GlobalScope.launch { - largeImage.image = Covers.getCoverImage(coverUrl, windowSize.toInt()) + try { + largeImage.image = Covers.getCoverImage(coverUrl, windowSize.toInt()) + } catch (e: IOException) { + onFx { + stage.close() + showError(e, "Cover could not be fetched.") + } + } } } diff --git a/src/main/xerus/monstercat/api/APIConnection.kt b/src/main/xerus/monstercat/api/APIConnection.kt index f156f82..72d0f6e 100644 --- a/src/main/xerus/monstercat/api/APIConnection.kt +++ b/src/main/xerus/monstercat/api/APIConnection.kt @@ -7,6 +7,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.launch import mu.KotlinLogging import org.apache.http.HttpResponse +import org.apache.http.client.ClientProtocolException import org.apache.http.client.config.CookieSpecs import org.apache.http.client.config.RequestConfig import org.apache.http.client.methods.CloseableHttpResponse @@ -123,6 +124,8 @@ class APIConnection(vararg path: String): HTTPQuery() { CONNECTSID.listen { updateConnectsid(it) } } + /**@return a [CloseableHttpResponse] resulting from the HTTP [request] + * @throws IOException when the request fails */ fun executeRequest(request: HttpUriRequest, context: HttpClientContext? = null): CloseableHttpResponse { logger.trace { "Connecting to ${request.uri}" } return httpClient.execute(request, context) diff --git a/src/main/xerus/monstercat/api/Covers.kt b/src/main/xerus/monstercat/api/Covers.kt index 9730eb8..ad09114 100644 --- a/src/main/xerus/monstercat/api/Covers.kt +++ b/src/main/xerus/monstercat/api/Covers.kt @@ -1,16 +1,21 @@ package xerus.monstercat.api import javafx.scene.image.Image +import mu.KotlinLogging import org.apache.http.HttpEntity +import org.apache.http.client.ClientProtocolException import org.apache.http.client.methods.HttpGet import xerus.ktutil.replaceIllegalFileChars import xerus.monstercat.cacheDir import java.io.File +import java.io.FileNotFoundException +import java.io.IOException import java.io.InputStream +import java.lang.IllegalArgumentException import java.net.URI object Covers { - + val logger = KotlinLogging.logger { } private val coverCacheDir = cacheDir.resolve("cover-images").apply { mkdirs() } private fun coverCacheFile(coverUrl: String, size: Int): File { @@ -20,15 +25,14 @@ object Covers { } /** Returns an Image of the cover in the requested size using caching. - * @param size the size of the Image that is returned - the image file will always be 64x64 */ - fun getThumbnailImage(coverUrl: String, size: Number = 64, invalidate: Boolean = false) = try { - getCover(coverUrl, 64, invalidate).use { createImage(it, size) } - } catch (e: Exception) { - null - } + * @param size the size of the Image that is returned - the image file will always be 64x64 + * @throws IOException propagation of [getCover]'s exception */ + fun getThumbnailImage(coverUrl: String, size: Number = 64, invalidate: Boolean = false) = + getCover(coverUrl, 64, invalidate).use { createImage(it, size) } /** Returns a larger Image of the cover in the requested size using caching. - * @param size the size of the Image that is returned - the image file will always be 1024x1024 */ + * @param size the size of the Image that is returned - the image file will always be 1024x1024 + * @throws IOException propagation of [getCover]'s exception */ fun getCoverImage(coverUrl: String, size: Int = 1024, invalidate: Boolean = false): Image = getCover(coverUrl, 1024, invalidate).use { createImage(it, size) } @@ -40,13 +44,17 @@ object Covers { * @param coverUrl the URL for fetching the cover * @param size the dimensions of the cover file * @param invalidate set to true to ignore already existing cache files + * @throws IOException propagation of [fetchCover]'s exception if it fails (connectivity issues) */ fun getCover(coverUrl: String, size: Int, invalidate: Boolean = false): InputStream { val coverFile = coverCacheFile(coverUrl, size) if(!invalidate) try { return coverFile.inputStream() - } catch(e: Exception) { + } catch (e: FileNotFoundException) { + logger.warn("Cover file at ${coverFile.path} not found, invalidating.", e) + } catch (e: SecurityException) { + logger.warn("Cover file at ${coverFile.path} cannot be opened, invalidating.", e) } fetchCover(coverUrl, size).content.use { input -> val tempFile = File.createTempFile(coverFile.name, size.toString()) @@ -61,7 +69,9 @@ object Covers { /** Fetches the given [coverUrl] with an [APIConnection] in the requested [size]. * @param coverUrl the base url to fetch the cover * @param size the size of the cover to be fetched from the api, with all powers of 2 being available. - * By default null, which results in the biggest size possible, usually between 2k and 8k. */ + * By default null, which results in the biggest size possible, usually between 2k and 8k. + * @throws IOException in case of timeout, connectivity issues... + */ fun fetchCover(coverUrl: String, size: Int? = null): HttpEntity = APIConnection.executeRequest(HttpGet(getCoverUrl(coverUrl, size))).entity diff --git a/src/main/xerus/monstercat/api/Player.kt b/src/main/xerus/monstercat/api/Player.kt index a0996f8..b98cd5f 100644 --- a/src/main/xerus/monstercat/api/Player.kt +++ b/src/main/xerus/monstercat/api/Player.kt @@ -30,6 +30,7 @@ import xerus.monstercat.Settings import xerus.monstercat.api.response.Release import xerus.monstercat.api.response.Track import xerus.monstercat.monsterUtilities +import java.io.IOException import java.nio.file.Files import java.nio.file.StandardOpenOption import java.util.* @@ -296,11 +297,15 @@ object Player: FadingHBox(true, targetHeight = 25) { logger.debug("Updating cover: $coverUrl") this.coverUrl = coverUrl GlobalScope.launch { - val image: Image? = coverUrl?.let { Covers.getThumbnailImage(it) } - onFx { - backgroundCover.value = image?.let { - Background(BackgroundImage(image, BackgroundRepeat.NO_REPEAT, BackgroundRepeat.NO_REPEAT, BackgroundPosition.CENTER, BackgroundSize(100.0, 100.0, true, true, true, true))) + try { + val image: Image? = coverUrl?.let { Covers.getThumbnailImage(it) } + onFx { + backgroundCover.value = image?.let { + Background(BackgroundImage(image, BackgroundRepeat.NO_REPEAT, BackgroundRepeat.NO_REPEAT, BackgroundPosition.CENTER, BackgroundSize(100.0, 100.0, true, true, true, true))) + } } + } catch (e: IOException) { + logger.warn("Background Cover could not be fetched.", e) } } } diff --git a/src/main/xerus/monstercat/downloader/SongView.kt b/src/main/xerus/monstercat/downloader/SongView.kt index 02daadf..d2eaf16 100644 --- a/src/main/xerus/monstercat/downloader/SongView.kt +++ b/src/main/xerus/monstercat/downloader/SongView.kt @@ -103,8 +103,7 @@ class SongView(private val sorter: ObservableValue): setOnMouseClicked { if(it.clickCount == 2) { val selected = selectionModel.selectedItem ?: return@setOnMouseClicked - val value = selected.value - when(value) { + when(val value = selected.value) { is Release -> Player.play(value) is Track -> Player.playTrack(value) else -> selected.isExpanded = !selected.isExpanded @@ -221,9 +220,9 @@ class SongView(private val sorter: ObservableValue): var image = Covers.getThumbnailImage(release.coverUrl, 16) fun invalidateImage() { image = Covers.getThumbnailImage(release.coverUrl, 16, true) - image?.onError { logger.debug("Failed to load coverUrl ${release.coverUrl} for $release", it) } + image.onError { logger.debug("Failed to load coverUrl ${release.coverUrl} for $release", it) } } - image?.onError { invalidateImage() } ?: invalidateImage() + image.onError { invalidateImage() } onFx { treeItem.graphic = ImageView(image) done++ From 0f20221b01975b546f91c6020c4504f9872bbceb Mon Sep 17 00:00:00 2001 From: Daniel THIRION Date: Thu, 19 Nov 2020 19:51:07 +0100 Subject: [PATCH 6/7] refactor(covers): use split instead of double substring --- src/main/xerus/monstercat/api/Covers.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/xerus/monstercat/api/Covers.kt b/src/main/xerus/monstercat/api/Covers.kt index ad09114..259563d 100644 --- a/src/main/xerus/monstercat/api/Covers.kt +++ b/src/main/xerus/monstercat/api/Covers.kt @@ -20,7 +20,7 @@ object Covers { private fun coverCacheFile(coverUrl: String, size: Int): File { coverCacheDir.mkdirs() - val newFile = coverCacheDir.resolve(coverUrl.substringBeforeLast('/').substringAfterLast('/').replaceIllegalFileChars()) + val newFile = coverCacheDir.resolve(coverUrl.split('/').let{ it[it.lastIndex - 1] }.replaceIllegalFileChars()) return coverCacheDir.resolve("${newFile.nameWithoutExtension}x$size") } From fea04f6899378b6cbf80ab893b98d0570703ad7f Mon Sep 17 00:00:00 2001 From: Daniel THIRION Date: Fri, 11 Dec 2020 00:59:46 +0100 Subject: [PATCH 7/7] fix(covers): remove unnecessary and excessive logging on invalidation --- src/main/xerus/monstercat/api/Covers.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/xerus/monstercat/api/Covers.kt b/src/main/xerus/monstercat/api/Covers.kt index 259563d..68f935d 100644 --- a/src/main/xerus/monstercat/api/Covers.kt +++ b/src/main/xerus/monstercat/api/Covers.kt @@ -52,9 +52,9 @@ object Covers { try { return coverFile.inputStream() } catch (e: FileNotFoundException) { - logger.warn("Cover file at ${coverFile.path} not found, invalidating.", e) + logger.warn("Cover file at ${coverFile.path} not found, invalidating.") } catch (e: SecurityException) { - logger.warn("Cover file at ${coverFile.path} cannot be opened, invalidating.", e) + logger.warn("Cover file at ${coverFile.path} cannot be opened, invalidating.") } fetchCover(coverUrl, size).content.use { input -> val tempFile = File.createTempFile(coverFile.name, size.toString())