From c4c01cf99b452b2a8e454adb4bf4c9368bb9b686 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Mon, 19 Sep 2022 21:15:54 +0100 Subject: [PATCH] Change user data fields to use maps instead of lists. --- .../core/datastore/IntToStringIdsMigration.kt | 8 +- .../core/datastore/ListToMapMigration.kt | 58 +++++++ .../datastore/NiaPreferencesDataSource.kt | 159 ++++++++---------- .../nowinandroid/data/user_preferences.proto | 13 +- .../datastore/IntToStringIdsMigrationTest.kt | 8 +- .../core/datastore/ListToMapMigrationTest.kt | 102 +++++++++++ .../UserPreferencesSerializerTest.kt | 4 +- 7 files changed, 252 insertions(+), 100 deletions(-) create mode 100644 core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigration.kt create mode 100644 core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigrationTest.kt diff --git a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigration.kt b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigration.kt index 4e6e49300..cd27bb015 100644 --- a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigration.kt +++ b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigration.kt @@ -28,15 +28,15 @@ object IntToStringIdsMigration : DataMigration { override suspend fun migrate(currentData: UserPreferences): UserPreferences = currentData.copy { // Migrate topic ids - followedTopicIds.clear() - followedTopicIds.addAll( + deprecatedFollowedTopicIds.clear() + deprecatedFollowedTopicIds.addAll( currentData.deprecatedIntFollowedTopicIdsList.map(Int::toString) ) deprecatedIntFollowedTopicIds.clear() // Migrate author ids - followedAuthorIds.clear() - followedAuthorIds.addAll( + deprecatedFollowedAuthorIds.clear() + deprecatedFollowedAuthorIds.addAll( currentData.deprecatedIntFollowedAuthorIdsList.map(Int::toString) ) deprecatedIntFollowedAuthorIds.clear() diff --git a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigration.kt b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigration.kt new file mode 100644 index 000000000..b88b27f74 --- /dev/null +++ b/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 { + + 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 + } +} diff --git a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt index 6d7028f0c..28e1df069 100644 --- a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt +++ b/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(), - followedTopics = it.followedTopicIdsList.toSet(), - followedAuthors = it.followedAuthorIdsList.toSet(), + bookmarkedNewsResources = it.bookmarkedNewsResourceIdsMap.keys, + followedTopics = it.followedTopicIdsMap.keys, + followedAuthors = it.followedAuthorIdsMap.keys, ) } - suspend fun setFollowedTopicIds(followedTopicIds: Set) = - userPreferences.setList( - listGetter = { it.followedTopicIds }, - listModifier = { followedTopicIds.toList() }, - clear = { it.clear() }, - addAll = { dslList, editedList -> dslList.addAll(editedList) } - ) + suspend fun setFollowedTopicIds(topicIds: Set) { + try { + userPreferences.updateData { + it.copy { + followedTopicIds.clear() + followedTopicIds.putAll(topicIds.associateWith { true }) + } + } + } catch (ioException: IOException) { + Log.e("NiaPreferences", "Failed to update user preferences", ioException) + } + } - suspend fun toggleFollowedTopicId(followedTopicId: String, followed: Boolean) = - userPreferences.editList( - add = followed, - value = followedTopicId, - listGetter = { it.followedTopicIds }, - clear = { it.clear() }, - addAll = { dslList, editedList -> dslList.addAll(editedList) } - ) + suspend fun toggleFollowedTopicId(topicId: String, followed: Boolean) { + try { + userPreferences.updateData { + it.copy { + if (followed) { + followedTopicIds.put(topicId, true) + } else { + followedTopicIds.remove(topicId) + } + } + } + } catch (ioException: IOException) { + Log.e("NiaPreferences", "Failed to update user preferences", ioException) + } + } - suspend fun setFollowedAuthorIds(followedAuthorIds: Set) = - userPreferences.setList( - listGetter = { it.followedAuthorIds }, - listModifier = { followedAuthorIds.toList() }, - clear = { it.clear() }, - addAll = { dslList, editedList -> dslList.addAll(editedList) } - ) + suspend fun setFollowedAuthorIds(authorIds: Set) { + try { + userPreferences.updateData { + it.copy { + followedAuthorIds.clear() + followedAuthorIds.putAll(authorIds.associateWith { true }) + } + } + } catch (ioException: IOException) { + Log.e("NiaPreferences", "Failed to update user preferences", ioException) + } + } - suspend fun toggleFollowedAuthorId(followedAuthorId: String, followed: Boolean) = - userPreferences.editList( - add = followed, - value = followedAuthorId, - listGetter = { it.followedAuthorIds }, - clear = { it.clear() }, - addAll = { dslList, editedList -> dslList.addAll(editedList) } - ) + suspend fun toggleFollowedAuthorId(authorId: String, followed: Boolean) { + try { + userPreferences.updateData { + it.copy { + if (followed) { + followedAuthorIds.put(authorId, true) + } else { + followedAuthorIds.remove(authorId) + } + } + } + } catch (ioException: IOException) { + Log.e("NiaPreferences", "Failed to update user preferences", ioException) + } + } - suspend fun toggleNewsResourceBookmark(newsResourceId: String, bookmarked: Boolean) = - userPreferences.editList( - add = bookmarked, - value = newsResourceId, - listGetter = { it.bookmarkedNewsResourceIds }, - clear = { it.clear() }, - addAll = { dslList, editedList -> dslList.addAll(editedList) } - ) + suspend fun toggleNewsResourceBookmark(newsResourceId: String, bookmarked: Boolean) { + try { + userPreferences.updateData { + it.copy { + if (bookmarked) { + bookmarkedNewsResourceIds.put(newsResourceId, true) + } 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 DataStore.editList( - add: Boolean, - value: String, - listGetter: (UserPreferencesKt.Dsl) -> DslList, - clear: UserPreferencesKt.Dsl.(DslList) -> Unit, - addAll: UserPreferencesKt.Dsl.(DslList, Iterable) -> 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 DataStore.setList( - listGetter: (UserPreferencesKt.Dsl) -> DslList, - listModifier: (DslList) -> List, - clear: UserPreferencesKt.Dsl.(DslList) -> Unit, - addAll: UserPreferencesKt.Dsl.(DslList, List) -> 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) - } - } } diff --git a/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto b/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto index 407031215..c7b199a33 100644 --- a/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto +++ b/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 followed_author_ids = 10; - repeated string bookmarked_news_resource_ids = 11; + repeated string deprecated_followed_topic_ids = 9; + repeated string deprecated_followed_author_ids = 10; + 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 followed_topic_ids = 13; + map followed_author_ids = 14; + map bookmarked_news_resource_ids = 15; } diff --git a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigrationTest.kt b/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigrationTest.kt index 8c62452b8..89035dc91 100644 --- a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/IntToStringIdsMigrationTest.kt +++ b/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(), - 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(), - 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 diff --git a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigrationTest.kt b/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/ListToMapMigrationTest.kt new file mode 100644 index 000000000..6fc06f585 --- /dev/null +++ b/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(), + 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(), + 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(), + 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) + } +} diff --git a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/UserPreferencesSerializerTest.kt b/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/UserPreferencesSerializerTest.kt index 3b4015495..cf3fcf338 100644 --- a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/UserPreferencesSerializerTest.kt +++ b/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.add("1") + followedTopicIds.put("0", true) + followedTopicIds.put("1", true) } val outputStream = ByteArrayOutputStream()