From 7ce093e239c706d13beb370f852e848a8826f68c Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Wed, 18 May 2022 13:39:12 +0300 Subject: [PATCH] Code review fixes. --- .../poll/DefaultPollAggregationProcessor.kt | 13 +- .../poll/PollAggregationProcessorTest.kt | 173 +++--------------- .../aggregation/poll/PollEventsTestData.kt | 171 +++++++++++++++++ .../android/sdk/test/fakes/FakeRealm.kt | 2 +- 4 files changed, 199 insertions(+), 160 deletions(-) create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollEventsTestData.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/DefaultPollAggregationProcessor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/DefaultPollAggregationProcessor.kt index 2f55eb940e..d4b414aaea 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/DefaultPollAggregationProcessor.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/DefaultPollAggregationProcessor.kt @@ -41,7 +41,6 @@ import org.matrix.android.sdk.internal.database.model.PollResponseAggregatedSumm import org.matrix.android.sdk.internal.database.query.create import org.matrix.android.sdk.internal.database.query.getOrCreate import org.matrix.android.sdk.internal.database.query.where -import timber.log.Timber import javax.inject.Inject class DefaultPollAggregationProcessor @Inject constructor() : PollAggregationProcessor { @@ -162,7 +161,6 @@ class DefaultPollAggregationProcessor @Inject constructor() : PollAggregationPro val isPollOwner = pollOwnerId == event.senderId if (!isPollOwner && !powerLevelsHelper.isUserAbleToRedact(event.senderId ?: "")) { - Timber.v("## Received poll.end event $pollEventId but user ${event.senderId} doesn't have enough power level in room $roomId") return false } @@ -182,17 +180,12 @@ class DefaultPollAggregationProcessor @Inject constructor() : PollAggregationPro } private fun getPollEvent(session: Session, roomId: String, eventId: String): TimelineEvent? { - return session.roomService().getRoom(roomId)?.getTimelineEvent(eventId) ?: return null.also { - Timber.v("## POLL target poll event $eventId not found in room $roomId") - } + return session.roomService().getRoom(roomId)?.getTimelineEvent(eventId) } private fun getPollContent(session: Session, roomId: String, eventId: String): MessagePollContent? { - val pollEvent = getPollEvent(session, roomId, eventId) ?: return null - - return pollEvent.getLastMessageContent() as? MessagePollContent ?: return null.also { - Timber.v("## POLL target poll event $eventId content is malformed") - } + val pollEvent = getPollEvent(session, roomId, eventId) + return pollEvent?.getLastMessageContent() as? MessagePollContent } private fun getAnnotationsSummaryEntity(realm: Realm, roomId: String, eventId: String): EventAnnotationsSummaryEntity { diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollAggregationProcessorTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollAggregationProcessorTest.kt index 9783a40d6c..dec325c8b2 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollAggregationProcessorTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollAggregationProcessorTest.kt @@ -47,145 +47,20 @@ import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent import org.matrix.android.sdk.internal.database.model.EventAnnotationsSummaryEntity import org.matrix.android.sdk.internal.database.model.EventAnnotationsSummaryEntityFields import org.matrix.android.sdk.internal.database.model.PollResponseAggregatedSummaryEntity +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.AN_EVENT_ID +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.AN_INVALID_POLL_RESPONSE_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_BROKEN_POLL_REPLACE_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_POLL_END_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_POLL_REFERENCE_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_POLL_REPLACE_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_POLL_RESPONSE_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_POLL_RESPONSE_EVENT_WITH_A_WRONG_REFERENCE +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_POLL_START_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_ROOM_ID +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_TIMELINE_EVENT +import org.matrix.android.sdk.internal.session.room.aggregation.poll.PollEventsTestData.A_USER_ID_1 import org.matrix.android.sdk.test.fakes.FakeRealm -private const val A_USER_ID_1 = "@user_1:matrix.org" -private const val A_ROOM_ID = "!sUeOGZKsBValPTUMax:matrix.org" -private const val AN_EVENT_ID = "\$vApgexcL8Vfh-WxYKsFKCDooo67ttbjm3TiVKXaWijU" - -private val A_POLL_CONTENT = MessagePollContent( - unstablePollCreationInfo = PollCreationInfo( - question = PollQuestion( - unstableQuestion = "What is your favourite coffee?" - ), - maxSelections = 1, - answers = listOf( - PollAnswer( - id = "5ef5f7b0-c9a1-49cf-a0b3-374729a43e76", - unstableAnswer = "Double Espresso" - ), - PollAnswer( - id = "ec1a4db0-46d8-4d7a-9bb6-d80724715938", - unstableAnswer = "Macchiato" - ), - PollAnswer( - id = "3677ca8e-061b-40ab-bffe-b22e4e88fcad", - unstableAnswer = "Iced Coffee" - ) - ) - ) -) - -private val A_POLL_RESPONSE_CONTENT = MessagePollResponseContent( - unstableResponse = PollResponse( - answers = listOf("5ef5f7b0-c9a1-49cf-a0b3-374729a43e76") - ), - relatesTo = RelationDefaultContent( - type = RelationType.REFERENCE, - eventId = AN_EVENT_ID - ) -) - -private val A_POLL_END_CONTENT = MessageEndPollContent( - relatesTo = RelationDefaultContent( - type = RelationType.REFERENCE, - eventId = AN_EVENT_ID - ) -) - -private val AN_INVALID_POLL_RESPONSE_CONTENT = MessagePollResponseContent( - unstableResponse = PollResponse( - answers = listOf("fake-option-id") - ), - relatesTo = RelationDefaultContent( - type = RelationType.REFERENCE, - eventId = AN_EVENT_ID - ) -) - -private val A_POLL_START_EVENT = Event( - type = EventType.POLL_START.first(), - eventId = AN_EVENT_ID, - originServerTs = 1652435922563, - senderId = A_USER_ID_1, - roomId = A_ROOM_ID, - content = A_POLL_CONTENT.toContent() -) - -private val A_POLL_RESPONSE_EVENT = Event( - type = EventType.POLL_RESPONSE.first(), - eventId = AN_EVENT_ID, - originServerTs = 1652435922563, - senderId = A_USER_ID_1, - roomId = A_ROOM_ID, - content = A_POLL_RESPONSE_CONTENT.toContent() -) - -private val A_POLL_END_EVENT = Event( - type = EventType.POLL_END.first(), - eventId = AN_EVENT_ID, - originServerTs = 1652435922563, - senderId = A_USER_ID_1, - roomId = A_ROOM_ID, - content = A_POLL_END_CONTENT.toContent() -) - -private val A_TIMELINE_EVENT = TimelineEvent( - root = A_POLL_START_EVENT, - localId = 1234, - eventId = AN_EVENT_ID, - displayIndex = 0, - senderInfo = SenderInfo(A_USER_ID_1, "A_USER_ID_1", true, null) -) - -private val A_POLL_RESPONSE_EVENT_WITH_A_WRONG_REFERENCE = A_POLL_RESPONSE_EVENT.copy( - content = A_POLL_RESPONSE_CONTENT - .copy( - relatesTo = RelationDefaultContent( - type = RelationType.REPLACE, - eventId = null - ) - ) - .toContent() -) - -private val A_POLL_REPLACE_EVENT = A_POLL_START_EVENT.copy( - content = A_POLL_CONTENT - .copy( - relatesTo = RelationDefaultContent( - type = RelationType.REPLACE, - eventId = AN_EVENT_ID - ) - ) - .toContent() -) - -private val A_BROKEN_POLL_REPLACE_EVENT = A_POLL_START_EVENT.copy( - content = A_POLL_CONTENT - .copy( - relatesTo = RelationDefaultContent( - type = RelationType.REPLACE, - eventId = null - ) - ) - .toContent() -) - -private val A_POLL_REFERENCE_EVENT = A_POLL_START_EVENT.copy( - content = A_POLL_CONTENT - .copy( - relatesTo = RelationDefaultContent( - type = RelationType.REFERENCE, - eventId = AN_EVENT_ID - ) - ) - .toContent() -) - -private val AN_INVALID_POLL_RESPONSE_EVENT = A_POLL_RESPONSE_EVENT.copy( - content = AN_INVALID_POLL_RESPONSE_CONTENT.toContent() -) - class PollAggregationProcessorTest { private val pollAggregationProcessor: PollAggregationProcessor = DefaultPollAggregationProcessor() @@ -200,38 +75,38 @@ class PollAggregationProcessorTest { } @Test - fun `given a poll start event which is not a replace is not processed by poll aggregator`() { + fun `given a poll start event, when processing, then is ignored and returns false`() { pollAggregationProcessor.handlePollStartEvent(realm.instance, A_POLL_START_EVENT).shouldBeFalse() } @Test - fun `given a poll start event with a reference is not processed by poll aggregator`() { + fun `given a poll start event with a reference, when processing, then is ignored and returns false`() { pollAggregationProcessor.handlePollStartEvent(realm.instance, A_POLL_REFERENCE_EVENT).shouldBeFalse() } @Test - fun `given a poll start event with a replace but without target event id is not processed by poll aggregator`() { + fun `given a poll start event with a replace relation but without a target event id, when processing, then is ignored and returns false`() { pollAggregationProcessor.handlePollStartEvent(realm.instance, A_BROKEN_POLL_REPLACE_EVENT).shouldBeFalse() } @Test - fun `given a poll start event with a replace is processed by poll aggregator`() { + fun `given a poll start event with a replace, when processing, then is processed and returns true`() { pollAggregationProcessor.handlePollStartEvent(realm.instance, A_POLL_REPLACE_EVENT).shouldBeTrue() } @Test - fun `given a poll response event with a broken reference is not processed by poll aggregator`() { + fun `given a poll response event with a broken reference, when processing, then is ignored and returns false`() { pollAggregationProcessor.handlePollResponseEvent(session, realm.instance, A_POLL_RESPONSE_EVENT_WITH_A_WRONG_REFERENCE).shouldBeFalse() } @Test - fun `given a poll response event with a reference is processed by poll aggregator`() { + fun `given a poll response event with a reference, when processing, then is processed and returns true`() { every { realm.instance.createObject(PollResponseAggregatedSummaryEntity::class.java) } returns PollResponseAggregatedSummaryEntity() pollAggregationProcessor.handlePollResponseEvent(session, realm.instance, A_POLL_RESPONSE_EVENT).shouldBeTrue() } @Test - fun `given a poll response event after poll is closed is not processed by poll aggregator`() { + fun `given a poll response event after poll is closed, when processing, then is ignored and returns false`() { every { realm.instance.createObject(PollResponseAggregatedSummaryEntity::class.java) } returns PollResponseAggregatedSummaryEntity().apply { closedTime = (A_POLL_RESPONSE_EVENT.originServerTs ?: 0) - 1 } @@ -239,7 +114,7 @@ class PollAggregationProcessorTest { } @Test - fun `given a poll response event which is already processed is not processed by poll aggregator`() { + fun `given a poll response event which is already processed, when processing, then is ignored and returns false`() { every { realm.instance.createObject(PollResponseAggregatedSummaryEntity::class.java) } returns PollResponseAggregatedSummaryEntity().apply { sourceEvents = RealmList(A_POLL_RESPONSE_EVENT.eventId) } @@ -247,27 +122,27 @@ class PollAggregationProcessorTest { } @Test - fun `given a poll response event which is not one of the options is not processed by poll aggregator`() { + fun `given a poll response event which is not one of the options, when processing, then is ignored and returns false`() { every { realm.instance.createObject(PollResponseAggregatedSummaryEntity::class.java) } returns PollResponseAggregatedSummaryEntity() pollAggregationProcessor.handlePollResponseEvent(session, realm.instance, AN_INVALID_POLL_RESPONSE_EVENT).shouldBeFalse() } @Test - fun `given a poll end event is processed by poll aggregator`() { + fun `given a poll end event, when processing, then is processed and return true`() { every { realm.instance.createObject(PollResponseAggregatedSummaryEntity::class.java) } returns PollResponseAggregatedSummaryEntity() val powerLevelsHelper = mockRedactionPowerLevels(A_USER_ID_1, true) pollAggregationProcessor.handlePollEndEvent(session, powerLevelsHelper, realm.instance, A_POLL_END_EVENT).shouldBeTrue() } @Test - fun `given a poll end event without redaction power level of event owner is processed by poll aggregator`() { + fun `given a poll end event for my own poll without enough redaction power level, when processing, then is processed and returns true`() { every { realm.instance.createObject(PollResponseAggregatedSummaryEntity::class.java) } returns PollResponseAggregatedSummaryEntity() val powerLevelsHelper = mockRedactionPowerLevels(A_USER_ID_1, false) pollAggregationProcessor.handlePollEndEvent(session, powerLevelsHelper, realm.instance, A_POLL_END_EVENT).shouldBeTrue() } @Test - fun `given a poll end event without redaction power level of non event owner is not processed by poll aggregator`() { + fun `given a poll end event without enough redaction power level, when is processed, then is ignored and return false`() { every { realm.instance.createObject(PollResponseAggregatedSummaryEntity::class.java) } returns PollResponseAggregatedSummaryEntity() val powerLevelsHelper = mockRedactionPowerLevels("another-sender-id", false) val event = A_POLL_END_EVENT.copy(senderId = "another-sender-id") diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollEventsTestData.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollEventsTestData.kt new file mode 100644 index 0000000000..129d49633e --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/aggregation/poll/PollEventsTestData.kt @@ -0,0 +1,171 @@ +/* + * Copyright 2022 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.session.room.aggregation.poll + +import org.matrix.android.sdk.api.session.events.model.Event +import org.matrix.android.sdk.api.session.events.model.EventType +import org.matrix.android.sdk.api.session.events.model.RelationType +import org.matrix.android.sdk.api.session.events.model.toContent +import org.matrix.android.sdk.api.session.room.model.message.MessageEndPollContent +import org.matrix.android.sdk.api.session.room.model.message.MessagePollContent +import org.matrix.android.sdk.api.session.room.model.message.MessagePollResponseContent +import org.matrix.android.sdk.api.session.room.model.message.PollAnswer +import org.matrix.android.sdk.api.session.room.model.message.PollCreationInfo +import org.matrix.android.sdk.api.session.room.model.message.PollQuestion +import org.matrix.android.sdk.api.session.room.model.message.PollResponse +import org.matrix.android.sdk.api.session.room.model.relation.RelationDefaultContent +import org.matrix.android.sdk.api.session.room.sender.SenderInfo +import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent + +object PollEventsTestData { + internal const val A_USER_ID_1 = "@user_1:matrix.org" + internal const val A_ROOM_ID = "!sUeOGZKsBValPTUMax:matrix.org" + internal const val AN_EVENT_ID = "\$vApgexcL8Vfh-WxYKsFKCDooo67ttbjm3TiVKXaWijU" + + internal val A_POLL_CONTENT = MessagePollContent( + unstablePollCreationInfo = PollCreationInfo( + question = PollQuestion( + unstableQuestion = "What is your favourite coffee?" + ), + maxSelections = 1, + answers = listOf( + PollAnswer( + id = "5ef5f7b0-c9a1-49cf-a0b3-374729a43e76", + unstableAnswer = "Double Espresso" + ), + PollAnswer( + id = "ec1a4db0-46d8-4d7a-9bb6-d80724715938", + unstableAnswer = "Macchiato" + ), + PollAnswer( + id = "3677ca8e-061b-40ab-bffe-b22e4e88fcad", + unstableAnswer = "Iced Coffee" + ) + ) + ) + ) + + internal val A_POLL_RESPONSE_CONTENT = MessagePollResponseContent( + unstableResponse = PollResponse( + answers = listOf("5ef5f7b0-c9a1-49cf-a0b3-374729a43e76") + ), + relatesTo = RelationDefaultContent( + type = RelationType.REFERENCE, + eventId = AN_EVENT_ID + ) + ) + + internal val A_POLL_END_CONTENT = MessageEndPollContent( + relatesTo = RelationDefaultContent( + type = RelationType.REFERENCE, + eventId = AN_EVENT_ID + ) + ) + + internal val AN_INVALID_POLL_RESPONSE_CONTENT = MessagePollResponseContent( + unstableResponse = PollResponse( + answers = listOf("fake-option-id") + ), + relatesTo = RelationDefaultContent( + type = RelationType.REFERENCE, + eventId = AN_EVENT_ID + ) + ) + + internal val A_POLL_START_EVENT = Event( + type = EventType.POLL_START.first(), + eventId = AN_EVENT_ID, + originServerTs = 1652435922563, + senderId = A_USER_ID_1, + roomId = A_ROOM_ID, + content = A_POLL_CONTENT.toContent() + ) + + internal val A_POLL_RESPONSE_EVENT = Event( + type = EventType.POLL_RESPONSE.first(), + eventId = AN_EVENT_ID, + originServerTs = 1652435922563, + senderId = A_USER_ID_1, + roomId = A_ROOM_ID, + content = A_POLL_RESPONSE_CONTENT.toContent() + ) + + internal val A_POLL_END_EVENT = Event( + type = EventType.POLL_END.first(), + eventId = AN_EVENT_ID, + originServerTs = 1652435922563, + senderId = A_USER_ID_1, + roomId = A_ROOM_ID, + content = A_POLL_END_CONTENT.toContent() + ) + + internal val A_TIMELINE_EVENT = TimelineEvent( + root = A_POLL_START_EVENT, + localId = 1234, + eventId = AN_EVENT_ID, + displayIndex = 0, + senderInfo = SenderInfo(A_USER_ID_1, "A_USER_ID_1", true, null) + ) + + internal val A_POLL_RESPONSE_EVENT_WITH_A_WRONG_REFERENCE = A_POLL_RESPONSE_EVENT.copy( + content = A_POLL_RESPONSE_CONTENT + .copy( + relatesTo = RelationDefaultContent( + type = RelationType.REPLACE, + eventId = null + ) + ) + .toContent() + ) + + internal val A_POLL_REPLACE_EVENT = A_POLL_START_EVENT.copy( + content = A_POLL_CONTENT + .copy( + relatesTo = RelationDefaultContent( + type = RelationType.REPLACE, + eventId = AN_EVENT_ID + ) + ) + .toContent() + ) + + internal val A_BROKEN_POLL_REPLACE_EVENT = A_POLL_START_EVENT.copy( + content = A_POLL_CONTENT + .copy( + relatesTo = RelationDefaultContent( + type = RelationType.REPLACE, + eventId = null + ) + ) + .toContent() + ) + + internal val A_POLL_REFERENCE_EVENT = A_POLL_START_EVENT.copy( + content = A_POLL_CONTENT + .copy( + relatesTo = RelationDefaultContent( + type = RelationType.REFERENCE, + eventId = AN_EVENT_ID + ) + ) + .toContent() + ) + + internal val AN_INVALID_POLL_RESPONSE_EVENT = A_POLL_RESPONSE_EVENT.copy( + content = AN_INVALID_POLL_RESPONSE_CONTENT.toContent() + ) +} diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeRealm.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeRealm.kt index a3462c4acc..c07f8e1873 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeRealm.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeRealm.kt @@ -28,7 +28,7 @@ internal class FakeRealm { val instance = mockk(relaxed = true) inline fun givenWhereReturns(result: T?): RealmQuery { - val queryResult = mockk>(relaxed = true) + val queryResult = mockk>() every { queryResult.findFirst() } returns result every { instance.where() } returns queryResult return queryResult