Skip to content

Commit 3bf9079

Browse files
MrHinshMrHinsh
authored
Implement work item type fileds validation (#2910)
Current validation of work item types is not sufficient, because it validates just if the source field exists in target. But there are numerous cases, when the field exist, but is not valid: **Field can have different data type.** Some of the different types are OK, some are not. For example migrating from number to string is OK, but the other way may not be. **Field have different allowed values.** Migration process will migrate not allowed values without any warning. This happened even to me, when I migrated our backlog. But then, when backlog item was opened in DevOps UI, it immediately reported error and did not allow to save the item until it was fixes. This happenes for example, when work item types change. Than both, source and target work items have field `System.State`, but the values for them are different. I implemented validation for these, so clients can quickly see, that there are some discrepancies between source and target work item. All this is by default reported as warning (som migration will stop). If the user resolves issue with invalid field (for example the values will be mapped using `FieldMappingTool`, it needs to state it in `FixedTargetFields` property of options. All fields listed there will not trigger warning. But the information about the differencies will still be logged, so user always knows about it.
2 parents b643971 + 3401c12 commit 3bf9079

File tree

3 files changed

+164
-15
lines changed

3 files changed

+164
-15
lines changed

‎src/MigrationTools.Clients.TfsObjectModel.Tests/Tools/FieldReferenceNameMappingToolOptionsTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ public class FieldReferenceNameMappingToolOptionsTests
1010
public void FieldMappingsMustNotBeNullWhenNormalized()
1111
{
1212
TfsWorkItemTypeValidatorToolOptions options = new() { Enabled = true };
13-
options.FieldMappings = null;
13+
options.SourceFieldMappings = null;
1414
options.Normalize();
1515

16-
Assert.IsNotNull(options.FieldMappings);
16+
Assert.IsNotNull(options.SourceFieldMappings);
1717
}
1818

1919
[TestMethod]
@@ -22,7 +22,7 @@ public void ShouldLookupValueCaseInsensitively()
2222
TfsWorkItemTypeValidatorToolOptions options = new()
2323
{
2424
Enabled = true,
25-
FieldMappings = new()
25+
SourceFieldMappings = new()
2626
{
2727
["wit"] = new()
2828
{
@@ -44,7 +44,7 @@ public void ShouldReturnSourceValueIfNotMapped()
4444
TfsWorkItemTypeValidatorToolOptions options = new()
4545
{
4646
Enabled = true,
47-
FieldMappings = new()
47+
SourceFieldMappings = new()
4848
{
4949
["wit"] = new()
5050
{
@@ -64,7 +64,7 @@ public void ShouldReturnEmptyStringIfMappedToEmptyString()
6464
TfsWorkItemTypeValidatorToolOptions options = new()
6565
{
6666
Enabled = true,
67-
FieldMappings = new()
67+
SourceFieldMappings = new()
6868
{
6969
["wit"] = new()
7070
{
@@ -86,7 +86,7 @@ public void ShouldHandleAllWorkItemTypes()
8686
TfsWorkItemTypeValidatorToolOptions options = new()
8787
{
8888
Enabled = true,
89-
FieldMappings = new()
89+
SourceFieldMappings = new()
9090
{
9191
[TfsWorkItemTypeValidatorToolOptions.AllWorkItemTypes] = new()
9292
{

‎src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemTypeValidatorTool.cs‎

Lines changed: 109 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Microsoft.Extensions.Logging;
55
using Microsoft.Extensions.Options;
66
using Microsoft.TeamFoundation.WorkItemTracking.Client;
7+
using MigrationTools.FieldMaps.AzureDevops.ObjectModel;
78
using MigrationTools.Tools.Infrastructure;
89
using MigrationTools.Tools.Interfaces;
910

@@ -103,16 +104,26 @@ private bool ValidateWorkItemTypeFields(WorkItemType sourceWit, WorkItemType tar
103104

104105
if (targetFields.ContainsKey(targetFieldName))
105106
{
106-
Log.LogDebug(" Source field '{sourceFieldName}' exists in '{targetWit}'.", sourceFieldName, targetWit.Name);
107+
if (sourceField.IsIdentity)
108+
{
109+
const string message = " Source field '{sourceFieldName}' is identity field."
110+
+ " Validation is not performed on identity fields, because they usually differ in allowed values.";
111+
Log.LogDebug(message, sourceFieldName);
112+
}
113+
else
114+
{
115+
ValidateField(sourceField, targetFields[targetFieldName], targetWit.Name);
116+
}
107117
}
108118
else
109119
{
110120
Log.LogWarning(" Missing field '{targetFieldName}' in '{targetWit}'.", targetFieldName, targetWit.Name);
111121
Log.LogInformation(" Source field reference name: {sourceFieldReferenceName}", sourceFieldName);
112122
Log.LogInformation(" Source field name: {sourceFieldName}", sourceField.Name);
113123
Log.LogInformation(" Field type: {fieldType}", sourceField.FieldType);
114-
IEnumerable<string> allowedValues = sourceField.AllowedValues.OfType<string>().Select(val => $"'{val}'");
115-
Log.LogInformation(" Allowed values: {allowedValues}", string.Join(", ", allowedValues));
124+
(string valueType, List<string> allowedValues) = GetAllowedValues(sourceField);
125+
Log.LogInformation(" Allowed values: {allowedValues}", string.Join(", ", allowedValues.Select(v => $"'{v}'")));
126+
Log.LogInformation(" Allowed values type: {allowedValuesType}", valueType);
116127
result = false;
117128
}
118129
}
@@ -124,6 +135,97 @@ private bool ValidateWorkItemTypeFields(WorkItemType sourceWit, WorkItemType tar
124135
return result;
125136
}
126137

138+
private void ValidateField(FieldDefinition sourceField, FieldDefinition targetField, string targetWitName)
139+
{
140+
// If target field is in 'FixedTargetFields' list, it means, that user resolved this filed somehow.
141+
// For example by value mapping. So any discrepancies found will be logged just as information.
142+
// Otherwise, discrepancies are logged as warning.
143+
LogLevel logLevel = Options.IsFieldFixed(targetWitName, targetField.ReferenceName)
144+
? LogLevel.Information
145+
: LogLevel.Warning;
146+
bool isValid = ValidateFieldType(sourceField, targetField, logLevel);
147+
isValid &= ValidateFieldAllowedValues(sourceField, targetField, logLevel);
148+
if (isValid)
149+
{
150+
Log.LogDebug(" Target field '{targetFieldName}' exists in '{targetWit}' and is valid.",
151+
targetField.ReferenceName, targetWitName);
152+
}
153+
else if (logLevel == LogLevel.Information)
154+
{
155+
Log.LogInformation(" Target field '{targetFieldName}' in '{targetWit}' is considered valid,"
156+
+ $" because it is listed in '{nameof(Options.FixedTargetFields)}'.",
157+
targetField.ReferenceName, targetWitName, sourceField.ReferenceName);
158+
}
159+
}
160+
161+
private bool ValidateFieldType(FieldDefinition sourceField, FieldDefinition targetField, LogLevel logLevel)
162+
{
163+
if (sourceField.FieldType != targetField.FieldType)
164+
{
165+
Log.Log(logLevel,
166+
" Source field '{sourceField}' and target field '{targetField}' have different types:"
167+
+ " source = '{sourceFieldType}', target = '{targetFieldType}'.",
168+
sourceField.ReferenceName, targetField.ReferenceName, sourceField.FieldType, targetField.FieldType);
169+
return false;
170+
}
171+
return true;
172+
}
173+
174+
private bool ValidateFieldAllowedValues(FieldDefinition sourceField, FieldDefinition targetField, LogLevel logLevel)
175+
{
176+
bool isValid = true;
177+
(string sourceValueType, List<string> sourceAllowedValues) = GetAllowedValues(sourceField);
178+
(string targetValueType, List<string> targetAllowedValues) = GetAllowedValues(targetField);
179+
if (!sourceValueType.Equals(targetValueType, StringComparison.OrdinalIgnoreCase))
180+
{
181+
isValid = false;
182+
Log.Log(logLevel,
183+
" Source field '{sourceField}' and target field '{targetField}' have different allowed values types:"
184+
+ " source = '{sourceFieldAllowedValueType}', target = '{targetFieldAllowedValueType}'.",
185+
sourceField.ReferenceName, targetField.ReferenceName, sourceValueType, targetValueType);
186+
}
187+
if (!AllowedValuesAreSame(sourceAllowedValues, targetAllowedValues))
188+
{
189+
isValid = false;
190+
Log.Log(logLevel,
191+
" Source field '{sourceField}' and target field '{targetField}' have different allowed values.",
192+
sourceField.ReferenceName, targetField.ReferenceName);
193+
Log.LogInformation(" Source allowed values: {sourceAllowedValues}",
194+
string.Join(", ", sourceAllowedValues.Select(val => $"'{val}'")));
195+
Log.LogInformation(" Target allowed values: {targetAllowedValues}",
196+
string.Join(", ", targetAllowedValues.Select(val => $"'{val}'")));
197+
}
198+
199+
return isValid;
200+
}
201+
202+
private bool AllowedValuesAreSame(List<string> sourceAllowedValues, List<string> targetAllowedValues)
203+
{
204+
if (sourceAllowedValues.Count != targetAllowedValues.Count)
205+
{
206+
return false;
207+
}
208+
foreach (string sourceValue in sourceAllowedValues)
209+
{
210+
if (!targetAllowedValues.Contains(sourceValue, StringComparer.OrdinalIgnoreCase))
211+
{
212+
return false;
213+
}
214+
}
215+
return true;
216+
}
217+
218+
private (string valueType, List<string> allowedValues) GetAllowedValues(FieldDefinition field)
219+
{
220+
string valueType = field.SystemType.Name;
221+
List<string> allowedValues = [];
222+
for (int i = 0; i < field.AllowedValues.Count; i++)
223+
{
224+
allowedValues.Add(field.AllowedValues[i]);
225+
}
226+
return (valueType, allowedValues);
227+
}
228+
127229
private bool ShouldValidateWorkItemType(string workItemTypeName)
128230
{
129231
if ((Options.IncludeWorkItemtypes.Count > 0)
@@ -181,11 +283,14 @@ private void LogValidationResult(bool isValid)
181283

182284
const string message1 = "Some work item types or their fields are not present in the target system (see previous logs)."
183285
+ " Either add these fields into target work items, or map source fields to other target fields"
184-
+ $" in options ({nameof(TfsWorkItemTypeValidatorToolOptions.FieldMappings)}).";
286+
+ $" in options ({nameof(TfsWorkItemTypeValidatorToolOptions.SourceFieldMappings)}).";
185287
Log.LogError(message1);
186288
const string message2 = "If you have some field mappings defined for validation, do not forget also to configure"
187289
+ $" proper field mapping in {nameof(FieldMappingTool)} so data will preserved during migration.";
188290
Log.LogInformation(message2);
291+
const string message3 = "If you have different allowed values in some field, either update target field to match"
292+
+ $" allowed values from source, or configure {nameof(FieldValueMap)} in {nameof(FieldMappingTool)}.";
293+
Log.LogInformation(message3);
189294
}
190295

191296
private static bool TryFindSimilarWorkItemType(

‎src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemTypeValidatorToolOptions.cs‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Linq;
34
using MigrationTools.Tools.Infrastructure;
45

56
namespace MigrationTools.Tools
@@ -29,7 +30,21 @@ public TfsWorkItemTypeValidatorToolOptions()
2930
/// target field name. Target field name can be empty string to indicate that this field will not be validated in target.
3031
/// As work item type name, you can use <c>*</c> to define mappings which will be applied to all work item types.
3132
/// </summary>
32-
public Dictionary<string, Dictionary<string, string>> FieldMappings { get; set; } = [];
33+
public Dictionary<string, Dictionary<string, string>> SourceFieldMappings { get; set; } = [];
34+
35+
/// <summary>
36+
/// <para>
37+
/// List of target fields, that are considered as fixed. It means, even if the field is different against
38+
/// source field, no warning will be triggered, jus information about the differences.
39+
/// Use this list, whan you know about the differences between fields, but resolved it for example by
40+
/// using <see cref="FieldMappingTool"/>.
41+
/// </para>
42+
/// <para>
43+
/// Key is target work item type name. As this name, you can use <c>*</c> to define fixed fields which will be applied
44+
/// to all work item types.
45+
/// </para>
46+
/// </summary>
47+
public Dictionary<string, List<string>> FixedTargetFields { get; set; } = [];
3348

3449
/// <summary>
3550
/// Normalizes properties, that all of them are set (not <see langword="null"/>) and all dictionaries uses
@@ -42,7 +57,7 @@ public void Normalize()
4257
return;
4358
}
4459

45-
Dictionary<string, Dictionary<string, string>> oldMappings = FieldMappings;
60+
Dictionary<string, Dictionary<string, string>> oldMappings = SourceFieldMappings;
4661
Dictionary<string, Dictionary<string, string>> newMappings = new(_normalizedComparer);
4762
if (oldMappings is not null)
4863
{
@@ -57,12 +72,41 @@ public void Normalize()
5772
}
5873
}
5974

60-
IncludeWorkItemtypes ??= [];
75+
Dictionary<string, List<string>> oldFixedFields = FixedTargetFields;
76+
Dictionary<string, List<string>> newFixedFields = new(_normalizedComparer);
77+
if (oldFixedFields is not null)
78+
{
79+
foreach (KeyValuePair<string, List<string>> mapping in oldFixedFields)
80+
{
81+
newFixedFields[mapping.Key.Trim()] = mapping.Value;
82+
}
83+
}
6184

62-
FieldMappings = newMappings;
85+
IncludeWorkItemtypes ??= [];
86+
FixedTargetFields = newFixedFields;
87+
SourceFieldMappings = newMappings;
6388
_isNormalized = true;
6489
}
6590

91+
/// <summary>
92+
/// Returns true, if field <paramref name="targetFieldName"/> from work item type <paramref name="workItemType"/>
93+
/// is in list of fixed target fields. Handles also fields defined for all work item types (<c>*</c>).
94+
/// </summary>
95+
/// <param name="workItemType">Work item type name.</param>
96+
/// <param name="targetFieldName">Target field reference name.</param>
97+
public bool IsFieldFixed(string workItemType, string targetFieldName)
98+
{
99+
if (FixedTargetFields.TryGetValue(workItemType, out List<string> fixedFields))
100+
{
101+
return fixedFields.Contains(targetFieldName, _normalizedComparer);
102+
}
103+
if (FixedTargetFields.TryGetValue(AllWorkItemTypes, out fixedFields))
104+
{
105+
return fixedFields.Contains(targetFieldName, _normalizedComparer);
106+
}
107+
return false;
108+
}
109+
66110
/// <summary>
67111
/// Search for mapped target field name for given <paramref name="sourceFieldName"/>. If there is no mapping for source
68112
/// field name, its value is returned as target field name.
@@ -97,7 +141,7 @@ public string GetTargetFieldName(string workItemType, string sourceFieldName, ou
97141

98142
private bool TryGetTargetFieldName(string workItemType, string sourceFieldName, out string targetFieldName)
99143
{
100-
if (FieldMappings.TryGetValue(workItemType, out Dictionary<string, string> mappings))
144+
if (SourceFieldMappings.TryGetValue(workItemType, out Dictionary<string, string> mappings))
101145
{
102146
if (mappings.TryGetValue(sourceFieldName, out targetFieldName))
103147
{

0 commit comments

Comments
 (0)