From 90e142d8a06af3e9dc145936a69ad5945c559036 Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 28 Aug 2026 18:42:29 +0200 Subject: [PATCH 01/12] feat: in-app pdf viewer --- app/build.gradle.kts | 1 + .../di/metro/AppSessionViewModelGraph.kt | 2 + .../routes/media/MediaNavigation3Entries.kt | 9 + .../navigation/routes/media/MediaRoutes.kt | 12 + .../runtime/WireNavigation3Contributions.kt | 2 +- .../WireNavigation3ProductionActions.kt | 11 + .../ConversationMessageComposer.kt | 2 + .../ConversationNavigation3Entries.kt | 9 + .../conversations/ConversationRouteScreen.kt | 2 + .../home/conversations/ConversationScreen.kt | 4 + .../messages/item/MessageClickActions.kt | 2 + .../messages/item/MessageContentAndStatus.kt | 4 + .../messages/item/MessageContentItem.kt | 1 + .../multipart/MultipartAttachmentsView.kt | 40 +- .../MultipartAttachmentsViewModel.kt | 8 + .../WireNavigation3ContributionsTest.kt | 2 +- .../MultipartAttachmentsViewModelTest.kt | 89 ++++- core/pdf-viewer/build.gradle.kts | 58 +++ .../com/wire/android/pdfviewer/PdfDocument.kt | 105 +++++ .../android/pdfviewer/PdfSourceResolver.kt | 106 +++++ .../com/wire/android/pdfviewer/PdfViewer.kt | 376 ++++++++++++++++++ .../wire/android/pdfviewer/PdfViewerState.kt | 48 +++ .../android/pdfviewer/PdfViewerViewModel.kt | 159 ++++++++ .../pdfviewer/PdfViewerViewModelGraph.kt | 39 ++ .../src/main/res/values/strings.xml | 11 + .../pdfviewer/PdfSourceResolverTest.kt | 96 +++++ crowdin.yml | 4 + features/cells/build.gradle.kts | 1 + .../cells/navigation/CellsNavigation3.kt | 12 + .../navigation/CellsNavigation3Entries.kt | 5 +- .../navigation/CellsNavigation3Renderer.kt | 15 + .../cells/ui/AllFilesNavigationActions.kt | 1 + .../feature/cells/ui/AllFilesScreen.kt | 1 + .../feature/cells/ui/CellScreenContent.kt | 2 + .../android/feature/cells/ui/CellViewModel.kt | 51 +-- .../feature/cells/ui/CellsFilesNavigation.kt | 2 + .../cells/ui/ConversationFilesScreen.kt | 1 + .../feature/cells/ui/search/SearchScreen.kt | 1 + .../cells/navigation/CellsNavigation3Test.kt | 1 + .../feature/cells/ui/CellViewModelTest.kt | 42 +- 40 files changed, 1266 insertions(+), 71 deletions(-) create mode 100644 core/pdf-viewer/build.gradle.kts create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerState.kt create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt create mode 100644 core/pdf-viewer/src/main/res/values/strings.xml create mode 100644 core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index faab97578ab..5bfb41bd28b 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -213,6 +213,7 @@ dependencies { implementationWithCoverage(projects.core.di) implementationWithCoverage(projects.core.media) implementationWithCoverage(projects.core.mediaPlayer) + implementationWithCoverage(projects.core.pdfViewer) implementationWithCoverage(projects.core.notification) implementationWithCoverage(projects.core.navigation) implementationWithCoverage(projects.core.search) diff --git a/app/src/main/kotlin/com/wire/android/di/metro/AppSessionViewModelGraph.kt b/app/src/main/kotlin/com/wire/android/di/metro/AppSessionViewModelGraph.kt index 22c93c6c327..6bb6fd3b28c 100644 --- a/app/src/main/kotlin/com/wire/android/di/metro/AppSessionViewModelGraph.kt +++ b/app/src/main/kotlin/com/wire/android/di/metro/AppSessionViewModelGraph.kt @@ -41,6 +41,7 @@ import com.wire.android.feature.meetings.ui.MeetingsManualViewModelFactoryMetroB import com.wire.android.feature.meetings.ui.MeetingsMetroViewModelBindings import com.wire.android.feature.sketch.SketchMetroViewModelBindings import com.wire.android.mediaplayer.MediaPlayerManualViewModelFactoryMetroBindings +import com.wire.android.pdfviewer.PdfViewerManualViewModelFactoryMetroBindings import com.wire.android.search.SearchManualViewModelFactoryMetroBindings import com.wire.android.ui.authentication.AuthenticationViewModelGraph import com.wire.android.ui.calling.CallingMetroViewModelBindings @@ -111,6 +112,7 @@ annotation class MetroSessionScope CoreUICommonManualViewModelFactoryMetroBindings::class, SearchManualViewModelFactoryMetroBindings::class, MediaPlayerManualViewModelFactoryMetroBindings::class, + PdfViewerManualViewModelFactoryMetroBindings::class, ImageLoadingModule::class, ] ) diff --git a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt index 8fe153c6165..7dc9cedec34 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt @@ -12,6 +12,7 @@ import com.wire.android.navigation.navigation3.WireNavigation3ResultType import com.wire.android.navigation.navigation3.WireNavigation3Runtime import com.wire.android.navigation.navigation3.wireEntry import com.wire.android.mediaplayer.VideoPlayer +import com.wire.android.pdfviewer.PdfViewer import com.wire.android.ui.home.FeatureFlagState import com.wire.android.ui.home.conversations.ConversationNavArgs import com.wire.android.ui.home.conversations.checkAssetRestrictionsViewModel @@ -122,6 +123,14 @@ internal fun mediaNavigation3Entries( onNavigateBack = runtime.navigator::goBack, ) } + wireEntry(presentation = WireEntryPresentation.PopUp) { route -> + PdfViewer( + localPath = route.localPath, + contentUrl = route.contentUrl, + fileName = route.fileName, + onNavigateBack = runtime.navigator::goBack, + ) + } wireEntry(presentation = WireEntryPresentation.PopUp) { route -> MessageDetailsRouteScreen(messageDetailsViewModel(route.toViewModelArgs()), runtime.navigator::goBack) } diff --git a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt index b1e29bd649b..450e4df5c0a 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt @@ -75,6 +75,18 @@ data class VideoPlayerRoute( companion object { const val ROUTE_ID = "app/video_player_screen" } } +@Serializable +data class PdfViewerRoute( + override val sessionId: WireSessionId, + val localPath: String?, + val contentUrl: String?, + val fileName: String?, + override val entryId: WireNavEntryId = WireNavEntryId.random(), +) : SessionRoute { + override val routeId = ROUTE_ID + companion object { const val ROUTE_ID = "app/pdf_viewer_screen" } +} + @Serializable data class MessageDetailsRoute( override val sessionId: WireSessionId, diff --git a/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3Contributions.kt b/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3Contributions.kt index ad061270856..08868526c54 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3Contributions.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3Contributions.kt @@ -95,7 +95,7 @@ internal data class WireNavigation3ContributionCatalog( * app-lock overlays finish the registry. An entry type must be owned by exactly one contribution. */ internal object WireNavigation3Contributions { - const val EXPECTED_ROUTE_REGISTRATION_COUNT: Int = 107 + const val EXPECTED_ROUTE_REGISTRATION_COUNT: Int = 109 const val EXPECTED_INSTALLER_COUNT: Int = 19 fun create( diff --git a/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt b/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt index 1ba53f2fc06..735fc625212 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt @@ -24,6 +24,7 @@ import com.wire.android.feature.cells.navigation.CellImageViewerRoute import com.wire.android.feature.cells.navigation.CellsFilesArguments import com.wire.android.feature.cells.navigation.CellsSearchType import com.wire.android.feature.cells.navigation.ConversationFilesRoute +import com.wire.android.feature.cells.navigation.PdfViewerRoute import com.wire.android.feature.cells.navigation.PublicLinkRoute import com.wire.android.feature.cells.navigation.SearchRoute import com.wire.android.feature.cells.navigation.VideoPlayerRoute @@ -196,6 +197,16 @@ internal class WireNavigation3ProductionActions( ) ) }, + showPdfViewer = { + navigate( + PdfViewerRoute( + sessionId = requireSession(), + localPath = it.localPath, + contentUrl = it.contentUrl, + fileName = it.name, + ) + ) + }, ) override val conversationList: ConversationListNavigationActions = ConversationListNavigationActions( openConversation = { openConversation(it.toProfileId()) }, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt index 5da7f936ddf..8c88b3b2224 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt @@ -58,6 +58,7 @@ internal fun ConversationMessageComposer( onAssetItemClicked: (String) -> Unit, onImageFullScreenMode: (UIMessage.Regular, Boolean, String?) -> Unit, onVideoClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, onReactionClicked: (String, String) -> Unit, onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit, onOpenProfile: (senderId: MessageSenderId) -> Unit, @@ -115,6 +116,7 @@ internal fun ConversationMessageComposer( onAssetClicked = onAssetItemClicked, onImageClicked = onImageFullScreenMode, onVideoClicked = onVideoClick, + onPdfClicked = onPdfClick, onLinkClicked = onLinkClick, onReplyClicked = onNavigateToReplyOriginalMessage, onResetSessionClicked = onResetSessionClicked, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt index 17e88003738..0bfd3dddfad 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt @@ -29,6 +29,7 @@ import com.wire.android.navigation.routes.media.MediaGalleryNavigation3ResultTyp import com.wire.android.navigation.routes.media.MediaGalleryResult import com.wire.android.navigation.routes.media.MediaGalleryResultAction import com.wire.android.navigation.routes.media.MediaGalleryRoute +import com.wire.android.navigation.routes.media.PdfViewerRoute import com.wire.android.navigation.routes.media.VideoPlayerRoute import com.wire.android.navigation.routes.media.MessageDetailsRoute import com.wire.android.navigation.routes.media.toLegacy @@ -251,6 +252,14 @@ private fun ConversationNavigation3Entry( ) } + override fun openPdfViewer(localPath: String?, contentUrl: String?, fileName: String?) { + runtime.navigator.navigate( + WireNavigationCommand( + PdfViewerRoute(route.sessionId, localPath, contentUrl, fileName) + ) + ) + } + override fun openDrawingCanvas(conversationName: String, tempWritableUri: Uri?) { drawingRequestId = runtime.navigateForResult( DrawingCanvasRoute( diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt index 5a429b66d70..e7d0493312a 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt @@ -52,6 +52,8 @@ internal interface ConversationRouteScreenNavigation { fun openVideoPlayer(localPath: String?, contentUrl: String?, fileName: String?) + fun openPdfViewer(localPath: String?, contentUrl: String?, fileName: String?) + fun openDrawingCanvas( conversationName: String, tempWritableUri: Uri?, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt index de7ba41a68a..8ad1a80ddfd 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt @@ -438,6 +438,7 @@ internal fun ConversationScreenRouteContent( } }, onVideoClick = navigation::openVideoPlayer, + onPdfClick = navigation::openPdfViewer, onStartCall = { conversationCallViewModel.startCallIfPossible(conversationInfoViewModel.conversationInfoViewState.conversationType) }, @@ -642,6 +643,7 @@ private fun ConversationScreenContent( onAssetItemClicked: (String) -> Unit, onImageFullScreenMode: (UIMessage.Regular, Boolean, String?) -> Unit, onVideoClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, onStartCall: () -> Unit, onJoinCall: () -> Unit, onReactionClick: (messageId: String, reactionEmoji: String) -> Unit, @@ -754,6 +756,7 @@ private fun ConversationScreenContent( onAssetItemClicked = onAssetItemClicked, onImageFullScreenMode = onImageFullScreenMode, onVideoClick = onVideoClick, + onPdfClick = onPdfClick, onReactionClicked = onReactionClick, onResetSessionClicked = onResetSessionClick, onOpenProfile = onOpenProfile, @@ -896,6 +899,7 @@ fun PreviewConversationScreen() = WireTheme { onAssetItemClicked = { }, onImageFullScreenMode = { _, _, _ -> }, onVideoClick = { _, _, _ -> }, + onPdfClick = { _, _, _ -> }, onStartCall = { }, onJoinCall = { }, onReactionClick = { _, _ -> }, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt index 62274c1d983..f822497c02d 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt @@ -30,6 +30,7 @@ sealed class MessageClickActions { open val onAssetClicked: (String) -> Unit = {} open val onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit = { _, _, _ -> } open val onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> } + open val onPdfClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> } open val onLinkClicked: (String) -> Unit = {} open val onReplyClicked: (UIMessage.Regular) -> Unit = {} open val onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit = { _, _ -> } @@ -48,6 +49,7 @@ sealed class MessageClickActions { override val onAssetClicked: (String) -> Unit = {}, override val onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit = { _, _, _ -> }, override val onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> }, + override val onPdfClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> }, override val onLinkClicked: (String) -> Unit = {}, override val onReplyClicked: (UIMessage.Regular) -> Unit = {}, override val onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit = { _, _ -> }, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt index 007570c5d4b..3311b7ffaaf 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt @@ -73,6 +73,7 @@ internal fun UIMessage.Regular.MessageContentAndStatus( onAssetClicked: (String) -> Unit, onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit, onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, onProfileClicked: (senderId: MessageSenderId) -> Unit, onLinkClicked: (String) -> Unit, onReplyClicked: (UIMessage.Regular) -> Unit, @@ -121,6 +122,7 @@ internal fun UIMessage.Regular.MessageContentAndStatus( onImageClick = onImageClickable, onMultipartImageClick = onMultipartImageClickable, onMultipartVideoClick = onVideoClicked, + onMultipartPdfClick = onPdfClicked, onOpenProfile = onProfileClicked, onLinkClick = onLinkClicked, onReplyClick = onReplyClickable, @@ -170,6 +172,7 @@ private fun MessageContent( onImageClick: Clickable, onMultipartImageClick: (String) -> Unit, onMultipartVideoClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onMultipartPdfClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, onOpenProfile: (senderId: MessageSenderId) -> Unit, onLinkClick: (String) -> Unit, onReplyClick: Clickable, @@ -464,6 +467,7 @@ private fun MessageContent( messageStyle = messageStyle, onImageAttachmentClick = onMultipartImageClick, onVideoAttachmentClick = onMultipartVideoClick, + onPdfAttachmentClick = onMultipartPdfClick, ) } diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentItem.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentItem.kt index 59184ca075e..b8bae4501d4 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentItem.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentItem.kt @@ -96,6 +96,7 @@ fun MessageContentItem( onAssetClicked = clickActions.onAssetClicked, onImageClicked = clickActions.onImageClicked, onVideoClicked = clickActions.onVideoClicked, + onPdfClicked = clickActions.onPdfClicked, searchQuery = searchQuery, accent = accent, onProfileClicked = clickActions.onProfileClicked, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt index d9581475c57..5a0cbceea7c 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt @@ -59,6 +59,7 @@ fun MultipartAttachmentsView( messageStyle: MessageStyle, onImageAttachmentClick: (String) -> Unit, onVideoAttachmentClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfAttachmentClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, modifier: Modifier = Modifier, viewModel: MultipartAttachmentsViewModel = when { LocalInspectionMode.current -> MultipartAttachmentsViewModelPreview @@ -68,6 +69,15 @@ fun MultipartAttachmentsView( // Collect to trigger recomposition when offline availability changes. val offlineAttachmentIds by viewModel.offlineAttachmentIds.collectAsStateWithLifecycle() + val handleClick: (MultipartAttachmentUi) -> Unit = { clicked -> + viewModel.onClick( + attachment = clicked, + openInImageViewer = onImageAttachmentClick, + openInVideoPlayer = { att -> onVideoAttachmentClick(att.localPath, att.contentUrl, att.fileName) }, + openInPdfViewer = { att -> onPdfAttachmentClick(att.localPath, att.contentUrl, att.fileName) }, + ) + } + // TODO I found out that empty attachments list is not handled here and it shows empty message with no information if (attachments.size == 1) { val attachment = attachments.first() @@ -86,15 +96,7 @@ fun MultipartAttachmentsView( }, item = it, messageStyle = messageStyle, - onClick = { - viewModel.onClick( - attachment = it, - openInImageViewer = onImageAttachmentClick, - openInVideoPlayer = { att -> - onVideoAttachmentClick(att.localPath, att.contentUrl, att.fileName) - }, - ) - }, + onClick = { handleClick(it) }, ) } } else { @@ -119,30 +121,14 @@ fun MultipartAttachmentsView( AttachmentsGrid( attachments = group.attachments, messageStyle = messageStyle, - onClick = { - viewModel.onClick( - attachment = it, - openInImageViewer = onImageAttachmentClick, - openInVideoPlayer = { att -> - onVideoAttachmentClick(att.localPath, att.contentUrl, att.fileName) - }, - ) - }, + onClick = handleClick, ) is MultipartAttachmentsViewModel.MultipartAttachmentGroup.Files -> AttachmentsList( attachments = group.attachments, messageStyle = messageStyle, - onClick = { - viewModel.onClick( - attachment = it, - openInImageViewer = onImageAttachmentClick, - openInVideoPlayer = { att -> - onVideoAttachmentClick(att.localPath, att.contentUrl, att.fileName) - }, - ) - }, + onClick = handleClick, ) } } diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt index e074186eb1e..336a544a4bb 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt @@ -62,6 +62,7 @@ interface MultipartAttachmentsViewModel { attachment: MultipartAttachmentUi, openInImageViewer: (String) -> Unit, openInVideoPlayer: (MultipartAttachmentUi) -> Unit, + openInPdfViewer: (MultipartAttachmentUi) -> Unit, ) fun mapAttachment(attachment: MessageAttachment): MultipartAttachmentUi { val isAvailableOffline = attachment.assetId() in offlineAttachmentIds.value @@ -128,6 +129,7 @@ object MultipartAttachmentsViewModelPreview : MultipartAttachmentsViewModel { attachment: MultipartAttachmentUi, openInImageViewer: (String) -> Unit, openInVideoPlayer: (MultipartAttachmentUi) -> Unit, + openInPdfViewer: (MultipartAttachmentUi) -> Unit, ) {} override fun onAttachmentsVisible(attachments: List) {} override fun onAttachmentsHidden(attachments: List) {} @@ -168,6 +170,7 @@ class MultipartAttachmentsViewModelImpl @AssistedInject constructor( attachment: MultipartAttachmentUi, openInImageViewer: (String) -> Unit, openInVideoPlayer: (MultipartAttachmentUi) -> Unit, + openInPdfViewer: (MultipartAttachmentUi) -> Unit, ) { when { attachment.isImage() && !attachment.fileNotFound() -> openInImageViewer(attachment.uuid) @@ -181,6 +184,9 @@ class MultipartAttachmentsViewModelImpl @AssistedInject constructor( attachment.isVideo() && (attachment.localFileAvailable() || attachment.canOpenWithUrl()) -> openInVideoPlayer(attachment) + attachment.isPdf() && (attachment.localFileAvailable() || attachment.canOpenWithUrl()) -> + openInPdfViewer(attachment) + attachment.localFileAvailable() -> openLocalFile(attachment) attachment.canOpenWithUrl() -> openUrl(attachment) else -> downloadAsset(attachment) @@ -275,6 +281,8 @@ private fun MultipartAttachmentUi.isImage() = AttachmentFileType.fromMimeType(mi private fun MultipartAttachmentUi.isVideo() = assetType == VIDEO +private fun MultipartAttachmentUi.isPdf() = assetType == PDF + private fun MessageAttachment.isMediaAttachment() = when (AttachmentFileType.fromMimeType(mimeType())) { IMAGE, VIDEO -> true diff --git a/app/src/test/kotlin/com/wire/android/navigation/runtime/WireNavigation3ContributionsTest.kt b/app/src/test/kotlin/com/wire/android/navigation/runtime/WireNavigation3ContributionsTest.kt index 3517a55f27b..cc2f2b3c132 100644 --- a/app/src/test/kotlin/com/wire/android/navigation/runtime/WireNavigation3ContributionsTest.kt +++ b/app/src/test/kotlin/com/wire/android/navigation/runtime/WireNavigation3ContributionsTest.kt @@ -90,7 +90,7 @@ class WireNavigation3ContributionsTest { } assertEquals(WireNavigation3Contributions.EXPECTED_ROUTE_REGISTRATION_COUNT, registrationCount) - assertEquals(107, registrationCount) + assertEquals(109, registrationCount) } @Test diff --git a/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt index b26d87e3df3..c6f5244957e 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt @@ -50,6 +50,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith typealias OpenImageCallback = (s: String) -> Unit +typealias OpenAttachmentCallback = (attachment: MultipartAttachmentUi) -> Unit @ExtendWith(CoroutineTestExtension::class) class MultipartAttachmentsViewModelTest { @@ -186,7 +187,7 @@ class MultipartAttachmentsViewModelTest { val callback = mockk(relaxed = true) - viewModel.onClick(testAttachmentUi, callback, {}) + viewModel.onClick(testAttachmentUi, callback, {}, {}) coVerify(exactly = 1) { callback.invoke(testAttachmentUi.uuid) } } @@ -203,7 +204,8 @@ class MultipartAttachmentsViewModelTest { transferStatus = AssetTransferStatus.NOT_FOUND, ), openInImageViewer = callback, - openInVideoPlayer = { } + openInVideoPlayer = { }, + openInPdfViewer = { }, ) coVerify(exactly = 0) { callback.invoke(testAttachmentUi.uuid) } @@ -220,10 +222,12 @@ class MultipartAttachmentsViewModelTest { viewModel.onClick( attachment = testAttachmentUi.copy( mimeType = "application/pdf", + assetType = AttachmentFileType.PDF, transferStatus = AssetTransferStatus.NOT_FOUND, ), openInImageViewer = callback, - openInVideoPlayer = { } + openInVideoPlayer = { }, + openInPdfViewer = { }, ) coVerify(exactly = 0) { callback.invoke(testAttachmentUi.uuid) } @@ -239,11 +243,13 @@ class MultipartAttachmentsViewModelTest { viewModel.onClick( attachment = testAttachmentUi.copy( - mimeType = "application/pdf", + mimeType = "application/zip", + assetType = AttachmentFileType.ARCHIVE, localPath = "local/path", ), openInImageViewer = callback, - openInVideoPlayer = { } + openInVideoPlayer = { }, + openInPdfViewer = { }, ) coVerify(exactly = 1) { arrangement.fileManager.openWithExternalApp(any(), any(), any(), any()) } @@ -258,16 +264,85 @@ class MultipartAttachmentsViewModelTest { viewModel.onClick( attachment = testAttachmentUi.copy( - mimeType = "application/pdf", + mimeType = "application/zip", + assetType = AttachmentFileType.ARCHIVE, contentUrl = "content/url", ), openInImageViewer = callback, - openInVideoPlayer = { } + openInVideoPlayer = { }, + openInPdfViewer = { }, ) coVerify(exactly = 1) { arrangement.fileManager.openUrlWithExternalApp(any(), any(), any()) } } + @Test + fun `with pdf attachment with local file available when clicked then pdf opened in internal viewer`() = runTest { + val (arrangement, viewModel) = Arrangement() + .arrange() + + val callback = mockk(relaxed = true) + val attachment = testAttachmentUi.copy( + mimeType = "application/pdf", + assetType = AttachmentFileType.PDF, + localPath = "local/path", + ) + + viewModel.onClick( + attachment = attachment, + openInImageViewer = { }, + openInVideoPlayer = { }, + openInPdfViewer = callback, + ) + + coVerify(exactly = 1) { callback.invoke(attachment) } + coVerify(exactly = 0) { arrangement.fileManager.openWithExternalApp(any(), any(), any(), any()) } + } + + @Test + fun `with pdf attachment openable via url when clicked then pdf opened in internal viewer`() = runTest { + val (arrangement, viewModel) = Arrangement() + .arrange() + + val callback = mockk(relaxed = true) + val attachment = testAttachmentUi.copy( + mimeType = "application/pdf", + assetType = AttachmentFileType.PDF, + contentUrl = "content/url", + ) + + viewModel.onClick( + attachment = attachment, + openInImageViewer = { }, + openInVideoPlayer = { }, + openInPdfViewer = callback, + ) + + coVerify(exactly = 1) { callback.invoke(attachment) } + coVerify(exactly = 0) { arrangement.fileManager.openUrlWithExternalApp(any(), any(), any()) } + } + + @Test + fun `with pdf attachment not downloaded yet when clicked then the viewer is not opened`() = runTest { + val (arrangement, viewModel) = Arrangement() + .arrange() + + val callback = mockk(relaxed = true) + + viewModel.onClick( + attachment = testAttachmentUi.copy( + mimeType = "application/pdf", + assetType = AttachmentFileType.PDF, + ), + openInImageViewer = { }, + openInVideoPlayer = { }, + openInPdfViewer = callback, + ) + + coVerify(exactly = 0) { callback.invoke(any()) } + coVerify(exactly = 0) { arrangement.fileManager.openWithExternalApp(any(), any(), any(), any()) } + } + // TODO: Refresh asset tests (part of refresh update PR) private class Arrangement { diff --git a/core/pdf-viewer/build.gradle.kts b/core/pdf-viewer/build.gradle.kts new file mode 100644 index 00000000000..ddc0ca08878 --- /dev/null +++ b/core/pdf-viewer/build.gradle.kts @@ -0,0 +1,58 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +plugins { + id(libs.plugins.wire.android.library.get().pluginId) + id(libs.plugins.wire.kover.get().pluginId) + id(BuildPlugins.junit5) + id(libs.plugins.wire.compose.compiler.get().pluginId) + alias(libs.plugins.compose.stability.analyzer) + alias(libs.plugins.ksp) +} + +android { + namespace = "com.wire.android.pdfviewer" +} + +dependencies { + + implementation(project(":core:di")) + implementation(project(":core:ui-common")) + + implementation(libs.androidx.core) + implementation(libs.androidx.appcompat) + implementation(libs.coroutines.android) + + val composeBom = enforcedPlatform(libs.compose.bom) + implementation(composeBom) + implementation(libs.compose.ui) + implementation(libs.compose.ui.graphics) + implementation(libs.compose.material3) + implementation(libs.compose.activity) + implementation(libs.androidx.lifecycle.viewModelCompose) + implementation(libs.compose.ui.preview) + implementation(libs.metrox.viewModelCompose) + debugImplementation(libs.compose.ui.tooling) + + testImplementation(libs.junit5.core) + testImplementation(libs.coroutines.test) + testImplementation(libs.mockk.core) + testImplementation(libs.turbine) + testRuntimeOnly(libs.junit5.engine) + testImplementation(testFixtures(project(":core:ui-common"))) + ksp(project(":ksp")) +} diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt new file mode 100644 index 00000000000..d0311f8a60d --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt @@ -0,0 +1,105 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.graphics.Bitmap +import android.graphics.Color +import android.graphics.pdf.PdfRenderer +import android.os.ParcelFileDescriptor +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import java.io.Closeable +import java.io.File + +/** + * Thin coroutine-friendly wrapper around the platform [PdfRenderer]. + * + * Everything goes through [mutex] because [PdfRenderer] only allows a single open page at a time + * and is not thread safe. Rendering happens entirely in-process — no network access and no + * third party parser — which keeps documents from ever leaving the device. + */ +internal class PdfDocument private constructor( + private val descriptor: ParcelFileDescriptor, + private val renderer: PdfRenderer, +) : Closeable { + + private val mutex = Mutex() + private var closed = false + + val pageCount: Int = renderer.pageCount + + /** Width / height of [pageIndex], used to reserve the right amount of space before rendering. */ + suspend fun aspectRatio(pageIndex: Int): Float = mutex.withLock { + if (closed) return DEFAULT_ASPECT_RATIO + renderer.openPage(pageIndex).use { page -> + if (page.height == 0) DEFAULT_ASPECT_RATIO else page.width.toFloat() / page.height + } + } + + /** + * Renders [pageIndex] into a bitmap [widthPx] wide, keeping the page aspect ratio. + * + * Returns `null` when the document was closed while the caller was waiting for the lock. + */ + suspend fun renderPage(pageIndex: Int, widthPx: Int): Bitmap? = mutex.withLock { + if (closed) return null + renderer.openPage(pageIndex).use { page -> + val safeWidth = widthPx.coerceIn(MIN_RENDER_WIDTH_PX, MAX_RENDER_WIDTH_PX) + val height = if (page.width == 0) { + safeWidth + } else { + (safeWidth.toLong() * page.height / page.width).toInt() + }.coerceIn(MIN_RENDER_WIDTH_PX, MAX_RENDER_WIDTH_PX) + + Bitmap.createBitmap(safeWidth, height, Bitmap.Config.ARGB_8888).apply { + // PdfRenderer draws only the page content, so the paper itself has to be painted. + eraseColor(Color.WHITE) + page.render(this, null, null, PdfRenderer.Page.RENDER_MODE_FOR_DISPLAY) + } + } + } + + override fun close() { + if (closed) return + closed = true + runCatching { renderer.close() } + runCatching { descriptor.close() } + } + + companion object { + const val DEFAULT_ASPECT_RATIO = 1f / 1.414f // A4 portrait + private const val MIN_RENDER_WIDTH_PX = 1 + private const val MAX_RENDER_WIDTH_PX = 4_096 + + /** + * Opens [file] for rendering, translating the platform failures into a [PdfViewerError]. + * + * [PdfRenderer] throws [SecurityException] for password protected documents and + * [java.io.IOException] for anything it cannot parse. + */ + fun open(file: File): Result = runCatching { + val descriptor = ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY) + try { + PdfDocument(descriptor, PdfRenderer(descriptor)) + } catch (error: Throwable) { + runCatching { descriptor.close() } + throw error + } + } + } +} diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt new file mode 100644 index 00000000000..3f3390aa9eb --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt @@ -0,0 +1,106 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.content.Context +import com.wire.android.di.ApplicationContext +import dev.zacsweers.metro.Inject +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import java.io.File +import java.net.HttpURLConnection +import java.net.URL +import java.security.MessageDigest + +/** + * Turns the arguments of the PDF screen into a readable local file. + * + * [android.graphics.pdf.PdfRenderer] needs a seekable file descriptor, so a remote document has + * to be fetched into the app cache first. Already downloaded files are reused, which keeps + * re-opening the same attachment instant. + */ +class PdfSourceResolver @Inject constructor( + @ApplicationContext private val context: Context, +) { + + suspend fun resolve( + localPath: String?, + contentUrl: String?, + dispatcher: CoroutineDispatcher = Dispatchers.IO, + ): Result = withContext(dispatcher) { + val localFile = localPath?.let(::File) + when { + localFile != null && localFile.isReadableFile() -> Result.success(localFile) + contentUrl != null -> download(contentUrl) + else -> Result.failure(PdfSourceException(PdfViewerError.FILE_NOT_FOUND)) + } + } + + private fun download(contentUrl: String): Result { + val target = cacheFileFor(contentUrl) + if (target.isReadableFile()) return Result.success(target) + + val partial = File(target.parentFile, "${target.name}$PARTIAL_SUFFIX") + return runCatching { + val connection = (URL(contentUrl).openConnection() as HttpURLConnection).apply { + connectTimeout = TIMEOUT_MS + readTimeout = TIMEOUT_MS + instanceFollowRedirects = true + } + try { + if (connection.responseCode !in HTTP_OK_RANGE) { + error("Unexpected response ${connection.responseCode} while fetching the document") + } + partial.parentFile?.mkdirs() + connection.inputStream.use { input -> + partial.outputStream().use { output -> input.copyTo(output) } + } + } finally { + connection.disconnect() + } + check(partial.renameTo(target)) { "Could not move the downloaded document into place" } + target + }.recoverCatching { cause -> + partial.delete() + throw PdfSourceException(PdfViewerError.DOWNLOAD_FAILED, cause) + } + } + + private fun cacheFileFor(contentUrl: String): File { + val digest = MessageDigest.getInstance("SHA-256") + .digest(contentUrl.toByteArray()) + .joinToString("") { "%02x".format(it) } + return File(File(context.cacheDir, CACHE_DIR_NAME), "$digest.pdf") + } + + private fun File.isReadableFile(): Boolean = isFile && canRead() && length() > 0 + + private companion object { + const val CACHE_DIR_NAME = "pdf-viewer" + const val PARTIAL_SUFFIX = ".part" + const val TIMEOUT_MS = 30_000 + val HTTP_OK_RANGE = 200..299 + } +} + +/** Carries the user-facing [error] out of [PdfSourceResolver]. */ +class PdfSourceException( + val error: PdfViewerError, + cause: Throwable? = null, +) : Exception(cause) diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt new file mode 100644 index 00000000000..dfb18997138 --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt @@ -0,0 +1,376 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.graphics.Bitmap +import androidx.compose.foundation.Image +import androidx.compose.foundation.background +import androidx.compose.foundation.gestures.awaitEachGesture +import androidx.compose.foundation.gestures.awaitFirstDown +import androidx.compose.foundation.gestures.calculatePan +import androidx.compose.foundation.gestures.calculateZoom +import androidx.compose.foundation.gestures.detectTapGestures +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.aspectRatio +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.LazyListState +import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.derivedStateOf +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableFloatStateOf +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clipToBounds +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.TransformOrigin +import androidx.compose.ui.graphics.asImageBitmap +import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.input.pointer.PointerEventPass +import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.layout.onSizeChanged +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.style.TextAlign +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.wire.android.ui.common.button.WirePrimaryButton +import com.wire.android.ui.common.colorsScheme +import com.wire.android.ui.common.dimensions +import com.wire.android.ui.common.preview.MultipleThemePreviews +import com.wire.android.ui.common.progress.CenteredCircularProgressBarIndicator +import com.wire.android.ui.common.progress.WireCircularProgressIndicator +import com.wire.android.ui.common.scaffold.WireScaffold +import com.wire.android.ui.common.topappbar.NavigationIconType +import com.wire.android.ui.common.topappbar.WireCenterAlignedTopAppBar +import com.wire.android.ui.common.typography +import com.wire.android.ui.theme.WireTheme +import kotlin.math.abs +import kotlin.math.ceil + +private const val MIN_ZOOM = 1f +private const val MAX_ZOOM = 5f + +/** Beyond this the extra pixels are no longer visible but the bitmaps get very expensive. */ +private const val MAX_RENDER_SCALE = 3f +private const val DOUBLE_TAP_ZOOM = 2.5f + +/** + * Reusable full screen PDF viewer. Shows either a local file ([localPath]) or a remote + * [contentUrl], which is fetched into the app cache before rendering. + * + * Callers own navigation via [onNavigateBack]; the ViewModel is resolved from the shared + * pdf-viewer Metro graph so any module can host this screen. + */ +@Composable +fun PdfViewer( + localPath: String?, + contentUrl: String?, + fileName: String?, + onNavigateBack: () -> Unit, + modifier: Modifier = Modifier, + viewModel: PdfViewerViewModel = pdfViewerViewModel(localPath, contentUrl, fileName), +) { + val state by viewModel.state.collectAsStateWithLifecycle() + PdfViewerContent( + state = state, + fileName = fileName, + onRetry = viewModel::retry, + onNavigateBack = onNavigateBack, + renderPage = viewModel::renderPage, + modifier = modifier, + ) +} + +@Composable +internal fun PdfViewerContent( + state: PdfViewerState, + fileName: String?, + onRetry: () -> Unit, + onNavigateBack: () -> Unit, + renderPage: suspend (pageIndex: Int, widthPx: Int) -> Bitmap?, + modifier: Modifier = Modifier, +) { + val listState = rememberLazyListState() + + WireScaffold( + modifier = modifier, + topBar = { + WireCenterAlignedTopAppBar( + title = fileName ?: stringResource(R.string.pdf_viewer_title), + navigationIconType = NavigationIconType.Back(), + onNavigationPressed = onNavigateBack, + subtitleContent = { + if (state is PdfViewerState.Content) { + PageIndicator(listState = listState, pageCount = state.pageCount) + } + }, + ) + }, + ) { innerPadding -> + Box( + modifier = Modifier + .padding(innerPadding) + .fillMaxSize() + .background(colorsScheme().background), + ) { + when (state) { + PdfViewerState.Loading -> CenteredCircularProgressBarIndicator() + is PdfViewerState.Failure -> PdfViewerFailure(error = state.error, onRetry = onRetry) + is PdfViewerState.Content -> PdfPages( + state = state, + listState = listState, + renderPage = renderPage, + ) + } + } + } +} + +@Composable +private fun PageIndicator(listState: LazyListState, pageCount: Int) { + val currentPage by remember(pageCount) { + derivedStateOf { (listState.firstVisibleItemIndex + 1).coerceIn(1, pageCount) } + } + Text( + text = stringResource(R.string.pdf_viewer_page_indicator, currentPage, pageCount), + style = typography().subline01, + color = colorsScheme().secondaryText, + ) +} + +@Composable +private fun PdfPages( + state: PdfViewerState.Content, + listState: LazyListState, + renderPage: suspend (pageIndex: Int, widthPx: Int) -> Bitmap?, +) { + var scale by remember { mutableFloatStateOf(MIN_ZOOM) } + var horizontalOffset by remember { mutableFloatStateOf(0f) } + var viewportWidthPx by remember { mutableIntStateOf(0) } + + fun applyTransform(zoomChange: Float, panX: Float) { + scale = (scale * zoomChange).coerceIn(MIN_ZOOM, MAX_ZOOM) + // Panning is only meaningful once the content is wider than the viewport. + val maxOffset = viewportWidthPx * (scale - MIN_ZOOM) / 2f + horizontalOffset = (horizontalOffset + panX).coerceIn(-maxOffset, maxOffset) + } + + Box( + modifier = Modifier + .fillMaxSize() + .clipToBounds() + .onSizeChanged { viewportWidthPx = it.width } + .zoomAndPan(currentScale = { scale }, onTransform = ::applyTransform) + .pointerInput(Unit) { + detectTapGestures( + onDoubleTap = { + if (scale > MIN_ZOOM) { + scale = MIN_ZOOM + horizontalOffset = 0f + } else { + scale = DOUBLE_TAP_ZOOM + } + }, + ) + }, + ) { + // Pages are rasterised at the zoomed width so text stays sharp instead of being upscaled. + val renderScale = ceil(scale).coerceIn(MIN_ZOOM, MAX_RENDER_SCALE) + val pageWidthPx = (viewportWidthPx * renderScale).toInt() + + LazyColumn( + state = listState, + modifier = Modifier + .fillMaxSize() + .graphicsLayer { + scaleX = scale + scaleY = scale + translationX = horizontalOffset + transformOrigin = TransformOrigin(pivotFractionX = 0.5f, pivotFractionY = 0f) + }, + contentPadding = PaddingValues(dimensions().spacing8x), + verticalArrangement = Arrangement.spacedBy(dimensions().spacing8x), + ) { + items(count = state.pageCount, key = { it }) { pageIndex -> + PdfPage( + pageIndex = pageIndex, + fallbackAspectRatio = state.firstPageAspectRatio, + widthPx = pageWidthPx, + renderPage = renderPage, + ) + } + } + } +} + +@Composable +private fun PdfPage( + pageIndex: Int, + fallbackAspectRatio: Float, + widthPx: Int, + renderPage: suspend (pageIndex: Int, widthPx: Int) -> Bitmap?, +) { + // Deliberately keyed on the page only: while a sharper bitmap is rendered after a zoom the + // previous one stays on screen instead of flashing back to a spinner. + var bitmap by remember(pageIndex) { mutableStateOf(null) } + + LaunchedEffect(pageIndex, widthPx) { + if (widthPx > 0) { + renderPage(pageIndex, widthPx)?.let { bitmap = it } + } + } + + val rendered = bitmap + val aspectRatio = when { + rendered != null && rendered.height > 0 -> rendered.width.toFloat() / rendered.height + else -> fallbackAspectRatio + } + + Box( + modifier = Modifier + .fillMaxWidth() + .aspectRatio(aspectRatio) + .background(Color.White), + contentAlignment = Alignment.Center, + ) { + if (rendered != null) { + Image( + bitmap = rendered.asImageBitmap(), + contentDescription = stringResource( + R.string.pdf_viewer_page_content_description, + pageIndex + 1, + ), + modifier = Modifier.fillMaxSize(), + contentScale = ContentScale.Fit, + ) + } else { + WireCircularProgressIndicator( + progressColor = colorsScheme().secondaryText, + size = dimensions().spacing32x, + ) + } + } +} + +@Composable +private fun PdfViewerFailure(error: PdfViewerError, onRetry: () -> Unit) { + Column( + modifier = Modifier + .fillMaxSize() + .padding(dimensions().spacing24x), + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Text( + text = stringResource(error.messageResId()), + style = typography().body01, + color = colorsScheme().onBackground, + textAlign = TextAlign.Center, + ) + if (error.isRetryable()) { + WirePrimaryButton( + onClick = onRetry, + text = stringResource(R.string.pdf_viewer_retry), + fillMaxWidth = false, + modifier = Modifier.padding(top = dimensions().spacing16x), + ) + } + } +} + +private fun PdfViewerError.messageResId(): Int = when (this) { + PdfViewerError.FILE_NOT_FOUND -> R.string.pdf_viewer_error_file_not_found + PdfViewerError.DOWNLOAD_FAILED -> R.string.pdf_viewer_error_download_failed + PdfViewerError.PASSWORD_PROTECTED -> R.string.pdf_viewer_error_password_protected + PdfViewerError.INVALID_DOCUMENT -> R.string.pdf_viewer_error_invalid_document +} + +private fun PdfViewerError.isRetryable(): Boolean = this == PdfViewerError.DOWNLOAD_FAILED + +/** + * Pinch to zoom plus horizontal panning, layered on top of the list's own vertical scrolling. + * + * Events are inspected on [PointerEventPass.Initial] so a two finger pinch is claimed before the + * list turns it into a scroll. Single finger gestures are only taken over when the content is + * zoomed in *and* the drag is mostly horizontal, which leaves vertical scrolling to the list. + */ +private fun Modifier.zoomAndPan( + currentScale: () -> Float, + onTransform: (zoomChange: Float, panX: Float) -> Unit, +): Modifier = pointerInput(Unit) { + awaitEachGesture { + awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Initial) + do { + val event = awaitPointerEvent(PointerEventPass.Initial) + val pressedPointers = event.changes.count { it.pressed } + val pan = event.calculatePan() + val handled = when { + pressedPointers > 1 -> { + onTransform(event.calculateZoom(), pan.x) + true + } + + currentScale() > MIN_ZOOM && abs(pan.x) > abs(pan.y) -> { + onTransform(1f, pan.x) + true + } + + else -> false + } + if (handled) { + event.changes.forEach { it.consume() } + } + } while (event.changes.any { it.pressed }) + } +} + +@MultipleThemePreviews +@Composable +fun PreviewPdfViewerLoading() = WireTheme { + PdfViewerContent( + state = PdfViewerState.Loading, + fileName = "Quarterly report.pdf", + onRetry = {}, + onNavigateBack = {}, + renderPage = { _, _ -> null }, + ) +} + +@MultipleThemePreviews +@Composable +fun PreviewPdfViewerFailure() = WireTheme { + PdfViewerContent( + state = PdfViewerState.Failure(PdfViewerError.DOWNLOAD_FAILED), + fileName = "Quarterly report.pdf", + onRetry = {}, + onNavigateBack = {}, + renderPage = { _, _ -> null }, + ) +} diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerState.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerState.kt new file mode 100644 index 00000000000..a0c992b0870 --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerState.kt @@ -0,0 +1,48 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +/** Everything the PDF screen needs to draw itself. */ +sealed interface PdfViewerState { + + /** The document is being fetched and/or parsed. */ + data object Loading : PdfViewerState + + /** The document is ready; [pageCount] pages can be requested from the ViewModel. */ + data class Content( + val pageCount: Int, + val firstPageAspectRatio: Float, + ) : PdfViewerState + + /** The document could not be shown. */ + data class Failure(val error: PdfViewerError) : PdfViewerState +} + +enum class PdfViewerError { + /** No local path and no content URL were given, or the local file is gone. */ + FILE_NOT_FOUND, + + /** The content URL could not be fetched. */ + DOWNLOAD_FAILED, + + /** The document is encrypted and needs a password, which is not supported. */ + PASSWORD_PROTECTED, + + /** The bytes are not a PDF we can parse. */ + INVALID_DOCUMENT, +} diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt new file mode 100644 index 00000000000..37129830012 --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt @@ -0,0 +1,159 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.graphics.Bitmap +import android.util.LruCache +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import com.wire.android.di.metro.WireAssistedViewModelBinding +import dev.zacsweers.metro.Assisted +import dev.zacsweers.metro.AssistedFactory +import dev.zacsweers.metro.AssistedInject +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import java.io.IOException + +/** + * Opens a single PDF from either a local file ([localPath]) or a remote [contentUrl] and renders + * its pages on demand. + * + * Arguments are passed through the assisted [Factory] instead of a navigation destination so the + * screen can be hosted from any module. + */ +@WireAssistedViewModelBinding(PdfViewerManualViewModelFactoryGroup::class) +class PdfViewerViewModel @AssistedInject constructor( + private val sourceResolver: PdfSourceResolver, + @Assisted val localPath: String?, + @Assisted val contentUrl: String?, + @Assisted val fileName: String?, +) : ViewModel() { + + @AssistedFactory + interface Factory { + fun create(localPath: String?, contentUrl: String?, fileName: String?): PdfViewerViewModel + } + + private val _state = MutableStateFlow(PdfViewerState.Loading) + val state: StateFlow = _state.asStateFlow() + + private var document: PdfDocument? = null + private var loadJob: Job? = null + + /** + * Keeps recently rendered pages around so scrolling back does not re-rasterise them. + * Sized against the heap rather than a page count because page bitmaps vary a lot in size. + */ + private val pageCache = object : LruCache(cacheSizeKb()) { + override fun sizeOf(key: String, value: Bitmap): Int = value.byteCount / BYTES_IN_KB + } + + init { + load() + } + + fun retry() { + if (_state.value is PdfViewerState.Loading) return + load() + } + + /** + * Renders [pageIndex] at [widthPx] and caches the result. Returns `null` when the document is + * not open (yet) or the page could not be rendered. + */ + suspend fun renderPage(pageIndex: Int, widthPx: Int): Bitmap? { + if (widthPx <= 0) return null + val current = document ?: return null + val key = "$pageIndex@$widthPx" + pageCache.get(key)?.let { return it } + + val rendered = withContext(renderDispatcher) { + runCatching { current.renderPage(pageIndex, widthPx) }.getOrNull() + } ?: return null + + pageCache.put(key, rendered) + return rendered + } + + private fun load() { + loadJob?.cancel() + closeDocument() + _state.value = PdfViewerState.Loading + loadJob = viewModelScope.launch { + val file = sourceResolver.resolve(localPath, contentUrl, renderDispatcher) + .getOrElse { cause -> + _state.value = PdfViewerState.Failure(cause.toViewerError()) + return@launch + } + + val opened = withContext(renderDispatcher) { PdfDocument.open(file) } + .getOrElse { cause -> + _state.value = PdfViewerState.Failure(cause.toViewerError()) + return@launch + } + + if (opened.pageCount == 0) { + opened.close() + _state.value = PdfViewerState.Failure(PdfViewerError.INVALID_DOCUMENT) + return@launch + } + + document = opened + _state.value = PdfViewerState.Content( + pageCount = opened.pageCount, + firstPageAspectRatio = withContext(renderDispatcher) { opened.aspectRatio(0) }, + ) + } + } + + private fun closeDocument() { + pageCache.evictAll() + document?.close() + document = null + } + + override fun onCleared() { + super.onCleared() + loadJob?.cancel() + closeDocument() + } + + private companion object { + val renderDispatcher: CoroutineDispatcher = Dispatchers.IO + const val BYTES_IN_KB = 1024 + const val CACHE_HEAP_FRACTION = 8 + + fun cacheSizeKb(): Int = + (Runtime.getRuntime().maxMemory() / BYTES_IN_KB / CACHE_HEAP_FRACTION) + .coerceAtLeast(1) + .toInt() + } +} + +private fun Throwable.toViewerError(): PdfViewerError = when { + this is PdfSourceException -> error + this is SecurityException -> PdfViewerError.PASSWORD_PROTECTED + this is IOException -> PdfViewerError.INVALID_DOCUMENT + else -> PdfViewerError.INVALID_DOCUMENT +} diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt new file mode 100644 index 00000000000..2566c815c68 --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt @@ -0,0 +1,39 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +@file:Suppress("MatchingDeclarationName") + +package com.wire.android.pdfviewer + +import androidx.compose.runtime.Composable +import com.wire.android.di.metro.WireAssistedViewModelFactoryGroup +import com.wire.android.di.metro.wireAssistedMetroViewModel + +@WireAssistedViewModelFactoryGroup +object PdfViewerManualViewModelFactoryGroup + +@Composable +fun pdfViewerViewModel( + localPath: String?, + contentUrl: String?, + fileName: String?, +): PdfViewerViewModel = + wireAssistedMetroViewModel( + instanceKey = "pdf_viewer_${localPath ?: contentUrl}" + ) { + pdfViewerViewModel(localPath, contentUrl, fileName) + } diff --git a/core/pdf-viewer/src/main/res/values/strings.xml b/core/pdf-viewer/src/main/res/values/strings.xml new file mode 100644 index 00000000000..b0c8f4a0ec8 --- /dev/null +++ b/core/pdf-viewer/src/main/res/values/strings.xml @@ -0,0 +1,11 @@ + + + Document + Page %1$d of %2$d + Page %1$d + This document is no longer available. + This document could not be downloaded. + This document is password protected and cannot be opened here. + This document could not be opened. + Try again + \ No newline at end of file diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt new file mode 100644 index 00000000000..ccacb4816f5 --- /dev/null +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt @@ -0,0 +1,96 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.content.Context +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.io.File + +internal class PdfSourceResolverTest { + + @TempDir + lateinit var tempDir: File + + @Test + fun givenAReadableLocalFile_whenResolving_thenThatFileIsReturnedWithoutDownloading() = runTest { + val document = File(tempDir, "document.pdf").apply { writeText("%PDF-1.4") } + val resolver = resolver() + + val result = resolver.resolve(document.absolutePath, contentUrl = null, dispatcher = Dispatchers.Default) + + assertEquals(document, result.getOrNull()) + } + + @Test + fun givenNoLocalFileAndNoUrl_whenResolving_thenItFailsAsNotFound() = runTest { + val resolver = resolver() + + val result = resolver.resolve(localPath = null, contentUrl = null, dispatcher = Dispatchers.Default) + + assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) + } + + @Test + fun givenAnEmptyLocalFileAndNoUrl_whenResolving_thenItFailsAsNotFound() = runTest { + val empty = File(tempDir, "empty.pdf").apply { createNewFile() } + val resolver = resolver() + + val result = resolver.resolve(empty.absolutePath, contentUrl = null, dispatcher = Dispatchers.Default) + + assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) + } + + @Test + fun givenAnUnusableUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { + val resolver = resolver() + + val result = resolver.resolve(localPath = null, contentUrl = "not a url", dispatcher = Dispatchers.Default) + + assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + } + + @Test + fun givenAMissingLocalPathAndAUrl_whenResolving_thenTheDownloadPathIsUsed() = runTest { + val resolver = resolver() + + val result = resolver.resolve( + localPath = File(tempDir, "gone.pdf").absolutePath, + contentUrl = "not a url", + dispatcher = Dispatchers.Default, + ) + + assertTrue(result.isFailure) + assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + } + + private fun resolver(): PdfSourceResolver { + val context = mockk() + every { context.cacheDir } returns File(tempDir, "cache") + return PdfSourceResolver(context) + } + + private fun Result.viewerError(): PdfViewerError? = + (exceptionOrNull() as? PdfSourceException)?.error +} diff --git a/crowdin.yml b/crowdin.yml index 60b5730b396..f0da0b601b9 100644 --- a/crowdin.yml +++ b/crowdin.yml @@ -15,6 +15,10 @@ files: [ "source": "/core/media-player/src/main/res/values/strings.xml", "translation": "/core/media-player/src/main/res/values-%two_letters_code%/%original_file_name%" }, + { + "source": "/core/pdf-viewer/src/main/res/values/strings.xml", + "translation": "/core/pdf-viewer/src/main/res/values-%two_letters_code%/%original_file_name%" + }, { "source": "/core/search/src/main/res/values/strings.xml", "translation": "/core/search/src/main/res/values-%two_letters_code%/%original_file_name%" diff --git a/features/cells/build.gradle.kts b/features/cells/build.gradle.kts index 703ff8e6b92..ac6b9de0768 100644 --- a/features/cells/build.gradle.kts +++ b/features/cells/build.gradle.kts @@ -17,6 +17,7 @@ dependencies { implementation(project(":core:navigation")) implementation(project(":core:ui-common")) implementation(project(":core:media-player")) + implementation(project(":core:pdf-viewer")) implementation(libs.compose.activity) implementation(libs.androidx.core) implementation(libs.androidx.appcompat) diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt index e8a91a51dc7..16e27cdb0df 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt @@ -201,6 +201,18 @@ data class AudioPlayerRoute( companion object { const val ROUTE_ID = "cells/audio_player_screen" } } +@Serializable +data class PdfViewerRoute( + override val sessionId: WireSessionId, + val localPath: String? = null, + val contentUrl: String? = null, + val fileName: String? = null, + override val entryId: WireNavEntryId = WireNavEntryId.random(), +) : CellsRoute { + override val routeId = ROUTE_ID + companion object { const val ROUTE_ID = "cells/pdf_viewer_screen" } +} + @Serializable enum class CellsSearchType { SHARED_DRIVE, DRIVE } diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Entries.kt b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Entries.kt index 75fede95524..77727952de4 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Entries.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Entries.kt @@ -30,7 +30,7 @@ val PublicLinkExpirationNavigation3ResultType: ) object CellsNavigation3Contribution { - const val ROUTE_REGISTRATION_COUNT: Int = 16 + const val ROUTE_REGISTRATION_COUNT: Int = 17 val resultTypes: List> = listOf( @@ -94,6 +94,9 @@ internal fun cellsNavigation3Entries( wireEntry(presentation = WireEntryPresentation.PopUp) { CellsNavigation3RouteScreen(it, runtime, onExitCells) } + wireEntry(presentation = WireEntryPresentation.PopUp) { + CellsNavigation3RouteScreen(it, runtime, onExitCells) + } wireEntry(presentation = WireEntryPresentation.PopUp) { CellsNavigation3RouteScreen(it, runtime, onExitCells) } diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt index fcf99164090..b102c0ef74f 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt @@ -54,6 +54,7 @@ import com.wire.android.feature.cells.ui.tags.AddRemoveTagsRouteScreen import com.wire.android.feature.cells.ui.versionHistoryViewModel import com.wire.android.feature.cells.ui.versioning.VersionHistoryRouteScreen import com.wire.android.mediaplayer.VideoPlayer +import com.wire.android.pdfviewer.PdfViewer import com.wire.android.navigation.navigation3.WireNavigation3ResultType import com.wire.android.navigation.navigation3.WireNavigation3Runtime import androidx.compose.ui.platform.LocalContext @@ -236,6 +237,12 @@ internal fun CellsNavigation3RouteScreen( fileName = route.fileName, onNavigateBack = navigateBack, ) + is PdfViewerRoute -> PdfViewer( + localPath = route.localPath, + contentUrl = route.contentUrl, + fileName = route.fileName, + onNavigateBack = navigateBack, + ) is AudioPlayerRoute -> CellAudioPlayerRouteScreen( onNavigateBack = navigateBack, viewModel = cellAudioPlayerViewModel( @@ -375,6 +382,14 @@ private class Navigation3CellsFilesNavigation( ) ) } + + override fun pdf(file: CellNodeUi.File) { + runtime.navigator.navigate( + WireNavigationCommand( + PdfViewerRoute(sessionId, file.localPath, file.contentUrl, file.name) + ) + ) + } } private fun completeBooleanResult(runtime: WireNavigation3Runtime, value: Boolean) { diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesNavigationActions.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesNavigationActions.kt index 80c38c3aaeb..ed46f497095 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesNavigationActions.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesNavigationActions.kt @@ -27,4 +27,5 @@ data class AllFilesNavigationActions( val showImageViewer: (CellNodeUi.File) -> Unit, val showVideoPlayer: (CellNodeUi.File) -> Unit, val showAudioPlayer: (CellNodeUi.File) -> Unit, + val showPdfViewer: (CellNodeUi.File) -> Unit, ) diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesScreen.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesScreen.kt index 2c029e9d8ea..44615c66c49 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesScreen.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesScreen.kt @@ -106,6 +106,7 @@ fun AllFilesScreen( showImageViewer = navigationActions.showImageViewer, showVideoViewer = navigationActions.showVideoPlayer, showAudioPlayer = navigationActions.showAudioPlayer, + showPdfViewer = navigationActions.showPdfViewer, fileReadyFlow = viewModel.fileReadyFlow, showViewerOnlyIcon = viewModel.drivePermissionsEnabled ) diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellScreenContent.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellScreenContent.kt index 58d79d34ebd..855eef174e6 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellScreenContent.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellScreenContent.kt @@ -105,6 +105,7 @@ internal fun CellScreenContent( showImageViewer: (CellNodeUi.File) -> Unit = {}, showVideoViewer: (CellNodeUi.File) -> Unit = {}, showAudioPlayer: (CellNodeUi.File) -> Unit = {}, + showPdfViewer: (CellNodeUi.File) -> Unit = {}, fileReadyFlow: Flow? = emptyFlow(), ) { @@ -266,6 +267,7 @@ internal fun CellScreenContent( is OpenImageViewer -> showImageViewer(action.file) is OpenVideoViewer -> showVideoViewer(action.file) is OpenAudioPlayer -> showAudioPlayer(action.file) + is OpenPdfViewer -> showPdfViewer(action.file) } } diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt index 27c9131d313..ff77fcd6fcb 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt @@ -434,22 +434,9 @@ class CellViewModel @AssistedInject constructor( @Suppress("ReturnCount") private fun openFileContentUrl(file: CellNodeUi.File) { - when (file.assetType) { - AttachmentFileType.IMAGE -> { - if (file.shouldOpenInAppImageViewer()) { - sendAction(OpenImageViewer(file)) - return - } - } - AttachmentFileType.VIDEO -> { - sendAction(OpenVideoViewer(file)) - return - } - AttachmentFileType.AUDIO -> { - sendAction(OpenAudioPlayer(file)) - return - } - else -> Unit + inAppViewerAction(file)?.let { + sendAction(it) + return } file.contentUrl?.let { url -> fileHelper.openAssetUrlWithExternalApp( @@ -464,22 +451,9 @@ class CellViewModel @AssistedInject constructor( @Suppress("ReturnCount") private fun openLocalFile(file: CellNodeUi.File) { - when (file.assetType) { - AttachmentFileType.IMAGE -> { - if (file.shouldOpenInAppImageViewer()) { - sendAction(OpenImageViewer(file)) - return - } - } - AttachmentFileType.VIDEO -> { - sendAction(OpenVideoViewer(file)) - return - } - AttachmentFileType.AUDIO -> { - sendAction(OpenAudioPlayer(file)) - return - } - else -> Unit + inAppViewerAction(file)?.let { + sendAction(it) + return } file.localPath?.let { path -> fileHelper.openAssetFileWithExternalApp( @@ -493,6 +467,18 @@ class CellViewModel @AssistedInject constructor( } } + /** + * The in-app viewer that can show [file], or null when the file has to be handed over to + * another app. + */ + private fun inAppViewerAction(file: CellNodeUi.File): CellViewAction? = when (file.assetType) { + AttachmentFileType.IMAGE -> OpenImageViewer(file).takeIf { file.shouldOpenInAppImageViewer() } + AttachmentFileType.VIDEO -> OpenVideoViewer(file) + AttachmentFileType.AUDIO -> OpenAudioPlayer(file) + AttachmentFileType.PDF -> OpenPdfViewer(file) + else -> null + } + private fun CellNodeUi.File.shouldOpenInAppImageViewer(): Boolean = inAppImageViewerEnabled && assetType == AttachmentFileType.IMAGE @@ -733,6 +719,7 @@ internal data object ShowOfflineFileSaved : CellViewAction internal data class OpenImageViewer(val file: CellNodeUi.File) : CellViewAction internal data class OpenVideoViewer(val file: CellNodeUi.File) : CellViewAction internal data class OpenAudioPlayer(val file: CellNodeUi.File) : CellViewAction +internal data class OpenPdfViewer(val file: CellNodeUi.File) : CellViewAction internal enum class CellError(val message: Int) { NO_APP_FOUND(R.string.no_app_found), diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellsFilesNavigation.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellsFilesNavigation.kt index 78532835241..5eb2560c9bb 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellsFilesNavigation.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellsFilesNavigation.kt @@ -26,6 +26,7 @@ internal interface CellsFilesNavigation { fun image(file: CellNodeUi.File) fun video(file: CellNodeUi.File) fun audio(file: CellNodeUi.File) + fun pdf(file: CellNodeUi.File) } @Suppress("TooManyFunctions") @@ -44,4 +45,5 @@ internal object NoOpCellsFilesNavigation : CellsFilesNavigation { override fun image(file: CellNodeUi.File) = Unit override fun video(file: CellNodeUi.File) = Unit override fun audio(file: CellNodeUi.File) = Unit + override fun pdf(file: CellNodeUi.File) = Unit } diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/ConversationFilesScreen.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/ConversationFilesScreen.kt index cebffa044db..a0766bb19f4 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/ConversationFilesScreen.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/ConversationFilesScreen.kt @@ -327,6 +327,7 @@ internal fun ConversationFilesScreenContent( showImageViewer = navigation::image, showVideoViewer = navigation::video, showAudioPlayer = navigation::audio, + showPdfViewer = navigation::pdf, retryEditNodeError = { retryEditNodeError(it) }, isRefreshing = isRefreshing, onRefresh = onRefresh, diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/search/SearchScreen.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/search/SearchScreen.kt index 04c2d447ba1..355489a022c 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/search/SearchScreen.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/search/SearchScreen.kt @@ -211,6 +211,7 @@ internal fun SearchRouteScreen( showImageViewer = navigation::image, showVideoViewer = navigation::video, showAudioPlayer = navigation::audio, + showPdfViewer = navigation::pdf, retryEditNodeError = { cellViewModel.editNode(it) }, isRefreshing = remember { mutableStateOf(false) }, onRefresh = { }, diff --git a/features/cells/src/test/kotlin/com/wire/android/feature/cells/navigation/CellsNavigation3Test.kt b/features/cells/src/test/kotlin/com/wire/android/feature/cells/navigation/CellsNavigation3Test.kt index 575050c32d0..2e5696edf49 100644 --- a/features/cells/src/test/kotlin/com/wire/android/feature/cells/navigation/CellsNavigation3Test.kt +++ b/features/cells/src/test/kotlin/com/wire/android/feature/cells/navigation/CellsNavigation3Test.kt @@ -41,6 +41,7 @@ class CellsNavigation3Test { CellImageViewerRoute(sessionId), VideoPlayerRoute(sessionId), AudioPlayerRoute(sessionId), + PdfViewerRoute(sessionId), SearchRoute(sessionId), ) diff --git a/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt b/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt index 874d0cee107..4c52df18ff0 100644 --- a/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt +++ b/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt @@ -261,13 +261,30 @@ class CellViewModelTest { .withLoadSuccess() .arrange() - val nonImageFile = testFiles[0].copy(mimeType = "application/pdf").toUiModel() + val nonImageFile = testFiles[0].copy(mimeType = "application/zip").toUiModel() viewModel.sendIntent(CellViewIntent.OnItemClick(nonImageFile)) coVerify(exactly = 1) { arrangement.fileHelper.openAssetFileWithExternalApp(any(), any(), any(), any()) } } + @Test + fun `given view model when pdf file clicked and local file is present then in-app pdf viewer is opened`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withLoadSuccess() + .arrange() + + val pdfFile = testFiles[0].copy(mimeType = "application/pdf").toUiModel() + + viewModel.actions.test { + viewModel.sendIntent(CellViewIntent.OnItemClick(pdfFile)) + + val action = awaitItem() + assert(action is OpenPdfViewer) + } + coVerify(exactly = 0) { arrangement.fileHelper.openAssetFileWithExternalApp(any(), any(), any(), any()) } + } + @Test fun `given in-app image viewer disabled when image file clicked and local file is not present and url is openable then url is opened`() = runTest { val (arrangement, viewModel) = Arrangement() @@ -316,7 +333,7 @@ class CellViewModelTest { .arrange() val testFile = testFiles[0].copy( - mimeType = "application/pdf", + mimeType = "text/plain", localPath = null, contentUrl = "https://example.com/file" ) @@ -326,6 +343,27 @@ class CellViewModelTest { coVerify(exactly = 1) { arrangement.fileHelper.openAssetUrlWithExternalApp(any(), any(), any()) } } + @Test + fun `given view model when pdf file clicked and only url is available then in-app pdf viewer is opened`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withLoadSuccess() + .arrange() + + val testFile = testFiles[0].copy( + mimeType = "application/pdf", + localPath = null, + contentUrl = "https://example.com/file" + ) + + viewModel.actions.test { + viewModel.sendIntent(CellViewIntent.OnItemClick(testFile.toUiModel())) + + val action = awaitItem() + assert(action is OpenPdfViewer) + } + coVerify(exactly = 0) { arrangement.fileHelper.openAssetUrlWithExternalApp(any(), any(), any()) } + } + @Test fun `given view model when file clicked and local file is not present and url is not openable then download starts immediately`() = runTest { From 3376a3f341e7d025f125409fe6af84ae74b364e7 Mon Sep 17 00:00:00 2001 From: ohassine Date: Mon, 31 Aug 2026 11:09:41 +0200 Subject: [PATCH 02/12] chore: unit test --- .../wire/android/pdfviewer/PageBitmapCache.kt | 69 +++++ .../android/pdfviewer/PdfViewerViewModel.kt | 43 +-- .../android/pdfviewer/PageBitmapCacheTest.kt | 108 +++++++ .../pdfviewer/PdfViewerViewModelTest.kt | 270 ++++++++++++++++++ 4 files changed, 460 insertions(+), 30 deletions(-) create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PageBitmapCache.kt create mode 100644 core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PageBitmapCacheTest.kt create mode 100644 core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PageBitmapCache.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PageBitmapCache.kt new file mode 100644 index 00000000000..d530edb7f39 --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PageBitmapCache.kt @@ -0,0 +1,69 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.graphics.Bitmap + +/** + * Least-recently-used cache of rendered pages, bounded by the total bitmap size in bytes rather + * than by a page count, because page bitmaps vary a lot in size. + * + * Deliberately not [android.util.LruCache]: that one is stubbed out in unit tests, and the byte + * accounting is the part worth covering. + */ +internal class PageBitmapCache(private val maxBytes: Long) { + + private val entries = LinkedHashMap(0, LOAD_FACTOR, true) + private var currentBytes = 0L + + @Synchronized + fun get(key: String): Bitmap? = entries[key] + + @Synchronized + fun put(key: String, bitmap: Bitmap) { + entries.put(key, bitmap)?.let { replaced -> currentBytes -= replaced.byteCount } + currentBytes += bitmap.byteCount + trimToSize() + } + + @Synchronized + fun clear() { + entries.clear() + currentBytes = 0 + } + + @Synchronized + fun size(): Int = entries.size + + /** Drops the least recently used entries until the cache fits again, always keeping the newest. */ + private fun trimToSize() { + val iterator = entries.entries.iterator() + while (currentBytes > maxBytes && entries.size > 1 && iterator.hasNext()) { + currentBytes -= iterator.next().value.byteCount + iterator.remove() + } + } + + companion object { + private const val LOAD_FACTOR = 0.75f + private const val HEAP_FRACTION = 8 + + /** Roughly an eighth of the heap, the same budget the platform LRU caches usually take. */ + fun defaultMaxBytes(): Long = (Runtime.getRuntime().maxMemory() / HEAP_FRACTION).coerceAtLeast(1) + } +} diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt index 37129830012..26614ae12fb 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt @@ -18,15 +18,13 @@ package com.wire.android.pdfviewer import android.graphics.Bitmap -import android.util.LruCache import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.wire.android.di.metro.WireAssistedViewModelBinding +import com.wire.android.util.dispatchers.DispatcherProvider import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedFactory import dev.zacsweers.metro.AssistedInject -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -45,6 +43,7 @@ import java.io.IOException @WireAssistedViewModelBinding(PdfViewerManualViewModelFactoryGroup::class) class PdfViewerViewModel @AssistedInject constructor( private val sourceResolver: PdfSourceResolver, + private val dispatchers: DispatcherProvider, @Assisted val localPath: String?, @Assisted val contentUrl: String?, @Assisted val fileName: String?, @@ -61,13 +60,8 @@ class PdfViewerViewModel @AssistedInject constructor( private var document: PdfDocument? = null private var loadJob: Job? = null - /** - * Keeps recently rendered pages around so scrolling back does not re-rasterise them. - * Sized against the heap rather than a page count because page bitmaps vary a lot in size. - */ - private val pageCache = object : LruCache(cacheSizeKb()) { - override fun sizeOf(key: String, value: Bitmap): Int = value.byteCount / BYTES_IN_KB - } + /** Keeps recently rendered pages around so scrolling back does not re-rasterise them. */ + private val pageCache = PageBitmapCache(PageBitmapCache.defaultMaxBytes()) init { load() @@ -88,7 +82,7 @@ class PdfViewerViewModel @AssistedInject constructor( val key = "$pageIndex@$widthPx" pageCache.get(key)?.let { return it } - val rendered = withContext(renderDispatcher) { + val rendered = withContext(dispatchers.io()) { runCatching { current.renderPage(pageIndex, widthPx) }.getOrNull() } ?: return null @@ -101,13 +95,13 @@ class PdfViewerViewModel @AssistedInject constructor( closeDocument() _state.value = PdfViewerState.Loading loadJob = viewModelScope.launch { - val file = sourceResolver.resolve(localPath, contentUrl, renderDispatcher) + val file = sourceResolver.resolve(localPath, contentUrl, dispatchers.io()) .getOrElse { cause -> _state.value = PdfViewerState.Failure(cause.toViewerError()) return@launch } - val opened = withContext(renderDispatcher) { PdfDocument.open(file) } + val opened = withContext(dispatchers.io()) { PdfDocument.open(file) } .getOrElse { cause -> _state.value = PdfViewerState.Failure(cause.toViewerError()) return@launch @@ -122,13 +116,13 @@ class PdfViewerViewModel @AssistedInject constructor( document = opened _state.value = PdfViewerState.Content( pageCount = opened.pageCount, - firstPageAspectRatio = withContext(renderDispatcher) { opened.aspectRatio(0) }, + firstPageAspectRatio = withContext(dispatchers.io()) { opened.aspectRatio(0) }, ) } } private fun closeDocument() { - pageCache.evictAll() + pageCache.clear() document?.close() document = null } @@ -138,22 +132,11 @@ class PdfViewerViewModel @AssistedInject constructor( loadJob?.cancel() closeDocument() } - - private companion object { - val renderDispatcher: CoroutineDispatcher = Dispatchers.IO - const val BYTES_IN_KB = 1024 - const val CACHE_HEAP_FRACTION = 8 - - fun cacheSizeKb(): Int = - (Runtime.getRuntime().maxMemory() / BYTES_IN_KB / CACHE_HEAP_FRACTION) - .coerceAtLeast(1) - .toInt() - } } -private fun Throwable.toViewerError(): PdfViewerError = when { - this is PdfSourceException -> error - this is SecurityException -> PdfViewerError.PASSWORD_PROTECTED - this is IOException -> PdfViewerError.INVALID_DOCUMENT +private fun Throwable.toViewerError(): PdfViewerError = when (this) { + is PdfSourceException -> error + is SecurityException -> PdfViewerError.PASSWORD_PROTECTED + is IOException -> PdfViewerError.INVALID_DOCUMENT else -> PdfViewerError.INVALID_DOCUMENT } diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PageBitmapCacheTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PageBitmapCacheTest.kt new file mode 100644 index 00000000000..d59877c32ed --- /dev/null +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PageBitmapCacheTest.kt @@ -0,0 +1,108 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.graphics.Bitmap +import io.mockk.every +import io.mockk.mockk +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Test + +internal class PageBitmapCacheTest { + + @Test + fun `given a cached page, when reading it back, then the same bitmap is returned`() { + val cache = PageBitmapCache(maxBytes = 1_000) + val page = bitmapOf(100) + + cache.put("0@100", page) + + assertSame(page, cache.get("0@100")) + assertNull(cache.get("1@100")) + } + + @Test + fun `given the budget is exceeded, when adding a page, then the least recently used one is dropped`() { + val cache = PageBitmapCache(maxBytes = 250) + cache.put("0", bitmapOf(100)) + cache.put("1", bitmapOf(100)) + + cache.put("2", bitmapOf(100)) + + assertNull(cache.get("0")) + assertNotNull(cache.get("1")) + assertNotNull(cache.get("2")) + } + + @Test + fun `given a page was read recently, when the budget is exceeded, then the other one is dropped first`() { + val cache = PageBitmapCache(maxBytes = 250) + cache.put("0", bitmapOf(100)) + cache.put("1", bitmapOf(100)) + + cache.get("0") + cache.put("2", bitmapOf(100)) + + assertNotNull(cache.get("0")) + assertNull(cache.get("1")) + } + + @Test + fun `given a page larger than the whole budget, when adding it, then it is still served`() { + val cache = PageBitmapCache(maxBytes = 10) + val huge = bitmapOf(5_000) + + cache.put("0", huge) + + assertSame(huge, cache.get("0")) + assertEquals(1, cache.size()) + } + + @Test + fun `given a replaced page, when accounting for the budget, then the old size is released`() { + val cache = PageBitmapCache(maxBytes = 250) + cache.put("0", bitmapOf(200)) + cache.put("0", bitmapOf(100)) + + cache.put("1", bitmapOf(100)) + + assertNotNull(cache.get("0")) + assertNotNull(cache.get("1")) + assertEquals(2, cache.size()) + } + + @Test + fun `given cached pages, when clearing, then nothing is served and the budget is free again`() { + val cache = PageBitmapCache(maxBytes = 250) + cache.put("0", bitmapOf(200)) + + cache.clear() + + assertNull(cache.get("0")) + assertEquals(0, cache.size()) + cache.put("1", bitmapOf(200)) + assertNotNull(cache.get("1")) + } + + private fun bitmapOf(bytes: Int): Bitmap = mockk(relaxed = true).also { + every { it.byteCount } returns bytes + } +} diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt new file mode 100644 index 00000000000..2c6d2fb1563 --- /dev/null +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt @@ -0,0 +1,270 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import android.graphics.Bitmap +import com.wire.android.config.CoroutineTestExtension +import com.wire.android.config.TestDispatcherProvider +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkObject +import io.mockk.verify +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import java.io.File +import java.io.IOException + +@ExtendWith(CoroutineTestExtension::class) +internal class PdfViewerViewModelTest { + + @AfterEach + fun tearDown() { + unmockkObject(PdfDocument.Companion) + } + + @Test + fun `given the document opens, when the view model is created, then the state exposes the page count`() = runTest { + val (_, viewModel) = Arrangement() + .withPageCount(7) + .withFirstPageAspectRatio(0.5f) + .arrange() + + assertEquals(PdfViewerState.Content(pageCount = 7, firstPageAspectRatio = 0.5f), viewModel.state.value) + } + + @Test + fun `given the source cannot be resolved, when the view model is created, then that error is exposed`() = runTest { + val (_, viewModel) = Arrangement() + .withResolveFailure(PdfViewerError.FILE_NOT_FOUND) + .arrange() + + assertEquals(PdfViewerState.Failure(PdfViewerError.FILE_NOT_FOUND), viewModel.state.value) + } + + @Test + fun `given the download fails, when the view model is created, then the failure is reported as a download error`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withResolveFailure(PdfViewerError.DOWNLOAD_FAILED) + .arrange() + + assertEquals(PdfViewerState.Failure(PdfViewerError.DOWNLOAD_FAILED), viewModel.state.value) + verify(exactly = 0) { PdfDocument.open(any()) } + coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any()) } + } + + @Test + fun `given the document is password protected, when opening it, then the state reports it as protected`() = runTest { + val (_, viewModel) = Arrangement() + .withOpenFailure(SecurityException("password required")) + .arrange() + + assertEquals(PdfViewerState.Failure(PdfViewerError.PASSWORD_PROTECTED), viewModel.state.value) + } + + @Test + fun `given the bytes cannot be parsed, when opening the document, then the state reports an invalid document`() = runTest { + val (_, viewModel) = Arrangement() + .withOpenFailure(IOException("not a pdf")) + .arrange() + + assertEquals(PdfViewerState.Failure(PdfViewerError.INVALID_DOCUMENT), viewModel.state.value) + } + + @Test + fun `given a document without pages, when opening it, then it is closed and reported as invalid`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withPageCount(0) + .arrange() + + assertEquals(PdfViewerState.Failure(PdfViewerError.INVALID_DOCUMENT), viewModel.state.value) + verify(exactly = 1) { arrangement.document.close() } + } + + @Test + fun `given an open document, when rendering the same page twice, then the second call comes from the cache`() = runTest { + val (arrangement, viewModel) = Arrangement().arrange() + + val first = viewModel.renderPage(pageIndex = 0, widthPx = 100) + val second = viewModel.renderPage(pageIndex = 0, widthPx = 100) + + assertSame(arrangement.bitmap, first) + assertSame(first, second) + coVerify(exactly = 1) { arrangement.document.renderPage(0, 100) } + } + + @Test + fun `given an open document, when the requested width changes, then the page is rendered again`() = runTest { + val (arrangement, viewModel) = Arrangement().arrange() + + viewModel.renderPage(pageIndex = 0, widthPx = 100) + viewModel.renderPage(pageIndex = 0, widthPx = 200) + + coVerify(exactly = 1) { arrangement.document.renderPage(0, 100) } + coVerify(exactly = 1) { arrangement.document.renderPage(0, 200) } + } + + @Test + fun `given a non positive width, when rendering, then nothing is rendered`() = runTest { + val (arrangement, viewModel) = Arrangement().arrange() + + assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 0)) + assertNull(viewModel.renderPage(pageIndex = 0, widthPx = -10)) + + coVerify(exactly = 0) { arrangement.document.renderPage(any(), any()) } + } + + @Test + fun `given the document failed to open, when rendering, then no bitmap is returned`() = runTest { + val (_, viewModel) = Arrangement() + .withOpenFailure(IOException("not a pdf")) + .arrange() + + assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) + } + + @Test + fun `given rendering a page fails, when rendering it again, then nothing was cached`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withRenderedPage(null) + .arrange() + + assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) + assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) + + coVerify(exactly = 2) { arrangement.document.renderPage(0, 100) } + } + + @Test + fun `given rendering throws, when rendering, then the failure is swallowed and no bitmap is returned`() = runTest { + val (_, viewModel) = Arrangement() + .withRenderFailure(OutOfMemoryError("bitmap too large")) + .arrange() + + assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) + } + + @Test + fun `given a failed load, when retrying, then the document is opened again`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withResolveFailure(PdfViewerError.DOWNLOAD_FAILED) + .arrange() + + arrangement.withResolveSuccess().withPageCount(3) + viewModel.retry() + + assertEquals(PdfViewerState.Content(pageCount = 3, firstPageAspectRatio = DEFAULT_ASPECT_RATIO), viewModel.state.value) + coVerify(exactly = 2) { arrangement.sourceResolver.resolve(any(), any(), any()) } + } + + @Test + fun `given a load already in flight, when retrying, then the second load is ignored`() = runTest { + val gate = CompletableDeferred() + val (arrangement, viewModel) = Arrangement() + .withResolveGatedBy(gate) + .arrange() + + assertEquals(PdfViewerState.Loading, viewModel.state.value) + viewModel.retry() + coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any()) } + + gate.complete(Unit) + + assertEquals( + PdfViewerState.Content(pageCount = DEFAULT_PAGE_COUNT, firstPageAspectRatio = DEFAULT_ASPECT_RATIO), + viewModel.state.value, + ) + coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any()) } + } + + private class Arrangement { + + val sourceResolver: PdfSourceResolver = mockk() + val document: PdfDocument = mockk(relaxed = true) + val bitmap: Bitmap = mockk(relaxed = true).also { every { it.byteCount } returns BITMAP_BYTES } + + private val file = File("document.pdf") + + init { + mockkObject(PdfDocument.Companion) + every { PdfDocument.open(any()) } returns Result.success(document) + every { document.pageCount } returns DEFAULT_PAGE_COUNT + coEvery { document.aspectRatio(any()) } returns DEFAULT_ASPECT_RATIO + coEvery { document.renderPage(any(), any()) } returns bitmap + withResolveSuccess() + } + + fun withResolveSuccess() = apply { + coEvery { sourceResolver.resolve(any(), any(), any()) } returns Result.success(file) + } + + fun withResolveFailure(error: PdfViewerError) = apply { + coEvery { sourceResolver.resolve(any(), any(), any()) } returns + Result.failure(PdfSourceException(error)) + } + + fun withResolveGatedBy(gate: CompletableDeferred) = apply { + coEvery { sourceResolver.resolve(any(), any(), any()) } coAnswers { + gate.await() + Result.success(file) + } + } + + fun withOpenFailure(cause: Throwable) = apply { + every { PdfDocument.open(any()) } returns Result.failure(cause) + } + + fun withPageCount(count: Int) = apply { + every { document.pageCount } returns count + } + + fun withFirstPageAspectRatio(ratio: Float) = apply { + coEvery { document.aspectRatio(0) } returns ratio + } + + fun withRenderedPage(bitmap: Bitmap?) = apply { + coEvery { document.renderPage(any(), any()) } returns bitmap + } + + fun withRenderFailure(cause: Throwable) = apply { + coEvery { document.renderPage(any(), any()) } throws cause + } + + fun arrange(): Pair = this to PdfViewerViewModel( + sourceResolver = sourceResolver, + dispatchers = TestDispatcherProvider(), + localPath = "local/document.pdf", + contentUrl = null, + fileName = "document.pdf", + ) + } + + private companion object { + const val DEFAULT_PAGE_COUNT = 3 + const val DEFAULT_ASPECT_RATIO = 0.7f + const val BITMAP_BYTES = 1024 + } +} From 4f4a5f7d9859a9635bd34fbbd151856489767cf4 Mon Sep 17 00:00:00 2001 From: ohassine Date: Tue, 1 Sep 2026 11:01:45 +0200 Subject: [PATCH 03/12] fix: race condition on pdf close --- .../com/wire/android/pdfviewer/PdfDocument.kt | 33 +++++++++++---- .../android/pdfviewer/PdfSourceResolver.kt | 39 +++++++++++++++-- .../android/pdfviewer/PdfViewerViewModel.kt | 13 +++++- .../pdfviewer/PdfSourceResolverTest.kt | 37 ++++++++++++++++ .../pdfviewer/PdfViewerViewModelTest.kt | 42 ++++++++++++++----- 5 files changed, 141 insertions(+), 23 deletions(-) diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt index d0311f8a60d..386bb608e03 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt @@ -21,10 +21,10 @@ import android.graphics.Bitmap import android.graphics.Color import android.graphics.pdf.PdfRenderer import android.os.ParcelFileDescriptor -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock import java.io.Closeable import java.io.File +import java.util.concurrent.locks.ReentrantLock +import kotlin.concurrent.withLock /** * Thin coroutine-friendly wrapper around the platform [PdfRenderer]. @@ -38,13 +38,22 @@ internal class PdfDocument private constructor( private val renderer: PdfRenderer, ) : Closeable { - private val mutex = Mutex() + /** + * Guards every access to [renderer], including [close]. + * + * A blocking lock rather than a coroutine `Mutex` on purpose: [close] is not a suspending + * function, so it could never have joined a `Mutex`, and releasing the native handle while a + * render is in flight crashes inside PdfRenderer — below the level any `runCatching` could + * recover from. Callers must therefore be off the main thread; [PdfViewerViewModel] dispatches + * all of them to IO. + */ + private val lock = ReentrantLock() private var closed = false val pageCount: Int = renderer.pageCount /** Width / height of [pageIndex], used to reserve the right amount of space before rendering. */ - suspend fun aspectRatio(pageIndex: Int): Float = mutex.withLock { + fun aspectRatio(pageIndex: Int): Float = lock.withLock { if (closed) return DEFAULT_ASPECT_RATIO renderer.openPage(pageIndex).use { page -> if (page.height == 0) DEFAULT_ASPECT_RATIO else page.width.toFloat() / page.height @@ -56,7 +65,7 @@ internal class PdfDocument private constructor( * * Returns `null` when the document was closed while the caller was waiting for the lock. */ - suspend fun renderPage(pageIndex: Int, widthPx: Int): Bitmap? = mutex.withLock { + fun renderPage(pageIndex: Int, widthPx: Int): Bitmap? = lock.withLock { if (closed) return null renderer.openPage(pageIndex).use { page -> val safeWidth = widthPx.coerceIn(MIN_RENDER_WIDTH_PX, MAX_RENDER_WIDTH_PX) @@ -74,11 +83,17 @@ internal class PdfDocument private constructor( } } + /** + * Releases the renderer and the file descriptor. Blocks until any in-flight page has finished + * rendering, so it must not be called from the main thread. + */ override fun close() { - if (closed) return - closed = true - runCatching { renderer.close() } - runCatching { descriptor.close() } + lock.withLock { + if (closed) return + closed = true + runCatching { renderer.close() } + runCatching { descriptor.close() } + } } companion object { diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt index 3f3390aa9eb..7dfc1daef73 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt @@ -24,9 +24,10 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import java.io.File -import java.net.HttpURLConnection +import java.net.InetAddress import java.net.URL import java.security.MessageDigest +import javax.net.ssl.HttpsURLConnection /** * Turns the arguments of the PDF screen into a readable local file. @@ -34,6 +35,11 @@ import java.security.MessageDigest * [android.graphics.pdf.PdfRenderer] needs a seekable file descriptor, so a remote document has * to be fetched into the app cache first. Already downloaded files are reused, which keeps * re-opening the same attachment instant. + * + * Remote [contentUrl] values are always pre-signed object-storage URLs whose authentication + * credentials are embedded in the URL query parameters (see [CellNodeDTO.preSignedGET]). + * No additional auth headers are required. [validateUrl] enforces HTTPS-only and blocks + * requests to private/loopback/link-local addresses to prevent SSRF. */ class PdfSourceResolver @Inject constructor( @ApplicationContext private val context: Context, @@ -53,15 +59,17 @@ class PdfSourceResolver @Inject constructor( } private fun download(contentUrl: String): Result { + validateUrl(contentUrl).onFailure { return Result.failure(it) } + val target = cacheFileFor(contentUrl) if (target.isReadableFile()) return Result.success(target) val partial = File(target.parentFile, "${target.name}$PARTIAL_SUFFIX") return runCatching { - val connection = (URL(contentUrl).openConnection() as HttpURLConnection).apply { + val connection = (URL(contentUrl).openConnection() as HttpsURLConnection).apply { connectTimeout = TIMEOUT_MS readTimeout = TIMEOUT_MS - instanceFollowRedirects = true + instanceFollowRedirects = false } try { if (connection.responseCode !in HTTP_OK_RANGE) { @@ -82,6 +90,31 @@ class PdfSourceResolver @Inject constructor( } } + /** + * Guards against SSRF by enforcing HTTPS and blocking requests to private/loopback/link-local + * addresses. The host is resolved once here; the connection later may re-resolve, but this + * catches the most common cases (literal IP addresses and predictable hostnames). + */ + private fun validateUrl(contentUrl: String): Result { + val url = runCatching { URL(contentUrl) }.getOrElse { + return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED, it)) + } + if (url.protocol != "https") { + return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED)) + } + val host = url.host?.takeIf { it.isNotEmpty() } + ?: return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED)) + val address = runCatching { InetAddress.getByName(host) }.getOrElse { + return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED, it)) + } + if (address.isLoopbackAddress || address.isLinkLocalAddress || + address.isSiteLocalAddress || address.isAnyLocalAddress + ) { + return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED)) + } + return Result.success(Unit) + } + private fun cacheFileFor(contentUrl: String): File { val digest = MessageDigest.getInstance("SHA-256") .digest(contentUrl.toByteArray()) diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt index 26614ae12fb..efe6d44a13c 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt @@ -25,7 +25,9 @@ import com.wire.android.util.dispatchers.DispatcherProvider import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedFactory import dev.zacsweers.metro.AssistedInject +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -57,9 +59,17 @@ class PdfViewerViewModel @AssistedInject constructor( private val _state = MutableStateFlow(PdfViewerState.Loading) val state: StateFlow = _state.asStateFlow() + @Volatile private var document: PdfDocument? = null private var loadJob: Job? = null + /** + * Releasing the document has to outlive [viewModelScope]: [PdfDocument.close] waits for an + * in-flight render before it frees the native handle, and that wait must neither block the + * main thread nor be cancelled halfway through. + */ + private val releaseScope = CoroutineScope(SupervisorJob() + dispatchers.io()) + /** Keeps recently rendered pages around so scrolling back does not re-rasterise them. */ private val pageCache = PageBitmapCache(PageBitmapCache.defaultMaxBytes()) @@ -123,8 +133,9 @@ class PdfViewerViewModel @AssistedInject constructor( private fun closeDocument() { pageCache.clear() - document?.close() + val open = document ?: return document = null + releaseScope.launch { open.close() } } override fun onCleared() { diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt index ccacb4816f5..7581f164607 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt @@ -71,6 +71,43 @@ internal class PdfSourceResolverTest { assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) } + @Test + fun givenAnHttpUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { + val resolver = resolver() + + val result = resolver.resolve(localPath = null, contentUrl = "http://example.com/doc.pdf", dispatcher = Dispatchers.Default) + + assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + } + + @Test + fun givenALoopbackUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { + val resolver = resolver() + + val result = resolver.resolve(localPath = null, contentUrl = "https://127.0.0.1/doc.pdf", dispatcher = Dispatchers.Default) + + assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + } + + @Test + fun givenAPrivateNetworkUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { + val resolver = resolver() + + val result = resolver.resolve(localPath = null, contentUrl = "https://192.168.1.1/doc.pdf", dispatcher = Dispatchers.Default) + + assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + } + + @Test + fun givenALinkLocalUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { + val resolver = resolver() + + // AWS/GCP metadata endpoint commonly used in SSRF attacks + val result = resolver.resolve(localPath = null, contentUrl = "https://169.254.169.254/latest/meta-data/", dispatcher = Dispatchers.Default) + + assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + } + @Test fun givenAMissingLocalPathAndAUrl_whenResolving_thenTheDownloadPathIsUsed() = runTest { val resolver = resolver() diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt index 2c6d2fb1563..0655de8eb1d 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt @@ -113,7 +113,7 @@ internal class PdfViewerViewModelTest { assertSame(arrangement.bitmap, first) assertSame(first, second) - coVerify(exactly = 1) { arrangement.document.renderPage(0, 100) } + verify(exactly = 1) { arrangement.document.renderPage(0, 100) } } @Test @@ -123,8 +123,8 @@ internal class PdfViewerViewModelTest { viewModel.renderPage(pageIndex = 0, widthPx = 100) viewModel.renderPage(pageIndex = 0, widthPx = 200) - coVerify(exactly = 1) { arrangement.document.renderPage(0, 100) } - coVerify(exactly = 1) { arrangement.document.renderPage(0, 200) } + verify(exactly = 1) { arrangement.document.renderPage(0, 100) } + verify(exactly = 1) { arrangement.document.renderPage(0, 200) } } @Test @@ -134,7 +134,7 @@ internal class PdfViewerViewModelTest { assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 0)) assertNull(viewModel.renderPage(pageIndex = 0, widthPx = -10)) - coVerify(exactly = 0) { arrangement.document.renderPage(any(), any()) } + verify(exactly = 0) { arrangement.document.renderPage(any(), any()) } } @Test @@ -155,7 +155,7 @@ internal class PdfViewerViewModelTest { assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) - coVerify(exactly = 2) { arrangement.document.renderPage(0, 100) } + verify(exactly = 2) { arrangement.document.renderPage(0, 100) } } @Test @@ -167,6 +167,28 @@ internal class PdfViewerViewModelTest { assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) } + @Test + fun `given a loaded document, when reloading, then the previous one is closed and a new one is opened`() = runTest { + val (arrangement, viewModel) = Arrangement().arrange() + + viewModel.retry() + + verify(exactly = 1) { arrangement.document.close() } + verify(exactly = 2) { PdfDocument.open(any()) } + } + + @Test + fun `given the document was closed by a reload that then failed, when rendering, then nothing is returned`() = runTest { + val (arrangement, viewModel) = Arrangement().arrange() + + arrangement.withOpenFailure(IOException("gone")) + viewModel.retry() + + // The old document is detached before it is closed, so no render can reach a closed renderer. + assertNull(viewModel.renderPage(pageIndex = 0, widthPx = 100)) + verify(exactly = 0) { arrangement.document.renderPage(any(), any()) } + } + @Test fun `given a failed load, when retrying, then the document is opened again`() = runTest { val (arrangement, viewModel) = Arrangement() @@ -212,8 +234,8 @@ internal class PdfViewerViewModelTest { mockkObject(PdfDocument.Companion) every { PdfDocument.open(any()) } returns Result.success(document) every { document.pageCount } returns DEFAULT_PAGE_COUNT - coEvery { document.aspectRatio(any()) } returns DEFAULT_ASPECT_RATIO - coEvery { document.renderPage(any(), any()) } returns bitmap + every { document.aspectRatio(any()) } returns DEFAULT_ASPECT_RATIO + every { document.renderPage(any(), any()) } returns bitmap withResolveSuccess() } @@ -242,15 +264,15 @@ internal class PdfViewerViewModelTest { } fun withFirstPageAspectRatio(ratio: Float) = apply { - coEvery { document.aspectRatio(0) } returns ratio + every { document.aspectRatio(0) } returns ratio } fun withRenderedPage(bitmap: Bitmap?) = apply { - coEvery { document.renderPage(any(), any()) } returns bitmap + every { document.renderPage(any(), any()) } returns bitmap } fun withRenderFailure(cause: Throwable) = apply { - coEvery { document.renderPage(any(), any()) } throws cause + every { document.renderPage(any(), any()) } throws cause } fun arrange(): Pair = this to PdfViewerViewModel( From 88cdf32e4e84455805368f5411a21dd6cca707bf Mon Sep 17 00:00:00 2001 From: ohassine Date: Tue, 1 Sep 2026 16:29:48 +0200 Subject: [PATCH 04/12] refactor: replace custom HTTP download with DownloadCellFileUseCase via PdfRemoteLoader --- .../android/di/accountScoped/CellsModule.kt | 20 ++++ .../routes/media/MediaNavigation3Entries.kt | 5 +- .../navigation/routes/media/MediaRoutes.kt | 9 +- .../WireNavigation3ProductionActions.kt | 5 +- .../common/multipart/MultipartAttachmentUi.kt | 2 + .../ConversationMessageComposer.kt | 2 +- .../ConversationNavigation3Entries.kt | 11 ++- .../conversations/ConversationRouteScreen.kt | 2 +- .../home/conversations/ConversationScreen.kt | 4 +- .../messages/item/MessageClickActions.kt | 4 +- .../messages/item/MessageContentAndStatus.kt | 4 +- .../multipart/MultipartAttachmentsView.kt | 4 +- .../MultipartAttachmentsViewModel.kt | 5 +- .../MultipartAttachmentsViewModelTest.kt | 4 +- .../wire/android/pdfviewer/PdfRemoteLoader.kt | 47 +++++++++ .../android/pdfviewer/PdfSourceResolver.kt | 87 +++++------------ .../com/wire/android/pdfviewer/PdfViewer.kt | 11 ++- .../android/pdfviewer/PdfViewerViewModel.kt | 21 +++- .../pdfviewer/PdfViewerViewModelGraph.kt | 9 +- .../pdfviewer/PdfSourceResolverTest.kt | 96 ++++++++++--------- .../pdfviewer/PdfViewerViewModelTest.kt | 19 ++-- .../cells/navigation/CellsNavigation3.kt | 5 +- .../navigation/CellsNavigation3Renderer.kt | 15 ++- 23 files changed, 238 insertions(+), 153 deletions(-) create mode 100644 core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfRemoteLoader.kt diff --git a/app/src/main/kotlin/com/wire/android/di/accountScoped/CellsModule.kt b/app/src/main/kotlin/com/wire/android/di/accountScoped/CellsModule.kt index e0bed7382b4..80515214ace 100644 --- a/app/src/main/kotlin/com/wire/android/di/accountScoped/CellsModule.kt +++ b/app/src/main/kotlin/com/wire/android/di/accountScoped/CellsModule.kt @@ -21,6 +21,7 @@ import com.wire.android.di.CurrentAccount import com.wire.android.di.KaliumCoreLogic import com.wire.android.feature.cells.util.FileNameResolver import com.wire.android.ui.home.conversations.model.messagetypes.multipart.CellAssetRefreshHelper +import com.wire.android.pdfviewer.PdfRemoteLoader import com.wire.kalium.cells.CellsScope import com.wire.kalium.cells.domain.CellUploadManager import com.wire.kalium.cells.domain.usecase.AddAttachmentDraftUseCase @@ -72,11 +73,14 @@ import com.wire.kalium.cells.domain.usecase.versioning.GetNodeVersionsUseCase import com.wire.kalium.cells.domain.usecase.versioning.RestoreNodeVersionUseCase import com.wire.kalium.cells.paginatedConversationsFlowUseCase import com.wire.kalium.cells.paginatedFilesFlowUseCase +import com.wire.kalium.common.functional.fold import com.wire.kalium.logic.CoreLogic import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.featureFlags.KaliumConfigs import dev.zacsweers.metro.BindingContainer import dev.zacsweers.metro.Provides +import java.io.IOException +import okio.Path.Companion.toOkioPath @Suppress("TooManyFunctions") @BindingContainer @@ -253,4 +257,20 @@ class CellsModule { @Provides fun provideGetUserNamesUseCase(cellsScope: CellsScope): GetUserNameUseCase = cellsScope.getUserName + + @Provides + fun providePdfRemoteLoader(download: DownloadCellFileUseCase): PdfRemoteLoader = + PdfRemoteLoader { assetId, remotePath, conversationId, assetSize, outFile -> + download( + assetId = assetId, + conversationId = conversationId, + outFilePath = outFile.toPath().toOkioPath(), + assetSize = assetSize, + remoteFilePath = remotePath, + onProgressUpdate = {}, + ).fold( + { failure -> Result.failure(IOException("PDF download failed: $failure")) }, + { Result.success(Unit) }, + ) + } } diff --git a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt index 7dc9cedec34..fb61869064b 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaNavigation3Entries.kt @@ -126,7 +126,10 @@ internal fun mediaNavigation3Entries( wireEntry(presentation = WireEntryPresentation.PopUp) { route -> PdfViewer( localPath = route.localPath, - contentUrl = route.contentUrl, + assetId = route.assetId, + remotePath = route.remotePath, + conversationId = route.conversationId, + assetSize = route.assetSize, fileName = route.fileName, onNavigateBack = runtime.navigator::goBack, ) diff --git a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt index 450e4df5c0a..ba4e9dd34d9 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/routes/media/MediaRoutes.kt @@ -78,9 +78,12 @@ data class VideoPlayerRoute( @Serializable data class PdfViewerRoute( override val sessionId: WireSessionId, - val localPath: String?, - val contentUrl: String?, - val fileName: String?, + val localPath: String? = null, + val assetId: String? = null, + val remotePath: String? = null, + val conversationId: String? = null, + val assetSize: Long = 0L, + val fileName: String? = null, override val entryId: WireNavEntryId = WireNavEntryId.random(), ) : SessionRoute { override val routeId = ROUTE_ID diff --git a/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt b/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt index 735fc625212..bd7ce44021d 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/runtime/WireNavigation3ProductionActions.kt @@ -202,7 +202,10 @@ internal class WireNavigation3ProductionActions( PdfViewerRoute( sessionId = requireSession(), localPath = it.localPath, - contentUrl = it.contentUrl, + assetId = it.uuid, + remotePath = it.remotePath, + conversationId = it.conversationId, + assetSize = it.size ?: 0L, fileName = it.name, ) ) diff --git a/app/src/main/kotlin/com/wire/android/ui/common/multipart/MultipartAttachmentUi.kt b/app/src/main/kotlin/com/wire/android/ui/common/multipart/MultipartAttachmentUi.kt index f036b8449b1..7212a94f8b2 100644 --- a/app/src/main/kotlin/com/wire/android/ui/common/multipart/MultipartAttachmentUi.kt +++ b/app/src/main/kotlin/com/wire/android/ui/common/multipart/MultipartAttachmentUi.kt @@ -33,6 +33,7 @@ data class MultipartAttachmentUi( val contentUrl: String? = null, val contentUrlExpiresAt: Long? = null, val previewUrl: String? = null, + val remotePath: String? = null, val mimeType: String, val assetType: AttachmentFileType, val assetSize: Long?, @@ -60,6 +61,7 @@ fun CellAssetContent.toUiModel(progress: Float?, isAvailableOffline: Boolean = f contentUrl = this.contentUrl, contentUrlExpiresAt = this.contentUrlExpiresAt, previewUrl = this.previewUrl, + remotePath = this.assetPath, mimeType = this.mimeType, assetType = AttachmentFileType.fromMimeType(mimeType), assetSize = this.assetSize, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt index 8c88b3b2224..1e7d534e7b6 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationMessageComposer.kt @@ -58,7 +58,7 @@ internal fun ConversationMessageComposer( onAssetItemClicked: (String) -> Unit, onImageFullScreenMode: (UIMessage.Regular, Boolean, String?) -> Unit, onVideoClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onPdfClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfClick: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, onReactionClicked: (String, String) -> Unit, onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit, onOpenProfile: (senderId: MessageSenderId) -> Unit, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt index 0bfd3dddfad..60a9cf43d6b 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt @@ -252,10 +252,17 @@ private fun ConversationNavigation3Entry( ) } - override fun openPdfViewer(localPath: String?, contentUrl: String?, fileName: String?) { + override fun openPdfViewer(localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) { runtime.navigator.navigate( WireNavigationCommand( - PdfViewerRoute(route.sessionId, localPath, contentUrl, fileName) + PdfViewerRoute( + sessionId = route.sessionId, + localPath = localPath, + assetId = assetId, + remotePath = remotePath, + assetSize = assetSize, + fileName = fileName, + ) ) ) } diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt index e7d0493312a..be993a37423 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationRouteScreen.kt @@ -52,7 +52,7 @@ internal interface ConversationRouteScreenNavigation { fun openVideoPlayer(localPath: String?, contentUrl: String?, fileName: String?) - fun openPdfViewer(localPath: String?, contentUrl: String?, fileName: String?) + fun openPdfViewer(localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) fun openDrawingCanvas( conversationName: String, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt index 8ad1a80ddfd..0fc6e210110 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt @@ -643,7 +643,7 @@ private fun ConversationScreenContent( onAssetItemClicked: (String) -> Unit, onImageFullScreenMode: (UIMessage.Regular, Boolean, String?) -> Unit, onVideoClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onPdfClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfClick: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, onStartCall: () -> Unit, onJoinCall: () -> Unit, onReactionClick: (messageId: String, reactionEmoji: String) -> Unit, @@ -899,7 +899,7 @@ fun PreviewConversationScreen() = WireTheme { onAssetItemClicked = { }, onImageFullScreenMode = { _, _, _ -> }, onVideoClick = { _, _, _ -> }, - onPdfClick = { _, _, _ -> }, + onPdfClick = { _, _, _, _, _ -> }, onStartCall = { }, onJoinCall = { }, onReactionClick = { _, _ -> }, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt index f822497c02d..ce08adbe92e 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt @@ -30,7 +30,7 @@ sealed class MessageClickActions { open val onAssetClicked: (String) -> Unit = {} open val onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit = { _, _, _ -> } open val onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> } - open val onPdfClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> } + open val onPdfClicked: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit = { _, _, _, _, _ -> } open val onLinkClicked: (String) -> Unit = {} open val onReplyClicked: (UIMessage.Regular) -> Unit = {} open val onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit = { _, _ -> } @@ -49,7 +49,7 @@ sealed class MessageClickActions { override val onAssetClicked: (String) -> Unit = {}, override val onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit = { _, _, _ -> }, override val onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> }, - override val onPdfClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> }, + override val onPdfClicked: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit = { _, _, _, _, _ -> }, override val onLinkClicked: (String) -> Unit = {}, override val onReplyClicked: (UIMessage.Regular) -> Unit = {}, override val onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit = { _, _ -> }, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt index 3311b7ffaaf..656fcb3477e 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt @@ -73,7 +73,7 @@ internal fun UIMessage.Regular.MessageContentAndStatus( onAssetClicked: (String) -> Unit, onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit, onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onPdfClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfClicked: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, onProfileClicked: (senderId: MessageSenderId) -> Unit, onLinkClicked: (String) -> Unit, onReplyClicked: (UIMessage.Regular) -> Unit, @@ -172,7 +172,7 @@ private fun MessageContent( onImageClick: Clickable, onMultipartImageClick: (String) -> Unit, onMultipartVideoClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onMultipartPdfClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onMultipartPdfClick: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, onOpenProfile: (senderId: MessageSenderId) -> Unit, onLinkClick: (String) -> Unit, onReplyClick: Clickable, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt index 5a0cbceea7c..7d06bfc7234 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt @@ -59,7 +59,7 @@ fun MultipartAttachmentsView( messageStyle: MessageStyle, onImageAttachmentClick: (String) -> Unit, onVideoAttachmentClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onPdfAttachmentClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, + onPdfAttachmentClick: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, modifier: Modifier = Modifier, viewModel: MultipartAttachmentsViewModel = when { LocalInspectionMode.current -> MultipartAttachmentsViewModelPreview @@ -74,7 +74,7 @@ fun MultipartAttachmentsView( attachment = clicked, openInImageViewer = onImageAttachmentClick, openInVideoPlayer = { att -> onVideoAttachmentClick(att.localPath, att.contentUrl, att.fileName) }, - openInPdfViewer = { att -> onPdfAttachmentClick(att.localPath, att.contentUrl, att.fileName) }, + openInPdfViewer = { att -> onPdfAttachmentClick(att.localPath, att.uuid, att.remotePath, att.assetSize ?: 0L, att.fileName) }, ) } diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt index 336a544a4bb..4b3b57ba3f3 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModel.kt @@ -184,7 +184,7 @@ class MultipartAttachmentsViewModelImpl @AssistedInject constructor( attachment.isVideo() && (attachment.localFileAvailable() || attachment.canOpenWithUrl()) -> openInVideoPlayer(attachment) - attachment.isPdf() && (attachment.localFileAvailable() || attachment.canOpenWithUrl()) -> + attachment.isPdf() && (attachment.localFileAvailable() || attachment.canDownloadRemotely()) -> openInPdfViewer(attachment) attachment.localFileAvailable() -> openLocalFile(attachment) @@ -291,4 +291,5 @@ private fun MessageAttachment.isMediaAttachment() = private fun MultipartAttachmentUi.fileNotFound() = transferStatus == AssetTransferStatus.NOT_FOUND private fun MultipartAttachmentUi.localFileAvailable() = localPath != null -private fun MultipartAttachmentUi.canOpenWithUrl() = contentUrl != null && assetType in listOf(IMAGE, VIDEO, PDF) +private fun MultipartAttachmentUi.canOpenWithUrl() = contentUrl != null && assetType in listOf(IMAGE, VIDEO) +private fun MultipartAttachmentUi.canDownloadRemotely() = remotePath != null && assetType == PDF diff --git a/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt index c6f5244957e..237e59bce57 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt @@ -300,7 +300,7 @@ class MultipartAttachmentsViewModelTest { } @Test - fun `with pdf attachment openable via url when clicked then pdf opened in internal viewer`() = runTest { + fun `with pdf attachment openable via remote path when clicked then pdf opened in internal viewer`() = runTest { val (arrangement, viewModel) = Arrangement() .arrange() @@ -308,7 +308,7 @@ class MultipartAttachmentsViewModelTest { val attachment = testAttachmentUi.copy( mimeType = "application/pdf", assetType = AttachmentFileType.PDF, - contentUrl = "content/url", + remotePath = "/cells/path/doc.pdf", ) viewModel.onClick( diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfRemoteLoader.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfRemoteLoader.kt new file mode 100644 index 00000000000..e8d47e19eca --- /dev/null +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfRemoteLoader.kt @@ -0,0 +1,47 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import java.io.File + +/** + * Downloads a remote PDF asset to a local [File]. + * + * Implementations are provided by the host module so that the pdf-viewer module + * stays decoupled from any specific networking or authentication stack. + * In production this is backed by [DownloadCellFileUseCase] which goes through + * the authenticated kalium S3 client. + */ +fun interface PdfRemoteLoader { + /** + * Downloads the asset identified by [assetId] / [remotePath] into [outFile]. + * + * @param assetId UUID of the cell asset. + * @param remotePath S3 object key / remote path of the asset. + * @param conversationId Optional conversation the asset belongs to (used for DB metadata). + * @param assetSize Expected byte size of the asset (used for progress tracking). + * @param outFile Target file to write the downloaded bytes into. + */ + suspend fun load( + assetId: String, + remotePath: String, + conversationId: String?, + assetSize: Long, + outFile: File, + ): Result +} diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt index 7dfc1daef73..a940c105789 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt @@ -24,64 +24,54 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import java.io.File -import java.net.InetAddress -import java.net.URL -import java.security.MessageDigest -import javax.net.ssl.HttpsURLConnection /** * Turns the arguments of the PDF screen into a readable local file. * - * [android.graphics.pdf.PdfRenderer] needs a seekable file descriptor, so a remote document has + * [android.graphics.pdf.PdfRenderer] needs a seekable file descriptor, so a remote asset has * to be fetched into the app cache first. Already downloaded files are reused, which keeps * re-opening the same attachment instant. * - * Remote [contentUrl] values are always pre-signed object-storage URLs whose authentication - * credentials are embedded in the URL query parameters (see [CellNodeDTO.preSignedGET]). - * No additional auth headers are required. [validateUrl] enforces HTTPS-only and blocks - * requests to private/loopback/link-local addresses to prevent SSRF. + * Remote downloads are delegated to [PdfRemoteLoader], which is backed in production by + * `DownloadCellFileUseCase` — the same authenticated kalium S3 client used for offline file + * downloads. This ensures authentication, retry logic, and download progress tracking are + * handled consistently with the rest of the app. */ class PdfSourceResolver @Inject constructor( @ApplicationContext private val context: Context, + private val remoteLoader: PdfRemoteLoader, ) { suspend fun resolve( localPath: String?, - contentUrl: String?, + assetId: String?, + remotePath: String?, + conversationId: String?, + assetSize: Long, dispatcher: CoroutineDispatcher = Dispatchers.IO, ): Result = withContext(dispatcher) { val localFile = localPath?.let(::File) when { localFile != null && localFile.isReadableFile() -> Result.success(localFile) - contentUrl != null -> download(contentUrl) + assetId != null && remotePath != null -> + download(assetId, remotePath, conversationId, assetSize) else -> Result.failure(PdfSourceException(PdfViewerError.FILE_NOT_FOUND)) } } - private fun download(contentUrl: String): Result { - validateUrl(contentUrl).onFailure { return Result.failure(it) } - - val target = cacheFileFor(contentUrl) + private suspend fun download( + assetId: String, + remotePath: String, + conversationId: String?, + assetSize: Long, + ): Result { + val target = cacheFileFor(assetId) if (target.isReadableFile()) return Result.success(target) val partial = File(target.parentFile, "${target.name}$PARTIAL_SUFFIX") return runCatching { - val connection = (URL(contentUrl).openConnection() as HttpsURLConnection).apply { - connectTimeout = TIMEOUT_MS - readTimeout = TIMEOUT_MS - instanceFollowRedirects = false - } - try { - if (connection.responseCode !in HTTP_OK_RANGE) { - error("Unexpected response ${connection.responseCode} while fetching the document") - } - partial.parentFile?.mkdirs() - connection.inputStream.use { input -> - partial.outputStream().use { output -> input.copyTo(output) } - } - } finally { - connection.disconnect() - } + partial.parentFile?.mkdirs() + remoteLoader.load(assetId, remotePath, conversationId, assetSize, partial).getOrThrow() check(partial.renameTo(target)) { "Could not move the downloaded document into place" } target }.recoverCatching { cause -> @@ -90,45 +80,14 @@ class PdfSourceResolver @Inject constructor( } } - /** - * Guards against SSRF by enforcing HTTPS and blocking requests to private/loopback/link-local - * addresses. The host is resolved once here; the connection later may re-resolve, but this - * catches the most common cases (literal IP addresses and predictable hostnames). - */ - private fun validateUrl(contentUrl: String): Result { - val url = runCatching { URL(contentUrl) }.getOrElse { - return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED, it)) - } - if (url.protocol != "https") { - return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED)) - } - val host = url.host?.takeIf { it.isNotEmpty() } - ?: return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED)) - val address = runCatching { InetAddress.getByName(host) }.getOrElse { - return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED, it)) - } - if (address.isLoopbackAddress || address.isLinkLocalAddress || - address.isSiteLocalAddress || address.isAnyLocalAddress - ) { - return Result.failure(PdfSourceException(PdfViewerError.DOWNLOAD_FAILED)) - } - return Result.success(Unit) - } - - private fun cacheFileFor(contentUrl: String): File { - val digest = MessageDigest.getInstance("SHA-256") - .digest(contentUrl.toByteArray()) - .joinToString("") { "%02x".format(it) } - return File(File(context.cacheDir, CACHE_DIR_NAME), "$digest.pdf") - } + private fun cacheFileFor(assetId: String): File = + File(File(context.cacheDir, CACHE_DIR_NAME), "$assetId.pdf") private fun File.isReadableFile(): Boolean = isFile && canRead() && length() > 0 private companion object { const val CACHE_DIR_NAME = "pdf-viewer" const val PARTIAL_SUFFIX = ".part" - const val TIMEOUT_MS = 30_000 - val HTTP_OK_RANGE = 200..299 } } diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt index dfb18997138..d6cb3be7c9d 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt @@ -82,8 +82,8 @@ private const val MAX_RENDER_SCALE = 3f private const val DOUBLE_TAP_ZOOM = 2.5f /** - * Reusable full screen PDF viewer. Shows either a local file ([localPath]) or a remote - * [contentUrl], which is fetched into the app cache before rendering. + * Reusable full screen PDF viewer. Shows either a local file ([localPath]) or a remote asset + * identified by [assetId] and [remotePath], which is fetched into the app cache before rendering. * * Callers own navigation via [onNavigateBack]; the ViewModel is resolved from the shared * pdf-viewer Metro graph so any module can host this screen. @@ -91,11 +91,14 @@ private const val DOUBLE_TAP_ZOOM = 2.5f @Composable fun PdfViewer( localPath: String?, - contentUrl: String?, + assetId: String?, + remotePath: String?, + conversationId: String?, + assetSize: Long, fileName: String?, onNavigateBack: () -> Unit, modifier: Modifier = Modifier, - viewModel: PdfViewerViewModel = pdfViewerViewModel(localPath, contentUrl, fileName), + viewModel: PdfViewerViewModel = pdfViewerViewModel(localPath, assetId, remotePath, conversationId, assetSize, fileName), ) { val state by viewModel.state.collectAsStateWithLifecycle() PdfViewerContent( diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt index efe6d44a13c..276eb366938 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt @@ -36,8 +36,8 @@ import kotlinx.coroutines.withContext import java.io.IOException /** - * Opens a single PDF from either a local file ([localPath]) or a remote [contentUrl] and renders - * its pages on demand. + * Opens a single PDF from either a local file ([localPath]) or a remote asset identified by + * [assetId] and [remotePath], and renders its pages on demand. * * Arguments are passed through the assisted [Factory] instead of a navigation destination so the * screen can be hosted from any module. @@ -47,18 +47,29 @@ class PdfViewerViewModel @AssistedInject constructor( private val sourceResolver: PdfSourceResolver, private val dispatchers: DispatcherProvider, @Assisted val localPath: String?, - @Assisted val contentUrl: String?, + @Assisted val assetId: String?, + @Assisted val remotePath: String?, + @Assisted val conversationId: String?, + @Assisted val assetSize: Long, @Assisted val fileName: String?, ) : ViewModel() { @AssistedFactory interface Factory { - fun create(localPath: String?, contentUrl: String?, fileName: String?): PdfViewerViewModel + fun create( + localPath: String?, + assetId: String?, + remotePath: String?, + conversationId: String?, + assetSize: Long, + fileName: String?, + ): PdfViewerViewModel } private val _state = MutableStateFlow(PdfViewerState.Loading) val state: StateFlow = _state.asStateFlow() + // Written from the main thread by load()/onCleared(), read from IO by renderPage(). @Volatile private var document: PdfDocument? = null private var loadJob: Job? = null @@ -105,7 +116,7 @@ class PdfViewerViewModel @AssistedInject constructor( closeDocument() _state.value = PdfViewerState.Loading loadJob = viewModelScope.launch { - val file = sourceResolver.resolve(localPath, contentUrl, dispatchers.io()) + val file = sourceResolver.resolve(localPath, assetId, remotePath, conversationId, assetSize, dispatchers.io()) .getOrElse { cause -> _state.value = PdfViewerState.Failure(cause.toViewerError()) return@launch diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt index 2566c815c68..d184310ccc1 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelGraph.kt @@ -29,11 +29,14 @@ object PdfViewerManualViewModelFactoryGroup @Composable fun pdfViewerViewModel( localPath: String?, - contentUrl: String?, + assetId: String?, + remotePath: String?, + conversationId: String?, + assetSize: Long, fileName: String?, ): PdfViewerViewModel = wireAssistedMetroViewModel( - instanceKey = "pdf_viewer_${localPath ?: contentUrl}" + instanceKey = "pdf_viewer_${localPath ?: assetId}" ) { - pdfViewerViewModel(localPath, contentUrl, fileName) + pdfViewerViewModel(localPath, assetId, remotePath, conversationId, assetSize, fileName) } diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt index 7581f164607..8a9fe36188b 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt @@ -18,6 +18,8 @@ package com.wire.android.pdfviewer import android.content.Context +import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.Dispatchers @@ -36,85 +38,68 @@ internal class PdfSourceResolverTest { @Test fun givenAReadableLocalFile_whenResolving_thenThatFileIsReturnedWithoutDownloading() = runTest { val document = File(tempDir, "document.pdf").apply { writeText("%PDF-1.4") } - val resolver = resolver() + val loader = mockk(relaxed = true) + val resolver = resolver(loader) - val result = resolver.resolve(document.absolutePath, contentUrl = null, dispatcher = Dispatchers.Default) + val result = resolver.resolve(document.absolutePath, assetId = null, remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) assertEquals(document, result.getOrNull()) + coVerify(exactly = 0) { loader.load(any(), any(), any(), any(), any()) } } @Test - fun givenNoLocalFileAndNoUrl_whenResolving_thenItFailsAsNotFound() = runTest { + fun givenNoLocalFileAndNoAssetInfo_whenResolving_thenItFailsAsNotFound() = runTest { val resolver = resolver() - val result = resolver.resolve(localPath = null, contentUrl = null, dispatcher = Dispatchers.Default) + val result = resolver.resolve(localPath = null, assetId = null, remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) } @Test - fun givenAnEmptyLocalFileAndNoUrl_whenResolving_thenItFailsAsNotFound() = runTest { + fun givenAnEmptyLocalFileAndNoAssetInfo_whenResolving_thenItFailsAsNotFound() = runTest { val empty = File(tempDir, "empty.pdf").apply { createNewFile() } val resolver = resolver() - val result = resolver.resolve(empty.absolutePath, contentUrl = null, dispatcher = Dispatchers.Default) + val result = resolver.resolve(empty.absolutePath, assetId = null, remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) } @Test - fun givenAnUnusableUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { - val resolver = resolver() + fun givenAssetIdAndRemotePath_whenLoaderFails_thenItFailsAsDownloadFailed() = runTest { + val loader = mockk { + coEvery { load(any(), any(), any(), any(), any()) } returns Result.failure(Exception("network error")) + } + val resolver = resolver(loader) - val result = resolver.resolve(localPath = null, contentUrl = "not a url", dispatcher = Dispatchers.Default) + val result = resolver.resolve(localPath = null, assetId = "asset-123", remotePath = "/cells/path/doc.pdf", conversationId = null, assetSize = 1024L, dispatcher = Dispatchers.Default) assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) } @Test - fun givenAnHttpUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { + fun givenAssetIdButNoRemotePath_whenResolving_thenItFailsAsNotFound() = runTest { val resolver = resolver() - val result = resolver.resolve(localPath = null, contentUrl = "http://example.com/doc.pdf", dispatcher = Dispatchers.Default) - - assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) - } - - @Test - fun givenALoopbackUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { - val resolver = resolver() - - val result = resolver.resolve(localPath = null, contentUrl = "https://127.0.0.1/doc.pdf", dispatcher = Dispatchers.Default) - - assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) - } - - @Test - fun givenAPrivateNetworkUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { - val resolver = resolver() + val result = resolver.resolve(localPath = null, assetId = "asset-123", remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) - val result = resolver.resolve(localPath = null, contentUrl = "https://192.168.1.1/doc.pdf", dispatcher = Dispatchers.Default) - - assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) - } - - @Test - fun givenALinkLocalUrl_whenResolving_thenItFailsAsDownloadFailed() = runTest { - val resolver = resolver() - - // AWS/GCP metadata endpoint commonly used in SSRF attacks - val result = resolver.resolve(localPath = null, contentUrl = "https://169.254.169.254/latest/meta-data/", dispatcher = Dispatchers.Default) - - assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) } @Test - fun givenAMissingLocalPathAndAUrl_whenResolving_thenTheDownloadPathIsUsed() = runTest { - val resolver = resolver() + fun givenAMissingLocalPathAndValidAssetInfo_whenResolving_thenTheDownloadPathIsUsed() = runTest { + val loader = mockk { + coEvery { load(any(), any(), any(), any(), any()) } returns Result.failure(Exception("network error")) + } + val resolver = resolver(loader) val result = resolver.resolve( localPath = File(tempDir, "gone.pdf").absolutePath, - contentUrl = "not a url", + assetId = "asset-123", + remotePath = "/cells/path/doc.pdf", + conversationId = null, + assetSize = 0L, dispatcher = Dispatchers.Default, ) @@ -122,10 +107,31 @@ internal class PdfSourceResolverTest { assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) } - private fun resolver(): PdfSourceResolver { + @Test + fun givenSuccessfulDownload_whenCachedFileExists_thenLoaderIsNotCalledAgain() = runTest { + val loader = mockk { + coEvery { load(any(), any(), any(), any(), any()) } coAnswers { + val outFile = arg(4) + outFile.writeText("%PDF-1.4") + Result.success(Unit) + } + } + val resolver = resolver(loader) + val args = arrayOf(null, "asset-abc", "/cells/path/doc.pdf", null, 0L, Dispatchers.Default) + + // First call — triggers download + resolver.resolve(null, "asset-abc", "/cells/path/doc.pdf", null, 0L, Dispatchers.Default) + // Second call — should use cache + val result = resolver.resolve(null, "asset-abc", "/cells/path/doc.pdf", null, 0L, Dispatchers.Default) + + assertTrue(result.isSuccess) + coVerify(exactly = 1) { loader.load(any(), any(), any(), any(), any()) } + } + + private fun resolver(loader: PdfRemoteLoader = mockk(relaxed = true)): PdfSourceResolver { val context = mockk() every { context.cacheDir } returns File(tempDir, "cache") - return PdfSourceResolver(context) + return PdfSourceResolver(context, loader) } private fun Result.viewerError(): PdfViewerError? = diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt index 0655de8eb1d..70bffce837b 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt @@ -73,7 +73,7 @@ internal class PdfViewerViewModelTest { assertEquals(PdfViewerState.Failure(PdfViewerError.DOWNLOAD_FAILED), viewModel.state.value) verify(exactly = 0) { PdfDocument.open(any()) } - coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any()) } + coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } } @Test @@ -199,7 +199,7 @@ internal class PdfViewerViewModelTest { viewModel.retry() assertEquals(PdfViewerState.Content(pageCount = 3, firstPageAspectRatio = DEFAULT_ASPECT_RATIO), viewModel.state.value) - coVerify(exactly = 2) { arrangement.sourceResolver.resolve(any(), any(), any()) } + coVerify(exactly = 2) { arrangement.sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } } @Test @@ -211,7 +211,7 @@ internal class PdfViewerViewModelTest { assertEquals(PdfViewerState.Loading, viewModel.state.value) viewModel.retry() - coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any()) } + coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } gate.complete(Unit) @@ -219,7 +219,7 @@ internal class PdfViewerViewModelTest { PdfViewerState.Content(pageCount = DEFAULT_PAGE_COUNT, firstPageAspectRatio = DEFAULT_ASPECT_RATIO), viewModel.state.value, ) - coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any()) } + coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } } private class Arrangement { @@ -240,16 +240,16 @@ internal class PdfViewerViewModelTest { } fun withResolveSuccess() = apply { - coEvery { sourceResolver.resolve(any(), any(), any()) } returns Result.success(file) + coEvery { sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } returns Result.success(file) } fun withResolveFailure(error: PdfViewerError) = apply { - coEvery { sourceResolver.resolve(any(), any(), any()) } returns + coEvery { sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } returns Result.failure(PdfSourceException(error)) } fun withResolveGatedBy(gate: CompletableDeferred) = apply { - coEvery { sourceResolver.resolve(any(), any(), any()) } coAnswers { + coEvery { sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } coAnswers { gate.await() Result.success(file) } @@ -279,7 +279,10 @@ internal class PdfViewerViewModelTest { sourceResolver = sourceResolver, dispatchers = TestDispatcherProvider(), localPath = "local/document.pdf", - contentUrl = null, + assetId = null, + remotePath = null, + conversationId = null, + assetSize = 0L, fileName = "document.pdf", ) } diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt index 16e27cdb0df..bce61de2226 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3.kt @@ -205,7 +205,10 @@ data class AudioPlayerRoute( data class PdfViewerRoute( override val sessionId: WireSessionId, val localPath: String? = null, - val contentUrl: String? = null, + val assetId: String? = null, + val remotePath: String? = null, + val conversationId: String? = null, + val assetSize: Long = 0L, val fileName: String? = null, override val entryId: WireNavEntryId = WireNavEntryId.random(), ) : CellsRoute { diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt index b102c0ef74f..6b96017b09c 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/navigation/CellsNavigation3Renderer.kt @@ -239,7 +239,10 @@ internal fun CellsNavigation3RouteScreen( ) is PdfViewerRoute -> PdfViewer( localPath = route.localPath, - contentUrl = route.contentUrl, + assetId = route.assetId, + remotePath = route.remotePath, + conversationId = route.conversationId, + assetSize = route.assetSize, fileName = route.fileName, onNavigateBack = navigateBack, ) @@ -386,7 +389,15 @@ private class Navigation3CellsFilesNavigation( override fun pdf(file: CellNodeUi.File) { runtime.navigator.navigate( WireNavigationCommand( - PdfViewerRoute(sessionId, file.localPath, file.contentUrl, file.name) + PdfViewerRoute( + sessionId = sessionId, + localPath = file.localPath, + assetId = file.uuid, + remotePath = file.remotePath, + conversationId = file.conversationId, + assetSize = file.size ?: 0L, + fileName = file.name, + ) ) ) } From 46f7b24b2f95d0923c5e4f349afe36be32c1be8f Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 4 Sep 2026 09:35:40 +0200 Subject: [PATCH 05/12] chore: detekt --- .../ConversationNavigation3Entries.kt | 5 +- .../messages/item/MessageClickActions.kt | 16 ++++++- .../messages/item/MessageContentAndStatus.kt | 18 ++++++-- .../multipart/MultipartAttachmentsView.kt | 8 +++- .../com/wire/android/pdfviewer/PdfDocument.kt | 1 + .../android/pdfviewer/PdfSourceResolver.kt | 1 + .../com/wire/android/pdfviewer/PdfViewer.kt | 9 +++- .../android/pdfviewer/PdfViewerViewModel.kt | 2 + .../pdfviewer/PdfSourceResolverTest.kt | 46 ++++++++++++++++--- 9 files changed, 90 insertions(+), 16 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt index f394b094d87..1f84ea16ef6 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationNavigation3Entries.kt @@ -21,8 +21,8 @@ import com.wire.android.navigation.navigation3.WireEntryProviderInstaller import com.wire.android.navigation.navigation3.WireNavigation3ResultType import com.wire.android.navigation.navigation3.WireNavigation3Runtime import com.wire.android.navigation.navigation3.wireEntry -import com.wire.android.navigation.routes.media.ImagesPreviewNavigation3ResultType import com.wire.android.navigation.routes.media.AuthenticatedImportMediaRoute +import com.wire.android.navigation.routes.media.ImagesPreviewNavigation3ResultType import com.wire.android.navigation.routes.media.ImagesPreviewResult import com.wire.android.navigation.routes.media.ImagesPreviewRoute import com.wire.android.navigation.routes.media.MediaConversationId @@ -30,10 +30,9 @@ import com.wire.android.navigation.routes.media.MediaGalleryNavigation3ResultTyp import com.wire.android.navigation.routes.media.MediaGalleryResult import com.wire.android.navigation.routes.media.MediaGalleryResultAction import com.wire.android.navigation.routes.media.MediaGalleryRoute -import com.wire.android.navigation.routes.media.PdfViewerRoute -import com.wire.android.navigation.routes.media.VideoPlayerRoute import com.wire.android.navigation.routes.media.MessageDetailsRoute import com.wire.android.navigation.routes.media.PdfViewerRoute +import com.wire.android.navigation.routes.media.VideoPlayerRoute import com.wire.android.navigation.routes.media.toLegacy import com.wire.android.ui.calling.conversationCallViewModel import com.wire.android.ui.home.conversations.details.ConversationDetailsId diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt index e4c55c5dc4a..0c55c7a1a6c 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageClickActions.kt @@ -31,7 +31,13 @@ sealed class MessageClickActions { open val onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit = { _, _, _ -> } open val onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> } open val onAudioClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> } - open val onPdfClicked: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit = { _, _, _, _, _ -> } + open val onPdfClicked: ( + localPath: String?, + assetId: String?, + remotePath: String?, + assetSize: Long, + fileName: String? + ) -> Unit = { _, _, _, _, _ -> } open val onLinkClicked: (String) -> Unit = {} open val onReplyClicked: (UIMessage.Regular) -> Unit = {} open val onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit = { _, _ -> } @@ -51,7 +57,13 @@ sealed class MessageClickActions { override val onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit = { _, _, _ -> }, override val onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> }, override val onAudioClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit = { _, _, _ -> }, - override val onPdfClicked: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit = { _, _, _, _, _ -> }, + override val onPdfClicked: ( + localPath: String?, + assetId: String?, + remotePath: String?, + assetSize: Long, + fileName: String? + ) -> Unit = { _, _, _, _, _ -> }, override val onLinkClicked: (String) -> Unit = {}, override val onReplyClicked: (UIMessage.Regular) -> Unit = {}, override val onResetSessionClicked: (senderUserId: UserId, clientId: String?) -> Unit = { _, _ -> }, diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt index 9c6e079b24c..a486611089c 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/messages/item/MessageContentAndStatus.kt @@ -74,7 +74,13 @@ internal fun UIMessage.Regular.MessageContentAndStatus( onImageClicked: (UIMessage.Regular, Boolean, String?) -> Unit, onVideoClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, onAudioClicked: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onPdfClicked: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, + onPdfClicked: ( + localPath: String?, + assetId: String?, + remotePath: String?, + assetSize: Long, + fileName: String? + ) -> Unit, onProfileClicked: (senderId: MessageSenderId) -> Unit, onLinkClicked: (String) -> Unit, onReplyClicked: (UIMessage.Regular) -> Unit, @@ -175,7 +181,13 @@ private fun MessageContent( onMultipartImageClick: (String) -> Unit, onMultipartVideoClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, onMultipartAudioClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onMultipartPdfClick: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, + onMultipartPdfClick: ( + localPath: String?, + assetId: String?, + remotePath: String?, + assetSize: Long, + fileName: String? + ) -> Unit, onOpenProfile: (senderId: MessageSenderId) -> Unit, onLinkClick: (String) -> Unit, onReplyClick: Clickable, @@ -472,7 +484,7 @@ private fun MessageContent( onVideoAttachmentClick = onMultipartVideoClick, onAudioAttachmentClick = onMultipartAudioClick, onPdfAttachmentClick = onMultipartPdfClick, - ) + ) } UIMessageContent.Deleted -> {} diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt index bb5db2357fc..f17f7c36d3d 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsView.kt @@ -60,7 +60,13 @@ fun MultipartAttachmentsView( onImageAttachmentClick: (String) -> Unit, onVideoAttachmentClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, onAudioAttachmentClick: (localPath: String?, contentUrl: String?, fileName: String?) -> Unit, - onPdfAttachmentClick: (localPath: String?, assetId: String?, remotePath: String?, assetSize: Long, fileName: String?) -> Unit, + onPdfAttachmentClick: ( + localPath: String?, + assetId: String?, + remotePath: String?, + assetSize: Long, + fileName: String? + ) -> Unit, modifier: Modifier = Modifier, viewModel: MultipartAttachmentsViewModel = when { LocalInspectionMode.current -> MultipartAttachmentsViewModelPreview diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt index 386bb608e03..f799d27dacb 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt @@ -107,6 +107,7 @@ internal class PdfDocument private constructor( * [PdfRenderer] throws [SecurityException] for password protected documents and * [java.io.IOException] for anything it cannot parse. */ + @Suppress("TooGenericExceptionCaught") fun open(file: File): Result = runCatching { val descriptor = ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY) try { diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt index a940c105789..4ba6fda089f 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt @@ -42,6 +42,7 @@ class PdfSourceResolver @Inject constructor( private val remoteLoader: PdfRemoteLoader, ) { + @Suppress("LongParameterList") suspend fun resolve( localPath: String?, assetId: String?, diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt index d6cb3be7c9d..d5a7f27bf58 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt @@ -98,7 +98,14 @@ fun PdfViewer( fileName: String?, onNavigateBack: () -> Unit, modifier: Modifier = Modifier, - viewModel: PdfViewerViewModel = pdfViewerViewModel(localPath, assetId, remotePath, conversationId, assetSize, fileName), + viewModel: PdfViewerViewModel = pdfViewerViewModel( + localPath = localPath, + assetId = assetId, + remotePath = remotePath, + conversationId = conversationId, + assetSize = assetSize, + fileName = fileName + ) ) { val state by viewModel.state.collectAsStateWithLifecycle() PdfViewerContent( diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt index 276eb366938..996fd9ad382 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt @@ -54,6 +54,7 @@ class PdfViewerViewModel @AssistedInject constructor( @Assisted val fileName: String?, ) : ViewModel() { + @Suppress("LongParameterList") @AssistedFactory interface Factory { fun create( @@ -97,6 +98,7 @@ class PdfViewerViewModel @AssistedInject constructor( * Renders [pageIndex] at [widthPx] and caches the result. Returns `null` when the document is * not open (yet) or the page could not be rendered. */ + @Suppress("ReturnCount") suspend fun renderPage(pageIndex: Int, widthPx: Int): Bitmap? { if (widthPx <= 0) return null val current = document ?: return null diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt index 8a9fe36188b..da1459dc3ba 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt @@ -41,7 +41,14 @@ internal class PdfSourceResolverTest { val loader = mockk(relaxed = true) val resolver = resolver(loader) - val result = resolver.resolve(document.absolutePath, assetId = null, remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) + val result = resolver.resolve( + localPath = document.absolutePath, + assetId = null, + remotePath = null, + conversationId = null, + assetSize = 0L, + dispatcher = Dispatchers.Default + ) assertEquals(document, result.getOrNull()) coVerify(exactly = 0) { loader.load(any(), any(), any(), any(), any()) } @@ -51,7 +58,14 @@ internal class PdfSourceResolverTest { fun givenNoLocalFileAndNoAssetInfo_whenResolving_thenItFailsAsNotFound() = runTest { val resolver = resolver() - val result = resolver.resolve(localPath = null, assetId = null, remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) + val result = resolver.resolve( + localPath = null, + assetId = null, + remotePath = null, + conversationId = null, + assetSize = 0L, + dispatcher = Dispatchers.Default + ) assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) } @@ -61,7 +75,14 @@ internal class PdfSourceResolverTest { val empty = File(tempDir, "empty.pdf").apply { createNewFile() } val resolver = resolver() - val result = resolver.resolve(empty.absolutePath, assetId = null, remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) + val result = resolver.resolve( + localPath = empty.absolutePath, + assetId = null, + remotePath = null, + conversationId = null, + assetSize = 0L, + dispatcher = Dispatchers.Default + ) assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) } @@ -73,7 +94,14 @@ internal class PdfSourceResolverTest { } val resolver = resolver(loader) - val result = resolver.resolve(localPath = null, assetId = "asset-123", remotePath = "/cells/path/doc.pdf", conversationId = null, assetSize = 1024L, dispatcher = Dispatchers.Default) + val result = resolver.resolve( + localPath = null, + assetId = "asset-123", + remotePath = "/cells/path/doc.pdf", + conversationId = null, + assetSize = 1024L, + dispatcher = Dispatchers.Default + ) assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) } @@ -82,7 +110,14 @@ internal class PdfSourceResolverTest { fun givenAssetIdButNoRemotePath_whenResolving_thenItFailsAsNotFound() = runTest { val resolver = resolver() - val result = resolver.resolve(localPath = null, assetId = "asset-123", remotePath = null, conversationId = null, assetSize = 0L, dispatcher = Dispatchers.Default) + val result = resolver.resolve( + localPath = null, + assetId = "asset-123", + remotePath = null, + conversationId = null, + assetSize = 0L, + dispatcher = Dispatchers.Default + ) assertEquals(PdfViewerError.FILE_NOT_FOUND, result.viewerError()) } @@ -117,7 +152,6 @@ internal class PdfSourceResolverTest { } } val resolver = resolver(loader) - val args = arrayOf(null, "asset-abc", "/cells/path/doc.pdf", null, 0L, Dispatchers.Default) // First call — triggers download resolver.resolve(null, "asset-abc", "/cells/path/doc.pdf", null, 0L, Dispatchers.Default) From 1a635892b6d337dd155522ae858b2cee7b1a8015 Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 4 Sep 2026 10:16:24 +0200 Subject: [PATCH 06/12] chore: lint --- core/pdf-viewer/lint-baseline.xml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 core/pdf-viewer/lint-baseline.xml diff --git a/core/pdf-viewer/lint-baseline.xml b/core/pdf-viewer/lint-baseline.xml new file mode 100644 index 00000000000..05a9be7dd95 --- /dev/null +++ b/core/pdf-viewer/lint-baseline.xml @@ -0,0 +1,4 @@ + + + + From c49500bc7aed27b174575bca3d6a45154acbfb4e Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 4 Sep 2026 10:25:58 +0200 Subject: [PATCH 07/12] chore: compose stability --- features/cells/stability/cells-debug.stability | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/features/cells/stability/cells-debug.stability b/features/cells/stability/cells-debug.stability index 20d2f947e94..d7e3224818e 100644 --- a/features/cells/stability/cells-debug.stability +++ b/features/cells/stability/cells-debug.stability @@ -79,7 +79,7 @@ internal fun com.wire.android.feature.cells.ui.CellListItem(cell: com.wire.andro - showViewerOnlyIcon: STABLE (primitive type) @Composable -internal fun com.wire.android.feature.cells.ui.CellScreenContent(actionsFlow: kotlinx.coroutines.flow.Flow, pagingListItems: androidx.paging.compose.LazyPagingItems, sendIntent: kotlin.Function1, openFolder: kotlin.Function3, menuState: kotlinx.coroutines.flow.Flow, showPublicLinkScreen: kotlin.Function1, showRenameScreen: kotlin.Function1, showMoveToFolderScreen: kotlin.Function3, showAddRemoveTagsScreen: kotlin.Function1, isRefreshing: androidx.compose.runtime.State, onRefresh: kotlin.Function0, isRestoreInProgress: kotlin.Boolean, isDeleteInProgress: kotlin.Boolean, modifier: androidx.compose.ui.Modifier, showViewerOnlyIcon: kotlin.Boolean, isRecycleBin: kotlin.Boolean, isAllFiles: kotlin.Boolean, isSearchResult: kotlin.Boolean, isOffline: kotlin.Boolean, isPullToRefreshEnabled: kotlin.Boolean, lazyListState: androidx.compose.foundation.lazy.LazyListState, retryEditNodeError: kotlin.Function1, showVersionHistoryScreen: kotlin.Function2, showImageViewer: kotlin.Function1, showVideoViewer: kotlin.Function1, showAudioPlayer: kotlin.Function1, fileReadyFlow: kotlinx.coroutines.flow.Flow?): kotlin.Unit +internal fun com.wire.android.feature.cells.ui.CellScreenContent(actionsFlow: kotlinx.coroutines.flow.Flow, pagingListItems: androidx.paging.compose.LazyPagingItems, sendIntent: kotlin.Function1, openFolder: kotlin.Function3, menuState: kotlinx.coroutines.flow.Flow, showPublicLinkScreen: kotlin.Function1, showRenameScreen: kotlin.Function1, showMoveToFolderScreen: kotlin.Function3, showAddRemoveTagsScreen: kotlin.Function1, isRefreshing: androidx.compose.runtime.State, onRefresh: kotlin.Function0, isRestoreInProgress: kotlin.Boolean, isDeleteInProgress: kotlin.Boolean, modifier: androidx.compose.ui.Modifier, showViewerOnlyIcon: kotlin.Boolean, isRecycleBin: kotlin.Boolean, isAllFiles: kotlin.Boolean, isSearchResult: kotlin.Boolean, isOffline: kotlin.Boolean, isPullToRefreshEnabled: kotlin.Boolean, lazyListState: androidx.compose.foundation.lazy.LazyListState, retryEditNodeError: kotlin.Function1, showVersionHistoryScreen: kotlin.Function2, showImageViewer: kotlin.Function1, showVideoViewer: kotlin.Function1, showAudioPlayer: kotlin.Function1, showPdfViewer: kotlin.Function1, fileReadyFlow: kotlinx.coroutines.flow.Flow?): kotlin.Unit skippable: false restartable: true params: @@ -109,6 +109,7 @@ internal fun com.wire.android.feature.cells.ui.CellScreenContent(actionsFlow: ko - showImageViewer: STABLE (function type) - showVideoViewer: STABLE (function type) - showAudioPlayer: STABLE (function type) + - showPdfViewer: STABLE (function type) - fileReadyFlow: RUNTIME (requires runtime check) @Composable From bd945bfa3acf3ff04c99ac5b27b383f79017def2 Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 4 Sep 2026 11:30:41 +0200 Subject: [PATCH 08/12] chore: compose stability --- app/stability/app-devDebug.stability | 31 ++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/app/stability/app-devDebug.stability b/app/stability/app-devDebug.stability index e86e91e213d..5192f7f275c 100644 --- a/app/stability/app-devDebug.stability +++ b/app/stability/app-devDebug.stability @@ -3267,7 +3267,7 @@ public fun com.wire.android.ui.debug.DebugDataOptions(appVersion: kotlin.String, - viewModel: RUNTIME (requires runtime check) @Composable -public fun com.wire.android.ui.debug.DebugDataOptionsContent(state: com.wire.android.ui.debug.DebugDataOptionsState, appVersion: kotlin.String, buildVariant: kotlin.String, onCopyText: kotlin.Function1, onDisableEventProcessingChange: kotlin.Function1, onRestartSlowSyncForRecovery: kotlin.Function0, onForceUpdateApiVersions: kotlin.Function0, enrollE2EICertificate: kotlin.Function0, e2eiCertificateExpirationInputState: androidx.compose.foundation.text.input.TextFieldState, handleE2EIEnrollmentResult: kotlin.Function1, dismissCertificateDialog: kotlin.Function0, checkCrlRevocationList: kotlin.Function0, forceCRLExpirationAfterOneMinute: kotlin.Boolean, onForceCRLExpirationAfterOneMinuteChange: kotlin.Function1, onResendFCMToken: kotlin.Function0, onShowFeatureFlags: kotlin.Function0, onShowCryptoStats: kotlin.Function0, onShowSecurityProviders: kotlin.Function0, onRepairFaultyRemovalKeys: kotlin.Function0, modifier: androidx.compose.ui.Modifier): kotlin.Unit +public fun com.wire.android.ui.debug.DebugDataOptionsContent(state: com.wire.android.ui.debug.DebugDataOptionsState, appVersion: kotlin.String, buildVariant: kotlin.String, onCopyText: kotlin.Function1, onDisableEventProcessingChange: kotlin.Function1, onRestartSlowSyncForRecovery: kotlin.Function0, onForceUpdateApiVersions: kotlin.Function0, enrollE2EICertificate: kotlin.Function0, e2eiCertificateExpirationInputState: androidx.compose.foundation.text.input.TextFieldState, handleE2EIEnrollmentResult: kotlin.Function1, dismissCertificateDialog: kotlin.Function0, checkCrlRevocationList: kotlin.Function0, forceCRLExpirationAfterOneMinute: kotlin.Boolean, onForceCRLExpirationAfterOneMinuteChange: kotlin.Function1, onResendFCMToken: kotlin.Function0, onShowFeatureFlags: kotlin.Function0, onShowCryptoStats: kotlin.Function0, onShowSecurityProviders: kotlin.Function0, onRepairFaultyRemovalKeys: kotlin.Function0, modifier: androidx.compose.ui.Modifier): kotlin.Unit skippable: true restartable: true params: @@ -3646,6 +3646,14 @@ public fun com.wire.android.ui.debug.securityProvidersViewModel(): com.wire.andr restartable: true params: +@Composable +public fun com.wire.android.ui.debug.securityproviders.CryptoServiceListItem(row: com.wire.android.ui.debug.securityproviders.CryptoServiceRow, modifier: androidx.compose.ui.Modifier): kotlin.Unit + skippable: true + restartable: true + params: + - row: STABLE (class with no mutable properties) + - modifier: STABLE (marked @Stable or @Immutable) + @Composable private fun com.wire.android.ui.debug.securityproviders.NetworkSection(network: com.wire.android.ui.debug.securityproviders.NetworkDiagnostics): kotlin.Unit skippable: false @@ -3700,7 +3708,7 @@ internal fun com.wire.android.ui.e2eiEnrollment.E2EIEnrollmentRouteScreen(viewMo - onOpenCertificateDetails: STABLE (function type) @Composable -private fun com.wire.android.ui.e2eiEnrollment.E2EIEnrollmentScreenContent(state: com.wire.android.ui.e2eiEnrollment.E2EIEnrollmentState, clearSessionState: com.wire.android.ui.authentication.devices.common.ClearSessionState, dismissSuccess: kotlin.Function0, dismissErrorDialog: kotlin.Function0, enrollE2EICertificate: kotlin.Function0, handleE2EIEnrollmentResult: kotlin.Function1, openCertificateDetails: kotlin.Function0, onBackButtonClicked: kotlin.Function0, onCancelEnrollmentClicked: kotlin.Function0, onProceedEnrollmentClicked: kotlin.Function0): kotlin.Unit +private fun com.wire.android.ui.e2eiEnrollment.E2EIEnrollmentScreenContent(state: com.wire.android.ui.e2eiEnrollment.E2EIEnrollmentState, clearSessionState: com.wire.android.ui.authentication.devices.common.ClearSessionState, dismissSuccess: kotlin.Function0, dismissErrorDialog: kotlin.Function0, enrollE2EICertificate: kotlin.Function0, handleE2EIEnrollmentResult: kotlin.Function1, openCertificateDetails: kotlin.Function0, onBackButtonClicked: kotlin.Function0, onCancelEnrollmentClicked: kotlin.Function0, onProceedEnrollmentClicked: kotlin.Function0): kotlin.Unit skippable: true restartable: true params: @@ -3716,7 +3724,7 @@ private fun com.wire.android.ui.e2eiEnrollment.E2EIEnrollmentScreenContent(state - onProceedEnrollmentClicked: STABLE (function type) @Composable -public fun com.wire.android.ui.e2eiEnrollment.GetE2EICertificateUI(enrollmentResultHandler: kotlin.Function1, isNewClient: kotlin.Boolean, viewModel: com.wire.android.ui.e2eiEnrollment.GetE2EICertificateViewModel): kotlin.Unit +public fun com.wire.android.ui.e2eiEnrollment.GetE2EICertificateUI(enrollmentResultHandler: kotlin.Function1, isNewClient: kotlin.Boolean, viewModel: com.wire.android.ui.e2eiEnrollment.GetE2EICertificateViewModel): kotlin.Unit skippable: false restartable: true params: @@ -4321,7 +4329,7 @@ private fun com.wire.android.ui.home.conversations.ConversationFoldersNavigation - runtime: STABLE (marked @Stable or @Immutable) @Composable -internal fun com.wire.android.ui.home.conversations.ConversationMessageComposer(conversationId: com.wire.kalium.logic.data.id.QualifiedID, bottomSheetVisible: kotlin.Boolean, lastUnreadMessageInstant: kotlinx.datetime.Instant?, unreadEventCount: kotlin.Int, playingAudioMessage: com.wire.android.media.audiomessage.PlayingAudioMessage, assetStatuses: kotlinx.collections.immutable.PersistentMap, selectedMessageId: kotlin.String?, messageComposerStateHolder: com.wire.android.ui.home.messagecomposer.state.MessageComposerStateHolder, attachments: kotlin.collections.List, messages: kotlinx.coroutines.flow.Flow>, onSendMessage: kotlin.Function1, onPingOptionClicked: kotlin.Function0, onImagesPicked: kotlin.Function2, kotlin.Boolean, kotlin.Unit>, onAttachmentPicked: kotlin.Function1, onAudioRecorded: kotlin.Function1, onAssetItemClicked: kotlin.Function1, onImageFullScreenMode: kotlin.Function3, onVideoClick: kotlin.Function3<@[ParameterName(name = \, onAudioClick: kotlin.Function3<@[ParameterName(name = \, onReactionClicked: kotlin.Function2, onResetSessionClicked: kotlin.Function2<@[ParameterName(name = \, onOpenProfile: kotlin.Function1<@[ParameterName(name = \, onUpdateConversationReadDate: kotlin.Function1, onShowEditingOptions: kotlin.Function1, onSwipedToReply: kotlin.Function1, onSelfDeletingMessageRead: kotlin.Function1, conversationDetailsData: com.wire.android.ui.home.conversations.info.ConversationDetailsData, onFailedMessageRetryClicked: kotlin.Function2, onFailedMessageCancelClicked: kotlin.Function1, onChangeSelfDeletionClicked: kotlin.Function1, onClearMentionSearchResult: kotlin.Function0, onLocationClicked: kotlin.Function0, onPermissionPermanentlyDenied: kotlin.Function1<@[ParameterName(name = \, tempWritableImageUri: android.net.Uri?, tempWritableVideoUri: android.net.Uri?, onLinkClick: kotlin.Function1, onNavigateToReplyOriginalMessage: kotlin.Function1, openDrawingCanvas: kotlin.Function0, onAttachmentClick: kotlin.Function1, onAttachmentMenuClick: kotlin.Function1, currentTimeInMillisFlow: kotlinx.coroutines.flow.Flow, onReachedOldestMessage: kotlin.Function0, showHistoryLoadingIndicator: kotlin.Boolean, isFetchingOlderMessages: kotlin.Boolean, hasMoreRemoteMessages: kotlin.Boolean, isBubbleUiEnabled: kotlin.Boolean, isWireCellsEnabled: kotlin.Boolean): kotlin.Unit +internal fun com.wire.android.ui.home.conversations.ConversationMessageComposer(conversationId: com.wire.kalium.logic.data.id.QualifiedID, bottomSheetVisible: kotlin.Boolean, lastUnreadMessageInstant: kotlinx.datetime.Instant?, unreadEventCount: kotlin.Int, playingAudioMessage: com.wire.android.media.audiomessage.PlayingAudioMessage, assetStatuses: kotlinx.collections.immutable.PersistentMap, selectedMessageId: kotlin.String?, messageComposerStateHolder: com.wire.android.ui.home.messagecomposer.state.MessageComposerStateHolder, attachments: kotlin.collections.List, messages: kotlinx.coroutines.flow.Flow>, onSendMessage: kotlin.Function1, onPingOptionClicked: kotlin.Function0, onImagesPicked: kotlin.Function2, kotlin.Boolean, kotlin.Unit>, onAttachmentPicked: kotlin.Function1, onAudioRecorded: kotlin.Function1, onAssetItemClicked: kotlin.Function1, onImageFullScreenMode: kotlin.Function3, onVideoClick: kotlin.Function3<@[ParameterName(name = \, onAudioClick: kotlin.Function3<@[ParameterName(name = \, onPdfClick: kotlin.Function5<@[ParameterName(name = \, onReactionClicked: kotlin.Function2, onResetSessionClicked: kotlin.Function2<@[ParameterName(name = \, onOpenProfile: kotlin.Function1<@[ParameterName(name = \, onUpdateConversationReadDate: kotlin.Function1, onShowEditingOptions: kotlin.Function1, onSwipedToReply: kotlin.Function1, onSelfDeletingMessageRead: kotlin.Function1, conversationDetailsData: com.wire.android.ui.home.conversations.info.ConversationDetailsData, onFailedMessageRetryClicked: kotlin.Function2, onFailedMessageCancelClicked: kotlin.Function1, onChangeSelfDeletionClicked: kotlin.Function1, onClearMentionSearchResult: kotlin.Function0, onLocationClicked: kotlin.Function0, onPermissionPermanentlyDenied: kotlin.Function1<@[ParameterName(name = \, tempWritableImageUri: android.net.Uri?, tempWritableVideoUri: android.net.Uri?, onLinkClick: kotlin.Function1, onNavigateToReplyOriginalMessage: kotlin.Function1, openDrawingCanvas: kotlin.Function0, onAttachmentClick: kotlin.Function1, onAttachmentMenuClick: kotlin.Function1, currentTimeInMillisFlow: kotlinx.coroutines.flow.Flow, onReachedOldestMessage: kotlin.Function0, showHistoryLoadingIndicator: kotlin.Boolean, isFetchingOlderMessages: kotlin.Boolean, hasMoreRemoteMessages: kotlin.Boolean, isBubbleUiEnabled: kotlin.Boolean, isWireCellsEnabled: kotlin.Boolean): kotlin.Unit skippable: false restartable: true params: @@ -4344,6 +4352,7 @@ internal fun com.wire.android.ui.home.conversations.ConversationMessageComposer( - onImageFullScreenMode: STABLE (function type) - onVideoClick: STABLE (function type) - onAudioClick: STABLE (function type) + - onPdfClick: STABLE (function type) - onReactionClicked: STABLE (function type) - onResetSessionClicked: STABLE (function type) - onOpenProfile: STABLE (function type) @@ -4400,7 +4409,7 @@ internal fun com.wire.android.ui.home.conversations.ConversationRouteScreen(navi - messageAttachmentsViewModel: UNSTABLE (has mutable properties or unstable members) @Composable -private fun com.wire.android.ui.home.conversations.ConversationScreenContent(bannerMessage: com.wire.android.util.ui.UIText?, messageComposerViewState: com.wire.android.ui.home.conversations.MessageComposerViewState, conversationCallViewState: com.wire.android.ui.home.conversations.call.ConversationCallViewState, conversationInfoViewState: com.wire.android.ui.home.conversations.info.ConversationInfoViewState, conversationMessagesViewState: com.wire.android.ui.home.conversations.messages.ConversationMessagesViewState, attachments: kotlin.collections.List, bottomSheetVisible: kotlin.Boolean, onOpenProfile: kotlin.Function1<@[ParameterName(name = \, onMessageDetailsClick: kotlin.Function2<@[ParameterName(name = \, onSendMessage: kotlin.Function1, onPingOptionClicked: kotlin.Function0, onImagesPicked: kotlin.Function2, kotlin.Boolean, kotlin.Unit>, onAttachmentPicked: kotlin.Function1, onAudioRecorded: kotlin.Function1, onDeleteMessage: kotlin.Function2, onAssetItemClicked: kotlin.Function1, onImageFullScreenMode: kotlin.Function3, onVideoClick: kotlin.Function3<@[ParameterName(name = \, onAudioClick: kotlin.Function3<@[ParameterName(name = \, onStartCall: kotlin.Function0, onJoinCall: kotlin.Function0, onReactionClick: kotlin.Function2<@[ParameterName(name = \, onResetSessionClick: kotlin.Function2<@[ParameterName(name = \, onUpdateConversationReadDate: kotlin.Function1, onDropDownClick: kotlin.Function0, onBackButtonClick: kotlin.Function0, composerMessages: kotlinx.coroutines.flow.SharedFlow, conversationMessages: kotlinx.coroutines.flow.SharedFlow, shareAsset: kotlin.Function2, onSelfDeletingMessageRead: kotlin.Function1, onNewSelfDeletingMessagesStatus: kotlin.Function1, tempWritableImageUri: android.net.Uri?, tempWritableVideoUri: android.net.Uri?, onFailedMessageRetryClicked: kotlin.Function2, onClearMentionSearchResult: kotlin.Function0, onPermissionPermanentlyDenied: kotlin.Function1<@[ParameterName(name = \, conversationScreenState: com.wire.android.ui.home.conversations.ConversationScreenState, messageComposerStateHolder: com.wire.android.ui.home.messagecomposer.state.MessageComposerStateHolder, onLinkClick: kotlin.Function1, openDrawingCanvas: kotlin.Function0, onAttachmentClick: kotlin.Function1, onAttachmentMenuClick: kotlin.Function1, currentTimeInMillisFlow: kotlinx.coroutines.flow.Flow, onReachedOldestMessage: kotlin.Function0, isFetchingOlderMessages: kotlin.Boolean, hasMoreRemoteMessages: kotlin.Boolean, isWireCellsEnabled: kotlin.Boolean): kotlin.Unit +private fun com.wire.android.ui.home.conversations.ConversationScreenContent(bannerMessage: com.wire.android.util.ui.UIText?, messageComposerViewState: com.wire.android.ui.home.conversations.MessageComposerViewState, conversationCallViewState: com.wire.android.ui.home.conversations.call.ConversationCallViewState, conversationInfoViewState: com.wire.android.ui.home.conversations.info.ConversationInfoViewState, conversationMessagesViewState: com.wire.android.ui.home.conversations.messages.ConversationMessagesViewState, attachments: kotlin.collections.List, bottomSheetVisible: kotlin.Boolean, onOpenProfile: kotlin.Function1<@[ParameterName(name = \, onMessageDetailsClick: kotlin.Function2<@[ParameterName(name = \, onSendMessage: kotlin.Function1, onPingOptionClicked: kotlin.Function0, onImagesPicked: kotlin.Function2, kotlin.Boolean, kotlin.Unit>, onAttachmentPicked: kotlin.Function1, onAudioRecorded: kotlin.Function1, onDeleteMessage: kotlin.Function2, onAssetItemClicked: kotlin.Function1, onImageFullScreenMode: kotlin.Function3, onVideoClick: kotlin.Function3<@[ParameterName(name = \, onAudioClick: kotlin.Function3<@[ParameterName(name = \, onPdfClick: kotlin.Function5<@[ParameterName(name = \, onStartCall: kotlin.Function0, onJoinCall: kotlin.Function0, onReactionClick: kotlin.Function2<@[ParameterName(name = \, onResetSessionClick: kotlin.Function2<@[ParameterName(name = \, onUpdateConversationReadDate: kotlin.Function1, onDropDownClick: kotlin.Function0, onBackButtonClick: kotlin.Function0, composerMessages: kotlinx.coroutines.flow.SharedFlow, conversationMessages: kotlinx.coroutines.flow.SharedFlow, shareAsset: kotlin.Function2, onSelfDeletingMessageRead: kotlin.Function1, onNewSelfDeletingMessagesStatus: kotlin.Function1, tempWritableImageUri: android.net.Uri?, tempWritableVideoUri: android.net.Uri?, onFailedMessageRetryClicked: kotlin.Function2, onClearMentionSearchResult: kotlin.Function0, onPermissionPermanentlyDenied: kotlin.Function1<@[ParameterName(name = \, conversationScreenState: com.wire.android.ui.home.conversations.ConversationScreenState, messageComposerStateHolder: com.wire.android.ui.home.messagecomposer.state.MessageComposerStateHolder, onLinkClick: kotlin.Function1, openDrawingCanvas: kotlin.Function0, onAttachmentClick: kotlin.Function1, onAttachmentMenuClick: kotlin.Function1, currentTimeInMillisFlow: kotlinx.coroutines.flow.Flow, onReachedOldestMessage: kotlin.Function0, isFetchingOlderMessages: kotlin.Boolean, hasMoreRemoteMessages: kotlin.Boolean, isWireCellsEnabled: kotlin.Boolean): kotlin.Unit skippable: false restartable: true params: @@ -4423,6 +4432,7 @@ private fun com.wire.android.ui.home.conversations.ConversationScreenContent(ban - onImageFullScreenMode: STABLE (function type) - onVideoClick: STABLE (function type) - onAudioClick: STABLE (function type) + - onPdfClick: STABLE (function type) - onStartCall: STABLE (function type) - onJoinCall: STABLE (function type) - onReactionClick: STABLE (function type) @@ -6414,7 +6424,7 @@ public fun com.wire.android.ui.home.conversations.messages.item.MessageContainer - isWireCellsEnabled: STABLE (primitive type) @Composable -private fun com.wire.android.ui.home.conversations.messages.item.MessageContent(message: com.wire.android.ui.home.conversations.model.UIMessage.Regular, messageContent: com.wire.android.ui.home.conversations.model.UIMessageContent.Regular?, searchQuery: kotlin.String, messageStyle: com.wire.android.ui.home.conversations.messages.item.MessageStyle, assetStatus: com.wire.kalium.logic.data.asset.AssetTransferStatus?, onAssetClick: com.wire.android.model.Clickable, onImageClick: com.wire.android.model.Clickable, onMultipartImageClick: kotlin.Function1, onMultipartVideoClick: kotlin.Function3<@[ParameterName(name = \, onMultipartAudioClick: kotlin.Function3<@[ParameterName(name = \, onOpenProfile: kotlin.Function1<@[ParameterName(name = \, onLinkClick: kotlin.Function1, onReplyClick: com.wire.android.model.Clickable, accent: com.wire.android.ui.theme.Accent, conversationAssetPathsViewModel: com.wire.android.ui.home.conversations.messages.item.ConversationAssetPathsViewModel): kotlin.Unit +private fun com.wire.android.ui.home.conversations.messages.item.MessageContent(message: com.wire.android.ui.home.conversations.model.UIMessage.Regular, messageContent: com.wire.android.ui.home.conversations.model.UIMessageContent.Regular?, searchQuery: kotlin.String, messageStyle: com.wire.android.ui.home.conversations.messages.item.MessageStyle, assetStatus: com.wire.kalium.logic.data.asset.AssetTransferStatus?, onAssetClick: com.wire.android.model.Clickable, onImageClick: com.wire.android.model.Clickable, onMultipartImageClick: kotlin.Function1, onMultipartVideoClick: kotlin.Function3<@[ParameterName(name = \, onMultipartAudioClick: kotlin.Function3<@[ParameterName(name = \, onMultipartPdfClick: kotlin.Function5<@[ParameterName(name = \, onOpenProfile: kotlin.Function1<@[ParameterName(name = \, onLinkClick: kotlin.Function1, onReplyClick: com.wire.android.model.Clickable, accent: com.wire.android.ui.theme.Accent, conversationAssetPathsViewModel: com.wire.android.ui.home.conversations.messages.item.ConversationAssetPathsViewModel): kotlin.Unit skippable: false restartable: true params: @@ -6428,6 +6438,7 @@ private fun com.wire.android.ui.home.conversations.messages.item.MessageContent( - onMultipartImageClick: STABLE (function type) - onMultipartVideoClick: STABLE (function type) - onMultipartAudioClick: STABLE (function type) + - onMultipartPdfClick: STABLE (function type) - onOpenProfile: STABLE (function type) - onLinkClick: STABLE (function type) - onReplyClick: STABLE (class with no mutable properties) @@ -6435,7 +6446,7 @@ private fun com.wire.android.ui.home.conversations.messages.item.MessageContent( - conversationAssetPathsViewModel: RUNTIME (requires runtime check) @Composable -internal fun com.wire.android.ui.home.conversations.messages.item.MessageContentAndStatus(message: com.wire.android.ui.home.conversations.model.UIMessage.Regular, assetStatus: com.wire.kalium.logic.data.asset.AssetTransferStatus?, searchQuery: kotlin.String, messageStyle: com.wire.android.ui.home.conversations.messages.item.MessageStyle, onAssetClicked: kotlin.Function1, onImageClicked: kotlin.Function3, onVideoClicked: kotlin.Function3<@[ParameterName(name = \, onAudioClicked: kotlin.Function3<@[ParameterName(name = \, onProfileClicked: kotlin.Function1<@[ParameterName(name = \, onLinkClicked: kotlin.Function1, onReplyClicked: kotlin.Function1, shouldDisplayMessageStatus: kotlin.Boolean, conversationDetailsData: com.wire.android.ui.home.conversations.info.ConversationDetailsData, accent: com.wire.android.ui.theme.Accent): kotlin.Unit +internal fun com.wire.android.ui.home.conversations.messages.item.MessageContentAndStatus(message: com.wire.android.ui.home.conversations.model.UIMessage.Regular, assetStatus: com.wire.kalium.logic.data.asset.AssetTransferStatus?, searchQuery: kotlin.String, messageStyle: com.wire.android.ui.home.conversations.messages.item.MessageStyle, onAssetClicked: kotlin.Function1, onImageClicked: kotlin.Function3, onVideoClicked: kotlin.Function3<@[ParameterName(name = \, onAudioClicked: kotlin.Function3<@[ParameterName(name = \, onPdfClicked: kotlin.Function5<@[ParameterName(name = \, onProfileClicked: kotlin.Function1<@[ParameterName(name = \, onLinkClicked: kotlin.Function1, onReplyClicked: kotlin.Function1, shouldDisplayMessageStatus: kotlin.Boolean, conversationDetailsData: com.wire.android.ui.home.conversations.info.ConversationDetailsData, accent: com.wire.android.ui.theme.Accent): kotlin.Unit skippable: false restartable: true params: @@ -6447,6 +6458,7 @@ internal fun com.wire.android.ui.home.conversations.messages.item.MessageContent - onImageClicked: STABLE (function type) - onVideoClicked: STABLE (function type) - onAudioClicked: STABLE (function type) + - onPdfClicked: STABLE (function type) - onProfileClicked: STABLE (function type) - onLinkClicked: STABLE (function type) - onReplyClicked: STABLE (function type) @@ -7186,7 +7198,7 @@ private fun com.wire.android.ui.home.conversations.model.messagetypes.multipart. - modifier: STABLE (marked @Stable or @Immutable) @Composable -public fun com.wire.android.ui.home.conversations.model.messagetypes.multipart.MultipartAttachmentsView(conversationId: com.wire.kalium.logic.data.id.QualifiedID, attachments: kotlin.collections.List, messageStyle: com.wire.android.ui.home.conversations.messages.item.MessageStyle, onImageAttachmentClick: kotlin.Function1, onVideoAttachmentClick: kotlin.Function3<@[ParameterName(name = \, onAudioAttachmentClick: kotlin.Function3<@[ParameterName(name = \, modifier: androidx.compose.ui.Modifier, viewModel: com.wire.android.ui.home.conversations.model.messagetypes.multipart.MultipartAttachmentsViewModel): kotlin.Unit +public fun com.wire.android.ui.home.conversations.model.messagetypes.multipart.MultipartAttachmentsView(conversationId: com.wire.kalium.logic.data.id.QualifiedID, attachments: kotlin.collections.List, messageStyle: com.wire.android.ui.home.conversations.messages.item.MessageStyle, onImageAttachmentClick: kotlin.Function1, onVideoAttachmentClick: kotlin.Function3<@[ParameterName(name = \, onAudioAttachmentClick: kotlin.Function3<@[ParameterName(name = \, onPdfAttachmentClick: kotlin.Function5<@[ParameterName(name = \, modifier: androidx.compose.ui.Modifier, viewModel: com.wire.android.ui.home.conversations.model.messagetypes.multipart.MultipartAttachmentsViewModel): kotlin.Unit skippable: false restartable: true params: @@ -7196,6 +7208,7 @@ public fun com.wire.android.ui.home.conversations.model.messagetypes.multipart.M - onImageAttachmentClick: STABLE (function type) - onVideoAttachmentClick: STABLE (function type) - onAudioAttachmentClick: STABLE (function type) + - onPdfAttachmentClick: STABLE (function type) - modifier: STABLE (marked @Stable or @Immutable) - viewModel: RUNTIME (requires runtime check) @@ -10959,7 +10972,7 @@ private fun com.wire.android.ui.settings.devices.DeviceDetailSectionContent(sect - titleTrailingItem: STABLE (composable function type) @Composable -public fun com.wire.android.ui.settings.devices.DeviceDetailsContent(state: com.wire.android.ui.settings.devices.model.DeviceDetailsState, passwordTextState: androidx.compose.foundation.text.input.TextFieldState, handleE2EIEnrollmentResult: kotlin.Function1, modifier: androidx.compose.ui.Modifier, onDeleteDevice: kotlin.Function0, onNavigateBack: kotlin.Function0, onNavigateToE2eiCertificateDetailsScreen: kotlin.Function1, onRemoveConfirm: kotlin.Function0, onDialogDismiss: kotlin.Function0, onErrorDialogDismiss: kotlin.Function0, enrollE2eiCertificate: kotlin.Function0, onUpdateClientVerification: kotlin.Function1, onEnrollE2EIErrorDismiss: kotlin.Function0, onEnrollE2EISuccessDismiss: kotlin.Function0, onBreakSession: kotlin.Function0): kotlin.Unit +public fun com.wire.android.ui.settings.devices.DeviceDetailsContent(state: com.wire.android.ui.settings.devices.model.DeviceDetailsState, passwordTextState: androidx.compose.foundation.text.input.TextFieldState, handleE2EIEnrollmentResult: kotlin.Function1, modifier: androidx.compose.ui.Modifier, onDeleteDevice: kotlin.Function0, onNavigateBack: kotlin.Function0, onNavigateToE2eiCertificateDetailsScreen: kotlin.Function1, onRemoveConfirm: kotlin.Function0, onDialogDismiss: kotlin.Function0, onErrorDialogDismiss: kotlin.Function0, enrollE2eiCertificate: kotlin.Function0, onUpdateClientVerification: kotlin.Function1, onEnrollE2EIErrorDismiss: kotlin.Function0, onEnrollE2EISuccessDismiss: kotlin.Function0, onBreakSession: kotlin.Function0): kotlin.Unit skippable: false restartable: true params: From 7f1ffb2111a2c519fa12f286082b910a9001dfdc Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 4 Sep 2026 11:53:32 +0200 Subject: [PATCH 09/12] chore: test --- .../android/feature/cells/ui/CellViewModelTest.kt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt b/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt index f244b5a7e04..9ab37cbbcbd 100644 --- a/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt +++ b/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt @@ -326,11 +326,13 @@ class CellViewModelTest { } @Test - fun `given view model when non-image file clicked and local file is not present and url is openable then url is opened`() = runTest { + fun `given view model when a file type that cannot be opened by url is clicked then it is downloaded first`() = runTest { val (arrangement, viewModel) = Arrangement() .withLoadSuccess() + .withDownloadSuccess() .arrange() + // Only image, video, audio and pdf can be opened straight from the content url. val testFile = testFiles[0].copy( mimeType = "text/plain", localPath = null, @@ -338,8 +340,10 @@ class CellViewModelTest { ) viewModel.sendIntent(CellViewIntent.OnItemClick(testFile.toUiModel())) + advanceUntilIdle() - coVerify(exactly = 1) { arrangement.fileHelper.openAssetUrlWithExternalApp(any(), any(), any()) } + coVerify(exactly = 0) { arrangement.fileHelper.openAssetUrlWithExternalApp(any(), any(), any()) } + coVerify(exactly = 1) { arrangement.downloadCellFileUseCase(any(), any(), any(), any(), any(), any(), any(), any()) } } @Test @@ -391,8 +395,8 @@ class CellViewModelTest { .arrange() // File has localPath from DB but also carries an error state (stale UI state) - // Use a non-image file so we can verify the external app opener is called - val testFile = testFiles[0].copy(localPath = "localPath", contentUrl = null, mimeType = "application/pdf").toUiModel() + // Use a file type without an in-app viewer so we can verify the external app opener is called + val testFile = testFiles[0].copy(localPath = "localPath", contentUrl = null, mimeType = "application/zip").toUiModel() .copy(openLoadState = OpenLoadState.Error) viewModel.sendIntent(CellViewIntent.OnItemClick(testFile)) From 48a77196e8fbceb0a9888d4d0a5d0fe741ab0ff1 Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 4 Sep 2026 12:51:44 +0200 Subject: [PATCH 10/12] chore: test --- .../multipart/MultipartAttachmentsViewModelTest.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt index 3f436a1571b..fe8dafde38a 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/multipart/MultipartAttachmentsViewModelTest.kt @@ -187,7 +187,7 @@ class MultipartAttachmentsViewModelTest { val callback = mockk(relaxed = true) - viewModel.onClick(testAttachmentUi, callback, {}, {}) + viewModel.onClick(testAttachmentUi, callback, {}, {}, {}) coVerify(exactly = 1) { callback.invoke(testAttachmentUi.uuid) } } @@ -259,12 +259,13 @@ class MultipartAttachmentsViewModelTest { } @Test - fun `with file attachment openable via url when clicked then file is opened via url`() = runTest { + fun `with file attachment that cannot be opened from its url when clicked then it is downloaded`() = runTest { val (arrangement, viewModel) = Arrangement() .arrange() val callback = mockk(relaxed = true) + // Only image, video and audio are opened straight from the content url. viewModel.onClick( attachment = testAttachmentUi.copy( mimeType = "application/zip", @@ -277,7 +278,8 @@ class MultipartAttachmentsViewModelTest { openInPdfViewer = { }, ) - coVerify(exactly = 1) { arrangement.fileManager.openUrlWithExternalApp(any(), any(), any()) } + coVerify(exactly = 0) { arrangement.fileManager.openUrlWithExternalApp(any(), any(), any()) } + coVerify(exactly = 1) { arrangement.download(any(), any(), any(), any(), any(), any(), any(), any()) } } @Test @@ -296,6 +298,7 @@ class MultipartAttachmentsViewModelTest { attachment = attachment, openInImageViewer = { }, openInVideoPlayer = { }, + openInAudioPlayer = { }, openInPdfViewer = callback, ) @@ -319,6 +322,7 @@ class MultipartAttachmentsViewModelTest { attachment = attachment, openInImageViewer = { }, openInVideoPlayer = { }, + openInAudioPlayer = { }, openInPdfViewer = callback, ) @@ -337,9 +341,11 @@ class MultipartAttachmentsViewModelTest { attachment = testAttachmentUi.copy( mimeType = "application/pdf", assetType = AttachmentFileType.PDF, + remotePath = null, ), openInImageViewer = { }, openInVideoPlayer = { }, + openInAudioPlayer = { }, openInPdfViewer = callback, ) @@ -440,6 +446,8 @@ class MultipartAttachmentsViewModelTest { source = AssetSource.CELL, fileName = "filename", localPath = null, + // Mirrors testAssetContent.assetPath, which toUiModel() maps onto remotePath. + remotePath = "/filename", mimeType = "image/png", assetType = AttachmentFileType.IMAGE, assetSize = 0, From 7fdcc1446c9cf88ad6dc5b241c34c24d79e2679d Mon Sep 17 00:00:00 2001 From: ohassine Date: Mon, 7 Sep 2026 13:48:02 +0200 Subject: [PATCH 11/12] chore: address cmments --- .../com/wire/android/pdfviewer/PdfDocument.kt | 100 ++++++++++++------ .../android/pdfviewer/PdfSourceResolver.kt | 14 +++ .../com/wire/android/pdfviewer/PdfViewer.kt | 22 +++- .../android/pdfviewer/PdfViewerViewModel.kt | 21 ++-- .../src/main/res/values/strings.xml | 1 + .../pdfviewer/PdfDocumentRenderSizeTest.kt | 98 +++++++++++++++++ .../pdfviewer/PdfSourceResolverTest.kt | 56 ++++++++++ .../pdfviewer/PdfViewerViewModelTest.kt | 60 ++++++++--- .../android/feature/cells/ui/CellViewModel.kt | 3 +- .../feature/cells/ui/CellViewModelTest.kt | 24 +++++ kalium | 2 +- 11 files changed, 347 insertions(+), 54 deletions(-) create mode 100644 core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt index f799d27dacb..08bb7596d2a 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfDocument.kt @@ -25,15 +25,16 @@ import java.io.Closeable import java.io.File import java.util.concurrent.locks.ReentrantLock import kotlin.concurrent.withLock +import kotlin.math.sqrt /** - * Thin coroutine-friendly wrapper around the platform [PdfRenderer]. + * Thin wrapper around the platform [PdfRenderer]. Obtain instances via [openPdfDocument]. * - * Everything goes through [mutex] because [PdfRenderer] only allows a single open page at a time + * Everything goes through [lock] because [PdfRenderer] only allows a single open page at a time * and is not thread safe. Rendering happens entirely in-process — no network access and no * third party parser — which keeps documents from ever leaving the device. */ -internal class PdfDocument private constructor( +internal class PdfDocument internal constructor( private val descriptor: ParcelFileDescriptor, private val renderer: PdfRenderer, ) : Closeable { @@ -52,12 +53,19 @@ internal class PdfDocument private constructor( val pageCount: Int = renderer.pageCount - /** Width / height of [pageIndex], used to reserve the right amount of space before rendering. */ + /** + * Width / height of [pageIndex], used to reserve the right amount of space before rendering. + * + * Never throws: `openPage` can fail on a malformed page, and callers reserve space from + * non-failing contexts, so a broken page falls back to the default ratio instead. + */ fun aspectRatio(pageIndex: Int): Float = lock.withLock { if (closed) return DEFAULT_ASPECT_RATIO - renderer.openPage(pageIndex).use { page -> - if (page.height == 0) DEFAULT_ASPECT_RATIO else page.width.toFloat() / page.height - } + runCatching { + renderer.openPage(pageIndex).use { page -> + if (page.height == 0) DEFAULT_ASPECT_RATIO else page.width.toFloat() / page.height + } + }.getOrDefault(DEFAULT_ASPECT_RATIO) } /** @@ -68,14 +76,11 @@ internal class PdfDocument private constructor( fun renderPage(pageIndex: Int, widthPx: Int): Bitmap? = lock.withLock { if (closed) return null renderer.openPage(pageIndex).use { page -> - val safeWidth = widthPx.coerceIn(MIN_RENDER_WIDTH_PX, MAX_RENDER_WIDTH_PX) - val height = if (page.width == 0) { - safeWidth - } else { - (safeWidth.toLong() * page.height / page.width).toInt() - }.coerceIn(MIN_RENDER_WIDTH_PX, MAX_RENDER_WIDTH_PX) - - Bitmap.createBitmap(safeWidth, height, Bitmap.Config.ARGB_8888).apply { + val (width, height) = renderSize(page.width, page.height, widthPx) + + // Must be ARGB_8888: PdfRenderer.Page.render rejects every other config except + // ALPHA_8 with "Unsupported pixel format". Memory is bounded by MAX_RENDER_PIXELS. + Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888).apply { // PdfRenderer draws only the page content, so the paper itself has to be painted. eraseColor(Color.WHITE) page.render(this, null, null, PdfRenderer.Page.RENDER_MODE_FOR_DISPLAY) @@ -97,25 +102,58 @@ internal class PdfDocument private constructor( } companion object { - const val DEFAULT_ASPECT_RATIO = 1f / 1.414f // A4 portrait - private const val MIN_RENDER_WIDTH_PX = 1 - private const val MAX_RENDER_WIDTH_PX = 4_096 + /** A4 portrait: the shape assumed for a page that could not be measured. */ + const val DEFAULT_ASPECT_RATIO = 1f / 1.414f + + const val MIN_RENDER_PX = 1 /** - * Opens [file] for rendering, translating the platform failures into a [PdfViewerError]. + * Total pixel budget for one page bitmap: 16 MB at [Bitmap.Config.ARGB_8888] * - * [PdfRenderer] throws [SecurityException] for password protected documents and - * [java.io.IOException] for anything it cannot parse. + * Bounding the area rather than the width is what keeps tall pages undistorted: a width + * cap alone would squash an A4 page once the requested width pushed its height past the + * limit. */ - @Suppress("TooGenericExceptionCaught") - fun open(file: File): Result = runCatching { - val descriptor = ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY) - try { - PdfDocument(descriptor, PdfRenderer(descriptor)) - } catch (error: Throwable) { - runCatching { descriptor.close() } - throw error - } - } + const val MAX_RENDER_PIXELS = 4_000_000L + } +} + +/** + * Opens [file] for rendering, translating the platform failures into a [PdfViewerError]. + * + * [PdfRenderer] throws [SecurityException] for password protected documents and + * [java.io.IOException] for anything it cannot parse. + */ +@Suppress("TooGenericExceptionCaught") +internal fun openPdfDocument(file: File): Result = runCatching { + val descriptor = ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY) + try { + PdfDocument(descriptor, PdfRenderer(descriptor)) + } catch (error: Throwable) { + runCatching { descriptor.close() } + throw error + } +} + +/** + * Size of the bitmap for a [pageWidth] x [pageHeight] page requested at [requestedWidth], + * preserving the page aspect ratio and never exceeding [PdfDocument.MAX_RENDER_PIXELS]. + */ +internal fun renderSize(pageWidth: Int, pageHeight: Int, requestedWidth: Int): Pair { + val ratio = if (pageWidth <= 0 || pageHeight <= 0) { + PdfDocument.DEFAULT_ASPECT_RATIO + } else { + pageWidth.toFloat() / pageHeight } + + val width = requestedWidth.coerceAtLeast(PdfDocument.MIN_RENDER_PX) + val height = (width / ratio).toInt().coerceAtLeast(PdfDocument.MIN_RENDER_PX) + + val pixels = width.toLong() * height + if (pixels <= PdfDocument.MAX_RENDER_PIXELS) return width to height + + // Shrink both axes by the same factor so the page keeps its shape. + val factor = sqrt(PdfDocument.MAX_RENDER_PIXELS.toDouble() / pixels) + return (width * factor).toInt().coerceAtLeast(PdfDocument.MIN_RENDER_PX) to + (height * factor).toInt().coerceAtLeast(PdfDocument.MIN_RENDER_PX) } diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt index 4ba6fda089f..11fdc2d7b67 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfSourceResolver.kt @@ -73,6 +73,9 @@ class PdfSourceResolver @Inject constructor( return runCatching { partial.parentFile?.mkdirs() remoteLoader.load(assetId, remotePath, conversationId, assetSize, partial).getOrThrow() + check(partial.length() > 0 && (assetSize <= 0 || partial.length() >= assetSize)) { + "Downloaded document is incomplete (${partial.length()} of $assetSize bytes)" + } check(partial.renameTo(target)) { "Could not move the downloaded document into place" } target }.recoverCatching { cause -> @@ -81,6 +84,17 @@ class PdfSourceResolver @Inject constructor( } } + /** + * Drops the cached copy of [assetId] so the next [resolve] downloads it again. + * + * Only ever touches this module's cache directory — a caller-supplied `localPath` belongs to + * whoever downloaded it and must not be deleted here. + */ + suspend fun invalidate(assetId: String?, dispatcher: CoroutineDispatcher = Dispatchers.IO) { + val id = assetId ?: return + withContext(dispatcher) { cacheFileFor(id).delete() } + } + private fun cacheFileFor(assetId: String): File = File(File(context.cacheDir, CACHE_DIR_NAME), "$assetId.pdf") diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt index d5a7f27bf58..cc4d51c7d2c 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewer.kt @@ -78,7 +78,7 @@ private const val MIN_ZOOM = 1f private const val MAX_ZOOM = 5f /** Beyond this the extra pixels are no longer visible but the bitmaps get very expensive. */ -private const val MAX_RENDER_SCALE = 3f +private const val MAX_RENDER_SCALE = 2f private const val DOUBLE_TAP_ZOOM = 2.5f /** @@ -250,10 +250,17 @@ private fun PdfPage( // Deliberately keyed on the page only: while a sharper bitmap is rendered after a zoom the // previous one stays on screen instead of flashing back to a spinner. var bitmap by remember(pageIndex) { mutableStateOf(null) } + var failed by remember(pageIndex) { mutableStateOf(false) } LaunchedEffect(pageIndex, widthPx) { if (widthPx > 0) { - renderPage(pageIndex, widthPx)?.let { bitmap = it } + val rendered = renderPage(pageIndex, widthPx) + if (rendered != null) { + bitmap = rendered + failed = false + } else { + failed = bitmap == null + } } } @@ -280,6 +287,14 @@ private fun PdfPage( modifier = Modifier.fillMaxSize(), contentScale = ContentScale.Fit, ) + } else if (failed) { + Text( + text = stringResource(R.string.pdf_viewer_error_page_not_rendered), + style = typography().body02, + color = colorsScheme().secondaryText, + textAlign = TextAlign.Center, + modifier = Modifier.padding(dimensions().spacing16x), + ) } else { WireCircularProgressIndicator( progressColor = colorsScheme().secondaryText, @@ -322,7 +337,8 @@ private fun PdfViewerError.messageResId(): Int = when (this) { PdfViewerError.INVALID_DOCUMENT -> R.string.pdf_viewer_error_invalid_document } -private fun PdfViewerError.isRetryable(): Boolean = this == PdfViewerError.DOWNLOAD_FAILED +private fun PdfViewerError.isRetryable(): Boolean = + this == PdfViewerError.DOWNLOAD_FAILED || this == PdfViewerError.INVALID_DOCUMENT /** * Pinch to zoom plus horizontal panning, layered on top of the list's own vertical scrolling. diff --git a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt index 996fd9ad382..82ff9f56aec 100644 --- a/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt +++ b/core/pdf-viewer/src/main/kotlin/com/wire/android/pdfviewer/PdfViewerViewModel.kt @@ -70,7 +70,6 @@ class PdfViewerViewModel @AssistedInject constructor( private val _state = MutableStateFlow(PdfViewerState.Loading) val state: StateFlow = _state.asStateFlow() - // Written from the main thread by load()/onCleared(), read from IO by renderPage(). @Volatile private var document: PdfDocument? = null private var loadJob: Job? = null @@ -82,7 +81,6 @@ class PdfViewerViewModel @AssistedInject constructor( */ private val releaseScope = CoroutineScope(SupervisorJob() + dispatchers.io()) - /** Keeps recently rendered pages around so scrolling back does not re-rasterise them. */ private val pageCache = PageBitmapCache(PageBitmapCache.defaultMaxBytes()) init { @@ -124,26 +122,37 @@ class PdfViewerViewModel @AssistedInject constructor( return@launch } - val opened = withContext(dispatchers.io()) { PdfDocument.open(file) } + val opened = withContext(dispatchers.io()) { openPdfDocument(file) } .getOrElse { cause -> - _state.value = PdfViewerState.Failure(cause.toViewerError()) + failAfterUnusableDownload(cause.toViewerError()) return@launch } if (opened.pageCount == 0) { opened.close() - _state.value = PdfViewerState.Failure(PdfViewerError.INVALID_DOCUMENT) + failAfterUnusableDownload(PdfViewerError.INVALID_DOCUMENT) return@launch } + val firstPageAspectRatio = withContext(dispatchers.io()) { opened.aspectRatio(0) } + document = opened _state.value = PdfViewerState.Content( pageCount = opened.pageCount, - firstPageAspectRatio = withContext(dispatchers.io()) { opened.aspectRatio(0) }, + firstPageAspectRatio = firstPageAspectRatio, ) } } + /** + * Reports [error] and evicts the cached download, so that retrying fetches the file again + * instead of re-opening the same unusable bytes forever. + */ + private suspend fun failAfterUnusableDownload(error: PdfViewerError) { + sourceResolver.invalidate(assetId, dispatchers.io()) + _state.value = PdfViewerState.Failure(error) + } + private fun closeDocument() { pageCache.clear() val open = document ?: return diff --git a/core/pdf-viewer/src/main/res/values/strings.xml b/core/pdf-viewer/src/main/res/values/strings.xml index b0c8f4a0ec8..acb823794ec 100644 --- a/core/pdf-viewer/src/main/res/values/strings.xml +++ b/core/pdf-viewer/src/main/res/values/strings.xml @@ -7,5 +7,6 @@ This document could not be downloaded. This document is password protected and cannot be opened here. This document could not be opened. + This page could not be displayed. Try again \ No newline at end of file diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt new file mode 100644 index 00000000000..257e5d93b4b --- /dev/null +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt @@ -0,0 +1,98 @@ +/* + * Wire + * Copyright (C) 2026 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.pdfviewer + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import kotlin.math.abs + +internal class PdfDocumentRenderSizeTest { + + @Test + fun `given a page that fits the budget, when sizing it, then the requested width is used`() { + val (width, height) = renderSize(A4_WIDTH_PT, A4_HEIGHT_PT, requestedWidth = 1080) + + assertEquals(1080, width) + assertAspectRatioPreserved(width, height) + } + + @Test + fun `given a tall page zoomed past the budget, when sizing it, then the aspect ratio is preserved`() { + // The regression: clamping height alone squashed an A4 page once the requested width + // pushed its height past the old 4096 px limit. + val (width, height) = renderSize(A4_WIDTH_PT, A4_HEIGHT_PT, requestedWidth = 1080 * 3) + + assertAspectRatioPreserved(width, height) + assertTrue(width < 1080 * 3, "expected the width to be scaled down, was $width") + } + + @Test + fun `given any zoom level, when sizing a page, then the pixel budget is respected`() { + listOf(1080, 2160, 3240, 20_000).forEach { requestedWidth -> + val (width, height) = renderSize(A4_WIDTH_PT, A4_HEIGHT_PT, requestedWidth) + + assertTrue( + width.toLong() * height <= PdfDocument.MAX_RENDER_PIXELS, + "budget exceeded at width $requestedWidth: ${width}x$height", + ) + } + } + + @Test + fun `given a landscape page, when sizing it, then the aspect ratio is preserved`() { + val (width, height) = renderSize(A4_HEIGHT_PT, A4_WIDTH_PT, requestedWidth = 4000) + + assertTrue(width > height, "landscape page should stay wider than tall, was ${width}x$height") + assertAspectRatioPreserved(width, height, expected = A4_HEIGHT_PT.toFloat() / A4_WIDTH_PT) + } + + @Test + fun `given a degenerate page size, when sizing it, then the default ratio is used`() { + val (width, height) = renderSize(pageWidth = 0, pageHeight = 0, requestedWidth = 1000) + + assertAspectRatioPreserved(width, height, expected = PdfDocument.DEFAULT_ASPECT_RATIO) + } + + @Test + fun `given a non positive requested width, when sizing it, then the bitmap is still valid`() { + val (width, height) = renderSize(A4_WIDTH_PT, A4_HEIGHT_PT, requestedWidth = 0) + + assertTrue(width >= 1 && height >= 1, "bitmap dimensions must stay positive, was ${width}x$height") + } + + private fun assertAspectRatioPreserved( + width: Int, + height: Int, + expected: Float = A4_WIDTH_PT.toFloat() / A4_HEIGHT_PT, + ) { + val actual = width.toFloat() / height + assertTrue( + abs(actual - expected) <= RATIO_TOLERANCE, + "expected ratio ~$expected but was $actual (${width}x$height)", + ) + } + + private companion object { + const val A4_WIDTH_PT = 595 + const val A4_HEIGHT_PT = 842 + + // Integer rounding of both axes moves the ratio slightly. + const val RATIO_TOLERANCE = 0.01f + } +} \ No newline at end of file diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt index da1459dc3ba..b6f4d7e5b3c 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfSourceResolverTest.kt @@ -162,6 +162,62 @@ internal class PdfSourceResolverTest { coVerify(exactly = 1) { loader.load(any(), any(), any(), any(), any()) } } + @Test + fun givenATruncatedDownload_whenResolving_thenItFailsAndNothingIsCached() = runTest { + val loader = writingLoader(bytes = 8) + val resolver = resolver(loader) + + // Promoting a short file would poison the cache: every later open would fail. + val result = resolver.resolve(null, "asset-short", "/cells/path/doc.pdf", null, 4096L, Dispatchers.Default) + + assertEquals(PdfViewerError.DOWNLOAD_FAILED, result.viewerError()) + + val second = resolver.resolve(null, "asset-short", "/cells/path/doc.pdf", null, 4096L, Dispatchers.Default) + assertTrue(second.isFailure) + coVerify(exactly = 2) { loader.load(any(), any(), any(), any(), any()) } + } + + @Test + fun givenACompleteDownload_whenResolving_thenItSucceeds() = runTest { + val resolver = resolver(writingLoader(bytes = 4096)) + + val result = resolver.resolve(null, "asset-full", "/cells/path/doc.pdf", null, 4096L, Dispatchers.Default) + + assertTrue(result.isSuccess) + assertEquals(4096L, result.getOrNull()?.length()) + } + + @Test + fun givenACachedAsset_whenInvalidated_thenTheNextResolveDownloadsAgain() = runTest { + val loader = writingLoader(bytes = 8) + val resolver = resolver(loader) + + resolver.resolve(null, "asset-abc", "/cells/path/doc.pdf", null, 0L, Dispatchers.Default) + resolver.invalidate("asset-abc", Dispatchers.Default) + val result = resolver.resolve(null, "asset-abc", "/cells/path/doc.pdf", null, 0L, Dispatchers.Default) + + assertTrue(result.isSuccess) + coVerify(exactly = 2) { loader.load(any(), any(), any(), any(), any()) } + } + + @Test + fun givenALocalFile_whenInvalidating_thenThatFileIsLeftAlone() = runTest { + val document = File(tempDir, "document.pdf").apply { writeText("%PDF-1.4") } + val resolver = resolver() + + // invalidate() must only ever touch this module's cache, never a caller's own download. + resolver.invalidate("document", Dispatchers.Default) + + assertTrue(document.exists()) + } + + private fun writingLoader(bytes: Int): PdfRemoteLoader = mockk { + coEvery { load(any(), any(), any(), any(), any()) } coAnswers { + arg(4).writeBytes(ByteArray(bytes)) + Result.success(Unit) + } + } + private fun resolver(loader: PdfRemoteLoader = mockk(relaxed = true)): PdfSourceResolver { val context = mockk() every { context.cacheDir } returns File(tempDir, "cache") diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt index 70bffce837b..50a6a65926c 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfViewerViewModelTest.kt @@ -24,8 +24,8 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk -import io.mockk.mockkObject -import io.mockk.unmockkObject +import io.mockk.mockkStatic +import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.test.runTest @@ -43,7 +43,7 @@ internal class PdfViewerViewModelTest { @AfterEach fun tearDown() { - unmockkObject(PdfDocument.Companion) + unmockkStatic(PDF_DOCUMENT_FILE_FACADE) } @Test @@ -72,7 +72,7 @@ internal class PdfViewerViewModelTest { .arrange() assertEquals(PdfViewerState.Failure(PdfViewerError.DOWNLOAD_FAILED), viewModel.state.value) - verify(exactly = 0) { PdfDocument.open(any()) } + verify(exactly = 0) { openPdfDocument(any()) } coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } } @@ -174,7 +174,7 @@ internal class PdfViewerViewModelTest { viewModel.retry() verify(exactly = 1) { arrangement.document.close() } - verify(exactly = 2) { PdfDocument.open(any()) } + verify(exactly = 2) { openPdfDocument(any()) } } @Test @@ -198,7 +198,7 @@ internal class PdfViewerViewModelTest { arrangement.withResolveSuccess().withPageCount(3) viewModel.retry() - assertEquals(PdfViewerState.Content(pageCount = 3, firstPageAspectRatio = DEFAULT_ASPECT_RATIO), viewModel.state.value) + assertEquals(PdfViewerState.Content(pageCount = 3, firstPageAspectRatio = TEST_ASPECT_RATIO), viewModel.state.value) coVerify(exactly = 2) { arrangement.sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } } @@ -216,12 +216,44 @@ internal class PdfViewerViewModelTest { gate.complete(Unit) assertEquals( - PdfViewerState.Content(pageCount = DEFAULT_PAGE_COUNT, firstPageAspectRatio = DEFAULT_ASPECT_RATIO), + PdfViewerState.Content(pageCount = DEFAULT_PAGE_COUNT, firstPageAspectRatio = TEST_ASPECT_RATIO), viewModel.state.value, ) coVerify(exactly = 1) { arrangement.sourceResolver.resolve(any(), any(), any(), any(), any(), any()) } } + @Test + fun `given the downloaded file cannot be opened, when loading, then the cached copy is evicted`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withOpenFailure(IOException("truncated")) + .arrange() + + // Without the eviction the same unusable bytes would be re-opened on every retry. + assertEquals(PdfViewerState.Failure(PdfViewerError.INVALID_DOCUMENT), viewModel.state.value) + coVerify(exactly = 1) { arrangement.sourceResolver.invalidate(any(), any()) } + } + + @Test + fun `given a document without pages, when loading, then the cached copy is evicted`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withPageCount(0) + .arrange() + + assertEquals(PdfViewerState.Failure(PdfViewerError.INVALID_DOCUMENT), viewModel.state.value) + coVerify(exactly = 1) { arrangement.sourceResolver.invalidate(any(), any()) } + } + + @Test + fun `given the source cannot be resolved, when loading, then no eviction is attempted`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withResolveFailure(PdfViewerError.FILE_NOT_FOUND) + .arrange() + + // Nothing was downloaded, so there is nothing to evict. + assertEquals(PdfViewerState.Failure(PdfViewerError.FILE_NOT_FOUND), viewModel.state.value) + coVerify(exactly = 0) { arrangement.sourceResolver.invalidate(any(), any()) } + } + private class Arrangement { val sourceResolver: PdfSourceResolver = mockk() @@ -231,11 +263,12 @@ internal class PdfViewerViewModelTest { private val file = File("document.pdf") init { - mockkObject(PdfDocument.Companion) - every { PdfDocument.open(any()) } returns Result.success(document) + mockkStatic(PDF_DOCUMENT_FILE_FACADE) + every { openPdfDocument(any()) } returns Result.success(document) every { document.pageCount } returns DEFAULT_PAGE_COUNT - every { document.aspectRatio(any()) } returns DEFAULT_ASPECT_RATIO + every { document.aspectRatio(any()) } returns TEST_ASPECT_RATIO every { document.renderPage(any(), any()) } returns bitmap + coEvery { sourceResolver.invalidate(any(), any()) } returns Unit withResolveSuccess() } @@ -256,7 +289,7 @@ internal class PdfViewerViewModelTest { } fun withOpenFailure(cause: Throwable) = apply { - every { PdfDocument.open(any()) } returns Result.failure(cause) + every { openPdfDocument(any()) } returns Result.failure(cause) } fun withPageCount(count: Int) = apply { @@ -288,8 +321,11 @@ internal class PdfViewerViewModelTest { } private companion object { + /** Kotlin file facade holding the top-level `openPdfDocument`. */ + const val PDF_DOCUMENT_FILE_FACADE = "com.wire.android.pdfviewer.PdfDocumentKt" + const val DEFAULT_PAGE_COUNT = 3 - const val DEFAULT_ASPECT_RATIO = 0.7f + const val TEST_ASPECT_RATIO = 0.7f const val BITMAP_BYTES = 1024 } } diff --git a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt index 3f5ddcd3046..a8e61f07457 100644 --- a/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt +++ b/features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt @@ -475,7 +475,8 @@ class CellViewModel @AssistedInject constructor( AttachmentFileType.IMAGE -> OpenImageViewer(file).takeIf { file.shouldOpenInAppImageViewer() } AttachmentFileType.VIDEO -> OpenVideoViewer(file) AttachmentFileType.AUDIO -> OpenAudioPlayer(file) - AttachmentFileType.PDF -> OpenPdfViewer(file) + AttachmentFileType.PDF -> + OpenPdfViewer(file).takeIf { file.localPath != null || file.remotePath != null } else -> null } diff --git a/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt b/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt index 9ab37cbbcbd..01221380f0a 100644 --- a/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt +++ b/features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/CellViewModelTest.kt @@ -352,6 +352,7 @@ class CellViewModelTest { .withLoadSuccess() .arrange() + // remotePath is what the in-app viewer downloads from; testFiles[0] provides one. val testFile = testFiles[0].copy( mimeType = "application/pdf", localPath = null, @@ -367,6 +368,29 @@ class CellViewModelTest { coVerify(exactly = 0) { arrangement.fileHelper.openAssetUrlWithExternalApp(any(), any(), any()) } } + @Test + fun `given view model when pdf file clicked with no local path and no remote path then url is opened externally`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withLoadSuccess() + .arrange() + + // Without a remote path the in-app viewer has nothing to download, so claiming the click + // would leave the user on an unrecoverable error screen instead of opening the file. + val testFile = testFiles[0].copy( + mimeType = "application/pdf", + localPath = null, + remotePath = null, + contentUrl = "https://example.com/file" + ) + + viewModel.actions.test { + viewModel.sendIntent(CellViewIntent.OnItemClick(testFile.toUiModel())) + + expectNoEvents() + } + coVerify(exactly = 1) { arrangement.fileHelper.openAssetUrlWithExternalApp(any(), any(), any()) } + } + @Test fun `given view model when file clicked and local file is not present and url is not openable then download starts immediately`() = runTest { diff --git a/kalium b/kalium index a1552ff4ba8..fae98945c4c 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit a1552ff4ba856589bb3662b6e3ca6ca28997a55e +Subproject commit fae98945c4c9abbd85e84e2a9b512dcb185bacca From ff3e5c9945cb643a45034a1f029ef9c0729e516a Mon Sep 17 00:00:00 2001 From: ohassine Date: Mon, 7 Sep 2026 16:25:07 +0200 Subject: [PATCH 12/12] chore: detekt --- .../com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt index 257e5d93b4b..af0187b1c4d 100644 --- a/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt +++ b/core/pdf-viewer/src/test/kotlin/com/wire/android/pdfviewer/PdfDocumentRenderSizeTest.kt @@ -95,4 +95,4 @@ internal class PdfDocumentRenderSizeTest { // Integer rounding of both axes moves the ratio slightly. const val RATIO_TOLERANCE = 0.01f } -} \ No newline at end of file +}