Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,6 @@ public class ChatChannelListController: DataController, DelegateCallable, DataSt
$0.controller(self, didChangeChannels: changes)
}
}

observer.onWillChange = { [weak self] in
self?.delegateCallback { [weak self] in
guard let self = self else {
log.warning("Callback called while self is nil")
return
}

$0.controllerWillChangeChannels(self)
}
}

return observer
}()

Expand Down Expand Up @@ -278,12 +266,6 @@ extension ChatChannelListController {

/// `ChatChannelListController` uses this protocol to communicate changes to its delegate.
public protocol ChatChannelListControllerDelegate: DataControllerStateDelegate {
/// The controller will update the list of observed channels.
///
/// - Parameter controller: The controller emitting the change callback.
///
func controllerWillChangeChannels(_ controller: ChatChannelListController)

/// The controller changed the list of observed channels.
///
/// - Parameters:
Expand All @@ -297,8 +279,6 @@ public protocol ChatChannelListControllerDelegate: DataControllerStateDelegate {
}

public extension ChatChannelListControllerDelegate {
func controllerWillChangeChannels(_ controller: ChatChannelListController) {}

func controller(
_ controller: ChatChannelListController,
didChangeChannels changes: [ListChange<ChatChannel>]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ import CoreData
import Foundation

class BackgroundDatabaseObserver<Item: Sendable, DTO: NSManagedObject>: @unchecked Sendable {
/// Called with the aggregated changes after the internal `NSFetchResultsController` calls `controllerWillChangeContent`
/// on its delegate.
var onWillChange: (@Sendable () -> Void)?

/// Called with the aggregated changes after the internal `NSFetchResultsController` calls `controllerDidChangeContent`
/// on its delegate.
var onDidChange: (@Sendable ([ListChange<Item>]) -> Void)?
Expand All @@ -30,24 +26,10 @@ class BackgroundDatabaseObserver<Item: Sendable, DTO: NSManagedObject>: @uncheck

private let queue = DispatchQueue(label: "io.getstream.list-database-observer", qos: .userInitiated)
private var _items: [Item]?

// State handling for supporting will change, because in the callback we should return the previous state.
private var _willChangeItems: [Item]?
private var _notifyingWillChange = false

/// The items that have been fetched and mapped
var rawItems: [Item] {
// During the onWillChange we swap the results back to the previous state because onWillChange
// is dispatched to the main thread and when the main thread handles it, observer has already processed
// the database change.
if onWillChange != nil {
let willChangeState: (active: Bool, cachedItems: [Item]?) = queue.sync { (_notifyingWillChange, _willChangeItems) }
if willChangeState.active {
return willChangeState.cachedItems ?? []
}
}

var rawItems: [Item]!
nonisolated(unsafe) var rawItems: [Item]!
frc.managedObjectContext.performAndWait {
// When we already have loaded items, reuse them, otherwise refetch all
rawItems = _items ?? updateItems(nil)
Expand Down Expand Up @@ -98,9 +80,6 @@ class BackgroundDatabaseObserver<Item: Sendable, DTO: NSManagedObject>: @uncheck
sectionNameKeyPath: nil,
cacheName: nil
)
changeAggregator.onWillChange = { [weak self] in
self?.notifyWillChange()
}
changeAggregator.onDidChange = { [weak self] changes in
guard let self else { return }
// Runs on the NSManagedObjectContext's queue, therefore skip performAndWait
Expand Down Expand Up @@ -133,33 +112,6 @@ class BackgroundDatabaseObserver<Item: Sendable, DTO: NSManagedObject>: @uncheck
}
}

private func notifyWillChange() {
guard let onWillChange = onWillChange else {
return
}
// Will change callback happens on the main thread but by that time the observer
// has already updated its local cached state. For allowing to access the previous
// state from the will change callback, there is no other way than caching previous state.
// This is used by the channel list delegate.

// `_items` is mutated by the NSManagedObjectContext's queue, here we are on that queue
// so it is safe to read the `_items` state from `self.queue`.
queue.sync {
_willChangeItems = _items
}
DispatchQueue.main.async { [weak self] in
guard let self else { return }
self.queue.async {
self._notifyingWillChange = true
}
onWillChange()
self.queue.async {
self._willChangeItems = nil
self._notifyingWillChange = false
}
}
}

private func notifyDidChange(changes: [ListChange<Item>]) {
guard let onDidChange = onDidChange else {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,6 @@ class ListChangeAggregator<DTO: NSManagedObject, Item>: NSObject, NSFetchedResul
/// Used for converting the `DTO`s provided by `FetchResultsController` to the resulting `Item`.
let itemCreator: (DTO) throws -> Item

/// Called when the aggregator is about to change the current content. It gets called when the `FetchedResultsController`
/// calls `controllerWillChangeContent` on its delegate.
var onWillChange: (() -> Void)?

/// Called with the aggregated changes after `FetchResultsController` calls controllerDidChangeContent` on its delegate.
var onDidChange: (([ListChange<Item>]) -> Void)?

Expand All @@ -167,7 +163,6 @@ class ListChangeAggregator<DTO: NSManagedObject, Item>: NSObject, NSFetchedResul
// This should ideally be in the extensions but it's not possible to implement @objc methods in extensions of generic types.

func controllerWillChangeContent(_ controller: NSFetchedResultsController<NSFetchRequestResult>) {
onWillChange?()
currentChanges = []
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,6 @@ open class ChatChannelListVC: _ViewController,

// MARK: - ChatChannelListControllerDelegate

open func controllerWillChangeChannels(_ controller: ChatChannelListController) {
collectionView.layoutIfNeeded()
}

open func controller(
_ controller: ChatChannelListController,
didChangeChannels changes: [ListChange<ChatChannel>]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,13 @@ import Foundation
// A concrete `ChannelListControllerDelegate` implementation allowing capturing the delegate calls
final class ChannelListController_Delegate: QueueAwareDelegate, ChatChannelListControllerDelegate {
@Atomic var state: DataController.State?
@Atomic var willChangeChannels_called = false
@Atomic var didChangeChannels_changes: [ListChange<ChatChannel>]?

func controller(_ controller: DataController, didChangeState state: DataController.State) {
self.state = state
validateQueue()
}

func controllerWillChangeChannels(_ controller: ChatChannelListController) {
willChangeChannels_called = true
validateQueue()
}

func controller(
_ controller: ChatChannelListController,
didChangeChannels changes: [ListChange<ChatChannel>]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,38 +651,26 @@ final class ChannelListController_Tests: XCTestCase {
let channel = try XCTUnwrap(client.databaseContainer.viewContext.channel(cid: cid)).asModel()

AssertAsync {
Assert.willBeTrue(delegate.willChangeChannels_called)
Assert.willBeEqual(delegate.didChangeChannels_changes, [.insert(channel, index: [0, 0])])
}
}

@MainActor func test_willAndDidCallbacks_areCalledInCorrectOrder() throws {
@MainActor func test_didCallbacks_areCalledInCorrectOrder() throws {
class Delegate: ChatChannelListControllerDelegate {
let cid: ChannelId

var willChangeCallbackCalled = false
var didChangeCallbackCalled = false

init(cid: ChannelId) {
self.cid = cid
}

func controllerWillChangeChannels(_ controller: ChatChannelListController) {
// Check the new channel is NOT in reported channels yet
XCTAssertFalse(controller.channels.contains { $0.cid == cid })
// Assert the "did" callback hasn't been called yet
XCTAssertFalse(didChangeCallbackCalled)
willChangeCallbackCalled = true
}

func controller(
_ controller: ChatChannelListController,
didChangeChannels changes: [ListChange<ChatChannel>]
) {
// Check the new channel is in reported channels
XCTAssertTrue(controller.channels.contains { $0.cid == cid })
// Assert the "will" callback has been called
XCTAssertTrue(willChangeCallbackCalled)
didChangeCallbackCalled = true
}
}
Expand All @@ -699,7 +687,6 @@ final class ChannelListController_Tests: XCTestCase {
}

AssertAsync {
Assert.willBeTrue(delegate.willChangeCallbackCalled)
Assert.willBeTrue(delegate.didChangeCallbackCalled)
}
}
Expand Down
10 changes: 0 additions & 10 deletions Tests/StreamChatTests/ListChangeAggregator_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,6 @@ final class ListChangeAggregator_Tests: XCTestCase {
super.tearDown()
}

func test_onWillChange_isCalled() {
// Set up aggregator callback
var callbackCalled = false
aggregator.onWillChange = { callbackCalled = true }

// Simulate FRC starts updating
aggregator.controllerWillChangeContent(fakeController)
XCTAssertTrue(callbackCalled)
}

func test_addingItems() {
// Set up aggregator callback
var result: [ListChange<String>]?
Expand Down
Loading