Remove FeatureFlag.MediaUploadOnSendQueue

This commit is contained in:
Benoit Marty
2025-08-12 15:39:37 +02:00
committed by Benoit Marty
parent d025ae63ee
commit ebecc6652b
10 changed files with 12 additions and 110 deletions

View File

@@ -31,8 +31,6 @@ import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.core.mimetype.MimeTypes.isMimeTypeImage
import io.element.android.libraries.core.mimetype.MimeTypes.isMimeTypeVideo
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.ProgressCallback
import io.element.android.libraries.matrix.api.permalink.PermalinkBuilder
@@ -57,7 +55,6 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
private val mediaSender: MediaSender,
private val permalinkBuilder: PermalinkBuilder,
private val temporaryUriDeleter: TemporaryUriDeleter,
private val featureFlagService: FeatureFlagService,
private val mediaOptimizationSelectorPresenterFactory: MediaOptimizationSelectorPresenter.Factory,
@SessionCoroutineScope private val sessionCoroutineScope: CoroutineScope,
private val dispatchers: CoroutineDispatchers,
@@ -85,7 +82,6 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(null) }
var useSendQueue by remember { mutableStateOf(false) }
var preprocessMediaJob by remember { mutableStateOf<Job?>(null) }
val mediaAttachment = attachment as Attachment.Media
@@ -98,10 +94,6 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
var displayFileTooLargeError by remember { mutableStateOf(false) }
LaunchedEffect(Unit) {
useSendQueue = featureFlagService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
}
LaunchedEffect(mediaOptimizationSelectorState.displayMediaSelectorViews) {
// If the media optimization selector is not displayed, we can pre-process the media
// to prepare it for sending. This is done to avoid blocking the UI thread when the
@@ -174,18 +166,17 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
.takeIf { it.isNotEmpty() }
// If we're supposed to send the media as a background job, we can dismiss this screen already
if (useSendQueue && coroutineContext.isActive) {
if (coroutineContext.isActive) {
onDoneListener()
}
// If using the send queue, send it using the session coroutine scope so it doesn't matter if this screen or the chat one are closed
val sendMediaCoroutineScope = if (useSendQueue) sessionCoroutineScope else coroutineScope
sendMediaCoroutineScope.launch(dispatchers.io) {
// Send the media using the session coroutine scope so it doesn't matter if this screen or the chat one are closed
sessionCoroutineScope.launch(dispatchers.io) {
sendPreProcessedMedia(
mediaUploadInfo = mediaUploadInfo,
caption = caption,
sendActionState = sendActionState,
dismissAfterSend = !useSendQueue,
dismissAfterSend = false,
inReplyToEventId = null,
)

View File

@@ -25,8 +25,6 @@ import io.element.android.features.messages.test.attachments.video.FakeMediaOpti
import io.element.android.libraries.androidutils.file.TemporaryUriDeleter
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.core.mimetype.MimeTypes
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.ProgressCallback
import io.element.android.libraries.matrix.api.media.AudioInfo
@@ -407,37 +405,7 @@ class AttachmentsPreviewPresenterTest {
}
@Test
fun `present - send media failure scenario without media queue`() = runTest {
val failure = MediaPreProcessor.Failure(null)
val sendFileResult =
lambdaRecorder<File, FileInfo, String?, String?, ProgressCallback?, EventId?, Result<FakeMediaUploadHandler>> { _, _, _, _, _, _ ->
Result.failure(failure)
}
val room = FakeJoinedRoom(
liveTimeline = FakeTimeline().apply {
sendFileLambda = sendFileResult
},
)
val presenter = createAttachmentsPreviewPresenter(room = room, mediaUploadOnSendQueueEnabled = false)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle)
initialState.eventSink(AttachmentsPreviewEvents.SendAttachment)
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing(displayProgress = false))
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.ReadyToUpload(mediaUploadInfo))
val failureState = awaitItem()
assertThat(failureState.sendActionState).isEqualTo(SendActionState.Failure(failure, mediaUploadInfo))
sendFileResult.assertions().isCalledOnce()
failureState.eventSink(AttachmentsPreviewEvents.CancelAndClearSendState)
val clearedState = awaitLastSequentialItem()
assertThat(clearedState.sendActionState).isEqualTo(SendActionState.Sending.ReadyToUpload(mediaUploadInfo))
}
}
@Test
fun `present - send media failure scenario with media queue`() = runTest {
fun `present - send media failure scenario`() = runTest {
val failure = MediaPreProcessor.Failure(null)
val sendFileResult =
lambdaRecorder<File, FileInfo, String?, String?, ProgressCallback?, EventId?, Result<FakeMediaUploadHandler>> { _, _, _, _, _, _ ->
@@ -449,7 +417,7 @@ class AttachmentsPreviewPresenterTest {
sendFileLambda = sendFileResult
},
)
val presenter = createAttachmentsPreviewPresenter(room = room, mediaUploadOnSendQueueEnabled = true, onDoneListener = onDoneListenerResult)
val presenter = createAttachmentsPreviewPresenter(room = room, onDoneListener = onDoneListenerResult)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
@@ -472,24 +440,7 @@ class AttachmentsPreviewPresenterTest {
}
@Test
fun `present - dismissing the progress dialog stops media upload without media queue`() = runTest {
val presenter = createAttachmentsPreviewPresenter(mediaUploadOnSendQueueEnabled = false)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle)
initialState.eventSink(AttachmentsPreviewEvents.SendAttachment)
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing(displayProgress = false))
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.ReadyToUpload(mediaUploadInfo))
initialState.eventSink(AttachmentsPreviewEvents.CancelAndClearSendState)
// The sending is cancelled and the state is kept at ReadyToUpload
ensureAllEventsConsumed()
}
}
@Test
fun `present - dismissing the progress dialog stops media upload with media queue`() = runTest {
fun `present - dismissing the progress dialog stops media upload`() = runTest {
val onDoneListenerResult = lambdaRecorder<Unit> {}
val presenter = createAttachmentsPreviewPresenter(
room = FakeJoinedRoom(
@@ -499,7 +450,6 @@ class AttachmentsPreviewPresenterTest {
}
}
),
mediaUploadOnSendQueueEnabled = true,
onDoneListener = onDoneListenerResult,
)
moleculeFlow(RecompositionMode.Immediate) {
@@ -535,7 +485,6 @@ class AttachmentsPreviewPresenterTest {
}
}
),
mediaUploadOnSendQueueEnabled = true,
onDoneListener = onDoneListenerResult,
mediaOptimizationSelectorPresenterFactory = FakeMediaOptimizationSelectorPresenterFactory {
MediaOptimizationSelectorState(
@@ -577,7 +526,6 @@ class AttachmentsPreviewPresenterTest {
}
}
),
mediaUploadOnSendQueueEnabled = true,
onDoneListener = onDoneListenerResult,
mediaOptimizationSelectorPresenterFactory = FakeMediaOptimizationSelectorPresenterFactory {
MediaOptimizationSelectorState(
@@ -633,7 +581,6 @@ class AttachmentsPreviewPresenterTest {
mediaPreProcessor: MediaPreProcessor = FakeMediaPreProcessor(),
temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(),
onDoneListener: OnDoneListener = OnDoneListener { lambdaError() },
mediaUploadOnSendQueueEnabled: Boolean = true,
displayMediaQualitySelectorViews: Boolean = false,
mediaOptimizationSelectorPresenterFactory: FakeMediaOptimizationSelectorPresenterFactory = FakeMediaOptimizationSelectorPresenterFactory(
fakePresenter = {
@@ -657,11 +604,6 @@ class AttachmentsPreviewPresenterTest {
}),
permalinkBuilder = permalinkBuilder,
temporaryUriDeleter = temporaryUriDeleter,
featureFlagService = FakeFeatureFlagService(
initialState = mapOf(
FeatureFlags.MediaUploadOnSendQueue.key to mediaUploadOnSendQueueEnabled,
),
),
sessionCoroutineScope = this,
dispatchers = testCoroutineDispatchers(),
mediaOptimizationSelectorPresenterFactory = mediaOptimizationSelectorPresenterFactory,

View File

@@ -75,13 +75,6 @@ enum class FeatureFlags(
defaultValue = { false },
isFinished = false,
),
MediaUploadOnSendQueue(
key = "feature.media_upload_through_send_queue",
title = "Media upload through send queue",
description = "Support for treating media uploads as regular events, with an improved retry and cancellation implementation.",
defaultValue = { true },
isFinished = true,
),
PrintLogsToLogcat(
key = "feature.print_logs_to_logcat",
title = "Print logs to logcat",

View File

@@ -14,7 +14,6 @@ import io.element.android.libraries.core.coroutine.childScope
import io.element.android.libraries.core.data.tryOrNull
import io.element.android.libraries.core.extensions.mapFailure
import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.DeviceId
import io.element.android.libraries.matrix.api.core.ProgressCallback
@@ -136,7 +135,6 @@ class RustMatrixClient(
baseCacheDirectory: File,
clock: SystemClock,
timelineEventTypeFilterFactory: TimelineEventTypeFilterFactory,
featureFlagService: FeatureFlagService,
) : MatrixClient {
override val sessionId: UserId = UserId(innerClient.userId())
override val deviceId: DeviceId = DeviceId(innerClient.deviceId())
@@ -205,7 +203,6 @@ class RustMatrixClient(
roomContentForwarder = RoomContentForwarder(innerRoomListService),
roomSyncSubscriber = roomSyncSubscriber,
timelineEventTypeFilterFactory = timelineEventTypeFilterFactory,
featureFlagService = featureFlagService,
roomMembershipObserver = roomMembershipObserver,
roomInfoMapper = roomInfoMapper,
)

View File

@@ -93,7 +93,6 @@ class RustMatrixClientFactory @Inject constructor(
baseCacheDirectory = cacheDirectory,
clock = clock,
timelineEventTypeFilterFactory = timelineEventTypeFilterFactory,
featureFlagService = featureFlagService,
).also {
Timber.tag(it.toString()).d("Creating Client with access token '$anonymizedAccessToken' and refresh token '$anonymizedRefreshToken'")
}

View File

@@ -11,7 +11,6 @@ import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.core.coroutine.childScope
import io.element.android.libraries.core.extensions.mapFailure
import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.matrix.api.core.DeviceId
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomAlias
@@ -84,7 +83,6 @@ class JoinedRustRoom(
private val coroutineDispatchers: CoroutineDispatchers,
private val systemClock: SystemClock,
private val roomContentForwarder: RoomContentForwarder,
private val featureFlagService: FeatureFlagService,
) : JoinedRoom, BaseRoom by baseRoom {
// Create a dispatcher for all room methods...
private val roomDispatcher = coroutineDispatchers.io.limitedParallelism(32)
@@ -478,7 +476,6 @@ class JoinedRustRoom(
dispatcher = roomDispatcher,
roomContentForwarder = roomContentForwarder,
onNewSyncedEvent = onNewSyncedEvent,
featureFlagsService = featureFlagService,
)
}
}

View File

@@ -9,7 +9,6 @@ package io.element.android.libraries.matrix.impl.room
import io.element.android.appconfig.TimelineConfig
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.matrix.api.core.DeviceId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
@@ -49,7 +48,6 @@ class RustRoomFactory(
private val innerRoomListService: InnerRoomListService,
private val roomSyncSubscriber: RoomSyncSubscriber,
private val timelineEventTypeFilterFactory: TimelineEventTypeFilterFactory,
private val featureFlagService: FeatureFlagService,
private val roomMembershipObserver: RoomMembershipObserver,
private val roomInfoMapper: RoomInfoMapper,
) {
@@ -127,7 +125,6 @@ class RustRoomFactory(
liveInnerTimeline = timeline,
coroutineDispatchers = dispatchers,
systemClock = systemClock,
featureFlagService = featureFlagService,
)
)
} else {

View File

@@ -8,8 +8,6 @@
package io.element.android.libraries.matrix.impl.timeline
import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.ProgressCallback
import io.element.android.libraries.matrix.api.core.RoomId
@@ -89,7 +87,6 @@ class RustTimeline(
private val coroutineScope: CoroutineScope,
private val dispatcher: CoroutineDispatcher,
private val roomContentForwarder: RoomContentForwarder,
private val featureFlagsService: FeatureFlagService,
onNewSyncedEvent: () -> Unit,
) : Timeline {
private val _timelineItems: MutableSharedFlow<List<MatrixTimelineItem>> =
@@ -342,7 +339,6 @@ class RustTimeline(
progressCallback: ProgressCallback?,
inReplyToEventId: EventId?,
): Result<MediaUploadHandler> {
val useSendQueue = featureFlagsService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
return sendAttachment(listOfNotNull(file, thumbnailFile)) {
inner.sendImage(
params = UploadParameters(
@@ -351,7 +347,7 @@ class RustTimeline(
formattedCaption = formattedCaption?.let {
FormattedBody(body = it, format = MessageFormat.Html)
},
useSendQueue = useSendQueue,
useSendQueue = true,
mentions = null,
inReplyTo = inReplyToEventId?.value,
),
@@ -371,7 +367,6 @@ class RustTimeline(
progressCallback: ProgressCallback?,
inReplyToEventId: EventId?,
): Result<MediaUploadHandler> {
val useSendQueue = featureFlagsService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
return sendAttachment(listOfNotNull(file, thumbnailFile)) {
inner.sendVideo(
params = UploadParameters(
@@ -380,7 +375,7 @@ class RustTimeline(
formattedCaption = formattedCaption?.let {
FormattedBody(body = it, format = MessageFormat.Html)
},
useSendQueue = useSendQueue,
useSendQueue = true,
mentions = null,
inReplyTo = inReplyToEventId?.value,
),
@@ -399,7 +394,6 @@ class RustTimeline(
progressCallback: ProgressCallback?,
inReplyToEventId: EventId?,
): Result<MediaUploadHandler> {
val useSendQueue = featureFlagsService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
return sendAttachment(listOf(file)) {
inner.sendAudio(
params = UploadParameters(
@@ -408,7 +402,7 @@ class RustTimeline(
formattedCaption = formattedCaption?.let {
FormattedBody(body = it, format = MessageFormat.Html)
},
useSendQueue = useSendQueue,
useSendQueue = true,
mentions = null,
inReplyTo = inReplyToEventId?.value,
),
@@ -426,7 +420,6 @@ class RustTimeline(
progressCallback: ProgressCallback?,
inReplyToEventId: EventId?,
): Result<MediaUploadHandler> {
val useSendQueue = featureFlagsService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
return sendAttachment(listOf(file)) {
inner.sendFile(
params = UploadParameters(
@@ -435,7 +428,7 @@ class RustTimeline(
formattedCaption = formattedCaption?.let {
FormattedBody(body = it, format = MessageFormat.Html)
},
useSendQueue = useSendQueue,
useSendQueue = true,
mentions = null,
inReplyTo = inReplyToEventId?.value,
),
@@ -489,7 +482,6 @@ class RustTimeline(
progressCallback: ProgressCallback?,
inReplyToEventId: EventId?,
): Result<MediaUploadHandler> {
val useSendQueue = featureFlagsService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
return sendAttachment(listOf(file)) {
inner.sendVoiceMessage(
params = UploadParameters(
@@ -497,7 +489,7 @@ class RustTimeline(
// Maybe allow a caption in the future?
caption = null,
formattedCaption = null,
useSendQueue = useSendQueue,
useSendQueue = true,
mentions = null,
inReplyTo = inReplyToEventId?.value,
),

View File

@@ -8,7 +8,6 @@
package io.element.android.libraries.matrix.impl
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiClient
import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiSyncService
import io.element.android.libraries.matrix.impl.room.FakeTimelineEventTypeFilterFactory
@@ -67,6 +66,5 @@ class RustMatrixClientTest {
baseCacheDirectory = File(""),
clock = FakeSystemClock(),
timelineEventTypeFilterFactory = FakeTimelineEventTypeFilterFactory(),
featureFlagService = FakeFeatureFlagService(),
)
}

View File

@@ -10,8 +10,6 @@ package io.element.android.libraries.matrix.impl.timeline
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
import io.element.android.libraries.matrix.api.room.JoinedRoom
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
import io.element.android.libraries.matrix.api.timeline.Timeline
@@ -98,7 +96,6 @@ private fun TestScope.createRustTimeline(
coroutineScope: CoroutineScope = backgroundScope,
dispatcher: CoroutineDispatcher = testCoroutineDispatchers().io,
roomContentForwarder: RoomContentForwarder = RoomContentForwarder(FakeFfiRoomListService()),
featureFlagsService: FeatureFlagService = FakeFeatureFlagService(),
onNewSyncedEvent: () -> Unit = {},
): RustTimeline {
return RustTimeline(
@@ -109,7 +106,6 @@ private fun TestScope.createRustTimeline(
coroutineScope = coroutineScope,
dispatcher = dispatcher,
roomContentForwarder = roomContentForwarder,
featureFlagsService = featureFlagsService,
onNewSyncedEvent = onNewSyncedEvent,
)
}