Skip to content

Commit

Permalink
fix: ignore unknown feature config events (#2217)
Browse files Browse the repository at this point in the history
* fix: ignore unknown feature config events

* detekt

* address PR comment

* detekt
  • Loading branch information
MohamadJaara authored Nov 13, 2023
1 parent 7cbe0fe commit cdf5567
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ sealed interface CoreFailure {
*/
data object SyncEventOrClientNotFound : FeatureFailure()

/**
* The desired event was not found when fetching pending events.
* This can happen when this client is old and the server have new event types
* that the client does not know how to handle.
* the event is skipped and the sync continues
*/
data object FeatureNotImplemented : FeatureFailure()
/**
* No common Protocol found in order to establish a conversation between parties.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ class EventMapper(
else -> MutedConversationStatus.AllAllowed
}

@Suppress("LongMethod")
private fun featureConfig(
id: String,
featureConfigUpdatedDTO: EventContentDTO.FeatureConfig.FeatureConfigUpdatedDTO,
Expand Down Expand Up @@ -565,7 +566,13 @@ class EventMapper(
featureConfigMapper.fromDTO(featureConfigUpdatedDTO.data as FeatureConfigData.AppLock)
)

else -> Event.FeatureConfig.UnknownFeatureUpdated(id, transient, live)
is FeatureConfigData.DigitalSignatures,
is FeatureConfigData.Legalhold,
is FeatureConfigData.SSO,
is FeatureConfigData.SearchVisibility,
is FeatureConfigData.SecondFactorPasswordChallenge,
is FeatureConfigData.Unknown,
is FeatureConfigData.ValidateSAMLEmails -> Event.FeatureConfig.UnknownFeatureUpdated(id, transient, live)
}

private fun conversationDeleted(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,6 @@ class UserSessionScope internal constructor(
mlsMigrationConfigHandler,
classifiedDomainsConfigHandler,
conferenceCallingConfigHandler,
secondFactorPasswordChallengeConfigHandler,
selfDeletingMessagesConfigHandler,
e2eiConfigHandler,
appLockConfigHandler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.wire.kalium.logic.data.event.EventLoggingStatus
import com.wire.kalium.logic.data.event.EventRepository
import com.wire.kalium.logic.data.event.logEventProcessing
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.flatMapLeft
import com.wire.kalium.logic.functional.onSuccess
import com.wire.kalium.logic.kaliumLogger
import com.wire.kalium.logic.sync.receiver.ConversationEventReceiver
Expand Down Expand Up @@ -97,17 +98,23 @@ internal class EventProcessorImpl(
is Event.UserProperty -> userPropertiesEventReceiver.onEvent(event)
is Event.Federation -> federationEventReceiver.onEvent(event)
}
}.onSuccess {
val logMap = mapOf<String, Any>(
"event" to event.toLogMap()
)
if (event.shouldUpdateLastProcessedEventId()) {
eventRepository.updateLastProcessedEventId(event.id)
logger.i("Updated lastProcessedEventId: ${logMap.toJsonElement()}")
} else {
logger.i("Skipping update of lastProcessedEventId: ${logMap.toJsonElement()}")
}.flatMapLeft {
if (it is CoreFailure.FeatureNotImplemented) {
Either.Right(Unit)
} else {
Either.Left(it)
}
}.onSuccess {
val logMap = mapOf<String, Any>(
"event" to event.toLogMap()
)
if (event.shouldUpdateLastProcessedEventId()) {
eventRepository.updateLastProcessedEventId(event.id)
logger.i("Updated lastProcessedEventId: ${logMap.toJsonElement()}")
} else {
logger.i("Skipping update of lastProcessedEventId: ${logMap.toJsonElement()}")
}
}
}

private fun Event.shouldUpdateLastProcessedEventId(): Boolean = !transient
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import com.wire.kalium.logic.feature.featureConfig.handler.FileSharingConfigHand
import com.wire.kalium.logic.feature.featureConfig.handler.GuestRoomConfigHandler
import com.wire.kalium.logic.feature.featureConfig.handler.MLSConfigHandler
import com.wire.kalium.logic.feature.featureConfig.handler.MLSMigrationConfigHandler
import com.wire.kalium.logic.feature.featureConfig.handler.SecondFactorPasswordChallengeConfigHandler
import com.wire.kalium.logic.feature.featureConfig.handler.SelfDeletingMessagesConfigHandler
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.onFailure
Expand All @@ -47,7 +46,6 @@ internal class FeatureConfigEventReceiverImpl internal constructor(
private val mlsMigrationConfigHandler: MLSMigrationConfigHandler,
private val classifiedDomainsConfigHandler: ClassifiedDomainsConfigHandler,
private val conferenceCallingConfigHandler: ConferenceCallingConfigHandler,
private val passwordChallengeConfigHandler: SecondFactorPasswordChallengeConfigHandler,
private val selfDeletingMessagesConfigHandler: SelfDeletingMessagesConfigHandler,
private val e2EIConfigHandler: E2EIConfigHandler,
private val appLockConfigHandler: AppLockConfigHandler
Expand Down Expand Up @@ -75,7 +73,7 @@ internal class FeatureConfigEventReceiverImpl internal constructor(
Pair("error", it)
)
}
}
}

@Suppress("LongMethod", "ComplexMethod")
private suspend fun handleFeatureConfigEvent(event: Event.FeatureConfig): Either<CoreFailure, Unit> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ import com.wire.kalium.logic.data.conversation.Conversation
import com.wire.kalium.logic.data.conversation.Conversation.Member
import com.wire.kalium.logic.data.conversation.MutedConversationStatus
import com.wire.kalium.logic.data.event.Event
import com.wire.kalium.logic.data.featureConfig.AppLockModel
import com.wire.kalium.logic.data.featureConfig.Status
import com.wire.kalium.logic.data.user.Connection
import com.wire.kalium.logic.data.user.ConnectionState
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.util.DateTimeUtil.toIsoDateTimeString
import com.wire.kalium.util.time.Second
import io.ktor.util.encodeBase64
import kotlinx.datetime.Instant

Expand Down Expand Up @@ -290,4 +293,20 @@ object TestEvent {
protocol = Conversation.Protocol.MIXED,
senderUserId = TestUser.OTHER_USER_ID
)

fun newFeatureConfigEvent() = Event.FeatureConfig.AppLockUpdated(
id = "eventId",
transient = false,
live = false,
model = AppLockModel(
inactivityTimeoutSecs = 60,
status = Status.ENABLED
)
)

fun newUnknownFeatureUpdate() = Event.FeatureConfig.UnknownFeatureUpdated(
id = "eventId",
transient = false,
live = false
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import com.wire.kalium.logic.sync.receiver.FederationEventReceiver
import com.wire.kalium.logic.sync.receiver.TeamEventReceiver
import com.wire.kalium.logic.sync.receiver.UserEventReceiver
import com.wire.kalium.logic.sync.receiver.UserPropertiesEventReceiver
import com.wire.kalium.logic.util.arrangement.eventHandler.FeatureConfigEventReceiverArrangement
import com.wire.kalium.logic.util.arrangement.eventHandler.FeatureConfigEventReceiverArrangementImpl
import com.wire.kalium.logic.util.shouldFail
import io.mockative.Mock
import io.mockative.any
Expand Down Expand Up @@ -221,18 +223,35 @@ class EventProcessorTest {
.wasNotInvoked()
}

private class Arrangement {
@Test
fun givenFailureWithFeatureNotImplemented_whenSyncing_thenLastProcessedEventIdIsUpdated() = runTest {
// Given
val event = TestEvent.newUnknownFeatureUpdate()

val (arrangement, eventProcessor) = Arrangement()
.arrange {
withUpdateLastProcessedEventId(event.id, Either.Right(Unit))
withFeatureConfigEventReceiverArrangement(result = Either.Left(CoreFailure.FeatureNotImplemented))
}

// When
eventProcessor.processEvent(event)

// Then
verify(arrangement.eventRepository)
.suspendFunction(arrangement.eventRepository::updateLastProcessedEventId)
.with(eq(event.id))
.wasInvoked(exactly = once)
}

private class Arrangement : FeatureConfigEventReceiverArrangement by FeatureConfigEventReceiverArrangementImpl() {

@Mock
val eventRepository = configure(mock(EventRepository::class)) { stubsUnitByDefault = true }

@Mock
val conversationEventReceiver = mock(ConversationEventReceiver::class)

@Mock
private val featureConfigEventReceiver: FeatureConfigEventReceiver =
mock(FeatureConfigEventReceiver::class)

@Mock
val userEventReceiver = mock(UserEventReceiver::class)

Expand All @@ -252,9 +271,10 @@ class EventProcessorTest {
withUserPropertiesEventReceiverSucceeding()
}

suspend fun withUpdateLastProcessedEventId(eventId: String, result: Either<StorageFailure, Unit>) = apply {
fun withUpdateLastProcessedEventId(eventId: String, result: Either<StorageFailure, Unit>) = apply {
given(eventRepository)
.coroutine { eventRepository.updateLastProcessedEventId(eventId) }
.suspendFunction(eventRepository::updateLastProcessedEventId)
.whenInvokedWith(eq(eventId))
.then { result }
}

Expand Down Expand Up @@ -310,14 +330,16 @@ class EventProcessorTest {
Either.Left(failure)
)

fun arrange() = this to EventProcessorImpl(
eventRepository,
conversationEventReceiver,
userEventReceiver,
teamEventReceiver,
featureConfigEventReceiver,
userPropertiesEventReceiver,
federationEventReceiver
)
fun arrange(block: Arrangement.() -> Unit = {}) = apply(block).let {
this to EventProcessorImpl(
eventRepository,
conversationEventReceiver,
userEventReceiver,
teamEventReceiver,
featureConfigEventReceiver,
userPropertiesEventReceiver,
federationEventReceiver
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ class FeatureConfigEventReceiverTest {
MLSMigrationConfigHandler(userConfigRepository, updateSupportedProtocolsAndResolveOneOnOnes),
ClassifiedDomainsConfigHandler(userConfigRepository),
ConferenceCallingConfigHandler(userConfigRepository),
SecondFactorPasswordChallengeConfigHandler(userConfigRepository),
SelfDeletingMessagesConfigHandler(userConfigRepository, kaliumConfigs),
E2EIConfigHandler(userConfigRepository),
AppLockConfigHandler(userConfigRepository)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Wire
* Copyright (C) 2023 Wire Swiss GmbH
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see http://www.gnu.org/licenses/.
*/
package com.wire.kalium.logic.util.arrangement.eventHandler

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.event.Event
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.sync.receiver.FeatureConfigEventReceiver
import io.mockative.Mock
import io.mockative.any
import io.mockative.given
import io.mockative.matchers.Matcher
import io.mockative.mock

internal interface FeatureConfigEventReceiverArrangement {
@Mock
val featureConfigEventReceiver: FeatureConfigEventReceiver

fun withFeatureConfigEventReceiverArrangement(
result: Either<CoreFailure, Unit>,
event: Matcher<Event.FeatureConfig> = any()
)
}

internal class FeatureConfigEventReceiverArrangementImpl : FeatureConfigEventReceiverArrangement {
@Mock
override val featureConfigEventReceiver: FeatureConfigEventReceiver = mock(FeatureConfigEventReceiver::class)

override fun withFeatureConfigEventReceiverArrangement(
result: Either<CoreFailure, Unit>,
event: Matcher<Event.FeatureConfig>
) {
given(featureConfigEventReceiver)
.suspendFunction(featureConfigEventReceiver::onEvent)
.whenInvokedWith(any())
.thenReturn(result)
}
}

0 comments on commit cdf5567

Please sign in to comment.