Skip to content

Commit 498f182

Browse files
authored
Fix for greek letters (#2712)
This pull request refactors the `TfsWorkItemMigrationProcessor` class by removing unused code, improving validation logic, and enhancing logging and telemetry. The changes aim to simplify the codebase, improve maintainability, and provide better insights during execution. Below are the most important changes grouped by theme: ### Code Simplification and Cleanup: * Removed the `ProgressTimer` class and associated logic, replacing it with simpler calculations for average and remaining time during work item processing. (`src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs`) [[1]](diffhunk://#diff-b0a23408b76051c7078de3664c94b03ef26ca367ede9f084067d8722393f130eL40-L70) [[2]](diffhunk://#diff-b0a23408b76051c7078de3664c94b03ef26ca367ede9f084067d8722393f130eL181) [[3]](diffhunk://#diff-b0a23408b76051c7078de3664c94b03ef26ca367ede9f084067d8722393f130eL643-R689) * Removed unused imports from `TfsValidateRequiredFieldTool.cs` to clean up dependencies. (`src/MigrationTools.Clients.TfsObjectModel/Tools/TfsValidateRequiredFieldTool.cs`) ### Validation Improvements: * Enhanced the `ValiddateWorkItemTypesExistInTarget` method to include detailed logging for missing work item types, suggestions for mapping based on character similarity, and explicit error messages for unmapped types. (`src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs`) * Fixed a typo in the method name `ValiddateWorkItemTypesExistInTarget` to maintain consistency. (`src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs`) ### Logging and Telemetry Enhancements: * Improved exception logging in `ValidatingRequiredField` to include the work item type name for better debugging. (`src/MigrationTools.Clients.TfsObjectModel/Tools/TfsValidateRequiredFieldTool.cs`) * Added telemetry tags and end time tracking to activities in `ProcessWorkItemAsync` for better observability. (`src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs`) [[1]](diffhunk://#diff-b0a23408b76051c7078de3664c94b03ef26ca367ede9f084067d8722393f130eR404) [[2]](diffhunk://#diff-b0a23408b76051c7078de3664c94b03ef26ca367ede9f084067d8722393f130eL533-R577) ### Configuration Update: * Updated the `launchSettings.json` file to use a specific configuration file path for execution, likely for testing or debugging purposes. (`src/MigrationTools.ConsoleFull/Properties/launchSettings.json`)
2 parents 5fa8251 + 89dea40 commit 498f182

File tree

4 files changed

+119
-90
lines changed

4 files changed

+119
-90
lines changed

src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs

Lines changed: 105 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -37,37 +37,6 @@ namespace MigrationTools.Processors
3737
/// <processingtarget>Work Items</processingtarget>
3838
public class TfsWorkItemMigrationProcessor : TfsProcessor
3939
{
40-
private class ProgressTimer
41-
{
42-
private int _totalCount;
43-
private int _processedCount;
44-
private TimeSpan _totalProcessedTime = TimeSpan.Zero;
45-
46-
public ProgressTimer(int totalCount)
47-
{
48-
_totalCount = totalCount;
49-
}
50-
51-
public int TotalCount => _totalCount;
52-
public int ProcessedCount => _processedCount;
53-
public TimeSpan TotalProcessedTime => _totalProcessedTime;
54-
55-
public void AddProcessedItem(TimeSpan processDuration, bool addToTotal)
56-
{
57-
if (addToTotal)
58-
{
59-
_totalCount++;
60-
}
61-
_processedCount++;
62-
_totalProcessedTime += processDuration;
63-
}
64-
65-
public TimeSpan AverageTime => ProcessedCount > 0
66-
? TimeSpan.FromSeconds(TotalProcessedTime.TotalSeconds / ProcessedCount)
67-
: TotalProcessedTime;
68-
69-
public TimeSpan RemainingTime => TimeSpan.FromTicks(AverageTime.Ticks * (TotalCount - ProcessedCount));
70-
}
7140

7241
private static int _count = 0;
7342
private static int _current = 0;
@@ -143,8 +112,8 @@ protected override void InternalExecute()
143112
sourceWorkItems.Count);
144113

145114
//////////////////////////////////////////////////
146-
ValidateAllWorkItemTypesHaveReflectedWorkItemIdField(sourceWorkItems);
147115
ValiddateWorkItemTypesExistInTarget(sourceWorkItems);
116+
ValidateAllWorkItemTypesHaveReflectedWorkItemIdField(sourceWorkItems);
148117
CommonTools.NodeStructure.ValidateAllNodesExistOrAreMapped(this, sourceWorkItems, Source.WorkItems.Project.Name, Target.WorkItems.Project.Name);
149118
ValidateAllUsersExistOrAreMapped(sourceWorkItems);
150119
//////////////////////////////////////////////////
@@ -178,7 +147,6 @@ protected override void InternalExecute()
178147
_current = 1;
179148
_count = sourceWorkItems.Count;
180149
_totalWorkItem = sourceWorkItems.Count;
181-
ProgressTimer progressTimer = new ProgressTimer(_totalWorkItem);
182150
ProcessorActivity.SetTag("source_workitems_to_process", sourceWorkItems.Count);
183151
foreach (WorkItemData sourceWorkItemData in sourceWorkItems)
184152
{
@@ -195,7 +163,7 @@ protected override void InternalExecute()
195163
{
196164
try
197165
{
198-
ProcessWorkItemAsync(sourceWorkItemData, progressTimer, Options.WorkItemCreateRetryLimit).Wait();
166+
ProcessWorkItemAsync(sourceWorkItemData, Options.WorkItemCreateRetryLimit).Wait();
199167

200168
stopwatch.Stop();
201169
var processingTime = stopwatch.Elapsed.TotalMilliseconds;
@@ -241,6 +209,7 @@ protected override void InternalExecute()
241209
{
242210
contextLog.Warning("The following items could not be migrated: {ItemIds}", string.Join(", ", _itemsInError));
243211
}
212+
244213
}
245214
}
246215

@@ -297,44 +266,111 @@ private void ValidateAllWorkItemTypesHaveReflectedWorkItemIdField(List<WorkItemD
297266
private void ValiddateWorkItemTypesExistInTarget(List<WorkItemData> sourceWorkItems)
298267
{
299268
contextLog.Information("Validating::Check that all work item types needed in the Target exist or are mapped");
300-
// get list of all work item types
301-
List<String> sourceWorkItemTypes = sourceWorkItems.SelectMany(x => x.Revisions.Values)
302-
//.Where(x => x.Fields[fieldName].Value.ToString().Contains("\\"))
303-
.Select(x => x.Type)
304-
.Distinct()
305-
.ToList();
306269

307-
Log.LogDebug("Validating::WorkItemTypes::sourceWorkItemTypes: {count} WorkItemTypes in the full source history {sourceWorkItemTypesString}", sourceWorkItemTypes.Count(), string.Join(",", sourceWorkItemTypes));
270+
var sourceWorkItemTypes = sourceWorkItems
271+
.SelectMany(x => x.Revisions.Values)
272+
.Select(x => x.Type)
273+
.Distinct()
274+
.ToList();
275+
276+
Log.LogDebug("Validating::WorkItemTypes::sourceWorkItemTypes: {count} WorkItemTypes in the full source history: {sourceWorkItemTypesString}",
277+
sourceWorkItemTypes.Count, string.Join(",", sourceWorkItemTypes));
278+
279+
var targetWorkItemTypes = Target.WorkItems.Project
280+
.ToProject()
281+
.WorkItemTypes
282+
.Cast<WorkItemType>()
283+
.Select(x => x.Name)
284+
.Distinct()
285+
.ToList();
286+
287+
288+
289+
Log.LogDebug("Validating::WorkItemTypes::targetWorkItemTypes: {count} WorkItemTypes in Target process: {targetWorkItemTypesString}",
290+
targetWorkItemTypes.Count, string.Join(",", targetWorkItemTypes));
291+
292+
var missingWorkItemTypes = new List<string>();
308293

309-
var targetWorkItemTypes = Target.WorkItems.Project.ToProject().WorkItemTypes.Cast<WorkItemType>().Select(x => x.Name);
310-
Log.LogDebug("Validating::WorkItemTypes::targetWorkItemTypes::{count} WorkItemTypes in Target process: {targetWorkItemTypesString}", targetWorkItemTypes.Count(), string.Join(",", targetWorkItemTypes));
294+
foreach (var sourceWit in sourceWorkItemTypes)
295+
{
296+
var witToFind = sourceWit;
297+
if (CommonTools.WorkItemTypeMapping.Mappings.ContainsKey(sourceWit))
298+
{
299+
witToFind = CommonTools.WorkItemTypeMapping.Mappings[sourceWit];
300+
Log.LogDebug("Validating::WorkItemTypes::sourceWit: `{sourceWit}` is mapped to `{witToFind}`", sourceWit, witToFind);
301+
}
302+
if (!targetWorkItemTypes.Contains(witToFind, StringComparer.OrdinalIgnoreCase))
303+
{
304+
missingWorkItemTypes.Add(witToFind);
305+
}
306+
}
311307

312-
var missingWorkItemTypes = sourceWorkItemTypes.Where(sourceWit => !targetWorkItemTypes.Contains(sourceWit)); // the real one
313-
if (missingWorkItemTypes.Count() > 0)
308+
if (missingWorkItemTypes.Count > 0)
314309
{
315-
Log.LogWarning("Validating::WorkItemTypes::targetWorkItemTypes::There are {count} WorkItemTypes that are used in the history of the Source and that do not exist in the Target. These will all need mapped using `WorkItemTypeDefinition` in the config. ", missingWorkItemTypes.Count());
310+
Log.LogWarning("There are {count} WorkItemTypes used in source history that do not exist in the target. These may need to be mapped.", missingWorkItemTypes.Count);
316311

317-
bool allTypesMapped = true;
318-
foreach (var missingWorkItemType in missingWorkItemTypes)
312+
foreach (var sourceWit in missingWorkItemTypes)
319313
{
320-
bool thisTypeMapped = true;
321-
if (!CommonTools.WorkItemTypeMapping.Mappings.ContainsKey(missingWorkItemType))
314+
Log.LogWarning("Missing Source WIT: '{sourceWit}' (Unicode: {unicode})",
315+
sourceWit,
316+
string.Join(" ", sourceWit.Select(c => $"U+{(int)c:X4}")));
317+
318+
// Try to suggest a match based on character similarity
319+
foreach (var targetWit in targetWorkItemTypes)
322320
{
323-
thisTypeMapped = false;
321+
if (AreVisuallySimilar(sourceWit, targetWit))
322+
{
323+
Log.LogWarning("→ Suggested mapping: \"{0}\": \"{1}\"", sourceWit, targetWit);
324+
}
325+
}
326+
327+
if (!CommonTools.WorkItemTypeMapping.Mappings.ContainsKey(sourceWit))
328+
{
329+
Log.LogWarning("WorkItemType '{0}' is not mapped in your config. Add to WorkItemTypeDefinition if needed.", sourceWit);
324330
}
325-
Log.LogWarning("Validating::WorkItemTypes::targetWorkItemTypes::{missingWorkItemType}::Mapped? {thisTypeMapped}", missingWorkItemType, thisTypeMapped.ToString());
326-
allTypesMapped &= thisTypeMapped;
327331
}
328-
if (!allTypesMapped)
332+
333+
var ex = new Exception("Some Work Item Types in the source do not exist in the target. Either map or filter them.");
334+
Log.LogError(ex, "Validation failed: unmapped or missing work item types.");
335+
Environment.Exit(-1);
336+
}
337+
}
338+
339+
340+
private static bool AreVisuallySimilar(string a, string b)
341+
{
342+
if (a.Equals(b, StringComparison.OrdinalIgnoreCase)) return false; // Already matched normally
343+
344+
if (a.Length != b.Length) return false;
345+
346+
int similarCount = 0;
347+
348+
for (int i = 0; i < a.Length; i++)
349+
{
350+
var aChar = a[i];
351+
var bChar = b[i];
352+
353+
if (aChar == bChar)
329354
{
330-
var ex = new Exception(
331-
"Not all WorkItemTypes present in the Source are present in the Target or mapped! Filter them from the query, or map the to target types.");
332-
Log.LogError(ex, "Not all WorkItemTypes present in the Source are present in the Target or mapped using `WorkItemTypeDefinition` in the config.");
333-
Environment.Exit(-1);
355+
similarCount++;
356+
continue;
357+
}
358+
359+
// Check known lookalike characters (expandable)
360+
if ((aChar == '\u0399' && bChar == 'I') || (aChar == 'I' && bChar == '\u0399'))
361+
{
362+
similarCount++;
363+
}
364+
else
365+
{
366+
return false;
334367
}
335368
}
369+
370+
return similarCount == a.Length;
336371
}
337372

373+
338374
private void ValidatePatTokenRequirement()
339375
{
340376
string collUrl = Target.Options.Collection.ToString();
@@ -365,6 +401,7 @@ private WorkItemData CreateWorkItem_Shell(ProjectData destProject, WorkItemData
365401
activity?.SetTagsFromOptions(Options);
366402
activity?.SetTag("http.request.method", "GET");
367403
activity?.SetTag("migrationtools.client", "TfsObjectModel");
404+
activity?.SetEndTime(activity.StartTimeUtc.AddSeconds(10));
368405

369406
WorkItem newwit;
370407
if (destProject.ToProject().WorkItemTypes.Contains(destType))
@@ -443,7 +480,7 @@ private void PopulateWorkItem(WorkItemData oldWorkItemData, WorkItemData newWork
443480
newWorkItem.Fields["Microsoft.VSTS.Common.ClosedDate"].Value = oldWorkItem.Fields["Microsoft.VSTS.Common.ClosedDate"].Value;
444481
}
445482
}
446-
catch (FieldDefinitionNotExistException)
483+
catch (FieldDefinitionNotExistException ex)
447484
{
448485
// Eat exception coz the TFS API Sucks
449486
}
@@ -530,13 +567,14 @@ private void ProcessWorkItemEmbeddedLinks(WorkItemData sourceWorkItem, WorkItemD
530567
}
531568
}
532569

533-
private async Task ProcessWorkItemAsync(WorkItemData sourceWorkItem, ProgressTimer progressTimer, int retryLimit = 5, int retries = 0)
570+
private async Task ProcessWorkItemAsync(WorkItemData sourceWorkItem, int retryLimit = 5, int retries = 0)
534571
{
535572
using (var activity = ActivitySourceProvider.ActivitySource.StartActivity("ProcessWorkItemAsync", ActivityKind.Client))
536573
{
537574
activity?.SetTagsFromOptions(Options);
538575
activity?.SetTag("http.request.method", "GET");
539576
activity?.SetTag("migrationtools.client", "TfsObjectModel");
577+
activity?.SetEndTime(activity.StartTimeUtc.AddSeconds(10));
540578
activity?.SetTag("SourceURL", Source.Options.Collection.ToString());
541579
activity?.SetTag("SourceWorkItem", sourceWorkItem.Id);
542580
activity?.SetTag("TargetURL", Target.Options.Collection.ToString());
@@ -624,7 +662,7 @@ private async Task ProcessWorkItemAsync(WorkItemData sourceWorkItem, ProgressTim
624662
{"Retrys", retries },
625663
{"RetryLimit", retryLimit }
626664
});
627-
await ProcessWorkItemAsync(sourceWorkItem, progressTimer, retryLimit, retries);
665+
await ProcessWorkItemAsync(sourceWorkItem, retryLimit, retries);
628666
}
629667
else
630668
{
@@ -640,18 +678,15 @@ private async Task ProcessWorkItemAsync(WorkItemData sourceWorkItem, ProgressTim
640678
Telemetry.TrackException(ex, activity.Tags);
641679
throw ex;
642680
}
643-
activity?.Stop();
644-
progressTimer.AddProcessedItem(activity.Duration, retries > 0);
681+
var average = new TimeSpan(0, 0, 0, 0, (int)(activity.Duration.TotalMilliseconds / _current));
682+
var remaining = new TimeSpan(0, 0, 0, 0, (int)(average.TotalMilliseconds * _count));
645683
TraceWriteLine(LogEventLevel.Information,
646-
"Processed {processedItemsCount} items from {totalItemsCount} with average process time {average:%s}.{average:%fff} s (total processing time is {totalProcessedTime:h\\:mm\\:ss}). "
647-
+ "Estimated time to completion is {remaining:%h} hours {remaining:%m} minutes {remaining:%s} seconds.",
684+
"Average time of {average:%s}.{average:%fff} per work item and {remaining:%h} hours {remaining:%m} minutes {remaining:%s}.{remaining:%fff} seconds estimated to completion",
648685
new Dictionary<string, object>() {
649-
{ "processedItemsCount", progressTimer.ProcessedCount},
650-
{ "totalItemsCount", progressTimer.TotalCount },
651-
{ "totalProcessedTime", progressTimer.TotalProcessedTime },
652-
{ "average", progressTimer.AverageTime },
653-
{ "remaining", progressTimer.RemainingTime }
686+
{"average", average},
687+
{"remaining", remaining}
654688
});
689+
activity?.Stop();
655690
activity?.SetStatus(ActivityStatusCode.Error);
656691
activity?.SetTag("http.response.status_code", "200");
657692

src/MigrationTools.Clients.TfsObjectModel/Tools/TfsEmbededImagesTool.cs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,17 @@
11
using System;
22
using System.Collections.Generic;
33
using System.IO;
4-
using System.Linq.Expressions;
54
using System.Net;
65
using System.Net.Http;
76
using System.Net.Http.Headers;
87
using System.Text.RegularExpressions;
9-
using System.Windows.Forms;
10-
using Microsoft.Extensions.DependencyInjection;
118
using Microsoft.Extensions.Logging;
129
using Microsoft.Extensions.Options;
1310
using Microsoft.TeamFoundation.Client;
1411
using Microsoft.TeamFoundation.WorkItemTracking.Client;
1512
using Microsoft.TeamFoundation.WorkItemTracking.WebApi;
16-
using MigrationTools._EngineV1.Configuration;
1713
using MigrationTools.DataContracts;
1814
using MigrationTools.Endpoints;
19-
using MigrationTools.Processors;
2015
using MigrationTools.Processors.Infrastructure;
2116
using MigrationTools.Tools.Infrastructure;
2217

@@ -28,8 +23,8 @@ public class TfsEmbededImagesTool : EmbededImagesRepairToolBase<TfsEmbededImages
2823
private const string RegexPatternForImageFileName = "(?<=FileName=)[^=]*";
2924
private const string TargetDummyWorkItemTitle = "***** DELETE THIS - Migration Tool Generated Dummy Work Item For TfsEmbededImagesTool *****";
3025

31-
private Project _targetProject;
32-
private TfsTeamProjectEndpointOptions _targetConfig;
26+
private Project _targetProject;
27+
private TfsTeamProjectEndpointOptions _targetConfig;
3328

3429
private readonly IDictionary<string, string> _cachedUploadedUrisBySourceValue;
3530

@@ -50,7 +45,7 @@ public int FixEmbededImages(TfsProcessor processor, WorkItemData sourceWorkItem,
5045
return 0;
5146
}
5247

53-
public void ProcessorExecutionEnd(TfsProcessor processor)
48+
public void ProcessorExecutionEnd(TfsProcessor processor)
5449
{
5550
_processor = processor;
5651
if (_targetDummyWorkItem != null)
@@ -159,9 +154,10 @@ private string UploadedAndRetrieveAttachmentLinkUrl(string matchedSourceUri, str
159154

160155
return attachRef.Url;
161156
}
162-
catch (Exception ex) {
157+
catch (Exception ex)
158+
{
163159
throw ex;
164-
}
160+
}
165161
finally
166162
{
167163
if (File.Exists(fullImageFilePath))
@@ -200,13 +196,13 @@ private Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models.AttachmentRefere
200196
});
201197

202198
var dummyWi = GetDummyWorkItem(wi.Type);
203-
var wii = httpClient.UpdateWorkItemAsync(payload, dummyWi.Id).GetAwaiter().GetResult();
199+
var wii = httpClient.UpdateWorkItemAsync(payload, dummyWi.Id, bypassRules: true).GetAwaiter().GetResult();
204200
if (wii != null)
205201
{
206202
payload[0].Operation = Microsoft.VisualStudio.Services.WebApi.Patch.Operation.Remove;
207203
payload[0].Path = "/relations/" + (wii.Relations.Count - 1);
208204
payload[0].Value = null;
209-
wii = httpClient.UpdateWorkItemAsync(payload, dummyWi.Id).GetAwaiter().GetResult();
205+
wii = httpClient.UpdateWorkItemAsync(payload, dummyWi.Id, bypassRules: true).GetAwaiter().GetResult();
210206
}
211207
else
212208
{
@@ -265,7 +261,8 @@ private WorkItem GetDummyWorkItem(WorkItemType type = null)
265261
if (_targetDummyWorkItem.Id == 0)
266262
{
267263
throw new Exception("The Dummy work Item cant be created due to a save failure. This is likley due to required fields on the Task or First work items type.");
268-
} else
264+
}
265+
else
269266
{
270267
Log.LogDebug("TfsEmbededImagesTool: Dummy workitem {id} created on the target collection.", _targetDummyWorkItem.Id);
271268
//_targetProject.Store.DestroyWorkItems(new List<int> { _targetDummyWorkItem.Id });

src/MigrationTools.Clients.TfsObjectModel/Tools/TfsValidateRequiredFieldTool.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@
44
using Microsoft.Extensions.DependencyInjection;
55
using Microsoft.Extensions.Logging;
66
using Microsoft.Extensions.Options;
7-
using Microsoft.TeamFoundation.TestManagement.WebApi;
87
using Microsoft.TeamFoundation.WorkItemTracking.Client;
98
using MigrationTools.DataContracts;
10-
using MigrationTools.Enrichers;
11-
using MigrationTools.Processors;
129
using MigrationTools.Processors.Infrastructure;
1310
using MigrationTools.Tools.Infrastructure;
1411
using MigrationTools.Tools.Interfaces;
@@ -56,7 +53,7 @@ public bool ValidatingRequiredField(TfsProcessor processor, string fieldToFind,
5653
}
5754
catch (WorkItemTypeDeniedOrNotExistException ex)
5855
{
59-
Log.LogWarning(ex, "ValidatingRequiredField: Unable to validate one of the work items as its returned by TFS but has been deleted");
56+
Log.LogWarning(ex, "ValidatingRequiredField: Unable to validate work item type {name} as its returned by the source but does not exist in the target", sourceWorkItemType.Name);
6057
}
6158

6259
}
@@ -81,4 +78,4 @@ public bool ValidatingRequiredField(TfsProcessor processor, string fieldToFind,
8178
// }
8279
//}
8380
}
84-
}
81+
}

0 commit comments

Comments
 (0)