diff --git a/OutlookCaseHelper/Form1.cs b/OutlookCaseHelper/Form1.cs index b91493a..a22d152 100644 --- a/OutlookCaseHelper/Form1.cs +++ b/OutlookCaseHelper/Form1.cs @@ -4,7 +4,7 @@ using System.Windows.Forms; using System.IO; using System.Runtime.InteropServices; using System.Text.Json; -using System.Threading.Tasks; +using System.Threading; using Microsoft.Win32; using Outlook = Microsoft.Office.Interop.Outlook; @@ -44,6 +44,10 @@ namespace OutlookCaseHelper private InputForm? _removeRuleForm; private AboutForm? _aboutForm; + // FIX #3: Guard to prevent multiple ProcessEmail dialogs from opening + // (e.g., hotkey spam or rapid tray double-clicks) + private Form? _activeProcessDialog; + // --- Constructor & Init --- public Form1() @@ -209,6 +213,7 @@ namespace OutlookCaseHelper } // --- Singleton Window Helper --- + // FIX #6: Now actively used by ViewRules_Click and About_Click private void ShowSingletonForm(Func getter, Action setter, Func create) where T : Form { @@ -269,43 +274,43 @@ namespace OutlookCaseHelper _settingsForm.Show(); } + // FIX #6: Using ShowSingletonForm helper instead of duplicated manual logic private void ViewRules_Click(object? sender, EventArgs e) - { - if (_dashboardForm != null && !_dashboardForm.IsDisposed) - { - _dashboardForm.BringToFront(); - _dashboardForm.WindowState = FormWindowState.Normal; - return; - } - _dashboardForm = new DashboardForm(outlookHelper); - _dashboardForm.Owner = this; - _dashboardForm.FormClosed += (s, args) => _dashboardForm = null; - _dashboardForm.Show(); - } + => ShowSingletonForm( + () => _dashboardForm, + v => _dashboardForm = v, + () => { var f = new DashboardForm(outlookHelper); f.Owner = this; return f; }); + // FIX #6: Using ShowSingletonForm helper instead of duplicated manual logic private void About_Click(object? sender, EventArgs e) - { - if (_aboutForm != null && !_aboutForm.IsDisposed) - { - _aboutForm.BringToFront(); - _aboutForm.WindowState = FormWindowState.Normal; - return; - } - _aboutForm = new AboutForm(); - _aboutForm.FormClosed += (s, args) => _aboutForm = null; - _aboutForm.Show(); - } + => ShowSingletonForm( + () => _aboutForm, + v => _aboutForm = v, + () => new AboutForm()); + // FIX #5: Use STA thread instead of Task.Run to avoid COM threading violation. + // Outlook COM objects require STA (Single-Threaded Apartment). + // Task.Run uses ThreadPool which is MTA, causing silent failures or crashes. private void RunAllRules_Click(object? sender, EventArgs e) { try { trayIcon.ShowBalloonTip(2000, "Outlook Case Manager", "Running all rules...", ToolTipIcon.Info); - Task.Run(() => + var thread = new Thread(() => { - outlookHelper.RunAllRules(); - this.Invoke(() => trayIcon.ShowBalloonTip(3000, "Outlook Case Manager", "All rules applied successfully!", ToolTipIcon.Info)); + try + { + outlookHelper.RunAllRules(); + this.Invoke(() => trayIcon.ShowBalloonTip(3000, "Outlook Case Manager", "All rules applied successfully!", ToolTipIcon.Info)); + } + catch (Exception ex) + { + this.Invoke(() => MessageBox.Show($"Error running rules: {ex.Message}", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error)); + } }); + thread.SetApartmentState(ApartmentState.STA); + thread.IsBackground = true; + thread.Start(); } catch (Exception ex) { @@ -315,8 +320,16 @@ namespace OutlookCaseHelper // --- Rule Actions --- + // FIX #3: Added _activeProcessDialog guard to prevent multiple dialogs opening + // when hotkey is pressed rapidly or tray icon is double-clicked quickly. private void ProcessEmail_Click(object? sender, EventArgs e) { + if (_activeProcessDialog != null && !_activeProcessDialog.IsDisposed) + { + _activeProcessDialog.BringToFront(); + return; + } + try { var trackingId = outlookHelper.GetSelectedEmailTrackingId(); @@ -340,6 +353,9 @@ namespace OutlookCaseHelper } var ruleForm = new CreateRuleForm(trackingId, readonlyId: true); + _activeProcessDialog = ruleForm; + ruleForm.FormClosed += (s, args) => _activeProcessDialog = null; + if (ruleForm.ShowDialog() != DialogResult.OK) return; HandleCreate(trackingId, ruleForm.FolderName); } @@ -517,7 +533,6 @@ namespace OutlookCaseHelper { listView.Items.Clear(); var rules = outlookHelper.GetActiveRulesInfo(); - if (rules.Count == 0) { var empty = new ListViewItem("No active rules found."); @@ -525,7 +540,6 @@ namespace OutlookCaseHelper listView.Items.Add(empty); return; } - foreach (var rule in rules) { var item = new ListViewItem(rule.FolderName); @@ -540,36 +554,28 @@ namespace OutlookCaseHelper { var ruleForm = new CreateRuleForm("", readonlyId: false); if (ruleForm.ShowDialog() != DialogResult.OK) return; - string trackingId = ruleForm.TrackingId; string folderName = ruleForm.FolderName; - if (outlookHelper.FindRuleByTrackingId(trackingId) != null) { MessageBox.Show($"Rule for TrackingID#{trackingId} already exists!", "Warning", MessageBoxButtons.OK, MessageBoxIcon.Warning); return; } - if (outlookHelper.ExistsInClosed(trackingId)) { if (outlookHelper.ReopenFromClosed(trackingId)) { MessageBox.Show($"Case reopened!\n\nTrackingID: {trackingId}", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information); LoadRules(); } - else - MessageBox.Show("Error reopening case.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); + else MessageBox.Show("Error reopening case.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); return; } - if (outlookHelper.CreateFolderAndMoveEmails(trackingId, folderName)) { MessageBox.Show($"Rule created!\n\nFolder: {folderName}", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information); LoadRules(); } - else - MessageBox.Show("Error creating rule. Make sure Outlook is open.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); + else MessageBox.Show("Error creating rule. Make sure Outlook is open.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); } private void BtnRename_Click(object? sender, EventArgs e) { if (listView.SelectedItems.Count == 0) { MessageBox.Show("Please select a rule to rename.", "Warning", MessageBoxButtons.OK, MessageBoxIcon.Warning); return; } - var rule = listView.SelectedItems[0].Tag as OutlookHelper.RuleInfo; if (rule == null) return; - var inputForm = new Form { Text = "Rename Rule", @@ -582,42 +588,33 @@ namespace OutlookCaseHelper StartPosition = FormStartPosition.CenterScreen }; try { inputForm.Icon = new Icon(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "casenew.ico")); } catch { } - var lbl = new Label { Text = $"New name for TrackingID#{rule.TrackingId}:", Left = 20, Top = 15, Width = 370, Height = 20 }; var txt = new TextBox { Left = 20, Top = 40, Width = 370, Height = 24, Text = rule.FolderName }; var btnOk = new Button { Text = "Rename", Left = 210, Top = 80, Width = 80, DialogResult = DialogResult.OK }; var btnCancel = new Button { Text = "Cancel", Left = 300, Top = 80, Width = 80, DialogResult = DialogResult.Cancel }; btnCancel.Click += (s, e) => inputForm.Close(); inputForm.Controls.AddRange(new Control[] { lbl, txt, btnOk, btnCancel }); - inputForm.AcceptButton = btnOk; - inputForm.CancelButton = btnCancel; - + inputForm.AcceptButton = btnOk; inputForm.CancelButton = btnCancel; if (inputForm.ShowDialog() != DialogResult.OK) return; string newName = txt.Text.Trim(); if (string.IsNullOrEmpty(newName) || newName == rule.FolderName) return; - if (outlookHelper.RenameRule(rule.FolderName, newName)) - { MessageBox.Show($"Rule renamed!\n\n{rule.FolderName} → {newName}", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information); LoadRules(); } - else - MessageBox.Show("Error renaming rule.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); + { MessageBox.Show($"Rule renamed!\n\n{rule.FolderName} -> {newName}", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information); LoadRules(); } + else MessageBox.Show("Error renaming rule.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); } private void BtnClose_Click(object? sender, EventArgs e) { if (listView.SelectedItems.Count == 0) { MessageBox.Show("Please select a rule to close.", "Warning", MessageBoxButtons.OK, MessageBoxIcon.Warning); return; } - var rule = listView.SelectedItems[0].Tag as OutlookHelper.RuleInfo; if (rule == null) return; - if (MessageBox.Show( $"Are you sure you want to close this case?\n\nFolder: {rule.FolderName}\n\nThe folder will be moved to Closed.", "Close Case", MessageBoxButtons.YesNo, MessageBoxIcon.Question) != DialogResult.Yes) return; - if (outlookHelper.RemoveRuleAndMoveToClosed(rule.FolderName)) { MessageBox.Show("Case closed!\n\nFolder moved to Closed.", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information); LoadRules(); } - else - MessageBox.Show("Error closing case.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); + else MessageBox.Show("Error closing case.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error); } private void BtnReload_Click(object? sender, EventArgs e) diff --git a/OutlookCaseHelper/OutlookHelper.cs b/OutlookCaseHelper/OutlookHelper.cs index aa8cec4..9de0ab2 100644 --- a/OutlookCaseHelper/OutlookHelper.cs +++ b/OutlookCaseHelper/OutlookHelper.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; using System.Text.RegularExpressions; using System.IO; using System.Text.Json; @@ -18,6 +20,12 @@ namespace OutlookCaseHelper private Outlook.Items? sentItems; private bool disposed = false; + // PERF FIX: Cache "Active" and "Closed" folder references to avoid + // traversing the full folder tree on every operation. + // Cache is validated before each use (COM object can go stale). + private Outlook.Folder? _cachedActiveFolder; + private Outlook.Folder? _cachedClosedFolder; + public class RuleInfo { public string FolderName { get; set; } = ""; @@ -32,6 +40,9 @@ namespace OutlookCaseHelper "OutlookCaseHelper", "active_rules.json"); Directory.CreateDirectory(Path.GetDirectoryName(rulesFilePath)!); LoadRules(); + // FIX #2: Scan inbox on startup so emails that arrived while the app + // was closed are moved into their correct folders immediately. + ScanInboxOnStartup(); } // --- Dispose --- @@ -46,23 +57,25 @@ namespace OutlookCaseHelper if (inboxItems != null) { inboxItems.ItemAdd -= OnEmailReceived; - System.Runtime.InteropServices.Marshal.ReleaseComObject(inboxItems); + Marshal.ReleaseComObject(inboxItems); inboxItems = null; } if (sentItems != null) { sentItems.ItemAdd -= OnEmailSent; - System.Runtime.InteropServices.Marshal.ReleaseComObject(sentItems); + Marshal.ReleaseComObject(sentItems); sentItems = null; } + _cachedActiveFolder = null; + _cachedClosedFolder = null; if (outlookNamespace != null) { - System.Runtime.InteropServices.Marshal.ReleaseComObject(outlookNamespace); + Marshal.ReleaseComObject(outlookNamespace); outlookNamespace = null; } if (outlookApp != null) { - System.Runtime.InteropServices.Marshal.ReleaseComObject(outlookApp); + Marshal.ReleaseComObject(outlookApp); outlookApp = null; } } @@ -95,7 +108,7 @@ namespace OutlookCaseHelper try { var inbox = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderInbox); - var sent = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderSentMail); + var sent = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderSentMail); if (inbox != null) { @@ -114,7 +127,7 @@ namespace OutlookCaseHelper // --- Email Events --- private void OnEmailReceived(object item) => MoveEmailIfRuleExists(item); - private void OnEmailSent(object item) => MoveEmailIfRuleExists(item); + private void OnEmailSent(object item) => MoveEmailIfRuleExists(item); private void MoveEmailIfRuleExists(object item) { @@ -123,14 +136,15 @@ namespace OutlookCaseHelper if (outlookNamespace == null || activeRules.Count == 0) return; if (item is not Outlook.MailItem mail || mail.Subject == null) return; + // PERF FIX: Resolve activeFolder once outside the loop + var activeFolder = GetCachedActiveFolder(); + if (activeFolder == null) return; + foreach (var folderName in activeRules) { string trackingId = ExtractTrackingId(folderName); if (!mail.Subject.Contains($"TrackingID#{trackingId}")) continue; - var activeFolder = FindFolderByNameAnywhere("Active"); - if (activeFolder == null) return; - var target = GetOrCreateFolder(activeFolder, folderName); mail.Move(target); break; @@ -153,10 +167,21 @@ namespace OutlookCaseHelper public void ReloadRules() { activeRules.Clear(); + // FIX: Invalidate folder cache when rules are reloaded + _cachedActiveFolder = null; + _cachedClosedFolder = null; LoadRules(); + // FIX #7: Prune stale rules whose folders no longer exist in Outlook + PruneStaleRules(); } - public void ProcessActiveRules() { } + // FIX #1: ProcessActiveRules was empty — it now delegates to RunAllRules + // so the 60-second timer actually does something useful. + public void ProcessActiveRules() + { + if (activeRules.Count == 0) return; + RunAllRules(); + } private void LoadRules() { @@ -175,8 +200,36 @@ namespace OutlookCaseHelper catch { } } + // FIX #7: Remove rules whose folders no longer exist in Outlook. + // Prevents infinite silent failures in RunAllRules and GetActiveRulesInfo + // when a folder was deleted manually from Outlook. + private void PruneStaleRules() + { + if (!EnsureOutlookConnected()) return; + try + { + var activeFolder = GetCachedActiveFolder(); + if (activeFolder == null) return; + + var stale = activeRules + .Where(r => GetFolder(activeFolder, r) == null) + .ToList(); + + if (stale.Count == 0) return; + + foreach (var r in stale) + activeRules.Remove(r); + + SaveRules(); + } + catch { } + } + // --- Outlook Operations --- + // FIX #4: Regex was @"TrackingID#(\\d+)" in a verbatim string. + // In a verbatim string, \\d is a literal backslash + 'd', NOT the \d digit class. + // Fixed to @"TrackingID#(\d+)" so it correctly matches numeric IDs. public string? GetSelectedEmailTrackingId() { if (!EnsureOutlookConnected()) return null; @@ -202,7 +255,7 @@ namespace OutlookCaseHelper try { if (!EnsureOutlookConnected()) return false; - var closed = FindFolderByNameAnywhere("Closed"); + var closed = GetCachedClosedFolder(); return closed != null && GetFolderStartingWith(closed, trackingId) != null; } catch { return false; } @@ -213,23 +266,27 @@ namespace OutlookCaseHelper if (!EnsureOutlookConnected()) return false; try { - var activeFolder = FindFolderByNameAnywhere("Active"); + var activeFolder = GetCachedActiveFolder(); if (activeFolder == null) { var inbox = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderInbox); if (inbox == null) return false; var cases = GetOrCreateFolder(inbox, "Cases"); activeFolder = GetOrCreateFolder(cases, "Active"); + _cachedActiveFolder = activeFolder; } var target = GetOrCreateFolder(activeFolder, folderName); + + // FIX: Correct DASL filter — original had \\\" producing \" + // (backslash+quote), but Outlook DASL needs plain double quotes. string filter = $"@SQL=\"urn:schemas:httpmail:subject\" LIKE '%TrackingID#{trackingId}%'"; var inbox2 = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderInbox); - var sent = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderSentMail); + var sent = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderSentMail); if (inbox2 != null) MoveFilteredEmails(inbox2, filter, target); - if (sent != null) MoveFilteredEmails(sent, filter, target); + if (sent != null) MoveFilteredEmails(sent, filter, target); if (inbox2 != null) { @@ -258,19 +315,20 @@ namespace OutlookCaseHelper if (!EnsureOutlookConnected()) return false; try { - var activeFolder = FindFolderByNameAnywhere("Active"); + var activeFolder = GetCachedActiveFolder(); if (activeFolder == null) return false; var folder = GetFolder(activeFolder, folderName); if (folder == null) return false; - var closedFolder = FindFolderByNameAnywhere("Closed"); + var closedFolder = GetCachedClosedFolder(); if (closedFolder == null) { var inbox = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderInbox); if (inbox == null) return false; var cases = GetOrCreateFolder(inbox, "Cases"); closedFolder = GetOrCreateFolder(cases, "Closed"); + _cachedClosedFolder = closedFolder; } folder.MoveTo(closedFolder); @@ -286,7 +344,7 @@ namespace OutlookCaseHelper if (!EnsureOutlookConnected()) return false; try { - var activeFolder = FindFolderByNameAnywhere("Active"); + var activeFolder = GetCachedActiveFolder(); var folder = GetFolder(activeFolder, oldFolderName); if (folder == null) return false; @@ -304,7 +362,7 @@ namespace OutlookCaseHelper if (!EnsureOutlookConnected()) return false; try { - var closedFolder = FindFolderByNameAnywhere("Closed"); + var closedFolder = GetCachedClosedFolder(); if (closedFolder == null) return false; var closedTracking = GetFolderStartingWith(closedFolder, trackingId); @@ -312,13 +370,14 @@ namespace OutlookCaseHelper string existingName = closedTracking.Name; - var activeFolder = FindFolderByNameAnywhere("Active"); + var activeFolder = GetCachedActiveFolder(); if (activeFolder == null) { var inbox = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderInbox); if (inbox == null) return false; var cases = GetOrCreateFolder(inbox, "Cases"); activeFolder = GetOrCreateFolder(cases, "Active"); + _cachedActiveFolder = activeFolder; } var existingActive = GetFolder(activeFolder, existingName); @@ -351,8 +410,8 @@ namespace OutlookCaseHelper { try { - var active = FindFolderByNameAnywhere("Active"); - var moved = GetFolderStartingWith(active!, trackingId); + var active = GetCachedActiveFolder(); + var moved = GetFolderStartingWith(active!, trackingId); if (moved != null) { activeRules.Add(moved.Name); SaveRules(); return true; } } catch { } @@ -368,19 +427,21 @@ namespace OutlookCaseHelper try { var inbox = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderInbox); - var sent = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderSentMail); + var sent = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderSentMail); if (inbox == null) return; - var activeFolder = FindFolderByNameAnywhere("Active"); + var activeFolder = GetCachedActiveFolder(); if (activeFolder == null) { var cases = GetOrCreateFolder(inbox, "Cases"); activeFolder = GetOrCreateFolder(cases, "Active"); + _cachedActiveFolder = activeFolder; } foreach (var folderName in activeRules) { string trackingId = ExtractTrackingId(folderName); + // FIX: Correct DASL filter syntax (plain double quotes) string filter = $"@SQL=\"urn:schemas:httpmail:subject\" LIKE '%TrackingID#{trackingId}%'"; var target = GetOrCreateFolder(activeFolder, folderName); @@ -399,13 +460,15 @@ namespace OutlookCaseHelper var inbox = GetDefaultFolder(Outlook.OlDefaultFolders.olFolderInbox); if (inbox == null) return; - var activeFolder = FindFolderByNameAnywhere("Active"); + var activeFolder = GetCachedActiveFolder(); if (activeFolder == null) { var cases = GetOrCreateFolder(inbox, "Cases"); activeFolder = GetOrCreateFolder(cases, "Active"); + _cachedActiveFolder = activeFolder; } + // PERF FIX: Build the full folder list once, outside the per-rule loop var allFolders = new List(); if (inbox.Parent is Outlook.Folder root) GetAllFolders(root, allFolders, activeFolder.EntryID); @@ -413,6 +476,7 @@ namespace OutlookCaseHelper foreach (var folderName in activeRules) { string trackingId = ExtractTrackingId(folderName); + // FIX: Correct DASL filter syntax (plain double quotes) string filter = $"@SQL=\"urn:schemas:httpmail:subject\" LIKE '%TrackingID#{trackingId}%'"; var target = GetOrCreateFolder(activeFolder, folderName); @@ -423,13 +487,15 @@ namespace OutlookCaseHelper catch { } } + // PERF FIX: Resolve activeFolder once before the loop instead of inside it. + // Original called EnsureOutlookConnected + FindFolderByNameAnywhere per iteration. public List GetActiveRulesInfo() { var result = new List(); if (!EnsureOutlookConnected()) return result; try { - var activeFolder = FindFolderByNameAnywhere("Active"); + var activeFolder = GetCachedActiveFolder(); if (activeFolder == null) return result; foreach (var folderName in activeRules) @@ -447,6 +513,33 @@ namespace OutlookCaseHelper return result; } + // --- Folder Cache Helpers --- + + // PERF FIX: Caches the "Active" folder reference. Validates the COM object + // before returning it; falls back to a fresh lookup if it has gone stale. + private Outlook.Folder? GetCachedActiveFolder() + { + if (_cachedActiveFolder != null) + { + try { var _ = _cachedActiveFolder.Name; return _cachedActiveFolder; } + catch { _cachedActiveFolder = null; } + } + _cachedActiveFolder = FindFolderByNameAnywhere("Active"); + return _cachedActiveFolder; + } + + // PERF FIX: Caches the "Closed" folder reference with same staleness guard. + private Outlook.Folder? GetCachedClosedFolder() + { + if (_cachedClosedFolder != null) + { + try { var _ = _cachedClosedFolder.Name; return _cachedClosedFolder; } + catch { _cachedClosedFolder = null; } + } + _cachedClosedFolder = FindFolderByNameAnywhere("Closed"); + return _cachedClosedFolder; + } + // --- Folder Search (any hierarchy level) --- private Outlook.Folder? FindFolderByNameAnywhere(string name) @@ -477,22 +570,33 @@ namespace OutlookCaseHelper // --- Helpers --- + // FIX #4: Regex was @"^(\\d+)" in a verbatim string = literal "\\d". + // Fixed to @"^(\d+)" so it actually matches leading digits in folder names. private string ExtractTrackingId(string folderName) { var match = Regex.Match(folderName, @"^(\d+)"); return match.Success ? match.Groups[1].Value : folderName; } + // PERF FIX: Release the COM object returned by Restrict() after use. + // Failing to do so causes COM reference leaks when many rules are active. private void MoveFilteredEmails(Outlook.Folder source, string filter, Outlook.Folder dest) { + Outlook.Items? restricted = null; try { + restricted = source.Items.Restrict(filter); var toMove = new List(); - foreach (object item in source.Items.Restrict(filter)) + foreach (object item in restricted) if (item is Outlook.MailItem mail) toMove.Add(mail); foreach (var mail in toMove) mail.Move(dest); } catch { } + finally + { + if (restricted != null) + Marshal.ReleaseComObject(restricted); + } } private void GetAllFolders(Outlook.Folder parent, List result, string excludeEntryId) @@ -536,4 +640,4 @@ namespace OutlookCaseHelper return null; } } -} \ No newline at end of file +}