From f4f55d68c2b998fd8c228b2e1464fde045371cb2 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 11:33:14 +0100 Subject: [PATCH 01/16] Add support for fullscreen images --- .../support/he/ui/AttachmentFullscreenImagePreview.kt | 7 ++++++- .../android/support/he/ui/HEConversationDetailScreen.kt | 7 +++++-- .../wordpress/android/support/he/ui/HESupportActivity.kt | 5 ++++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt index 66aa7f362d8a..a535a8e638dd 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt @@ -40,13 +40,15 @@ import androidx.compose.ui.window.DialogProperties import coil.compose.SubcomposeAsyncImage import coil.request.ImageRequest import org.wordpress.android.R +import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.ui.compose.theme.AppThemeM3 @Composable fun AttachmentFullscreenImagePreview( imageUrl: String, onDismiss: () -> Unit, - onDownload: () -> Unit = {} + onDownload: () -> Unit = {}, + accountStore: AccountStore? = null, ) { var scale by remember { mutableFloatStateOf(1f) } var offsetX by remember { mutableFloatStateOf(0f) } @@ -90,6 +92,9 @@ fun AttachmentFullscreenImagePreview( model = ImageRequest.Builder(LocalContext.current) .data(imageUrl) .crossfade(true) + .apply { + addHeader("Authorization", "Bearer ${accountStore?.accessToken}") + } .build(), contentDescription = attachmentImageDescription, modifier = Modifier diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt index bf3a8dd4e731..fea47eb9e0d9 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt @@ -59,6 +59,7 @@ import coil.decode.VideoFrameDecoder import coil.request.ImageRequest import coil.request.videoFrameMillis import org.wordpress.android.R +import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.support.aibot.util.formatRelativeTime import org.wordpress.android.support.he.model.AttachmentState import org.wordpress.android.support.he.model.ConversationStatus @@ -87,7 +88,8 @@ fun HEConversationDetailScreen( attachmentState: AttachmentState = AttachmentState(), attachmentActionsListener: AttachmentActionsListener, onDownloadAttachment: (SupportAttachment) -> Unit = {}, - videoUrlResolver: org.wordpress.android.support.he.util.VideoUrlResolver? = null + videoUrlResolver: org.wordpress.android.support.he.util.VideoUrlResolver? = null, + accountStore: AccountStore? = null, ) { val listState = rememberLazyListState() val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) @@ -240,7 +242,8 @@ fun HEConversationDetailScreen( onDismiss = { previewAttachment = null }, onDownload = { onDownloadAttachment(attachment) - } + }, + accountStore = accountStore, ) } AttachmentType.Video -> { diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt index 9cf20c3691e0..7e4cb6b3969c 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt @@ -28,6 +28,7 @@ import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.launch import org.wordpress.android.ui.compose.theme.AppThemeM3 import org.wordpress.android.R +import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.support.common.ui.ConversationsSupportViewModel import org.wordpress.android.support.he.util.AttachmentActionsListener @@ -44,6 +45,7 @@ class HESupportActivity : AppCompatActivity() { @Inject lateinit var fileDownloadManager: ReaderFileDownloadManager @Inject lateinit var appLogWrapper: AppLogWrapper @Inject lateinit var videoUrlResolver: VideoUrlResolver + @Inject lateinit var accountStore: AccountStore private val viewModel by viewModels() private lateinit var composeView: ComposeView @@ -205,7 +207,8 @@ class HESupportActivity : AppCompatActivity() { } // Start download with proper filename fileDownloadManager.downloadFile(attachment.url, attachment.filename) - } + }, + accountStore = accountStore ) } } From 73ebae5f550469efb9d7a009cc36e79553dc9764 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 11:36:12 +0100 Subject: [PATCH 02/16] Supporting thumbnails authentication --- .../he/ui/HEConversationDetailScreen.kt | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt index fea47eb9e0d9..5a1bff8afe8a 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt @@ -167,7 +167,8 @@ fun HEConversationDetailScreen( message = message, timestamp = formatRelativeTime(message.createdAt, resources), onPreviewAttachment = { attachment -> previewAttachment = attachment }, - onDownloadAttachment = onDownloadAttachment + onDownloadAttachment = onDownloadAttachment, + accountStore = accountStore ) } @@ -360,7 +361,8 @@ private fun MessageItem( message: SupportMessage, timestamp: String, onPreviewAttachment: (SupportAttachment) -> Unit, - onDownloadAttachment: (SupportAttachment) -> Unit + onDownloadAttachment: (SupportAttachment) -> Unit, + accountStore: AccountStore? = null, ) { val messageDescription = "${message.authorName}, $timestamp. ${message.formattedText}" @@ -421,7 +423,8 @@ private fun MessageItem( AttachmentsList( attachments = message.attachments, onPreviewAttachment = onPreviewAttachment, - onDownloadAttachment = onDownloadAttachment + onDownloadAttachment = onDownloadAttachment, + accountStore = accountStore ) } } @@ -432,7 +435,8 @@ private fun MessageItem( private fun AttachmentsList( attachments: List, onPreviewAttachment: (SupportAttachment) -> Unit, - onDownloadAttachment: (SupportAttachment) -> Unit + onDownloadAttachment: (SupportAttachment) -> Unit, + accountStore: AccountStore? = null, ) { FlowRow( horizontalArrangement = Arrangement.spacedBy(8.dp), @@ -446,7 +450,8 @@ private fun AttachmentsList( AttachmentType.Image, AttachmentType.Video -> onPreviewAttachment(attachment) else -> onDownloadAttachment(attachment) } - } + }, + accountStore = accountStore ) } } @@ -455,7 +460,8 @@ private fun AttachmentsList( @Composable private fun AttachmentItem( attachment: SupportAttachment, - onClick: () -> Unit + onClick: () -> Unit, + accountStore: AccountStore? = null, ) { val iconRes = when (attachment.type) { AttachmentType.Image -> R.drawable.ic_image_white_24dp @@ -486,6 +492,9 @@ private fun AttachmentItem( videoFrameMillis(0) // Get first frame } } + .apply { + addHeader("Authorization", "Bearer ${accountStore?.accessToken}") + } .build(), contentDescription = attachment.filename, modifier = Modifier.fillMaxSize(), From 71b0c80f13b3c47f11ba7c4f3b70f7a4986ba690 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 11:46:51 +0100 Subject: [PATCH 03/16] Some refactoring to avoid propagate the accountStore or the token --- .../he/ui/AttachmentFullscreenImagePreview.kt | 7 ++-- .../he/ui/HEConversationDetailScreen.kt | 36 +++++++++++-------- .../support/he/ui/HESupportActivity.kt | 5 ++- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt index a535a8e638dd..22be332f2d79 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt @@ -41,14 +41,15 @@ import coil.compose.SubcomposeAsyncImage import coil.request.ImageRequest import org.wordpress.android.R import org.wordpress.android.fluxc.store.AccountStore +import org.wordpress.android.support.he.ui.HESupportActivity.Companion.AUTHORIZATION_TAG import org.wordpress.android.ui.compose.theme.AppThemeM3 @Composable fun AttachmentFullscreenImagePreview( imageUrl: String, + onGetAuthorizationHeaderArgument: () -> String, onDismiss: () -> Unit, onDownload: () -> Unit = {}, - accountStore: AccountStore? = null, ) { var scale by remember { mutableFloatStateOf(1f) } var offsetX by remember { mutableFloatStateOf(0f) } @@ -93,7 +94,7 @@ fun AttachmentFullscreenImagePreview( .data(imageUrl) .crossfade(true) .apply { - addHeader("Authorization", "Bearer ${accountStore?.accessToken}") + addHeader(AUTHORIZATION_TAG, onGetAuthorizationHeaderArgument.invoke()) } .build(), contentDescription = attachmentImageDescription, @@ -186,6 +187,7 @@ private fun AttachmentFullscreenImagePreviewPreview() { AppThemeM3(isDarkTheme = false) { AttachmentFullscreenImagePreview( imageUrl = "https://via.placeholder.com/800x600", + onGetAuthorizationHeaderArgument = { "" }, onDismiss = { }, onDownload = { } ) @@ -198,6 +200,7 @@ private fun AttachmentFullscreenImagePreviewPreviewDark() { AppThemeM3(isDarkTheme = true) { AttachmentFullscreenImagePreview( imageUrl = "https://via.placeholder.com/800x600", + onGetAuthorizationHeaderArgument = { "" }, onDismiss = { }, onDownload = { } ) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt index 5a1bff8afe8a..119993a7ffba 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt @@ -68,6 +68,7 @@ import org.wordpress.android.support.he.model.AttachmentType import org.wordpress.android.support.he.model.SupportAttachment import org.wordpress.android.support.he.model.SupportConversation import org.wordpress.android.support.he.model.SupportMessage +import org.wordpress.android.support.he.ui.HESupportActivity.Companion.AUTHORIZATION_TAG import org.wordpress.android.support.he.util.AttachmentActionsListener import org.wordpress.android.support.he.util.generateSampleHESupportConversations import org.wordpress.android.ui.compose.components.MainTopAppBar @@ -89,7 +90,7 @@ fun HEConversationDetailScreen( attachmentActionsListener: AttachmentActionsListener, onDownloadAttachment: (SupportAttachment) -> Unit = {}, videoUrlResolver: org.wordpress.android.support.he.util.VideoUrlResolver? = null, - accountStore: AccountStore? = null, + onGetAuthorizationHeaderArgument: () -> String, ) { val listState = rememberLazyListState() val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) @@ -168,7 +169,7 @@ fun HEConversationDetailScreen( timestamp = formatRelativeTime(message.createdAt, resources), onPreviewAttachment = { attachment -> previewAttachment = attachment }, onDownloadAttachment = onDownloadAttachment, - accountStore = accountStore + onGetAuthorizationHeaderArgument = onGetAuthorizationHeaderArgument ) } @@ -240,21 +241,22 @@ fun HEConversationDetailScreen( AttachmentType.Image -> { AttachmentFullscreenImagePreview( imageUrl = attachment.url, + onGetAuthorizationHeaderArgument = onGetAuthorizationHeaderArgument, onDismiss = { previewAttachment = null }, onDownload = { onDownloadAttachment(attachment) - }, - accountStore = accountStore, + } ) } AttachmentType.Video -> { AttachmentFullscreenVideoPlayer( videoUrl = attachment.url, + onGetAuthorizationHeaderArgument = onGetAuthorizationHeaderArgument, onDismiss = { previewAttachment = null }, onDownload = { onDownloadAttachment(attachment) }, - videoUrlResolver = videoUrlResolver + videoUrlResolver = videoUrlResolver, ) } else -> { @@ -362,7 +364,7 @@ private fun MessageItem( timestamp: String, onPreviewAttachment: (SupportAttachment) -> Unit, onDownloadAttachment: (SupportAttachment) -> Unit, - accountStore: AccountStore? = null, + onGetAuthorizationHeaderArgument: () -> String, ) { val messageDescription = "${message.authorName}, $timestamp. ${message.formattedText}" @@ -424,7 +426,7 @@ private fun MessageItem( attachments = message.attachments, onPreviewAttachment = onPreviewAttachment, onDownloadAttachment = onDownloadAttachment, - accountStore = accountStore + onGetAuthorizationHeaderArgument = onGetAuthorizationHeaderArgument ) } } @@ -436,7 +438,7 @@ private fun AttachmentsList( attachments: List, onPreviewAttachment: (SupportAttachment) -> Unit, onDownloadAttachment: (SupportAttachment) -> Unit, - accountStore: AccountStore? = null, + onGetAuthorizationHeaderArgument: () -> String, ) { FlowRow( horizontalArrangement = Arrangement.spacedBy(8.dp), @@ -451,7 +453,7 @@ private fun AttachmentsList( else -> onDownloadAttachment(attachment) } }, - accountStore = accountStore + onGetAuthorizationHeaderArgument = onGetAuthorizationHeaderArgument ) } } @@ -461,7 +463,7 @@ private fun AttachmentsList( private fun AttachmentItem( attachment: SupportAttachment, onClick: () -> Unit, - accountStore: AccountStore? = null, + onGetAuthorizationHeaderArgument: () -> String, ) { val iconRes = when (attachment.type) { AttachmentType.Image -> R.drawable.ic_image_white_24dp @@ -493,7 +495,7 @@ private fun AttachmentItem( } } .apply { - addHeader("Authorization", "Bearer ${accountStore?.accessToken}") + addHeader(AUTHORIZATION_TAG, onGetAuthorizationHeaderArgument.invoke()) } .build(), contentDescription = attachment.filename, @@ -598,7 +600,8 @@ private fun HEConversationDetailScreenPreview() { override fun onRemoveImage(uri: Uri) { // stub } - } + }, + onGetAuthorizationHeaderArgument = { "" }, ) } } @@ -622,7 +625,8 @@ private fun HEConversationDetailScreenPreviewDark() { override fun onRemoveImage(uri: Uri) { // stub } - } + }, + onGetAuthorizationHeaderArgument = { "" }, ) } } @@ -648,7 +652,8 @@ private fun HEConversationDetailScreenWordPressPreview() { override fun onRemoveImage(uri: Uri) { // stub } - } + }, + onGetAuthorizationHeaderArgument = { "" }, ) } } @@ -673,7 +678,8 @@ private fun HEConversationDetailScreenPreviewWordPressDark() { override fun onRemoveImage(uri: Uri) { // stub } - } + }, + onGetAuthorizationHeaderArgument = { "" }, ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt index 7e4cb6b3969c..a46ec22c1de0 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt @@ -208,7 +208,7 @@ class HESupportActivity : AppCompatActivity() { // Start download with proper filename fileDownloadManager.downloadFile(attachment.url, attachment.filename) }, - accountStore = accountStore + onGetAuthorizationHeaderArgument = { "$BEARER_TAG ${accountStore?.accessToken}" } ) } } @@ -279,6 +279,9 @@ class HESupportActivity : AppCompatActivity() { companion object { + const val AUTHORIZATION_TAG = "Authorization" + private const val BEARER_TAG = "Bearer" + @JvmStatic fun createIntent(context: Context): Intent = Intent(context, HESupportActivity::class.java) } From 4b6db9f61b9ab385faf1b733c6d739c4dcd09767 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 15:22:10 +0100 Subject: [PATCH 04/16] Downloading and playing video --- .../support/he/model/VideoDownloadState.kt | 10 ++ .../he/ui/AttachmentFullscreenVideoPlayer.kt | 76 ++++++++------- .../he/ui/HEConversationDetailScreen.kt | 19 +++- .../support/he/ui/HESupportActivity.kt | 14 ++- .../support/he/ui/HESupportViewModel.kt | 93 +++++++++++++++++++ 5 files changed, 175 insertions(+), 37 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt b/WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt new file mode 100644 index 000000000000..d854a9bb7744 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt @@ -0,0 +1,10 @@ +package org.wordpress.android.support.he.model + +import java.io.File + +sealed class VideoDownloadState { + object Idle : VideoDownloadState() + object Downloading : VideoDownloadState() + data class Success(val file: File) : VideoDownloadState() + data class Error(val message: String) : VideoDownloadState() +} diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt index 78d247990bb8..fbc69d84229b 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt @@ -1,7 +1,6 @@ package org.wordpress.android.support.he.ui import android.view.ViewGroup -import androidx.core.net.toUri import android.widget.FrameLayout import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement @@ -22,6 +21,7 @@ import androidx.compose.material3.IconButton import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -37,49 +37,57 @@ import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView import androidx.compose.ui.window.Dialog import androidx.compose.ui.window.DialogProperties +import androidx.core.net.toUri import com.google.android.exoplayer2.MediaItem import com.google.android.exoplayer2.Player import com.google.android.exoplayer2.SimpleExoPlayer import com.google.android.exoplayer2.ui.PlayerView import org.wordpress.android.R +import org.wordpress.android.support.he.model.VideoDownloadState import org.wordpress.android.support.he.util.VideoUrlResolver +import org.wordpress.android.util.AppLog +import java.io.File @Composable fun AttachmentFullscreenVideoPlayer( videoUrl: String, onDismiss: () -> Unit, onDownload: () -> Unit = {}, + onGetAuthorizationHeaderArgument: () -> String, + downloadState: VideoDownloadState, + onStartVideoDownload: (String, String) -> Unit, + onResetVideoDownloadState: () -> Unit = {}, videoUrlResolver: VideoUrlResolver? = null ) { val context = LocalContext.current - var hasError by remember { mutableStateOf(false) } - var resolvedUrl by remember { mutableStateOf(null) } - var isResolving by remember { mutableStateOf(true) } + var localVideoFile by remember { mutableStateOf(null) } + + // Start download when composable is first launched + LaunchedEffect(videoUrl) { + onStartVideoDownload(videoUrl, onGetAuthorizationHeaderArgument()) + } - // Resolve URL redirects before playing - androidx.compose.runtime.LaunchedEffect(videoUrl) { - if (videoUrlResolver != null) { - resolvedUrl = videoUrlResolver.resolveUrl(videoUrl) - } else { - resolvedUrl = videoUrl + // Update local file when download succeeds + LaunchedEffect(downloadState) { + if (downloadState is VideoDownloadState.Success) { + localVideoFile = downloadState.file } - isResolving = false } - val exoPlayer = remember(resolvedUrl) { - // Don't create player until URL is resolved - val url = resolvedUrl ?: return@remember null + val exoPlayer = remember(localVideoFile) { + // Don't create player until video is downloaded + val file = localVideoFile ?: return@remember null SimpleExoPlayer.Builder(context).build().apply { - // Add error listener + // Add error listener for logging addListener(object : Player.EventListener { override fun onPlayerError(error: com.google.android.exoplayer2.ExoPlaybackException) { - hasError = true + AppLog.e(AppLog.T.SUPPORT, "Video playback error", error) } }) - // Simple configuration using MediaItem - val mediaItem = MediaItem.fromUri(url.toUri()) + // Play from local file + val mediaItem = MediaItem.fromUri(file.toUri()) setMediaItem(mediaItem) prepare() playWhenReady = true @@ -87,17 +95,16 @@ fun AttachmentFullscreenVideoPlayer( } } - DisposableEffect(Unit) { - onDispose { - exoPlayer?.stop() - exoPlayer?.release() - } + fun closeFullScreen() { + exoPlayer?.stop() + exoPlayer?.release() + onDismiss() + onResetVideoDownloadState() } Dialog( onDismissRequest = { - exoPlayer?.stop() - onDismiss() + closeFullScreen() }, properties = DialogProperties( usePlatformDefaultWidth = false, @@ -110,15 +117,16 @@ fun AttachmentFullscreenVideoPlayer( .fillMaxSize() .background(Color.Black) ) { - when { - isResolving -> { - // Show loading indicator while resolving URL + when (downloadState) { + is VideoDownloadState.Idle, + is VideoDownloadState.Downloading -> { + // Show loading indicator while downloading video CircularProgressIndicator( modifier = Modifier.align(Alignment.Center), color = Color.White ) } - hasError -> { + is VideoDownloadState.Error -> { // Show error message when video fails to load Column( modifier = Modifier @@ -146,17 +154,16 @@ fun AttachmentFullscreenVideoPlayer( ) Button( onClick = { - exoPlayer?.stop() onDownload() - onDismiss() + closeFullScreen() } ) { Text(stringResource(R.string.he_support_download_video_button)) } } } - else -> { - // Show video player when URL is resolved and no error + is VideoDownloadState.Success -> { + // Show video player when video is downloaded successfully exoPlayer?.let { player -> AndroidView( factory = { ctx -> @@ -206,8 +213,7 @@ fun AttachmentFullscreenVideoPlayer( // Close button IconButton( onClick = { - exoPlayer?.stop() - onDismiss() + closeFullScreen() } ) { Icon( diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt index 119993a7ffba..86904f60214f 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt @@ -68,6 +68,7 @@ import org.wordpress.android.support.he.model.AttachmentType import org.wordpress.android.support.he.model.SupportAttachment import org.wordpress.android.support.he.model.SupportConversation import org.wordpress.android.support.he.model.SupportMessage +import org.wordpress.android.support.he.model.VideoDownloadState import org.wordpress.android.support.he.ui.HESupportActivity.Companion.AUTHORIZATION_TAG import org.wordpress.android.support.he.util.AttachmentActionsListener import org.wordpress.android.support.he.util.generateSampleHESupportConversations @@ -91,6 +92,9 @@ fun HEConversationDetailScreen( onDownloadAttachment: (SupportAttachment) -> Unit = {}, videoUrlResolver: org.wordpress.android.support.he.util.VideoUrlResolver? = null, onGetAuthorizationHeaderArgument: () -> String, + videoDownloadState: VideoDownloadState, + onStartVideoDownload: (String, String) -> Unit, + onResetVideoDownloadState: () -> Unit = {}, ) { val listState = rememberLazyListState() val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) @@ -252,7 +256,12 @@ fun HEConversationDetailScreen( AttachmentFullscreenVideoPlayer( videoUrl = attachment.url, onGetAuthorizationHeaderArgument = onGetAuthorizationHeaderArgument, - onDismiss = { previewAttachment = null }, + downloadState = videoDownloadState, + onStartVideoDownload = onStartVideoDownload, + onResetVideoDownloadState = onResetVideoDownloadState, + onDismiss = { + previewAttachment = null + }, onDownload = { onDownloadAttachment(attachment) }, @@ -602,6 +611,8 @@ private fun HEConversationDetailScreenPreview() { } }, onGetAuthorizationHeaderArgument = { "" }, + videoDownloadState = VideoDownloadState.Idle, + onStartVideoDownload = { _, _ -> }, ) } } @@ -627,6 +638,8 @@ private fun HEConversationDetailScreenPreviewDark() { } }, onGetAuthorizationHeaderArgument = { "" }, + videoDownloadState = VideoDownloadState.Idle, + onStartVideoDownload = { _, _ -> }, ) } } @@ -654,6 +667,8 @@ private fun HEConversationDetailScreenWordPressPreview() { } }, onGetAuthorizationHeaderArgument = { "" }, + videoDownloadState = VideoDownloadState.Idle, + onStartVideoDownload = { _, _ -> }, ) } } @@ -680,6 +695,8 @@ private fun HEConversationDetailScreenPreviewWordPressDark() { } }, onGetAuthorizationHeaderArgument = { "" }, + videoDownloadState = VideoDownloadState.Idle, + onStartVideoDownload = { _, _ -> }, ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt index a46ec22c1de0..a3c4e7850f42 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt @@ -88,6 +88,12 @@ class HESupportActivity : AppCompatActivity() { viewModel.init() } + override fun onDestroy() { + super.onDestroy() + // Cleanup cached video files + viewModel.cleanupVideoCache() + } + private fun observeNavigationEvents() { lifecycleScope.launch { @@ -176,6 +182,7 @@ class HESupportActivity : AppCompatActivity() { val isSendingMessage by viewModel.isSendingMessage.collectAsState() val messageSendResult by viewModel.messageSendResult.collectAsState() val attachmentState by viewModel.attachmentState.collectAsState() + val videoDownloadState by viewModel.videoDownloadState.collectAsState() selectedConversation?.let { conversation -> HEConversationDetailScreen( @@ -208,7 +215,12 @@ class HESupportActivity : AppCompatActivity() { // Start download with proper filename fileDownloadManager.downloadFile(attachment.url, attachment.filename) }, - onGetAuthorizationHeaderArgument = { "$BEARER_TAG ${accountStore?.accessToken}" } + onGetAuthorizationHeaderArgument = { "$BEARER_TAG ${accountStore.accessToken}" }, + videoDownloadState = videoDownloadState, + onStartVideoDownload = { url, authHeader -> + viewModel.downloadVideoToTempFile(url, authHeader) + }, + onResetVideoDownloadState = { viewModel.resetVideoDownloadState() } ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt index 7f684fa45a8e..8b19387cdfb8 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt @@ -17,11 +17,15 @@ import org.wordpress.android.support.common.ui.ConversationsSupportViewModel import org.wordpress.android.support.he.model.AttachmentState import org.wordpress.android.support.he.model.MessageSendResult import org.wordpress.android.support.he.model.SupportConversation +import org.wordpress.android.support.he.model.VideoDownloadState import org.wordpress.android.support.he.repository.CreateConversationResult import org.wordpress.android.support.he.repository.HESupportRepository import org.wordpress.android.support.he.util.TempAttachmentsUtil import org.wordpress.android.util.AppLog import org.wordpress.android.util.NetworkUtilsWrapper +import java.io.File +import java.net.HttpURLConnection +import java.net.URL import javax.inject.Inject import javax.inject.Named @@ -48,6 +52,13 @@ class HESupportViewModel @Inject constructor( private val _attachmentState = MutableStateFlow(AttachmentState()) val attachmentState: StateFlow = _attachmentState.asStateFlow() + // Cache for downloaded video files (videoUrl -> tempFile) + private val videoCache = mutableMapOf() + + // Video download state + private val _videoDownloadState = MutableStateFlow(VideoDownloadState.Idle) + val videoDownloadState: StateFlow = _videoDownloadState.asStateFlow() + override fun initRepository(accessToken: String) { heSupportRepository.init(accessToken) } @@ -275,4 +286,86 @@ class HESupportViewModel @Inject constructor( fun notifyGeneralError() { _errorMessage.value = ErrorType.GENERAL } + + /** + * Downloads a video to a temporary file with caching and state management. + * Updates videoDownloadState as it progresses. + */ + fun downloadVideoToTempFile(videoUrl: String, authHeader: String) { + viewModelScope.launch(ioDispatcher) { + try { + _videoDownloadState.value = VideoDownloadState.Idle + // Check cache first + videoCache[videoUrl]?.let { cachedFile -> + if (cachedFile.exists()) { + AppLog.d(AppLog.T.SUPPORT, "Using cached video file for: $videoUrl") + _videoDownloadState.value = VideoDownloadState.Success(cachedFile) + return@launch + } else { + // File was deleted, remove from cache + videoCache.remove(videoUrl) + } + } + + // Start downloading + _videoDownloadState.value = VideoDownloadState.Downloading + AppLog.d(AppLog.T.SUPPORT, "Downloading video to temp file: $videoUrl") + + val tempFile = File.createTempFile("video_", ".mp4", application.cacheDir) + val connection = URL(videoUrl).openConnection() as HttpURLConnection + connection.requestMethod = "GET" + connection.setRequestProperty("Authorization", authHeader) + connection.instanceFollowRedirects = true + + try { + connection.connect() + + val responseCode = connection.responseCode + AppLog.d(AppLog.T.SUPPORT, "Download response code: $responseCode") + + if (responseCode == HttpURLConnection.HTTP_OK) { + connection.inputStream.use { input -> + tempFile.outputStream().use { output -> + input.copyTo(output) + } + } + // Cache the downloaded file + videoCache[videoUrl] = tempFile + AppLog.d(AppLog.T.SUPPORT, "Video downloaded and cached: ${tempFile.absolutePath}") + _videoDownloadState.value = VideoDownloadState.Success(tempFile) + } else { + val errorMsg = "Failed to download video. Response code: $responseCode" + AppLog.e(AppLog.T.SUPPORT, errorMsg) + _videoDownloadState.value = VideoDownloadState.Error(errorMsg) + } + } finally { + connection.disconnect() + } + } catch (e: Exception) { + AppLog.e(AppLog.T.SUPPORT, "Error downloading video", e) + _videoDownloadState.value = VideoDownloadState.Error(e.message ?: "Unknown error") + } + } + } + + /** + * Resets the video download state to Idle. Call this when closing the video player. + */ + fun resetVideoDownloadState() { + _videoDownloadState.value = VideoDownloadState.Idle + } + + /** + * Cleans up all cached video files. Call this when the activity is destroyed. + */ + fun cleanupVideoCache() { + AppLog.d(AppLog.T.SUPPORT, "Cleaning up ${videoCache.size} cached video files") + videoCache.values.forEach { file -> + if (file.exists()) { + AppLog.d(AppLog.T.SUPPORT, "Deleting temp video file: ${file.absolutePath}") + file.delete() + } + } + videoCache.clear() + } } From 349296f3538a311b494caa5d7ac1419386b14911 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 15:22:41 +0100 Subject: [PATCH 05/16] Removing VideoUrlResolver --- .../he/ui/AttachmentFullscreenVideoPlayer.kt | 3 - .../support/he/util/VideoUrlResolver.kt | 68 ------------------- 2 files changed, 71 deletions(-) delete mode 100644 WordPress/src/main/java/org/wordpress/android/support/he/util/VideoUrlResolver.kt diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt index fbc69d84229b..d0ca120bf781 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt @@ -20,7 +20,6 @@ import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -44,7 +43,6 @@ import com.google.android.exoplayer2.SimpleExoPlayer import com.google.android.exoplayer2.ui.PlayerView import org.wordpress.android.R import org.wordpress.android.support.he.model.VideoDownloadState -import org.wordpress.android.support.he.util.VideoUrlResolver import org.wordpress.android.util.AppLog import java.io.File @@ -57,7 +55,6 @@ fun AttachmentFullscreenVideoPlayer( downloadState: VideoDownloadState, onStartVideoDownload: (String, String) -> Unit, onResetVideoDownloadState: () -> Unit = {}, - videoUrlResolver: VideoUrlResolver? = null ) { val context = LocalContext.current var localVideoFile by remember { mutableStateOf(null) } diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/util/VideoUrlResolver.kt b/WordPress/src/main/java/org/wordpress/android/support/he/util/VideoUrlResolver.kt deleted file mode 100644 index 4db12bd8c248..000000000000 --- a/WordPress/src/main/java/org/wordpress/android/support/he/util/VideoUrlResolver.kt +++ /dev/null @@ -1,68 +0,0 @@ -package org.wordpress.android.support.he.util - -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext -import okhttp3.OkHttpClient -import okhttp3.Request -import org.wordpress.android.fluxc.utils.AppLogWrapper -import org.wordpress.android.util.AppLog -import java.util.concurrent.TimeUnit -import javax.inject.Inject - -private const val SUCCESSFULLY_RESOLVED_CODE = 206 -private const val TIMEOUT_SECONDS = 30L - -/** - * Helper class to resolve video URLs that may have redirect chains. - * This is particularly useful for Zendesk attachment URLs which use multiple redirects - * with authentication tokens. - */ -class VideoUrlResolver @Inject constructor( - private val appLogWrapper: AppLogWrapper -) { - private val client by lazy { - OkHttpClient.Builder() - .followRedirects(true) - .followSslRedirects(true) - .connectTimeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .readTimeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .writeTimeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .build() - } - /** - * Resolves a video URL by following all redirects and returning the final URL. - * - * @param url The original video URL - * @return The final URL after following all redirects, or the original URL if resolution fails - */ - @Suppress("TooGenericExceptionCaught") - suspend fun resolveUrl(url: String): String = withContext(Dispatchers.IO) { - try { - val request = Request.Builder() - .url(url) - .get() - .header("Range", "bytes=0-0") // Request only first byte to minimize data transfer - .build() - - client.newCall(request).execute().use { response -> - val finalUrl = response.request.url.toString() - - when { - response.isSuccessful || response.code == SUCCESSFULLY_RESOLVED_CODE -> { - finalUrl - } - finalUrl != url -> { - // Even if response isn't successful, use the final URL if it's different - finalUrl - } - else -> { - url - } - } - } - } catch (e: Exception) { - appLogWrapper.e(AppLog.T.UTILS, "Error resolving support url: ${e.stackTraceToString()}") - url - } - } -} From 0b4f43f08e8e265df550b980422162bccce6f333 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 15:34:22 +0100 Subject: [PATCH 06/16] Code simplification and refactor --- .../support/he/model/VideoDownloadState.kt | 2 +- .../he/ui/AttachmentFullscreenVideoPlayer.kt | 10 ++-- .../he/ui/HEConversationDetailScreen.kt | 13 ++---- .../support/he/ui/HESupportActivity.kt | 10 ++-- .../support/he/ui/HESupportViewModel.kt | 46 +++++-------------- .../support/he/util/TempAttachmentsUtil.kt | 35 ++++++++++++++ 6 files changed, 60 insertions(+), 56 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt b/WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt index d854a9bb7744..5705494a753c 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/model/VideoDownloadState.kt @@ -5,6 +5,6 @@ import java.io.File sealed class VideoDownloadState { object Idle : VideoDownloadState() object Downloading : VideoDownloadState() + object Error : VideoDownloadState() data class Success(val file: File) : VideoDownloadState() - data class Error(val message: String) : VideoDownloadState() } diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt index d0ca120bf781..4a5de056f4b2 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt @@ -51,9 +51,8 @@ fun AttachmentFullscreenVideoPlayer( videoUrl: String, onDismiss: () -> Unit, onDownload: () -> Unit = {}, - onGetAuthorizationHeaderArgument: () -> String, downloadState: VideoDownloadState, - onStartVideoDownload: (String, String) -> Unit, + onStartVideoDownload: (String) -> Unit, onResetVideoDownloadState: () -> Unit = {}, ) { val context = LocalContext.current @@ -61,7 +60,7 @@ fun AttachmentFullscreenVideoPlayer( // Start download when composable is first launched LaunchedEffect(videoUrl) { - onStartVideoDownload(videoUrl, onGetAuthorizationHeaderArgument()) + onStartVideoDownload(videoUrl) } // Update local file when download succeeds @@ -194,9 +193,8 @@ fun AttachmentFullscreenVideoPlayer( // Download button IconButton( onClick = { - exoPlayer?.stop() - onDownload.invoke() - onDismiss.invoke() + onDownload() + closeFullScreen() } ) { Icon( diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt index 86904f60214f..682839afe8d1 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt @@ -90,10 +90,9 @@ fun HEConversationDetailScreen( attachmentState: AttachmentState = AttachmentState(), attachmentActionsListener: AttachmentActionsListener, onDownloadAttachment: (SupportAttachment) -> Unit = {}, - videoUrlResolver: org.wordpress.android.support.he.util.VideoUrlResolver? = null, onGetAuthorizationHeaderArgument: () -> String, videoDownloadState: VideoDownloadState, - onStartVideoDownload: (String, String) -> Unit, + onStartVideoDownload: (String) -> Unit, onResetVideoDownloadState: () -> Unit = {}, ) { val listState = rememberLazyListState() @@ -255,7 +254,6 @@ fun HEConversationDetailScreen( AttachmentType.Video -> { AttachmentFullscreenVideoPlayer( videoUrl = attachment.url, - onGetAuthorizationHeaderArgument = onGetAuthorizationHeaderArgument, downloadState = videoDownloadState, onStartVideoDownload = onStartVideoDownload, onResetVideoDownloadState = onResetVideoDownloadState, @@ -265,7 +263,6 @@ fun HEConversationDetailScreen( onDownload = { onDownloadAttachment(attachment) }, - videoUrlResolver = videoUrlResolver, ) } else -> { @@ -612,7 +609,7 @@ private fun HEConversationDetailScreenPreview() { }, onGetAuthorizationHeaderArgument = { "" }, videoDownloadState = VideoDownloadState.Idle, - onStartVideoDownload = { _, _ -> }, + onStartVideoDownload = { _ -> }, ) } } @@ -639,7 +636,7 @@ private fun HEConversationDetailScreenPreviewDark() { }, onGetAuthorizationHeaderArgument = { "" }, videoDownloadState = VideoDownloadState.Idle, - onStartVideoDownload = { _, _ -> }, + onStartVideoDownload = { _ -> }, ) } } @@ -668,7 +665,7 @@ private fun HEConversationDetailScreenWordPressPreview() { }, onGetAuthorizationHeaderArgument = { "" }, videoDownloadState = VideoDownloadState.Idle, - onStartVideoDownload = { _, _ -> }, + onStartVideoDownload = { _ -> }, ) } } @@ -696,7 +693,7 @@ private fun HEConversationDetailScreenPreviewWordPressDark() { }, onGetAuthorizationHeaderArgument = { "" }, videoDownloadState = VideoDownloadState.Idle, - onStartVideoDownload = { _, _ -> }, + onStartVideoDownload = { _ -> }, ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt index a3c4e7850f42..8fc43a3c0a60 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportActivity.kt @@ -32,7 +32,6 @@ import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.support.common.ui.ConversationsSupportViewModel import org.wordpress.android.support.he.util.AttachmentActionsListener -import org.wordpress.android.support.he.util.VideoUrlResolver import org.wordpress.android.ui.photopicker.MediaPickerConstants import org.wordpress.android.ui.reader.ReaderFileDownloadManager import org.wordpress.android.ui.mediapicker.MediaPickerSetup @@ -44,7 +43,6 @@ import javax.inject.Inject class HESupportActivity : AppCompatActivity() { @Inject lateinit var fileDownloadManager: ReaderFileDownloadManager @Inject lateinit var appLogWrapper: AppLogWrapper - @Inject lateinit var videoUrlResolver: VideoUrlResolver @Inject lateinit var accountStore: AccountStore private val viewModel by viewModels() @@ -191,7 +189,6 @@ class HESupportActivity : AppCompatActivity() { isLoading = isLoadingConversation, isSendingMessage = isSendingMessage, messageSendResult = messageSendResult, - videoUrlResolver = videoUrlResolver, onBackClick = { viewModel.onBackClick() }, onSendMessage = { message, includeAppLogs -> viewModel.onAddMessageToConversation( @@ -215,10 +212,10 @@ class HESupportActivity : AppCompatActivity() { // Start download with proper filename fileDownloadManager.downloadFile(attachment.url, attachment.filename) }, - onGetAuthorizationHeaderArgument = { "$BEARER_TAG ${accountStore.accessToken}" }, + onGetAuthorizationHeaderArgument = { viewModel.getAuthorizationHeader() }, videoDownloadState = videoDownloadState, - onStartVideoDownload = { url, authHeader -> - viewModel.downloadVideoToTempFile(url, authHeader) + onStartVideoDownload = { url -> + viewModel.downloadVideoToTempFile(url) }, onResetVideoDownloadState = { viewModel.resetVideoDownloadState() } ) @@ -292,7 +289,6 @@ class HESupportActivity : AppCompatActivity() { companion object { const val AUTHORIZATION_TAG = "Authorization" - private const val BEARER_TAG = "Bearer" @JvmStatic fun createIntent(context: Context): Intent = Intent(context, HESupportActivity::class.java) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt index 8b19387cdfb8..0bbc95192514 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt @@ -24,8 +24,6 @@ import org.wordpress.android.support.he.util.TempAttachmentsUtil import org.wordpress.android.util.AppLog import org.wordpress.android.util.NetworkUtilsWrapper import java.io.File -import java.net.HttpURLConnection -import java.net.URL import javax.inject.Inject import javax.inject.Named @@ -41,6 +39,7 @@ class HESupportViewModel @Inject constructor( ) : ConversationsSupportViewModel(accountStore, appLogWrapper, networkUtilsWrapper) { companion object { const val MAX_TOTAL_SIZE_BYTES = 20L * 1024 * 1024 // 20MB total + private const val BEARER_TAG = "Bearer" } private val _isSendingMessage = MutableStateFlow(false) val isSendingMessage: StateFlow = _isSendingMessage.asStateFlow() @@ -291,7 +290,7 @@ class HESupportViewModel @Inject constructor( * Downloads a video to a temporary file with caching and state management. * Updates videoDownloadState as it progresses. */ - fun downloadVideoToTempFile(videoUrl: String, authHeader: String) { + fun downloadVideoToTempFile(videoUrl: String) { viewModelScope.launch(ioDispatcher) { try { _videoDownloadState.value = VideoDownloadState.Idle @@ -310,40 +309,17 @@ class HESupportViewModel @Inject constructor( // Start downloading _videoDownloadState.value = VideoDownloadState.Downloading AppLog.d(AppLog.T.SUPPORT, "Downloading video to temp file: $videoUrl") - - val tempFile = File.createTempFile("video_", ".mp4", application.cacheDir) - val connection = URL(videoUrl).openConnection() as HttpURLConnection - connection.requestMethod = "GET" - connection.setRequestProperty("Authorization", authHeader) - connection.instanceFollowRedirects = true - - try { - connection.connect() - - val responseCode = connection.responseCode - AppLog.d(AppLog.T.SUPPORT, "Download response code: $responseCode") - - if (responseCode == HttpURLConnection.HTTP_OK) { - connection.inputStream.use { input -> - tempFile.outputStream().use { output -> - input.copyTo(output) - } - } - // Cache the downloaded file - videoCache[videoUrl] = tempFile - AppLog.d(AppLog.T.SUPPORT, "Video downloaded and cached: ${tempFile.absolutePath}") - _videoDownloadState.value = VideoDownloadState.Success(tempFile) - } else { - val errorMsg = "Failed to download video. Response code: $responseCode" - AppLog.e(AppLog.T.SUPPORT, errorMsg) - _videoDownloadState.value = VideoDownloadState.Error(errorMsg) - } - } finally { - connection.disconnect() + val tempFile = tempAttachmentsUtil.createVideoTempFile(videoUrl, getAuthorizationHeader()) + if (tempFile == null) { + _videoDownloadState.value = VideoDownloadState.Error + } else { + // Cache the downloaded file + videoCache[videoUrl] = tempFile + _videoDownloadState.value = VideoDownloadState.Success(tempFile) } } catch (e: Exception) { AppLog.e(AppLog.T.SUPPORT, "Error downloading video", e) - _videoDownloadState.value = VideoDownloadState.Error(e.message ?: "Unknown error") + _videoDownloadState.value = VideoDownloadState.Error } } } @@ -368,4 +344,6 @@ class HESupportViewModel @Inject constructor( } videoCache.clear() } + + fun getAuthorizationHeader():String = "$BEARER_TAG ${accountStore.accessToken}" } diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt index 9278d1d89412..2ebdd44c0dba 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt @@ -6,11 +6,15 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.modules.IO_THREAD +import org.wordpress.android.support.he.model.VideoDownloadState import org.wordpress.android.util.AppLog import java.io.File +import java.net.HttpURLConnection +import java.net.URL import javax.inject.Inject import javax.inject.Named import kotlin.collections.forEach +import kotlin.collections.set class TempAttachmentsUtil @Inject constructor( @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher, @@ -84,4 +88,35 @@ class TempAttachmentsUtil @Inject constructor( // Default to jpg if we can't determine the extension return "jpg" } + + suspend fun createVideoTempFile(videoUrl: String, authHeader: String): File? = withContext(ioDispatcher) { + val tempFile = File.createTempFile("video_", ".mp4", application.cacheDir) + val connection = URL(videoUrl).openConnection() as HttpURLConnection + connection.requestMethod = "GET" + connection.setRequestProperty("Authorization", authHeader) + connection.instanceFollowRedirects = true + + try { + connection.connect() + + val responseCode = connection.responseCode + AppLog.d(AppLog.T.SUPPORT, "Download response code: $responseCode") + + if (responseCode == HttpURLConnection.HTTP_OK) { + connection.inputStream.use { input -> + tempFile.outputStream().use { output -> + input.copyTo(output) + } + } + + AppLog.d(AppLog.T.SUPPORT, "Video downloaded: ${tempFile.absolutePath}") + tempFile + } else { + AppLog.e(AppLog.T.SUPPORT, "Failed to download video. Response code: $responseCode") + null + } + } finally { + connection.disconnect() + } + } } From cedc5889a2da610fb51197c0efe12a203ffd2113 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 15:44:12 +0100 Subject: [PATCH 07/16] Adding new tests --- .../support/he/ui/HESupportViewModelTest.kt | 284 ++++++++++++++++++ 1 file changed, 284 insertions(+) diff --git a/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt index 4e5b3a743f69..af0beb59688e 100644 --- a/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt @@ -958,6 +958,290 @@ class HESupportViewModelTest : BaseUnitTest() { // endregion + // region Video download tests + + @Test + fun `downloadVideoToTempFile sets state to Downloading when starting download`() = test { + val videoUrl = "https://example.com/video.mp4" + val tempFile = java.io.File.createTempFile("test_video", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile) + + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + // State should transition through Idle -> Downloading -> Success + assertThat(viewModel.videoDownloadState.value) + .isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Success::class.java) + + tempFile.delete() + } + + @Test + fun `downloadVideoToTempFile sets state to Success with file when download succeeds`() = test { + val videoUrl = "https://example.com/video.mp4" + val tempFile = java.io.File.createTempFile("test_video", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile) + + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + val state = viewModel.videoDownloadState.value + assertThat(state).isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Success::class.java) + assertThat((state as org.wordpress.android.support.he.model.VideoDownloadState.Success).file) + .isEqualTo(tempFile) + + tempFile.delete() + } + + @Test + fun `downloadVideoToTempFile sets state to Error when download fails`() = test { + val videoUrl = "https://example.com/video.mp4" + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(null) + + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + assertThat(viewModel.videoDownloadState.value) + .isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Error::class.java) + } + + @Test + fun `downloadVideoToTempFile sets state to Error when exception occurs`() = test { + val videoUrl = "https://example.com/video.mp4" + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenThrow(RuntimeException("Network error")) + + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + assertThat(viewModel.videoDownloadState.value) + .isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Error::class.java) + } + + @Test + fun `downloadVideoToTempFile returns cached file when available`() = test { + val videoUrl = "https://example.com/video.mp4" + val tempFile = java.io.File.createTempFile("test_video", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile) + + // First download + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + // Second download should use cache + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + // Should only call createVideoTempFile once + verify(tempAttachmentsUtil, org.mockito.kotlin.times(1)) + .createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader()) + + val state = viewModel.videoDownloadState.value + assertThat(state).isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Success::class.java) + assertThat((state as org.wordpress.android.support.he.model.VideoDownloadState.Success).file) + .isEqualTo(tempFile) + + tempFile.delete() + } + + @Test + fun `downloadVideoToTempFile removes deleted cached file and re-downloads`() = test { + val videoUrl = "https://example.com/video.mp4" + val tempFile1 = java.io.File.createTempFile("test_video1", ".mp4") + val tempFile2 = java.io.File.createTempFile("test_video2", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile1) + .thenReturn(tempFile2) + + // First download + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + // Delete the cached file + tempFile1.delete() + + // Second download should detect deleted file and re-download + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + // Should call createVideoTempFile twice + verify(tempAttachmentsUtil, org.mockito.kotlin.times(2)) + .createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader()) + + val state = viewModel.videoDownloadState.value + assertThat(state).isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Success::class.java) + assertThat((state as org.wordpress.android.support.he.model.VideoDownloadState.Success).file) + .isEqualTo(tempFile2) + + tempFile2.delete() + } + + @Test + fun `downloadVideoToTempFile caches multiple different videos`() = test { + val videoUrl1 = "https://example.com/video1.mp4" + val videoUrl2 = "https://example.com/video2.mp4" + val tempFile1 = java.io.File.createTempFile("test_video1", ".mp4") + val tempFile2 = java.io.File.createTempFile("test_video2", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl1, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile1) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl2, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile2) + + // Download first video + viewModel.downloadVideoToTempFile(videoUrl1) + advanceUntilIdle() + + // Download second video + viewModel.downloadVideoToTempFile(videoUrl2) + advanceUntilIdle() + + // Download first video again - should use cache + viewModel.downloadVideoToTempFile(videoUrl1) + advanceUntilIdle() + + // Should only call createVideoTempFile once per unique URL + verify(tempAttachmentsUtil, org.mockito.kotlin.times(1)) + .createVideoTempFile(videoUrl1, viewModel.getAuthorizationHeader()) + verify(tempAttachmentsUtil, org.mockito.kotlin.times(1)) + .createVideoTempFile(videoUrl2, viewModel.getAuthorizationHeader()) + + tempFile1.delete() + tempFile2.delete() + } + + @Test + fun `resetVideoDownloadState sets state to Idle`() = test { + val videoUrl = "https://example.com/video.mp4" + val tempFile = java.io.File.createTempFile("test_video", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile) + + // Download video to set state to Success + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + assertThat(viewModel.videoDownloadState.value) + .isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Success::class.java) + + // Reset state + viewModel.resetVideoDownloadState() + + assertThat(viewModel.videoDownloadState.value) + .isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Idle::class.java) + + tempFile.delete() + } + + @Test + fun `cleanupVideoCache deletes all cached video files`() = test { + val videoUrl1 = "https://example.com/video1.mp4" + val videoUrl2 = "https://example.com/video2.mp4" + val tempFile1 = java.io.File.createTempFile("test_video1", ".mp4") + val tempFile2 = java.io.File.createTempFile("test_video2", ".mp4") + + // Create actual temp files that exist + tempFile1.writeText("test content 1") + tempFile2.writeText("test content 2") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl1, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile1) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl2, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile2) + + // Download both videos to cache them + viewModel.downloadVideoToTempFile(videoUrl1) + advanceUntilIdle() + viewModel.downloadVideoToTempFile(videoUrl2) + advanceUntilIdle() + + assertThat(tempFile1.exists()).isTrue + assertThat(tempFile2.exists()).isTrue + + // Cleanup cache + viewModel.cleanupVideoCache() + + assertThat(tempFile1.exists()).isFalse + assertThat(tempFile2.exists()).isFalse + } + + @Test + fun `cleanupVideoCache clears cache map`() = test { + val videoUrl = "https://example.com/video.mp4" + val tempFile = java.io.File.createTempFile("test_video", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile) + + // Download video to cache it + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + // Cleanup cache + viewModel.cleanupVideoCache() + + // Try to download again - should call createVideoTempFile again (not use cache) + val tempFile2 = java.io.File.createTempFile("test_video2", ".mp4") + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile2) + + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + verify(tempAttachmentsUtil, org.mockito.kotlin.times(2)) + .createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader()) + + tempFile2.delete() + } + + @Test + fun `cleanupVideoCache handles non-existent files gracefully`() = test { + val videoUrl = "https://example.com/video.mp4" + val tempFile = java.io.File.createTempFile("test_video", ".mp4") + + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + .thenReturn(tempFile) + + // Download video to cache it + viewModel.downloadVideoToTempFile(videoUrl) + advanceUntilIdle() + + // Manually delete the file before cleanup + tempFile.delete() + assertThat(tempFile.exists()).isFalse + + // Cleanup should not throw exception + viewModel.cleanupVideoCache() + } + + @Test + fun `getAuthorizationHeader returns Bearer token format`() { + val expectedHeader = "Bearer $testAccessToken" + + val actualHeader = viewModel.getAuthorizationHeader() + + assertThat(actualHeader).isEqualTo(expectedHeader) + } + + @Test + fun `videoDownloadState is Idle initially`() { + assertThat(viewModel.videoDownloadState.value) + .isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Idle::class.java) + } + + // endregion + // Helper functions private fun createTestConversation( id: Long, From d5c7cb2968abf41b0e82e501f6b7fc9a0667de32 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 15:46:23 +0100 Subject: [PATCH 08/16] Detekt --- .../android/support/he/ui/AttachmentFullscreenImagePreview.kt | 1 - .../android/support/he/ui/HEConversationDetailScreen.kt | 1 - .../org/wordpress/android/support/he/ui/HESupportViewModel.kt | 1 + .../wordpress/android/support/he/util/TempAttachmentsUtil.kt | 2 -- 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt index 22be332f2d79..b8928b3ce1bd 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt @@ -40,7 +40,6 @@ import androidx.compose.ui.window.DialogProperties import coil.compose.SubcomposeAsyncImage import coil.request.ImageRequest import org.wordpress.android.R -import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.support.he.ui.HESupportActivity.Companion.AUTHORIZATION_TAG import org.wordpress.android.ui.compose.theme.AppThemeM3 diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt index 682839afe8d1..0d624d3eb34e 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt @@ -59,7 +59,6 @@ import coil.decode.VideoFrameDecoder import coil.request.ImageRequest import coil.request.videoFrameMillis import org.wordpress.android.R -import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.support.aibot.util.formatRelativeTime import org.wordpress.android.support.he.model.AttachmentState import org.wordpress.android.support.he.model.ConversationStatus diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt index 0bbc95192514..5f8f9b923f5b 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt @@ -290,6 +290,7 @@ class HESupportViewModel @Inject constructor( * Downloads a video to a temporary file with caching and state management. * Updates videoDownloadState as it progresses. */ + @Suppress("TooGenericExceptionCaught") fun downloadVideoToTempFile(videoUrl: String) { viewModelScope.launch(ioDispatcher) { try { diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt index 2ebdd44c0dba..d55e05eb91be 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt @@ -6,7 +6,6 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.modules.IO_THREAD -import org.wordpress.android.support.he.model.VideoDownloadState import org.wordpress.android.util.AppLog import java.io.File import java.net.HttpURLConnection @@ -14,7 +13,6 @@ import java.net.URL import javax.inject.Inject import javax.inject.Named import kotlin.collections.forEach -import kotlin.collections.set class TempAttachmentsUtil @Inject constructor( @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher, From 47f8c5b68505b4b4e72b4367443b59198ddbf4cd Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 16:14:26 +0100 Subject: [PATCH 09/16] Properly handling video download errors --- .../support/he/util/TempAttachmentsUtil.kt | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt index d55e05eb91be..d60193daa7b6 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt @@ -87,14 +87,21 @@ class TempAttachmentsUtil @Inject constructor( return "jpg" } + @Suppress("TooGenericExceptionCaught") suspend fun createVideoTempFile(videoUrl: String, authHeader: String): File? = withContext(ioDispatcher) { - val tempFile = File.createTempFile("video_", ".mp4", application.cacheDir) - val connection = URL(videoUrl).openConnection() as HttpURLConnection - connection.requestMethod = "GET" - connection.setRequestProperty("Authorization", authHeader) - connection.instanceFollowRedirects = true + var tempFile: File? = null + var connection: HttpURLConnection? = null try { + tempFile = File.createTempFile("video_", ".mp4", application.cacheDir) + connection = (URL(videoUrl).openConnection() as HttpURLConnection).apply { + requestMethod = "GET" + setRequestProperty("Authorization", authHeader) + instanceFollowRedirects = true + connectTimeout = 30_000 // 30 seconds + readTimeout = 60_000 // 60 seconds + } + connection.connect() val responseCode = connection.responseCode @@ -111,10 +118,15 @@ class TempAttachmentsUtil @Inject constructor( tempFile } else { AppLog.e(AppLog.T.SUPPORT, "Failed to download video. Response code: $responseCode") + tempFile?.delete() null } + } catch (e: Exception) { + AppLog.e(AppLog.T.SUPPORT, "Error downloading video: ${e.message}") + tempFile?.delete() + null } finally { - connection.disconnect() + connection?.disconnect() } } } From f5d8c717e2b007a08a5b45a5085ed28beda794f2 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 16:20:30 +0100 Subject: [PATCH 10/16] Removing authHeader from function arguments --- .../support/he/ui/HESupportViewModel.kt | 2 +- .../support/he/util/TempAttachmentsUtil.kt | 6 ++- .../support/he/ui/HESupportViewModelTest.kt | 38 +++++++++---------- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt index 5f8f9b923f5b..1703cbeece4f 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt @@ -310,7 +310,7 @@ class HESupportViewModel @Inject constructor( // Start downloading _videoDownloadState.value = VideoDownloadState.Downloading AppLog.d(AppLog.T.SUPPORT, "Downloading video to temp file: $videoUrl") - val tempFile = tempAttachmentsUtil.createVideoTempFile(videoUrl, getAuthorizationHeader()) + val tempFile = tempAttachmentsUtil.createVideoTempFile(videoUrl) if (tempFile == null) { _videoDownloadState.value = VideoDownloadState.Error } else { diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt index d60193daa7b6..ef63d1ca2ba2 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt @@ -4,6 +4,7 @@ import android.app.Application import android.net.Uri import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext +import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.modules.IO_THREAD import org.wordpress.android.util.AppLog @@ -18,6 +19,7 @@ class TempAttachmentsUtil @Inject constructor( @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher, private val appLogWrapper: AppLogWrapper, private val application: Application, + private val accountStore: AccountStore ) { @Suppress("TooGenericExceptionCaught") suspend fun createTempFilesFrom(uris: List): List = withContext(ioDispatcher) { @@ -88,7 +90,7 @@ class TempAttachmentsUtil @Inject constructor( } @Suppress("TooGenericExceptionCaught") - suspend fun createVideoTempFile(videoUrl: String, authHeader: String): File? = withContext(ioDispatcher) { + suspend fun createVideoTempFile(videoUrl: String): File? = withContext(ioDispatcher) { var tempFile: File? = null var connection: HttpURLConnection? = null @@ -96,7 +98,7 @@ class TempAttachmentsUtil @Inject constructor( tempFile = File.createTempFile("video_", ".mp4", application.cacheDir) connection = (URL(videoUrl).openConnection() as HttpURLConnection).apply { requestMethod = "GET" - setRequestProperty("Authorization", authHeader) + setRequestProperty("Authorization", "Bearer ${accountStore.accessToken}") instanceFollowRedirects = true connectTimeout = 30_000 // 30 seconds readTimeout = 60_000 // 60 seconds diff --git a/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt index af0beb59688e..b076c085b95f 100644 --- a/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/support/he/ui/HESupportViewModelTest.kt @@ -965,7 +965,7 @@ class HESupportViewModelTest : BaseUnitTest() { val videoUrl = "https://example.com/video.mp4" val tempFile = java.io.File.createTempFile("test_video", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile) viewModel.downloadVideoToTempFile(videoUrl) @@ -983,7 +983,7 @@ class HESupportViewModelTest : BaseUnitTest() { val videoUrl = "https://example.com/video.mp4" val tempFile = java.io.File.createTempFile("test_video", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile) viewModel.downloadVideoToTempFile(videoUrl) @@ -1001,7 +1001,7 @@ class HESupportViewModelTest : BaseUnitTest() { fun `downloadVideoToTempFile sets state to Error when download fails`() = test { val videoUrl = "https://example.com/video.mp4" - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(null) viewModel.downloadVideoToTempFile(videoUrl) @@ -1015,7 +1015,7 @@ class HESupportViewModelTest : BaseUnitTest() { fun `downloadVideoToTempFile sets state to Error when exception occurs`() = test { val videoUrl = "https://example.com/video.mp4" - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenThrow(RuntimeException("Network error")) viewModel.downloadVideoToTempFile(videoUrl) @@ -1030,7 +1030,7 @@ class HESupportViewModelTest : BaseUnitTest() { val videoUrl = "https://example.com/video.mp4" val tempFile = java.io.File.createTempFile("test_video", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile) // First download @@ -1043,7 +1043,7 @@ class HESupportViewModelTest : BaseUnitTest() { // Should only call createVideoTempFile once verify(tempAttachmentsUtil, org.mockito.kotlin.times(1)) - .createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader()) + .createVideoTempFile(videoUrl) val state = viewModel.videoDownloadState.value assertThat(state).isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Success::class.java) @@ -1059,7 +1059,7 @@ class HESupportViewModelTest : BaseUnitTest() { val tempFile1 = java.io.File.createTempFile("test_video1", ".mp4") val tempFile2 = java.io.File.createTempFile("test_video2", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile1) .thenReturn(tempFile2) @@ -1076,7 +1076,7 @@ class HESupportViewModelTest : BaseUnitTest() { // Should call createVideoTempFile twice verify(tempAttachmentsUtil, org.mockito.kotlin.times(2)) - .createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader()) + .createVideoTempFile(videoUrl) val state = viewModel.videoDownloadState.value assertThat(state).isInstanceOf(org.wordpress.android.support.he.model.VideoDownloadState.Success::class.java) @@ -1093,9 +1093,9 @@ class HESupportViewModelTest : BaseUnitTest() { val tempFile1 = java.io.File.createTempFile("test_video1", ".mp4") val tempFile2 = java.io.File.createTempFile("test_video2", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl1, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl1)) .thenReturn(tempFile1) - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl2, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl2)) .thenReturn(tempFile2) // Download first video @@ -1112,9 +1112,9 @@ class HESupportViewModelTest : BaseUnitTest() { // Should only call createVideoTempFile once per unique URL verify(tempAttachmentsUtil, org.mockito.kotlin.times(1)) - .createVideoTempFile(videoUrl1, viewModel.getAuthorizationHeader()) + .createVideoTempFile(videoUrl1) verify(tempAttachmentsUtil, org.mockito.kotlin.times(1)) - .createVideoTempFile(videoUrl2, viewModel.getAuthorizationHeader()) + .createVideoTempFile(videoUrl2) tempFile1.delete() tempFile2.delete() @@ -1125,7 +1125,7 @@ class HESupportViewModelTest : BaseUnitTest() { val videoUrl = "https://example.com/video.mp4" val tempFile = java.io.File.createTempFile("test_video", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile) // Download video to set state to Success @@ -1155,9 +1155,9 @@ class HESupportViewModelTest : BaseUnitTest() { tempFile1.writeText("test content 1") tempFile2.writeText("test content 2") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl1, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl1)) .thenReturn(tempFile1) - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl2, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl2)) .thenReturn(tempFile2) // Download both videos to cache them @@ -1181,7 +1181,7 @@ class HESupportViewModelTest : BaseUnitTest() { val videoUrl = "https://example.com/video.mp4" val tempFile = java.io.File.createTempFile("test_video", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile) // Download video to cache it @@ -1193,14 +1193,14 @@ class HESupportViewModelTest : BaseUnitTest() { // Try to download again - should call createVideoTempFile again (not use cache) val tempFile2 = java.io.File.createTempFile("test_video2", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile2) viewModel.downloadVideoToTempFile(videoUrl) advanceUntilIdle() verify(tempAttachmentsUtil, org.mockito.kotlin.times(2)) - .createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader()) + .createVideoTempFile(videoUrl) tempFile2.delete() } @@ -1210,7 +1210,7 @@ class HESupportViewModelTest : BaseUnitTest() { val videoUrl = "https://example.com/video.mp4" val tempFile = java.io.File.createTempFile("test_video", ".mp4") - whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl, viewModel.getAuthorizationHeader())) + whenever(tempAttachmentsUtil.createVideoTempFile(videoUrl)) .thenReturn(tempFile) // Download video to cache it From 5e3d69768f4d28804770385190fe248d9f302d41 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 16:29:56 +0100 Subject: [PATCH 11/16] Caching filepaths instead of Files --- .../support/he/ui/HESupportViewModel.kt | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt index 1703cbeece4f..a44f610d7a04 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HESupportViewModel.kt @@ -51,8 +51,9 @@ class HESupportViewModel @Inject constructor( private val _attachmentState = MutableStateFlow(AttachmentState()) val attachmentState: StateFlow = _attachmentState.asStateFlow() - // Cache for downloaded video files (videoUrl -> tempFile) - private val videoCache = mutableMapOf() + // Cache for downloaded video file paths (videoUrl -> file path) + // Stores paths instead of File objects to minimize memory footprint + private val videoCache = mutableMapOf() // Video download state private val _videoDownloadState = MutableStateFlow(VideoDownloadState.Idle) @@ -294,9 +295,9 @@ class HESupportViewModel @Inject constructor( fun downloadVideoToTempFile(videoUrl: String) { viewModelScope.launch(ioDispatcher) { try { - _videoDownloadState.value = VideoDownloadState.Idle - // Check cache first - videoCache[videoUrl]?.let { cachedFile -> + // Check cache first (before setting state to avoid unnecessary state changes) + videoCache[videoUrl]?.let { cachedFilePath -> + val cachedFile = File(cachedFilePath) if (cachedFile.exists()) { AppLog.d(AppLog.T.SUPPORT, "Using cached video file for: $videoUrl") _videoDownloadState.value = VideoDownloadState.Success(cachedFile) @@ -314,8 +315,8 @@ class HESupportViewModel @Inject constructor( if (tempFile == null) { _videoDownloadState.value = VideoDownloadState.Error } else { - // Cache the downloaded file - videoCache[videoUrl] = tempFile + // Cache the downloaded file path + videoCache[videoUrl] = tempFile.absolutePath _videoDownloadState.value = VideoDownloadState.Success(tempFile) } } catch (e: Exception) { @@ -337,9 +338,10 @@ class HESupportViewModel @Inject constructor( */ fun cleanupVideoCache() { AppLog.d(AppLog.T.SUPPORT, "Cleaning up ${videoCache.size} cached video files") - videoCache.values.forEach { file -> + videoCache.values.forEach { filePath -> + val file = File(filePath) if (file.exists()) { - AppLog.d(AppLog.T.SUPPORT, "Deleting temp video file: ${file.absolutePath}") + AppLog.d(AppLog.T.SUPPORT, "Deleting temp video file: $filePath") file.delete() } } @@ -347,4 +349,13 @@ class HESupportViewModel @Inject constructor( } fun getAuthorizationHeader():String = "$BEARER_TAG ${accountStore.accessToken}" + + /** + * Called when the ViewModel is destroyed. Ensures video cache cleanup even if Activity + * onDestroy() is not called (e.g., process death). + */ + override fun onCleared() { + super.onCleared() + cleanupVideoCache() + } } From 79b3d14d887a86771801c49cdcadedcd83cd8daa Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 16:32:31 +0100 Subject: [PATCH 12/16] Skipping multiple authHeader calls on recomposition --- .../android/support/he/ui/HEConversationDetailScreen.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt index 0d624d3eb34e..3e0a551df042 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/HEConversationDetailScreen.kt @@ -470,6 +470,9 @@ private fun AttachmentItem( onClick: () -> Unit, onGetAuthorizationHeaderArgument: () -> String, ) { + // Cache authorization header to avoid repeated function calls during composition + val authorizationHeader = remember { onGetAuthorizationHeaderArgument() } + val iconRes = when (attachment.type) { AttachmentType.Image -> R.drawable.ic_image_white_24dp AttachmentType.Video -> R.drawable.ic_video_camera_white_24dp @@ -500,7 +503,7 @@ private fun AttachmentItem( } } .apply { - addHeader(AUTHORIZATION_TAG, onGetAuthorizationHeaderArgument.invoke()) + addHeader(AUTHORIZATION_TAG, authorizationHeader) } .build(), contentDescription = attachment.filename, From dd5d520437f5333a67f11897f79828273879398c Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 16:48:51 +0100 Subject: [PATCH 13/16] Avoid creating function multiple times in composition --- .../he/ui/AttachmentFullscreenVideoPlayer.kt | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt index 4a5de056f4b2..1718741eeeca 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenVideoPlayer.kt @@ -91,16 +91,9 @@ fun AttachmentFullscreenVideoPlayer( } } - fun closeFullScreen() { - exoPlayer?.stop() - exoPlayer?.release() - onDismiss() - onResetVideoDownloadState() - } - Dialog( onDismissRequest = { - closeFullScreen() + closeFullScreen(exoPlayer, onDismiss, onResetVideoDownloadState) }, properties = DialogProperties( usePlatformDefaultWidth = false, @@ -151,7 +144,7 @@ fun AttachmentFullscreenVideoPlayer( Button( onClick = { onDownload() - closeFullScreen() + closeFullScreen(exoPlayer, onDismiss, onResetVideoDownloadState) } ) { Text(stringResource(R.string.he_support_download_video_button)) @@ -194,7 +187,7 @@ fun AttachmentFullscreenVideoPlayer( IconButton( onClick = { onDownload() - closeFullScreen() + closeFullScreen(exoPlayer, onDismiss, onResetVideoDownloadState) } ) { Icon( @@ -208,7 +201,7 @@ fun AttachmentFullscreenVideoPlayer( // Close button IconButton( onClick = { - closeFullScreen() + closeFullScreen(exoPlayer, onDismiss, onResetVideoDownloadState) } ) { Icon( @@ -222,3 +215,14 @@ fun AttachmentFullscreenVideoPlayer( } } } + +fun closeFullScreen( + exoPlayer: SimpleExoPlayer?, + onDismiss: () -> Unit, + onResetVideoDownloadState: () -> Unit, +) { + exoPlayer?.stop() + exoPlayer?.release() + onDismiss() + onResetVideoDownloadState() +} From db7d8f1b5ecc1d3172d9fdeb5a1510f16eed9379 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 16:55:00 +0100 Subject: [PATCH 14/16] detekt --- .../android/support/he/util/TempAttachmentsUtil.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt index ef63d1ca2ba2..e90a59734ffb 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt @@ -21,6 +21,11 @@ class TempAttachmentsUtil @Inject constructor( private val application: Application, private val accountStore: AccountStore ) { + companion object { + private const val CONNECTION_TIMEOUT_MS = 30_000 // 30 seconds + private const val READ_TIMEOUT_MS = 60_000 // 60 seconds + } + @Suppress("TooGenericExceptionCaught") suspend fun createTempFilesFrom(uris: List): List = withContext(ioDispatcher) { uris.map{ it.toTempFile() } @@ -100,8 +105,8 @@ class TempAttachmentsUtil @Inject constructor( requestMethod = "GET" setRequestProperty("Authorization", "Bearer ${accountStore.accessToken}") instanceFollowRedirects = true - connectTimeout = 30_000 // 30 seconds - readTimeout = 60_000 // 60 seconds + connectTimeout = CONNECTION_TIMEOUT_MS + readTimeout = READ_TIMEOUT_MS } connection.connect() From cfec6f2a1048a81bc75d05331dc17370caf4bc8b Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 17:02:52 +0100 Subject: [PATCH 15/16] Fixing sonarqube warning --- .../android/support/he/util/TempAttachmentsUtil.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt index e90a59734ffb..454fdf66513f 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/util/TempAttachmentsUtil.kt @@ -124,13 +124,14 @@ class TempAttachmentsUtil @Inject constructor( AppLog.d(AppLog.T.SUPPORT, "Video downloaded: ${tempFile.absolutePath}") tempFile } else { - AppLog.e(AppLog.T.SUPPORT, "Failed to download video. Response code: $responseCode") - tempFile?.delete() + val deleted = tempFile?.delete() + AppLog.e(AppLog.T.SUPPORT, "Failed to download video. Deleted: ${deleted} - " + + "Response code: $responseCode") null } } catch (e: Exception) { - AppLog.e(AppLog.T.SUPPORT, "Error downloading video: ${e.message}") - tempFile?.delete() + val deleted = tempFile?.delete() + AppLog.e(AppLog.T.SUPPORT, "Error downloading video: ${e.message} Deleted: ${deleted}") null } finally { connection?.disconnect() From 5f73fa6a9a36946f769b2e8cdc0689475effa764 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 19 Nov 2025 17:30:48 +0100 Subject: [PATCH 16/16] Reduce header recomposition in imagepreview --- .../support/he/ui/AttachmentFullscreenImagePreview.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt index b8928b3ce1bd..87df10671c6b 100644 --- a/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt +++ b/WordPress/src/main/java/org/wordpress/android/support/he/ui/AttachmentFullscreenImagePreview.kt @@ -54,6 +54,9 @@ fun AttachmentFullscreenImagePreview( var offsetX by remember { mutableFloatStateOf(0f) } var offsetY by remember { mutableFloatStateOf(0f) } + // Cache authorization header to avoid repeated function calls during composition + val authorizationHeader = remember { onGetAuthorizationHeaderArgument() } + // Load semantics val loadingImageDescription = stringResource(R.string.he_support_loading_image) val attachmentImageDescription = stringResource(R.string.he_support_attachment_image) @@ -93,7 +96,7 @@ fun AttachmentFullscreenImagePreview( .data(imageUrl) .crossfade(true) .apply { - addHeader(AUTHORIZATION_TAG, onGetAuthorizationHeaderArgument.invoke()) + addHeader(AUTHORIZATION_TAG, authorizationHeader) } .build(), contentDescription = attachmentImageDescription,