Skip to content

Commit

Permalink
Revert "Ability to switch temporary storage to use either memory or d…
Browse files Browse the repository at this point in the history
…isk (#166)"

This reverts commit 09887b1.
  • Loading branch information
mocsharp authored Sep 23, 2022
1 parent 09887b1 commit daf78fe
Show file tree
Hide file tree
Showing 26 changed files with 80 additions and 353 deletions.
2 changes: 0 additions & 2 deletions src/Api/Storage/Payload.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ public enum PayloadState

public bool HasTimedOut { get => ElapsedTime().TotalSeconds >= Timeout; }

public TimeSpan Elapsed { get { return DateTime.UtcNow.Subtract(DateTimeCreated); } }

public string CallingAeTitle { get => Files.OfType<DicomFileStorageMetadata>().Select(p => p.CallingAeTitle).FirstOrDefault(); }

public string CalledAeTitle { get => Files.OfType<DicomFileStorageMetadata>().Select(p => p.CalledAeTitle).FirstOrDefault(); }
Expand Down
25 changes: 3 additions & 22 deletions src/Api/Storage/StorageObjectMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

using System;
using System.IO;
using System.Runtime;
using System.Text.Json.Serialization;
using Ardalis.GuardClauses;

Expand Down Expand Up @@ -124,27 +123,9 @@ public void SetUploaded(string bucketName)

if (Data is not null && Data.CanSeek)
{
if (Data is FileStream fileStream)
{
var filename = fileStream.Name;
Data.Close();
Data.Dispose();
Data = null;
System.IO.File.Delete(filename);
}
else // MemoryStream
{
Data.Close();
Data.Dispose();
Data = null;

// When IG stores all received/downloaded data in-memory using MemoryStream, LOH grows tremendously and thus impacts the performance and
// memory usage. The following makes sure LOH is compacted after the data is uploaded.
GCSettings.LargeObjectHeapCompactionMode = GCLargeObjectHeapCompactionMode.CompactOnce;
#pragma warning disable S1215 // "GC.Collect" should not be called
GC.Collect();
#pragma warning restore S1215 // "GC.Collect" should not be called
}
Data.Close();
Data.Dispose();
Data = null;
}
}

Expand Down
38 changes: 1 addition & 37 deletions src/Configuration/ConfigurationValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

using System;
using System.Collections.Generic;
using System.IO;
using System.IO.Abstractions;
using System.Text.RegularExpressions;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
Expand All @@ -31,18 +29,16 @@ namespace Monai.Deploy.InformaticsGateway.Configuration
public class ConfigurationValidator : IValidateOptions<InformaticsGatewayConfiguration>
{
private readonly ILogger<ConfigurationValidator> _logger;
private readonly IFileSystem _fileSystem;
private readonly List<string> _validationErrors;

/// <summary>
/// Initializes an instance of the <see cref="ConfigurationValidator"/> class.
/// </summary>
/// <param name="configuration">InformaticsGatewayConfiguration to be validated</param>
/// <param name="logger">Logger to be used by ConfigurationValidator</param>
public ConfigurationValidator(ILogger<ConfigurationValidator> logger, IFileSystem fileSystem)
public ConfigurationValidator(ILogger<ConfigurationValidator> logger)
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_fileSystem = fileSystem ?? throw new ArgumentNullException(nameof(fileSystem));
_validationErrors = new List<string>();
}

Expand Down Expand Up @@ -86,38 +82,6 @@ private bool IsStorageValid(StorageConfiguration storage)
valid &= IsValueInRange("InformaticsGateway>storage>reserveSpaceGB", 1, 999, storage.ReserveSpaceGB);
valid &= IsValueInRange("InformaticsGateway>storage>payloadProcessThreads", 1, 128, storage.PayloadProcessThreads);
valid &= IsValueInRange("InformaticsGateway>storage>concurrentUploads", 1, 128, storage.ConcurrentUploads);
valid &= IsValueInRange("InformaticsGateway>storage>memoryThreshold", 1, int.MaxValue, storage.MemoryThreshold);

