From 42c9b8fe41eed7447e7e1d7f69d4136b57d14a68 Mon Sep 17 00:00:00 2001 From: Gabor Keszthelyi Date: Thu, 25 Jan 2018 15:46:32 +0100 Subject: [PATCH 1/3] Add nullability annotations to ContentSet methods. #650 --- .idea/codeInsightSettings.xml | 9 +++ .../java/org/dmfs/tasks/model/ContentSet.java | 79 +++++++++++-------- .../tasks/utils/OnContentLoadedListener.java | 3 +- 3 files changed, 55 insertions(+), 36 deletions(-) create mode 100644 .idea/codeInsightSettings.xml diff --git a/.idea/codeInsightSettings.xml b/.idea/codeInsightSettings.xml new file mode 100644 index 000000000..b380aeb58 --- /dev/null +++ b/.idea/codeInsightSettings.xml @@ -0,0 +1,9 @@ + + + + + javax.annotation.Nonnull + javax.annotation.Nullable + + + \ No newline at end of file diff --git a/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java b/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java index 1e32f0f21..3d4f66562 100644 --- a/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java +++ b/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java @@ -22,6 +22,8 @@ import android.net.Uri; import android.os.Parcel; import android.os.Parcelable; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.util.Log; import org.dmfs.tasks.utils.AsyncContentLoader; @@ -112,13 +114,8 @@ private ContentSet() * @param uri * A content URI, either a directory URI or an item URI. */ - public ContentSet(Uri uri) + public ContentSet(@NonNull Uri uri) { - if (uri == null) - { - throw new IllegalArgumentException("uri must not be null"); - } - mUri = uri; } @@ -129,13 +126,8 @@ public ContentSet(Uri uri) * @param other * The {@link ContentSet} to clone. */ - public ContentSet(ContentSet other) + public ContentSet(@NonNull ContentSet other) { - if (other == null) - { - throw new IllegalArgumentException("other must not be null"); - } - if (other.mBeforeContentValues != null) { mBeforeContentValues = new ContentValues(other.mBeforeContentValues); @@ -159,7 +151,7 @@ public ContentSet(ContentSet other) * @param mapper * The {@link ContentValueMapper} to use when loading the values. */ - public void update(Context context, ContentValueMapper mapper) + public void update(@NonNull Context context, @NonNull ContentValueMapper mapper) { String itemType = context.getContentResolver().getType(mUri); if (itemType != null && !itemType.startsWith(ContentResolver.CURSOR_DIR_BASE_TYPE)) @@ -175,7 +167,7 @@ public void update(Context context, ContentValueMapper mapper) @Override - public void onContentLoaded(ContentValues values) + public void onContentLoaded(@NonNull ContentValues values) { mBeforeContentValues = values; mLoading = false; @@ -200,7 +192,7 @@ public boolean isLoading() * @param context * A context. */ - public void delete(Context context) + public void delete(@NonNull Context context) { if (mUri != null) { @@ -226,10 +218,16 @@ public void delete(Context context) } - public Uri persist(Context context) + @NonNull + public Uri persist(@NonNull Context context) { if (mAfterContentValues == null || mAfterContentValues.size() == 0) { + if (mUri == null) + { + throw new IllegalStateException("Task uri is null, cannot persist(). (Called after delete()?)"); + } + // nothing to do here return mUri; } @@ -247,6 +245,11 @@ else if (isUpdate()) mAfterContentValues = null; + if (mUri == null) + { + throw new IllegalStateException("Task uri is null, cannot persist(). (Called after delete()?)"); + } + return mUri; } @@ -263,26 +266,26 @@ public boolean isUpdate() } - public boolean containsKey(String key) + public boolean containsKey(@NonNull String key) { return mAfterContentValues != null && mAfterContentValues.containsKey(key) || mBeforeContentValues != null && mBeforeContentValues.containsKey(key); } - public boolean persistsKey(String key) + private boolean persistsKey(@NonNull String key) { return mAfterContentValues != null && mAfterContentValues.containsKey(key); } - public boolean updatesAnyKey(Set keys) + public boolean updatesAnyKey(@NonNull Set keys) { if (mAfterContentValues == null) { return false; } - Set keySet = new HashSet(mAfterKeys); + Set keySet = new HashSet<>(mAfterKeys); keySet.retainAll(keys); @@ -291,7 +294,7 @@ public boolean updatesAnyKey(Set keys) } - public void ensureUpdates(Set keys) + public void ensureUpdates(@NonNull Set keys) { if (mBeforeContentValues == null || keys == null || keys.isEmpty()) { @@ -328,6 +331,7 @@ public void ensureUpdates(Set keys) } + @NonNull private ContentValues ensureAfter() { ContentValues values = mAfterContentValues; @@ -336,13 +340,13 @@ private ContentValues ensureAfter() values = new ContentValues(); mAfterContentValues = values; // also create mAfterKeys - mAfterKeys = new HashSet(); + mAfterKeys = new HashSet<>(); } return values; } - public void put(String key, Integer value) + public void put(@NonNull String key, @Nullable Integer value) { Integer oldValue = getAsInteger(key); if (value != null && !value.equals(oldValue) || value == null && oldValue != null) @@ -367,7 +371,8 @@ public void put(String key, Integer value) } - public Integer getAsInteger(String key) + @Nullable + public Integer getAsInteger(@NonNull String key) { final ContentValues after = mAfterContentValues; if (after != null && after.containsKey(key)) @@ -378,7 +383,7 @@ public Integer getAsInteger(String key) } - public void put(String key, Long value) + public void put(@NonNull String key, @Nullable Long value) { Long oldValue = getAsLong(key); if (value != null && !value.equals(oldValue) || value == null && oldValue != null) @@ -402,7 +407,8 @@ public void put(String key, Long value) } - public Long getAsLong(String key) + @Nullable + public Long getAsLong(@NonNull String key) { final ContentValues after = mAfterContentValues; if (after != null && after.containsKey(key)) @@ -413,7 +419,7 @@ public Long getAsLong(String key) } - public void put(String key, String value) + public void put(@NonNull String key, @Nullable String value) { String oldValue = getAsString(key); if (value != null && !value.equals(oldValue) || value == null && oldValue != null) @@ -437,7 +443,8 @@ public void put(String key, String value) } - public String getAsString(String key) + @Nullable + public String getAsString(@NonNull String key) { final ContentValues after = mAfterContentValues; if (after != null && after.containsKey(key)) @@ -448,7 +455,7 @@ public String getAsString(String key) } - public void put(String key, Float value) + public void put(@NonNull String key, @Nullable Float value) { Float oldValue = getAsFloat(key); if (value != null && !value.equals(oldValue) || value == null && oldValue != null) @@ -472,7 +479,8 @@ public void put(String key, Float value) } - public Float getAsFloat(String key) + @Nullable + public Float getAsFloat(@Nullable String key) { final ContentValues after = mAfterContentValues; if (after != null && after.containsKey(key)) @@ -489,6 +497,7 @@ public Float getAsFloat(String key) * * @return The {@link Uri}. */ + @Nullable public Uri getUri() { return mUri; @@ -529,7 +538,7 @@ public void finishBulkUpdate() * @param key * The key of the value to remove. */ - public void remove(String key) + public void remove(@NonNull String key) { if (mAfterContentValues != null) { @@ -544,7 +553,7 @@ else if (mBeforeContentValues != null && mBeforeContentValues.get(key) != null) } - public void addOnChangeListener(OnContentChangeListener listener, String key, boolean notify) + public void addOnChangeListener(@NonNull OnContentChangeListener listener, @Nullable String key, boolean notify) { Set listenerSet = mOnChangeListeners.get(key); if (listenerSet == null) @@ -563,7 +572,7 @@ public void addOnChangeListener(OnContentChangeListener listener, String key, bo } - public void removeOnChangeListener(OnContentChangeListener listener, String key) + public void removeOnChangeListener(@NonNull OnContentChangeListener listener, @Nullable String key) { Set listenerSet = mOnChangeListeners.get(key); if (listenerSet != null) @@ -573,7 +582,7 @@ public void removeOnChangeListener(OnContentChangeListener listener, String key) } - private void notifyUpdateListeners(String key) + private void notifyUpdateListeners(@NonNull String key) { Set listenerSet = mOnChangeListeners.get(key); if (listenerSet != null) @@ -632,7 +641,7 @@ public void writeToParcel(Parcel dest, int flags) } - public void readFromParcel(Parcel source) + private void readFromParcel(Parcel source) { ClassLoader loader = getClass().getClassLoader(); mUri = source.readParcelable(loader); diff --git a/opentasks/src/main/java/org/dmfs/tasks/utils/OnContentLoadedListener.java b/opentasks/src/main/java/org/dmfs/tasks/utils/OnContentLoadedListener.java index 612be08fb..ce8f3544c 100644 --- a/opentasks/src/main/java/org/dmfs/tasks/utils/OnContentLoadedListener.java +++ b/opentasks/src/main/java/org/dmfs/tasks/utils/OnContentLoadedListener.java @@ -17,6 +17,7 @@ package org.dmfs.tasks.utils; import android.content.ContentValues; +import android.support.annotation.NonNull; /** @@ -32,5 +33,5 @@ public interface OnContentLoadedListener * @param values * The loaded {@link ContentValues}. */ - public void onContentLoaded(ContentValues values); + void onContentLoaded(@NonNull ContentValues values); } From 41540999029606b956c4517fdbc34fb06eebbd1b Mon Sep 17 00:00:00 2001 From: Gabor Keszthelyi Date: Fri, 26 Jan 2018 15:21:21 +0100 Subject: [PATCH 2/3] Remove mAfterKeys compatibility workaround from ContentSet. --- .../java/org/dmfs/tasks/model/ContentSet.java | 49 +------------------ 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java b/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java index 3d4f66562..df0341361 100644 --- a/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java +++ b/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java @@ -83,16 +83,6 @@ public final class ContentSet implements OnContentLoadedListener, Parcelable */ private final Set mPendingNotifications = new HashSet(); - /** - * Holds the name of the keys we've updated in {@link #mAfterContentValues}. - *

