test(opml): empty title (#782)

app/src/main/java/me/ash/reader/domain/service/OpmlService.kt

@@ -46,7 +46,7 @@ class OpmlService @Inject constructor(
         withContext(ioDispatcher) {
             val defaultGroup = groupDao.queryById(getDefaultGroupId(context.currentAccountId))!!
             val groupWithFeedList =
-                OPMLDataSource.parseFileInputStream(inputStream, defaultGroup)
+                OPMLDataSource.parseFileInputStream(inputStream, defaultGroup, context.currentAccountId)
             groupWithFeedList.forEach { groupWithFeed ->
                 if (groupWithFeed.group != defaultGroup) {
                     groupDao.insert(groupWithFeed.group)

app/src/main/java/me/ash/reader/infrastructure/rss/OPMLDataSource.kt

@@ -2,13 +2,13 @@ package me.ash.reader.infrastructure.rss
 
 import android.content.Context
 import be.ceau.opml.OpmlParser
+import be.ceau.opml.entity.Outline
 import dagger.hilt.android.qualifiers.ApplicationContext
 import kotlinx.coroutines.CoroutineDispatcher
 import me.ash.reader.domain.model.feed.Feed
 import me.ash.reader.domain.model.group.Group
 import me.ash.reader.domain.model.group.GroupWithFeed
 import me.ash.reader.infrastructure.di.IODispatcher
-import me.ash.reader.ui.ext.currentAccountId
 import me.ash.reader.ui.ext.extractDomain
 import me.ash.reader.ui.ext.spacerDollar
 import java.io.InputStream
@@ -26,78 +26,63 @@ class OPMLDataSource @Inject constructor(
     suspend fun parseFileInputStream(
         inputStream: InputStream,
         defaultGroup: Group,
+        targetAccountId: Int,
     ): List<GroupWithFeed> {
-        val accountId = context.currentAccountId
         val opml = OpmlParser().parse(inputStream)
         val groupWithFeedList = mutableListOf<GroupWithFeed>().also {
             it.addGroup(defaultGroup)
         }
 
-        opml.body.outlines.forEach {
+        for (outline in opml.body.outlines) {
             // Only feeds
-            if (it.subElements.isEmpty()) {
+            if (outline.subElements.isEmpty()) {
                 // It's a empty group
-                if (!it.attributes.containsKey("xmlUrl")) {
-                    if (!it.attributes.getOrDefault("isDefault", null).toBoolean()) {
+                if (!outline.attributes.containsKey("xmlUrl")) {
+                    if (!outline.isDefaultGroup()) {
                         groupWithFeedList.addGroup(
                             Group(
-                                id = context.currentAccountId.spacerDollar(
-                                    UUID.randomUUID().toString()
-                                ),
-                                name = it.attributes.getOrDefault("title", null) ?: it.text!!,
-                                accountId = accountId,
+                                id = targetAccountId.spacerDollar(UUID.randomUUID().toString()),
+                                name = outline.extractName(),
+                                accountId = targetAccountId,
                             )
                         )
                     }
                 } else {
                     groupWithFeedList.addFeedToDefault(
                         Feed(
-                            id = context.currentAccountId.spacerDollar(
-                                UUID.randomUUID().toString()
-                            ),
-                            name = it.attributes.getOrDefault("title", null) ?: it.text!!,
-                            url = it.attributes.getOrDefault("xmlUrl", null)
-                                ?: throw IllegalArgumentException("xmlUrl is null"),
+                            id = targetAccountId.spacerDollar(UUID.randomUUID().toString()),
+                            name = outline.extractName(),
+                            url = outline.extractUrl() ?: continue,
                             groupId = defaultGroup.id,
-                            accountId = accountId,
-                            isNotification = it.attributes.getOrDefault("isNotification", null)
-                                .toBoolean(),
-                            isFullContent = it.attributes.getOrDefault("isFullContent", null)
-                                .toBoolean(),
+                            accountId = targetAccountId,
+                            isNotification = outline.extractPresetNotification(),
+                            isFullContent = outline.extractPresetFullContent(),
                         )
                     )
                 }
             } else {
                 var groupId = defaultGroup.id
-                if (!it.attributes.getOrDefault("isDefault", null).toBoolean()) {
-                    groupId =
-                        context.currentAccountId.spacerDollar(UUID.randomUUID().toString())
+                if (!outline.isDefaultGroup()) {
+                    groupId = targetAccountId.spacerDollar(UUID.randomUUID().toString())
                     groupWithFeedList.addGroup(
                         Group(
                             id = groupId,
-                            name = it.attributes.getOrDefault("title", null) ?: it.text!!,
-                            accountId = accountId,
+                            name = outline.extractName(),
+                            accountId = targetAccountId,
                         )
                     )
                 }
-                it.subElements.forEach { outline ->
-                    if (outline != null && outline.attributes != null) {
-                        val xmlUrl = outline.attributes.getOrDefault("xmlUrl", null)
-                            ?: throw IllegalArgumentException("${outline.attributes} xmlUrl is null")
+                for (subOutline in outline.subElements) {
+                    if (subOutline != null && subOutline.attributes != null) {
                         groupWithFeedList.addFeed(
                             Feed(
-                                id = context.currentAccountId.spacerDollar(
-                                    UUID.randomUUID().toString()
-                                ),
-                                name = outline.attributes.getOrDefault("title", null)
-                                    ?: outline.text ?: xmlUrl.extractDomain(),
-                                url = xmlUrl,
+                                id = targetAccountId.spacerDollar(UUID.randomUUID().toString()),
+                                name = subOutline.extractName(),
+                                url = subOutline.extractUrl() ?: continue,
                                 groupId = groupId,
-                                accountId = accountId,
-                                isNotification = outline.attributes.getOrDefault("isNotification",
-                                    null).toBoolean(),
-                                isFullContent = outline.attributes.getOrDefault("isFullContent",
-                                    null).toBoolean(),
+                                accountId = targetAccountId,
+                                isNotification = subOutline.extractPresetNotification(),
+                                isFullContent = subOutline.extractPresetFullContent(),
                             )
                         )
                     }
@@ -118,4 +103,30 @@ class OPMLDataSource @Inject constructor(
     private fun MutableList<GroupWithFeed>.addFeedToDefault(feed: Feed) {
         first().feeds.add(feed)
     }
+
+    private fun Outline?.extractName(): String {
+        if (this == null) return ""
+        return attributes.getOrDefault("title", null)
+            ?: text
+            ?: attributes.getOrDefault("xmlUrl", null).extractDomain()
+            ?: attributes.getOrDefault("htmlUrl", null).extractDomain()
+            ?: attributes.getOrDefault("url", null).extractDomain()
+            ?: ""
+    }
+
+    private fun Outline?.extractUrl(): String? {
+        if (this == null) return null
+        val url = attributes.getOrDefault("xmlUrl", null)
+            ?: attributes.getOrDefault("url", null)
+        return if (url.isNullOrBlank()) null else url
+    }
+
+    private fun Outline?.extractPresetNotification(): Boolean =
+        this?.attributes?.getOrDefault("isNotification", null).toBoolean()
+
+    private fun Outline?.extractPresetFullContent(): Boolean =
+        this?.attributes?.getOrDefault("isFullContent", null).toBoolean()
+
+    private fun Outline?.isDefaultGroup(): Boolean =
+        this?.attributes?.getOrDefault("isDefault", null).toBoolean()
 }

app/src/main/java/me/ash/reader/ui/ext/StringExt.kt

@@ -51,13 +51,13 @@ fun String?.orNotEmpty(l: (value: String) -> String): String =
 
 fun String.requiresBidi(): Boolean = Bidi.requiresBidi(this.toCharArray(), 0, this.length)
 
-fun String?.extractDomain(): String {
-    if (this.isNullOrBlank()) return ""
+fun String?.extractDomain(): String? {
+    if (this.isNullOrBlank()) return null
     val urlMatchResult = Regex("(?<=://)([\\w\\d.-]+)").find(this)
     if (urlMatchResult != null) {
         return urlMatchResult.value
     }
     val domainRegex = Regex("[\\w\\d.-]+\\.[\\w\\d.-]+")
     val domainMatchResult = domainRegex.find(this)
-    return domainMatchResult?.value ?: ""
-}
+    return domainMatchResult?.value
+}

app/src/test/java/me/ash/reader/infrastructure/rss/OPMLDataSourceTest.kt

@@ -5,26 +5,21 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.runBlocking
 import me.ash.reader.domain.model.group.Group
 import me.ash.reader.domain.model.group.GroupWithFeed
-import me.ash.reader.ui.ext.currentAccountId
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
-import org.mockito.Mockito.`when`
 import org.mockito.junit.MockitoJUnitRunner
 import org.mockito.kotlin.mock
 
 internal const val OPML_TEMPLATE: String = """
-<?xml version="1.0" encoding="UTF-8"?>
 <opml version="1.0">
     <head>
         <title>Import OPML Unit Test👿</title>
     </head>
      <body>
-        <outline text="Blogs" title="Blogs">
-            {{var}}
-        </outline>
+        {{var}}
     </body>
 </opml>
 """
@@ -48,24 +43,33 @@ class OPMLDataSourceTest {
     fun setUp() {
         mockContext = mock<Context> { }
         mockIODispatcher = mock<CoroutineDispatcher> {}
-        `when`(mockContext.currentAccountId).thenReturn(1)
         opmlDataSource = OPMLDataSource(mockContext, mockIODispatcher)
     }
 
+    private fun fill(value: String): String = OPML_TEMPLATE.replace("{{var}}", value)
+
+    private fun parse(opml: String): List<GroupWithFeed> = runBlocking {
+        opmlDataSource.parseFileInputStream(
+            inputStream = opml.byteInputStream(Charsets.UTF_8),
+            defaultGroup = defaultGroup,
+            targetAccountId = 1
+        )
+    }
+
     @Test
     fun testEmptyTitle() {
         val opml = fill("""
-            <outline type="rss" xmlUrl="https://ash7.io/index.xml" htmlUrl="https://ash7.io"/>
+            <outline text="Blogs" title="Blogs">
+                <outline type="rss" xmlUrl="https://ash7.io/index.xml" htmlUrl="https://ash7.io"/>
+            </outline>
         """)
-
-        runBlocking {
-            val result: List<GroupWithFeed> = opmlDataSource.parseFileInputStream(
-                inputStream = opml.byteInputStream(Charsets.UTF_8),
-                defaultGroup = defaultGroup
-            )
-            Assert.assertTrue("", result.size == 1)
-        }
+        val result = parse(opml)
+        Assert.assertEquals(2, result.size)
+        Assert.assertEquals("Default", result[0].group.name)
+        Assert.assertEquals(0, result[0].feeds.size)
+        Assert.assertEquals("Blogs", result[1].group.name)
+        Assert.assertEquals(1, result[1].feeds.size)
+        Assert.assertEquals("ash7.io", result[1].feeds[0].name)
+        Assert.assertEquals("https://ash7.io/index.xml", result[1].feeds[0].url)
     }
 }
-
-private fun fill(str: String): String = OPML_TEMPLATE.replace("{{var}}", str)

app/src/test/java/me/ash/reader/ui/ext/StringExtKtTest.kt

@@ -10,6 +10,8 @@ class StringExtTest {
 
     @Test
     fun testExtractDomain() {
+        Assert.assertEquals(null, "".extractDomain())
+        Assert.assertEquals(null, null.extractDomain())
         var case = "https://ash7.io"
         Assert.assertEquals("ash7.io", case.extractDomain())
         case = "ash7.io"