Improve icons quality for local account feeds

api/src/main/java/com/readrops/api/localfeed/LocalRSSHelper.kt

@@ -3,7 +3,6 @@ package com.readrops.api.localfeed
 import com.gitlab.mvysny.konsumexml.Konsumer
 import com.gitlab.mvysny.konsumexml.Names
 import com.readrops.api.utils.extensions.checkRoot
-import java.io.InputStream
 
 object LocalRSSHelper {
 
@@ -26,12 +25,11 @@ object LocalRSSHelper {
             RSS_1_CONTENT_TYPE -> RSSType.RSS_1
             RSS_2_CONTENT_TYPE -> RSSType.RSS_2
             ATOM_CONTENT_TYPE -> RSSType.ATOM
-            JSON_CONTENT_TYPE, JSONFEED_CONTENT_TYPE -> RSSType.JSONFEED
+            JSONFEED_CONTENT_TYPE -> RSSType.JSONFEED
             else -> RSSType.UNKNOWN
         }
     }
 
-    @JvmStatic
     fun isRSSType(type: String?): Boolean =
             if (type != null) getRSSType(type) != RSSType.UNKNOWN else false
 

api/src/main/java/com/readrops/api/utils/HtmlParser.kt

@@ -2,10 +2,13 @@ package com.readrops.api.utils
 
 import android.nfc.FormatException
 import com.readrops.api.localfeed.LocalRSSHelper
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 import okhttp3.OkHttpClient
 import okhttp3.Request
 import org.jsoup.Jsoup
 import org.jsoup.nodes.Document
