From d922dd2f2e2c023a6ce1bd989bdcb703aa68eed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Kaan=20K=C3=B6se?= Date: Sun, 12 Apr 2026 15:56:27 +0200 Subject: [PATCH] Fix online search dedupe and pane layout scrolling --- .../MailItem/MailListInitializationOptions.cs | 1 + Wino.Core.Tests/Services/MailFetchingTests.cs | 148 +++++++++++++++++- Wino.Core/Synchronizers/GmailSynchronizer.cs | 10 +- Wino.Core/Synchronizers/ImapSynchronizer.cs | 11 +- .../Synchronizers/OutlookSynchronizer.cs | 27 +++- Wino.Mail.ViewModels/MailListPageViewModel.cs | 50 +++++- Wino.Mail.WinUI/Views/WinoAppShell.xaml | 31 ++-- Wino.Mail.WinUI/Views/WinoAppShell.xaml.cs | 33 ++++ Wino.Services/MailService.cs | 42 ++++- 9 files changed, 318 insertions(+), 35 deletions(-) diff --git a/Wino.Core.Domain/Models/MailItem/MailListInitializationOptions.cs b/Wino.Core.Domain/Models/MailItem/MailListInitializationOptions.cs index 7d97e19d..0ac98064 100644 --- a/Wino.Core.Domain/Models/MailItem/MailListInitializationOptions.cs +++ b/Wino.Core.Domain/Models/MailItem/MailListInitializationOptions.cs @@ -15,5 +15,6 @@ public record MailListInitializationOptions(IEnumerable Folders string SearchQuery, ConcurrentDictionary ExistingUniqueIds = null, List PreFetchMailCopies = null, + bool DeduplicateByServerId = false, int Skip = 0, int Take = 0); diff --git a/Wino.Core.Tests/Services/MailFetchingTests.cs b/Wino.Core.Tests/Services/MailFetchingTests.cs index 66d8bf2e..61fa2437 100644 --- a/Wino.Core.Tests/Services/MailFetchingTests.cs +++ b/Wino.Core.Tests/Services/MailFetchingTests.cs @@ -55,6 +55,7 @@ public class MailFetchingTests : IAsyncLifetime Id = Guid.NewGuid(), MailAccountId = _testAccount.Id, FolderName = "Inbox", + RemoteFolderId = "inbox", SpecialFolderType = SpecialFolderType.Inbox, IsSystemFolder = true, IsSynchronizationEnabled = true @@ -190,6 +191,112 @@ public class MailFetchingTests : IAsyncLifetime "self-sent mail must use account metadata for the sender contact"); } + [Fact] + public async Task FetchMailsAsync_PreFetchedOnlineSearch_DeduplicatesByServerIdWithinAccount() + { + var archiveFolder = await CreateFolderAsync(_testAccount, "Archive", "archive", SpecialFolderType.Archive); + var sharedId = "server-mail-1"; + var olderCopy = BuildMail(_inboxFolder.Id, DateTime.UtcNow.AddMinutes(-5)); + olderCopy.Id = sharedId; + var newerCopy = BuildMail(archiveFolder.Id, DateTime.UtcNow); + newerCopy.Id = sharedId; + + var options = BuildOptions([_inboxFolder, archiveFolder], createThreads: false, deduplicateByServerId: true) with + { + PreFetchMailCopies = [olderCopy, newerCopy] + }; + + var result = await _mailService.FetchMailsAsync(options); + + result.Should().HaveCount(1, "online search should show one visible result per server message within an account"); + result.Single().UniqueId.Should().Be(newerCopy.UniqueId, "the newest copy should win when the searched folders tie"); + } + + [Fact] + public async Task FetchMailsAsync_PreFetchedOnlineSearch_KeepsSameServerIdAcrossAccountsSeparate() + { + var secondAccount = await CreateAccountAsync("Second Account", "second@test.local"); + var secondInbox = await CreateFolderAsync(secondAccount, "Inbox", "inbox-2", SpecialFolderType.Inbox); + const string sharedId = "server-mail-2"; + + var firstAccountCopy = BuildMail(_inboxFolder.Id, DateTime.UtcNow.AddMinutes(-1)); + firstAccountCopy.Id = sharedId; + + var secondAccountCopy = BuildMail(secondInbox.Id, DateTime.UtcNow); + secondAccountCopy.Id = sharedId; + + var options = BuildOptions([_inboxFolder, secondInbox], createThreads: false, deduplicateByServerId: true) with + { + PreFetchMailCopies = [firstAccountCopy, secondAccountCopy] + }; + + var result = await _mailService.FetchMailsAsync(options); + + result.Should().HaveCount(2, "dedupe should be scoped per account, not just per server id string"); + result.Select(m => m.AssignedAccount!.Id).Should().BeEquivalentTo([_testAccount.Id, secondAccount.Id]); + } + + [Fact] + public async Task FetchMailsAsync_PreFetchedOnlineSearch_PrefersActiveFolderCopy() + { + var archiveFolder = await CreateFolderAsync(_testAccount, "Archive", "archive-active", SpecialFolderType.Archive); + const string sharedId = "server-mail-3"; + + var activeFolderCopy = BuildMail(_inboxFolder.Id, DateTime.UtcNow.AddMinutes(-5)); + activeFolderCopy.Id = sharedId; + + var newerNonActiveCopy = BuildMail(archiveFolder.Id, DateTime.UtcNow); + newerNonActiveCopy.Id = sharedId; + + var options = BuildOptions([_inboxFolder], createThreads: false, deduplicateByServerId: true) with + { + PreFetchMailCopies = [activeFolderCopy, newerNonActiveCopy] + }; + + var result = await _mailService.FetchMailsAsync(options); + + result.Should().HaveCount(1); + result.Single().FolderId.Should().Be(_inboxFolder.Id, "a copy from the actively searched folder should win over newer non-searched copies"); + } + + [Fact] + public async Task CreateAssignmentAsync_ExistingAssignment_IsIgnored() + { + var archiveFolder = await CreateFolderAsync(_testAccount, "Archive", "archive-existing", SpecialFolderType.Archive); + const string sharedId = "server-mail-4"; + + await _databaseService.Connection.InsertAllAsync(new[] + { + BuildMail(_inboxFolder.Id, DateTime.UtcNow.AddMinutes(-1), id: sharedId), + BuildMail(archiveFolder.Id, DateTime.UtcNow, id: sharedId) + }); + + await _mailService.CreateAssignmentAsync(_testAccount.Id, sharedId, archiveFolder.RemoteFolderId); + + var count = await _databaseService.Connection.Table().Where(mail => mail.Id == sharedId).CountAsync(); + count.Should().Be(2, "re-creating an existing folder assignment must not insert another row"); + } + + [Fact] + public async Task CreateAssignmentAsync_NewAssignment_CreatesAdditionalRow() + { + var archiveFolder = await CreateFolderAsync(_testAccount, "Archive", "archive-new", SpecialFolderType.Archive); + const string sharedId = "server-mail-5"; + + await _databaseService.Connection.InsertAsync( + BuildMail(_inboxFolder.Id, DateTime.UtcNow, id: sharedId), + typeof(MailCopy)); + + await _mailService.CreateAssignmentAsync(_testAccount.Id, sharedId, archiveFolder.RemoteFolderId); + + var insertedCopies = await _databaseService.Connection.Table() + .Where(mail => mail.Id == sharedId) + .ToListAsync(); + + insertedCopies.Should().HaveCount(2, "adding a new folder assignment should still clone one additional local row"); + insertedCopies.Select(mail => mail.FolderId).Should().BeEquivalentTo([_inboxFolder.Id, archiveFolder.Id]); + } + // ── Performance: 1 000 mails / ~70 threads ───────────────────────────────── /// @@ -315,12 +422,13 @@ public class MailFetchingTests : IAsyncLifetime Guid folderId, DateTime creationDate, string? threadId = null, - string fromAddress = "external@example.com") + string fromAddress = "external@example.com", + string? id = null) { return new MailCopy { UniqueId = Guid.NewGuid(), - Id = Guid.NewGuid().ToString(), + Id = id ?? Guid.NewGuid().ToString(), FileId = Guid.NewGuid(), FolderId = folderId, Subject = $"Subject {Guid.NewGuid():N}", @@ -336,7 +444,8 @@ public class MailFetchingTests : IAsyncLifetime private static MailListInitializationOptions BuildOptions( IEnumerable folders, bool createThreads = true, - int take = 0) + int take = 0, + bool deduplicateByServerId = false) { return new MailListInitializationOptions( Folders: folders, @@ -345,9 +454,42 @@ public class MailFetchingTests : IAsyncLifetime CreateThreads: createThreads, IsFocusedOnly: null, SearchQuery: null, + DeduplicateByServerId: deduplicateByServerId, Take: take); } + private async Task CreateAccountAsync(string name, string address) + { + var account = new MailAccount + { + Id = Guid.NewGuid(), + Name = name, + Address = address, + SenderName = name, + ProviderType = MailProviderType.IMAP4 + }; + + await _databaseService.Connection.InsertAsync(account, typeof(MailAccount)); + return account; + } + + private async Task CreateFolderAsync(MailAccount account, string name, string remoteFolderId, SpecialFolderType specialFolderType) + { + var folder = new MailItemFolder + { + Id = Guid.NewGuid(), + MailAccountId = account.Id, + FolderName = name, + RemoteFolderId = remoteFolderId, + SpecialFolderType = specialFolderType, + IsSystemFolder = true, + IsSynchronizationEnabled = true + }; + + await _databaseService.Connection.InsertAsync(folder, typeof(MailItemFolder)); + return folder; + } + /// /// Builds a MailService wired to real FolderService, AccountService, and ContactService /// all backed by the shared in-memory database, so the full SQL batch path is exercised. diff --git a/Wino.Core/Synchronizers/GmailSynchronizer.cs b/Wino.Core/Synchronizers/GmailSynchronizer.cs index 9c28831b..5c7a915c 100644 --- a/Wino.Core/Synchronizers/GmailSynchronizer.cs +++ b/Wino.Core/Synchronizers/GmailSynchronizer.cs @@ -1390,6 +1390,12 @@ public class GmailSynchronizer : WinoSynchronizer folder?.SpecialFolderType == SpecialFolderType.Archive || folder?.RemoteFolderId == ServiceConstants.ARCHIVE_LABEL_ID; + var distinctFolders = folders? + .Where(folder => folder != null) + .GroupBy(folder => folder.Id) + .Select(group => group.First()) + .ToList(); + var messageIds = new HashSet(StringComparer.Ordinal); async Task CollectMessageIdsAsync(UsersResource.MessagesResource.ListRequest request) @@ -1421,7 +1427,7 @@ public class GmailSynchronizer : WinoSynchronizer searchResults = []; - List searchResultFolderMailUids = []; + var distinctFolders = folders? + .Where(folder => folder != null) + .GroupBy(folder => folder.Id) + .Select(group => group.First()) + .ToList() ?? []; - foreach (var folder in folders) + HashSet searchResultFolderMailUids = new(StringComparer.Ordinal); + + foreach (var folder in distinctFolders) { if (folder is not MailItemFolder localFolder) continue; diff --git a/Wino.Core/Synchronizers/OutlookSynchronizer.cs b/Wino.Core/Synchronizers/OutlookSynchronizer.cs index d9f90295..f2668474 100644 --- a/Wino.Core/Synchronizers/OutlookSynchronizer.cs +++ b/Wino.Core/Synchronizers/OutlookSynchronizer.cs @@ -267,18 +267,32 @@ public class OutlookSynchronizer : WinoSynchronizer DownloadSearchResultMessageAsync(messageId, assignedFolder, existingMessageIds: null, cancellationToken); + + private async Task DownloadSearchResultMessageAsync(string messageId, + MailItemFolder assignedFolder, + ISet existingMessageIds, + CancellationToken cancellationToken = default) { if (string.IsNullOrWhiteSpace(messageId) || assignedFolder == null) return; // Online search can return the same message across repeated invocations/races. // Guard before network+MIME download and before database insert. - var existing = await _outlookChangeProcessor.AreMailsExistsAsync([messageId]).ConfigureAwait(false); - if (existing.Contains(messageId)) + if (existingMessageIds?.Contains(messageId) == true) { return; } + if (existingMessageIds == null) + { + var existing = await _outlookChangeProcessor.AreMailsExistsAsync([messageId]).ConfigureAwait(false); + if (existing.Contains(messageId)) + { + return; + } + } + Log.Information("Downloading search result message {messageId} for {Name} - {FolderName}", messageId, Account.Name, assignedFolder.FolderName); // Outlook message handling was a little strange. @@ -314,6 +328,8 @@ public class OutlookSynchronizer : WinoSynchronizer> SynchronizeFolderAsync(MailItemFolder folder, CancellationToken cancellationToken = default) @@ -2226,10 +2242,11 @@ public class OutlookSynchronizer : WinoSynchronizer(locallyExistingMails, StringComparer.Ordinal); // Find messages that are not downloaded yet. List messagesToDownload = []; - foreach (var id in messageIdsWithKnownFolder.Except(locallyExistingMails, StringComparer.Ordinal)) + foreach (var id in messageIdsWithKnownFolder.Except(existingMessageIds, StringComparer.Ordinal)) { if (messagesById.TryGetValue(id, out var message)) { @@ -2239,7 +2256,7 @@ public class OutlookSynchronizer : WinoSynchronizer folder != null) + .GroupBy(folder => folder.Id) + .Select(group => group.First()) + .ToList(); + + var foldersByAccount = distinctFolders .GroupBy(a => a.MailAccountId) .ToList(); @@ -1101,13 +1107,44 @@ public partial class MailListPageViewModel : MailBaseViewModel, var allResults = await Task.WhenAll(searchTasks).ConfigureAwait(false); - return allResults - .SelectMany(a => a) - .GroupBy(a => a.UniqueId) - .Select(a => a.First()) + var accountIdsByFolderId = distinctFolders.ToDictionary(folder => folder.Id, folder => folder.MailAccountId); + var preferredFolderIds = distinctFolders.Select(folder => folder.Id).ToHashSet(); + + return DeduplicateOnlineSearchResults(allResults.SelectMany(a => a), accountIdsByFolderId, preferredFolderIds); + } + + private static List DeduplicateOnlineSearchResults(IEnumerable results, + IReadOnlyDictionary accountIdsByFolderId, + ISet preferredFolderIds) + { + if (results == null) return []; + + return results + .Where(mail => mail != null) + .GroupBy(mail => (ResolveMailAccountId(mail, accountIdsByFolderId), ResolveSearchMailId(mail))) + .Select(group => group + .OrderByDescending(mail => preferredFolderIds.Contains(mail.FolderId)) + .ThenByDescending(mail => mail.CreationDate) + .ThenBy(mail => mail.FolderId) + .ThenBy(mail => mail.UniqueId) + .First()) .ToList(); } + private static Guid ResolveMailAccountId(MailCopy mail, IReadOnlyDictionary accountIdsByFolderId) + { + if (mail?.AssignedAccount != null) + return mail.AssignedAccount.Id; + + if (mail != null && accountIdsByFolderId.TryGetValue(mail.FolderId, out var accountId)) + return accountId; + + return Guid.Empty; + } + + private static string ResolveSearchMailId(MailCopy mail) + => string.IsNullOrWhiteSpace(mail?.Id) ? mail?.UniqueId.ToString("N") ?? string.Empty : mail.Id; + private async Task InitializeFolderAsync() { if (SelectedFilterOption == null || SelectedFolderPivot == null || SelectedSortingOption == null) @@ -1188,7 +1225,8 @@ public partial class MailListPageViewModel : MailBaseViewModel, SelectedFolderPivot.IsFocused, isDoingOnlineSearch ? string.Empty : SearchQuery, MailCollection.MailCopyIdHashSet, - onlineSearchItems); + onlineSearchItems, + DeduplicateByServerId: isDoingOnlineSearch); items = await _mailService.FetchMailsAsync(initializationOptions, cancellationToken).ConfigureAwait(false); diff --git a/Wino.Mail.WinUI/Views/WinoAppShell.xaml b/Wino.Mail.WinUI/Views/WinoAppShell.xaml index 8b1df082..786b1d7e 100644 --- a/Wino.Mail.WinUI/Views/WinoAppShell.xaml +++ b/Wino.Mail.WinUI/Views/WinoAppShell.xaml @@ -642,22 +642,27 @@ - - - - + + + + + diff --git a/Wino.Mail.WinUI/Views/WinoAppShell.xaml.cs b/Wino.Mail.WinUI/Views/WinoAppShell.xaml.cs index 2f06a996..99733842 100644 --- a/Wino.Mail.WinUI/Views/WinoAppShell.xaml.cs +++ b/Wino.Mail.WinUI/Views/WinoAppShell.xaml.cs @@ -29,6 +29,7 @@ using Wino.Mail.ViewModels.Data; using Wino.Mail.WinUI.ViewModels; using Wino.Mail.WinUI.Controls; using Wino.Mail.WinUI.Helpers; +using Wino.Helpers; using Wino.MenuFlyouts; using Wino.MenuFlyouts.Context; using Wino.Messaging.Client.Accounts; @@ -50,10 +51,15 @@ public sealed partial class WinoAppShell : Views.Abstract.WinoAppShellAbstract, { private const string StateDefaultShellContent = "DefaultShellContentState"; private const string StateEventDetailsContent = "EventDetailsContentState"; + private const int PaneCustomContentRowIndex = 4; + private const int PaneItemsContainerRowIndex = 6; private WinoApplicationMode? _activeMode; private bool _isSyncingNavigationViewSelection; private bool _isSynchronizingVisibleDateRangeCalendar; private bool _isPreparedForWindowClose; + private Grid? _paneContentGrid; + private RowDefinition? _paneCustomContentRowDefinition; + private RowDefinition? _paneItemsContainerRowDefinition; public WinoAppShell() { @@ -681,6 +687,22 @@ public sealed partial class WinoAppShell : Views.Abstract.WinoAppShellAbstract, private void UpdateNavigationPaneLayout(NavigationViewDisplayMode displayMode) { + EnsureNavigationPaneLayoutParts(); + + bool shouldStretchCustomPane = displayMode == NavigationViewDisplayMode.Expanded + && navigationView.IsPaneOpen + && (ViewModel.IsCalendarMode || ViewModel.IsContactsMode); + + if (_paneCustomContentRowDefinition != null && _paneItemsContainerRowDefinition != null) + { + _paneCustomContentRowDefinition.Height = shouldStretchCustomPane + ? new GridLength(1, GridUnitType.Star) + : GridLength.Auto; + _paneItemsContainerRowDefinition.Height = shouldStretchCustomPane + ? GridLength.Auto + : new GridLength(1, GridUnitType.Star); + } + if (displayMode == NavigationViewDisplayMode.Expanded && navigationView.IsPaneOpen) { if (ViewModel.IsCalendarMode) @@ -710,6 +732,17 @@ public sealed partial class WinoAppShell : Views.Abstract.WinoAppShellAbstract, : new Thickness(0); } + private void EnsureNavigationPaneLayoutParts() + { + _paneContentGrid ??= WinoVisualTreeHelper.GetChildObject(navigationView, "PaneContentGrid"); + + if (_paneContentGrid == null || _paneContentGrid.RowDefinitions.Count <= PaneItemsContainerRowIndex) + return; + + _paneCustomContentRowDefinition ??= _paneContentGrid.RowDefinitions[PaneCustomContentRowIndex]; + _paneItemsContainerRowDefinition ??= _paneContentGrid.RowDefinitions[PaneItemsContainerRowIndex]; + } + private async void OnPreviewKeyDown(object sender, KeyRoutedEventArgs e) { if (e.KeyStatus.RepeatCount > 1 || ShouldIgnoreShortcut()) diff --git a/Wino.Services/MailService.cs b/Wino.Services/MailService.cs index a5bd21f5..85fb0c02 100644 --- a/Wino.Services/MailService.cs +++ b/Wino.Services/MailService.cs @@ -246,11 +246,13 @@ public class MailService : BaseDatabaseService, IMailService private static List ApplyOptionsToPreFetchedMails(MailListInitializationOptions options) { var allowedFolderIds = options.Folders.Select(f => f.Id).ToHashSet(); + var accountIdsByFolderId = options.Folders + .Where(folder => folder != null) + .GroupBy(folder => folder.Id) + .ToDictionary(group => group.Key, group => group.First().MailAccountId); IEnumerable query = options.PreFetchMailCopies - .Where(m => m != null && allowedFolderIds.Contains(m.FolderId)) - .GroupBy(m => m.UniqueId) - .Select(g => g.First()); + .Where(m => m != null && allowedFolderIds.Contains(m.FolderId)); switch (options.FilterType) { @@ -285,6 +287,19 @@ public class MailService : BaseDatabaseService, IMailService query = query.Where(m => !options.ExistingUniqueIds.ContainsKey(m.UniqueId)); } + query = options.DeduplicateByServerId + ? query + .GroupBy(m => (ResolveMailAccountId(m, accountIdsByFolderId), ResolveServerMailId(m))) + .Select(group => group + .OrderByDescending(m => allowedFolderIds.Contains(m.FolderId)) + .ThenByDescending(m => m.CreationDate) + .ThenBy(m => m.FolderId) + .ThenBy(m => m.UniqueId) + .First()) + : query + .GroupBy(m => m.UniqueId) + .Select(group => group.First()); + query = options.SortingOptionType switch { SortingOptionType.Sender => query.OrderBy(m => m.FromName).ThenByDescending(m => m.CreationDate), @@ -304,6 +319,20 @@ public class MailService : BaseDatabaseService, IMailService return query.ToList(); } + private static Guid ResolveMailAccountId(MailCopy mail, IReadOnlyDictionary accountIdsByFolderId) + { + if (mail?.AssignedAccount != null) + return mail.AssignedAccount.Id; + + if (mail != null && accountIdsByFolderId.TryGetValue(mail.FolderId, out var accountId)) + return accountId; + + return Guid.Empty; + } + + private static string ResolveServerMailId(MailCopy mail) + => string.IsNullOrWhiteSpace(mail?.Id) ? mail?.UniqueId.ToString("N") ?? string.Empty : mail.Id; + public async Task> FetchMailsAsync(MailListInitializationOptions options, CancellationToken cancellationToken = default) { List mails; @@ -777,6 +806,13 @@ public class MailService : BaseDatabaseService, IMailService return; } + if (await IsMailExistsAsync(mailCopyId, localFolder.Id).ConfigureAwait(false)) + { + _logger.Debug("Skipping assignment creation for {MailCopyId} because folder {FolderId} already has a local copy.", + mailCopyId, localFolder.Id); + return; + } + var mailCopy = await GetSingleMailItemWithoutFolderAssignmentAsync(mailCopyId); if (mailCopy == null)