8000 Sync: Optimize bookmarks JSON provision (#3241) · transmatecode/Android@704b2c3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 704b2c3

Browse files
malmsteinmalmstein
authored
Sync: Optimize bookmarks JSON provision (duckduckgo#3241)
Task/Issue URL: https://app.asana.com/0/0/1204770002414582/f ### Description We can improve the number of syncable objects sent when requesting a patch to Sync. Instead of sending the folder and all it’s children as separate objects, we only send the objects needed. ### Steps to test this PR Details in task
1 parent 8b02e80 commit 704b2c3

File tree

2 files changed

+70
-75
lines changed

2 files changed

+70
-75
lines changed

‎app/src/test/java/com/duckduckgo/app/sync/SavedSitesSyncDataProviderTest.kt

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import androidx.room.Room
2121
import androidx.test.ext.junit.runners.AndroidJUnit4
2222
import androidx.test.platform.app.InstrumentationRegistry
2323
import com.duckduckgo.app.CoroutineTestRule
24-
import com.duckduckgo.app.FileUtilities
2524
import com.duckduckgo.app.global.db.AppDatabase
2625
import com.duckduckgo.app.global.formatters.time.DatabaseDateFormatter
2726
import com.duckduckgo.savedsites.api.SavedSitesRepository
@@ -144,9 +143,7 @@ class SavedSitesSyncDataProviderTest {
144143
}
145144

146145
@Test
147-
fun whenFirstSyncAndUsersHasFoldersThenChangesAreFormatted() {
148-
val updatesJSON = FileUtilities.loadText(javaClass.classLoader!!, "json/parser_folders.json")
149-
146+
fun whenNewBookmarksSinceLastSyncThenChangesContainData() {
150147
repository.insert(bookmark3)
151148
repository.insert(bookmark4)
152149

@@ -160,9 +157,7 @@ class SavedSitesSyncDataProviderTest {
160157
}
161158

162159
@Test
163-
fun whenFirstSyncAndUsersHasFavoritesAndSubfoldersThenChangesAreFormatted() {
164-
val updatesJSON = FileUtilities.loadText(javaClass.classLoader!!, "json/parser_folders_and_favourites.json")
165-
160+
fun whenNewFoldersAndBookmarksAndFavouritesSinceLastSyncThenChangesContainData() {
166161
repository.insert(bookmark1)
167162
repository.insert(bookmark2)
168163
repository.insert(favourite1)
@@ -188,9 +183,10 @@ class SavedSitesSyncDataProviderTest {
188183
}
189184

190185
@Test
191-
fun whenChangesAfterLastSyncInFavoritesThenChangesAreFormatted() {
186+
fun whenNewFavouritesSinceLastSyncThenChangesContainData() {
192187
val modificationTimestamp = DatabaseDateFormatter.iso8601()
193188
val lastSyncTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
189+
setLastSyncTime(lastSyncTimestamp)
194190

195191
repository.insert(bookmark3.copy(lastModified = modificationTimestamp))
196192
repository.insert(bookmark4.copy(lastModified = modificationTimestamp))
@@ -199,30 +195,28 @@ class SavedSitesSyncDataProviderTest {
199195
val changes = parser.changesSince(lastSyncTimestamp)
200196

201197
assertTrue(changes.isNotEmpty())
202-
assertTrue(changes[0].id == bookmark1.id)
198+
assertTrue(changes[0].id == favoritesFolder.id)
203199
assertTrue(changes[0].client_last_modified == modificationTimestamp)
204200
assertTrue(changes[0].deleted == null)
205-
assertTrue(changes[1].id == favoritesFolder.id)
201+
assertTrue(changes[0].folder!!.children == listOf(bookmark1.id))
202+
assertTrue(changes[1].id == bookmarksRootFolder.id)
206203
assertTrue(changes[1].client_last_modified == modificationTimestamp)
207-
assertTrue(changes[1].deleted == null)
208-
assertTrue(changes[1].folder!!.children == listOf(bookmark1.id))
204+
assertTrue(changes[1].folder!!.children == listOf(bookmark3.id, bookmark4.id, bookmark1.id))
209205
assertTrue(changes[2].id == bookmark3.id)
210206
assertTrue(changes[2].client_last_modified == modificationTimestamp)
211207
assertTrue(changes[2].deleted == null)
212208
assertTrue(changes[3].id == bookmark4.id)
213209
assertTrue(changes[3].client_last_modified == modificationTimestamp)
214210
assertTrue(changes[3].deleted == null)
215-
assertTrue(changes[4].id == bookmarksRootFolder.id)
216-
assertTrue(changes[4].client_last_modified == modificationTimestamp)
217-
assertTrue(changes[4].folder!!.children == listOf(bookmark3.id, bookmark4.id, bookmark1.id))
211+
assertTrue(changes[4].id == favourite1.id)
218212
assertTrue(changes[4].deleted == null)
219213
}
220214

221215
@Test
222-
fun whenNoChangesAfterLastSyncAreEmptyThenChangesAreEmpty() {
216+
fun whenNoChangesAfterLastSyncThenChangesAreEmpty() {
223217
val modificationTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
224218
val lastSyncTimestamp = DatabaseDateFormatter.iso8601()
225-
store.modifiedSince = lastSyncTimestamp
219+
setLastSyncTime(lastSyncTimestamp)
226220

227221
repository.insert(bookmark3.copy(lastModified = modificationTimestamp))
228222
repository.insert(bookmark4.copy(lastModified = modificationTimestamp))
@@ -233,9 +227,10 @@ class SavedSitesSyncDataProviderTest {
233227
}
234228

235229
@Test
236-
fun whenBookmarkDeletedAfterLastSyncThenDataIsCorrect() {
230+
fun whenBookmarkDeletedAfterLastSyncThenChangesContainData() {
237231
val modificationTimestamp = DatabaseDateFormatter.iso8601()
238232
val lastSyncTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
233+
setLastSyncTime(lastSyncTimestamp)
239234

240235
val modifiedBookmark3 = bookmark3.copy(lastModified = modificationTimestamp)
241236
val modifiedBookmark4 = bookmark4.copy(lastModified = modificationTimestamp)
@@ -246,18 +241,19 @@ class SavedSitesSyncDataProviderTest {
246241

247242
val changes = parser.changesSince(lastSyncTimestamp)
248243
assertTrue(changes.isNotEmpty())
249-
assertTrue(changes[0].id == bookmark3.id)
244+
assertTrue(changes[0].id == bookmarksRootFolder.id)
250245
assertTrue(changes[0].deleted == null)
251-
assertTrue(changes[1].id == bookmark3.parentId)
246+
assertTrue(changes[0].folder!!.children == listOf(bookmark3.id))
247+
assertTrue(changes[1].id == bookmark3.id)
252248
assertTrue(changes[1].deleted == null)
253-
assertTrue(changes[1].folder!!.children == listOf(bookmark3.id))
254249
assertTrue(changes[2].id == bookmark4.id)
255250
assertTrue(changes[2].deleted == "1")
256251
}
257252

258253
@Test
259-
fun whenFolderDeletedAfterLastSyncThenDataIsCorrect() {
254+
fun whenFolderDeletedAfterLastSyncThenChangesContainData() {
260255
val lastSyncTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
256+
setLastSyncTime(lastSyncTimestamp)
261257

262258
repository.insert(bookmark1)
263259
repository.insert(bookmark2)
@@ -281,8 +277,9 @@ class SavedSitesSyncDataProviderTest {
281277
}
282278

283279
@Test
284-
fun whenFavouriteDeletedAfterLastSyncThenDataIsCorrect() {
280+
fun whenFavouriteDeletedAfterLastSyncThenChangesContainData() {
285281
val lastSyncTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
282+
setLastSyncTime(lastSyncTimestamp)
286283

287284
repository.insert(favourite1)
288285
repository.insert(bookmark3)
@@ -296,8 +293,9 @@ class SavedSitesSyncDataProviderTest {
296293
}
297294

298295
@Test
299-
fun whenBookmarkAndFavouriteDeletedAfterLastSyncThenDataIsCorrect() {
296+
fun whenFavouritesAndBookmarksDeletedAfterLastSyncThenChangesContainData() {
300297
val lastSyncTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
298+
setLastSyncTime(lastSyncTimestamp)
301299

302300
repository.insert(bookmark1)
303301
repository.insert(favourite1)
@@ -316,23 +314,29 @@ class SavedSitesSyncDataProviderTest {
316314
}
317315

318316
@Test
319-
fun whenMovingABookmarkToAnotherFolderThenDataIsCorrect() {
317+
fun whenFolderMovedToAnotherFolderAfterLastSyncThenChangesContainData() {
320318
val beforeLastSyncTimestamp = DatabaseDateFormatter.iso8601(threeHoursAgo)
321-
val lastSyncTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
322319
val modificationTimestamp = DatabaseDateFormatter.iso8601(oneHourAgo)
320+
val lastSyncTimestamp = DatabaseDateFormatter.iso8601(twoHoursAgo)
321+
setLastSyncTime(lastSyncTimestamp)
323322

324-
val modifiedBookmark1 = bookmark1.copy(lastModified = modificationTimestamp)
325-
val modifiedBookmark2 = bookmark2.copy(lastModified = beforeLastSyncTimestamp)
323+
val modifiedBookmark1 = bookmark1.copy(lastModified = beforeLastSyncTimestamp, parentId = subFolder.id)
324+
val modifiedFolder = subFolder.copy(lastModified = modificationTimestamp)
326325

327326
repository.insert(modifiedBookmark1)
328-
repository.insert(modifiedBookmark2)
329-
repository.insert(subFolder)
330-
repository.updateBookmark(modifiedBookmark1.copy(parentId = subFolder.id), SavedSitesNames.BOOKMARKS_ROOT)
327+
repository.insert(modifiedFolder)
331328

332329
val changes = parser.changesSince(lastSyncTimestamp)
333-
assertTrue(changes.filter { it.id == modifiedBookmark1.id } != null)
334-
assertTrue(changes.filter { it.id == subFolder.id } != null)
335-
assertTrue(changes.filter { it.id == SavedSitesNames.BOOKMARKS_ROOT } != null)
330+
assertTrue(changes.isNotEmpty())
331+
assertTrue(changes.size == 2)
332+
assertTrue(changes[0].id == bookmarksRootFolder.id)
333+
assertTrue(changes[0].folder!!.children == listOf(subFolder.id))
334+
assertTrue(changes[1].id == subFolder.id)
335+
assertTrue(changes[1].folder!!.children == listOf(bookmark1.id))
336+
}
337+
338+
private fun setLastSyncTime(lastSyncTimestamp: String) {
339+
store.modifiedSince = lastSyncTimestamp
336340
}
337341

338342
private fun fromSavedSite(savedSite: SavedSite): SyncBookmarkEntry {

‎saved-sites/saved-sites-impl/src/main/java/com/duckduckgo/savedsites/impl/sync/SavedSitesSyncDataProvider.kt

Lines changed: 32 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import com.duckduckgo.di.scopes.AppScope
2222
import com.duckduckgo.savedsites.api.SavedSitesRepository
2323
import com.duckduckgo.savedsites.api.models.BookmarkFolder
2424
import com.duckduckgo.savedsites.api.models.SavedSite
25-
import com.duckduckgo.savedsites.api.models.SavedSite.Bookmark
2625
import com.duckduckgo.savedsites.api.models.SavedSitesNames
2726
import com.duckduckgo.savedsites.impl.sync.algorithm.isDeleted
2827
import com.duckduckgo.sync.api.SyncCrypto
@@ -34,7 +33,6 @@ import com.squareup.anvil.annotations.ContributesMultibinding
3433
import com.squareup.moshi.JsonAdapter
3534
import com.squareup.moshi.Moshi
3635
import javax.inject.Inject
37-
import org.threeten.bp.OffsetDateTime
3836
import timber.log.Timber
3937

4038
@ContributesMultibinding(scope = AppScope::class, boundType = SyncableDataProvider::class)
@@ -67,16 +65,20 @@ class SavedSitesSyncDataProvider @Inject constructor(
6765
Timber.d("Sync-Feature: generating changes since $since")
6866
val updates = mutableListOf<SyncBookmarkEntry>()
6967

68+
// we start adding individual folders that have been modified
7069
val folders = repository.getFoldersModifiedSince(since)
7170
folders.forEach { folder ->
7271
if (folder.isDeleted()) {
7372
updates.add(deletedEntry(folder.id, folder.deleted!!))
7473
} else {
75-
addFolderContent(folder.id, updates, since)
74+
val folderEntry = addFolderEntry(folder.id)
75+
if (folderEntry != null) {
76+
updates.add(folderEntry)
77+
}
7678
}
7779
}
7880

79-
// bookmarks that were deleted or edited won't be part of the previous check
81+
// then we add individual bookmarks that have been modified
8082
val bookmarks = repository.getBookmarksModifiedSince(since)
8183
bookmarks.forEach { bookmark ->
8284
if (bookmark.isDeleted()) {
@@ -110,14 +112,35 @@ class SavedSitesSyncDataProvider @Inject constructor(
110112
}
111113
}
112114

113-
addFolderContent(SavedSitesNames.BOOKMARKS_ROOT, updates)
115+
addFolderEntryWithContent(SavedSitesNames.BOOKMARKS_ROOT, updates)
114116
return updates.distinct()
115117
}
116118

117-
private fun addFolderContent(
119+
private fun addFolderEntry(folderId: String): SyncBookmarkEntry? {
120+
val folderContent = repository.getAllFolderContentSync(folderId)
121+
val storedFolder = repository.getFolder(folderId)
122+
val folder = if (storedFolder != null) {
123+
val childrenIds = mutableListOf<String>()
124+
for (bookmark in folderContent.first) {
125+
if (bookmark.deleted == null) {
126+
childrenIds.add(bookmark.id)
127+
}
128+
}
129+
for (eachFolder in folderContent.second) {
130+
if (eachFolder.deleted == null) {
131+
childrenIds.add(eachFolder.id)
132+
}
133+
}
134+
encryptFolder(storedFolder, childrenIds)
135+
} else {
136+
null
137+
}
138+
return folder
139+
}
140+
141+
private fun addFolderEntryWithContent(
118142
folderId: String,
119143
updates: MutableList<SyncBookmarkEntry>,
120-
lastModified: String = "",
121144
): List<SyncBookmarkEntry> {
122145
repository.getAllFolderContentSync(folderId).apply {
123146
val folder = repository.getFolder(folderId)
@@ -128,19 +151,15 @@ class SavedSitesSyncDataProvider @Inject constructor(
128151
updates.add(deletedEntry(bookmark.id, bookmark.deleted!!))
129152
} else {
130153
childrenIds.add(bookmark.id)
131-
if (bookmark.modifiedSince(lastModified)) {
132-
updates.add(encryptSavedSite(bookmark))
133-
}
154+
updates.add(encryptSavedSite(bookmark))
134155
}
135156
}
136157
for (eachFolder in this.second) {
137158
if (eachFolder.deleted != null) {
138159
updates.add(deletedEntry(eachFolder.id, eachFolder.deleted!!))
139160
} else {
140161
childrenIds.add(eachFolder.id)
141-
if (eachFolder.modifiedSince(lastModified)) {
142-
addFolderContent(eachFolder.id, updates)
143-
}
162+
addFolderEntryWithContent(eachFolder.id, updates)
144163
}
145164
}
146165
updates.add(encryptFolder(folder, childrenIds))
@@ -149,34 +168,6 @@ class SavedSitesSyncDataProvider @Inject constructor(
149168
return updates
150169
}
151170

152-
private fun BookmarkFolder.modifiedSince(since: String): Boolean {
153-
return if (since.isEmpty()) {
154-
true
155-
} else {
156-
if (this.lastModified == null) {
157-
true
158-
} else {
159-
val entityModified = OffsetDateTime.parse(this.lastModified)
160-
val sinceModified = OffsetDateTime.parse(since)
161-
entityModified.isAfter(sinceModified)
162-
}
163-
}
164-
}
165-
166-
private fun Bookmark.modifiedSince(since: String): Boolean {
167-
return if (since.isEmpty()) {
168-
true
169-
} else {
170-
if (this.lastModified == null) {
171-
true
172-
} else {
173-
val entityModified = OffsetDateTime.parse(this.lastModified)
174-
val sinceModified = OffsetDateTime.parse(since)
175-
entityModified.isAfter(sinceModified)
176-
}
177-
}
178-
}
179-
180171
private fun encryptSavedSite(
181172
savedSite: SavedSite,
182173
): SyncBookmarkEntry {

0 commit comments

Comments
 (0)
0