From dfceaf992ff85af81d3cdef3cbb49cc0ae88becd Mon Sep 17 00:00:00 2001 From: Victor Chang <vicchang@nvidia.com> Date: Tue, 4 Oct 2022 11:58:38 -0700 Subject: [PATCH 1/4] gh-195 APIs to update DICOM source and destination Signed-off-by: Victor Chang <vicchang@nvidia.com> --- docs/api/rest/config.md | 136 +++++++++++++++--- src/CLI/Commands/DestinationCommand.cs | 56 ++++++++ src/CLI/Commands/SourceCommand.cs | 52 +++++++ src/CLI/ExitCodes.cs | 2 + src/CLI/Logging/Log.cs | 12 ++ src/CLI/Test/DestinationCommandTest.cs | 68 +++++++++ src/CLI/Test/SourceCommandTest.cs | 66 +++++++++ src/Client/Services/AeTitle{T}Service.cs | 12 ++ .../Logging/Log.8000.HttpServices.cs | 8 +- .../Http/DestinationAeTitleController.cs | 56 +++++++- .../Services/Http/MonaiHealthCheck.cs | 1 - .../Services/Http/SourceAeTitleController.cs | 60 +++++++- .../Http/DestinationAeTitleControllerTest.cs | 92 +++++++++++- .../Http/SourceAeTitleControllerTest.cs | 91 +++++++++++- 14 files changed, 678 insertions(+), 34 deletions(-) diff --git a/docs/api/rest/config.md b/docs/api/rest/config.md index 97f63979c..874b9bebe 100644 --- a/docs/api/rest/config.md +++ b/docs/api/rest/config.md @@ -144,16 +144,16 @@ Response Content Type: JSON - [MonaiApplicationEntity](xref:Monai.Deploy.Informa ```bash curl --location --request POST 'http://localhost:5000/config/ae/' \ ---header 'Content-Type: application/json' \ ---data-raw '{ - "name": "breast-tumor", - "aeTitle": "BREASTV1", - "timeout": 5, - "workflows": [ - "3f6a08a1-0dea-44e9-ab82-1ff1adf43a8e" - ] - } -}' + --header 'Content-Type: application/json' \ + --data-raw '{ + "name": "breast-tumor", + "aeTitle": "BREASTV1", + "timeout": 5, + "workflows": [ + "3f6a08a1-0dea-44e9-ab82-1ff1adf43a8e" + ] + } + }' ``` ### Example Response @@ -310,12 +310,56 @@ Response Content Type: JSON - [SourceApplicationEntity](xref:Monai.Deploy.Inform ```bash curl --location --request POST 'http://localhost:5000/config/source' \ ---header 'Content-Type: application/json' \ ---data-raw '{ - "name": "USEAST", - "hostIp": "10.20.3.4", - "aeTitle": "PACSUSEAST" -}' + --header 'Content-Type: application/json' \ + --data-raw '{ + "name": "USEAST", + "hostIp": "10.20.3.4", + "aeTitle": "PACSUSEAST" + }' +``` + +### Example Response + +```json +{ + "name": "USEAST", + "aeTitle": "PACSUSEAST", + "hostIp": "10.20.3.4" +} +``` + +--- + +## PUT /config/source + +Updates an existing calling (source) AE Title. + +### Parameters + +See the [SourceApplicationEntity](xref:Monai.Deploy.InformaticsGateway.Api.SourceApplicationEntity) +class definition for details. + +### Responses + +Response Content Type: JSON - [SourceApplicationEntity](xref:Monai.Deploy.InformaticsGateway.Api.SourceApplicationEntity). + +| Code | Description | +| ---- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 200 | AE Title updated successfully. | +| 400 | Validation error. The response will be a [Problem details](https://datatracker.ietf.org/doc/html/rfc7807) object with details of the validation errors . | +| 404 | DICOM source cannot be found. | +| 500 | Server error. The response will be a [Problem details](https://datatracker.ietf.org/doc/html/rfc7807) object with server error details. | + +### Example Request + +```bash +curl --location --request PUT 'http://localhost:5000/config/source' \ + --header 'Content-Type: application/json' \ + --data-raw '{ + "name": "USEAST", + "hostIp": "10.20.3.4", + "aeTitle": "PACSUSEAST" + }' ``` ### Example Response @@ -479,6 +523,52 @@ curl --location --request DELETE 'http://localhost:5000/config/destination/cecho --- +## PUT /config/destination + +Updates an existing DICOM destination. + +### Parameters + +See the [DestinationApplicationEntity](xref:Monai.Deploy.InformaticsGateway.Api.DestinationApplicationEntity) +class definition for details. + +### Responses + +Response Content Type: JSON - [DestinationApplicationEntity](xref:Monai.Deploy.InformaticsGateway.Api.DestinationApplicationEntity). + +| Code | Description | +| ---- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 200 | DICOM destination updated successfully. | +| 400 | Validation error. The response will be a [Problem details](https://datatracker.ietf.org/doc/html/rfc7807) object with details of the validation errors . | +| 404 | DICOM destination cannot be found. | +| 500 | Server error. The response will be a [Problem details](https://datatracker.ietf.org/doc/html/rfc7807) object with server error details. | + +### Example Request + +```bash +curl --location --request PUT 'http://localhost:5000/config/destination' \ + --header 'Content-Type: application/json' \ + --data-raw '{ + "name": "USEAST", + "hostIp": "10.20.3.4", + "port": 104, + "aeTitle": "PACSUSEAST" + }' +``` + +### Example Response + +```json +{ + "port": 104, + "name": "USEAST", + "aeTitle": "PACSUSEAST", + "hostIp": "10.20.3.4" +} +``` + +--- + ## POST /config/destination Adds a new DICOM destination AET for exporting results to. @@ -502,13 +592,13 @@ Response Content Type: JSON - [DestinationApplicationEntity](xref:Monai.Deploy.I ```bash curl --location --request POST 'http://localhost:5000/config/destination' \ ---header 'Content-Type: application/json' \ ---data-raw '{ - "name": "USEAST", - "hostIp": "10.20.3.4", - "port": 104, - "aeTitle": "PACSUSEAST" -}' + --header 'Content-Type: application/json' \ + --data-raw '{ + "name": "USEAST", + "hostIp": "10.20.3.4", + "port": 104, + "aeTitle": "PACSUSEAST" + }' ``` ### Example Response diff --git a/src/CLI/Commands/DestinationCommand.cs b/src/CLI/Commands/DestinationCommand.cs index a04929972..044000b8f 100644 --- a/src/CLI/Commands/DestinationCommand.cs +++ b/src/CLI/Commands/DestinationCommand.cs @@ -41,6 +41,7 @@ public DestinationCommand() : base("dst", "Configure DICOM destinations") AddAlias("destination"); SetupAddDestinationCommand(); + SetupEditDestinationCommand(); SetupRemoveDestinationCommand(); SetupListDestinationCommand(); SetupCEchoCommand(); @@ -67,6 +68,23 @@ private void SetupListDestinationCommand() listCommand.Handler = CommandHandler.Create<DestinationApplicationEntity, IHost, bool, CancellationToken>(ListDestinationHandlerAsync); } + private void SetupEditDestinationCommand() + { + var addCommand = new Command("update", "Update a new DICOM destination"); + AddCommand(addCommand); + + var nameOption = new Option<string>(new string[] { "--name", "-n" }, "Name of the DICOM destination") { IsRequired = false }; + addCommand.AddOption(nameOption); + var aeTitleOption = new Option<string>(new string[] { "--aetitle", "-a" }, "AE Title of the DICOM destination") { IsRequired = true }; + addCommand.AddOption(aeTitleOption); + var hostOption = new Option<string>(new string[] { "--host-ip", "-h" }, "Host or IP address of the DICOM destination") { IsRequired = true }; + addCommand.AddOption(hostOption); + var portOption = new Option<int>(new string[] { "--port", "-p" }, "Listening port of the DICOM destination") { IsRequired = true }; + addCommand.AddOption(portOption); + + addCommand.Handler = CommandHandler.Create<DestinationApplicationEntity, IHost, bool, CancellationToken>(EditDestinationHandlerAsync); + } + private void SetupRemoveDestinationCommand() { var removeCommand = new Command("rm", "Remove a DICOM destination"); @@ -234,6 +252,44 @@ private async Task<int> RemoveDestinationHandlerAsync(string name, IHost host, b return ExitCodes.Success; } + private async Task<int> EditDestinationHandlerAsync(DestinationApplicationEntity entity, IHost host, bool verbose, CancellationToken cancellationToken) + { + Guard.Against.Null(entity, nameof(entity)); + Guard.Against.Null(host, nameof(host)); + + LogVerbose(verbose, host, "Configuring services..."); + var configService = host.Services.GetRequiredService<IConfigurationService>(); + var client = host.Services.GetRequiredService<IInformaticsGatewayClient>(); + var logger = CreateLogger<DestinationCommand>(host); + + Guard.Against.Null(logger, nameof(logger), "Logger is unavailable."); + Guard.Against.Null(configService, nameof(configService), "Configuration service is unavailable."); + Guard.Against.Null(client, nameof(client), $"{Strings.ApplicationName} client is unavailable."); + + try + { + CheckConfiguration(configService); + client.ConfigureServiceUris(configService.Configurations.InformaticsGatewayServerUri); + + LogVerbose(verbose, host, $"Connecting to {Strings.ApplicationName} at {configService.Configurations.InformaticsGatewayServerEndpoint}..."); + LogVerbose(verbose, host, $"Updating DICOM destination {entity.AeTitle}..."); + var result = await client.DicomDestinations.Update(entity, cancellationToken).ConfigureAwait(false); + + logger.DicomDestinationCreated(result.Name, result.AeTitle, result.HostIp, result.Port); + } + catch (ConfigurationException ex) + { + logger.ConfigurationException(ex.Message); + return ExitCodes.Config_NotConfigured; + } + catch (Exception ex) + { + logger.ErrorUpdatingDicomDestination(entity.AeTitle, ex.Message); + return ExitCodes.DestinationAe_ErrorUpdate; + } + return ExitCodes.Success; + } + private async Task<int> AddDestinationHandlerAsync(DestinationApplicationEntity entity, IHost host, bool verbose, CancellationToken cancellationToken) { Guard.Against.Null(entity, nameof(entity)); diff --git a/src/CLI/Commands/SourceCommand.cs b/src/CLI/Commands/SourceCommand.cs index 9689637f2..dc94e702b 100644 --- a/src/CLI/Commands/SourceCommand.cs +++ b/src/CLI/Commands/SourceCommand.cs @@ -40,10 +40,12 @@ public SourceCommand() : base("src", "Configure DICOM sources") AddAlias("source"); SetupAddSourceCommand(); + SetupUpdateSourceCommand(); SetupRemoveSourceCommand(); SetupListSourceCommand(); } + private void SetupListSourceCommand() { var listCommand = new Command("ls", "List all DICOM sources"); @@ -79,6 +81,20 @@ private void SetupAddSourceCommand() addCommand.Handler = CommandHandler.Create<SourceApplicationEntity, IHost, bool, CancellationToken>(AddSourceHandlerAsync); } + private void SetupUpdateSourceCommand() + { + var addCommand = new Command("update", "Update a new DICOM source"); + AddCommand(addCommand); + + var nameOption = new Option<string>(new string[] { "--name", "-n" }, "Name of the DICOM source") { IsRequired = false }; + addCommand.AddOption(nameOption); + var aeTitleOption = new Option<string>(new string[] { "--aetitle", "-a" }, "AE Title of the DICOM source") { IsRequired = true }; + addCommand.AddOption(aeTitleOption); + var hostOption = new Option<string>(new string[] { "--host-ip", "-h" }, "Host or IP address of the DICOM source") { IsRequired = true }; + addCommand.AddOption(hostOption); + + addCommand.Handler = CommandHandler.Create<SourceApplicationEntity, IHost, bool, CancellationToken>(UpdateSourceHandlerAsync); + } private async Task<int> ListSourceHandlerAsync(SourceApplicationEntity entity, IHost host, bool verbose, CancellationToken cancellationTokena) { @@ -217,5 +233,41 @@ private async Task<int> AddSourceHandlerAsync(SourceApplicationEntity entity, IH } return ExitCodes.Success; } + private async Task<int> UpdateSourceHandlerAsync(SourceApplicationEntity entity, IHost host, bool verbose, CancellationToken cancellationTokena) + { + Guard.Against.Null(entity, nameof(entity)); + Guard.Against.Null(host, nameof(host)); + + LogVerbose(verbose, host, "Configuring services..."); + var configService = host.Services.GetRequiredService<IConfigurationService>(); + var client = host.Services.GetRequiredService<IInformaticsGatewayClient>(); + var logger = CreateLogger<SourceCommand>(host); + + Guard.Against.Null(logger, nameof(logger), "Logger is unavailable."); + Guard.Against.Null(configService, nameof(configService), "Configuration service is unavailable."); + Guard.Against.Null(client, nameof(client), $"{Strings.ApplicationName} client is unavailable."); + + try + { + CheckConfiguration(configService); + client.ConfigureServiceUris(configService.Configurations.InformaticsGatewayServerUri); + LogVerbose(verbose, host, $"Connecting to {Strings.ApplicationName} at {configService.Configurations.InformaticsGatewayServerEndpoint}..."); + LogVerbose(verbose, host, $"Updating DICOM source {entity.AeTitle}..."); + var result = await client.DicomSources.Update(entity, cancellationTokena).ConfigureAwait(false); + + logger.DicomSourceUpdated(result.Name, result.AeTitle, result.HostIp); + } + catch (ConfigurationException ex) + { + logger.ConfigurationException(ex.Message); + return ExitCodes.Config_NotConfigured; + } + catch (Exception ex) + { + logger.ErrorUpdatingDicomSource(entity.AeTitle, ex.Message); + return ExitCodes.SourceAe_ErrorUpdate; + } + return ExitCodes.Success; + } } } diff --git a/src/CLI/ExitCodes.cs b/src/CLI/ExitCodes.cs index 35577333a..52679a7b9 100644 --- a/src/CLI/ExitCodes.cs +++ b/src/CLI/ExitCodes.cs @@ -33,10 +33,12 @@ public static class ExitCodes public const int DestinationAe_ErrorDelete = 301; public const int DestinationAe_ErrorCreate = 302; public const int DestinationAe_ErrorCEcho = 303; + public const int DestinationAe_ErrorUpdate = 304; public const int SourceAe_ErrorList = 400; public const int SourceAe_ErrorDelete = 401; public const int SourceAe_ErrorCreate = 402; + public const int SourceAe_ErrorUpdate = 403; public const int Restart_Cancelled = 500; public const int Restart_Error = 501; diff --git a/src/CLI/Logging/Log.cs b/src/CLI/Logging/Log.cs index e485a4970..58b2b9598 100644 --- a/src/CLI/Logging/Log.cs +++ b/src/CLI/Logging/Log.cs @@ -169,6 +169,18 @@ public static partial class Log [LoggerMessage(EventId = 30055, Level = LogLevel.Critical, Message = "C-ECHO to {name} failed: {error}.")] public static partial void ErrorCEchogDicomDestination(this ILogger logger, string name, string error); + [LoggerMessage(EventId = 30056, Level = LogLevel.Information, Message = "DICOM destination updated:\r\n\tName: {name}\r\n\tAE Title: {aeTitle}\r\n\tHost/IP Address: {hostIp}\r\n\tPort: {port}")] + public static partial void DicomDestinationUpdated(this ILogger logger, string name, string aeTitle, string hostIp, int port); + + [LoggerMessage(EventId = 30057, Level = LogLevel.Critical, Message = "Error updating DICOM destination {aeTitle}: {message}")] + public static partial void ErrorUpdatingDicomDestination(this ILogger logger, string aeTitle, string message); + + [LoggerMessage(EventId = 30058, Level = LogLevel.Information, Message = "DICOM source updated:\r\n\tName: {name}\r\n\tAE Title: {aeTitle}\r\n\tHost/IP Address: {hostIp}")] + public static partial void DicomSourceUpdated(this ILogger logger, string name, string aeTitle, string hostIp); + + [LoggerMessage(EventId = 30059, Level = LogLevel.Critical, Message = "Error updating DICOM source {aeTitle}: {message}")] + public static partial void ErrorUpdatingDicomSource(this ILogger logger, string aeTitle, string message); + // Docker Runner [LoggerMessage(EventId = 31000, Level = LogLevel.Debug, Message = "Checking for existing {applicationName} ({version}) containers...")] public static partial void CheckingExistingAppContainer(this ILogger logger, string applicationName, string version); diff --git a/src/CLI/Test/DestinationCommandTest.cs b/src/CLI/Test/DestinationCommandTest.cs index 1382e4169..7b742171a 100644 --- a/src/CLI/Test/DestinationCommandTest.cs +++ b/src/CLI/Test/DestinationCommandTest.cs @@ -338,5 +338,73 @@ public async Task DstCEcho_Command_ConfigurationException() _logger.VerifyLoggingMessageBeginsWith("Please execute `testhost config init` to intialize Informatics Gateway.", LogLevel.Critical, Times.Once()); } + + + [Fact(DisplayName = "dst update command")] + public async Task DstUpdate_Command() + { + var command = "dst update -n MyName -a MyAET -h MyHost -p 100"; + var result = _paser.Parse(command); + Assert.Equal(ExitCodes.Success, result.Errors.Count); + + var entity = new DestinationApplicationEntity() + { + Name = result.CommandResult.Children[0].Tokens[0].Value, + AeTitle = result.CommandResult.Children[1].Tokens[0].Value, + HostIp = result.CommandResult.Children[2].Tokens[0].Value, + Port = int.Parse(result.CommandResult.Children[3].Tokens[0].Value, System.Globalization.NumberStyles.Integer, CultureInfo.InvariantCulture), + }; + + Assert.Equal("MyName", entity.Name); + Assert.Equal("MyAET", entity.AeTitle); + Assert.Equal("MyHost", entity.HostIp); + Assert.Equal(100, entity.Port); + + _informaticsGatewayClient.Setup(p => p.DicomDestinations.Update(It.IsAny<DestinationApplicationEntity>(), It.IsAny<CancellationToken>())) + .ReturnsAsync(entity); + + int exitCode = await _paser.InvokeAsync(command); + + Assert.Equal(ExitCodes.Success, exitCode); + + _informaticsGatewayClient.Verify(p => p.ConfigureServiceUris(It.IsAny<Uri>()), Times.Once()); + _informaticsGatewayClient.Verify( + p => p.DicomDestinations.Update( + It.Is<DestinationApplicationEntity>(o => o.AeTitle == entity.AeTitle && o.Name == entity.Name && o.HostIp == entity.HostIp && o.Port == entity.Port), + It.IsAny<CancellationToken>()), Times.Once()); + } + + [Fact(DisplayName = "dst update command exception")] + public async Task DstUpdate_Command_Exception() + { + var command = "dst update -n MyName -a MyAET --apps App MyCoolApp TheApp"; + _informaticsGatewayClient.Setup(p => p.DicomDestinations.Update(It.IsAny<DestinationApplicationEntity>(), It.IsAny<CancellationToken>())) + .Throws(new Exception("error")); + + int exitCode = await _paser.InvokeAsync(command); + + Assert.Equal(ExitCodes.DestinationAe_ErrorUpdate, exitCode); + + _informaticsGatewayClient.Verify(p => p.ConfigureServiceUris(It.IsAny<Uri>()), Times.Once()); + _informaticsGatewayClient.Verify(p => p.DicomDestinations.Update(It.IsAny<DestinationApplicationEntity>(), It.IsAny<CancellationToken>()), Times.Once()); + + _logger.VerifyLoggingMessageBeginsWith("Error updating DICOM destination", LogLevel.Critical, Times.Once()); + } + + [Fact(DisplayName = "dst update command configuration exception")] + public async Task DstUpdate_Command_ConfigurationException() + { + var command = "dst update -n MyName -a MyAET --apps App MyCoolApp TheApp"; + _configurationService.SetupGet(p => p.IsInitialized).Returns(false); + + int exitCode = await _paser.InvokeAsync(command); + + Assert.Equal(ExitCodes.Config_NotConfigured, exitCode); + + _informaticsGatewayClient.Verify(p => p.ConfigureServiceUris(It.IsAny<Uri>()), Times.Never()); + _informaticsGatewayClient.Verify(p => p.DicomDestinations.List(It.IsAny<CancellationToken>()), Times.Never()); + + _logger.VerifyLoggingMessageBeginsWith("Please execute `testhost config init` to intialize Informatics Gateway.", LogLevel.Critical, Times.Once()); + } } } diff --git a/src/CLI/Test/SourceCommandTest.cs b/src/CLI/Test/SourceCommandTest.cs index 833bd16d7..e4d071bca 100644 --- a/src/CLI/Test/SourceCommandTest.cs +++ b/src/CLI/Test/SourceCommandTest.cs @@ -281,5 +281,71 @@ public async Task SrcList_Command_Empty() _logger.VerifyLogging("No DICOM sources configured.", LogLevel.Warning, Times.Once()); } + + [Fact(DisplayName = "src update comand")] + public async Task SrcUpdate_Command() + { + var command = "src update -n MyName -a MyAET -h MyHost"; + var result = _paser.Parse(command); + Assert.Equal(ExitCodes.Success, result.Errors.Count); + + var entity = new SourceApplicationEntity() + { + Name = result.CommandResult.Children[0].Tokens[0].Value, + AeTitle = result.CommandResult.Children[1].Tokens[0].Value, + HostIp = result.CommandResult.Children[2].Tokens[0].Value, + }; + + Assert.Equal("MyName", entity.Name); + Assert.Equal("MyAET", entity.AeTitle); + Assert.Equal("MyHost", entity.HostIp); + + _informaticsGatewayClient.Setup(p => p.DicomSources.Update(It.IsAny<SourceApplicationEntity>(), It.IsAny<CancellationToken>())) + .ReturnsAsync(entity); + + int exitCode = await _paser.InvokeAsync(command); + + Assert.Equal(ExitCodes.Success, exitCode); + + _informaticsGatewayClient.Verify(p => p.ConfigureServiceUris(It.IsAny<Uri>()), Times.Once()); + _informaticsGatewayClient.Verify( + p => p.DicomSources.Update( + It.Is<SourceApplicationEntity>(o => o.AeTitle == entity.AeTitle && o.Name == entity.Name && o.HostIp == entity.HostIp), + It.IsAny<CancellationToken>()), Times.Once()); + } + + [Fact(DisplayName = "src update comand exception")] + public async Task SrcUpdate_Command_Exception() + { + var command = "src update -n MyName -a MyAET --apps App MyCoolApp TheApp"; + _informaticsGatewayClient.Setup(p => p.DicomSources.Update(It.IsAny<SourceApplicationEntity>(), It.IsAny<CancellationToken>())) + .Throws(new Exception("error")); + + int exitCode = await _paser.InvokeAsync(command); + + Assert.Equal(ExitCodes.SourceAe_ErrorUpdate, exitCode); + + _informaticsGatewayClient.Verify(p => p.ConfigureServiceUris(It.IsAny<Uri>()), Times.Once()); + _informaticsGatewayClient.Verify(p => p.DicomSources.Update(It.IsAny<SourceApplicationEntity>(), It.IsAny<CancellationToken>()), Times.Once()); + + _logger.VerifyLoggingMessageBeginsWith("Error updating DICOM source", LogLevel.Critical, Times.Once()); + } + + [Fact(DisplayName = "src update comand configuration exception")] + public async Task SrcUpdate_Command_ConfigurationException() + { + var command = "src update -n MyName -a MyAET --apps App MyCoolApp TheApp"; + _configurationService.SetupGet(p => p.IsInitialized).Returns(false); + + int exitCode = await _paser.InvokeAsync(command); + + Assert.Equal(ExitCodes.Config_NotConfigured, exitCode); + + _informaticsGatewayClient.Verify(p => p.ConfigureServiceUris(It.IsAny<Uri>()), Times.Never()); + _informaticsGatewayClient.Verify(p => p.MonaiScpAeTitle.List(It.IsAny<CancellationToken>()), Times.Never()); + + _logger.VerifyLoggingMessageBeginsWith("Please execute `testhost config init` to intialize Informatics Gateway.", LogLevel.Critical, Times.Once()); + } + } } diff --git a/src/Client/Services/AeTitle{T}Service.cs b/src/Client/Services/AeTitle{T}Service.cs index c571953ed..45f5779c5 100644 --- a/src/Client/Services/AeTitle{T}Service.cs +++ b/src/Client/Services/AeTitle{T}Service.cs @@ -35,6 +35,8 @@ public interface IAeTitleService<T> Task<T> Create(T item, CancellationToken cancellationToken); + Task<T> Update(T item, CancellationToken cancellationToken); + Task<T> Delete(string aeTitle, CancellationToken cancellationToken); Task CEcho(string name, CancellationToken cancellationToken); @@ -104,5 +106,15 @@ public async Task CEcho(string name, CancellationToken cancellationToken) var response = await HttpClient.GetAsync($"{Route}/cecho/{name}", cancellationToken).ConfigureAwait(false); await response.EnsureSuccessStatusCodeWithProblemDetails(Logger).ConfigureAwait(false); } + + public async Task<T> Update(T item, CancellationToken cancellationToken) + { + Guard.Against.Null(item, nameof(item)); + + Logger.SendingRequestTo(Route); + var response = await HttpClient.PutAsJsonAsync(Route, item, Configuration.JsonSerializationOptions, cancellationToken).ConfigureAwait(false); + await response.EnsureSuccessStatusCodeWithProblemDetails(Logger).ConfigureAwait(false); + return await response.Content.ReadAsAsync<T>(cancellationToken).ConfigureAwait(false); + } } } diff --git a/src/InformaticsGateway/Logging/Log.8000.HttpServices.cs b/src/InformaticsGateway/Logging/Log.8000.HttpServices.cs index bc8b96e60..b52f0eb9e 100644 --- a/src/InformaticsGateway/Logging/Log.8000.HttpServices.cs +++ b/src/InformaticsGateway/Logging/Log.8000.HttpServices.cs @@ -43,7 +43,7 @@ public static partial class Log [LoggerMessage(EventId = 8010, Level = LogLevel.Information, Message = "DICOM destination added AE Title={aeTitle}, Host/IP={hostIp}.")] public static partial void DestinationApplicationEntityAdded(this ILogger logger, string aeTitle, string hostIp); - [LoggerMessage(EventId = 8011, Level = LogLevel.Information, Message = "MONAI SCP Application Entity deleted {name}.")] + [LoggerMessage(EventId = 8011, Level = LogLevel.Information, Message = "DICOM destination deleted {name}.")] public static partial void DestinationApplicationEntityDeleted(this ILogger logger, string name); [LoggerMessage(EventId = 8012, Level = LogLevel.Error, Message = "Error querying DICOM destinations.")] @@ -58,6 +58,9 @@ public static partial class Log [LoggerMessage(EventId = 8015, Level = LogLevel.Error, Message = "Error C-ECHO to DICOM destination {name}.")] public static partial void ErrorCEechoDestinationApplicationEntity(this ILogger logger, string name, Exception ex); + [LoggerMessage(EventId = 8016, Level = LogLevel.Information, Message = "DICOM destination updated {name}: AE Title={aeTitle}, Host/IP={hostIp}, Port={port}.")] + public static partial void DestinationApplicationEntityUpdated(this ILogger logger, string name, string aeTitle, string hostIp, int port); + // Source AE Title Controller [LoggerMessage(EventId = 8020, Level = LogLevel.Information, Message = "DICOM source added AE Title={aeTitle}, Host/IP={hostIp}.")] public static partial void SourceApplicationEntityAdded(this ILogger logger, string aeTitle, string hostIp); @@ -74,6 +77,9 @@ public static partial class Log [LoggerMessage(EventId = 8024, Level = LogLevel.Error, Message = "Error deleting DICOM source.")] public static partial void ErrorDeletingSourceApplicationEntity(this ILogger logger, Exception ex); + [LoggerMessage(EventId = 8025, Level = LogLevel.Information, Message = "DICOM source updated {name}: AE Title={aeTitle}, Host/IP={hostIp}.")] + public static partial void SourceApplicationEntityUpdated(this ILogger logger, string name, string aeTitle, string hostIp); + // Inference API [LoggerMessage(EventId = 8030, Level = LogLevel.Error, Message = "Failed to retrieve status for TransactionId/JobId={transactionId}.")] public static partial void ErrorRetrievingJobStatus(this ILogger logger, string transactionId, Exception ex); diff --git a/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs b/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs index 5d01aadae..ace57f5c0 100644 --- a/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs +++ b/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs @@ -153,7 +153,7 @@ public async Task<ActionResult<string>> Create(DestinationApplicationEntity item { item.SetDefaultValues(); - Validate(item); + ValidateCreate(item); await _repository.AddAsync(item).ConfigureAwait(false); await _repository.SaveChangesAsync().ConfigureAwait(false); @@ -175,6 +175,46 @@ public async Task<ActionResult<string>> Create(DestinationApplicationEntity item } } + [HttpPut] + [Produces("application/json")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status500InternalServerError)] + public async Task<ActionResult<DestinationApplicationEntity>> Edit(DestinationApplicationEntity item) + { + try + { + if (item is null) + { + return NotFound(); + } + + var destinationApplicationEntity = await _repository.FindAsync(item.Name).ConfigureAwait(false); + if (destinationApplicationEntity is null) + { + return NotFound(); + } + + item.SetDefaultValues(); + + destinationApplicationEntity.AeTitle = item.AeTitle; + destinationApplicationEntity.HostIp = item.HostIp; + destinationApplicationEntity.Port = item.Port; + + ValidateUpdate(destinationApplicationEntity); + + _ = _repository.Update(destinationApplicationEntity); + await _repository.SaveChangesAsync(HttpContext.RequestAborted).ConfigureAwait(false); + _logger.DestinationApplicationEntityUpdated(item.Name, item.AeTitle, item.HostIp, item.Port); + return Ok(destinationApplicationEntity); + } + catch (Exception ex) + { + _logger.ErrorDeletingDestinationApplicationEntity(ex); + return Problem(title: "Error updating DICOM destination.", statusCode: StatusCodes.Status500InternalServerError, detail: ex.Message); + } + } + [HttpDelete("{name}")] [Produces("application/json")] [ProducesResponseType(StatusCodes.Status200OK)] @@ -203,7 +243,7 @@ public async Task<ActionResult<DestinationApplicationEntity>> Delete(string name } } - private void Validate(DestinationApplicationEntity item) + private void ValidateCreate(DestinationApplicationEntity item) { if (_repository.Any(p => p.Name.Equals(item.Name))) { @@ -218,5 +258,17 @@ private void Validate(DestinationApplicationEntity item) throw new ConfigurationException(string.Join(Environment.NewLine, validationErrors)); } } + + private void ValidateUpdate(DestinationApplicationEntity item) + { + if (_repository.Any(p => !p.Name.Equals(item.Name) && p.AeTitle.Equals(item.AeTitle) && p.HostIp.Equals(item.HostIp) && p.Port.Equals(item.Port))) + { + throw new ObjectExistsException($"A DICOM destination with the same AE Title '{item.AeTitle}', host/IP Address '{item.HostIp}' and port '{item.Port}' already exists."); + } + if (!item.IsValid(out var validationErrors)) + { + throw new ConfigurationException(string.Join(Environment.NewLine, validationErrors)); + } + } } } diff --git a/src/InformaticsGateway/Services/Http/MonaiHealthCheck.cs b/src/InformaticsGateway/Services/Http/MonaiHealthCheck.cs index d4ae19334..007995a2f 100644 --- a/src/InformaticsGateway/Services/Http/MonaiHealthCheck.cs +++ b/src/InformaticsGateway/Services/Http/MonaiHealthCheck.cs @@ -16,7 +16,6 @@ using System; using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Diagnostics.HealthChecks; diff --git a/src/InformaticsGateway/Services/Http/SourceAeTitleController.cs b/src/InformaticsGateway/Services/Http/SourceAeTitleController.cs index e8c18250d..0478e9832 100644 --- a/src/InformaticsGateway/Services/Http/SourceAeTitleController.cs +++ b/src/InformaticsGateway/Services/Http/SourceAeTitleController.cs @@ -100,7 +100,7 @@ public async Task<ActionResult<string>> Create(SourceApplicationEntity item) try { item.SetDefaultValues(); - Validate(item); + ValidateCreate(item); await _repository.AddAsync(item).ConfigureAwait(false); await _repository.SaveChangesAsync().ConfigureAwait(false); @@ -122,6 +122,50 @@ public async Task<ActionResult<string>> Create(SourceApplicationEntity item) } } + [HttpPut] + [Produces("application/json")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status500InternalServerError)] + public async Task<ActionResult<SourceApplicationEntity>> Edit(SourceApplicationEntity item) + { + try + { + if (item is null) + { + return NotFound(); + } + + var sourceApplicationEntity = await _repository.FindAsync(item.Name).ConfigureAwait(false); + if (sourceApplicationEntity is null) + { + return NotFound(); + } + + item.SetDefaultValues(); + + sourceApplicationEntity.AeTitle = item.AeTitle; + sourceApplicationEntity.HostIp = item.HostIp; + + ValidateEdit(sourceApplicationEntity); + + _ = _repository.Update(sourceApplicationEntity); + await _repository.SaveChangesAsync(HttpContext.RequestAborted).ConfigureAwait(false); + _logger.SourceApplicationEntityUpdated(item.Name, item.AeTitle, item.HostIp); + return Ok(sourceApplicationEntity); + } + catch (ConfigurationException ex) + { + return Problem(title: "Validation error.", statusCode: (int)System.Net.HttpStatusCode.BadRequest, detail: ex.Message); + } + catch (Exception ex) + { + _logger.ErrorDeletingDestinationApplicationEntity(ex); + return Problem(title: "Error updating DICOM source.", statusCode: StatusCodes.Status500InternalServerError, detail: ex.Message); + } + } + [HttpDelete("{name}")] [Produces("application/json")] [ProducesResponseType(StatusCodes.Status200OK)] @@ -150,7 +194,7 @@ public async Task<ActionResult<SourceApplicationEntity>> Delete(string name) } } - private void Validate(SourceApplicationEntity item) + private void ValidateCreate(SourceApplicationEntity item) { if (_repository.Any(p => p.Name.Equals(item.Name))) { @@ -165,5 +209,17 @@ private void Validate(SourceApplicationEntity item) throw new ConfigurationException(string.Join(Environment.NewLine, validationErrors)); } } + + private void ValidateEdit(SourceApplicationEntity item) + { + if (_repository.Any(p => !item.Name.Equals(p.Name) && item.AeTitle.Equals(p.AeTitle) && item.HostIp.Equals(p.HostIp))) + { + throw new ObjectExistsException($"A DICOM source with the same AE Title '{item.AeTitle}' and host/IP address '{item.HostIp}' already exists."); + } + if (!item.IsValid(out var validationErrors)) + { + throw new ConfigurationException(string.Join(Environment.NewLine, validationErrors)); + } + } } } diff --git a/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs b/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs index 964b91307..49bdff6e5 100644 --- a/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs +++ b/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs @@ -29,6 +29,7 @@ using Monai.Deploy.InformaticsGateway.Services.Http; using Monai.Deploy.InformaticsGateway.Services.Scu; using Moq; +using Org.BouncyCastle.Asn1.Cms; using xRetry; using Xunit; @@ -380,9 +381,96 @@ public async Task Create_ShallReturnCreatedAtAction() #endregion Create + #region Update + + [RetryFact(5, 250, DisplayName = "Update - Shall return updated")] + public async Task Update_ReturnsUpdated() + { + var entity = new DestinationApplicationEntity + { + AeTitle = "AET", + HostIp = "host", + Name = "AET", + Port = 123, + }; + + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(entity)); + _repository.Setup(p => p.Remove(It.IsAny<DestinationApplicationEntity>())); + _repository.Setup(p => p.SaveChangesAsync(It.IsAny<CancellationToken>())); + _repository.Setup(p => p.Update(It.IsAny<DestinationApplicationEntity>())); + + var result = await _controller.Edit(entity); + var okResult = result.Result as OkObjectResult; + Assert.Equal(StatusCodes.Status200OK, okResult.StatusCode); + var updatedEntity = okResult.Value as DestinationApplicationEntity; + + Assert.Equal(entity.AeTitle, updatedEntity.AeTitle); + Assert.Equal(entity.HostIp, updatedEntity.HostIp); + Assert.Equal(entity.Name, updatedEntity.Name); + Assert.Equal(entity.Port, updatedEntity.Port); + + _repository.Verify(p => p.FindAsync(entity.Name), Times.Once()); + _repository.Verify(p => p.Update(entity), Times.Once()); + _repository.Verify(p => p.SaveChangesAsync(It.IsAny<CancellationToken>()), Times.Once()); + } + + [RetryFact(5, 250, DisplayName = "Update - Shall return 404 if input is null")] + public async Task Update_Returns404IfInputIsNull() + { + var result = await _controller.Edit(null); + + Assert.IsType<NotFoundResult>(result.Result); + } + + [RetryFact(5, 250, DisplayName = "Update - Shall return 404 if not found")] + public async Task Update_Returns404IfNotFound() + { + var entity = new DestinationApplicationEntity + { + AeTitle = "AET", + HostIp = "host", + Name = "AET", + Port = 123, + }; + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(default(DestinationApplicationEntity))); + + var result = await _controller.Edit(entity); + + Assert.IsType<NotFoundResult>(result.Result); + _repository.Verify(p => p.FindAsync(entity.Name), Times.Once()); + } + + [RetryFact(5, 250, DisplayName = "Update - Shall return problem on failure")] + public async Task Update_ShallReturnProblemOnFailure() + { + var value = "AET"; + var entity = new DestinationApplicationEntity + { + AeTitle = value, + HostIp = "host", + Name = value, + Port = 123, + }; + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(entity)); + _repository.Setup(p => p.Update(It.IsAny<DestinationApplicationEntity>())).Throws(new Exception("error")); + + var result = await _controller.Edit(entity); + + var objectResult = result.Result as ObjectResult; + Assert.NotNull(objectResult); + var problem = objectResult.Value as ProblemDetails; + Assert.NotNull(problem); + Assert.Equal("Error updating DICOM destination.", problem.Title); + Assert.Equal("error", problem.Detail); + Assert.Equal((int)HttpStatusCode.InternalServerError, problem.Status); + _repository.Verify(p => p.FindAsync(value), Times.Once()); + } + + #endregion Update + #region Delete - [RetryFact(5, 250, DisplayName = "GetAeTitle - Shall return deleted object")] + [RetryFact(5, 250, DisplayName = "Delete - Shall return deleted object")] public async Task Delete_ReturnsDeleted() { var value = "AET"; @@ -406,7 +494,7 @@ public async Task Delete_ReturnsDeleted() _repository.Verify(p => p.SaveChangesAsync(It.IsAny<CancellationToken>()), Times.Once()); } - [RetryFact(5, 250, DisplayName = "GetAeTitle - Shall return 404 if not found")] + [RetryFact(5, 250, DisplayName = "Delete - Shall return 404 if not found")] public async Task Delete_Returns404IfNotFound() { var value = "AET"; diff --git a/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs b/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs index 17876f2db..b10fc4147 100644 --- a/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs +++ b/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs @@ -65,13 +65,15 @@ public SourceAeTitleControllerTest() }; }); + var controllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() }; _repository = new Mock<IInformaticsGatewayRepository<SourceApplicationEntity>>(); _controller = new SourceAeTitleController( _logger.Object, _repository.Object) { - ProblemDetailsFactory = _problemDetailsFactory.Object + ProblemDetailsFactory = _problemDetailsFactory.Object, + ControllerContext = controllerContext }; } @@ -271,9 +273,92 @@ public async Task Create_ShallReturnCreatedAtAction() #endregion Create + #region Update + + [RetryFact(5, 250, DisplayName = "Update - Shall return updated")] + public async Task Update_ReturnsUpdated() + { + var entity = new SourceApplicationEntity + { + AeTitle = "AET", + HostIp = "host", + Name = "AET", + }; + + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(entity)); + _repository.Setup(p => p.Remove(It.IsAny<SourceApplicationEntity>())); + _repository.Setup(p => p.SaveChangesAsync(It.IsAny<CancellationToken>())); + _repository.Setup(p => p.Update(It.IsAny<SourceApplicationEntity>())); + + var result = await _controller.Edit(entity); + var okResult = result.Result as OkObjectResult; + Assert.Equal(StatusCodes.Status200OK, okResult.StatusCode); + var updatedEntity = okResult.Value as SourceApplicationEntity; + + Assert.Equal(entity.AeTitle, updatedEntity.AeTitle); + Assert.Equal(entity.HostIp, updatedEntity.HostIp); + Assert.Equal(entity.Name, updatedEntity.Name); + + _repository.Verify(p => p.FindAsync(entity.Name), Times.Once()); + _repository.Verify(p => p.Update(entity), Times.Once()); + _repository.Verify(p => p.SaveChangesAsync(It.IsAny<CancellationToken>()), Times.Once()); + } + + [RetryFact(5, 250, DisplayName = "Update - Shall return 404 if input is null")] + public async Task Update_Returns404IfInputIsNull() + { + var result = await _controller.Edit(null); + + Assert.IsType<NotFoundResult>(result.Result); + } + + [RetryFact(5, 250, DisplayName = "Update - Shall return 404 if not found")] + public async Task Update_Returns404IfNotFound() + { + var entity = new SourceApplicationEntity + { + AeTitle = "AET", + HostIp = "host", + Name = "AET", + }; + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(default(SourceApplicationEntity))); + + var result = await _controller.Edit(entity); + + Assert.IsType<NotFoundResult>(result.Result); + _repository.Verify(p => p.FindAsync(entity.Name), Times.Once()); + } + + [RetryFact(5, 250, DisplayName = "Update - Shall return problem on failure")] + public async Task Update_ShallReturnProblemOnFailure() + { + var value = "AET"; + var entity = new SourceApplicationEntity + { + AeTitle = value, + HostIp = "host", + Name = value, + }; + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(entity)); + _repository.Setup(p => p.Update(It.IsAny<SourceApplicationEntity>())).Throws(new Exception("error")); + + var result = await _controller.Edit(entity); + + var objectResult = result.Result as ObjectResult; + Assert.NotNull(objectResult); + var problem = objectResult.Value as ProblemDetails; + Assert.NotNull(problem); + Assert.Equal("Error updating DICOM source.", problem.Title); + Assert.Equal("error", problem.Detail); + Assert.Equal((int)HttpStatusCode.InternalServerError, problem.Status); + _repository.Verify(p => p.FindAsync(value), Times.Once()); + } + + #endregion Update + #region Delete - [RetryFact(5, 250, DisplayName = "GetAeTitle - Shall return deleted object")] + [RetryFact(5, 250, DisplayName = "Delete - Shall return deleted object")] public async Task Delete_ReturnsDeleted() { var value = "AET"; @@ -297,7 +382,7 @@ public async Task Delete_ReturnsDeleted() _repository.Verify(p => p.SaveChangesAsync(It.IsAny<CancellationToken>()), Times.Once()); } - [RetryFact(5, 250, DisplayName = "GetAeTitle - Shall return 404 if not found")] + [RetryFact(5, 250, DisplayName = "Delete - Shall return 404 if not found")] public async Task Delete_Returns404IfNotFound() { var value = "AET"; From e0167ed8fe3fb70f2858bf50bda33db3144a21d6 Mon Sep 17 00:00:00 2001 From: Victor Chang <vicchang@nvidia.com> Date: Tue, 4 Oct 2022 13:19:02 -0700 Subject: [PATCH 2/4] Use ICollectionFIxture to share SCP listener context Signed-off-by: Victor Chang <vicchang@nvidia.com> --- .../Services/Export/ScuExportServiceTest.cs | 26 +++---------------- .../Test/Services/Scu/ScuServiceTest.cs | 26 +++---------------- .../Test/Shared/DicomScpFixture.cs | 8 +++++- 3 files changed, 13 insertions(+), 47 deletions(-) diff --git a/src/InformaticsGateway/Test/Services/Export/ScuExportServiceTest.cs b/src/InformaticsGateway/Test/Services/Export/ScuExportServiceTest.cs index 65fde92bc..e53f812be 100644 --- a/src/InformaticsGateway/Test/Services/Export/ScuExportServiceTest.cs +++ b/src/InformaticsGateway/Test/Services/Export/ScuExportServiceTest.cs @@ -42,7 +42,8 @@ namespace Monai.Deploy.InformaticsGateway.Test.Services.Export { - public class ScuExportServiceTest : IClassFixture<DicomScpFixture>, IDisposable + [Collection("SCP Listener")] + public class ScuExportServiceTest { private readonly Mock<IStorageService> _storageService; private readonly Mock<IMessageBrokerSubscriberService> _messageSubscriberService; @@ -57,8 +58,7 @@ public class ScuExportServiceTest : IClassFixture<DicomScpFixture>, IDisposable private readonly CancellationTokenSource _cancellationTokenSource; private readonly DicomScpFixture _dicomScp; - private readonly int _port = 11104; - private bool _disposedValue; + private readonly int _port = 1104; public ScuExportServiceTest(DicomScpFixture dicomScp) { @@ -529,25 +529,5 @@ private async Task StopAndVerify(ScuExportService service) _logger.VerifyLogging($"{service.ServiceName} is stopping.", LogLevel.Information, Times.Once()); Thread.Sleep(500); } - - protected virtual void Dispose(bool disposing) - { - if (!_disposedValue) - { - if (disposing) - { - _dicomScp.Dispose(); - } - - _disposedValue = true; - } - } - - public void Dispose() - { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Dispose(disposing: true); - GC.SuppressFinalize(this); - } } } diff --git a/src/InformaticsGateway/Test/Services/Scu/ScuServiceTest.cs b/src/InformaticsGateway/Test/Services/Scu/ScuServiceTest.cs index 8c5c44aee..11b563b01 100644 --- a/src/InformaticsGateway/Test/Services/Scu/ScuServiceTest.cs +++ b/src/InformaticsGateway/Test/Services/Scu/ScuServiceTest.cs @@ -31,10 +31,11 @@ namespace Monai.Deploy.InformaticsGateway.Test.Services.Scu { - public class ScuServiceTest : IClassFixture<DicomScpFixture>, IDisposable + [Collection("SCP Listener")] + public class ScuServiceTest { private readonly DicomScpFixture _dicomScp; - private readonly int _port = 11105; + private readonly int _port = 1104; private readonly Mock<IServiceScopeFactory> _serviceScopeFactory; private readonly Mock<ILogger<ScuService>> _logger; @@ -45,7 +46,6 @@ public class ScuServiceTest : IClassFixture<DicomScpFixture>, IDisposable private readonly CancellationTokenSource _cancellationTokenSource; private readonly ServiceProvider _serviceProvider; private readonly Mock<IServiceScope> _serviceScope; - private bool _disposedValue; public ScuServiceTest(DicomScpFixture dicomScp) { @@ -156,25 +156,5 @@ public async Task GivenACEchoRequest_WhenRemoteServerIsUnreachable_ReturnStatusA Assert.Equal(ResponseError.Unhandled, response.Error); Assert.StartsWith("One or more error", response.Message); } - - protected virtual void Dispose(bool disposing) - { - if (!_disposedValue) - { - if (disposing) - { - _dicomScp.Dispose(); - } - - _disposedValue = true; - } - } - - public void Dispose() - { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Dispose(disposing: true); - GC.SuppressFinalize(this); - } } } diff --git a/src/InformaticsGateway/Test/Shared/DicomScpFixture.cs b/src/InformaticsGateway/Test/Shared/DicomScpFixture.cs index e2f35440f..77924612e 100644 --- a/src/InformaticsGateway/Test/Shared/DicomScpFixture.cs +++ b/src/InformaticsGateway/Test/Shared/DicomScpFixture.cs @@ -21,9 +21,15 @@ using FellowOakDicom; using FellowOakDicom.Network; using Microsoft.Extensions.Logging; +using Xunit; namespace Monai.Deploy.InformaticsGateway.SharedTest { + [CollectionDefinition("SCP Listener")] + public class DicomScpFixtureCollection : ICollectionFixture<DicomScpFixture> + { + } + public class DicomScpFixture : IDisposable { internal static string s_aETITLE = "STORESCP"; @@ -37,7 +43,7 @@ public DicomScpFixture() { } - public void Start(int port = 11104) + public void Start(int port = 1104) { if (_server is null) { From 3bd0921061034019ad84942642821f48ee218961 Mon Sep 17 00:00:00 2001 From: Victor Chang <vicchang@nvidia.com> Date: Tue, 4 Oct 2022 14:33:36 -0700 Subject: [PATCH 3/4] Additional unit tests for Update API Signed-off-by: Victor Chang <vicchang@nvidia.com> --- src/Client/Test/AeTitleServiceTest.cs | 70 +++++++++++++++++++ .../Http/DestinationAeTitleController.cs | 4 ++ .../Http/DestinationAeTitleControllerTest.cs | 25 ++++++- .../Http/SourceAeTitleControllerTest.cs | 23 ++++++ 4 files changed, 121 insertions(+), 1 deletion(-) diff --git a/src/Client/Test/AeTitleServiceTest.cs b/src/Client/Test/AeTitleServiceTest.cs index 181970029..ef68418bd 100644 --- a/src/Client/Test/AeTitleServiceTest.cs +++ b/src/Client/Test/AeTitleServiceTest.cs @@ -109,6 +109,76 @@ public async Task Create_ReturnsAProblem() Assert.Equal($"HTTP Status: {problem.Status}. {problem.Detail}", result.Message); } + [Fact(DisplayName = "AE Title - Update")] + public async Task Update() + { + var aet = new SourceApplicationEntity() + { + AeTitle = "Test", + Name = "Test Name", + HostIp = "1.2.3.4" + }; + + var json = JsonSerializer.Serialize(aet, Configuration.JsonSerializationOptions); + + var rootUri = new Uri("http://localhost:5000"); + var uriPath = "config/source"; + + var httpResponse = new HttpResponseMessage + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent(json, Encoding.UTF8, "application/json") + }; + + var httpClient = SetupHttpClientMock(rootUri, HttpMethod.Put, httpResponse); + + var service = new AeTitleService<SourceApplicationEntity>(uriPath, httpClient, _logger.Object); + + var result = await service.Update(aet, CancellationToken.None); + + Assert.Equal(aet.AeTitle, result.AeTitle); + Assert.Equal(aet.Name, result.Name); + Assert.Equal(aet.HostIp, result.HostIp); + } + + [Fact(DisplayName = "AE Title - Update returns a problem")] + public async Task Update_ReturnsAProblem() + { + var aet = new DestinationApplicationEntity() + { + AeTitle = "Test", + Name = "Test Name", + HostIp = "host", + Port = 123 + }; + + var problem = new ProblemDetails + { + Title = "Problem Title", + Detail = "Problem Detail", + Status = 500 + }; + + var json = JsonSerializer.Serialize(problem, Configuration.JsonSerializationOptions); + + var rootUri = new Uri("http://localhost:5000"); + var uriPath = "config/destination"; + + var httpResponse = new HttpResponseMessage + { + StatusCode = HttpStatusCode.InternalServerError, + Content = new StringContent(json, Encoding.UTF8, "application/json") + }; + + var httpClient = SetupHttpClientMock(rootUri, HttpMethod.Put, httpResponse); + + var service = new AeTitleService<DestinationApplicationEntity>(uriPath, httpClient, _logger.Object); + + var result = await Assert.ThrowsAsync<ProblemException>(async () => await service.Update(aet, CancellationToken.None)); + + Assert.Equal($"HTTP Status: {problem.Status}. {problem.Detail}", result.Message); + } + [Fact(DisplayName = "AE Title - Get")] public async Task Get() { diff --git a/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs b/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs index ace57f5c0..7a0bc3b08 100644 --- a/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs +++ b/src/InformaticsGateway/Services/Http/DestinationAeTitleController.cs @@ -208,6 +208,10 @@ public async Task<ActionResult<DestinationApplicationEntity>> Edit(DestinationAp _logger.DestinationApplicationEntityUpdated(item.Name, item.AeTitle, item.HostIp, item.Port); return Ok(destinationApplicationEntity); } + catch (ConfigurationException ex) + { + return Problem(title: "Validation error.", statusCode: (int)System.Net.HttpStatusCode.BadRequest, detail: ex.Message); + } catch (Exception ex) { _logger.ErrorDeletingDestinationApplicationEntity(ex); diff --git a/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs b/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs index 49bdff6e5..7d14b2758 100644 --- a/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs +++ b/src/InformaticsGateway/Test/Services/Http/DestinationAeTitleControllerTest.cs @@ -29,7 +29,6 @@ using Monai.Deploy.InformaticsGateway.Services.Http; using Monai.Deploy.InformaticsGateway.Services.Scu; using Moq; -using Org.BouncyCastle.Asn1.Cms; using xRetry; using Xunit; @@ -466,6 +465,30 @@ public async Task Update_ShallReturnProblemOnFailure() _repository.Verify(p => p.FindAsync(value), Times.Once()); } + [RetryFact(5, 250, DisplayName = "Update - Shall return problem on validation failure")] + public async Task Update_ShallReturnBadRequestWithBadAeTitle() + { + var aeTitle = "TOOOOOOOOOOOOOOOOOOOOOOOLONG"; + var entity = new DestinationApplicationEntity + { + Name = aeTitle, + AeTitle = aeTitle, + HostIp = "host", + Port = 1 + }; + + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(entity)); + var result = await _controller.Edit(entity); + + var objectResult = result.Result as ObjectResult; + Assert.NotNull(objectResult); + var problem = objectResult.Value as ProblemDetails; + Assert.NotNull(problem); + Assert.Equal("Validation error.", problem.Title); + Assert.Equal($"'{aeTitle}' is not a valid AE Title (source: DestinationApplicationEntity).", problem.Detail); + Assert.Equal((int)HttpStatusCode.BadRequest, problem.Status); + } + #endregion Update #region Delete diff --git a/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs b/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs index b10fc4147..5cf5d1d73 100644 --- a/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs +++ b/src/InformaticsGateway/Test/Services/Http/SourceAeTitleControllerTest.cs @@ -354,6 +354,29 @@ public async Task Update_ShallReturnProblemOnFailure() _repository.Verify(p => p.FindAsync(value), Times.Once()); } + [RetryFact(5, 250, DisplayName = "Update - Shall return problem on validation failure")] + public async Task Update_ShallReturnBadRequestWithBadAeTitle() + { + var aeTitle = "TOOOOOOOOOOOOOOOOOOOOOOOLONG"; + var entity = new SourceApplicationEntity + { + Name = aeTitle, + AeTitle = aeTitle, + HostIp = "host", + }; + + _repository.Setup(p => p.FindAsync(It.IsAny<string>())).Returns(Task.FromResult(entity)); + var result = await _controller.Edit(entity); + + var objectResult = result.Result as ObjectResult; + Assert.NotNull(objectResult); + var problem = objectResult.Value as ProblemDetails; + Assert.NotNull(problem); + Assert.Equal("Validation error.", problem.Title); + Assert.Equal($"'{aeTitle}' is not a valid AE Title (source: SourceApplicationEntity).", problem.Detail); + Assert.Equal((int)HttpStatusCode.BadRequest, problem.Status); + } + #endregion Update #region Delete From 4d25fdfc05f4644124d037f641ef264247aec469 Mon Sep 17 00:00:00 2001 From: Victor Chang <vicchang@nvidia.com> Date: Tue, 4 Oct 2022 14:37:43 -0700 Subject: [PATCH 4/4] Log association elapsed time Signed-off-by: Victor Chang <vicchang@nvidia.com> --- .../Logging/Log.100.200.ScpService.cs | 4 ++-- .../Services/Scp/ScpServiceInternal.cs | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/InformaticsGateway/Logging/Log.100.200.ScpService.cs b/src/InformaticsGateway/Logging/Log.100.200.ScpService.cs index e71cd49b8..1075403be 100644 --- a/src/InformaticsGateway/Logging/Log.100.200.ScpService.cs +++ b/src/InformaticsGateway/Logging/Log.100.200.ScpService.cs @@ -86,8 +86,8 @@ public static partial class Log [LoggerMessage(EventId = 208, Level = LogLevel.Warning, Message = "Aborted {source} with reason {reason}.")] public static partial void CStoreAbort(this ILogger logger, DicomAbortSource source, DicomAbortReason reason); - [LoggerMessage(EventId = 209, Level = LogLevel.Information, Message = "Association release request received.")] - public static partial void CStoreAssociationReleaseRequest(this ILogger logger); + [LoggerMessage(EventId = 209, Level = LogLevel.Information, Message = "Association release request received. Connection Time: {elapsedTime}.")] + public static partial void CStoreAssociationReleaseRequest(this ILogger logger, TimeSpan elapsedTime); [LoggerMessage(EventId = 210, Level = LogLevel.Information, Message = "Association received from {host}:{port}.")] public static partial void CStoreAssociationReceived(this ILogger logger, string host, int port); diff --git a/src/InformaticsGateway/Services/Scp/ScpServiceInternal.cs b/src/InformaticsGateway/Services/Scp/ScpServiceInternal.cs index 2f683ceaa..a584b89a0 100644 --- a/src/InformaticsGateway/Services/Scp/ScpServiceInternal.cs +++ b/src/InformaticsGateway/Services/Scp/ScpServiceInternal.cs @@ -41,6 +41,7 @@ internal class ScpServiceInternal : private IApplicationEntityManager _associationDataProvider; private IDisposable _loggerScope; private Guid _associationId; + private DateTimeOffset? _associationReceived; public ScpServiceInternal(INetworkStream stream, Encoding fallbackEncoding, FellowOakDicom.Log.ILogger log, DicomServiceDependencies dicomServiceDependencies) : base(stream, fallbackEncoding, log, dicomServiceDependencies) @@ -106,13 +107,20 @@ public void OnReceiveAbort(DicomAbortSource source, DicomAbortReason reason) /// <returns></returns> public Task OnReceiveAssociationReleaseRequestAsync() { - _logger?.CStoreAssociationReleaseRequest(); + var associationElapsed = TimeSpan.Zero; + if (_associationReceived.HasValue) + { + associationElapsed = DateTimeOffset.UtcNow.Subtract(_associationReceived.Value); + } + + _logger?.CStoreAssociationReleaseRequest(associationElapsed); return SendAssociationReleaseResponseAsync(); } public Task OnReceiveAssociationRequestAsync(DicomAssociation association) { Interlocked.Increment(ref ScpService.ActiveConnections); + _associationReceived = DateTimeOffset.UtcNow; _associationDataProvider = UserState as IApplicationEntityManager; if (_associationDataProvider is null)