- * Before Android SDK level 11 there is no {@link ContentValues#keySet()} method. To be able to determine the keys in there we have to maintain the set - * ourselves. - *

- * Don't use this before calling {@link #ensureAfter()} at least once. - */ - private Set mAfterKeys; - /** * Indicates that loading is in process. */ @@ -136,7 +126,6 @@ public ContentSet(@NonNull ContentSet other) if (other.mAfterContentValues != null) { mAfterContentValues = new ContentValues(other.mAfterContentValues); - mAfterKeys = new HashSet(other.mAfterKeys); } mUri = other.mUri; @@ -202,7 +191,6 @@ public void delete(@NonNull Context context) context.getContentResolver().delete(mUri, null, null); mBeforeContentValues = null; mAfterContentValues = null; - mAfterKeys = null; mUri = null; } else @@ -285,7 +273,7 @@ public boolean updatesAnyKey(@NonNull Set keys) return false; } - Set keySet = new HashSet<>(mAfterKeys); + Set keySet = new HashSet<>(mAfterContentValues.keySet()); keySet.retainAll(keys); @@ -296,7 +284,7 @@ public boolean updatesAnyKey(@NonNull Set keys) public void ensureUpdates(@NonNull Set keys) { - if (mBeforeContentValues == null || keys == null || keys.isEmpty()) + if (mBeforeContentValues == null || keys.isEmpty()) { // nothing to do return; @@ -339,8 +327,6 @@ private ContentValues ensureAfter() { values = new ContentValues(); mAfterContentValues = values; - // also create mAfterKeys - mAfterKeys = new HashSet<>(); } return values; } @@ -358,14 +344,12 @@ public void put(@NonNull String key, @Nullable Integer value) { // value equals before value, so remove it from after values mAfterContentValues.remove(key); - mAfterKeys.remove(key); notifyUpdateListeners(key); return; } } // value has changed, update ensureAfter().put(key, value); - mAfterKeys.add(key); notifyUpdateListeners(key); } } @@ -395,13 +379,11 @@ public void put(@NonNull String key, @Nullable Long value) { // value equals before value, so remove it from after values mAfterContentValues.remove(key); - mAfterKeys.remove(key); notifyUpdateListeners(key); return; } } ensureAfter().put(key, value); - mAfterKeys.add(key); notifyUpdateListeners(key); } } @@ -431,13 +413,11 @@ public void put(@NonNull String key, @Nullable String value) { // value equals before value, so remove it from after values mAfterContentValues.remove(key); - mAfterKeys.remove(key); notifyUpdateListeners(key); return; } } ensureAfter().put(key, value); - mAfterKeys.add(key); notifyUpdateListeners(key); } } @@ -467,13 +447,11 @@ public void put(@NonNull String key, @Nullable Float value) { // value equals before value, so remove it from after values mAfterContentValues.remove(key); - mAfterKeys.remove(key); notifyUpdateListeners(key); return; } } ensureAfter().put(key, value); - mAfterKeys.add(key); notifyUpdateListeners(key); } } @@ -543,12 +521,10 @@ public void remove(@NonNull String key) if (mAfterContentValues != null) { mAfterContentValues.putNull(key); - mAfterKeys.add(key); } else if (mBeforeContentValues != null && mBeforeContentValues.get(key) != null) { ensureAfter().putNull(key); - mAfterKeys.add(key); } } @@ -628,16 +604,6 @@ public void writeToParcel(Parcel dest, int flags) dest.writeParcelable(mUri, flags); dest.writeParcelable(mBeforeContentValues, flags); dest.writeParcelable(mAfterContentValues, flags); - - if (mAfterContentValues != null) - { - // It's not possible to write a Set to a parcel, so write the number of members and each member individually. - dest.writeInt(mAfterKeys.size()); - for (String key : mAfterKeys) - { - dest.writeString(key); - } - } } @@ -647,17 +613,6 @@ private void readFromParcel(Parcel source) mUri = source.readParcelable(loader); mBeforeContentValues = source.readParcelable(loader); mAfterContentValues = source.readParcelable(loader); - - if (mAfterContentValues != null) - { - int count = source.readInt(); - Set keys = new HashSet(); - while (--count >= 0) - { - keys.add(source.readString()); - } - mAfterKeys = keys; - } } From 4453861aa3882ae2ed8e6ca48354adfcfdb56563 Mon Sep 17 00:00:00 2001 From: Gabor Keszthelyi Date: Fri, 26 Jan 2018 15:36:48 +0100 Subject: [PATCH 3/3] Add javadocs about the key parameter of the listener registration methods. --- .../src/main/java/org/dmfs/tasks/model/ContentSet.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java b/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java index df0341361..6b803925e 100644 --- a/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java +++ b/opentasks/src/main/java/org/dmfs/tasks/model/ContentSet.java @@ -529,6 +529,10 @@ else if (mBeforeContentValues != null && mBeforeContentValues.get(key) != null) } + /** + * @param key + * the specific key in a {@link ContentSet} to listen the changes for, or null to be notified of full reloads. + */ public void addOnChangeListener(@NonNull OnContentChangeListener listener, @Nullable String key, boolean notify) { Set listenerSet = mOnChangeListeners.get(key); @@ -548,6 +552,11 @@ public void addOnChangeListener(@NonNull OnContentChangeListener listener, @Null } + /** + * @param key + * The specific key in a {@link ContentSet} to remove the listening about, or null to remove listening from full reloads. See {@link + * #addOnChangeListener(OnContentChangeListener, String, boolean)} as well. + */ public void removeOnChangeListener(@NonNull OnContentChangeListener listener, @Nullable String key) { Set listenerSet = mOnChangeListeners.get(key);