Refactor notifications adapter (#985)

* Fix unnecessary reloading of notifications

This removes topId as it is not needed and just plainly uses status id
if needed. During initial loading of notifications topId/bottomId are
not set so we ended up reloading everything.

* Refactor notifications adapter

Use AsyncListDiffer for updating notifications just like in timelines.

* Cleanup in NotificationsFragment

app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java

@@ -20,11 +20,6 @@ import android.graphics.Color;
 import android.graphics.PorterDuff;
 import android.graphics.Typeface;
 import android.graphics.drawable.Drawable;
-import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
-import androidx.core.content.ContextCompat;
-import androidx.core.text.BidiFormatter;
-import androidx.recyclerview.widget.RecyclerView;
 import android.text.InputFilter;
 import android.text.SpannableStringBuilder;
 import android.text.Spanned;
@@ -53,12 +48,25 @@ import com.keylesspalace.tusky.viewdata.StatusViewData;
 import com.squareup.picasso.Picasso;
 
 import java.text.SimpleDateFormat;
-import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 import java.util.Locale;
 
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+import androidx.core.content.ContextCompat;
+import androidx.core.text.BidiFormatter;
+import androidx.recyclerview.widget.RecyclerView;
+
 public class NotificationsAdapter extends RecyclerView.Adapter {
+
+    public interface AdapterDataSource<T> {
+        int getItemCount();
+
+        T getItemAt(int pos);
+    }
+
+
     private static final int VIEW_TYPE_MENTION = 0;
     private static final int VIEW_TYPE_STATUS_NOTIFICATION = 1;
     private static final int VIEW_TYPE_FOLLOW = 2;
@@ -67,17 +75,18 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
     private static final InputFilter[] COLLAPSE_INPUT_FILTER = new InputFilter[] { SmartLengthInputFilter.INSTANCE };
     private static final InputFilter[] NO_INPUT_FILTER = new InputFilter[0];
 
-    private List<NotificationViewData> notifications;
     private StatusActionListener statusListener;
     private NotificationActionListener notificationActionListener;
     private boolean mediaPreviewEnabled;
     private boolean useAbsoluteTime;
     private BidiFormatter bidiFormatter;
+    private AdapterDataSource<NotificationViewData> dataSource;
 
-    public NotificationsAdapter(StatusActionListener statusListener,
+    public NotificationsAdapter(AdapterDataSource<NotificationViewData> dataSource,
+                                StatusActionListener statusListener,
                                 NotificationActionListener notificationActionListener) {
         super();
-        notifications = new ArrayList<>();
+        this.dataSource = dataSource;
         this.statusListener = statusListener;
         this.notificationActionListener = notificationActionListener;
         mediaPreviewEnabled = true;
@@ -115,8 +124,8 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
 
     @Override
     public void onBindViewHolder(@NonNull RecyclerView.ViewHolder viewHolder, int position) {
-        if (position < notifications.size()) {
+        if (position < this.dataSource.getItemCount()) {
-            NotificationViewData notification = notifications.get(position);
+            NotificationViewData notification = dataSource.getItemAt(position);
             if (notification instanceof NotificationViewData.Placeholder) {
                 NotificationViewData.Placeholder placeholder = ((NotificationViewData.Placeholder) notification);
                 PlaceholderViewHolder holder = (PlaceholderViewHolder) viewHolder;
@@ -170,12 +179,12 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
 
     @Override
     public int getItemCount() {
-        return notifications.size();
+        return dataSource.getItemCount();
     }
 
     @Override
     public int getItemViewType(int position) {
-        NotificationViewData notification = notifications.get(position);
+        NotificationViewData notification = dataSource.getItemAt(position);
         if (notification instanceof NotificationViewData.Concrete) {
             NotificationViewData.Concrete concrete = ((NotificationViewData.Concrete) notification);
             switch (concrete.getType()) {
@@ -199,36 +208,6 @@ public class NotificationsAdapter extends RecyclerView.Adapter {
 
     }
 
-    public void update(@Nullable List<NotificationViewData> newNotifications) {
-        if (newNotifications == null || newNotifications.isEmpty()) {
-            return;
-        }
-        notifications.clear();
-        notifications.addAll(newNotifications);
-        notifyDataSetChanged();
-    }
-
-    public void updateItemWithNotify(int position, NotificationViewData notification,
-                                     boolean notifyAdapter) {
-        notifications.set(position, notification);
-        if (notifyAdapter) notifyItemChanged(position);
-    }
-
-    public void addItems(List<NotificationViewData> newNotifications) {
-        notifications.addAll(newNotifications);
-        notifyItemRangeInserted(notifications.size(), newNotifications.size());
-    }
-
-    public void removeItemAndNotify(int position) {
-        notifications.remove(position);
-        notifyItemRemoved(position);
-    }
-
-    public void clear() {
-        notifications.clear();
-        notifyDataSetChanged();
-    }
-
     public void setMediaPreviewEnabled(boolean enabled) {
         mediaPreviewEnabled = enabled;
     }

app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java

@@ -58,7 +58,6 @@ import com.keylesspalace.tusky.viewdata.NotificationViewData;
 import com.keylesspalace.tusky.viewdata.StatusViewData;
 
 import java.io.IOException;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
@@ -70,11 +69,16 @@ import androidx.annotation.Nullable;
 import androidx.arch.core.util.Function;
 import androidx.core.util.Pair;
 import androidx.lifecycle.Lifecycle;
+import androidx.recyclerview.widget.AsyncDifferConfig;
+import androidx.recyclerview.widget.AsyncListDiffer;
+import androidx.recyclerview.widget.DiffUtil;
 import androidx.recyclerview.widget.DividerItemDecoration;
 import androidx.recyclerview.widget.LinearLayoutManager;
+import androidx.recyclerview.widget.ListUpdateCallback;
 import androidx.recyclerview.widget.RecyclerView;
 import androidx.recyclerview.widget.SimpleItemAnimator;
 import androidx.swiperefreshlayout.widget.SwipeRefreshLayout;
+import at.connyduck.sparkbutton.helpers.Utils;
 import io.reactivex.android.schedulers.AndroidSchedulers;
 import kotlin.Unit;
 import kotlin.collections.CollectionsKt;
@@ -94,6 +98,7 @@ public class NotificationsFragment extends SFragment implements
     private static final String TAG = "NotificationF"; // logging tag
 
     private static final int LOAD_AT_ONCE = 30;
+    private int maxPlaceholderId = 0;
 
     private enum FetchEnd {
         TOP,
@@ -106,13 +111,14 @@ public class NotificationsFragment extends SFragment implements
      * and reuse in different places as needed.
      */
     private static final class Placeholder {
-        private static final Placeholder INSTANCE = new Placeholder();
+        final long id;
 
-        public static Placeholder getInstance() {
+        public static Placeholder getInstance(long id) {
-            return INSTANCE;
+            return new Placeholder(id);
         }
 
-        private Placeholder() {
+        private Placeholder(long id) {
+            this.id = id;
         }
     }
 
@@ -155,7 +161,7 @@ public class NotificationsFragment extends SFragment implements
                         alwaysShowSensitiveMedia
                 );
             } else {
-                return new NotificationViewData.Placeholder(false);
+                return new NotificationViewData.Placeholder(input.asLeft().id, false);
             }
         }
     });
@@ -200,7 +206,7 @@ public class NotificationsFragment extends SFragment implements
 
         recyclerView.addItemDecoration(new DividerItemDecoration(context, DividerItemDecoration.VERTICAL));
 
-        adapter = new NotificationsAdapter(this, this);
+        adapter = new NotificationsAdapter(dataSource, this, this);
         SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(getActivity());
         alwaysShowSensitiveMedia = accountManager.getActiveAccount().getAlwaysShowSensitiveMedia();
         boolean mediaPreviewEnabled = accountManager.getActiveAccount().getMediaPreviewEnabled();
@@ -380,11 +386,9 @@ public class NotificationsFragment extends SFragment implements
                 viewDataBuilder.createStatusViewData(), viewdata.isExpanded());
 
         notifications.setPairedItem(position, newViewData);
-
+        updateAdapter();
-        adapter.updateItemWithNotify(position, newViewData, true);
     }
 
-
     @Override
     public void onFavourite(final boolean favourite, final int position) {
         final Notification notification = notifications.get(position).asRight();
@@ -417,8 +421,7 @@ public class NotificationsFragment extends SFragment implements
                 viewDataBuilder.createStatusViewData(), viewdata.isExpanded());
 
         notifications.setPairedItem(position, newViewData);
-
+        updateAdapter();
-        adapter.updateItemWithNotify(position, newViewData, true);
     }
 
     @Override
@@ -457,7 +460,7 @@ public class NotificationsFragment extends SFragment implements
         NotificationViewData notificationViewData = new NotificationViewData.Concrete(old.getType(),
                 old.getId(), old.getAccount(), statusViewData, expanded);
         notifications.setPairedItem(position, notificationViewData);
-        adapter.updateItemWithNotify(position, notificationViewData, false);
+        updateAdapter();
     }
 
     @Override
@@ -471,7 +474,7 @@ public class NotificationsFragment extends SFragment implements
         NotificationViewData notificationViewData = new NotificationViewData.Concrete(old.getType(),
                 old.getId(), old.getAccount(), statusViewData, old.isExpanded());
         notifications.setPairedItem(position, notificationViewData);
-        adapter.updateItemWithNotify(position, notificationViewData, false);
+        updateAdapter();
     }
 
     @Override
@@ -485,10 +488,11 @@ public class NotificationsFragment extends SFragment implements
                 return;
             }
             sendFetchNotificationsRequest(previous.getId(), next.getId(), FetchEnd.MIDDLE, position);
+            Placeholder placeholder = notifications.get(position).asLeft();
             NotificationViewData notificationViewData =
-                    new NotificationViewData.Placeholder(true);
+                    new NotificationViewData.Placeholder(placeholder.id, true);
             notifications.setPairedItem(position, notificationViewData);
-            adapter.updateItemWithNotify(position, notificationViewData, false);
+            updateAdapter();
         } else {
             Log.d(TAG, "error loading more");
         }
@@ -526,7 +530,7 @@ public class NotificationsFragment extends SFragment implements
                 concreteNotification.isExpanded()
         );
         notifications.setPairedItem(position, updatedNotification);
-        adapter.updateItemWithNotify(position, updatedNotification, false);
+        updateAdapter();
 
         // Since we cannot notify to the RecyclerView right away because it may be scrolling
         // we run this when the RecyclerView is done doing measurements and other calculations.
@@ -582,7 +586,7 @@ public class NotificationsFragment extends SFragment implements
     @Override
     public void removeItem(int position) {
         notifications.remove(position);
-        adapter.update(notifications.getPairedCopy());
+        updateAdapter();
     }
 
     private void removeAllByAccountId(String accountId) {
@@ -595,7 +599,7 @@ public class NotificationsFragment extends SFragment implements
                 iterator.remove();
             }
         }
