russhwolf / multiplatform-settings

A Kotlin Multiplatform library for saving simple key-value data
Apache License 2.0
1.49k stars 64 forks source link

serializedValue does not work with ListSerializer(String.serializer()) #162

Closed vanniktech closed 8 months ago

vanniktech commented 11 months ago

The following test fails on the last assertEquals:

class SomeTest {
  @Test fun foo() {
    val preferences = Preferences(MapSettings())
    assertEquals(expected = emptyList(), actual = preferences.list)

    val list = listOf("Hello", "World")
    preferences.list = list
    assertEquals(expected = list, actual = preferences.list)
  }

  class Preferences(settings: Settings) {
    @OptIn(ExperimentalSerializationApi::class, ExperimentalSettingsApi::class)
    var list by settings.serializedValue(
      serializer = ListSerializer(String.serializer()),
      key = "something",
      defaultValue = emptyList(),
    )
  }
}

I would have expected to have a properly saved list but instead it's empty.

Public reproducer: https://github.com/vanniktech/playground/pull/116

However, nullableSerializedValue seems to work. You can check yourself by applying the following patch file:

diff --git a/kmp/src/commonTest/kotlin/com/vanniktech/playground/kmp/SomeTest.kt b/kmp/src/commonTest/kotlin/com/vanniktech/playground/kmp/SomeTest.kt
index 308463e..3372a64 100644
--- a/kmp/src/commonTest/kotlin/com/vanniktech/playground/kmp/SomeTest.kt
+++ b/kmp/src/commonTest/kotlin/com/vanniktech/playground/kmp/SomeTest.kt
@@ -3,7 +3,7 @@ package com.vanniktech.playground.kmp
 import com.russhwolf.settings.ExperimentalSettingsApi
 import com.russhwolf.settings.MapSettings
 import com.russhwolf.settings.Settings
-import com.russhwolf.settings.serialization.serializedValue
+import com.russhwolf.settings.serialization.nullableSerializedValue
 import kotlinx.serialization.ExperimentalSerializationApi
 import kotlinx.serialization.builtins.ListSerializer
 import kotlinx.serialization.builtins.serializer
@@ -13,7 +13,7 @@ import kotlin.test.assertEquals
 class SomeTest {
   @Test fun foo() {
     val preferences = Preferences(MapSettings())
-    assertEquals(expected = emptyList(), actual = preferences.list)
+    assertEquals(expected = null, actual = preferences.list)

     val list = listOf("Hello", "World")
     preferences.list = list
@@ -22,10 +22,9 @@ class SomeTest {

   class Preferences(settings: Settings) {
     @OptIn(ExperimentalSerializationApi::class, ExperimentalSettingsApi::class)
-    var list by settings.serializedValue(
+    var list by settings.nullableSerializedValue(
       serializer = ListSerializer(String.serializer()),
       key = "something",
-      defaultValue = emptyList(),
     )
   }
 }
russhwolf commented 11 months ago

Thanks for the reproducer! This looks like it's probably the same issue as #160.

vanniktech commented 11 months ago

nullableSerializedValue does work when using MapSettings(); however on my Android App nullableSerializedValue didn't work correctly with the Shared Preferences Settings implementation.

russhwolf commented 10 months ago

Take a look at the 1.1 branch. I think I have this fixed but I'm not clear on the shared prefs difference you mentioned. I do expect possible behavior variations in the old implementation between the first time you deserialize and subsequent times, so that might account for what you saw, but I'm not certain.

russhwolf commented 8 months ago

The issues I can reproduce are fixed in 1.1.0. Let me know if there's still anything that isn't working for you.