-
Notifications
You must be signed in to change notification settings - Fork 15
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.
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
refactor: create local stores and UTS - WPB-12100 #2141
base: develop
Are you sure you want to change the base?
Changes from all commits
204fd96
07e8c01
ee3927c
3a3234b
8d72857
30796bf
c47a3e2
6e80d78
0a90373
a185f03
205b4d2
8aebf87
aa8da54
36e1e98
3d9ee9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,9 +68,9 @@ | |
|
||
for try await connections in connectionsPager { | ||
await withThrowingTaskGroup(of: Void.self) { taskGroup in | ||
for connection in connections { | ||
Check warning on line 71 in WireDomain/Sources/WireDomain/Repositories/Connections/ConnectionsRepository.swift GitHub Actions / Test ResultsTask-isolated value of type '() async throws -> ()' passed as a strongly transferred parameter; later accesses could race; this is an error in the Swift 6 language mode
|
||
taskGroup.addTask { | ||
try await connectionsLocalStore.storeConnection(connection) | ||
try await connectionsLocalStore.storeConnection(connection.toDomainModel()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prepare data for the store by mapping it to a domain model (so the local store doesn't know about the API layer) |
||
} | ||
} | ||
} | ||
|
@@ -80,7 +80,7 @@ | |
public func updateConnection( | ||
_ connection: Connection | ||
) async throws { | ||
try await connectionsLocalStore.storeConnection(connection) | ||
try await connectionsLocalStore.storeConnection(connection.toDomainModel()) | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// | ||
// Wire | ||
// Copyright (C) 2024 Wire Swiss GmbH | ||
// | ||
// This program is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
// | ||
// This program is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
// | ||
// You should have received a copy of the GNU General Public License | ||
// along with this program. If not, see http://www.gnu.org/licenses/. | ||
// | ||
|
||
Check warning on line 18 in WireDomain/Sources/WireDomain/Repositories/Connections/Models/ConnectionInfo.swift GitHub Actions / Test ResultsAdd '@preconcurrency' to suppress 'Sendable'-related warnings from module 'WireDataModel'
|
||
import WireDataModel | ||
|
||
public struct ConnectionInfo: Sendable { | ||
public let senderID: UUID? | ||
public let receiverID: UUID? | ||
Check warning on line 23 in WireDomain/Sources/WireDomain/Repositories/Connections/Models/ConnectionInfo.swift GitHub Actions / Test ResultsStored property 'receiverQualifiedID' of 'Sendable'-conforming struct 'ConnectionInfo' has non-sendable type 'QualifiedID?'; this is an error in the Swift 6 language mode
|
||
public let receiverQualifiedID: WireDataModel.QualifiedID? | ||
public let conversationID: UUID? | ||
Check warning on line 25 in WireDomain/Sources/WireDomain/Repositories/Connections/Models/ConnectionInfo.swift GitHub Actions / Test ResultsStored property 'qualifiedConversationID' of 'Sendable'-conforming struct 'ConnectionInfo' has non-sendable type 'QualifiedID?'; this is an error in the Swift 6 language mode
|
||
public let qualifiedConversationID: WireDataModel.QualifiedID? | ||
public let lastUpdate: Date | ||
public let status: WireDataModel.ZMConnectionStatus | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,7 @@ | |
// along with this program. If not, see http://www.gnu.org/licenses/. | ||
// | ||
|
||
import CoreData | ||
import WireAPI | ||
Check warning on line 19 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsAdd '@preconcurrency' to suppress 'Sendable'-related warnings from module 'WireDataModel'
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This local store still uses a WireAPI.Conversation object I didn't do it in this PR because we still have conversation events to tackle but eventually it will be mapped to a domain model as well. We also have a SystemMessage model and message creation related methods in this repo this will be removed (in another PR) now that we have a Message dedicated component (Repository and local store) |
||
import WireDataModel | ||
|
||
// sourcery: AutoMockable | ||
|
@@ -399,9 +398,9 @@ | |
to conversation: ZMConversation | ||
) async throws { | ||
typealias UserAndRole = (user: ZMUser, role: Role?) | ||
|
||
Check warning on line 401 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsType 'ZMUser' does not conform to the 'Sendable' protocol; this is an error in the Swift 6 language mode
Check warning on line 401 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsType 'Role' does not conform to the 'Sendable' protocol; this is an error in the Swift 6 language mode
Check warning on line 401 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsType 'ZMUser' does not conform to the 'Sendable' protocol; this is an error in the Swift 6 language mode
Check warning on line 401 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsType 'Role' does not conform to the 'Sendable' protocol; this is an error in the Swift 6 language mode
|
||
let usersAndRoles = await withTaskGroup(of: UserAndRole?.self) { taskGroup in | ||
for newParticipant in participants { | ||
Check warning on line 403 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsType 'ZMUser' does not conform to the 'Sendable' protocol; this is an error in the Swift 6 language mode
Check warning on line 403 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsType 'Role' does not conform to the 'Sendable' protocol; this is an error in the Swift 6 language mode
Check warning on line 403 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift GitHub Actions / Test ResultsTask-isolated value of type '() async -> UserAndRole?' (aka '() async -> Optional<(user: ZMUser, role: Optional<Role>)>') passed as a strongly transferred parameter; later accesses could race; this is an error in the Swift 6 language mode
|
||
taskGroup.addTask { [self] in | ||
let user = await userLocalStore.fetchOrCreateUser( | ||
id: newParticipant.id, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
// | ||
// Wire | ||
// Copyright (C) 2024 Wire Swiss GmbH | ||
// | ||
// This program is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
// | ||
// This program is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
// | ||
// You should have received a copy of the GNU General Public License | ||
// along with this program. If not, see http://www.gnu.org/licenses/. | ||
// | ||
|
||
import WireDataModel | ||
|
||
// sourcery: AutoMockable | ||
public protocol ConversationLabelsLocalStoreProtocol { | ||
|
||
/// Save label and related conversations objects to local storage. | ||
/// - Parameter conversationLabel: conversation label from WireAPI | ||
|
||
func storeLabel( | ||
_ conversationLabel: ConversationLabelInfo | ||
) async throws | ||
|
||
/// Delete old `folder` labels and related conversations objects from local storage. | ||
/// - Parameter excludedLabels: remote labels that should be excluded from deletion. | ||
/// - Only old labels of type `folder` are deleted, `favorite` labels always remain in the local storage. | ||
|
||
func deleteOldLabelsLocally( | ||
excludedLabels: [ConversationLabelInfo] | ||
) async throws | ||
} | ||
|
||
public final class ConversationLabelsLocalStore: ConversationLabelsLocalStoreProtocol { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This store was created. Storage operations were moved out from the related repository. |
||
|
||
// MARK: - Error | ||
|
||
enum Error: Swift.Error { | ||
case failedToStoreLabelLocally(UUID) | ||
} | ||
|
||
// MARK: - Properties | ||
|
||
private let context: NSManagedObjectContext | ||
private let logger = WireLogger(tag: "conversation-labels") | ||
|
||
// MARK: - Object lifecycle | ||
|
||
init( | ||
context: NSManagedObjectContext | ||
) { | ||
self.context = context | ||
} | ||
|
||
// MARK: - Public | ||
|
||
/// Save label and related conversations objects to local storage. | ||
/// - Parameter conversationLabel: conversation label from WireAPI | ||
|
||
public func storeLabel( | ||
_ conversationLabel: ConversationLabelInfo | ||
) async throws { | ||
try await context.perform { [context] in | ||
var created = false | ||
let label: Label? = if conversationLabel.type == Label.Kind.favorite.rawValue { | ||
Label.fetchFavoriteLabel(in: context) | ||
} else { | ||
Label.fetchOrCreate(remoteIdentifier: conversationLabel.id, create: true, in: context, created: &created) | ||
} | ||
|
||
guard let label else { | ||
throw Error.failedToStoreLabelLocally(conversationLabel.id) | ||
} | ||
|
||
label.name = conversationLabel.name | ||
label.kind = Label.Kind(rawValue: conversationLabel.type) ?? .folder | ||
|
||
let conversations = ZMConversation.fetchObjects( | ||
withRemoteIdentifiers: Set(conversationLabel.conversationIDs), | ||
in: context | ||
) as? Set<ZMConversation> ?? Set() | ||
|
||
label.conversations = conversations | ||
label.modifiedKeys = nil | ||
|
||
do { | ||
try context.save() | ||
} catch { | ||
throw Error.failedToStoreLabelLocally(conversationLabel.id) | ||
} | ||
} | ||
} | ||
|
||
public func deleteOldLabelsLocally( | ||
excludedLabels: [ConversationLabelInfo] | ||
) async throws { | ||
try await context.perform { [self] in | ||
let uuids = excludedLabels.map { $0.id.uuidData as NSData } | ||
let predicateFormat = "type == \(Label.Kind.folder.rawValue) AND NOT remoteIdentifier_data IN %@" | ||
|
||
let predicate = NSPredicate( | ||
format: predicateFormat, | ||
uuids as CVarArg | ||
) | ||
|
||
let fetchRequest: NSFetchRequest<NSFetchRequestResult> | ||
fetchRequest = NSFetchRequest(entityName: Label.entityName()) | ||
fetchRequest.predicate = predicate | ||
|
||
/// Since batch operations bypass the context processing, | ||
/// relationships rules are often ignored (e.g delete rule) | ||
/// Nevertheless, CoreData automatically handles two specific scenarios: | ||
/// `Cascade` delete rule and `Nullify` delete rule on an optional property | ||
/// Since `conversations` is nullify and optional, we can safely perform a batch delete. | ||
|
||
let deleteRequest = NSBatchDeleteRequest( | ||
fetchRequest: fetchRequest | ||
) | ||
|
||
deleteRequest.resultType = .resultTypeObjectIDs | ||
|
||
do { | ||
let batchDelete = try context.execute(deleteRequest) as? NSBatchDeleteResult | ||
|
||
guard let deleteResult = batchDelete?.result as? [NSManagedObjectID] else { | ||
throw ConversationLabelsRepositoryError.failedToDeleteStoredLabels | ||
} | ||
|
||
let deletedObjects: [AnyHashable: Any] = [ | ||
NSDeletedObjectsKey: deleteResult | ||
] | ||
|
||
/// Since `NSBatchDeleteRequest` only operates at the SQL level (in the persistent store itself), | ||
/// we need to manually update our in-memory objects after execution. | ||
|
||
NSManagedObjectContext.mergeChanges( | ||
fromRemoteContextSave: deletedObjects, | ||
into: [context] | ||
) | ||
|
||
} catch { | ||
logger.error("Failed to delete old labels: \(error)") | ||
throw error | ||
} | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// | ||
// Wire | ||
// Copyright (C) 2024 Wire Swiss GmbH | ||
// | ||
// This program is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
// | ||
// This program is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
// | ||
// You should have received a copy of the GNU General Public License | ||
// along with this program. If not, see http://www.gnu.org/licenses/. | ||
// | ||
|
||
import WireAPI | ||
|
||
extension WireAPI.ConversationLabel { | ||
|
||
func toDomainModel() -> ConversationLabelInfo { | ||
.init( | ||
id: id, | ||
name: name, | ||
type: type, | ||
conversationIDs: conversationIDs | ||
) | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll see changes like this in this PR: storage layer should not be aware of API objects so we create a domain model between repository and local store, here's the steps:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So now, it looks like all NSManagedObjects or context are isolated to Local stores, right?
I wonder if they would be cases where we want to be more flexible : thinking about the save contexts strategy we discussed.
Or going even further isolate all this to a WireStorage package