Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use view context in StreamChat #3550

Merged
merged 3 commits into from
Jan 4, 2025
Merged

Do not use view context in StreamChat #3550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90f61c9
Do not use view context in StreamChat
laevandus Jan 3, 2025
12ae0d0
Fix tests
laevandus Jan 3, 2025
0bb5ff3
Merge branch 'develop' into view-context-cleanup
laevandus Jan 3, 2025
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
17 changes: 4 additions & 13 deletions Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1670,26 +1670,17 @@ private extension ChatChannelController {
}
guard let cid = self.cid else { return nil }
let sortAscending = self.messageOrdering == .topToBottom ? false : true
var deletedMessageVisibility: ChatClientConfig.DeletedMessageVisibility?
var shouldShowShadowedMessages: Bool?
self.client.databaseContainer.viewContext.performAndWait { [weak self] in
guard let self = self else {
log.warning("Callback called while self is nil")
return
}
deletedMessageVisibility = self.client.databaseContainer.viewContext.deletedMessagesVisibility
shouldShowShadowedMessages = self.client.databaseContainer.viewContext.shouldShowShadowedMessages
}

let deletedMessageVisibility = client.config.deletedMessagesVisibility
let shouldShowShadowedMessages = client.config.shouldShowShadowedMessages
let pageSize = channelQuery.pagination?.pageSize ?? .messagesPageSize
let observer = BackgroundListDatabaseObserver(
database: client.databaseContainer,
fetchRequest: MessageDTO.messagesFetchRequest(
for: cid,
pageSize: pageSize,
sortAscending: sortAscending,
deletedMessagesVisibility: deletedMessageVisibility ?? .visibleForCurrentUser,
shouldShowShadowedMessages: shouldShowShadowedMessages ?? false
deletedMessagesVisibility: deletedMessageVisibility,
shouldShowShadowedMessages: shouldShowShadowedMessages
),
itemCreator: { try $0.asModel() as ChatMessage },
itemReuseKeyPaths: (\ChatMessage.id, \MessageDTO.id)
Expand Down
2 changes: 1 addition & 1 deletion Sources/StreamChat/Database/DatabaseContainer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ class DatabaseContainer: NSPersistentContainer, @unchecked Sendable {
}

func readAndWait<T>(_ actions: (DatabaseSession) throws -> T) throws -> T {
let context = viewContext
let context = backgroundReadOnlyContext
Copy link
Contributor Author

@laevandus laevandus Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a mistake in PR:3541

var result: T?
var readError: Error?
context.performAndWait {
Expand Down
7 changes: 5 additions & 2 deletions Sources/StreamChat/StateLayer/ChatClient+Factory.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ extension ChatClient {
/// Creates an instance of ``ConnectedUser`` which represents the logged-in user state and its actions.
///
/// - Throws: An error if no user is currently logged-in.
@MainActor public func makeConnectedUser() throws -> ConnectedUser {
let user = try CurrentUserDTO.load(context: databaseContainer.viewContext)
public func makeConnectedUser() throws -> ConnectedUser {
let user = try databaseContainer.readAndWait { session in
guard let dto = session.currentUser else { throw ClientError.CurrentUserDoesNotExist() }
return try dto.asModel()
}
return ConnectedUser(user: user, client: self)
Comment on lines -13 to 18
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All our other make methods do not require @MainActor, therefore we should skip it here as well. Downside is that this change would generate a warning that no await is needed if it was called outside of MainActor. Can we make this change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's fine

}
}
Expand Down
2 changes: 1 addition & 1 deletion TestTools/StreamChatTestTools/Mocks/StreamChat/ChatClient_Mock.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ extension ChatClient {
}

/// Create a new instance of mock `ChatClient`
static func mock(config: ChatClientConfig? = nil, bundle: Bundle? = nil) -> ChatClient {
static func mock(config: ChatClientConfig? = nil, bundle: Bundle? = nil) -> ChatClient_Mock {
.init(
config: config ?? defaultMockedConfig,
environment: .init(
Expand Down
18 changes: 14 additions & 4 deletions Tests/StreamChatTests/Controllers/ChannelController/ChannelController_Tests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ final class ChannelController_Tests: XCTestCase {
override func setUp() {
super.setUp()

setUp(with: ChatClient_Mock.defaultMockedConfig)
}

func setUp(with config: ChatClientConfig) {
env = TestEnvironment()
client = ChatClient.mock
client = ChatClient.mock(config: config)
channelId = ChannelId.unique
controller = ChatChannelController(
channelQuery: .init(cid: channelId),
Expand Down Expand Up @@ -1229,7 +1233,9 @@ final class ChannelController_Tests: XCTestCase {

func test_deletedMessages_withVisibleForCurrentUser_messageVisibility() throws {
// Simulate the config setting
client.databaseContainer.viewContext.deletedMessagesVisibility = .visibleForCurrentUser
var config = ChatClient_Mock.defaultMockedConfig
config.deletedMessagesVisibility = .visibleForCurrentUser
setUp(with: config)

let currentUserID: UserId = .unique

Expand Down Expand Up @@ -1264,7 +1270,9 @@ final class ChannelController_Tests: XCTestCase {

func test_deletedMessages_withAlwaysHidden_messageVisibility() throws {
// Simulate the config setting
client.databaseContainer.viewContext.deletedMessagesVisibility = .alwaysHidden
var config = ChatClient_Mock.defaultMockedConfig
config.deletedMessagesVisibility = .alwaysHidden
setUp(with: config)

let currentUserID: UserId = .unique

Expand Down Expand Up @@ -1334,7 +1342,9 @@ final class ChannelController_Tests: XCTestCase {

func test_shadowedMessages_whenVisible() throws {
// Simulate the config setting
client.databaseContainer.viewContext.shouldShowShadowedMessages = true
var config = ChatClient_Mock.defaultMockedConfig
config.shouldShowShadowedMessages = true
setUp(with: config)

let currentUserID: UserId = .unique

Expand Down
Loading