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.

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

fix: waiting for quicksync to finish to send message - WPB-15405 #2508

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion Cartfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ github "wireapp/cryptobox-ios" "v1.1.0_xcframework_arm64simulator"
github "wireapp/ZipArchive" "v2.4.2"
github "wireapp/libPhoneNumber-iOS" "1.1"
github "wireapp/ocmock" "v3.4.3_Xcode14.3.1"
github "wireapp/core-crypto" "v3.0.1"
github "wireapp/core-crypto" "v3.0.2"
binary "wire-avs.json"
2 changes: 1 addition & 1 deletion Cartfile.resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
binary "wire-avs.json" "10.0.4"
github "wireapp/ZipArchive" "v2.4.2"
github "wireapp/core-crypto" "v3.0.1"
github "wireapp/core-crypto" "v3.0.2"
github "wireapp/cryptobox-ios" "v1.1.0_xcframework_arm64simulator"
github "wireapp/generic-message-proto" "v1.51.0"
github "wireapp/libPhoneNumber-iOS" "1.1"
Expand Down
4 changes: 2 additions & 2 deletions WireAnalytics/Sources/WireDatadog/WireFakeDatadog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ public import WireLogging

public final class WireDatadog {

public var userIdentifier: String {
""
public var userIdentifier: String? {
nil
}

public init(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,6 @@ struct ConversationCreateEventProcessor: ConversationCreateEventProcessorProtoco
let conversation = event.conversation
let timestamp = event.timestamp

let existingConversation = await repository.fetchConversation(
id: conversationID.uuid,
domain: conversationID.domain
)

guard existingConversation == nil else {
WireLogger.eventProcessing.warn("Conversation already exists, aborting...")
return
}

await repository.storeConversation(
conversation.toDomainModel(),
timestamp: timestamp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ extension ConversationLocalStore {

func updateMembers(
from conversation: Conversation,
for localConversation: ZMConversation
for localConversation: ZMConversation,
shouldRemoveParticipants: Bool = true
) {
guard let members = conversation.members else {
return
Expand All @@ -84,7 +85,8 @@ extension ConversationLocalStore {

localConversation.updateMembers(
otherMembers,
selfUserRole: selfUserRole
selfUserRole: selfUserRole,
shouldRemoveParticipants: shouldRemoveParticipants
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ extension ConversationLocalStore {
case .mixed:
break /// no update, ignore
case .mls:
localConversation.appendMLSMigrationFinalizedSystemMessage(sender: sender, at: .now)
localConversation.appendMLSMigrationFinalizedSystemMessageIfNeeded(sender: sender, at: .now)
localConversation.messageProtocol = newMessageProtocol
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,8 @@ public final class ConversationLocalStore: ConversationLocalStoreProtocol {
from: conversation,
for: localConversation,
serverTimestamp: serverTimestamp,
isFederationEnabled: isFederationEnabled
isFederationEnabled: isFederationEnabled,
shouldRemoveParticipants: false
)

updateConversationStatus(
Expand Down Expand Up @@ -1067,7 +1068,8 @@ public final class ConversationLocalStore: ConversationLocalStoreProtocol {
from conversation: Conversation,
for localConversation: ZMConversation,
serverTimestamp: Date,
isFederationEnabled: Bool
isFederationEnabled: Bool,
shouldRemoveParticipants: Bool = true
) {
updateAttributes(
from: conversation,
Expand All @@ -1082,7 +1084,8 @@ public final class ConversationLocalStore: ConversationLocalStoreProtocol {

updateMembers(
from: conversation,
for: localConversation
for: localConversation,
shouldRemoveParticipants: shouldRemoveParticipants
)

updateConversationTimestamps(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,11 @@ struct OneOnOneResolver: OneOnOneResolverProtocol {
mlsConversation.mutableMessages.union(proteusConversation.allMessages)
}

// insert system message that we moved from proteus to MLS
let sender = ZMUser.selfUser(in: context)
mlsConversation.appendMLSMigrationFinalizedSystemMessage(sender: sender, at: .now)

if !allProteusConversations.isEmpty {
// insert system message that we moved from proteus to MLS
let sender = ZMUser.selfUser(in: context)
mlsConversation.appendMLSMigrationFinalizedSystemMessageIfNeeded(sender: sender, at: .now)

mlsConversation.isForcedReadOnly = false
// update just to be sure
mlsConversation.needsToBeUpdatedFromBackend = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ final class ConversationCreateEventProcessorTests: XCTestCase {
func testProcessEvent_It_Invokes_Repo_Methods() async {
// Mock

repository.fetchConversationIdDomain_MockMethod = { _, _ in nil }
repository.storeConversationTimestamp_MockMethod = { _, _ in }

// When
Expand All @@ -55,7 +54,6 @@ final class ConversationCreateEventProcessorTests: XCTestCase {

// Then

XCTAssertEqual(repository.fetchConversationIdDomain_Invocations.count, 1)
XCTAssertEqual(repository.storeConversationTimestamp_Invocations.count, 1)
}

Expand Down
2 changes: 1 addition & 1 deletion wire-avs.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"10.0.4": "https://github.com/wireapp/wire-avs/releases/download/10.0.4/avs.xcframework.zip",
"10.0.5": "https://github.com/wireapp/wire-avs/releases/download/10.0.5/avs.xcframework.zip",
}
33 changes: 28 additions & 5 deletions wire-ios-data-model/Source/MLS/MLSService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ public final class MLSService: MLSServiceInterface {

private static let epochChangeBufferSize: Int = 1000

private let maxRetryAttempts = 3

weak var delegate: MLSServiceDelegate?

// MARK: - Life cycle
Expand Down Expand Up @@ -1312,6 +1314,10 @@ public final class MLSService: MLSServiceInterface {
do {
logger.info("repairing out of sync conversation... (\(groupID.safeForLoggingDescription))")

// In case of `WrongEpoch` error, local and remote epochs have diverged so we may have missed events.
// This ensures we're on the latest state.
await syncStatus.performQuickSync()

guard let conversationInfo = fetchConversationInfo(
with: groupID,
in: context
Expand Down Expand Up @@ -1871,7 +1877,8 @@ public final class MLSService: MLSServiceInterface {

private func retryOnCommitFailure(
for groupID: MLSGroupID,
operation: @escaping () async throws -> Void
operation: @escaping () async throws -> Void,
retryCount: Int = 0
) async throws {

do {
Expand All @@ -1883,11 +1890,19 @@ public final class MLSService: MLSServiceInterface {
logger.info("sync finished, committing pending proposals...")
try await commitPendingProposals(in: groupID)

} catch CommitError.failedToSendCommit(recovery: .retryAfterQuickSync, _) {
} catch CommitError.failedToSendCommit(recovery: .retryAfterQuickSync, cause: let error) {
logger.warn("failed to send commit, syncing then retrying operation...")
await syncStatus.performQuickSync()
logger.info("sync finished, retying operation...")
try await retryOnCommitFailure(for: groupID, operation: operation)

guard retryCount <= maxRetryAttempts else {
throw error
}

var currentRetryCount = retryCount
currentRetryCount += 1

try await retryOnCommitFailure(for: groupID, operation: operation, retryCount: currentRetryCount)

} catch CommitError.failedToSendCommit(recovery: .retryAfterRepairingGroup, _) {
logger.warn("failed to send commit, repairing group then retrying operation...")
Expand All @@ -1899,9 +1914,17 @@ public final class MLSService: MLSServiceInterface {
logger.warn("failed to send commit, giving up...")
throw error

} catch ExternalCommitError.failedToSendCommit(recovery: .retry, _) {
} catch ExternalCommitError.failedToSendCommit(recovery: .retry, cause: let error) {
logger.warn("failed to send external commit, retrying operation...")
try await retryOnCommitFailure(for: groupID, operation: operation)

guard retryCount <= maxRetryAttempts else {
throw error
}

var currentRetryCount = retryCount
currentRetryCount += 1

try await retryOnCommitFailure(for: groupID, operation: operation, retryCount: currentRetryCount)

} catch ExternalCommitError.failedToSendCommit(recovery: .giveUp, cause: let error) {
logger.warn("failed to send external commit, giving up...")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,14 @@ public struct OneOnOneMigrator: OneOnOneMigratorInterface {
mlsConversation.mutableMessages.union(proteusConversation.allMessages)
}

// insert system message that we moved from proteus to MLS
let sender = ZMUser.selfUser(in: context)
mlsConversation.appendMLSMigrationFinalizedSystemMessage(sender: sender, at: .now)

if !proteusConversations.isEmpty {
// insert system message that we moved from proteus to MLS
let sender = ZMUser.selfUser(in: context)
mlsConversation.appendMLSMigrationFinalizedSystemMessageIfNeeded(sender: sender, at: .now)

// update just to be sure
mlsConversation.needsToBeUpdatedFromBackend = true
}

// switch active conversation
otherUser.oneOnOneConversation = mlsConversation
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,34 @@ public extension ZMConversation {

// MARK: - MLS Migration

func appendMLSMigrationFinalizedSystemMessage(
func appendMLSMigrationFinalizedSystemMessageIfNeeded(
sender: ZMUser,
at timestamp: Date
) {
guard let context = managedObjectContext else {
return
}

if !migrationFinalizedSystemMessageExists(in: context) {
appendMLSMigrationFinalizedSystemMessage(sender: sender, at: timestamp)
}
}

private func migrationFinalizedSystemMessageExists(in context: NSManagedObjectContext) -> Bool {
let request: NSFetchRequest<ZMSystemMessage> = NSFetchRequest(entityName: ZMSystemMessage.entityName())
request.predicate = NSPredicate(
format: "%K == %d AND %K == %@",
#keyPath(ZMSystemMessage.systemMessageType),
ZMSystemMessageType.mlsMigrationFinalized.rawValue,
#keyPath(ZMSystemMessage.visibleInConversation),
self
)
request.fetchLimit = 1
let messageCount = context.countOrAssert(request: request)
return messageCount == 1
}

private func appendMLSMigrationFinalizedSystemMessage(
sender: ZMUser,
at timestamp: Date
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,26 @@ public extension ZMConversation {
}
}

func updateMembers(_ usersAndRoles: [(ZMUser, Role?)], selfUserRole: Role?) {
func updateMembers(
_ usersAndRoles: [(ZMUser, Role?)],
selfUserRole: Role?,
shouldRemoveParticipants: Bool = true
) {
guard let context = managedObjectContext else {
return
}

let allParticipants = Set(usersAndRoles.map(\.0))
let removedParticipants = localParticipantsExcludingSelf.subtracting(allParticipants)
addParticipantsAndUpdateConversationState(usersAndRoles: usersAndRoles)
removeParticipantsAndUpdateConversationState(
users: removedParticipants,
initiatingUser: ZMUser.selfUser(in: context)
)

if shouldRemoveParticipants {
let allParticipants = Set(usersAndRoles.map(\.0))
let removedParticipants = localParticipantsExcludingSelf.subtracting(allParticipants)

removeParticipantsAndUpdateConversationState(
users: removedParticipants,
initiatingUser: ZMUser.selfUser(in: context)
)
}

let selfUser = ZMUser.selfUser(in: context)
if let role = selfUserRole {
Expand Down
Loading