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

Improved online search performance when doing local operations #584

Merged
merged 3 commits into from
Feb 23, 2025
Merged
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
14 changes: 11 additions & 3 deletions Wino.Core.Domain/Interfaces/IImapSynchronizerStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,24 @@ public interface IImapSynchronizerStrategy
/// <param name="synchronizer">Imap synchronizer that downloads messages.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>List of new downloaded message ids that don't exist locally.</returns>
Task<List<string>> HandleSynchronizationAsync(IImapClient client, MailItemFolder folder, IImapSynchronizer synchronizer, CancellationToken cancellationToken = default);
Task<List<string>> HandleSynchronizationAsync(IImapClient client,
MailItemFolder folder,
IImapSynchronizer synchronizer,
CancellationToken cancellationToken = default);

/// <summary>
/// Downloads given set of messages from the folder.
/// Folder is expected to be opened and synchronizer is connected.
/// </summary>
/// <param name="synchronizer">Synchronizer that performs the action.</param>
/// <param name="folder">Remote folder to download messages from.</param>
/// <param name="remoteFolder">Remote folder to download messages from.</param>
/// <param name="localFolder">Local folder to assign mails to.</param>
/// <param name="uniqueIdSet">Set of message uniqueids.</param>
/// <param name="cancellationToken">Cancellation token.</param>
Task DownloadMessagesAsync(IImapSynchronizer synchronizer, IMailFolder folder, UniqueIdSet uniqueIdSet, CancellationToken cancellationToken = default);
Task DownloadMessagesAsync(IImapSynchronizer synchronizer,
IMailFolder remoteFolder,
MailItemFolder localFolder,
UniqueIdSet uniqueIdSet,
CancellationToken cancellationToken = default);
}

17 changes: 15 additions & 2 deletions Wino.Core.Domain/Interfaces/IMailService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ public interface IMailService
/// Returns the single mail item with the given mail copy id.
/// Caution: This method is not safe. Use other overrides.
/// </summary>
/// <param name="mailCopyId"></param>
/// <returns></returns>
Task<MailCopy> GetSingleMailItemAsync(string mailCopyId);

/// <summary>
/// Returns the multiple mail item with the given mail copy ids.
/// Caution: This method is not safe. Use other overrides.
/// </summary>
Task<List<MailCopy>> GetMailItemsAsync(IEnumerable<string> mailCopyIds);
Task<List<IMailItem>> FetchMailsAsync(MailListInitializationOptions options, CancellationToken cancellationToken = default);

/// <summary>
Expand Down Expand Up @@ -88,6 +92,15 @@ public interface IMailService
/// <param name="mailCopyId">Native mail id of the message.</param>
Task<bool> IsMailExistsAsync(string mailCopyId);

/// <summary>
/// Checks whether the given mail copy ids exists in the database.
/// Safely used for Outlook to prevent downloading the same mail twice.
/// For Gmail, it should be avoided since one mail may belong to multiple folders.
/// </summary>
/// <param name="mailCopyIds">Native mail id of the messages.</param>
/// <returns>List of Mail ids that already exists in the database.</returns>
Task<List<string>> AreMailsExistsAsync(IEnumerable<string> mailCopyIds);

/// <summary>
/// Returns all mails for given folder id.
/// </summary>
Expand Down
8 changes: 8 additions & 0 deletions Wino.Core/Integration/Processors/DefaultChangeProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public interface IDefaultChangeProcessor

Task UpdateCalendarDeltaSynchronizationToken(Guid calendarId, string deltaToken);
Task<MailCopy> GetMailCopyAsync(string mailCopyId);
Task<List<MailCopy>> GetMailCopiesAsync(IEnumerable<string> mailCopyIds);
Task CreateMailRawAsync(MailAccount account, MailItemFolder mailItemFolder, NewMailItemPackage package);
Task DeleteUserMailCacheAsync(Guid accountId);

Expand All @@ -73,6 +74,7 @@ public interface IDefaultChangeProcessor
/// <param name="folderId">Folder's local id.</param>
/// <returns>Whether mail exists in the folder or not.</returns>
Task<bool> IsMailExistsInFolderAsync(string messageId, Guid folderId);
Task<List<string>> AreMailsExistsAsync(IEnumerable<string> mailCopyIds);
}

public interface IGmailChangeProcessor : IDefaultChangeProcessor
Expand Down Expand Up @@ -139,9 +141,15 @@ public Task ChangeFlagStatusAsync(string mailCopyId, bool isFlagged)
public Task<bool> IsMailExistsAsync(string messageId)
=> MailService.IsMailExistsAsync(messageId);

public Task<List<string>> AreMailsExistsAsync(IEnumerable<string> mailCopyIds)
=> MailService.AreMailsExistsAsync(mailCopyIds);

