feat: ignore unknown feature config (#2224)

* feat: ignore unknown feature config * detekt
wireapp · Nov 15, 2023 · b29d505 · b29d505
1 parent d6c8d60
commit b29d505
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 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
@@ -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
@@ -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<CoreFailure, Unit> =
+    override suspend fun processEvent(event: Event): Either<CoreFailure, Unit> {
         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<String, Any>(
                 "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/...c/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiver.kt b/...c/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<CoreFailure, Unit> =
+    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<CoreFailure, Unit> =
-        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
@@ -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/...c/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/FeatureConfigEventReceiverTest.kt b/...c/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()