Merge pull request #833 from Project-MONAI/AC-2258

neildsouth · web-flow · commit e3daeb7dc158 · 2023-06-07T11:40:31.000+01:00
fixing email validation
diff --git a/src/Shared/Shared/Utilities/DicomTagUtilities.cs b/src/Shared/Shared/Utilities/DicomTagUtilities.cs
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+using System.Text.RegularExpressions;
 using FellowOakDicom;
 
 namespace Monai.Deploy.WorkflowManager.Shared.Utilities
@@ -22,7 +23,7 @@ public static class DicomTagUtilities
     {
         public static DicomTag GetDicomTagByName(string tag)
         {
-            return DicomDictionary.Default[tag];
+            return DicomDictionary.Default[tag] ?? DicomDictionary.Default[Regex.Replace(tag, @"\s+", "", RegexOptions.None, TimeSpan.FromSeconds(1))];
         }
 
         public static (bool valid, IList<string> invalidTags) DicomTagsValid(IEnumerable<string> dicomTags)
diff --git a/src/WorkflowManager/WorkflowManager/Validators/WorkflowValidator.cs b/src/WorkflowManager/WorkflowManager/Validators/WorkflowValidator.cs
@@ -457,7 +457,7 @@ private void ValidateEmailTask(TaskObject currentTask)
             if (emailsSpecified)
             {
                 var emails = currentTask.Args[RecipientEmails] ?? string.Empty;
-                var formattedEmails = emails.Split(',').Where(e => !string.IsNullOrWhiteSpace(e.Trim()));
+                var formattedEmails = emails.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
 
                 if (!formattedEmails.Any())
                 {
@@ -498,7 +498,7 @@ private void ValidateEmailTask(TaskObject currentTask)
             if (rolesSpecified)
             {
                 var roles = currentTask.Args[RecipientRoles] ?? string.Empty;
-                var formattedRoles = roles.Split(',').Where(r => !string.IsNullOrWhiteSpace(r.Trim()));
+                var formattedRoles = roles.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
 
                 if (!formattedRoles.Any())
                 {
@@ -514,15 +514,15 @@ private void ValidateEmailTask(TaskObject currentTask)
             }
 
             var metadataValues = currentTask.Args[MetadataValues] ?? string.Empty;
-            var formattedMetadataValues = metadataValues.Split(',').Where(m => !string.IsNullOrWhiteSpace(m.Trim()));
+            var formattedMetadataValues = metadataValues.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
 
             if (!formattedMetadataValues.Any())
             {
                 Errors.Add($"Argument '{MetadataValues}' for task {currentTask.Id} must be a comma seperated list of DICOM metadata tag names.");
                 return;
             }
 
-            var disallowedTags = _options.Value.DicomTagsDisallowed.Split(',').Select(t => t.Trim());
+            var disallowedTags = _options.Value.DicomTagsDisallowed.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
             var intersect = formattedMetadataValues.Intersect(disallowedTags);
 
             if (intersect.Any())
diff --git a/tests/UnitTests/WorkflowManager.Tests/Validators/WorkflowValidatorTests.cs b/tests/UnitTests/WorkflowManager.Tests/Validators/WorkflowValidatorTests.cs
@@ -15,6 +15,7 @@
  */
 
 using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -722,6 +723,100 @@ public async Task ValidateWorkflow_ValidatesEmptyWorkflow_ReturnsErrorsAndHasCor
             Assert.Contains(error4, errors);
         }
 
+        //"metadata_values": "Study Instance UID, Series Instance UID"
+        [Fact]
+        public async Task ValidateWorkflow_ValidateWorkflow_WithPluginArgs_ReturnsNoErrors()
+        {
+            var workflow = new Workflow
+            {
+                Name = "Workflowname1",
+                Description = "Workflowdesc1",
+                Version = "1",
+                InformaticsGateway = new InformaticsGateway
+                {
+                    AeTitle = "aetitle",
+                    ExportDestinations = new string[] { "oneDestination", "twoDestination", "threeDestination" }
+                },
+                Tasks = new TaskObject[]
+                {
+                    new TaskObject
+                    {
+                        Id = "rootTask",
+                        Type = "router",
+                        Description = "TestDesc",
+                        TaskDestinations = new TaskDestination[]
+                        {
+                            new TaskDestination
+                            {
+                                Name = "EmailTask"
+                            }
+                        },
+                        ExportDestinations = new ExportDestination[]
+                        {
+                            new ExportDestination { Name = "oneDestination" },
+                            new ExportDestination { Name = "twoDestination" },
+                        },
+                        Artifacts = new ArtifactMap
+                        {
+                            Output = new Artifact[]
+                            {
+                                new Artifact
+                                {
+                                    Name = "non_unique_artifact",
+                                    Mandatory = true,
+                                    Value = "Example Value"
+                                }
+                            }
+                        }
+                    },
+
+                    #region SuccessfulTasksPath
+
+                    new TaskObject
+                    {
+
+                        Id = "EmailTask",
+                        Type = "email",
+                        Description = "Email plugin Task 1",
+                        Artifacts = new ArtifactMap{ Input = new Artifact[] {
+                                new Artifact
+                                {
+                                    Name = "ExampleArtifact",
+                                    Value = "{{ context.value.EmailTask_InvalidEmailsArg.artifact.test }}"
+                                },
+                        } },
+                        Args = new Dictionary<string, string>{
+                            { "metadata_values", "Study Instance UID, Series Instance UID"},
+                            { "workflow_name", "Workflow Name"},
+                            { "recipient_emails", "neil.south@blah.com"}
+                        },
+                        TaskDestinations = new TaskDestination[] {
+                            new TaskDestination
+                            {
+                                Name = "taskdesc2"
+                            }
+                        }
+                    },
+                    new TaskObject
+                    {
+                        Id = "taskdesc2",
+                        Type = "router",
+                        Description = "TestDesc",
+                        TaskDestinations = Array.Empty<TaskDestination>()
+                    }
+                    #endregion SuccessfulTasksPath
+                }
+            };
+
+            _workflowService.Setup(w => w.GetByNameAsync(It.IsAny<string>()))
+                .ReturnsAsync(null, TimeSpan.FromSeconds(.1));
+
+            var errors = await _workflowValidator.ValidateWorkflow(workflow);
+
+            Assert.True(errors.Count == 0);
+
+        }
+
         [Fact]
         public async Task ValidateWorkflow_ValidateWorkflow_ReturnsNoErrors()
         {

Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,7 @@`
`14`	`14`	`* limitations under the License.`
`15`	`15`	`*/`
`16`	`16`
	`17`	`+using System.Text.RegularExpressions;`
`17`	`18`	`using FellowOakDicom;`
`18`	`19`
`19`	`20`	`namespace Monai.Deploy.WorkflowManager.Shared.Utilities`
`@@ -22,7 +23,7 @@ public static class DicomTagUtilities`
`22`	`23`	`{`
`23`	`24`	`public static DicomTag GetDicomTagByName(string tag)`
`24`	`25`	`{`
`25`		`- return DicomDictionary.Default[tag];`
	`26`	`+ return DicomDictionary.Default[tag] ?? DicomDictionary.Default[Regex.Replace(tag, @"\s+", "", RegexOptions.None, TimeSpan.FromSeconds(1))];`
`26`	`27`	`}`
`27`	`28`
`28`	`29`	`public static (bool valid, IList<string> invalidTags) DicomTagsValid(IEnumerable<string> dicomTags)`
Original file line number	Diff line number	Diff line change
`@@ -457,7 +457,7 @@ private void ValidateEmailTask(TaskObject currentTask)`
`457`	`457`	`if (emailsSpecified)`
`458`	`458`	`{`
`459`	`459`	`var emails = currentTask.Args[RecipientEmails] ?? string.Empty;`
`460`		`- var formattedEmails = emails.Split(',').Where(e => !string.IsNullOrWhiteSpace(e.Trim()));`
	`460`	`+ var formattedEmails = emails.Split(',', StringSplitOptions.TrimEntries \| StringSplitOptions.RemoveEmptyEntries);`
`461`	`461`
`462`	`462`	`if (!formattedEmails.Any())`
`463`	`463`	`{`
`@@ -498,7 +498,7 @@ private void ValidateEmailTask(TaskObject currentTask)`
`498`	`498`	`if (rolesSpecified)`
`499`	`499`	`{`
`500`	`500`	`var roles = currentTask.Args[RecipientRoles] ?? string.Empty;`
`501`		`- var formattedRoles = roles.Split(',').Where(r => !string.IsNullOrWhiteSpace(r.Trim()));`
	`501`	`+ var formattedRoles = roles.Split(',', StringSplitOptions.TrimEntries \| StringSplitOptions.RemoveEmptyEntries);`
`502`	`502`
`503`	`503`	`if (!formattedRoles.Any())`
`504`	`504`	`{`
`@@ -514,15 +514,15 @@ private void ValidateEmailTask(TaskObject currentTask)`
`514`	`514`	`}`
`515`	`515`
`516`	`516`	`var metadataValues = currentTask.Args[MetadataValues] ?? string.Empty;`
`517`		`- var formattedMetadataValues = metadataValues.Split(',').Where(m => !string.IsNullOrWhiteSpace(m.Trim()));`
	`517`	`+ var formattedMetadataValues = metadataValues.Split(',', StringSplitOptions.TrimEntries \| StringSplitOptions.RemoveEmptyEntries);`
`518`	`518`
`519`	`519`	`if (!formattedMetadataValues.Any())`
`520`	`520`	`{`
`521`	`521`	`Errors.Add($"Argument '{MetadataValues}' for task {currentTask.Id} must be a comma seperated list of DICOM metadata tag names.");`
`522`	`522`	`return;`
`523`	`523`	`}`
`524`	`524`
`525`		`- var disallowedTags = _options.Value.DicomTagsDisallowed.Split(',').Select(t => t.Trim());`
	`525`	`+ var disallowedTags = _options.Value.DicomTagsDisallowed.Split(',', StringSplitOptions.TrimEntries \| StringSplitOptions.RemoveEmptyEntries);`
`526`	`526`	`var intersect = formattedMetadataValues.Intersect(disallowedTags);`
`527`	`527`
`528`	`528`	`if (intersect.Any())`