Reuse conversation tree where possible when selecting a status in a conversation

Tusker/Screens/Conversation/ConversationCollectionViewController.swift

@@ -45,11 +45,15 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
             (collectionView.cellForItem(at: $0) as? TimelineStatusCollectionViewCell)?.trailingSwipeActions()
         }
         config.itemSeparatorHandler = { [unowned self] indexPath, sectionConfig in
-            let rowsInSection = self.collectionView.numberOfItems(inSection: indexPath.section)
-            let lastInSection = indexPath.row == rowsInSection - 1
             var config = sectionConfig
             config.topSeparatorVisibility = .hidden
-            config.bottomSeparatorVisibility = lastInSection ? .visible : .hidden
+            if case .ancestors = self.dataSource.sectionIdentifier(for: indexPath.section) {
+                config.bottomSeparatorVisibility = .hidden
+            } else if indexPath.row == self.collectionView.numberOfItems(inSection: indexPath.section) - 1 {
+                config.bottomSeparatorVisibility = .visible
+            } else {
+                config.bottomSeparatorVisibility = .hidden
+            }
             config.bottomSeparatorInsets = TimelineStatusCollectionViewCell.separatorInsets
             return config
         }
@@ -89,7 +93,7 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
         }
         return UICollectionViewDiffableDataSource(collectionView: collectionView) { [unowned self] collectionView, indexPath, itemIdentifier in
             switch itemIdentifier {
-            case let .status(id: id, state: state, prevLink: prevLink, nextLink: nextLink):
+            case let .status(id: id, node: _, state: state, prevLink: prevLink, nextLink: nextLink):
                 if id == self.mainStatusID {
                     return collectionView.dequeueConfiguredReusableCell(using: mainStatusCell, for: indexPath, item: (id, state, prevLink))
                 } else {
@@ -113,36 +117,28 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
         loadViewIfNeeded()
         
         var snapshot = NSDiffableDataSourceSnapshot<Section, Item>()
-        snapshot.appendSections([.statuses])
+        snapshot.appendSections([.ancestors, .mainStatus])
         
         if status.inReplyToID != nil {
-            snapshot.appendItems([.loadingIndicator], toSection: .statuses)
+            snapshot.appendItems([.loadingIndicator], toSection: .ancestors)
         }
         
-        let mainStatusItem = Item.status(id: mainStatusID, state: mainStatusState, prevLink: status.inReplyToID != nil, nextLink: false)
+        // this will be replace with the actual node in the tree once it's loaded
-        snapshot.appendItems([mainStatusItem], toSection: .statuses)
+        let tempMainNode = ConversationNode(status: status)
+        let mainStatusItem = Item.status(id: mainStatusID, node: tempMainNode, state: mainStatusState, prevLink: status.inReplyToID != nil, nextLink: false)
+        snapshot.appendItems([mainStatusItem], toSection: .mainStatus)
         
         dataSource.apply(snapshot, animatingDifferences: false)
     }
     
-    func addContext(_ context: ConversationContext, for mainStatus: StatusMO) async {
+    func addTree(_ tree: ConversationTree, mainStatus: StatusMO) {
-        await mastodonController.persistentContainer.addAll(statuses: context.ancestors + context.descendants)
-        
-        // fetch all descendant status managed objects
-        let descendantIDs = context.descendants.map(\.id)
-        let request = StatusMO.fetchRequest()
-        request.predicate = NSPredicate(format: "id IN %@", descendantIDs)
-        let descendants = try? mastodonController.persistentContainer.viewContext.fetch(request)
-        
-        let tree = ConversationTree.build(from: context, mainStatus: mainStatus, descendants: descendants ?? [])
-        
         var snapshot = dataSource.snapshot()
         snapshot.deleteItems([.loadingIndicator])
-        let mainStatusItem = Item.status(id: mainStatusID, state: mainStatusState, prevLink: mainStatus.inReplyToID != nil, nextLink: false)
+        let mainStatusItem = Item.status(id: mainStatusID, node: tree.mainStatus, state: mainStatusState, prevLink: mainStatus.inReplyToID != nil, nextLink: false)
-        let parentItems = tree.ancestors.enumerated().map { index, id in
+        let parentItems = tree.ancestors.enumerated().map { index, node in
-            Item.status(id: id, state: .unknown, prevLink: index > 0, nextLink: true)
+            Item.status(id: node.status.id, node: node, state: .unknown, prevLink: index > 0, nextLink: true)
         }
-        snapshot.insertItems(parentItems, beforeItem: mainStatusItem)
+        snapshot.appendItems(parentItems, toSection: .ancestors)
         snapshot.reloadItems([mainStatusItem])
 
         // convert sub-threads into items for section and add to snapshot
@@ -156,7 +152,7 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
                 position = .centeredVertically
             } else {
                 item = snapshot.itemIdentifiers.first {
-                    if case .status(id: self.statusIDToScrollToOnLoad, _, _, _) = $0 {
+                    if case .status(id: self.statusIDToScrollToOnLoad, _, _, _, _) = $0 {
                         return true
                     } else {
                         return false
@@ -185,7 +181,7 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
         for node in childThreads {
             let section = Section.childThread(firstStatusID: node.status.id)
             snapshot.appendSections([section])
-            snapshot.appendItems([.status(id: node.status.id, state: .unknown, prevLink: false, nextLink: !node.children.isEmpty)], toSection: section)
+            snapshot.appendItems([.status(id: node.status.id, node: node, state: .unknown, prevLink: false, nextLink: !node.children.isEmpty)], toSection: section)
             
             var currentNode = node
             while true {
@@ -208,7 +204,7 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
                 }
 
                 currentNode = next
-                snapshot.appendItems([.status(id: next.status.id, state: .unknown, prevLink: true, nextLink: !next.children.isEmpty)], toSection: section)
+                snapshot.appendItems([.status(id: next.status.id, node: next, state: .unknown, prevLink: true, nextLink: !next.children.isEmpty)], toSection: section)
             }
         }
     }
@@ -217,7 +213,7 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
         var snapshot = dataSource.snapshot()
         var cellsToMask: [StatusCollectionViewCell] = []
         for item in snapshot.itemIdentifiers {
-            guard case .status(id: _, state: let state, prevLink: _, nextLink: _) = item,
+            guard case .status(id: _, node: _, state: let state, prevLink: _, nextLink: _) = item,
                   state.collapsible == true else {
                 continue
             }
@@ -248,17 +244,18 @@ class ConversationCollectionViewController: UIViewController, CollectionViewCont
 
 extension ConversationCollectionViewController {
     enum Section: Hashable {
-        case statuses
+        case ancestors
+        case mainStatus
         case childThread(firstStatusID: String)
     }
     enum Item: Hashable {
-        case status(id: String, state: CollapseState, prevLink: Bool, nextLink: Bool)
+        case status(id: String, node: ConversationNode, state: CollapseState, prevLink: Bool, nextLink: Bool)
         case expandThread(childThreads: [ConversationNode], inline: Bool)
         case loadingIndicator
         
         static func ==(lhs: Item, rhs: Item) -> Bool {
             switch (lhs, rhs) {
-            case let (.status(id: a, state: _, prevLink: aPrev, nextLink: aNext), .status(id: b, state: _, prevLink: bPrev, nextLink: bNext)):
+            case let (.status(id: a, node: _, state: _, prevLink: aPrev, nextLink: aNext), .status(id: b, node: _, state: _, prevLink: bPrev, nextLink: bNext)):
                 return a == b && aPrev == bPrev && aNext == bNext
             case let (.expandThread(childThreads: a, inline: aInline), .expandThread(childThreads: b, inline: bInline)):
                 return a.count == b.count && zip(a, b).allSatisfy { $0.status.id == $1.status.id } && aInline == bInline
@@ -271,7 +268,7 @@ extension ConversationCollectionViewController {
         
         func hash(into hasher: inout Hasher) {
             switch self {
-            case let .status(id: id, state: _, prevLink: prevLink, nextLink: nextLink):
+            case let .status(id: id, node: _, state: _, prevLink: prevLink, nextLink: nextLink):
                 hasher.combine(0)
                 hasher.combine(id)
                 hasher.combine(prevLink)
@@ -292,7 +289,7 @@ extension ConversationCollectionViewController {
 extension ConversationCollectionViewController: UICollectionViewDelegate {
     func collectionView(_ collectionView: UICollectionView, shouldSelectItemAt indexPath: IndexPath) -> Bool {
         switch dataSource.itemIdentifier(for: indexPath) {
-        case .status(id: let id, state: _, prevLink: _, nextLink: _):
+        case .status(id: let id, node: _, state: _, prevLink: _, nextLink: _):
             return id != mainStatusID
         case .expandThread(childThreads: _, inline: _):
             return true
@@ -307,12 +304,24 @@ extension ConversationCollectionViewController: UICollectionViewDelegate {
             break
         case .loadingIndicator:
             break
-        case .status(id: let id, state: let state, _, _):
+        case .status(id: let id, node: let node, state: let state, _, _):
+            // we can only take the fast path if the user tapped on a descendant status.
+            // if the current main status is C, or one of its descendants, and the user taps A, then B won't be loaded:
+            //    A
+            //   / \
+            //  B   C
+            if case .childThread(_) = dataSource.sectionIdentifier(for: indexPath.section) {
+                let tree = ConversationTree(ancestors: buildNewAncestors(above: indexPath), mainStatus: node)
+                let conv = ConversationViewController(preloadedTree: tree, state: state.copy(), mastodonController: mastodonController)
+                conv.showStatusesAutomatically = showStatusesAutomatically
+                show(conv)
+            } else {
                 selected(status: id, state: state.copy())
+            }
         case .expandThread(childThreads: let childThreads, inline: _):
-            if case .status(id: let id, state: let state, _, _) = dataSource.itemIdentifier(for: IndexPath(row: indexPath.row - 1, section: indexPath.section)) {
+            if case .status(id: _, node: let node, state: let state, _, _) = dataSource.itemIdentifier(for: IndexPath(row: indexPath.row - 1, section: indexPath.section)) {
-                // todo: it would be nice to avoid re-fetching the context here, since we should have all the necessary information already
+                let tree = ConversationTree(ancestors: buildNewAncestors(above: indexPath), mainStatus: node)
-                let conv = ConversationViewController(for: id, state: state.copy(), mastodonController: mastodonController)
+                let conv = ConversationViewController(preloadedTree: tree, state: state.copy(), mastodonController: mastodonController)
                 conv.statusIDToScrollToOnLoad = childThreads.first!.status.id
                 conv.showStatusesAutomatically = showStatusesAutomatically
                 show(conv)
@@ -320,6 +329,34 @@ extension ConversationCollectionViewController: UICollectionViewDelegate {
         }
     }
     
+    // ConversationNode doesn't know about its parent, so we reconstruct that info from the data source
+    private func buildNewAncestors(above indexPath: IndexPath) -> [ConversationNode] {
+        let snapshot = dataSource.snapshot()
+        let currentAncestors = snapshot.itemIdentifiers(inSection: .ancestors).compactMap {
+            if case .status(id: _, node: let node, _, _, _) = $0 {
+                return node
+            } else {
+                return nil
+            }
+        }
+        let currentMainStatus = snapshot.itemIdentifiers(inSection: .mainStatus).compactMap {
+            if case .status(id: _, node: let node, _, _, _) = $0 {
+                return node
+            } else {
+                return nil
+            }
+        }
+        let parentsInCurrentSection = snapshot.itemIdentifiers(inSection: dataSource.sectionIdentifier(for: indexPath.section)!)[0..<indexPath.row].compactMap {
+            if case .status(id: _, node: let node, _, _, _) = $0 {
+                return node
+            } else {
+                return nil
+            }
+        }
+        return currentAncestors + currentMainStatus + parentsInCurrentSection
+    }
+    
+    
     func collectionView(_ collectionView: UICollectionView, contextMenuConfigurationForItemAt indexPath: IndexPath, point: CGPoint) -> UIContextMenuConfiguration? {
         return (collectionView.cellForItem(at: indexPath) as? TimelineStatusCollectionViewCell)?.contextMenuConfiguration()
     }

Tusker/Screens/Conversation/ConversationTree.swift

@@ -19,32 +19,56 @@ class ConversationNode {
     }
 }
 struct ConversationTree {
-    let ancestors: [String]
+    let ancestors: [ConversationNode]
-    let descendants: [ConversationNode]
+    let mainStatus: ConversationNode
-    
+    var descendants: [ConversationNode] {
-    static func build(from context: ConversationContext, mainStatus: StatusMO, descendants: [StatusMO]) -> ConversationTree {
+        mainStatus.children
-        let ancestors = getDirectParents(inReplyTo: mainStatus.inReplyToID, from: context)
-        let childThreads = getDescendantThreads(mainStatus: mainStatus, descendants: descendants)
-        return ConversationTree(ancestors: ancestors, descendants: childThreads)
     }
     
-    private static func getDirectParents(inReplyTo inReplyToID: String?, from context: ConversationContext) -> [String] {
+    init(ancestors: [ConversationNode], mainStatus: ConversationNode) {
-        var statuses = context.ancestors
+        self.ancestors = ancestors
-        var parents = [String]()
+        self.mainStatus = mainStatus
+    }
     
-        var parentID: String? = inReplyToID
+    static func build(for mainStatus: StatusMO, ancestors: [StatusMO], descendants: [StatusMO]) -> ConversationTree {
+        let mainStatusNode = ConversationNode(status: mainStatus)
+        let ancestors = buildAncestorNodes(mainStatusNode: mainStatusNode, ancestors: ancestors)
+        buildDescendantNodes(mainStatusNode: mainStatusNode, descendants: descendants)
+        return ConversationTree(ancestors: ancestors, mainStatus: mainStatusNode)
+    }
+    
+    private static func buildAncestorNodes(mainStatusNode: ConversationNode, ancestors: [StatusMO]) -> [ConversationNode] {
+        var statuses = ancestors
+        var parents = [ConversationNode]()
+        
+        var parentID: String? = mainStatusNode.status.inReplyToID
         
         while let currentParentID = parentID,
               let parentIndex = statuses.firstIndex(where: { $0.id == currentParentID }) {
             let parentStatus = statuses.remove(at: parentIndex)
-            parents.insert(parentStatus.id, at: 0)
+            
+            let node = ConversationNode(status: parentStatus)
+            parents.insert(node, at: 0)
+            
             parentID = parentStatus.inReplyToID
         }
         
+        // once the parents list is built and in-order, then we walk through and set each node's children
+        for (index, node) in parents.enumerated() {
+            if index == parents.count - 1 {
+                // the last parent is the direct parent of the main status
+                node.children = [mainStatusNode]
+            } else {
+                // otherwise, it's the parent of the status that comes immediately after it in the parents list
+                node.children = [parents[index + 1]]
+            }
+        }
+        
         return parents
     }
     
-    private static func getDescendantThreads(mainStatus: StatusMO, descendants: [StatusMO]) -> [ConversationNode] {
+    // doesn't return anything, since we're modifying the main status node in-place
+    private static func buildDescendantNodes(mainStatusNode: ConversationNode, descendants: [StatusMO]) {
         var descendants = descendants
         
         func removeAllInReplyTo(id: String) -> [StatusMO] {
@@ -53,12 +77,11 @@ struct ConversationTree {
             return statuses
         }
         
-        let mainStatusNode = ConversationNode(status: mainStatus)
         var nodes: [String: ConversationNode] = [
-            mainStatus.id: mainStatusNode
+            mainStatusNode.status.id: mainStatusNode
         ]
         
-        var idsToCheck = [mainStatus.id]
+        var idsToCheck = [mainStatusNode.status.id]
         
         while !idsToCheck.isEmpty {
             let inReplyToID = idsToCheck.removeFirst()
@@ -74,7 +97,5 @@ struct ConversationTree {
                 nodeForID.children.append(replyNode)
             }
         }
-        
-        return mainStatusNode.children
     }
 }

Tusker/Screens/Conversation/ConversationViewController.swift

@@ -77,6 +77,14 @@ class ConversationViewController: UIViewController {
         super.init(nibName: nil, bundle: nil)
     }
     
+    init(preloadedTree: ConversationTree, state mainStatusState: CollapseState, mastodonController: MastodonController) {
+        self.mode = .preloaded(preloadedTree)
+        self.mainStatusState = mainStatusState
+        self.mastodonController = mastodonController
+
+        super.init(nibName: nil, bundle: nil)
+    }
+    
     required init?(coder: NSCoder) {
         fatalError("init(coder:) has not been implemented")
     }
@@ -115,12 +123,18 @@ class ConversationViewController: UIViewController {
     override func viewWillAppear(_ animated: Bool) {
         super.viewWillAppear(animated)
         
-        Task {
         if case .unloaded = state {
+            if case .preloaded(let tree) = mode {
+                // when everything is preloaded, we're on the fast path and want to avoid any async work
+                // just kicking off a MainActor task causes a delay before the content appears, even if the task doesn't suspend
+                mainStatusLoaded(tree.mainStatus.status)
+            } else {
+                Task { @MainActor in
                     await loadMainStatus()
                 }
             }
         }
+    }
     
     @objc private func handleStatusDeleted(_ notification: Foundation.Notification) {
         guard let userInfo = notification.userInfo,
@@ -142,10 +156,21 @@ class ConversationViewController: UIViewController {
     
     // MARK: Loading
     
+    @MainActor
     private func loadMainStatus() async {
-        guard let mainStatusID = await resolveStatusIfNecessary() else {
+        let mainStatusID: String
+        switch mode {
+        case .localID(let id):
+            mainStatusID = id
+        case .resolve(let url):
+            if let id = await resolveStatus(url: url) {
+                mainStatusID = id
+            } else {
                 return
             }
+        case .preloaded(_):
+            fatalError("unreachable")
+        }
         
         @MainActor
         func doLoadMainStatus() async -> StatusMO? {
@@ -166,7 +191,7 @@ class ConversationViewController: UIViewController {
             Task {
                 await doLoadMainStatus()
             }
-            await mainStatusLoaded(cached)
+            mainStatusLoaded(cached)
         } else {
             // otherwise, show a loading indicator while loading the main status
             let indicator = UIActivityIndicatorView(style: .medium)
@@ -174,17 +199,13 @@ class ConversationViewController: UIViewController {
             state = .loading(indicator)
             
             if let status = await doLoadMainStatus() {
-                await mainStatusLoaded(status)
+                mainStatusLoaded(status)
             }
         }
     }
     
     @MainActor
-    private func resolveStatusIfNecessary() async -> String? {
+    private func resolveStatus(url: URL) async -> String? {
-        switch mode {
-        case .localID(let id):
-            return id
-        case .resolve(let url):
         let indicator = UIActivityIndicatorView(style: .medium)
         indicator.startAnimating()
         state = .loading(indicator)
@@ -204,44 +225,68 @@ class ConversationViewController: UIViewController {
             return nil
         }
     }
-    }
     
-    @MainActor
+    private func mainStatusLoaded(_ mainStatus: StatusMO) {
-    private func mainStatusLoaded(_ mainStatus: StatusMO) async {
         let vc = ConversationCollectionViewController(for: mainStatus.id, state: mainStatusState, mastodonController: mastodonController)
         vc.statusIDToScrollToOnLoad = statusIDToScrollToOnLoad ?? mainStatus.id
         vc.showStatusesAutomatically = showStatusesAutomatically
         vc.addMainStatus(mainStatus)
         state = .displaying(vc)
         
-        await loadContext(for: mainStatus)
+        if case .preloaded(let tree) = mode {
+            vc.addTree(tree, mainStatus: mainStatus)
+        } else {
+            Task { @MainActor in
+                await loadTree(for: mainStatus)
+            }
+        }
     }
     
     @MainActor
-    private func loadContext(for mainStatus: StatusMO) async {
+    private func loadTree(for mainStatus: StatusMO) async {
-        guard case .displaying(_) = state else {
+        guard case .displaying(_) = state,
+              let context = await loadContext(for: mainStatus) else {
             return
         }
         
-        let request = Status.getContext(mainStatus.id)
+        await mastodonController.persistentContainer.addAll(statuses: context.ancestors + context.descendants)
-        do {
+        
-            let (context, _) = try await mastodonController.run(request)
+        let ancestorIDs = context.ancestors.map(\.id)
+        let ancestorsReq = StatusMO.fetchRequest()
+        ancestorsReq.predicate = NSPredicate(format: "id in %@", ancestorIDs)
+        let ancestors = try? mastodonController.persistentContainer.viewContext.fetch(ancestorsReq)
+        
+        let descendantIDs = context.descendants.map(\.id)
+        let descendantsReq = StatusMO.fetchRequest()
+        descendantsReq.predicate = NSPredicate(format: "id IN %@", descendantIDs)
+        let descendants = try? mastodonController.persistentContainer.viewContext.fetch(descendantsReq)
+        
+        let tree = ConversationTree.build(for: mainStatus, ancestors: ancestors ?? [], descendants: descendants ?? [])
+        
         guard case .displaying(let vc) = state else {
             return
         }
+        vc.addTree(tree, mainStatus: mainStatus)
+    }
     
-            await vc.addContext(context, for: mainStatus)
+    private func loadContext(for mainStatus: StatusMO) async -> ConversationContext? {
+        let request = Status.getContext(mainStatus.id)
+        do {
+            let (context, _) = try await mastodonController.run(request)
+            return context
         } catch {
             guard case .displaying(_) = state else {
-                return
+                return nil
             }
             let error = error as! Client.Error
             
             let config = ToastConfiguration(from: error, with: "Error Loading Context", in: self) { [weak self] toast in
                 toast.dismissToast(animated: true)
-                await self?.loadContext(for: mainStatus)
+                await self?.loadTree(for: mainStatus)
             }
             self.showToast(configuration: config, animated: true)
+            
+            return nil
         }
     }
     
@@ -341,6 +386,7 @@ extension ConversationViewController {
     enum Mode {
         case localID(String)
         case resolve(URL)
+        case preloaded(ConversationTree)
     }
 }