Skip to content

Commit 41236d6

Browse files
mahibimahibi
authored
Merge pull request #6733 from nextcloud/backport/6730/stable-25.0.x
[stable-25.0.x] Fix no supported api exception
2 parents 43bf7da + 26e5cab commit 41236d6

7 files changed

Lines changed: 72 additions & 112 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/chat/data/network/ChatNetworkDataSource.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import retrofit2.Response
2323

2424
@Suppress("LongParameterList", "TooManyFunctions")
2525
interface ChatNetworkDataSource {
26-
fun getRoom(user: User, roomToken: String): Observable<ConversationModel>
26+
suspend fun getRoom(user: User, roomToken: String): ConversationModel
2727
fun getCapabilities(user: User, roomToken: String): Observable<SpreedCapability>
2828
fun joinRoom(user: User, roomToken: String, roomPassword: String): Observable<ConversationModel>
2929
fun setReminder(

‎app/src/main/java/com/nextcloud/talk/chat/data/network/RetrofitChatNetwork.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@ import retrofit2.Response
2727

2828
class RetrofitChatNetwork(private val ncApi: NcApi, private val ncApiCoroutines: NcApiCoroutines) :
2929
ChatNetworkDataSource {
30-
override fun getRoom(user: User, roomToken: String): Observable<ConversationModel> {
30+
override suspend fun getRoom(user: User, roomToken: String): ConversationModel {
3131
val credentials: String = ApiUtils.getCredentials(user.username, user.token)!!
3232
val apiVersion = ApiUtils.getConversationApiVersion(user, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V3, 1))
3333

34-
return ncApi.getRoom(
34+
val roomOverall = ncApiCoroutines.getRoom(
3535
credentials,
3636
ApiUtils.getUrlForRoom(apiVersion, user.baseUrl!!, roomToken)
37-
).map { ConversationModel.mapToConversationModel(it.ocs?.data!!, user) }
37+
)
38+
return ConversationModel.mapToConversationModel(roomOverall.ocs?.data!!, user)
3839
}
3940

4041
override fun getCapabilities(user: User, roomToken: String): Observable<SpreedCapability> {

‎app/src/main/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModel.kt‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,25 @@ class ConversationInfoViewModel @Inject constructor(
173173
return uiItems
174174
}
175175

176+
@Suppress("Detekt.TooGenericExceptionCaught")
176177
fun getRoom(user: User, token: String) {
177178
currentUser = user
178179
currentToken = token
179180
if (databaseStorageModule == null) {
180181
databaseStorageModule = DatabaseStorageModule(user, token)
181182
}
182183
_uiState.update { it.copy(isLoading = true) }
183-
chatNetworkDataSource.getRoom(user, token)
184-
.subscribeOn(Schedulers.io())
185-
?.observeOn(AndroidSchedulers.mainThread())
186-
?.subscribe(GetRoomObserver())
184+
viewModelScope.launch {
185+
try {
186+
val conversationModel = chatNetworkDataSource.getRoom(user, token)
187+
_uiState.update { it.copy(conversation = conversationModel) }
188+
getCapabilities(user, token, conversationModel)
189+
} catch (e: Exception) {
190+
Log.e(TAG, "Error when fetching room", e)
191+
_uiState.update { it.copy(isLoading = false) }
192+
_uiEvent.emit(ConversationInfoUiEvent.ShowSnackbar(R.string.nc_common_error_sorry))
193+
}
194+
}
187195
}
188196

189197
@Suppress("Detekt.TooGenericExceptionCaught")
@@ -906,25 +914,6 @@ class ConversationInfoViewModel @Inject constructor(
906914
}
907915
}
908916

909-
inner class GetRoomObserver : Observer<ConversationModel> {
910-
override fun onSubscribe(d: Disposable) {
911-
// unused atm
912-
}
913-
override fun onNext(conversationModel: ConversationModel) {
914-
_uiState.update { it.copy(conversation = conversationModel) }
915-
currentUser?.let { getCapabilities(it, currentToken, conversationModel) }
916-
}
917-
override fun onError(e: Throwable) {
918-
Log.e(TAG, "Error when fetching room")
919-
_uiState.update { it.copy(isLoading = false) }
920-
viewModelScope.launch {
921-
_uiEvent.emit(ConversationInfoUiEvent.ShowSnackbar(R.string.nc_common_error_sorry))
922-
}
923-
}
924-
override fun onComplete() {
925-
// unused atm
926-
}
927-
}
928917
companion object {
929918
private val TAG = ConversationInfoViewModel::class.simpleName
930919
private const val NEW_CONVERSATION_PARTICIPANTS_SEPARATOR = ", "

‎app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt‎

Lines changed: 31 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ import com.nextcloud.talk.utils.ApiUtils
2727
import com.nextcloud.talk.utils.CapabilitiesUtil.isUserStatusAvailable
2828
import com.nextcloud.talk.utils.SpreedFeatures
2929
import com.nextcloud.talk.utils.withRetry
30-
import io.reactivex.Observer
3130
import io.reactivex.android.schedulers.AndroidSchedulers
32-
import io.reactivex.disposables.Disposable
3331
import io.reactivex.schedulers.Schedulers
3432
import kotlinx.coroutines.CoroutineScope
3533
import kotlinx.coroutines.Dispatchers
@@ -45,7 +43,6 @@ import kotlinx.coroutines.flow.flatMapLatest
4543
import kotlinx.coroutines.flow.map
4644
import kotlinx.coroutines.coroutineScope
4745
import kotlinx.coroutines.launch
48-
import kotlinx.coroutines.runBlocking
4946
import kotlinx.coroutines.sync.Semaphore
5047
import kotlinx.coroutines.sync.withPermit
5148
import javax.inject.Inject
@@ -117,52 +114,41 @@ class OfflineFirstConversationsRepository @Inject constructor(
117114
}
118115
}
119116

117+
@Suppress("Detekt.TooGenericExceptionCaught")
120118
override fun getRoom(user: User, roomToken: String): Job =
121119
scope.launch {
122-
chatNetworkDataSource.getRoom(user, roomToken)
123-
.subscribeOn(Schedulers.io())
124-
?.observeOn(AndroidSchedulers.mainThread())
125-
?.subscribe(object : Observer<ConversationModel> {
126-
override fun onSubscribe(p0: Disposable) {
127-
// unused atm
128-
}
129-
130-
override fun onError(e: Throwable) {
131-
runBlocking {
132-
// In case network is offline or call fails
133-
val id = user.id!!
134-
val model = getConversation(id, roomToken)
135-
if (model != null) {
136-
_conversationFlow.emit(model)
137-
} else {
138-
Log.e(TAG, "Conversation model not found on device database")
139-
}
140-
}
141-
}
142-
143-
override fun onComplete() {
144-
// unused atm
145-
}
120+
try {
121+
val model = chatNetworkDataSource.getRoom(user, roomToken)
122+
val existingEntity = dao.getConversationForUser(user.id!!, model.token).first()
123+
model.hiddenUpcomingEvent = existingEntity?.hiddenUpcomingEvent
124+
_conversationFlow.emit(model)
125+
val previous = existingEntity?.let { mapOf(it.internalId to it) }.orEmpty()
126+
val entityList = conversationListUpdater.preservePendingLocalState(
127+
previous,
128+
listOf(model.asEntity())
129+
)
130+
try {
131+
dao.upsertConversations(user.id!!, entityList)
132+
} catch (e: SQLiteConstraintException) {
133+
Log.w(TAG, "Skipping conversation upsert for removed account ${user.id}", e)
134+
}
135+
} catch (e: Exception) {
136+
// In case network is offline, the call fails, or getRoom can't resolve a supported
137+
// conversation API version (e.g. capabilities not loaded yet)
138+
fallBackToLocalConversation(user, roomToken, e)
139+
}
140+
}
146141

147-
override fun onNext(model: ConversationModel) {
148-
runBlocking {
149-
val existingEntity = dao.getConversationForUser(user.id!!, model.token).first()
150-
model.hiddenUpcomingEvent = existingEntity?.hiddenUpcomingEvent
151-
_conversationFlow.emit(model)
152-
val previous = existingEntity?.let { mapOf(it.internalId to it) }.orEmpty()
153-
val entityList = conversationListUpdater.preservePendingLocalState(
154-
previous,
155-
listOf(model.asEntity())
156-
)
157-
try {
158-
dao.upsertConversations(user.id!!, entityList)
159-
} catch (e: SQLiteConstraintException) {
160-
Log.w(TAG, "Skipping conversation upsert for removed account ${user.id}", e)
161-
}
162-
}
163-
}
164-
})
142+
private suspend fun fallBackToLocalConversation(user: User, roomToken: String, e: Throwable) {
143+
Log.e(TAG, "Failed to fetch room $roomToken from server", e)
144+
val id = user.id!!
145+
val model = getConversation(id, roomToken)
146+
if (model != null) {
147+
_conversationFlow.emit(model)
148+
} else {
149+
Log.e(TAG, "Conversation model not found on device database")
165150
}
151+
}
166152

167153
override suspend fun updateConversation(conversationModel: ConversationModel) {
168154
val entity = conversationModel.asEntity()

‎app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt‎

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor
249249
getNcDataAndShowNotification(mainActivityIntent)
250250
}
251251

252-
@Suppress("LongMethod")
252+
@Suppress("LongMethod", "TooGenericExceptionCaught")
253253
private fun handleCallPushMessage() {
254254
val userBeingCalled = userManager.getUserWithId(user.id!!).blockingGet()
255255

@@ -395,35 +395,20 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor
395395
checkIfCallIsActive(conversation)
396396
}
397397

398-
chatNetworkDataSource?.getRoom(userBeingCalled, roomToken = pushMessage.id!!)
399-
?.subscribeOn(Schedulers.io())
400-
?.observeOn(Schedulers.io())
401-
?.subscribe(object : Observer<ConversationModel> {
402-
override fun onSubscribe(d: Disposable) {
403-
// unused atm
404-
}
405-
406-
override fun onNext(conversation: ConversationModel) {
407-
if (userManager.setUserAsActive(userBeingCalled!!).blockingGet()) {
408-
if (CapabilitiesUtil.isCallEndToEndEncryptionEnabled(
409-
userBeingCalled?.capabilities?.spreedCapability
410-
)
411-
) {
412-
showEndToEndEncryptionUnsupportedNotification(conversation)
413-
} else {
414-
prepareCallNotificationScreen(conversation)
415-
}
416-
}
417-
}
418-
419-
override fun onError(e: Throwable) {
420-
Log.e(TAG, "Failed to get room", e)
421-
}
398+
val conversation = try {
399+
runBlocking { chatNetworkDataSource?.getRoom(userBeingCalled, roomToken = pushMessage.id!!) }
400+
} catch (e: Exception) {
401+
Log.e(TAG, "Failed to get room", e)
402+
null
403+
}
422404

423-
override fun onComplete() {
424-
// unused atm
425-
}
426-
})
405+
if (conversation != null && userManager.setUserAsActive(userBeingCalled!!).blockingGet()) {
406+
if (CapabilitiesUtil.isCallEndToEndEncryptionEnabled(userBeingCalled?.capabilities?.spreedCapability)) {
407+
showEndToEndEncryptionUnsupportedNotification(conversation)
408+
} else {
409+
prepareCallNotificationScreen(conversation)
410+
}
411+
}
427412
}
428413

429414
private fun initNcApiAndCredentials() {

‎app/src/test/java/com/nextcloud/talk/conversationlist/data/network/ConversationListFreshnessIntegrationTest.kt‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,10 @@ class ConversationListFreshnessIntegrationTest {
186186
conversationListUpdater,
187187
ApplicationProvider.getApplicationContext()
188188
)
189-
whenever(chatNetwork.getRoom(any(), any())).thenReturn(
190-
Observable.just(
191-
ConversationModel.mapToConversationModel(
192-
staleServerRoom(lastReadMessage = 10, unreadMessages = 2),
193-
user
194-
)
189+
wheneverBlocking { chatNetwork.getRoom(any(), any()) }.thenReturn(
190+
ConversationModel.mapToConversationModel(
191+
staleServerRoom(lastReadMessage = 10, unreadMessages = 2),
192+
user
195193
)
196194
)
197195

‎app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ import org.mockito.kotlin.whenever
6060
* returned [kotlinx.coroutines.Job]), so `getRooms(user).join()` does not wait for it. Assertions
6161
* about the catch-up therefore poll with a real timeout ([AWAIT_TIMEOUT_MILLIS]) rather than just
6262
* asserting right after `join()`; skip assertions instead wait a fixed delay
63-
* ([AFTER_DELAY_MILLIS]) and check nothing arrived. The RxJava chain in [getRoom] behaves the
64-
* same way, for the same reason.
63+
* ([AFTER_DELAY_MILLIS]) and check nothing arrived. [getRoom]'s emission to
64+
* [OfflineFirstConversationsRepository.conversationFlow] has the same race, since that flow has no
65+
* replay buffer and drops emissions with no subscriber yet.
6566
*/
6667
@Suppress("TooManyFunctions")
6768
class OfflineFirstConversationsRepositoryTest {
@@ -362,7 +363,7 @@ class OfflineFirstConversationsRepositoryTest {
362363
conversation(token = ROOM_TOKEN, lastActivity = 5, unreadMessages = 2),
363364
user()
364365
)
365-
whenever(chatNetworkDataSource.getRoom(any(), any())).thenReturn(Observable.just(fetched))
366+
wheneverBlocking { chatNetworkDataSource.getRoom(any(), any()) }.thenReturn(fetched)
366367

367368
repository.getRoom(user(), ROOM_TOKEN).join()
368369

@@ -377,8 +378,8 @@ class OfflineFirstConversationsRepositoryTest {
377378
runBlocking {
378379
val existing = conversation(token = ROOM_TOKEN, lastActivity = 4, unreadMessages = 1).asEntity(ACCOUNT_ID)
379380
whenever(dao.getConversationForUser(ACCOUNT_ID, ROOM_TOKEN)).thenReturn(flowOf(existing))
380-
whenever(chatNetworkDataSource.getRoom(any(), any()))
381-
.thenReturn(Observable.error(RuntimeException("network failure")))
381+
wheneverBlocking { chatNetworkDataSource.getRoom(any(), any()) }
382+
.thenThrow(RuntimeException("network failure"))
382383

383384
val emissions = mutableListOf<ConversationModel>()
384385
val collector = launch(Dispatchers.IO) {

0 commit comments

Comments
 (0)