-        adapter.update(notifications.getPairedCopy());
+        updateAdapter();
     }
 
     private void onLoadMore() {
@@ -610,16 +614,24 @@ public class NotificationsFragment extends SFragment implements
         if (notifications.size() > 0) {
             Either<Placeholder, Notification> last = notifications.get(notifications.size() - 1);
             if (last.isRight()) {
-                notifications.add(new Either.Left(Placeholder.getInstance()));
+                final Placeholder placeholder = newPlaceholder();
-                NotificationViewData viewData = new NotificationViewData.Placeholder(true);
+                notifications.add(new Either.Left<>(placeholder));
+                NotificationViewData viewData =
+                        new NotificationViewData.Placeholder(placeholder.id, true);
                 notifications.setPairedItem(notifications.size() - 1, viewData);
-                recyclerView.post(() -> adapter.addItems(Collections.singletonList(viewData)));
+                updateAdapter();
             }
         }
 
         sendFetchNotificationsRequest(bottomId, null, FetchEnd.BOTTOM, -1);
     }
 
+    private Placeholder newPlaceholder() {
+        Placeholder placeholder = Placeholder.getInstance(maxPlaceholderId);
+        maxPlaceholderId--;
+        return placeholder;
+    }
+
     private void jumpToTop() {
         layoutManager.scrollToPosition(0);
         scrollListener.reset();
@@ -669,11 +681,6 @@ public class NotificationsFragment extends SFragment implements
         List<HttpHeaderLink> links = HttpHeaderLink.parse(linkHeader);
         switch (fetchEnd) {
             case TOP: {
-                HttpHeaderLink previous = HttpHeaderLink.findByRelationType(links, "prev");
-                String uptoId = null;
-                if (previous != null) {
-                    uptoId = previous.uri.getQueryParameter("since_id");
-                }
                 update(notifications, null);
                 break;
             }
@@ -691,20 +698,12 @@ public class NotificationsFragment extends SFragment implements
                 if (!this.notifications.isEmpty()
                         && !this.notifications.get(this.notifications.size() - 1).isRight()) {
                     this.notifications.remove(this.notifications.size() - 1);
-                    adapter.removeItemAndNotify(this.notifications.size());
+                    updateAdapter();
                 }
 
                 if (adapter.getItemCount() > 0) {
                     addItems(notifications, fromId);
                 } else {
-                    /* If this is the first fetch, also save the id from the "previous" link and
-                     * treat this operation as a refresh so the scroll position doesn't get pushed
-                     * down to the end. */
-                    HttpHeaderLink previous = HttpHeaderLink.findByRelationType(links, "prev");
-                    String uptoId = null;
-                    if (previous != null) {
-                        uptoId = previous.uri.getQueryParameter("since_id");
-                    }
                     update(notifications, fromId);
                 }
 
@@ -733,10 +732,11 @@ public class NotificationsFragment extends SFragment implements
     private void onFetchNotificationsFailure(Exception exception, FetchEnd fetchEnd, int position) {
         swipeRefreshLayout.setRefreshing(false);
         if (fetchEnd == FetchEnd.MIDDLE && !notifications.get(position).isRight()) {
+            Placeholder placeholder = notifications.get(position).asLeft();
             NotificationViewData placeholderVD =
-                    new NotificationViewData.Placeholder(false);
+                    new NotificationViewData.Placeholder(placeholder.id, false);
             notifications.setPairedItem(position, placeholderVD);
-            adapter.updateItemWithNotify(position, placeholderVD, true);
+            updateAdapter();
         } else if (this.notifications.isEmpty()) {
             this.statusView.setVisibility(View.VISIBLE);
             swipeRefreshLayout.setEnabled(false);
@@ -798,14 +798,14 @@ public class NotificationsFragment extends SFragment implements
             int newIndex = liftedNew.indexOf(notifications.get(0));
             if (newIndex == -1) {
                 if (index == -1 && liftedNew.size() >= LOAD_AT_ONCE) {
-                    liftedNew.add(new Either.Left(Placeholder.getInstance()));
+                    liftedNew.add(new Either.Left<>(newPlaceholder()));
                 }
                 notifications.addAll(0, liftedNew);
             } else {
                 notifications.addAll(0, liftedNew.subList(0, newIndex));
             }
         }
-        adapter.update(notifications.getPairedCopy());
+        updateAdapter();
     }
 
     private void addItems(List<Notification> newNotifications, @Nullable String fromId) {
@@ -818,10 +818,7 @@ public class NotificationsFragment extends SFragment implements
         Either<Placeholder, Notification> last = notifications.get(end - 1);
         if (last != null && liftedNew.indexOf(last) == -1) {
             notifications.addAll(liftedNew);
-            List<NotificationViewData> newViewDatas = notifications.getPairedCopy()
+            updateAdapter();
-                    .subList(notifications.size() - newNotifications.size(),
-                            notifications.size());
-            adapter.addItems(newViewDatas);
         }
     }
 
@@ -830,7 +827,7 @@ public class NotificationsFragment extends SFragment implements
         notifications.remove(pos);
 
         if (ListUtils.isEmpty(newNotifications)) {
-            adapter.update(notifications.getPairedCopy());
+            updateAdapter();
             return;
         }
 
@@ -840,11 +837,11 @@ public class NotificationsFragment extends SFragment implements
         // If we fetched at least as much it means that there are more posts to load and we should
         // insert new placeholder
         if (newNotifications.size() >= LOAD_AT_ONCE) {
-            liftedNew.add(new Either.Left(Placeholder.getInstance()));
+            liftedNew.add(new Either.Left<>(newPlaceholder()));
         }
 
         notifications.addAll(pos, liftedNew);
-        adapter.update(notifications.getPairedCopy());
+        updateAdapter();
     }
 
     private final Function<Notification, Either<Placeholder, Notification>> notificationLifter =
@@ -855,8 +852,8 @@ public class NotificationsFragment extends SFragment implements
     }
 
     private void fullyRefresh() {
-        adapter.clear();
         notifications.clear();
+        updateAdapter();
         sendFetchNotificationsRequest(null, null, FetchEnd.TOP, -1);
     }
 
