Merge pull request #85 from f2prateek/set-non-null

Only allow NonNull values on Preference#set.

rx-preferences/src/main/java/com/f2prateek/rx/preferences2/Preference.java

@@ -3,7 +3,6 @@
 import android.content.SharedPreferences;
 import android.support.annotation.CheckResult;
 import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
 import io.reactivex.Observable;
 import io.reactivex.functions.Consumer;
 
@@ -43,7 +42,7 @@ interface Converter<T> {
    * Change this preference's stored value to {@code value}. A value of {@code null} will delete
    * the preference.
    */
-  void set(@Nullable T value);
+  void set(@NonNull T value);
 
   /** Returns true if this preference has a stored value. */
   boolean isSet();

rx-preferences/src/main/java/com/f2prateek/rx/preferences2/RealPreference.java

@@ -1,9 +1,10 @@
 package com.f2prateek.rx.preferences2;
 
+import static com.f2prateek.rx.preferences2.Preconditions.checkNotNull;
+
 import android.content.SharedPreferences;
 import android.support.annotation.CheckResult;
 import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
 import io.reactivex.Observable;
 import io.reactivex.functions.Consumer;
 import io.reactivex.functions.Function;
@@ -65,22 +66,19 @@ interface Adapter<T> {
     return adapter.get(key, preferences);
   }
 
-  @Override public synchronized void set(@Nullable T value) {
+  @Override public void set(@NonNull T value) {
+    checkNotNull(value, "value == null");
     SharedPreferences.Editor editor = preferences.edit();
-    if (value == null) {
-      editor.remove(key);
-    } else {
-      adapter.set(key, value, editor);
-    }
+    adapter.set(key, value, editor);
     editor.apply();
   }
 
   @Override public boolean isSet() {
     return preferences.contains(key);
   }
 
-  @Override public void delete() {
-    set(null);
+  @Override public synchronized void delete() {
+    preferences.edit().remove(key).apply();
   }
 
   @Override @CheckResult @NonNull public Observable<T> asObservable() {

rx-preferences/src/test/java/com/f2prateek/rx/preferences2/PreferenceTest.java

@@ -115,38 +115,55 @@ public class PreferenceTest {
     assertThat(preferences.getString("foo8", null)).isEqualTo("1,2");
   }
 
-  @Test public void setNullDeletes() {
-    preferences.edit().putBoolean("foo1", true).commit();
-    rxPreferences.getBoolean("foo1").set(null);
-    assertThat(preferences.contains("foo1")).isFalse();
+  @SuppressWarnings("ConstantConditions")
+  @Test public void setNullThrows() {
+    try {
+      rxPreferences.getBoolean("foo1").set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
 
-    preferences.edit().putString("foo2", "ROCK").commit();
-    rxPreferences.getEnum("foo2", ROCK, Roshambo.class).set(null);
-    assertThat(preferences.contains("foo2")).isFalse();
+    try {
+      rxPreferences.getEnum("foo2", ROCK, Roshambo.class).set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
 
-    preferences.edit().putFloat("foo3", 1f).commit();
-    rxPreferences.getFloat("foo3").set(null);
-    assertThat(preferences.contains("foo3")).isFalse();
+    try {
+      rxPreferences.getFloat("foo3").set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
 
-    preferences.edit().putInt("foo4", 1).commit();
-    rxPreferences.getInteger("foo4").set(null);
-    assertThat(preferences.contains("foo4")).isFalse();
+    try {
+      rxPreferences.getInteger("foo4").set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
 
-    preferences.edit().putLong("foo5", 1L).commit();
-    rxPreferences.getLong("foo5").set(null);
-    assertThat(preferences.contains("foo5")).isFalse();
+    try {
+      rxPreferences.getLong("foo5").set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
 
-    preferences.edit().putString("foo6", "bar").commit();
-    rxPreferences.getString("foo6").set(null);
-    assertThat(preferences.contains("foo6")).isFalse();
+    try {
+      rxPreferences.getString("foo6").set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
 
-    preferences.edit().putStringSet("foo7", singleton("bar")).commit();
-    rxPreferences.getStringSet("foo7").set(null);
-    assertThat(preferences.contains("foo7")).isFalse();
+    try {
+      rxPreferences.getStringSet("foo7").set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
 
-    preferences.edit().putString("foo8", "1,2").commit();
-    rxPreferences.getObject("foo8", new Point(1, 2), pointConverter).set(null);
-    assertThat(preferences.contains("foo8")).isFalse();
+    try {
+      rxPreferences.getObject("foo8", new Point(1, 2), pointConverter).set(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
   }
 
   @Test public void isSet() {
@@ -175,10 +192,12 @@ public class PreferenceTest {
   @Test public void converterMayNotReturnNull() {
     Preference<Point> preference =
         rxPreferences.getObject("foo", new Point(0, 0), new Preference.Converter<Point>() {
+          @SuppressWarnings("ConstantConditions")
           @NonNull @Override public Point deserialize(@NonNull String serialized) {
             return null;
           }
 
+          @SuppressWarnings("ConstantConditions")
           @NonNull @Override public String serialize(@NonNull Point value) {
             return null;
           }
@@ -246,7 +265,10 @@ public class PreferenceTest {
     consumer.accept("baz");
     assertThat(preferences.getString("foo", null)).isEqualTo("baz");
 
-    consumer.accept(null);
-    assertThat(preferences.contains("foo")).isFalse();
+    try {
+      consumer.accept(null);
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("value == null");
+    }
   }
 }