From bac4352de001ef73241a9aa9f4704af5e82c3181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stano=20Gabo=20Pe=C5=A5ko?= Date: Tue, 3 Dec 2024 19:14:36 +0100 Subject: [PATCH] Refactor StringManipulator tool so it works just with string and not the field --- .../TfsWorkItemMigrationProcessor.cs | 5 +- .../Tools/MockStringManipulatorTool.cs | 11 +- .../StringManipulatorEnricherTests.cs | 141 ++++++++++++------ .../Interfaces/IStringManipulatorTool.cs | 10 +- .../Tools/StringManipulatorTool.cs | 66 ++++---- 5 files changed, 135 insertions(+), 98 deletions(-) diff --git a/src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs b/src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs index 1137cf602..57fef8a95 100644 --- a/src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs +++ b/src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs @@ -449,8 +449,9 @@ private void PopulateWorkItem(WorkItemData oldWorkItemData, WorkItemData newWork switch (f.FieldDefinition.FieldType) { case FieldType.String: - CommonTools.StringManipulator.ProcessorExecutionWithFieldItem(null, oldWorkItemData.Fields[f.ReferenceName]); - newWorkItem.Fields[f.ReferenceName].Value = oldWorkItemData.Fields[f.ReferenceName].Value; + string oldValue = oldWorkItem.Fields[f.ReferenceName].Value.ToString(); + string newValue = CommonTools.StringManipulator.ProcessString(oldValue); + newWorkItem.Fields[f.ReferenceName].Value = newValue; break; default: newWorkItem.Fields[f.ReferenceName].Value = oldWorkItem.Fields[f.ReferenceName].Value; diff --git a/src/MigrationTools.Shadows/Tools/MockStringManipulatorTool.cs b/src/MigrationTools.Shadows/Tools/MockStringManipulatorTool.cs index 480fb173a..62357242d 100644 --- a/src/MigrationTools.Shadows/Tools/MockStringManipulatorTool.cs +++ b/src/MigrationTools.Shadows/Tools/MockStringManipulatorTool.cs @@ -1,17 +1,10 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using MigrationTools.DataContracts; -using MigrationTools.Processors.Infrastructure; -using MigrationTools.Tools.Interfaces; +using MigrationTools.Tools.Interfaces; namespace MigrationTools.Tools.Shadows { public class MockStringManipulatorTool : IStringManipulatorTool { - public void ProcessorExecutionWithFieldItem(IProcessor processor, FieldItem fieldItem) + public string? ProcessString(string? value) { throw new NotImplementedException(); } diff --git a/src/MigrationTools.Tests/ProcessorEnrichers/StringManipulatorEnricherTests.cs b/src/MigrationTools.Tests/ProcessorEnrichers/StringManipulatorEnricherTests.cs index dc0445635..4f49b815b 100644 --- a/src/MigrationTools.Tests/ProcessorEnrichers/StringManipulatorEnricherTests.cs +++ b/src/MigrationTools.Tests/ProcessorEnrichers/StringManipulatorEnricherTests.cs @@ -2,12 +2,8 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.VisualStudio.TestTools.UnitTesting; using MigrationTools.DataContracts; -using MigrationTools.Endpoints; -using MigrationTools.Processors; -using MigrationTools.Tests; -using Microsoft.Extensions.Options; -using MigrationTools.Tools; using MigrationTools.Shadows; +using MigrationTools.Tools; namespace MigrationTools.ProcessorEnrichers.Tests { @@ -21,7 +17,7 @@ public void StringManipulatorTool_ConfigureTest() var options = new StringManipulatorToolOptions(); options.Enabled = true; options.MaxStringLength = 10; - options.Manipulators = new List + options.Manipulators = new List { new RegexStringManipulator { @@ -40,10 +36,10 @@ public void StringManipulatorTool_ConfigureTest() [TestMethod(), TestCategory("L1")] public void StringManipulatorTool_RegexTest() { - var options = new StringManipulatorToolOptions(); + var options = new StringManipulatorToolOptions(); options.Enabled = true; - options.MaxStringLength = 10; - options.Manipulators = new List + options.MaxStringLength = 10; + options.Manipulators = new List { new RegexStringManipulator { @@ -56,18 +52,10 @@ public void StringManipulatorTool_RegexTest() var x = GetStringManipulatorTool(options); - var fieldItem = new FieldItem - { - FieldType = "String", - internalObject = null, - ReferenceName = "Custom.Test", - Name = "Test", - Value = "Test" - }; - - x.ProcessorExecutionWithFieldItem(null, fieldItem); + string value = "Test"; + string? newValue = x.ProcessString(value); - Assert.AreEqual("Test 2", fieldItem.Value); + Assert.AreEqual("Test 2", newValue); } [TestMethod(), TestCategory("L1")] @@ -75,21 +63,13 @@ public void StringManipulatorTool_LengthShorterThanMaxTest() { var options = new StringManipulatorToolOptions(); options.Enabled = true; - options.MaxStringLength = 10; + options.MaxStringLength = 10; var x = GetStringManipulatorTool(options); - var fieldItem = new FieldItem - { - FieldType = "String", - internalObject = null, - ReferenceName = "Custom.Test", - Name = "Test", - Value = "Test" - }; - - x.ProcessorExecutionWithFieldItem(null, fieldItem); + string value = "Test"; + string? newValue = x.ProcessString(value); - Assert.AreEqual(4, fieldItem.Value.ToString().Length); + Assert.AreEqual(4, newValue.Length); } [TestMethod(), TestCategory("L1")] @@ -100,24 +80,93 @@ public void StringManipulatorTool_LengthLongerThanMaxTest() options.MaxStringLength = 10; var x = GetStringManipulatorTool(options); - var fieldItem = new FieldItem - { - FieldType = "String", - internalObject = null, - ReferenceName = "Custom.Test", - Name = "Test", - Value = "Test Test Test Test Test Test Test Test Test Test Test Test Test" - }; + string value = "Test Test Test Test Test Test Test Test Test Test Test Test Test"; + string? newValue = x.ProcessString(value); - x.ProcessorExecutionWithFieldItem(null, fieldItem); + Assert.AreEqual(10, newValue.Length); + } - Assert.AreEqual(10, fieldItem.Value.ToString().Length); + [DataTestMethod(), TestCategory("L1")] + [DataRow(null, null)] + [DataRow("", "")] + [DataRow("lorem", "lorem")] + public void StringManipulatorTool_Disabled(string? value, string? expected) + { + var options = new StringManipulatorToolOptions(); + options.Enabled = false; + options.MaxStringLength = 15; + options.Manipulators = new List + { + new RegexStringManipulator + { + Enabled = true, + Pattern = "(^.*$)", + Replacement = "$1 $1", + Description = "Test" + } + }; + var x = GetStringManipulatorTool(options); + + string? newValue = x.ProcessString(value); + Assert.AreEqual(expected, newValue); } - private static StringManipulatorTool GetStringManipulatorTool() + [DataTestMethod(), TestCategory("L1")] + [DataRow(null, null)] + [DataRow("", " ")] + [DataRow("lorem", "lorem lorem")] + [DataRow("lorem ipsum", "lorem ipsum lor")] + public void StringManipulatorTool_Process(string? value, string? expected) { - var options = new StringManipulatorToolOptions(); - return GetStringManipulatorTool(options); + var options = new StringManipulatorToolOptions(); + options.Enabled = true; + options.MaxStringLength = 15; + options.Manipulators = new List + { + new RegexStringManipulator + { + Enabled = true, + Pattern = "(^.*$)", + Replacement = "$1 $1", + Description = "Test" + } + }; + var x = GetStringManipulatorTool(options); + + string? newValue = x.ProcessString(value); + Assert.AreEqual(expected, newValue); + } + + [DataTestMethod(), TestCategory("L1")] + [DataRow(null, null)] + [DataRow("", " 1 2")] + [DataRow("lorem", "lorem 1 2")] + public void StringManipulatorTool_MultipleManipulators(string? value, string? expected) + { + var options = new StringManipulatorToolOptions(); + options.Enabled = true; + options.MaxStringLength = 15; + options.Manipulators = new List + { + new RegexStringManipulator + { + Enabled = true, + Pattern = "(^.*$)", + Replacement = "$1 1", + Description = "Add 1" + }, + new RegexStringManipulator + { + Enabled = true, + Pattern = "(^.*$)", + Replacement = "$1 2", + Description = "Add 2" + } + }; + var x = GetStringManipulatorTool(options); + + string? newValue = x.ProcessString(value); + Assert.AreEqual(expected, newValue); } private static StringManipulatorTool GetStringManipulatorTool(StringManipulatorToolOptions options) @@ -134,4 +183,4 @@ private static StringManipulatorTool GetStringManipulatorTool(StringManipulatorT return services.BuildServiceProvider().GetService(); } } -} \ No newline at end of file +} diff --git a/src/MigrationTools/Tools/Interfaces/IStringManipulatorTool.cs b/src/MigrationTools/Tools/Interfaces/IStringManipulatorTool.cs index 0c3b40e4b..4e1bf1a15 100644 --- a/src/MigrationTools/Tools/Interfaces/IStringManipulatorTool.cs +++ b/src/MigrationTools/Tools/Interfaces/IStringManipulatorTool.cs @@ -1,13 +1,7 @@ -using System; -using System.Collections.Generic; -using System.Text; -using MigrationTools.DataContracts; -using MigrationTools.Processors.Infrastructure; - -namespace MigrationTools.Tools.Interfaces +namespace MigrationTools.Tools.Interfaces { public interface IStringManipulatorTool { - void ProcessorExecutionWithFieldItem(IProcessor processor, FieldItem fieldItem); + string? ProcessString(string? value); } } diff --git a/src/MigrationTools/Tools/StringManipulatorTool.cs b/src/MigrationTools/Tools/StringManipulatorTool.cs index 1dcc0cebe..d9d78f89e 100644 --- a/src/MigrationTools/Tools/StringManipulatorTool.cs +++ b/src/MigrationTools/Tools/StringManipulatorTool.cs @@ -1,13 +1,8 @@ using System; using System.Collections.Generic; -using System.Text; using System.Text.RegularExpressions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using MigrationTools.DataContracts; -using MigrationTools.Enrichers; -using MigrationTools.Processors; -using MigrationTools.Processors.Infrastructure; using MigrationTools.Tools.Infrastructure; using MigrationTools.Tools.Interfaces; @@ -19,20 +14,27 @@ namespace MigrationTools.Tools public class StringManipulatorTool : Tool, IStringManipulatorTool { - public StringManipulatorTool(IOptions options, IServiceProvider services, ILogger logger, ITelemetryLogger telemetryLogger) - : base(options, services, logger, telemetryLogger) + public StringManipulatorTool( + IOptions options, + IServiceProvider services, + ILogger logger, + ITelemetryLogger telemetryLogger) + : base(options, services, logger, telemetryLogger) { } - public void ProcessorExecutionWithFieldItem(IProcessor processor, FieldItem fieldItem) + public string? ProcessString(string? value) { - Log.LogDebug("{WorkItemProcessorEnricher}::ProcessorExecutionWithFieldItem", GetType().Name); + const string logPrefix = nameof(StringManipulatorTool) + "::" + nameof(ProcessString); + Log.LogDebug(logPrefix); + + string result = value; if (!Options.Enabled) { - Log.LogDebug("{WorkItemProcessorEnricher}::ProcessorExecutionWithFieldItem::Disabled", GetType().Name); - return; + Log.LogDebug(logPrefix + "::Disabled"); + return result; } - if (fieldItem.FieldType == "String" && fieldItem.Value != null) + if (value is not null) { if (!HasManipulators()) { @@ -42,45 +44,43 @@ public void ProcessorExecutionWithFieldItem(IProcessor processor, FieldItem fiel { if (manipulator.Enabled) { - Log.LogDebug("{WorkItemProcessorEnricher}::ProcessorExecutionWithFieldItem::Running::{Description} with {pattern}", GetType().Name, manipulator.Description, manipulator.Pattern); - var originalValue = fieldItem.Value; - fieldItem.Value = Regex.Replace((string)fieldItem.Value, manipulator.Pattern, manipulator.Replacement); - Log.LogTrace("{WorkItemProcessorEnricher}::ProcessorExecutionWithFieldItem::Running::{Description}::Original::{@OriginalValue}", GetType().Name, manipulator.Description, originalValue); - Log.LogTrace("{WorkItemProcessorEnricher}::ProcessorExecutionWithFieldItem::Running::{Description}::New::{@fieldItemValue}", GetType().Name, manipulator.Description, fieldItem.Value); + Log.LogDebug(logPrefix + "::Running::{Description} with {pattern}", manipulator.Description, manipulator.Pattern); + string oldValue = result; + result = Regex.Replace(result, manipulator.Pattern, manipulator.Replacement); + Log.LogTrace(logPrefix + "::Running::{Description}::Original::{@oldValue}", manipulator.Description, oldValue); + Log.LogTrace(logPrefix + "::Running::{Description}::New::{@newValue}", manipulator.Description, result); } else { - Log.LogDebug("{WorkItemProcessorEnricher}::ProcessorExecutionWithFieldItem::Disabled::{Description}", GetType().Name, manipulator.Description); + Log.LogDebug(logPrefix + "::Disabled::{Description}", manipulator.Description); } } - if (HasStringTooLong(fieldItem)) + if (IsStringTooLong(result)) { - fieldItem.Value = fieldItem.Value.ToString().Substring(0, Math.Min(fieldItem.Value.ToString().Length, Options.MaxStringLength)); + result = result.Substring(0, Options.MaxStringLength); } } - + return result; } private void AddDefaultManipulator() { - if (Options.Manipulators == null) + if (Options.Manipulators is null) { Options.Manipulators = new List(); } - Options.Manipulators.Add(new RegexStringManipulator() { Enabled = true, Description = "Default: Removes invalid chars!", Pattern = "[^( -~)\n\r\t]+", Replacement = "" }); + Options.Manipulators.Add(new RegexStringManipulator() + { + Enabled = true, + Description = "Default: Removes invalid chars!", + Pattern = "[^( -~)\n\r\t]+", + Replacement = "" + }); } - private bool HasStringTooLong(FieldItem fieldItem) - { - return fieldItem.Value.ToString().Length > 0 && fieldItem.Value.ToString().Length > Options.MaxStringLength; - } + private bool IsStringTooLong(string value) => (value.Length > 0) && (value.Length > Options.MaxStringLength); - private bool HasManipulators() - { - return Options.Manipulators != null && Options.Manipulators.Count > 0; - } + private bool HasManipulators() => Options.Manipulators?.Count > 0; } - } -