From 13b495b0f6f5b04ac1ff488028ac97259befe99f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Kaan=20K=C3=B6se?= Date: Wed, 19 Mar 2025 23:22:57 +0100 Subject: [PATCH] Fixed the Gmail sync identifier update issue and removed the batch message download. --- .../Interfaces/IAccountService.cs | 16 +- .../Processors/DefaultChangeProcessor.cs | 5 +- .../Processors/OutlookChangeProcessor.cs | 2 +- Wino.Core/Synchronizers/GmailSynchronizer.cs | 148 ++++++------------ Wino.Services/AccountService.cs | 48 +++--- 5 files changed, 90 insertions(+), 129 deletions(-) diff --git a/Wino.Core.Domain/Interfaces/IAccountService.cs b/Wino.Core.Domain/Interfaces/IAccountService.cs index 1efc115a..96c38315 100644 --- a/Wino.Core.Domain/Interfaces/IAccountService.cs +++ b/Wino.Core.Domain/Interfaces/IAccountService.cs @@ -62,15 +62,6 @@ public interface IAccountService /// Account id to remove from Task ClearAccountAttentionAsync(Guid accountId); - /// - /// Updates the account synchronization identifier. - /// For example: Gmail uses this identifier to keep track of the last synchronization. - /// Update is ignored for Gmail if the new identifier is older than the current one. - /// - /// Identifier to update - /// Current account synchronization modifier. - Task UpdateSynchronizationIdentifierAsync(Guid accountId, string newIdentifier); - /// /// Renames the merged inbox with the given id. /// @@ -164,4 +155,11 @@ public interface IAccountService /// Account id. /// Reason for the cache reset. Task DeleteAccountMailCacheAsync(Guid accountId, AccountCacheResetReason accountCacheResetReason); + + /// + /// Updates the synchronization identifier for a specific account asynchronously. + /// + /// Identifies the account for which the synchronization identifier is being updated. + /// Represents the new synchronization identifier to be set for the specified account. + Task UpdateSyncIdentifierRawAsync(Guid accountId, string syncIdentifier); } diff --git a/Wino.Core/Integration/Processors/DefaultChangeProcessor.cs b/Wino.Core/Integration/Processors/DefaultChangeProcessor.cs index 89c103f7..a7a98ea3 100644 --- a/Wino.Core/Integration/Processors/DefaultChangeProcessor.cs +++ b/Wino.Core/Integration/Processors/DefaultChangeProcessor.cs @@ -24,7 +24,7 @@ namespace Wino.Core.Integration.Processors; public interface IDefaultChangeProcessor { Task UpdateAccountAsync(MailAccount account); - Task UpdateAccountDeltaSynchronizationIdentifierAsync(Guid accountId, string deltaSynchronizationIdentifier); + // Task UpdateAccountDeltaSynchronizationIdentifierAsync(Guid accountId, string deltaSynchronizationIdentifier); Task DeleteAssignmentAsync(Guid accountId, string mailCopyId, string remoteFolderId); Task ChangeMailReadStatusAsync(string mailCopyId, bool isRead); Task ChangeFlagStatusAsync(string mailCopyId, bool isFlagged); @@ -64,6 +64,7 @@ public interface IDefaultChangeProcessor /// Whether mail exists in the folder or not. Task IsMailExistsInFolderAsync(string messageId, Guid folderId); Task> AreMailsExistsAsync(IEnumerable mailCopyIds); + Task UpdateAccountDeltaSynchronizationIdentifierAsync(Guid accountId, string synchronizationDeltaIdentifier); } public interface IGmailChangeProcessor : IDefaultChangeProcessor @@ -122,7 +123,7 @@ public class DefaultChangeProcessor(IDatabaseService databaseService, private readonly IMimeFileService _mimeFileService = mimeFileService; public Task UpdateAccountDeltaSynchronizationIdentifierAsync(Guid accountId, string synchronizationDeltaIdentifier) - => AccountService.UpdateSynchronizationIdentifierAsync(accountId, synchronizationDeltaIdentifier); + => AccountService.UpdateSyncIdentifierRawAsync(accountId, synchronizationDeltaIdentifier); public Task ChangeFlagStatusAsync(string mailCopyId, bool isFlagged) => MailService.ChangeFlagStatusAsync(mailCopyId, isFlagged); diff --git a/Wino.Core/Integration/Processors/OutlookChangeProcessor.cs b/Wino.Core/Integration/Processors/OutlookChangeProcessor.cs index 74efaea1..3b72c33e 100644 --- a/Wino.Core/Integration/Processors/OutlookChangeProcessor.cs +++ b/Wino.Core/Integration/Processors/OutlookChangeProcessor.cs @@ -22,7 +22,7 @@ public class OutlookChangeProcessor(IDatabaseService databaseService, { public Task ResetAccountDeltaTokenAsync(Guid accountId) - => AccountService.UpdateSynchronizationIdentifierAsync(accountId, null); + => AccountService.UpdateSyncIdentifierRawAsync(accountId, string.Empty); public async Task ResetFolderDeltaTokenAsync(Guid folderId) { diff --git a/Wino.Core/Synchronizers/GmailSynchronizer.cs b/Wino.Core/Synchronizers/GmailSynchronizer.cs index b495c7c7..a74e8740 100644 --- a/Wino.Core/Synchronizers/GmailSynchronizer.cs +++ b/Wino.Core/Synchronizers/GmailSynchronizer.cs @@ -274,7 +274,10 @@ public class GmailSynchronizer : WinoSynchronizer a.HistoryId); - if (maxHistoryId != null) + if (deltaChanges.Any()) { - // TODO: This is not good. Centralize the identifier fetch and prevent direct access here. - Account.SynchronizationDeltaIdentifier = await _gmailChangeProcessor.UpdateAccountDeltaSynchronizationIdentifierAsync(Account.Id, maxHistoryId.ToString()).ConfigureAwait(false); + var maxHistoryId = deltaChanges.Where(a => a.HistoryId != null).Max(a => a.HistoryId); - _logger.Debug("Final sync identifier {SynchronizationDeltaIdentifier}", Account.SynchronizationDeltaIdentifier); + await UpdateAccountSyncIdentifierAsync(maxHistoryId); + + if (maxHistoryId != null) + { + // TODO: This is not good. Centralize the identifier fetch and prevent direct access here. + // Account.SynchronizationDeltaIdentifier = await _gmailChangeProcessor.UpdateAccountDeltaSynchronizationIdentifierAsync(Account.Id, maxHistoryId.ToString()).ConfigureAwait(false); + + _logger.Debug("Final sync identifier {SynchronizationDeltaIdentifier}", Account.SynchronizationDeltaIdentifier); + } } // Get all unred new downloaded items and return in the result. @@ -310,6 +319,26 @@ public class GmailSynchronizer : WinoSynchronizer SynchronizeCalendarEventsInternalAsync(CalendarSynchronizationOptions options, CancellationToken cancellationToken = default) { _logger.Information("Internal calendar synchronization started for {Name}", Account.Name); @@ -640,87 +669,6 @@ public class GmailSynchronizer : WinoSynchronizer - /// Downloads given message ids per batch and processes them. - /// - /// Gmail message ids to download. - /// Cancellation token. - private async Task BatchDownloadMessagesAsync(List messageIds, CancellationToken cancellationToken = default) - { - var totalDownloadCount = messageIds.Count(); - - if (totalDownloadCount == 0) return; - - var downloadedItemCount = 0; - - _logger.Debug("Batch downloading {Count} messages for {Name}", messageIds.Count(), Account.Name); - - var allDownloadRequests = messageIds.Select(CreateSingleMessageGet); - - // Respect the batch size limit for batch requests. - var batchedDownloadRequests = allDownloadRequests.Batch((int)MaximumAllowedBatchRequestSize); - - _logger.Information("Total items to download: {TotalDownloadCount}. Created {Count} batch download requests for {Name}.", batchedDownloadRequests.Count(), Account.Name, totalDownloadCount); - - // Gmail SDK's BatchRequest has Action delegate for callback, not Task. - // Therefore it's not possible to make sure that downloaded item is processed in the database before this - // async callback is finished. Therefore we need to wrap all local database processings into task list and wait all of them to finish - // Batch execution finishes after response parsing is done. - - var batchProcessCallbacks = new List>(); - - foreach (var batchBundle in batchedDownloadRequests) - { - cancellationToken.ThrowIfCancellationRequested(); - - var batchRequest = new BatchRequest(_gmailService); - - // Queue each request into this batch. - batchBundle.ForEach(request => - { - batchRequest.Queue(request, (content, error, index, message) => - { - var downloadingMessageId = messageIds.ElementAt(index); - var downloadTask = HandleSingleItemDownloadedCallbackAsync(content, error, downloadingMessageId, cancellationToken); - - batchProcessCallbacks.Add(downloadTask); - - downloadedItemCount++; - - var progressValue = downloadedItemCount * 100 / Math.Max(1, totalDownloadCount); - - PublishSynchronizationProgress(progressValue); - }); - }); - - _logger.Information("Executing batch download with {Count} items.", batchRequest.Count); - - await batchRequest.ExecuteAsync(cancellationToken); - - // This is important due to bug in Gmail SDK. - // We force GC here to prevent Out of Memory exception. - // https://github.com/googleapis/google-api-dotnet-client/issues/2603 - - GC.Collect(); - } - - // Wait for all processing to finish. - await Task.WhenAll(batchProcessCallbacks).ConfigureAwait(false); - - // Try to update max history id. - var historyIdMessages = batchProcessCallbacks.Select(a => a.Result).Where(a => a?.HistoryId != null); - - if (historyIdMessages.Any()) - { - var maxHistoryId = historyIdMessages.Max(a => a.HistoryId.Value); - - if (maxHistoryId > 0) - { - Account.SynchronizationDeltaIdentifier = await _gmailChangeProcessor.UpdateAccountDeltaSynchronizationIdentifierAsync(Account.Id, maxHistoryId.ToString()).ConfigureAwait(false); - } - } - } - /// /// Processes the delta changes for the given history changes. /// Message downloads are not handled here since it's better to batch them. @@ -1060,7 +1008,10 @@ public class GmailSynchronizer : WinoSynchronizer currentIdentifier) + var newHistoryId = historyId.Value; + + return Account.SynchronizationDeltaIdentifier == null || + (ulong.TryParse(Account.SynchronizationDeltaIdentifier, out ulong currentIdentifier) && newHistoryId > currentIdentifier); + } + + private async Task UpdateAccountSyncIdentifierAsync(ulong? historyId) + { + if (ShouldUpdateSyncIdentifier(historyId)) { - Account.SynchronizationDeltaIdentifier = await _gmailChangeProcessor.UpdateAccountDeltaSynchronizationIdentifierAsync(Account.Id, message.HistoryId.ToString()); + Account.SynchronizationDeltaIdentifier = await _gmailChangeProcessor.UpdateAccountDeltaSynchronizationIdentifierAsync(Account.Id, historyId.Value.ToString()); } } @@ -1252,7 +1208,7 @@ public class GmailSynchronizer : WinoSynchronizer folderBundle) { diff --git a/Wino.Services/AccountService.cs b/Wino.Services/AccountService.cs index 0559ccee..49fad9de 100644 --- a/Wino.Services/AccountService.cs +++ b/Wino.Services/AccountService.cs @@ -67,6 +67,12 @@ public class AccountService : BaseDatabaseService, IAccountService WeakReferenceMessenger.Default.Send(new AccountsMenuRefreshRequested()); } + public async Task UpdateSyncIdentifierRawAsync(Guid accountId, string syncIdentifier) + { + await Connection.ExecuteAsync("UPDATE MailAccount SET SynchronizationDeltaIdentifier = ? WHERE Id = ?", syncIdentifier, accountId); + return syncIdentifier; + } + public async Task UnlinkMergedInboxAsync(Guid mergedInboxId) { var mergedInbox = await Connection.Table().FirstOrDefaultAsync(a => a.Id == mergedInboxId).ConfigureAwait(false); @@ -547,33 +553,33 @@ public class AccountService : BaseDatabaseService, IAccountService await Connection.InsertAsync(customServerInformation); } - public async Task UpdateSynchronizationIdentifierAsync(Guid accountId, string newIdentifier) - { - var account = await GetAccountAsync(accountId); + //public async Task UpdateSynchronizationIdentifierAsync(Guid accountId, string newIdentifier) + //{ + // var account = await GetAccountAsync(accountId); - if (account == null) - { - _logger.Error("Could not find account with id {AccountId}", accountId); - return string.Empty; - } + // if (account == null) + // { + // _logger.Error("Could not find account with id {AccountId}", accountId); + // return string.Empty; + // } - var currentIdentifier = account.SynchronizationDeltaIdentifier; + // var currentIdentifier = account.SynchronizationDeltaIdentifier; - bool shouldUpdateIdentifier = account.ProviderType == MailProviderType.Gmail ? - string.IsNullOrEmpty(currentIdentifier) ? true : !string.IsNullOrEmpty(currentIdentifier) - && ulong.TryParse(currentIdentifier, out ulong currentIdentifierValue) - && ulong.TryParse(newIdentifier, out ulong newIdentifierValue) - && newIdentifierValue > currentIdentifierValue : true; + // bool shouldUpdateIdentifier = account.ProviderType == MailProviderType.Gmail ? + // string.IsNullOrEmpty(currentIdentifier) ? true : !string.IsNullOrEmpty(currentIdentifier) + // && ulong.TryParse(currentIdentifier, out ulong currentIdentifierValue) + // && ulong.TryParse(newIdentifier, out ulong newIdentifierValue) + // && newIdentifierValue > currentIdentifierValue : true; - if (shouldUpdateIdentifier) - { - account.SynchronizationDeltaIdentifier = newIdentifier; + // if (shouldUpdateIdentifier) + // { + // account.SynchronizationDeltaIdentifier = newIdentifier; - await UpdateAccountAsync(account); - } + // await UpdateAccountAsync(account); + // } - return account.SynchronizationDeltaIdentifier; - } + // return account.SynchronizationDeltaIdentifier; + //} public async Task UpdateAccountOrdersAsync(Dictionary accountIdOrderPair) {