Reworked unobserved change handling to try to resolve crash (regenerate changeset)

pull/988/head
Morgan Pretty 8 months ago
parent 69de29912c
commit 7d38c9066c

@ -264,7 +264,7 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
private var lastInteractionIdMarkedAsRead: Int64? = nil
private var lastInteractionTimestampMsMarkedAsRead: Int64 = 0
public private(set) var unobservedInteractionDataChanges: ([SectionModel], StagedChangeset<[SectionModel]>)?
public private(set) var unobservedInteractionDataChanges: [SectionModel]?
public private(set) var interactionData: [SectionModel] = []
public private(set) var reactionExpandedInteractionIds: Set<Int64> = []
public private(set) var pagedDataObserver: PagedDatabaseObserver<Interaction, MessageViewModel>?
@ -273,14 +273,15 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
didSet {
// When starting to observe interaction changes we want to trigger a UI update just in case the
// data was changed while we weren't observing
if let changes: ([SectionModel], StagedChangeset<[SectionModel]>) = self.unobservedInteractionDataChanges {
let performChange: (([SectionModel], StagedChangeset<[SectionModel]>) -> ())? = onInteractionChange
switch Thread.isMainThread {
case true: performChange?(changes.0, changes.1)
case false: DispatchQueue.main.async { performChange?(changes.0, changes.1) }
}
if let changes: [SectionModel] = self.unobservedInteractionDataChanges {
PagedData.processAndTriggerUpdates(
updatedData: changes,
currentDataRetriever: { [weak self] in self?.interactionData },
onDataChangeRetriever: { [weak self] in self?.onInteractionChange },
onUnobservedDataChange: { [weak self] updatedData in
self?.unobservedInteractionDataChanges = updatedData
}
)
self.unobservedInteractionDataChanges = nil
}
}
@ -424,11 +425,8 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
),
currentDataRetriever: { self?.interactionData },
onDataChangeRetriever: { self?.onInteractionChange },
onUnobservedDataChange: { updatedData, changeset in
self?.unobservedInteractionDataChanges = (changeset.isEmpty ?
nil :
(updatedData, changeset)
)
onUnobservedDataChange: { updatedData in
self?.unobservedInteractionDataChanges = updatedData
}
)
}
@ -688,7 +686,7 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
guard let currentPageInfo: PagedData.PageInfo = self.pagedDataObserver?.pageInfo.wrappedValue else { return }
/// **MUST** have the same logic as in the 'PagedDataObserver.onChangeUnsorted' above
let currentData: [SectionModel] = (unobservedInteractionDataChanges?.0 ?? interactionData)
let currentData: [SectionModel] = (unobservedInteractionDataChanges ?? interactionData)
PagedData.processAndTriggerUpdates(
updatedData: process(
@ -699,11 +697,8 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
),
currentDataRetriever: { [weak self] in self?.interactionData },
onDataChangeRetriever: { [weak self] in self?.onInteractionChange },
onUnobservedDataChange: { [weak self] updatedData, changeset in
self?.unobservedInteractionDataChanges = (changeset.isEmpty ?
nil :
(updatedData, changeset)
)
onUnobservedDataChange: { [weak self] updatedData in
self?.unobservedInteractionDataChanges = updatedData
}
)
}