public Task<MailCopy> GetMailCopyAsync(string mailCopyId)
=> MailService.GetSingleMailItemAsync(mailCopyId);

public Task<List<MailCopy>> GetMailCopiesAsync(IEnumerable<string> mailCopyIds)
=> MailService.GetMailItemsAsync(mailCopyIds);

public Task ChangeMailReadStatusAsync(string mailCopyId, bool isRead)
=> MailService.ChangeReadStatusAsync(mailCopyId, isRead);

Expand Down
38 changes: 6 additions & 32 deletions Wino.Core/Synchronizers/GmailSynchronizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ public override async Task<List<MailCopy>> OnlineSearchAsync(string queryText, L

string pageToken = null;

var messagesToDownload = new List<Message>();
List<Message> messagesToDownload = [];

do
{
Expand All @@ -1030,7 +1030,7 @@ public override async Task<List<MailCopy>> OnlineSearchAsync(string queryText, L
// Ignore the folders if the query starts with these keywords.
// User is trying to list everything.
}
else if (folders?.Any() ?? false)
else if (folders?.Count > 0)
{
request.LabelIds = folders.Select(a => a.RemoteFolderId).ToList();
}
Expand All @@ -1044,49 +1044,23 @@ public override async Task<List<MailCopy>> OnlineSearchAsync(string queryText, L
if (response.Messages == null) break;

// Handle skipping manually
foreach (var message in response.Messages)
{
messagesToDownload.Add(message);
}
messagesToDownload.AddRange(response.Messages);

pageToken = response.NextPageToken;
} while (!string.IsNullOrEmpty(pageToken));

// Do not download messages that exists, but return them for listing.

var messageIds = messagesToDownload.Select(a => a.Id).ToList();

List<string> downloadRequireMessageIds = new();
var messageIds = messagesToDownload.Select(a => a.Id);

foreach (var messageId in messageIds)
{
var exists = await _gmailChangeProcessor.IsMailExistsAsync(messageId).ConfigureAwait(false);

if (!exists)
{
downloadRequireMessageIds.Add(messageId);
}
}
var downloadRequireMessageIds = messageIds.Except(await _gmailChangeProcessor.AreMailsExistsAsync(messageIds));

// Download missing messages.
await BatchDownloadMessagesAsync(downloadRequireMessageIds, cancellationToken);

// Get results from database and return.

var searchResults = new List<MailCopy>();

foreach (var messageId in messageIds)
{
var copy = await _gmailChangeProcessor.GetMailCopyAsync(messageId).ConfigureAwait(false);

if (copy == null) continue;

searchResults.Add(copy);
}

return searchResults;

// TODO: Return the search result ids.
return await _gmailChangeProcessor.GetMailCopiesAsync(messageIds);
}

public override async Task DownloadMissingMimeMessageAsync(IMailItem mailItem,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,13 @@ protected async Task<List<string>> HandleChangedUIdsAsync(IImapSynchronizer sync
// Fetch the new mails in batch.

var batchedMessageIds = newMessageIds.Batch(50).ToList();
var downloadTasks = new List<Task>();

// Create tasks for each batch.
foreach (var group in batchedMessageIds)
{
downloadedMessageIds.AddRange(group.Select(a => MailkitClientExtensions.CreateUid(Folder.Id, a.Id)));
var task = DownloadMessagesAsync(synchronizer, remoteFolder, new UniqueIdSet(group), cancellationToken);
downloadTasks.Add(task);
await DownloadMessagesAsync(synchronizer, remoteFolder, Folder, new UniqueIdSet(group), cancellationToken).ConfigureAwait(false);
}

// Wait for all batches to complete.
await Task.WhenAll(downloadTasks).ConfigureAwait(false);

return downloadedMessageIds;
}

Expand Down Expand Up @@ -167,6 +161,7 @@ protected async Task ManageUUIdBasedDeletedMessagesAsync(MailItemFolder localFol

public async Task DownloadMessagesAsync(IImapSynchronizer synchronizer,
IMailFolder folder,
MailItemFolder localFolder,
UniqueIdSet uniqueIdSet,
CancellationToken cancellationToken = default)
{
Expand All @@ -178,7 +173,7 @@ public async Task DownloadMessagesAsync(IImapSynchronizer synchronizer,

var creationPackage = new ImapMessageCreationPackage(summary, mimeMessage);

var mailPackages = await synchronizer.CreateNewMailPackagesAsync(creationPackage, Folder, cancellationToken).ConfigureAwait(false);
var mailPackages = await synchronizer.CreateNewMailPackagesAsync(creationPackage, localFolder, cancellationToken).ConfigureAwait(false);

if (mailPackages != null)
{
Expand All @@ -187,7 +182,7 @@ public async Task DownloadMessagesAsync(IImapSynchronizer synchronizer,
// Local draft is mapped. We don't need to create a new mail copy.
if (package == null) continue;

await MailService.CreateMailAsync(Folder.MailAccountId, package).ConfigureAwait(false);
await MailService.CreateMailAsync(localFolder.MailAccountId, package).ConfigureAwait(false);
}
}
}
Expand Down
42 changes: 18 additions & 24 deletions Wino.Core/Synchronizers/ImapSynchronizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -639,52 +639,48 @@ public override async Task<List<MailCopy>> OnlineSearchAsync(string queryText, L
{
client = await _clientPool.GetClientAsync().ConfigureAwait(false);

var searchResults = new List<MailCopy>();
List<string> searchResultFolderMailUids = new();
List<MailCopy> searchResults = [];
List<string> searchResultFolderMailUids = [];

foreach (var folder in folders)
{
var remoteFolder = await client.GetFolderAsync(folder.RemoteFolderId).ConfigureAwait(false);
var remoteFolder = await client.GetFolderAsync(folder.RemoteFolderId, cancellationToken).ConfigureAwait(false);
await remoteFolder.OpenAsync(FolderAccess.ReadOnly, cancellationToken).ConfigureAwait(false);

// Look for subject and body.
var query = SearchQuery.BodyContains(queryText).Or(SearchQuery.SubjectContains(queryText));

var searchResultsInFolder = await remoteFolder.SearchAsync(query, cancellationToken).ConfigureAwait(false);
var nonExisttingUniqueIds = new List<UniqueId>();
Dictionary<string, UniqueId> searchResultsIdsInFolder = [];

foreach (var searchResultId in searchResultsInFolder)
{
var folderMailUid = MailkitClientExtensions.CreateUid(folder.Id, searchResultId.Id);
searchResultFolderMailUids.Add(folderMailUid);
searchResultsIdsInFolder.Add(folderMailUid, searchResultId);
}

bool exists = await _imapChangeProcessor.IsMailExistsAsync(folderMailUid);
// Populate no foundIds
var foundIds = await _imapChangeProcessor.AreMailsExistsAsync(searchResultsIdsInFolder.Select(a => a.Key));
var notFoundIds = searchResultsIdsInFolder.Keys.Except(foundIds);

if (!exists)
{
nonExisttingUniqueIds.Add(searchResultId);
}
List<UniqueId> nonExistingUniqueIds = [];
foreach (var nonExistingId in notFoundIds)
{
nonExistingUniqueIds.Add(searchResultsIdsInFolder[nonExistingId]);
}

if (nonExisttingUniqueIds.Any())
if (nonExistingUniqueIds.Count != 0)
{
var syncStrategy = _imapSynchronizationStrategyProvider.GetSynchronizationStrategy(client);
await syncStrategy.DownloadMessagesAsync(this, remoteFolder, new UniqueIdSet(nonExisttingUniqueIds, SortOrder.Ascending), cancellationToken).ConfigureAwait(false);
}

await remoteFolder.CloseAsync().ConfigureAwait(false);
}

foreach (var messageId in searchResultFolderMailUids)
{
var copy = await _imapChangeProcessor.GetMailCopyAsync(messageId).ConfigureAwait(false);

if (copy == null) continue;
await syncStrategy.DownloadMessagesAsync(this, remoteFolder, folder as MailItemFolder, new UniqueIdSet(nonExistingUniqueIds, SortOrder.Ascending), cancellationToken).ConfigureAwait(false);
}

searchResults.Add(copy);
await remoteFolder.CloseAsync(cancellationToken: cancellationToken).ConfigureAwait(false);
}

return searchResults;
return await _imapChangeProcessor.GetMailCopiesAsync(searchResultFolderMailUids);
}
catch (Exception ex)
{
Expand All @@ -700,8 +696,6 @@ public override async Task<List<MailCopy>> OnlineSearchAsync(string queryText, L

_clientPool.Release(client);
}

return new List<MailCopy>();
}

private async Task<IEnumerable<string>> SynchronizeFolderInternalAsync(MailItemFolder folder, CancellationToken cancellationToken = default)
Expand Down
13 changes: 1 addition & 12 deletions Wino.Core/Synchronizers/OutlookSynchronizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1054,18 +1054,7 @@ public override async Task<List<MailCopy>> OnlineSearchAsync(string queryText, L
}

// Get results from database and return.
var searchResults = new List<MailCopy>();

foreach (var messageId in existingMessageIds)
{
var copy = await _outlookChangeProcessor.GetMailCopyAsync(messageId).ConfigureAwait(false);

if (copy == null) continue;

searchResults.Add(copy);
}

return searchResults;
return await _outlookChangeProcessor.GetMailCopiesAsync(existingMessageIds);
}

private async Task<MimeMessage> DownloadMimeMessageAsync(string messageId, CancellationToken cancellationToken = default)
Expand Down
Loading