mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-11-03 16:10:20 +00:00
Stop the timeline from requesting back pagination whilst loading existing items. (#2969)
* Handle Rust error "can't subscribe to the back-pagination status on a focused timeline" * Fix a bug where the timeline was attempting to paginate before adding the items. * Stop the timeline from paginating whilst loading existing items.
This commit is contained in:
@@ -229,7 +229,7 @@ struct RoomMemberState {
|
|||||||
/// Is also nice to have this as a wrapper for any state that is directly connected to the timeline.
|
/// Is also nice to have this as a wrapper for any state that is directly connected to the timeline.
|
||||||
struct TimelineViewState {
|
struct TimelineViewState {
|
||||||
var isLive = true
|
var isLive = true
|
||||||
var paginationState = PaginationState.default
|
var paginationState = PaginationState.initial
|
||||||
|
|
||||||
/// The room is in the process of loading items from a new timeline (switching to/from a detached timeline).
|
/// The room is in the process of loading items from a new timeline (switching to/from a detached timeline).
|
||||||
var isSwitchingTimelines = false
|
var isSwitchingTimelines = false
|
||||||
|
|||||||
@@ -117,7 +117,7 @@ class TimelineTableViewController: UIViewController {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// The state of pagination (in both directions) of the current timeline.
|
/// The state of pagination (in both directions) of the current timeline.
|
||||||
var paginationState: PaginationState = .default {
|
var paginationState: PaginationState = .initial {
|
||||||
didSet {
|
didSet {
|
||||||
// Paginate again if the threshold hasn't been satisfied.
|
// Paginate again if the threshold hasn't been satisfied.
|
||||||
paginatePublisher.send(())
|
paginatePublisher.send(())
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
|
|||||||
|
|
||||||
private var roomTimelineObservationToken: TaskHandle?
|
private var roomTimelineObservationToken: TaskHandle?
|
||||||
|
|
||||||
private let paginationStateSubject = CurrentValueSubject<PaginationState, Never>(.default)
|
private let paginationStateSubject = CurrentValueSubject<PaginationState, Never>(.initial)
|
||||||
var paginationState: PaginationState {
|
var paginationState: PaginationState {
|
||||||
paginationStateSubject.value
|
paginationStateSubject.value
|
||||||
}
|
}
|
||||||
@@ -74,6 +74,17 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A continuation to signal whether the initial timeline items have been loaded and processed.
|
||||||
|
private var hasLoadedInitialItemsContinuation: CheckedContinuation<Void, Never>?
|
||||||
|
/// A method that allows `await`ing the first update of timeline items from the listener, as the items
|
||||||
|
/// aren't added directly to the provider upon initialisation and may take some time to come in.
|
||||||
|
func waitForInitialItems() async {
|
||||||
|
guard itemProxies.isEmpty else { return }
|
||||||
|
return await withCheckedContinuation { continuation in
|
||||||
|
hasLoadedInitialItemsContinuation = continuation
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// MARK: - Private
|
// MARK: - Private
|
||||||
|
|
||||||
private func updateItemsWithDiffs(_ diffs: [TimelineDiff]) {
|
private func updateItemsWithDiffs(_ diffs: [TimelineDiff]) {
|
||||||
@@ -100,6 +111,11 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
|
|||||||
}
|
}
|
||||||
|
|
||||||
MXLog.verbose("Finished applying diffs, current items (\(itemProxies.count)) : \(itemProxies.map(\.debugIdentifier))")
|
MXLog.verbose("Finished applying diffs, current items (\(itemProxies.count)) : \(itemProxies.map(\.debugIdentifier))")
|
||||||
|
|
||||||
|
if let hasLoadedInitialItemsContinuation {
|
||||||
|
hasLoadedInitialItemsContinuation.resume()
|
||||||
|
self.hasLoadedInitialItemsContinuation = nil
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// swiftlint:disable:next cyclomatic_complexity
|
// swiftlint:disable:next cyclomatic_complexity
|
||||||
|
|||||||
@@ -18,6 +18,10 @@ import Combine
|
|||||||
import Foundation
|
import Foundation
|
||||||
import MatrixRustSDK
|
import MatrixRustSDK
|
||||||
|
|
||||||
|
enum PaginationDirection: String {
|
||||||
|
case backwards, forwards
|
||||||
|
}
|
||||||
|
|
||||||
enum PaginationStatus {
|
enum PaginationStatus {
|
||||||
case idle
|
case idle
|
||||||
case timelineEndReached
|
case timelineEndReached
|
||||||
@@ -25,7 +29,9 @@ enum PaginationStatus {
|
|||||||
}
|
}
|
||||||
|
|
||||||
struct PaginationState: Equatable {
|
struct PaginationState: Equatable {
|
||||||
static var `default` = PaginationState(backward: .idle, forward: .timelineEndReached)
|
/// An initial state that is used to prevent pagination whilst loading the timeline.
|
||||||
|
/// Once the initial items are loaded the TimelineProxy will publish the correct value.
|
||||||
|
static var initial = PaginationState(backward: .timelineEndReached, forward: .timelineEndReached)
|
||||||
let backward: PaginationStatus
|
let backward: PaginationStatus
|
||||||
let forward: PaginationStatus
|
let forward: PaginationStatus
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -23,7 +23,8 @@ final class TimelineProxy: TimelineProxyProtocol {
|
|||||||
|
|
||||||
private var backPaginationStatusObservationToken: TaskHandle?
|
private var backPaginationStatusObservationToken: TaskHandle?
|
||||||
|
|
||||||
private let backPaginationSubscriptionSubject = CurrentValueSubject<PaginationStatus, Never>(.idle)
|
// The default values don't matter here, they will be updated when calling subscribeToPagination.
|
||||||
|
private let backPaginationStatusSubject = CurrentValueSubject<PaginationStatus, Never>(.timelineEndReached)
|
||||||
private let forwardPaginationStatusSubject = CurrentValueSubject<PaginationStatus, Never>(.timelineEndReached)
|
private let forwardPaginationStatusSubject = CurrentValueSubject<PaginationStatus, Never>(.timelineEndReached)
|
||||||
|
|
||||||
let isLive: Bool
|
let isLive: Bool
|
||||||
@@ -48,14 +49,19 @@ final class TimelineProxy: TimelineProxyProtocol {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
let paginationStatePublisher = backPaginationSubscriptionSubject
|
let paginationStatePublisher = backPaginationStatusSubject
|
||||||
.combineLatest(forwardPaginationStatusSubject)
|
.combineLatest(forwardPaginationStatusSubject)
|
||||||
.map { PaginationState(backward: $0.0, forward: $0.1) }
|
.map { PaginationState(backward: $0.0, forward: $0.1) }
|
||||||
.eraseToAnyPublisher()
|
.eraseToAnyPublisher()
|
||||||
|
|
||||||
await subscribeToPagination()
|
await subscribeToPagination()
|
||||||
|
|
||||||
innerTimelineProvider = await RoomTimelineProvider(timeline: timeline, isLive: isLive, paginationStatePublisher: paginationStatePublisher)
|
let provider = await RoomTimelineProvider(timeline: timeline, isLive: isLive, paginationStatePublisher: paginationStatePublisher)
|
||||||
|
// Make sure the existing items are built so that we have content in the timeline before
|
||||||
|
// determining whether or not the timeline should paginate to load more items.
|
||||||
|
await provider.waitForInitialItems()
|
||||||
|
|
||||||
|
innerTimelineProvider = provider
|
||||||
}
|
}
|
||||||
|
|
||||||
func fetchDetails(for eventID: String) {
|
func fetchDetails(for eventID: String) {
|
||||||
@@ -75,6 +81,22 @@ final class TimelineProxy: TimelineProxyProtocol {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func paginateBackwards(requestSize: UInt16) async -> Result<Void, TimelineProxyError> {
|
func paginateBackwards(requestSize: UInt16) async -> Result<Void, TimelineProxyError> {
|
||||||
|
// We can't subscribe to back pagination on detached timelines and as live timelines
|
||||||
|
// can be shared between multiple instances of the same room on the stack, it is
|
||||||
|
// safer to still use the subscription logic for back pagination when live.
|
||||||
|
await if isLive {
|
||||||
|
paginateBackwardsOnLive(requestSize: requestSize)
|
||||||
|
} else {
|
||||||
|
focussedPaginate(.backwards, requestSize: requestSize)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func paginateForwards(requestSize: UInt16) async -> Result<Void, TimelineProxyError> {
|
||||||
|
await focussedPaginate(.forwards, requestSize: requestSize)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Paginate backwards using the subscription from Rust to drive the pagination state.
|
||||||
|
private func paginateBackwardsOnLive(requestSize: UInt16) async -> Result<Void, TimelineProxyError> {
|
||||||
MXLog.info("Paginating backwards")
|
MXLog.info("Paginating backwards")
|
||||||
|
|
||||||
do {
|
do {
|
||||||
@@ -88,26 +110,36 @@ final class TimelineProxy: TimelineProxyProtocol {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func paginateForwards(requestSize: UInt16) async -> Result<Void, TimelineProxyError> {
|
/// Paginate forward or backwards using our own logic to drive the pagination state as the
|
||||||
// This extra check is necessary as forwards pagination status doesn't support subscribing.
|
/// Rust subscription isn't allowed on focussed/detached timelines.
|
||||||
|
private func focussedPaginate(_ direction: PaginationDirection, requestSize: UInt16) async -> Result<Void, TimelineProxyError> {
|
||||||
|
let subject = switch direction {
|
||||||
|
case .backwards: backPaginationStatusSubject
|
||||||
|
case .forwards: forwardPaginationStatusSubject
|
||||||
|
}
|
||||||
|
|
||||||
|
// This extra check is necessary as detached timelines don't support subscribing to pagination status.
|
||||||
// We need it to make sure we send a valid status after a failure.
|
// We need it to make sure we send a valid status after a failure.
|
||||||
guard forwardPaginationStatusSubject.value == .idle else {
|
guard subject.value == .idle else {
|
||||||
MXLog.error("Attempting to paginate forwards when already at the end.")
|
MXLog.error("Attempting to paginate \(direction.rawValue) when already at the end.")
|
||||||
return .failure(.failedPaginatingEndReached)
|
return .failure(.failedPaginatingEndReached)
|
||||||
}
|
}
|
||||||
|
|
||||||
MXLog.info("Paginating forwards")
|
MXLog.info("Paginating \(direction.rawValue)")
|
||||||
forwardPaginationStatusSubject.send(.paginating)
|
subject.send(.paginating)
|
||||||
|
|
||||||
do {
|
do {
|
||||||
let timelineEndReached = try await timeline.focusedPaginateForwards(numEvents: requestSize)
|
let timelineEndReached = try await switch direction {
|
||||||
MXLog.info("Finished paginating forwards")
|
case .backwards: timeline.paginateBackwards(numEvents: requestSize)
|
||||||
|
case .forwards: timeline.focusedPaginateForwards(numEvents: requestSize)
|
||||||
|
}
|
||||||
|
MXLog.info("Finished paginating \(direction.rawValue)")
|
||||||
|
|
||||||
forwardPaginationStatusSubject.send(timelineEndReached ? .timelineEndReached : .idle)
|
subject.send(timelineEndReached ? .timelineEndReached : .idle)
|
||||||
return .success(())
|
return .success(())
|
||||||
} catch {
|
} catch {
|
||||||
MXLog.error("Failed paginating forwards with error: \(error)")
|
MXLog.error("Failed paginating \(direction.rawValue) with error: \(error)")
|
||||||
forwardPaginationStatusSubject.send(.idle)
|
subject.send(.idle)
|
||||||
return .failure(.sdkError(error))
|
return .failure(.sdkError(error))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -499,26 +531,31 @@ final class TimelineProxy: TimelineProxyProtocol {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private func subscribeToPagination() async {
|
private func subscribeToPagination() async {
|
||||||
let backPaginationListener = RoomPaginationStatusListener { [weak self] status in
|
if isLive {
|
||||||
guard let self else {
|
let backPaginationListener = RoomPaginationStatusListener { [weak self] status in
|
||||||
return
|
guard let self else {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
switch status {
|
||||||
|
case .idle(let hitStartOfTimeline):
|
||||||
|
backPaginationStatusSubject.send(hitStartOfTimeline ? .timelineEndReached : .idle)
|
||||||
|
case .paginating:
|
||||||
|
backPaginationStatusSubject.send(.paginating)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
switch status {
|
do {
|
||||||
case .idle(let hitStartOfTimeline):
|
backPaginationStatusObservationToken = try await timeline.subscribeToBackPaginationStatus(listener: backPaginationListener)
|
||||||
backPaginationSubscriptionSubject.send(hitStartOfTimeline ? .timelineEndReached : .idle)
|
} catch {
|
||||||
case .paginating:
|
MXLog.error("Failed to subscribe to back pagination status with error: \(error)")
|
||||||
backPaginationSubscriptionSubject.send(.paginating)
|
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
// Detached timelines don't support observation, set the initial state ourself.
|
||||||
|
backPaginationStatusSubject.send(.idle)
|
||||||
}
|
}
|
||||||
|
|
||||||
do {
|
// Detached timelines don't support observation, set the initial state ourself.
|
||||||
backPaginationStatusObservationToken = try await timeline.subscribeToBackPaginationStatus(listener: backPaginationListener)
|
|
||||||
} catch {
|
|
||||||
MXLog.error("Failed to subscribe to back pagination status with error: \(error)")
|
|
||||||
}
|
|
||||||
|
|
||||||
// Forward pagination doesn't support observation, set the initial state ourself.
|
|
||||||
forwardPaginationStatusSubject.send(isLive ? .timelineEndReached : .idle)
|
forwardPaginationStatusSubject.send(isLive ? .timelineEndReached : .idle)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user