From ec755998c409d694f780655332891a0e230c44dd Mon Sep 17 00:00:00 2001 From: Jack Schofield Date: Thu, 22 Sep 2022 09:22:25 +0100 Subject: [PATCH 1/4] fix workflow validation error Signed-off-by: Jack Schofield Signed-off-by: RemakingEden Signed-off-by: Jack Schofield --- .../Extensions/ValidationExtensions.cs | 11 +++++++++++ .../PayloadListener/Extensions/WorkflowExtensions.cs | 7 +++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/WorkflowManager/PayloadListener/Extensions/ValidationExtensions.cs b/src/WorkflowManager/PayloadListener/Extensions/ValidationExtensions.cs index ba7653e90..e4c005f0a 100644 --- a/src/WorkflowManager/PayloadListener/Extensions/ValidationExtensions.cs +++ b/src/WorkflowManager/PayloadListener/Extensions/ValidationExtensions.cs @@ -16,6 +16,7 @@ using Ardalis.GuardClauses; using Monai.Deploy.Messaging.Events; +using Monai.Deploy.WorkflowManager.Contracts.Models; namespace Monai.Deploy.WorkflowManager.PayloadListener.Extensions { @@ -38,6 +39,16 @@ public static bool IsValid(this WorkflowRequestEvent workflowRequestMessage, out return valid; } + public static bool IsInformaticsGatewayNotNull(string source, InformaticsGateway informaticsGateway, IList validationErrors) + { + Guard.Against.NullOrWhiteSpace(source, nameof(source)); + + if (informaticsGateway is not null) return true; + + validationErrors?.Add($"'{nameof(informaticsGateway)}' cannot be null (source: {source})."); + return false; + } + public static bool IsAeTitleValid(string source, string aeTitle, IList validationErrors) { Guard.Against.NullOrWhiteSpace(source, nameof(source)); diff --git a/src/WorkflowManager/PayloadListener/Extensions/WorkflowExtensions.cs b/src/WorkflowManager/PayloadListener/Extensions/WorkflowExtensions.cs index 0cb5c6469..f61e26768 100644 --- a/src/WorkflowManager/PayloadListener/Extensions/WorkflowExtensions.cs +++ b/src/WorkflowManager/PayloadListener/Extensions/WorkflowExtensions.cs @@ -79,12 +79,11 @@ public static bool IsDescriptionValid(string source, string description, IList validationErrors = null) { Guard.Against.NullOrWhiteSpace(source, nameof(source)); - Guard.Against.Null(informaticsGateway, nameof(informaticsGateway)); var valid = true; - - valid &= ValidationExtensions.IsAeTitleValid(informaticsGateway.GetType().Name, informaticsGateway.AeTitle, validationErrors); - valid &= IsExportDestinationsValid(informaticsGateway.GetType().Name, informaticsGateway.ExportDestinations, validationErrors); + valid &= ValidationExtensions.IsInformaticsGatewayNotNull(source, informaticsGateway, validationErrors); + valid &= ValidationExtensions.IsAeTitleValid(nameof(informaticsGateway), informaticsGateway?.AeTitle, validationErrors); + valid &= IsExportDestinationsValid(nameof(informaticsGateway), informaticsGateway?.ExportDestinations, validationErrors); return valid; } From 0d923c5fe9a0d8bc46883e1188dbb1970f1c9f3e Mon Sep 17 00:00:00 2001 From: RemakingEden Date: Thu, 22 Sep 2022 10:15:16 +0100 Subject: [PATCH 2/4] Added tests for partially empty and empty workflow body Signed-off-by: RemakingEden Signed-off-by: Jack Schofield --- .../Features/WorkflowApi.feature | 2 ++ .../TestData/WorkflowObjectTestData.cs | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature b/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature index ec96f9470..6c5e5a43f 100644 --- a/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature +++ b/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature @@ -114,6 +114,8 @@ Scenario Outline: Update workflow with invalid details | /workflows/c86a437d-d026-4bdf-b1df-c7a6372b89e3 | Invalid_Workflow_0_Tasks | Missing Workflow Tasks | | /workflows/c86a437d-d026-4bdf-b1df-c7a6372b89e3 | Invalid_Workflow_Version_Null | Missing Workflow Version | | /workflows/c86a437d-d026-4bdf-b1df-c7a6372b89e3 | Invalid_Workflow_Version_Blank | Missing Workflow Version | + | /workflows/c86a437d-d026-4bdf-b1df-c7a6372b89e3 | Invalid_Workflow_Body_Object | 'informaticsGateway' cannot be null | + | /workflows/c86a437d-d026-4bdf-b1df-c7a6372b89e3 | Empty_Workflow_Body | '' is not a valid Workflow Description | @UpdateWorkflows diff --git a/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/TestData/WorkflowObjectTestData.cs b/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/TestData/WorkflowObjectTestData.cs index dd2834fd3..67b370c45 100644 --- a/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/TestData/WorkflowObjectTestData.cs +++ b/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/TestData/WorkflowObjectTestData.cs @@ -669,6 +669,21 @@ public static class WorkflowObjectsTestData } } }, + new WorkflowObjectTestData() + { + Name = "Invalid_Workflow_Body_Object", + Workflow = new Workflow() + { + Name = "Artifact 1", + Description = "Artifact 1", + Version = "" + } + }, + new WorkflowObjectTestData() + { + Name = "Empty_Workflow_Body", + Workflow = new Workflow() + }, }; } } From f4b1c53aa2b206b15f9eb0c17f037e4ddb795fce Mon Sep 17 00:00:00 2001 From: RemakingEden Date: Thu, 22 Sep 2022 10:16:56 +0100 Subject: [PATCH 3/4] Added test for POST Signed-off-by: RemakingEden Signed-off-by: Jack Schofield --- .../Features/WorkflowApi.feature | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature b/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature index 6c5e5a43f..79a976ca5 100644 --- a/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature +++ b/tests/IntegrationTests/WorkflowExecutor.IntegrationTests/Features/WorkflowApi.feature @@ -160,9 +160,11 @@ Scenario Outline: Add workflow with invalid details | Invalid_Workflow_TaskID_Content | Contains Invalid Characters. | | Invalid_Workflow_Unreferenced_Task | Found Task(s) without any task destinations to it | | Invalid_Workflow_Loopback_Task | Detected task convergence on path | - | Invalid_Workflow_0_Tasks | Missing Workflow Tasks | + | Invalid_Workflow_0_Tasks | Missing Workflow Tasks | | Invalid_Workflow_Version_Null | Missing Workflow Version | | Invalid_Workflow_Version_Blank | Missing Workflow Version | + | Invalid_Workflow_Body_Object | 'informaticsGateway' cannot be null | + | Empty_Workflow_Body | '' is not a valid Workflow Description | @DeleteWorkflows Scenario: Delete a workflow with one revision From 8877c5fd1f181c94dbd988cf9e81ded9a85b9043 Mon Sep 17 00:00:00 2001 From: Jack Schofield Date: Thu, 22 Sep 2022 11:27:18 +0100 Subject: [PATCH 4/4] add tests Signed-off-by: Jack Schofield --- .../Controllers/WorkflowsControllerTests.cs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/UnitTests/WorkflowManager.Tests/Controllers/WorkflowsControllerTests.cs b/tests/UnitTests/WorkflowManager.Tests/Controllers/WorkflowsControllerTests.cs index 387e6144d..62d9e3ca8 100644 --- a/tests/UnitTests/WorkflowManager.Tests/Controllers/WorkflowsControllerTests.cs +++ b/tests/UnitTests/WorkflowManager.Tests/Controllers/WorkflowsControllerTests.cs @@ -663,5 +663,43 @@ public void ValidateWorkflow_ValidatesAWorkflow_ReturnsTrueAndHasCorrectValidati WorkflowValidator.Reset(); } } + + [Fact] + public void ValidateWorkflow_ValidatesEmptyWorkflow_ReturnsTrueAndHasCorrectValidationResultsAsync() + { + for (var i = 0; i < 15; i++) + { + var workflow = new Workflow(); + + var workflowHasErrors = WorkflowValidator.ValidateWorkflow(workflow, out var results); + + Assert.True(workflowHasErrors); + + Assert.Equal(7, results.Errors.Count); + + var error1 = "'' is not a valid Workflow Description (source: Unnamed workflow)."; + Assert.Contains(error1, results.Errors); + + var error2 = "'informaticsGateway' cannot be null (source: Unnamed workflow)."; + Assert.Contains(error2, results.Errors); + + var error3 = "'' is not a valid AE Title (source: informaticsGateway)."; + Assert.Contains(error3, results.Errors); + + var error4 = "'' is not a valid Informatics Gateway - exportDestinations (source: informaticsGateway)."; + Assert.Contains(error4, results.Errors); + + var error5 = "Missing Workflow Name."; + Assert.Contains(error5, results.Errors); + + var error6 = "Missing Workflow Version."; + Assert.Contains(error6, results.Errors); + + var error7 = "Missing Workflow Tasks."; + Assert.Contains(error7, results.Errors); + + WorkflowValidator.Reset(); + } + } } }