From cf0629796555de0d53af65dd29d296a9507840e8 Mon Sep 17 00:00:00 2001 From: Neil South Date: Fri, 5 Apr 2024 11:51:57 +0100 Subject: [PATCH 01/12] refactor to match ticked field name predicate Signed-off-by: Neil South --- src/WorkflowManager/Contracts/Models/Workflow.cs | 4 ++-- .../WorkflowExecuter/Services/WorkflowExecuterService.cs | 4 ++-- .../Services/WorkflowExecuterServiceTests.cs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/WorkflowManager/Contracts/Models/Workflow.cs b/src/WorkflowManager/Contracts/Models/Workflow.cs index f09178c99..ab53b299d 100755 --- a/src/WorkflowManager/Contracts/Models/Workflow.cs +++ b/src/WorkflowManager/Contracts/Models/Workflow.cs @@ -39,8 +39,8 @@ public class Workflow [JsonProperty(PropertyName = "dataRetentionDays")] public int? DataRetentionDays { get; set; } = 3;// note. -1 = never delete - [JsonProperty(PropertyName = "conditions")] - public string[] Conditions { get; set; } = []; + [JsonProperty(PropertyName = "predicate")] + public string[] Predicate { get; set; } = []; } } diff --git a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs index a8779ea1c..361cdddae 100644 --- a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs +++ b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs @@ -1120,9 +1120,9 @@ private async Task ClinicalReviewTimeOutEvent(WorkflowInstance workflowIns // check if the conditionals allow the workflow to be created - if (workflow.Workflow.Conditions.Length != 0) + if (workflow.Workflow.Predicate.Length != 0) { - var conditionalMet = _conditionalParameterParser.TryParse(workflow.Workflow.Conditions, workflowInstance, out var resolvedConditional); + var conditionalMet = _conditionalParameterParser.TryParse(workflow.Workflow.Predicate, workflowInstance, out var resolvedConditional); if (conditionalMet is false) { return null; diff --git a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs index 3312894b9..c6697ccb5 100755 --- a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs +++ b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs @@ -3903,7 +3903,7 @@ public async Task ProcessPayload_With_Failing_Workflow_Conditional_Should_Not_Pr Description = "taskdesc" } ], - Conditions = ["{{ context.dicom.series.any('0010','0040') }} == 'lordge'"] + Predicate = ["{{ context.dicom.series.any('0010','0040') }} == 'lordge'"] } } }; @@ -3956,7 +3956,7 @@ public async Task ProcessPayload_With_Passing_Workflow_Conditional_Should_Procce Description = "taskdesc" } ], - Conditions = ["{{ context.dicom.series.any('0010','0040') }} == 'lordge'"] + Predicate = ["{{ context.dicom.series.any('0010','0040') }} == 'lordge'"] } } }; From 76cef11143c3dd03f805aecdc2b411d74d1cf9dc Mon Sep 17 00:00:00 2001 From: Neil South Date: Fri, 5 Apr 2024 12:23:04 +0100 Subject: [PATCH 02/12] adding info to docs Signed-off-by: Neil South --- guidelines/mwm-workflow-spec.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/guidelines/mwm-workflow-spec.md b/guidelines/mwm-workflow-spec.md index 275d1e470..0b3957a79 100644 --- a/guidelines/mwm-workflow-spec.md +++ b/guidelines/mwm-workflow-spec.md @@ -120,6 +120,8 @@ This is the top-level object in a workflow spec. It contains the following prope |description|Optional[str] (200)| |informatics_gateway|[InformaticsGateway](#informatics-gateway)| |tasks|list[[Task](#tasks)]| +|dataRetentionDays|int| +|predicate|string[]| The following is an example of the structure of a workflow. @@ -134,8 +136,9 @@ The following is an example of the structure of a workflow. ┗ tasks\     ┣ task1\     ┣ task2\ -    ┗ task3 - +    ┗ task3\ +┣ dataRetentionDays\ +┣ predicate [A detailed breakdown of predicate logic can be found here.](https://github.com/Project-MONAI/monai-deploy-workflow-manager/blob/develop/guidelines/mwm-conditionals.md) #### Examples @@ -159,6 +162,8 @@ An example of a workflow with two tasks: "ORTHANC" ] }, + "dataRetentionDays": -1, + "predicate" : [] "tasks": [ { "id": "mean-pixel-calc", From ba3ef1dc5db30e775ac456af0a84d9e1f9240d41 Mon Sep 17 00:00:00 2001 From: Neil South Date: Tue, 14 May 2024 10:50:16 +0100 Subject: [PATCH 03/12] adding new dailyStats endpont Signed-off-by: Neil South --- .../Models/ApplicationReviewStatus.cs | 26 ++++ .../Contracts/Models/ExecutionStatDTO.cs | 1 - .../Models/ExecutionStatDayOverview.cs | 39 +++++ .../ITaskExecutionStatsRepository.cs | 11 +- .../TaskExecutionStatsRepository.cs | 26 ++-- .../Controllers/TaskStatsController.cs | 141 +++++++++++++----- 6 files changed, 190 insertions(+), 54 deletions(-) create mode 100644 src/WorkflowManager/Contracts/Models/ApplicationReviewStatus.cs create mode 100644 src/WorkflowManager/Contracts/Models/ExecutionStatDayOverview.cs diff --git a/src/WorkflowManager/Contracts/Models/ApplicationReviewStatus.cs b/src/WorkflowManager/Contracts/Models/ApplicationReviewStatus.cs new file mode 100644 index 000000000..4c6279b6e --- /dev/null +++ b/src/WorkflowManager/Contracts/Models/ApplicationReviewStatus.cs @@ -0,0 +1,26 @@ +/* + * Copyright 2023 MONAI Consortium + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +namespace Monai.Deploy.WorkflowManager.Common.Contracts.Models +{ + public enum ApplicationReviewStatus + { + Approved, + Rejected, + Cancelled, + AwaitingReview + } +} diff --git a/src/WorkflowManager/Contracts/Models/ExecutionStatDTO.cs b/src/WorkflowManager/Contracts/Models/ExecutionStatDTO.cs index 80316be22..e402b4e49 100644 --- a/src/WorkflowManager/Contracts/Models/ExecutionStatDTO.cs +++ b/src/WorkflowManager/Contracts/Models/ExecutionStatDTO.cs @@ -36,5 +36,4 @@ public ExecutionStatDTO(ExecutionStats stats) public double ExecutionDurationSeconds { get; set; } public string Status { get; set; } = "Created"; } - } diff --git a/src/WorkflowManager/Contracts/Models/ExecutionStatDayOverview.cs b/src/WorkflowManager/Contracts/Models/ExecutionStatDayOverview.cs new file mode 100644 index 000000000..bfc109466 --- /dev/null +++ b/src/WorkflowManager/Contracts/Models/ExecutionStatDayOverview.cs @@ -0,0 +1,39 @@ +/* + * Copyright 2023 MONAI Consortium + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using Newtonsoft.Json; + +namespace Monai.Deploy.WorkflowManager.Common.Contracts.Models +{ + public class ExecutionStatDayOverview + { + [JsonProperty("date")] + public DateOnly Date { get; set; } + [JsonProperty("total_executions")] + public int TotalExecutions { get; set; } + [JsonProperty("total_failures")] + public int TotalFailures { get; set; } + [JsonProperty("total_approvals")] + public int TotalApprovals { get; set; } + [JsonProperty("total_rejections")] + public int TotalRejections { get; set; } + [JsonProperty("total_cancelled")] + public int TotalCancelled { get; set; } + [JsonProperty("total_awaiting_review")] + public int TotalAwaitingReview { get; set; } + } +} diff --git a/src/WorkflowManager/Database/Interfaces/ITaskExecutionStatsRepository.cs b/src/WorkflowManager/Database/Interfaces/ITaskExecutionStatsRepository.cs index 998f52eff..c55e87421 100644 --- a/src/WorkflowManager/Database/Interfaces/ITaskExecutionStatsRepository.cs +++ b/src/WorkflowManager/Database/Interfaces/ITaskExecutionStatsRepository.cs @@ -52,6 +52,15 @@ public interface ITaskExecutionStatsRepository /// Task UpdateExecutionStatsAsync(TaskCancellationEvent taskCanceledEvent, string workflowId, string correlationId); + /// + /// Returns all entries between the two given dates + /// + /// start of the range. + /// end of the range. + /// optional workflow id. + /// optional task id. + /// a collections of stats + Task> GetAllStatsAsync(DateTime startTime, DateTime endTime, string workflowId = "", string taskId = ""); /// /// Returns paged entries between the two given dates /// @@ -62,7 +71,7 @@ public interface ITaskExecutionStatsRepository /// optional workflow id. /// optional task id. /// a collections of stats - Task> GetStatsAsync(DateTime startTime, DateTime endTime, int pageSize = 10, int pageNumber = 1, string workflowId = "", string taskId = ""); + Task> GetStatsAsync(DateTime startTime, DateTime endTime, int? pageSize = 10, int? pageNumber = 1, string workflowId = "", string taskId = ""); /// /// Return the count of the entries with this status, or all if no status given. diff --git a/src/WorkflowManager/Database/Repositories/TaskExecutionStatsRepository.cs b/src/WorkflowManager/Database/Repositories/TaskExecutionStatsRepository.cs index d1b372f5b..777411797 100644 --- a/src/WorkflowManager/Database/Repositories/TaskExecutionStatsRepository.cs +++ b/src/WorkflowManager/Database/Repositories/TaskExecutionStatsRepository.cs @@ -19,7 +19,6 @@ using System.Linq; using System.Linq.Expressions; using System.Threading.Tasks; -using Ardalis.GuardClauses; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Monai.Deploy.Messaging.Events; @@ -40,11 +39,7 @@ public TaskExecutionStatsRepository( IOptions databaseSettings, ILogger logger) { - if (client == null) - { - throw new ArgumentNullException(nameof(client)); - } - + _ = client ?? throw new ArgumentNullException(nameof(client)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); var mongoDatabase = client.GetDatabase(databaseSettings.Value.DatabaseName, null); _taskExecutionStatsCollection = mongoDatabase.GetCollection("ExecutionStats", null); @@ -149,17 +144,24 @@ await _taskExecutionStatsCollection.UpdateOneAsync(o => } } - public async Task> GetStatsAsync(DateTime startTime, DateTime endTime, int pageSize = 10, int pageNumber = 1, string workflowId = "", string taskId = "") + public async Task> GetAllStatsAsync(DateTime startTime, DateTime endTime, string workflowId = "", string taskId = "") + { + return await GetStatsAsync(startTime, endTime, null, null, workflowId, taskId); + } + + public async Task> GetStatsAsync(DateTime startTime, DateTime endTime, int? pageSize = 10, int? pageNumber = 1, string workflowId = "", string taskId = "") { CreateFilter(startTime, endTime, workflowId, taskId, out var builder, out var filter); filter &= builder.Where(GetExecutedTasksFilter()); - var result = await _taskExecutionStatsCollection.Find(filter) - .Limit(pageSize) - .Skip((pageNumber - 1) * pageSize) - .ToListAsync(); - return result; + var result = _taskExecutionStatsCollection.Find(filter); + if (pageSize is not null) + { + result = result.Limit(pageSize).Skip((pageNumber - 1) * pageSize); + } + + return await result.ToListAsync(); } private static ExecutionStats ExposeExecutionStats(ExecutionStats taskExecutionStats, TaskExecution taskUpdateEvent) diff --git a/src/WorkflowManager/WorkflowManager/Controllers/TaskStatsController.cs b/src/WorkflowManager/WorkflowManager/Controllers/TaskStatsController.cs index 7a4fa2bc4..b96823f07 100644 --- a/src/WorkflowManager/WorkflowManager/Controllers/TaskStatsController.cs +++ b/src/WorkflowManager/WorkflowManager/Controllers/TaskStatsController.cs @@ -113,6 +113,59 @@ public async Task GetOverviewAsync([FromQuery] DateTime startTime } } + /// + /// Get execution daily stats for a given time period. + /// + /// TimeFiler defining start and end times, plus paging options. + /// WorkflowId if you want stats just for a given workflow. (both workflowId and TaskId must be given, if you give one). + /// a paged obect with all the stat details. + [ProducesResponseType(typeof(StatsPagedResponse>), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status500InternalServerError)] + [HttpGet("dailystats")] + public async Task GetDailyStatsAsync([FromQuery] TimeFilter filter, string workflowId = "") + { + SetUpFilter(filter, out var route, out var pageSize, out var validFilter); + + try + { + var allStats = await _repository.GetAllStatsAsync(filter.StartTime, filter.EndTime, workflowId, string.Empty); + var statsDto = allStats + .OrderBy(a => a.StartedUTC) + .GroupBy(s => s.StartedUTC.Date) + .Select(g => new ExecutionStatDayOverview + { + Date = DateOnly.FromDateTime(g.Key.Date), + TotalExecutions = g.Count(), + TotalFailures = g.Count(i => string.Compare(i.Status, "Failed", true) == 0), + TotalApprovals = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.Approved.ToString(), true) == 0), + TotalRejections = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.Rejected.ToString(), true) == 0), + TotalCancelled = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.Cancelled.ToString(), true) == 0), + TotalAwaitingReview = g.Count(i => string.Compare(i.Status, ApplicationReviewStatus.AwaitingReview.ToString(), true) == 0), + }); + + var pagedStats = statsDto.Skip((filter.PageNumber - 1) * pageSize).Take(pageSize); + + var res = CreateStatsPagedResponse(pagedStats, validFilter, statsDto.Count(), _uriService, route); + var (avgTotalExecution, avgArgoExecution) = await _repository.GetAverageStats(filter.StartTime, filter.EndTime, workflowId, string.Empty); + + res.PeriodStart = filter.StartTime; + res.PeriodEnd = filter.EndTime; + res.TotalExecutions = allStats.Count(); + res.TotalSucceeded = statsDto.Sum(s => s.TotalApprovals); + res.TotalFailures = statsDto.Sum(s => s.TotalFailures); + res.TotalInprogress = statsDto.Sum(s => s.TotalAwaitingReview); + res.AverageTotalExecutionSeconds = Math.Round(avgTotalExecution, 2); + res.AverageArgoExecutionSeconds = Math.Round(avgArgoExecution, 2); + + return Ok(res); + } + catch (Exception e) + { + _logger.GetStatsAsyncError(e); + return Problem($"Unexpected error occurred: {e.Message}", $"tasks/stats", InternalServerError); + } + } + /// /// Get execution stats for a given time period. /// @@ -133,63 +186,71 @@ public async Task GetStatsAsync([FromQuery] TimeFilter filter, st return Problem("Failed to validate ids, not a valid guid", "tasks/stats/", BadRequest); } - if (filter.EndTime == default) - { - filter.EndTime = DateTime.Now; - } + SetUpFilter(filter, out var route, out var pageSize, out var validFilter); - if (filter.StartTime == default) + try { - filter.StartTime = new DateTime(2023, 1, 1); - } - - var route = Request?.Path.Value ?? string.Empty; - var pageSize = filter.PageSize ?? Options.Value.EndpointSettings?.DefaultPageSize ?? 10; - var max = Options.Value.EndpointSettings?.MaxPageSize ?? 20; - var validFilter = new PaginationFilter(filter.PageNumber, pageSize, max); + var allStats = await _repository.GetStatsAsync(filter.StartTime, filter.EndTime, pageSize, filter.PageNumber, workflowId, taskId); + var statsDto = allStats + .OrderBy(a => a.StartedUTC) + .Select(s => new ExecutionStatDTO(s)); - try + var res = await GatherPagedStats(filter, workflowId, taskId, route, validFilter, statsDto); + return Ok(res); + } + catch (Exception e) { - workflowId ??= string.Empty; - taskId ??= string.Empty; - var allStats = _repository.GetStatsAsync(filter.StartTime, filter.EndTime, pageSize, filter.PageNumber, workflowId, taskId); + _logger.GetStatsAsyncError(e); + return Problem($"Unexpected error occurred: {e.Message}", $"tasks/stats", InternalServerError); + } + } - var successes = _repository.GetStatsStatusSucceededCountAsync(filter.StartTime, filter.EndTime, workflowId, taskId); + private async Task>> GatherPagedStats(TimeFilter filter, string workflowId, string taskId, string route, PaginationFilter validFilter, IEnumerable statsDto) + { + workflowId ??= string.Empty; + taskId ??= string.Empty; - var fails = _repository.GetStatsStatusFailedCountAsync(filter.StartTime, filter.EndTime, workflowId, taskId); + var successes = _repository.GetStatsStatusSucceededCountAsync(filter.StartTime, filter.EndTime, workflowId, taskId); - var rangeCount = _repository.GetStatsTotalCompleteExecutionsCountAsync(filter.StartTime, filter.EndTime, workflowId, taskId); + var fails = _repository.GetStatsStatusFailedCountAsync(filter.StartTime, filter.EndTime, workflowId, taskId); - var stats = _repository.GetAverageStats(filter.StartTime, filter.EndTime, workflowId, taskId); + var rangeCount = _repository.GetStatsTotalCompleteExecutionsCountAsync(filter.StartTime, filter.EndTime, workflowId, taskId); - var running = _repository.GetStatsStatusCountAsync(filter.StartTime, filter.EndTime, TaskExecutionStatus.Accepted.ToString(), workflowId, taskId); + var stats = _repository.GetAverageStats(filter.StartTime, filter.EndTime, workflowId, taskId); - await Task.WhenAll(allStats, fails, rangeCount, stats, running); + var running = _repository.GetStatsStatusCountAsync(filter.StartTime, filter.EndTime, TaskExecutionStatus.Accepted.ToString(), workflowId, taskId); - ExecutionStatDTO[] statsDto; + await Task.WhenAll(fails, rangeCount, stats, running); - statsDto = allStats.Result - .OrderBy(a => a.StartedUTC) - .Select(s => new ExecutionStatDTO(s)) - .ToArray(); + var res = CreateStatsPagedResponse(statsDto, validFilter, rangeCount.Result, _uriService, route); - var res = CreateStatsPagedResponse(statsDto, validFilter, rangeCount.Result, _uriService, route); + res.PeriodStart = filter.StartTime; + res.PeriodEnd = filter.EndTime; + res.TotalExecutions = rangeCount.Result; + res.TotalSucceeded = successes.Result; + res.TotalFailures = fails.Result; + res.TotalInprogress = running.Result; + res.AverageTotalExecutionSeconds = Math.Round(stats.Result.avgTotalExecution, 2); + res.AverageArgoExecutionSeconds = Math.Round(stats.Result.avgArgoExecution, 2); + return res; + } - res.PeriodStart = filter.StartTime; - res.PeriodEnd = filter.EndTime; - res.TotalExecutions = rangeCount.Result; - res.TotalSucceeded = successes.Result; - res.TotalFailures = fails.Result; - res.TotalInprogress = running.Result; - res.AverageTotalExecutionSeconds = Math.Round(stats.Result.avgTotalExecution, 2); - res.AverageArgoExecutionSeconds = Math.Round(stats.Result.avgArgoExecution, 2); - return Ok(res); + private void SetUpFilter(TimeFilter filter, out string route, out int pageSize, out PaginationFilter validFilter) + { + if (filter.EndTime == default) + { + filter.EndTime = DateTime.Now; } - catch (Exception e) + + if (filter.StartTime == default) { - _logger.GetStatsAsyncError(e); - return Problem($"Unexpected error occurred: {e.Message}", $"tasks/stats", InternalServerError); + filter.StartTime = new DateTime(2023, 1, 1); } + + route = Request?.Path.Value ?? string.Empty; + pageSize = filter.PageSize ?? Options.Value.EndpointSettings?.DefaultPageSize ?? 10; + var max = Options.Value.EndpointSettings?.MaxPageSize ?? 20; + validFilter = new PaginationFilter(filter.PageNumber, pageSize, max); } } } From 44697c0e81adcdf74cd8058dc5a5857d792af1b9 Mon Sep 17 00:00:00 2001 From: Neil South Date: Tue, 14 May 2024 11:22:04 +0100 Subject: [PATCH 04/12] adding some tests Signed-off-by: Neil South --- .../TaskExecutionStatsControllerTests.cs | 63 ++++++++++++++++++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs b/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs index 3a6f1ca50..b6a8c4e7f 100644 --- a/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs +++ b/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs @@ -44,6 +44,9 @@ public class ExecutionStatsControllerTests private readonly Mock _uriService; private readonly IOptions _options; private readonly ExecutionStats[] _executionStats; + private readonly DateTime _startTime; + + #pragma warning disable CS8602 // Dereference of a possibly null reference. #pragma warning disable CS8604 // Possible null reference argument. #pragma warning disable CS8600 // Converting null literal or possible null value to non-nullable type. @@ -55,18 +58,19 @@ public ExecutionStatsControllerTests() _uriService = new Mock(); StatsController = new TaskStatsController(_options, _uriService.Object, _logger.Object, _repo.Object); - var startTime = new DateTime(2023, 4, 4); + _startTime = new DateTime(2023, 4, 4); _executionStats = new ExecutionStats[] { new ExecutionStats { ExecutionId = Guid.NewGuid().ToString(), - StartedUTC = startTime, + StartedUTC = _startTime, WorkflowInstanceId= "workflow", TaskId = "task", }, }; _repo.Setup(w => w.GetStatsAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(_executionStats); + _repo.Setup(w => w.GetAllStatsAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(_executionStats); _repo.Setup(w => w.GetStatsStatusCountAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(_executionStats.Count()); _repo.Setup(w => w.GetStatsTotalCompleteExecutionsCountAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(_executionStats.Count()); } @@ -277,6 +281,61 @@ public async Task GetStatsAsync_Only_Find_Matching_Results() var pagegedResults = objectResult.Value as StatsPagedResponse>; Assert.Equal(1, pagegedResults.TotalRecords); } + + [Fact] + public async Task GetDailyStatsAsync_ReturnsList() + { + _uriService.Setup(s => s.GetPageUriString(It.IsAny(), It.IsAny())).Returns(() => "unitTest"); + + var result = await StatsController.GetDailyStatsAsync(new TimeFilter(), ""); + + var objectResult = Assert.IsType(result); + + var responseValue = (StatsPagedResponse>)objectResult.Value; + responseValue.Data.First().Date.Should().Be(DateOnly.FromDateTime( _startTime)); + responseValue.FirstPage.Should().Be("unitTest"); + responseValue.LastPage.Should().Be("unitTest"); + responseValue.PageNumber.Should().Be(1); + responseValue.PageSize.Should().Be(10); + responseValue.TotalPages.Should().Be(1); + responseValue.TotalRecords.Should().Be(1); + responseValue.Succeeded.Should().Be(true); + responseValue.PreviousPage.Should().Be(null); + responseValue.NextPage.Should().Be(null); + responseValue.Errors.Should().BeNullOrEmpty(); + } + + [Fact] + public async Task GetAllStatsAsync_ServiceException_ReturnProblem() + { + _repo.Setup(w => w.GetAllStatsAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).ThrowsAsync(new Exception()); + + var result = await StatsController.GetDailyStatsAsync(new TimeFilter(), ""); + + var objectResult = Assert.IsType(result); + Assert.Equal((int)HttpStatusCode.InternalServerError, objectResult.StatusCode); + + const string expectedInstance = "tasks/stats"; + Assert.StartsWith(expectedInstance, ((ProblemDetails)objectResult.Value).Instance); + } + + [Fact] + public async Task GetAllStatsAsync_Pass_All_Arguments_To_GetStatsAsync_In_Repo() + { + var startTime = new DateTime(2023, 4, 4); + var endTime = new DateTime(2023, 4, 5); + const int pageNumber = 15; + const int pageSize = 9; + + var result = await StatsController.GetDailyStatsAsync(new TimeFilter { StartTime = startTime, EndTime = endTime, PageNumber = pageNumber, PageSize = pageSize }, "workflow"); + + _repo.Verify(v => v.GetAllStatsAsync( + It.Is(d => d.Equals(startTime)), + It.Is(d => d.Equals(endTime)), + It.Is(s => s.Equals("workflow")), + It.Is(s => s.Equals(""))) + ); + } } #pragma warning restore CS8604 // Possible null reference argument. #pragma warning restore CS8602 // Dereference of a possibly null reference. From 47f0192a70538ddbb8f5d9d73aafe01ae2f27ec5 Mon Sep 17 00:00:00 2001 From: Neil South Date: Mon, 20 May 2024 12:24:20 +0100 Subject: [PATCH 05/12] fix for task update updating to same status Signed-off-by: Neil South --- .../Logging/Log.200000.Workflow.cs | 3 + .../Logging/Log.700000.Artifact.cs | 7 ++- .../Services/WorkflowExecuterService.cs | 8 ++- .../Services/WorkflowExecuterServiceTests.cs | 56 +++++++++++++++++++ 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/WorkflowManager/Logging/Log.200000.Workflow.cs b/src/WorkflowManager/Logging/Log.200000.Workflow.cs index 62527c0cc..d42d8cddb 100644 --- a/src/WorkflowManager/Logging/Log.200000.Workflow.cs +++ b/src/WorkflowManager/Logging/Log.200000.Workflow.cs @@ -84,6 +84,9 @@ public static partial class Log [LoggerMessage(EventId = 200020, Level = LogLevel.Warning, Message = "Use new ArtifactReceived Queue for continuation messages.")] public static partial void DontUseWorkflowReceivedForPayload(this ILogger logger); + [LoggerMessage(EventId = 200021, Level = LogLevel.Trace, Message = "The task execution status for task {taskId} is already {status}. Payload: {payloadId}")] + public static partial void TaskStatusUpdateNotNeeded(this ILogger logger, string payloadId, string taskId, string status); + // Conditions Resolver [LoggerMessage(EventId = 210000, Level = LogLevel.Warning, Message = "Failed to parse condition: {condition}. resolvedConditional: {resolvedConditional}")] public static partial void FailedToParseCondition(this ILogger logger, string resolvedConditional, string condition, Exception ex); diff --git a/src/WorkflowManager/Logging/Log.700000.Artifact.cs b/src/WorkflowManager/Logging/Log.700000.Artifact.cs index 246e14f2d..b90d6d1f3 100644 --- a/src/WorkflowManager/Logging/Log.700000.Artifact.cs +++ b/src/WorkflowManager/Logging/Log.700000.Artifact.cs @@ -60,12 +60,15 @@ public static partial class Log [LoggerMessage(EventId = 700012, Level = LogLevel.Error, Message = "Error finding Task :{taskId}")] public static partial void ErrorFindingTask(this ILogger logger, string taskId); - [LoggerMessage(EventId = 700013, Level = LogLevel.Error, Message = "Error finding Task :{taskId} or previousTask {previousTask}")] - public static partial void ErrorFindingTaskOrPrevious(this ILogger logger, string taskId, string previousTask); + //[LoggerMessage(EventId = 700013, Level = LogLevel.Error, Message = "Error finding Task :{taskId} or previousTask {previousTask}")] + //public static partial void ErrorFindingTaskOrPrevious(this ILogger logger, string taskId, string previousTask); [LoggerMessage(EventId = 700014, Level = LogLevel.Warning, Message = "Error Task :{taskId} cant be trigger as it has missing artifacts {missingtypesJson}")] public static partial void ErrorTaskMissingArtifacts(this ILogger logger, string taskId, string missingtypesJson); + [LoggerMessage(EventId = 700015, Level = LogLevel.Warning, Message = "Error Task :{taskId} cant be trigger as it has missing artifacts {artifactName}")] + public static partial void ErrorFindingArtifactInPrevious(this ILogger logger, string taskId, string artifactName); + } } diff --git a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs index 361cdddae..7ca2a1cec 100644 --- a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs +++ b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs @@ -376,7 +376,7 @@ private Dictionary GetTasksInput(WorkflowRevision workflowTe var matchType = previousTask.Artifacts.Output.FirstOrDefault(t => t.Name == artifact.Name); if (matchType is null) { - _logger.ErrorFindingTaskOrPrevious(taskId, previousTaskId); + _logger.ErrorFindingArtifactInPrevious(taskId, artifact.Name); } else { @@ -481,6 +481,12 @@ public async Task ProcessTaskUpdate(TaskUpdateEvent message) await ClinicalReviewTimeOutEvent(workflowInstance, currentTask, message.CorrelationId); } + if (message.Status == currentTask.Status) + { + _logger.TaskStatusUpdateNotNeeded(workflowInstance.PayloadId, message.TaskId, message.Status.ToString()); + return true; + } + if (!message.Status.IsTaskExecutionStatusUpdateValid(currentTask.Status)) { _logger.TaskStatusUpdateNotValid(workflowInstance.PayloadId, message.TaskId, currentTask.Status.ToString(), message.Status.ToString()); diff --git a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs index c6697ccb5..485aa6a99 100755 --- a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs +++ b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs @@ -3977,6 +3977,62 @@ public async Task ProcessPayload_With_Passing_Workflow_Conditional_Should_Procce Assert.True(result); } + + [Fact] + public async Task ProcessPayload_With_Empty_Workflow_Conditional_Should_Procced() + { + var workflowRequest = new WorkflowRequestEvent + { + Bucket = "testbucket", + DataTrigger = new DataOrigin { Source = "aetitle", Destination = "aetitle" }, + CorrelationId = Guid.NewGuid().ToString(), + Timestamp = DateTime.UtcNow + }; + + var workflows = new List + { + new() { + Id = Guid.NewGuid().ToString(), + WorkflowId = Guid.NewGuid().ToString(), + Revision = 1, + Workflow = new Workflow + { + Name = "Workflowname", + Description = "Workflowdesc", + Version = "1", + InformaticsGateway = new InformaticsGateway + { + AeTitle = "aetitle" + }, + Tasks = + [ + new TaskObject { + Id = Guid.NewGuid().ToString(), + Type = "type", + Description = "taskdesc" + } + ], + Predicate = [] + } + } + }; + + _dicom.Setup(w => w.GetAnyValueAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(() => "lordge"); + + _workflowRepository.Setup(w => w.GetWorkflowsByAeTitleAsync(It.IsAny>())).ReturnsAsync(workflows); + _workflowRepository.Setup(w => w.GetWorkflowsForWorkflowRequestAsync(It.IsAny(), It.IsAny())).ReturnsAsync(workflows); + _workflowRepository.Setup(w => w.GetByWorkflowIdAsync(workflows[0].WorkflowId)).ReturnsAsync(workflows[0]); + _workflowInstanceRepository.Setup(w => w.CreateAsync(It.IsAny>())).ReturnsAsync(true); + _workflowInstanceRepository.Setup(w => w.GetByWorkflowsIdsAsync(It.IsAny>())).ReturnsAsync(new List()); + _workflowInstanceRepository.Setup(w => w.UpdateTaskStatusAsync(It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(true); + + var result = await WorkflowExecuterService.ProcessPayload(workflowRequest, new Payload() { Id = Guid.NewGuid().ToString() }); + + _messageBrokerPublisherService.Verify(w => w.Publish(_configuration.Value.Messaging.Topics.TaskDispatchRequest, It.IsAny()), Times.Once()); + + Assert.True(result); + } } #pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. } From 4193ed39e7aa8e9c7992ce8344e9846e0d5717a6 Mon Sep 17 00:00:00 2001 From: Neil South Date: Mon, 20 May 2024 14:38:20 +0100 Subject: [PATCH 06/12] adding test for same statues Signed-off-by: Neil South --- .../Services/WorkflowExecuterServiceTests.cs | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs index 485aa6a99..966404893 100755 --- a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs +++ b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs @@ -3702,6 +3702,88 @@ public async Task ProcessTaskUpdate_ValidTaskUpdateEventWithExportHl7TaskDestina response.Should().BeTrue(); } + [Fact] + public async Task ProcessTaskUpdate_ValidTaskUpdateEventWith_Same_Status_returns_true() + { + var workflowInstanceId = Guid.NewGuid().ToString(); + var taskId = Guid.NewGuid().ToString(); + + var updateEvent = new TaskUpdateEvent + { + WorkflowInstanceId = workflowInstanceId, + TaskId = "pizza", + ExecutionId = Guid.NewGuid().ToString(), + Status = TaskExecutionStatus.Succeeded, + }; + + var workflowId = Guid.NewGuid().ToString(); + + var workflow = new WorkflowRevision + { + Id = Guid.NewGuid().ToString(), + WorkflowId = workflowId, + Revision = 1, + Workflow = new Workflow + { + Name = "Workflowname2", + Description = "Workflowdesc2", + Version = "1", + InformaticsGateway = new InformaticsGateway + { + AeTitle = "aetitle" + }, + Tasks = new TaskObject[] + { + new TaskObject { + Id = "pizza", + Type = "type", + Description = "taskdesc", + TaskDestinations = new TaskDestination[] + { + new TaskDestination + { + Name = "exporttaskid" + }, + } + } + } + } + }; + + var workflowInstance = new WorkflowInstance + { + Id = workflowInstanceId, + WorkflowId = workflowId, + WorkflowName = workflow.Workflow.Name, + PayloadId = Guid.NewGuid().ToString(), + Status = Status.Created, + BucketId = "bucket", + Tasks = new List + { + new TaskExecution + { + TaskId = "pizza", + Status = TaskExecutionStatus.Succeeded + } + } + }; + + _workflowInstanceRepository.Setup(w => w.UpdateTaskStatusAsync(It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(true); + _workflowInstanceRepository.Setup(w => w.GetByWorkflowInstanceIdAsync(workflowInstance.Id)).ReturnsAsync(workflowInstance); + _workflowInstanceRepository.Setup(w => w.UpdateTasksAsync(workflowInstance.Id, It.IsAny>())).ReturnsAsync(true); + _workflowRepository.Setup(w => w.GetByWorkflowIdAsync(workflowInstance.WorkflowId)).ReturnsAsync(workflow); + _payloadService.Setup(p => p.GetByIdAsync(It.IsAny())).ReturnsAsync(new Payload { PatientDetails = new PatientDetails { } }); + _artifactMapper.Setup(a => a.ConvertArtifactVariablesToPath(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(new Dictionary { { "dicomexport", "/dcm" } }); + + var response = await WorkflowExecuterService.ProcessTaskUpdate(updateEvent); + + _messageBrokerPublisherService.Verify(w => w.Publish(_configuration.Value.Messaging.Topics.ExportHL7, It.IsAny()), Times.Exactly(0)); + + _logger.Verify(logger => logger.IsEnabled(LogLevel.Trace),Times.Once); + + response.Should().BeTrue(); + } + [Fact] public async Task ProcessPayload_With_Multiple_Taskdestinations_One_Has_Inputs() { From 73e6d374030646dbc66b466f11bf8c981c17fb09 Mon Sep 17 00:00:00 2001 From: Neil South Date: Wed, 22 May 2024 16:48:30 +0100 Subject: [PATCH 07/12] adding triggered workflow names to payload Signed-off-by: Neil South --- .../Common/Interfaces/IPayloadService.cs | 8 ++++ .../Common/Services/PayloadService.cs | 7 ++++ .../M006_Payload_triggeredWorkflows.cs | 42 +++++++++++++++++++ .../Contracts/Models/Payload.cs | 13 +++--- .../Services/EventPayloadRecieverService.cs | 5 +++ .../Services/WorkflowExecuterService.cs | 6 ++- .../Services/WorkflowExecuterServiceTests.cs | 42 +++++++++++++++++++ 7 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 src/WorkflowManager/Contracts/Migrations/M006_Payload_triggeredWorkflows.cs diff --git a/src/WorkflowManager/Common/Interfaces/IPayloadService.cs b/src/WorkflowManager/Common/Interfaces/IPayloadService.cs index 6c28f99a0..03c145236 100644 --- a/src/WorkflowManager/Common/Interfaces/IPayloadService.cs +++ b/src/WorkflowManager/Common/Interfaces/IPayloadService.cs @@ -62,5 +62,13 @@ Task> GetAllAsync(int? skip = null, /// /// date of expiry or null Task GetExpiry(DateTime now, string? workflowInstanceId); + + /// + /// Updates a payload + /// + /// payload id to update. + /// updated payload. + /// true if the update is successful, false otherwise. + Task UpdateAsync(Payload payload); } } diff --git a/src/WorkflowManager/Common/Services/PayloadService.cs b/src/WorkflowManager/Common/Services/PayloadService.cs index 7ceeebd0b..8d0b4ae1f 100644 --- a/src/WorkflowManager/Common/Services/PayloadService.cs +++ b/src/WorkflowManager/Common/Services/PayloadService.cs @@ -259,5 +259,12 @@ public async Task UpdateWorkflowInstanceIdsAsync(string payloadId, IEnumer return false; } } + + public Task UpdateAsync(Payload payload) + { + ArgumentNullException.ThrowIfNull(payload, nameof(payload)); + + return _payloadRepository.UpdateAsync(payload); + } } } diff --git a/src/WorkflowManager/Contracts/Migrations/M006_Payload_triggeredWorkflows.cs b/src/WorkflowManager/Contracts/Migrations/M006_Payload_triggeredWorkflows.cs new file mode 100644 index 000000000..c17df5075 --- /dev/null +++ b/src/WorkflowManager/Contracts/Migrations/M006_Payload_triggeredWorkflows.cs @@ -0,0 +1,42 @@ +// +// Copyright 2023 Guy’s and St Thomas’ NHS Foundation Trust +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using Monai.Deploy.WorkflowManager.Common.Contracts.Models; +using Mongo.Migration.Migrations.Document; +using MongoDB.Bson; + +namespace Monai.Deploy.WorkflowManager.Common.Contracts.Migrations +{ + public class M006_Payload_triggeredWorkflows : DocumentMigration + { + public M006_Payload_triggeredWorkflows() : base("1.0.6") { } + + public override void Up(BsonDocument document) + { + document.Add("TriggeredWorkflowNames", BsonNull.Create(null).ToJson(), true); + } + + public override void Down(BsonDocument document) + { + try + { + document.Remove("TriggeredWorkflowNames"); + } + catch + { // can ignore we dont want failures stopping startup ! + } + } + } +} diff --git a/src/WorkflowManager/Contracts/Models/Payload.cs b/src/WorkflowManager/Contracts/Models/Payload.cs index 83108e3a4..e1dd3fb52 100755 --- a/src/WorkflowManager/Contracts/Models/Payload.cs +++ b/src/WorkflowManager/Contracts/Models/Payload.cs @@ -27,7 +27,7 @@ namespace Monai.Deploy.WorkflowManager.Common.Contracts.Models { - [CollectionLocation("Payloads"), RuntimeVersion("1.0.5")] + [CollectionLocation("Payloads"), RuntimeVersion("1.0.6")] public class Payload : IDocument { [JsonConverter(typeof(DocumentVersionConvert)), BsonSerializer(typeof(DocumentVersionConverBson))] @@ -40,10 +40,13 @@ public class Payload : IDocument public string PayloadId { get; set; } = string.Empty; [JsonProperty(PropertyName = "workflows")] - public IEnumerable Workflows { get; set; } = new List(); + public IEnumerable Workflows { get; set; } = []; + + [JsonProperty(PropertyName = "workflow_names")] + public List TriggeredWorkflowNames { get; set; } = []; [JsonProperty(PropertyName = "workflow_instance_ids")] - public IEnumerable WorkflowInstanceIds { get; set; } = new List(); + public IEnumerable WorkflowInstanceIds { get; set; } = []; [JsonProperty(PropertyName = "file_count")] public int FileCount { get; set; } @@ -61,10 +64,10 @@ public class Payload : IDocument public PayloadDeleted PayloadDeleted { get; set; } = PayloadDeleted.No; [JsonProperty(PropertyName = "files")] - public IList Files { get; set; } = new List(); + public IList Files { get; set; } = []; [JsonProperty(PropertyName = "patient_details")] - public PatientDetails PatientDetails { get; set; } = new PatientDetails(); + public PatientDetails PatientDetails { get; set; } = new(); public DataOrigin DataTrigger { get; set; } = new DataOrigin { DataService = DataService.DIMSE }; diff --git a/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs b/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs index c1ca83402..7d125606e 100644 --- a/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs +++ b/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs @@ -92,6 +92,11 @@ public async Task ReceiveWorkflowPayload(MessageReceivedEventArgs message) return; } + + if (string.IsNullOrWhiteSpace(string.Join("", payload.TriggeredWorkflowNames)) is false) + { + await PayloadService.UpdateAsync(payload); + } } else { diff --git a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs index 361cdddae..1c6583386 100644 --- a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs +++ b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs @@ -153,14 +153,14 @@ public async Task ProcessPayload(WorkflowRequestEvent message, Payload pay return false; } - workflowInstances.AddRange(newInstances); + workflowInstances.AddRange(newInstances!); var existingInstances = await _workflowInstanceRepository.GetByWorkflowsIdsAsync(workflowInstances.Select(w => w.WorkflowId).ToList()); workflowInstances.RemoveAll(i => existingInstances.Any(e => e.WorkflowId == i.WorkflowId && e.PayloadId == i.PayloadId)); - if (workflowInstances.Any()) + if (workflowInstances.Count != 0) { processed &= await _workflowInstanceRepository.CreateAsync(workflowInstances); @@ -180,6 +180,8 @@ public async Task ProcessPayload(WorkflowRequestEvent message, Payload pay { await ProcessFirstWorkflowTask(workflowInstance, message.CorrelationId, payload); } + payload.WorkflowInstanceIds = workflowInstances.Select(w => w.Id).ToList(); + payload.TriggeredWorkflowNames = workflowInstances.Select(w => w.WorkflowName).ToList(); return true; } diff --git a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs index c6697ccb5..9413f5ba7 100755 --- a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs +++ b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs @@ -3977,6 +3977,48 @@ public async Task ProcessPayload_With_Passing_Workflow_Conditional_Should_Procce Assert.True(result); } + + [Fact] + public async Task ProcessPayload_Payload_Should_Include_triggered_workflow_names() + { + var workflowRequest = new WorkflowRequestEvent + { + Bucket = "testbucket", + DataTrigger = new DataOrigin { Source = "aetitle", Destination = "aetitle" }, + CorrelationId = Guid.NewGuid().ToString(), + Timestamp = DateTime.UtcNow + }; + + var workflows = new List + { + new() { + Id = Guid.NewGuid().ToString(), + WorkflowId = Guid.NewGuid().ToString(), + Revision = 1, + Workflow = new Workflow + { + Name = "Workflowname", + } + } + }; + + _dicom.Setup(w => w.GetAnyValueAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(() => "lordge"); + + _workflowRepository.Setup(w => w.GetWorkflowsByAeTitleAsync(It.IsAny>())).ReturnsAsync(workflows); + _workflowRepository.Setup(w => w.GetWorkflowsForWorkflowRequestAsync(It.IsAny(), It.IsAny())).ReturnsAsync(workflows); + _workflowRepository.Setup(w => w.GetByWorkflowIdAsync(workflows[0].WorkflowId)).ReturnsAsync(workflows[0]); + _workflowInstanceRepository.Setup(w => w.CreateAsync(It.IsAny>())).ReturnsAsync(true); + _workflowInstanceRepository.Setup(w => w.GetByWorkflowsIdsAsync(It.IsAny>())).ReturnsAsync(new List()); + _workflowInstanceRepository.Setup(w => w.UpdateTaskStatusAsync(It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(true); + var payload = new Payload() { Id = Guid.NewGuid().ToString() }; + var result = await WorkflowExecuterService.ProcessPayload(workflowRequest, payload); + + + Assert.Contains(workflows[0].Workflow!.Name, payload.TriggeredWorkflowNames); + Assert.Contains(workflows[0].WorkflowId, payload.WorkflowInstanceIds); + Assert.True(result); + } } #pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. } From 4eb56e49bfab5c25845098f91e435180b53b7196 Mon Sep 17 00:00:00 2001 From: Neil South Date: Wed, 22 May 2024 17:05:18 +0100 Subject: [PATCH 08/12] fix for workflowId Signed-off-by: Neil South --- .../WorkflowExecuter/Services/WorkflowExecuterService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs index 7ab8d721a..bacfb5413 100644 --- a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs +++ b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs @@ -180,7 +180,7 @@ public async Task ProcessPayload(WorkflowRequestEvent message, Payload pay { await ProcessFirstWorkflowTask(workflowInstance, message.CorrelationId, payload); } - payload.WorkflowInstanceIds = workflowInstances.Select(w => w.Id).ToList(); + payload.WorkflowInstanceIds = workflowInstances.Select(w => w.WorkflowId).ToList(); payload.TriggeredWorkflowNames = workflowInstances.Select(w => w.WorkflowName).ToList(); return true; From 51874760003d57009b6979687c685056c04a2c93 Mon Sep 17 00:00:00 2001 From: Neil South Date: Thu, 23 May 2024 09:14:49 +0100 Subject: [PATCH 09/12] add storage of workflow name in payload Signed-off-by: Neil South --- src/TaskManager/Plug-ins/Argo/ArgoClient.cs | 1 - src/TaskManager/Plug-ins/Email/EmailPlugin.cs | 1 - .../Common/Interfaces/IPayloadService.cs | 9 +------ .../Common/Services/PayloadService.cs | 19 +++++-------- .../Common/Services/WorkflowService.cs | 1 - .../Database/Interfaces/IPayloadRepository.cs | 10 +------ .../Repositories/PayloadRepository.cs | 27 +++++-------------- .../Services/EventPayloadRecieverService.cs | 3 ++- .../WorkflowExecuter/Common/ArtifactMapper.cs | 1 - .../Services/WorkflowExecuterService.cs | 7 ++--- .../Services/PayloadServiceTests.cs | 4 +-- .../Services/WorkflowExecuterServiceTests.cs | 3 +-- .../TaskExecutionStatsControllerTests.cs | 2 +- 13 files changed, 22 insertions(+), 66 deletions(-) diff --git a/src/TaskManager/Plug-ins/Argo/ArgoClient.cs b/src/TaskManager/Plug-ins/Argo/ArgoClient.cs index 53e8b43bf..e67e0873e 100755 --- a/src/TaskManager/Plug-ins/Argo/ArgoClient.cs +++ b/src/TaskManager/Plug-ins/Argo/ArgoClient.cs @@ -17,7 +17,6 @@ using System.Globalization; using System.Text; using Argo; -using Ardalis.GuardClauses; using Microsoft.Extensions.Logging; using Monai.Deploy.WorkflowManager.TaskManager.Argo.Logging; using System.Net; diff --git a/src/TaskManager/Plug-ins/Email/EmailPlugin.cs b/src/TaskManager/Plug-ins/Email/EmailPlugin.cs index 1dc4d8aff..454c8df04 100644 --- a/src/TaskManager/Plug-ins/Email/EmailPlugin.cs +++ b/src/TaskManager/Plug-ins/Email/EmailPlugin.cs @@ -16,7 +16,6 @@ using System.Net.Mail; -using Ardalis.GuardClauses; using FellowOakDicom; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; diff --git a/src/WorkflowManager/Common/Interfaces/IPayloadService.cs b/src/WorkflowManager/Common/Interfaces/IPayloadService.cs index 03c145236..9702defb6 100644 --- a/src/WorkflowManager/Common/Interfaces/IPayloadService.cs +++ b/src/WorkflowManager/Common/Interfaces/IPayloadService.cs @@ -48,13 +48,6 @@ Task> GetAllAsync(int? skip = null, /// payload id to delete. Task DeletePayloadFromStorageAsync(string payloadId); - /// - /// Updates a payload - /// - /// - /// - Task UpdateWorkflowInstanceIdsAsync(string payloadId, IEnumerable workflowInstances); - /// /// Gets the expiry date for a payload. /// @@ -69,6 +62,6 @@ Task> GetAllAsync(int? skip = null, /// payload id to update. /// updated payload. /// true if the update is successful, false otherwise. - Task UpdateAsync(Payload payload); + Task UpdateAsyncWorkflowIds(Payload payload); } } diff --git a/src/WorkflowManager/Common/Services/PayloadService.cs b/src/WorkflowManager/Common/Services/PayloadService.cs index 8d0b4ae1f..e5ced724b 100644 --- a/src/WorkflowManager/Common/Services/PayloadService.cs +++ b/src/WorkflowManager/Common/Services/PayloadService.cs @@ -213,7 +213,7 @@ public async Task DeletePayloadFromStorageAsync(string payloadId) // update the payload to in progress before we request deletion from storage payload.PayloadDeleted = PayloadDeleted.InProgress; - await _payloadRepository.UpdateAsync(payload); + await _payloadRepository.UpdateAsyncWorkflowIds(payload); // run deletion in alternative thread so the user isn't held up #pragma warning disable CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed @@ -238,7 +238,7 @@ public async Task DeletePayloadFromStorageAsync(string payloadId) } finally { - await _payloadRepository.UpdateAsync(payload); + await _payloadRepository.UpdateAsyncWorkflowIds(payload); } }); #pragma warning restore CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed @@ -246,18 +246,11 @@ public async Task DeletePayloadFromStorageAsync(string payloadId) return true; } - public async Task UpdateWorkflowInstanceIdsAsync(string payloadId, IEnumerable workflowInstances) + public Task UpdateAsyncWorkflowIds(Payload payload) { - if (await _payloadRepository.UpdateAssociatedWorkflowInstancesAsync(payloadId, workflowInstances)) - { - _logger.PayloadUpdated(payloadId); - return true; - } - else - { - _logger.PayloadUpdateFailed(payloadId); - return false; - } + ArgumentNullException.ThrowIfNull(payload, nameof(payload)); + + return _payloadRepository.UpdateAsyncWorkflowIds(payload); } public Task UpdateAsync(Payload payload) diff --git a/src/WorkflowManager/Common/Services/WorkflowService.cs b/src/WorkflowManager/Common/Services/WorkflowService.cs index 5d8790ba6..2788dc884 100644 --- a/src/WorkflowManager/Common/Services/WorkflowService.cs +++ b/src/WorkflowManager/Common/Services/WorkflowService.cs @@ -14,7 +14,6 @@ * limitations under the License. */ -using Ardalis.GuardClauses; using Microsoft.Extensions.Logging; using Monai.Deploy.WorkflowManager.Common.Miscellaneous.Interfaces; using Monai.Deploy.WorkflowManager.Common.Contracts.Models; diff --git a/src/WorkflowManager/Database/Interfaces/IPayloadRepository.cs b/src/WorkflowManager/Database/Interfaces/IPayloadRepository.cs index 92f55c37c..9861ccf19 100644 --- a/src/WorkflowManager/Database/Interfaces/IPayloadRepository.cs +++ b/src/WorkflowManager/Database/Interfaces/IPayloadRepository.cs @@ -51,15 +51,7 @@ public interface IPayloadRepository /// /// The payload to update. /// The updated payload. - Task UpdateAsync(Payload payload); - - /// - /// Updates a payload in the database. - /// - /// - /// - /// - Task UpdateAssociatedWorkflowInstancesAsync(string payloadId, IEnumerable workflowInstances); + Task UpdateAsyncWorkflowIds(Payload payload); /// /// Gets all the payloads that might need deleted diff --git a/src/WorkflowManager/Database/Repositories/PayloadRepository.cs b/src/WorkflowManager/Database/Repositories/PayloadRepository.cs index a4fc8cee8..f050d928d 100644 --- a/src/WorkflowManager/Database/Repositories/PayloadRepository.cs +++ b/src/WorkflowManager/Database/Repositories/PayloadRepository.cs @@ -18,7 +18,6 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using Ardalis.GuardClauses; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Monai.Deploy.WorkflowManager.Common.Contracts.Models; @@ -123,7 +122,7 @@ public async Task GetByIdAsync(string payloadId) return payload; } - public async Task UpdateAsync(Payload payload) + public async Task UpdateAsyncWorkflowIds(Payload payload) { ArgumentNullException.ThrowIfNull(payload, nameof(payload)); @@ -132,31 +131,17 @@ public async Task UpdateAsync(Payload payload) var filter = Builders.Filter.Eq(p => p.PayloadId, payload.PayloadId); await _payloadCollection.ReplaceOneAsync(filter, payload); - return true; - } - catch (Exception ex) - { - _logger.DbUpdatePayloadError(payload.PayloadId, ex); - return false; - } - } - - public async Task UpdateAssociatedWorkflowInstancesAsync(string payloadId, IEnumerable workflowInstances) - { - Guard.Against.NullOrEmpty(workflowInstances, nameof(workflowInstances)); - ArgumentNullException.ThrowIfNullOrWhiteSpace(payloadId, nameof(payloadId)); - - try - { await _payloadCollection.FindOneAndUpdateAsync( - i => i.Id == payloadId, - Builders.Update.Set(p => p.WorkflowInstanceIds, workflowInstances)); + i => i.Id == payload.Id, + Builders.Update + .Set(p => p.TriggeredWorkflowNames, payload.TriggeredWorkflowNames) + .Set(p => p.WorkflowInstanceIds, payload.WorkflowInstanceIds)); return true; } catch (Exception ex) { - _logger.DbUpdateWorkflowInstanceError(ex); + _logger.DbUpdatePayloadError(payload.PayloadId, ex); return false; } } diff --git a/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs b/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs index 7d125606e..edce5156a 100644 --- a/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs +++ b/src/WorkflowManager/PayloadListener/Services/EventPayloadRecieverService.cs @@ -95,8 +95,9 @@ public async Task ReceiveWorkflowPayload(MessageReceivedEventArgs message) if (string.IsNullOrWhiteSpace(string.Join("", payload.TriggeredWorkflowNames)) is false) { - await PayloadService.UpdateAsync(payload); + await PayloadService.UpdateAsyncWorkflowIds(payload); } + } else { diff --git a/src/WorkflowManager/WorkflowExecuter/Common/ArtifactMapper.cs b/src/WorkflowManager/WorkflowExecuter/Common/ArtifactMapper.cs index 52ea89352..6b2cc6502 100755 --- a/src/WorkflowManager/WorkflowExecuter/Common/ArtifactMapper.cs +++ b/src/WorkflowManager/WorkflowExecuter/Common/ArtifactMapper.cs @@ -14,7 +14,6 @@ * limitations under the License. */ -using Ardalis.GuardClauses; using Microsoft.Extensions.Logging; using Monai.Deploy.Storage.API; using Monai.Deploy.WorkflowManager.Common.Contracts.Models; diff --git a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs index bacfb5413..88213ba2e 100644 --- a/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs +++ b/src/WorkflowManager/WorkflowExecuter/Services/WorkflowExecuterService.cs @@ -163,9 +163,6 @@ public async Task ProcessPayload(WorkflowRequestEvent message, Payload pay if (workflowInstances.Count != 0) { processed &= await _workflowInstanceRepository.CreateAsync(workflowInstances); - - var workflowInstanceIds = workflowInstances.Select(workflowInstance => workflowInstance.Id); - await _payloadService.UpdateWorkflowInstanceIdsAsync(payload.Id, workflowInstanceIds).ConfigureAwait(false); } workflowInstances.AddRange(existingInstances.Where(e => e.PayloadId == message.PayloadId.ToString())); @@ -180,7 +177,7 @@ public async Task ProcessPayload(WorkflowRequestEvent message, Payload pay { await ProcessFirstWorkflowTask(workflowInstance, message.CorrelationId, payload); } - payload.WorkflowInstanceIds = workflowInstances.Select(w => w.WorkflowId).ToList(); + payload.WorkflowInstanceIds = workflowInstances.Select(w => w.Id).ToList(); payload.TriggeredWorkflowNames = workflowInstances.Select(w => w.WorkflowName).ToList(); return true; @@ -1171,7 +1168,7 @@ private static WorkflowInstance MakeInstance(WorkflowRequestEvent message, Workf { Id = workflowInstanceId, WorkflowId = workflow.WorkflowId, - WorkflowName = workflow.Workflow.Name, + WorkflowName = workflow.Workflow?.Name ?? "", PayloadId = message.PayloadId.ToString(), StartTime = DateTime.UtcNow, Status = Status.Created, diff --git a/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs b/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs index 6b3f9557d..179b63df7 100644 --- a/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs +++ b/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs @@ -333,7 +333,7 @@ public async Task DeletePayloadFromStorageAsync_ReturnsTrue() var payloadId = Guid.NewGuid().ToString(); _payloadRepository.Setup(p => p.GetByIdAsync(It.IsAny())).ReturnsAsync(() => new Payload()); - _payloadRepository.Setup(p => p.UpdateAsync(It.IsAny())).ReturnsAsync(() => true); + _payloadRepository.Setup(p => p.UpdateAsyncWorkflowIds(It.IsAny())).ReturnsAsync(() => true); _workflowInstanceRepository.Setup(r => r.GetByPayloadIdsAsync(It.IsAny>())).ReturnsAsync(() => new List()); _storageService.Setup(s => s.RemoveObjectsAsync(It.IsAny(), It.IsAny>(), default)); @@ -374,7 +374,7 @@ public async Task DeletePayloadFromStorageAsync_ThrowsMonaiBadRequestExceptionWh var payloadId = Guid.NewGuid().ToString(); _payloadRepository.Setup(p => p.GetByIdAsync(It.IsAny())).ReturnsAsync(() => new Payload()); - _payloadRepository.Setup(p => p.UpdateAsync(It.IsAny())).ReturnsAsync(() => true); + _payloadRepository.Setup(p => p.UpdateAsyncWorkflowIds(It.IsAny())).ReturnsAsync(() => true); _workflowInstanceRepository.Setup(r => r.GetByPayloadIdsAsync(It.IsAny>())).ReturnsAsync(() => new List { new WorkflowInstance diff --git a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs index ab398c459..886ab451e 100644 --- a/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs +++ b/tests/UnitTests/WorkflowExecuter.Tests/Services/WorkflowExecuterServiceTests.cs @@ -3779,7 +3779,7 @@ public async Task ProcessTaskUpdate_ValidTaskUpdateEventWith_Same_Status_returns _messageBrokerPublisherService.Verify(w => w.Publish(_configuration.Value.Messaging.Topics.ExportHL7, It.IsAny()), Times.Exactly(0)); - _logger.Verify(logger => logger.IsEnabled(LogLevel.Trace),Times.Once); + _logger.Verify(logger => logger.IsEnabled(LogLevel.Trace), Times.Once); response.Should().BeTrue(); } @@ -4154,7 +4154,6 @@ public async Task ProcessPayload_Payload_Should_Include_triggered_workflow_names Assert.Contains(workflows[0].Workflow!.Name, payload.TriggeredWorkflowNames); - Assert.Contains(workflows[0].WorkflowId, payload.WorkflowInstanceIds); Assert.True(result); } } diff --git a/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs b/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs index b6a8c4e7f..e3c81a073 100644 --- a/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs +++ b/tests/UnitTests/WorkflowManager.Tests/Controllers/TaskExecutionStatsControllerTests.cs @@ -292,7 +292,7 @@ public async Task GetDailyStatsAsync_ReturnsList() var objectResult = Assert.IsType(result); var responseValue = (StatsPagedResponse>)objectResult.Value; - responseValue.Data.First().Date.Should().Be(DateOnly.FromDateTime( _startTime)); + responseValue.Data.First().Date.Should().Be(DateOnly.FromDateTime(_startTime)); responseValue.FirstPage.Should().Be("unitTest"); responseValue.LastPage.Should().Be("unitTest"); responseValue.PageNumber.Should().Be(1); From 8b09c91454beecdd973c0fc2e5d8516d0a4a752f Mon Sep 17 00:00:00 2001 From: Neil South Date: Thu, 23 May 2024 09:23:17 +0100 Subject: [PATCH 10/12] added missing change Signed-off-by: Neil South --- src/WorkflowManager/Common/Services/PayloadService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WorkflowManager/Common/Services/PayloadService.cs b/src/WorkflowManager/Common/Services/PayloadService.cs index e5ced724b..f2c7517bb 100644 --- a/src/WorkflowManager/Common/Services/PayloadService.cs +++ b/src/WorkflowManager/Common/Services/PayloadService.cs @@ -257,7 +257,7 @@ public Task UpdateAsync(Payload payload) { ArgumentNullException.ThrowIfNull(payload, nameof(payload)); - return _payloadRepository.UpdateAsync(payload); + return _payloadRepository.UpdateAsyncWorkflowIds(payload); } } } From e57c58bb9867273e189bf744123868047a56cc48 Mon Sep 17 00:00:00 2001 From: Neil South Date: Thu, 23 May 2024 11:00:05 +0100 Subject: [PATCH 11/12] addded more code coverage Signed-off-by: Neil South --- src/WorkflowManager/Common/Services/PayloadService.cs | 7 ------- .../Common.Tests/Services/PayloadServiceTests.cs | 11 +++++++++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/WorkflowManager/Common/Services/PayloadService.cs b/src/WorkflowManager/Common/Services/PayloadService.cs index f2c7517bb..173854d39 100644 --- a/src/WorkflowManager/Common/Services/PayloadService.cs +++ b/src/WorkflowManager/Common/Services/PayloadService.cs @@ -252,12 +252,5 @@ public Task UpdateAsyncWorkflowIds(Payload payload) return _payloadRepository.UpdateAsyncWorkflowIds(payload); } - - public Task UpdateAsync(Payload payload) - { - ArgumentNullException.ThrowIfNull(payload, nameof(payload)); - - return _payloadRepository.UpdateAsyncWorkflowIds(payload); - } } } diff --git a/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs b/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs index 179b63df7..7cec3977a 100644 --- a/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs +++ b/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs @@ -488,5 +488,16 @@ public async Task PayloadServiceCreate_Should_Call_GetExpiry() Assert.Equal(99, (int)daysdiff); } + + [Fact] + public async Task UpdateAsync_NullPayload_ThrowsArgumentNullException() + { + // Arrange + Payload payload = null; + + // Act & Assert + await Assert.ThrowsAsync(() => PayloadService.UpdateAsyncWorkflowIds(payload)); + } + } } From 8a764fb7a144898fbe0f066b53e8eb88a6deebce Mon Sep 17 00:00:00 2001 From: Neil South Date: Thu, 23 May 2024 14:36:24 +0100 Subject: [PATCH 12/12] fix for GetSeriesInstanceUID in payload table Signed-off-by: Neil South --- src/WorkflowManager/Common/Services/PayloadService.cs | 2 +- src/WorkflowManager/Contracts/Models/Payload.cs | 4 ++-- src/WorkflowManager/Contracts/Models/PayloadDto.cs | 1 + src/WorkflowManager/Storage/Services/DicomService.cs | 2 +- tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs | 4 ++++ 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/WorkflowManager/Common/Services/PayloadService.cs b/src/WorkflowManager/Common/Services/PayloadService.cs index 173854d39..ddb2cc2c8 100644 --- a/src/WorkflowManager/Common/Services/PayloadService.cs +++ b/src/WorkflowManager/Common/Services/PayloadService.cs @@ -248,7 +248,7 @@ public async Task DeletePayloadFromStorageAsync(string payloadId) public Task UpdateAsyncWorkflowIds(Payload payload) { - ArgumentNullException.ThrowIfNull(payload, nameof(payload)); + ArgumentNullException.ThrowIfNull(payload); return _payloadRepository.UpdateAsyncWorkflowIds(payload); } diff --git a/src/WorkflowManager/Contracts/Models/Payload.cs b/src/WorkflowManager/Contracts/Models/Payload.cs index e1dd3fb52..fb78bdcd9 100755 --- a/src/WorkflowManager/Contracts/Models/Payload.cs +++ b/src/WorkflowManager/Contracts/Models/Payload.cs @@ -42,8 +42,8 @@ public class Payload : IDocument [JsonProperty(PropertyName = "workflows")] public IEnumerable Workflows { get; set; } = []; - [JsonProperty(PropertyName = "workflow_names")] - public List TriggeredWorkflowNames { get; set; } = []; + [JsonProperty(PropertyName = "triggered_workflow_names")] + public IEnumerable TriggeredWorkflowNames { get; set; } = []; [JsonProperty(PropertyName = "workflow_instance_ids")] public IEnumerable WorkflowInstanceIds { get; set; } = []; diff --git a/src/WorkflowManager/Contracts/Models/PayloadDto.cs b/src/WorkflowManager/Contracts/Models/PayloadDto.cs index c76f1bd33..6fa47934e 100644 --- a/src/WorkflowManager/Contracts/Models/PayloadDto.cs +++ b/src/WorkflowManager/Contracts/Models/PayloadDto.cs @@ -37,6 +37,7 @@ public PayloadDto(Payload payload) PatientDetails = payload.PatientDetails; PayloadDeleted = payload.PayloadDeleted; SeriesInstanceUid = payload.SeriesInstanceUid; + TriggeredWorkflowNames = payload.TriggeredWorkflowNames; } [JsonProperty(PropertyName = "payload_status")] diff --git a/src/WorkflowManager/Storage/Services/DicomService.cs b/src/WorkflowManager/Storage/Services/DicomService.cs index b94e79a17..eeb8ce8ba 100644 --- a/src/WorkflowManager/Storage/Services/DicomService.cs +++ b/src/WorkflowManager/Storage/Services/DicomService.cs @@ -291,7 +291,7 @@ public string GetValue(Dictionary dict, string keyId) if (dict.TryGetValue(DicomTagConstants.SeriesInstanceUIDTag, out var value)) { - return value.Value.ToString(); + return JsonConvert.SerializeObject(value.Value); } return null; } diff --git a/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs b/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs index 7cec3977a..c3931e7ca 100644 --- a/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs +++ b/tests/UnitTests/Common.Tests/Services/PayloadServiceTests.cs @@ -492,11 +492,15 @@ public async Task PayloadServiceCreate_Should_Call_GetExpiry() [Fact] public async Task UpdateAsync_NullPayload_ThrowsArgumentNullException() { + +#pragma warning disable CS8604 // Possible null reference argument. // Arrange Payload payload = null; // Act & Assert + await Assert.ThrowsAsync(() => PayloadService.UpdateAsyncWorkflowIds(payload)); +#pragma warning restore CS8604 // Possible null reference argument. } }