@@ -875,4 +872,67 @@ public class NotificationsFragment extends SFragment implements
         }
         return null;
     }
+
+    private void updateAdapter() {
+        differ.submitList(notifications.getPairedCopy());
+    }
+
+    private final ListUpdateCallback listUpdateCallback = new ListUpdateCallback() {
+        @Override
+        public void onInserted(int position, int count) {
+            if (isAdded()) {
+                adapter.notifyItemRangeInserted(position, count);
+                Context context = getContext();
+                if (position == 0 && context != null) {
+                    recyclerView.scrollBy(0, Utils.dpToPx(context, -30));
+                }
+            }
+        }
+
+        @Override
+        public void onRemoved(int position, int count) {
+            adapter.notifyItemRangeRemoved(position, count);
+        }
+
+        @Override
+        public void onMoved(int fromPosition, int toPosition) {
+            adapter.notifyItemMoved(fromPosition, toPosition);
+        }
+
+        @Override
+        public void onChanged(int position, int count, Object payload) {
+            adapter.notifyItemRangeChanged(position, count, payload);
+        }
+    };
+
+    private final AsyncListDiffer<NotificationViewData>
+            differ = new AsyncListDiffer<>(listUpdateCallback,
+            new AsyncDifferConfig.Builder<>(diffCallback).build());
+
+    private final NotificationsAdapter.AdapterDataSource<NotificationViewData> dataSource =
+            new NotificationsAdapter.AdapterDataSource<NotificationViewData>() {
+                @Override
+                public int getItemCount() {
+                    return differ.getCurrentList().size();
+                }
+
+                @Override
+                public NotificationViewData getItemAt(int pos) {
+                    return differ.getCurrentList().get(pos);
+                }
+            };
+
+    private static final DiffUtil.ItemCallback<NotificationViewData> diffCallback
+            = new DiffUtil.ItemCallback<NotificationViewData>() {
+
+        @Override
+        public boolean areItemsTheSame(NotificationViewData oldItem, NotificationViewData newItem) {
+            return oldItem.getViewDataId() == newItem.getViewDataId();
+        }
+
+        @Override
+        public boolean areContentsTheSame(NotificationViewData oldItem, NotificationViewData newItem) {
+            return oldItem.deepEquals(newItem);
+        }
+    };
 }