if (storage.TemporaryDataStorage == TemporaryDataStorageLocation.Disk)
{
valid &= IsNotNullOrWhiteSpace("InformaticsGateway>storage>bufferRootPath", storage.BufferStorageRootPath);
valid &= IsValidDirectory("InformaticsGateway>storage>bufferRootPath", storage.BufferStorageRootPath);
valid &= IsValueInRange("InformaticsGateway>storage>bufferSize", 1, int.MaxValue, storage.BufferSize);
}

return valid;
}

private bool IsValidDirectory(string source, string directory)
{
var valid = true;
try
{
if (!_fileSystem.Directory.Exists(directory))
{
valid = false;
_validationErrors.Add($"Directory `{directory}` specified in `{source}` does not exist.");
}
else
{
using var _ = _fileSystem.File.Create(Path.Combine(directory, Path.GetRandomFileName()), 1, FileOptions.DeleteOnClose);
}
}
catch (Exception ex)
{
valid = false;
_validationErrors.Add($"Directory `{directory}` specified in `{source}` is not accessible: {ex.Message}.");
}
return valid;
}

Expand Down
15 changes: 0 additions & 15 deletions src/Configuration/StorageConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,13 @@ namespace Monai.Deploy.InformaticsGateway.Configuration
{
public class StorageConfiguration : StorageServiceConfiguration
{
/// <summary>
/// Gets or sets whether to store temporary data in <c>Memory</c> or on <c>Disk</c>.
/// Defaults to <c>Memory</c>.
/// </summary>
[ConfigurationKeyName("tempStorageLocation")]
public TemporaryDataStorageLocation TemporaryDataStorage { get; set; } = TemporaryDataStorageLocation.Memory;

/// <summary>
/// Gets or sets the path used for buffering incoming data.
/// Defaults to <c>./temp</c>.
/// </summary>
[ConfigurationKeyName("bufferRootPath")]
public string BufferStorageRootPath { get; set; } = "./temp";

/// <summary>
/// Gets or sets the number of bytes buffered for reads and writes to the temporary file.
/// Defaults to <c>128000</c>.
/// </summary>
[ConfigurationKeyName("bufferSize")]
public int BufferSize { get; set; } = 128000;


/// <summary>
/// Gets or set the maximum memory buffer size in bytes with default to 30MiB.
/// </summary>
Expand Down
24 changes: 0 additions & 24 deletions src/Configuration/TemporaryDataStorageLocation.cs

This file was deleted.

38 changes: 7 additions & 31 deletions src/Configuration/Test/ConfigurationValidatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/

using System;
using System.IO;
using System.IO.Abstractions;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Monai.Deploy.InformaticsGateway.SharedTest;
Expand All @@ -28,19 +26,17 @@ namespace Monai.Deploy.InformaticsGateway.Configuration.Test
public class ConfigurationValidatorTest
{
private readonly Mock<ILogger<ConfigurationValidator>> _logger;
private readonly Mock<IFileSystem> _fileSystem;

public ConfigurationValidatorTest()
{
_logger = new Mock<ILogger<ConfigurationValidator>>();
_fileSystem = new Mock<IFileSystem>();
}

[Fact(DisplayName = "ConfigurationValidator test with all valid settings")]
public void AllValid()
{
var config = MockValidConfiguration();
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
Assert.True(valid == ValidateOptionsResult.Success);
}

Expand All @@ -50,7 +46,7 @@ public void InvalidScpPort()
var config = MockValidConfiguration();
config.Dicom.Scp.Port = Int32.MaxValue;

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);

var validationMessage = $"Invalid port number '{Int32.MaxValue}' specified for InformaticsGateway>dicom>scp>port.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -63,7 +59,7 @@ public void InvalidScpMaxAssociations()
var config = MockValidConfiguration();
config.Dicom.Scp.MaximumNumberOfAssociations = 0;

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);

var validationMessage = $"Value of InformaticsGateway>dicom>scp>max-associations must be between {1} and {1000}.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -76,7 +72,7 @@ public void StorageWithInvalidWatermark()
var config = MockValidConfiguration();
config.Storage.Watermark = 1000;

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);