+import org.jsoup.nodes.Element
 
 data class ParsingResult(
     val url: String,
@@ -14,42 +17,59 @@ data class ParsingResult(
 
 object HtmlParser {
 
+    @Throws(FormatException::class)
     suspend fun getFaviconLink(url: String, client: OkHttpClient): String? {
         val document = getHTMLHeadFromUrl(url, client)
-        val elements = document.select("link")
 
-        for (element in elements) {
+        val links = document.select("link")
-            if (element.attributes()["rel"].lowercase().contains("icon")) {
+            .filter { element -> element.attributes()["rel"].contains("icon") }
-                return element.absUrl("href")
+            .sortedWith(compareByDescending<Element> {
+                it.attributes()["rel"] == "apple-touch-icon"
+            }.thenByDescending { element ->
+                val sizes = element.attr("sizes")
+
+                if (sizes.isNotEmpty()) {
+                    try {
+                        sizes.filter { it.isDigit() }
+                            .toInt()
+                    } catch (e: Exception) {
+                        0
                     }
+                } else {
+                    0
+                }
+            })
+
+        return links.firstOrNull()
+            ?.absUrl("href")
     }
 
-        return null
+    @Throws(FormatException::class)
-    }
-
     suspend fun getFeedLink(url: String, client: OkHttpClient): List<ParsingResult> {
-        val results = mutableListOf<ParsingResult>()
-
         val document = getHTMLHeadFromUrl(url, client)
-        val elements = document.select("link")
 
-        for (element in elements) {
+        return document.select("link")
+            .filter { element ->
                 val type = element.attributes()["type"]
-
+                LocalRSSHelper.isRSSType(type)
-            if (LocalRSSHelper.isRSSType(type)) {
+            }.map {
-                results += ParsingResult(
+                ParsingResult(
-                    url = element.absUrl("href"),
+                    url = it.absUrl("href"),
-                    label = element.attributes()["title"]
+                    label = it.attributes()["title"]
                 )
             }
     }
 
-        return results
+    private suspend fun getHTMLHeadFromUrl(url: String, client: OkHttpClient): Document =
-    }
+        withContext(Dispatchers.IO) {
-
+            client.newCall(
-    private fun getHTMLHeadFromUrl(url: String, client: OkHttpClient): Document {
+                Request.Builder()
-        client.newCall(Request.Builder().url(url).build()).execute().use { response ->
+                    .url(url)
-            if (response.header(ApiUtils.CONTENT_TYPE_HEADER)!!.contains(ApiUtils.HTML_CONTENT_TYPE)
+                    .build()
+            ).execute()
+                .use { response ->
+                    if (response.header(ApiUtils.CONTENT_TYPE_HEADER)!!
+                            .contains(ApiUtils.HTML_CONTENT_TYPE)
                     ) {
                         val body = response.body!!.source()
 
@@ -64,25 +84,29 @@ object HtmlParser {
                                     stringBuilder.append(currentLine)
                                     collectionStarted = true
                                 }
+
                                 currentLine.contains("</head>") -> {
                                     stringBuilder.append(currentLine)
                                     break
                                 }
+
                                 collectionStarted -> {
                                     stringBuilder.append(currentLine)
                                 }
                             }
                         }
 
-                if (!stringBuilder.contains("<head>") || !stringBuilder.contains("</head>"))
+                        if (!stringBuilder.contains("<head>") || !stringBuilder.contains("</head>")) {
-                    throw FormatException("Failed to get HTML head")
+                            body.close()
+                            throw FormatException("Failed to get HTML head from $url")
+                        }
 
                         body.close()
-                return Jsoup.parse(stringBuilder.toString(), url)
+                        Jsoup.parse(stringBuilder.toString(), url)
                     } else {
-                throw FormatException("The response is not a html file")
+                        response.close()
+                        throw FormatException("Response from $url is not a html file")
                     }
                 }
         }
-
 }

api/src/test/java/com/readrops/api/utils/HtmlParserTest.kt

@@ -2,19 +2,21 @@ package com.readrops.api.utils
 
 import android.nfc.FormatException
 import com.readrops.api.TestUtils
-import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.test.runTest
 import okhttp3.OkHttpClient
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
 import okio.Buffer
+import org.junit.After
+import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.koin.dsl.module
 import org.koin.test.KoinTest
 import org.koin.test.KoinTestRule
+import org.koin.test.get
 import java.net.HttpURLConnection
 import java.util.concurrent.TimeUnit
-import kotlin.test.assertEquals
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
@@ -34,18 +36,18 @@ class HtmlParserTest : KoinTest {
         })
     }
 
-    @Test
+    @Before
     fun before() {
         mockServer.start()
     }
 
-    @Test
+    @After
     fun after() {
         mockServer.shutdown()
     }
 
     @Test
-    fun getFeedLinkTest() {
+    fun getFeedLinkTest() = runTest {
         val stream = TestUtils.loadResource("utils/file.html")
 
         mockServer.enqueue(
@@ -54,19 +56,14 @@ class HtmlParserTest : KoinTest {
                 .setBody(Buffer().readFrom(stream))
         )
 
-        runBlocking {
+        val links = HtmlParser.getFeedLink(mockServer.url("/rss").toString(), get())
-            val result =
-                HtmlParser.getFeedLink(mockServer.url("/rss").toString(), koinTestRule.koin.get())
 
-            assertTrue { result.size == 1 }
+        assertTrue { links.size == 2 }
-            assertTrue { result.first().url.endsWith("/rss") }
+        assertTrue { links.all { it.label!!.contains("The Mozilla Blog") } }
-            assertEquals("RSS", result.first().label)
-
-        }
     }
 
     @Test(expected = FormatException::class)
-    fun getFeedLinkWithoutHeadTest() {
+    fun getFeedLinkWithoutHeadTest() = runTest {
         val stream = TestUtils.loadResource("utils/file_without_head.html")
 
         mockServer.enqueue(
@@ -75,21 +72,21 @@ class HtmlParserTest : KoinTest {
                 .setBody(Buffer().readFrom(stream))
         )
 
-        runBlocking { HtmlParser.getFeedLink(mockServer.url("/rss").toString(), koinTestRule.koin.get()) }
+        HtmlParser.getFeedLink(mockServer.url("/rss").toString(), get())
     }
 
     @Test(expected = FormatException::class)
-    fun getFeedLinkNoHtmlFileTest() {
+    fun getFeedLinkNoHtmlFileTest() = runTest {
         mockServer.enqueue(
             MockResponse().setResponseCode(HttpURLConnection.HTTP_OK)
-                .addHeader(ApiUtils.CONTENT_TYPE_HEADER, "application/rss+xml"))
+                .addHeader(ApiUtils.CONTENT_TYPE_HEADER, "application/rss+xml")
+        )
 
-
+        HtmlParser.getFeedLink(mockServer.url("/rss").toString(), get())
-        runBlocking { HtmlParser.getFeedLink(mockServer.url("/rss").toString(), koinTestRule.koin.get()) }
     }
 
     @Test
-    fun getFaviconLinkTest() {
+    fun getFaviconLinkTest() = runTest {
         val stream = TestUtils.loadResource("utils/file.html")
 
         mockServer.enqueue(
@@ -98,15 +95,12 @@ class HtmlParserTest : KoinTest {
                 .setBody(Buffer().readFrom(stream))
         )
 
-        runBlocking {
+        val link = HtmlParser.getFaviconLink(mockServer.url("/rss").toString(), get())
-            val result = HtmlParser.getFaviconLink(mockServer.url("/rss").toString(), koinTestRule.koin.get())
+        assertTrue { link!!.contains("apple-touch-icon") }
-
-            assertTrue { result!!.contains("favicon.ico") }
-        }
     }
 
     @Test
-    fun getFaviconLinkWithoutHeadTest() {
+    fun getFaviconLinkWithoutHeadTest() = runTest {
         val stream = TestUtils.loadResource("utils/file_without_icon.html")
 
         mockServer.enqueue(
@@ -115,10 +109,7 @@ class HtmlParserTest : KoinTest {
                 .setBody(Buffer().readFrom(stream))
         )
 
-        runBlocking {
+        val link = HtmlParser.getFaviconLink(mockServer.url("/rss").toString(), get())
-            val result = HtmlParser.getFaviconLink(mockServer.url("/rss").toString(), koinTestRule.koin.get())
+        assertNull(link)
-
-            assertNull(result)
-        }
     }
 }

app/src/main/java/com/readrops/app/repositories/LocalRSSRepository.kt

@@ -129,7 +129,7 @@ class LocalRSSRepository(
                     feedUrl?.let { color = FeedColors.getFeedColor(it) }
                 }
             } catch (e: Exception) {
-                Log.d("LocalRSSRepository", "insertFeed: ${e.message}")
+                Log.e("LocalRSSRepository", "insertFeed: ${e.message}")
             }
 
             id = database.feedDao().insert(this).toInt()