app/src/main/java/com/keylesspalace/tusky/viewdata/NotificationViewData.java

@@ -18,11 +18,13 @@ package com.keylesspalace.tusky.viewdata;
 import com.keylesspalace.tusky.entity.Account;
 import com.keylesspalace.tusky.entity.Notification;
 
-import io.reactivex.annotations.NonNull;
+import java.util.Objects;
+
+import io.reactivex.annotations.Nullable;
 
 /**
  * Created by charlag on 12/07/2017.
- *
+ * <p>
  * Class to represent data required to display either a notification or a placeholder.
  * It is either a {@link Placeholder} or a {@link Concrete}.
  * It is modelled this way because close relationship between placeholder and concrete notification
@@ -35,16 +37,20 @@ public abstract class NotificationViewData {
     private NotificationViewData() {
     }
 
-    public static  final class Concrete extends NotificationViewData {
+    public abstract long getViewDataId();
+
+    public abstract boolean deepEquals(NotificationViewData other);
+
+    public static final class Concrete extends NotificationViewData {
         private final Notification.Type type;
         private final String id;
         private final Account account;
-        @NonNull
+        @Nullable
         private final StatusViewData.Concrete statusViewData;
         private final boolean isExpanded;
 
         public Concrete(Notification.Type type, String id, Account account,
-                        @NonNull StatusViewData.Concrete statusViewData, boolean isExpanded) {
+                        @Nullable StatusViewData.Concrete statusViewData, boolean isExpanded) {
             this.type = type;
             this.id = id;
             this.account = account;
@@ -64,7 +70,7 @@ public abstract class NotificationViewData {
             return account;
         }
 
-        @NonNull
+        @Nullable
         public StatusViewData.Concrete getStatusViewData() {
             return statusViewData;
         }
@@ -72,17 +78,56 @@ public abstract class NotificationViewData {
         public boolean isExpanded() {
             return isExpanded;
         }
+
+        @Override
+        public long getViewDataId() {
+            return id.hashCode();
+        }
+
+        @Override
+        public boolean deepEquals(NotificationViewData o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            Concrete concrete = (Concrete) o;
+            return isExpanded == concrete.isExpanded &&
+                    type == concrete.type &&
+                    Objects.equals(id, concrete.id) &&
+                    account.getId().equals(concrete.account.getId()) &&
+                    (statusViewData == concrete.statusViewData ||
+                            statusViewData != null &&
+                                    statusViewData.deepEquals(concrete.statusViewData));
+        }
+
+        @Override
+        public int hashCode() {
+
+            return Objects.hash(type, id, account, statusViewData, isExpanded);
+        }
     }
 
     public static final class Placeholder extends NotificationViewData {
+        private final long id;
         private final boolean isLoading;
 
-        public Placeholder(boolean isLoading) {
+        public Placeholder(long id, boolean isLoading) {
+            this.id = id;
             this.isLoading = isLoading;
         }
 
         public boolean isLoading() {
             return isLoading;
         }
+
+        @Override
+        public long getViewDataId() {
+            return id;
+        }
+
+        @Override
+        public boolean deepEquals(NotificationViewData other) {
+            if (!(other instanceof Placeholder)) return false;
+            Placeholder that = (Placeholder) other;
+            return isLoading == that.isLoading && id == that.id;
+        }
     }
 }