swift/Beam
swift/Beam
1
0
Fork 0
mirror of https://github.com/element-hq/element-x-ios.git synced 2025-03-10 21:39:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

Handle media source validation more gracefully. (#3571)

Browse Source 
* Handle media source validation more gracefully.

* Fix unit tests.
This commit is contained in:
Doug 2024-11-29 12:42:27 +00:00 committed by GitHub
parent c8627cfd64
commit 49a94e0bdb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 95 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
ElementX/Sources/Other/SwiftUI/Views/LoadableImage.swift
View File

@ -21,7 +21,7 @@ enum LoadableImageMediaType: Equatable {
}
struct LoadableImage<TransformerView: View, PlaceholderView: View>: View {
private let mediaSource: MediaSourceProxy
private let mediaSource: MediaSourceProxy?
private let mediaType: LoadableImageMediaType
private let blurhash: String?
private let size: CGSize?
@ -60,16 +60,17 @@ struct LoadableImage<TransformerView: View, PlaceholderView: View>: View {
mediaProvider: MediaProviderProtocol?,
transformer: @escaping (AnyView) -> TransformerView = { $0 },
placeholder: @escaping () -> PlaceholderView) {
self.init(mediaSource: MediaSourceProxy(url: url, mimeType: nil),
mediaType: mediaType,
blurhash: blurhash,
size: size,
mediaProvider: mediaProvider,
transformer: transformer,
placeholder: placeholder)
mediaSource = try? MediaSourceProxy(url: url, mimeType: nil)
self.mediaType = mediaType
self.blurhash = blurhash
self.size = size
self.mediaProvider = mediaProvider
self.transformer = transformer
self.placeholder = placeholder
}
var body: some View {
if let mediaSource {
LoadableImageContent(mediaSource: mediaSource,
mediaType: mediaType,
blurhash: blurhash,
@ -78,9 +79,12 @@ struct LoadableImage<TransformerView: View, PlaceholderView: View>: View {
transformer: transformer,
placeholder: placeholder)
.id(stableMediaIdentifier)
} else {
placeholder()
}
}
private var stableMediaIdentifier: String {
private var stableMediaIdentifier: String? {
switch mediaType {
case .timelineItem(let uniqueID):
// Consider media for the same item to be the same view
@ -88,7 +92,7 @@ struct LoadableImage<TransformerView: View, PlaceholderView: View>: View {
default:
// Binds the lifecycle of the LoadableImage to the associated URL.
// This fixes the problem of the cache returning old values after a change in the URL.
mediaSource.url.absoluteString
mediaSource?.url.absoluteString
}
}
}

3
ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift
View File

@ -363,7 +363,8 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
}
// We don't actually know the mime type here, assume it's an image.
if case let .success(file) = await mediaProvider.loadFileFromSource(.init(url: url, mimeType: "image/jpeg")) {
if let mediaSource = try? MediaSourceProxy(url: url, mimeType: "image/jpeg"),
case let .success(file) = await mediaProvider.loadFileFromSource(mediaSource) {
state.bindings.mediaPreviewItem = MediaPreviewItem(file: file, title: roomProxy.infoPublisher.value.displayName)
}
}

3
ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift
View File

@ -165,7 +165,8 @@ class RoomMemberDetailsScreenViewModel: RoomMemberDetailsScreenViewModelType, Ro
defer { userIndicatorController.retractIndicatorWithId(loadingIndicatorIdentifier) }
// We don't actually know the mime type here, assume it's an image.
if case let .success(file) = await mediaProvider.loadFileFromSource(.init(url: url, mimeType: "image/jpeg")) {
if let mediaSource = try? MediaSourceProxy(url: url, mimeType: "image/jpeg"),
case let .success(file) = await mediaProvider.loadFileFromSource(mediaSource) {
state.bindings.mediaPreviewItem = MediaPreviewItem(file: file, title: roomMemberProxy.displayName)
}
}

3
ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenViewModel.swift
View File

@ -104,7 +104,8 @@ class UserProfileScreenViewModel: UserProfileScreenViewModelType, UserProfileScr
defer { hideLoadingIndicator() }
// We don't actually know the mime type here, assume it's an image.
if case let .success(file) = await mediaProvider.loadFileFromSource(.init(url: url, mimeType: "image/jpeg")) {
if let mediaSource = try? MediaSourceProxy(url: url, mimeType: "image/jpeg"),
case let .success(file) = await mediaProvider.loadFileFromSource(mediaSource) {
state.bindings.mediaPreviewItem = MediaPreviewItem(file: file, title: userProfile.displayName)
}
}

7
ElementX/Sources/Services/Media/Provider/MediaSourceProxy.swift
View File

@ -23,11 +23,8 @@ struct MediaSourceProxy: Hashable {
self.mimeType = mimeType
}
init(url: URL, mimeType: String?) {
guard let mediaSource = try? MediaSource.fromUrl(url: url.absoluteString) else {
fatalError("Unable to create MediaSource from URL: \(url.absoluteString)")
}
underlyingSource = mediaSource
init(url: URL, mimeType: String?) throws {
underlyingSource = try MediaSource.fromUrl(url: url.absoluteString)
self.url = URL(string: underlyingSource.url())
self.mimeType = mimeType
}

4
ElementX/Sources/Services/Notification/Proxy/NotificationItemProxy.swift
View File

@ -55,7 +55,7 @@ struct NotificationItemProxy: NotificationItemProxyProtocol {
var senderAvatarMediaSource: MediaSourceProxy? {
if let senderAvatarURLString = notificationItem.senderInfo.avatarUrl,
let senderAvatarURL = URL(string: senderAvatarURLString) {
return MediaSourceProxy(url: senderAvatarURL, mimeType: nil)
return try? MediaSourceProxy(url: senderAvatarURL, mimeType: nil)
}
return nil
}
@ -63,7 +63,7 @@ struct NotificationItemProxy: NotificationItemProxyProtocol {
var roomAvatarMediaSource: MediaSourceProxy? {
if let roomAvatarURLString = notificationItem.roomInfo.avatarUrl,
let roomAvatarURL = URL(string: roomAvatarURLString) {
return MediaSourceProxy(url: roomAvatarURL, mimeType: nil)
return try? MediaSourceProxy(url: roomAvatarURL, mimeType: nil)
}
return nil
}

4
ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift
View File

@ -195,9 +195,7 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
}
}
if let avatarURL {
let mediaSource = MediaSourceProxy(url: avatarURL, mimeType: nil)
if let avatarURL, let mediaSource = try? MediaSourceProxy(url: avatarURL, mimeType: nil) {
if case let .success(avatarData) = await mediaProvider.loadThumbnailForSource(source: mediaSource, size: .init(width: 100, height: 100)) {
sendMessageIntent.setImage(INImage(imageData: avatarData), forParameterNamed: \.speakableGroupName)
} else {

22
ElementX/Sources/Services/Timeline/TimelineItemProxy.swift
View File

@ -243,7 +243,11 @@ struct VideoInfoProxy: Hashable {
}
static var mockVideo: VideoInfoProxy {
.init(source: .init(url: .mockMXCVideo, mimeType: nil),
guard let mediaSource = try? MediaSourceProxy(url: .mockMXCVideo, mimeType: nil) else {
fatalError("Invalid mock media source URL")
}
return .init(source: mediaSource,
duration: 100,
size: .init(width: 1920, height: 1080),
aspectRatio: 1.78,
@ -269,10 +273,6 @@ struct ImageInfoProxy: Hashable {
self.init(source: .init(source: source, mimeType: mimeType), width: width, height: height, mimeType: mimeType)
}
init(url: URL, width: UInt64?, height: UInt64?, mimeType: String?) {
self.init(source: .init(url: url, mimeType: mimeType), width: width, height: height, mimeType: mimeType)
}
init(source: MediaSourceProxy, width: UInt64?, height: UInt64?, mimeType: String?) {
self.source = source
@ -292,14 +292,22 @@ struct ImageInfoProxy: Hashable {
}
static var mockImage: ImageInfoProxy {
.init(source: .init(url: .mockMXCImage, mimeType: "image/png"),
guard let mediaSource = try? MediaSourceProxy(url: .mockMXCImage, mimeType: "image/png") else {
fatalError("Invalid mock media source URL")
}
return .init(source: mediaSource,
size: .init(width: 100, height: 100),
aspectRatio: 1,
mimeType: "image/png")
}
static var mockThumbnail: ImageInfoProxy {
.init(source: .init(url: .mockMXCImage, mimeType: nil),
guard let mediaSource = try? MediaSourceProxy(url: .mockMXCImage, mimeType: "image/png") else {
fatalError("Invalid mock media source URL")
}
return .init(source: mediaSource,
size: nil,
aspectRatio: nil,
mimeType: nil)

11
ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift
View File

@ -33,12 +33,7 @@ struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol {
case .redactedMessage:
return buildRedactedTimelineItem(eventItemProxy, isOutgoing)
case .sticker(let body, let imageInfo, let mediaSource):
guard let url = URL(string: mediaSource.url()) else {
MXLog.error("Invalid sticker url string: \(mediaSource.url())")
return buildUnsupportedTimelineItem(eventItemProxy, "m.sticker", "Invalid Sticker URL", isOutgoing)
}
return buildStickerTimelineItem(eventItemProxy, body, imageInfo, url, isOutgoing)
return buildStickerTimelineItem(eventItemProxy, body, imageInfo, mediaSource, isOutgoing)
case .failedToParseMessageLike(let eventType, let error):
return buildUnsupportedTimelineItem(eventItemProxy, eventType, error, isOutgoing)
case .failedToParseState(let eventType, _, let error):
@ -119,9 +114,9 @@ struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol {
private func buildStickerTimelineItem(_ eventItemProxy: EventTimelineItemProxy,
_ body: String,
_ info: MatrixRustSDK.ImageInfo,
_ imageURL: URL,
_ mediaSource: MediaSource,
_ isOutgoing: Bool) -> RoomTimelineItemProtocol {
let imageInfo = ImageInfoProxy(url: imageURL, width: info.width, height: info.height, mimeType: info.mimetype)
let imageInfo = ImageInfoProxy(source: mediaSource, width: info.width, height: info.height, mimeType: info.mimetype)
return StickerRoomTimelineItem(id: eventItemProxy.id,
body: body,

8
UnitTests/Sources/MediaProvider/MediaLoaderTests.swift
View File

@ -10,12 +10,12 @@ import MatrixRustSDK
import XCTest
final class MediaLoaderTests: XCTestCase {
func testMediaRequestCoalescing() async {
func testMediaRequestCoalescing() async throws {
let mediaLoadingClient = ClientSDKMock()
mediaLoadingClient.getMediaContentMediaSourceReturnValue = Data()
let mediaLoader = MediaLoader(client: mediaLoadingClient)
let mediaSource = MediaSourceProxy(url: .mockMXCFile, mimeType: nil)
let mediaSource = try MediaSourceProxy(url: .mockMXCFile, mimeType: nil)
do {
for _ in 1...10 {
@ -28,12 +28,12 @@ final class MediaLoaderTests: XCTestCase {
}
}
func testMediaThumbnailRequestCoalescing() async {
func testMediaThumbnailRequestCoalescing() async throws {
let mediaLoadingClient = ClientSDKMock()
mediaLoadingClient.getMediaThumbnailMediaSourceWidthHeightReturnValue = Data()
let mediaLoader = MediaLoader(client: mediaLoadingClient)
let mediaSource = MediaSourceProxy(url: .mockMXCImage, mimeType: nil)
let mediaSource = try MediaSourceProxy(url: .mockMXCImage, mimeType: nil)
do {
for _ in 1...10 {

26
UnitTests/Sources/MediaProvider/MediaProviderTests.swift
View File

@ -36,7 +36,7 @@ final class MediaProviderTests: XCTestCase {
return
}
let loadTask = mediaProvider.loadImageRetryingOnReconnection(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"))
let loadTask = try mediaProvider.loadImageRetryingOnReconnection(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"))
let connectivitySubject = CurrentValueSubject<NetworkMonitorReachability, Never>(.unreachable)
@ -59,7 +59,7 @@ final class MediaProviderTests: XCTestCase {
}
func testLoadingRetriedOnReconnectionCancelsAfterSecondFailure() async throws {
let loadTask = mediaProvider.loadImageRetryingOnReconnection(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"))
let loadTask = try mediaProvider.loadImageRetryingOnReconnection(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"))
let connectivitySubject = CurrentValueSubject<NetworkMonitorReachability, Never>(.reachable)
@ -73,7 +73,7 @@ final class MediaProviderTests: XCTestCase {
}
func test_whenImageFromSourceWithSourceNil_nilReturned() throws {
let image = mediaProvider.imageFromSource(nil, size: Avatars.Size.room(on: .timeline).scaledSize)
let image = try mediaProvider.imageFromSource(nil, size: Avatars.Size.room(on: .timeline).scaledSize)
XCTAssertNil(image)
}
@ -83,13 +83,13 @@ final class MediaProviderTests: XCTestCase {
let key = "\(url.absoluteString){\(avatarSize.scaledValue),\(avatarSize.scaledValue)}"
let imageForKey = UIImage()
imageCache.retrievedImagesInMemory[key] = imageForKey
let image = mediaProvider.imageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
let image = try mediaProvider.imageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
size: avatarSize.scaledSize)
XCTAssertEqual(image, imageForKey)
}
func test_whenImageFromSourceWithSourceNotNilAndImageNotCached_nilReturned() throws {
let image = mediaProvider.imageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
let image = try mediaProvider.imageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
size: Avatars.Size.room(on: .timeline).scaledSize)
XCTAssertNil(image)
}
@ -100,7 +100,7 @@ final class MediaProviderTests: XCTestCase {
let key = "\(url.absoluteString){\(avatarSize.scaledValue),\(avatarSize.scaledValue)}"
let imageForKey = UIImage()
imageCache.retrievedImagesInMemory[key] = imageForKey
let result = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
let result = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
size: avatarSize.scaledSize)
XCTAssertEqual(Result.success(imageForKey), result)
}
@ -111,7 +111,7 @@ final class MediaProviderTests: XCTestCase {
let key = "\(url.absoluteString){\(avatarSize.scaledValue),\(avatarSize.scaledValue)}"
let imageForKey = UIImage()
imageCache.retrievedImages[key] = imageForKey
let result = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
let result = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
size: avatarSize.scaledSize)
XCTAssertEqual(Result.success(imageForKey), result)
}
@ -122,7 +122,7 @@ final class MediaProviderTests: XCTestCase {
mediaLoader.loadMediaThumbnailForSourceWidthHeightReturnValue = expectedImage.pngData()
let result = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
let result = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
size: avatarSize.scaledSize)
switch result {
case .success(let image):
@ -140,7 +140,7 @@ final class MediaProviderTests: XCTestCase {
mediaLoader.loadMediaThumbnailForSourceWidthHeightReturnValue = expectedImage.pngData()
_ = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
_ = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: url, mimeType: "image/jpeg"),
size: avatarSize.scaledSize)
let storedImage = try XCTUnwrap(imageCache.storedImages[key])
XCTAssertEqual(expectedImage.pngData(), storedImage.pngData())
@ -151,7 +151,7 @@ final class MediaProviderTests: XCTestCase {
mediaLoader.loadMediaContentForSourceReturnValue = expectedImage.pngData()
let result = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
let result = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
size: nil)
switch result {
case .success(let image):
@ -164,7 +164,7 @@ final class MediaProviderTests: XCTestCase {
func test_whenLoadImageFromSourceAndImageNotCachedAndRetrieveImageFailsAndLoadImageThumbnailFails_errorIsThrown() async throws {
mediaLoader.loadMediaThumbnailForSourceWidthHeightThrowableError = MediaProviderTestsError.error
let result = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
let result = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
size: Avatars.Size.room(on: .timeline).scaledSize)
switch result {
case .success:
@ -177,7 +177,7 @@ final class MediaProviderTests: XCTestCase {
func test_whenLoadImageFromSourceAndImageNotCachedAndRetrieveImageFailsAndNoAvatarSizeAndLoadImageContentFails_errorIsThrown() async throws {
mediaLoader.loadMediaContentForSourceThrowableError = MediaProviderTestsError.error
let result = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
let result = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
size: nil)
switch result {
case .success:
@ -190,7 +190,7 @@ final class MediaProviderTests: XCTestCase {
func test_whenLoadImageFromSourceAndImageNotCachedAndRetrieveImageFailsAndImageThumbnailIsLoadedWithCorruptedData_errorIsThrown() async throws {
mediaLoader.loadMediaThumbnailForSourceWidthHeightReturnValue = Data()
let result = await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
let result = try await mediaProvider.loadImageFromSource(MediaSourceProxy(url: .mockMXCImage, mimeType: "image/jpeg"),
size: Avatars.Size.room(on: .timeline).scaledSize)
switch result {
case .success:

2
UnitTests/Sources/VoiceMessageCacheTests.swift
View File

@ -26,7 +26,7 @@ class VoiceMessageCacheTests: XCTestCase {
voiceMessageCache.clearCache()
fileManager = FileManager.default
mediaSource = MediaSourceProxy(url: someURL, mimeType: "audio/ogg")
mediaSource = try MediaSourceProxy(url: someURL, mimeType: "audio/ogg")
// Create the temporary directory we will use
try fileManager.createDirectory(at: testTemporaryDirectory, withIntermediateDirectories: true)

12
UnitTests/Sources/VoiceMessageMediaManagerTests.swift
View File

@ -28,7 +28,7 @@ class VoiceMessageMediaManagerTests: XCTestCase {
func testLoadVoiceMessageFromSourceUnsupportedMedia() async throws {
// Only "audio/ogg" file are supported
let unsupportedMediaSource = MediaSourceProxy(url: someURL, mimeType: "audio/wav")
let unsupportedMediaSource = try MediaSourceProxy(url: someURL, mimeType: "audio/wav")
do {
_ = try await voiceMessageMediaManager.loadVoiceMessageFromSource(unsupportedMediaSource, body: nil)
XCTFail("A `VoiceMessageMediaManagerError.unsupportedMimeTye` error is expected")
@ -49,7 +49,7 @@ class VoiceMessageMediaManagerTests: XCTestCase {
let cachedConvertedFileURL = URL("/some/url/cached_converted_file.m4a")
voiceMessageCache.fileURLForReturnValue = nil
let mediaSource = MediaSourceProxy(url: someURL, mimeType: "audio/ogg; codecs=opus")
let mediaSource = try MediaSourceProxy(url: someURL, mimeType: "audio/ogg; codecs=opus")
mediaProvider.loadFileFromSourceFilenameReturnValue = .success(MediaFileHandleProxy.unmanaged(url: loadedFile))
voiceMessageCache.cacheMediaSourceUsingMoveReturnValue = .success(cachedConvertedFileURL)
@ -67,7 +67,7 @@ class VoiceMessageMediaManagerTests: XCTestCase {
func testLoadVoiceMessageFromSourceAlreadyCached() async throws {
// Check if the file is already present in cache
voiceMessageCache.fileURLForReturnValue = URL("/converted_file/url")
let mediaSource = MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
let mediaSource = try MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
let url = try await voiceMessageMediaManager.loadVoiceMessageFromSource(mediaSource, body: nil)
XCTAssertEqual(url, URL("/converted_file/url"))
// The file must have be search in the cache
@ -81,7 +81,7 @@ class VoiceMessageMediaManagerTests: XCTestCase {
// An error must be reported if the file cannot be retrieved
do {
voiceMessageCache.fileURLForReturnValue = nil
let mediaSource = MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
let mediaSource = try MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
_ = try await voiceMessageMediaManager.loadVoiceMessageFromSource(mediaSource, body: nil)
XCTFail("A `MediaProviderError.failedRetrievingFile` error is expected")
} catch {
@ -102,7 +102,7 @@ class VoiceMessageMediaManagerTests: XCTestCase {
// Check if the file is not already present in cache
voiceMessageCache.fileURLForReturnValue = nil
let mediaSource = MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
let mediaSource = try MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
mediaProvider.loadFileFromSourceFilenameReturnValue = .success(MediaFileHandleProxy.unmanaged(url: loadedFile))
let audioConverter = AudioConverterMock()
voiceMessageCache.cacheMediaSourceUsingMoveReturnValue = .success(cachedConvertedFileURL)
@ -145,7 +145,7 @@ class VoiceMessageMediaManagerTests: XCTestCase {
voiceMessageCache: voiceMessageCache,
audioConverter: audioConverter)
let mediaSource = MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
let mediaSource = try MediaSourceProxy(url: someURL, mimeType: audioOGGMimeType)
for _ in 0..<10 {
let url = try await voiceMessageMediaManager.loadVoiceMessageFromSource(mediaSource, body: nil)
XCTAssertEqual(url, cachedConvertedFileURL)