From b0a31304a1ae0649c478884996013c70718486dc Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 31 Oct 2022 17:04:49 +0100 Subject: [PATCH 01/47] Update seek bar tick progress while playing --- .../factory/VoiceBroadcastItemFactory.kt | 3 + .../item/AbsMessageVoiceBroadcastItem.kt | 4 + .../MessageVoiceBroadcastListeningItem.kt | 30 +++++++- .../listening/VoiceBroadcastPlayerImpl.kt | 74 +++++++++++++++++-- 4 files changed, 100 insertions(+), 11 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt index 5d9c663210..06d3563303 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt @@ -18,6 +18,7 @@ package im.vector.app.features.home.room.detail.timeline.factory import im.vector.app.core.resources.ColorProvider import im.vector.app.core.resources.DrawableProvider import im.vector.app.features.displayname.getBestName +import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.home.room.detail.timeline.helper.AvatarSizeProvider import im.vector.app.features.home.room.detail.timeline.helper.VoiceBroadcastEventsGroup import im.vector.app.features.home.room.detail.timeline.item.AbsMessageItem @@ -44,6 +45,7 @@ class VoiceBroadcastItemFactory @Inject constructor( private val drawableProvider: DrawableProvider, private val voiceBroadcastRecorder: VoiceBroadcastRecorder?, private val voiceBroadcastPlayer: VoiceBroadcastPlayer, + private val playbackTracker: AudioMessagePlaybackTracker, ) { fun create( @@ -71,6 +73,7 @@ class VoiceBroadcastItemFactory @Inject constructor( recorderName = params.event.root.stateKey?.let { session.getUserOrDefault(it) }?.toMatrixItem()?.getBestName().orEmpty(), recorder = voiceBroadcastRecorder, player = voiceBroadcastPlayer, + playbackTracker = playbackTracker, roomItem = session.getRoom(params.event.roomId)?.roomSummary()?.toMatrixItem(), colorProvider = colorProvider, drawableProvider = drawableProvider, diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt index 7ada0c71f2..9ea0a634c5 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt @@ -25,6 +25,7 @@ import im.vector.app.R import im.vector.app.core.extensions.tintBackground import im.vector.app.core.resources.ColorProvider import im.vector.app.core.resources.DrawableProvider +import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.recording.VoiceBroadcastRecorder @@ -40,6 +41,8 @@ abstract class AbsMessageVoiceBroadcastItem<H : AbsMessageVoiceBroadcastItem.Hol protected val recorderName get() = voiceBroadcastAttributes.recorderName protected val recorder get() = voiceBroadcastAttributes.recorder protected val player get() = voiceBroadcastAttributes.player + protected val playbackTracker get() = voiceBroadcastAttributes.playbackTracker + protected val duration get() = voiceBroadcastAttributes.duration protected val roomItem get() = voiceBroadcastAttributes.roomItem protected val colorProvider get() = voiceBroadcastAttributes.colorProvider protected val drawableProvider get() = voiceBroadcastAttributes.drawableProvider @@ -98,6 +101,7 @@ abstract class AbsMessageVoiceBroadcastItem<H : AbsMessageVoiceBroadcastItem.Hol val recorderName: String, val recorder: VoiceBroadcastRecorder?, val player: VoiceBroadcastPlayer, + val playbackTracker: AudioMessagePlaybackTracker, val roomItem: MatrixItem?, val colorProvider: ColorProvider, val drawableProvider: DrawableProvider, diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index a2d1e30c99..ca643eb8c1 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -27,6 +27,7 @@ import com.airbnb.epoxy.EpoxyModelClass import im.vector.app.R import im.vector.app.core.epoxy.onClick import im.vector.app.features.home.room.detail.RoomDetailAction.VoiceBroadcastAction +import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer import im.vector.app.features.voicebroadcast.views.VoiceBroadcastMetadataView @@ -34,6 +35,7 @@ import im.vector.app.features.voicebroadcast.views.VoiceBroadcastMetadataView abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem<MessageVoiceBroadcastListeningItem.Holder>() { private lateinit var playerListener: VoiceBroadcastPlayer.Listener + private var isUserSeeking = false override fun bind(holder: Holder) { super.bind(holder) @@ -86,15 +88,36 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } private fun bindSeekBar(holder: Holder) { - holder.durationView.text = formatPlaybackTime(voiceBroadcastAttributes.duration) - holder.seekBar.max = voiceBroadcastAttributes.duration + holder.durationView.text = formatPlaybackTime(duration) + holder.seekBar.max = duration holder.seekBar.setOnSeekBarChangeListener(object : SeekBar.OnSeekBarChangeListener { override fun onProgressChanged(seekBar: SeekBar, progress: Int, fromUser: Boolean) = Unit - override fun onStartTrackingTouch(seekBar: SeekBar) = Unit + override fun onStartTrackingTouch(seekBar: SeekBar) { + isUserSeeking = true + } override fun onStopTrackingTouch(seekBar: SeekBar) { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcastId, seekBar.progress)) + isUserSeeking = false + } + }) + playbackTracker.track(voiceBroadcastId, object : AudioMessagePlaybackTracker.Listener { + override fun onUpdate(state: AudioMessagePlaybackTracker.Listener.State) { + when (state) { + is AudioMessagePlaybackTracker.Listener.State.Paused -> { + if (!isUserSeeking) { + holder.seekBar.progress = state.playbackTime + } + } + is AudioMessagePlaybackTracker.Listener.State.Playing -> { + if (!isUserSeeking) { + holder.seekBar.progress = state.playbackTime + } + } + AudioMessagePlaybackTracker.Listener.State.Idle -> Unit + is AudioMessagePlaybackTracker.Listener.State.Recording -> Unit + } } }) } @@ -105,6 +128,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem super.unbind(holder) player.removeListener(voiceBroadcastId, playerListener) holder.seekBar.setOnSeekBarChangeListener(null) + playbackTracker.untrack(voiceBroadcastId) } override fun getViewStubId() = STUB_ID diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 166e5a12e5..4fbaee8374 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -29,6 +29,7 @@ import im.vector.app.features.voicebroadcast.listening.usecase.GetLiveVoiceBroad import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.sequence import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastUseCase +import im.vector.lib.core.utils.timer.CountUpTimer import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -37,6 +38,7 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.room.model.message.MessageAudioContent import org.matrix.android.sdk.api.session.room.model.message.MessageAudioEvent @@ -60,6 +62,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private var voiceBroadcastStateJob: Job? = null private val mediaPlayerListener = MediaPlayerListener() + private val playbackTicker = PlaybackTicker() private var currentMediaPlayer: MediaPlayer? = null private var nextMediaPlayer: MediaPlayer? = null @@ -79,6 +82,24 @@ class VoiceBroadcastPlayerImpl @Inject constructor( field = value // Notify state change to all the listeners attached to the current voice broadcast id currentVoiceBroadcastId?.let { voiceBroadcastId -> + when (value) { + State.PLAYING -> { + playbackTracker.startPlayback(voiceBroadcastId) + playbackTicker.startPlaybackTicker(voiceBroadcastId) + } + State.PAUSED -> { + playbackTracker.pausePlayback(voiceBroadcastId) + playbackTicker.stopPlaybackTicker() + } + State.BUFFERING -> { + playbackTracker.pausePlayback(voiceBroadcastId) + playbackTicker.stopPlaybackTicker() + } + State.IDLE -> { + playbackTracker.stopPlayback(voiceBroadcastId) + playbackTicker.stopPlaybackTicker() + } + } listeners[voiceBroadcastId]?.forEach { listener -> listener.onStateChanged(value) } } } @@ -99,15 +120,16 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } override fun pause() { - currentMediaPlayer?.pause() - currentVoiceBroadcastId?.let { playbackTracker.pausePlayback(it) } playingState = State.PAUSED + currentMediaPlayer?.pause() } override fun stop() { + // Update state + playingState = State.IDLE + // Stop playback currentMediaPlayer?.stop() - currentVoiceBroadcastId?.let { playbackTracker.stopPlayback(it) } isLive = false // Release current player @@ -126,9 +148,6 @@ class VoiceBroadcastPlayerImpl @Inject constructor( fetchPlaylistJob?.cancel() fetchPlaylistJob = null - // Update state - playingState = State.IDLE - // Clear playlist playlist = emptyList() currentSequence = null @@ -218,7 +237,6 @@ class VoiceBroadcastPlayerImpl @Inject constructor( if (position > 0) { currentMediaPlayer?.seekTo(position) } - currentVoiceBroadcastId?.let { playbackTracker.startPlayback(it) } currentSequence = computedSequence withContext(Dispatchers.Main) { playingState = State.PLAYING } nextMediaPlayer = prepareNextMediaPlayer() @@ -231,7 +249,6 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun resumePlayback() { currentMediaPlayer?.start() - currentVoiceBroadcastId?.let { playbackTracker.startPlayback(it) } playingState = State.PLAYING } @@ -352,4 +369,45 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun getVoiceBroadcastDuration() = playlist.lastOrNull()?.let { it.startTime + it.audioEvent.duration } ?: 0 private data class PlaylistItem(val audioEvent: MessageAudioEvent, val startTime: Int) + + private inner class PlaybackTicker( + private var playbackTicker: CountUpTimer? = null, + ) { + + fun startPlaybackTicker(id: String) { + playbackTicker?.stop() + playbackTicker = CountUpTimer().apply { + tickListener = object : CountUpTimer.TickListener { + override fun onTick(milliseconds: Long) { + onPlaybackTick(id) + } + } + resume() + } + onPlaybackTick(id) + } + + private fun onPlaybackTick(id: String) { + if (currentMediaPlayer?.isPlaying.orFalse()) { + val itemStartPosition = currentSequence?.let { seq -> playlist.find { it.audioEvent.sequence == seq } }?.startTime + val currentVoiceBroadcastPosition = itemStartPosition?.plus(currentMediaPlayer?.currentPosition ?: 0) + if (currentVoiceBroadcastPosition != null) { + val totalDuration = getVoiceBroadcastDuration() + val percentage = currentVoiceBroadcastPosition.toFloat() / totalDuration + playbackTracker.updatePlayingAtPlaybackTime(id, currentVoiceBroadcastPosition, percentage) + } else { + playbackTracker.stopPlayback(id) + stopPlaybackTicker() + } + } else { + playbackTracker.stopPlayback(id) + stopPlaybackTicker() + } + } + + fun stopPlaybackTicker() { + playbackTicker?.stop() + playbackTicker = null + } + } } From 20d62b14dea44cacfa8c770a339bd37b3bcaaf14 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Tue, 1 Nov 2022 10:28:24 +0100 Subject: [PATCH 02/47] Changelog --- changelog.d/7496.wip | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7496.wip diff --git a/changelog.d/7496.wip b/changelog.d/7496.wip new file mode 100644 index 0000000000..49d15d084f --- /dev/null +++ b/changelog.d/7496.wip @@ -0,0 +1 @@ +[Voice Broadcast] Add seekbar in listening tile From 6d850b30306936b5b4602b48242c3524b5dfb730 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Thu, 3 Nov 2022 16:17:55 +0100 Subject: [PATCH 03/47] Create VoiceBroadcast model with roomId and eventId --- .../home/room/detail/RoomDetailAction.kt | 5 +- .../home/room/detail/TimelineViewModel.kt | 4 +- .../factory/VoiceBroadcastItemFactory.kt | 9 ++-- .../item/AbsMessageVoiceBroadcastItem.kt | 5 +- .../MessageVoiceBroadcastListeningItem.kt | 12 ++--- .../voicebroadcast/VoiceBroadcastHelper.kt | 7 +-- .../listening/VoiceBroadcastPlayer.kt | 16 +++--- .../listening/VoiceBroadcastPlayerImpl.kt | 49 +++++++++---------- .../GetLiveVoiceBroadcastChunksUseCase.kt | 15 +++--- .../voicebroadcast/model/VoiceBroadcast.kt | 22 +++++++++ ...se.kt => GetVoiceBroadcastEventUseCase.kt} | 14 +++--- 11 files changed, 93 insertions(+), 65 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/voicebroadcast/model/VoiceBroadcast.kt rename vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/{GetVoiceBroadcastUseCase.kt => GetVoiceBroadcastEventUseCase.kt} (72%) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt index 8c49213a42..ba0f7dbdf8 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt @@ -20,6 +20,7 @@ import android.net.Uri import android.view.View import im.vector.app.core.platform.VectorViewModelAction import im.vector.app.features.call.conference.ConferenceEvent +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import org.matrix.android.sdk.api.session.content.ContentAttachmentData import org.matrix.android.sdk.api.session.room.model.message.MessageStickerContent import org.matrix.android.sdk.api.session.room.model.message.MessageWithAttachmentContent @@ -129,10 +130,10 @@ sealed class RoomDetailAction : VectorViewModelAction { } sealed class Listening : VoiceBroadcastAction() { - data class PlayOrResume(val voiceBroadcastId: String) : Listening() + data class PlayOrResume(val voiceBroadcast: VoiceBroadcast) : Listening() object Pause : Listening() object Stop : Listening() - data class SeekTo(val voiceBroadcastId: String, val positionMillis: Int) : Listening() + data class SeekTo(val voiceBroadcast: VoiceBroadcast, val positionMillis: Int) : Listening() } } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt index 3f4fae1ce9..252823b2a6 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt @@ -634,10 +634,10 @@ class TimelineViewModel @AssistedInject constructor( VoiceBroadcastAction.Recording.Pause -> voiceBroadcastHelper.pauseVoiceBroadcast(room.roomId) VoiceBroadcastAction.Recording.Resume -> voiceBroadcastHelper.resumeVoiceBroadcast(room.roomId) VoiceBroadcastAction.Recording.Stop -> voiceBroadcastHelper.stopVoiceBroadcast(room.roomId) - is VoiceBroadcastAction.Listening.PlayOrResume -> voiceBroadcastHelper.playOrResumePlayback(room.roomId, action.voiceBroadcastId) + is VoiceBroadcastAction.Listening.PlayOrResume -> voiceBroadcastHelper.playOrResumePlayback(action.voiceBroadcast) VoiceBroadcastAction.Listening.Pause -> voiceBroadcastHelper.pausePlayback() VoiceBroadcastAction.Listening.Stop -> voiceBroadcastHelper.stopPlayback() - is VoiceBroadcastAction.Listening.SeekTo -> voiceBroadcastHelper.seekTo(action.voiceBroadcastId, action.positionMillis) + is VoiceBroadcastAction.Listening.SeekTo -> voiceBroadcastHelper.seekTo(action.voiceBroadcast, action.positionMillis) } } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt index 06d3563303..e4f7bed72f 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt @@ -29,6 +29,7 @@ import im.vector.app.features.home.room.detail.timeline.item.MessageVoiceBroadca import im.vector.app.features.home.room.detail.timeline.item.MessageVoiceBroadcastRecordingItem_ import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer import im.vector.app.features.voicebroadcast.model.MessageVoiceBroadcastInfoContent +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.model.asVoiceBroadcastEvent import im.vector.app.features.voicebroadcast.recording.VoiceBroadcastRecorder @@ -60,14 +61,14 @@ class VoiceBroadcastItemFactory @Inject constructor( val voiceBroadcastEventsGroup = params.eventsGroup?.let { VoiceBroadcastEventsGroup(it) } ?: return null val voiceBroadcastEvent = voiceBroadcastEventsGroup.getLastDisplayableEvent().root.asVoiceBroadcastEvent() ?: return null val voiceBroadcastContent = voiceBroadcastEvent.content ?: return null - val voiceBroadcastId = voiceBroadcastEventsGroup.voiceBroadcastId + val voiceBroadcast = VoiceBroadcast(voiceBroadcastId = voiceBroadcastEventsGroup.voiceBroadcastId, roomId = params.event.roomId) val isRecording = voiceBroadcastContent.voiceBroadcastState != VoiceBroadcastState.STOPPED && voiceBroadcastEvent.root.stateKey == session.myUserId && messageContent.deviceId == session.sessionParams.deviceId val voiceBroadcastAttributes = AbsMessageVoiceBroadcastItem.Attributes( - voiceBroadcastId = voiceBroadcastId, + voiceBroadcast = voiceBroadcast, voiceBroadcastState = voiceBroadcastContent.voiceBroadcastState, duration = voiceBroadcastEventsGroup.getDuration(), recorderName = params.event.root.stateKey?.let { session.getUserOrDefault(it) }?.toMatrixItem()?.getBestName().orEmpty(), @@ -92,7 +93,7 @@ class VoiceBroadcastItemFactory @Inject constructor( voiceBroadcastAttributes: AbsMessageVoiceBroadcastItem.Attributes, ): MessageVoiceBroadcastRecordingItem { return MessageVoiceBroadcastRecordingItem_() - .id("voice_broadcast_${voiceBroadcastAttributes.voiceBroadcastId}") + .id("voice_broadcast_${voiceBroadcastAttributes.voiceBroadcast.voiceBroadcastId}") .attributes(attributes) .voiceBroadcastAttributes(voiceBroadcastAttributes) .highlighted(highlight) @@ -105,7 +106,7 @@ class VoiceBroadcastItemFactory @Inject constructor( voiceBroadcastAttributes: AbsMessageVoiceBroadcastItem.Attributes, ): MessageVoiceBroadcastListeningItem { return MessageVoiceBroadcastListeningItem_() - .id("voice_broadcast_${voiceBroadcastAttributes.voiceBroadcastId}") + .id("voice_broadcast_${voiceBroadcastAttributes.voiceBroadcast.voiceBroadcastId}") .attributes(attributes) .voiceBroadcastAttributes(voiceBroadcastAttributes) .highlighted(highlight) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt index 9ea0a634c5..0329adf12b 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt @@ -27,6 +27,7 @@ import im.vector.app.core.resources.ColorProvider import im.vector.app.core.resources.DrawableProvider import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.recording.VoiceBroadcastRecorder import org.matrix.android.sdk.api.util.MatrixItem @@ -36,7 +37,7 @@ abstract class AbsMessageVoiceBroadcastItem<H : AbsMessageVoiceBroadcastItem.Hol @EpoxyAttribute lateinit var voiceBroadcastAttributes: Attributes - protected val voiceBroadcastId get() = voiceBroadcastAttributes.voiceBroadcastId + protected val voiceBroadcast get() = voiceBroadcastAttributes.voiceBroadcast protected val voiceBroadcastState get() = voiceBroadcastAttributes.voiceBroadcastState protected val recorderName get() = voiceBroadcastAttributes.recorderName protected val recorder get() = voiceBroadcastAttributes.recorder @@ -95,7 +96,7 @@ abstract class AbsMessageVoiceBroadcastItem<H : AbsMessageVoiceBroadcastItem.Hol } data class Attributes( - val voiceBroadcastId: String, + val voiceBroadcast: VoiceBroadcast, val voiceBroadcastState: VoiceBroadcastState?, val duration: Int, val recorderName: String, diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index ca643eb8c1..5e3cc6fba8 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -46,7 +46,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem playerListener = VoiceBroadcastPlayer.Listener { state -> renderPlayingState(holder, state) } - player.addListener(voiceBroadcastId, playerListener) + player.addListener(voiceBroadcast, playerListener) bindSeekBar(holder) } @@ -77,7 +77,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem VoiceBroadcastPlayer.State.PAUSED -> { playPauseButton.setImageResource(R.drawable.ic_play_pause_play) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_play_voice_broadcast) - playPauseButton.onClick { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcastId)) } + playPauseButton.onClick { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) } seekBar.isEnabled = false } VoiceBroadcastPlayer.State.BUFFERING -> { @@ -98,11 +98,11 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } override fun onStopTrackingTouch(seekBar: SeekBar) { - callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcastId, seekBar.progress)) + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcast, seekBar.progress)) isUserSeeking = false } }) - playbackTracker.track(voiceBroadcastId, object : AudioMessagePlaybackTracker.Listener { + playbackTracker.track(voiceBroadcast.voiceBroadcastId, object : AudioMessagePlaybackTracker.Listener { override fun onUpdate(state: AudioMessagePlaybackTracker.Listener.State) { when (state) { is AudioMessagePlaybackTracker.Listener.State.Paused -> { @@ -126,9 +126,9 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem override fun unbind(holder: Holder) { super.unbind(holder) - player.removeListener(voiceBroadcastId, playerListener) + player.removeListener(voiceBroadcast, playerListener) holder.seekBar.setOnSeekBarChangeListener(null) - playbackTracker.untrack(voiceBroadcastId) + playbackTracker.untrack(voiceBroadcast.voiceBroadcastId) } override fun getViewStubId() = STUB_ID diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt index 7864d3b4e3..6839056520 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt @@ -17,6 +17,7 @@ package im.vector.app.features.voicebroadcast import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import im.vector.app.features.voicebroadcast.recording.usecase.PauseVoiceBroadcastUseCase import im.vector.app.features.voicebroadcast.recording.usecase.ResumeVoiceBroadcastUseCase import im.vector.app.features.voicebroadcast.recording.usecase.StartVoiceBroadcastUseCase @@ -41,14 +42,14 @@ class VoiceBroadcastHelper @Inject constructor( suspend fun stopVoiceBroadcast(roomId: String) = stopVoiceBroadcastUseCase.execute(roomId) - fun playOrResumePlayback(roomId: String, voiceBroadcastId: String) = voiceBroadcastPlayer.playOrResume(roomId, voiceBroadcastId) + fun playOrResumePlayback(voiceBroadcast: VoiceBroadcast) = voiceBroadcastPlayer.playOrResume(voiceBroadcast) fun pausePlayback() = voiceBroadcastPlayer.pause() fun stopPlayback() = voiceBroadcastPlayer.stop() - fun seekTo(voiceBroadcastId: String, positionMillis: Int) { - if (voiceBroadcastPlayer.currentVoiceBroadcastId == voiceBroadcastId) { + fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int) { + if (voiceBroadcastPlayer.currentVoiceBroadcast == voiceBroadcast) { voiceBroadcastPlayer.seekTo(positionMillis) } } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt index 2a2a549af0..b4806ba57d 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt @@ -16,12 +16,14 @@ package im.vector.app.features.voicebroadcast.listening +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast + interface VoiceBroadcastPlayer { /** - * The current playing voice broadcast identifier, if any. + * The current playing voice broadcast, if any. */ - val currentVoiceBroadcastId: String? + val currentVoiceBroadcast: VoiceBroadcast? /** * The current playing [State], [State.IDLE] by default. @@ -31,7 +33,7 @@ interface VoiceBroadcastPlayer { /** * Start playback of the given voice broadcast. */ - fun playOrResume(roomId: String, voiceBroadcastId: String) + fun playOrResume(voiceBroadcast: VoiceBroadcast) /** * Pause playback of the current voice broadcast, if any. @@ -49,14 +51,14 @@ interface VoiceBroadcastPlayer { fun seekTo(positionMillis: Int) /** - * Add a [Listener] to the given voice broadcast id. + * Add a [Listener] to the given voice broadcast. */ - fun addListener(voiceBroadcastId: String, listener: Listener) + fun addListener(voiceBroadcast: VoiceBroadcast, listener: Listener) /** - * Remove a [Listener] from the given voice broadcast id. + * Remove a [Listener] from the given voice broadcast. */ - fun removeListener(voiceBroadcastId: String, listener: Listener) + fun removeListener(voiceBroadcast: VoiceBroadcast, listener: Listener) /** * Player states. diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 4fbaee8374..fc983e4112 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -26,9 +26,10 @@ import im.vector.app.features.voicebroadcast.duration import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.Listener import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.State import im.vector.app.features.voicebroadcast.listening.usecase.GetLiveVoiceBroadcastChunksUseCase +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.sequence -import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastUseCase +import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastEventUseCase import im.vector.lib.core.utils.timer.CountUpTimer import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -51,7 +52,7 @@ import javax.inject.Singleton class VoiceBroadcastPlayerImpl @Inject constructor( private val sessionHolder: ActiveSessionHolder, private val playbackTracker: AudioMessagePlaybackTracker, - private val getVoiceBroadcastUseCase: GetVoiceBroadcastUseCase, + private val getVoiceBroadcastEventUseCase: GetVoiceBroadcastEventUseCase, private val getLiveVoiceBroadcastChunksUseCase: GetLiveVoiceBroadcastChunksUseCase ) : VoiceBroadcastPlayer { @@ -73,7 +74,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private var isLive: Boolean = false - override var currentVoiceBroadcastId: String? = null + override var currentVoiceBroadcast: VoiceBroadcast? = null override var playingState = State.IDLE @MainThread @@ -81,7 +82,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( Timber.w("## VoiceBroadcastPlayer state: $field -> $value") field = value // Notify state change to all the listeners attached to the current voice broadcast id - currentVoiceBroadcastId?.let { voiceBroadcastId -> + currentVoiceBroadcast?.voiceBroadcastId?.let { voiceBroadcastId -> when (value) { State.PLAYING -> { playbackTracker.startPlayback(voiceBroadcastId) @@ -103,17 +104,16 @@ class VoiceBroadcastPlayerImpl @Inject constructor( listeners[voiceBroadcastId]?.forEach { listener -> listener.onStateChanged(value) } } } - private var currentRoomId: String? = null /** * Map voiceBroadcastId to listeners. */ private val listeners: MutableMap<String, CopyOnWriteArrayList<Listener>> = mutableMapOf() - override fun playOrResume(roomId: String, voiceBroadcastId: String) { - val hasChanged = currentVoiceBroadcastId != voiceBroadcastId + override fun playOrResume(voiceBroadcast: VoiceBroadcast) { + val hasChanged = currentVoiceBroadcast != voiceBroadcast when { - hasChanged -> startPlayback(roomId, voiceBroadcastId) + hasChanged -> startPlayback(voiceBroadcast) playingState == State.PAUSED -> resumePlayback() else -> Unit } @@ -152,37 +152,35 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playlist = emptyList() currentSequence = null - currentRoomId = null - currentVoiceBroadcastId = null + currentVoiceBroadcast = null } - override fun addListener(voiceBroadcastId: String, listener: Listener) { - listeners[voiceBroadcastId]?.add(listener) ?: run { - listeners[voiceBroadcastId] = CopyOnWriteArrayList<Listener>().apply { add(listener) } + override fun addListener(voiceBroadcast: VoiceBroadcast, listener: Listener) { + listeners[voiceBroadcast.voiceBroadcastId]?.add(listener) ?: run { + listeners[voiceBroadcast.voiceBroadcastId] = CopyOnWriteArrayList<Listener>().apply { add(listener) } } - if (voiceBroadcastId == currentVoiceBroadcastId) listener.onStateChanged(playingState) else listener.onStateChanged(State.IDLE) + listener.onStateChanged(if (voiceBroadcast == currentVoiceBroadcast) playingState else State.IDLE) } - override fun removeListener(voiceBroadcastId: String, listener: Listener) { - listeners[voiceBroadcastId]?.remove(listener) + override fun removeListener(voiceBroadcast: VoiceBroadcast, listener: Listener) { + listeners[voiceBroadcast.voiceBroadcastId]?.remove(listener) } - private fun startPlayback(roomId: String, eventId: String) { + private fun startPlayback(voiceBroadcast: VoiceBroadcast) { // Stop listening previous voice broadcast if any if (playingState != State.IDLE) stop() - currentRoomId = roomId - currentVoiceBroadcastId = eventId + currentVoiceBroadcast = voiceBroadcast playingState = State.BUFFERING - val voiceBroadcastState = getVoiceBroadcastUseCase.execute(roomId, eventId)?.content?.voiceBroadcastState + val voiceBroadcastState = getVoiceBroadcastEventUseCase.execute(voiceBroadcast)?.content?.voiceBroadcastState isLive = voiceBroadcastState != null && voiceBroadcastState != VoiceBroadcastState.STOPPED - fetchPlaylistAndStartPlayback(roomId, eventId) + fetchPlaylistAndStartPlayback(voiceBroadcast) } - private fun fetchPlaylistAndStartPlayback(roomId: String, voiceBroadcastId: String) { - fetchPlaylistJob = getLiveVoiceBroadcastChunksUseCase.execute(roomId, voiceBroadcastId) + private fun fetchPlaylistAndStartPlayback(voiceBroadcast: VoiceBroadcast) { + fetchPlaylistJob = getLiveVoiceBroadcastChunksUseCase.execute(voiceBroadcast) .onEach(this::updatePlaylist) .launchIn(coroutineScope) } @@ -347,9 +345,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override fun onCompletion(mp: MediaPlayer) { if (nextMediaPlayer != null) return - val roomId = currentRoomId ?: return - val voiceBroadcastId = currentVoiceBroadcastId ?: return - val voiceBroadcastEventContent = getVoiceBroadcastUseCase.execute(roomId, voiceBroadcastId)?.content ?: return + val voiceBroadcast = currentVoiceBroadcast ?: return + val voiceBroadcastEventContent = getVoiceBroadcastEventUseCase.execute(voiceBroadcast)?.content ?: return isLive = voiceBroadcastEventContent.voiceBroadcastState != null && voiceBroadcastEventContent.voiceBroadcastState != VoiceBroadcastState.STOPPED if (!isLive && voiceBroadcastEventContent.lastChunkSequence == currentSequence) { diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt index 4f9f2de673..2e8fc31870 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt @@ -19,11 +19,12 @@ package im.vector.app.features.voicebroadcast.listening.usecase import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.features.voicebroadcast.getVoiceBroadcastEventId import im.vector.app.features.voicebroadcast.isVoiceBroadcast +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.model.asVoiceBroadcastEvent import im.vector.app.features.voicebroadcast.sequence -import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastUseCase +import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastEventUseCase import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow @@ -44,19 +45,19 @@ import javax.inject.Inject */ class GetLiveVoiceBroadcastChunksUseCase @Inject constructor( private val activeSessionHolder: ActiveSessionHolder, - private val getVoiceBroadcastUseCase: GetVoiceBroadcastUseCase, + private val getVoiceBroadcastEventUseCase: GetVoiceBroadcastEventUseCase, ) { - fun execute(roomId: String, voiceBroadcastId: String): Flow<List<MessageAudioEvent>> { + fun execute(voiceBroadcast: VoiceBroadcast): Flow<List<MessageAudioEvent>> { val session = activeSessionHolder.getSafeActiveSession() ?: return emptyFlow() - val room = session.roomService().getRoom(roomId) ?: return emptyFlow() + val room = session.roomService().getRoom(voiceBroadcast.roomId) ?: return emptyFlow() val timeline = room.timelineService().createTimeline(null, TimelineSettings(5)) // Get initial chunks - val existingChunks = room.timelineService().getTimelineEventsRelatedTo(RelationType.REFERENCE, voiceBroadcastId) + val existingChunks = room.timelineService().getTimelineEventsRelatedTo(RelationType.REFERENCE, voiceBroadcast.voiceBroadcastId) .mapNotNull { timelineEvent -> timelineEvent.root.asMessageAudioEvent().takeIf { it.isVoiceBroadcast() } } - val voiceBroadcastEvent = getVoiceBroadcastUseCase.execute(roomId, voiceBroadcastId) + val voiceBroadcastEvent = getVoiceBroadcastEventUseCase.execute(voiceBroadcast) val voiceBroadcastState = voiceBroadcastEvent?.content?.voiceBroadcastState return if (voiceBroadcastState == null || voiceBroadcastState == VoiceBroadcastState.STOPPED) { @@ -82,7 +83,7 @@ class GetLiveVoiceBroadcastChunksUseCase @Inject constructor( lastSequence = stopEvent.content?.lastChunkSequence } - val newChunks = newEvents.mapToChunkEvents(voiceBroadcastId, voiceBroadcastEvent.root.senderId) + val newChunks = newEvents.mapToChunkEvents(voiceBroadcast.voiceBroadcastId, voiceBroadcastEvent.root.senderId) // Notify about new chunks if (newChunks.isNotEmpty()) { diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/model/VoiceBroadcast.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/model/VoiceBroadcast.kt new file mode 100644 index 0000000000..62207d5b87 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/model/VoiceBroadcast.kt @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.voicebroadcast.model + +data class VoiceBroadcast( + val voiceBroadcastId: String, + val roomId: String, +) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastUseCase.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt similarity index 72% rename from vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastUseCase.kt rename to vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt index d08fa14a95..26ba3209b7 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt @@ -16,6 +16,7 @@ package im.vector.app.features.voicebroadcast.usecase +import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent import im.vector.app.features.voicebroadcast.model.asVoiceBroadcastEvent import org.matrix.android.sdk.api.session.Session @@ -24,17 +25,18 @@ import org.matrix.android.sdk.api.session.getRoom import timber.log.Timber import javax.inject.Inject -class GetVoiceBroadcastUseCase @Inject constructor( +class GetVoiceBroadcastEventUseCase @Inject constructor( private val session: Session, ) { - fun execute(roomId: String, eventId: String): VoiceBroadcastEvent? { - val room = session.getRoom(roomId) ?: error("Unknown roomId: $roomId") + fun execute(voiceBroadcast: VoiceBroadcast): VoiceBroadcastEvent? { + val room = session.getRoom(voiceBroadcast.roomId) ?: error("Unknown roomId: ${voiceBroadcast.roomId}") - Timber.d("## GetVoiceBroadcastUseCase: get voice broadcast $eventId") + Timber.d("## GetVoiceBroadcastUseCase: get voice broadcast $voiceBroadcast") - val initialEvent = room.timelineService().getTimelineEvent(eventId)?.root?.asVoiceBroadcastEvent() // Fallback to initial event - val relatedEvents = room.timelineService().getTimelineEventsRelatedTo(RelationType.REFERENCE, eventId).sortedBy { it.root.originServerTs } + val initialEvent = room.timelineService().getTimelineEvent(voiceBroadcast.voiceBroadcastId)?.root?.asVoiceBroadcastEvent() + val relatedEvents = room.timelineService().getTimelineEventsRelatedTo(RelationType.REFERENCE, voiceBroadcast.voiceBroadcastId) + .sortedBy { it.root.originServerTs } return relatedEvents.mapNotNull { it.root.asVoiceBroadcastEvent() }.lastOrNull() ?: initialEvent } } From 44c0378de8b59a69a7d585bfb83cda8418273205 Mon Sep 17 00:00:00 2001 From: Onuray Sahin <onurays@element.io> Date: Mon, 7 Nov 2022 14:46:32 +0300 Subject: [PATCH 04/47] Fix description of verified sessions. --- library/ui-strings/src/main/res/values/strings.xml | 2 ++ .../devices/v2/othersessions/OtherSessionsFragment.kt | 5 ++++- .../settings/devices/v2/overview/SessionOverviewFragment.kt | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index 450eb64849..370005363d 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -3373,7 +3373,9 @@ <string name="device_manager_learn_more_sessions_unverified_title">Unverified sessions</string> <string name="device_manager_learn_more_sessions_unverified">Unverified sessions are sessions that have logged in with your credentials but not been cross-verified.\n\nYou should make especially certain that you recognise these sessions as they could represent an unauthorised use of your account.</string> <string name="device_manager_learn_more_sessions_verified_title">Verified sessions</string> + <!-- TODO TO BE REMOVED --> <string name="device_manager_learn_more_sessions_verified">Verified sessions have logged in with your credentials and then been verified, either using your secure passphrase or by cross-verifying.\n\nThis means they hold encryption keys for your previous messages, and confirm to other users you are communicating with that these sessions are really you.</string> + <string name="device_manager_learn_more_sessions_verified_description">Verified sessions are anywhere you are using ${app_name} after entering your passphrase or confirming your identity with another verified session.\n\nThis means that you have all the keys needed to unlock your encrypted messages and confirm to other users that you trust this session.</string> <string name="device_manager_learn_more_session_rename_title">Renaming sessions</string> <string name="device_manager_learn_more_session_rename">Other users in direct messages and rooms that you join are able to view a full list of your sessions.\n\nThis provides them with confidence that they are really speaking to you, but it also means they can see the session name you enter here.</string> <string name="labs_enable_session_manager_title">Enable new session manager</string> diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsFragment.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsFragment.kt index 4f1c8353f5..c5c7ab634c 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsFragment.kt @@ -196,7 +196,10 @@ class OtherSessionsFragment : ) ) views.otherSessionsNotFoundTextView.text = getString(R.string.device_manager_other_sessions_no_verified_sessions_found) - updateSecurityLearnMoreButton(R.string.device_manager_learn_more_sessions_verified_title, R.string.device_manager_learn_more_sessions_verified) + updateSecurityLearnMoreButton( + R.string.device_manager_learn_more_sessions_verified_title, + R.string.device_manager_learn_more_sessions_verified_description + ) } DeviceManagerFilterType.UNVERIFIED -> { views.otherSessionsSecurityRecommendationView.render( diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewFragment.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewFragment.kt index 620372f810..c1d332fd23 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewFragment.kt @@ -284,7 +284,7 @@ class SessionOverviewFragment : R.string.device_manager_verification_status_unverified } val descriptionResId = if (isVerified) { - R.string.device_manager_learn_more_sessions_verified + R.string.device_manager_learn_more_sessions_verified_description } else { R.string.device_manager_learn_more_sessions_unverified } From e30cbd5916c25916da438dd2afed5aff68f1edb9 Mon Sep 17 00:00:00 2001 From: Onuray Sahin <onurays@element.io> Date: Mon, 7 Nov 2022 14:51:18 +0300 Subject: [PATCH 05/47] Add changelog. --- changelog.d/7533.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7533.bugfix diff --git a/changelog.d/7533.bugfix b/changelog.d/7533.bugfix new file mode 100644 index 0000000000..5e603ece22 --- /dev/null +++ b/changelog.d/7533.bugfix @@ -0,0 +1 @@ +Fix description of verified sessions From d89ef6988b6e894040da5e447cb781903b565767 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 11:35:50 +0100 Subject: [PATCH 06/47] Improve player seek --- .../MessageVoiceBroadcastListeningItem.kt | 7 +- .../VoiceBroadcastExtensions.kt | 5 + .../voicebroadcast/VoiceBroadcastHelper.kt | 4 +- .../listening/VoiceBroadcastPlayer.kt | 4 +- .../listening/VoiceBroadcastPlayerImpl.kt | 228 +++++++++--------- .../GetLiveVoiceBroadcastChunksUseCase.kt | 5 +- .../usecase/GetVoiceBroadcastEventUseCase.kt | 35 ++- 7 files changed, 157 insertions(+), 131 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index 5e3cc6fba8..19caf3d8ba 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -71,18 +71,14 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem playPauseButton.setImageResource(R.drawable.ic_play_pause_pause) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_pause_voice_broadcast) playPauseButton.onClick { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) } - seekBar.isEnabled = true } VoiceBroadcastPlayer.State.IDLE, VoiceBroadcastPlayer.State.PAUSED -> { playPauseButton.setImageResource(R.drawable.ic_play_pause_play) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_play_voice_broadcast) playPauseButton.onClick { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) } - seekBar.isEnabled = false - } - VoiceBroadcastPlayer.State.BUFFERING -> { - seekBar.isEnabled = true } + VoiceBroadcastPlayer.State.BUFFERING -> Unit } } } @@ -112,6 +108,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } is AudioMessagePlaybackTracker.Listener.State.Playing -> { if (!isUserSeeking) { +// Timber.d("Voice Broadcast | AudioMessagePlaybackTracker.Listener.onUpdate - duration: $duration, playbackTime: ${state.playbackTime}") holder.seekBar.progress = state.playbackTime } } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt index 48554f51d0..a1328c0ba3 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt @@ -17,6 +17,8 @@ package im.vector.app.features.voicebroadcast import im.vector.app.features.voicebroadcast.model.VoiceBroadcastChunk +import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent +import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import org.matrix.android.sdk.api.session.events.model.Content import org.matrix.android.sdk.api.session.events.model.getRelationContent import org.matrix.android.sdk.api.session.events.model.toModel @@ -34,3 +36,6 @@ fun MessageAudioEvent.getVoiceBroadcastChunk(): VoiceBroadcastChunk? { val MessageAudioEvent.sequence: Int? get() = getVoiceBroadcastChunk()?.sequence val MessageAudioEvent.duration get() = content.audioInfo?.duration ?: content.audioWaveformInfo?.duration ?: 0 + +val VoiceBroadcastEvent.isLive + get() = content?.voiceBroadcastState != null && content?.voiceBroadcastState != VoiceBroadcastState.STOPPED diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt index 6839056520..3661928fa5 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt @@ -49,8 +49,6 @@ class VoiceBroadcastHelper @Inject constructor( fun stopPlayback() = voiceBroadcastPlayer.stop() fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int) { - if (voiceBroadcastPlayer.currentVoiceBroadcast == voiceBroadcast) { - voiceBroadcastPlayer.seekTo(positionMillis) - } + voiceBroadcastPlayer.seekTo(voiceBroadcast, positionMillis) } } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt index b4806ba57d..36e75236ad 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt @@ -46,9 +46,9 @@ interface VoiceBroadcastPlayer { fun stop() /** - * Seek to the given playback position, is milliseconds. + * Seek the given voice broadcast playback to the given position, is milliseconds. */ - fun seekTo(positionMillis: Int) + fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int) /** * Add a [Listener] to the given voice broadcast. diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index fc983e4112..773883d81a 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -18,16 +18,18 @@ package im.vector.app.features.voicebroadcast.listening import android.media.AudioAttributes import android.media.MediaPlayer +import android.media.MediaPlayer.OnPreparedListener import androidx.annotation.MainThread import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.voice.VoiceFailure import im.vector.app.features.voicebroadcast.duration +import im.vector.app.features.voicebroadcast.isLive import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.Listener import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.State import im.vector.app.features.voicebroadcast.listening.usecase.GetLiveVoiceBroadcastChunksUseCase import im.vector.app.features.voicebroadcast.model.VoiceBroadcast -import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState +import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent import im.vector.app.features.voicebroadcast.sequence import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastEventUseCase import im.vector.lib.core.utils.timer.CountUpTimer @@ -38,7 +40,6 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.room.model.message.MessageAudioContent @@ -47,6 +48,7 @@ import timber.log.Timber import java.util.concurrent.CopyOnWriteArrayList import javax.inject.Inject import javax.inject.Singleton +import kotlin.math.absoluteValue @Singleton class VoiceBroadcastPlayerImpl @Inject constructor( @@ -60,19 +62,20 @@ class VoiceBroadcastPlayerImpl @Inject constructor( get() = sessionHolder.getActiveSession() private val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) - private var voiceBroadcastStateJob: Job? = null + private var fetchPlaylistTask: Job? = null + private var voiceBroadcastStateTask: Job? = null private val mediaPlayerListener = MediaPlayerListener() private val playbackTicker = PlaybackTicker() private var currentMediaPlayer: MediaPlayer? = null private var nextMediaPlayer: MediaPlayer? = null - private var currentSequence: Int? = null - private var fetchPlaylistJob: Job? = null private var playlist = emptyList<PlaylistItem>() - - private var isLive: Boolean = false + private var currentSequence: Int? = null + private var currentVoiceBroadcastEvent: VoiceBroadcastEvent? = null + private val isLive get() = currentVoiceBroadcastEvent?.isLive.orFalse() + private val lastSequence get() = currentVoiceBroadcastEvent?.content?.lastChunkSequence override var currentVoiceBroadcast: VoiceBroadcast? = null @@ -81,33 +84,10 @@ class VoiceBroadcastPlayerImpl @Inject constructor( set(value) { Timber.w("## VoiceBroadcastPlayer state: $field -> $value") field = value - // Notify state change to all the listeners attached to the current voice broadcast id - currentVoiceBroadcast?.voiceBroadcastId?.let { voiceBroadcastId -> - when (value) { - State.PLAYING -> { - playbackTracker.startPlayback(voiceBroadcastId) - playbackTicker.startPlaybackTicker(voiceBroadcastId) - } - State.PAUSED -> { - playbackTracker.pausePlayback(voiceBroadcastId) - playbackTicker.stopPlaybackTicker() - } - State.BUFFERING -> { - playbackTracker.pausePlayback(voiceBroadcastId) - playbackTicker.stopPlaybackTicker() - } - State.IDLE -> { - playbackTracker.stopPlayback(voiceBroadcastId) - playbackTicker.stopPlaybackTicker() - } - } - listeners[voiceBroadcastId]?.forEach { listener -> listener.onStateChanged(value) } - } + onPlayingStateChanged(value) } - /** - * Map voiceBroadcastId to listeners. - */ + /** Map voiceBroadcastId to listeners.*/ private val listeners: MutableMap<String, CopyOnWriteArrayList<Listener>> = mutableMapOf() override fun playOrResume(voiceBroadcast: VoiceBroadcast) { @@ -120,38 +100,28 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } override fun pause() { - playingState = State.PAUSED currentMediaPlayer?.pause() + playingState = State.PAUSED } override fun stop() { // Update state playingState = State.IDLE - // Stop playback - currentMediaPlayer?.stop() - isLive = false + // Stop and release media players + stopPlayer() - // Release current player - release(currentMediaPlayer) - currentMediaPlayer = null - - // Release next player - release(nextMediaPlayer) - nextMediaPlayer = null - - // Do not observe anymore voice broadcast state changes - voiceBroadcastStateJob?.cancel() - voiceBroadcastStateJob = null - - // Do not fetch the playlist anymore - fetchPlaylistJob?.cancel() - fetchPlaylistJob = null + // Do not observe anymore voice broadcast changes + fetchPlaylistTask?.cancel() + fetchPlaylistTask = null + voiceBroadcastStateTask?.cancel() + voiceBroadcastStateTask = null // Clear playlist playlist = emptyList() currentSequence = null + currentVoiceBroadcastEvent = null currentVoiceBroadcast = null } @@ -174,13 +144,18 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playingState = State.BUFFERING - val voiceBroadcastState = getVoiceBroadcastEventUseCase.execute(voiceBroadcast)?.content?.voiceBroadcastState - isLive = voiceBroadcastState != null && voiceBroadcastState != VoiceBroadcastState.STOPPED + observeVoiceBroadcastLiveState(voiceBroadcast) fetchPlaylistAndStartPlayback(voiceBroadcast) } + private fun observeVoiceBroadcastLiveState(voiceBroadcast: VoiceBroadcast) { + voiceBroadcastStateTask = getVoiceBroadcastEventUseCase.execute(voiceBroadcast) + .onEach { currentVoiceBroadcastEvent = it.getOrNull() } + .launchIn(coroutineScope) + } + private fun fetchPlaylistAndStartPlayback(voiceBroadcast: VoiceBroadcast) { - fetchPlaylistJob = getLiveVoiceBroadcastChunksUseCase.execute(voiceBroadcast) + fetchPlaylistTask = getLiveVoiceBroadcastChunksUseCase.execute(voiceBroadcast) .onEach(this::updatePlaylist) .launchIn(coroutineScope) } @@ -204,40 +179,51 @@ class VoiceBroadcastPlayerImpl @Inject constructor( when (playingState) { State.PLAYING -> { if (nextMediaPlayer == null) { - coroutineScope.launch { nextMediaPlayer = prepareNextMediaPlayer() } + prepareNextMediaPlayer() } } State.PAUSED -> { if (nextMediaPlayer == null) { - coroutineScope.launch { nextMediaPlayer = prepareNextMediaPlayer() } + prepareNextMediaPlayer() } } State.BUFFERING -> { val newMediaContent = getNextAudioContent() - if (newMediaContent != null) startPlayback() + if (newMediaContent != null) { + val savedPosition = currentVoiceBroadcast?.let { playbackTracker.getPlaybackTime(it.voiceBroadcastId) } + startPlayback(savedPosition) + } + } + State.IDLE -> { + val savedPosition = currentVoiceBroadcast?.let { playbackTracker.getPlaybackTime(it.voiceBroadcastId) } + startPlayback(savedPosition) } - State.IDLE -> startPlayback() } } - private fun startPlayback(sequence: Int? = null, position: Int = 0) { + private fun startPlayback(position: Int? = null) { + stopPlayer() + val playlistItem = when { - sequence != null -> playlist.find { it.audioEvent.sequence == sequence } + position != null -> playlist.lastOrNull { it.startTime <= position } isLive -> playlist.lastOrNull() else -> playlist.firstOrNull() } val content = playlistItem?.audioEvent?.content ?: run { Timber.w("## VoiceBroadcastPlayer: No content to play"); return } - val computedSequence = playlistItem.audioEvent.sequence + val sequence = playlistItem.audioEvent.sequence + val sequencePosition = position?.let { it - playlistItem.startTime } ?: 0 coroutineScope.launch { try { - currentMediaPlayer = prepareMediaPlayer(content) - currentMediaPlayer?.start() - if (position > 0) { - currentMediaPlayer?.seekTo(position) + prepareMediaPlayer(content) { mp -> + currentMediaPlayer = mp + currentSequence = sequence + mp.start() + if (sequencePosition > 0) { + mp.seekTo(sequencePosition) + } + playingState = State.PLAYING + prepareNextMediaPlayer() } - currentSequence = computedSequence - withContext(Dispatchers.Main) { playingState = State.PLAYING } - nextMediaPlayer = prepareNextMediaPlayer() } catch (failure: Throwable) { Timber.e(failure, "Unable to start playback") throw VoiceFailure.UnableToPlay(failure) @@ -250,20 +236,12 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playingState = State.PLAYING } - override fun seekTo(positionMillis: Int) { - val duration = getVoiceBroadcastDuration() - val playlistItem = playlist.lastOrNull { it.startTime <= positionMillis } ?: return - val audioEvent = playlistItem.audioEvent - val eventPosition = positionMillis - playlistItem.startTime - - Timber.d("## Voice Broadcast | seekTo - duration=$duration, position=$positionMillis, sequence=${audioEvent.sequence}, sequencePosition=$eventPosition") - - tryOrNull { currentMediaPlayer?.stop() } - release(currentMediaPlayer) - tryOrNull { nextMediaPlayer?.stop() } - release(nextMediaPlayer) - - startPlayback(audioEvent.sequence, eventPosition) + override fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int) { + if (voiceBroadcast != currentVoiceBroadcast) { + playbackTracker.updatePausedAtPlaybackTime(voiceBroadcast.voiceBroadcastId, positionMillis, 0f) + } else { + startPlayback(positionMillis) + } } private fun getNextAudioContent(): MessageAudioContent? { @@ -273,12 +251,24 @@ class VoiceBroadcastPlayerImpl @Inject constructor( return playlist.find { it.audioEvent.sequence == nextSequence }?.audioEvent?.content } - private suspend fun prepareNextMediaPlayer(): MediaPlayer? { - val nextContent = getNextAudioContent() ?: return null - return prepareMediaPlayer(nextContent) + private fun prepareNextMediaPlayer() { + nextMediaPlayer = null + val nextContent = getNextAudioContent() + if (nextContent != null) { + coroutineScope.launch { + prepareMediaPlayer(nextContent) { mp -> + if (nextMediaPlayer == null) { + nextMediaPlayer = mp + currentMediaPlayer?.setNextMediaPlayer(mp) + } else { + mp.release() + } + } + } + } } - private suspend fun prepareMediaPlayer(messageAudioContent: MessageAudioContent): MediaPlayer { + private suspend fun prepareMediaPlayer(messageAudioContent: MessageAudioContent, onPreparedListener: OnPreparedListener): MediaPlayer { // Download can fail val audioFile = try { session.fileService().downloadFile(messageAudioContent) @@ -299,57 +289,55 @@ class VoiceBroadcastPlayerImpl @Inject constructor( setDataSource(fis.fd) setOnInfoListener(mediaPlayerListener) setOnErrorListener(mediaPlayerListener) + setOnPreparedListener(onPreparedListener) setOnCompletionListener(mediaPlayerListener) prepare() } } } - private fun release(mp: MediaPlayer?) { - mp?.apply { - release() - setOnInfoListener(null) - setOnCompletionListener(null) - setOnErrorListener(null) + private fun stopPlayer() { + tryOrNull { currentMediaPlayer?.stop() } + currentMediaPlayer?.release() + currentMediaPlayer = null + + nextMediaPlayer?.release() + nextMediaPlayer = null + } + + private fun onPlayingStateChanged(playingState: State) { + // Notify state change to all the listeners attached to the current voice broadcast id + currentVoiceBroadcast?.voiceBroadcastId?.let { voiceBroadcastId -> + when (playingState) { + State.PLAYING -> playbackTicker.startPlaybackTicker(voiceBroadcastId) + State.PAUSED, + State.BUFFERING, + State.IDLE -> playbackTicker.stopPlaybackTicker(voiceBroadcastId) + } + listeners[voiceBroadcastId]?.forEach { listener -> listener.onStateChanged(playingState) } } } private inner class MediaPlayerListener : MediaPlayer.OnInfoListener, - MediaPlayer.OnPreparedListener, MediaPlayer.OnCompletionListener, MediaPlayer.OnErrorListener { override fun onInfo(mp: MediaPlayer, what: Int, extra: Int): Boolean { when (what) { MediaPlayer.MEDIA_INFO_STARTED_AS_NEXT -> { - release(currentMediaPlayer) - currentMediaPlayer = mp currentSequence = currentSequence?.plus(1) - coroutineScope.launch { nextMediaPlayer = prepareNextMediaPlayer() } + currentMediaPlayer = mp + prepareNextMediaPlayer() } } return false } - override fun onPrepared(mp: MediaPlayer) { - when (mp) { - currentMediaPlayer -> { - nextMediaPlayer?.let { mp.setNextMediaPlayer(it) } - } - nextMediaPlayer -> { - tryOrNull { currentMediaPlayer?.setNextMediaPlayer(mp) } - } - } - } - override fun onCompletion(mp: MediaPlayer) { if (nextMediaPlayer != null) return - val voiceBroadcast = currentVoiceBroadcast ?: return - val voiceBroadcastEventContent = getVoiceBroadcastEventUseCase.execute(voiceBroadcast)?.content ?: return - isLive = voiceBroadcastEventContent.voiceBroadcastState != null && voiceBroadcastEventContent.voiceBroadcastState != VoiceBroadcastState.STOPPED - if (!isLive && voiceBroadcastEventContent.lastChunkSequence == currentSequence) { + if (!isLive && lastSequence == currentSequence) { // We'll not receive new chunks anymore so we can stop the live listening stop() } else { @@ -388,23 +376,31 @@ class VoiceBroadcastPlayerImpl @Inject constructor( if (currentMediaPlayer?.isPlaying.orFalse()) { val itemStartPosition = currentSequence?.let { seq -> playlist.find { it.audioEvent.sequence == seq } }?.startTime val currentVoiceBroadcastPosition = itemStartPosition?.plus(currentMediaPlayer?.currentPosition ?: 0) + Timber.d("Voice Broadcast | VoiceBroadcastPlayerImpl - sequence: $currentSequence, itemStartPosition $itemStartPosition, currentMediaPlayer=$currentMediaPlayer, currentMediaPlayer?.currentPosition: ${currentMediaPlayer?.currentPosition}") if (currentVoiceBroadcastPosition != null) { val totalDuration = getVoiceBroadcastDuration() val percentage = currentVoiceBroadcastPosition.toFloat() / totalDuration playbackTracker.updatePlayingAtPlaybackTime(id, currentVoiceBroadcastPosition, percentage) } else { - playbackTracker.stopPlayback(id) - stopPlaybackTicker() + stopPlaybackTicker(id) } } else { - playbackTracker.stopPlayback(id) - stopPlaybackTicker() + stopPlaybackTicker(id) } } - fun stopPlaybackTicker() { + fun stopPlaybackTicker(id: String) { playbackTicker?.stop() playbackTicker = null + + val totalDuration = getVoiceBroadcastDuration() + val playbackTime = playbackTracker.getPlaybackTime(id) + val remainingTime = totalDuration - playbackTime + if (remainingTime < 1000) { + playbackTracker.updatePausedAtPlaybackTime(id, 0, 0f) + } else { + playbackTracker.pausePlayback(id) + } } } } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt index 2e8fc31870..33e370e9bc 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt @@ -29,9 +29,12 @@ import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.lastOrNull import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.runningReduce +import kotlinx.coroutines.runBlocking import org.matrix.android.sdk.api.session.events.model.RelationType import org.matrix.android.sdk.api.session.room.model.message.MessageAudioEvent import org.matrix.android.sdk.api.session.room.model.message.asMessageAudioEvent @@ -57,7 +60,7 @@ class GetLiveVoiceBroadcastChunksUseCase @Inject constructor( val existingChunks = room.timelineService().getTimelineEventsRelatedTo(RelationType.REFERENCE, voiceBroadcast.voiceBroadcastId) .mapNotNull { timelineEvent -> timelineEvent.root.asMessageAudioEvent().takeIf { it.isVoiceBroadcast() } } - val voiceBroadcastEvent = getVoiceBroadcastEventUseCase.execute(voiceBroadcast) + val voiceBroadcastEvent = runBlocking { getVoiceBroadcastEventUseCase.execute(voiceBroadcast).firstOrNull()?.getOrNull() } val voiceBroadcastState = voiceBroadcastEvent?.content?.voiceBroadcastState return if (voiceBroadcastState == null || voiceBroadcastState == VoiceBroadcastState.STOPPED) { diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt index 26ba3209b7..7106322f06 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt @@ -16,12 +16,26 @@ package im.vector.app.features.voicebroadcast.usecase +import im.vector.app.features.voicebroadcast.VoiceBroadcastConstants import im.vector.app.features.voicebroadcast.model.VoiceBroadcast import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent +import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.model.asVoiceBroadcastEvent +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.mapNotNull +import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.events.model.RelationType import org.matrix.android.sdk.api.session.getRoom +import org.matrix.android.sdk.api.util.Optional +import org.matrix.android.sdk.api.util.toOptional +import org.matrix.android.sdk.flow.flow +import org.matrix.android.sdk.flow.unwrap import timber.log.Timber import javax.inject.Inject @@ -29,14 +43,27 @@ class GetVoiceBroadcastEventUseCase @Inject constructor( private val session: Session, ) { - fun execute(voiceBroadcast: VoiceBroadcast): VoiceBroadcastEvent? { + fun execute(voiceBroadcast: VoiceBroadcast): Flow<Optional<VoiceBroadcastEvent>> { val room = session.getRoom(voiceBroadcast.roomId) ?: error("Unknown roomId: ${voiceBroadcast.roomId}") Timber.d("## GetVoiceBroadcastUseCase: get voice broadcast $voiceBroadcast") val initialEvent = room.timelineService().getTimelineEvent(voiceBroadcast.voiceBroadcastId)?.root?.asVoiceBroadcastEvent() - val relatedEvents = room.timelineService().getTimelineEventsRelatedTo(RelationType.REFERENCE, voiceBroadcast.voiceBroadcastId) - .sortedBy { it.root.originServerTs } - return relatedEvents.mapNotNull { it.root.asVoiceBroadcastEvent() }.lastOrNull() ?: initialEvent + val latestEvent = room.timelineService().getTimelineEventsRelatedTo(RelationType.REFERENCE, voiceBroadcast.voiceBroadcastId) + .mapNotNull { it.root.asVoiceBroadcastEvent() } + .maxByOrNull { it.root.originServerTs ?: 0 } + ?: initialEvent + + return when (latestEvent?.content?.voiceBroadcastState) { + null, VoiceBroadcastState.STOPPED -> flowOf(latestEvent.toOptional()) + else -> { + room.flow() + .liveStateEvent(VoiceBroadcastConstants.STATE_ROOM_VOICE_BROADCAST_INFO, QueryStringValue.Equals(latestEvent.root.stateKey.orEmpty())) + .unwrap() + .mapNotNull { it.asVoiceBroadcastEvent() } + .filter { it.reference?.eventId == voiceBroadcast.voiceBroadcastId } + .map { it.toOptional() } + } + } } } From 392fe6fa329d84805f553986b8a338018ee2c095 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 15:47:10 +0100 Subject: [PATCH 07/47] Transform TickListener to fun interface --- .../vector/lib/attachmentviewer/VideoViewHolder.kt | 14 ++++++-------- .../im/vector/lib/core/utils/timer/CountUpTimer.kt | 2 +- .../vector/app/features/call/webrtc/WebRtcCall.kt | 10 ++++------ .../room/detail/composer/AudioMessageHelper.kt | 12 ++---------- .../composer/voice/VoiceMessageRecorderView.kt | 8 +++----- .../location/live/map/LiveLocationUserItem.kt | 6 ++---- .../listening/VoiceBroadcastPlayerImpl.kt | 6 +----- 7 files changed, 19 insertions(+), 39 deletions(-) diff --git a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/VideoViewHolder.kt b/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/VideoViewHolder.kt index 92d28d26c9..07c7b4588f 100644 --- a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/VideoViewHolder.kt +++ b/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/VideoViewHolder.kt @@ -103,14 +103,12 @@ class VideoViewHolder constructor(itemView: View) : views.videoView.setOnPreparedListener { stopTimer() countUpTimer = CountUpTimer(100).also { - it.tickListener = object : CountUpTimer.TickListener { - override fun onTick(milliseconds: Long) { - val duration = views.videoView.duration - val progress = views.videoView.currentPosition - val isPlaying = views.videoView.isPlaying -// Log.v("FOO", "isPlaying $isPlaying $progress/$duration") - eventListener?.get()?.onEvent(AttachmentEvents.VideoEvent(isPlaying, progress, duration)) - } + it.tickListener = CountUpTimer.TickListener { + val duration = views.videoView.duration + val progress = views.videoView.currentPosition + val isPlaying = views.videoView.isPlaying + // Log.v("FOO", "isPlaying $isPlaying $progress/$duration") + eventListener?.get()?.onEvent(AttachmentEvents.VideoEvent(isPlaying, progress, duration)) } it.resume() } diff --git a/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt b/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt index e9d311fe03..a4fd8bb4e1 100644 --- a/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt +++ b/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt @@ -66,7 +66,7 @@ class CountUpTimer(private val intervalInMs: Long = 1_000) { coroutineScope.cancel() } - interface TickListener { + fun interface TickListener { fun onTick(milliseconds: Long) } } diff --git a/vector/src/main/java/im/vector/app/features/call/webrtc/WebRtcCall.kt b/vector/src/main/java/im/vector/app/features/call/webrtc/WebRtcCall.kt index 00b9a76de7..0bf70690ba 100644 --- a/vector/src/main/java/im/vector/app/features/call/webrtc/WebRtcCall.kt +++ b/vector/src/main/java/im/vector/app/features/call/webrtc/WebRtcCall.kt @@ -167,12 +167,10 @@ class WebRtcCall( private var screenSender: RtpSender? = null private val timer = CountUpTimer(1000L).apply { - tickListener = object : CountUpTimer.TickListener { - override fun onTick(milliseconds: Long) { - val formattedDuration = formatDuration(Duration.ofMillis(milliseconds)) - listeners.forEach { - tryOrNull { it.onTick(formattedDuration) } - } + tickListener = CountUpTimer.TickListener { milliseconds -> + val formattedDuration = formatDuration(Duration.ofMillis(milliseconds)) + listeners.forEach { + tryOrNull { it.onTick(formattedDuration) } } } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/AudioMessageHelper.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/AudioMessageHelper.kt index bede02c17f..eddfe500b3 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/AudioMessageHelper.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/AudioMessageHelper.kt @@ -199,11 +199,7 @@ class AudioMessageHelper @Inject constructor( private fun startRecordingAmplitudes() { amplitudeTicker?.stop() amplitudeTicker = CountUpTimer(50).apply { - tickListener = object : CountUpTimer.TickListener { - override fun onTick(milliseconds: Long) { - onAmplitudeTick() - } - } + tickListener = CountUpTimer.TickListener { onAmplitudeTick() } resume() } } @@ -234,11 +230,7 @@ class AudioMessageHelper @Inject constructor( private fun startPlaybackTicker(id: String) { playbackTicker?.stop() playbackTicker = CountUpTimer().apply { - tickListener = object : CountUpTimer.TickListener { - override fun onTick(milliseconds: Long) { - onPlaybackTick(id) - } - } + tickListener = CountUpTimer.TickListener { onPlaybackTick(id) } resume() } onPlaybackTick(id) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt index 13e0477ab6..a7b926f29a 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt @@ -189,11 +189,9 @@ class VoiceMessageRecorderView @JvmOverloads constructor( val startMs = ((clock.epochMillis() - startAt)).coerceAtLeast(0) recordingTicker?.stop() recordingTicker = CountUpTimer().apply { - tickListener = object : CountUpTimer.TickListener { - override fun onTick(milliseconds: Long) { - val isLocked = startFromLocked || lastKnownState is RecordingUiState.Locked - onRecordingTick(isLocked, milliseconds + startMs) - } + tickListener = CountUpTimer.TickListener { milliseconds -> + val isLocked = startFromLocked || lastKnownState is RecordingUiState.Locked + onRecordingTick(isLocked, milliseconds + startMs) } resume() } diff --git a/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationUserItem.kt b/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationUserItem.kt index bab7f4c7f9..c108e83e76 100644 --- a/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationUserItem.kt +++ b/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationUserItem.kt @@ -79,10 +79,8 @@ abstract class LiveLocationUserItem : VectorEpoxyModel<LiveLocationUserItem.Hold } } - holder.timer.tickListener = object : CountUpTimer.TickListener { - override fun onTick(milliseconds: Long) { - holder.itemLastUpdatedAtTextView.text = getFormattedLastUpdatedAt(locationUpdateTimeMillis) - } + holder.timer.tickListener = CountUpTimer.TickListener { + holder.itemLastUpdatedAtTextView.text = getFormattedLastUpdatedAt(locationUpdateTimeMillis) } holder.timer.resume() diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 773883d81a..bf8ff11043 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -362,11 +362,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( fun startPlaybackTicker(id: String) { playbackTicker?.stop() playbackTicker = CountUpTimer().apply { - tickListener = object : CountUpTimer.TickListener { - override fun onTick(milliseconds: Long) { - onPlaybackTick(id) - } - } + tickListener = CountUpTimer.TickListener { onPlaybackTick(id) } resume() } onPlaybackTick(id) From dae4162e7506fb620968b14331541ad6465c347e Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 15:54:28 +0100 Subject: [PATCH 08/47] VoiceBroadcastPlayerImpl - use session coroutine scope --- .../listening/VoiceBroadcastPlayerImpl.kt | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index bf8ff11043..6eb9cbc735 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -22,6 +22,7 @@ import android.media.MediaPlayer.OnPreparedListener import androidx.annotation.MainThread import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker +import im.vector.app.features.session.coroutineScope import im.vector.app.features.voice.VoiceFailure import im.vector.app.features.voicebroadcast.duration import im.vector.app.features.voicebroadcast.isLive @@ -33,10 +34,7 @@ import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent import im.vector.app.features.voicebroadcast.sequence import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastEventUseCase import im.vector.lib.core.utils.timer.CountUpTimer -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch @@ -48,7 +46,6 @@ import timber.log.Timber import java.util.concurrent.CopyOnWriteArrayList import javax.inject.Inject import javax.inject.Singleton -import kotlin.math.absoluteValue @Singleton class VoiceBroadcastPlayerImpl @Inject constructor( @@ -58,10 +55,9 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private val getLiveVoiceBroadcastChunksUseCase: GetLiveVoiceBroadcastChunksUseCase ) : VoiceBroadcastPlayer { - private val session - get() = sessionHolder.getActiveSession() + private val session get() = sessionHolder.getActiveSession() + private val sessionScope get() = session.coroutineScope - private val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) private var fetchPlaylistTask: Job? = null private var voiceBroadcastStateTask: Job? = null @@ -151,13 +147,13 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun observeVoiceBroadcastLiveState(voiceBroadcast: VoiceBroadcast) { voiceBroadcastStateTask = getVoiceBroadcastEventUseCase.execute(voiceBroadcast) .onEach { currentVoiceBroadcastEvent = it.getOrNull() } - .launchIn(coroutineScope) + .launchIn(sessionScope) } private fun fetchPlaylistAndStartPlayback(voiceBroadcast: VoiceBroadcast) { fetchPlaylistTask = getLiveVoiceBroadcastChunksUseCase.execute(voiceBroadcast) .onEach(this::updatePlaylist) - .launchIn(coroutineScope) + .launchIn(sessionScope) } private fun updatePlaylist(audioEvents: List<MessageAudioEvent>) { @@ -212,7 +208,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( val content = playlistItem?.audioEvent?.content ?: run { Timber.w("## VoiceBroadcastPlayer: No content to play"); return } val sequence = playlistItem.audioEvent.sequence val sequencePosition = position?.let { it - playlistItem.startTime } ?: 0 - coroutineScope.launch { + sessionScope.launch { try { prepareMediaPlayer(content) { mp -> currentMediaPlayer = mp @@ -255,7 +251,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( nextMediaPlayer = null val nextContent = getNextAudioContent() if (nextContent != null) { - coroutineScope.launch { + sessionScope.launch { prepareMediaPlayer(nextContent) { mp -> if (nextMediaPlayer == null) { nextMediaPlayer = mp From c85b159952accaff1b7d65f027ced44fe790f0e4 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 17:12:02 +0100 Subject: [PATCH 09/47] VoiceBroadcastPlayer - Extract some code to VoiceBroadcastPlaylist --- .../VoiceBroadcastExtensions.kt | 7 +- .../listening/VoiceBroadcastPlayerImpl.kt | 60 +++++------------ .../listening/VoiceBroadcastPlaylist.kt | 67 +++++++++++++++++++ 3 files changed, 91 insertions(+), 43 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt index a1328c0ba3..fa8033a211 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastExtensions.kt @@ -16,9 +16,11 @@ package im.vector.app.features.voicebroadcast +import im.vector.app.features.voicebroadcast.model.MessageVoiceBroadcastInfoContent import im.vector.app.features.voicebroadcast.model.VoiceBroadcastChunk import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState +import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.session.events.model.Content import org.matrix.android.sdk.api.session.events.model.getRelationContent import org.matrix.android.sdk.api.session.events.model.toModel @@ -38,4 +40,7 @@ val MessageAudioEvent.sequence: Int? get() = getVoiceBroadcastChunk()?.sequence val MessageAudioEvent.duration get() = content.audioInfo?.duration ?: content.audioWaveformInfo?.duration ?: 0 val VoiceBroadcastEvent.isLive - get() = content?.voiceBroadcastState != null && content?.voiceBroadcastState != VoiceBroadcastState.STOPPED + get() = content?.isLive.orFalse() + +val MessageVoiceBroadcastInfoContent.isLive + get() = voiceBroadcastState != null && voiceBroadcastState != VoiceBroadcastState.STOPPED diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 6eb9cbc735..de4f965a59 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -24,7 +24,6 @@ import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.session.coroutineScope import im.vector.app.features.voice.VoiceFailure -import im.vector.app.features.voicebroadcast.duration import im.vector.app.features.voicebroadcast.isLive import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.Listener import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.State @@ -41,7 +40,6 @@ import kotlinx.coroutines.launch import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.room.model.message.MessageAudioContent -import org.matrix.android.sdk.api.session.room.model.message.MessageAudioEvent import timber.log.Timber import java.util.concurrent.CopyOnWriteArrayList import javax.inject.Inject @@ -67,11 +65,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private var currentMediaPlayer: MediaPlayer? = null private var nextMediaPlayer: MediaPlayer? = null - private var playlist = emptyList<PlaylistItem>() - private var currentSequence: Int? = null + private val playlist = VoiceBroadcastPlaylist() private var currentVoiceBroadcastEvent: VoiceBroadcastEvent? = null - private val isLive get() = currentVoiceBroadcastEvent?.isLive.orFalse() - private val lastSequence get() = currentVoiceBroadcastEvent?.content?.lastChunkSequence override var currentVoiceBroadcast: VoiceBroadcast? = null @@ -114,8 +109,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( voiceBroadcastStateTask = null // Clear playlist - playlist = emptyList() - currentSequence = null + playlist.reset() currentVoiceBroadcastEvent = null currentVoiceBroadcast = null @@ -152,25 +146,13 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun fetchPlaylistAndStartPlayback(voiceBroadcast: VoiceBroadcast) { fetchPlaylistTask = getLiveVoiceBroadcastChunksUseCase.execute(voiceBroadcast) - .onEach(this::updatePlaylist) + .onEach { + playlist.setItems(it) + onPlaylistUpdated() + } .launchIn(sessionScope) } - private fun updatePlaylist(audioEvents: List<MessageAudioEvent>) { - val sorted = audioEvents.sortedBy { it.sequence?.toLong() ?: it.root.originServerTs } - val chunkPositions = sorted - .map { it.duration } - .runningFold(0) { acc, i -> acc + i } - .dropLast(1) - playlist = sorted.mapIndexed { index, messageAudioEvent -> - PlaylistItem( - audioEvent = messageAudioEvent, - startTime = chunkPositions.getOrNull(index) ?: 0 - ) - } - onPlaylistUpdated() - } - private fun onPlaylistUpdated() { when (playingState) { State.PLAYING -> { @@ -201,18 +183,18 @@ class VoiceBroadcastPlayerImpl @Inject constructor( stopPlayer() val playlistItem = when { - position != null -> playlist.lastOrNull { it.startTime <= position } - isLive -> playlist.lastOrNull() + position != null -> playlist.findByPosition(position) + currentVoiceBroadcastEvent?.isLive.orFalse() -> playlist.lastOrNull() else -> playlist.firstOrNull() } val content = playlistItem?.audioEvent?.content ?: run { Timber.w("## VoiceBroadcastPlayer: No content to play"); return } - val sequence = playlistItem.audioEvent.sequence + val sequence = playlistItem.audioEvent.sequence ?: run { Timber.w("## VoiceBroadcastPlayer: playlist item has no sequence"); return } val sequencePosition = position?.let { it - playlistItem.startTime } ?: 0 sessionScope.launch { try { prepareMediaPlayer(content) { mp -> currentMediaPlayer = mp - currentSequence = sequence + playlist.currentSequence = sequence mp.start() if (sequencePosition > 0) { mp.seekTo(sequencePosition) @@ -241,10 +223,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } private fun getNextAudioContent(): MessageAudioContent? { - val nextSequence = currentSequence?.plus(1) - ?: playlist.lastOrNull()?.audioEvent?.sequence - ?: 1 - return playlist.find { it.audioEvent.sequence == nextSequence }?.audioEvent?.content + return playlist.getNextItem()?.audioEvent?.content } private fun prepareNextMediaPlayer() { @@ -322,7 +301,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override fun onInfo(mp: MediaPlayer, what: Int, extra: Int): Boolean { when (what) { MediaPlayer.MEDIA_INFO_STARTED_AS_NEXT -> { - currentSequence = currentSequence?.plus(1) + playlist.currentSequence++ currentMediaPlayer = mp prepareNextMediaPlayer() } @@ -333,7 +312,9 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override fun onCompletion(mp: MediaPlayer) { if (nextMediaPlayer != null) return - if (!isLive && lastSequence == currentSequence) { + val content = currentVoiceBroadcastEvent?.content + val isLive = content?.isLive.orFalse() + if (!isLive && content?.lastChunkSequence == playlist.currentSequence) { // We'll not receive new chunks anymore so we can stop the live listening stop() } else { @@ -347,10 +328,6 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } - private fun getVoiceBroadcastDuration() = playlist.lastOrNull()?.let { it.startTime + it.audioEvent.duration } ?: 0 - - private data class PlaylistItem(val audioEvent: MessageAudioEvent, val startTime: Int) - private inner class PlaybackTicker( private var playbackTicker: CountUpTimer? = null, ) { @@ -366,12 +343,11 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun onPlaybackTick(id: String) { if (currentMediaPlayer?.isPlaying.orFalse()) { - val itemStartPosition = currentSequence?.let { seq -> playlist.find { it.audioEvent.sequence == seq } }?.startTime + val itemStartPosition = playlist.currentItem?.startTime val currentVoiceBroadcastPosition = itemStartPosition?.plus(currentMediaPlayer?.currentPosition ?: 0) Timber.d("Voice Broadcast | VoiceBroadcastPlayerImpl - sequence: $currentSequence, itemStartPosition $itemStartPosition, currentMediaPlayer=$currentMediaPlayer, currentMediaPlayer?.currentPosition: ${currentMediaPlayer?.currentPosition}") if (currentVoiceBroadcastPosition != null) { - val totalDuration = getVoiceBroadcastDuration() - val percentage = currentVoiceBroadcastPosition.toFloat() / totalDuration + val percentage = currentVoiceBroadcastPosition.toFloat() / playlist.duration playbackTracker.updatePlayingAtPlaybackTime(id, currentVoiceBroadcastPosition, percentage) } else { stopPlaybackTicker(id) @@ -385,7 +361,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playbackTicker?.stop() playbackTicker = null - val totalDuration = getVoiceBroadcastDuration() + val totalDuration = playlist.duration val playbackTime = playbackTracker.getPlaybackTime(id) val remainingTime = totalDuration - playbackTime if (remainingTime < 1000) { diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt new file mode 100644 index 0000000000..5cd6efc28a --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.voicebroadcast.listening + +import im.vector.app.features.voicebroadcast.duration +import im.vector.app.features.voicebroadcast.sequence +import org.matrix.android.sdk.api.session.room.model.message.MessageAudioEvent + +class VoiceBroadcastPlaylist( + private val items: MutableList<PlaylistItem> = mutableListOf(), +) : List<PlaylistItem> by items { + + var currentSequence = 1 + val currentItem get() = findBySequence(currentSequence) + + val duration + get() = items.lastOrNull()?.let { it.startTime + it.audioEvent.duration } ?: 0 + + fun setItems(audioEvents: List<MessageAudioEvent>) { + items.clear() + val sorted = audioEvents.sortedBy { it.sequence?.toLong() ?: it.root.originServerTs } + val chunkPositions = sorted + .map { it.duration } + .runningFold(0) { acc, i -> acc + i } + .dropLast(1) + val newItems = sorted.mapIndexed { index, messageAudioEvent -> + PlaylistItem( + audioEvent = messageAudioEvent, + startTime = chunkPositions.getOrNull(index) ?: 0 + ) + } + items.addAll(newItems) + } + + fun reset() { + currentSequence = 1 + items.clear() + } + + fun findByPosition(positionMillis: Int): PlaylistItem? { + return items.lastOrNull { it.startTime <= positionMillis } + } + + fun findBySequence(sequenceNumber: Int): PlaylistItem? { + return items.find { it.audioEvent.sequence == sequenceNumber } + } + + fun getNextItem() = findBySequence(currentSequence.plus(1)) + + fun firstOrNull() = findBySequence(1) +} + +data class PlaylistItem(val audioEvent: MessageAudioEvent, val startTime: Int) From 37c75354bed4a884c03c2ca15a6f22e32d05f572 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 18:18:26 +0100 Subject: [PATCH 10/47] VoiceBroadcastPlayer - Reorganize some code --- .../listening/VoiceBroadcastPlayerImpl.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index de4f965a59..a38442a19a 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -56,16 +56,16 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private val session get() = sessionHolder.getActiveSession() private val sessionScope get() = session.coroutineScope - private var fetchPlaylistTask: Job? = null - private var voiceBroadcastStateTask: Job? = null - private val mediaPlayerListener = MediaPlayerListener() private val playbackTicker = PlaybackTicker() + private val playlist = VoiceBroadcastPlaylist() + + private var fetchPlaylistTask: Job? = null + private var voiceBroadcastStateObserver: Job? = null private var currentMediaPlayer: MediaPlayer? = null private var nextMediaPlayer: MediaPlayer? = null - private val playlist = VoiceBroadcastPlaylist() private var currentVoiceBroadcastEvent: VoiceBroadcastEvent? = null override var currentVoiceBroadcast: VoiceBroadcast? = null @@ -105,8 +105,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor( // Do not observe anymore voice broadcast changes fetchPlaylistTask?.cancel() fetchPlaylistTask = null - voiceBroadcastStateTask?.cancel() - voiceBroadcastStateTask = null + voiceBroadcastStateObserver?.cancel() + voiceBroadcastStateObserver = null // Clear playlist playlist.reset() @@ -139,7 +139,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } private fun observeVoiceBroadcastLiveState(voiceBroadcast: VoiceBroadcast) { - voiceBroadcastStateTask = getVoiceBroadcastEventUseCase.execute(voiceBroadcast) + voiceBroadcastStateObserver = getVoiceBroadcastEventUseCase.execute(voiceBroadcast) .onEach { currentVoiceBroadcastEvent = it.getOrNull() } .launchIn(sessionScope) } @@ -345,7 +345,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( if (currentMediaPlayer?.isPlaying.orFalse()) { val itemStartPosition = playlist.currentItem?.startTime val currentVoiceBroadcastPosition = itemStartPosition?.plus(currentMediaPlayer?.currentPosition ?: 0) - Timber.d("Voice Broadcast | VoiceBroadcastPlayerImpl - sequence: $currentSequence, itemStartPosition $itemStartPosition, currentMediaPlayer=$currentMediaPlayer, currentMediaPlayer?.currentPosition: ${currentMediaPlayer?.currentPosition}") + Timber.d("Voice Broadcast | VoiceBroadcastPlayerImpl - sequence: ${playlist.currentSequence}, itemStartPosition $itemStartPosition, currentMediaPlayer=$currentMediaPlayer, currentMediaPlayer?.currentPosition: ${currentMediaPlayer?.currentPosition}") if (currentVoiceBroadcastPosition != null) { val percentage = currentVoiceBroadcastPosition.toFloat() / playlist.duration playbackTracker.updatePlayingAtPlaybackTime(id, currentVoiceBroadcastPosition, percentage) From b87b2cbb63aea881d753759482167e89f0c5ca0c Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 18:28:26 +0100 Subject: [PATCH 11/47] Remove useless method --- .../listening/VoiceBroadcastPlayerImpl.kt | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index a38442a19a..de01661431 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -166,8 +166,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } State.BUFFERING -> { - val newMediaContent = getNextAudioContent() - if (newMediaContent != null) { + val nextItem = playlist.getNextItem() + if (nextItem != null) { val savedPosition = currentVoiceBroadcast?.let { playbackTracker.getPlaybackTime(it.voiceBroadcastId) } startPlayback(savedPosition) } @@ -222,14 +222,10 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } - private fun getNextAudioContent(): MessageAudioContent? { - return playlist.getNextItem()?.audioEvent?.content - } - private fun prepareNextMediaPlayer() { nextMediaPlayer = null - val nextContent = getNextAudioContent() - if (nextContent != null) { + val nextItem = playlist.getNextItem() + if (nextItem != null) { sessionScope.launch { prepareMediaPlayer(nextContent) { mp -> if (nextMediaPlayer == null) { From a3cd861e15d09ec29ae29f84b3586523647c22e6 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 18:29:01 +0100 Subject: [PATCH 12/47] Add isPreparingNextPlayer flag --- .../listening/VoiceBroadcastPlayerImpl.kt | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index de01661431..adc1912140 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -65,6 +65,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private var currentMediaPlayer: MediaPlayer? = null private var nextMediaPlayer: MediaPlayer? = null + private var isPreparingNextPlayer: Boolean = false private var currentVoiceBroadcastEvent: VoiceBroadcastEvent? = null @@ -156,12 +157,12 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun onPlaylistUpdated() { when (playingState) { State.PLAYING -> { - if (nextMediaPlayer == null) { + if (nextMediaPlayer == null && !isPreparingNextPlayer) { prepareNextMediaPlayer() } } State.PAUSED -> { - if (nextMediaPlayer == null) { + if (nextMediaPlayer == null && !isPreparingNextPlayer) { prepareNextMediaPlayer() } } @@ -223,17 +224,14 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } private fun prepareNextMediaPlayer() { - nextMediaPlayer = null val nextItem = playlist.getNextItem() if (nextItem != null) { + isPreparingNextPlayer = true sessionScope.launch { - prepareMediaPlayer(nextContent) { mp -> - if (nextMediaPlayer == null) { - nextMediaPlayer = mp - currentMediaPlayer?.setNextMediaPlayer(mp) - } else { - mp.release() - } + prepareMediaPlayer(nextItem.audioEvent.content) { mp -> + nextMediaPlayer = mp + currentMediaPlayer?.setNextMediaPlayer(mp) + isPreparingNextPlayer = false } } } @@ -274,6 +272,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( nextMediaPlayer?.release() nextMediaPlayer = null + isPreparingNextPlayer = false } private fun onPlayingStateChanged(playingState: State) { From a3201555462586c837f883216170668e2ed7840a Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 18:36:37 +0100 Subject: [PATCH 13/47] reset nextMediaPlayer when item has changed --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index adc1912140..da3a9559a4 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -298,6 +298,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( MediaPlayer.MEDIA_INFO_STARTED_AS_NEXT -> { playlist.currentSequence++ currentMediaPlayer = mp + nextMediaPlayer = null prepareNextMediaPlayer() } } From 43a112839f6bcd2bfb17dc3bf36c16f92cd3d58d Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 18:49:10 +0100 Subject: [PATCH 14/47] Fix seek when playlist is not loaded --- .../listening/VoiceBroadcastPlayerImpl.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index da3a9559a4..7b8d8c9547 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -358,12 +358,14 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playbackTicker = null val totalDuration = playlist.duration - val playbackTime = playbackTracker.getPlaybackTime(id) - val remainingTime = totalDuration - playbackTime - if (remainingTime < 1000) { - playbackTracker.updatePausedAtPlaybackTime(id, 0, 0f) - } else { - playbackTracker.pausePlayback(id) + if (totalDuration > 0) { + val playbackTime = playbackTracker.getPlaybackTime(id) + val remainingTime = totalDuration - playbackTime + if (remainingTime < 1000) { + playbackTracker.updatePausedAtPlaybackTime(id, 0, 0f) + } else { + playbackTracker.pausePlayback(id) + } } } } From 266236c1e5daee19bdb6e632ab74850a30ed3c7e Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 23:20:22 +0100 Subject: [PATCH 15/47] set playlist.currentSequence null by default --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 2 +- .../voicebroadcast/listening/VoiceBroadcastPlaylist.kt | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 7b8d8c9547..921e0e69ea 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -296,7 +296,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override fun onInfo(mp: MediaPlayer, what: Int, extra: Int): Boolean { when (what) { MediaPlayer.MEDIA_INFO_STARTED_AS_NEXT -> { - playlist.currentSequence++ + playlist.currentSequence = playlist.currentSequence?.inc() currentMediaPlayer = mp nextMediaPlayer = null prepareNextMediaPlayer() diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt index 5cd6efc28a..ff388c2313 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt @@ -24,8 +24,8 @@ class VoiceBroadcastPlaylist( private val items: MutableList<PlaylistItem> = mutableListOf(), ) : List<PlaylistItem> by items { - var currentSequence = 1 - val currentItem get() = findBySequence(currentSequence) + var currentSequence: Int? = null + val currentItem get() = currentSequence?.let { findBySequence(it) } val duration get() = items.lastOrNull()?.let { it.startTime + it.audioEvent.duration } ?: 0 @@ -47,7 +47,7 @@ class VoiceBroadcastPlaylist( } fun reset() { - currentSequence = 1 + currentSequence = null items.clear() } @@ -59,7 +59,7 @@ class VoiceBroadcastPlaylist( return items.find { it.audioEvent.sequence == sequenceNumber } } - fun getNextItem() = findBySequence(currentSequence.plus(1)) + fun getNextItem() = findBySequence(currentSequence?.plus(1) ?: 1) fun firstOrNull() = findBySequence(1) } From a47e3c1233999b8e5ebbd83974626b86d58488a0 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 23:20:54 +0100 Subject: [PATCH 16/47] Improve playing state updates --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 921e0e69ea..bfbc53fd78 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -74,9 +74,11 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override var playingState = State.IDLE @MainThread set(value) { - Timber.w("## VoiceBroadcastPlayer state: $field -> $value") - field = value - onPlayingStateChanged(value) + if (field != value) { + Timber.w("## VoiceBroadcastPlayer state: $field -> $value") + field = value + onPlayingStateChanged(value) + } } /** Map voiceBroadcastId to listeners.*/ @@ -299,6 +301,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playlist.currentSequence = playlist.currentSequence?.inc() currentMediaPlayer = mp nextMediaPlayer = null + playingState = State.PLAYING prepareNextMediaPlayer() } } From b2f35fa1352f1b3771d8d3aa9231340560bebdab Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 23:21:18 +0100 Subject: [PATCH 17/47] Improve PlaybackTicker --- .../helper/AudioMessagePlaybackTracker.kt | 2 +- .../listening/VoiceBroadcastPlayerImpl.kt | 51 ++++++++++--------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt index 6937cd3a46..7e40b92ac8 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt @@ -127,7 +127,7 @@ class AudioMessagePlaybackTracker @Inject constructor() { } } - private fun getPercentage(id: String): Float { + fun getPercentage(id: String): Float { return when (val state = states[id]) { is Listener.State.Playing -> state.percentage is Listener.State.Paused -> state.percentage diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index bfbc53fd78..50a3002c0d 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -340,34 +340,37 @@ class VoiceBroadcastPlayerImpl @Inject constructor( onPlaybackTick(id) } - private fun onPlaybackTick(id: String) { - if (currentMediaPlayer?.isPlaying.orFalse()) { - val itemStartPosition = playlist.currentItem?.startTime - val currentVoiceBroadcastPosition = itemStartPosition?.plus(currentMediaPlayer?.currentPosition ?: 0) - Timber.d("Voice Broadcast | VoiceBroadcastPlayerImpl - sequence: ${playlist.currentSequence}, itemStartPosition $itemStartPosition, currentMediaPlayer=$currentMediaPlayer, currentMediaPlayer?.currentPosition: ${currentMediaPlayer?.currentPosition}") - if (currentVoiceBroadcastPosition != null) { - val percentage = currentVoiceBroadcastPosition.toFloat() / playlist.duration - playbackTracker.updatePlayingAtPlaybackTime(id, currentVoiceBroadcastPosition, percentage) - } else { - stopPlaybackTicker(id) - } - } else { - stopPlaybackTicker(id) - } - } - fun stopPlaybackTicker(id: String) { playbackTicker?.stop() playbackTicker = null + onPlaybackTick(id) + } - val totalDuration = playlist.duration - if (totalDuration > 0) { - val playbackTime = playbackTracker.getPlaybackTime(id) - val remainingTime = totalDuration - playbackTime - if (remainingTime < 1000) { - playbackTracker.updatePausedAtPlaybackTime(id, 0, 0f) - } else { - playbackTracker.pausePlayback(id) + private fun onPlaybackTick(id: String) { + val currentItem = playlist.currentItem ?: return + val itemStartTime = currentItem.startTime + val duration = playlist.duration + when (playingState) { + State.PLAYING, + State.PAUSED -> { + Timber.d("Voice Broadcast | VoiceBroadcastPlayerImpl - sequence: ${playlist.currentSequence}, itemStartTime $itemStartTime, currentMediaPlayer=$currentMediaPlayer, currentMediaPlayer?.currentPosition: ${currentMediaPlayer?.currentPosition}") + val position = itemStartTime + (currentMediaPlayer?.currentPosition ?: 0) + val percentage = position.toFloat() / playlist.duration + if (playingState == State.PLAYING) { + playbackTracker.updatePlayingAtPlaybackTime(id, position, percentage) + } else { + playbackTracker.updatePausedAtPlaybackTime(id, position, percentage) + } + } + State.BUFFERING, + State.IDLE -> { + val playbackTime = playbackTracker.getPlaybackTime(id) + val percentage = playbackTracker.getPercentage(id) + if (playingState == State.IDLE && duration > 0 && (duration - playbackTime) < 1000) { + playbackTracker.updatePausedAtPlaybackTime(id, 0, 0f) + } else { + playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) + } } } } From 436e76c7563f3f529f395630c75dd59ef32436aa Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 23:42:02 +0100 Subject: [PATCH 18/47] Fix seek on paused state --- .../home/room/detail/RoomDetailAction.kt | 2 +- .../home/room/detail/TimelineViewModel.kt | 2 +- .../MessageVoiceBroadcastListeningItem.kt | 2 +- .../voicebroadcast/VoiceBroadcastHelper.kt | 4 +- .../listening/VoiceBroadcastPlayer.kt | 2 +- .../listening/VoiceBroadcastPlayerImpl.kt | 40 ++++++++++++++----- 6 files changed, 37 insertions(+), 15 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt index ba0f7dbdf8..faee8f652c 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt @@ -133,7 +133,7 @@ sealed class RoomDetailAction : VectorViewModelAction { data class PlayOrResume(val voiceBroadcast: VoiceBroadcast) : Listening() object Pause : Listening() object Stop : Listening() - data class SeekTo(val voiceBroadcast: VoiceBroadcast, val positionMillis: Int) : Listening() + data class SeekTo(val voiceBroadcast: VoiceBroadcast, val positionMillis: Int, val duration: Int) : Listening() } } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt index 252823b2a6..ef238d56e6 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt @@ -637,7 +637,7 @@ class TimelineViewModel @AssistedInject constructor( is VoiceBroadcastAction.Listening.PlayOrResume -> voiceBroadcastHelper.playOrResumePlayback(action.voiceBroadcast) VoiceBroadcastAction.Listening.Pause -> voiceBroadcastHelper.pausePlayback() VoiceBroadcastAction.Listening.Stop -> voiceBroadcastHelper.stopPlayback() - is VoiceBroadcastAction.Listening.SeekTo -> voiceBroadcastHelper.seekTo(action.voiceBroadcast, action.positionMillis) + is VoiceBroadcastAction.Listening.SeekTo -> voiceBroadcastHelper.seekTo(action.voiceBroadcast, action.positionMillis, action.duration) } } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index 19caf3d8ba..ebbfe13730 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -94,7 +94,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } override fun onStopTrackingTouch(seekBar: SeekBar) { - callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcast, seekBar.progress)) + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcast, seekBar.progress, duration)) isUserSeeking = false } }) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt index 3661928fa5..38fb157748 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastHelper.kt @@ -48,7 +48,7 @@ class VoiceBroadcastHelper @Inject constructor( fun stopPlayback() = voiceBroadcastPlayer.stop() - fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int) { - voiceBroadcastPlayer.seekTo(voiceBroadcast, positionMillis) + fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int, duration: Int) { + voiceBroadcastPlayer.seekTo(voiceBroadcast, positionMillis, duration) } } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt index 36e75236ad..8c11db4f43 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt @@ -48,7 +48,7 @@ interface VoiceBroadcastPlayer { /** * Seek the given voice broadcast playback to the given position, is milliseconds. */ - fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int) + fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int, duration: Int) /** * Add a [Listener] to the given voice broadcast. diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 50a3002c0d..0937977b70 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -94,8 +94,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } override fun pause() { - currentMediaPlayer?.pause() - playingState = State.PAUSED + pausePlayback() } override fun stop() { @@ -212,16 +211,39 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } - private fun resumePlayback() { - currentMediaPlayer?.start() - playingState = State.PLAYING + private fun pausePlayback(positionMillis: Int? = null) { + if (positionMillis == null) { + currentMediaPlayer?.pause() + } else { + stopPlayer() + currentVoiceBroadcast?.voiceBroadcastId?.let { id -> + playbackTracker.updatePausedAtPlaybackTime(id, positionMillis, positionMillis.toFloat() / playlist.duration) + } + } + playingState = State.PAUSED } - override fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int) { - if (voiceBroadcast != currentVoiceBroadcast) { - playbackTracker.updatePausedAtPlaybackTime(voiceBroadcast.voiceBroadcastId, positionMillis, 0f) + private fun resumePlayback() { + if (currentMediaPlayer != null) { + currentMediaPlayer?.start() + playingState = State.PLAYING } else { - startPlayback(positionMillis) + val position = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPlaybackTime(it) } + startPlayback(position) + } + } + + override fun seekTo(voiceBroadcast: VoiceBroadcast, positionMillis: Int, duration: Int) { + when { + voiceBroadcast != currentVoiceBroadcast -> { + playbackTracker.updatePausedAtPlaybackTime(voiceBroadcast.voiceBroadcastId, positionMillis, positionMillis.toFloat() / duration) + } + playingState == State.PLAYING || playingState == State.BUFFERING -> { + startPlayback(positionMillis) + } + playingState == State.IDLE || playingState == State.PAUSED -> { + pausePlayback(positionMillis) + } } } From 7d51a265222c5bcabee7fe3e92a7a956e6c37377 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Fri, 4 Nov 2022 23:45:38 +0100 Subject: [PATCH 19/47] Decrease tick interval --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 0937977b70..f7e296ffed 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -355,7 +355,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( fun startPlaybackTicker(id: String) { playbackTicker?.stop() - playbackTicker = CountUpTimer().apply { + playbackTicker = CountUpTimer(50L).apply { tickListener = CountUpTimer.TickListener { onPlaybackTick(id) } resume() } @@ -388,7 +388,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( State.IDLE -> { val playbackTime = playbackTracker.getPlaybackTime(id) val percentage = playbackTracker.getPercentage(id) - if (playingState == State.IDLE && duration > 0 && (duration - playbackTime) < 1000) { + if (playingState == State.IDLE && duration > 0 && (duration - playbackTime) < 100) { playbackTracker.updatePausedAtPlaybackTime(id, 0, 0f) } else { playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) From baa9cb39b0ff0755376b612e7d5c10fdcab439cb Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Sat, 5 Nov 2022 00:06:00 +0100 Subject: [PATCH 20/47] Fix broken live listening --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index f7e296ffed..b613d99d33 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -171,12 +171,12 @@ class VoiceBroadcastPlayerImpl @Inject constructor( val nextItem = playlist.getNextItem() if (nextItem != null) { val savedPosition = currentVoiceBroadcast?.let { playbackTracker.getPlaybackTime(it.voiceBroadcastId) } - startPlayback(savedPosition) + startPlayback(savedPosition?.takeIf { it > 0 }) } } State.IDLE -> { val savedPosition = currentVoiceBroadcast?.let { playbackTracker.getPlaybackTime(it.voiceBroadcastId) } - startPlayback(savedPosition) + startPlayback(savedPosition?.takeIf { it > 0 }) } } } @@ -389,7 +389,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( val playbackTime = playbackTracker.getPlaybackTime(id) val percentage = playbackTracker.getPercentage(id) if (playingState == State.IDLE && duration > 0 && (duration - playbackTime) < 100) { - playbackTracker.updatePausedAtPlaybackTime(id, 0, 0f) + playbackTracker.stopPlayback(id) } else { playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) } From c5e6eb0d0e131f86ba9da3af94fa8c0455ebbc6e Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Sat, 5 Nov 2022 00:07:02 +0100 Subject: [PATCH 21/47] Remove some logs --- .../detail/timeline/item/MessageVoiceBroadcastListeningItem.kt | 1 - .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 1 - 2 files changed, 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index ebbfe13730..bdd0670029 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -108,7 +108,6 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } is AudioMessagePlaybackTracker.Listener.State.Playing -> { if (!isUserSeeking) { -// Timber.d("Voice Broadcast | AudioMessagePlaybackTracker.Listener.onUpdate - duration: $duration, playbackTime: ${state.playbackTime}") holder.seekBar.progress = state.playbackTime } } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index b613d99d33..9fb7c3ccb5 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -375,7 +375,6 @@ class VoiceBroadcastPlayerImpl @Inject constructor( when (playingState) { State.PLAYING, State.PAUSED -> { - Timber.d("Voice Broadcast | VoiceBroadcastPlayerImpl - sequence: ${playlist.currentSequence}, itemStartTime $itemStartTime, currentMediaPlayer=$currentMediaPlayer, currentMediaPlayer?.currentPosition: ${currentMediaPlayer?.currentPosition}") val position = itemStartTime + (currentMediaPlayer?.currentPosition ?: 0) val percentage = position.toFloat() / playlist.duration if (playingState == State.PLAYING) { From aa8eec221a1551b6d737e6a58feffe8f82fd3da7 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Sat, 5 Nov 2022 00:29:11 +0100 Subject: [PATCH 22/47] Enable fast backward/forward buttons --- .../MessageVoiceBroadcastListeningItem.kt | 59 +++++++++++++------ .../listening/VoiceBroadcastPlayerImpl.kt | 13 ++-- ...e_event_voice_broadcast_listening_stub.xml | 4 +- 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index bdd0670029..558f81b5fa 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -28,6 +28,7 @@ import im.vector.app.R import im.vector.app.core.epoxy.onClick import im.vector.app.features.home.room.detail.RoomDetailAction.VoiceBroadcastAction import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker +import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker.Listener.State import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer import im.vector.app.features.voicebroadcast.views.VoiceBroadcastMetadataView @@ -48,6 +49,32 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } player.addListener(voiceBroadcast, playerListener) bindSeekBar(holder) + bindButtons(holder) + } + + private fun bindButtons(holder: Holder) { + with(holder) { + playPauseButton.onClick { + when (player.playingState) { + VoiceBroadcastPlayer.State.PLAYING -> { + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) + } + VoiceBroadcastPlayer.State.PAUSED, + VoiceBroadcastPlayer.State.IDLE -> { + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) + } + VoiceBroadcastPlayer.State.BUFFERING -> Unit + } + } + fastBackwardButton.onClick { + val newPos = seekBar.progress.minus(30_000).coerceIn(0, duration) + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcast, newPos, duration)) + } + fastForwardButton.onClick { + val newPos = seekBar.progress.plus(30_000).coerceIn(0, duration) + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcast, newPos, duration)) + } + } } override fun renderMetadata(holder: Holder) { @@ -63,20 +90,15 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem bufferingView.isVisible = state == VoiceBroadcastPlayer.State.BUFFERING playPauseButton.isVisible = state != VoiceBroadcastPlayer.State.BUFFERING - fastBackwardButton.isInvisible = true - fastForwardButton.isInvisible = true - when (state) { VoiceBroadcastPlayer.State.PLAYING -> { playPauseButton.setImageResource(R.drawable.ic_play_pause_pause) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_pause_voice_broadcast) - playPauseButton.onClick { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) } } VoiceBroadcastPlayer.State.IDLE, VoiceBroadcastPlayer.State.PAUSED -> { playPauseButton.setImageResource(R.drawable.ic_play_pause_play) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_play_voice_broadcast) - playPauseButton.onClick { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) } } VoiceBroadcastPlayer.State.BUFFERING -> Unit } @@ -99,25 +121,24 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } }) playbackTracker.track(voiceBroadcast.voiceBroadcastId, object : AudioMessagePlaybackTracker.Listener { - override fun onUpdate(state: AudioMessagePlaybackTracker.Listener.State) { - when (state) { - is AudioMessagePlaybackTracker.Listener.State.Paused -> { - if (!isUserSeeking) { - holder.seekBar.progress = state.playbackTime - } - } - is AudioMessagePlaybackTracker.Listener.State.Playing -> { - if (!isUserSeeking) { - holder.seekBar.progress = state.playbackTime - } - } - AudioMessagePlaybackTracker.Listener.State.Idle -> Unit - is AudioMessagePlaybackTracker.Listener.State.Recording -> Unit + override fun onUpdate(state: State) { + renderBackwardForwardButtons(holder, state) + if (!isUserSeeking) { + holder.seekBar.progress = playbackTracker.getPlaybackTime(voiceBroadcast.voiceBroadcastId) } } }) } + private fun renderBackwardForwardButtons(holder: Holder, playbackState: State) { + val isPlayingOrPaused = playbackState is State.Playing || playbackState is State.Paused + val playbackTime = playbackTracker.getPlaybackTime(voiceBroadcast.voiceBroadcastId) + val canBackward = isPlayingOrPaused && playbackTime > 0 + val canForward = isPlayingOrPaused && playbackTime < duration + holder.fastBackwardButton.isInvisible = !canBackward + holder.fastForwardButton.isInvisible = !canForward + } + private fun formatPlaybackTime(time: Int) = DateUtils.formatElapsedTime((time / 1000).toLong()) override fun unbind(holder: Holder) { diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 9fb7c3ccb5..020edc283a 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -371,7 +371,6 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun onPlaybackTick(id: String) { val currentItem = playlist.currentItem ?: return val itemStartTime = currentItem.startTime - val duration = playlist.duration when (playingState) { State.PLAYING, State.PAUSED -> { @@ -383,15 +382,13 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playbackTracker.updatePausedAtPlaybackTime(id, position, percentage) } } - State.BUFFERING, - State.IDLE -> { + State.BUFFERING -> { val playbackTime = playbackTracker.getPlaybackTime(id) val percentage = playbackTracker.getPercentage(id) - if (playingState == State.IDLE && duration > 0 && (duration - playbackTime) < 100) { - playbackTracker.stopPlayback(id) - } else { - playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) - } + playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) + } + State.IDLE -> { + playbackTracker.stopPlayback(id) } } } diff --git a/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml b/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml index bed9407dfa..150f1cb281 100644 --- a/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml +++ b/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml @@ -100,7 +100,7 @@ android:id="@+id/fastBackwardButton" android:layout_width="24dp" android:layout_height="24dp" - android:background="@android:color/transparent" + android:background="@drawable/bg_rounded_button" android:contentDescription="@string/a11y_voice_broadcast_fast_backward" android:src="@drawable/ic_player_backward_30" app:tint="?vctr_content_secondary" /> @@ -127,7 +127,7 @@ android:id="@+id/fastForwardButton" android:layout_width="24dp" android:layout_height="24dp" - android:background="@android:color/transparent" + android:background="@drawable/bg_rounded_button" android:contentDescription="@string/a11y_voice_broadcast_fast_forward" android:src="@drawable/ic_player_forward_30" app:tint="?vctr_content_secondary" /> From 1c40f9c5e82ca4128238a42c99652954c787d33e Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 11:40:57 +0100 Subject: [PATCH 23/47] Minor cleanup --- .../MessageVoiceBroadcastListeningItem.kt | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index 558f81b5fa..a43783a626 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -44,9 +44,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } private fun bindVoiceBroadcastItem(holder: Holder) { - playerListener = VoiceBroadcastPlayer.Listener { state -> - renderPlayingState(holder, state) - } + playerListener = VoiceBroadcastPlayer.Listener { renderPlayingState(holder, it) } player.addListener(voiceBroadcast, playerListener) bindSeekBar(holder) bindButtons(holder) @@ -56,13 +54,9 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem with(holder) { playPauseButton.onClick { when (player.playingState) { - VoiceBroadcastPlayer.State.PLAYING -> { - callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) - } + VoiceBroadcastPlayer.State.PLAYING -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) VoiceBroadcastPlayer.State.PAUSED, - VoiceBroadcastPlayer.State.IDLE -> { - callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) - } + VoiceBroadcastPlayer.State.IDLE -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) VoiceBroadcastPlayer.State.BUFFERING -> Unit } } @@ -106,20 +100,22 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } private fun bindSeekBar(holder: Holder) { - holder.durationView.text = formatPlaybackTime(duration) - holder.seekBar.max = duration - holder.seekBar.setOnSeekBarChangeListener(object : SeekBar.OnSeekBarChangeListener { - override fun onProgressChanged(seekBar: SeekBar, progress: Int, fromUser: Boolean) = Unit + with(holder) { + durationView.text = formatPlaybackTime(duration) + seekBar.max = duration + seekBar.setOnSeekBarChangeListener(object : SeekBar.OnSeekBarChangeListener { + override fun onProgressChanged(seekBar: SeekBar, progress: Int, fromUser: Boolean) = Unit - override fun onStartTrackingTouch(seekBar: SeekBar) { - isUserSeeking = true - } + override fun onStartTrackingTouch(seekBar: SeekBar) { + isUserSeeking = true + } - override fun onStopTrackingTouch(seekBar: SeekBar) { - callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcast, seekBar.progress, duration)) - isUserSeeking = false - } - }) + override fun onStopTrackingTouch(seekBar: SeekBar) { + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.SeekTo(voiceBroadcast, seekBar.progress, duration)) + isUserSeeking = false + } + }) + } playbackTracker.track(voiceBroadcast.voiceBroadcastId, object : AudioMessagePlaybackTracker.Listener { override fun onUpdate(state: State) { renderBackwardForwardButtons(holder, state) From 226e2026a12452884b05434815afe609e671a98d Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 11:42:04 +0100 Subject: [PATCH 24/47] Remove item listeners --- .../timeline/item/MessageVoiceBroadcastListeningItem.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index a43783a626..c83b1f6954 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -140,8 +140,13 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem override fun unbind(holder: Holder) { super.unbind(holder) player.removeListener(voiceBroadcast, playerListener) - holder.seekBar.setOnSeekBarChangeListener(null) playbackTracker.untrack(voiceBroadcast.voiceBroadcastId) + with(holder) { + seekBar.onClick(null) + playPauseButton.onClick(null) + fastForwardButton.onClick(null) + fastBackwardButton.onClick(null) + } } override fun getViewStubId() = STUB_ID From 6b57b1190cccbe8e73e8e5b3934c14ad399b57eb Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 11:46:30 +0100 Subject: [PATCH 25/47] Make AudioMessagePlaybackTracker.Listener interface funny --- .../helper/AudioMessagePlaybackTracker.kt | 2 +- .../detail/timeline/item/MessageAudioItem.kt | 16 +++++++--------- .../item/MessageVoiceBroadcastListeningItem.kt | 12 +++++------- .../detail/timeline/item/MessageVoiceItem.kt | 16 +++++++--------- 4 files changed, 20 insertions(+), 26 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt index 7e40b92ac8..91f27ce5a8 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt @@ -148,7 +148,7 @@ class AudioMessagePlaybackTracker @Inject constructor() { const val RECORDING_ID = "RECORDING_ID" } - interface Listener { + fun interface Listener { fun onUpdate(state: State) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageAudioItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageAudioItem.kt index fda9a1465f..3e8d6cb487 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageAudioItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageAudioItem.kt @@ -140,16 +140,14 @@ abstract class MessageAudioItem : AbsMessageItem<MessageAudioItem.Holder>() { } private fun renderStateBasedOnAudioPlayback(holder: Holder) { - audioMessagePlaybackTracker.track(attributes.informationData.eventId, object : AudioMessagePlaybackTracker.Listener { - override fun onUpdate(state: AudioMessagePlaybackTracker.Listener.State) { - when (state) { - is AudioMessagePlaybackTracker.Listener.State.Idle -> renderIdleState(holder) - is AudioMessagePlaybackTracker.Listener.State.Playing -> renderPlayingState(holder, state) - is AudioMessagePlaybackTracker.Listener.State.Paused -> renderPausedState(holder, state) - is AudioMessagePlaybackTracker.Listener.State.Recording -> Unit - } + audioMessagePlaybackTracker.track(attributes.informationData.eventId) { state -> + when (state) { + is AudioMessagePlaybackTracker.Listener.State.Idle -> renderIdleState(holder) + is AudioMessagePlaybackTracker.Listener.State.Playing -> renderPlayingState(holder, state) + is AudioMessagePlaybackTracker.Listener.State.Paused -> renderPausedState(holder, state) + is AudioMessagePlaybackTracker.Listener.State.Recording -> Unit } - }) + } } private fun renderIdleState(holder: Holder) { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index c83b1f6954..1076e06b7e 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -116,14 +116,12 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } }) } - playbackTracker.track(voiceBroadcast.voiceBroadcastId, object : AudioMessagePlaybackTracker.Listener { - override fun onUpdate(state: State) { - renderBackwardForwardButtons(holder, state) - if (!isUserSeeking) { - holder.seekBar.progress = playbackTracker.getPlaybackTime(voiceBroadcast.voiceBroadcastId) - } + playbackTracker.track(voiceBroadcast.voiceBroadcastId) { playbackState -> + renderBackwardForwardButtons(holder, playbackState) + if (!isUserSeeking) { + holder.seekBar.progress = playbackTracker.getPlaybackTime(voiceBroadcast.voiceBroadcastId) } - }) + } } private fun renderBackwardForwardButtons(holder: Holder, playbackState: State) { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt index e057950790..d3f320db7d 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt @@ -122,16 +122,14 @@ abstract class MessageVoiceItem : AbsMessageItem<MessageVoiceItem.Holder>() { true } - audioMessagePlaybackTracker.track(attributes.informationData.eventId, object : AudioMessagePlaybackTracker.Listener { - override fun onUpdate(state: AudioMessagePlaybackTracker.Listener.State) { - when (state) { - is AudioMessagePlaybackTracker.Listener.State.Idle -> renderIdleState(holder, waveformColorIdle, waveformColorPlayed) - is AudioMessagePlaybackTracker.Listener.State.Playing -> renderPlayingState(holder, state, waveformColorIdle, waveformColorPlayed) - is AudioMessagePlaybackTracker.Listener.State.Paused -> renderPausedState(holder, state, waveformColorIdle, waveformColorPlayed) - is AudioMessagePlaybackTracker.Listener.State.Recording -> Unit - } + audioMessagePlaybackTracker.track(attributes.informationData.eventId) { state -> + when (state) { + is AudioMessagePlaybackTracker.Listener.State.Idle -> renderIdleState(holder, waveformColorIdle, waveformColorPlayed) + is AudioMessagePlaybackTracker.Listener.State.Playing -> renderPlayingState(holder, state, waveformColorIdle, waveformColorPlayed) + is AudioMessagePlaybackTracker.Listener.State.Paused -> renderPausedState(holder, state, waveformColorIdle, waveformColorPlayed) + is AudioMessagePlaybackTracker.Listener.State.Recording -> Unit } - }) + } } private fun getTouchedPositionPercentage(motionEvent: MotionEvent, view: View) = (motionEvent.x / view.width).coerceIn(0f, 1f) From 305a362e9ee15e6a915053a5457d70e6de087dc8 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 12:04:30 +0100 Subject: [PATCH 26/47] Fix play action on other voice broadcast than the current one --- .../item/MessageVoiceBroadcastListeningItem.kt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index 1076e06b7e..4b91bbfb0e 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -27,7 +27,6 @@ import com.airbnb.epoxy.EpoxyModelClass import im.vector.app.R import im.vector.app.core.epoxy.onClick import im.vector.app.features.home.room.detail.RoomDetailAction.VoiceBroadcastAction -import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker.Listener.State import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer import im.vector.app.features.voicebroadcast.views.VoiceBroadcastMetadataView @@ -53,11 +52,15 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem private fun bindButtons(holder: Holder) { with(holder) { playPauseButton.onClick { - when (player.playingState) { - VoiceBroadcastPlayer.State.PLAYING -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) - VoiceBroadcastPlayer.State.PAUSED, - VoiceBroadcastPlayer.State.IDLE -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) - VoiceBroadcastPlayer.State.BUFFERING -> Unit + if (player.currentVoiceBroadcast == voiceBroadcast) { + when (player.playingState) { + VoiceBroadcastPlayer.State.PLAYING -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) + VoiceBroadcastPlayer.State.PAUSED, + VoiceBroadcastPlayer.State.IDLE -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) + VoiceBroadcastPlayer.State.BUFFERING -> Unit + } + } else { + callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) } } fastBackwardButton.onClick { From be18f4ec78af8bb77f3ff99f65157f3c12aaca35 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 14:09:15 +0100 Subject: [PATCH 27/47] remove unused imports --- .../listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt | 1 - .../voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt index 33e370e9bc..d12a329142 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/usecase/GetLiveVoiceBroadcastChunksUseCase.kt @@ -31,7 +31,6 @@ import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.flow.lastOrNull import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.runningReduce import kotlinx.coroutines.runBlocking diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt index 7106322f06..696d300fc3 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastEventUseCase.kt @@ -22,9 +22,7 @@ import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState import im.vector.app.features.voicebroadcast.model.asVoiceBroadcastEvent import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.filter -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull @@ -56,7 +54,7 @@ class GetVoiceBroadcastEventUseCase @Inject constructor( return when (latestEvent?.content?.voiceBroadcastState) { null, VoiceBroadcastState.STOPPED -> flowOf(latestEvent.toOptional()) - else -> { + else -> { room.flow() .liveStateEvent(VoiceBroadcastConstants.STATE_ROOM_VOICE_BROADCAST_INFO, QueryStringValue.Equals(latestEvent.root.stateKey.orEmpty())) .unwrap() From 9e83d88f08ae492a2f1271f0df255edf66c546c4 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 14:52:53 +0100 Subject: [PATCH 28/47] Fix seek position when listening another voice broadcast --- .../listening/VoiceBroadcastPlayerImpl.kt | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 020edc283a..977b3906e0 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -216,8 +216,10 @@ class VoiceBroadcastPlayerImpl @Inject constructor( currentMediaPlayer?.pause() } else { stopPlayer() - currentVoiceBroadcast?.voiceBroadcastId?.let { id -> - playbackTracker.updatePausedAtPlaybackTime(id, positionMillis, positionMillis.toFloat() / playlist.duration) + val voiceBroadcastId = currentVoiceBroadcast?.voiceBroadcastId + val duration = playlist.duration.takeIf { it > 0 } + if (voiceBroadcastId != null && duration != null) { + playbackTracker.updatePausedAtPlaybackTime(voiceBroadcastId, positionMillis, positionMillis.toFloat() / duration) } } playingState = State.PAUSED @@ -312,6 +314,22 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } + private fun getCurrentPlaybackPosition(): Int? { + val playlistPosition = playlist.currentItem?.startTime + val computedPosition = currentMediaPlayer?.currentPosition?.let { playlistPosition?.plus(it) } ?: playlistPosition + val savedPosition = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPlaybackTime(it) } + return computedPosition ?: savedPosition + } + + private fun getCurrentPlaybackPercentage(): Float? { + val playlistPosition = playlist.currentItem?.startTime + val computedPosition = currentMediaPlayer?.currentPosition?.let { playlistPosition?.plus(it) } ?: playlistPosition + val duration = playlist.duration.takeIf { it > 0 } + val computedPercentage = if (computedPosition != null && duration != null) computedPosition.toFloat() / duration else null + val savedPercentage = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPercentage(it) } + return computedPercentage ?: savedPercentage + } + private inner class MediaPlayerListener : MediaPlayer.OnInfoListener, MediaPlayer.OnCompletionListener, @@ -369,26 +387,26 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } private fun onPlaybackTick(id: String) { - val currentItem = playlist.currentItem ?: return - val itemStartTime = currentItem.startTime + val playbackTime = getCurrentPlaybackPosition() + val percentage = getCurrentPlaybackPercentage() when (playingState) { - State.PLAYING, - State.PAUSED -> { - val position = itemStartTime + (currentMediaPlayer?.currentPosition ?: 0) - val percentage = position.toFloat() / playlist.duration - if (playingState == State.PLAYING) { - playbackTracker.updatePlayingAtPlaybackTime(id, position, percentage) - } else { - playbackTracker.updatePausedAtPlaybackTime(id, position, percentage) + State.PLAYING -> { + if (playbackTime != null && percentage != null) { + playbackTracker.updatePlayingAtPlaybackTime(id, playbackTime, percentage) } } + State.PAUSED, State.BUFFERING -> { - val playbackTime = playbackTracker.getPlaybackTime(id) - val percentage = playbackTracker.getPercentage(id) - playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) + if (playbackTime != null && percentage != null) { + playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) + } } State.IDLE -> { - playbackTracker.stopPlayback(id) + if (playbackTime == null || percentage == null || playbackTime == playlist.duration) { + playbackTracker.stopPlayback(id) + } else { + playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) + } } } } From 4e533667278c5a5f6900edebc58c5d6cad4c7725 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 15:46:52 +0100 Subject: [PATCH 29/47] Fix default visibility of fast backward/forward buttons --- ...timeline_event_voice_broadcast_listening_stub.xml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml b/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml index 150f1cb281..1d31afba99 100644 --- a/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml +++ b/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml @@ -103,7 +103,9 @@ android:background="@drawable/bg_rounded_button" android:contentDescription="@string/a11y_voice_broadcast_fast_backward" android:src="@drawable/ic_player_backward_30" - app:tint="?vctr_content_secondary" /> + android:visibility="invisible" + app:tint="?vctr_content_secondary" + tools:visibility="visible" /> <ImageButton android:id="@+id/playPauseButton" @@ -121,7 +123,9 @@ android:layout_height="@dimen/voice_broadcast_player_button_size" android:contentDescription="@string/a11y_voice_broadcast_buffering" android:indeterminate="true" - android:indeterminateTint="?vctr_content_secondary" /> + android:indeterminateTint="?vctr_content_secondary" + android:visibility="gone" + tools:visibility="visible" /> <ImageButton android:id="@+id/fastForwardButton" @@ -130,7 +134,9 @@ android:background="@drawable/bg_rounded_button" android:contentDescription="@string/a11y_voice_broadcast_fast_forward" android:src="@drawable/ic_player_forward_30" - app:tint="?vctr_content_secondary" /> + android:visibility="invisible" + app:tint="?vctr_content_secondary" + tools:visibility="visible" /> <SeekBar android:id="@+id/seekBar" From c1dd66003a3a5b60f88e32f58b0e2d4c5dac0838 Mon Sep 17 00:00:00 2001 From: Florian Renaud <florianr@element.io> Date: Mon, 7 Nov 2022 16:05:06 +0100 Subject: [PATCH 30/47] improve end of voice broadcast check --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 977b3906e0..6a6dc6a9e8 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -402,7 +402,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } State.IDLE -> { - if (playbackTime == null || percentage == null || playbackTime == playlist.duration) { + if (playbackTime == null || percentage == null || (playlist.duration - playbackTime) < 50) { playbackTracker.stopPlayback(id) } else { playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) From 06538276d9948dd467cd1bce88a7c11d598af6f5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Nov 2022 23:04:44 +0000 Subject: [PATCH 31/47] Bump kotlin-gradle-plugin from 1.7.20 to 1.7.21 Bumps [kotlin-gradle-plugin](https://github.com/JetBrains/kotlin) from 1.7.20 to 1.7.21. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md) - [Commits](https://github.com/JetBrains/kotlin/commits) --- updated-dependencies: - dependency-name: org.jetbrains.kotlin:kotlin-gradle-plugin dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> --- dependencies.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies.gradle b/dependencies.gradle index 751cba4d44..cc5cb73586 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -8,7 +8,7 @@ ext.versions = [ def gradle = "7.3.1" // Ref: https://kotlinlang.org/releases.html -def kotlin = "1.7.20" +def kotlin = "1.7.21" def kotlinCoroutines = "1.6.4" def dagger = "2.44" def appDistribution = "16.0.0-beta05" From ba6d414f67483bc13001409e13d54f25e84dd8ba Mon Sep 17 00:00:00 2001 From: Onuray Sahin <onurays@element.io> Date: Wed, 9 Nov 2022 16:59:02 +0300 Subject: [PATCH 32/47] Code review fix. --- library/ui-strings/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index 370005363d..5a37cc7f7c 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -3375,7 +3375,7 @@ <string name="device_manager_learn_more_sessions_verified_title">Verified sessions</string> <!-- TODO TO BE REMOVED --> <string name="device_manager_learn_more_sessions_verified">Verified sessions have logged in with your credentials and then been verified, either using your secure passphrase or by cross-verifying.\n\nThis means they hold encryption keys for your previous messages, and confirm to other users you are communicating with that these sessions are really you.</string> - <string name="device_manager_learn_more_sessions_verified_description">Verified sessions are anywhere you are using ${app_name} after entering your passphrase or confirming your identity with another verified session.\n\nThis means that you have all the keys needed to unlock your encrypted messages and confirm to other users that you trust this session.</string> + <string name="device_manager_learn_more_sessions_verified_description">Verified sessions are anywhere you are using this account after entering your passphrase or confirming your identity with another verified session.\n\nThis means that you have all the keys needed to unlock your encrypted messages and confirm to other users that you trust this session.</string> <string name="device_manager_learn_more_session_rename_title">Renaming sessions</string> <string name="device_manager_learn_more_session_rename">Other users in direct messages and rooms that you join are able to view a full list of your sessions.\n\nThis provides them with confidence that they are really speaking to you, but it also means they can see the session name you enter here.</string> <string name="labs_enable_session_manager_title">Enable new session manager</string> From 40e960f19e23201fc83e11b24f800e38daa5ccd6 Mon Sep 17 00:00:00 2001 From: Onuray Sahin <onurays@element.io> Date: Wed, 9 Nov 2022 20:41:53 +0300 Subject: [PATCH 33/47] Lint fix. --- library/ui-strings/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index b8fc5e4fbd..f05a2a11e6 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -3379,7 +3379,7 @@ <string name="device_manager_learn_more_sessions_unverified">Unverified sessions are sessions that have logged in with your credentials but not been cross-verified.\n\nYou should make especially certain that you recognise these sessions as they could represent an unauthorised use of your account.</string> <string name="device_manager_learn_more_sessions_verified_title">Verified sessions</string> <!-- TODO TO BE REMOVED --> - <string name="device_manager_learn_more_sessions_verified">Verified sessions have logged in with your credentials and then been verified, either using your secure passphrase or by cross-verifying.\n\nThis means they hold encryption keys for your previous messages, and confirm to other users you are communicating with that these sessions are really you.</string> + <string name="device_manager_learn_more_sessions_verified" tools:ignore="UnusedResources">Verified sessions have logged in with your credentials and then been verified, either using your secure passphrase or by cross-verifying.\n\nThis means they hold encryption keys for your previous messages, and confirm to other users you are communicating with that these sessions are really you.</string> <string name="device_manager_learn_more_sessions_verified_description">Verified sessions are anywhere you are using this account after entering your passphrase or confirming your identity with another verified session.\n\nThis means that you have all the keys needed to unlock your encrypted messages and confirm to other users that you trust this session.</string> <string name="device_manager_learn_more_session_rename_title">Renaming sessions</string> <string name="device_manager_learn_more_session_rename">Other users in direct messages and rooms that you join are able to view a full list of your sessions.\n\nThis provides them with confidence that they are really speaking to you, but it also means they can see the session name you enter here.</string> From 41ab29d4c008c7abd61db0f6cdc860cf01e894e9 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Wed, 2 Nov 2022 17:17:44 +0100 Subject: [PATCH 34/47] Adding changelog entry --- changelog.d/7512.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7512.feature diff --git a/changelog.d/7512.feature b/changelog.d/7512.feature new file mode 100644 index 0000000000..00411a75ad --- /dev/null +++ b/changelog.d/7512.feature @@ -0,0 +1 @@ +Push notifications toggle: align implementation for current session From 2941cfa329519de3248e437f7e92de3c5d7ab802 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Thu, 3 Nov 2022 11:39:56 +0100 Subject: [PATCH 35/47] Adding use cases to handle toggle of push notifications for current session --- .../src/main/res/values/strings.xml | 3 +- .../vector/app/core/pushers/PushersManager.kt | 6 -- ...leNotificationsForCurrentSessionUseCase.kt | 44 ++++++++++++ ...leNotificationsForCurrentSessionUseCase.kt | 72 +++++++++++++++++++ ...rSettingsNotificationPreferenceFragment.kt | 32 +++------ 5 files changed, 126 insertions(+), 31 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt create mode 100644 vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index f05a2a11e6..372692770e 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -1679,7 +1679,8 @@ <string name="create_new_room">Create New Room</string> <string name="create_new_space">Create New Space</string> <string name="error_no_network">No network. Please check your Internet connection.</string> - <string name="error_check_network">Something went wrong. Please check your network connection and try again.</string> + <!-- TODO delete --> + <string name="error_check_network" tools:ignore="UnusedResources">Something went wrong. Please check your network connection and try again.</string> <string name="change_room_directory_network">"Change network"</string> <string name="please_wait">"Please wait…"</string> <string name="updating_your_data">Updating your data…</string> diff --git a/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt b/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt index cda6f5bae8..6f186262fc 100644 --- a/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt +++ b/vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt @@ -97,12 +97,6 @@ class PushersManager @Inject constructor( return session.pushersService().getPushers().firstOrNull { it.deviceId == deviceId } } - suspend fun togglePusherForCurrentSession(enable: Boolean) { - val session = activeSessionHolder.getSafeActiveSession() ?: return - val pusher = getPusherForCurrentSession() ?: return - session.pushersService().togglePusher(pusher, enable) - } - suspend fun unregisterEmailPusher(email: String) { val currentSession = activeSessionHolder.getSafeActiveSession() ?: return currentSession.pushersService().removeEmailPusher(email) diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt new file mode 100644 index 0000000000..8962e8d67d --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications + +import im.vector.app.core.di.ActiveSessionHolder +import im.vector.app.core.pushers.PushersManager +import im.vector.app.core.pushers.UnifiedPushHelper +import im.vector.app.features.settings.devices.v2.notification.CheckIfCanTogglePushNotificationsViaPusherUseCase +import im.vector.app.features.settings.devices.v2.notification.TogglePushNotificationUseCase +import javax.inject.Inject + +class DisableNotificationsForCurrentSessionUseCase @Inject constructor( + private val activeSessionHolder: ActiveSessionHolder, + private val unifiedPushHelper: UnifiedPushHelper, + private val pushersManager: PushersManager, + private val checkIfCanTogglePushNotificationsViaPusherUseCase: CheckIfCanTogglePushNotificationsViaPusherUseCase, + private val togglePushNotificationUseCase: TogglePushNotificationUseCase, +) { + + // TODO add unit tests + suspend fun execute() { + val session = activeSessionHolder.getSafeActiveSession() ?: return + val deviceId = session.sessionParams.deviceId ?: return + if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute()) { + togglePushNotificationUseCase.execute(deviceId, enabled = false) + } else { + unifiedPushHelper.unregister(pushersManager) + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt new file mode 100644 index 0000000000..ef37f67bef --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications + +import androidx.fragment.app.FragmentActivity +import im.vector.app.core.di.ActiveSessionHolder +import im.vector.app.core.pushers.FcmHelper +import im.vector.app.core.pushers.PushersManager +import im.vector.app.core.pushers.UnifiedPushHelper +import im.vector.app.features.settings.devices.v2.notification.CheckIfCanTogglePushNotificationsViaPusherUseCase +import im.vector.app.features.settings.devices.v2.notification.TogglePushNotificationUseCase +import javax.inject.Inject +import kotlin.coroutines.resume +import kotlin.coroutines.resumeWithException +import kotlin.coroutines.suspendCoroutine + +class EnableNotificationsForCurrentSessionUseCase @Inject constructor( + private val activeSessionHolder: ActiveSessionHolder, + private val unifiedPushHelper: UnifiedPushHelper, + private val pushersManager: PushersManager, + private val fcmHelper: FcmHelper, + private val checkIfCanTogglePushNotificationsViaPusherUseCase: CheckIfCanTogglePushNotificationsViaPusherUseCase, + private val togglePushNotificationUseCase: TogglePushNotificationUseCase, +) { + + // TODO add unit tests + suspend fun execute(fragmentActivity: FragmentActivity) { + val pusherForCurrentSession = pushersManager.getPusherForCurrentSession() + if (pusherForCurrentSession == null) { + registerPusher(fragmentActivity) + } + + if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute()) { + val session = activeSessionHolder.getSafeActiveSession() ?: return + val deviceId = session.sessionParams.deviceId ?: return + togglePushNotificationUseCase.execute(deviceId, enabled = true) + } + } + + private suspend fun registerPusher(fragmentActivity: FragmentActivity) { + suspendCoroutine { continuation -> + try { + unifiedPushHelper.register(fragmentActivity) { + if (unifiedPushHelper.isEmbeddedDistributor()) { + fcmHelper.ensureFcmTokenIsRetrieved( + fragmentActivity, + pushersManager, + registerPusher = true + ) + } + continuation.resume(Unit) + } + } catch (error: Exception) { + continuation.resumeWithException(error) + } + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt index f800c518f3..4a43a20de3 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt @@ -57,7 +57,6 @@ import im.vector.app.features.settings.VectorSettingsBaseFragment import im.vector.app.features.settings.VectorSettingsFragmentInteractionListener import im.vector.lib.core.utils.compat.getParcelableExtraCompat import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.Session @@ -81,6 +80,8 @@ class VectorSettingsNotificationPreferenceFragment : @Inject lateinit var guardServiceStarter: GuardServiceStarter @Inject lateinit var vectorFeatures: VectorFeatures @Inject lateinit var notificationPermissionManager: NotificationPermissionManager + @Inject lateinit var disableNotificationsForCurrentSessionUseCase: DisableNotificationsForCurrentSessionUseCase + @Inject lateinit var enableNotificationsForCurrentSessionUseCase: EnableNotificationsForCurrentSessionUseCase override var titleRes: Int = R.string.settings_notifications override val preferenceXmlRes = R.xml.vector_settings_notifications @@ -126,28 +127,12 @@ class VectorSettingsNotificationPreferenceFragment : it.setTransactionalSwitchChangeListener(lifecycleScope) { isChecked -> if (isChecked) { - unifiedPushHelper.register(requireActivity()) { - // Update the summary - if (unifiedPushHelper.isEmbeddedDistributor()) { - fcmHelper.ensureFcmTokenIsRetrieved( - requireActivity(), - pushersManager, - vectorPreferences.areNotificationEnabledForDevice() - ) - } - findPreference<VectorPreference>(VectorPreferences.SETTINGS_NOTIFICATION_METHOD_KEY) - ?.summary = unifiedPushHelper.getCurrentDistributorName() - lifecycleScope.launch { - val result = runCatching { - pushersManager.togglePusherForCurrentSession(true) - } + enableNotificationsForCurrentSessionUseCase.execute(requireActivity()) - result.exceptionOrNull()?.let { _ -> - Toast.makeText(context, R.string.error_check_network, Toast.LENGTH_SHORT).show() - it.isChecked = false - } - } - } + findPreference<VectorPreference>(VectorPreferences.SETTINGS_NOTIFICATION_METHOD_KEY) + ?.summary = unifiedPushHelper.getCurrentDistributorName() + + // TODO test with API 33 notificationPermissionManager.eventuallyRequestPermission( requireActivity(), postPermissionLauncher, @@ -155,8 +140,7 @@ class VectorSettingsNotificationPreferenceFragment : ignorePreference = true ) } else { - unifiedPushHelper.unregister(pushersManager) - session.pushersService().refreshPushers() + disableNotificationsForCurrentSessionUseCase.execute() notificationPermissionManager.eventuallyRevokePermission(requireActivity()) } } From 67d2a6faab0979c48e1b9d9f43de8565d06e0635 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Thu, 3 Nov 2022 16:31:41 +0100 Subject: [PATCH 36/47] Use the preference value to render the push notifications toggle --- ...rSettingsNotificationPreferenceFragment.kt | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt index 4a43a20de3..58f86bc949 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsNotificationPreferenceFragment.kt @@ -120,31 +120,25 @@ class VectorSettingsNotificationPreferenceFragment : (pref as SwitchPreference).isChecked = areNotifEnabledAtAccountLevel } - findPreference<SwitchPreference>(VectorPreferences.SETTINGS_ENABLE_THIS_DEVICE_PREFERENCE_KEY)?.let { - pushersManager.getPusherForCurrentSession()?.let { pusher -> - it.isChecked = pusher.enabled - } + findPreference<SwitchPreference>(VectorPreferences.SETTINGS_ENABLE_THIS_DEVICE_PREFERENCE_KEY) + ?.setTransactionalSwitchChangeListener(lifecycleScope) { isChecked -> + if (isChecked) { + enableNotificationsForCurrentSessionUseCase.execute(requireActivity()) - it.setTransactionalSwitchChangeListener(lifecycleScope) { isChecked -> - if (isChecked) { - enableNotificationsForCurrentSessionUseCase.execute(requireActivity()) + findPreference<VectorPreference>(VectorPreferences.SETTINGS_NOTIFICATION_METHOD_KEY) + ?.summary = unifiedPushHelper.getCurrentDistributorName() - findPreference<VectorPreference>(VectorPreferences.SETTINGS_NOTIFICATION_METHOD_KEY) - ?.summary = unifiedPushHelper.getCurrentDistributorName() - - // TODO test with API 33 - notificationPermissionManager.eventuallyRequestPermission( - requireActivity(), - postPermissionLauncher, - showRationale = false, - ignorePreference = true - ) - } else { - disableNotificationsForCurrentSessionUseCase.execute() - notificationPermissionManager.eventuallyRevokePermission(requireActivity()) + notificationPermissionManager.eventuallyRequestPermission( + requireActivity(), + postPermissionLauncher, + showRationale = false, + ignorePreference = true + ) + } else { + disableNotificationsForCurrentSessionUseCase.execute() + notificationPermissionManager.eventuallyRevokePermission(requireActivity()) + } } - } - } findPreference<VectorPreference>(VectorPreferences.SETTINGS_FDROID_BACKGROUND_SYNC_MODE)?.let { it.onPreferenceClickListener = Preference.OnPreferenceClickListener { From 24a5cfa9e54fed6b9d9aaebd5f9471222e883786 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Thu, 3 Nov 2022 16:33:16 +0100 Subject: [PATCH 37/47] Listen for pusher or account data changes to update the local setting --- .../HomeServerCapabilitiesService.kt | 8 +++ .../DefaultHomeServerCapabilitiesService.kt | 6 +++ .../HomeServerCapabilitiesDataSource.kt | 17 +++++- .../vector/app/core/di/ActiveSessionHolder.kt | 1 + .../EnableNotificationsSettingUpdater.kt | 41 ++++++++++++++ ...ableNotificationsSettingOnChangeUseCase.kt | 53 +++++++++++++++++++ .../ConfigureAndStartSessionUseCase.kt | 4 ++ ...TogglePushNotificationsViaPusherUseCase.kt | 36 +++++++++++++ ...ePushNotificationsViaAccountDataUseCase.kt | 15 +++--- ...TogglePushNotificationsViaPusherUseCase.kt | 19 +++---- .../GetNotificationsStatusUseCase.kt | 34 ++++++------ .../TogglePushNotificationUseCase.kt | 4 +- .../v2/overview/SessionOverviewViewModel.kt | 8 +-- ...leNotificationsForCurrentSessionUseCase.kt | 2 +- ...leNotificationsForCurrentSessionUseCase.kt | 4 +- 15 files changed, 206 insertions(+), 46 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt create mode 100644 vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt create mode 100644 vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/homeserver/HomeServerCapabilitiesService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/homeserver/HomeServerCapabilitiesService.kt index 9d2c48e194..c65a5382fb 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/homeserver/HomeServerCapabilitiesService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/homeserver/HomeServerCapabilitiesService.kt @@ -16,6 +16,9 @@ package org.matrix.android.sdk.api.session.homeserver +import androidx.lifecycle.LiveData +import org.matrix.android.sdk.api.util.Optional + /** * This interface defines a method to retrieve the homeserver capabilities. */ @@ -30,4 +33,9 @@ interface HomeServerCapabilitiesService { * Get the HomeServer capabilities. */ fun getHomeServerCapabilities(): HomeServerCapabilities + + /** + * Get a LiveData on the HomeServer capabilities. + */ + fun getHomeServerCapabilitiesLive(): LiveData<Optional<HomeServerCapabilities>> } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/DefaultHomeServerCapabilitiesService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/DefaultHomeServerCapabilitiesService.kt index 4c755b54b5..eb9e862de2 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/DefaultHomeServerCapabilitiesService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/DefaultHomeServerCapabilitiesService.kt @@ -16,8 +16,10 @@ package org.matrix.android.sdk.internal.session.homeserver +import androidx.lifecycle.LiveData import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilities import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilitiesService +import org.matrix.android.sdk.api.util.Optional import javax.inject.Inject internal class DefaultHomeServerCapabilitiesService @Inject constructor( @@ -33,4 +35,8 @@ internal class DefaultHomeServerCapabilitiesService @Inject constructor( return homeServerCapabilitiesDataSource.getHomeServerCapabilities() ?: HomeServerCapabilities() } + + override fun getHomeServerCapabilitiesLive(): LiveData<Optional<HomeServerCapabilities>> { + return homeServerCapabilitiesDataSource.getHomeServerCapabilitiesLive() + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/HomeServerCapabilitiesDataSource.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/HomeServerCapabilitiesDataSource.kt index 6c913fa41e..beb1e67e40 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/HomeServerCapabilitiesDataSource.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/homeserver/HomeServerCapabilitiesDataSource.kt @@ -16,9 +16,14 @@ package org.matrix.android.sdk.internal.session.homeserver +import androidx.lifecycle.LiveData +import androidx.lifecycle.Transformations import com.zhuinden.monarchy.Monarchy import io.realm.Realm +import io.realm.kotlin.where import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilities +import org.matrix.android.sdk.api.util.Optional +import org.matrix.android.sdk.api.util.toOptional import org.matrix.android.sdk.internal.database.mapper.HomeServerCapabilitiesMapper import org.matrix.android.sdk.internal.database.model.HomeServerCapabilitiesEntity import org.matrix.android.sdk.internal.database.query.get @@ -26,7 +31,7 @@ import org.matrix.android.sdk.internal.di.SessionDatabase import javax.inject.Inject internal class HomeServerCapabilitiesDataSource @Inject constructor( - @SessionDatabase private val monarchy: Monarchy + @SessionDatabase private val monarchy: Monarchy, ) { fun getHomeServerCapabilities(): HomeServerCapabilities? { return Realm.getInstance(monarchy.realmConfiguration).use { realm -> @@ -35,4 +40,14 @@ internal class HomeServerCapabilitiesDataSource @Inject constructor( } } } + + fun getHomeServerCapabilitiesLive(): LiveData<Optional<HomeServerCapabilities>> { + val liveData = monarchy.findAllMappedWithChanges( + { realm: Realm -> realm.where<HomeServerCapabilitiesEntity>() }, + { HomeServerCapabilitiesMapper.map(it) } + ) + return Transformations.map(liveData) { + it.firstOrNull().toOptional() + } + } } diff --git a/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt b/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt index 7e4f73e7a5..1e9f080303 100644 --- a/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt +++ b/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt @@ -19,6 +19,7 @@ package im.vector.app.core.di import android.content.Context import im.vector.app.ActiveSessionDataSource import im.vector.app.core.extensions.startSyncing +import im.vector.app.core.notification.EnableNotificationsSettingUpdater import im.vector.app.core.pushers.UnifiedPushHelper import im.vector.app.core.services.GuardServiceStarter import im.vector.app.core.session.ConfigureAndStartSessionUseCase diff --git a/vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt b/vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt new file mode 100644 index 0000000000..21febaee9d --- /dev/null +++ b/vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.core.notification + +import im.vector.app.features.session.coroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.launch +import org.matrix.android.sdk.api.session.Session +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class EnableNotificationsSettingUpdater @Inject constructor( + private val updateEnableNotificationsSettingOnChangeUseCase: UpdateEnableNotificationsSettingOnChangeUseCase, +) { + + private var job: Job? = null + + fun onSessionsStarted(session: Session) { + job?.cancel() + job = session.coroutineScope.launch { + updateEnableNotificationsSettingOnChangeUseCase.execute(session) + .launchIn(this) + } + } +} diff --git a/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt b/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt new file mode 100644 index 0000000000..a6bcf17b5c --- /dev/null +++ b/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.core.notification + +import im.vector.app.features.settings.VectorPreferences +import im.vector.app.features.settings.devices.v2.notification.GetNotificationsStatusUseCase +import im.vector.app.features.settings.devices.v2.notification.NotificationsStatus +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.onEach +import org.matrix.android.sdk.api.session.Session +import timber.log.Timber +import javax.inject.Inject + +/** + * Listen for changes in either Pusher or Account data to update the local enable notifications + * setting for the current device. + */ +class UpdateEnableNotificationsSettingOnChangeUseCase @Inject constructor( + private val vectorPreferences: VectorPreferences, + private val getNotificationsStatusUseCase: GetNotificationsStatusUseCase, +) { + + // TODO add unit tests + fun execute(session: Session): Flow<NotificationsStatus> { + val deviceId = session.sessionParams.deviceId ?: return emptyFlow() + return getNotificationsStatusUseCase.execute(session, deviceId) + .onEach(::updatePreference) + } + + private fun updatePreference(notificationStatus: NotificationsStatus) { + Timber.d("updatePreference with status=$notificationStatus") + when (notificationStatus) { + NotificationsStatus.ENABLED -> vectorPreferences.setNotificationEnabledForDevice(true) + NotificationsStatus.DISABLED -> vectorPreferences.setNotificationEnabledForDevice(false) + else -> Unit + } + } +} diff --git a/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt b/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt index a5e1fe68bd..00dc1ab5f9 100644 --- a/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt @@ -19,6 +19,7 @@ package im.vector.app.core.session import android.content.Context import dagger.hilt.android.qualifiers.ApplicationContext import im.vector.app.core.extensions.startSyncing +import im.vector.app.core.notification.EnableNotificationsSettingUpdater import im.vector.app.core.session.clientinfo.UpdateMatrixClientInfoUseCase import im.vector.app.features.call.webrtc.WebRtcCallManager import im.vector.app.features.settings.VectorPreferences @@ -32,8 +33,10 @@ class ConfigureAndStartSessionUseCase @Inject constructor( private val webRtcCallManager: WebRtcCallManager, private val updateMatrixClientInfoUseCase: UpdateMatrixClientInfoUseCase, private val vectorPreferences: VectorPreferences, + private val enableNotificationsSettingUpdater: EnableNotificationsSettingUpdater, ) { + // TODO update unit tests suspend fun execute(session: Session, startSyncing: Boolean = true) { Timber.i("Configure and start session for ${session.myUserId}. startSyncing: $startSyncing") session.open() @@ -46,5 +49,6 @@ class ConfigureAndStartSessionUseCase @Inject constructor( if (vectorPreferences.isClientInfoRecordingEnabled()) { updateMatrixClientInfoUseCase.execute(session) } + enableNotificationsSettingUpdater.onSessionsStarted(session) } } diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt new file mode 100644 index 0000000000..963768ca04 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.devices.v2.notification + +import androidx.lifecycle.asFlow +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.flow.unwrap +import javax.inject.Inject + +class CanTogglePushNotificationsViaPusherUseCase @Inject constructor() { + + fun execute(session: Session): Flow<Boolean> { + return session + .homeServerCapabilitiesService() + .getHomeServerCapabilitiesLive() + .asFlow() + .unwrap() + .map { it.canRemotelyTogglePushNotificationsOfDevices } + } +} diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt index dbf9adca14..194a2aebbf 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt @@ -16,18 +16,15 @@ package im.vector.app.features.settings.devices.v2.notification -import im.vector.app.core.di.ActiveSessionHolder +import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.accountdata.UserAccountDataTypes import javax.inject.Inject -class CheckIfCanTogglePushNotificationsViaAccountDataUseCase @Inject constructor( - private val activeSessionHolder: ActiveSessionHolder, -) { +class CheckIfCanTogglePushNotificationsViaAccountDataUseCase @Inject constructor() { - fun execute(deviceId: String): Boolean { - return activeSessionHolder - .getSafeActiveSession() - ?.accountDataService() - ?.getUserAccountDataEvent(UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + deviceId) != null + fun execute(session: Session, deviceId: String): Boolean { + return session + .accountDataService() + .getUserAccountDataEvent(UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + deviceId) != null } } diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt index 0d5bce663a..ca314bf145 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt @@ -16,20 +16,15 @@ package im.vector.app.features.settings.devices.v2.notification -import im.vector.app.core.di.ActiveSessionHolder -import org.matrix.android.sdk.api.extensions.orFalse +import org.matrix.android.sdk.api.session.Session import javax.inject.Inject -class CheckIfCanTogglePushNotificationsViaPusherUseCase @Inject constructor( - private val activeSessionHolder: ActiveSessionHolder, -) { +class CheckIfCanTogglePushNotificationsViaPusherUseCase @Inject constructor() { - fun execute(): Boolean { - return activeSessionHolder - .getSafeActiveSession() - ?.homeServerCapabilitiesService() - ?.getHomeServerCapabilities() - ?.canRemotelyTogglePushNotificationsOfDevices - .orFalse() + fun execute(session: Session): Boolean { + return session + .homeServerCapabilitiesService() + .getHomeServerCapabilities() + .canRemotelyTogglePushNotificationsOfDevices } } diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt index 69659bf23f..03e4e31f2e 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt @@ -16,12 +16,13 @@ package im.vector.app.features.settings.devices.v2.notification -import im.vector.app.core.di.ActiveSessionHolder import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import org.matrix.android.sdk.api.account.LocalNotificationSettingsContent +import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.accountdata.UserAccountDataTypes import org.matrix.android.sdk.api.session.events.model.toModel import org.matrix.android.sdk.flow.flow @@ -29,16 +30,13 @@ import org.matrix.android.sdk.flow.unwrap import javax.inject.Inject class GetNotificationsStatusUseCase @Inject constructor( - private val activeSessionHolder: ActiveSessionHolder, - private val checkIfCanTogglePushNotificationsViaPusherUseCase: CheckIfCanTogglePushNotificationsViaPusherUseCase, + private val canTogglePushNotificationsViaPusherUseCase: CanTogglePushNotificationsViaPusherUseCase, private val checkIfCanTogglePushNotificationsViaAccountDataUseCase: CheckIfCanTogglePushNotificationsViaAccountDataUseCase, ) { - fun execute(deviceId: String): Flow<NotificationsStatus> { - val session = activeSessionHolder.getSafeActiveSession() + fun execute(session: Session, deviceId: String): Flow<NotificationsStatus> { return when { - session == null -> flowOf(NotificationsStatus.NOT_SUPPORTED) - checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(deviceId) -> { + checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(session, deviceId) -> { session.flow() .liveUserAccountData(UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + deviceId) .unwrap() @@ -46,15 +44,19 @@ class GetNotificationsStatusUseCase @Inject constructor( .map { if (it == true) NotificationsStatus.ENABLED else NotificationsStatus.DISABLED } .distinctUntilChanged() } - checkIfCanTogglePushNotificationsViaPusherUseCase.execute() -> { - session.flow() - .livePushers() - .map { it.filter { pusher -> pusher.deviceId == deviceId } } - .map { it.takeIf { it.isNotEmpty() }?.any { pusher -> pusher.enabled } } - .map { if (it == true) NotificationsStatus.ENABLED else NotificationsStatus.DISABLED } - .distinctUntilChanged() - } - else -> flowOf(NotificationsStatus.NOT_SUPPORTED) + else -> canTogglePushNotificationsViaPusherUseCase.execute(session) + .flatMapLatest { canToggle -> + if (canToggle) { + session.flow() + .livePushers() + .map { it.filter { pusher -> pusher.deviceId == deviceId } } + .map { it.takeIf { it.isNotEmpty() }?.any { pusher -> pusher.enabled } } + .map { if (it == true) NotificationsStatus.ENABLED else NotificationsStatus.DISABLED } + .distinctUntilChanged() + } else { + flowOf(NotificationsStatus.NOT_SUPPORTED) + } + } } } } diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt index be9012e9f1..7969bbbe9b 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt @@ -31,14 +31,14 @@ class TogglePushNotificationUseCase @Inject constructor( suspend fun execute(deviceId: String, enabled: Boolean) { val session = activeSessionHolder.getSafeActiveSession() ?: return - if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute()) { + if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute(session)) { val devicePusher = session.pushersService().getPushers().firstOrNull { it.deviceId == deviceId } devicePusher?.let { pusher -> session.pushersService().togglePusher(pusher, enabled) } } - if (checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(deviceId)) { + if (checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(session, deviceId)) { val newNotificationSettingsContent = LocalNotificationSettingsContent(isSilenced = !enabled) session.accountDataService().updateUserAccountData( UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + deviceId, diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt index 9c4ece7e02..a56872e648 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt @@ -96,9 +96,11 @@ class SessionOverviewViewModel @AssistedInject constructor( } private fun observeNotificationsStatus(deviceId: String) { - getNotificationsStatusUseCase.execute(deviceId) - .onEach { setState { copy(notificationsStatus = it) } } - .launchIn(viewModelScope) + activeSessionHolder.getSafeActiveSession()?.let { session -> + getNotificationsStatusUseCase.execute(session, deviceId) + .onEach { setState { copy(notificationsStatus = it) } } + .launchIn(viewModelScope) + } } override fun handle(action: SessionOverviewAction) { diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt index 8962e8d67d..d66bf8b789 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt @@ -35,7 +35,7 @@ class DisableNotificationsForCurrentSessionUseCase @Inject constructor( suspend fun execute() { val session = activeSessionHolder.getSafeActiveSession() ?: return val deviceId = session.sessionParams.deviceId ?: return - if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute()) { + if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute(session)) { togglePushNotificationUseCase.execute(deviceId, enabled = false) } else { unifiedPushHelper.unregister(pushersManager) diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt index ef37f67bef..cee653380a 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt @@ -44,8 +44,8 @@ class EnableNotificationsForCurrentSessionUseCase @Inject constructor( registerPusher(fragmentActivity) } - if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute()) { - val session = activeSessionHolder.getSafeActiveSession() ?: return + val session = activeSessionHolder.getSafeActiveSession() ?: return + if (checkIfCanTogglePushNotificationsViaPusherUseCase.execute(session)) { val deviceId = session.sessionParams.deviceId ?: return togglePushNotificationUseCase.execute(deviceId, enabled = true) } From 6239b3e68618cbbdc9cf18f6afa8cbdda483ec30 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Thu, 3 Nov 2022 17:48:49 +0100 Subject: [PATCH 38/47] Adding some TODOs --- .../notification/CanTogglePushNotificationsViaPusherUseCase.kt | 1 + .../CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt | 1 + .../CheckIfCanTogglePushNotificationsViaPusherUseCase.kt | 1 + .../devices/v2/notification/GetNotificationsStatusUseCase.kt | 1 + .../devices/v2/notification/TogglePushNotificationUseCase.kt | 1 + 5 files changed, 5 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt index 963768ca04..af15e2f349 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt @@ -25,6 +25,7 @@ import javax.inject.Inject class CanTogglePushNotificationsViaPusherUseCase @Inject constructor() { + // TODO add unit tests fun execute(session: Session): Flow<Boolean> { return session .homeServerCapabilitiesService() diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt index 194a2aebbf..85abd7cd35 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt @@ -22,6 +22,7 @@ import javax.inject.Inject class CheckIfCanTogglePushNotificationsViaAccountDataUseCase @Inject constructor() { + // TODO update unit tests fun execute(session: Session, deviceId: String): Boolean { return session .accountDataService() diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt index ca314bf145..9c2a471120 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt @@ -21,6 +21,7 @@ import javax.inject.Inject class CheckIfCanTogglePushNotificationsViaPusherUseCase @Inject constructor() { + // TODO update unit tests fun execute(session: Session): Boolean { return session .homeServerCapabilitiesService() diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt index 03e4e31f2e..ed1d54e118 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt @@ -34,6 +34,7 @@ class GetNotificationsStatusUseCase @Inject constructor( private val checkIfCanTogglePushNotificationsViaAccountDataUseCase: CheckIfCanTogglePushNotificationsViaAccountDataUseCase, ) { + // TODO update unit tests fun execute(session: Session, deviceId: String): Flow<NotificationsStatus> { return when { checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(session, deviceId) -> { diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt index 7969bbbe9b..28018f9687 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt @@ -28,6 +28,7 @@ class TogglePushNotificationUseCase @Inject constructor( private val checkIfCanTogglePushNotificationsViaAccountDataUseCase: CheckIfCanTogglePushNotificationsViaAccountDataUseCase, ) { + // TODO update unit tests suspend fun execute(deviceId: String, enabled: Boolean) { val session = activeSessionHolder.getSafeActiveSession() ?: return From 18929324fee53d6fc81a4c252a65534c073c25b4 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Fri, 4 Nov 2022 10:28:24 +0100 Subject: [PATCH 39/47] Updating existing unit tests --- .../vector/app/core/di/ActiveSessionHolder.kt | 1 - .../ConfigureAndStartSessionUseCase.kt | 1 - ...ePushNotificationsViaAccountDataUseCase.kt | 1 - ...TogglePushNotificationsViaPusherUseCase.kt | 1 - .../GetNotificationsStatusUseCase.kt | 1 - .../TogglePushNotificationUseCase.kt | 1 - .../app/core/pushers/PushersManagerTest.kt | 16 ------ .../ConfigureAndStartSessionUseCaseTest.kt | 6 +++ ...hNotificationsViaAccountDataUseCaseTest.kt | 18 +++---- ...lePushNotificationsViaPusherUseCaseTest.kt | 25 ++------- .../GetNotificationsStatusUseCaseTest.kt | 51 +++++++------------ .../TogglePushNotificationUseCaseTest.kt | 18 ++++--- .../overview/SessionOverviewViewModelTest.kt | 9 ++-- .../FakeEnableNotificationsSettingUpdater.kt | 31 +++++++++++ 14 files changed, 82 insertions(+), 98 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeEnableNotificationsSettingUpdater.kt diff --git a/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt b/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt index 1e9f080303..7e4f73e7a5 100644 --- a/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt +++ b/vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt @@ -19,7 +19,6 @@ package im.vector.app.core.di import android.content.Context import im.vector.app.ActiveSessionDataSource import im.vector.app.core.extensions.startSyncing -import im.vector.app.core.notification.EnableNotificationsSettingUpdater import im.vector.app.core.pushers.UnifiedPushHelper import im.vector.app.core.services.GuardServiceStarter import im.vector.app.core.session.ConfigureAndStartSessionUseCase diff --git a/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt b/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt index 00dc1ab5f9..71863b8642 100644 --- a/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt @@ -36,7 +36,6 @@ class ConfigureAndStartSessionUseCase @Inject constructor( private val enableNotificationsSettingUpdater: EnableNotificationsSettingUpdater, ) { - // TODO update unit tests suspend fun execute(session: Session, startSyncing: Boolean = true) { Timber.i("Configure and start session for ${session.myUserId}. startSyncing: $startSyncing") session.open() diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt index 85abd7cd35..194a2aebbf 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCase.kt @@ -22,7 +22,6 @@ import javax.inject.Inject class CheckIfCanTogglePushNotificationsViaAccountDataUseCase @Inject constructor() { - // TODO update unit tests fun execute(session: Session, deviceId: String): Boolean { return session .accountDataService() diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt index 9c2a471120..ca314bf145 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCase.kt @@ -21,7 +21,6 @@ import javax.inject.Inject class CheckIfCanTogglePushNotificationsViaPusherUseCase @Inject constructor() { - // TODO update unit tests fun execute(session: Session): Boolean { return session .homeServerCapabilitiesService() diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt index ed1d54e118..03e4e31f2e 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCase.kt @@ -34,7 +34,6 @@ class GetNotificationsStatusUseCase @Inject constructor( private val checkIfCanTogglePushNotificationsViaAccountDataUseCase: CheckIfCanTogglePushNotificationsViaAccountDataUseCase, ) { - // TODO update unit tests fun execute(session: Session, deviceId: String): Flow<NotificationsStatus> { return when { checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(session, deviceId) -> { diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt index 28018f9687..7969bbbe9b 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCase.kt @@ -28,7 +28,6 @@ class TogglePushNotificationUseCase @Inject constructor( private val checkIfCanTogglePushNotificationsViaAccountDataUseCase: CheckIfCanTogglePushNotificationsViaAccountDataUseCase, ) { - // TODO update unit tests suspend fun execute(deviceId: String, enabled: Boolean) { val session = activeSessionHolder.getSafeActiveSession() ?: return diff --git a/vector/src/test/java/im/vector/app/core/pushers/PushersManagerTest.kt b/vector/src/test/java/im/vector/app/core/pushers/PushersManagerTest.kt index 113a810ac2..7a1833e057 100644 --- a/vector/src/test/java/im/vector/app/core/pushers/PushersManagerTest.kt +++ b/vector/src/test/java/im/vector/app/core/pushers/PushersManagerTest.kt @@ -29,7 +29,6 @@ import im.vector.app.test.fixtures.CryptoDeviceInfoFixture.aCryptoDeviceInfo import im.vector.app.test.fixtures.PusherFixture import im.vector.app.test.fixtures.SessionParamsFixture import io.mockk.mockk -import kotlinx.coroutines.test.runTest import org.amshove.kluent.shouldBeEqualTo import org.junit.Test import org.matrix.android.sdk.api.session.crypto.model.UnsignedDeviceInfo @@ -101,19 +100,4 @@ class PushersManagerTest { pusher shouldBeEqualTo expectedPusher } - - @Test - fun `when togglePusherForCurrentSession, then do service toggle pusher`() = runTest { - val deviceId = "device_id" - val sessionParams = SessionParamsFixture.aSessionParams( - credentials = CredentialsFixture.aCredentials(deviceId = deviceId) - ) - session.givenSessionParams(sessionParams) - val pusher = PusherFixture.aPusher(deviceId = deviceId) - pushersService.givenGetPushers(listOf(pusher)) - - pushersManager.togglePusherForCurrentSession(true) - - pushersService.verifyTogglePusherCalled(pusher, true) - } } diff --git a/vector/src/test/java/im/vector/app/core/session/ConfigureAndStartSessionUseCaseTest.kt b/vector/src/test/java/im/vector/app/core/session/ConfigureAndStartSessionUseCaseTest.kt index 8d4507e85d..861e59e0f1 100644 --- a/vector/src/test/java/im/vector/app/core/session/ConfigureAndStartSessionUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/core/session/ConfigureAndStartSessionUseCaseTest.kt @@ -19,6 +19,7 @@ package im.vector.app.core.session import im.vector.app.core.extensions.startSyncing import im.vector.app.core.session.clientinfo.UpdateMatrixClientInfoUseCase import im.vector.app.test.fakes.FakeContext +import im.vector.app.test.fakes.FakeEnableNotificationsSettingUpdater import im.vector.app.test.fakes.FakeSession import im.vector.app.test.fakes.FakeVectorPreferences import im.vector.app.test.fakes.FakeWebRtcCallManager @@ -43,12 +44,14 @@ class ConfigureAndStartSessionUseCaseTest { private val fakeWebRtcCallManager = FakeWebRtcCallManager() private val fakeUpdateMatrixClientInfoUseCase = mockk<UpdateMatrixClientInfoUseCase>() private val fakeVectorPreferences = FakeVectorPreferences() + private val fakeEnableNotificationsSettingUpdater = FakeEnableNotificationsSettingUpdater() private val configureAndStartSessionUseCase = ConfigureAndStartSessionUseCase( context = fakeContext.instance, webRtcCallManager = fakeWebRtcCallManager.instance, updateMatrixClientInfoUseCase = fakeUpdateMatrixClientInfoUseCase, vectorPreferences = fakeVectorPreferences.instance, + enableNotificationsSettingUpdater = fakeEnableNotificationsSettingUpdater.instance, ) @Before @@ -68,6 +71,7 @@ class ConfigureAndStartSessionUseCaseTest { fakeWebRtcCallManager.givenCheckForProtocolsSupportIfNeededSucceeds() coJustRun { fakeUpdateMatrixClientInfoUseCase.execute(any()) } fakeVectorPreferences.givenIsClientInfoRecordingEnabled(isEnabled = true) + fakeEnableNotificationsSettingUpdater.givenOnSessionsStarted(fakeSession) // When configureAndStartSessionUseCase.execute(fakeSession, startSyncing = true) @@ -87,6 +91,7 @@ class ConfigureAndStartSessionUseCaseTest { fakeWebRtcCallManager.givenCheckForProtocolsSupportIfNeededSucceeds() coJustRun { fakeUpdateMatrixClientInfoUseCase.execute(any()) } fakeVectorPreferences.givenIsClientInfoRecordingEnabled(isEnabled = false) + fakeEnableNotificationsSettingUpdater.givenOnSessionsStarted(fakeSession) // When configureAndStartSessionUseCase.execute(fakeSession, startSyncing = true) @@ -106,6 +111,7 @@ class ConfigureAndStartSessionUseCaseTest { fakeWebRtcCallManager.givenCheckForProtocolsSupportIfNeededSucceeds() coJustRun { fakeUpdateMatrixClientInfoUseCase.execute(any()) } fakeVectorPreferences.givenIsClientInfoRecordingEnabled(isEnabled = true) + fakeEnableNotificationsSettingUpdater.givenOnSessionsStarted(fakeSession) // When configureAndStartSessionUseCase.execute(fakeSession, startSyncing = false) diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest.kt index 0303444605..37433364e8 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest.kt @@ -16,7 +16,7 @@ package im.vector.app.features.settings.devices.v2.notification -import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fakes.FakeSession import io.mockk.mockk import org.amshove.kluent.shouldBeEqualTo import org.junit.Test @@ -26,18 +26,15 @@ private const val A_DEVICE_ID = "device-id" class CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest { - private val fakeActiveSessionHolder = FakeActiveSessionHolder() + private val fakeSession = FakeSession() private val checkIfCanTogglePushNotificationsViaAccountDataUseCase = - CheckIfCanTogglePushNotificationsViaAccountDataUseCase( - activeSessionHolder = fakeActiveSessionHolder.instance, - ) + CheckIfCanTogglePushNotificationsViaAccountDataUseCase() @Test fun `given current session and an account data for the device id when execute then result is true`() { // Given - fakeActiveSessionHolder - .fakeSession + fakeSession .accountDataService() .givenGetUserAccountDataEventReturns( type = UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + A_DEVICE_ID, @@ -45,7 +42,7 @@ class CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest { ) // When - val result = checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(A_DEVICE_ID) + val result = checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, A_DEVICE_ID) // Then result shouldBeEqualTo true @@ -54,8 +51,7 @@ class CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest { @Test fun `given current session and NO account data for the device id when execute then result is false`() { // Given - fakeActiveSessionHolder - .fakeSession + fakeSession .accountDataService() .givenGetUserAccountDataEventReturns( type = UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + A_DEVICE_ID, @@ -63,7 +59,7 @@ class CheckIfCanTogglePushNotificationsViaAccountDataUseCaseTest { ) // When - val result = checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(A_DEVICE_ID) + val result = checkIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, A_DEVICE_ID) // Then result shouldBeEqualTo false diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCaseTest.kt index 51874be1bc..508a05acd6 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CheckIfCanTogglePushNotificationsViaPusherUseCaseTest.kt @@ -16,7 +16,7 @@ package im.vector.app.features.settings.devices.v2.notification -import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fakes.FakeSession import im.vector.app.test.fixtures.aHomeServerCapabilities import org.amshove.kluent.shouldBeEqualTo import org.junit.Test @@ -25,37 +25,22 @@ private val A_HOMESERVER_CAPABILITIES = aHomeServerCapabilities(canRemotelyToggl class CheckIfCanTogglePushNotificationsViaPusherUseCaseTest { - private val fakeActiveSessionHolder = FakeActiveSessionHolder() + private val fakeSession = FakeSession() private val checkIfCanTogglePushNotificationsViaPusherUseCase = - CheckIfCanTogglePushNotificationsViaPusherUseCase( - activeSessionHolder = fakeActiveSessionHolder.instance, - ) + CheckIfCanTogglePushNotificationsViaPusherUseCase() @Test fun `given current session when execute then toggle capability is returned`() { // Given - fakeActiveSessionHolder - .fakeSession + fakeSession .fakeHomeServerCapabilitiesService .givenCapabilities(A_HOMESERVER_CAPABILITIES) // When - val result = checkIfCanTogglePushNotificationsViaPusherUseCase.execute() + val result = checkIfCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) // Then result shouldBeEqualTo A_HOMESERVER_CAPABILITIES.canRemotelyTogglePushNotificationsOfDevices } - - @Test - fun `given no current session when execute then false is returned`() { - // Given - fakeActiveSessionHolder.givenGetSafeActiveSessionReturns(null) - - // When - val result = checkIfCanTogglePushNotificationsViaPusherUseCase.execute() - - // Then - result shouldBeEqualTo false - } } diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCaseTest.kt index b13018a20d..b38367b098 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/GetNotificationsStatusUseCaseTest.kt @@ -17,7 +17,7 @@ package im.vector.app.features.settings.devices.v2.notification import androidx.arch.core.executor.testing.InstantTaskExecutorRule -import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fakes.FakeSession import im.vector.app.test.fixtures.PusherFixture import im.vector.app.test.testDispatcher import io.mockk.every @@ -25,6 +25,7 @@ import io.mockk.mockk import io.mockk.verifyOrder import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.firstOrNull +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain @@ -44,17 +45,16 @@ class GetNotificationsStatusUseCaseTest { @get:Rule val instantTaskExecutorRule = InstantTaskExecutorRule() - private val fakeActiveSessionHolder = FakeActiveSessionHolder() - private val fakeCheckIfCanTogglePushNotificationsViaPusherUseCase = - mockk<CheckIfCanTogglePushNotificationsViaPusherUseCase>() + private val fakeSession = FakeSession() private val fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase = mockk<CheckIfCanTogglePushNotificationsViaAccountDataUseCase>() + private val fakeCanTogglePushNotificationsViaPusherUseCase = + mockk<CanTogglePushNotificationsViaPusherUseCase>() private val getNotificationsStatusUseCase = GetNotificationsStatusUseCase( - activeSessionHolder = fakeActiveSessionHolder.instance, - checkIfCanTogglePushNotificationsViaPusherUseCase = fakeCheckIfCanTogglePushNotificationsViaPusherUseCase, checkIfCanTogglePushNotificationsViaAccountDataUseCase = fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase, + canTogglePushNotificationsViaPusherUseCase = fakeCanTogglePushNotificationsViaPusherUseCase, ) @Before @@ -67,33 +67,21 @@ class GetNotificationsStatusUseCaseTest { Dispatchers.resetMain() } - @Test - fun `given NO current session when execute then resulting flow contains NOT_SUPPORTED value`() = runTest { - // Given - fakeActiveSessionHolder.givenGetSafeActiveSessionReturns(null) - - // When - val result = getNotificationsStatusUseCase.execute(A_DEVICE_ID) - - // Then - result.firstOrNull() shouldBeEqualTo NotificationsStatus.NOT_SUPPORTED - } - @Test fun `given current session and toggle is not supported when execute then resulting flow contains NOT_SUPPORTED value`() = runTest { // Given - every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute() } returns false - every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(A_DEVICE_ID) } returns false + every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, A_DEVICE_ID) } returns false + every { fakeCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } returns flowOf(false) // When - val result = getNotificationsStatusUseCase.execute(A_DEVICE_ID) + val result = getNotificationsStatusUseCase.execute(fakeSession, A_DEVICE_ID) // Then result.firstOrNull() shouldBeEqualTo NotificationsStatus.NOT_SUPPORTED verifyOrder { // we should first check account data - fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(A_DEVICE_ID) - fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute() + fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, A_DEVICE_ID) + fakeCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } } @@ -106,12 +94,12 @@ class GetNotificationsStatusUseCaseTest { enabled = true, ) ) - fakeActiveSessionHolder.fakeSession.pushersService().givenPushersLive(pushers) - every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute() } returns true - every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(A_DEVICE_ID) } returns false + fakeSession.pushersService().givenPushersLive(pushers) + every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, A_DEVICE_ID) } returns false + every { fakeCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } returns flowOf(true) // When - val result = getNotificationsStatusUseCase.execute(A_DEVICE_ID) + val result = getNotificationsStatusUseCase.execute(fakeSession, A_DEVICE_ID) // Then result.firstOrNull() shouldBeEqualTo NotificationsStatus.ENABLED @@ -120,8 +108,7 @@ class GetNotificationsStatusUseCaseTest { @Test fun `given current session and toggle via account data is supported when execute then resulting flow contains status based on settings value`() = runTest { // Given - fakeActiveSessionHolder - .fakeSession + fakeSession .accountDataService() .givenGetUserAccountDataEventReturns( type = UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + A_DEVICE_ID, @@ -129,11 +116,11 @@ class GetNotificationsStatusUseCaseTest { isSilenced = false ).toContent(), ) - every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute() } returns false - every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(A_DEVICE_ID) } returns true + every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, A_DEVICE_ID) } returns true + every { fakeCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } returns flowOf(false) // When - val result = getNotificationsStatusUseCase.execute(A_DEVICE_ID) + val result = getNotificationsStatusUseCase.execute(fakeSession, A_DEVICE_ID) // Then result.firstOrNull() shouldBeEqualTo NotificationsStatus.ENABLED diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCaseTest.kt index 0a649354f9..35c5979e53 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/TogglePushNotificationUseCaseTest.kt @@ -49,10 +49,11 @@ class TogglePushNotificationUseCaseTest { PusherFixture.aPusher(deviceId = sessionId, enabled = false), PusherFixture.aPusher(deviceId = "another id", enabled = false) ) - activeSessionHolder.fakeSession.pushersService().givenPushersLive(pushers) - activeSessionHolder.fakeSession.pushersService().givenGetPushers(pushers) - every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute() } returns true - every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(sessionId) } returns false + val fakeSession = activeSessionHolder.fakeSession + fakeSession.pushersService().givenPushersLive(pushers) + fakeSession.pushersService().givenGetPushers(pushers) + every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } returns true + every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, sessionId) } returns false // When togglePushNotificationUseCase.execute(sessionId, true) @@ -69,13 +70,14 @@ class TogglePushNotificationUseCaseTest { PusherFixture.aPusher(deviceId = sessionId, enabled = false), PusherFixture.aPusher(deviceId = "another id", enabled = false) ) - activeSessionHolder.fakeSession.pushersService().givenPushersLive(pushers) - activeSessionHolder.fakeSession.accountDataService().givenGetUserAccountDataEventReturns( + val fakeSession = activeSessionHolder.fakeSession + fakeSession.pushersService().givenPushersLive(pushers) + fakeSession.accountDataService().givenGetUserAccountDataEventReturns( UserAccountDataTypes.TYPE_LOCAL_NOTIFICATION_SETTINGS + sessionId, LocalNotificationSettingsContent(isSilenced = true).toContent() ) - every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute() } returns false - every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(sessionId) } returns true + every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } returns false + every { fakeCheckIfCanTogglePushNotificationsViaAccountDataUseCase.execute(fakeSession, sessionId) } returns true // When togglePushNotificationUseCase.execute(sessionId, true) diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt index f26c818e1d..444e14ec0e 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt @@ -37,6 +37,8 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.just +import io.mockk.justRun +import io.mockk.justRun import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.runs @@ -98,7 +100,7 @@ class SessionOverviewViewModelTest { every { SystemClock.elapsedRealtime() } returns 1234 givenVerificationService() - every { fakeGetNotificationsStatusUseCase.execute(A_SESSION_ID_1) } returns flowOf(notificationsStatus) + every { fakeGetNotificationsStatusUseCase.execute(fakeActiveSessionHolder.fakeSession, A_SESSION_ID_1) } returns flowOf(notificationsStatus) } private fun givenVerificationService(): FakeVerificationService { @@ -412,13 +414,10 @@ class SessionOverviewViewModelTest { @Test fun `when viewModel init, then observe pushers and emit to state`() { - val notificationStatus = NotificationsStatus.ENABLED - every { fakeGetNotificationsStatusUseCase.execute(A_SESSION_ID_1) } returns flowOf(notificationStatus) - val viewModel = createViewModel() viewModel.test() - .assertLatestState { state -> state.notificationsStatus == notificationStatus } + .assertLatestState { state -> state.notificationsStatus == notificationsStatus } .finish() } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeEnableNotificationsSettingUpdater.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeEnableNotificationsSettingUpdater.kt new file mode 100644 index 0000000000..a78dd1a34b --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeEnableNotificationsSettingUpdater.kt @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.core.notification.EnableNotificationsSettingUpdater +import io.mockk.justRun +import io.mockk.mockk +import org.matrix.android.sdk.api.session.Session + +class FakeEnableNotificationsSettingUpdater { + + val instance = mockk<EnableNotificationsSettingUpdater>() + + fun givenOnSessionsStarted(session: Session) { + justRun { instance.onSessionsStarted(session) } + } +} From e5e971683b6faa9a85049080f743177b944cfe67 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Fri, 4 Nov 2022 11:47:23 +0100 Subject: [PATCH 40/47] Adding unit tests on CanTogglePushNotificationsViaPusherUseCase --- ...TogglePushNotificationsViaPusherUseCase.kt | 1 - ...lePushNotificationsViaPusherUseCaseTest.kt | 65 +++++++++++++++++++ .../FakeHomeServerCapabilitiesService.kt | 10 +++ 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCaseTest.kt diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt index af15e2f349..963768ca04 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt @@ -25,7 +25,6 @@ import javax.inject.Inject class CanTogglePushNotificationsViaPusherUseCase @Inject constructor() { - // TODO add unit tests fun execute(session: Session): Flow<Boolean> { return session .homeServerCapabilitiesService() diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCaseTest.kt new file mode 100644 index 0000000000..997fa827f5 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCaseTest.kt @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.devices.v2.notification + +import im.vector.app.test.fakes.FakeFlowLiveDataConversions +import im.vector.app.test.fakes.FakeSession +import im.vector.app.test.fakes.givenAsFlow +import im.vector.app.test.fixtures.aHomeServerCapabilities +import io.mockk.unmockkAll +import kotlinx.coroutines.flow.firstOrNull +import kotlinx.coroutines.test.runTest +import org.amshove.kluent.shouldBeEqualTo +import org.junit.After +import org.junit.Before +import org.junit.Test + +private val A_HOMESERVER_CAPABILITIES = aHomeServerCapabilities(canRemotelyTogglePushNotificationsOfDevices = true) + +class CanTogglePushNotificationsViaPusherUseCaseTest { + + private val fakeSession = FakeSession() + private val fakeFlowLiveDataConversions = FakeFlowLiveDataConversions() + + private val canTogglePushNotificationsViaPusherUseCase = + CanTogglePushNotificationsViaPusherUseCase() + + @Before + fun setUp() { + fakeFlowLiveDataConversions.setup() + } + + @After + fun tearDown() { + unmockkAll() + } + + @Test + fun `given current session when execute then flow of the toggle capability is returned`() = runTest { + // Given + fakeSession + .fakeHomeServerCapabilitiesService + .givenCapabilitiesLiveReturns(A_HOMESERVER_CAPABILITIES) + .givenAsFlow() + + // When + val result = canTogglePushNotificationsViaPusherUseCase.execute(fakeSession).firstOrNull() + + // Then + result shouldBeEqualTo A_HOMESERVER_CAPABILITIES.canRemotelyTogglePushNotificationsOfDevices + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerCapabilitiesService.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerCapabilitiesService.kt index 006789f62b..c816c51c0f 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerCapabilitiesService.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerCapabilitiesService.kt @@ -16,14 +16,24 @@ package im.vector.app.test.fakes +import androidx.lifecycle.LiveData +import androidx.lifecycle.MutableLiveData import io.mockk.every import io.mockk.mockk import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilities import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilitiesService +import org.matrix.android.sdk.api.util.Optional +import org.matrix.android.sdk.api.util.toOptional class FakeHomeServerCapabilitiesService : HomeServerCapabilitiesService by mockk() { fun givenCapabilities(homeServerCapabilities: HomeServerCapabilities) { every { getHomeServerCapabilities() } returns homeServerCapabilities } + + fun givenCapabilitiesLiveReturns(homeServerCapabilities: HomeServerCapabilities): LiveData<Optional<HomeServerCapabilities>> { + return MutableLiveData(homeServerCapabilities.toOptional()).also { + every { getHomeServerCapabilitiesLive() } returns it + } + } } From 2eeb04426b49ab5496e5e5fce8a493795fd8505c Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Fri, 4 Nov 2022 14:22:50 +0100 Subject: [PATCH 41/47] Adding unit tests on DisableNotificationsForCurrentSessionUseCase --- ...leNotificationsForCurrentSessionUseCase.kt | 1 - ...tificationsForCurrentSessionUseCaseTest.kt | 78 +++++++++++++++++++ .../app/test/fakes/FakePushersManager.kt | 25 ++++++ .../app/test/fakes/FakeUnifiedPushHelper.kt | 36 +++++++++ 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt index d66bf8b789..61c884f0bc 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt @@ -31,7 +31,6 @@ class DisableNotificationsForCurrentSessionUseCase @Inject constructor( private val togglePushNotificationUseCase: TogglePushNotificationUseCase, ) { - // TODO add unit tests suspend fun execute() { val session = activeSessionHolder.getSafeActiveSession() ?: return val deviceId = session.sessionParams.deviceId ?: return diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt new file mode 100644 index 0000000000..e460413a39 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications + +import im.vector.app.features.settings.devices.v2.notification.CheckIfCanTogglePushNotificationsViaPusherUseCase +import im.vector.app.features.settings.devices.v2.notification.TogglePushNotificationUseCase +import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fakes.FakePushersManager +import im.vector.app.test.fakes.FakeUnifiedPushHelper +import io.mockk.coJustRun +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.Test + +private const val A_SESSION_ID = "session-id" + +class DisableNotificationsForCurrentSessionUseCaseTest { + + private val fakeActiveSessionHolder = FakeActiveSessionHolder() + private val fakeUnifiedPushHelper = FakeUnifiedPushHelper() + private val fakePushersManager = FakePushersManager() + private val fakeCheckIfCanTogglePushNotificationsViaPusherUseCase = mockk<CheckIfCanTogglePushNotificationsViaPusherUseCase>() + private val fakeTogglePushNotificationUseCase = mockk<TogglePushNotificationUseCase>() + + private val disableNotificationsForCurrentSessionUseCase = DisableNotificationsForCurrentSessionUseCase( + activeSessionHolder = fakeActiveSessionHolder.instance, + unifiedPushHelper = fakeUnifiedPushHelper.instance, + pushersManager = fakePushersManager.instance, + checkIfCanTogglePushNotificationsViaPusherUseCase = fakeCheckIfCanTogglePushNotificationsViaPusherUseCase, + togglePushNotificationUseCase = fakeTogglePushNotificationUseCase, + ) + + @Test + fun `given toggle via pusher is possible when execute then disable notification via toggle of existing pusher`() = runTest { + // Given + val fakeSession = fakeActiveSessionHolder.fakeSession + fakeSession.givenSessionId(A_SESSION_ID) + every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } returns true + coJustRun { fakeTogglePushNotificationUseCase.execute(A_SESSION_ID, any()) } + + // When + disableNotificationsForCurrentSessionUseCase.execute() + + // Then + coVerify { fakeTogglePushNotificationUseCase.execute(A_SESSION_ID, false) } + } + + @Test + fun `given toggle via pusher is NOT possible when execute then disable notification by unregistering the pusher`() = runTest { + // Given + val fakeSession = fakeActiveSessionHolder.fakeSession + fakeSession.givenSessionId(A_SESSION_ID) + every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute(fakeSession) } returns false + fakeUnifiedPushHelper.givenUnregister(fakePushersManager.instance) + + // When + disableNotificationsForCurrentSessionUseCase.execute() + + // Then + fakeUnifiedPushHelper.verifyUnregister(fakePushersManager.instance) + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt b/vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt new file mode 100644 index 0000000000..245662b0c6 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.core.pushers.PushersManager +import io.mockk.mockk + +class FakePushersManager { + + val instance = mockk<PushersManager>() +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt new file mode 100644 index 0000000000..d7985d9757 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.core.pushers.PushersManager +import im.vector.app.core.pushers.UnifiedPushHelper +import io.mockk.coJustRun +import io.mockk.coVerify +import io.mockk.mockk + +class FakeUnifiedPushHelper { + + val instance = mockk<UnifiedPushHelper>() + + fun givenUnregister(pushersManager: PushersManager) { + coJustRun { instance.unregister(pushersManager) } + } + + fun verifyUnregister(pushersManager: PushersManager) { + coVerify { instance.unregister(pushersManager) } + } +} From b43c3a8502e6a18df0f2198cbb9ebacb3f487ac6 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Fri, 4 Nov 2022 14:42:52 +0100 Subject: [PATCH 42/47] Adding unit tests on UpdateEnableNotificationsSettingOnChangeUseCase --- .../EnableNotificationsSettingUpdater.kt | 2 - ...ableNotificationsSettingOnChangeUseCase.kt | 11 ++- ...NotificationsSettingOnChangeUseCaseTest.kt | 90 +++++++++++++++++++ .../overview/SessionOverviewViewModelTest.kt | 12 ++- .../FakeGetNotificationsStatusUseCase.kt | 37 ++++++++ .../app/test/fakes/FakeVectorPreferences.kt | 11 ++- 6 files changed, 150 insertions(+), 13 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCaseTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeGetNotificationsStatusUseCase.kt diff --git a/vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt b/vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt index 21febaee9d..81b524cde9 100644 --- a/vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt +++ b/vector/src/main/java/im/vector/app/core/notification/EnableNotificationsSettingUpdater.kt @@ -18,7 +18,6 @@ package im.vector.app.core.notification import im.vector.app.features.session.coroutineScope import kotlinx.coroutines.Job -import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.launch import org.matrix.android.sdk.api.session.Session import javax.inject.Inject @@ -35,7 +34,6 @@ class EnableNotificationsSettingUpdater @Inject constructor( job?.cancel() job = session.coroutineScope.launch { updateEnableNotificationsSettingOnChangeUseCase.execute(session) - .launchIn(this) } } } diff --git a/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt b/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt index a6bcf17b5c..55a2cfdc64 100644 --- a/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt @@ -19,8 +19,7 @@ package im.vector.app.core.notification import im.vector.app.features.settings.VectorPreferences import im.vector.app.features.settings.devices.v2.notification.GetNotificationsStatusUseCase import im.vector.app.features.settings.devices.v2.notification.NotificationsStatus -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.onEach import org.matrix.android.sdk.api.session.Session import timber.log.Timber @@ -35,11 +34,11 @@ class UpdateEnableNotificationsSettingOnChangeUseCase @Inject constructor( private val getNotificationsStatusUseCase: GetNotificationsStatusUseCase, ) { - // TODO add unit tests - fun execute(session: Session): Flow<NotificationsStatus> { - val deviceId = session.sessionParams.deviceId ?: return emptyFlow() - return getNotificationsStatusUseCase.execute(session, deviceId) + suspend fun execute(session: Session) { + val deviceId = session.sessionParams.deviceId ?: return + getNotificationsStatusUseCase.execute(session, deviceId) .onEach(::updatePreference) + .collect() } private fun updatePreference(notificationStatus: NotificationsStatus) { diff --git a/vector/src/test/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCaseTest.kt b/vector/src/test/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCaseTest.kt new file mode 100644 index 0000000000..5cced75735 --- /dev/null +++ b/vector/src/test/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCaseTest.kt @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.core.notification + +import im.vector.app.features.settings.devices.v2.notification.NotificationsStatus +import im.vector.app.test.fakes.FakeGetNotificationsStatusUseCase +import im.vector.app.test.fakes.FakeSession +import im.vector.app.test.fakes.FakeVectorPreferences +import kotlinx.coroutines.test.runTest +import org.junit.Test + +private const val A_SESSION_ID = "session-id" + +class UpdateEnableNotificationsSettingOnChangeUseCaseTest { + + private val fakeSession = FakeSession().also { it.givenSessionId(A_SESSION_ID) } + private val fakeVectorPreferences = FakeVectorPreferences() + private val fakeGetNotificationsStatusUseCase = FakeGetNotificationsStatusUseCase() + + private val updateEnableNotificationsSettingOnChangeUseCase = UpdateEnableNotificationsSettingOnChangeUseCase( + vectorPreferences = fakeVectorPreferences.instance, + getNotificationsStatusUseCase = fakeGetNotificationsStatusUseCase.instance, + ) + + @Test + fun `given notifications are enabled when execute then setting is updated to true`() = runTest { + // Given + fakeGetNotificationsStatusUseCase.givenExecuteReturns( + fakeSession, + A_SESSION_ID, + NotificationsStatus.ENABLED, + ) + fakeVectorPreferences.givenSetNotificationEnabledForDevice() + + // When + updateEnableNotificationsSettingOnChangeUseCase.execute(fakeSession) + + // Then + fakeVectorPreferences.verifySetNotificationEnabledForDevice(true) + } + + @Test + fun `given notifications are disabled when execute then setting is updated to false`() = runTest { + // Given + fakeGetNotificationsStatusUseCase.givenExecuteReturns( + fakeSession, + A_SESSION_ID, + NotificationsStatus.DISABLED, + ) + fakeVectorPreferences.givenSetNotificationEnabledForDevice() + + // When + updateEnableNotificationsSettingOnChangeUseCase.execute(fakeSession) + + // Then + fakeVectorPreferences.verifySetNotificationEnabledForDevice(false) + } + + @Test + fun `given notifications toggle is not supported when execute then nothing is done`() = runTest { + // Given + fakeGetNotificationsStatusUseCase.givenExecuteReturns( + fakeSession, + A_SESSION_ID, + NotificationsStatus.NOT_SUPPORTED, + ) + fakeVectorPreferences.givenSetNotificationEnabledForDevice() + + // When + updateEnableNotificationsSettingOnChangeUseCase.execute(fakeSession) + + // Then + fakeVectorPreferences.verifySetNotificationEnabledForDevice(true, inverse = true) + fakeVectorPreferences.verifySetNotificationEnabledForDevice(false, inverse = true) + } +} diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt index 444e14ec0e..86a9969a6a 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt @@ -22,11 +22,11 @@ import com.airbnb.mvrx.Success import com.airbnb.mvrx.test.MavericksTestRule import im.vector.app.features.settings.devices.v2.DeviceFullInfo import im.vector.app.features.settings.devices.v2.RefreshDevicesUseCase -import im.vector.app.features.settings.devices.v2.notification.GetNotificationsStatusUseCase import im.vector.app.features.settings.devices.v2.notification.NotificationsStatus import im.vector.app.features.settings.devices.v2.signout.InterceptSignoutFlowResponseUseCase import im.vector.app.features.settings.devices.v2.verification.CheckIfCurrentSessionCanBeVerifiedUseCase import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fakes.FakeGetNotificationsStatusUseCase import im.vector.app.test.fakes.FakePendingAuthHandler import im.vector.app.test.fakes.FakeSignoutSessionsUseCase import im.vector.app.test.fakes.FakeTogglePushNotificationUseCase @@ -77,7 +77,7 @@ class SessionOverviewViewModelTest { private val fakePendingAuthHandler = FakePendingAuthHandler() private val refreshDevicesUseCase = mockk<RefreshDevicesUseCase>(relaxed = true) private val togglePushNotificationUseCase = FakeTogglePushNotificationUseCase() - private val fakeGetNotificationsStatusUseCase = mockk<GetNotificationsStatusUseCase>() + private val fakeGetNotificationsStatusUseCase = FakeGetNotificationsStatusUseCase() private val notificationsStatus = NotificationsStatus.ENABLED private fun createViewModel() = SessionOverviewViewModel( @@ -90,7 +90,7 @@ class SessionOverviewViewModelTest { activeSessionHolder = fakeActiveSessionHolder.instance, refreshDevicesUseCase = refreshDevicesUseCase, togglePushNotificationUseCase = togglePushNotificationUseCase.instance, - getNotificationsStatusUseCase = fakeGetNotificationsStatusUseCase, + getNotificationsStatusUseCase = fakeGetNotificationsStatusUseCase.instance, ) @Before @@ -100,7 +100,11 @@ class SessionOverviewViewModelTest { every { SystemClock.elapsedRealtime() } returns 1234 givenVerificationService() - every { fakeGetNotificationsStatusUseCase.execute(fakeActiveSessionHolder.fakeSession, A_SESSION_ID_1) } returns flowOf(notificationsStatus) + fakeGetNotificationsStatusUseCase.givenExecuteReturns( + fakeActiveSessionHolder.fakeSession, + A_SESSION_ID_1, + notificationsStatus + ) } private fun givenVerificationService(): FakeVerificationService { diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeGetNotificationsStatusUseCase.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeGetNotificationsStatusUseCase.kt new file mode 100644 index 0000000000..a9c1b37d69 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeGetNotificationsStatusUseCase.kt @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.features.settings.devices.v2.notification.GetNotificationsStatusUseCase +import im.vector.app.features.settings.devices.v2.notification.NotificationsStatus +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.flow.flowOf +import org.matrix.android.sdk.api.session.Session + +class FakeGetNotificationsStatusUseCase { + + val instance = mockk<GetNotificationsStatusUseCase>() + + fun givenExecuteReturns( + session: Session, + sessionId: String, + notificationsStatus: NotificationsStatus + ) { + every { instance.execute(session, sessionId) } returns flowOf(notificationsStatus) + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt index cd4f70bf63..4baa7e2b90 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt @@ -18,6 +18,7 @@ package im.vector.app.test.fakes import im.vector.app.features.settings.VectorPreferences import io.mockk.every +import io.mockk.justRun import io.mockk.mockk import io.mockk.verify @@ -42,5 +43,13 @@ class FakeVectorPreferences { } fun givenTextFormatting(isEnabled: Boolean) = - every { instance.isTextFormattingEnabled() } returns isEnabled + every { instance.isTextFormattingEnabled() } returns isEnabled + + fun givenSetNotificationEnabledForDevice() { + justRun { instance.setNotificationEnabledForDevice(any()) } + } + + fun verifySetNotificationEnabledForDevice(enabled: Boolean, inverse: Boolean = false) { + verify(inverse = inverse) { instance.setNotificationEnabledForDevice(enabled) } + } } From ced4bf3573a51b13ac5211980415978c686ed770 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Fri, 4 Nov 2022 15:10:19 +0100 Subject: [PATCH 43/47] Adding unit tests on EnableNotificationsForCurrentSessionUseCase --- ...leNotificationsForCurrentSessionUseCase.kt | 1 - ...tificationsForCurrentSessionUseCaseTest.kt | 87 +++++++++++++++++++ .../im/vector/app/test/fakes/FakeFcmHelper.kt | 44 ++++++++++ .../app/test/fakes/FakePushersManager.kt | 6 ++ .../app/test/fakes/FakeUnifiedPushHelper.kt | 17 ++++ 5 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 vector/src/test/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCaseTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeFcmHelper.kt diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt index cee653380a..180627a15f 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCase.kt @@ -37,7 +37,6 @@ class EnableNotificationsForCurrentSessionUseCase @Inject constructor( private val togglePushNotificationUseCase: TogglePushNotificationUseCase, ) { - // TODO add unit tests suspend fun execute(fragmentActivity: FragmentActivity) { val pusherForCurrentSession = pushersManager.getPusherForCurrentSession() if (pusherForCurrentSession == null) { diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCaseTest.kt new file mode 100644 index 0000000000..eb6629cb13 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/EnableNotificationsForCurrentSessionUseCaseTest.kt @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications + +import androidx.fragment.app.FragmentActivity +import im.vector.app.features.settings.devices.v2.notification.CheckIfCanTogglePushNotificationsViaPusherUseCase +import im.vector.app.features.settings.devices.v2.notification.TogglePushNotificationUseCase +import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fakes.FakeFcmHelper +import im.vector.app.test.fakes.FakePushersManager +import im.vector.app.test.fakes.FakeUnifiedPushHelper +import io.mockk.coJustRun +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.Test + +private const val A_SESSION_ID = "session-id" + +class EnableNotificationsForCurrentSessionUseCaseTest { + + private val fakeActiveSessionHolder = FakeActiveSessionHolder() + private val fakeUnifiedPushHelper = FakeUnifiedPushHelper() + private val fakePushersManager = FakePushersManager() + private val fakeFcmHelper = FakeFcmHelper() + private val fakeCheckIfCanTogglePushNotificationsViaPusherUseCase = mockk<CheckIfCanTogglePushNotificationsViaPusherUseCase>() + private val fakeTogglePushNotificationUseCase = mockk<TogglePushNotificationUseCase>() + + private val enableNotificationsForCurrentSessionUseCase = EnableNotificationsForCurrentSessionUseCase( + activeSessionHolder = fakeActiveSessionHolder.instance, + unifiedPushHelper = fakeUnifiedPushHelper.instance, + pushersManager = fakePushersManager.instance, + fcmHelper = fakeFcmHelper.instance, + checkIfCanTogglePushNotificationsViaPusherUseCase = fakeCheckIfCanTogglePushNotificationsViaPusherUseCase, + togglePushNotificationUseCase = fakeTogglePushNotificationUseCase, + ) + + @Test + fun `given no existing pusher for current session when execute then a new pusher is registered`() = runTest { + // Given + val fragmentActivity = mockk<FragmentActivity>() + fakePushersManager.givenGetPusherForCurrentSessionReturns(null) + fakeUnifiedPushHelper.givenRegister(fragmentActivity) + fakeUnifiedPushHelper.givenIsEmbeddedDistributorReturns(true) + fakeFcmHelper.givenEnsureFcmTokenIsRetrieved(fragmentActivity, fakePushersManager.instance) + every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute(fakeActiveSessionHolder.fakeSession) } returns false + + // When + enableNotificationsForCurrentSessionUseCase.execute(fragmentActivity) + + // Then + fakeUnifiedPushHelper.verifyRegister(fragmentActivity) + fakeFcmHelper.verifyEnsureFcmTokenIsRetrieved(fragmentActivity, fakePushersManager.instance, registerPusher = true) + } + + @Test + fun `given toggle via Pusher is possible when execute then current pusher is toggled to true`() = runTest { + // Given + val fragmentActivity = mockk<FragmentActivity>() + fakePushersManager.givenGetPusherForCurrentSessionReturns(mockk()) + val fakeSession = fakeActiveSessionHolder.fakeSession + fakeSession.givenSessionId(A_SESSION_ID) + every { fakeCheckIfCanTogglePushNotificationsViaPusherUseCase.execute(fakeActiveSessionHolder.fakeSession) } returns true + coJustRun { fakeTogglePushNotificationUseCase.execute(A_SESSION_ID, any()) } + + // When + enableNotificationsForCurrentSessionUseCase.execute(fragmentActivity) + + // Then + coVerify { fakeTogglePushNotificationUseCase.execute(A_SESSION_ID, true) } + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeFcmHelper.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeFcmHelper.kt new file mode 100644 index 0000000000..11abf18794 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeFcmHelper.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import androidx.fragment.app.FragmentActivity +import im.vector.app.core.pushers.FcmHelper +import im.vector.app.core.pushers.PushersManager +import io.mockk.justRun +import io.mockk.mockk +import io.mockk.verify + +class FakeFcmHelper { + + val instance = mockk<FcmHelper>() + + fun givenEnsureFcmTokenIsRetrieved( + fragmentActivity: FragmentActivity, + pushersManager: PushersManager, + ) { + justRun { instance.ensureFcmTokenIsRetrieved(fragmentActivity, pushersManager, any()) } + } + + fun verifyEnsureFcmTokenIsRetrieved( + fragmentActivity: FragmentActivity, + pushersManager: PushersManager, + registerPusher: Boolean, + ) { + verify { instance.ensureFcmTokenIsRetrieved(fragmentActivity, pushersManager, registerPusher) } + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt b/vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt index 245662b0c6..46d852f4f8 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakePushersManager.kt @@ -17,9 +17,15 @@ package im.vector.app.test.fakes import im.vector.app.core.pushers.PushersManager +import io.mockk.every import io.mockk.mockk +import org.matrix.android.sdk.api.session.pushers.Pusher class FakePushersManager { val instance = mockk<PushersManager>() + + fun givenGetPusherForCurrentSessionReturns(pusher: Pusher?) { + every { instance.getPusherForCurrentSession() } returns pusher + } } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt index d7985d9757..1f2cc8a1ce 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeUnifiedPushHelper.kt @@ -16,16 +16,29 @@ package im.vector.app.test.fakes +import androidx.fragment.app.FragmentActivity import im.vector.app.core.pushers.PushersManager import im.vector.app.core.pushers.UnifiedPushHelper import io.mockk.coJustRun import io.mockk.coVerify +import io.mockk.every import io.mockk.mockk +import io.mockk.verify class FakeUnifiedPushHelper { val instance = mockk<UnifiedPushHelper>() + fun givenRegister(fragmentActivity: FragmentActivity) { + every { instance.register(fragmentActivity, any()) } answers { + secondArg<Runnable>().run() + } + } + + fun verifyRegister(fragmentActivity: FragmentActivity) { + verify { instance.register(fragmentActivity, any()) } + } + fun givenUnregister(pushersManager: PushersManager) { coJustRun { instance.unregister(pushersManager) } } @@ -33,4 +46,8 @@ class FakeUnifiedPushHelper { fun verifyUnregister(pushersManager: PushersManager) { coVerify { instance.unregister(pushersManager) } } + + fun givenIsEmbeddedDistributorReturns(isEmbedded: Boolean) { + every { instance.isEmbeddedDistributor() } returns isEmbedded + } } From 163bf57fdaf001bd7f8366b850acad8e43fb0f4b Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Tue, 8 Nov 2022 17:14:57 +0100 Subject: [PATCH 44/47] Removing non necessary debug log --- .../UpdateEnableNotificationsSettingOnChangeUseCase.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt b/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt index 55a2cfdc64..36df939bad 100644 --- a/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/notification/UpdateEnableNotificationsSettingOnChangeUseCase.kt @@ -22,7 +22,6 @@ import im.vector.app.features.settings.devices.v2.notification.NotificationsStat import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.onEach import org.matrix.android.sdk.api.session.Session -import timber.log.Timber import javax.inject.Inject /** @@ -42,7 +41,6 @@ class UpdateEnableNotificationsSettingOnChangeUseCase @Inject constructor( } private fun updatePreference(notificationStatus: NotificationsStatus) { - Timber.d("updatePreference with status=$notificationStatus") when (notificationStatus) { NotificationsStatus.ENABLED -> vectorPreferences.setNotificationEnabledForDevice(true) NotificationsStatus.DISABLED -> vectorPreferences.setNotificationEnabledForDevice(false) From ba5a433cafc8b6ed6cdace2a526bead13c08bf34 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Tue, 8 Nov 2022 17:19:06 +0100 Subject: [PATCH 45/47] Adding distinctUntilChanged for flow of remote toggle via Pusher capability --- .../notification/CanTogglePushNotificationsViaPusherUseCase.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt index 963768ca04..0125d92ba6 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/notification/CanTogglePushNotificationsViaPusherUseCase.kt @@ -18,6 +18,7 @@ package im.vector.app.features.settings.devices.v2.notification import androidx.lifecycle.asFlow import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.flow.unwrap @@ -32,5 +33,6 @@ class CanTogglePushNotificationsViaPusherUseCase @Inject constructor() { .asFlow() .unwrap() .map { it.canRemotelyTogglePushNotificationsOfDevices } + .distinctUntilChanged() } } From 6ec33f1264ecc59b26a8db8844dafacaa9f83a18 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL <maxime.naturel@niji.fr> Date: Wed, 9 Nov 2022 09:33:39 +0100 Subject: [PATCH 46/47] Removing unused imports --- .../devices/v2/overview/SessionOverviewViewModelTest.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt index 86a9969a6a..1a57b76020 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt @@ -37,8 +37,6 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.just -import io.mockk.justRun -import io.mockk.justRun import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.runs From c07b110b99c93ab59b8944b688405ee79dc9a95c Mon Sep 17 00:00:00 2001 From: Amit Kumar <mr.doc10jl96@gmail.com> Date: Thu, 10 Nov 2022 16:13:09 +0530 Subject: [PATCH 47/47] Add spannable tracking around SyncResponseHandler (#7514) * Add spannable tracking around SyncResponseHandler * Update LICENSE header * Refactor handleResponse and MetricsExtensions * Update changelog.d * Improve code docs and comments * Check if Sentry is enabled before tracking --- changelog.d/7514.sdk | 1 + .../sdk/api/extensions/MetricsExtensions.kt | 34 +++- .../sdk/api/metrics/SpannableMetricPlugin.kt | 36 ++++ .../api/metrics/SyncDurationMetricPlugin.kt | 32 ++++ .../sdk/internal/crypto/DeviceListManager.kt | 2 +- .../session/sync/SyncResponseHandler.kt | 175 +++++++++++++----- .../analytics/metrics/VectorPlugins.kt | 4 +- .../sentry/SentryDownloadDeviceKeysMetrics.kt | 6 +- .../sentry/SentrySyncDurationMetrics.kt | 89 +++++++++ 9 files changed, 323 insertions(+), 56 deletions(-) create mode 100644 changelog.d/7514.sdk create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SpannableMetricPlugin.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SyncDurationMetricPlugin.kt create mode 100644 vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentrySyncDurationMetrics.kt diff --git a/changelog.d/7514.sdk b/changelog.d/7514.sdk new file mode 100644 index 0000000000..f335156a49 --- /dev/null +++ b/changelog.d/7514.sdk @@ -0,0 +1 @@ +[Metrics] Add `SpannableMetricPlugin` to support spans within transactions. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/extensions/MetricsExtensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/extensions/MetricsExtensions.kt index 9487a27086..7f0e828f62 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/extensions/MetricsExtensions.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/extensions/MetricsExtensions.kt @@ -17,25 +17,51 @@ package org.matrix.android.sdk.api.extensions import org.matrix.android.sdk.api.metrics.MetricPlugin +import org.matrix.android.sdk.api.metrics.SpannableMetricPlugin import kotlin.contracts.ExperimentalContracts import kotlin.contracts.InvocationKind import kotlin.contracts.contract /** * Executes the given [block] while measuring the transaction. + * + * @param block Action/Task to be executed within this span. */ @OptIn(ExperimentalContracts::class) -inline fun measureMetric(metricMeasurementPlugins: List<MetricPlugin>, block: () -> Unit) { +inline fun List<MetricPlugin>.measureMetric(block: () -> Unit) { contract { callsInPlace(block, InvocationKind.EXACTLY_ONCE) } try { - metricMeasurementPlugins.forEach { plugin -> plugin.startTransaction() } // Start the transaction. + this.forEach { plugin -> plugin.startTransaction() } // Start the transaction. block() } catch (throwable: Throwable) { - metricMeasurementPlugins.forEach { plugin -> plugin.onError(throwable) } // Capture if there is any exception thrown. + this.forEach { plugin -> plugin.onError(throwable) } // Capture if there is any exception thrown. throw throwable } finally { - metricMeasurementPlugins.forEach { plugin -> plugin.finishTransaction() } // Finally, finish this transaction. + this.forEach { plugin -> plugin.finishTransaction() } // Finally, finish this transaction. + } +} + +/** + * Executes the given [block] while measuring a span. + * + * @param operation Name of the new span. + * @param description Description of the new span. + * @param block Action/Task to be executed within this span. + */ +@OptIn(ExperimentalContracts::class) +inline fun List<SpannableMetricPlugin>.measureSpan(operation: String, description: String, block: () -> Unit) { + contract { + callsInPlace(block, InvocationKind.EXACTLY_ONCE) + } + try { + this.forEach { plugin -> plugin.startSpan(operation, description) } // Start the transaction. + block() + } catch (throwable: Throwable) { + this.forEach { plugin -> plugin.onError(throwable) } // Capture if there is any exception thrown. + throw throwable + } finally { + this.forEach { plugin -> plugin.finishSpan() } // Finally, finish this transaction. } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SpannableMetricPlugin.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SpannableMetricPlugin.kt new file mode 100644 index 0000000000..54aa21877e --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SpannableMetricPlugin.kt @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2022 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.api.metrics + +/** + * A plugin that tracks span along with transactions. + */ +interface SpannableMetricPlugin : MetricPlugin { + + /** + * Starts the span for a sub-task. + * + * @param operation Name of the new span. + * @param description Description of the new span. + */ + fun startSpan(operation: String, description: String) + + /** + * Finish the span when sub-task is completed. + */ + fun finishSpan() +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SyncDurationMetricPlugin.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SyncDurationMetricPlugin.kt new file mode 100644 index 0000000000..79ece002e9 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/metrics/SyncDurationMetricPlugin.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2022 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.api.metrics + +import org.matrix.android.sdk.api.logger.LoggerTag +import timber.log.Timber + +private val loggerTag = LoggerTag("SyncDurationMetricPlugin", LoggerTag.CRYPTO) + +/** + * An spannable metric plugin for sync response handling task. + */ +interface SyncDurationMetricPlugin : SpannableMetricPlugin { + + override fun logTransaction(message: String?) { + Timber.tag(loggerTag.value).v("## syncResponseHandler() : $message") + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt index 2ac6b8c854..7e9e156003 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt @@ -355,7 +355,7 @@ internal class DeviceListManager @Inject constructor( val relevantPlugins = metricPlugins.filterIsInstance<DownloadDeviceKeysMetricsPlugin>() val response: KeysQueryResponse - measureMetric(relevantPlugins) { + relevantPlugins.measureMetric { response = try { downloadKeysForUsersTask.execute(params) } catch (throwable: Throwable) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt index 05216d1de1..05d50d9595 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt @@ -17,6 +17,11 @@ package org.matrix.android.sdk.internal.session.sync import com.zhuinden.monarchy.Monarchy +import io.realm.Realm +import org.matrix.android.sdk.api.MatrixConfiguration +import org.matrix.android.sdk.api.extensions.measureMetric +import org.matrix.android.sdk.api.extensions.measureSpan +import org.matrix.android.sdk.api.metrics.SyncDurationMetricPlugin import org.matrix.android.sdk.api.session.pushrules.PushRuleService import org.matrix.android.sdk.api.session.pushrules.RuleScope import org.matrix.android.sdk.api.session.sync.InitialSyncStep @@ -52,9 +57,12 @@ internal class SyncResponseHandler @Inject constructor( private val tokenStore: SyncTokenStore, private val processEventForPushTask: ProcessEventForPushTask, private val pushRuleService: PushRuleService, - private val presenceSyncHandler: PresenceSyncHandler + private val presenceSyncHandler: PresenceSyncHandler, + matrixConfiguration: MatrixConfiguration, ) { + private val relevantPlugins = matrixConfiguration.metricPlugins.filterIsInstance<SyncDurationMetricPlugin>() + suspend fun handleResponse( syncResponse: SyncResponse, fromToken: String?, @@ -63,39 +71,91 @@ internal class SyncResponseHandler @Inject constructor( val isInitialSync = fromToken == null Timber.v("Start handling sync, is InitialSync: $isInitialSync") - measureTimeMillis { - if (!cryptoService.isStarted()) { - Timber.v("Should start cryptoService") - cryptoService.start() - } - cryptoService.onSyncWillProcess(isInitialSync) - }.also { - Timber.v("Finish handling start cryptoService in $it ms") - } + relevantPlugins.measureMetric { + startCryptoService(isInitialSync) - // Handle the to device events before the room ones - // to ensure to decrypt them properly - measureTimeMillis { - Timber.v("Handle toDevice") - reportSubtask(reporter, InitialSyncStep.ImportingAccountCrypto, 100, 0.1f) { - if (syncResponse.toDevice != null) { - cryptoSyncHandler.handleToDevice(syncResponse.toDevice, reporter) + // Handle the to device events before the room ones + // to ensure to decrypt them properly + handleToDevice(syncResponse, reporter) + + val aggregator = SyncResponsePostTreatmentAggregator() + + // Prerequisite for thread events handling in RoomSyncHandler + // Disabled due to the new fallback + // if (!lightweightSettingsStorage.areThreadMessagesEnabled()) { + // threadsAwarenessHandler.fetchRootThreadEventsIfNeeded(syncResponse) + // } + + startMonarchyTransaction(syncResponse, isInitialSync, reporter, aggregator) + + aggregateSyncResponse(aggregator) + + postTreatmentSyncResponse(syncResponse, isInitialSync) + + markCryptoSyncCompleted(syncResponse) + + handlePostSync() + + Timber.v("On sync completed") + } + } + + private fun startCryptoService(isInitialSync: Boolean) { + relevantPlugins.measureSpan("task", "start_crypto_service") { + measureTimeMillis { + if (!cryptoService.isStarted()) { + Timber.v("Should start cryptoService") + cryptoService.start() } + cryptoService.onSyncWillProcess(isInitialSync) + }.also { + Timber.v("Finish handling start cryptoService in $it ms") } - }.also { - Timber.v("Finish handling toDevice in $it ms") } - val aggregator = SyncResponsePostTreatmentAggregator() + } - // Prerequisite for thread events handling in RoomSyncHandler -// Disabled due to the new fallback -// if (!lightweightSettingsStorage.areThreadMessagesEnabled()) { -// threadsAwarenessHandler.fetchRootThreadEventsIfNeeded(syncResponse) -// } + private suspend fun handleToDevice(syncResponse: SyncResponse, reporter: ProgressReporter?) { + relevantPlugins.measureSpan("task", "handle_to_device") { + measureTimeMillis { + Timber.v("Handle toDevice") + reportSubtask(reporter, InitialSyncStep.ImportingAccountCrypto, 100, 0.1f) { + if (syncResponse.toDevice != null) { + cryptoSyncHandler.handleToDevice(syncResponse.toDevice, reporter) + } + } + }.also { + Timber.v("Finish handling toDevice in $it ms") + } + } + } + private suspend fun startMonarchyTransaction( + syncResponse: SyncResponse, + isInitialSync: Boolean, + reporter: ProgressReporter?, + aggregator: SyncResponsePostTreatmentAggregator + ) { // Start one big transaction - monarchy.awaitTransaction { realm -> - // IMPORTANT nothing should be suspend here as we are accessing the realm instance (thread local) + relevantPlugins.measureSpan("task", "monarchy_transaction") { + monarchy.awaitTransaction { realm -> + // IMPORTANT nothing should be suspend here as we are accessing the realm instance (thread local) + handleRooms(reporter, syncResponse, realm, isInitialSync, aggregator) + handleAccountData(reporter, realm, syncResponse) + handlePresence(realm, syncResponse) + + tokenStore.saveToken(realm, syncResponse.nextBatch) + } + } + } + + private fun handleRooms( + reporter: ProgressReporter?, + syncResponse: SyncResponse, + realm: Realm, + isInitialSync: Boolean, + aggregator: SyncResponsePostTreatmentAggregator + ) { + relevantPlugins.measureSpan("task", "handle_rooms") { measureTimeMillis { Timber.v("Handle rooms") reportSubtask(reporter, InitialSyncStep.ImportingAccountRoom, 1, 0.8f) { @@ -106,7 +166,11 @@ internal class SyncResponseHandler @Inject constructor( }.also { Timber.v("Finish handling rooms in $it ms") } + } + } + private fun handleAccountData(reporter: ProgressReporter?, realm: Realm, syncResponse: SyncResponse) { + relevantPlugins.measureSpan("task", "handle_account_data") { measureTimeMillis { reportSubtask(reporter, InitialSyncStep.ImportingAccountData, 1, 0.1f) { Timber.v("Handle accountData") @@ -115,44 +179,59 @@ internal class SyncResponseHandler @Inject constructor( }.also { Timber.v("Finish handling accountData in $it ms") } + } + } + private fun handlePresence(realm: Realm, syncResponse: SyncResponse) { + relevantPlugins.measureSpan("task", "handle_presence") { measureTimeMillis { Timber.v("Handle Presence") presenceSyncHandler.handle(realm, syncResponse.presence) }.also { Timber.v("Finish handling Presence in $it ms") } - tokenStore.saveToken(realm, syncResponse.nextBatch) } + } - // Everything else we need to do outside the transaction - measureTimeMillis { - aggregatorHandler.handle(aggregator) - }.also { - Timber.v("Aggregator management took $it ms") - } - - measureTimeMillis { - syncResponse.rooms?.let { - checkPushRules(it, isInitialSync) - userAccountDataSyncHandler.synchronizeWithServerIfNeeded(it.invite) - dispatchInvitedRoom(it) + private suspend fun aggregateSyncResponse(aggregator: SyncResponsePostTreatmentAggregator) { + relevantPlugins.measureSpan("task", "aggregator_management") { + // Everything else we need to do outside the transaction + measureTimeMillis { + aggregatorHandler.handle(aggregator) + }.also { + Timber.v("Aggregator management took $it ms") } - }.also { - Timber.v("SyncResponse.rooms post treatment took $it ms") } + } - measureTimeMillis { - cryptoSyncHandler.onSyncCompleted(syncResponse) - }.also { - Timber.v("cryptoSyncHandler.onSyncCompleted took $it ms") + private suspend fun postTreatmentSyncResponse(syncResponse: SyncResponse, isInitialSync: Boolean) { + relevantPlugins.measureSpan("task", "sync_response_post_treatment") { + measureTimeMillis { + syncResponse.rooms?.let { + checkPushRules(it, isInitialSync) + userAccountDataSyncHandler.synchronizeWithServerIfNeeded(it.invite) + dispatchInvitedRoom(it) + } + }.also { + Timber.v("SyncResponse.rooms post treatment took $it ms") + } } + } - // post sync stuffs + private fun markCryptoSyncCompleted(syncResponse: SyncResponse) { + relevantPlugins.measureSpan("task", "crypto_sync_handler_onSyncCompleted") { + measureTimeMillis { + cryptoSyncHandler.onSyncCompleted(syncResponse) + }.also { + Timber.v("cryptoSyncHandler.onSyncCompleted took $it ms") + } + } + } + + private fun handlePostSync() { monarchy.writeAsync { roomSyncHandler.postSyncSpaceHierarchyHandle(it) } - Timber.v("On sync completed") } private fun dispatchInvitedRoom(roomsSyncResponse: RoomsSyncResponse) { diff --git a/vector/src/main/java/im/vector/app/features/analytics/metrics/VectorPlugins.kt b/vector/src/main/java/im/vector/app/features/analytics/metrics/VectorPlugins.kt index 64f143a2fd..4278c1011b 100644 --- a/vector/src/main/java/im/vector/app/features/analytics/metrics/VectorPlugins.kt +++ b/vector/src/main/java/im/vector/app/features/analytics/metrics/VectorPlugins.kt @@ -17,6 +17,7 @@ package im.vector.app.features.analytics.metrics import im.vector.app.features.analytics.metrics.sentry.SentryDownloadDeviceKeysMetrics +import im.vector.app.features.analytics.metrics.sentry.SentrySyncDurationMetrics import org.matrix.android.sdk.api.metrics.MetricPlugin import javax.inject.Inject import javax.inject.Singleton @@ -27,9 +28,10 @@ import javax.inject.Singleton @Singleton data class VectorPlugins @Inject constructor( val sentryDownloadDeviceKeysMetrics: SentryDownloadDeviceKeysMetrics, + val sentrySyncDurationMetrics: SentrySyncDurationMetrics, ) { /** * Returns [List] of all [MetricPlugin] hold by this class. */ - fun plugins(): List<MetricPlugin> = listOf(sentryDownloadDeviceKeysMetrics) + fun plugins(): List<MetricPlugin> = listOf(sentryDownloadDeviceKeysMetrics, sentrySyncDurationMetrics) } diff --git a/vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentryDownloadDeviceKeysMetrics.kt b/vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentryDownloadDeviceKeysMetrics.kt index 92213d380c..488b72bfd9 100644 --- a/vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentryDownloadDeviceKeysMetrics.kt +++ b/vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentryDownloadDeviceKeysMetrics.kt @@ -26,8 +26,10 @@ class SentryDownloadDeviceKeysMetrics @Inject constructor() : DownloadDeviceKeys private var transaction: ITransaction? = null override fun startTransaction() { - transaction = Sentry.startTransaction("download_device_keys", "task") - logTransaction("Sentry transaction started") + if (Sentry.isEnabled()) { + transaction = Sentry.startTransaction("download_device_keys", "task") + logTransaction("Sentry transaction started") + } } override fun finishTransaction() { diff --git a/vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentrySyncDurationMetrics.kt b/vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentrySyncDurationMetrics.kt new file mode 100644 index 0000000000..d69ed01526 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/analytics/metrics/sentry/SentrySyncDurationMetrics.kt @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.analytics.metrics.sentry + +import io.sentry.ISpan +import io.sentry.ITransaction +import io.sentry.Sentry +import io.sentry.SpanStatus +import org.matrix.android.sdk.api.metrics.SyncDurationMetricPlugin +import java.util.EmptyStackException +import java.util.Stack +import javax.inject.Inject + +/** + * Sentry based implementation of SyncDurationMetricPlugin. + */ +class SentrySyncDurationMetrics @Inject constructor() : SyncDurationMetricPlugin { + private var transaction: ITransaction? = null + + // Stacks to keep spans in LIFO order. + private var spans: Stack<ISpan> = Stack() + + /** + * Starts the span for a sub-task. + * + * @param operation Name of the new span. + * @param description Description of the new span. + * + * @throws IllegalStateException if this is called without starting a transaction ie. `measureSpan` must be called within `measureMetric`. + */ + override fun startSpan(operation: String, description: String) { + if (Sentry.isEnabled()) { + val span = Sentry.getSpan() ?: throw IllegalStateException("measureSpan block must be called within measureMetric") + val innerSpan = span.startChild(operation, description) + spans.push(innerSpan) + logTransaction("Sentry span started: operation=[$operation], description=[$description]") + } + } + + override fun finishSpan() { + try { + spans.pop() + } catch (e: EmptyStackException) { + null + }?.finish() + logTransaction("Sentry span finished") + } + + override fun startTransaction() { + if (Sentry.isEnabled()) { + transaction = Sentry.startTransaction("sync_response_handler", "task", true) + logTransaction("Sentry transaction started") + } + } + + override fun finishTransaction() { + transaction?.finish() + logTransaction("Sentry transaction finished") + } + + override fun onError(throwable: Throwable) { + try { + spans.peek() + } catch (e: EmptyStackException) { + null + }?.apply { + this.throwable = throwable + this.status = SpanStatus.INTERNAL_ERROR + } ?: transaction?.apply { + this.throwable = throwable + this.status = SpanStatus.INTERNAL_ERROR + } + logTransaction("Sentry transaction encountered error ${throwable.message}") + } +}