From 4f4e292ff184fd0a371a7cc5deba2e1a98756ec2 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Tue, 25 Jun 2019 23:28:38 -0700 Subject: [PATCH 01/13] Initial working version based on CsQuery --- Mail2Bug/Email/EmailBodyProcessingUtils.cs | 73 +++++++++++++++---- Mail2Bug/Mail2Bug.csproj | 3 + .../SimpleBugStrategy.cs | 7 +- .../WorkItemManagement/IWorkItemManager.cs | 3 +- .../WorkItemManagement/TFSWorkItemManager.cs | 4 +- .../WorkItemManagement/WorkItemManagerMock.cs | 2 +- Mail2Bug/packages.config | 1 + 7 files changed, 72 insertions(+), 21 deletions(-) diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index cbf9f49..7f7e1e3 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -3,15 +3,21 @@ using System.Linq; using System.Text; using System.Text.RegularExpressions; +using CsQuery; namespace Mail2Bug.Email { public class EmailBodyProcessingUtils { public static string GetLastMessageText(IIncomingEmailMessage message) + { + return message.IsHtmlBody ? GetLastMessageText_Html(message.RawBody) : GetLastMessageText_PlainText(message); + } + + private static string GetLastMessageText_PlainText(IIncomingEmailMessage message) { var lastMessage = new StringBuilder(); - lastMessage.Append(message.PlainTextBody); + lastMessage.Append(message.RawBody); var next = GetReplySeperatorIndex(lastMessage.ToString()); @@ -23,6 +29,48 @@ public static string GetLastMessageText(IIncomingEmailMessage message) return lastMessage.ToString(); } + public static string GetLastMessageText_Html(string rawBody) + { + CQ dom = rawBody; + + foreach (IDomObject element in dom["*"]) + { + if (!element.ChildElements.Any() && !string.IsNullOrWhiteSpace(element.InnerText)) + { + var separatorIndex = IndexOfAny(element.InnerText, MessageBorderMarkers); + if (separatorIndex.HasValue) + { + element.InnerText = element.InnerText.Substring(0, separatorIndex.Value); + RemoveSubsequent(element); + break; + } + } + } + + return dom.Render(); + } + + private static void RemoveSubsequent(IDomObject element) + { + IDomObject nextNode; + while ((nextNode = element.NextSibling) != null) + { + nextNode.Remove(); + } + + if (element.ParentNode is IDomObject parentElement) + { + RemoveSubsequent(parentElement); + } + } + + private static readonly string[] MessageBorderMarkers = + { + "_____", + "-----Original Message", + "From:", + }; + /// /// Get the seperate denotation between reply content and original content. /// @@ -30,20 +78,17 @@ public static string GetLastMessageText(IIncomingEmailMessage message) /// private static int GetReplySeperatorIndex(string description) { - var indices = new List - { - description.IndexOf("_____", StringComparison.Ordinal), - description.IndexOf("-----Original Message", StringComparison.Ordinal), - description.IndexOf("From:", StringComparison.Ordinal) - }; - - indices.RemoveAll(x => x < 0); - if (indices.Count == 0) - { - return description.Length; - } + return IndexOfAny(description, MessageBorderMarkers) ?? description.Length; + } - return indices.Min(); + private static int? IndexOfAny(string text, IEnumerable searchTerms) + { + return searchTerms + .Select(search => text.IndexOf(search, StringComparison.Ordinal)) + .Where(index => index >= 0) + .OrderBy(index => index) + .Cast() + .FirstOrDefault(); } public static string ConvertHtmlMessageToPlainText(string text) diff --git a/Mail2Bug/Mail2Bug.csproj b/Mail2Bug/Mail2Bug.csproj index 270c8e0..7a59c63 100644 --- a/Mail2Bug/Mail2Bug.csproj +++ b/Mail2Bug/Mail2Bug.csproj @@ -36,6 +36,9 @@ 4 + + ..\packages\CsQuery.1.3.4\lib\net40\CsQuery.dll + ..\packages\Hyak.Common.1.0.2\lib\net45\Hyak.Common.dll True diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index 86c640b..ad60396 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -118,7 +118,7 @@ private void TryApplyFieldOverrides(Dictionary overrides, int wo try { Logger.DebugFormat("Overrides found. Calling 'ModifyWorkItem'"); - _workItemManager.ModifyWorkItem(workItemId, "", overrides); + _workItemManager.ModifyWorkItem(workItemId, "", false, overrides); } catch (Exception ex) { @@ -139,10 +139,11 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) workItemUpdates["Changed By"] = resolver.Sender; } + string lastMessageText = message.GetLastMessageText(); if (_config.WorkItemSettings.ApplyOverridesDuringUpdate) { var extractor = new OverridesExtractor(_config); - var overrides = extractor.GetOverrides(message.GetLastMessageText()); + var overrides = extractor.GetOverrides(lastMessageText); Logger.DebugFormat("Found {0} overrides for update message", overrides.Count); @@ -150,7 +151,7 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) } // Construct the text to be appended - _workItemManager.ModifyWorkItem(workItemId, message.GetLastMessageText(), workItemUpdates); + _workItemManager.ModifyWorkItem(workItemId, lastMessageText, message.IsHtmlBody, workItemUpdates); ProcessAttachments(message, workItemId); diff --git a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs index 10df5e3..40d5be7 100644 --- a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs @@ -17,8 +17,9 @@ public interface IWorkItemManager /// The ID of the bug to modify /// Comment to add to description + /// /// List of fields to change - void ModifyWorkItem(int workItemId, string comment, Dictionary values); + void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values); INameResolver GetNameResolver(); diff --git a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs index 0c68071..ec274b5 100644 --- a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs @@ -303,7 +303,7 @@ public int CreateWorkItem(Dictionary values) /// The ID of the work item to modify /// Comment to add to description /// List of fields to change - public void ModifyWorkItem(int workItemId, string comment, Dictionary values) + public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values) { if (workItemId <= 0) return; @@ -311,7 +311,7 @@ public void ModifyWorkItem(int workItemId, string comment, Dictionary"); + workItem.History = commentIsHtml ? comment : comment.Replace("\n", "
"); foreach (var key in values.Keys) { TryApplyFieldValue(workItem, key, values[key]); diff --git a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs index 623d273..e450f65 100644 --- a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs +++ b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs @@ -80,7 +80,7 @@ public int CreateWorkItem(Dictionary values) return id; } - public void ModifyWorkItem(int workItemId, string comment, Dictionary values) + public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values) { if (ThrowOnModifyBug != null) throw ThrowOnModifyBug; diff --git a/Mail2Bug/packages.config b/Mail2Bug/packages.config index 2bb9b80..a8c4648 100644 --- a/Mail2Bug/packages.config +++ b/Mail2Bug/packages.config @@ -1,5 +1,6 @@  + From ac05f09a55ff3b8cef1401bc294769d5e0d2c7d2 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Tue, 25 Jun 2019 23:38:29 -0700 Subject: [PATCH 02/13] revert bad change --- Mail2Bug/Email/EmailBodyProcessingUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index 7f7e1e3..5497f7f 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -17,7 +17,7 @@ public static string GetLastMessageText(IIncomingEmailMessage message) private static string GetLastMessageText_PlainText(IIncomingEmailMessage message) { var lastMessage = new StringBuilder(); - lastMessage.Append(message.RawBody); + lastMessage.Append(message.PlainTextBody); var next = GetReplySeperatorIndex(lastMessage.ToString()); From 51b2e7e17c7c20d26902c35c5fb0a50e35bbf3b7 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Wed, 26 Jun 2019 22:33:41 -0700 Subject: [PATCH 03/13] Initial hacky way to support embedded images --- .../Email/EWS/EWSIncomingFileAttachment.cs | 6 +++++ .../Email/EWS/EWSIncomingItemAttachment.cs | 6 +++++ Mail2Bug/Email/EmailBodyProcessingUtils.cs | 19 +++++++++++++++ Mail2Bug/Email/IIncomingEmailAttachment.cs | 1 + .../SimpleBugStrategy.cs | 24 ++++++++++++++----- .../WorkItemManagement/IWorkItemManager.cs | 2 +- .../WorkItemManagement/TFSWorkItemManager.cs | 16 +++++++++++-- .../WorkItemManagement/WorkItemManagerMock.cs | 6 ++--- 8 files changed, 68 insertions(+), 12 deletions(-) diff --git a/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs b/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs index 0026371..4263d24 100644 --- a/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs +++ b/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs @@ -28,6 +28,12 @@ public string SaveAttachmentToFile(string filename) return filename; } + public string ContentId + { + get => _attachment.ContentId; + set => _attachment.ContentId = value; + } + private static readonly ILog Logger = LogManager.GetLogger(typeof(EWSIncomingFileAttachment)); } } diff --git a/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs b/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs index 4bef1c2..0955f10 100644 --- a/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs +++ b/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs @@ -38,6 +38,12 @@ public string SaveAttachmentToFile(string filename) return filename; } + public string ContentId + { + get => _attachment.ContentId; + set => _attachment.ContentId = value; + } + private static readonly ILog Logger = LogManager.GetLogger(typeof (EWSIncomingItemAttachment)); } } diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index 5497f7f..d273981 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -123,5 +123,24 @@ private static string ResolveOridnalToChar(Match ordinalMatch) var unicodeChar = (char)int.Parse(ordinalMatch.Groups["ordinal"].Value); return new string(new[] {unicodeChar}); } + + public static string FixUpImgLinks(string description, IDictionary messageContentIdToTfsGuid) + { + CQ dom = description; + foreach (var pair in messageContentIdToTfsGuid) + { + string contentId = pair.Key; + string guid = pair.Value; + + string originalImgSrc = $"cid:{contentId}"; + var matchingImgLinks = dom[$"img[src$='{originalImgSrc}']"]; + foreach (IDomObject img in matchingImgLinks) + { + img.SetAttribute("src", guid); + } + } + + return dom.Render(); + } } } diff --git a/Mail2Bug/Email/IIncomingEmailAttachment.cs b/Mail2Bug/Email/IIncomingEmailAttachment.cs index 4c42cd5..d8b32a7 100644 --- a/Mail2Bug/Email/IIncomingEmailAttachment.cs +++ b/Mail2Bug/Email/IIncomingEmailAttachment.cs @@ -7,5 +7,6 @@ public interface IIncomingEmailAttachment { string SaveAttachmentToFile(); string SaveAttachmentToFile(string filename); + string ContentId { get; set; } } } diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index ad60396..01982ad 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -88,7 +88,7 @@ private void AttachMessageToWorkItem(IIncomingEmailMessage message, int workItem // Remove the file once we're done attaching it tfc.AddFile(filePath, false); - _workItemManager.AttachFiles(workItemId, new List { filePath }); + _workItemManager.AttachFiles(workItemId, new List { new MessageAttachmentInfo(filePath, string.Empty) }); } } @@ -172,21 +172,20 @@ private void ProcessAttachments(IIncomingEmailMessage message, int workItemId) /// Take attachments from the current mail message and put them in a work item /// /// - private static TempFileCollection SaveAttachments(IIncomingEmailMessage message) + private static IReadOnlyCollection SaveAttachments(IIncomingEmailMessage message) { - var attachmentFiles = new TempFileCollection(); - + List result = new List(); foreach (var attachment in message.Attachments) { var filename = attachment.SaveAttachmentToFile(); if (filename != null) { - attachmentFiles.AddFile(filename, false); + result.Add(new MessageAttachmentInfo(filename, attachment.ContentId)); Logger.InfoFormat("Attachment saved to file {0}", filename); } } - return attachmentFiles; + return result; } private static readonly ILog Logger = LogManager.GetLogger(typeof(SimpleBugStrategy)); @@ -196,4 +195,17 @@ public void Dispose() DisposeUtils.DisposeIfDisposable(_workItemManager); } } + + public class MessageAttachmentInfo + { + public MessageAttachmentInfo(string filePath, string contentId) + { + FilePath = filePath; + ContentId = contentId; + } + + public string FilePath { get; } + + public string ContentId { get; } + } } diff --git a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs index 40d5be7..fcb54c3 100644 --- a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs @@ -5,7 +5,7 @@ namespace Mail2Bug.WorkItemManagement { public interface IWorkItemManager { - void AttachFiles(int workItemId, List fileList); + void AttachFiles(int workItemId, IReadOnlyCollection fileList); SortedList WorkItemsCache { get; } diff --git a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs index ec274b5..da9b3b6 100644 --- a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs @@ -242,7 +242,7 @@ private string GetPatFromConfig() _config.TfsServerConfig.ServiceIdentityPatKeyVaultSecret); } - public void AttachFiles(int workItemId, List fileList) + public void AttachFiles(int workItemId, IReadOnlyCollection fileList) { if (workItemId <= 0) return; @@ -251,7 +251,19 @@ public void AttachFiles(int workItemId, List fileList) WorkItem workItem = _tfsStore.GetWorkItem(workItemId); workItem.Open(); - fileList.ForEach(file => workItem.Attachments.Add(new Attachment(file))); + var description = workItem.Fields["Description"].Value.ToString(); + IDictionary messageContentIdToUrl = new Dictionary(fileList.Count); + + foreach (var attachment in fileList) + { + var tfsAttachment = new Attachment(attachment.FilePath); + workItem.Attachments.Add(tfsAttachment); + messageContentIdToUrl.Add(attachment.ContentId, tfsAttachment.Uri.ToString()); + } + + description = EmailBodyProcessingUtils.FixUpImgLinks(description, messageContentIdToUrl); + workItem.Fields["Description"].Value = description; + ValidateAndSaveWorkItem(workItem); } catch (Exception exception) diff --git a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs index e450f65..68beaa6 100644 --- a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs +++ b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs @@ -20,9 +20,9 @@ public WorkItemManagerMock(string keyField, INameResolver resolver = null) WorkItemsCache = new SortedList(); } - public void AttachFiles(int workItemId, List fileList) + public void AttachFiles(int workItemId, IReadOnlyCollection fileList) { - foreach (var filename in fileList.Where(filename => !File.Exists(filename))) + foreach (var filename in fileList.Where(filename => !File.Exists(filename.FilePath))) { Logger.ErrorFormat("Couldn't find attachment file {0}", filename); } @@ -37,7 +37,7 @@ public void AttachFiles(int workItemId, List fileList) Attachments[workItemId] = new List(); } - Attachments[workItemId].AddRange(fileList); + Attachments[workItemId].AddRange(fileList.Select(f => f.FilePath)); } public void CacheWorkItem(int workItemId) From 7f91bfefa492bc650e50e0d45a0039c4af17cd3b Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Wed, 26 Jun 2019 23:09:29 -0700 Subject: [PATCH 04/13] Support images in both description and comments. Do so in a less hacky way --- Mail2Bug/Email/EmailBodyProcessingUtils.cs | 19 +++-- .../SimpleBugStrategy.cs | 69 +++++++++++++------ .../WorkItemManagement/IWorkItemManager.cs | 7 +- .../WorkItemManagement/TFSWorkItemManager.cs | 42 +++++++---- .../WorkItemManagement/WorkItemManagerMock.cs | 4 +- .../Mocks/Email/IncomingAttachmentMock.cs | 2 + 6 files changed, 98 insertions(+), 45 deletions(-) diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index d273981..f873b65 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -4,6 +4,8 @@ using System.Text; using System.Text.RegularExpressions; using CsQuery; +using Mail2Bug.MessageProcessingStrategies; +using Microsoft.TeamFoundation.WorkItemTracking.Internals; namespace Mail2Bug.Email { @@ -124,19 +126,22 @@ private static string ResolveOridnalToChar(Match ordinalMatch) return new string(new[] {unicodeChar}); } - public static string FixUpImgLinks(string description, IDictionary messageContentIdToTfsGuid) + public static string FixUpImgLinks(string originalHtml, System.Collections.Generic.IReadOnlyCollection attachments) { - CQ dom = description; - foreach (var pair in messageContentIdToTfsGuid) + if (attachments == null || attachments.Count == 0) { - string contentId = pair.Key; - string guid = pair.Value; + return originalHtml; + } - string originalImgSrc = $"cid:{contentId}"; + CQ dom = originalHtml; + foreach (var attachment in attachments) + { + string originalImgSrc = $"cid:{attachment.ContentId}"; var matchingImgLinks = dom[$"img[src$='{originalImgSrc}']"]; + var newSrc = new Uri(attachment.FilePath); foreach (IDomObject img in matchingImgLinks) { - img.SetAttribute("src", guid); + img.SetAttribute("src", newSrc.AbsoluteUri); } } diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index 01982ad..38e2580 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -1,7 +1,6 @@ using System; using System.CodeDom.Compiler; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Linq; using log4net; @@ -49,9 +48,11 @@ private void NewWorkItem(IIncomingEmailMessage message) { var workItemUpdates = new Dictionary(); - InitWorkItemFields(message, workItemUpdates); + var attachments = SaveAttachments(message); - var workItemId = _workItemManager.CreateWorkItem(workItemUpdates); + InitWorkItemFields(message, workItemUpdates, attachments); + + var workItemId = _workItemManager.CreateWorkItem(workItemUpdates, attachments); Logger.InfoFormat("Added new work item {0} for message with subject: {1} (conversation index:{2})", workItemId, message.Subject, message.ConversationId); @@ -60,12 +61,13 @@ private void NewWorkItem(IIncomingEmailMessage message) // Since the work item *has* been created, failures in this stage are not treated as critical var overrides = new OverridesExtractor(_config).GetOverrides(message); TryApplyFieldOverrides(overrides, workItemId); - ProcessAttachments(message, workItemId); if (_config.WorkItemSettings.AttachOriginalMessage) { AttachMessageToWorkItem(message, workItemId, "OriginalMessage"); } + + attachments.DeleteLocalFiles(); } catch (Exception ex) { @@ -92,7 +94,7 @@ private void AttachMessageToWorkItem(IIncomingEmailMessage message, int workItem } } - private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary workItemUpdates) + private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary workItemUpdates, MessageAttachmentCollection attachments) { var resolver = new SpecialValueResolver(message, _workItemManager.GetNameResolver()); @@ -103,8 +105,14 @@ private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary overrides, int workItemId) @@ -118,7 +126,7 @@ private void TryApplyFieldOverrides(Dictionary overrides, int wo try { Logger.DebugFormat("Overrides found. Calling 'ModifyWorkItem'"); - _workItemManager.ModifyWorkItem(workItemId, "", false, overrides); + _workItemManager.ModifyWorkItem(workItemId, "", false, overrides, null); } catch (Exception ex) { @@ -150,10 +158,12 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) overrides.ToList().ForEach(x => workItemUpdates[x.Key] = x.Value); } + var attachments = SaveAttachments(message); + // Construct the text to be appended - _workItemManager.ModifyWorkItem(workItemId, lastMessageText, message.IsHtmlBody, workItemUpdates); + _workItemManager.ModifyWorkItem(workItemId, lastMessageText, message.IsHtmlBody, workItemUpdates, attachments); - ProcessAttachments(message, workItemId); + attachments.DeleteLocalFiles(); if (_config.WorkItemSettings.AttachUpdateMessages) { @@ -161,26 +171,19 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) } } - private void ProcessAttachments(IIncomingEmailMessage message, int workItemId) - { - var attachmentFiles = SaveAttachments(message); - _workItemManager.AttachFiles(workItemId, (from object file in attachmentFiles select file.ToString()).ToList()); - attachmentFiles.Delete(); - } - /// /// Take attachments from the current mail message and put them in a work item /// /// - private static IReadOnlyCollection SaveAttachments(IIncomingEmailMessage message) + private static MessageAttachmentCollection SaveAttachments(IIncomingEmailMessage message) { - List result = new List(); + var result = new MessageAttachmentCollection(); foreach (var attachment in message.Attachments) { var filename = attachment.SaveAttachmentToFile(); if (filename != null) { - result.Add(new MessageAttachmentInfo(filename, attachment.ContentId)); + result.Add(filename, attachment.ContentId); Logger.InfoFormat("Attachment saved to file {0}", filename); } } @@ -196,6 +199,32 @@ public void Dispose() } } + public class MessageAttachmentCollection + { + private readonly List _attachments; + private readonly TempFileCollection _tempFileCollection; + + public IReadOnlyCollection Attachments => _attachments; + public IEnumerable LocalFilePaths => _attachments.Select(a => a.FilePath); + + public MessageAttachmentCollection() + { + _attachments = new List(); + _tempFileCollection = new TempFileCollection(); + } + + public void Add(string localFilePath, string contentId) + { + _attachments.Add(new MessageAttachmentInfo(localFilePath, contentId)); + _tempFileCollection.AddFile(localFilePath, keepFile: false); + } + + public void DeleteLocalFiles() + { + _tempFileCollection.Delete(); + } + } + public class MessageAttachmentInfo { public MessageAttachmentInfo(string filePath, string contentId) diff --git a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs index fcb54c3..ef306bb 100644 --- a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs @@ -12,14 +12,17 @@ public interface IWorkItemManager void CacheWorkItem(int workItemId); /// Field Values + /// /// Bug ID - int CreateWorkItem(Dictionary values); + int CreateWorkItem(Dictionary values, MessageAttachmentCollection attachments); /// The ID of the bug to modify /// Comment to add to description /// /// List of fields to change - void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values); + /// + void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values, + MessageAttachmentCollection attachments); INameResolver GetNameResolver(); diff --git a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs index da9b3b6..8e32269 100644 --- a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs @@ -6,6 +6,7 @@ using System.Net; using System.Text; using log4net; +using Mail2Bug.Email; using Mail2Bug.ExceptionClasses; using Mail2Bug.Helpers; using Mail2Bug.MessageProcessingStrategies; @@ -242,7 +243,7 @@ private string GetPatFromConfig() _config.TfsServerConfig.ServiceIdentityPatKeyVaultSecret); } - public void AttachFiles(int workItemId, IReadOnlyCollection fileList) + public void AttachFiles(int workItemId, IReadOnlyCollection messageAttachments) { if (workItemId <= 0) return; @@ -251,19 +252,11 @@ public void AttachFiles(int workItemId, IReadOnlyCollection messageContentIdToUrl = new Dictionary(fileList.Count); - - foreach (var attachment in fileList) + foreach (var attachment in messageAttachments) { - var tfsAttachment = new Attachment(attachment.FilePath); - workItem.Attachments.Add(tfsAttachment); - messageContentIdToUrl.Add(attachment.ContentId, tfsAttachment.Uri.ToString()); + workItem.Attachments.Add(new Attachment(attachment.FilePath)); } - description = EmailBodyProcessingUtils.FixUpImgLinks(description, messageContentIdToUrl); - workItem.Fields["Description"].Value = description; - ValidateAndSaveWorkItem(workItem); } catch (Exception exception) @@ -273,8 +266,9 @@ public void AttachFiles(int workItemId, IReadOnlyCollectionThe list of fields and their desired values to apply to the work item + /// /// Work item ID of the newly created work item - public int CreateWorkItem(Dictionary values) + public int CreateWorkItem(Dictionary values, MessageAttachmentCollection attachments) { if (values == null) { @@ -306,6 +300,11 @@ public int CreateWorkItem(Dictionary values) TryApplyFieldValue(workItem, AssignedToFieldKey, values[AssignedToFieldKey]); } + foreach (var attachmentPath in attachments.LocalFilePaths) + { + workItem.Attachments.Add(new Attachment(attachmentPath)); + } + ValidateAndSaveWorkItem(workItem); CacheWorkItem(workItem); @@ -314,8 +313,10 @@ public int CreateWorkItem(Dictionary values) /// The ID of the work item to modify /// Comment to add to description + /// /// List of fields to change - public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values) + /// + public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values, MessageAttachmentCollection attachments) { if (workItemId <= 0) return; @@ -323,12 +324,25 @@ public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, D workItem.Open(); - workItem.History = commentIsHtml ? comment : comment.Replace("\n", "
"); + if (commentIsHtml) + { + workItem.History = EmailBodyProcessingUtils.FixUpImgLinks(comment, attachments.Attachments); + } + else + { + workItem.History = comment.Replace("\n", "
"); + } + foreach (var key in values.Keys) { TryApplyFieldValue(workItem, key, values[key]); } + foreach (var attachment in attachments.Attachments) + { + workItem.Attachments.Add(new Attachment(attachment.FilePath)); + } + ValidateAndSaveWorkItem(workItem); workItem.Save(); diff --git a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs index 68beaa6..221c4be 100644 --- a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs +++ b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs @@ -54,7 +54,7 @@ public void CacheWorkItem(int workItemId) WorkItemsCache[conversationId] = workItemId; } - public int CreateWorkItem(Dictionary values) + public int CreateWorkItem(Dictionary values, MessageAttachmentCollection attachments) { if (ThrowOnCreateBug != null) throw ThrowOnCreateBug; @@ -80,7 +80,7 @@ public int CreateWorkItem(Dictionary values) return id; } - public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values) + public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values, MessageAttachmentCollection attachments) { if (ThrowOnModifyBug != null) throw ThrowOnModifyBug; diff --git a/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs b/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs index 00d4bf4..4c0136b 100644 --- a/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs +++ b/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs @@ -37,6 +37,8 @@ public string SaveAttachmentToFile(string filename) return filename; } + public string ContentId { get; set; } + public Exception ExceptionToThrow { get; set; } public byte[] Data = new byte[1]; From 2ea86387bf8ef569a2b925711da07153c97232b8 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Tue, 2 Jul 2019 17:28:53 -0700 Subject: [PATCH 05/13] Add UTs + comments --- Mail2Bug/Email/EmailBodyProcessingUtils.cs | 10 ++- .../SimpleBugStrategy.cs | 2 +- .../WorkItemManagement/TFSWorkItemManager.cs | 2 +- .../EmailBodyProcessingUtilsUnitTest.cs | 86 +++++++++++++++++++ 4 files changed, 97 insertions(+), 3 deletions(-) diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index f873b65..d5d3c10 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -126,7 +126,11 @@ private static string ResolveOridnalToChar(Match ordinalMatch) return new string(new[] {unicodeChar}); } - public static string FixUpImgLinks(string originalHtml, System.Collections.Generic.IReadOnlyCollection attachments) + /// + /// If an email embeds an email inline in its html, that embedded image won't display correctly unless we modify the html. + /// This method does that, given information about email's known attachments + /// + public static string UpdateEmbeddedImageLinks(string originalHtml, System.Collections.Generic.IReadOnlyCollection attachments) { if (attachments == null || attachments.Count == 0) { @@ -138,6 +142,10 @@ public static string FixUpImgLinks(string originalHtml, System.Collections.Gener { string originalImgSrc = $"cid:{attachment.ContentId}"; var matchingImgLinks = dom[$"img[src$='{originalImgSrc}']"]; + + // This may point to the file on the local file-system if we haven't yet uploaded the attachment + // However, the work item APIs seem to magically 'just work' with this and update the URI to point to the uploaded location + // If for some reason that stops working, we'd need to either infer the uploaded URI or upload first and mutate the html afterward var newSrc = new Uri(attachment.FilePath); foreach (IDomObject img in matchingImgLinks) { diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index 38e2580..8d69c6d 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -108,7 +108,7 @@ private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary + + + +"; + + IReadOnlyCollection attachmentInfo = new List + { + new MessageAttachmentInfo(@"x:\image.png", "123"), + }; + + // Note: it's acceptable to not preserve whitespace / insert empty HTML tags because we're + // manipulating HTML, not plain text. As long as the rendered page isn't impacted, all is well + string expected = @" + + +"; + + string actual = EmailBodyProcessingUtils.UpdateEmbeddedImageLinks(original, attachmentInfo); + Assert.AreEqual(expected, actual); + } + + [TestMethod] + public void TestGetLastMessageText_NoPrevious() + { + string original = @" + +This is a boring email. + +"; + + // Note: it's acceptable to not preserve whitespace / insert empty HTML tags because we're + // manipulating HTML, not plain text. As long as the rendered page isn't impacted, all is well + string expected = @" +This is a boring email. + +"; + + string actual = EmailBodyProcessingUtils.GetLastMessageText_Html(original); + Assert.AreEqual(expected, actual); + } + + [TestMethod] + public void TestGetLastMessageText_Previous_FromColon() + { + string original = @" + +
+

This is random text with some custom styling and outlook-generated elements.

+
+
+

From: someAddress;
Subject: RE: Build error +

+
+
+

+   +

+

text of the reply +

+
+
Something after the containing div
+ +"; + + // Note: it's acceptable to not preserve whitespace because it's + // manipulating HTML, not plain text. As long as the rendered page isn't impacted, all is well + // Note that we expect that both + // 1. Elements following the latest message are removed + // 2. Anything in the same element as the latest message but after the start of the previous should be cleared out + string expected = @" +
+

This is random text with some custom styling and outlook-generated elements.

+
+
+

"; + + string actual = EmailBodyProcessingUtils.GetLastMessageText_Html(original); + Assert.AreEqual(expected, actual); + } + private static string Normalize(string text) { var normalized = text.Replace("\r\n", "\n"); From de963add4635d6c4c51a117793370907ece1f75b Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Tue, 2 Jul 2019 17:38:15 -0700 Subject: [PATCH 06/13] Split classes into their own files --- Mail2Bug/Email/MessageAttachmentCollection.cs | 35 +++++++++++++++++ Mail2Bug/Email/MessageAttachmentInfo.cs | 18 +++++++++ Mail2Bug/Mail2Bug.csproj | 2 + .../SimpleBugStrategy.cs | 39 ------------------- .../WorkItemManagement/IWorkItemManager.cs | 1 + .../WorkItemManagement/WorkItemManagerMock.cs | 1 + .../EmailBodyProcessingUtilsUnitTest.cs | 1 - 7 files changed, 57 insertions(+), 40 deletions(-) create mode 100644 Mail2Bug/Email/MessageAttachmentCollection.cs create mode 100644 Mail2Bug/Email/MessageAttachmentInfo.cs diff --git a/Mail2Bug/Email/MessageAttachmentCollection.cs b/Mail2Bug/Email/MessageAttachmentCollection.cs new file mode 100644 index 0000000..9d708a1 --- /dev/null +++ b/Mail2Bug/Email/MessageAttachmentCollection.cs @@ -0,0 +1,35 @@ +using System.CodeDom.Compiler; +using System.Collections.Generic; +using System.Linq; + +namespace Mail2Bug.Email +{ + /// + /// Collection of Exchange email attachments that have been downloaded locally + /// + public class MessageAttachmentCollection + { + private readonly List _attachments; + private readonly TempFileCollection _tempFileCollection; + + public IReadOnlyCollection Attachments => _attachments; + public IEnumerable LocalFilePaths => _attachments.Select(a => a.FilePath); + + public MessageAttachmentCollection() + { + _attachments = new List(); + _tempFileCollection = new TempFileCollection(); + } + + public void Add(string localFilePath, string contentId) + { + _attachments.Add(new MessageAttachmentInfo(localFilePath, contentId)); + _tempFileCollection.AddFile(localFilePath, keepFile: false); + } + + public void DeleteLocalFiles() + { + _tempFileCollection.Delete(); + } + } +} \ No newline at end of file diff --git a/Mail2Bug/Email/MessageAttachmentInfo.cs b/Mail2Bug/Email/MessageAttachmentInfo.cs new file mode 100644 index 0000000..3d8859b --- /dev/null +++ b/Mail2Bug/Email/MessageAttachmentInfo.cs @@ -0,0 +1,18 @@ +namespace Mail2Bug.Email +{ + /// + /// Represents basic information about an Exchange email attachment that has been downloaded locally and has a known exchange content id + /// + public class MessageAttachmentInfo + { + public MessageAttachmentInfo(string filePath, string contentId) + { + FilePath = filePath; + ContentId = contentId; + } + + public string FilePath { get; } + + public string ContentId { get; } + } +} \ No newline at end of file diff --git a/Mail2Bug/Mail2Bug.csproj b/Mail2Bug/Mail2Bug.csproj index 7a59c63..0369b6d 100644 --- a/Mail2Bug/Mail2Bug.csproj +++ b/Mail2Bug/Mail2Bug.csproj @@ -323,6 +323,8 @@ + + diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index 8d69c6d..22941a4 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -198,43 +198,4 @@ public void Dispose() DisposeUtils.DisposeIfDisposable(_workItemManager); } } - - public class MessageAttachmentCollection - { - private readonly List _attachments; - private readonly TempFileCollection _tempFileCollection; - - public IReadOnlyCollection Attachments => _attachments; - public IEnumerable LocalFilePaths => _attachments.Select(a => a.FilePath); - - public MessageAttachmentCollection() - { - _attachments = new List(); - _tempFileCollection = new TempFileCollection(); - } - - public void Add(string localFilePath, string contentId) - { - _attachments.Add(new MessageAttachmentInfo(localFilePath, contentId)); - _tempFileCollection.AddFile(localFilePath, keepFile: false); - } - - public void DeleteLocalFiles() - { - _tempFileCollection.Delete(); - } - } - - public class MessageAttachmentInfo - { - public MessageAttachmentInfo(string filePath, string contentId) - { - FilePath = filePath; - ContentId = contentId; - } - - public string FilePath { get; } - - public string ContentId { get; } - } } diff --git a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs index ef306bb..ec188a5 100644 --- a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using Mail2Bug.Email; using Mail2Bug.MessageProcessingStrategies; namespace Mail2Bug.WorkItemManagement diff --git a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs index 221c4be..43460a8 100644 --- a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs +++ b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs @@ -4,6 +4,7 @@ using System.IO; using System.Linq; using log4net; +using Mail2Bug.Email; using Mail2Bug.MessageProcessingStrategies; namespace Mail2Bug.WorkItemManagement diff --git a/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs b/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs index 62db664..24b8df7 100644 --- a/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs +++ b/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs @@ -5,7 +5,6 @@ using System.Text; using System.Text.RegularExpressions; using Mail2Bug.Email; -using Mail2Bug.MessageProcessingStrategies; using Mail2Bug.TestHelpers; using Mail2BugUnitTests.Mocks.Email; using Microsoft.Test.Text; From 9ecdbe5c87062b8712883d5e440612dc0969d03f Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Wed, 3 Jul 2019 15:49:34 -0700 Subject: [PATCH 07/13] Gate new behavior behind an EnableExperimentalHtmlFeatures setting --- Mail2Bug/Config.cs | 1 + Mail2Bug/Email/EWS/EWSIncomingMessage.cs | 4 ++-- Mail2Bug/Email/EmailBodyProcessingUtils.cs | 4 ++-- Mail2Bug/Email/IIncomingEmailMessage.cs | 2 +- .../MessageProcessingStrategies/SimpleBugStrategy.cs | 7 ++++--- Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs | 5 ++++- Mail2BugUnitTests/AckTemplateUnitTest.cs | 2 +- Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs | 8 ++++---- .../Mocks/Email/IncomingEmailMessageMock.cs | 6 +++--- Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs | 10 +++++----- 10 files changed, 27 insertions(+), 22 deletions(-) diff --git a/Mail2Bug/Config.cs b/Mail2Bug/Config.cs index 47d7a93..515e9d4 100644 --- a/Mail2Bug/Config.cs +++ b/Mail2Bug/Config.cs @@ -130,6 +130,7 @@ public enum ProcessingStrategyType public bool ApplyOverridesDuringUpdate { get; set; } public bool AttachOriginalMessage { get; set; } public bool AttachUpdateMessages { get; set; } + public bool EnableExperimentalHtmlFeatures { get; set; } public ProcessingStrategyType ProcessingStrategy = ProcessingStrategyType.SimpleBugStrategy; } diff --git a/Mail2Bug/Email/EWS/EWSIncomingMessage.cs b/Mail2Bug/Email/EWS/EWSIncomingMessage.cs index e7f0c27..538eda5 100644 --- a/Mail2Bug/Email/EWS/EWSIncomingMessage.cs +++ b/Mail2Bug/Email/EWS/EWSIncomingMessage.cs @@ -116,9 +116,9 @@ public string SaveToFile(string filename) return filename; } - public string GetLastMessageText() + public string GetLastMessageText(bool enableExperimentalHtmlFeatures) { - return EmailBodyProcessingUtils.GetLastMessageText(this); + return EmailBodyProcessingUtils.GetLastMessageText(this, enableExperimentalHtmlFeatures); } public void Delete() diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index d5d3c10..93be84d 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -11,9 +11,9 @@ namespace Mail2Bug.Email { public class EmailBodyProcessingUtils { - public static string GetLastMessageText(IIncomingEmailMessage message) + public static string GetLastMessageText(IIncomingEmailMessage message, bool enableExperimentalHtmlFeatures) { - return message.IsHtmlBody ? GetLastMessageText_Html(message.RawBody) : GetLastMessageText_PlainText(message); + return enableExperimentalHtmlFeatures && message.IsHtmlBody ? GetLastMessageText_Html(message.RawBody) : GetLastMessageText_PlainText(message); } private static string GetLastMessageText_PlainText(IIncomingEmailMessage message) diff --git a/Mail2Bug/Email/IIncomingEmailMessage.cs b/Mail2Bug/Email/IIncomingEmailMessage.cs index 143b708..05d1ab7 100644 --- a/Mail2Bug/Email/IIncomingEmailMessage.cs +++ b/Mail2Bug/Email/IIncomingEmailMessage.cs @@ -43,7 +43,7 @@ public interface IIncomingEmailMessage /// messages dropped, whereas the body includes all of that "history". /// /// - string GetLastMessageText(); + string GetLastMessageText(bool enableExperimentalHtmlFeatures); /// /// Deletes the message diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index 22941a4..e367d82 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -103,10 +103,11 @@ private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary p.Field.Equals("BugOwner") || p.Field.Equals("Priority")); + var placeholdersWithDefaults = placeholders.Where(p => p.Field.Equals("BugOwner") || p.Field.Equals("Priority")).ToList(); Assert.AreEqual(2, placeholdersWithDefaults.Count(), "Bad test data; all expected placeholders not found."); var workItem = new WorkItemFieldsMock(fields); diff --git a/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs b/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs index 24b8df7..a36ff08 100644 --- a/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs +++ b/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs @@ -31,7 +31,7 @@ public void TestGetLastMessageTextBasic() } message.PlainTextBody = bodyBuilder.ToString(); - Assert.AreEqual(lastMessageText, EmailBodyProcessingUtils.GetLastMessageText(message), "Verifying extracted last message text correctness"); + Assert.AreEqual(lastMessageText, EmailBodyProcessingUtils.GetLastMessageText(message, true), "Verifying extracted last message text correctness"); } [TestMethod] @@ -94,7 +94,7 @@ public void TestUpdateEmbeddedImageLinks_Basic() "; string actual = EmailBodyProcessingUtils.UpdateEmbeddedImageLinks(original, attachmentInfo); - Assert.AreEqual(expected, actual); + Assert.AreEqual(Normalize(expected), Normalize(actual)); } [TestMethod] @@ -114,7 +114,7 @@ This is a boring email. "; string actual = EmailBodyProcessingUtils.GetLastMessageText_Html(original); - Assert.AreEqual(expected, actual); + Assert.AreEqual(Normalize(expected), Normalize(actual)); } [TestMethod] @@ -153,7 +153,7 @@ public void TestGetLastMessageText_Previous_FromColon()

"; string actual = EmailBodyProcessingUtils.GetLastMessageText_Html(original); - Assert.AreEqual(expected, actual); + Assert.AreEqual(Normalize(expected), Normalize(actual)); } private static string Normalize(string text) diff --git a/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs b/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs index 493da6c..d9f6f8a 100644 --- a/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs +++ b/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs @@ -49,7 +49,7 @@ public static IncomingEmailMessageMock CreateWithRandomData(int numAttachments) mock.CcNames = GetRandomNamesList(mock.CcAddresses.Count()); mock.SentOn = new DateTime(Rand.Next(2012, 2525), Rand.Next(1, 12), Rand.Next(1, 28)); mock.ReceivedOn = new DateTime(Rand.Next(2012, 2525), Rand.Next(1, 12), Rand.Next(1, 28)); - mock.IsHtmlBody = Rand.Next(0, 1) == 0; + mock.IsHtmlBody = false; // this isn't html unless the creator specifically makes it so var attachments = new List(numAttachments); for (var i = 0; i < numAttachments; i++) @@ -103,9 +103,9 @@ public string SaveToFile(string filename) return filename; } - public string GetLastMessageText() + public string GetLastMessageText(bool enableExperimentalHtmlFeatures) { - return EmailBodyProcessingUtils.GetLastMessageText(this); + return EmailBodyProcessingUtils.GetLastMessageText(this, enableExperimentalHtmlFeatures); } public void CopyToFolder(string destinationFolder) diff --git a/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs b/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs index 11746dc..34d3320 100644 --- a/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs +++ b/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs @@ -201,7 +201,7 @@ public void TestProcessingEmailThreadImpl(bool overrideChangedBy) { expectedValues["Changed By"] = message3.SenderName; } - expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(message2.GetLastMessageText() + message3.GetLastMessageText()); + expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(message2.GetLastMessageText(true) + message3.GetLastMessageText(true)); ValidateBugValues(expectedValues, bugFields); } @@ -249,7 +249,7 @@ public void TestApplyingOverridesInUpdateMessage() expectedValues["Changed By"] = message4.SenderName; expectedValues[WorkItemManagerMock.HistoryField] = - TextUtils.FixLineBreaks(message2.GetLastMessageText() + message3.GetLastMessageText() + message4.GetLastMessageText()); + TextUtils.FixLineBreaks(message2.GetLastMessageText(true) + message3.GetLastMessageText(true) + message4.GetLastMessageText(true)); expectedValues[mnemonicDef.Field] = mnemonicDef.Value; expectedValues[explicitOverride1.Key] = explicitOverride1.Value; @@ -284,7 +284,7 @@ public void TestAttachingUpdateMessages() expectedValues["Changed By"] = message3.SenderName; expectedValues[WorkItemManagerMock.HistoryField] = - TextUtils.FixLineBreaks(message2.GetLastMessageText() + message3.GetLastMessageText()); + TextUtils.FixLineBreaks(message2.GetLastMessageText(true) + message3.GetLastMessageText(true)); ValidateBugValues(expectedValues, bugFields); @@ -325,7 +325,7 @@ public void TestAppendOnlyMessageBody() var expectedValues = new Dictionary(); expectedValues["Changed By"] = appendOnlyMessage.SenderName; - expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText()); + expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText(false)); Assert.AreEqual(2, workItemManagerMock.Bugs.Count, "Only one bug should exist"); ValidateBugValues(expectedValues, workItemManagerMock.Bugs[newBugId]); @@ -388,7 +388,7 @@ public void TestAppendOnlyMessageSubject() var expectedValues = new Dictionary(); expectedValues["Changed By"] = appendOnlyMessage.SenderName; - expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText()); + expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText(true)); Assert.AreEqual(2, workItemManagerMock.Bugs.Count, "Only one bug should exist"); ValidateBugValues(expectedValues, workItemManagerMock.Bugs[newBugId]); From 47a5cfbe9b70bfa41c2c463699311291b690fb4a Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Wed, 3 Jul 2019 16:15:58 -0700 Subject: [PATCH 08/13] Tweaks/fixes --- Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs | 3 ++- Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs | 5 +---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index e367d82..f75de8e 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -162,7 +162,8 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) var attachments = SaveAttachments(message); // Construct the text to be appended - _workItemManager.ModifyWorkItem(workItemId, lastMessageText, message.IsHtmlBody, workItemUpdates, attachments); + bool commentIsHtml = message.IsHtmlBody && _config.WorkItemSettings.EnableExperimentalHtmlFeatures; + _workItemManager.ModifyWorkItem(workItemId, lastMessageText, commentIsHtml, workItemUpdates, attachments); attachments.DeleteLocalFiles(); diff --git a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs index ef3c34f..a1c2afc 100644 --- a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs @@ -326,10 +326,7 @@ public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, D if (commentIsHtml) { - if (_config.WorkItemSettings.EnableExperimentalHtmlFeatures) - { - workItem.History = EmailBodyProcessingUtils.UpdateEmbeddedImageLinks(comment, attachments.Attachments); - } + workItem.History = EmailBodyProcessingUtils.UpdateEmbeddedImageLinks(comment, attachments.Attachments); } else { From ed9f1d8eebb718b626d4bef36023e29b76c42e19 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Wed, 3 Jul 2019 16:21:19 -0700 Subject: [PATCH 09/13] Ensure locally-downloaded attachments are deleted even if errors are encountered --- Mail2Bug/Email/MessageAttachmentCollection.cs | 10 +++- .../SimpleBugStrategy.cs | 53 +++++++++---------- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/Mail2Bug/Email/MessageAttachmentCollection.cs b/Mail2Bug/Email/MessageAttachmentCollection.cs index 9d708a1..6d9ceb4 100644 --- a/Mail2Bug/Email/MessageAttachmentCollection.cs +++ b/Mail2Bug/Email/MessageAttachmentCollection.cs @@ -1,4 +1,5 @@ -using System.CodeDom.Compiler; +using System; +using System.CodeDom.Compiler; using System.Collections.Generic; using System.Linq; @@ -7,7 +8,7 @@ namespace Mail2Bug.Email /// /// Collection of Exchange email attachments that have been downloaded locally /// - public class MessageAttachmentCollection + public class MessageAttachmentCollection : IDisposable { private readonly List _attachments; private readonly TempFileCollection _tempFileCollection; @@ -31,5 +32,10 @@ public void DeleteLocalFiles() { _tempFileCollection.Delete(); } + + public void Dispose() + { + DeleteLocalFiles(); + } } } \ No newline at end of file diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index f75de8e..e2c05c1 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -48,34 +48,33 @@ private void NewWorkItem(IIncomingEmailMessage message) { var workItemUpdates = new Dictionary(); - var attachments = SaveAttachments(message); - - InitWorkItemFields(message, workItemUpdates, attachments); + using (var attachments = SaveAttachments(message)) + { + InitWorkItemFields(message, workItemUpdates, attachments); - var workItemId = _workItemManager.CreateWorkItem(workItemUpdates, attachments); - Logger.InfoFormat("Added new work item {0} for message with subject: {1} (conversation index:{2})", - workItemId, message.Subject, message.ConversationId); + int workItemId = _workItemManager.CreateWorkItem(workItemUpdates, attachments); + Logger.InfoFormat("Added new work item {0} for message with subject: {1} (conversation index:{2})", + workItemId, message.Subject, message.ConversationId); - try - { - // Since the work item *has* been created, failures in this stage are not treated as critical - var overrides = new OverridesExtractor(_config).GetOverrides(message); - TryApplyFieldOverrides(overrides, workItemId); + try + { + // Since the work item *has* been created, failures in this stage are not treated as critical + var overrides = new OverridesExtractor(_config).GetOverrides(message); + TryApplyFieldOverrides(overrides, workItemId); - if (_config.WorkItemSettings.AttachOriginalMessage) + if (_config.WorkItemSettings.AttachOriginalMessage) + { + AttachMessageToWorkItem(message, workItemId, "OriginalMessage"); + } + } + catch (Exception ex) { - AttachMessageToWorkItem(message, workItemId, "OriginalMessage"); + Logger.ErrorFormat("Exception caught while applying settings to work item {0}\n{1}", workItemId, ex); } - attachments.DeleteLocalFiles(); - } - catch (Exception ex) - { - Logger.ErrorFormat("Exception caught while applying settings to work item {0}\n{1}", workItemId, ex); + var workItem = _workItemManager.GetWorkItemFields(workItemId); + _ackEmailHandler.SendAckEmail(message, workItem); } - - var workItem = _workItemManager.GetWorkItemFields(workItemId); - _ackEmailHandler.SendAckEmail(message, workItem); } private void AttachMessageToWorkItem(IIncomingEmailMessage message, int workItemId, string prefix) @@ -159,13 +158,11 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) overrides.ToList().ForEach(x => workItemUpdates[x.Key] = x.Value); } - var attachments = SaveAttachments(message); - - // Construct the text to be appended - bool commentIsHtml = message.IsHtmlBody && _config.WorkItemSettings.EnableExperimentalHtmlFeatures; - _workItemManager.ModifyWorkItem(workItemId, lastMessageText, commentIsHtml, workItemUpdates, attachments); - - attachments.DeleteLocalFiles(); + using (var attachments = SaveAttachments(message)) + { + bool commentIsHtml = message.IsHtmlBody && _config.WorkItemSettings.EnableExperimentalHtmlFeatures; + _workItemManager.ModifyWorkItem(workItemId, lastMessageText, commentIsHtml, workItemUpdates, attachments); + } if (_config.WorkItemSettings.AttachUpdateMessages) { From b99f3122b094da722467d8390e145fc81e581496 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Thu, 25 Jul 2019 10:17:15 -0700 Subject: [PATCH 10/13] Fix NullReferenceException --- Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs index a1c2afc..34ab95e 100644 --- a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs @@ -338,9 +338,12 @@ public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, D TryApplyFieldValue(workItem, key, values[key]); } - foreach (var attachment in attachments.Attachments) + if (attachments != null) { - workItem.Attachments.Add(new Attachment(attachment.FilePath)); + foreach (var attachment in attachments.Attachments) + { + workItem.Attachments.Add(new Attachment(attachment.FilePath)); + } } ValidateAndSaveWorkItem(workItem); From 7c5b186ec5d53216673ab45708d2a64ae45bfc59 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Tue, 30 Jul 2019 17:56:15 -0700 Subject: [PATCH 11/13] Fix bug where overrides/@@@s were including html artifacts in replies --- .../SimpleBugStrategy.cs | 13 ++++++-- .../SimpleBugStrategyUnitTest.cs | 33 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index e2c05c1..51b5b69 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -147,11 +147,19 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) workItemUpdates["Changed By"] = resolver.Sender; } - string lastMessageText = message.GetLastMessageText(_config.WorkItemSettings.EnableExperimentalHtmlFeatures); + bool isHtmlEnabled = _config.WorkItemSettings.EnableExperimentalHtmlFeatures; + bool commentIsHtml = message.IsHtmlBody && isHtmlEnabled; + + string lastMessageText = message.GetLastMessageText(isHtmlEnabled); if (_config.WorkItemSettings.ApplyOverridesDuringUpdate) { + // OverrideExtractor can't currently handle HTML input, so we need to make sure we pass it the plain text version + string lastMessagePlainText = commentIsHtml + ? message.GetLastMessageText(enableExperimentalHtmlFeatures: false) + : lastMessageText; + var extractor = new OverridesExtractor(_config); - var overrides = extractor.GetOverrides(lastMessageText); + var overrides = extractor.GetOverrides(lastMessagePlainText); Logger.DebugFormat("Found {0} overrides for update message", overrides.Count); @@ -160,7 +168,6 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) using (var attachments = SaveAttachments(message)) { - bool commentIsHtml = message.IsHtmlBody && _config.WorkItemSettings.EnableExperimentalHtmlFeatures; _workItemManager.ModifyWorkItem(workItemId, lastMessageText, commentIsHtml, workItemUpdates, attachments); } diff --git a/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs b/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs index 34d3320..a80c2fa 100644 --- a/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs +++ b/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs @@ -257,6 +257,39 @@ public void TestApplyingOverridesInUpdateMessage() Assert.IsFalse(bugFields.ContainsKey(explicitOverride2.Key)); } + [TestMethod] + public void TestApplyingOverridesInUpdateMessage_Html() + { + var seed = _rand.Next(); + + Logger.InfoFormat("Using seed {0}", seed); + + var mailManager = new MailManagerMock(); + var explicitOverride1 = new KeyValuePair("IsThisExplicit?","Indeed"); + var explicitLine1 = string.Format("\n###{0}:{1}", explicitOverride1.Key, explicitOverride1.Value); + var message1 = mailManager.AddMessage(false); + + var htmlText = string.Format("

{0}

", explicitLine1); + var message2 = mailManager.AddReply(message1, htmlText); + message2.IsHtmlBody = true; + message2.PlainTextBody = explicitLine1; + + var instanceConfig = GetConfig().Instances.First(); + instanceConfig.WorkItemSettings.ApplyOverridesDuringUpdate = true; + instanceConfig.WorkItemSettings.EnableExperimentalHtmlFeatures = true; + + var workItemManagerMock = new WorkItemManagerMock(instanceConfig.WorkItemSettings.ConversationIndexFieldName); + ProcessMailbox(mailManager, instanceConfig, workItemManagerMock); + + Assert.AreEqual(1, workItemManagerMock.Bugs.Count, "Only one bug should exist"); + var bug = workItemManagerMock.Bugs.First(); + var bugFields = bug.Value; + + Assert.IsTrue(bugFields.ContainsKey(explicitOverride1.Key)); + string actualValue = bugFields[explicitOverride1.Key]; + Assert.AreEqual(explicitOverride1.Value, actualValue); + } + [TestMethod] public void TestAttachingUpdateMessages() { From 5d05bcc88029c0cf80778149621edc9835bcb889 Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Thu, 1 Aug 2019 09:47:31 -0700 Subject: [PATCH 12/13] Avoid including line-like message delimiters when detecting last message in html --- Mail2Bug/Email/EmailBodyProcessingUtils.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index 93be84d..c4020f2 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -4,8 +4,6 @@ using System.Text; using System.Text.RegularExpressions; using CsQuery; -using Mail2Bug.MessageProcessingStrategies; -using Microsoft.TeamFoundation.WorkItemTracking.Internals; namespace Mail2Bug.Email { @@ -35,8 +33,23 @@ public static string GetLastMessageText_Html(string rawBody) { CQ dom = rawBody; + const string messageSeparatorStyle = "border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"; + foreach (IDomObject element in dom["*"]) { + // Lots of email clients insert html elements as message delimiters which have styling but no inner text + // This block checks for some of these patterns + if (element.NodeName == "DIV") + { + if (element.Id == "divRplyFwdMsg" || element.Id == "x_divRplyFwdMsg" || messageSeparatorStyle.Equals(element.GetAttribute("style"))) + { + IDomContainer parent = element.ParentNode; + element.Remove(); + RemoveSubsequent(parent); + break; + } + } + if (!element.ChildElements.Any() && !string.IsNullOrWhiteSpace(element.InnerText)) { var separatorIndex = IndexOfAny(element.InnerText, MessageBorderMarkers); From d85a390996eec400349dd90b230ea7d08900ecdf Mon Sep 17 00:00:00 2001 From: Brian Berger Date: Fri, 9 Aug 2019 17:20:16 -0700 Subject: [PATCH 13/13] Prevent duplicate attachments from being uploaded --- .../WorkItemManagement/TFSWorkItemManager.cs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs index 34ab95e..18c9dce 100644 --- a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs @@ -340,9 +340,27 @@ public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, D if (attachments != null) { + string GetAttachmentKey(string fileName, long length) + { + return $"{fileName}|{length}"; + } + + var existingAttachments = new HashSet(workItem.Attachments + .OfType() + .Select(attachment => GetAttachmentKey(attachment.Name, attachment.Length))); + foreach (var attachment in attachments.Attachments) { - workItem.Attachments.Add(new Attachment(attachment.FilePath)); + var localFileInfo = new FileInfo(attachment.FilePath); + string key = GetAttachmentKey(localFileInfo.Name, localFileInfo.Length); + + // If there's already an attachment with the same file name and size, don't bother re-uploading it + // TODO: this may break embedded images for html replies since we update the img tag above to point to a local path we don't upload + // However, we haven't confirmed this breaks and replying with an identical image is unlikely, so we ignore this for now + if (!existingAttachments.Contains(key)) + { + workItem.Attachments.Add(new Attachment(attachment.FilePath)); + } } }