add task id naming validation

guidelines/mwm-workflow-spec.md

@@ -122,7 +122,7 @@ All task objects can have these attributes:-
 
 | Property | Type | Description |
 |------|------|------|
-|id|str (50)|The id for this task. This should be unique within the current workflow.|
+|id|str (50)|The id for this task. This should be unique within the current workflow. Supported chars: Alphanumeric _ - |
 |description|str (2000)|A human readable task description|
 |type|str (2000)|The task type - this determines the plugin that will be used to execute the task. See [task types](#task-types) for supported tasks.|
 |timeout_minutes|number|How long the task is allowed to run before it's canceled|

src/WorkflowManager/Controllers/WorkflowsController.cs

@@ -26,8 +26,8 @@
 using Monai.Deploy.WorkflowManager.Contracts.Models;
 using Monai.Deploy.WorkflowManager.Contracts.Responses;
 using Monai.Deploy.WorkflowManager.Filter;
-using Monai.Deploy.WorkflowManager.PayloadListener.Extensions;
 using Monai.Deploy.WorkflowManager.Services;
+using Monai.Deploy.WorkflowManager.Validators;
 
 namespace Monai.Deploy.WorkflowManager.Controllers
 {
@@ -133,8 +133,9 @@ public async Task<IActionResult> GetAsync([FromRoute] string id)
         [HttpPost]
         public async Task<IActionResult> CreateAsync([FromBody] Workflow workflow)
         {
-            if (!workflow.IsValid(out var validationErrors))
+            if (WorkflowValidator.ValidateWorkflow(workflow, out var results))
             {
+                var validationErrors = string.Join(", ", results.Errors);
                 _logger.LogDebug($"{nameof(CreateAsync)} - Failed to validate {nameof(workflow)}: {validationErrors}");
 
                 return Problem($"Failed to validate {nameof(workflow)}: {string.Join(", ", validationErrors)}", $"/workflows", BadRequest);
@@ -171,8 +172,9 @@ public async Task<IActionResult> UpdateAsync([FromBody] Workflow workflow, [From
                 return Problem($"Failed to validate {nameof(id)}, not a valid guid", $"/workflows/{id}", BadRequest);
             }
 
-            if (!workflow.IsValid(out var validationErrors))
+            if (WorkflowValidator.ValidateWorkflow(workflow, out var results))
             {
+                var validationErrors = string.Join(", ", results.Errors);
                 _logger.LogDebug($"{nameof(UpdateAsync)} - Failed to validate {nameof(workflow)}: {validationErrors}");
 
                 return Problem($"Failed to validate {nameof(workflow)}: {string.Join(", ", validationErrors)}", $"/workflows/{id}", BadRequest);

src/WorkflowManager/Validators/WorkflowValidator.cs

@@ -16,6 +16,7 @@
 
 using System.Collections.Generic;
 using System.Linq;
+using System.Text.RegularExpressions;
 using Monai.Deploy.WorkflowManager.Contracts.Models;
 using Monai.Deploy.WorkflowManager.PayloadListener.Extensions;
 
@@ -147,6 +148,16 @@ private static void ValidateWorkflowSpec(Workflow workflow)
             {
                 Errors.Add("Missing Workflow Tasks.");
             }
+
+            var taskIds = workflow.Tasks.Select(t => t.Id);
+            var pattern = new Regex(@"^[a-zA-Z0-9-_]+$");
+            foreach (var taskId in taskIds)
+            {
+                if (pattern.IsMatch(taskId) is false)
+                {
+                    Errors.Add($"TaskId: {taskId} Contains Invalid Characters.");
+                }
+            }
         }
 
         private static void ValidateTask(TaskObject[] tasks, TaskObject currentTask, int iterationCount, List<string> paths = null)

tests/UnitTests/WorkflowManager.Tests/Controllers/WorkflowsControllerTests.cs

@@ -484,7 +484,7 @@ public async Task DeleteAsync_WorkflowsGivenInvalidId_ShouldBadRequest()
         [Fact]
         public void ValidateWorkflow_ValidatesAWorkflow_ReturnsTrueAndHasCorrectValidationResultsAsync()
         {
-            for (int i = 0; i < 15; i++)
+            for (var i = 0; i < 15; i++)
             {
                 var workflow =
                 new WorkflowRevision
@@ -626,6 +626,11 @@ public void ValidateWorkflow_ValidatesAWorkflow_ReturnsTrueAndHasCorrectValidati
                                 Id = "taskdesc3",
                                 Type = "type",
                                 Description = "taskdesc",
+                            },
+                            new TaskObject {
+                                Id = "task_de.sc3?",
+                                Type = "type",
+                                Description = "invalidid",
                             }
                         }
                     }
@@ -635,15 +640,15 @@ public void ValidateWorkflow_ValidatesAWorkflow_ReturnsTrueAndHasCorrectValidati
 
                 Assert.True(workflowHasErrors);
 
-                Assert.Equal(24, results.Errors.Count);
+                Assert.Equal(26, results.Errors.Count);
 
                 var successPath = "rootTask => taskSucessdesc1 => taskSucessdesc2";
                 Assert.Contains(successPath, results.SuccessfulPaths);
 
                 var expectedConvergenceError = "Detected task convergence on path: rootTask => taskdesc1 => taskdesc2 => ∞";
                 Assert.Contains(expectedConvergenceError, results.Errors);
 
-                var unreferencedTaskError = "Found Task(s) without any task destinations to it: taskdesc3";
+                var unreferencedTaskError = "Found Task(s) without any task destinations to it: taskdesc3,task_de.sc3?";
                 Assert.Contains(unreferencedTaskError, results.Errors);
 
                 var loopingTasksError = "Detected task convergence on path: rootTask => taskLoopdesc1 => taskLoopdesc2 => taskLoopdesc3 => taskLoopdesc4 => ∞";
@@ -652,6 +657,9 @@ public void ValidateWorkflow_ValidatesAWorkflow_ReturnsTrueAndHasCorrectValidati
                 var missingDestinationError = "Missing destination DoesNotExistDestination in task taskLoopdesc4";
                 Assert.Contains(missingDestinationError, results.Errors);
 
+                var invalidTaskId = "TaskId: task_de.sc3? Contains Invalid Characters.";
+                Assert.Contains(invalidTaskId, results.Errors);
+
                 WorkflowValidator.Reset();
             }
         }