Skip to content

Commit dc898fe

Browse files
mocsharpmocsharp
authored
Revert "Ability to switch temporary storage to use either memory or disk (#166)" (#167)
This reverts commit 09887b1.
1 parent 09887b1 commit dc898fe

26 files changed

+80
-353
lines changed

‎src/Api/Storage/Payload.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ public enum PayloadState
6666

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

69-
public TimeSpan Elapsed { get { return DateTime.UtcNow.Subtract(DateTimeCreated); } }
70-
7169
public string CallingAeTitle { get => Files.OfType<DicomFileStorageMetadata>().Select(p => p.CallingAeTitle).FirstOrDefault(); }
7270

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

‎src/Api/Storage/StorageObjectMetadata.cs

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
using System;
1818
using System.IO;
19-
using System.Runtime;
2019
using System.Text.Json.Serialization;
2120
using Ardalis.GuardClauses;
2221

@@ -124,27 +123,9 @@ public void SetUploaded(string bucketName)
124123

125124
if (Data is not null && Data.CanSeek)
126125
{
127-
if (Data is FileStream fileStream)
128-
{
129-
var filename = fileStream.Name;
130-
Data.Close();
131-
Data.Dispose();
132-
Data = null;
133-
System.IO.File.Delete(filename);
134-
}
135-
else // MemoryStream
136-
{
137-
Data.Close();
138-
Data.Dispose();
139-
Data = null;
140-
141-
// When IG stores all received/downloaded data in-memory using MemoryStream, LOH grows tremendously and thus impacts the performance and
142-
// memory usage. The following makes sure LOH is compacted after the data is uploaded.
143-
GCSettings.LargeObjectHeapCompactionMode = GCLargeObjectHeapCompactionMode.CompactOnce;
144-
#pragma warning disable S1215 // "GC.Collect" should not be called
145-
GC.Collect();
146-
#pragma warning restore S1215 // "GC.Collect" should not be called
147-
}
126+
Data.Close();
127+
Data.Dispose();
128+
Data = null;
148129
}
149130
}
150131

‎src/Configuration/ConfigurationValidator.cs

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717

1818
using System;
1919
using System.Collections.Generic;
20-
using System.IO;
21-
using System.IO.Abstractions;
2220
using System.Text.RegularExpressions;
2321
using Microsoft.Extensions.Logging;
2422
using Microsoft.Extensions.Options;
@@ -31,18 +29,16 @@ namespace Monai.Deploy.InformaticsGateway.Configuration
3129
public class ConfigurationValidator : IValidateOptions<InformaticsGatewayConfiguration>
3230
{
3331
private readonly ILogger<ConfigurationValidator> _logger;
34-
private readonly IFileSystem _fileSystem;
3532
private readonly List<string> _validationErrors;
3633

3734
/// <summary>
3835
/// Initializes an instance of the <see cref="ConfigurationValidator"/> class.
3936
/// </summary>
4037
/// <param name="configuration">InformaticsGatewayConfiguration to be validated</param>
4138
/// <param name="logger">Logger to be used by ConfigurationValidator</param>
42-
public ConfigurationValidator(ILogger<ConfigurationValidator> logger, IFileSystem fileSystem)
39+
public ConfigurationValidator(ILogger<ConfigurationValidator> logger)
4340
{
4441
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
45-
_fileSystem = fileSystem ?? throw new ArgumentNullException(nameof(fileSystem));
4642
_validationErrors = new List<string>();
4743
}
4844

@@ -86,38 +82,6 @@ private bool IsStorageValid(StorageConfiguration storage)
8682
valid &= IsValueInRange("InformaticsGateway>storage>reserveSpaceGB", 1, 999, storage.ReserveSpaceGB);
8783
valid &= IsValueInRange("InformaticsGateway>storage>payloadProcessThreads", 1, 128, storage.PayloadProcessThreads);
8884
valid &= IsValueInRange("InformaticsGateway>storage>concurrentUploads", 1, 128, storage.ConcurrentUploads);
89-
valid &= IsValueInRange("InformaticsGateway>storage>memoryThreshold", 1, int.MaxValue, storage.MemoryThreshold);
90-
91-
if (storage.TemporaryDataStorage == TemporaryDataStorageLocation.Disk)
92-
{
93-
valid &= IsNotNullOrWhiteSpace("InformaticsGateway>storage>bufferRootPath", storage.BufferStorageRootPath);
94-
valid &= IsValidDirectory("InformaticsGateway>storage>bufferRootPath", storage.BufferStorageRootPath);
95-
valid &= IsValueInRange("InformaticsGateway>storage>bufferSize", 1, int.MaxValue, storage.BufferSize);
96-
}
97-
98-
return valid;
99-
}
100-
101-
private bool IsValidDirectory(string source, string directory)
102-
{
103-
var valid = true;
104-
try
105-
{
106-
if (!_fileSystem.Directory.Exists(directory))
107-
{
108-
valid = false;
109-
_validationErrors.Add($"Directory `{directory}` specified in `{source}` does not exist.");
110-
}
111-
else
112-
{
113-
using var _ = _fileSystem.File.Create(Path.Combine(directory, Path.GetRandomFileName()), 1, FileOptions.DeleteOnClose);
114-
}
115-
}
116-
catch (Exception ex)
117-
{
118-
valid = false;
119-
_validationErrors.Add($"Directory `{directory}` specified in `{source}` is not accessible: {ex.Message}.");
120-
}
12185
return valid;
12286
}
12387