var validationMessage = "Value of InformaticsGateway>storage>watermark must be between 1 and 100.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -89,7 +85,7 @@ public void StorageWithInvalidReservedSpace()
var config = MockValidConfiguration();
config.Storage.ReserveSpaceGB = 9999;

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);

var validationMessage = "Value of InformaticsGateway>storage>reserveSpaceGB must be between 1 and 999.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -102,7 +98,7 @@ public void StorageWithInvalidTemporaryBucketName()
var config = MockValidConfiguration();
config.Storage.TemporaryStorageBucket = " ";

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);

var validationMessages = new[] { "Value for InformaticsGateway>storage>temporaryBucketName is required.", "Value for InformaticsGateway>storage>temporaryBucketName does not conform to Amazon S3 bucket naming requirements." };
Assert.Equal(string.Join(Environment.NewLine, validationMessages), valid.FailureMessage);
Expand All @@ -118,7 +114,7 @@ public void StorageWithInvalidBucketName()
var config = MockValidConfiguration();
config.Storage.StorageServiceBucketName = "";

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);

var validationMessages = new[] { "Value for InformaticsGateway>storage>bucketName is required.", "Value for InformaticsGateway>storage>bucketName does not conform to Amazon S3 bucket naming requirements." };
Assert.Equal(string.Join(Environment.NewLine, validationMessages), valid.FailureMessage);
Expand All @@ -128,26 +124,6 @@ public void StorageWithInvalidBucketName()
}
}

[Fact(DisplayName = "ConfigurationValidator test with inaccessible directory")]
public void StorageWithInaccessbleDirectory()
{
_fileSystem.Setup(p => p.Directory.Exists(It.IsAny<string>())).Returns(true);
_fileSystem.Setup(p => p.File.Create(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<FileOptions>())).Throws(new UnauthorizedAccessException("error"));

var config = MockValidConfiguration();
config.Storage.TemporaryDataStorage = TemporaryDataStorageLocation.Disk;
config.Storage.BufferStorageRootPath = "/blabla";

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessages = new[] { $"Directory `/blabla` specified in `InformaticsGateway>storage>bufferRootPath` is not accessible: error." };
Assert.Equal(string.Join(Environment.NewLine, validationMessages), valid.FailureMessage);
foreach (var message in validationMessages)
{
_logger.VerifyLogging(message, LogLevel.Error, Times.Once());
}
}

private static InformaticsGatewayConfiguration MockValidConfiguration()
{
var config = new InformaticsGatewayConfiguration();
Expand Down
1 change: 0 additions & 1 deletion src/Database/PayloadConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ public void Configure(EntityTypeBuilder<Payload> builder)
builder.Ignore(j => j.CalledAeTitle);
builder.Ignore(j => j.CallingAeTitle);
builder.Ignore(j => j.HasTimedOut);
builder.Ignore(j => j.Elapsed);
builder.Ignore(j => j.Count);
}
}
Expand Down
75 changes: 12 additions & 63 deletions src/InformaticsGateway/Common/FileStorageMetadataExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,92 +14,41 @@
* limitations under the License.
*/

using System.IO.Abstractions;
using System.IO;
using System.Text;
using System.Threading.Tasks;
using Ardalis.GuardClauses;
using FellowOakDicom;
using Monai.Deploy.InformaticsGateway.Api.Storage;
using Monai.Deploy.InformaticsGateway.Configuration;

