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

Jump to bottom

refactor: process conversation access update event - WPB-10164 #2028

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+381 −16
Open

refactor: process conversation access update event - WPB-10164 #2028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
487a01f
process conversation access roles and modes, add UTs - WPB-10164
jullianm Oct 14, 2024
f220075
rename repo method to fetchOrCreateConversation - WPB-10164 (#2028)
jullianm Oct 14, 2024
92d9ad1
reuse fetchOrCreate method - WPB-10164 (#2028)
jullianm Oct 14, 2024
8f5048f
remove unnecessary throws - WPB-10164 (#2028)
jullianm Oct 14, 2024
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
12 changes: 12 additions & 0 deletions .../Sources/WireAPI/Models/UpdateEvent/ConversationEvent/ConversationAccessUpdateEvent.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,16 @@

public let legacyAccessRole: ConversationAccessRoleLegacy?

public init(conversationID: ConversationID,
senderID: UserID,
accessModes: Set<ConversationAccessMode>,
accessRoles: Set<ConversationAccessRole>,
legacyAccessRole: ConversationAccessRoleLegacy?) {

Check warning on line 49 in WireAPI/Sources/WireAPI/Models/UpdateEvent/ConversationEvent/ConversationAccessUpdateEvent.swift

View workflow job for this annotation

GitHub Actions / SwiftFormat 

Wrap the opening brace of multiline statements. (wrapMultilineStatementBraces)
self.conversationID = conversationID
self.senderID = senderID
self.accessModes = accessModes
self.accessRoles = accessRoles
self.legacyAccessRole = legacyAccessRole
}

}
12 changes: 12 additions & 0 deletions WireDomain/Project/WireDomain Project.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
C97C015A2CB40010000683C5 /* FederationDeleteEventProcessorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C97C01572CB40010000683C5 /* FederationDeleteEventProcessorTests.swift */; };
C97C015C2CB40038000683C5 /* UserPropertiesSetEventProcessorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C97C015B2CB40038000683C5 /* UserPropertiesSetEventProcessorTests.swift */; };
C97C015F2CB40EF3000683C5 /* PushSupportedProtocolsUseCaseTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C97C015E2CB40EF3000683C5 /* PushSupportedProtocolsUseCaseTests.swift */; };
C97C01B52CBD236F000683C5 /* ConversationAccessUpdateEventProcessorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C97C01B42CBD236F000683C5 /* ConversationAccessUpdateEventProcessorTests.swift */; };
C9841A1F2CA309310062A834 /* MockFeatureConfigRepositoryProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9841A1E2CA309310062A834 /* MockFeatureConfigRepositoryProtocol.swift */; };
C99322D22C986E3A0065E10F /* TeamRepository.swift in Sources */ = {isa = PBXBuildFile; fileRef = C99322B22C986E3A0065E10F /* TeamRepository.swift */; };
C99322D32C986E3A0065E10F /* TeamRepositoryError.swift in Sources */ = {isa = PBXBuildFile; fileRef = C99322B32C986E3A0065E10F /* TeamRepositoryError.swift */; };
Expand Down Expand Up @@ -163,6 +164,7 @@
C97C01572CB40010000683C5 /* FederationDeleteEventProcessorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FederationDeleteEventProcessorTests.swift; sourceTree = "<group>"; };
C97C015B2CB40038000683C5 /* UserPropertiesSetEventProcessorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserPropertiesSetEventProcessorTests.swift; sourceTree = "<group>"; };
C97C015E2CB40EF3000683C5 /* PushSupportedProtocolsUseCaseTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PushSupportedProtocolsUseCaseTests.swift; sourceTree = "<group>"; };
C97C01B42CBD236F000683C5 /* ConversationAccessUpdateEventProcessorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConversationAccessUpdateEventProcessorTests.swift; sourceTree = "<group>"; };
C9841A1E2CA309310062A834 /* MockFeatureConfigRepositoryProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockFeatureConfigRepositoryProtocol.swift; sourceTree = "<group>"; };
C99322B22C986E3A0065E10F /* TeamRepository.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TeamRepository.swift; sourceTree = "<group>"; };
C99322B32C986E3A0065E10F /* TeamRepositoryError.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TeamRepositoryError.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -440,6 +442,14 @@
path = UseCases;
sourceTree = "<group>";
};
C97C01B32CBD2349000683C5 /* ConversationEventProcessorTests */ = {
isa = PBXGroup;
children = (
C97C01B42CBD236F000683C5 /* ConversationAccessUpdateEventProcessorTests.swift */,
);
path = ConversationEventProcessorTests;
sourceTree = "<group>";
};
C9841A1D2CA309090062A834 /* Mock */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -558,6 +568,7 @@
C9C8FDCD2C9DBE0E00702B91 /* Event Processing */ = {
isa = PBXGroup;
children = (
C97C01B32CBD2349000683C5 /* ConversationEventProcessorTests */,
C97C01582CB40010000683C5 /* FederationEventProcessor */,
C9C8FDC42C9DBE0E00702B91 /* FeatureConfigEventProcessor */,
C9C8FDC82C9DBE0E00702B91 /* TeamEventProcessor */,
Expand Down Expand Up @@ -976,6 +987,7 @@
files = (
C9F691292C9B164A008CC41F /* UserPushRemoveEventProcessorTests.swift in Sources */,
C93961932C91B15B00EA971A /* ConversationRepositoryTests.swift in Sources */,
C97C01B52CBD236F000683C5 /* ConversationAccessUpdateEventProcessorTests.swift in Sources */,
C93961922C91B12800EA971A /* TestError.swift in Sources */,
EEC410262C60D48900E89394 /* SyncManagerTests.swift in Sources */,
C9C8FDD32C9DBE0E00702B91 /* UserLegalHoldDisableEventProcessorTests.swift in Sources */,
Expand Down
42 changes: 38 additions & 4 deletions .../Event Processing/ConversationEventProcessor/ConversationAccessUpdateEventProcessor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// along with this program. If not, see http://www.gnu.org/licenses/.
//

import CoreData
import WireAPI

/// Process conversation access update events.
Expand All @@ -26,15 +27,48 @@ protocol ConversationAccessUpdateEventProcessorProtocol {
///
/// - Parameter event: A conversation access update event.

func processEvent(_ event: ConversationAccessUpdateEvent) async throws
func processEvent(_ event: ConversationAccessUpdateEvent) async

}

struct ConversationAccessUpdateEventProcessor: ConversationAccessUpdateEventProcessorProtocol {

func processEvent(_: ConversationAccessUpdateEvent) async throws {
// TODO: [WPB-10164]
assertionFailure("not implemented yet")
let context: NSManagedObjectContext
let repository: any ConversationRepositoryProtocol

func processEvent(_ event: ConversationAccessUpdateEvent) async {
let conversationID = event.conversationID

let localConversation = await repository.fetchOrCreateConversation(
with: conversationID.uuid,
domain: conversationID.domain
)

let accessRoles = if let legacyAccessRole = event.legacyAccessRole {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm... do we even need to handle the legacy roles anymore? The newer roles are always present.

getAccessRoles(from: legacyAccessRole)
} else {
event.accessRoles
}

await context.perform {
localConversation.accessModeStrings = event.accessModes.map(\.rawValue)
localConversation.accessRoleStringsV2 = accessRoles.map(\.rawValue)
Copy link
Collaborator

Choose a reason for hiding this comment

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

context.save() ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think saves should happen on a higher level. For instance:

  • Get batch of pending events
  • Process batch of events (no saves in-between)
  • After successfully processing all events in batch, one single save, then delete those pending events (they are consumed now

We need to take care that any committed work isn't repeated in case of any interruption to the event processing.

Comment on lines +53 to +55
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be part of the repository?

}
}

private func getAccessRoles(
from legacyRole: ConversationAccessRoleLegacy
) -> Set<ConversationAccessRole> {
switch legacyRole {
case .team:
[.teamMember]
case .activated:
[.teamMember, .nonTeamMember, .guest]
case .nonActivated:
[.teamMember, .nonTeamMember, .guest, .service]
case .private:
[]
}
}

}
39 changes: 31 additions & 8 deletions WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@
/// Check out the Confluence page for full details [here](https://wearezeta.atlassian.net/wiki/spaces/ENGINEERIN/pages/20514628/Conversations)
public protocol ConversationLocalStoreProtocol {

/// Fetches or creates a conversation locally.
/// - parameter id: The ID of the conversation.
/// - parameter domain: The domain of the conversation if any.
///
/// - returns: The `ZMConversation` found or created locally.

func fetchOrCreateConversation(
with id: UUID,
domain: String?
) async -> ZMConversation

/// Stores a given conversation locally.
/// - Parameter conversation: The conversation to store locally.
/// - Parameter isFederationEnabled: A flag indicating whether a `Federation` is enabled.
Expand Down Expand Up @@ -105,6 +116,19 @@

// MARK: - Public

public func fetchOrCreateConversation(
with id: UUID,
domain: String?
) async -> ZMConversation {
await context.perform { [context] in
ZMConversation.fetchOrCreate(
with: id,
domain: domain,
in: context
)
}
}

public func storeConversation(
_ conversation: WireAPI.Conversation,
isFederationEnabled: Bool
Expand Down Expand Up @@ -474,14 +498,13 @@
domain: String?,
handler: @escaping (ZMConversation) -> (ZMConversation, MLSGroupID?)
) async -> (ZMConversation, MLSGroupID?) {
await context.perform { [self] in
let conversation = ZMConversation.fetchOrCreate(
with: conversationID,
domain: domain,
in: context
)

return handler(conversation)
let conversation = await fetchOrCreateConversation(
with: conversationID,
domain: domain
)

Check warning on line 505 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift

View workflow job for this annotation

GitHub Actions / SwiftFormat 

Remove trailing space at end of a line. (trailingSpace)

Check failure on line 505 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift

View workflow job for this annotation

GitHub Actions / SwiftLint 

Lines should not have trailing whitespace (trailing_whitespace)
return await context.perform {
handler(conversation)
}
}

Expand Down
21 changes: 21 additions & 0 deletions WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationRepository.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@
/// Facilitate access to conversations related domain objects.
public protocol ConversationRepositoryProtocol {

/// Fetches or creates a conversation locally.
/// - parameter id: The ID of the conversation.
/// - parameter domain: The domain of the conversation if any.
///
/// - returns: The `ZMConversation` found or created locally.

func fetchOrCreateConversation(
with id: UUID,
domain: String?
) async -> ZMConversation

/// Fetches and persists all conversations

func pullConversations() async throws
Expand Down Expand Up @@ -91,6 +102,16 @@

// MARK: - Public

public func fetchOrCreateConversation(
with id: UUID,
domain: String?
) async -> ZMConversation {
await conversationsLocalStore.fetchOrCreateConversation(
with: id,
domain: domain
)
}

public func pullConversations() async throws {
var qualifiedIds: [WireAPI.QualifiedID]

Expand All @@ -115,7 +136,7 @@
let failedConversationsQualifiedIds = conversationList.failed

for conversation in foundConversations {
taskGroup.addTask { [self] in

Check warning on line 139 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationRepository.swift

View workflow job for this annotation

GitHub Actions / Test Results

Capture of 'self' with non-sendable type 'ConversationRepository' in a `@Sendable` closure 

Capture of 'self' with non-sendable type 'ConversationRepository' in a `@Sendable` closure
await conversationsLocalStore.storeConversation(
conversation,
isFederationEnabled: backendInfo.isFederationEnabled
Expand All @@ -124,7 +145,7 @@
}

for id in missingConversationsQualifiedIds {
taskGroup.addTask { [self] in

Check warning on line 148 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationRepository.swift

View workflow job for this annotation

GitHub Actions / Test Results

Capture of 'self' with non-sendable type 'ConversationRepository' in a `@Sendable` closure 

Capture of 'self' with non-sendable type 'ConversationRepository' in a `@Sendable` closure
await conversationsLocalStore.storeConversationNeedsBackendUpdate(
true,
qualifiedId: id
Expand All @@ -133,7 +154,7 @@
}

for id in failedConversationsQualifiedIds {
taskGroup.addTask { [self] in

Check warning on line 157 in WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationRepository.swift

View workflow job for this annotation

GitHub Actions / Test Results

Capture of 'self' with non-sendable type 'ConversationRepository' in a `@Sendable` closure 

Capture of 'self' with non-sendable type 'ConversationRepository' in a `@Sendable` closure
await conversationsLocalStore.storeFailedConversation(
withQualifiedId: id
)
Expand Down
36 changes: 36 additions & 0 deletions WireDomain/Sources/WireDomainSupport/Sourcery/generated/AutoMockable.generated.swift
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading