apply() is lossy

[kurtisnelson]

Using apply() to save preferences is dangerous as it can and will silently fail. Instead, all sets should use commit() and return a completable, throwing an error if commit() returns false.

[JakeWharton]

Somewhere you dropped an "optionally" from your feature request, right? Almost no one will want to use this as it destroys the ergonomics of the API and few will be using preferences for anything where the absence of durability is a real problem.

Prior art: #110 #42

[kurtisnelson]

The existing API could at least track the failure somewhere. At scale I've gotten many a bug report where a feature seemed to be not working where in fact a preference was not persisting. It's super handy to at least have the exception in my crash reporting system so I know what happened.

[sevar83]

I think I've got a race condition because of the apply(). My theory is that when another observer subscribes before the data is actually updated within apply() then the observer will get stale data.

p.s. More about the problem: https://stackoverflow.com/questions/49593476/android-sharedpreferences-apply-race-condition Look at the Mark Keen's comment.

[kirillzh]

I'm seeing that the library creates a new Preferences.Editor every time changes are made. Considering that apply() is synchronous and atomic but it's called from different Editor instances, would using the same instance of Editor fix the problem?

[sirknigget]

We also encountered a major problem because of apply(). We NEED the atomic persistence to disk.

I agree with not destroying the API, but why not expose the possibility from the library? Currently it's not possible to perform commit() using RxSharedPreferences. How about some atomicSet() method?

[kurtisnelson]

I ended up writing a library that provides a proper async key-value store: https://github.com/uber/simple-store

[sirknigget]

Still missing this basic functionality. The current library completely hides basic Android API stuff that I would normally use, and I end up having to patch around RxSharedPreferences in order to be able to:

• Call Editor.commit() when setting a preference.
• Access the SharedPreferences field inside a RealPreference ( current it's: private final SharedPreferences preferences; )

In the current situation I have to either fork rx-preferences and change those library classes, patch around those limitations, or move to using another library.

I think it's a very small, easy to implement request, and it doesn't break anything. Just add "setSync" to RealPreference which would call Editor.commit()...