namespace Monai.Deploy.InformaticsGateway.Common
{
internal static class FileStorageMetadataExtensions
{
public static async Task SetDataStreams(
this DicomFileStorageMetadata dicomFileStorageMetadata,
DicomFile dicomFile,
string dicomJson,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
public static async Task SetDataStreams(this DicomFileStorageMetadata dicomFileStorageMetadata, DicomFile dicomFile, string dicomJson)
{
Guard.Against.Null(dicomFile, nameof(dicomFile));
Guard.Against.Null(dicomJson, nameof(dicomJson)); // allow empty here

switch (storageLocation)
{
case TemporaryDataStorageLocation.Disk:
Guard.Against.Null(fileSystem, nameof(fileSystem));
Guard.Against.NullOrWhiteSpace(temporaryStoragePath, nameof(temporaryStoragePath));

var tempFile = fileSystem.Path.Combine(temporaryStoragePath, $@"{System.DateTime.UtcNow.Ticks}.tmp");
dicomFileStorageMetadata.File.Data = fileSystem.File.Create(tempFile);
break;
default:
dicomFileStorageMetadata.File.Data = new System.IO.MemoryStream();
break;
}

dicomFileStorageMetadata.File.Data = new MemoryStream();
await dicomFile.SaveAsync(dicomFileStorageMetadata.File.Data).ConfigureAwait(false);
dicomFileStorageMetadata.File.Data.Seek(0, System.IO.SeekOrigin.Begin);
dicomFileStorageMetadata.File.Data.Seek(0, SeekOrigin.Begin);

await SetTextStream(dicomFileStorageMetadata.JsonFile, dicomJson, storageLocation, fileSystem, temporaryStoragePath);
SetTextStream(dicomFileStorageMetadata.JsonFile, dicomJson);
}

public static async Task SetDataStream(
this FhirFileStorageMetadata fhirFileStorageMetadata,
string json,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
=> await SetTextStream(fhirFileStorageMetadata.File, json, storageLocation, fileSystem, temporaryStoragePath);
public static void SetDataStream(this FhirFileStorageMetadata fhirFileStorageMetadata, string json)
=> SetTextStream(fhirFileStorageMetadata.File, json);

public static async Task SetDataStream(
this Hl7FileStorageMetadata hl7FileStorageMetadata,
string message,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
=> await SetTextStream(hl7FileStorageMetadata.File, message, storageLocation, fileSystem, temporaryStoragePath);
public static void SetDataStream(this Hl7FileStorageMetadata hl7FileStorageMetadata, string message)
=> SetTextStream(hl7FileStorageMetadata.File, message);

private static async Task SetTextStream(
StorageObjectMetadata storageObjectMetadata,
string message,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
private static void SetTextStream(StorageObjectMetadata storageObjectMetadata, string message)
{
Guard.Against.Null(message, nameof(message)); // allow empty here

switch (storageLocation)
{
case TemporaryDataStorageLocation.Disk:
Guard.Against.Null(fileSystem, nameof(fileSystem));
Guard.Against.NullOrWhiteSpace(temporaryStoragePath, nameof(temporaryStoragePath));

var tempFile = fileSystem.Path.Combine(temporaryStoragePath, $@"{System.DateTime.UtcNow.Ticks}.tmp");
var stream = fileSystem.File.Create(tempFile);
var data = Encoding.UTF8.GetBytes(message);
await stream.WriteAsync(data, 0, data.Length);
storageObjectMetadata.Data = stream;
break;
default:
storageObjectMetadata.Data = new System.IO.MemoryStream(Encoding.UTF8.GetBytes(message));
break;
}

storageObjectMetadata.Data.Seek(0, System.IO.SeekOrigin.Begin);
storageObjectMetadata.Data = new MemoryStream(Encoding.UTF8.GetBytes(message));
storageObjectMetadata.Data.Seek(0, SeekOrigin.Begin);
}
}
}
Loading

0 comments on commit daf78fe

Please sign in to comment.