Clean up conversation list data loading sequence.
- The Paging library was giving us empty paged lists when loading was invalidated, but only *sometimes*. This library, man. Fixed it by ignoring invalid lists, which you'd think the library would do for us... - Noticed we were doing a ton of list refreshes because of how we were listening to archive count. Switched from combine to switchMap. - Noticed that we could become double-subscribed to LiveDatas in the ConversationListFragment if you went to archived. Fixed by observing on the fragment's view lifecycle. Fixes #9803
This commit is contained in:
Greyson Parrelli
parent
9c63b37bb4
commit
e504ffa225
4 changed files with 34 additions and 28 deletions
|
|
@ -14,7 +14,6 @@ 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 org.thoughtcrime.securesms.util.concurrent.SignalExecutors;
|
||||
import org.thoughtcrime.securesms.util.paging.Invalidator;
|
||||
import org.thoughtcrime.securesms.util.paging.SizeFixResult;
|
||||
|
|
@ -83,9 +82,10 @@ class ConversationDataSource extends PositionalDataSource<ConversationMessage> {
|
|||
.toList();
|
||||
|
||||
callback.onResult(items, params.requestedStartPosition, result.getTotal());
|
||||
Log.d(TAG, "[Initial Load] " + (System.currentTimeMillis() - start) + " ms | thread: " + threadId + ", start: " + params.requestedStartPosition + ", requestedSize: " + params.requestedLoadSize + ", actualSize: " + result.getItems().size() + ", totalCount: " + result.getTotal());
|
||||
} else {
|
||||
Log.d(TAG, "[Initial Load] " + (System.currentTimeMillis() - start) + " ms | thread: " + threadId + ", start: " + params.requestedStartPosition + ", requestedSize: " + params.requestedLoadSize + ", totalCount: " + totalCount + " -- invalidated");
|
||||
}
|
||||
|
||||
Log.d(TAG, "[Initial Load] " + (System.currentTimeMillis() - start) + " ms | thread: " + threadId + ", start: " + params.requestedStartPosition + ", size: " + params.requestedLoadSize + (isInvalid() ? " -- invalidated" : ""));
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
|
|||
import org.thoughtcrime.securesms.logging.Log;
|
||||
import org.thoughtcrime.securesms.recipients.Recipient;
|
||||
import org.thoughtcrime.securesms.util.ThrottledDebouncer;
|
||||
import org.thoughtcrime.securesms.util.Util;
|
||||
import org.thoughtcrime.securesms.util.concurrent.SignalExecutors;
|
||||
import org.thoughtcrime.securesms.util.paging.Invalidator;
|
||||
import org.thoughtcrime.securesms.util.paging.SizeFixResult;
|
||||
|
|
@ -84,11 +85,11 @@ abstract class ConversationListDataSource extends PositionalDataSource<Conversat
|
|||
|
||||
if (!isInvalid()) {
|
||||
SizeFixResult<Conversation> result = SizeFixResult.ensureMultipleOfPageSize(conversations, params.requestedStartPosition, params.pageSize, totalCount);
|
||||
|
||||
callback.onResult(result.getItems(), params.requestedStartPosition, result.getTotal());
|
||||
Log.d(TAG, "[Initial Load] " + (System.currentTimeMillis() - start) + " ms | start: " + params.requestedStartPosition + ", requestedSize: " + params.requestedLoadSize + ", actualSize: " + result.getItems().size() + ", totalCount: " + result.getTotal() + ", class: " + getClass().getSimpleName());
|
||||
} else {
|
||||
Log.d(TAG, "[Initial Load] " + (System.currentTimeMillis() - start) + " ms | start: " + params.requestedStartPosition + ", requestedSize: " + params.requestedLoadSize + ", totalCount: " + totalCount + ", class: " + getClass().getSimpleName() + " -- invalidated");
|
||||
}
|
||||
|
||||
Log.d(TAG, "[Initial Load] " + (System.currentTimeMillis() - start) + " ms | start: " + params.requestedStartPosition + ", size: " + params.requestedLoadSize + ", totalCount: " + totalCount + ", class: " + getClass().getSimpleName() + (isInvalid() ? " -- invalidated" : ""));
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
|||
|
|
@ -500,9 +500,9 @@ public class ConversationListFragment extends MainFragment implements ActionMode
|
|||
private void initializeViewModel() {
|
||||
viewModel = ViewModelProviders.of(this, new ConversationListViewModel.Factory(isArchived())).get(ConversationListViewModel.class);
|
||||
|
||||
viewModel.getSearchResult().observe(this, this::onSearchResultChanged);
|
||||
viewModel.getMegaphone().observe(this, this::onMegaphoneChanged);
|
||||
viewModel.getConversationList().observe(this, this::onSubmitList);
|
||||
viewModel.getSearchResult().observe(getViewLifecycleOwner(), this::onSearchResultChanged);
|
||||
viewModel.getMegaphone().observe(getViewLifecycleOwner(), this::onMegaphoneChanged);
|
||||
viewModel.getConversationList().observe(getViewLifecycleOwner(), this::onSubmitList);
|
||||
|
||||
ProcessLifecycleOwner.get().getLifecycle().addObserver(new DefaultLifecycleObserver() {
|
||||
@Override
|
||||
|
|
@ -751,6 +751,7 @@ public class ConversationListFragment extends MainFragment implements ActionMode
|
|||
|
||||
private void onSubmitList(@NonNull ConversationListViewModel.ConversationList conversationList) {
|
||||
if (conversationList.isEmpty()) {
|
||||
Log.i(TAG, "Received an empty data set.");
|
||||
list.setVisibility(View.INVISIBLE);
|
||||
emptyState.setVisibility(View.VISIBLE);
|
||||
emptyImage.setImageResource(EMPTY_IMAGES[(int) (Math.random() * EMPTY_IMAGES.length)]);
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import android.text.TextUtils;
|
|||
import androidx.annotation.NonNull;
|
||||
import androidx.lifecycle.LiveData;
|
||||
import androidx.lifecycle.MutableLiveData;
|
||||
import androidx.lifecycle.Transformations;
|
||||
import androidx.lifecycle.ViewModel;
|
||||
import androidx.lifecycle.ViewModelProvider;
|
||||
import androidx.paging.DataSource;
|
||||
|
|
@ -19,6 +20,7 @@ import org.thoughtcrime.securesms.conversationlist.model.SearchResult;
|
|||
import org.thoughtcrime.securesms.database.DatabaseContentProviders;
|
||||
import org.thoughtcrime.securesms.database.DatabaseFactory;
|
||||
import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
|
||||
import org.thoughtcrime.securesms.logging.Log;
|
||||
import org.thoughtcrime.securesms.megaphone.Megaphone;
|
||||
import org.thoughtcrime.securesms.megaphone.MegaphoneRepository;
|
||||
import org.thoughtcrime.securesms.megaphone.Megaphones;
|
||||
|
|
@ -31,10 +33,11 @@ import org.thoughtcrime.securesms.util.paging.Invalidator;
|
|||
|
||||
class ConversationListViewModel extends ViewModel {
|
||||
|
||||
private static final String TAG = Log.tag(ConversationListViewModel.class);
|
||||
|
||||
private final Application application;
|
||||
private final MutableLiveData<Megaphone> megaphone;
|
||||
private final MutableLiveData<SearchResult> searchResult;
|
||||
private final MutableLiveData<Integer> archivedCount;
|
||||
private final LiveData<ConversationList> conversationList;
|
||||
private final SearchRepository searchRepository;
|
||||
private final MegaphoneRepository megaphoneRepository;
|
||||
|
|
@ -48,7 +51,6 @@ class ConversationListViewModel extends ViewModel {
|
|||
this.application = application;
|
||||
this.megaphone = new MutableLiveData<>();
|
||||
this.searchResult = new MutableLiveData<>();
|
||||
this.archivedCount = new MutableLiveData<>();
|
||||
this.searchRepository = searchRepository;
|
||||
this.megaphoneRepository = ApplicationDependencies.getMegaphoneRepository();
|
||||
this.debouncer = new Debouncer(300);
|
||||
|
|
@ -59,10 +61,6 @@ class ConversationListViewModel extends ViewModel {
|
|||
if (!TextUtils.isEmpty(getLastQuery())) {
|
||||
searchRepository.query(getLastQuery(), searchResult::postValue);
|
||||
}
|
||||
|
||||
if (!isArchived) {
|
||||
updateArchivedCount();
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -77,15 +75,27 @@ class ConversationListViewModel extends ViewModel {
|
|||
.setInitialLoadKey(0)
|
||||
.build();
|
||||
|
||||
if (isArchived) {
|
||||
this.archivedCount.setValue(0);
|
||||
} else {
|
||||
updateArchivedCount();
|
||||
}
|
||||
|
||||
application.getContentResolver().registerContentObserver(DatabaseContentProviders.ConversationList.CONTENT_URI, true, observer);
|
||||
|
||||
this.conversationList = LiveDataUtil.combineLatest(conversationList, this.archivedCount, ConversationList::new);
|
||||
this.conversationList = Transformations.switchMap(conversationList, conversation -> {
|
||||
if (conversation.getDataSource().isInvalid()) {
|
||||
Log.w(TAG, "Received an invalid conversation list. Ignoring.");
|
||||
return new MutableLiveData<>();
|
||||
}
|
||||
|
||||
MutableLiveData<ConversationList> updated = new MutableLiveData<>();
|
||||
|
||||
if (isArchived) {
|
||||
updated.postValue(new ConversationList(conversation, 0));
|
||||
} else {
|
||||
SignalExecutors.BOUNDED.execute(() -> {
|
||||
int archiveCount = DatabaseFactory.getThreadDatabase(application).getArchivedConversationListCount();
|
||||
updated.postValue(new ConversationList(conversation, archiveCount));
|
||||
});
|
||||
}
|
||||
|
||||
return updated;
|
||||
});
|
||||
}
|
||||
|
||||
@NonNull LiveData<SearchResult> getSearchResult() {
|
||||
|
|
@ -140,12 +150,6 @@ class ConversationListViewModel extends ViewModel {
|
|||
application.getContentResolver().unregisterContentObserver(observer);
|
||||
}
|
||||
|
||||
private void updateArchivedCount() {
|
||||
SignalExecutors.BOUNDED.execute(() -> {
|
||||
archivedCount.postValue(DatabaseFactory.getThreadDatabase(application).getArchivedConversationListCount());
|
||||
});
|
||||
}
|
||||
|
||||
public static class Factory extends ViewModelProvider.NewInstanceFactory {
|
||||
|
||||
private final boolean isArchived;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue