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

Remove some usage of MatrixCallback

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/relation/RelationService.kt

@@ -66,8 +66,8 @@ interface RelationService {
      * @param targetEventId the id of the event being reacted
      * @param reaction the reaction (preferably emoji)
      */
-    fun undoReaction(targetEventId: String,
+    suspend fun undoReaction(targetEventId: String,
-                     reaction: String): Cancelable
+                             reaction: String): Cancelable
 
     /**
      * Edit a poll.

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

@@ -21,7 +21,6 @@ import com.zhuinden.monarchy.Monarchy
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
-import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.room.model.EventAnnotationsSummary
 import org.matrix.android.sdk.api.session.room.model.message.PollType
@@ -32,19 +31,15 @@ import org.matrix.android.sdk.api.util.NoOpCancellable
 import org.matrix.android.sdk.api.util.Optional
 import org.matrix.android.sdk.api.util.toOptional
 import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider
-import org.matrix.android.sdk.internal.crypto.DefaultCryptoService
 import org.matrix.android.sdk.internal.database.mapper.TimelineEventMapper
 import org.matrix.android.sdk.internal.database.mapper.asDomain
 import org.matrix.android.sdk.internal.database.model.EventAnnotationsSummaryEntity
 import org.matrix.android.sdk.internal.database.model.TimelineEventEntity
 import org.matrix.android.sdk.internal.database.query.where
 import org.matrix.android.sdk.internal.di.SessionDatabase
-import org.matrix.android.sdk.internal.di.UserId
 import org.matrix.android.sdk.internal.session.room.relation.threads.FetchThreadTimelineTask
 import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
 import org.matrix.android.sdk.internal.session.room.send.queue.EventSenderProcessor
-import org.matrix.android.sdk.internal.task.TaskExecutor
-import org.matrix.android.sdk.internal.task.configureWith
 import org.matrix.android.sdk.internal.util.fetchCopyMap
 import timber.log.Timber
 
@@ -54,15 +49,12 @@ internal class DefaultRelationService @AssistedInject constructor(
         private val eventSenderProcessor: EventSenderProcessor,
         private val eventFactory: LocalEchoEventFactory,
         private val cryptoSessionInfoProvider: CryptoSessionInfoProvider,
-        private val cryptoService: DefaultCryptoService,
         private val findReactionEventForUndoTask: FindReactionEventForUndoTask,
         private val fetchEditHistoryTask: FetchEditHistoryTask,
         private val fetchThreadTimelineTask: FetchThreadTimelineTask,
         private val timelineEventMapper: TimelineEventMapper,
-        @UserId private val userId: String,
+        @SessionDatabase private val monarchy: Monarchy
-        @SessionDatabase private val monarchy: Monarchy,
+) : RelationService {
-        private val taskExecutor: TaskExecutor) :
-        RelationService {
 
     @AssistedFactory
     interface Factory {
@@ -84,39 +76,31 @@ internal class DefaultRelationService @AssistedInject constructor(
                         .none { it.addedByMe && it.key == reaction }) {
             val event = eventFactory.createReactionEvent(roomId, targetEventId, reaction)
                     .also { saveLocalEcho(it) }
-            return eventSenderProcessor.postEvent(event, false /* reaction are not encrypted*/)
+            eventSenderProcessor.postEvent(event, false /* reaction are not encrypted*/)
         } else {
             Timber.w("Reaction already added")
             NoOpCancellable
         }
     }
 
