From 424fd1347d4c916581e13954d8e62f18af6836b1 Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 3 Jun 2019 18:23:40 +0200 Subject: [PATCH] Code review --- .../database/query/EventEntityQueries.kt | 28 +---------- .../room/EventRelationsAggregationTask.kt | 46 +++++++++---------- .../room/send/LocalEchoEventFactory.kt | 21 ++++----- 3 files changed, 34 insertions(+), 61 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/EventEntityQueries.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/EventEntityQueries.kt index c366ff75..719407ca 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/EventEntityQueries.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/EventEntityQueries.kt @@ -16,7 +16,6 @@ package im.vector.matrix.android.internal.database.query -import im.vector.matrix.android.api.session.room.send.SendState import im.vector.matrix.android.internal.database.model.ChunkEntity import im.vector.matrix.android.internal.database.model.EventEntity import im.vector.matrix.android.internal.database.model.EventEntity.LinkFilterMode.* @@ -43,35 +42,12 @@ internal fun EventEntity.Companion.where(realm: Realm, query.equalTo(EventEntityFields.TYPE, type) } return when (linkFilterMode) { - LINKED_ONLY -> query.equalTo(EventEntityFields.IS_UNLINKED, false) + LINKED_ONLY -> query.equalTo(EventEntityFields.IS_UNLINKED, false) UNLINKED_ONLY -> query.equalTo(EventEntityFields.IS_UNLINKED, true) - BOTH -> query + BOTH -> query } } -//internal fun EventEntity.Companion.unsent(realm: Realm, -// roomId: String? = null): RealmQuery { -// val query = realm.where() -// if (roomId != null) { -// query.equalTo(EventEntityFields.ROOM_ID, roomId) -// } -// query.equalTo(EventEntityFields.SEND_STATE_STR, SendState.UNSENT.name) -// return query -//} -// -//internal fun EventEntity.Companion.byTypes(realm: Realm, -// types: List): RealmQuery { -// val query = realm.where() -// types.forEachIndexed { index, type -> -// if (index == 0) { -// query.equalTo(EventEntityFields.TYPE, type) -// } else { -// query.or().equalTo(EventEntityFields.TYPE, type) -// } -// } -// return query -//} - internal fun EventEntity.Companion.latestEvent(realm: Realm, roomId: String, diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt index 5181bb30..4ff661e2 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt @@ -50,19 +50,19 @@ internal class DefaultEventRelationsAggregationTask(private val monarchy: Monarc } } - fun update(realm: Realm, events: List>?, userId: String) { - events?.forEach { pair -> + private fun update(realm: Realm, events: List>, userId: String) { + events.forEach { pair -> val roomId = pair.first.roomId ?: return@forEach val event = pair.first val sendState = pair.second val isLocalEcho = sendState == SendState.UNSENT when (event.type) { - EventType.REACTION -> { + EventType.REACTION -> { //we got a reaction!! Timber.v("###REACTION in room $roomId") handleReaction(event, roomId, realm, userId, isLocalEcho) } - EventType.MESSAGE -> { + EventType.MESSAGE -> { if (event.unsignedData?.relations?.annotations != null) { Timber.v("###REACTION Agreggation in room $roomId for event ${event.eventId}") handleInitialAggregatedRelations(event, roomId, event.unsignedData.relations.annotations, realm) @@ -80,7 +80,7 @@ internal class DefaultEventRelationsAggregationTask(private val monarchy: Monarc val eventToPrune = event.redacts?.let { EventEntity.where(realm, eventId = it).findFirst() } ?: return when (eventToPrune.type) { - EventType.MESSAGE -> { + EventType.MESSAGE -> { Timber.d("REDACTION for message ${eventToPrune.eventId}") val unsignedData = EventMapper.map(eventToPrune).unsignedData ?: UnsignedData(null, null) @@ -174,7 +174,7 @@ internal class DefaultEventRelationsAggregationTask(private val monarchy: Monarc } } - fun handleReaction(event: Event, roomId: String, realm: Realm, userId: String, isLocalEcho: Boolean) { + private fun handleReaction(event: Event, roomId: String, realm: Realm, userId: String, isLocalEcho: Boolean) { event.content.toModel()?.let { content -> //rel_type must be m.annotation if (RelationType.ANNOTATION == content.relatesTo?.type) { @@ -236,7 +236,7 @@ internal class DefaultEventRelationsAggregationTask(private val monarchy: Monarc /** * Called when an event is deleted */ - fun handleRedactionOfReplace(redacted: EventEntity, relatedEventId: String, realm: Realm) { + private fun handleRedactionOfReplace(redacted: EventEntity, relatedEventId: String, realm: Realm) { Timber.d("Handle redaction of m.replace") val eventSummary = EventAnnotationsSummaryEntity.where(realm, relatedEventId).findFirst() if (eventSummary == null) { @@ -270,40 +270,40 @@ internal class DefaultEventRelationsAggregationTask(private val monarchy: Monarc } fun handleReactionRedact(eventToPrune: EventEntity, realm: Realm, userId: String) { - Timber.d("REDACTION of reaction ${eventToPrune.eventId}") + Timber.v("REDACTION of reaction ${eventToPrune.eventId}") //delete a reaction, need to update the annotation summary if any val reactionContent: ReactionContent = EventMapper.map(eventToPrune).content.toModel() ?: return val eventThatWasReacted = reactionContent.relatesTo?.eventId ?: return - val reactionkey = reactionContent.relatesTo.key - Timber.d("REMOVE reaction for key $reactionkey") + val reactionKey = reactionContent.relatesTo.key + Timber.v("REMOVE reaction for key $reactionKey") val summary = EventAnnotationsSummaryEntity.where(realm, eventThatWasReacted).findFirst() if (summary != null) { summary.reactionsSummary.where() - .equalTo(ReactionAggregatedSummaryEntityFields.KEY, reactionkey) - .findFirst()?.let { summary -> - Timber.d("Find summary for key with ${summary.sourceEvents.size} known reactions (count:${summary.count})") - Timber.d("Known reactions ${summary.sourceEvents.joinToString(",")}") - if (summary.sourceEvents.contains(eventToPrune.eventId)) { - Timber.d("REMOVE reaction for key $reactionkey") - summary.sourceEvents.remove(eventToPrune.eventId) - Timber.d("Known reactions after ${summary.sourceEvents.joinToString(",")}") - summary.count = summary.count - 1 + .equalTo(ReactionAggregatedSummaryEntityFields.KEY, reactionKey) + .findFirst()?.let { aggregation -> + Timber.v("Find summary for key with ${aggregation.sourceEvents.size} known reactions (count:${aggregation.count})") + Timber.v("Known reactions ${aggregation.sourceEvents.joinToString(",")}") + if (aggregation.sourceEvents.contains(eventToPrune.eventId)) { + Timber.v("REMOVE reaction for key $reactionKey") + aggregation.sourceEvents.remove(eventToPrune.eventId) + Timber.v("Known reactions after ${aggregation.sourceEvents.joinToString(",")}") + aggregation.count = aggregation.count - 1 if (eventToPrune.sender == userId) { //Was it a redact on my reaction? - summary.addedByMe = false + aggregation.addedByMe = false } - if (summary.count == 0) { + if (aggregation.count == 0) { //delete! - summary.deleteFromRealm() + aggregation.deleteFromRealm() } } else { Timber.e("## Cannot remove summary from count, corresponding reaction ${eventToPrune.eventId} is not known") } } } else { - Timber.e("## Cannot find summary for key $reactionkey") + Timber.e("## Cannot find summary for key $reactionKey") } } } \ No newline at end of file diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt index 582862d6..e54af1ca 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt @@ -17,7 +17,6 @@ package im.vector.matrix.android.internal.session.room.send import android.media.MediaMetadataRetriever -import android.text.TextUtils import com.zhuinden.monarchy.Monarchy import im.vector.matrix.android.R import im.vector.matrix.android.api.auth.data.Credentials @@ -39,6 +38,7 @@ import im.vector.matrix.android.internal.util.StringProvider import im.vector.matrix.android.internal.util.tryTransactionAsync import org.commonmark.parser.Parser import org.commonmark.renderer.html.HtmlRenderer +import java.util.* /** * Creates local echo of events for room events. @@ -57,7 +57,7 @@ internal class LocalEchoEventFactory(private val credentials: Credentials, priva val document = parser.parse(text) val renderer = HtmlRenderer.builder().build() val htmlText = renderer.render(document) - if (isFormattedTextPertinent(text, htmlText)) { //FIX that + if (isFormattedTextPertinent(text, htmlText)) { //FIXME return createFormattedTextEvent(roomId, text, htmlText) } } @@ -114,7 +114,7 @@ internal class LocalEchoEventFactory(private val credentials: Credentials, priva ContentAttachmentData.Type.IMAGE -> createImageEvent(roomId, attachment) ContentAttachmentData.Type.VIDEO -> createVideoEvent(roomId, attachment) ContentAttachmentData.Type.AUDIO -> createAudioEvent(roomId, attachment) - ContentAttachmentData.Type.FILE -> createFileEvent(roomId, attachment) + ContentAttachmentData.Type.FILE -> createFileEvent(roomId, attachment) } } @@ -234,7 +234,7 @@ internal class LocalEchoEventFactory(private val credentials: Credentials, priva } private fun dummyEventId(roomId: String): String { - return "m.${txNCounter++}" + return "m.${UUID.randomUUID()}" } fun createReplyTextEvent(roomId: String, eventReplied: Event, replyText: String): Event? { @@ -299,11 +299,11 @@ internal class LocalEchoEventFactory(private val credentials: Credentials, priva } return content.body to formattedText } - MessageType.MSGTYPE_FILE -> return stringProvider.getString(R.string.reply_to_a_file) to null - MessageType.MSGTYPE_AUDIO -> return stringProvider.getString(R.string.reply_to_an_audio_file) to null - MessageType.MSGTYPE_IMAGE -> return stringProvider.getString(R.string.reply_to_an_image) to null - MessageType.MSGTYPE_VIDEO -> return stringProvider.getString(R.string.reply_to_a_video) to null - else -> return (content?.body ?: "") to null + MessageType.MSGTYPE_FILE -> return stringProvider.getString(R.string.reply_to_a_file) to null + MessageType.MSGTYPE_AUDIO -> return stringProvider.getString(R.string.reply_to_an_audio_file) to null + MessageType.MSGTYPE_IMAGE -> return stringProvider.getString(R.string.reply_to_an_image) to null + MessageType.MSGTYPE_VIDEO -> return stringProvider.getString(R.string.reply_to_a_video) to null + else -> return (content?.body ?: "") to null } @@ -350,7 +350,4 @@ internal class LocalEchoEventFactory(private val credentials: Credentials, priva } } - companion object { - var txNCounter = System.currentTimeMillis() - } }