From ed0825112d304706190b1c9b22503edb9aa220ae Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 12 May 2020 10:57:40 -0400 Subject: [PATCH] Fix some ordering problems with conversation data loading. --- .../conversation/ConversationDataSource.java | 31 +++++++++---- .../conversation/ConversationFragment.java | 15 ++++++- .../conversation/ConversationViewModel.java | 44 ++++++++++++++----- 3 files changed, 69 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationDataSource.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationDataSource.java index f1be41453f..4289dd0b8f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationDataSource.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationDataSource.java @@ -12,6 +12,7 @@ import org.thoughtcrime.securesms.database.DatabaseFactory; import org.thoughtcrime.securesms.database.MmsSmsDatabase; import org.thoughtcrime.securesms.database.model.MessageRecord; import org.thoughtcrime.securesms.logging.Log; +import org.thoughtcrime.securesms.util.Util; import java.util.ArrayList; import java.util.List; @@ -23,12 +24,14 @@ class ConversationDataSource extends PositionalDataSource { private static final String TAG = Log.tag(ConversationDataSource.class); - private final Context context; - private final long threadId; + private final Context context; + private final long threadId; + private final DataUpdatedCallback dataUpdateCallback; - private ConversationDataSource(@NonNull Context context, long threadId) { - this.context = context; - this.threadId = threadId; + private ConversationDataSource(@NonNull Context context, long threadId, @NonNull DataUpdatedCallback dataUpdateCallback) { + this.context = context; + this.threadId = threadId; + this.dataUpdateCallback = dataUpdateCallback; ContentObserver contentObserver = new ContentObserver(null) { @Override @@ -56,6 +59,8 @@ class ConversationDataSource extends PositionalDataSource { } callback.onResult(records, params.requestedStartPosition, db.getConversationCount(threadId)); + Util.runOnMain(dataUpdateCallback::onDataUpdated); + Log.d(TAG, "[Initial Load] " + (System.currentTimeMillis() - start) + " ms" + (isInvalid() ? " -- invalidated" : "")); } @@ -74,22 +79,30 @@ class ConversationDataSource extends PositionalDataSource { } callback.onResult(records); + Util.runOnMain(dataUpdateCallback::onDataUpdated); + Log.d(TAG, "[Update] " + (System.currentTimeMillis() - start) + " ms" + (isInvalid() ? " -- invalidated" : "")); } + interface DataUpdatedCallback { + void onDataUpdated(); + } + static class Factory extends DataSource.Factory { - private final Context context; - private final long threadId; + private final Context context; + private final long threadId; + private final DataUpdatedCallback callback; - Factory(Context context, long threadId) { + Factory(Context context, long threadId, @NonNull DataUpdatedCallback callback) { this.context = context; this.threadId = threadId; + this.callback = callback; } @Override public @NonNull DataSource create() { - return new ConversationDataSource(context, threadId); + return new ConversationDataSource(context, threadId, callback); } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java index 265c1614ae..f5f599e3f2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java @@ -864,6 +864,8 @@ public class ConversationFragment extends Fragment { } private void presentConversationMetadata(@NonNull ConversationData conversation) { + Log.d(TAG, "presentConversationMetadata()"); + ConversationAdapter adapter = getListAdapter(); if (adapter == null) { return; @@ -946,9 +948,18 @@ public class ConversationFragment extends Fragment { private void moveToMessagePosition(int position, @Nullable Runnable onMessageNotFound) { if (position >= 0) { list.scrollToPosition(position); - getListAdapter().pulseHighlightItem(position); + + if (getListAdapter() == null || getListAdapter().getItem(position) == null) { + Log.i(TAG, "[moveToMessagePosition] Position " + position + " not currently populated. Scheduling a jump."); + conversationViewModel.scheduleForNextMessageUpdate(() -> { + list.scrollToPosition(position); + getListAdapter().pulseHighlightItem(position); + }); + } else { + getListAdapter().pulseHighlightItem(position); + } } else { - Log.w(TAG, "Tried to navigate to message, but it wasn't found."); + Log.w(TAG, "[moveToMessagePosition] Tried to navigate to message, but it wasn't found."); if (onMessageNotFound != null) { onMessageNotFound.run(); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java index afb9d86b7d..c6037f1601 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java @@ -3,8 +3,11 @@ package org.thoughtcrime.securesms.conversation; import android.app.Application; import androidx.annotation.NonNull; +import androidx.arch.core.util.Function; import androidx.lifecycle.LiveData; +import androidx.lifecycle.MediatorLiveData; import androidx.lifecycle.MutableLiveData; +import androidx.lifecycle.Observer; import androidx.lifecycle.Transformations; import androidx.lifecycle.ViewModel; import androidx.lifecycle.ViewModelProvider; @@ -17,9 +20,13 @@ import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.logging.Log; import org.thoughtcrime.securesms.mediasend.Media; import org.thoughtcrime.securesms.mediasend.MediaRepository; +import org.thoughtcrime.securesms.util.Util; import org.thoughtcrime.securesms.util.concurrent.SignalExecutors; +import org.whispersystems.libsignal.util.Pair; +import java.util.LinkedList; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; class ConversationViewModel extends ViewModel { @@ -32,8 +39,9 @@ class ConversationViewModel extends ViewModel { private final MutableLiveData threadId; private final LiveData> messages; private final LiveData conversationMetadata; + private final List onNextMessageLoad; - private int jumpToPosition; + private int jumpToPosition; private ConversationViewModel() { this.context = ApplicationDependencies.getApplication(); @@ -41,21 +49,27 @@ class ConversationViewModel extends ViewModel { this.conversationRepository = new ConversationRepository(); this.recentMedia = new MutableLiveData<>(); this.threadId = new MutableLiveData<>(); + this.onNextMessageLoad = new CopyOnWriteArrayList<>(); - messages = Transformations.switchMap(threadId, thread -> { - DataSource.Factory factory = new ConversationDataSource.Factory(context, thread); + LiveData>> messagesForThreadId = Transformations.switchMap(threadId, thread -> { + DataSource.Factory factory = new ConversationDataSource.Factory(context, thread, this::onMessagesUpdated); PagedList.Config config = new PagedList.Config.Builder() .setPageSize(25) .setInitialLoadSizeHint(25) .build(); - return new LivePagedListBuilder<>(factory, config).setFetchExecutor(SignalExecutors.BOUNDED) - .setInitialLoadKey(Math.max(jumpToPosition, 0)) - .build(); + return Transformations.map(new LivePagedListBuilder<>(factory, config).setFetchExecutor(SignalExecutors.BOUNDED) + .setInitialLoadKey(Math.max(jumpToPosition, 0)) + .build(), + input -> new Pair<>(thread, input)); }); - conversationMetadata = Transformations.switchMap(threadId, thread -> { - LiveData data = conversationRepository.getConversationData(thread, jumpToPosition); + this.messages = Transformations.map(messagesForThreadId, Pair::second); + + LiveData threadIdForLoadedMessages = Transformations.distinctUntilChanged(Transformations.map(messagesForThreadId, Pair::first)); + + conversationMetadata = Transformations.switchMap(threadIdForLoadedMessages, m -> { + LiveData data = conversationRepository.getConversationData(m, jumpToPosition); jumpToPosition = -1; return data; }); @@ -92,6 +106,18 @@ class ConversationViewModel extends ViewModel { return conversationMetadata.getValue() != null ? conversationMetadata.getValue().getLastSeenPosition() : 0; } + void scheduleForNextMessageUpdate(@NonNull Runnable runnable) { + onNextMessageLoad.add(runnable); + } + + private void onMessagesUpdated() { + for (Runnable runnable : onNextMessageLoad) { + runnable.run(); + } + + onNextMessageLoad.clear(); + } + static class Factory extends ViewModelProvider.NewInstanceFactory { @Override public @NonNull T create(@NonNull Class modelClass) { @@ -99,6 +125,4 @@ class ConversationViewModel extends ViewModel { return modelClass.cast(new ConversationViewModel()); } } - - }