From 2a209d41c6540e4c97a6458dffc601801e43bdd5 Mon Sep 17 00:00:00 2001 From: mammabear123 <127075115+mammabear123@users.noreply.github.com> Date: Tue, 16 Jan 2024 16:09:21 +0800 Subject: [PATCH 1/3] Provide ability to handle multiple custom fields with the same name. Note: this commit only adds the capability, further changes are required to switch usages of TryGetValue() in the field mappers of FieldMapperUtils for FieldMapperUtils.TryGetFirstValue(). --- .../JiraExport/IJiraProvider.cs | 4 ++ .../JiraExport/JiraProvider.cs | 59 +++++++++++++------ .../RevisionUtils/FieldMapperUtils.cs | 21 +++++++ 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/WorkItemMigrator/JiraExport/IJiraProvider.cs b/src/WorkItemMigrator/JiraExport/IJiraProvider.cs index 83696f7a..2b9e0052 100644 --- a/src/WorkItemMigrator/JiraExport/IJiraProvider.cs +++ b/src/WorkItemMigrator/JiraExport/IJiraProvider.cs @@ -29,7 +29,11 @@ public interface IJiraProvider bool GetCustomFieldSerializer(string customType, out ICustomFieldValueSerializer serializer); + /// string GetCustomId(string propertyName); + /// + List GetCustomIdList(string propertyName); + Task>> DownloadAttachments(JiraRevision rev); IEnumerable GetCommitRepositories(string issueId); diff --git a/src/WorkItemMigrator/JiraExport/JiraProvider.cs b/src/WorkItemMigrator/JiraExport/JiraProvider.cs index cf3002ca..ed2e7bf0 100644 --- a/src/WorkItemMigrator/JiraExport/JiraProvider.cs +++ b/src/WorkItemMigrator/JiraExport/JiraProvider.cs @@ -401,9 +401,30 @@ public string GetUserEmail(string usernameOrAccountId) } } + /// + /// Return the custom field id corresponding to the given field/property name. + /// + /// Blindly chooses the first matching value. This is often right, but doesn't handle the scenario where a + /// Jira instance has multiple custom fields with the same name. To handle that scenario, use instead. public string GetCustomId(string propertyName) { - var customId = string.Empty; + var allIds = GetCustomIdList(propertyName); + var customId = allIds.FirstOrDefault(); + if (allIds?.Count() > 1) + { + Logger.Log(LogLevel.Warning, $"Multiple fields found for {propertyName}. Selecting {customId}."); + } + + return customId; + } + + /// + /// Returns a list of custom field ids corresponding to the given field/property name. + /// + /// The property/field name of the custom field. + /// One or more custom field ids that match the name provided. + public List GetCustomIdList(string propertyName) + { JArray response = null; if (JiraNameFieldCache == null) @@ -412,19 +433,24 @@ public string GetCustomId(string propertyName) JiraNameFieldCache = CreateFieldCacheLookup(response, "name", "id"); } - customId = GetItemFromFieldCache(propertyName, JiraNameFieldCache); + var customIds = GetItemListFromFieldCache(propertyName, JiraNameFieldCache); - if (string.IsNullOrEmpty(customId)) + if (!customIds.Any()) { if (JiraKeyFieldCache == null) { response = response ?? (JArray)_jiraServiceWrapper.RestClient.ExecuteRequestAsync(Method.GET, $"{JiraApiV2}/field").Result; JiraKeyFieldCache = CreateFieldCacheLookup(response, "key", "id"); } - customId = GetItemFromFieldCache(propertyName, JiraKeyFieldCache); + customIds = GetItemListFromFieldCache(propertyName, JiraKeyFieldCache); + } + + if (customIds.Count == 0) + { + Logger.Log(LogLevel.Warning, $"Custom field {propertyName} could not be found."); } - return customId; + return customIds; } private ILookup CreateFieldCacheLookup(JArray response, string key, string value) @@ -435,19 +461,18 @@ private ILookup CreateFieldCacheLookup(JArray response, string k .ToLookup(l => l.key, l => l.value); } - private string GetItemFromFieldCache(string propertyName, ILookup cache) + /// + /// Returns a list of cache values corresponding to the given name. + /// + /// The name to lookup in the cache. + /// The cache to be interrogated. + /// + private List GetItemListFromFieldCache(string propertyName, ILookup cache) { - string customId = null; - var query = cache.FirstOrDefault(x => x.Key.Equals(propertyName.ToLower())); - if (query != null) - { - customId = query.Any() ? query.First() : null; - if (query.Count() > 1) - { - Logger.Log(LogLevel.Warning, $"Multiple fields found for {propertyName}. Selecting {customId}."); - } - } - return customId; + var fieldList = cache.FirstOrDefault(x => x.Key.Equals(propertyName.ToLower()))?.ToList() + ?? new List(0); + + return fieldList; } public IEnumerable GetCommitRepositories(string issueId) diff --git a/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs b/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs index ca12aff5..4cfb8e5d 100644 --- a/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs +++ b/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs @@ -183,6 +183,27 @@ public static object MapSprint(string iterationPathsString) return iterationPath; } + /// + /// Find the first custom field with the right name that has a value. + /// + /// JiraRevision.Fields dictionary. + /// The IDs of the Custom Fields to search - more than one custom field can have the same name. + /// The value of the first field in the list that has a value. Null if no values are found. + /// True if one of the identified custom fields contains a value. + internal static bool TryGetFirstValue(this Dictionary fields, List customFieldIds, out object fieldValueObject) + { + foreach(var name in customFieldIds) + { + if (fields.TryGetValue(name, out fieldValueObject) && fieldValueObject != null) + { + return true; + } + } + + fieldValueObject = null; + return false; + } + private static readonly Dictionary CalculatedLexoRanks = new Dictionary(); private static readonly Dictionary CalculatedRanks = new Dictionary(); From f5b34ed86eec8d93755ab86aab90f38a6bec1f51 Mon Sep 17 00:00:00 2001 From: mammabear123 <127075115+mammabear123@users.noreply.github.com> Date: Thu, 25 Jan 2024 22:47:29 +0800 Subject: [PATCH 2/3] Add some tests for TryGetFirstValue() --- .../RevisionUtils/FieldMapperUtils.cs | 4 + .../RevisionUtils/FieldMapperUtilsTests.cs | 127 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs b/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs index 4cfb8e5d..7e269bd3 100644 --- a/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs +++ b/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs @@ -12,8 +12,12 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Text.RegularExpressions; +[assembly: InternalsVisibleTo("Migration.Jira-Export.Tests")] + + namespace JiraExport { public static class FieldMapperUtils diff --git a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs index 64168d11..7b356271 100644 --- a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs +++ b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs @@ -489,5 +489,132 @@ public void { Assert.That(FieldMapperUtils.MapLexoRank("0|hzyxfj:hzyxfj"), Is.EqualTo(1088341183.1088341M)); } + + + /** + **************** TryGetFirstValue Tests **************** + */ + + [Test] + public void TryGetFirstValue_ShouldReturnTrue_WhenMatchingFieldExists() + { + // Arrange + var fields = new Dictionary + { + { "field1", "value1" }, + { "field2", "value2" }, + { "field3", "value3" } + }; + var customFieldIds = new List { "field2", "field3" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsTrue(result); + Assert.AreEqual("value2", fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenNoMatchingFieldExists() + { + // Arrange + var fields = new Dictionary + { + { "field1", "value1" }, + { "field2", "value2" }, + { "field3", "value3" } + }; + var customFieldIds = new List { "field4", "field5" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenFieldsDictionaryIsEmpty() + { + // Arrange + var fields = new Dictionary(); + var customFieldIds = new List { "field1", "field2" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenCustomFieldIdsListIsEmpty() + { + // Arrange + var fields = new Dictionary + { + { "field1", "value1" }, + { "field2", "value2" }, + { "field3", "value3" } + }; + var customFieldIds = new List(); + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnTrue_WhenMultipleMatchingFieldsExistButOnlyOneHasNonNullValue() + { + // Arrange + var fields = new Dictionary + { + { "field1", null }, + { "field2", "value2" }, + { "field3", null } + }; + var customFieldIds = new List { "field1", "field3", "field2" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsTrue(result); + Assert.AreEqual("value2", fieldValueObject); + } + + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenMultipleMatchingFieldsExistButOnlyAllAreNullValue() + { + // Arrange + var fields = new Dictionary + { + { "field1", null }, + { "field2", null }, + { "field3", null } + }; + var customFieldIds = new List { "field1", "field3", "field2" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } } } \ No newline at end of file From 12b2af4ac3bb0e90fb4f1fe437ae1ab742d915a7 Mon Sep 17 00:00:00 2001 From: mammabear123 <127075115+mammabear123@users.noreply.github.com> Date: Sun, 28 Jan 2024 01:45:47 +0800 Subject: [PATCH 3/3] IfChanged uses new "same name" custom field handling. New methods GetCustomIdList() and TryGetFirstValue() have been added to handle multiple custom fields that have the same name. This change modifies IfChanged() to actually start using the new code. --- src/WorkItemMigrator/JiraExport/JiraMapper.cs | 9 +- .../JiraMapperTests.cs | 103 ++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/WorkItemMigrator/JiraExport/JiraMapper.cs b/src/WorkItemMigrator/JiraExport/JiraMapper.cs index 76b06ebf..962fca04 100644 --- a/src/WorkItemMigrator/JiraExport/JiraMapper.cs +++ b/src/WorkItemMigrator/JiraExport/JiraMapper.cs @@ -379,16 +379,19 @@ private HashSet InitializeTypeMappings() return types; } - private Func IfChanged(string sourceField, bool isCustomField, Func mapperFunc = null) + internal Func IfChanged(string sourceField, bool isCustomField, Func mapperFunc = null) { + List sourceFields = null; if (isCustomField) { - sourceField = _jiraProvider.GetCustomId(sourceField) ?? sourceField; + sourceFields = _jiraProvider.GetCustomIdList(sourceField); } + sourceFields = sourceFields ?? new List { sourceField }; + return (r) => { - if (r.Fields.TryGetValue(sourceField.ToLower(), out object value)) + if (r.Fields.TryGetFirstValue(sourceFields, out object value)) { if (mapperFunc != null) { diff --git a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs index 702f0845..969fa7e1 100644 --- a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs +++ b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs @@ -8,6 +8,7 @@ using Newtonsoft.Json.Linq; using NSubstitute; using NUnit.Framework; +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -304,5 +305,107 @@ private JiraItem createJiraItem() return jiraItem; } + + + /* + * ******* IfChanged() ****** + */ + + [Test] + public void IfChanged_SourceFieldExists_ReturnsTrueAndValue() + { + // Arrange + var jiraMapper = createJiraMapper(); + //var revision = Substitute.For(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + revision.Fields.Add("sourceField", "value"); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("sourceField", false) (revision); + + // Assert + Assert.IsTrue(boolResult); + Assert.AreEqual("value", valueResult); + } + + [Test] + public void IfChanged_SourceFieldDoesNotExist_ReturnsFalseAndNull() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("sourceField", false) (revision); + + // Assert + Assert.IsFalse(boolResult); + Assert.IsNull(valueResult); + } + + [Test] + public void IfChanged_CustomFieldExists_ReturnsTrueAndValue() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + revision.Fields.Add("customField", "value"); + + var jiraProvider = Substitute.For(); + jiraProvider.GetCustomIdList("customField").Returns(new List { "customField" }); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("customField", true) (revision); + + // Assert + Assert.IsTrue(boolResult); + Assert.AreEqual("value", valueResult); + } + + [Test] + public void IfChanged_CustomFieldDoesNotExist_ReturnsFalseAndNull() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + + //_jiraProvider.GetCustomIdList("customField").Returns(new List { "customField" }); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("customField", true) (revision); + + // Assert + Assert.IsFalse(boolResult); + Assert.IsNull(valueResult); + } + + [Test] + public void IfChanged_SourceFieldExistsWithMapperFunc_ReturnsTrueAndMappedValue() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + revision.Fields.Add("sourceField", 10); + + // mapper takes expected 10 value and concatenates "XX" on the end. + Func mapperFunc = (value) => value.ToString() + "XX"; + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("sourceField", false, mapperFunc) (revision); + + // Assert + Assert.IsTrue(boolResult); + Assert.AreEqual("10XX", valueResult); + } } } \ No newline at end of file