@ -211,11 +211,8 @@ public class HomeViewModel {
updatedData: self?.process(data: updatedData, for: updatedPageInfo),
currentDataRetriever: { self?.threadData },
onDataChangeRetriever: { self?.onThreadChange },
onUnobservedDataChange: { updatedData, changeset in
self?.unobservedThreadDataChanges = (changeset.isEmpty ?
nil :
(updatedData, changeset)
)
onUnobservedDataChange: { updatedData in
self?.unobservedThreadDataChanges = updatedData
}
)
@ -280,7 +277,7 @@ public class HomeViewModel {
else { return }
/// **MUST** have the same logic as in the 'PagedDataObserver.onChangeUnsorted' above
let currentData: [SectionModel] = (self.unobservedThreadDataChanges?.0 ?? self.threadData)
let currentData: [SectionModel] = (self.unobservedThreadDataChanges ?? self.threadData)
let updatedThreadData: [SectionModel] = self.process(
data: (currentData.first(where: { $0.model == .threads })?.elements ?? []),
for: currentPageInfo
@ -288,13 +285,10 @@ public class HomeViewModel {
PagedData.processAndTriggerUpdates(
updatedData: updatedThreadData,
currentDataRetriever: { [weak self] in (self?.unobservedThreadDataChanges?.0 ?? self?.threadData) },
currentDataRetriever: { [weak self] in (self?.unobservedThreadDataChanges ?? self?.threadData) },
onDataChangeRetriever: { [weak self] in self?.onThreadChange },
onUnobservedDataChange: { [weak self] updatedData, changeset in
self?.unobservedThreadDataChanges = (changeset.isEmpty ?
nil :
(updatedData, changeset)
)
onUnobservedDataChange: { [weak self] updatedData in
self?.unobservedThreadDataChanges = updatedData
}
)
}
@ -302,22 +296,25 @@ public class HomeViewModel {
// MARK: - Thread Data
private var hasReceivedInitialThreadData: Bool = false
public private(set) var unobservedThreadDataChanges: ([SectionModel], StagedChangeset<[SectionModel]>)?
public private(set) var unobservedThreadDataChanges: [SectionModel]?
public private(set) var threadData: [SectionModel] = []
public private(set) var pagedDataObserver: PagedDatabaseObserver<SessionThread, SessionThreadViewModel>?
public var onThreadChange: (([SectionModel], StagedChangeset<[SectionModel]>) -> ())? {
didSet {
guard onThreadChange != nil else { return }
// When starting to observe interaction changes we want to trigger a UI update just in case the
// data was changed while we weren't observing
if let changes: ([SectionModel], StagedChangeset<[SectionModel]>) = self.unobservedThreadDataChanges {
let performChange: (([SectionModel], StagedChangeset<[SectionModel]>) -> ())? = onThreadChange
switch Thread.isMainThread {
case true: performChange?(changes.0, changes.1)
case false: DispatchQueue.main.async { performChange?(changes.0, changes.1) }
}
if let changes: [SectionModel] = self.unobservedThreadDataChanges {
PagedData.processAndTriggerUpdates(
updatedData: changes,
currentDataRetriever: { [weak self] in self?.threadData },
onDataChangeRetriever: { [weak self] in self?.onThreadChange },
onUnobservedDataChange: { [weak self] updatedData in
self?.unobservedThreadDataChanges = updatedData
}
)
self.unobservedThreadDataChanges = nil
}
}

@ -43,20 +43,21 @@ public class MediaGalleryViewModel {
public private(set) var pagedDataObserver: PagedDatabaseObserver<Attachment, Item>?
/// This value is the current state of a gallery view
private var unobservedGalleryDataChanges: ([SectionModel], StagedChangeset<[SectionModel]>)?
private var unobservedGalleryDataChanges: [SectionModel]?
public private(set) var galleryData: [SectionModel] = []
public var onGalleryChange: (([SectionModel], StagedChangeset<[SectionModel]>) -> ())? {
didSet {
// When starting to observe interaction changes we want to trigger a UI update just in case the
// data was changed while we weren't observing
if let changes: ([SectionModel], StagedChangeset<[SectionModel]>) = self.unobservedGalleryDataChanges {
let performChange: (([SectionModel], StagedChangeset<[SectionModel]>) -> ())? = onGalleryChange
switch Thread.isMainThread {
case true: performChange?(changes.0, changes.1)
case false: DispatchQueue.main.async { performChange?(changes.0, changes.1) }
}
if let changes: [SectionModel] = self.unobservedGalleryDataChanges {
PagedData.processAndTriggerUpdates(
updatedData: changes,
currentDataRetriever: { [weak self] in self?.galleryData },
onDataChangeRetriever: { [weak self] in self?.onGalleryChange },
onUnobservedDataChange: { [weak self] updatedData in
self?.unobservedGalleryDataChanges = updatedData
}
)
self.unobservedGalleryDataChanges = nil
}
}
@ -104,11 +105,8 @@ public class MediaGalleryViewModel {
updatedData: self?.process(data: updatedData, for: updatedPageInfo),
currentDataRetriever: { self?.galleryData },
onDataChangeRetriever: { self?.onGalleryChange },
onUnobservedDataChange: { updatedData, changeset in
self?.unobservedGalleryDataChanges = (changeset.isEmpty ?
nil :
(updatedData, changeset)
)
onUnobservedDataChange: { updatedData in
self?.unobservedGalleryDataChanges = updatedData
}
)
}
@ -132,7 +130,7 @@ public class MediaGalleryViewModel {
// we don't want to mess with the initial view controller behaviour)
guard !performInitialQuerySync else {
loadInitialData()
updateGalleryData(self.unobservedGalleryDataChanges?.0 ?? [])
updateGalleryData(self.unobservedGalleryDataChanges ?? [])
return
}

@ -1017,10 +1017,21 @@ public enum PagedData {
updatedData: [SectionModel]?,
currentDataRetriever: @escaping (() -> [SectionModel]?),
onDataChangeRetriever: @escaping (() -> (([SectionModel], StagedChangeset<[SectionModel]>) -> ())?),
onUnobservedDataChange: @escaping (([SectionModel], StagedChangeset<[SectionModel]>) -> Void)
onUnobservedDataChange: @escaping (([SectionModel]) -> Void)
) {
guard let updatedData: [SectionModel] = updatedData else { return }
/// If we don't have a callback then store the changes to be sent back through this function if we ever start
/// observing again (when we have the callback it needs to do the data updating as it's tied to UI updates
/// and can cause crashes if not updated in the correct order)
///
/// **Note:** We do this even if the 'changeset' is empty because if this change reverts a previous change we
/// need to ensure the `onUnobservedDataChange` gets cleared so it doesn't end up in an invalid state
guard let onDataChange: (([SectionModel], StagedChangeset<[SectionModel]>) -> ()) = onDataChangeRetriever() else {
onUnobservedDataChange(updatedData)
return
}
// Note: While it would be nice to generate the changeset on a background thread it introduces
// a multi-threading issue where a data change can come in while the table is processing multiple
// updates resulting in the data being in a partially updated state (which makes the subsequent
@ -1033,17 +1044,6 @@ public enum PagedData {
target: updatedData
)
/// If we have the callback then trigger it, otherwise just store the changes to be sent to the callback if we ever
/// start observing again (when we have the callback it needs to do the data updating as it's tied to UI updates
/// and can cause crashes if not updated in the correct order)
///
/// **Note:** We do this even if the 'changeset' is empty because if this change reverts a previous change we
/// need to ensure the `onUnobservedDataChange` gets cleared so it doesn't end up in an invalid state
guard let onDataChange: (([SectionModel], StagedChangeset<[SectionModel]>) -> ()) = onDataChangeRetriever() else {
onUnobservedDataChange(updatedData, changeset)
return
}
// No need to do anything if there were no changes
guard !changeset.isEmpty else { return }

Loading…
Cancel
Save