From cefdbe1d083bbb7bd612260efa44b46b8fdf7808 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 26 Aug 2020 19:21:14 +0200 Subject: [PATCH 1/4] Add CheckNumberType in json to fix sending in room v6 --- .../network/parsing/CheckNumberType.kt | 66 +++++++++++++++++++ .../internal/worker/WorkerParamsFactory.kt | 15 ++++- 2 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/parsing/CheckNumberType.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/parsing/CheckNumberType.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/parsing/CheckNumberType.kt new file mode 100644 index 0000000000..d4ceca2006 --- /dev/null +++ b/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?, moshi: Moshi): JsonAdapter<*>? { + if (type !== Any::class.java) { + return null + } + val delegate: JsonAdapter = moshi.nextAdapter(this, Any::class.java, emptySet()) + return object : JsonAdapter() { + @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) + } + } + } + } + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/WorkerParamsFactory.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/WorkerParamsFactory.kt index 2b7cba0b0c..b162566403 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/WorkerParamsFactory.kt +++ b/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 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) } From dc4135b506139042d4eed14bf6ddac89f55a0a20 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 26 Aug 2020 19:21:41 +0200 Subject: [PATCH 2/4] Remove unnecessary code now we have an other way to keep number types --- .../internal/session/room/send/SendEventWorker.kt | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt index 2868ce29c1..d37b5e90c2 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt +++ b/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,12 +47,10 @@ 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 val event: Event? = null, val eventId: String? = null, val roomId: String? = null, val type: String? = null, - val contentStr: String? = null, override val lastFailureMessage: String? = null ) : SessionWorkerParams { @@ -61,7 +59,7 @@ internal class SendEventWorker(context: Context, eventId = event.eventId, roomId = event.roomId, type = event.type, - contentStr = ContentMapper.map(event.content), + event = event, lastFailureMessage = lastFailureMessage ) } @@ -91,7 +89,7 @@ internal class SendEventWorker(context: Context, .also { Timber.e("Work cancelled due to input error from parent") } } return try { - sendEvent(params.eventId, params.roomId, params.type, params.contentStr) + sendEvent(params.eventId, params.roomId, params.type, params.event?.content) Result.success() } catch (exception: Throwable) { // It does start from 0, we want it to stop if it fails the third time @@ -105,10 +103,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(eventBus) { - apiCall = roomAPI.send(eventId, roomId, type, contentStr) + apiCall = roomAPI.send(eventId, roomId, type, content) } localEchoRepository.updateSendState(eventId, SendState.SENT) } From 3c0177c2dd1a42f76380a533e5b4348d3e2f289b Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 26 Aug 2020 19:25:20 +0200 Subject: [PATCH 3/4] Update CHANGES --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index c923053b24..5d58668790 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,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 🗣: - From 9c1c9f96e192953e129198aecf6dde436164b216 Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 27 Aug 2020 10:34:40 +0200 Subject: [PATCH 4/4] Room v6: finish cleaning up --- matrix-sdk-android/build.gradle | 2 - .../sdk/internal/network/RetrofitFactory.kt | 4 +- .../sdk/internal/session/room/RoomAPI.kt | 15 -------- .../room/relation/DefaultRelationService.kt | 2 +- .../session/room/send/EncryptEventWorker.kt | 9 +++-- .../MultipleEventSendingDispatcherWorker.kt | 2 +- .../session/room/send/RoomEventSender.kt | 2 +- .../session/room/send/SendEventWorker.kt | 37 ++++++++----------- 8 files changed, 25 insertions(+), 48 deletions(-) diff --git a/matrix-sdk-android/build.gradle b/matrix-sdk-android/build.gradle index d70b0bbe84..90bdf02243 100644 --- a/matrix-sdk-android/build.gradle +++ b/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' diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/RetrofitFactory.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/RetrofitFactory.kt index 368611dd7d..89a0ce597a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/RetrofitFactory.kt +++ b/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() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomAPI.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomAPI.kt index 25dcc69fa8..d8df86be8f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomAPI.kt +++ b/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 - /** - * 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 - /** * Get the context surrounding an event. * diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt index 2199193de0..111551d66d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt +++ b/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(sendWorkData, startChain) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/EncryptEventWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/EncryptEventWorker.kt index d23835e838..f878df52b2 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/EncryptEventWorker.kt +++ b/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 - ?: "Error") + val nextWorkerParams = SendEventWorker.Params( + sessionId = params.sessionId, + event = localEvent, + lastFailureMessage = error?.localizedMessage ?: "Error" + ) return Result.success(WorkerParamsFactory.toData(nextWorkerParams)) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MultipleEventSendingDispatcherWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MultipleEventSendingDispatcherWorker.kt index 73791e8412..ead2dc9377 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MultipleEventSendingDispatcherWorker.kt +++ b/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(sendWorkData, startChain) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/RoomEventSender.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/RoomEventSender.kt index 65c692f42e..e46adeb9c1 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/RoomEventSender.kt +++ b/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(sendWorkData, startChain) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt index d37b5e90c2..5da14f0a41 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt @@ -47,22 +47,11 @@ internal class SendEventWorker(context: Context, @JsonClass(generateAdapter = true) internal data class Params( override val sessionId: String, + override val lastFailureMessage: String? = null, val event: Event? = null, - val eventId: String? = null, - val roomId: String? = null, - val type: 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, - event = event, - lastFailureMessage = lastFailureMessage - ) - } + // Keep for compat at the moment, will be removed later + val eventId: String? = null + ) : SessionWorkerParams @Inject lateinit var localEchoRepository: LocalEchoRepository @Inject lateinit var roomAPI: RoomAPI @@ -75,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 - if (params.event?.eventId != null) { - localEchoRepository.updateSendState(params.event.eventId, SendState.UNDELIVERED) + + val event = params.event + if (event?.eventId == null || event.roomId == null) { + // 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.event?.content) + 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()