‎src/Configuration/StorageConfiguration.cs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,13 @@ namespace Monai.Deploy.InformaticsGateway.Configuration
2222
{
2323
public class StorageConfiguration : StorageServiceConfiguration
2424
{
25-
/// <summary>
26-
/// Gets or sets whether to store temporary data in <c>Memory</c> or on <c>Disk</c>.
27-
/// Defaults to <c>Memory</c>.
28-
/// </summary>
29-
[ConfigurationKeyName("tempStorageLocation")]
30-
public TemporaryDataStorageLocation TemporaryDataStorage { get; set; } = TemporaryDataStorageLocation.Memory;
31-
3225
/// <summary>
3326
/// Gets or sets the path used for buffering incoming data.
3427
/// Defaults to <c>./temp</c>.
3528
/// </summary>
3629
[ConfigurationKeyName("bufferRootPath")]
3730
public string BufferStorageRootPath { get; set; } = "./temp";
3831

39-
/// <summary>
40-
/// Gets or sets the number of bytes buffered for reads and writes to the temporary file.
41-
/// Defaults to <c>128000</c>.
42-
/// </summary>
43-
[ConfigurationKeyName("bufferSize")]
44-
public int BufferSize { get; set; } = 128000;
45-
46-
4732
/// <summary>
4833
/// Gets or set the maximum memory buffer size in bytes with default to 30MiB.
4934
/// </summary>

‎src/Configuration/TemporaryDataStorageLocation.cs

Lines changed: 0 additions & 24 deletions
This file was deleted.

‎src/Configuration/Test/ConfigurationValidatorTest.cs

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616

1717
using System;
18-
using System.IO;
19-
using System.IO.Abstractions;
2018
using Microsoft.Extensions.Logging;
2119
using Microsoft.Extensions.Options;
2220
using Monai.Deploy.InformaticsGateway.SharedTest;
@@ -28,19 +26,17 @@ namespace Monai.Deploy.InformaticsGateway.Configuration.Test
2826
public class ConfigurationValidatorTest
2927
{
3028
private readonly Mock<ILogger<ConfigurationValidator>> _logger;
31-
private readonly Mock<IFileSystem> _fileSystem;
3229

3330
public ConfigurationValidatorTest()
3431
{
3532
_logger = new Mock<ILogger<ConfigurationValidator>>();
36-
_fileSystem = new Mock<IFileSystem>();
3733
}
3834

3935
[Fact(DisplayName = "ConfigurationValidator test with all valid settings")]
4036
public void AllValid()
4137
{
4238
var config = MockValidConfiguration();
43-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
39+
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
4440
Assert.True(valid == ValidateOptionsResult.Success);
4541
}
4642

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

53-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
49+
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
5450

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

66-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
62+
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
6763

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

79-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
75+
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
8076

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

92-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
88+
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
9389

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

105-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
101+
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
106102

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

121-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
117+
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
122118

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

131-
[Fact(DisplayName = "ConfigurationValidator test with inaccessible directory")]
132-
public void StorageWithInaccessbleDirectory()
133-
{
134-
_fileSystem.Setup(p => p.Directory.Exists(It.IsAny<string>())).Returns(true);
135-
_fileSystem.Setup(p => p.File.Create(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<FileOptions>())).Throws(new UnauthorizedAccessException("error"));
136-
137-
var config = MockValidConfiguration();
138-
config.Storage.TemporaryDataStorage = TemporaryDataStorageLocation.Disk;
139-
config.Storage.BufferStorageRootPath = "/blabla";
140-
141-
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
142-
143-
var validationMessages = new[] { $"Directory `/blabla` specified in `InformaticsGateway>storage>bufferRootPath` is not accessible: error." };
144-
Assert.Equal(string.Join(Environment.NewLine, validationMessages), valid.FailureMessage);
145-
foreach (var message in validationMessages)
146-
{
147-
_logger.VerifyLogging(message, LogLevel.Error, Times.Once());
148-
}
149-
}
150-
151127
private static InformaticsGatewayConfiguration MockValidConfiguration()
152128
{
153129
var config = new InformaticsGatewayConfiguration();

‎src/Database/PayloadConfiguration.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ public void Configure(EntityTypeBuilder<Payload> builder)
5757
builder.Ignore(j => j.CalledAeTitle);
5858
builder.Ignore(j => j.CallingAeTitle);
5959
builder.Ignore(j => j.HasTimedOut);
60-
builder.Ignore(j => j.Elapsed);
6160
builder.Ignore(j => j.Count);
6261
}
6362
}

‎src/InformaticsGateway/Common/FileStorageMetadataExtensions.cs

Lines changed: 12 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -14,92 +14,41 @@
1414
* limitations under the License.
1515
*/
1616

17-
using System.IO.Abstractions;
17+
using System.IO;
1818
using System.Text;
1919
using System.Threading.Tasks;
2020
using Ardalis.GuardClauses;
2121
using FellowOakDicom;
2222
using Monai.Deploy.InformaticsGateway.Api.Storage;
23-
using Monai.Deploy.InformaticsGateway.Configuration;
2423

2524
namespace Monai.Deploy.InformaticsGateway.Common
2625
{
2726
internal static class FileStorageMetadataExtensions
2827
{
29-
public static async Task SetDataStreams(
30-
this DicomFileStorageMetadata dicomFileStorageMetadata,
31-
DicomFile dicomFile,
32-
string dicomJson,
33-
TemporaryDataStorageLocation storageLocation,
34-
IFileSystem fileSystem = null,
35-
string temporaryStoragePath = "")
28+
public static async Task SetDataStreams(this DicomFileStorageMetadata dicomFileStorageMetadata, DicomFile dicomFile, string dicomJson)
3629
{
3730
Guard.Against.Null(dicomFile, nameof(dicomFile));
3831
Guard.Against.Null(dicomJson, nameof(dicomJson)); // allow empty here
3932

40-
switch (storageLocation)
41-
{
42-
case TemporaryDataStorageLocation.Disk:
43-
Guard.Against.Null(fileSystem, nameof(fileSystem));
44-
Guard.Against.NullOrWhiteSpace(temporaryStoragePath, nameof(temporaryStoragePath));
45-
46-
var tempFile = fileSystem.Path.Combine(temporaryStoragePath, $@"{System.DateTime.UtcNow.Ticks}.tmp");
47-
dicomFileStorageMetadata.File.Data = fileSystem.File.Create(tempFile);
48-
break;
49-
default:
50-
dicomFileStorageMetadata.File.Data = new System.IO.MemoryStream();
51-
break;
52-
}
53-
33+
dicomFileStorageMetadata.File.Data = new MemoryStream();
5434
await dicomFile.SaveAsync(dicomFileStorageMetadata.File.Data).ConfigureAwait(false);
55-
dicomFileStorageMetadata.File.Data.Seek(0, System.IO.SeekOrigin.Begin);
35+
dicomFileStorageMetadata.File.Data.Seek(0, SeekOrigin.Begin);
5636

57-
await SetTextStream(dicomFileStorageMetadata.JsonFile, dicomJson, storageLocation, fileSystem, temporaryStoragePath);
37+
SetTextStream(dicomFileStorageMetadata.JsonFile, dicomJson);
5838
}
5939

60-
public static async Task SetDataStream(
61-
this FhirFileStorageMetadata fhirFileStorageMetadata,
62-
string json,
63-
TemporaryDataStorageLocation storageLocation,
64-
IFileSystem fileSystem = null,
65-
string temporaryStoragePath = "")
66-
=> await SetTextStream(fhirFileStorageMetadata.File, json, storageLocation, fileSystem, temporaryStoragePath);
40+
public static void SetDataStream(this FhirFileStorageMetadata fhirFileStorageMetadata, string json)
41+
=> SetTextStream(fhirFileStorageMetadata.File, json);
6742

68-
public static async Task SetDataStream(
69-
this Hl7FileStorageMetadata hl7FileStorageMetadata,
70-
string message,
71-
TemporaryDataStorageLocation storageLocation,
72-
IFileSystem fileSystem = null,
73-
string temporaryStoragePath = "")
74-
=> await SetTextStream(hl7FileStorageMetadata.File, message, storageLocation, fileSystem, temporaryStoragePath);
43+
public static void SetDataStream(this Hl7FileStorageMetadata hl7FileStorageMetadata, string message)
44+
=> SetTextStream(hl7FileStorageMetadata.File, message);
7545

76-
private static async Task SetTextStream(
77-
StorageObjectMetadata storageObjectMetadata,
78-
string message,
79-
TemporaryDataStorageLocation storageLocation,
80-
IFileSystem fileSystem = null,
81-
string temporaryStoragePath = "")
46+
private static void SetTextStream(StorageObjectMetadata storageObjectMetadata, string message)
8247
{
8348
Guard.Against.Null(message, nameof(message)); // allow empty here
8449

85-
switch (storageLocation)
86-
{
87-
case TemporaryDataStorageLocation.Disk:
88-
Guard.Against.Null(fileSystem, nameof(fileSystem));
89-
Guard.Against.NullOrWhiteSpace(temporaryStoragePath, nameof(temporaryStoragePath));
90-
91-
var tempFile = fileSystem.Path.Combine(temporaryStoragePath, $@"{System.DateTime.UtcNow.Ticks}.tmp");
92-
var stream = fileSystem.File.Create(tempFile);
93-
var data = Encoding.UTF8.GetBytes(message);
94-
await stream.WriteAsync(data, 0, data.Length);
95-
storageObjectMetadata.Data = stream;
96-
break;
97-
default:
98-
storageObjectMetadata.Data = new System.IO.MemoryStream(Encoding.UTF8.GetBytes(message));
99-
break;
100-
}
101-
102-
storageObjectMetadata.Data.Seek(0, System.IO.SeekOrigin.Begin);
50+
storageObjectMetadata.Data = new MemoryStream(Encoding.UTF8.GetBytes(message));
51+
storageObjectMetadata.Data.Seek(0, SeekOrigin.Begin);
10352
}
10453
}
10554
}

0 commit comments

Comments
 (0)