multiplatform-settings icon indicating copy to clipboard operation
multiplatform-settings copied to clipboard

.remove() in Seriliazation module

Open mdhanif-simformsolutions opened this issue 3 years ago • 7 comments

I am trying to remove() a serialized class but settings.remove("key") doesn't seems to work for Serialized module.

I am using no-arg module to instantiate settings variable. My settings.remove(key) works with basic datatypes like int string but it doesn't work with the serialized classes.

For eg :- If I have a class such as data class User(name: String, number: String) I store it using settings.encodeValue(User.serializer(), "key", user) now it does save it correctly no issues till here. Now If at some point I want to remove my User from the shared preference I try to do it with settings.remove("key") and it doesn't do anything I can still get my User object from the preferences like this settings.decodeValueOrNull(User.serializer(), "key") where as expected should be null.

Also if i try to check if value is stored for my particular key using settings.contains("key") then it always return false no matter if I have that in my preference or not.

Same happens with iOS as well.

Also the only thing which works is either I clear all of my preference using settings.clear() or I explicitly set my User variable as null as a workaround. Is this the expected behaviour or am I missing something with Serialization module ?

This is currently the expected behavior, because contains() and remove() don't know anything about the serialization module. But I see how that can be unintuitive, and I should think about whether there are better ways to handle this.

In the meantime, if you use User.serializer().nullable, there will also be a boolean at "user?" indicating whether its present or not, and you'll be able to pass null to encodeValue() to clear it.

russhwolf avatar Mar 02 '21 03:03 russhwolf

Hi. thanks for the quick response I can still pass null to encodeValue() just to clear it if my User class contains everything optional and it will do the same trick for now.

Encountered this problem too. It would be great if it could be fixed.

And workaround with encodeValue(null) not works for me

Setttings.encodeValue(
                        serializer = serializer.nullable,
                        key = key.key,
                        value = null
                )

september669 avatar Jul 08 '21 07:07 september669

Can you say more about what doesn't work?

russhwolf avatar Jul 08 '21 22:07 russhwolf

Hello, Maybe I can add more info on this matter. Here is my reduced case:

The model:

@kotlinx.serialization.Serializable
data class User(
    val name: String,
    val company: String? = null
)

The repository:

class UserRepository : KoinComponent {
    private val settings: Settings by inject()

    fun addUser(user: User) {
        settings.encodeValue(User.serializer(), user.name, user)
    }

    fun deleteUser(user: User) {
        settings.encodeValue(User.serializer().nullable, user.name, null)
    }

    fun getKeysSize() = settings.keys.size
}

The test:

@Test
fun addAndRemoveUser() {
    // given
    val repo = AccountSettingsRepository()
    val user = User("Bob")

    // when
    repo.addUser(user)
    repo.deleteUser(user)

    // then
    assertEquals(0, repo.getKeysSize())
}

The test fails because after the deleteUser method, the keys are the following:

  • Bob.company?
  • Bob.name
  • Bob?

I guess the error is that it adds nullability on the User object instead of removing the constructed keys upon the User. Maybe it is because I use a key that is built upon an element of User (and my real case uses a key that is the concatenation of three elements).

For now, my hack to circumvent this issue is to do a brutal:

settings.keys
    .filter { it.startsWith(user.name) }
    .forEach { settings.remove(it) }

Hope this will help to find a fix for this bug. Maybe an explicit method like settings.removeValue(serializer, key) would clarify the removal of serialized objects. Thanks!

stephanepechard avatar Jun 10 '22 07:06 stephanepechard

Thanks, that makes it a bit more clear. I do like the idea of a serialization-aware versions of remove() and contains(), though I'm not sure what the behavior of the contains()-analog would be if the value is partially present. But remove() seems like it would probably be straightforward.

russhwolf avatar Jun 10 '22 23:06 russhwolf

Here's my current thinking about handling these APIs when a value is partially present. (Ironically, the complications are in the opposite place that I expected).

In the existing API, decodeValue() doesn't handle partial values and falls back to the default or null value instead. So it makes sense for serialization-aware containsValue() to do the same thing.

But there is a choice to make in how to handle removeValue() with a partially-present value. Option 1 is to clear whatever we do find, so that after a removeValue() call the user can be certain that no data for that object remains in the Settings store. Option 2 is to do nothing, because there's a chance the partial values are just there due to an accidental naming collision or other coincidence. For those who care about this API, which of those behaviors would be preferable? Which would be less surprising?

There's a draft PR in #117 which uses option 1 for the removeValue() API because that was my first naive implementation, but I don't have a strong preference right now for which option to actually use.

russhwolf avatar Jun 18 '22 21:06 russhwolf

Hi, I encountered the same problem.

I have this code in my application:

class DecodedItem<T> internal constructor(
        private val key: String,
        private val settings: ObservableSettings,
        private val serializer: KSerializer<T>
    ) {

        @OptIn(ExperimentalSerializationApi::class)
        var value: T?
            get() {
                return settings.decodeValueOrNull(key = key, serializer = serializer)
            }
            set(newValue) {
                _mutableStateFlow.value = newValue

                if (newValue == null) {
                    settings.remove(key)
                    return
                }
                settings.encodeValue(serializer, key, newValue)
            }

        private val _mutableStateFlow: MutableStateFlow<T?> = MutableStateFlow(value)
        val stateFlow: StateFlow<T?> = _mutableStateFlow.asStateFlow()
    }

After applying settings.remove(key), the user data is not removed.

Regarding your last comment, I think it would be logical to use first option to handle removeValue. It was a surprise to me that calling remove does not remove data from KVS. I managed to figure it out only by looking at the implementation of this function and seeing that it has nothing to do with the serialization module.

So far I got around this problem by creating another logical variable responsible for data availability. @russhwolf But I really want this problem to be solved soon by API of this library.

kramlex avatar Jun 11 '23 05:06 kramlex

This will be released soon as part of version 1.1. I opted to include a flag in removeValue() to toggle the behavior if the serialized value is only partially present. The default behavior without the flag is to remove partial values. Last call if anyone has opinions on that choice.

russhwolf avatar Aug 20 '23 02:08 russhwolf

This was included in version 1.1.0

russhwolf avatar Oct 10 '23 02:10 russhwolf