-    override fun undoReaction(targetEventId: String, reaction: String): Cancelable {
+    override suspend fun undoReaction(targetEventId: String, reaction: String): Cancelable {
         val params = FindReactionEventForUndoTask.Params(
                 roomId,
                 targetEventId,
                 reaction
         )
-        // TODO We should avoid using MatrixCallback internally
+
-        val callback = object : MatrixCallback<FindReactionEventForUndoTask.Result> {
+        val data = findReactionEventForUndoTask.executeRetry(params, Int.MAX_VALUE)
-            override fun onSuccess(data: FindReactionEventForUndoTask.Result) {
+
-                if (data.redactEventId == null) {
+        return if (data.redactEventId == null) {
-                    Timber.w("Cannot find reaction to undo (not yet synced?)")
+            Timber.w("Cannot find reaction to undo (not yet synced?)")
-                    // TODO?
+            // TODO?
-                }
+            NoOpCancellable
-                data.redactEventId?.let { toRedact ->
+        } else {
-                    val redactEvent = eventFactory.createRedactEvent(roomId, toRedact, null)
+            val redactEvent = eventFactory.createRedactEvent(roomId, data.redactEventId, null)
-                            .also { saveLocalEcho(it) }
+                    .also { saveLocalEcho(it) }
-                    eventSenderProcessor.postRedaction(redactEvent, null)
+            eventSenderProcessor.postRedaction(redactEvent, null)
-                }
-            }
         }
-        return findReactionEventForUndoTask
-                .configureWith(params) {
-                    this.retryCount = Int.MAX_VALUE
-                    this.callback = callback
-                }
-                .executeBy(taskExecutor)
     }
 
     override fun editPoll(targetEvent: TimelineEvent,

vector/src/main/java/im/vector/app/AutoRageShaker.kt

@@ -16,7 +16,6 @@
 
 package im.vector.app
 
-import android.content.Context
 import android.content.SharedPreferences
 import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.features.rageshake.BugReporter
@@ -46,7 +45,6 @@ class AutoRageShaker @Inject constructor(
         private val sessionDataSource: ActiveSessionDataSource,
         private val activeSessionHolder: ActiveSessionHolder,
         private val bugReporter: BugReporter,
-        private val context: Context,
         private val vectorPreferences: VectorPreferences
 ) : Session.Listener, SharedPreferences.OnSharedPreferenceChangeListener {
 
@@ -136,7 +134,6 @@ class AutoRageShaker @Inject constructor(
 
     private fun sendRageShake(target: E2EMessageDetected) {
         bugReporter.sendBugReport(
-                context = context,
                 reportType = ReportType.AUTO_UISI,
                 withDevicesLogs = true,
                 withCrashLogs = true,
@@ -218,7 +215,6 @@ class AutoRageShaker @Inject constructor(
         val matchingIssue = event.content?.get("recipient_rageshake")?.toString() ?: ""
 
         bugReporter.sendBugReport(
-                context = context,
                 reportType = ReportType.AUTO_UISI_SENDER,
                 withDevicesLogs = true,
                 withCrashLogs = true,

vector/src/main/java/im/vector/app/VectorApplication.kt

@@ -120,7 +120,7 @@ class VectorApplication :
         vectorAnalytics.init()
         invitesAcceptor.initialize()
         autoRageShaker.initialize()
-        vectorUncaughtExceptionHandler.activate(this)
+        vectorUncaughtExceptionHandler.activate()
 
         // Remove Log handler statically added by Jitsi
         Timber.forest()

vector/src/main/java/im/vector/app/features/home/HomeActivity.kt

@@ -473,14 +473,14 @@ class HomeActivity :
     override fun onResume() {
         super.onResume()
 
-        if (vectorUncaughtExceptionHandler.didAppCrash(this)) {
+        if (vectorUncaughtExceptionHandler.didAppCrash()) {
-            vectorUncaughtExceptionHandler.clearAppCrashStatus(this)
+            vectorUncaughtExceptionHandler.clearAppCrashStatus()
 
             MaterialAlertDialogBuilder(this)
                     .setMessage(R.string.send_bug_report_app_crashed)
                     .setCancelable(false)
                     .setPositiveButton(R.string.yes) { _, _ -> bugReporter.openBugReportScreen(this) }
-                    .setNegativeButton(R.string.no) { _, _ -> bugReporter.deleteCrashFile(this) }
+                    .setNegativeButton(R.string.no) { _, _ -> bugReporter.deleteCrashFile() }
                     .show()
         } else {
             showDisclaimerDialog(this)

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

@@ -740,14 +740,22 @@ class TimelineViewModel @AssistedInject constructor(
     }
 
     private fun handleUndoReact(action: RoomDetailAction.UndoReaction) {
-        room.undoReaction(action.targetEventId, action.reaction)
+        viewModelScope.launch {
+            tryOrNull {
+                room.undoReaction(action.targetEventId, action.reaction)
+            }
+        }
     }
 
     private fun handleUpdateQuickReaction(action: RoomDetailAction.UpdateQuickReactAction) {
         if (action.add) {
             room.sendReaction(action.targetEventId, action.selectedReaction)
         } else {
-            room.undoReaction(action.targetEventId, action.selectedReaction)
+            viewModelScope.launch {
+                tryOrNull {
+                    room.undoReaction(action.targetEventId, action.selectedReaction)
+                }
+            }
         }
     }
 

vector/src/main/java/im/vector/app/features/rageshake/BugReportActivity.kt

@@ -151,7 +151,7 @@ class BugReportActivity : VectorBaseActivity<ActivityBugReportBinding>() {
         views.bugReportProgressView.isVisible = true
         views.bugReportProgressView.progress = 0
 
-        bugReporter.sendBugReport(this,
+        bugReporter.sendBugReport(
                 reportType,
                 views.bugReportButtonIncludeLogs.isChecked,
                 views.bugReportButtonIncludeCrashLogs.isChecked,
@@ -249,7 +249,7 @@ class BugReportActivity : VectorBaseActivity<ActivityBugReportBinding>() {
 
     override fun onBackPressed() {
         // Ensure there is no crash status remaining, which will be sent later on by mistake
-        bugReporter.deleteCrashFile(this)
+        bugReporter.deleteCrashFile()
 
         super.onBackPressed()
     }

vector/src/main/java/im/vector/app/features/rageshake/BugReporter.kt

@@ -68,6 +68,7 @@ import javax.inject.Singleton
  */
 @Singleton
 class BugReporter @Inject constructor(
+        private val context: Context,
         private val activeSessionHolder: ActiveSessionHolder,
         private val versionProvider: VersionProvider,
         private val vectorPreferences: VectorPreferences,
@@ -153,7 +154,6 @@ class BugReporter @Inject constructor(
     /**
      * Send a bug report.
      *
-     * @param context           the application context
      * @param reportType        The report type (bug, suggestion, feedback)
      * @param withDevicesLogs   true to include the device log
      * @param withCrashLogs     true to include the crash logs
@@ -163,8 +163,7 @@ class BugReporter @Inject constructor(
      * @param listener          the listener
      */
     @SuppressLint("StaticFieldLeak")
-    fun sendBugReport(context: Context,
+    fun sendBugReport(reportType: ReportType,
-                      reportType: ReportType,
                       withDevicesLogs: Boolean,
                       withCrashLogs: Boolean,
                       withKeyRequestHistory: Boolean,
@@ -182,7 +181,7 @@ class BugReporter @Inject constructor(
             var reportURL: String? = null
             withContext(Dispatchers.IO) {
                 var bugDescription = theBugDescription
-                val crashCallStack = getCrashDescription(context)
+                val crashCallStack = getCrashDescription()
 
                 if (null != crashCallStack) {
                     bugDescription += "\n\n\n\n--------------------------------- crash call stack ---------------------------------\n"
@@ -203,7 +202,7 @@ class BugReporter @Inject constructor(
                 }
 
                 if (!mIsCancelled && (withCrashLogs || withDevicesLogs)) {
-                    val gzippedLogcat = saveLogCat(context, false)
+                    val gzippedLogcat = saveLogCat(false)
 
                     if (null != gzippedLogcat) {
                         if (gzippedFiles.size == 0) {
@@ -213,7 +212,7 @@ class BugReporter @Inject constructor(
                         }
                     }
 
-                    val crashDescription = getCrashFile(context)
+                    val crashDescription = getCrashFile()
                     if (crashDescription.exists()) {
                         val compressedCrashDescription = compressFile(crashDescription)
 
@@ -265,7 +264,7 @@ class BugReporter @Inject constructor(
                     // build the multi part request
                     val builder = BugReporterMultipartBody.Builder()
                             .addFormDataPart("text", text)
-                            .addFormDataPart("app", rageShakeAppNameForReport(context, reportType))
+                            .addFormDataPart("app", rageShakeAppNameForReport(reportType))
                             .addFormDataPart("user_agent", Matrix.getInstance(context).getUserAgent())
                             .addFormDataPart("user_id", userId)
                             .addFormDataPart("can_contact", canContact.toString())
@@ -352,9 +351,9 @@ class BugReporter @Inject constructor(
                         }
                     }
 
-                    if (getCrashFile(context).exists()) {
+                    if (getCrashFile().exists()) {
                         builder.addFormDataPart("label", "crash")
-                        deleteCrashFile(context)
+                        deleteCrashFile()
                     }
 
                     val requestBody = builder.build()
@@ -487,20 +486,16 @@ class BugReporter @Inject constructor(
         activity.startActivity(BugReportActivity.intent(activity, reportType))
     }
 
-    private fun rageShakeAppNameForReport(context: Context, reportType: ReportType): String {
+    private fun rageShakeAppNameForReport(reportType: ReportType): String {
         // As per https://github.com/matrix-org/rageshake
         // app: Identifier for the application (eg 'riot-web').
         // Should correspond to a mapping configured in the configuration file for github issue reporting to work.
         // (see R.string.bug_report_url for configured RS server)
-        return when (reportType) {
+        return context.getString(when (reportType) {
             ReportType.AUTO_UISI_SENDER,
-            ReportType.AUTO_UISI -> {
+            ReportType.AUTO_UISI -> R.string.bug_report_auto_uisi_app_name
-                context.getString(R.string.bug_report_auto_uisi_app_name)
+            else                 -> R.string.bug_report_app_name
-            }
+        })
-            else                 -> {
-               context.getString(R.string.bug_report_app_name)
-            }
-        }
     }
 // ==============================================================================================================
 // crash report management
@@ -509,20 +504,17 @@ class BugReporter @Inject constructor(
     /**
      * Provides the crash file
      *
-     * @param context the context
      * @return the crash file
      */
-    private fun getCrashFile(context: Context): File {
+    private fun getCrashFile(): File {
         return File(context.cacheDir.absolutePath, CRASH_FILENAME)
     }
 
     /**
      * Remove the crash file
-     *
-     * @param context
      */
-    fun deleteCrashFile(context: Context) {
+    fun deleteCrashFile() {
-        val crashFile = getCrashFile(context)
+        val crashFile = getCrashFile()
 
         if (crashFile.exists()) {
             crashFile.delete()
@@ -535,11 +527,10 @@ class BugReporter @Inject constructor(
     /**
      * Save the crash report
      *
-     * @param context          the context
      * @param crashDescription teh crash description
      */
-    fun saveCrashReport(context: Context, crashDescription: String) {
+    fun saveCrashReport(crashDescription: String) {
-        val crashFile = getCrashFile(context)
+        val crashFile = getCrashFile()
 
         if (crashFile.exists()) {
             crashFile.delete()
@@ -557,11 +548,10 @@ class BugReporter @Inject constructor(
     /**
      * Read the crash description file and return its content.
      *
-     * @param context teh context
      * @return the crash description
      */
-    private fun getCrashDescription(context: Context): String? {
+    private fun getCrashDescription(): String? {
-        val crashFile = getCrashFile(context)
+        val crashFile = getCrashFile()
 
         if (crashFile.exists()) {
             try {
@@ -650,11 +640,10 @@ class BugReporter @Inject constructor(
     /**
      * Save the logcat
      *
-     * @param context       the context
      * @param isErrorLogcat true to save the error logcat
      * @return the file if the operation succeeds
      */
-    private fun saveLogCat(context: Context, isErrorLogcat: Boolean): File? {
+    private fun saveLogCat(isErrorLogcat: Boolean): File? {
         val logCatErrFile = File(context.cacheDir.absolutePath, if (isErrorLogcat) LOG_CAT_ERROR_FILENAME else LOG_CAT_FILENAME)
 
         if (logCatErrFile.exists()) {

vector/src/main/java/im/vector/app/features/rageshake/VectorUncaughtExceptionHandler.kt

@@ -30,9 +30,12 @@ import javax.inject.Inject
 import javax.inject.Singleton
 
 @Singleton
-class VectorUncaughtExceptionHandler @Inject constructor(private val bugReporter: BugReporter,
+class VectorUncaughtExceptionHandler @Inject constructor(
-                                                         private val versionProvider: VersionProvider,
+        context: Context,
-                                                         private val versionCodeProvider: VersionCodeProvider) : Thread.UncaughtExceptionHandler {
+        private val bugReporter: BugReporter,
+        private val versionProvider: VersionProvider,
+        private val versionCodeProvider: VersionCodeProvider
+) : Thread.UncaughtExceptionHandler {
 
     // key to save the crash status
     companion object {
@@ -41,13 +44,12 @@ class VectorUncaughtExceptionHandler @Inject constructor(private val bugReporter
 
     private var previousHandler: Thread.UncaughtExceptionHandler? = null
 
-    private lateinit var context: Context
+    private val preferences = DefaultSharedPreferences.getInstance(context)
 
     /**
      * Activate this handler
      */
-    fun activate(context: Context) {
+    fun activate() {
-        this.context = context
         previousHandler = Thread.getDefaultUncaughtExceptionHandler()
         Thread.setDefaultUncaughtExceptionHandler(this)
     }
@@ -61,7 +63,7 @@ class VectorUncaughtExceptionHandler @Inject constructor(private val bugReporter
      */
     override fun uncaughtException(thread: Thread, throwable: Throwable) {
         Timber.v("Uncaught exception: $throwable")
-        DefaultSharedPreferences.getInstance(context).edit {
+        preferences.edit {
             putBoolean(PREFS_CRASH_KEY, true)
         }
         val b = StringBuilder()
@@ -103,7 +105,7 @@ class VectorUncaughtExceptionHandler @Inject constructor(private val bugReporter
         val bugDescription = b.toString()
         Timber.e("FATAL EXCEPTION $bugDescription")
 
-        bugReporter.saveCrashReport(context, bugDescription)
+        bugReporter.saveCrashReport(bugDescription)
 
         // Show the classical system popup
         previousHandler?.uncaughtException(thread, throwable)
@@ -114,16 +116,15 @@ class VectorUncaughtExceptionHandler @Inject constructor(private val bugReporter
      *
      * @return true if the application crashed
      */
-    fun didAppCrash(context: Context): Boolean {
+    fun didAppCrash(): Boolean {
-        return DefaultSharedPreferences.getInstance(context)
+        return preferences.getBoolean(PREFS_CRASH_KEY, false)
-                .getBoolean(PREFS_CRASH_KEY, false)
     }
 
     /**
      * Clear the crash status
      */
-    fun clearAppCrashStatus(context: Context) {
+    fun clearAppCrashStatus() {
-        DefaultSharedPreferences.getInstance(context).edit {
+        preferences.edit {
             remove(PREFS_CRASH_KEY)
         }
     }