Skip to content

Commit 049d676

Browse files
google-labs-jules[bot]google-labs-jules[bot]
committed
feat: Refactor NewsLabelManager to prevent ANR
Refactored `NewsLabelManager` to use a `NewsRepository` and coroutines to move database operations off the main thread, fixing an ANR. - Introduced `addLabel` and `removeLabel` suspend functions in `NewsRepository`. - `NewsLabelManager` now uses a `CoroutineScope` to call these repository methods. - Disabled the "add label" button during the async operation to prevent rapid double-clicks. - Fixed a resource leak in `AdapterNews` by properly managing the Realm instance lifecycle. - Fixed a regression in `AdapterNews` that was causing the wrong user to be displayed.
1 parent 83a17ba commit 049d676

File tree

5 files changed

+113
-95
lines changed

5 files changed

+113
-95
lines changed

‎app/src/main/java/org/ole/planet/myplanet/repository/NewsRepository.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,6 @@ interface NewsRepository {
88
suspend fun getNewsWithReplies(newsId: String): Pair<RealmNews?, List<RealmNews>>
99
suspend fun getCommunityVisibleNews(userIdentifier: String): List<RealmNews>
1010
suspend fun createNews(map: HashMap<String?, String>, user: RealmUserModel?): RealmNews
11+
suspend fun addLabel(newsId: String, label: String): Boolean
12+
suspend fun removeLabel(newsId: String, label: String): Boolean
1113
}

‎app/src/main/java/org/ole/planet/myplanet/repository/NewsRepositoryImpl.kt‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,35 @@ class NewsRepositoryImpl @Inject constructor(
7272
false
7373
}
7474
}
75+
76+
override suspend fun addLabel(newsId: String, label: String): Boolean {
77+
return withRealmAsync { realm ->
78+
val managedNews = realm.where(RealmNews::class.java)
79+
.equalTo("id", newsId)
80+
.findFirst()
81+
82+
if (managedNews != null) {
83+
if (managedNews.labels?.contains(label) != true) {
84+
managedNews.labels?.add(label)
85+
return@withRealmAsync true
86+
}
87+
}
88+
false
89+
}
90+
}
91+
92+
override suspend fun removeLabel(newsId: String, label: String): Boolean {
93+
return withRealmAsync { realm ->
94+
val managedNews = realm.where(RealmNews::class.java)
95+
.equalTo("id", newsId)
96+
.findFirst()
97+
98+
if (managedNews != null) {
99+
if (managedNews.labels?.remove(label) == true) {
100+
return@withRealmAsync true
101+
}
102+
}
103+
false
104+
}
105+
}
75106
}

‎app/src/main/java/org/ole/planet/myplanet/ui/news/AdapterNews.kt‎

