Skip to content

Commit

Permalink
Fix a potential race condition when redacting a message. (#3061)
Browse files Browse the repository at this point in the history 
* Refactor the timeline item menu action provider.

- Move it into its own struct.
- Use an item, not an ID so it doesn't randomly change.
- Move permissions into the room screen view model.

* Use the stable ID when redacting/editing/forwarding a message.

Just like we do when fetching the item in the actions menu.
  • Loading branch information
@pixlwave
pixlwave authored Jul 18, 2024
1 parent 2fb7f65 commit 6339fe2
Show file tree
Hide file tree
Showing 12 changed files with 337 additions and 275 deletions.
32 changes: 24 additions & 8 deletions ElementX.xcodeproj/project.pbxproj
View file

Large diffs are not rendered by default.

103 changes: 0 additions & 103 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenInteractionHandler.swift
View file
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
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
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
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

0 comments on commit 6339fe2

Please sign in to comment.