Compare commits

...

1 Commits

Author SHA1 Message Date
wellington c680bb5571 General error: cancel, multiple popup and startup rule to see the folders
Enhance OutlookCaseHelper with performance and stability fixes

Improved threading for Outlook COM objects to use STA, preventing threading violations. Added guards to manage dialog instances and introduced a helper method to reduce code duplication for form handling. Enhanced error handling for better user feedback and implemented caching for folder references to optimize performance. Refined rule management logic, fixed regex for tracking ID extraction, and ensured proper COM object management to prevent memory leaks. General code cleanup for improved readability and maintainability.
2026-03-30 19:58:15 +01:00
2 changed files with 180 additions and 79 deletions
+47 -50
View File
@@ -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<T>(Func<T?> getter, Action<T?> setter, Func<T> 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(() =>
{
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)
+124 -20
View File
@@ -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;
}
}
@@ -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,16 +266,20 @@ 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);
@@ -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,7 +410,7 @@ namespace OutlookCaseHelper
{
try
{
var active = FindFolderByNameAnywhere("Active");
var active = GetCachedActiveFolder();
var moved = GetFolderStartingWith(active!, trackingId);
if (moved != null) { activeRules.Add(moved.Name); SaveRules(); return true; }
}
@@ -371,16 +430,18 @@ namespace OutlookCaseHelper
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<Outlook.Folder>();
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<RuleInfo> GetActiveRulesInfo()
{
var result = new List<RuleInfo>();
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<Outlook.MailItem>();
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<Outlook.Folder> result, string excludeEntryId)