Fix bookmarks import and export bug retry (#4080)

<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL:
https://app.asana.com/0/488551667048375/1205538580267653/f

### Description
Fixes two issues:
- Favorited bookmarks are no longer duplicated on import
- Bookmark/folder order is respected for import and export

### Steps to test this PR
- [ ] Create some folders and bookmarks
- [ ] Make some of the bookmarks favorites (Be sure to add a favorite to
a subfolder)
- [ ] Rearrange the folders / bookmarks
- [ ] Export bookmarks
- [ ] Delete everything
- [ ] Import bookmarks
- [ ] Verify that the imported items match the exported list

---------

Co-authored-by: David González <[email protected]>

app/src/test/java/com/duckduckgo/app/bookmarks/model/SavedSitesRepositoryTest.kt

@@ -92,10 +92,7 @@ class SavedSitesRepositoryTest {
 
     @Test
     fun whenNoDataThenFolderContentisEmpty() = runTest {
-        repository.getFolderContentSync(SavedSitesNames.BOOKMARKS_ROOT).let { result ->
-            assert(result.first.isEmpty())
-            assert(result.second.isEmpty())
-        }
+        assert(repository.getFolderTreeItems(SavedSitesNames.BOOKMARKS_ROOT).isEmpty())
     }
 
     @Test
@@ -107,10 +104,7 @@ class SavedSitesRepositoryTest {
         val relation = givenFolderWithContent(SavedSitesNames.BOOKMARKS_ROOT, entities)
         savedSitesRelationsDao.insertList(relation)
 
-        repository.getFolderContentSync(SavedSitesNames.BOOKMARKS_ROOT).let { result ->
-            assert(result.first.size == totalBookmarks)
-            assert(result.second.isEmpty())
-        }
+        assert(repository.getFolderTreeItems(SavedSitesNames.BOOKMARKS_ROOT).size == totalBookmarks)
     }
 
     @Test
@@ -127,9 +121,10 @@ class SavedSitesRepositoryTest {
         val relation = givenFolderWithContent(SavedSitesNames.BOOKMARKS_ROOT, entities.plus(folders))
         savedSitesRelationsDao.insertList(relation)
 
-        repository.getFolderContentSync(SavedSitesNames.BOOKMARKS_ROOT).let { result ->
-            assert(result.first.size == totalBookmarks)
-            assert(result.second.size == totalFolders)
+        repository.getFolderTreeItems(SavedSitesNames.BOOKMARKS_ROOT).let { result ->
+            assert(result.size == totalBookmarks + totalFolders)
+            assert(result.filter { it.url != null }.size == totalBookmarks)
+            assert(result.filter { it.url == null }.size == totalFolders)
         }
     }
 
@@ -147,10 +142,7 @@ class SavedSitesRepositoryTest {
         val relation = givenFolderWithContent(SavedSitesNames.BOOKMARKS_ROOT, entities.plus(folders))
         savedSitesRelationsDao.insertList(relation)
 
-        repository.getFolderContentSync("12").let { result ->
-            assert(result.first.isEmpty())
-            assert(result.second.isEmpty())
-        }
+        assert(repository.getFolderTreeItems("12").isEmpty())
     }
 
     @Test
@@ -494,10 +486,9 @@ class SavedSitesRepositoryTest {
     @Test
     fun whenBookmarkAddedToRootThenGetFolderReturnsRootFolder() = runTest {
         val bookmark = repository.insertBookmark(title = "name", url = "foo.com")
-        repository.getFolderContentSync(bookmark.parentId).let { result ->
-            Assert.assertTrue(result.first.size == 1)
-            Assert.assertTrue(result.first.first().id == bookmark.id)
-            Assert.assertTrue(result.second.isEmpty())
+        repository.getFolderTreeItems(bookmark.parentId).let { result ->
+            Assert.assertTrue(result.size == 1)
+            Assert.assertTrue(result.first().id == bookmark.id)
         }
     }
 
@@ -539,9 +530,8 @@ class SavedSitesRepositoryTest {
         val bookmarkTwo = repository.insertBookmark(title = "two", url = "footwo.com")
         repository.updateBookmark(bookmarkTwo.copy(parentId = folder.id), bookmarkTwo.parentId)
 
-        repository.getFolderContentSync(folder.id).let { result ->
-            Assert.assertTrue(result.first.size == 2)
-            Assert.assertTrue(result.second.isEmpty())
+        repository.getFolderTreeItems(folder.id).let { result ->
+            Assert.assertTrue(result.size == 2)
         }
     }
 
@@ -561,10 +551,11 @@ class SavedSitesRepositoryTest {
 
         repository.insert(BookmarkFolder(id = "folder3", name = "folder2", lastModified = "timestamp", parentId = "folder2"))
 
-        repository.getFolderContentSync(folder.id).let { result ->
-            Assert.assertTrue(result.first.size == 2)
-            Assert.assertTrue(result.second.size == 1)
-            Assert.assertEquals(result.second.first().id, "folder3")
+        repository.getFolderTreeItems(folder.id).let { result ->
+            Assert.assertTrue(result.size == 3)
+            Assert.assertTrue(result.filter { it.url != null }.size == 2)
+            Assert.assertTrue(result.filter { it.url == null }.size == 1)
+            Assert.assertEquals(result[2].id, "folder3")
         }
     }
 
@@ -576,15 +567,11 @@ class SavedSitesRepositoryTest {
         val folderTwo =
             repository.insert(BookmarkFolder(id = "folder2", name = "folder2", lastModified = "timestamp", parentId = SavedSitesNames.BOOKMARKS_ROOT))
 
-        repository.getFolderContentSync(SavedSitesNames.BOOKMARKS_ROOT).let { result ->
-            assertEquals(listOf(bookmark), result.first)
-        }
+        assertEquals(bookmark.id, repository.getFolderTreeItems(SavedSitesNames.BOOKMARKS_ROOT).first().id)
 
         val updatedBookmark = bookmark.copy(parentId = folderTwo.id)
         repository.updateBookmark(updatedBookmark, bookmark.parentId)
-        repository.getFolderContentSync(SavedSitesNames.BOOKMARKS_ROOT).let { updatedResult ->
-            assertTrue(updatedResult.first.isEmpty())
-        }
+        assertTrue(repository.getFolderTreeItems(SavedSitesNames.BOOKMARKS_ROOT).filterNot { it.url == null }.isEmpty())
     }
 
     @Test
@@ -849,18 +836,12 @@ class SavedSitesRepositoryTest {
 
         givenFolderWithEntities(parentFolder.id, totalBookmarks, totalFolders)
 
-        repository.getFolderContentSync(parentFolder.id).let { result ->
-
-            assert(result.first.size == 10)
-            assert(result.second.size == 3)
-        }
-
-        repository.getFolderContentSync(SavedSitesNames.BOOKMARKS_ROOT).let { result ->
-
-            val folder = result.second.first()
+        assert(repository.getFolderTreeItems(SavedSitesNames.BOOKMARKS_ROOT).size == 1)
+        assert(repository.getFolderTreeItems(parentFolder.id).size == 13)
 
-            assert(folder.numBookmarks == 10)
-            assert(folder.numFolders == 3)
+        repository.getFolderTreeItems(parentFolder.id).let { result ->
+            assert(result.filter { it.url != null }.size == 10)
+            assert(result.filter { it.url == null }.size == 3)
         }
     }
 

app/src/test/java/com/duckduckgo/app/bookmarks/service/SavedSitesExporterTest.kt

@@ -27,14 +27,14 @@ import com.duckduckgo.common.test.CoroutineTestRule
 import com.duckduckgo.savedsites.api.SavedSitesRepository
 import com.duckduckgo.savedsites.api.models.BookmarkFolder
 import com.duckduckgo.savedsites.api.models.FolderBranch
+import com.duckduckgo.savedsites.api.models.FolderTreeItem
 import com.duckduckgo.savedsites.api.models.SavedSite
 import com.duckduckgo.savedsites.api.models.SavedSite.Bookmark
 import com.duckduckgo.savedsites.api.models.SavedSitesNames
 import com.duckduckgo.savedsites.api.models.TreeNode
 import com.duckduckgo.savedsites.api.service.ExportSavedSitesResult
 import com.duckduckgo.savedsites.impl.RealFavoritesDelegate
 import com.duckduckgo.savedsites.impl.RealSavedSitesRepository
-import com.duckduckgo.savedsites.impl.service.FolderTreeItem
 import com.duckduckgo.savedsites.impl.service.RealSavedSitesExporter
 import com.duckduckgo.savedsites.impl.service.RealSavedSitesParser
 import com.duckduckgo.savedsites.store.SavedSitesEntitiesDao

app/src/testInternal/java/com/duckduckgo/app/bookmarks/model/SavedSitesParserTest.kt

@@ -25,13 +25,15 @@ import com.duckduckgo.app.sync.FakeDisplayModeSettingsRepository
 import com.duckduckgo.common.test.CoroutineTestRule
 import com.duckduckgo.common.test.FileUtilities
 import com.duckduckgo.savedsites.api.SavedSitesRepository
+import com.duckduckgo.savedsites.api.models.BookmarkFolder
+import com.duckduckgo.savedsites.api.models.FolderTreeItem
 import com.duckduckgo.savedsites.api.models.SavedSite
+import com.duckduckgo.savedsites.api.models.SavedSite.Bookmark
 import com.duckduckgo.savedsites.api.models.SavedSite.Favorite
 import com.duckduckgo.savedsites.api.models.SavedSitesNames
 import com.duckduckgo.savedsites.api.models.TreeNode
 import com.duckduckgo.savedsites.impl.RealFavoritesDelegate
 import com.duckduckgo.savedsites.impl.RealSavedSitesRepository
-import com.duckduckgo.savedsites.impl.service.FolderTreeItem
 import com.duckduckgo.savedsites.impl.service.RealSavedSitesParser
 import com.duckduckgo.savedsites.store.SavedSitesEntitiesDao
 import com.duckduckgo.savedsites.store.SavedSitesRelationsDao
@@ -160,16 +162,16 @@ class SavedSitesParserTest {
         val inputStream = FileUtilities.loadResource(javaClass.classLoader!!, "bookmarks/bookmarks_firefox.html")
         val document = Jsoup.parse(inputStream, Charsets.UTF_8.name(), "duckduckgo.com")
 
-        val bookmarks = parser.parseHtml(document, repository)
+        val bookmarks = parser.parseHtml(document, repository).filterIsInstance<Bookmark>()
 
         Assert.assertEquals(17, bookmarks.size)
 
         val firstBookmark = bookmarks.first()
-        Assert.assertEquals("https://support.mozilla.org/en-US/products/firefox", firstBookmark.url)
+        Assert.assertEquals("https://support.mozilla.org/en-US/products/firefox", (firstBookmark as Bookmark).url)
         Assert.assertEquals("Get Help", firstBookmark.title)
 
         val lastBookmark = bookmarks.last()
-        Assert.assertEquals("https://www.mozilla.org/en-US/firefox/central/", lastBookmark.url)
+        Assert.assertEquals("https://www.mozilla.org/en-US/firefox/central/", (lastBookmark as Bookmark).url)
         Assert.assertEquals("Getting Started", lastBookmark.title)
     }
 
@@ -178,19 +180,19 @@ class SavedSitesParserTest {
         val inputStream = FileUtilities.loadResource(javaClass.classLoader!!, "bookmarks/bookmarks_brave.html")
         val document = Jsoup.parse(inputStream, Charsets.UTF_8.name(), "duckduckgo.com")
 
-        val bookmarks = parser.parseHtml(document, repository)
+        val bookmarks = parser.parseHtml(document, repository).filterIsInstance<Bookmark>()
 
         Assert.assertEquals(12, bookmarks.size)
 
         val firstBookmark = bookmarks.first()
         Assert.assertEquals(
             "https://www.theguardian.com/international",
-            firstBookmark.url,
+            (firstBookmark as Bookmark).url,
         )
         Assert.assertEquals("News, sport and opinion from the Guardian's global edition | The Guardian", firstBookmark.title)
 
         val lastBookmark = bookmarks.last()
-        Assert.assertEquals("https://www.macrumors.com/", lastBookmark.url)
+        Assert.assertEquals("https://www.macrumors.com/", (lastBookmark as Bookmark).url)
         Assert.assertEquals("MacRumors: Apple News and Rumors", lastBookmark.title)
     }
 
@@ -199,19 +201,19 @@ class SavedSitesParserTest {
         val inputStream = FileUtilities.loadResource(javaClass.classLoader!!, "bookmarks/bookmarks_chrome.html")
         val document = Jsoup.parse(inputStream, Charsets.UTF_8.name(), "duckduckgo.com")
 
-        val bookmarks = parser.parseHtml(document, repository)
+        val bookmarks = parser.parseHtml(document, repository).filterIsInstance<Bookmark>()
 
         Assert.assertEquals(12, bookmarks.size)
 
         val firstBookmark = bookmarks.first()
         Assert.assertEquals(
             "https://www.theguardian.com/international",
-            firstBookmark.url,
+            (firstBookmark as Bookmark).url,
         )
         Assert.assertEquals("News, sport and opinion from the Guardian's global edition | The Guardian", firstBookmark.title)
 
         val lastBookmark = bookmarks.last()
-        Assert.assertEquals("https://www.macrumors.com/", lastBookmark.url)
+        Assert.assertEquals("https://www.macrumors.com/", (lastBookmark as Bookmark).url)
         Assert.assertEquals("MacRumors: Apple News and Rumors", lastBookmark.title)
     }
 
@@ -220,41 +222,40 @@ class SavedSitesParserTest {
         val inputStream = FileUtilities.loadResource(javaClass.classLoader!!, "bookmarks/bookmarks_ddg_android.html")
         val document = Jsoup.parse(inputStream, Charsets.UTF_8.name(), "duckduckgo.com")
 
-        val bookmarks = parser.parseHtml(document, repository)
+        val bookmarks = parser.parseHtml(document, repository).filterNot { it is BookmarkFolder }
 
         Assert.assertEquals(13, bookmarks.size)
 
         val firstBookmark = bookmarks.first()
         Assert.assertEquals(
             "https://www.theguardian.com/international",
-            firstBookmark.url,
+            (firstBookmark as Bookmark).url,
         )
         Assert.assertEquals("News, sport and opinion from the Guardian's global edition | The Guardian", firstBookmark.title)
 
         val lastBookmark = bookmarks.last()
-        Assert.assertEquals("https://www.apple.com/uk/", lastBookmark.url)
+        Assert.assertEquals("https://www.apple.com/uk/", (lastBookmark as Favorite).url)
         Assert.assertEquals("Apple (United Kingdom)", lastBookmark.title)
-        Assert.assertTrue(lastBookmark is Favorite)
     }
 
     @Test
     fun canImportBookmarksFromDDGMacOS() = runTest {
         val inputStream = FileUtilities.loadResource(javaClass.classLoader!!, "bookmarks/bookmarks_ddg_macos.html")
         val document = Jsoup.parse(inputStream, Charsets.UTF_8.name(), "duckduckgo.com")
 
-        val bookmarks = parser.parseHtml(document, repository)
+        val bookmarks = parser.parseHtml(document, repository).filterIsInstance<Bookmark>()
 
         Assert.assertEquals(13, bookmarks.size)
 
         val firstBookmark = bookmarks.first()
         Assert.assertEquals(
             "https://www.theguardian.com/international",
-            firstBookmark.url,
+            (firstBookmark as Bookmark).url,
         )
         Assert.assertEquals("News, sport and opinion from the Guardian's global edition | The Guardian", firstBookmark.title)
 
         val lastBookmark = bookmarks.last()
-        Assert.assertEquals("https://www.apple.com/uk/", lastBookmark.url)
+        Assert.assertEquals("https://www.apple.com/uk/", (lastBookmark as Bookmark).url)
         Assert.assertEquals("Apple (United Kingdom)", lastBookmark.title)
     }
 
@@ -263,19 +264,19 @@ class SavedSitesParserTest {
         val inputStream = FileUtilities.loadResource(javaClass.classLoader!!, "bookmarks/bookmarks_safari.html")
         val document = Jsoup.parse(inputStream, Charsets.UTF_8.name(), "duckduckgo.com")
 
-        val bookmarks = parser.parseHtml(document, repository)
+        val bookmarks = parser.parseHtml(document, repository).filterIsInstance<Bookmark>()
 
         Assert.assertEquals(14, bookmarks.size)
 
         val firstBookmark = bookmarks.first()
         Assert.assertEquals(
             "https://www.apple.com/uk",
-            firstBookmark.url,
+            (firstBookmark as Bookmark).url,
         )
         Assert.assertEquals("Apple", firstBookmark.title)
 
         val lastBookmark = bookmarks.last()
-        Assert.assertEquals("https://www.macrumors.com/", lastBookmark.url)
+        Assert.assertEquals("https://www.macrumors.com/", (lastBookmark as Bookmark).url)
         Assert.assertEquals("MacRumors: Apple News and Rumors", lastBookmark.title)
     }
 

saved-sites/saved-sites-api/src/main/java/com/duckduckgo/savedsites/api/SavedSitesRepository.kt

@@ -19,6 +19,7 @@ package com.duckduckgo.savedsites.api
 import com.duckduckgo.savedsites.api.models.BookmarkFolder
 import com.duckduckgo.savedsites.api.models.BookmarkFolderItem
 import com.duckduckgo.savedsites.api.models.FolderBranch
+import com.duckduckgo.savedsites.api.models.FolderTreeItem
 import com.duckduckgo.savedsites.api.models.SavedSite
 import com.duckduckgo.savedsites.api.models.SavedSite.Bookmark
 import com.duckduckgo.savedsites.api.models.SavedSite.Favorite
@@ -42,9 +43,9 @@ interface SavedSitesRepository {
     /**
      * Returns all [Bookmark] and [BookmarkFolder] inside a folder
      * @param folderId the id of the folder.
-     * @return [Pair] of [Bookmark] and [BookmarkFolder] inside a folder
+     * @return [FolderTreeItem]s inside a folder
      */
-    fun getFolderContentSync(folderId: String): Pair<List<Bookmark>, List<BookmarkFolder>>
+    fun getFolderTreeItems(folderId: String): List<FolderTreeItem>
 
     /**
      * Returns complete list of [BookmarkFolderItem] inside a folder. This method traverses all folders.

saved-sites/saved-sites-api/src/main/java/com/duckduckgo/savedsites/api/models/Models.kt

@@ -94,3 +94,14 @@ object SavedSitesNames {
     const val BOOKMARKS_NAME = "Bookmarks"
     const val BOOMARKS_ROOT_ID = 0L
 }
+
+/**
+ * Used to build up a folder tree of [Bookmark]s and [BookmarkFolder]s
+ */
+data class FolderTreeItem(
+    val id: String,
+    val name: String,
+    val parentId: String,
+    val url: String?,
+    val depth: Int = 0,
+)

saved-sites/saved-sites-api/src/main/java/com/duckduckgo/savedsites/api/service/SavedSitesImporter.kt

@@ -17,7 +17,6 @@
 package com.duckduckgo.savedsites.api.service
 
 import android.net.Uri
-import com.duckduckgo.savedsites.api.models.SavedSite
 
 /**
  * Class that takes care of importing [SavedSites]
@@ -34,6 +33,6 @@ interface SavedSitesImporter {
 }
 
 sealed class ImportSavedSitesResult {
-    data class Success(val savedSites: List<SavedSite>) : ImportSavedSitesResult()
+    data class Success(val savedSites: List<Any>) : ImportSavedSitesResult()
     data class Error(val exception: Exception) : ImportSavedSitesResult()
 }