f2prateek / rx-preferences

Reactive SharedPreferences for Android
http://f2prateek.com/2015/10/05/rx-preferences/
Apache License 2.0
1.54k stars 132 forks source link

StringAdapter returns null #131

Closed rjrjr closed 4 years ago

rjrjr commented 4 years ago

StringAdapter assumes that it will never read a null from SharedPreferences.getString, which is not true. It enforces this invariant with an assert, which is a no-op in most builds:

    assert value != null; // Not called unless key is present.

SharedPreferences is happy to record null values, and report that they have been set. For example, this test hits the assertion above:

  @Test fun sharedPreferencesStringAdapterIsNullDefensive() {
    sharedPreferences.edit().putString("fnord", null).commit()
    assertThat(sharedPreferences.contains("fnord"))
    assertThat(sharedPreferences.getString("fnord", null)).isNull()

    val pref = createPreferences().getString("fnord")
    // We never reach the assertThat below, due to the assert in the getString call above.

    assertThat(pref.get()).isEqualTo("")
  }
rjrjr commented 4 years ago

We're hitting this issue in code that I recently converted to RxSharedPreferences. It's reading existing string values, and apparently some of them are null.

rjrjr commented 4 years ago

Should be able to fix this in RealPreference.get, if we just stop relying on SharedPreferences.contains.

rjrjr commented 4 years ago

Similar test as an addition to PreferenceTest:

  @Test public void legacyNullString() {
    preferences.edit()
        .putString("someString", null)
        .commit();

    assertThat(rxPreferences.getString("someString").get()).isEqualTo("");
  }
f2prateek commented 4 years ago

Similar to https://github.com/f2prateek/rx-preferences/issues/123