Lines changed: 40 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,22 @@ import com.google.gson.Gson
2626
import com.google.gson.JsonArray
2727
import com.google.gson.JsonObject
2828
import io.realm.Case
29-
import io.realm.Realm
3029
import io.realm.RealmList
3130
import io.realm.Sort
3231
import java.io.File
3332
import java.util.Calendar
3433
import java.util.Locale
34+
import androidx.lifecycle.findViewTreeLifecycleOwner
35+
import io.realm.Realm
36+
import kotlinx.coroutines.CoroutineScope
3537
import org.ole.planet.myplanet.R
3638
import org.ole.planet.myplanet.databinding.RowNewsBinding
3739
import org.ole.planet.myplanet.model.Conversation
3840
import org.ole.planet.myplanet.model.RealmMyLibrary
3941
import org.ole.planet.myplanet.model.RealmMyTeam
4042
import org.ole.planet.myplanet.model.RealmNews
4143
import org.ole.planet.myplanet.model.RealmUserModel
44+
import org.ole.planet.myplanet.repository.NewsRepository
4245
import org.ole.planet.myplanet.service.UserProfileDbHandler
4346
import org.ole.planet.myplanet.ui.chat.ChatAdapter
4447
import org.ole.planet.myplanet.utilities.Constants.PREFS_NAME
@@ -51,8 +54,15 @@ import org.ole.planet.myplanet.utilities.SharedPrefManager
5154
import org.ole.planet.myplanet.utilities.TimeUtils.formatDate
5255
import org.ole.planet.myplanet.utilities.Utilities
5356
import org.ole.planet.myplanet.utilities.makeExpandable
54-
55-
class AdapterNews(var context: Context, private var currentUser: RealmUserModel?, private val parentNews: RealmNews?, private val teamName: String = "", private val teamId: String? = null, private val userProfileDbHandler: UserProfileDbHandler) : ListAdapter<RealmNews?, RecyclerView.ViewHolder?>(
57+
class AdapterNews(
58+
var context: Context,
59+
private var currentUser: RealmUserModel?,
60+
private val parentNews: RealmNews?,
61+
private val newsRepository: NewsRepository,
62+
private val teamName: String = "",
63+
private val teamId: String? = null,
64+
private val userProfileDbHandler: UserProfileDbHandler
65+
) : ListAdapter<RealmNews?, RecyclerView.ViewHolder?>(
5666
DiffUtils.itemCallback(
5767
areItemsTheSame = { oldItem, newItem ->
5868
if (oldItem === newItem) return@itemCallback true
@@ -74,12 +84,12 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
7484
if (!oldItem.isValid || !newItem.isValid) return@itemCallback false
7585

7686
oldItem.id == newItem.id &&
77-
oldItem.time == newItem.time &&
78-
oldItem.isEdited == newItem.isEdited &&
79-
oldItem.message == newItem.message &&
80-
oldItem.userName == newItem.userName &&
81-
oldItem.userId == newItem.userId &&
82-
oldItem.sharedBy == newItem.sharedBy
87+
oldItem.time == newItem.time &&
88+
oldItem.isEdited == newItem.isEdited &&
89+
oldItem.message == newItem.message &&
90+
oldItem.userName == newItem.userName &&
91+
oldItem.userId == newItem.userId &&
92+
oldItem.sharedBy == newItem.sharedBy
8393
} catch (e: Exception) {
8494
false
8595
}
@@ -88,7 +98,7 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
8898
) {
8999
private var listener: OnNewsItemClickListener? = null
90100
private var imageList: RealmList<String>? = null
91-
lateinit var mRealm: Realm
101+
private lateinit var mRealm: Realm
92102
private var fromLogin = false
93103
private var nonTeamMember = false
94104
private var sharedPreferences: SharedPrefManager? = null
@@ -97,7 +107,7 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
97107
private var labelManager: NewsLabelManager? = null
98108
private val gson = Gson()
99109
private val profileDbHandler = userProfileDbHandler
100-
lateinit var settings: SharedPreferences
110+
private lateinit var settings: SharedPreferences
101111
private val userCache = mutableMapOf<String, RealmUserModel?>()
102112
private val leadersList: List<RealmUserModel> by lazy {
103113
val raw = settings.getString("communityLeaders", "") ?: ""
@@ -134,21 +144,11 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
134144
this.listener = listener
135145
}
136146

137-
fun setmRealm(mRealm: Realm?) {
138-
if (mRealm != null) {
139-
this.mRealm = mRealm
140-
labelManager = NewsLabelManager(context, this.mRealm)
141-
}
142-
}
143-
144147
override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder {
145148
val binding = RowNewsBinding.inflate(LayoutInflater.from(parent.context), parent, false)
146149
sharedPreferences = SharedPrefManager(context)
147150
user = userProfileDbHandler.userModel
148151
settings = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE)
149-
if (::mRealm.isInitialized) {
150-
if (labelManager == null) labelManager = NewsLabelManager(context, mRealm)
151-
}
152152
return ViewHolderNews(binding)
153153
}
154154

@@ -177,8 +177,12 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
177177
loadImage(viewHolder.binding, news)
178178
showReplyButton(viewHolder, news, position)
179179
val canManageLabels = canAddLabel(news)
180-
labelManager?.setupAddLabelMenu(viewHolder.binding, news, canManageLabels)
181-
news.let { labelManager?.showChips(viewHolder.binding, it, canManageLabels) }
180+
holder.itemView.findViewTreeLifecycleOwner()?.let { lifecycleOwner ->
181+
val coroutineScope = lifecycleOwner.lifecycleScope
182+
labelManager = NewsLabelManager(context, newsRepository, coroutineScope)
183+
labelManager?.setupAddLabelMenu(viewHolder.binding, news, canManageLabels)
184+
news.let { labelManager?.showChips(viewHolder.binding, it, canManageLabels) }
185+
}
182186

183187
handleChat(viewHolder, news)
184188

@@ -243,7 +247,7 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
243247
val userModel = when {
244248
userId.isNullOrEmpty() -> null
245249
userCache.containsKey(userId) -> userCache[userId]
246-
::mRealm.isInitialized -> {
250+
else -> {
247251
val managedUser = mRealm.where(RealmUserModel::class.java)
248252
.equalTo("id", userId)
249253
.findFirst()
@@ -261,7 +265,6 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
261265
}
262266
detachedUser ?: managedUser
263267
}
264-
else -> null
265268
}
266269
val userFullName = userModel?.getFullNameWithMiddleName()?.trim()
267270
if (userModel != null && currentUser != null) {
@@ -399,7 +402,7 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
399402
private fun submitListSafely(list: List<RealmNews?>, commitCallback: Runnable? = null) {
400403
userCache.clear()
401404
val detachedList = list.map { news ->
402-
if (news?.isValid == true && ::mRealm.isInitialized) {
405+
if (news?.isValid == true) {
403406
try {
404407
mRealm.copyFromRealm(news)
405408
} catch (e: Exception) {
@@ -791,4 +794,15 @@ class AdapterNews(var context: Context, private var currentUser: RealmUserModel?
791794
}
792795
}
793796

797+
override fun onAttachedToRecyclerView(recyclerView: RecyclerView) {
798+
super.onAttachedToRecyclerView(recyclerView)
799+
this.recyclerView = recyclerView
800+
mRealm = Realm.getDefaultInstance()
801+
}
802+
803+
override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) {
804+
super.onDetachedFromRecyclerView(recyclerView)
805+
this.recyclerView = null
806+
mRealm.close()
807+
}
794808
}

‎app/src/main/java/org/ole/planet/myplanet/ui/news/NewsFragment.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,8 @@ class NewsFragment : BaseNewsFragment() {
244244
val sortedList = updatedListAsMutable.sortedWith(compareByDescending { news ->
245245
getSortDate(news)
246246
})
247-
adapterNews = AdapterNews(requireActivity(), user, null, "", null, userProfileDbHandler)
247+
adapterNews = AdapterNews(requireActivity(), user, null, newsRepository, "", null, userProfileDbHandler)
248248

249-
adapterNews?.setmRealm(mRealm)
250249
adapterNews?.setFromLogin(requireArguments().getBoolean("fromLogin"))
251250
adapterNews?.setListener(this)
252251
adapterNews?.registerAdapterDataObserver(observer)

‎app/src/main/java/org/ole/planet/myplanet/ui/news/NewsLabelManager.kt‎

Lines changed: 39 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,23 @@ import android.content.Context
44
import android.view.MenuItem
55
import android.view.View
66
import fisk.chipcloud.ChipCloud
7-
import io.realm.Realm
8-
import io.realm.RealmList
7+
import kotlinx.coroutines.CoroutineScope
8+
import kotlinx.coroutines.Dispatchers
9+
import kotlinx.coroutines.launch
10+
import kotlinx.coroutines.withContext
911
import java.util.Locale
10-
import java.util.concurrent.atomic.AtomicBoolean
1112
import org.ole.planet.myplanet.R
1213
import org.ole.planet.myplanet.databinding.RowNewsBinding
1314
import org.ole.planet.myplanet.model.RealmNews
15+
import org.ole.planet.myplanet.repository.NewsRepository
1416
import org.ole.planet.myplanet.utilities.Constants
1517
import org.ole.planet.myplanet.utilities.Utilities
1618

17-
class NewsLabelManager(private val context: Context, private val realm: Realm) {
19+
class NewsLabelManager(
20+
private val context: Context,
21+
private val newsRepository: NewsRepository,
22+
private val coroutineScope: CoroutineScope
23+
) {
1824
fun setupAddLabelMenu(binding: RowNewsBinding, news: RealmNews?, canManageLabels: Boolean) {
1925
binding.btnAddLabel.setOnClickListener(null)
2026
binding.btnAddLabel.isEnabled = canManageLabels
@@ -35,45 +41,26 @@ class NewsLabelManager(private val context: Context, private val realm: Realm) {
3541
val selectedLabel = Constants.LABELS[menuItem.title]
3642
val newsId = news?.id
3743
if (selectedLabel != null && newsId != null) {
38-
if (news?.labels?.contains(selectedLabel) == true) {
39-
return@setOnMenuItemClickListener true
40-
}
41-
42-
val labelAdded = AtomicBoolean(false)
43-
realm.executeTransactionAsync({ transactionRealm ->
44-
val managedNews = transactionRealm.where(RealmNews::class.java)
45-
.equalTo("id", newsId)
46-
.findFirst()
47-
if (managedNews != null) {
48-
var managedLabels = managedNews.labels
49-
if (managedLabels == null) {
50-
managedLabels = RealmList()
51-
managedNews.labels = managedLabels
52-
}
53-
if (!managedLabels.contains(selectedLabel)) {
54-
managedLabels.add(selectedLabel)
55-
labelAdded.set(true)
44+
binding.btnAddLabel.isEnabled = false
45+
coroutineScope.launch {
46+
try {
47+
val success = newsRepository.addLabel(newsId, selectedLabel)
48+
if (success) {
49+
withContext(Dispatchers.Main) {
50+
news.labels?.add(selectedLabel)
51+
Utilities.toast(context, context.getString(R.string.label_added))
52+
showChips(binding, news, canManageLabels)
53+
}
5654
}
57-
}
58-
}, {
59-
if (labelAdded.get()) {
60-
val managedNews = realm.where(RealmNews::class.java)
61-
.equalTo("id", newsId)
62-
.findFirst()
63-
val managedLabels = managedNews?.labels
64-
val newLabels = RealmList<String>().apply {
65-
managedLabels?.forEach { add(it) }
55+
} finally {
56+
withContext(Dispatchers.Main) {
57+
binding.btnAddLabel.isEnabled = true
6658
}
67-
news?.labels = newLabels
68-
Utilities.toast(context, context.getString(R.string.label_added))
69-
news?.let { showChips(binding, it, canManageLabels) }
7059
}
71-
}, { error ->
72-
error.printStackTrace()
73-
})
74-
return@setOnMenuItemClickListener false
60+
}
61+
return@setOnMenuItemClickListener true
7562
}
76-
true
63+
false
7764
}
7865
menu.show()
7966
}
@@ -99,36 +86,22 @@ class NewsLabelManager(private val context: Context, private val realm: Realm) {
9986
}
10087
val newsId = news.id
10188
if (selectedLabel != null && newsId != null) {
102-
val labelRemoved = AtomicBoolean(false)
103-
realm.executeTransactionAsync({ transactionRealm ->
104-
val managedNews = transactionRealm.where(RealmNews::class.java)
105-
.equalTo("id", newsId)
106-
.findFirst()
107-
if (managedNews != null) {
108-
var managedLabels = managedNews.labels
109-
if (managedLabels == null) {
110-
managedLabels = RealmList()
111-
managedNews.labels = managedLabels
112-
}
113-
if (managedLabels.remove(selectedLabel)) {
114-
labelRemoved.set(true)
89+
chipCloud.isEnabled = false
90+
coroutineScope.launch {
91+
try {
92+
val success = newsRepository.removeLabel(newsId, selectedLabel)
93+
if (success) {
94+
withContext(Dispatchers.Main) {
95+
news.labels?.remove(selectedLabel)
96+
showChips(binding, news, canManageLabels)
97+
}
11598
}
116-
}
117-
}, {
118-
if (labelRemoved.get()) {
119-
val managedNews = realm.where(RealmNews::class.java)
120-
.equalTo("id", newsId)
121-
.findFirst()
122-
val managedLabels = managedNews?.labels
123-
val newLabels = RealmList<String>().apply {
124-
managedLabels?.forEach { add(it) }
99+
} finally {
100+
withContext(Dispatchers.Main) {
101+
chipCloud.isEnabled = true
125102
}
126-
news.labels = newLabels
127-
showChips(binding, news, canManageLabels)
128103
}
129-
}, { error ->
130-
error.printStackTrace()
131-
})
104+
}
132105
}
133106
}
134107
}
@@ -178,4 +151,3 @@ class NewsLabelManager(private val context: Context, private val realm: Realm) {
178151
}
179152
}
180153
}
181-

0 commit comments

Comments
 (0)