Merge pull request #3091 from vector-im/feature/bma/cleanup

Migrate Room method to suspend and do some cleanup

CHANGES.md

@@ -24,6 +24,7 @@ Translations 🗣:
  -
 
 SDK API changes ⚠️:
+ - Several Services have been migrated to coroutines (#2449)
  - Removes filtering options on Timeline.
 
 Build 🧱:

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/search/SearchMessagesTest.kt

@@ -17,115 +17,38 @@
 package org.matrix.android.sdk.session.search
 
 import org.junit.Assert.assertTrue
-import org.junit.Assert.fail
 import org.junit.FixMethodOrder
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import org.junit.runners.MethodSorters
 import org.matrix.android.sdk.InstrumentedTest
-import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.extensions.orFalse
 import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.message.MessageContent
 import org.matrix.android.sdk.api.session.room.timeline.TimelineSettings
 import org.matrix.android.sdk.api.session.search.SearchResult
 import org.matrix.android.sdk.common.CommonTestHelper
+import org.matrix.android.sdk.common.CryptoTestData
 import org.matrix.android.sdk.common.CryptoTestHelper
-import org.matrix.android.sdk.common.TestConstants
 import java.util.concurrent.CountDownLatch
-import java.util.concurrent.TimeUnit
 
 @RunWith(JUnit4::class)
 @FixMethodOrder(MethodSorters.JVM)
 class SearchMessagesTest : InstrumentedTest {
 
-    private val MESSAGE = "Lorem ipsum dolor sit amet"
+    companion object {
+        private const val MESSAGE = "Lorem ipsum dolor sit amet"
+    }
 
     private val commonTestHelper = CommonTestHelper(context())
     private val cryptoTestHelper = CryptoTestHelper(commonTestHelper)
 
     @Test
     fun sendTextMessageAndSearchPartOfItUsingSession() {
-        val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(false)
-        val aliceSession = cryptoTestData.firstSession
-        val aliceRoomId = cryptoTestData.roomId
-        aliceSession.cryptoService().setWarnOnUnknownDevices(false)
-        val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!!
-        val aliceTimeline = roomFromAlicePOV.createTimeline(null, TimelineSettings(10))
-        aliceTimeline.start()
-
-        commonTestHelper.sendTextMessage(
-                roomFromAlicePOV,
-                MESSAGE,
-                2)
-
-        run {
-            val lock = CountDownLatch(1)
-
-            val eventListener = commonTestHelper.createEventListener(lock) { snapshot ->
-                snapshot.count { it.root.content.toModel<MessageContent>()?.body?.startsWith(MESSAGE).orFalse() } == 2
-            }
-
-            aliceTimeline.addListener(eventListener)
-            commonTestHelper.await(lock)
-
-            val data = commonTestHelper.runBlockingTest {
-                aliceSession
-                        .searchService()
-                        .search(
-                                searchTerm = "lore",
-                                limit = 10,
-                                includeProfile = true,
-                                afterLimit = 0,
-                                beforeLimit = 10,
-                                orderByRecent = true,
-                                nextBatch = null,
-                                roomId = aliceRoomId
-                        )
-            }
-            assertTrue(data.results?.size == 2)
-            assertTrue(
-                    data.results
-                            ?.all {
-                                (it.event.content?.get("body") as? String)?.startsWith(MESSAGE).orFalse()
-                            }.orFalse()
-            )
-
-            aliceTimeline.removeAllListeners()
-            cryptoTestData.cleanUp(commonTestHelper)
-        }
-
-        aliceSession.startSync(true)
-    }
-
-    @Test
-    fun sendTextMessageAndSearchPartOfItUsingRoom() {
-        val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(false)
-        val aliceSession = cryptoTestData.firstSession
-        val aliceRoomId = cryptoTestData.roomId
-        aliceSession.cryptoService().setWarnOnUnknownDevices(false)
-        val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!!
-        val aliceTimeline = roomFromAlicePOV.createTimeline(null, TimelineSettings(10))
-        aliceTimeline.start()
-
-        commonTestHelper.sendTextMessage(
-                roomFromAlicePOV,
-                MESSAGE,
-                2)
-
-        run {
-            var lock = CountDownLatch(1)
-
-            val eventListener = commonTestHelper.createEventListener(lock) { snapshot ->
-                snapshot.count { it.root.content.toModel<MessageContent>()?.body?.startsWith(MESSAGE).orFalse() } == 2
-            }
-
-            aliceTimeline.addListener(eventListener)
-            commonTestHelper.await(lock)
-
-            lock = CountDownLatch(1)
-            roomFromAlicePOV
+        doTest { cryptoTestData ->
+            cryptoTestData.firstSession
+                    .searchService()
                     .search(
                             searchTerm = "lore",
                             limit = 10,
@@ -134,32 +57,64 @@ class SearchMessagesTest : InstrumentedTest {
                             beforeLimit = 10,
                             orderByRecent = true,
                             nextBatch = null,
-                            callback = object : MatrixCallback<SearchResult> {
-                                override fun onSuccess(data: SearchResult) {
-                                    super.onSuccess(data)
-                                    assertTrue(data.results?.size == 2)
-                                    assertTrue(
-                                            data.results
-                                                    ?.all {
-                                                        (it.event.content?.get("body") as? String)?.startsWith(MESSAGE).orFalse()
-                                                    }.orFalse()
-                                    )
-                                    lock.countDown()
-                                }
-
-                                override fun onFailure(failure: Throwable) {
-                                    super.onFailure(failure)
-                                    fail(failure.localizedMessage)
-                                    lock.countDown()
-                                }
-                            }
+                            roomId = cryptoTestData.roomId
                     )
-            lock.await(TestConstants.timeOutMillis, TimeUnit.MILLISECONDS)
+        }
+    }
 
-            aliceTimeline.removeAllListeners()
-            cryptoTestData.cleanUp(commonTestHelper)
+    @Test
+    fun sendTextMessageAndSearchPartOfItUsingRoom() {
+        doTest { cryptoTestData ->
+            cryptoTestData.firstSession
+                    .getRoom(cryptoTestData.roomId)!!
+                    .search(
+                            searchTerm = "lore",
+                            limit = 10,
+                            includeProfile = true,
+                            afterLimit = 0,
+                            beforeLimit = 10,
+                            orderByRecent = true,
+                            nextBatch = null
+                    )
+        }
+    }
+
+    private fun doTest(block: suspend (CryptoTestData) -> SearchResult) {
+        val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceInARoom(false)
+        val aliceSession = cryptoTestData.firstSession
+        val aliceRoomId = cryptoTestData.roomId
+        val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!!
+        val aliceTimeline = roomFromAlicePOV.createTimeline(null, TimelineSettings(10))
+        aliceTimeline.start()
+
+        val lock = CountDownLatch(1)
+
+        val eventListener = commonTestHelper.createEventListener(lock) { snapshot ->
+            snapshot.count { it.root.content.toModel<MessageContent>()?.body?.startsWith(MESSAGE).orFalse() } == 2
         }
 
-        aliceSession.startSync(true)
+        aliceTimeline.addListener(eventListener)
+
+        commonTestHelper.sendTextMessage(
+                roomFromAlicePOV,
+                MESSAGE,
+                2)
+
+        commonTestHelper.await(lock)
+
+        val data = commonTestHelper.runBlockingTest {
+            block.invoke(cryptoTestData)
+        }
+
+        assertTrue(data.results?.size == 2)
+        assertTrue(
+                data.results
+                        ?.all {
+                            (it.event.content?.get("body") as? String)?.startsWith(MESSAGE).orFalse()
+                        }.orFalse()
+        )
+
+        aliceTimeline.removeAllListeners()
+        cryptoTestData.cleanUp(commonTestHelper)
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/Room.kt

@@ -17,7 +17,6 @@
 package org.matrix.android.sdk.api.session.room
 
 import androidx.lifecycle.LiveData
-import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.session.room.alias.AliasService
 import org.matrix.android.sdk.api.session.room.call.RoomCallService
 import org.matrix.android.sdk.api.session.room.crypto.RoomCryptoService
@@ -35,7 +34,6 @@ import org.matrix.android.sdk.api.session.room.timeline.TimelineService
 import org.matrix.android.sdk.api.session.room.typing.TypingService
 import org.matrix.android.sdk.api.session.room.uploads.UploadsService
 import org.matrix.android.sdk.api.session.search.SearchResult
-import org.matrix.android.sdk.api.util.Cancelable
 import org.matrix.android.sdk.api.util.Optional
 
 /**
@@ -86,12 +84,11 @@ interface Room :
      * @param includeProfile requests that the server returns the historic profile information for the users that sent the events that were returned.
      * @param callback Callback to get the search result
      */
-    fun search(searchTerm: String,
+    suspend fun search(searchTerm: String,
                nextBatch: String?,
                orderByRecent: Boolean,
                limit: Int,
                beforeLimit: Int,
                afterLimit: Int,
-               includeProfile: Boolean,
-               callback: MatrixCallback<SearchResult>): Cancelable
+               includeProfile: Boolean): SearchResult
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/MatrixCallbackDelegate.kt

@@ -1,24 +0,0 @@
-/*
- * Copyright 2020 The Matrix.org Foundation C.I.C.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.matrix.android.sdk.api.util
-
-import org.matrix.android.sdk.api.MatrixCallback
-
-/**
- * Simple MatrixCallback implementation which delegate its calls to another callback
- */
-open class MatrixCallbackDelegate<T>(private val callback: MatrixCallback<T>) : MatrixCallback<T> by callback

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/model/MXQueuedEncryption.kt

@@ -1,34 +0,0 @@
-/*
- * Copyright 2020 The Matrix.org Foundation C.I.C.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.matrix.android.sdk.internal.crypto.model
-
-import org.matrix.android.sdk.api.MatrixCallback
-import org.matrix.android.sdk.api.session.events.model.Content
-
-class MXQueuedEncryption {
-
-    /**
-     * The data to encrypt.
-     */
-    var eventContent: Content? = null
-    var eventType: String? = null
-
-    /**
-     * the asynchronous callback
-     */
-    var apiCallback: MatrixCallback<Content>? = null
-}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/Try.kt

@@ -21,7 +21,6 @@ import arrow.core.Success
 import arrow.core.Try
 import arrow.core.TryOf
 import arrow.core.fix
-import org.matrix.android.sdk.api.MatrixCallback
 
 inline fun <A> TryOf<A>.onError(f: (Throwable) -> Unit): Try<A> = fix()
         .fold(
@@ -32,10 +31,6 @@ inline fun <A> TryOf<A>.onError(f: (Throwable) -> Unit): Try<A> = fix()
                 { Success(it) }
         )
 
-fun <A> Try<A>.foldToCallback(callback: MatrixCallback<A>): Unit = fold(
-        { callback.onFailure(it) },
-        { callback.onSuccess(it) })
-
 /**
  * Same as doOnNext for Observables
  */

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt

@@ -17,7 +17,6 @@
 package org.matrix.android.sdk.internal.session.room
 
 import androidx.lifecycle.LiveData
-import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.session.crypto.CryptoService
 import org.matrix.android.sdk.api.session.events.model.EventType
 import org.matrix.android.sdk.api.session.room.Room
@@ -37,14 +36,11 @@ import org.matrix.android.sdk.api.session.room.timeline.TimelineService
 import org.matrix.android.sdk.api.session.room.typing.TypingService
 import org.matrix.android.sdk.api.session.room.uploads.UploadsService
 import org.matrix.android.sdk.api.session.search.SearchResult
-import org.matrix.android.sdk.api.util.Cancelable
 import org.matrix.android.sdk.api.util.Optional
 import org.matrix.android.sdk.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM
 import org.matrix.android.sdk.internal.session.room.state.SendStateTask
 import org.matrix.android.sdk.internal.session.room.summary.RoomSummaryDataSource
 import org.matrix.android.sdk.internal.session.search.SearchTask
-import org.matrix.android.sdk.internal.task.TaskExecutor
-import org.matrix.android.sdk.internal.task.configureWith
 import org.matrix.android.sdk.internal.util.awaitCallback
 import java.security.InvalidParameterException
 import javax.inject.Inject
@@ -66,7 +62,6 @@ internal class DefaultRoom @Inject constructor(override val roomId: String,
                                                private val relationService: RelationService,
                                                private val roomMembersService: MembershipService,
                                                private val roomPushRuleService: RoomPushRuleService,
-                                               private val taskExecutor: TaskExecutor,
                                                private val sendStateTask: SendStateTask,
                                                private val searchTask: SearchTask) :
         Room,
@@ -133,16 +128,15 @@ internal class DefaultRoom @Inject constructor(override val roomId: String,
         }
     }
 
-    override fun search(searchTerm: String,
-                        nextBatch: String?,
-                        orderByRecent: Boolean,
-                        limit: Int,
-                        beforeLimit: Int,
-                        afterLimit: Int,
-                        includeProfile: Boolean,
-                        callback: MatrixCallback<SearchResult>): Cancelable {
-        return searchTask
-                .configureWith(SearchTask.Params(
+    override suspend fun search(searchTerm: String,
+                                nextBatch: String?,
+                                orderByRecent: Boolean,
+                                limit: Int,
+                                beforeLimit: Int,
+                                afterLimit: Int,
+                                includeProfile: Boolean): SearchResult {
+        return searchTask.execute(
+                SearchTask.Params(
                         searchTerm = searchTerm,
                         roomId = roomId,
                         nextBatch = nextBatch,
@@ -151,8 +145,7 @@ internal class DefaultRoom @Inject constructor(override val roomId: String,
                         beforeLimit = beforeLimit,
                         afterLimit = afterLimit,
                         includeProfile = includeProfile
-                )) {
-                    this.callback = callback
-                }.executeBy(taskExecutor)
+                )
+        )
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomFactory.kt

@@ -36,7 +36,6 @@ import org.matrix.android.sdk.internal.session.room.timeline.DefaultTimelineServ
 import org.matrix.android.sdk.internal.session.room.typing.DefaultTypingService
 import org.matrix.android.sdk.internal.session.room.uploads.DefaultUploadsService
 import org.matrix.android.sdk.internal.session.search.SearchTask
-import org.matrix.android.sdk.internal.task.TaskExecutor
 import javax.inject.Inject
 
 internal interface RoomFactory {
@@ -60,7 +59,6 @@ internal class DefaultRoomFactory @Inject constructor(private val cryptoService:
                                                       private val relationServiceFactory: DefaultRelationService.Factory,
                                                       private val membershipServiceFactory: DefaultMembershipService.Factory,
                                                       private val roomPushRuleServiceFactory: DefaultRoomPushRuleService.Factory,
-                                                      private val taskExecutor: TaskExecutor,
                                                       private val sendStateTask: SendStateTask,
                                                       private val searchTask: SearchTask) :
         RoomFactory {
@@ -84,7 +82,6 @@ internal class DefaultRoomFactory @Inject constructor(private val cryptoService:
                 relationService = relationServiceFactory.create(roomId),
                 roomMembersService = membershipServiceFactory.create(roomId),
                 roomPushRuleService = roomPushRuleServiceFactory.create(roomId),
-                taskExecutor = taskExecutor,
                 sendStateTask = sendStateTask,
                 searchTask = searchTask
         )

vector/src/main/java/im/vector/app/features/home/room/detail/search/SearchViewModel.kt

@@ -24,26 +24,24 @@ import com.airbnb.mvrx.MvRxViewModelFactory
 import com.airbnb.mvrx.Success
 import com.airbnb.mvrx.ViewModelContext
 import dagger.assisted.Assisted
-import dagger.assisted.AssistedInject
 import dagger.assisted.AssistedFactory
+import dagger.assisted.AssistedInject
 import im.vector.app.core.extensions.exhaustive
 import im.vector.app.core.platform.VectorViewModel
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.failure.Failure
 import org.matrix.android.sdk.api.session.Session
-import org.matrix.android.sdk.api.session.room.Room
 import org.matrix.android.sdk.api.session.search.SearchResult
-import org.matrix.android.sdk.api.util.Cancelable
-import org.matrix.android.sdk.internal.util.awaitCallback
 
 class SearchViewModel @AssistedInject constructor(
         @Assisted private val initialState: SearchViewState,
         session: Session
 ) : VectorViewModel<SearchViewState, SearchAction, SearchViewEvents>(initialState) {
 
-    private var room: Room? = session.getRoom(initialState.roomId)
+    private val room = session.getRoom(initialState.roomId)
 
-    private var currentTask: Cancelable? = null
+    private var currentTask: Job? = null
 
     private var nextBatch: String? = null
 
@@ -92,6 +90,7 @@ class SearchViewModel @AssistedInject constructor(
     }
 
     private fun startSearching(isNextBatch: Boolean) = withState { state ->
+        if (room == null) return@withState
         if (state.searchTerm == null) return@withState
 
         // There is no batch to retrieve
@@ -108,20 +107,17 @@ class SearchViewModel @AssistedInject constructor(
 
         currentTask?.cancel()
 
-        viewModelScope.launch {
+        currentTask = viewModelScope.launch {
             try {
-                val result = awaitCallback<SearchResult> {
-                    currentTask = room?.search(
-                            searchTerm = state.searchTerm,
-                            nextBatch = nextBatch,
-                            orderByRecent = true,
-                            beforeLimit = 0,
-                            afterLimit = 0,
-                            includeProfile = true,
-                            limit = 20,
-                            callback = it
-                    )
-                }
+                val result = room.search(
+                        searchTerm = state.searchTerm,
+                        nextBatch = nextBatch,
+                        orderByRecent = true,
+                        beforeLimit = 0,
+                        afterLimit = 0,
+                        includeProfile = true,
+                        limit = 20
+                )
                 onSearchResultSuccess(result)
             } catch (failure: Throwable) {
                 if (failure is Failure.Cancelled) return@launch

vector/src/main/java/im/vector/app/features/widgets/WidgetAPICallback.kt

@@ -1,36 +0,0 @@
-/*
- * Copyright (c) 2020 New Vector Ltd
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package im.vector.app.features.widgets
-
-import im.vector.app.R
-import im.vector.app.core.resources.StringProvider
-import org.matrix.android.sdk.api.MatrixCallback
-import org.matrix.android.sdk.api.session.widgets.WidgetPostAPIMediator
-import org.matrix.android.sdk.api.util.JsonDict
-
-class WidgetAPICallback(private val postAPIMediator: WidgetPostAPIMediator,
-                        private val eventData: JsonDict,
-                        private val stringProvider: StringProvider) : MatrixCallback<Any> {
-
-    override fun onFailure(failure: Throwable) {
-        postAPIMediator.sendError(stringProvider.getString(R.string.widget_integration_failed_to_send_request), eventData)
-    }
-
-    override fun onSuccess(data: Any) {
-        postAPIMediator.sendSuccess(eventData)
-    }
-}

vector/src/main/java/im/vector/app/features/widgets/WidgetPostAPIHandler.kt

@@ -464,10 +464,6 @@ class WidgetPostAPIHandler @AssistedInject constructor(@Assisted private val roo
         return false
     }
 
-    private fun createWidgetAPICallback(widgetPostAPIMediator: WidgetPostAPIMediator, eventData: JsonDict): WidgetAPICallback {
-        return WidgetAPICallback(widgetPostAPIMediator, eventData, stringProvider)
-    }
-
     private fun launchWidgetAPIAction(widgetPostAPIMediator: WidgetPostAPIMediator, eventData: JsonDict, block: suspend () -> Unit): Job {
         return GlobalScope.launch {
             kotlin.runCatching {