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);