Merge pull request #2010 from vector-im/feature/fix_send_room_v6

Feature/fix send room v6

CHANGES.md

@@ -15,6 +15,7 @@ Bugfix 🐛:
  - Fix FontSize issue (#1483, #1787)
  - Fix bad color for settings icon on Android < 24 (#1786)
  - Change user or room avatar: when selecting Gallery, I'm not proposed to crop the selected image (#1590)
+ - Fix uploads still don't work with room v6 (#1879)
 
 Translations 🗣:
  -

matrix-sdk-android/build.gradle

@@ -131,8 +131,6 @@ dependencies {
     // Network
     implementation "com.squareup.retrofit2:retrofit:$retrofit_version"
     implementation "com.squareup.retrofit2:converter-moshi:$retrofit_version"
-    implementation "com.squareup.retrofit2:converter-scalars:$retrofit_version"
-
 
     implementation(platform("com.squareup.okhttp3:okhttp-bom:4.8.1"))
     implementation 'com.squareup.okhttp3:okhttp'

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/RetrofitFactory.kt

@@ -19,13 +19,12 @@ package org.matrix.android.sdk.internal.network
 
 import com.squareup.moshi.Moshi
 import dagger.Lazy
-import org.matrix.android.sdk.internal.util.ensureTrailingSlash
 import okhttp3.Call
 import okhttp3.OkHttpClient
 import okhttp3.Request
+import org.matrix.android.sdk.internal.util.ensureTrailingSlash
 import retrofit2.Retrofit
 import retrofit2.converter.moshi.MoshiConverterFactory
-import retrofit2.converter.scalars.ScalarsConverterFactory
 import javax.inject.Inject
 
 internal class RetrofitFactory @Inject constructor(private val moshi: Moshi) {
@@ -50,7 +49,6 @@ internal class RetrofitFactory @Inject constructor(private val moshi: Moshi) {
                         return okHttpClient.get().newCall(request)
                     }
                 })
-                .addConverterFactory(ScalarsConverterFactory.create())
                 .addConverterFactory(UnitConverterFactory)
                 .addConverterFactory(MoshiConverterFactory.create(moshi))
                 .build()

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/parsing/CheckNumberType.kt

@@ -0,0 +1,66 @@
+/*
+ * 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 org.matrix.android.sdk.internal.network.parsing
+
+import androidx.annotation.Nullable
+import com.squareup.moshi.JsonAdapter
+import com.squareup.moshi.JsonReader
+import com.squareup.moshi.JsonWriter
+
+import com.squareup.moshi.Moshi
+import java.io.IOException
+import java.lang.reflect.Type
+import java.math.BigDecimal
+
+/**
+ * This is used to check if NUMBER in json is integer or double, so we can preserve typing when serializing/deserializing in a row.
+ */
+interface CheckNumberType {
+
+    companion object {
+        val JSON_ADAPTER_FACTORY = object : JsonAdapter.Factory {
+            @Nullable
+            override fun create(type: Type, annotations: Set<Annotation?>?, moshi: Moshi): JsonAdapter<*>? {
+                if (type !== Any::class.java) {
+                    return null
+                }
+                val delegate: JsonAdapter<Any> = moshi.nextAdapter(this, Any::class.java, emptySet())
+                return object : JsonAdapter<Any?>() {
+                    @Nullable
+                    @Throws(IOException::class)
+                    override fun fromJson(reader: JsonReader): Any? {
+                        return if (reader.peek() !== JsonReader.Token.NUMBER) {
+                            delegate.fromJson(reader)
+                        } else {
+                            val numberAsString = reader.nextString()
+                            val decimal = BigDecimal(numberAsString)
+                            if (decimal.scale() <= 0) {
+                                decimal.intValueExact()
+                            } else {
+                                decimal.toDouble()
+                            }
+                        }
+                    }
+
+                    override fun toJson(writer: JsonWriter, value: Any?) {
+                        delegate.toJson(writer, value)
+                    }
+                }
+            }
+        }
+    }
+}

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

@@ -130,21 +130,6 @@ internal interface RoomAPI {
              @Body content: Content?
     ): Call<SendResponse>
 
-    /**
-     * Send an event to a room.
-     *
-     * @param txId      the transaction Id
-     * @param roomId    the room id
-     * @param eventType the event type
-     * @param content   the event content as string
-     */
-    @PUT(NetworkConstants.URI_API_PREFIX_PATH_R0 + "rooms/{roomId}/send/{eventType}/{txId}")
-    fun send(@Path("txId") txId: String,
-             @Path("roomId") roomId: String,
-             @Path("eventType") eventType: String,
-             @Body content: String?
-    ): Call<SendResponse>
-
     /**
      * Get the context surrounding an event.
      *

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

@@ -208,7 +208,7 @@ internal class DefaultRelationService @AssistedInject constructor(
     }
 
     private fun createSendEventWork(event: Event, startChain: Boolean): OneTimeWorkRequest {
-        val sendContentWorkerParams = SendEventWorker.Params(sessionId, event)
+        val sendContentWorkerParams = SendEventWorker.Params(sessionId = sessionId, event = event)
         val sendWorkData = WorkerParamsFactory.toData(sendContentWorkerParams)
         return timeLineSendEventWorkCommon.createWork<SendEventWorker>(sendWorkData, startChain)
     }

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

@@ -120,7 +120,7 @@ internal class EncryptEventWorker(context: Context, params: WorkerParameters)
                 localEchoRepository.updateEncryptedEcho(localEvent.eventId, safeResult.eventContent, decryptionLocalEcho)
             }
 
-            val nextWorkerParams = SendEventWorker.Params(params.sessionId, encryptedEvent)
+            val nextWorkerParams = SendEventWorker.Params(sessionId = params.sessionId, event = encryptedEvent)
             return Result.success(WorkerParamsFactory.toData(nextWorkerParams))
         } else {
             val sendState = when (error) {
@@ -129,8 +129,11 @@ internal class EncryptEventWorker(context: Context, params: WorkerParameters)
             }
             localEchoRepository.updateSendState(localEvent.eventId, sendState)
             // always return success, or the chain will be stuck for ever!
-            val nextWorkerParams = SendEventWorker.Params(params.sessionId, localEvent, error?.localizedMessage
+            val nextWorkerParams = SendEventWorker.Params(
-                    ?: "Error")
+                    sessionId = params.sessionId,
+                    event = localEvent,
+                    lastFailureMessage = error?.localizedMessage ?: "Error"
+            )
             return Result.success(WorkerParamsFactory.toData(nextWorkerParams))
         }
     }

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

@@ -105,7 +105,7 @@ internal class MultipleEventSendingDispatcherWorker(context: Context, params: Wo
     }
 
     private fun createSendEventWork(sessionId: String, event: Event, startChain: Boolean): OneTimeWorkRequest {
-        val sendContentWorkerParams = SendEventWorker.Params(sessionId, event)
+        val sendContentWorkerParams = SendEventWorker.Params(sessionId = sessionId, event = event)
         val sendWorkData = WorkerParamsFactory.toData(sendContentWorkerParams)
 
         return timelineSendEventWorkCommon.createWork<SendEventWorker>(sendWorkData, startChain)

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

@@ -65,7 +65,7 @@ internal class RoomEventSender @Inject constructor(
     }
 
     private fun createSendEventWork(event: Event, startChain: Boolean): OneTimeWorkRequest {
-        val sendContentWorkerParams = SendEventWorker.Params(sessionId, event)
+        val sendContentWorkerParams = SendEventWorker.Params(sessionId = sessionId, event = event)
         val sendWorkData = WorkerParamsFactory.toData(sendContentWorkerParams)
 
         return timelineSendEventWorkCommon.createWork<SendEventWorker>(sendWorkData, startChain)

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

@@ -21,16 +21,16 @@ import android.content.Context
 import androidx.work.CoroutineWorker
 import androidx.work.WorkerParameters
 import com.squareup.moshi.JsonClass
+import org.greenrobot.eventbus.EventBus
 import org.matrix.android.sdk.api.failure.shouldBeRetried
+import org.matrix.android.sdk.api.session.events.model.Content
 import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.room.send.SendState
-import org.matrix.android.sdk.internal.database.mapper.ContentMapper
 import org.matrix.android.sdk.internal.network.executeRequest
 import org.matrix.android.sdk.internal.session.room.RoomAPI
 import org.matrix.android.sdk.internal.worker.SessionWorkerParams
 import org.matrix.android.sdk.internal.worker.WorkerParamsFactory
 import org.matrix.android.sdk.internal.worker.getSessionComponent
-import org.greenrobot.eventbus.EventBus
 import timber.log.Timber
 import javax.inject.Inject
 
@@ -47,24 +47,11 @@ internal class SendEventWorker(context: Context,
     @JsonClass(generateAdapter = true)
     internal data class Params(
             override val sessionId: String,
-            // TODO remove after some time, it's used for compat
+            override val lastFailureMessage: String? = null,
             val event: Event? = null,
-            val eventId: String? = null,
+            // Keep for compat at the moment, will be removed later
-            val roomId: String? = null,
+            val eventId: String? = null
-            val type: String? = null,
+    ) : SessionWorkerParams
-            val contentStr: String? = null,
-            override val lastFailureMessage: String? = null
-    ) : SessionWorkerParams {
-
-        constructor(sessionId: String, event: Event, lastFailureMessage: String? = null) : this(
-                sessionId = sessionId,
-                eventId = event.eventId,
-                roomId = event.roomId,
-                type = event.type,
-                contentStr = ContentMapper.map(event.content),
-                lastFailureMessage = lastFailureMessage
-        )
-    }
 
     @Inject lateinit var localEchoRepository: LocalEchoRepository
     @Inject lateinit var roomAPI: RoomAPI
@@ -77,27 +64,31 @@ internal class SendEventWorker(context: Context,
 
         val sessionComponent = getSessionComponent(params.sessionId) ?: return Result.success()
         sessionComponent.inject(this)
-        if (params.eventId == null || params.roomId == null || params.type == null) {
+
-            // compat with old params, make it fail if any
+        val event = params.event
-            if (params.event?.eventId != null) {
+        if (event?.eventId == null || event.roomId == null) {
-                localEchoRepository.updateSendState(params.event.eventId, SendState.UNDELIVERED)
+            // Old way of sending
+            if (params.eventId != null) {
+                localEchoRepository.updateSendState(params.eventId, SendState.UNDELIVERED)
             }
             return Result.success()
+                    .also { Timber.e("Work cancelled due to bad input data") }
         }
+
         if (params.lastFailureMessage != null) {
-            localEchoRepository.updateSendState(params.eventId, SendState.UNDELIVERED)
+            localEchoRepository.updateSendState(event.eventId, SendState.UNDELIVERED)
             // Transmit the error
             return Result.success(inputData)
                     .also { Timber.e("Work cancelled due to input error from parent") }
         }
         return try {
-            sendEvent(params.eventId, params.roomId, params.type, params.contentStr)
+            sendEvent(event.eventId, event.roomId, event.type, event.content)
             Result.success()
         } catch (exception: Throwable) {
             // It does start from 0, we want it to stop if it fails the third time
             val currentAttemptCount = runAttemptCount + 1
             if (currentAttemptCount >= MAX_NUMBER_OF_RETRY_BEFORE_FAILING || !exception.shouldBeRetried()) {
-                localEchoRepository.updateSendState(params.eventId, SendState.UNDELIVERED)
+                localEchoRepository.updateSendState(event.eventId, SendState.UNDELIVERED)
                 return Result.success()
             } else {
                 Result.retry()
@@ -105,10 +96,10 @@ internal class SendEventWorker(context: Context,
         }
     }
 
-    private suspend fun sendEvent(eventId: String, roomId: String, type: String, contentStr: String?) {
+    private suspend fun sendEvent(eventId: String, roomId: String, type: String, content: Content?) {
         localEchoRepository.updateSendState(eventId, SendState.SENDING)
         executeRequest<SendResponse>(eventBus) {
-            apiCall = roomAPI.send(eventId, roomId, type, contentStr)
+            apiCall = roomAPI.send(eventId, roomId, type, content)
         }
         localEchoRepository.updateSendState(eventId, SendState.SENT)
     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/WorkerParamsFactory.kt

@@ -19,13 +19,23 @@ package org.matrix.android.sdk.internal.worker
 
 import androidx.work.Data
 import org.matrix.android.sdk.internal.di.MoshiProvider
+import org.matrix.android.sdk.internal.network.parsing.CheckNumberType
 
-object WorkerParamsFactory {
+internal object WorkerParamsFactory {
+
+    val moshi by lazy {
+        // We are adding the CheckNumberType as we are serializing/deserializing multiple time in a row
+        // and we lost typing information doing so.
+        // We don't want this check to be done on all adapters, so we just add it here.
+        MoshiProvider.providesMoshi()
+                .newBuilder()
+                .add(CheckNumberType.JSON_ADAPTER_FACTORY)
+                .build()
+    }
 
     const val KEY = "WORKER_PARAMS_JSON"
 
     inline fun <reified T> toData(params: T): Data {
-        val moshi = MoshiProvider.providesMoshi()
         val adapter = moshi.adapter(T::class.java)
         val json = adapter.toJson(params)
         return Data.Builder().putString(KEY, json).build()
@@ -36,7 +46,6 @@ object WorkerParamsFactory {
         return if (json == null) {
             null
         } else {
-            val moshi = MoshiProvider.providesMoshi()
             val adapter = moshi.adapter(T::class.java)
             adapter.fromJson(json)
         }