Merge pull request #294 from dturner/proto-list-to-map

Change user data fields to use maps instead of lists.

core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigration.kt

@@ -28,15 +28,15 @@ object IntToStringIdsMigration : DataMigration<UserPreferences> {
     override suspend fun migrate(currentData: UserPreferences): UserPreferences =
         currentData.copy {
             // Migrate topic ids
-            followedTopicIds.clear()
+            deprecatedFollowedTopicIds.clear()
-            followedTopicIds.addAll(
+            deprecatedFollowedTopicIds.addAll(
                 currentData.deprecatedIntFollowedTopicIdsList.map(Int::toString)
             )
             deprecatedIntFollowedTopicIds.clear()
 
             // Migrate author ids
-            followedAuthorIds.clear()
+            deprecatedFollowedAuthorIds.clear()
-            followedAuthorIds.addAll(
+            deprecatedFollowedAuthorIds.addAll(
                 currentData.deprecatedIntFollowedAuthorIdsList.map(Int::toString)
             )
             deprecatedIntFollowedAuthorIds.clear()

core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigration.kt

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.samples.apps.nowinandroid.core.datastore
+
+import androidx.datastore.core.DataMigration
+
+/**
+ * Migrates from using lists to maps for user data.
+ */
+object ListToMapMigration : DataMigration<UserPreferences> {
+
+    override suspend fun cleanUp() = Unit
+
+    override suspend fun migrate(currentData: UserPreferences): UserPreferences =
+        currentData.copy {
+            // Migrate topic id lists
+            followedTopicIds.clear()
+            followedTopicIds.putAll(
+                currentData.deprecatedFollowedTopicIdsList.associateWith { true }
+            )
+            deprecatedFollowedTopicIds.clear()
+
+            // Migrate author ids
+            followedAuthorIds.clear()
+            followedAuthorIds.putAll(
+                currentData.deprecatedFollowedAuthorIdsList.associateWith { true }
+            )
+            deprecatedFollowedAuthorIds.clear()
+
+            // Migrate bookmarks
+            bookmarkedNewsResourceIds.clear()
+            bookmarkedNewsResourceIds.putAll(
+                currentData.deprecatedBookmarkedNewsResourceIdsList.associateWith { true }
+            )
+            deprecatedBookmarkedNewsResourceIds.clear()
+
+            // Mark migration as complete
+            hasDoneListToMapMigration = true
+        }
+
+    override suspend fun shouldMigrate(currentData: UserPreferences): Boolean {
+        return !currentData.hasDoneListToMapMigration
+    }
+}

core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt

@@ -18,8 +18,6 @@ package com.google.samples.apps.nowinandroid.core.datastore
 
 import android.util.Log
 import androidx.datastore.core.DataStore
-import com.google.protobuf.kotlin.DslList
-import com.google.protobuf.kotlin.DslProxy
 import com.google.samples.apps.nowinandroid.core.model.data.UserData
 import java.io.IOException
 import javax.inject.Inject
@@ -33,54 +31,85 @@ class NiaPreferencesDataSource @Inject constructor(
     val userDataStream = userPreferences.data
         .map {
             UserData(
-                bookmarkedNewsResources = it.bookmarkedNewsResourceIdsList.toSet(),
+                bookmarkedNewsResources = it.bookmarkedNewsResourceIdsMap.keys,
-                followedTopics = it.followedTopicIdsList.toSet(),
+                followedTopics = it.followedTopicIdsMap.keys,
-                followedAuthors = it.followedAuthorIdsList.toSet(),
+                followedAuthors = it.followedAuthorIdsMap.keys,
             )
         }
 
-    suspend fun setFollowedTopicIds(followedTopicIds: Set<String>) =
+    suspend fun setFollowedTopicIds(topicIds: Set<String>) {
-        userPreferences.setList(
+        try {
-            listGetter = { it.followedTopicIds },
+            userPreferences.updateData {
-            listModifier = { followedTopicIds.toList() },
+                it.copy {
-            clear = { it.clear() },
+                    followedTopicIds.clear()
-            addAll = { dslList, editedList -> dslList.addAll(editedList) }
+                    followedTopicIds.putAll(topicIds.associateWith { true })
-        )
+                }
+            }
+        } catch (ioException: IOException) {
+            Log.e("NiaPreferences", "Failed to update user preferences", ioException)
+        }
+    }
 
-    suspend fun toggleFollowedTopicId(followedTopicId: String, followed: Boolean) =
+    suspend fun toggleFollowedTopicId(topicId: String, followed: Boolean) {
-        userPreferences.editList(
+        try {
-            add = followed,
+            userPreferences.updateData {
-            value = followedTopicId,
+                it.copy {
-            listGetter = { it.followedTopicIds },
+                    if (followed) {
-            clear = { it.clear() },
+                        followedTopicIds.put(topicId, true)
-            addAll = { dslList, editedList -> dslList.addAll(editedList) }
+                    } else {
-        )
+                        followedTopicIds.remove(topicId)
+                    }
+                }
+            }
+        } catch (ioException: IOException) {
+            Log.e("NiaPreferences", "Failed to update user preferences", ioException)
+        }
+    }
 
-    suspend fun setFollowedAuthorIds(followedAuthorIds: Set<String>) =
+    suspend fun setFollowedAuthorIds(authorIds: Set<String>) {
-        userPreferences.setList(
+        try {
-            listGetter = { it.followedAuthorIds },
+            userPreferences.updateData {
-            listModifier = { followedAuthorIds.toList() },
+                it.copy {
-            clear = { it.clear() },
+                    followedAuthorIds.clear()
-            addAll = { dslList, editedList -> dslList.addAll(editedList) }
+                    followedAuthorIds.putAll(authorIds.associateWith { true })
-        )
+                }
+            }
+        } catch (ioException: IOException) {
+            Log.e("NiaPreferences", "Failed to update user preferences", ioException)
+        }
+    }
 
-    suspend fun toggleFollowedAuthorId(followedAuthorId: String, followed: Boolean) =
+    suspend fun toggleFollowedAuthorId(authorId: String, followed: Boolean) {
-        userPreferences.editList(
+        try {
-            add = followed,
+            userPreferences.updateData {
-            value = followedAuthorId,
+                it.copy {
-            listGetter = { it.followedAuthorIds },
+                    if (followed) {
-            clear = { it.clear() },
+                        followedAuthorIds.put(authorId, true)
-            addAll = { dslList, editedList -> dslList.addAll(editedList) }
+                    } else {
-        )
+                        followedAuthorIds.remove(authorId)
+                    }
+                }
+            }
+        } catch (ioException: IOException) {
+            Log.e("NiaPreferences", "Failed to update user preferences", ioException)
+        }
+    }
 
-    suspend fun toggleNewsResourceBookmark(newsResourceId: String, bookmarked: Boolean) =
+    suspend fun toggleNewsResourceBookmark(newsResourceId: String, bookmarked: Boolean) {
-        userPreferences.editList(
+        try {
-            add = bookmarked,
+            userPreferences.updateData {
-            value = newsResourceId,
+                it.copy {
-            listGetter = { it.bookmarkedNewsResourceIds },
+                    if (bookmarked) {
-            clear = { it.clear() },
+                        bookmarkedNewsResourceIds.put(newsResourceId, true)
-            addAll = { dslList, editedList -> dslList.addAll(editedList) }
+                    } else {
-        )
+                        bookmarkedNewsResourceIds.remove(newsResourceId)
+                    }
+                }
+            }
+        } catch (ioException: IOException) {
+            Log.e("NiaPreferences", "Failed to update user preferences", ioException)
+        }
+    }
 
     suspend fun getChangeListVersions() = userPreferences.data
         .map {
@@ -119,48 +148,4 @@ class NiaPreferencesDataSource @Inject constructor(
             Log.e("NiaPreferences", "Failed to update user preferences", ioException)
         }
     }
-
-    /**
-     * Adds or removes [value] from the [DslList] provided by [listGetter]
-     */
-    private suspend fun <T : DslProxy> DataStore<UserPreferences>.editList(
-        add: Boolean,
-        value: String,
-        listGetter: (UserPreferencesKt.Dsl) -> DslList<String, T>,
-        clear: UserPreferencesKt.Dsl.(DslList<String, T>) -> Unit,
-        addAll: UserPreferencesKt.Dsl.(DslList<String, T>, Iterable<String>) -> Unit
-    ) {
-        setList(
-            listGetter = listGetter,
-            listModifier = { currentList ->
-                if (add) currentList + value
-                else currentList - value
-            },
-            clear = clear,
-            addAll = addAll
-        )
-    }
-
-    /**
-     * Sets the value provided by [listModifier] into the [DslList] read by [listGetter]
-     */
-    private suspend fun <T : DslProxy> DataStore<UserPreferences>.setList(
-        listGetter: (UserPreferencesKt.Dsl) -> DslList<String, T>,
-        listModifier: (DslList<String, T>) -> List<String>,
-        clear: UserPreferencesKt.Dsl.(DslList<String, T>) -> Unit,
-        addAll: UserPreferencesKt.Dsl.(DslList<String, T>, List<String>) -> Unit
-    ) {
-        try {
-            updateData {
-                it.copy {
-                    val dslList = listGetter(this)
-                    val newList = listModifier(dslList)
-                    clear(dslList)
-                    addAll(dslList, newList)
-                }
-            }
-        } catch (ioException: IOException) {
-            Log.e("NiaPreferences", "Failed to update user preferences", ioException)
-        }
-    }
 }

core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto

@@ -28,7 +28,14 @@ message UserPreferences {
     int32 newsResourceChangeListVersion = 6;
     repeated int32 deprecated_int_followed_author_ids = 7;
     bool has_done_int_to_string_id_migration = 8;
-    repeated string followed_topic_ids = 9;
+    repeated string deprecated_followed_topic_ids = 9;
-    repeated string followed_author_ids = 10;
+    repeated string deprecated_followed_author_ids = 10;
-    repeated string bookmarked_news_resource_ids = 11;
+    repeated string deprecated_bookmarked_news_resource_ids = 11;
+    bool has_done_list_to_map_migration = 12;
+
+    // Each map is used to store a set of string IDs. The bool has no meaning, but proto3 doesn't
+    // have a Set type so this is the closest we can get to a Set.
+    map<string, bool> followed_topic_ids = 13;
+    map<string, bool> followed_author_ids = 14;
+    map<string, bool> bookmarked_news_resource_ids = 15;
 }

core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigrationTest.kt

@@ -35,7 +35,7 @@ class IntToStringIdsMigrationTest {
         // Assert that there are no string topic ids yet
         assertEquals(
             emptyList<String>(),
-            preMigrationUserPreferences.followedTopicIdsList
+            preMigrationUserPreferences.deprecatedFollowedTopicIdsList
         )
 
         // Run the migration
@@ -45,7 +45,7 @@ class IntToStringIdsMigrationTest {
         // Assert the deprecated int topic ids have been migrated to the string topic ids
         assertEquals(
             userPreferences {
-                followedTopicIds.addAll(listOf("1", "2", "3"))
+                deprecatedFollowedTopicIds.addAll(listOf("1", "2", "3"))
                 hasDoneIntToStringIdMigration = true
             },
             postMigrationUserPreferences
@@ -64,7 +64,7 @@ class IntToStringIdsMigrationTest {
         // Assert that there are no string author ids yet
         assertEquals(
             emptyList<String>(),
-            preMigrationUserPreferences.followedAuthorIdsList
+            preMigrationUserPreferences.deprecatedFollowedAuthorIdsList
         )
 
         // Run the migration
@@ -74,7 +74,7 @@ class IntToStringIdsMigrationTest {
         // Assert the deprecated int author ids have been migrated to the string author ids
         assertEquals(
             userPreferences {
-                followedAuthorIds.addAll(listOf("4", "5", "6"))
+                deprecatedFollowedAuthorIds.addAll(listOf("4", "5", "6"))
                 hasDoneIntToStringIdMigration = true
             },
             postMigrationUserPreferences

core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigrationTest.kt

@@ -0,0 +1,102 @@
+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.samples.apps.nowinandroid.core.datastore
+
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert
+import org.junit.Test
+
+class ListToMapMigrationTest {
+
+    @Test
+    fun ListToMapMigration_should_migrate_topic_ids() = runTest {
+        // Set up existing preferences with topic ids
+        val preMigrationUserPreferences = userPreferences {
+            deprecatedFollowedTopicIds.addAll(listOf("1", "2", "3"))
+        }
+        // Assert that there are no topic ids in the map yet
+        Assert.assertEquals(
+            emptyMap<String, Boolean>(),
+            preMigrationUserPreferences.followedTopicIdsMap
+        )
+
+        // Run the migration
+        val postMigrationUserPreferences =
+            ListToMapMigration.migrate(preMigrationUserPreferences)
+
+        // Assert the deprecated topic ids have been migrated to the topic ids map
+        Assert.assertEquals(
+            mapOf("1" to true, "2" to true, "3" to true),
+            postMigrationUserPreferences.followedTopicIdsMap
+        )
+
+        // Assert that the migration has been marked complete
+        Assert.assertTrue(postMigrationUserPreferences.hasDoneListToMapMigration)
+    }
+
+    @Test
+    fun ListToMapMigration_should_migrate_author_ids() = runTest {
+        // Set up existing preferences with author ids
+        val preMigrationUserPreferences = userPreferences {
+            deprecatedFollowedAuthorIds.addAll(listOf("4", "5", "6"))
+        }
+        // Assert that there are no author ids in the map yet
+        Assert.assertEquals(
+            emptyMap<String, Boolean>(),
+            preMigrationUserPreferences.followedAuthorIdsMap
+        )
+
+        // Run the migration
+        val postMigrationUserPreferences =
+            ListToMapMigration.migrate(preMigrationUserPreferences)
+
+        // Assert the deprecated author ids have been migrated to the author ids map
+        Assert.assertEquals(
+            mapOf("4" to true, "5" to true, "6" to true),
+            postMigrationUserPreferences.followedAuthorIdsMap
+        )
+
+        // Assert that the migration has been marked complete
+        Assert.assertTrue(postMigrationUserPreferences.hasDoneListToMapMigration)
+    }
+
+    @Test
+    fun ListToMapMigration_should_migrate_bookmarks() = runTest {
+        // Set up existing preferences with bookmarks
+        val preMigrationUserPreferences = userPreferences {
+            deprecatedBookmarkedNewsResourceIds.addAll(listOf("7", "8", "9"))
+        }
+        // Assert that there are no bookmarks in the map yet
+        Assert.assertEquals(
+            emptyMap<String, Boolean>(),
+            preMigrationUserPreferences.bookmarkedNewsResourceIdsMap
+        )
+
+        // Run the migration
+        val postMigrationUserPreferences =
+            ListToMapMigration.migrate(preMigrationUserPreferences)
+
+        // Assert the deprecated bookmarks have been migrated to the bookmarks map
+        Assert.assertEquals(
+            mapOf("7" to true, "8" to true, "9" to true),
+            postMigrationUserPreferences.bookmarkedNewsResourceIdsMap
+        )
+
+        // Assert that the migration has been marked complete
+        Assert.assertTrue(postMigrationUserPreferences.hasDoneListToMapMigration)
+    }
+}

core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/UserPreferencesSerializerTest.kt

@@ -39,8 +39,8 @@ class UserPreferencesSerializerTest {
     @Test
     fun writingAndReadingUserPreferences_outputsCorrectValue() = runTest {
         val expectedUserPreferences = userPreferences {
-            followedTopicIds.add("0")
+            followedTopicIds.put("0", true)
-            followedTopicIds.add("1")
+            followedTopicIds.put("1", true)
         }
 
         val outputStream = ByteArrayOutputStream()