Skip to content

Commit ecfd924

Browse files
coolteyWilliamraidbrant
authored
Remove allowMainThreadQueries and update DAO functions (#5730)
* Initial commit of adding suspend modifier to DAO functions * More updates * fix errors * More updates regarding syntax fixes * tune up * Seen vs unseen * update function for talk page holder * Remove Transaction * Add transaction back * Revert "Add transaction back" This reverts commit 586d455. * Update the logic of posting the flowBus * Adding suspend to Notification functions * Fix test * remove duplicated function * Simplify * Update transation for moving lists * Keep same reset and logout and add coroutine for notificationDao only * Follow-up to updating DAO: improve efficiency. (#5854) * Use MainScope instead of AppCompatActivity --------- Co-authored-by: William Rai <[email protected]> Co-authored-by: Dmitry Brant <[email protected]>
1 parent 4906f26 commit ecfd924

28 files changed

+554
-378
lines changed

‎app/src/androidTest/java/org/wikipedia/database/AppDatabaseTests.kt

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,15 @@ import androidx.room.Room
55
import androidx.test.core.app.ApplicationProvider
66
import androidx.test.ext.junit.runners.AndroidJUnit4
77
import androidx.test.platform.app.InstrumentationRegistry
8-
import kotlinx.coroutines.*
9-
import kotlinx.coroutines.flow.count
10-
import kotlinx.coroutines.flow.first
11-
import org.hamcrest.CoreMatchers.*
8+
import kotlinx.coroutines.CoroutineExceptionHandler
9+
import kotlinx.coroutines.CoroutineScope
10+
import kotlinx.coroutines.Dispatchers
11+
import kotlinx.coroutines.launch
12+
import kotlinx.coroutines.runBlocking
13+
import kotlinx.coroutines.withContext
14+
import org.hamcrest.CoreMatchers.equalTo
15+
import org.hamcrest.CoreMatchers.notNullValue
16+
import org.hamcrest.CoreMatchers.nullValue
1217
import org.hamcrest.MatcherAssert.assertThat
1318
import org.junit.After
1419
import org.junit.Before
@@ -22,7 +27,7 @@ import org.wikipedia.search.db.RecentSearchDao
2227
import org.wikipedia.talk.db.TalkPageSeen
2328
import org.wikipedia.talk.db.TalkPageSeenDao
2429
import org.wikipedia.util.log.L
25-
import java.util.*
30+
import java.util.Date
2631

2732
@RunWith(AndroidJUnit4::class)
2833
class AppDatabaseTests {
@@ -100,8 +105,8 @@ class AppDatabaseTests {
100105

101106
notificationDao.insertNotifications(notifications)
102107

103-
var enWikiList = notificationDao.getNotificationsByWiki(listOf("enwiki")).first()
104-
val zhWikiList = notificationDao.getNotificationsByWiki(listOf("zhwiki")).first()
108+
var enWikiList = notificationDao.getNotificationsByWiki(listOf("enwiki"))
109+
val zhWikiList = notificationDao.getNotificationsByWiki(listOf("zhwiki"))
105110
assertThat(enWikiList, notNullValue())
106111
assertThat(enWikiList.first().id, equalTo(123759827))
107112
assertThat(zhWikiList.first().id, equalTo(2470933))
@@ -114,18 +119,18 @@ class AppDatabaseTests {
114119
notificationDao.updateNotification(firstEnNotification)
115120

116121
// get updated item
117-
enWikiList = notificationDao.getNotificationsByWiki(listOf("enwiki")).first()
122+
enWikiList = notificationDao.getNotificationsByWiki(listOf("enwiki"))
118123
assertThat(enWikiList.first().id, equalTo(123759827))
119124
assertThat(enWikiList.first().isUnread, equalTo(true))
120125

121126
notificationDao.deleteNotification(firstEnNotification)
122127
assertThat(notificationDao.getAllNotifications().size, equalTo(2))
123-
assertThat(notificationDao.getNotificationsByWiki(listOf("enwiki")).first().size, equalTo(1))
128+
assertThat(notificationDao.getNotificationsByWiki(listOf("enwiki")).size, equalTo(1))
124129

125-
notificationDao.deleteNotification(notificationDao.getNotificationsByWiki(listOf("enwiki")).first().first())
126-
assertThat(notificationDao.getNotificationsByWiki(listOf("enwiki")).first().isEmpty(), equalTo(true))
130+
notificationDao.deleteNotification(notificationDao.getNotificationsByWiki(listOf("enwiki")).first())
131+
assertThat(notificationDao.getNotificationsByWiki(listOf("enwiki")).isEmpty(), equalTo(true))
127132

128-
notificationDao.deleteNotification(notificationDao.getNotificationsByWiki(listOf("zhwiki")).first().first())
133+
notificationDao.deleteNotification(notificationDao.getNotificationsByWiki(listOf("zhwiki")).first())
129134
assertThat(notificationDao.getAllNotifications().isEmpty(), equalTo(true))
130135
}
131136
}

‎app/src/androidTest/java/org/wikipedia/database/UpgradeFromPreRoomTest.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import androidx.room.Room
44
import androidx.room.testing.MigrationTestHelper
55
import androidx.test.core.app.ApplicationProvider
66
import androidx.test.platform.app.InstrumentationRegistry
7-
import kotlinx.coroutines.flow.first
87
import kotlinx.coroutines.runBlocking
98
import org.hamcrest.CoreMatchers.equalTo
109
import org.hamcrest.CoreMatchers.nullValue
@@ -123,7 +122,7 @@ class UpgradeFromPreRoomTest(private val fromVersion: Int) {
123122
val historyEntry = historyDao.findEntryBy("ru.wikipedia.org", "ru", "Обама,_Барак")!!
124123
assertThat(historyEntry.displayTitle, equalTo("Обама, Барак"))
125124

126-
val talkPageSeen = talkPageSeenDao.getAll().first()
125+
val talkPageSeen = talkPageSeenDao.getAll()
127126
if (fromVersion == 22) {
128127
assertThat(talkPageSeen.count(), equalTo(2))
129128
assertThat(offlineObjectDao.getOfflineObject("https://en.wikipedia.org/api/rest_v1/page/summary/Joe_Biden")!!.path, equalTo("/data/user/0/org.wikipedia.dev/files/offline_files/481b1ef996728fd9994bd97ab19733d8"))

‎app/src/main/java/org/wikipedia/WikipediaApp.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,9 @@ class WikipediaApp : Application() {
258258
Prefs.tempAccountCreateDay = 0L
259259
Prefs.tempAccountDialogShown = false
260260
SharedPreferenceCookieManager.instance.clearAllCookies()
261-
AppDatabase.instance.notificationDao().deleteAll()
261+
MainScope().launch {
262+
AppDatabase.instance.notificationDao().deleteAll()
263+
}
262264
FlowEventBus.post(LoggedOutEvent())
263265
L.d("Logout complete.")
264266
}

‎app/src/main/java/org/wikipedia/database/AppDatabase.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,6 @@ abstract class AppDatabase : RoomDatabase() {
354354
MIGRATION_23_24, MIGRATION_24_25, MIGRATION_25_26, MIGRATION_26_27,
355355
MIGRATION_26_28, MIGRATION_27_28, MIGRATION_28_29, MIGRATION_29_30,
356356
MIGRATION_30_31)
357-
.allowMainThreadQueries() // TODO: remove after resolving main thread issues in DAO classes
358357
.fallbackToDestructiveMigration(false)
359358
.build()
360359
}

‎app/src/main/java/org/wikipedia/dataclient/okhttp/OfflineCacheInterceptor.kt

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,35 @@
11
package org.wikipedia.dataclient.okhttp
22

3-
import okhttp3.*
3+
import kotlinx.coroutines.runBlocking
4+
import okhttp3.Interceptor
5+
import okhttp3.MediaType
46
import okhttp3.MediaType.Companion.toMediaTypeOrNull
5-
import okio.*
7+
import okhttp3.Protocol
8+
import okhttp3.Request
9+
import okhttp3.Response
10+
import okhttp3.ResponseBody
11+
import okio.Buffer
12+
import okio.BufferedSink
13+
import okio.BufferedSource
14+
import okio.Source
15+
import okio.Timeout
16+
import okio.buffer
17+
import okio.sink
18+
import okio.source
19+
import okio.use
620
import org.wikipedia.WikipediaApp
721
import org.wikipedia.database.AppDatabase
822
import org.wikipedia.offline.db.OfflineObject
923
import org.wikipedia.util.StringUtil
1024
import org.wikipedia.util.UriUtil
1125
import org.wikipedia.util.log.L
12-
import java.io.*
26+
import java.io.BufferedReader
27+
import java.io.File
28+
import java.io.FileInputStream
29+
import java.io.FileOutputStream
1330
import java.io.IOException
14-
import java.util.*
31+
import java.io.InputStreamReader
32+
import java.io.OutputStreamWriter
1533

1634
class OfflineCacheInterceptor : Interceptor {
1735

@@ -49,7 +67,9 @@ class OfflineCacheInterceptor : Interceptor {
4967
throw networkException
5068
}
5169
}
52-
val obj = AppDatabase.instance.offlineObjectDao().findObject(url, lang)
70+
val obj = runBlocking {
71+
AppDatabase.instance.offlineObjectDao().findObject(url, lang)
72+
}
5373
if (obj == null) {
5474
L.w("Offline object not present in database.")
5575
throw networkException
@@ -139,8 +159,8 @@ class OfflineCacheInterceptor : Interceptor {
139159
} ?: response
140160
}
141161

142-
private inner class CacheWritingSource constructor(private val source: BufferedSource, private val cacheSink: BufferedSink,
143-
private val obj: OfflineObject, private val title: String) : Source {
162+
private inner class CacheWritingSource(private val source: BufferedSource, private val cacheSink: BufferedSink,
163+
private val obj: OfflineObject, private val title: String) : Source {
144164
private var cacheRequestClosed = false
145165
private var failed = false
146166

@@ -163,8 +183,9 @@ class OfflineCacheInterceptor : Interceptor {
163183
// The cache response is complete!
164184
cacheSink.close()
165185
if (!failed) {
166-
// update the record in the database!
167-
AppDatabase.instance.offlineObjectDao().addObject(obj.url, obj.lang, obj.path, title)
186+
runBlocking {
187+
AppDatabase.instance.offlineObjectDao().addObject(obj.url, obj.lang, obj.path, title)
188+
}
168189
}
169190
}
170191
return -1
@@ -192,9 +213,9 @@ class OfflineCacheInterceptor : Interceptor {
192213
}
193214
}
194215

195-
private inner class CacheWritingResponseBody constructor(private val source: Source,
196-
private val contentType: String?,
197-
private val contentLength: Long) : ResponseBody() {
216+
private inner class CacheWritingResponseBody(private val source: Source,
217+
private val contentType: String?,
218+
private val contentLength: Long) : ResponseBody() {
198219
override fun contentType(): MediaType? {
199220
return contentType?.toMediaTypeOrNull()
200221
}
@@ -208,8 +229,8 @@ class OfflineCacheInterceptor : Interceptor {
208229
}
209230
}
210231

211-
private inner class CachedResponseBody constructor(private val file: File,
212-
private val contentType: String?) : ResponseBody() {
232+
private inner class CachedResponseBody(private val file: File,
233+
private val contentType: String?) : ResponseBody() {
213234
override fun contentType(): MediaType? {
214235
return contentType?.toMediaTypeOrNull()
215236
}

‎app/src/main/java/org/wikipedia/notifications/NotificationPollBroadcastReceiver.kt

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import android.os.SystemClock
99
import androidx.annotation.StringRes
1010
import androidx.core.app.PendingIntentCompat
1111
import androidx.core.app.RemoteInput
12+
import kotlinx.coroutines.CoroutineExceptionHandler
1213
import kotlinx.coroutines.Dispatchers
14+
import kotlinx.coroutines.MainScope
15+
import kotlinx.coroutines.launch
1316
import kotlinx.coroutines.withContext
1417
import org.wikipedia.Constants
1518
import org.wikipedia.R
@@ -146,26 +149,30 @@ class NotificationPollBroadcastReceiver : BroadcastReceiver() {
146149
return
147150
}
148151

149-
// The notifications that we need to display are those that don't exist in our db yet.
150-
val notificationsToDisplay = notifications.filter {
151-
AppDatabase.instance.notificationDao().getNotificationById(it.wiki, it.id) == null
152-
}
153-
AppDatabase.instance.notificationDao().insertNotifications(notificationsToDisplay)
152+
MainScope().launch(CoroutineExceptionHandler { _, throwable ->
153+
L.w(throwable)
154+
}) {
155+
// The notifications that we need to display are those that don't exist in our db yet.
156+
val notificationsToDisplay = notifications.filter {
157+
AppDatabase.instance.notificationDao().getNotificationById(it.wiki, it.id) == null
158+
}
159+
AppDatabase.instance.notificationDao().insertNotifications(notificationsToDisplay)
154160

155-
if (notificationsToDisplay.isNotEmpty()) {
156-
Prefs.notificationUnreadCount = notificationsToDisplay.size
157-
FlowEventBus.post(UnreadNotificationsEvent())
158-
}
161+
if (notificationsToDisplay.isNotEmpty()) {
162+
Prefs.notificationUnreadCount = notificationsToDisplay.size
163+
FlowEventBus.post(UnreadNotificationsEvent())
164+
}
159165

160-
if (notificationsToDisplay.size > 2) {
161-
// Record that there is an incoming notification to track/compare further actions on it.
162-
NotificationPresenter.showMultipleUnread(context, notificationsToDisplay.size)
163-
} else {
164-
for (n in notificationsToDisplay) {
166+
if (notificationsToDisplay.size > 2) {
165167
// Record that there is an incoming notification to track/compare further actions on it.
166-
NotificationPresenter.showNotification(context, n,
167-
dbWikiNameMap.getOrElse(n.wiki) { n.wiki },
168-
dbWikiSiteMap.getValue(n.wiki).languageCode)
168+
NotificationPresenter.showMultipleUnread(context, notificationsToDisplay.size)
169+
} else {
170+
for (n in notificationsToDisplay) {
171+
// Record that there is an incoming notification to track/compare further actions on it.
172+
NotificationPresenter.showNotification(context, n,
173+
dbWikiNameMap.getOrElse(n.wiki) { n.wiki },
174+
dbWikiSiteMap.getValue(n.wiki).languageCode)
175+
}
169176
}
170177
}
171178
}

‎app/src/main/java/org/wikipedia/notifications/NotificationRepository.kt

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@ import org.wikipedia.notifications.db.NotificationDao
88

99
class NotificationRepository(private val notificationDao: NotificationDao) {
1010

11-
fun getAllNotifications() = notificationDao.getAllNotifications()
12-
13-
private fun insertNotifications(notifications: List<Notification>) {
14-
notificationDao.insertNotifications(notifications)
15-
}
11+
suspend fun getAllNotifications() = notificationDao.getAllNotifications()
1612

1713
suspend fun updateNotification(notification: Notification) {
1814
notificationDao.updateNotification(notification)
@@ -28,7 +24,7 @@ class NotificationRepository(private val notificationDao: NotificationDao) {
2824
var newContinueStr: String? = null
2925
val response = ServiceFactory.get(WikipediaApp.instance.wikiSite).getAllNotifications(wikiList, filter, continueStr)
3026
response.query?.notifications?.let {
31-
insertNotifications(it.list.orEmpty())
27+
notificationDao.insertNotifications(it.list.orEmpty())
3228
newContinueStr = it.continueStr
3329
}
3430
return newContinueStr

‎app/src/main/java/org/wikipedia/notifications/NotificationViewModel.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class NotificationViewModel : ViewModel() {
4242
fetchAndSave()
4343
}
4444

45-
private fun filterAndPostNotifications() {
45+
private suspend fun filterAndPostNotifications() {
4646
val pair = Pair(processList(notificationRepository.getAllNotifications()), !currentContinueStr.isNullOrEmpty())
4747
_uiState.value = Resource.Success(pair)
4848
}

‎app/src/main/java/org/wikipedia/notifications/db/NotificationDao.kt

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@ import androidx.room.Insert
66
import androidx.room.OnConflictStrategy
77
import androidx.room.Query
88
import androidx.room.Update
9-
import kotlinx.coroutines.flow.Flow
109

1110
@Dao
1211
interface NotificationDao {
1312
@Insert(onConflict = OnConflictStrategy.REPLACE)
14-
fun insertNotifications(notifications: List<Notification>)
13+
suspend fun insertNotifications(notifications: List<Notification>)
1514

1615
@Update(onConflict = OnConflictStrategy.REPLACE)
1716
suspend fun updateNotification(notification: Notification)
@@ -20,14 +19,14 @@ interface NotificationDao {
2019
suspend fun deleteNotification(notification: Notification)
2120

2221
@Query("DELETE FROM Notification")
23-
fun deleteAll()
22+
suspend fun deleteAll()
2423

2524
@Query("SELECT * FROM Notification")
26-
fun getAllNotifications(): List<Notification>
25+
suspend fun getAllNotifications(): List<Notification>
2726

2827
@Query("SELECT * FROM Notification WHERE `wiki` IN (:wiki)")
29-
fun getNotificationsByWiki(wiki: List<String>): Flow<List<Notification>>
28+
suspend fun getNotificationsByWiki(wiki: List<String>): List<Notification>
3029

3130
@Query("SELECT * FROM Notification WHERE `wiki` IN (:wiki) AND `id` IN (:id)")
32-
fun getNotificationById(wiki: String, id: Long): Notification?
31+
suspend fun getNotificationById(wiki: String, id: Long): Notification?
3332
}

0 commit comments

Comments
 (0)