Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ignore unknown feature config (#2224) #2231

Merged
merged 2 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -115,6 +108,7 @@ internal class EventProcessorImpl(
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 @@ -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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
Loading