From 1e6f7d4ec8196843e261b3a11f52b2609495a3dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Tue, 8 Feb 2022 15:17:40 -0500
Subject: [PATCH] smartlist: prevent memory leak of rx subscription
Change-Id: I8f9ecf12798e58f312e31bca672974f6db2af7db
---
.../cx/ring/fragments/SmartListFragment.kt | 16 ++--
.../main/java/cx/ring/utils/ActionHelper.kt | 8 +-
.../kotlin/net/jami/model/Conversation.kt | 4 +-
.../net/jami/smartlist/SmartListPresenter.kt | 84 ++++++-------------
.../net/jami/smartlist/SmartListView.kt | 4 +-
5 files changed, 42 insertions(+), 74 deletions(-)
diff --git a/ring-android/app/src/main/java/cx/ring/fragments/SmartListFragment.kt b/ring-android/app/src/main/java/cx/ring/fragments/SmartListFragment.kt
index 89652ee21..2771bdc22 100644
--- a/ring-android/app/src/main/java/cx/ring/fragments/SmartListFragment.kt
+++ b/ring-android/app/src/main/java/cx/ring/fragments/SmartListFragment.kt
@@ -231,12 +231,12 @@ class SmartListFragment : BaseSupportFragment<SmartListPresenter, SmartListView>
menu?.findItem(R.id.menu_overflow)?.isVisible = visible
}
- override fun removeConversation(conversationUri: Uri) {
- presenter.removeConversation(conversationUri)
+ override fun removeConversation(accountId: String, conversationUri: Uri) {
+ presenter.removeConversation(accountId, conversationUri)
}
- override fun clearConversation(conversationUri: Uri) {
- presenter.clearConversation(conversationUri)
+ override fun clearConversation(accountId: String, conversationUri: Uri) {
+ presenter.clearConversation(accountId, conversationUri)
}
override fun copyContactNumberToClipboard(contactNumber: String) {
@@ -293,12 +293,12 @@ class SmartListFragment : BaseSupportFragment<SmartListPresenter, SmartListView>
}
}
- override fun displayClearDialog(conversationUri: Uri) {
- ActionHelper.launchClearAction(requireContext(), conversationUri, this@SmartListFragment)
+ override fun displayClearDialog(accountId: String, conversationUri: Uri) {
+ ActionHelper.launchClearAction(requireContext(), accountId, conversationUri, this@SmartListFragment)
}
- override fun displayDeleteDialog(conversationUri: Uri) {
- ActionHelper.launchDeleteAction(requireContext(), conversationUri, this@SmartListFragment)
+ override fun displayDeleteDialog(accountId: String, conversationUri: Uri) {
+ ActionHelper.launchDeleteAction(requireContext(), accountId, conversationUri, this@SmartListFragment)
}
override fun copyNumber(uri: Uri) {
diff --git a/ring-android/app/src/main/java/cx/ring/utils/ActionHelper.kt b/ring-android/app/src/main/java/cx/ring/utils/ActionHelper.kt
index 2cd5b481c..d02659454 100644
--- a/ring-android/app/src/main/java/cx/ring/utils/ActionHelper.kt
+++ b/ring-android/app/src/main/java/cx/ring/utils/ActionHelper.kt
@@ -36,23 +36,23 @@ object ActionHelper {
const val ACTION_DELETE = 2
const val ACTION_BLOCK = 3
- fun launchClearAction(context: Context, uri: Uri, callback: ConversationActionCallback) {
+ fun launchClearAction(context: Context, accountId: String, uri: Uri, callback: ConversationActionCallback) {
MaterialAlertDialogBuilder(context)
.setTitle(R.string.conversation_action_history_clear_title)
.setMessage(R.string.conversation_action_history_clear_message)
.setPositiveButton(android.R.string.ok) { dialog: DialogInterface?, whichButton: Int ->
- callback.clearConversation(uri)
+ callback.clearConversation(accountId, uri)
}
.setNegativeButton(android.R.string.cancel) { dialog: DialogInterface?, whichButton: Int -> }
.show()
}
- fun launchDeleteAction(context: Context, uri: Uri, callback: ConversationActionCallback) {
+ fun launchDeleteAction(context: Context, accountId: String, uri: Uri, callback: ConversationActionCallback) {
MaterialAlertDialogBuilder(context)
.setTitle(R.string.conversation_action_remove_this_title)
.setMessage(R.string.conversation_action_remove_this_message)
.setPositiveButton(android.R.string.ok) { dialog: DialogInterface?, whichButton: Int ->
- callback.removeConversation(uri)
+ callback.removeConversation(accountId, uri)
}
.setNegativeButton(android.R.string.cancel) { dialog: DialogInterface?, whichButton: Int -> }
.show()
diff --git a/ring-android/libjamiclient/src/main/kotlin/net/jami/model/Conversation.kt b/ring-android/libjamiclient/src/main/kotlin/net/jami/model/Conversation.kt
index 1ee162bd1..26e6f663b 100644
--- a/ring-android/libjamiclient/src/main/kotlin/net/jami/model/Conversation.kt
+++ b/ring-android/libjamiclient/src/main/kotlin/net/jami/model/Conversation.kt
@@ -621,8 +621,8 @@ class Conversation : ConversationHistory {
}
interface ConversationActionCallback {
- fun removeConversation(conversationUri: Uri)
- fun clearConversation(conversationUri: Uri)
+ fun removeConversation(accountId: String, conversationUri: Uri)
+ fun clearConversation(accountId: String, conversationUri: Uri)
fun copyContactNumberToClipboard(contactNumber: String)
}
diff --git a/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListPresenter.kt b/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListPresenter.kt
index 8cff553d7..f01490bf7 100644
--- a/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListPresenter.kt
+++ b/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListPresenter.kt
@@ -22,11 +22,8 @@ package net.jami.smartlist
import io.reactivex.rxjava3.core.Observable
import io.reactivex.rxjava3.core.Scheduler
-import io.reactivex.rxjava3.disposables.Disposable
import io.reactivex.rxjava3.schedulers.Schedulers
import io.reactivex.rxjava3.subjects.BehaviorSubject
-import io.reactivex.rxjava3.subjects.PublishSubject
-import io.reactivex.rxjava3.subjects.Subject
import net.jami.model.Account
import net.jami.model.Conversation
import net.jami.model.Uri
@@ -38,42 +35,23 @@ import javax.inject.Inject
import javax.inject.Named
class SmartListPresenter @Inject constructor(
- private val mConversationFacade: ConversationFacade,
- @param:Named("UiScheduler") private val mUiScheduler: Scheduler
+ private val conversationFacade: ConversationFacade,
+ @param:Named("UiScheduler") private val uiScheduler: Scheduler
) : RootPresenter<SmartListView>() {
- private var mQueryDisposable: Disposable? = null
- private val mCurrentQuery: Subject<String> = BehaviorSubject.createDefault("")
- private val mQuery: Subject<String> = PublishSubject.create()
- private val mDebouncedQuery = mQuery.debounce(350, TimeUnit.MILLISECONDS)
- private val accountSubject: Observable<Account> = mConversationFacade
+ private val querySubject = BehaviorSubject.createDefault("")
+ private val debouncedQuery = querySubject.debounce{ item ->
+ if (item.isEmpty()) Observable.empty() else Observable.timer(350, TimeUnit.MILLISECONDS)
+ }.distinctUntilChanged()
+ private val accountSubject: Observable<Account> = conversationFacade
.currentAccountSubject
- .doOnNext { a: Account -> mAccount = a }
- private var mAccount: Account? = null
+ .doOnNext { a: Account -> currentAccountId = a.accountId }
+ private var currentAccountId: String = ""
override fun bindView(view: SmartListView) {
super.bindView(view)
view.setLoading(true)
- /*mCompositeDisposable.add(mConversationFacade.getFullList(accountSubject, mCurrentQuery, true)
- .switchMap { viewModels ->
- if (viewModels.isEmpty()) ConversationItemViewModel.EMPTY_RESULTS
- else Observable.combineLatest(viewModels) { obs -> obs.mapTo(ArrayList(obs.size)) { ob -> ob as ConversationItemViewModel } }
- .throttleLatest(150, TimeUnit.MILLISECONDS, mUiScheduler)
- }
- .observeOn(mUiScheduler)
- .subscribe({ viewModels: MutableList<ConversationItemViewModel> ->
- val v = this.view ?: return@subscribe
- v.setLoading(false)
- if (viewModels.isEmpty()) {
- v.hideList()
- v.displayNoConversationMessage()
- } else {
- v.hideNoConversationMessage()
- v.updateList(viewModels, mCompositeDisposable)
- }
- }) { e: Throwable -> Log.w(TAG, "showConversations error ", e) })*/
-
- mCompositeDisposable.add(mConversationFacade.getFullConversationList(accountSubject, mCurrentQuery)
- .observeOn(mUiScheduler)
+ mCompositeDisposable.add(conversationFacade.getFullConversationList(accountSubject, debouncedQuery)
+ .observeOn(uiScheduler)
.subscribe { list ->
val v = this.view ?: return@subscribe
v.setLoading(false)
@@ -82,24 +60,14 @@ class SmartListPresenter @Inject constructor(
v.displayNoConversationMessage()
} else {
v.hideNoConversationMessage()
- v.updateList(list, mConversationFacade, mCompositeDisposable)
+ v.updateList(list, conversationFacade, mCompositeDisposable)
}
})
}
fun queryTextChanged(query: String) {
- if (query.isEmpty()) {
- mQueryDisposable?.let { disposable ->
- disposable.dispose()
- mQueryDisposable = null
- }
- mCurrentQuery.onNext(query)
- } else {
- if (mQueryDisposable == null) {
- mQueryDisposable = mDebouncedQuery.subscribe { t: String -> mCurrentQuery.onNext(t) }
- }
- mQuery.onNext(query)
- }
+ Log.w(TAG, "queryTextChanged $query")
+ querySubject.onNext(query)
}
fun conversationClicked(viewModel: Conversation) {
@@ -123,7 +91,7 @@ class SmartListPresenter @Inject constructor(
}
fun startConversation(uri: Uri) {
- view?.goToConversation(mAccount!!.accountId, uri)
+ view?.goToConversation(currentAccountId, uri)
}
fun copyNumber(conversationItemViewModel: Conversation) {
@@ -135,27 +103,27 @@ class SmartListPresenter @Inject constructor(
}
}
- fun clearConversation(conversationItemViewModel: Conversation) {
- view?.displayClearDialog(conversationItemViewModel.uri)
+ fun clearConversation(conversation: Conversation) {
+ view?.displayClearDialog(conversation.accountId, conversation.uri)
}
- fun clearConversation(uri: Uri?) {
- mCompositeDisposable.add(mConversationFacade
- .clearHistory(mAccount!!.accountId, uri!!)
+ fun clearConversation(accountId: String, uri: Uri) {
+ mCompositeDisposable.add(conversationFacade
+ .clearHistory(accountId, uri)
.subscribeOn(Schedulers.computation()).subscribe())
}
- fun removeConversation(conversationItemViewModel: Conversation) {
- view?.displayDeleteDialog(conversationItemViewModel.uri)
+ fun removeConversation(conversation: Conversation) {
+ view?.displayDeleteDialog(conversation.accountId, conversation.uri)
}
- fun removeConversation(uri: Uri) {
- mCompositeDisposable.add(mConversationFacade.removeConversation(mAccount!!.accountId, uri)
+ fun removeConversation(accountId: String, uri: Uri) {
+ mCompositeDisposable.add(conversationFacade.removeConversation(accountId, uri)
.subscribe())
}
- fun banContact(conversationItemViewModel: Conversation) {
- mConversationFacade.banConversation(conversationItemViewModel.accountId, conversationItemViewModel.uri)
+ fun banContact(conversation: Conversation) {
+ conversationFacade.banConversation(conversation.accountId, conversation.uri)
}
fun clickQRSearch() {
diff --git a/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListView.kt b/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListView.kt
index b29e8ba6c..2290e6914 100644
--- a/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListView.kt
+++ b/ring-android/libjamiclient/src/main/kotlin/net/jami/smartlist/SmartListView.kt
@@ -28,8 +28,8 @@ interface SmartListView {
fun displayChooseNumberDialog(numbers: Array<CharSequence>)
fun displayNoConversationMessage()
fun displayConversationDialog(conversationItemViewModel: Conversation)
- fun displayClearDialog(conversationUri: Uri)
- fun displayDeleteDialog(conversationUri: Uri)
+ fun displayClearDialog(accountId: String, conversationUri: Uri)
+ fun displayDeleteDialog(accountId: String, conversationUri: Uri)
fun copyNumber(uri: Uri)
fun setLoading(loading: Boolean)
fun displayMenuItem()
--
GitLab