diff --git a/.editorconfig b/.editorconfig
index db6a2a36a..cf5540801 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -33,3 +33,4 @@ dotnet_diagnostic.SYSLIB1045.severity = none
dotnet_diagnostic.IDE0005.severity = error
csharp_style_namespace_declarations = file_scoped:error
+dotnet_style_namespace_match_folder = true:error
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 995f6260f..0a9f43e13 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -95,6 +95,17 @@ jobs:
- name: "[linux] install RDMP databases"
if: ${{ matrix.os == 'linux' }}
run: ${{ env.rdmp-cli-dir }}/rdmp install --createdatabasetimeout 180 ${{ env.rdmp_conn_str }} TEST_
+ - name: Setup Java JDK
+ uses: actions/setup-java@v4.6.0
+ with:
+ java-version: ${{ env.java-version }}
+ distribution: ${{ env.java-distribution }}
+ - name: "Download ctp-anon-cli jar"
+ run: |
+ set -euxo pipefail
+ ctp_jar_ver=$(grep -E 'TEST_CTP_JAR_VERSION = "(.*)";' tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/FixtureSetup.cs | cut -d'"' -f2)
+ ./bin/smi/downloadCtpAnonJar.py "${ctp_jar_ver}"
+ java -jar "./data/ctp/ctp-anon-cli-${ctp_jar_ver}.jar" --version
- name: show dotnet info
run: dotnet --info
- name: build, test, and package dotnet
diff --git a/Directory.Build.props b/Directory.Build.props
index 97d32c4ed..021f23f0f 100644
--- a/Directory.Build.props
+++ b/Directory.Build.props
@@ -5,6 +5,7 @@
true
full
true
+ true
false
false
false
diff --git a/bin/smi/downloadCtpAnonJar.py b/bin/smi/downloadCtpAnonJar.py
new file mode 100755
index 000000000..650b1c103
--- /dev/null
+++ b/bin/smi/downloadCtpAnonJar.py
@@ -0,0 +1,37 @@
+#!/usr/bin/env python3
+import argparse
+import os
+import sys
+import urllib.request
+from collections.abc import Sequence
+
+sys.path.append(os.path.join(os.path.dirname(__file__), ".."))
+import common # noqa: E402
+
+_CTP_JAR_DIR = f"{common.PROJ_ROOT}/data/ctp"
+
+
+def main(argv: Sequence[str] | None = None) -> int:
+
+ parser = argparse.ArgumentParser()
+ parser.add_argument(
+ "version",
+ )
+ args = parser.parse_args(argv)
+
+ url = (
+ "https://github.com/SMI/ctp-anon-cli/releases/download/"
+ f"v{args.version}/ctp-anon-cli-{args.version}.jar"
+ )
+ file = os.path.join(_CTP_JAR_DIR, f"ctp-anon-cli-{args.version}.jar")
+ if os.path.isfile(file):
+ return 0
+
+ print(f"Downloading {url} to {file}")
+ urllib.request.urlretrieve(url, file)
+
+ return 0
+
+
+if __name__ == "__main__":
+ raise SystemExit(main())
diff --git a/data/ctp/.gitignore b/data/ctp/.gitignore
new file mode 100644
index 000000000..d392f0e82
--- /dev/null
+++ b/data/ctp/.gitignore
@@ -0,0 +1 @@
+*.jar
diff --git a/data/ctp/ctp-whitelist.script b/data/ctp/ctp-allowlist.script
similarity index 100%
rename from data/ctp/ctp-whitelist.script
rename to data/ctp/ctp-allowlist.script
diff --git a/docs/services/ctp-anonymiser.md b/docs/services/ctp-anonymiser.md
index 9b2c09f4b..3427569fa 100644
--- a/docs/services/ctp-anonymiser.md
+++ b/docs/services/ctp-anonymiser.md
@@ -37,4 +37,4 @@ removed all PII from the file, it will not proceed if it cannot insert redacted
## CLI Options
-The anonymisation script can be specified using the `-a` option. An example script can be viewed [here](/data/ctp/ctp-whitelist.script).
+The anonymisation script can be specified using the `-a` option. An example script can be viewed [here](/data/ctp/ctp-allowlist.script).
diff --git a/news/2053-feature.md b/news/2053-feature.md
new file mode 100644
index 000000000..36f1c9d6c
--- /dev/null
+++ b/news/2053-feature.md
@@ -0,0 +1 @@
+Finish dicom-anonymiser CTP implementation and expand tests
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleData/1.2.276.0.7230010.3.1.4.1787205428.166.1117461927.60.dcm b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleData/1.2.276.0.7230010.3.1.4.1787205428.166.1117461927.60.dcm
deleted file mode 100644
index b517ac44d..000000000
Binary files a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleData/1.2.276.0.7230010.3.1.4.1787205428.166.1117461927.60.dcm and /dev/null differ
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleData/1.2.840.113619.2.1.2411.1031152382.365.1.736169244.dcm b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleData/1.2.840.113619.2.1.2411.1031152382.365.1.736169244.dcm
deleted file mode 100644
index ecfe6ce89..000000000
Binary files a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleData/1.2.840.113619.2.1.2411.1031152382.365.1.736169244.dcm and /dev/null differ
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-cohort-extractor-terminal-output.png b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-cohort-extractor-terminal-output.png
deleted file mode 100644
index ecc3678b1..000000000
Binary files a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-cohort-extractor-terminal-output.png and /dev/null differ
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-image-output.png b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-image-output.png
deleted file mode 100644
index 8f7cf616d..000000000
Binary files a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-image-output.png and /dev/null differ
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-original-image.png b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-original-image.png
deleted file mode 100644
index 04cb93820..000000000
Binary files a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-original-image.png and /dev/null differ
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-terminal-output.png b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-terminal-output.png
deleted file mode 100644
index 802cb51d6..000000000
Binary files a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-dicom-anonymiser-terminal-output.png and /dev/null differ
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-extract-images-terminal-output.png b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-extract-images-terminal-output.png
deleted file mode 100644
index db1599ad9..000000000
Binary files a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleScreenshots/smi-extract-images-terminal-output.png and /dev/null differ
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleWorkflow/extractRoot/extractme.csv b/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleWorkflow/extractRoot/extractme.csv
deleted file mode 100644
index 8051e652b..000000000
--- a/src/SmiServices/Microservices/DicomAnonymiser/AnonymiserData/sampleWorkflow/extractRoot/extractme.csv
+++ /dev/null
@@ -1,2 +0,0 @@
-SOPInstanceUID
-1.2.840.113619.2.1.2411.1031152382.365.1.736169244.dcm
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactory.cs b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactory.cs
index 2a5ca308b..a451f0543 100644
--- a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactory.cs
+++ b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactory.cs
@@ -14,8 +14,8 @@ public static IDicomAnonymiser CreateAnonymiser(GlobalOptions options)
return anonymiserType switch
{
AnonymiserType.DefaultAnonymiser => new DefaultAnonymiser(options),
- // TODO(rkm 2021-12-07) Can remove the LGTM ignore once an AnonymiserType is implemented
- _ => throw new NotImplementedException($"No case for AnonymiserType '{anonymiserType}'"), // lgtm[cs/constant-condition]
+ AnonymiserType.SmiCtpAnonymiser => new SmiCtpAnonymiser(options),
+ _ => throw new NotImplementedException($"No case for AnonymiserType '{anonymiserType}'"),
};
}
}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserType.cs b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserType.cs
index 60f85e7da..2b8f8cef0 100644
--- a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserType.cs
+++ b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/AnonymiserType.cs
@@ -6,5 +6,8 @@ public enum AnonymiserType
/// Unused placeholder value
///
None = 0,
+
DefaultAnonymiser = 1,
+
+ SmiCtpAnonymiser = 2,
}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/DefaultAnonymiser.cs b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/DefaultAnonymiser.cs
index 9f058d5d6..7ca8344c5 100644
--- a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/DefaultAnonymiser.cs
+++ b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/DefaultAnonymiser.cs
@@ -2,137 +2,43 @@
using SmiServices.Common.Messages.Extraction;
using SmiServices.Common.Options;
using System;
-using System.Collections.Generic;
-using System.Diagnostics;
using System.IO.Abstractions;
namespace SmiServices.Microservices.DicomAnonymiser.Anonymisers;
-public class DefaultAnonymiser : IDicomAnonymiser
+public class DefaultAnonymiser : IDicomAnonymiser, IDisposable
{
private readonly ILogger _logger = LogManager.GetCurrentClassLogger();
- private readonly DicomAnonymiserOptions _options;
- private const string _bash = "/bin/bash";
+ private readonly SmiCtpAnonymiser _ctpAnonymiser;
public DefaultAnonymiser(GlobalOptions globalOptions)
{
- if (globalOptions.DicomAnonymiserOptions == null)
- throw new ArgumentNullException(nameof(globalOptions));
+ var dicomAnonymiserOptions = globalOptions.DicomAnonymiserOptions!;
- if (globalOptions.LoggingOptions == null)
- throw new ArgumentNullException(nameof(globalOptions));
-
- _options = globalOptions.DicomAnonymiserOptions;
- }
-
- ///
- /// Creates a process with the given parameters
- ///
- private static Process CreateProcess(string fileName, string arguments, string? workingDirectory = null, Dictionary? environmentVariables = null)
- {
- var process = new Process
- {
- StartInfo = new ProcessStartInfo
- {
- FileName = fileName,
- Arguments = arguments,
- UseShellExecute = false,
- RedirectStandardOutput = true,
- RedirectStandardError = true,
- WorkingDirectory = workingDirectory ?? string.Empty
- }
- };
-
- if (environmentVariables != null)
- {
- foreach (var variable in environmentVariables)
- {
- process.StartInfo.EnvironmentVariables[variable.Key] = variable.Value;
- }
- }
-
- return process;
- }
-
- private Process CreateCTPAnonProcess(IFileInfo sourceFile, IFileInfo destFile)
- {
- string arguments = $"-jar {_options.CtpAnonCliJar} -a {_options.CtpAllowlistScript} -s false {sourceFile} {destFile}";
-
- return CreateProcess("java", arguments);
- }
-
- private Process CreatePixelAnonProcess(IFileInfo sourceFile, IFileInfo destFile)
- {
- string activateCommand = $"source {_options.VirtualEnvPath}/bin/activate";
- string arguments = $"-c \"{activateCommand} && {_options.DicomPixelAnonPath}/dicom_pixel_anon.sh -o {destFile} {sourceFile}\"";
-
- return CreateProcess(_bash, arguments, _options.DicomPixelAnonPath);
- }
-
- // TODO (da 2024-02-23) Use StructuredReports repository to access SRAnonTool
- private Process CreateSRAnonProcess(IFileInfo sourceFile, IFileInfo destFile)
- {
- string arguments = $"{_options.SRAnonymiserToolPath} -i {sourceFile} -o {destFile} -s /Users/daniyalarshad/EPCC/github/NationalSafeHaven/opt/semehr";
- _ = new Dictionary { { "SMI_ROOT", $"{_options.SmiServicesPath}" } };
-
- return CreateProcess(_bash, arguments);
+ _ctpAnonymiser = new SmiCtpAnonymiser(globalOptions);
}
///
/// Anonymises a DICOM file based on image modality
///
- public ExtractedFileStatus Anonymise(ExtractFileMessage message, IFileInfo sourceFile, IFileInfo destFile, out string anonymiserStatusMessage)
+ public ExtractedFileStatus Anonymise(IFileInfo sourceFile, IFileInfo destFile, string modality, out string? anonymiserStatusMessage)
{
- _logger.Info($"Anonymising {sourceFile} to {destFile}");
-
- if (!RunProcessAndCheckSuccess(CreateCTPAnonProcess(sourceFile, destFile), "CTP Anonymiser"))
+ var status = _ctpAnonymiser.Anonymise(sourceFile, destFile, modality, out string? ctpStatusMessage);
+ if (status != ExtractedFileStatus.Anonymised)
{
- anonymiserStatusMessage = "Error running CTP anonymiser";
- return ExtractedFileStatus.ErrorWontRetry;
+ anonymiserStatusMessage = ctpStatusMessage;
+ return status;
}
- if (message.Modality == "SR")
- {
- if (!RunProcessAndCheckSuccess(CreateSRAnonProcess(sourceFile, destFile), "SR Anonymiser"))
- {
- anonymiserStatusMessage = "Error running SR anonymiser";
- return ExtractedFileStatus.ErrorWontRetry;
- }
- }
- else
- {
- if (!RunProcessAndCheckSuccess(CreatePixelAnonProcess(sourceFile, destFile), "Pixel Anonymiser"))
- {
- anonymiserStatusMessage = "Error running PIXEL anonymiser";
- return ExtractedFileStatus.ErrorWontRetry;
- }
- }
+ // TODO(rkm 2024-12-17) Implement SR anon here (instead of via CTP), and add pixel anonymiser
- anonymiserStatusMessage = "Anonymisation successful";
+ anonymiserStatusMessage = null;
return ExtractedFileStatus.Anonymised;
}
- ///
- /// Runs a process and logs the result
- ///
- private bool RunProcessAndCheckSuccess(Process process, string processName)
+ public void Dispose()
{
- process.Start();
- process.WaitForExit();
-
- var returnCode = process.ExitCode.ToString();
- LogProcessResult(processName, returnCode, process);
-
- return returnCode == "0";
+ GC.SuppressFinalize(this);
+ _ctpAnonymiser.Dispose();
}
-
- ///
- /// Logs the result of a process
- ///
- private void LogProcessResult(string processName, string returnCode, Process process)
- {
- var output = returnCode == "0" ? process.StandardOutput.ReadToEnd() : process.StandardError.ReadToEnd();
- _logger.Info($"{(returnCode == "0" ? "SUCCESS" : "ERROR")} [{processName}]: Return Code {returnCode}\n{output}");
- }
-
}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/DicomAnonymiserConfigs.json b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/DicomAnonymiserConfigs.json
deleted file mode 100644
index 4e12a6a7e..000000000
--- a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/DicomAnonymiserConfigs.json
+++ /dev/null
@@ -1,12 +0,0 @@
-{
- "virtualEnvPath": "/Users/daniyalarshad/EPCC/github/NationalSafeHaven/venv",
- "smiServicesPath": "/Users/daniyalarshad/EPCC/github/NationalSafeHaven/SmiServices",
- "smiLogsPath": "/Users/daniyalarshad/EPCC/github/SmiLogs",
- "shellScriptPath": "/Users/daniyalarshad/EPCC/github/NationalSafeHaven/dicompixelanon/src/applications/dicom_pixel_anon.sh",
- "dicomPixelAnonPath": "/Users/daniyalarshad/EPCC/github/NationalSafeHaven/dicompixelanon",
- "ctpJarPath": "/Users/daniyalarshad/EPCC/github/ctp-anon-cli/ctp-anon-cli-0.5.0.jar",
- "ctpWhiteListScriptPath": "/Users/daniyalarshad/EPCC/github/SmiServices/data/ctp/ctp-whitelist.script",
- "srAnonToolPath": "/Users/daniyalarshad/EPCC/github/SmiServices/src/applications/SRAnonTool/CTP_SRAnonTool.sh",
- "dicomToTextScriptPath": "/Users/daniyalarshad/EPCC/github/NationalSafeHaven/SmiServices/src/applications/SRAnonTool/CTP_DicomToText.py",
- "anonymiseSRScriptPath": "/Users/daniyalarshad/EPCC/github/NationalSafeHaven/CogStack-SemEHR/anonymisation/anonymiser.py"
-}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/IDicomAnonymiser.cs b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/IDicomAnonymiser.cs
index 4b3e9b7e8..921236d24 100644
--- a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/IDicomAnonymiser.cs
+++ b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/IDicomAnonymiser.cs
@@ -6,13 +6,13 @@ namespace SmiServices.Microservices.DicomAnonymiser.Anonymisers;
public interface IDicomAnonymiser
{
///
- /// Anonymise the specified to based on the provided modality.
+ /// Anonymise the specified to .
/// Implementations should assume that already exists, and does not exist.
///
- ///
///
///
+ ///
///
///
- ExtractedFileStatus Anonymise(ExtractFileMessage message, IFileInfo sourceFile, IFileInfo destFile, out string anonymiserStatusMessage);
+ ExtractedFileStatus Anonymise(IFileInfo sourceFile, IFileInfo destFile, string modality, out string? anonymiserStatusMessage);
}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/SmiCtpAnonymiser.cs b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/SmiCtpAnonymiser.cs
new file mode 100644
index 000000000..4ef70607a
--- /dev/null
+++ b/src/SmiServices/Microservices/DicomAnonymiser/Anonymisers/SmiCtpAnonymiser.cs
@@ -0,0 +1,112 @@
+using NLog;
+using SmiServices.Common.Messages.Extraction;
+using SmiServices.Common.Options;
+using SmiServices.Microservices.DicomAnonymiser.Helpers;
+using System;
+using System.Diagnostics;
+using System.IO;
+using System.IO.Abstractions;
+using System.Threading;
+
+namespace SmiServices.Microservices.DicomAnonymiser.Anonymisers;
+
+public class SmiCtpAnonymiser : IDicomAnonymiser, IDisposable
+{
+ private readonly ILogger _logger = LogManager.GetCurrentClassLogger();
+ private readonly Process _ctpProcess;
+
+ // NOTE(rkm 2025-01-08) This sometimes takes more than 10s in CI for some reason
+ private readonly TimeSpan CTP_TIMEOUT = TimeSpan.FromSeconds(20);
+
+ public SmiCtpAnonymiser(GlobalOptions globalOptions)
+ {
+ var dicomAnonymiserOptions = globalOptions.DicomAnonymiserOptions ?? throw new ArgumentException($"{nameof(globalOptions.DicomAnonymiserOptions)} was null", nameof(globalOptions));
+
+ if (!File.Exists(dicomAnonymiserOptions.CtpAnonCliJar))
+ throw new ArgumentException($"{nameof(dicomAnonymiserOptions.CtpAnonCliJar)} '{dicomAnonymiserOptions.CtpAnonCliJar}' does not exist", nameof(globalOptions));
+
+ if (!File.Exists(dicomAnonymiserOptions.CtpAllowlistScript))
+ throw new ArgumentException($"{nameof(dicomAnonymiserOptions.CtpAllowlistScript)} '{dicomAnonymiserOptions.CtpAllowlistScript}' does not exist", nameof(globalOptions));
+
+ var srAnonTool = "false";
+ if (!string.IsNullOrWhiteSpace(dicomAnonymiserOptions.SRAnonymiserToolPath))
+ {
+ if (!File.Exists(dicomAnonymiserOptions.SRAnonymiserToolPath))
+ throw new ArgumentException($"{nameof(dicomAnonymiserOptions.SRAnonymiserToolPath)} '{dicomAnonymiserOptions.SRAnonymiserToolPath}' does not exist", nameof(globalOptions));
+ srAnonTool = dicomAnonymiserOptions.SRAnonymiserToolPath;
+ }
+
+ var ctpArgs = $"-jar {dicomAnonymiserOptions.CtpAnonCliJar} --anon-script {dicomAnonymiserOptions.CtpAllowlistScript} --sr-anon-tool {srAnonTool} --daemonize";
+ _logger.Info($"Starting ctp with: java {ctpArgs}");
+
+ var ready = false;
+ _ctpProcess = ProcessWrapper.CreateProcess("java", ctpArgs);
+ _ctpProcess.OutputDataReceived += OnCtpOutputDataReceived;
+ _ctpProcess.ErrorDataReceived += (_, args) => _logger.Debug(args.Data);
+ _ctpProcess.Start();
+ _ctpProcess.BeginOutputReadLine();
+ _ctpProcess.BeginErrorReadLine();
+
+ lock (_ctpProcess)
+ Monitor.Wait(_ctpProcess, CTP_TIMEOUT);
+
+ if (!ready)
+ {
+ _ctpProcess.Kill();
+ throw new Exception($"Did not receive READY before timeout");
+ }
+
+ void OnCtpOutputDataReceived(object _, DataReceivedEventArgs args)
+ {
+ _logger.Debug($"[ctp-anon-cli stdout] {args.Data}");
+ if ("READY" != args.Data) return;
+
+ ready = true;
+ lock (_ctpProcess)
+ Monitor.Pulse(_ctpProcess);
+ }
+ }
+
+ public ExtractedFileStatus Anonymise(IFileInfo sourceFile, IFileInfo destFile, string modality, out string? anonymiserStatusMessage)
+ {
+ var args = $"{sourceFile.FullName} {destFile.FullName}";
+ string? result = null;
+
+ _ctpProcess.OutputDataReceived += CtpProcessOnOutputDataReceived;
+
+ _logger.Debug($"[ctp-anon-cli stdin ] {args}");
+ _ctpProcess.StandardInput.WriteLine(args);
+
+ lock (args)
+ Monitor.Wait(args);
+
+ _ctpProcess.OutputDataReceived -= CtpProcessOnOutputDataReceived;
+
+ ExtractedFileStatus status;
+ if (result == "OK")
+ {
+ anonymiserStatusMessage = null;
+ status = ExtractedFileStatus.Anonymised;
+ }
+ else
+ {
+ anonymiserStatusMessage = result;
+ status = ExtractedFileStatus.ErrorWontRetry;
+ }
+
+ return status;
+
+ void CtpProcessOnOutputDataReceived(object _, DataReceivedEventArgs e)
+ {
+ result = e.Data;
+ lock (args)
+ Monitor.Pulse(args);
+ }
+ }
+
+ public void Dispose()
+ {
+ GC.SuppressFinalize(this);
+ _ctpProcess.Dispose();
+ }
+}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/DicomAnonymiserConsumer.cs b/src/SmiServices/Microservices/DicomAnonymiser/DicomAnonymiserConsumer.cs
index 232336cbb..37e44ba0e 100644
--- a/src/SmiServices/Microservices/DicomAnonymiser/DicomAnonymiserConsumer.cs
+++ b/src/SmiServices/Microservices/DicomAnonymiser/DicomAnonymiserConsumer.cs
@@ -1,4 +1,3 @@
-using FellowOakDicom;
using NLog;
using SmiServices.Common.Messages;
using SmiServices.Common.Messages.Extraction;
@@ -6,20 +5,15 @@
using SmiServices.Common.Options;
using SmiServices.Microservices.DicomAnonymiser.Anonymisers;
using System;
-using System.IO;
using System.IO.Abstractions;
namespace SmiServices.Microservices.DicomAnonymiser;
public class DicomAnonymiserConsumer : Consumer
{
- // TODO (da 2024-02-23) Additional Requirement: Message Batching
- // https://github.com/SMI/SmiServices/blob/main/src/microservices/Microservices.CohortPackager/Messaging/AnonVerificationMessageConsumer.cs#L72
- // https://github.com/SMI/SmiServices/blob/main/src/microservices/Microservices.MongoDbPopulator/Messaging/MongoDbPopulatorMessageConsumer.cs#L56
-
private readonly ILogger _logger = LogManager.GetCurrentClassLogger();
- private readonly DicomAnonymiserOptions _options;
private readonly IFileSystem _fileSystem;
+ private readonly DicomAnonymiserOptions _options;
private readonly string _fileSystemRoot;
private readonly string _extractRoot;
private readonly IDicomAnonymiser _anonymiser;
@@ -35,11 +29,11 @@ public DicomAnonymiserConsumer(
)
{
_options = options ?? throw new ArgumentNullException(nameof(options));
+ _fileSystem = fileSystem ?? new FileSystem();
_fileSystemRoot = fileSystemRoot ?? throw new ArgumentNullException(nameof(fileSystemRoot));
_extractRoot = extractRoot ?? throw new ArgumentNullException(nameof(extractRoot));
_anonymiser = anonymiser ?? throw new ArgumentNullException(nameof(anonymiser));
_statusMessageProducer = statusMessageProducer ?? throw new ArgumentNullException(nameof(statusMessageProducer));
- _fileSystem = fileSystem ?? new FileSystem();
if (!_fileSystem.Directory.Exists(_fileSystemRoot))
throw new Exception($"Filesystem root does not exist: '{fileSystemRoot}'");
@@ -68,7 +62,7 @@ protected override void ProcessMessageImpl(IMessageHeader header, ExtractFileMes
return;
}
- if (_options.FailIfSourceWriteable && !sourceFileAbs.Attributes.HasFlag(FileAttributes.ReadOnly))
+ if (_options.FailIfSourceWriteable && !sourceFileAbs.Attributes.HasFlag(System.IO.FileAttributes.ReadOnly))
{
statusMessage.Status = ExtractedFileStatus.ErrorWontRetry;
statusMessage.StatusMessage = $"Source file was writeable and FailIfSourceWriteable is set: '{sourceFileAbs}'";
@@ -84,7 +78,7 @@ protected override void ProcessMessageImpl(IMessageHeader header, ExtractFileMes
// NOTE(rkm 2021-12-07) Since this directory should have already been created, we treat this more like an assertion and throw if not found.
// This helps prevent a flood of messages if e.g. the filesystem is temporarily unavailable.
if (!_fileSystem.Directory.Exists(extractionDirAbs))
- throw new DirectoryNotFoundException($"Expected extraction directory to exist: '{extractionDirAbs}'");
+ throw new System.IO.DirectoryNotFoundException($"Expected extraction directory to exist: '{extractionDirAbs}'");
var destFileAbs = _fileSystem.FileInfo.New(_fileSystem.Path.Combine(extractionDirAbs, message.OutputPath));
@@ -92,55 +86,30 @@ protected override void ProcessMessageImpl(IMessageHeader header, ExtractFileMes
_logger.Debug($"Anonymising '{sourceFileAbs}' to '{destFileAbs}'");
- // TODO (rkm 2024-02-09) Extract modality from cohort extractor instead of opening DICOM file
- DicomFile dicomFile = DicomFile.Open(sourceFileAbs.FullName);
- message.Modality = dicomFile.Dataset.GetSingleValue(DicomTag.Modality);
-
- Console.WriteLine("[DICOM] Modality: " + message.Modality);
- Console.WriteLine("[DICOM] Source File: " + message.DicomFilePath);
- Console.WriteLine("[DICOM] Dest File: " + message.OutputPath);
-
- ExtractedFileStatus anonymiserStatus = ExtractedFileStatus.None;
- string anonymiserStatusMessage = "";
-
- try
- {
- anonymiserStatus = _anonymiser.Anonymise(message, sourceFileAbs, destFileAbs, out anonymiserStatusMessage);
- }
- catch (Exception e)
- {
- var msg = $"Error anonymising '{sourceFileAbs}'";
- _logger.Error(e, msg);
+ var anonymiserStatus = _anonymiser.Anonymise(sourceFileAbs, destFileAbs, message.Modality, out var anonymiserStatusMessage);
- statusMessage.Status = ExtractedFileStatus.ErrorWontRetry;
- statusMessage.StatusMessage = $"{msg}. Exception message: {e.Message}";
- statusMessage.OutputFilePath = null;
- _statusMessageProducer.SendMessage(statusMessage, header, _options.RoutingKeyFailure);
+ var logMessage = $"Anonymisation of '{sourceFileAbs}' returned {anonymiserStatus}";
+ if (anonymiserStatus != ExtractedFileStatus.Anonymised)
+ logMessage += $" with message {anonymiserStatusMessage}";
+ _logger.Info(logMessage);
- Ack(header, tag);
- }
+ statusMessage.Status = anonymiserStatus;
+ statusMessage.StatusMessage = anonymiserStatusMessage;
string routingKey;
if (anonymiserStatus == ExtractedFileStatus.Anonymised)
{
- _logger.Info($"Anonymisation of '{sourceFileAbs}' successful");
- statusMessage.Status = anonymiserStatus;
- statusMessage.StatusMessage = anonymiserStatusMessage;
routingKey = _options.RoutingKeySuccess ?? "verify";
}
else
{
- _logger.Info($"Anonymisation of '{sourceFileAbs}' failed");
statusMessage.OutputFilePath = null;
- statusMessage.Status = anonymiserStatus;
- statusMessage.StatusMessage = anonymiserStatusMessage;
routingKey = _options.RoutingKeyFailure ?? "noverify";
}
_statusMessageProducer.SendMessage(statusMessage, header, routingKey);
Ack(header, tag);
- return;
}
}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/Helpers/ProcessWrapper.cs b/src/SmiServices/Microservices/DicomAnonymiser/Helpers/ProcessWrapper.cs
new file mode 100644
index 000000000..1a671a25a
--- /dev/null
+++ b/src/SmiServices/Microservices/DicomAnonymiser/Helpers/ProcessWrapper.cs
@@ -0,0 +1,35 @@
+using System.Collections.Generic;
+using System.Diagnostics;
+
+namespace SmiServices.Microservices.DicomAnonymiser.Helpers;
+
+internal static class ProcessWrapper
+{
+ public static Process CreateProcess(
+ string fileName,
+ string? arguments,
+ string? workingDirectory = null,
+ Dictionary? environmentVariables = null
+ )
+ {
+ var process = new Process
+ {
+ StartInfo = new ProcessStartInfo
+ {
+ FileName = fileName,
+ Arguments = arguments,
+ UseShellExecute = false,
+ RedirectStandardInput = true,
+ RedirectStandardOutput = true,
+ RedirectStandardError = true,
+ WorkingDirectory = workingDirectory ?? string.Empty,
+ }
+ };
+
+ if (environmentVariables != null)
+ foreach (var variable in environmentVariables)
+ process.StartInfo.EnvironmentVariables[variable.Key] = variable.Value;
+
+ return process;
+ }
+}
diff --git a/src/SmiServices/Microservices/DicomAnonymiser/README.md b/src/SmiServices/Microservices/DicomAnonymiser/README.md
deleted file mode 100644
index e19294209..000000000
--- a/src/SmiServices/Microservices/DicomAnonymiser/README.md
+++ /dev/null
@@ -1,171 +0,0 @@
-# Dicom Anonymiser (Microservice)
-
-The dicom anonymiser also known as generic wrapper microservice streamlines the existing dicom processing workflow. It serves as a versatile tool capable of calling external programs to perform tasks such as anonymisation of [dicom tags](https://github.com/SMI/ctp-anon-cli), [pixel data](https://github.com/SMI/dicompixelanon) and/or [structured reports](https://github.com/SMI/StructuredReports) based on the dicom image modality.
-
-One of the key strengths of this microservice lies in its ability to offer a unified interface for different types of external programs (for e.g., written in python or java), set configuration options, manage logging, and handle other environment variables. This flexibility allows the microservice to be easily integrated into the existing C# SmiServices architecture.
-
-> Additional requirements which require further work _(i) Safe Haven Environment Testing_ and _(ii) Message Batching Mechanism (optional)_
-
-## Getting Started
-
-As of 1 June 2024, the dicom anonymiser microservice is functional (supports anonymisation of dicom tags, pixel data and structured reports), however, it is still in its infancy stage. To get started, ensure that the following programs/services are installed on your local machine in the same directory as the SmiServices repository:
-
-1. [dicom tags anonymisation](https://github.com/SMI/dicompixelanon)
-1. [dicom pixel anonymisation](https://github.com/SMI/ctp-anon-cli)
-1. [structured reports anonymisation](https://github.com/SMI/StructuredReports)
-
-Once these programs/services are installed, you will need to update the configuration file _default.yaml_ under `SmiServices/data/microserviceConfigs/` with the correct paths to the external programs. For example (see below):
-
-```
-// dicom tags anonymisation:
- CtpAnonCliJar: "path/to/ctp-anon-cli/ctp-anon-cli-0.5.0.jar"
- CtpAllowlistScript: "path/to/SmiServices/data/ctp/ctp-whitelist.script"
-
-// dicom pixel anonymisation:
- VirtualEnvPath: "path/to/virtualenv/as/created/in/dicompixelanon"
- DicomPixelAnonPath: "path/to/dicompixelanon/src/applications/"
-
-// structured reports anonymisation:
- SmiServicesPath: "path/to/SmiServices"
- SRAnonymiserToolPath: "path/to/StructuredReports/src/applications/SRAnonTool/CTP_SRAnonTool.sh"
-```
-
-The `Microservices.DicomAnonymiser/AnonymiserData/` directory contains the following subdirectories:
-
-1. `sampleData/`: contains test dicom images (with SOPInstanceUID as their filename) to be anonymised.
-1. `sampleScreenshots/`: contains screenshots from the dicom anonymiser microservice example workflow.
-1. `sampleWorkflows/`: contains the extractRoot and fileSystemRoot subdirectories for testing purposes.
-
-> Note: If you are using the extractRoot and fileSystemRoot directories within the `sampleWorkflow/` directory, ensure that the `default.yaml` configuration file is updated with the correct paths and that the sample dicom files are placed in the fileSystemRoot directory.
-
-## Example Workflow
-
-**1. Docker (Container)**
-
-Initiate Essential Services (RabbitMQ, MSSQL, MongoDB, MariaDB, Redis)
-
-```
-git clone https://github.com/SMI/SmiServices.git
-cd SmiServices/utils/docker-compose
-docker-compose -f linux-dotnet-arm.yml up
-```
-
-**2. RDMP (Application)**
-
-Setup RDMP (Github)
-
-```
-git clone git@github.com:HicServices/RDMP.git
-cd RDMP/Tools/rdmp
-dotnet clean
-dotnet build
-cd bin/Debug/net8.0
-./rdmp install localhost RDMP_ -e -D -u sa -p "YourStrongPassw0rd"
-```
-
-Update Databases Connection (.yaml)
-
-```
-cd RDMP/Tools/rdmp
-cat > Databases.yaml << EOF
-CatalogueConnectionString: Server=localhost;Database=RDMP_Catalogue;User ID=sa;Password=YourStrongPassw0rd;Trust Server Certificate=true;
-DataExportConnectionString: Server=localhost;Database=RDMP_DataExport;User ID=sa;Password=YourStrongPassw0rd;Trust Server Certificate=true;
-EOF
-```
-
-**3. Extract Images (Application)**
-
-> ExtractImages: Reads UIDs from a CSV file and generates ExtractionRequestMessage and audit message ExtractionRequestInfoMessage.
-
-Sample Image (.dcm)
-
-
-
-Generate UIDs List (.csv)
-
-```
-cd SmiServices/src/microservices/Microservices.DicomAnonymiser/AnonymiserData/sampleWorkflow/extractRoot
-cat > extractme.csv << _EOF
-SOPInstanceUID
-1.2.840.113619.2.1.2411.1031152382.365.1.736169244.dcm
-_EOF
-```
-
-Check RDMPOptions (smi-dataExtract.yaml)
-
-Check if the _CatalogueConnectionString_ and _DataExportConnectionString_ are properly set.
-
-```
-cd SmiServices/data/microserviceConfigs
-cat default.yaml
-
-// CatalogueConnectionString: 'Server=localhost;Database=RDMP_Catalogue;User ID=sa;Password=YourStrongPassw0rd;Trust Server Certificate=true;'
-//DataExportConnectionString: 'Server=localhost;Database=RDMP_DataExport;User ID=sa;Password=YourStrongPassw0rd;Trust Server Certificate=true;'
-```
-
-Run Extract Images (.smi)
-
-```
-cd SmiServices/src/applications/Applications.SmiRunner/bin/net7.0
-
-./smi extract-images -y "path/to/SmiServices/data/microserviceConfigs/default.yaml" -p "project-001" -c "path/to/Microservices.DicomAnonymiser/AnonymiserData/sampleWorkflow/extractRoot/extractme.csv"
-```
-
-
-
-**4. Cohort Extractor (Microservice)**
-
-> CohortExtractor: Looks up SeriesInstanceUIDs in ExtractionRequestMessage and does relational database lookup(s) to resolve into physical image file location. Generates ExtractFileMessage and audit message ExtractFileCollectionInfoMessage.
-
-Check CohortExtractorOptions (smi-dataExtract.yaml)
-
-```
-cd SmiServices/data/microserviceConfigs
-cat default.yaml
-
-// RequestFulfillerType: "Microservices.CohortExtractor.Execution.RequestFulfillers.FakeFulfiller"
-```
-
-Run Cohort Extractor (.smi)
-
-```
-cd SmiServices/src/applications/Applications.SmiRunner/bin/net7.0
-
-./smi cohort-extractor -y "path/to/SmiServices/data/microserviceConfigs/default.yaml"
-```
-
-
-
-**5. Dicom Anonymiser (Microservice)**
-
-Run DICOM Anonymiser (.smi)
-
-```
-./smi dicom-anonymiser -y "path/to/SmiServices/data/microserviceConfigs/default.yaml"
-```
-
-
-
-View Anonymised DICOMS
-
-```
-cd dicompixelanon/src/applications
-./dcmaudit.py -i
-```
-
-
diff --git a/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/Anonymisers/SmiCtpAnonymiserTests.cs b/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/Anonymisers/SmiCtpAnonymiserTests.cs
new file mode 100644
index 000000000..a8c0bd275
--- /dev/null
+++ b/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/Anonymisers/SmiCtpAnonymiserTests.cs
@@ -0,0 +1,60 @@
+using FellowOakDicom;
+using NUnit.Framework;
+using SmiServices.Common.Messages.Extraction;
+using SmiServices.Common.Options;
+using SmiServices.Microservices.DicomAnonymiser.Anonymisers;
+using SmiServices.UnitTests.TestCommon;
+using System.IO;
+using System.IO.Abstractions;
+
+namespace SmiServices.IntegrationTests.Microservices.DicomAnonymiser.Anonymisers;
+
+internal class SmiCtpAnonymiserTests
+{
+ [Test]
+ public void Anonymise_HappyPath_IsOk()
+ {
+ // Arrange
+
+ var globals = new GlobalOptionsFactory().Load(nameof(Anonymise_HappyPath_IsOk));
+ globals.DicomAnonymiserOptions!.CtpAnonCliJar = FixtureSetup.CtpJarPath;
+ globals.DicomAnonymiserOptions.CtpAllowlistScript = FixtureSetup.CtpAllowlistPath;
+ globals.DicomAnonymiserOptions.SRAnonymiserToolPath = null;
+
+ var ds = new DicomDataset
+ {
+ { DicomTag.SOPClassUID, DicomUID.CTImageStorage },
+ { DicomTag.StudyInstanceUID, "1" },
+ { DicomTag.SeriesInstanceUID, "2" },
+ { DicomTag.SOPInstanceUID, "3" },
+ };
+ var srcDcm = new DicomFile(ds);
+
+ using var tempDir = new DisposableTempDir();
+ var srcPath = Path.Combine(tempDir, "in.dcm");
+ srcDcm.Save(srcPath);
+ File.SetAttributes(srcPath, FileAttributes.ReadOnly);
+
+ var fileSystem = new FileSystem();
+ var srcFile = fileSystem.FileInfo.New(srcPath);
+ var destPath = Path.Combine(tempDir, "out.dcm");
+ var destFile = fileSystem.FileInfo.New(destPath);
+
+ using var anonymiser = new SmiCtpAnonymiser(globals);
+
+ // Act
+
+ var status = anonymiser.Anonymise(srcFile, destFile, "CT", out var message);
+
+ // Assert
+
+ Assert.Multiple(() =>
+ {
+
+ Assert.That(status, Is.EqualTo(ExtractedFileStatus.Anonymised));
+ Assert.That(message, Is.Null);
+ });
+
+ File.SetAttributes(srcPath, FileAttributes.Normal);
+ }
+}
diff --git a/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/DicomAnonymiserHostTests.cs b/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/DicomAnonymiserHostTests.cs
index d7e22570e..a0c34b54c 100644
--- a/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/DicomAnonymiserHostTests.cs
+++ b/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/DicomAnonymiserHostTests.cs
@@ -19,34 +19,10 @@ namespace SmiServices.IntegrationTests.Microservices.DicomAnonymiser;
[RequiresRabbit]
public class DicomAnonymiserHostTests
{
- #region Fixture Methods
-
- // Private fields (_tempTestDir, _dicomRoot, _fakeDicom) that
- // are used in the setup and teardown of each test.
private DirectoryInfo _tempTestDir = null!;
private DirectoryInfo _dicomRoot = null!;
private string _fakeDicom = null!;
-
- // [OneTimeSetUp] and [OneTimeTearDown] methods are run once
- // before and after all the tests in the class, respectively.
- // In this case, the setup method is used to set up a logger.
- [OneTimeSetUp]
- public void OneTimeSetUp()
- {
- }
-
- [OneTimeTearDown]
- public void OneTimeTearDown() { }
-
- #endregion
-
- #region Test Methods
-
- // [SetUp] and [TearDown] methods are run before and after each
- // test, respectively. In this case, the setup method creates a
- // temporary directory and a fake DICOM file, and the teardown
- // method deletes the temporary directory.
[SetUp]
public void SetUp()
{
@@ -64,33 +40,15 @@ public void TearDown()
_tempTestDir.Delete(recursive: true);
}
- #endregion
-
- #region Tests
-
- // The Integration_HappyPath_MockAnonymiser method is a test case
- // structured in the Arrange-Act-Assert pattern. It tests for the
- // scenario where the DICOM Anonymiser successfully anonymises a
- // DICOM file.
[Test]
public void Integration_HappyPath_MockAnonymiser()
{
- // Arrange
- // It sets up the necessary objects and state for the test. This
- // includes creating a mock DICOM Anonymiser, setting file paths,
- // and creating a DicomAnonymiserHost.
-
GlobalOptions globals = new GlobalOptionsFactory().Load(nameof(Integration_HappyPath_MockAnonymiser));
globals.FileSystemOptions!.FileSystemRoot = _dicomRoot.FullName;
var extractRoot = Directory.CreateDirectory(Path.Combine(_tempTestDir.FullName, "extractRoot"));
globals.FileSystemOptions.ExtractRoot = extractRoot.FullName;
- // NOTE: The commented out code below is an alternative way to create
- // a fake DICOM file, however, it is not used in this test.
- // File.Create(_fakeDicom).Dispose();
- // File.SetAttributes(_fakeDicom, File.GetAttributes(_fakeDicom) | FileAttributes.ReadOnly);
-
var dicomFile = new DicomFile();
dicomFile.Dataset.Add(DicomTag.PatientID, "12345678");
dicomFile.Dataset.Add(DicomTag.Modality, "CT");
@@ -122,18 +80,14 @@ public void Integration_HappyPath_MockAnonymiser()
OutputPath = "foo-an.dcm",
};
- // The test uses the Moq library to create a mock implementation of
- // the IDicomAnonymiser interface. This allows the test to control the
- // behavior of the DICOM Anonymiser and verify that it is called with
- // the correct arguments.
var mockAnonymiser = new Mock(MockBehavior.Strict);
mockAnonymiser
.Setup(
x => x.Anonymise(
- It.Is(x => x.ExtractionJobIdentifier == testExtractFileMessage.ExtractionJobIdentifier),
- It.Is(x => x.FullName == _fakeDicom),
- It.Is(x => x.FullName == Path.Combine(extractDirAbs.FullName, "foo-an.dcm")),
- out It.Ref.IsAny
+ It.IsAny(),
+ It.IsAny(),
+ It.IsAny(),
+ out It.Ref.IsAny
)
)
.Callback(() => File.Create(expectedAnonPathAbs).Dispose())
@@ -145,45 +99,33 @@ out It.Ref.IsAny
List statusMessages = [];
- using (
- var tester = new MicroserviceTester(
- globals.RabbitOptions!,
- globals.DicomAnonymiserOptions.AnonFileConsumerOptions!
- )
- )
- {
- tester.CreateExchange(statusExchange, successQueue, isSecondaryBinding: false, routingKey: "verify");
- tester.CreateExchange(statusExchange, failureQueue, isSecondaryBinding: true, routingKey: "noverify");
-
- tester.SendMessage(globals.DicomAnonymiserOptions.AnonFileConsumerOptions!, new MessageHeader(), testExtractFileMessage);
+ using var tester = new MicroserviceTester(
+ globals.RabbitOptions!,
+ globals.DicomAnonymiserOptions.AnonFileConsumerOptions!
+ );
- var host = new DicomAnonymiserHost(globals, mockAnonymiser.Object);
+ tester.CreateExchange(statusExchange, successQueue, isSecondaryBinding: false, routingKey: "verify");
+ tester.CreateExchange(statusExchange, failureQueue, isSecondaryBinding: true, routingKey: "noverify");
- // Act
- // It starts the DicomAnonymiserHost and waits for it to process
- //a message.
+ tester.SendMessage(globals.DicomAnonymiserOptions.AnonFileConsumerOptions!, new MessageHeader(), testExtractFileMessage);
- host.Start();
+ var host = new DicomAnonymiserHost(globals, mockAnonymiser.Object);
- var timeoutSecs = 10;
+ host.Start();
- while (statusMessages.Count == 0 && timeoutSecs > 0)
- {
- statusMessages.AddRange(tester.ConsumeMessages(successQueue).Select(x => x.Item2));
- statusMessages.AddRange(tester.ConsumeMessages(failureQueue).Select(x => x.Item2));
+ var timeoutSecs = 10;
- --timeoutSecs;
- if (statusMessages.Count == 0)
- Thread.Sleep(TimeSpan.FromSeconds(1));
- }
+ while (statusMessages.Count == 0 && timeoutSecs > 0)
+ {
+ statusMessages.AddRange(tester.ConsumeMessages(successQueue).Select(x => x.Item2));
+ statusMessages.AddRange(tester.ConsumeMessages(failureQueue).Select(x => x.Item2));
- host.Stop("Test end");
+ --timeoutSecs;
+ if (statusMessages.Count == 0)
+ Thread.Sleep(TimeSpan.FromSeconds(1));
}
- // Assert
- // It checks that the expected outcome has occurred. In this case, it
- // checks that the status message indicates that the file was anonymised
- // and that the anonymised file exists.
+ host.Stop("Test end");
var statusMessage = statusMessages.Single();
Assert.Multiple(() =>
@@ -192,6 +134,4 @@ out It.Ref.IsAny
Assert.That(File.Exists(expectedAnonPathAbs), Is.True);
});
}
-
- #endregion
}
diff --git a/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/FixtureSetup.cs b/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/FixtureSetup.cs
new file mode 100644
index 000000000..72db15821
--- /dev/null
+++ b/tests/SmiServices.IntegrationTests/Microservices/DicomAnonymiser/FixtureSetup.cs
@@ -0,0 +1,24 @@
+using NUnit.Framework;
+using SmiServices.UnitTests.TestCommon;
+using System.IO;
+
+namespace SmiServices.IntegrationTests.Microservices.DicomAnonymiser;
+
+[SetUpFixture]
+internal class FixtureSetup
+{
+ private const string TEST_CTP_JAR_VERSION = "0.7.0";
+
+ public static string CtpJarPath;
+ public static string CtpAllowlistPath;
+
+ [OneTimeSetUp]
+ public void OneTimeSetUp()
+ {
+ CtpJarPath = Path.Join(TestDirectoryHelpers.SlnDirectoryInfo().FullName, $"data/ctp/ctp-anon-cli-{TEST_CTP_JAR_VERSION}.jar");
+ Assert.That(File.Exists(CtpJarPath), Is.True, $"Expected {CtpJarPath} to exist");
+
+ CtpAllowlistPath = Path.Join(TestDirectoryHelpers.SlnDirectoryInfo().FullName, $"data/ctp/ctp-allowlist.script");
+ Assert.That(File.Exists(CtpAllowlistPath), Is.True, $"Expected {CtpAllowlistPath} to exist");
+ }
+}
diff --git a/tests/SmiServices.UnitTests/LoggerFixture.cs b/tests/SmiServices.UnitTests/LoggerFixture.cs
index e2b1e5351..463fa27a9 100644
--- a/tests/SmiServices.UnitTests/LoggerFixture.cs
+++ b/tests/SmiServices.UnitTests/LoggerFixture.cs
@@ -17,7 +17,7 @@ public static void Setup()
var consoleTarget = new ConsoleTarget(TestLoggerName)
{
- Layout = @"${longdate}|${level}|${logger}|${message}|${exception:format=toString,Data:maxInnerExceptionLevel=5}",
+ Layout = @"${longdate}|${level:padding=-5}|${logger}|${message}|${exception:format=toString,Data:maxInnerExceptionLevel=5}",
AutoFlush = true
};
diff --git a/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactoryTests.cs b/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactoryTests.cs
index d7e5cfb61..6498f4fb1 100644
--- a/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactoryTests.cs
+++ b/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/Anonymisers/AnonymiserFactoryTests.cs
@@ -7,53 +7,73 @@ namespace SmiServices.UnitTests.Microservices.DicomAnonymiser.Anonymisers;
public class AnonymiserFactoryTests
{
- #region Fixture Methods
-
- [OneTimeSetUp]
- public void OneTimeSetUp()
+ [Test]
+ public void CreateAnonymiser_ValidAnonymiser_IsOk()
{
- }
-
- [OneTimeTearDown]
- public void OneTimeTearDown() { }
-
- #endregion
+ // Arrange
- #region Test Methods
+ var globals = new GlobalOptions
+ {
+ DicomAnonymiserOptions = new DicomAnonymiserOptions
+ {
+ AnonymiserType = "SmiCtpAnonymiser",
+ CtpAnonCliJar = "missing.jar",
+ }
+ };
- [SetUp]
- public void SetUp() { }
+ // Act
- [TearDown]
- public void TearDown() { }
+ void call() => AnonymiserFactory.CreateAnonymiser(globals);
- #endregion
+ // Assert
- #region Tests
+ var exc = Assert.Throws(call);
+ Assert.That(exc.Message, Is.EqualTo("CtpAnonCliJar 'missing.jar' does not exist (Parameter 'globalOptions')"));
+ }
[Test]
public void CreateAnonymiser_InvalidAnonymiserName_ThrowsException()
{
- var e = Assert.Throws(() =>
+ // Arrange
+
+ var globals = new GlobalOptions
{
- // TODO (da 2024-02-28) Review if this is the correct way to test this
- // AnonymiserFactory.CreateAnonymiser(new DefaultAnonymiser { AnonymiserType = "whee" });
- AnonymiserFactory.CreateAnonymiser(new GlobalOptions { DicomAnonymiserOptions = new DicomAnonymiserOptions { AnonymiserType = "whee" } });
- });
- Assert.That(e!.Message, Is.EqualTo("Could not parse 'whee' to a valid AnonymiserType"));
+ DicomAnonymiserOptions = new DicomAnonymiserOptions
+ {
+ AnonymiserType = "whee",
+ }
+ };
+
+ // Act
+
+ void call() => AnonymiserFactory.CreateAnonymiser(globals);
+
+ // Assert
+
+ var e = Assert.Throws(call);
+ Assert.That(e.Message, Is.EqualTo("Could not parse 'whee' to a valid AnonymiserType"));
}
[Test]
public void CreateAnonymiser_NoCaseForAnonymiser_ThrowsException()
{
- var e = Assert.Throws(() =>
+ // Arrange
+
+ var globals = new GlobalOptions
{
- // TODO (da 2024-02-28) Review if this is the correct way to test this
- // AnonymiserFactory.CreateAnonymiser(new DicomAnonymiserOptions { AnonymiserType = "None" });
- AnonymiserFactory.CreateAnonymiser(new GlobalOptions { DicomAnonymiserOptions = new DicomAnonymiserOptions { AnonymiserType = "None" } });
- });
- Assert.That(e!.Message, Is.EqualTo("No case for AnonymiserType 'None'"));
- }
+ DicomAnonymiserOptions = new DicomAnonymiserOptions
+ {
+ AnonymiserType = "None",
+ }
+ };
- #endregion
+ // Act
+
+ void call() => AnonymiserFactory.CreateAnonymiser(globals);
+
+ // Assert
+
+ var e = Assert.Throws(call);
+ Assert.That(e.Message, Is.EqualTo("No case for AnonymiserType 'None'"));
+ }
}
diff --git a/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/DicomAnonymiserConsumerTests.cs b/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/DicomAnonymiserConsumerTests.cs
index 078844e52..2cdcc9c72 100644
--- a/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/DicomAnonymiserConsumerTests.cs
+++ b/tests/SmiServices.UnitTests/Microservices/DicomAnonymiser/DicomAnonymiserConsumerTests.cs
@@ -83,6 +83,7 @@ public void SetUp()
ExtractionDirectory = extractDirName,
DicomFilePath = "foo.dcm",
OutputPath = "foo-an.dcm",
+ Modality = "CT",
};
_options = new DicomAnonymiserOptions
@@ -121,10 +122,6 @@ public void TearDown() { }
#region Tests
- // TODO (da 2024-03-28) Extract modality from cohort extractor instead of opening DICOM file
- // This test is disabled because of FellowOakDicom.DicomFileException caused by DicomFile.Open
- // Once the above TODO is implemented, this test can be enabled.
- /*
[Test]
public void ProcessMessageImpl_HappyPath()
{
@@ -132,10 +129,10 @@ public void ProcessMessageImpl_HappyPath()
Expression> expectedAnonCall =
x => x.Anonymise(
- It.Is(x => x == _extractFileMessage),
It.Is(x => x.FullName == _sourceDcmPathAbs),
It.Is(x => x.FullName == _mockFs.Path.Combine(_extractDir, _extractFileMessage.OutputPath)),
- out It.Ref.IsAny
+ "CT",
+ out It.Ref.IsAny
);
var mockAnonymiser = new Mock(MockBehavior.Strict);
@@ -160,7 +157,8 @@ out It.Ref.IsAny
var consumer = GetNewDicomAnonymiserConsumer(mockAnonymiser.Object, mockProducerModel.Object);
// Act
- consumer.TestMessage(_extractFileMessage);
+
+ consumer.ProcessMessage(new MessageHeader(), _extractFileMessage, 1);
// Assert
@@ -169,7 +167,6 @@ out It.Ref.IsAny
mockAnonymiser.Verify(expectedAnonCall, Times.Once);
mockProducerModel.Verify(expectedSendCall, Times.Once);
}
- */
[Test]
public void ProcessMessageImpl_IsIdentifiableExtraction_ThrowsException()
@@ -296,29 +293,27 @@ public void ProcessMessageImpl_ExtractionDirMissing_ThrowsException()
});
}
- // TODO (da 2024-03-28) Extract modality from cohort extractor instead of opening DICOM file
- // This test is disabled because of FellowOakDicom.DicomFileException caused by DicomFile.Open
- // Once the above TODO is implemented, this test can be enabled.
- /*
+ private class FailingAnonymiser : IDicomAnonymiser
+ {
+ public ExtractedFileStatus Anonymise(IFileInfo sourceFile, IFileInfo destFile, string modality, out string? anonymiserStatusMessage)
+ {
+ anonymiserStatusMessage = "oh no!";
+ return ExtractedFileStatus.ErrorWontRetry;
+ }
+ }
+
[Test]
public void ProcessMessageImpl_AnonymisationFailed_AcksWithFailureStatus()
{
// Arrange
- var mockAnonymiser = new Mock(MockBehavior.Strict);
- mockAnonymiser
- .Setup(x => x.Anonymise(
- It.IsAny(),
- It.IsAny(),
- It.IsAny(),
- out It.Ref.IsAny))
- .Throws(new Exception("oh no"));
+ var anonymiser = new FailingAnonymiser();
Expression> expectedCall =
x => x.SendMessage(
It.Is(x =>
x.Status == ExtractedFileStatus.ErrorWontRetry &&
- x.StatusMessage!.StartsWith($"Error anonymising '{_sourceDcmPathAbs}'. Exception message: IDicomAnonymiser") &&
+ x.StatusMessage!.StartsWith("oh no!") &&
x.OutputFilePath == null
),
It.IsAny(),
@@ -328,10 +323,10 @@ public void ProcessMessageImpl_AnonymisationFailed_AcksWithFailureStatus()
var mockProducerModel = new Mock();
mockProducerModel.Setup(expectedCall);
- var consumer = GetNewDicomAnonymiserConsumer(null, mockProducerModel.Object);
+ var consumer = GetNewDicomAnonymiserConsumer(anonymiser, mockProducerModel.Object);
// Act
- consumer.TestMessage(_extractFileMessage);
+ consumer.ProcessMessage(new MessageHeader(), _extractFileMessage, 1);
// Assert
@@ -339,7 +334,6 @@ public void ProcessMessageImpl_AnonymisationFailed_AcksWithFailureStatus()
mockProducerModel.Verify(expectedCall, Times.Once);
}
- */
#endregion
}
diff --git a/tests/SmiServices.UnitTests/TestCommon/DisposableTempDir.cs b/tests/SmiServices.UnitTests/TestCommon/DisposableTempDir.cs
new file mode 100644
index 000000000..b5b57f72d
--- /dev/null
+++ b/tests/SmiServices.UnitTests/TestCommon/DisposableTempDir.cs
@@ -0,0 +1,32 @@
+using NUnit.Framework;
+using System;
+using System.IO;
+
+namespace SmiServices.UnitTests.TestCommon;
+
+public sealed class DisposableTempDir : IDisposable
+{
+ private const string PREFIX = "smiservices-nunit-";
+ public readonly DirectoryInfo DirectoryInfo;
+
+ public DisposableTempDir()
+ {
+ DirectoryInfo = Directory.CreateTempSubdirectory(PREFIX);
+ TestContext.Out.WriteLine($"[{GetType().Name}] Created {DirectoryInfo}");
+ }
+
+ public void Dispose()
+ {
+ try
+ {
+ DirectoryInfo.Delete(recursive: true);
+ TestContext.Out.WriteLine($"[{GetType().Name}] Deleted {DirectoryInfo}");
+ }
+ catch (UnauthorizedAccessException)
+ {
+ TestContext.Error.WriteLine($"[{GetType().Name}] Could not delete {DirectoryInfo}");
+ }
+ }
+
+ public static implicit operator string(DisposableTempDir d) => d.DirectoryInfo.FullName;
+}
diff --git a/tests/SmiServices.UnitTests/TestCommon/TestDirectoryHelpers.cs b/tests/SmiServices.UnitTests/TestCommon/TestDirectoryHelpers.cs
new file mode 100644
index 000000000..bdbb64ca2
--- /dev/null
+++ b/tests/SmiServices.UnitTests/TestCommon/TestDirectoryHelpers.cs
@@ -0,0 +1,25 @@
+using System.IO;
+
+namespace SmiServices.UnitTests.TestCommon;
+
+public static class TestDirectoryHelpers
+{
+ private static DirectoryInfo? _slnDirInfo;
+
+ public static DirectoryInfo SlnDirectoryInfo()
+ {
+ if (_slnDirInfo != null)
+ return _slnDirInfo;
+
+ var directory = new DirectoryInfo(Directory.GetCurrentDirectory());
+
+ while (directory != null && directory.GetFiles("*.sln").Length == 0)
+ directory = directory.Parent;
+
+ if (directory == null)
+ throw new FileNotFoundException("Could not find sln file");
+
+ _slnDirInfo = directory;
+ return directory;
+ }
+}