From bfeb0e356ab44bc86b4694362c4b6e9e9f106362 Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Wed, 15 Nov 2023 17:41:25 +0100 Subject: [PATCH] feat: ignore unknown feature config (#2224) * feat: ignore unknown feature config * detekt (cherry picked from commit b29d505a6942c0a1d0f75f8288e4a9ef3e17c980) --- .../com/wire/kalium/logic/CoreFailure.kt | 7 --- .../logic/sync/incremental/EventProcessor.kt | 44 +++++++-------- .../receiver/FeatureConfigEventReceiver.kt | 54 +++++++++---------- .../sync/incremental/EventProcessorTest.kt | 21 -------- .../FeatureConfigEventReceiverTest.kt | 12 +++++ 5 files changed, 58 insertions(+), 80 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt index 3bafa321f4f..c7e04e47653 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt @@ -100,13 +100,6 @@ 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. * Could be, for example, that the desired user only supports Proteus, but we only support MLS. diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessor.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessor.kt index 373f5ef0097..1df1a61f9df 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessor.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessor.kt @@ -25,7 +25,6 @@ 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 @@ -75,35 +74,29 @@ internal class EventProcessorImpl( override var disableEventProcessing: Boolean = false - override suspend fun processEvent(event: Event): Either = + override suspend fun processEvent(event: Event): Either { if (disableEventProcessing) { logger.w("Skipping processing of $event due to debug option") - Either.Right(Unit) - } else { - when (event) { - is Event.Conversation -> conversationEventReceiver.onEvent(event) - is Event.User -> userEventReceiver.onEvent(event) - is Event.FeatureConfig -> featureConfigEventReceiver.onEvent(event) - is Event.Unknown -> { - kaliumLogger - .logEventProcessing( - EventLoggingStatus.SKIPPED, - event - ) - // Skipping event = success - Either.Right(Unit) - } + return Either.Right(Unit) + } - is Event.Team -> teamEventReceiver.onEvent(event) - is Event.UserProperty -> userPropertiesEventReceiver.onEvent(event) - is Event.Federation -> federationEventReceiver.onEvent(event) - } - }.flatMapLeft { - if (it is CoreFailure.FeatureNotImplemented) { + return when (event) { + is Event.Conversation -> conversationEventReceiver.onEvent(event) + is Event.User -> userEventReceiver.onEvent(event) + is Event.FeatureConfig -> featureConfigEventReceiver.onEvent(event) + is Event.Unknown -> { + kaliumLogger + .logEventProcessing( + EventLoggingStatus.SKIPPED, + event + ) + // Skipping event = success Either.Right(Unit) - } else { - Either.Left(it) } + + is Event.Team -> teamEventReceiver.onEvent(event) + is Event.UserProperty -> userPropertiesEventReceiver.onEvent(event) + is Event.Federation -> federationEventReceiver.onEvent(event) }.onSuccess { val logMap = mapOf( "event" to event.toLogMap() @@ -115,6 +108,7 @@ internal class EventProcessorImpl( logger.i("Skipping update of lastProcessedEventId: ${logMap.toJsonElement()}") } } + } private fun Event.shouldUpdateLastProcessedEventId(): Boolean = !transient } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiver.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiver.kt index 9c6efada5e6..346f9c0822b 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiver.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiver.kt @@ -60,33 +60,33 @@ internal class FeatureConfigEventReceiverImpl internal constructor( ) } .onFailure { - if (it is CoreFailure.FeatureNotImplemented) { - kaliumLogger.logEventProcessing( - EventLoggingStatus.SKIPPED, - event, - Pair("info", "Ignoring unknown feature config update") - ) - } else { - kaliumLogger.logEventProcessing( - EventLoggingStatus.FAILURE, - event, - Pair("error", it) - ) - } - } + kaliumLogger.logEventProcessing( + EventLoggingStatus.FAILURE, + event, + Pair("error", it) + ) + } + +@Suppress("LongMethod", "ComplexMethod") +private suspend fun handleFeatureConfigEvent(event: Event.FeatureConfig): Either = + when (event) { + is Event.FeatureConfig.FileSharingUpdated -> fileSharingConfigHandler.handle(event.model) + is Event.FeatureConfig.MLSUpdated -> mlsConfigHandler.handle(event.model, duringSlowSync = false) + is Event.FeatureConfig.MLSMigrationUpdated -> mlsMigrationConfigHandler.handle(event.model, duringSlowSync = false) + is Event.FeatureConfig.ClassifiedDomainsUpdated -> classifiedDomainsConfigHandler.handle(event.model) + is Event.FeatureConfig.ConferenceCallingUpdated -> conferenceCallingConfigHandler.handle(event.model) + is Event.FeatureConfig.GuestRoomLinkUpdated -> guestRoomConfigHandler.handle(event.model) + is Event.FeatureConfig.SelfDeletingMessagesConfig -> selfDeletingMessagesConfigHandler.handle(event.model) + is Event.FeatureConfig.MLSE2EIUpdated -> e2EIConfigHandler.handle(event.model) + is Event.FeatureConfig.AppLockUpdated -> appLockConfigHandler.handle(event.model) + is Event.FeatureConfig.UnknownFeatureUpdated -> { + kaliumLogger.logEventProcessing( + EventLoggingStatus.SKIPPED, + event, + Pair("info", "Ignoring unknown feature config update") + ) - @Suppress("LongMethod", "ComplexMethod") - private suspend fun handleFeatureConfigEvent(event: Event.FeatureConfig): Either = - when (event) { - is Event.FeatureConfig.FileSharingUpdated -> fileSharingConfigHandler.handle(event.model) - is Event.FeatureConfig.MLSUpdated -> mlsConfigHandler.handle(event.model, duringSlowSync = false) - is Event.FeatureConfig.MLSMigrationUpdated -> mlsMigrationConfigHandler.handle(event.model, duringSlowSync = false) - is Event.FeatureConfig.ClassifiedDomainsUpdated -> classifiedDomainsConfigHandler.handle(event.model) - is Event.FeatureConfig.ConferenceCallingUpdated -> conferenceCallingConfigHandler.handle(event.model) - is Event.FeatureConfig.GuestRoomLinkUpdated -> guestRoomConfigHandler.handle(event.model) - is Event.FeatureConfig.SelfDeletingMessagesConfig -> selfDeletingMessagesConfigHandler.handle(event.model) - is Event.FeatureConfig.MLSE2EIUpdated -> e2EIConfigHandler.handle(event.model) - is Event.FeatureConfig.AppLockUpdated -> appLockConfigHandler.handle(event.model) - is Event.FeatureConfig.UnknownFeatureUpdated -> Either.Left(CoreFailure.FeatureNotImplemented) + Either.Right(Unit) } + } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessorTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessorTest.kt index 9498adb8bdc..7e45631c333 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessorTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/incremental/EventProcessorTest.kt @@ -223,27 +223,6 @@ class EventProcessorTest { .wasNotInvoked() } - @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 diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiverTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiverTest.kt index 80f1557ea7b..beec4564067 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiverTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiverTest.kt @@ -41,8 +41,11 @@ import com.wire.kalium.logic.data.message.TeamSelfDeleteTimer import com.wire.kalium.logic.data.message.TeamSettingsSelfDeletionStatus import com.wire.kalium.logic.feature.user.UpdateSupportedProtocolsAndResolveOneOnOnesUseCase import com.wire.kalium.logic.featureFlags.KaliumConfigs +import com.wire.kalium.logic.framework.TestEvent import com.wire.kalium.logic.framework.TestUser import com.wire.kalium.logic.functional.Either +import com.wire.kalium.logic.util.shouldFail +import com.wire.kalium.logic.util.shouldSucceed import io.mockative.Mock import io.mockative.any import io.mockative.classOf @@ -279,6 +282,15 @@ class FeatureConfigEventReceiverTest { .wasInvoked(once) } + @Test + fun givenUnknownFeatureConfig_whenPrecessing_thenReturnSuccess() = runTest { + val newUnknownFeatureUpdate = TestEvent.newUnknownFeatureUpdate() + val (_, handler) = Arrangement() + .arrange() + + handler.onEvent(newUnknownFeatureUpdate).shouldSucceed() + } + private class Arrangement { var kaliumConfigs = KaliumConfigs()