Enforce case-insensitive behaviour for matching and replacement in TfsNodeStructureTool#3130
Conversation
…test Agent-Logs-Url: https://github.com/nkdAgility/azure-devops-migration-tools/sessions/fdf18f43-12a5-4009-a641-35a9a4e7ba08 Co-authored-by: MrHinsh <5205575+MrHinsh@users.noreply.github.com>
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://blue-river-093197403-3130.westeurope.5.azurestaticapps.net |
There was a problem hiding this comment.
Pull request overview
Fixes a bug in TfsNodeStructureTool.GetNewNodeName where mapping rules could match case-insensitively but fail to apply because the actual replacements were case-sensitive.
Changes:
- Apply
RegexOptions.IgnoreCaseconsistently to both mapper-rule and last-resortRegex.Replacecalls. - Add an L0 unit test verifying that a mixed-case mapping rule correctly transforms a differently-cased source path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/MigrationTools.Clients.TfsObjectModel/Tools/TfsNodeStructureTool.cs | Makes regex replacement behavior case-insensitive to match the existing case-insensitive matching behavior. |
| src/MigrationTools.Clients.TfsObjectModel.Tests/Tools/TfsNodeStructureTests.cs | Adds a regression test covering case-insensitive replacement for node path mappings. |
| Log.LogDebug("NodeStructureEnricher.GetNewNodeName::Mappers::{key}", mapper.Match); | ||
| if (Regex.IsMatch(sourceNodePath, mapper.Match, RegexOptions.IgnoreCase)) | ||
| { | ||
| Log.LogDebug("NodeStructureEnricher.GetNewNodeName::Mappers::{key}::Match", mapper.Match); | ||
| string replacement = Regex.Replace(sourceNodePath, mapper.Match, mapper.Replacement); | ||
| string replacement = Regex.Replace(sourceNodePath, mapper.Match, mapper.Replacement, RegexOptions.IgnoreCase); |
There was a problem hiding this comment.
The mapping regex patterns (mapper.Match) come from configuration. Using Regex.IsMatch/Regex.Replace without a match timeout can allow pathological patterns to cause excessive CPU (catastrophic backtracking) during migrations. Consider using the Regex.* overloads that accept a TimeSpan matchTimeout (for both IsMatch and Replace) and handling RegexMatchTimeoutException to surface a clear configuration error.
TfsNodeStructureTool.GetNewNodeNamematched mapping rules withRegexOptions.IgnoreCasebut performed the replacement without it — causing silent failures where a rule matched but the transformation was never applied when casing differed.Changes
TfsNodeStructureTool.cs: AddedRegexOptions.IgnoreCaseto bothRegex.Replacecalls inGetNewNodeName:IsMatchhad it)TfsNodeStructureTests.cs: AddedGetNewNodeName_WithCaseDifferenceInSourcePath_AppliesMappingConsistently— verifies that a lowercase source path is correctly transformed by a mixed-caseMatchpattern.Things to be aware of
"My message that contains {item} and {item2}", item, item2! Do not use$"My message that contains {item} and {item2}"to pass text into the log strings as this disables Serilog's ability to pass that data as telemetry to Application Insights and for log highlighting.Type of change
How Has This Been Tested?
GetNewNodeName_WithCaseDifferenceInSourcePath_AppliesMappingConsistently— new unit test covering the case-insensitive replacement pathTfsNodeStructureToolTestsL0/L1 tests confirm no regression in normal-casing scenariosChecklist: