Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a potential race condition when redacting a message. #3061

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Fix a potential race condition when redacting a message. #3061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1230b7d
Refactor the timeline item menu action provider.
pixlwave Jul 18, 2024
d0de242
Use the stable ID when redacting/editing/forwarding a message.
pixlwave Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions ElementX.xcodeproj/project.pbxproj
View file
Open in desktop

Large diffs are not rendered by default.

103 changes: 0 additions & 103 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenInteractionHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ class RoomScreenInteractionHandler {
}
}

private var canCurrentUserRedactOthers = false
private var canCurrentUserRedactSelf = false
private var resumeVoiceMessagePlaybackAfterScrubbing = false

init(roomProxy: RoomProxyProtocol,
Expand All @@ -84,17 +82,12 @@ class RoomScreenInteractionHandler {
self.appSettings = appSettings
self.analyticsService = analyticsService
pollInteractionHandler = PollInteractionHandler(analyticsService: analyticsService, roomProxy: roomProxy)

// Set initial values for redacting from the macOS context menu.
Task { await updatePermissions() }
}

// MARK: Timeline Item Action Menu

func displayTimelineItemActionMenu(for itemID: TimelineItemIdentifier) {
Task {
await updatePermissions()

guard let timelineItem = timelineController.timelineItems.firstUsingStableID(itemID),
let eventTimelineItem = timelineItem as? EventBasedTimelineItemProtocol else {
// Don't show a menu for non-event based items.
Expand All @@ -106,84 +99,6 @@ class RoomScreenInteractionHandler {
}
}

// swiftlint:disable:next cyclomatic_complexity
func timelineItemMenuActionsForItemId(_ itemID: TimelineItemIdentifier) -> TimelineItemMenuActions? {
guard let timelineItem = timelineController.timelineItems.firstUsingStableID(itemID),
let item = timelineItem as? EventBasedTimelineItemProtocol else {
// Don't show a context menu for non-event based items.
return nil
}

if timelineItem is StateRoomTimelineItem {
// Don't show a context menu for state events.
return nil
}

var debugActions: [TimelineItemMenuAction] = []
if appSettings.viewSourceEnabled {
debugActions.append(.viewSource)
}

if let encryptedItem = timelineItem as? EncryptedRoomTimelineItem {
switch encryptedItem.encryptionType {
case .megolmV1AesSha2(let sessionID, _):
debugActions.append(.retryDecryption(sessionID: sessionID))
default:
break
}

return .init(actions: [.copyPermalink], debugActions: debugActions)
}

var actions: [TimelineItemMenuAction] = []

if item.canBeRepliedTo {
if let messageItem = item as? EventBasedMessageTimelineItemProtocol {
actions.append(.reply(isThread: messageItem.isThreaded))
} else {
actions.append(.reply(isThread: false))
}
}

if item.isForwardable {
actions.append(.forward(itemID: itemID))
}

if item.isEditable {
actions.append(.edit)
}

if item.isCopyable {
actions.append(.copy)
}

if item.isRemoteMessage {
actions.append(.copyPermalink)
}

if canRedactItem(item), let poll = item.pollIfAvailable, !poll.hasEnded, let eventID = itemID.eventID {
actions.append(.endPoll(pollStartID: eventID))
}

if canRedactItem(item) {
actions.append(.redact)
}

if !item.isOutgoing {
actions.append(.report)
}

if item.hasFailedToSend {
actions = actions.filter(\.canAppearInFailedEcho)
}

if item.isRedacted {
actions = actions.filter(\.canAppearInRedacted)
}

return .init(actions: actions, debugActions: debugActions)
}

func handleTimelineItemMenuAction(_ action: TimelineItemMenuAction, itemID: TimelineItemIdentifier) {
guard let timelineItem = timelineController.timelineItems.firstUsingStableID(itemID),
let eventTimelineItem = timelineItem as? EventBasedTimelineItemProtocol else {
Expand Down Expand Up @@ -602,24 +517,6 @@ class RoomScreenInteractionHandler {

// MARK: - Private

private func updatePermissions() async {
if case let .success(value) = await roomProxy.canUserRedactOther(userID: roomProxy.ownUserID) {
canCurrentUserRedactOthers = value
} else {
canCurrentUserRedactOthers = false
}

if case let .success(value) = await roomProxy.canUserRedactOwn(userID: roomProxy.ownUserID) {
canCurrentUserRedactSelf = value
} else {
canCurrentUserRedactSelf = false
}
}

private func canRedactItem(_ item: EventBasedTimelineItemProtocol) -> Bool {
item.isOutgoing ? canCurrentUserRedactSelf : canCurrentUserRedactOthers && !roomProxy.isDirect
}

private func buildReplyInfo(for item: EventBasedTimelineItemProtocol) -> ReplyInfo {
switch item {
case let messageItem as EventBasedMessageTimelineItemProtocol:
Expand Down
6 changes: 3 additions & 3 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,15 @@ struct RoomScreenViewState: BindableState {
var timelineViewState: TimelineViewState // check the doc before changing this

var ownUserID: String
var canCurrentUserRedactOthers = false
var canCurrentUserRedactSelf = false
var isViewSourceEnabled: Bool

var canJoinCall = false
var hasOngoingCall = false

var bindings: RoomScreenViewStateBindings

/// A closure providing the actions to show when long pressing on an item in the timeline.
var timelineItemMenuActionProvider: (@MainActor (_ itemId: TimelineItemIdentifier) -> TimelineItemMenuActions?)?

/// A closure providing the associated audio player state for an item in the timeline.
var audioPlayerStateProvider: (@MainActor (_ itemId: TimelineItemIdentifier) -> AudioPlayerState?)?
}
Expand Down
33 changes: 25 additions & 8 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
isEncryptedOneToOneRoom: roomProxy.isEncryptedOneToOneRoom,
timelineViewState: TimelineViewState(focussedEvent: focussedEventID.map { .init(eventID: $0, appearance: .immediate) }),
ownUserID: roomProxy.ownUserID,
isViewSourceEnabled: appSettings.viewSourceEnabled,
hasOngoingCall: roomProxy.hasOngoingCall,
bindings: .init(reactionsCollapsed: [:])),
imageProvider: mediaProvider)
Expand All @@ -98,13 +99,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
setupSubscriptions()
setupDirectRoomSubscriptionsIfNeeded()

state.timelineItemMenuActionProvider = { [weak self] itemID -> TimelineItemMenuActions? in
guard let self else {
return nil
}

return self.roomScreenInteractionHandler.timelineItemMenuActionsForItemId(itemID)
}
// Set initial values for redacting from the macOS context menu.
Task { await updatePermissions() }

state.audioPlayerStateProvider = { [weak self] itemID -> AudioPlayerState? in
guard let self else {
Expand Down Expand Up @@ -351,6 +347,20 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}
}

private func updatePermissions() async {
if case let .success(value) = await roomProxy.canUserRedactOther(userID: roomProxy.ownUserID) {
state.canCurrentUserRedactOthers = value
} else {
state.canCurrentUserRedactOthers = false
}

if case let .success(value) = await roomProxy.canUserRedactOwn(userID: roomProxy.ownUserID) {
state.canCurrentUserRedactSelf = value
} else {
state.canCurrentUserRedactSelf = false
}
}

private func setupSubscriptions() {
timelineController.callbacks
.receive(on: DispatchQueue.main)
Expand Down Expand Up @@ -393,6 +403,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
.weakAssign(to: \.state.showReadReceipts, on: self)
.store(in: &cancellables)

appSettings.$viewSourceEnabled
.weakAssign(to: \.state.isViewSourceEnabled, on: self)
.store(in: &cancellables)

roomProxy.membersPublisher
.receive(on: DispatchQueue.main)
.sink { [weak self] in self?.updateMembers($0) }
Expand Down Expand Up @@ -429,7 +443,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
case .displayRoomMemberDetails(userID: let userID):
actionsSubject.send(.displayRoomMemberDetails(userID: userID))
case .showActionMenu(let actionMenuInfo):
state.bindings.actionMenuInfo = actionMenuInfo
Task {
await self.updatePermissions()
self.state.bindings.actionMenuInfo = actionMenuInfo
}
case .showDebugInfo(let debugInfo):
state.bindings.debugInfo = debugInfo
}
Expand Down
6 changes: 3 additions & 3 deletions ...een/View/TimelineItemMacContextMenu.swift → ...ItemMenu/TimelineItemMacContextMenu.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import SwiftUI
/// The contents of the context menu shown when right clicking an item in the timeline on a Mac
struct TimelineItemMacContextMenu: View {
let item: RoomTimelineItemProtocol
let actionProvider: (@MainActor (_ itemId: TimelineItemIdentifier) -> TimelineItemMenuActions?)?
let actionProvider: TimelineItemMenuActionProvider
let send: (TimelineItemMenuAction) -> Void

var body: some View {
if ProcessInfo.processInfo.isiOSAppOnMac {
if let menuActions = actionProvider?(item.id) {
if let menuActions = actionProvider.makeActions() {
Section {
if item.isReactable {
if !menuActions.reactions.isEmpty {
if #available(iOS 17.0, *) {
let reactions = (item as? EventBasedTimelineItemProtocol)?.properties.reactions ?? []
ControlGroup {
Expand Down
Loading
Loading