From 8b0e9d2e5dae0cd17176885c24cefb563af800e5 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 15:43:40 +0100 Subject: [PATCH 01/18] rename DefaultProjectPathResolver to StudySeriesOriginalFilenameProjectPathResolver --- ...ver.cs => StudySeriesOriginalFilenameProjectPathResolver.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/{DefaultProjectPathResolver.cs => StudySeriesOriginalFilenameProjectPathResolver.cs} (95%) diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/DefaultProjectPathResolver.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs similarity index 95% rename from src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/DefaultProjectPathResolver.cs rename to src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs index 64cb2b26d..76fb998ff 100644 --- a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/DefaultProjectPathResolver.cs +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs @@ -5,7 +5,7 @@ namespace SmiServices.Microservices.CohortExtractor.ProjectPathResolvers { - public class DefaultProjectPathResolver : IProjectPathResolver + public class StudySeriesOriginalFilenameProjectPathResolver : IProjectPathResolver { public string AnonExt { get; protected set; } = "-an.dcm"; public string IdentExt { get; protected set; } = ".dcm"; From 95687e3608fbed3fb2ae9757c93942bd8367ef6a Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 15:47:15 +0100 Subject: [PATCH 02/18] revert hidden directory handling (#1506) --- ...riesOriginalFilenameProjectPathResolver.cs | 8 ++----- .../DefaultProjectPathResolverTest.cs | 21 ------------------- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs index 76fb998ff..ab026416a 100644 --- a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs @@ -39,13 +39,9 @@ public string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessag if (!replaced) fileName += extToUse; - // Remove any leading periods from the UIDs - string? studyUID = result.StudyTagValue?.TrimStart('.'); - string? seriesUID = result.SeriesTagValue?.TrimStart('.'); - return Path.Combine( - studyUID ?? "unknown", - seriesUID ?? "unknown", + result.StudyTagValue ?? "unknown", + result.SeriesTagValue ?? "unknown", fileName); } } diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/DefaultProjectPathResolverTest.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/DefaultProjectPathResolverTest.cs index d7cfc48ed..2c8f1771f 100644 --- a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/DefaultProjectPathResolverTest.cs +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/DefaultProjectPathResolverTest.cs @@ -125,27 +125,6 @@ public void Test_DefaultProjectPathResolver_IdentExtraction() "file.dcm"))); } - [TestCase(".study", "series")] - [TestCase(".study", ".series")] - [TestCase(null, ".series")] - [TestCase(".study", null)] - public void TestDefaultProjectPathResolver_HiddenDirectories(string? study, string? series) - { - var result = new QueryToExecuteResult( - "foo.dcm", - study, - series, - "sop", - false, - null); - - Assert.That( - new DefaultProjectPathResolver().GetOutputPath(result, _requestMessage), Is.EqualTo(Path.Combine( - study?.TrimStart('.') ?? "unknown", - series?.TrimStart('.') ?? "unknown", - "foo-an.dcm"))); - } - #endregion } } From 753bff7f35c7115dbf100ba112bacef1bf7547fe Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 15:49:27 +0100 Subject: [PATCH 03/18] rename test file to match src --- ...s => StudySeriesOriginalFilenameProjectPathResolverTests.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/SmiServices.UnitTests/Microservices/CohortExtractor/{DefaultProjectPathResolverTest.cs => StudySeriesOriginalFilenameProjectPathResolverTests.cs} (98%) diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/DefaultProjectPathResolverTest.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesOriginalFilenameProjectPathResolverTests.cs similarity index 98% rename from tests/SmiServices.UnitTests/Microservices/CohortExtractor/DefaultProjectPathResolverTest.cs rename to tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesOriginalFilenameProjectPathResolverTests.cs index 2c8f1771f..5c65aafca 100644 --- a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/DefaultProjectPathResolverTest.cs +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesOriginalFilenameProjectPathResolverTests.cs @@ -8,7 +8,7 @@ namespace SmiServices.UnitTests.Microservices.CohortExtractor { - public class DefaultProjectPathResolverTest + public class StudySeriesOriginalFilenameProjectPathResolverTests { #region Fixture Methods From 8df5e7eb58ad07280a2cfcf1b3dc2b135c759756 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 15:52:46 +0100 Subject: [PATCH 04/18] add IFileSystem and extract constants --- .../ProjectPathResolverConstants.cs | 13 +++++++++++++ ...eriesOriginalFilenameProjectPathResolver.cs | 18 +++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-) create mode 100644 src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverConstants.cs diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverConstants.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverConstants.cs new file mode 100644 index 000000000..e35589ef5 --- /dev/null +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverConstants.cs @@ -0,0 +1,13 @@ +namespace SmiServices.Microservices.CohortExtractor.ProjectPathResolvers; +public static class ProjectPathResolverConstants +{ + /// + /// Extension used when creating anonymised files + /// + public const string ANON_EXT = "-an.dcm"; + + /// + /// Extension used when creating "identifiable" files + /// + public const string IDENT_EXT = ".dcm"; +} diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs index ab026416a..b2a9f608d 100644 --- a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs @@ -1,17 +1,21 @@ using SmiServices.Common.Messages.Extraction; using SmiServices.Microservices.CohortExtractor.RequestFulfillers; using System; -using System.IO; +using System.IO.Abstractions; namespace SmiServices.Microservices.CohortExtractor.ProjectPathResolvers { public class StudySeriesOriginalFilenameProjectPathResolver : IProjectPathResolver { - public string AnonExt { get; protected set; } = "-an.dcm"; - public string IdentExt { get; protected set; } = ".dcm"; - private static readonly string[] _replaceableExtensions = [".dcm", ".dicom"]; + private readonly IFileSystem _fileSystem; + + public StudySeriesOriginalFilenameProjectPathResolver(IFileSystem fileSystem) + { + _fileSystem = fileSystem; + } + /// /// Returns the output path for the anonymised file, relative to the ExtractionDirectory /// @@ -20,10 +24,10 @@ public class StudySeriesOriginalFilenameProjectPathResolver : IProjectPathResolv /// public string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessage message) { - string extToUse = message.IsIdentifiableExtraction ? IdentExt : AnonExt; + string extToUse = message.IsIdentifiableExtraction ? ProjectPathResolverConstants.IDENT_EXT : ProjectPathResolverConstants.ANON_EXT; // The extension of the input DICOM file can be anything (or nothing), but here we try to standardise the output file name to have the required extension - string fileName = Path.GetFileName(result.FilePathValue); + string fileName = _fileSystem.Path.GetFileName(result.FilePathValue); if (string.IsNullOrWhiteSpace(fileName)) throw new ArgumentNullException(nameof(result)); @@ -39,7 +43,7 @@ public string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessag if (!replaced) fileName += extToUse; - return Path.Combine( + return _fileSystem.Path.Combine( result.StudyTagValue ?? "unknown", result.SeriesTagValue ?? "unknown", fileName); From 489d9cd8fa037c2dac4b950d41245612f2402a56 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 15:53:06 +0100 Subject: [PATCH 05/18] add StudySeriesSOPProjectPathResolver --- .../StudySeriesSOPProjectPathResolver.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesSOPProjectPathResolver.cs diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesSOPProjectPathResolver.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesSOPProjectPathResolver.cs new file mode 100644 index 000000000..f05928f43 --- /dev/null +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesSOPProjectPathResolver.cs @@ -0,0 +1,31 @@ +using SmiServices.Common.Messages.Extraction; +using SmiServices.Microservices.CohortExtractor.RequestFulfillers; +using System.IO.Abstractions; + +namespace SmiServices.Microservices.CohortExtractor.ProjectPathResolvers; + +/// +/// Generates output paths in the form: +/// StudyInstanceUID/SeriesInstanceUID/SOPInstanceUID-an.dcm +/// +public class StudySeriesSOPProjectPathResolver : IProjectPathResolver +{ + private readonly IFileSystem _fileSystem; + + public StudySeriesSOPProjectPathResolver(IFileSystem fileSystem) + { + _fileSystem = fileSystem; + } + + /// + public string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessage request) + { + string extToUse = request.IsIdentifiableExtraction ? ProjectPathResolverConstants.IDENT_EXT : ProjectPathResolverConstants.ANON_EXT; + + return _fileSystem.Path.Combine( + result.StudyTagValue, + result.SeriesTagValue, + $"{result.InstanceTagValue}{extToUse}" + ); + } +} From c52ea60c0b9c25b13b75a296ce58a82fcea27a9f Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 16:01:57 +0100 Subject: [PATCH 06/18] disallow null Study/SeriesInstanceUIDs in extractions --- .../StudySeriesOriginalFilenameProjectPathResolver.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs index b2a9f608d..27d635545 100644 --- a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs @@ -44,9 +44,10 @@ public string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessag fileName += extToUse; return _fileSystem.Path.Combine( - result.StudyTagValue ?? "unknown", - result.SeriesTagValue ?? "unknown", - fileName); + result.StudyTagValue, + result.SeriesTagValue, + fileName + ); } } } From 0f307b4c62e910ba877aaab938af5b318e2b3b1d Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 16:17:46 +0100 Subject: [PATCH 07/18] add fileSystem param to CohortExtractorHost --- .../Microservices/CohortExtractor/CohortExtractorHost.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs b/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs index f92f580f2..38a1adc2c 100644 --- a/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs +++ b/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs @@ -16,6 +16,7 @@ using SmiServices.Microservices.CohortExtractor.RequestFulfillers; using SmiServices.Microservices.CohortExtractor.RequestFulfillers.Dynamic; using System; +using System.IO.Abstractions; using System.Linq; using System.Text.RegularExpressions; @@ -42,13 +43,16 @@ public class CohortExtractorHost : MicroserviceHost private IProjectPathResolver? _pathResolver; private IProducerModel? _fileMessageProducer; + private readonly IFileSystem _fileSystem; + /// /// Creates a new instance of the host with the given /// /// Settings for the microservice (location of rabbit, queue names etc) /// Optional override for the value specified in /// Optional override for the value specified in - public CohortExtractorHost(GlobalOptions options, IAuditExtractions? auditor, IExtractionRequestFulfiller? fulfiller) + /// + public CohortExtractorHost(GlobalOptions options, IAuditExtractions? auditor, IExtractionRequestFulfiller? fulfiller, IFileSystem? fileSystem = null) : base(options) { _consumerOptions = options.CohortExtractorOptions!; @@ -56,6 +60,7 @@ public CohortExtractorHost(GlobalOptions options, IAuditExtractions? auditor, IE _auditor = auditor; _fulfiller = fulfiller; + _fileSystem = fileSystem ?? new FileSystem(); } /// @@ -156,7 +161,7 @@ private void InitializeExtractionSources(IRDMPPlatformRepositoryServiceLocator r } _pathResolver = string.IsNullOrWhiteSpace(_consumerOptions.ProjectPathResolverType) - ? new DefaultProjectPathResolver() + ? new StudySeriesOriginalFilenameProjectPathResolver(_fileSystem) : ObjectFactory.CreateInstance( _consumerOptions.ProjectPathResolverType, typeof(IProjectPathResolver).Assembly, repositoryLocator); } From 1e8ea62761ce09216fcbfa92f65a00f9615ecaaf Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 16:18:15 +0100 Subject: [PATCH 08/18] fixup NoSuffixProjectPathResolver --- .../NoSuffixProjectPathResolver.cs | 14 ++++++++++---- ...udySeriesOriginalFilenameProjectPathResolver.cs | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/NoSuffixProjectPathResolver.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/NoSuffixProjectPathResolver.cs index 5e49d9575..01b16e948 100644 --- a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/NoSuffixProjectPathResolver.cs +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/NoSuffixProjectPathResolver.cs @@ -1,13 +1,19 @@ +using SmiServices.Common.Messages.Extraction; +using SmiServices.Microservices.CohortExtractor.RequestFulfillers; +using System.IO.Abstractions; + namespace SmiServices.Microservices.CohortExtractor.ProjectPathResolvers { /// - /// Acts like but with no "-an" component to indicate files have been anonymised. In most cases this results in an identical filename to the source file but can include the addition of a .dcm extension where it is missing + /// Acts like but with no "-an" component to indicate files have been anonymised. In most cases this results in an identical filename to the source file but can include the addition of a .dcm extension where it is missing /// - public class NoSuffixProjectPathResolver : DefaultProjectPathResolver + public class NoSuffixProjectPathResolver : StudySeriesOriginalFilenameProjectPathResolver { - public NoSuffixProjectPathResolver() + public NoSuffixProjectPathResolver(IFileSystem fileSystem) : base(fileSystem) { } + + public override string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessage message) { - AnonExt = ".dcm"; + return base.GetOutputPath(result, message).Replace("-an", ""); } } } diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs index 27d635545..67dcb3ed7 100644 --- a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/StudySeriesOriginalFilenameProjectPathResolver.cs @@ -22,7 +22,7 @@ public StudySeriesOriginalFilenameProjectPathResolver(IFileSystem fileSystem) /// /// /// - public string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessage message) + public virtual string GetOutputPath(QueryToExecuteResult result, ExtractionRequestMessage message) { string extToUse = message.IsIdentifiableExtraction ? ProjectPathResolverConstants.IDENT_EXT : ProjectPathResolverConstants.ANON_EXT; From f16686012146f64b97bb7f674fa3da825d916110 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 16:19:41 +0100 Subject: [PATCH 09/18] disallow null Study/Series/SOPInstanceUIDs in CohortExtractor and fixup tests --- .../RequestFulfillers/QueryToExecute.cs | 23 ++-- .../RequestFulfillers/QueryToExecuteResult.cs | 12 +- .../ExtractionRequestQueueConsumerTest.cs | 15 ++- .../NoSuffixProjectPathResolverTests.cs | 90 +++++++------- ...riginalFilenameProjectPathResolverTests.cs | 117 +++++++++--------- 5 files changed, 136 insertions(+), 121 deletions(-) diff --git a/src/SmiServices/Microservices/CohortExtractor/RequestFulfillers/QueryToExecute.cs b/src/SmiServices/Microservices/CohortExtractor/RequestFulfillers/QueryToExecute.cs index e388c049b..ce1a0f9eb 100644 --- a/src/SmiServices/Microservices/CohortExtractor/RequestFulfillers/QueryToExecute.cs +++ b/src/SmiServices/Microservices/CohortExtractor/RequestFulfillers/QueryToExecute.cs @@ -117,10 +117,10 @@ public IEnumerable Execute(string valueToLookup, List Execute(string valueToLookup, List { public readonly string FilePathValue; - public readonly string? StudyTagValue; - public readonly string? SeriesTagValue; - public readonly string? InstanceTagValue; + public readonly string StudyTagValue; + public readonly string SeriesTagValue; + public readonly string InstanceTagValue; public readonly bool Reject; public readonly string? RejectReason; public QueryToExecuteResult( string filePathValue, - string? studyTagValue, - string? seriesTagValue, - string? instanceTagValue, + string studyTagValue, + string seriesTagValue, + string instanceTagValue, bool rejection, string? rejectionReason ) diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs index e4afc0bee..23084e10a 100644 --- a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs @@ -12,7 +12,8 @@ using SmiServices.Microservices.CohortExtractor.RequestFulfillers; using SmiServices.UnitTests.Common; using System; -using System.Collections.Generic; +using System.IO.Abstractions; +using System.IO.Abstractions.TestingHelpers; using System.Threading; namespace SmiServices.UnitTests.Microservices.CohortExtractor.Messaging @@ -21,10 +22,13 @@ public class ExtractionRequestQueueConsumerTest { #region Fixture Methods + private IFileSystem _fileSystem; + [OneTimeSetUp] public void OneTimeSetUp() { TestLogger.Setup(); + MessageHeader.CurrentProgramName = nameof(ExtractionRequestQueueConsumerTest); } [OneTimeTearDown] @@ -35,7 +39,10 @@ public void OneTimeTearDown() { } #region Test Methods [SetUp] - public void SetUp() { } + public void SetUp() + { + _fileSystem = new MockFileSystem(); + } [TearDown] public void TearDown() { } @@ -68,7 +75,7 @@ public void Test_ExtractionRequestQueueConsumer_IdentExtraction_RoutingKey() /// /// /// - private static void AssertMessagePublishedWithSpecifiedKey(GlobalOptions globals, bool isIdentifiableExtraction, string expectedRoutingKey) + private void AssertMessagePublishedWithSpecifiedKey(GlobalOptions globals, bool isIdentifiableExtraction, string expectedRoutingKey) { var fakeFulfiller = new FakeFulfiller(); @@ -101,7 +108,7 @@ private static void AssertMessagePublishedWithSpecifiedKey(GlobalOptions globals var consumer = new ExtractionRequestQueueConsumer( globals.CohortExtractorOptions!, fakeFulfiller, - new NullAuditExtractions(), new DefaultProjectPathResolver(), + new NullAuditExtractions(), new StudySeriesOriginalFilenameProjectPathResolver(_fileSystem), mockFileMessageProducerModel.Object, mockFileInfoMessageProducerModel.Object); diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs index 0c81b2995..3e2d57446 100644 --- a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs @@ -3,77 +3,81 @@ using SmiServices.Common.Messages.Extraction; using SmiServices.Microservices.CohortExtractor.ProjectPathResolvers; using SmiServices.Microservices.CohortExtractor.RequestFulfillers; -using System.IO; +using SmiServices.UnitTests.Common; +using System.IO.Abstractions; +using System.IO.Abstractions.TestingHelpers; namespace SmiServices.UnitTests.Microservices.CohortExtractor { - public class NoSuffixProjectPathResolverTests : Tests.Common.UnitTests + public class NoSuffixProjectPathResolverTests { + private IFileSystem _fileSystem; - [TestCase("study", "series")] - [TestCase("study", null)] - [TestCase(null, "series")] - [TestCase(null, null)] - public void TestDefaultProjectPathResolver_IdParts(string? study, string? series) + [SetUp] + public void SetUp() { + TestLogger.Setup(); + _fileSystem = new MockFileSystem(); + } + + [Test] + public void GetOutputPath_Basic() + { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", "foo.dcm"); + var resolver = new NoSuffixProjectPathResolver(_fileSystem); var result = new QueryToExecuteResult( "foo.dcm", - study, - series, + "study", + "series", "sop", - false, - null); + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage(); + + // Act + + var actualPath = resolver.GetOutputPath(result, message); - Assert.That( - new NoSuffixProjectPathResolver().GetOutputPath(result, new ExtractionRequestMessage()), Is.EqualTo(Path.Combine( - study ?? "unknown", - series ?? "unknown", - "foo.dcm"))); + // Assert + Assert.That(actualPath, Is.EqualTo(expectedPath)); } [TestCase("file.dcm", "file.dcm")] [TestCase("file.dcm", "file.dicom")] [TestCase("file.dcm", "file")] [TestCase("file.foo.dcm", "file.foo")] - public void TestDefaultProjectPathResolver_Extensions(string expectedOutput, string inputFile) + public void GetOutputPath_Extensions(string expectedOutput, string inputFile) { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", expectedOutput); + var resolver = new NoSuffixProjectPathResolver(_fileSystem); var result = new QueryToExecuteResult( - Path.Combine("foo", inputFile), + inputFile, "study", "series", "sop", - false, - null); + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage(); - Assert.That( - new NoSuffixProjectPathResolver().GetOutputPath(result, new ExtractionRequestMessage()), Is.EqualTo(Path.Combine( - "study", - "series", - expectedOutput))); - } + // Act - [Test] - public void TestDefaultProjectPathResolver_Both() - { - var result = new QueryToExecuteResult( - Path.Combine("foo", "file"), - "study", - null, - "sop", - false, - null); + var actualPath = resolver.GetOutputPath(result, message); + + // Assert - Assert.That( - new NoSuffixProjectPathResolver().GetOutputPath(result, new ExtractionRequestMessage()), Is.EqualTo(Path.Combine( - "study", - "unknown", - "file.dcm"))); + Assert.That(actualPath, Is.EqualTo(expectedPath)); } [Test] - public void TestCreatingByReflection() + public void CanBeConstructedByReflection() { - var instance = new MicroserviceObjectFactory().CreateInstance("SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.NoSuffixProjectPathResolver", typeof(IProjectPathResolver).Assembly, RepositoryLocator); + var instance = new MicroserviceObjectFactory().CreateInstance("SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.NoSuffixProjectPathResolver", typeof(IProjectPathResolver).Assembly, _fileSystem); Assert.That(instance, Is.InstanceOf()); } } diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesOriginalFilenameProjectPathResolverTests.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesOriginalFilenameProjectPathResolverTests.cs index 5c65aafca..c1e7aa05f 100644 --- a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesOriginalFilenameProjectPathResolverTests.cs +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesOriginalFilenameProjectPathResolverTests.cs @@ -3,7 +3,8 @@ using SmiServices.Microservices.CohortExtractor.ProjectPathResolvers; using SmiServices.Microservices.CohortExtractor.RequestFulfillers; using SmiServices.UnitTests.Common; -using System.IO; +using System.IO.Abstractions; +using System.IO.Abstractions.TestingHelpers; namespace SmiServices.UnitTests.Microservices.CohortExtractor @@ -13,6 +14,7 @@ public class StudySeriesOriginalFilenameProjectPathResolverTests #region Fixture Methods private ExtractionRequestMessage _requestMessage = null!; + private IFileSystem _fileSystem; [OneTimeSetUp] public void OneTimeSetUp() @@ -33,7 +35,10 @@ public void OneTimeTearDown() { } #region Test Methods [SetUp] - public void SetUp() { } + public void SetUp() + { + _fileSystem = new MockFileSystem(); + } [TearDown] public void TearDown() { } @@ -42,87 +47,85 @@ public void TearDown() { } #region Tests - [TestCase("study", "series")] - [TestCase("study", null)] - [TestCase(null, "series")] - [TestCase(null, null)] - public void TestDefaultProjectPathResolver_IdParts(string? study, string? series) + [Test] + public void GetOutputPath_Basic() { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", "foo-an.dcm"); + var resolver = new StudySeriesOriginalFilenameProjectPathResolver(_fileSystem); var result = new QueryToExecuteResult( "foo.dcm", - study, - series, + "study", + "series", "sop", - false, - null); - - Assert.That( - new DefaultProjectPathResolver().GetOutputPath(result, _requestMessage), Is.EqualTo(Path.Combine( - study ?? "unknown", - series ?? "unknown", - "foo-an.dcm"))); + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage(); + + // Act + + var actualPath = resolver.GetOutputPath(result, message); + + // Assert + Assert.That(actualPath, Is.EqualTo(expectedPath)); } [TestCase("file-an.dcm", "file.dcm")] [TestCase("file-an.dcm", "file.dicom")] [TestCase("file-an.dcm", "file")] [TestCase("file.foo-an.dcm", "file.foo")] - public void TestDefaultProjectPathResolver_Extensions(string expectedOutput, string inputFile) + public void GetOutputPath_Extensions(string expectedOutput, string inputFile) { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", expectedOutput); + var resolver = new StudySeriesOriginalFilenameProjectPathResolver(_fileSystem); var result = new QueryToExecuteResult( - Path.Combine("foo", inputFile), + inputFile, "study", "series", "sop", - false, - null); - - Assert.That( - new DefaultProjectPathResolver().GetOutputPath(result, _requestMessage), Is.EqualTo(Path.Combine( - "study", - "series", - expectedOutput))); + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage(); + + // Act + + var actualPath = resolver.GetOutputPath(result, message); + + // Assert + Assert.That(actualPath, Is.EqualTo(expectedPath)); } [Test] - public void TestDefaultProjectPathResolver_Both() + public void GetOutputPath_IdentExtraction() { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", "foo.dcm"); + var resolver = new StudySeriesOriginalFilenameProjectPathResolver(_fileSystem); var result = new QueryToExecuteResult( - Path.Combine("foo", "file"), + "foo.dcm", "study", - null, + "series", "sop", - false, - null); - - Assert.That( - new DefaultProjectPathResolver().GetOutputPath(result, _requestMessage), Is.EqualTo(Path.Combine( - "study", - "unknown", - "file-an.dcm"))); - } - - [Test] - public void Test_DefaultProjectPathResolver_IdentExtraction() - { - var requestMessage = new ExtractionRequestMessage + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage() { IsIdentifiableExtraction = true, }; - var result = new QueryToExecuteResult( - Path.Combine("foo", "file"), - "study", - null, - "sop", - false, - null); - - Assert.That( - new DefaultProjectPathResolver().GetOutputPath(result, requestMessage), Is.EqualTo(Path.Combine( - "study", - "unknown", - "file.dcm"))); + // Act + + var actualPath = resolver.GetOutputPath(result, message); + + // Assert + Assert.That(actualPath, Is.EqualTo(expectedPath)); } #endregion From 4739ab47dab952b10746f2a6aa000956dfc60eb5 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 16:27:13 +0100 Subject: [PATCH 10/18] add tests for StudySeriesSOPProjectPathResolver --- .../StudySeriesSOPProjectPathResolverTests.cs | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesSOPProjectPathResolverTests.cs diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesSOPProjectPathResolverTests.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesSOPProjectPathResolverTests.cs new file mode 100644 index 000000000..96dfcbe49 --- /dev/null +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/StudySeriesSOPProjectPathResolverTests.cs @@ -0,0 +1,133 @@ +using NUnit.Framework; +using SmiServices.Common.Messages.Extraction; +using SmiServices.Microservices.CohortExtractor.ProjectPathResolvers; +using SmiServices.Microservices.CohortExtractor.RequestFulfillers; +using SmiServices.UnitTests.Common; +using System.IO.Abstractions; +using System.IO.Abstractions.TestingHelpers; + + +namespace SmiServices.UnitTests.Microservices.CohortExtractor +{ + public class StudySeriesSOPProjectPathResolverTests + { + #region Fixture Methods + + private ExtractionRequestMessage _requestMessage = null!; + private IFileSystem _fileSystem; + + [OneTimeSetUp] + public void OneTimeSetUp() + { + TestLogger.Setup(); + + _requestMessage = new ExtractionRequestMessage + { + IsIdentifiableExtraction = false, + }; + } + + [OneTimeTearDown] + public void OneTimeTearDown() { } + + #endregion + + #region Test Methods + + [SetUp] + public void SetUp() + { + _fileSystem = new MockFileSystem(); + } + + [TearDown] + public void TearDown() { } + + #endregion + + #region Tests + + [Test] + public void GetOutputPath_Basic() + { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", "sop-an.dcm"); + var resolver = new StudySeriesSOPProjectPathResolver(_fileSystem); + var result = new QueryToExecuteResult( + "foo.dcm", + "study", + "series", + "sop", + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage(); + + // Act + + var actualPath = resolver.GetOutputPath(result, message); + + // Assert + Assert.That(actualPath, Is.EqualTo(expectedPath)); + } + + [TestCase("file.dcm")] + [TestCase("file.dicom")] + [TestCase("file")] + [TestCase("file.foo")] + public void GetOutputPath_Extensions(string inputFile) + { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", "sop-an.dcm"); + var resolver = new StudySeriesSOPProjectPathResolver(_fileSystem); + var result = new QueryToExecuteResult( + inputFile, + "study", + "series", + "sop", + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage(); + + // Act + + var actualPath = resolver.GetOutputPath(result, message); + + // Assert + Assert.That(actualPath, Is.EqualTo(expectedPath)); + } + + [Test] + public void GetOutputPath_IdentExtraction() + { + // Arrange + + var expectedPath = _fileSystem.Path.Combine("study", "series", "sop.dcm"); + var resolver = new StudySeriesSOPProjectPathResolver(_fileSystem); + var result = new QueryToExecuteResult( + "foo.dcm", + "study", + "series", + "sop", + rejection: false, + rejectionReason: null + ); + var message = new ExtractionRequestMessage() + { + IsIdentifiableExtraction = true, + }; + + // Act + + var actualPath = resolver.GetOutputPath(result, message); + + // Assert + Assert.That(actualPath, Is.EqualTo(expectedPath)); + } + + #endregion + } +} From 23e71c646a6d251a03b7f0160cfc7a37db7e6674 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 16:32:32 +0100 Subject: [PATCH 11/18] make StudySeriesSOPProjectPathResolver the default in CohortExtractor --- .../Microservices/CohortExtractor/CohortExtractorHost.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs b/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs index 38a1adc2c..b4b1ae248 100644 --- a/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs +++ b/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs @@ -161,7 +161,7 @@ private void InitializeExtractionSources(IRDMPPlatformRepositoryServiceLocator r } _pathResolver = string.IsNullOrWhiteSpace(_consumerOptions.ProjectPathResolverType) - ? new StudySeriesOriginalFilenameProjectPathResolver(_fileSystem) + ? new StudySeriesSOPProjectPathResolver(_fileSystem) : ObjectFactory.CreateInstance( _consumerOptions.ProjectPathResolverType, typeof(IProjectPathResolver).Assembly, repositoryLocator); } From 0851349164cdc19730479940fcd9122b984b9f30 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 4 Oct 2024 16:33:34 +0100 Subject: [PATCH 12/18] add news file --- news/1957-change.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 news/1957-change.md diff --git a/news/1957-change.md b/news/1957-change.md new file mode 100644 index 000000000..baa626be2 --- /dev/null +++ b/news/1957-change.md @@ -0,0 +1,5 @@ +Refactor project path resolvers in CohortExtractor: +- `DefaultProjectPathResolver` is now `StudySeriesOriginalFilenameProjectPathResolver` +- Undo handling UIDs with leading dot (#1506) as this was causing difficulties with path lookups elsewhere +- Add `StudySeriesSOPProjectPathResolver` which generates filenames using SOPInstanceUID instead of the original file name. This is now the default path resolver +- Disallow null Study/Series/SOPInstanceUID values, which should never occur in practice \ No newline at end of file From 6ec6759c74286b3dffd899e41fe3316916398112 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 4 Oct 2024 15:34:44 +0000 Subject: [PATCH 13/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- news/1957-change.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/news/1957-change.md b/news/1957-change.md index baa626be2..7c9ef928b 100644 --- a/news/1957-change.md +++ b/news/1957-change.md @@ -1,5 +1,6 @@ Refactor project path resolvers in CohortExtractor: + - `DefaultProjectPathResolver` is now `StudySeriesOriginalFilenameProjectPathResolver` - Undo handling UIDs with leading dot (#1506) as this was causing difficulties with path lookups elsewhere - Add `StudySeriesSOPProjectPathResolver` which generates filenames using SOPInstanceUID instead of the original file name. This is now the default path resolver -- Disallow null Study/Series/SOPInstanceUID values, which should never occur in practice \ No newline at end of file +- Disallow null Study/Series/SOPInstanceUID values, which should never occur in practice From ed725f2e7442e99154cb6c307a886710a03f03a5 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Sun, 6 Oct 2024 16:11:43 +0100 Subject: [PATCH 14/18] update defaults in configs --- data/microserviceConfigs/default.yaml | 2 +- docs/config.yaml | 2 +- tests/SmiServices.UnitTests/default.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/data/microserviceConfigs/default.yaml b/data/microserviceConfigs/default.yaml index 93806848f..71b653c78 100644 --- a/data/microserviceConfigs/default.yaml +++ b/data/microserviceConfigs/default.yaml @@ -77,7 +77,7 @@ CohortExtractorOptions: AuditorType: "SmiServices.Microservices.CohortExtractor.Audit.NullAuditExtractions" RequestFulfillerType: "SmiServices.Microservices.CohortExtractor.RequestFulfillers.FromCataloguesExtractionRequestFulfiller" - ProjectPathResolverType: "SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.DefaultProjectPathResolver" + ProjectPathResolverType: "SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.StudySeriesSOPProjectPathResolver" ExtractAnonRoutingKey: anon ExtractIdentRoutingKey: ident # Writes (Producer) to this exchange diff --git a/docs/config.yaml b/docs/config.yaml index 4d6f52731..0a0c6aebd 100644 --- a/docs/config.yaml +++ b/docs/config.yaml @@ -77,7 +77,7 @@ CohortExtractorOptions: AuditorType: "Microservices.CohortExtractor.Audit.NullAuditExtractions" RequestFulfillerType: "Microservices.CohortExtractor.Execution.RequestFulfillers.FromCataloguesExtractionRequestFulfiller" - ProjectPathResolverType: "Microservices.CohortExtractor.Execution.ProjectPathResolvers.DefaultProjectPathResolver" + ProjectPathResolverType: "Microservices.CohortExtractor.Execution.ProjectPathResolvers.StudySeriesSOPProjectPathResolver" ExtractAnonRoutingKey: anon ExtractIdentRoutingKey: ident # Writes (Producer) to this exchange diff --git a/tests/SmiServices.UnitTests/default.yaml b/tests/SmiServices.UnitTests/default.yaml index 93806848f..71b653c78 100644 --- a/tests/SmiServices.UnitTests/default.yaml +++ b/tests/SmiServices.UnitTests/default.yaml @@ -77,7 +77,7 @@ CohortExtractorOptions: AuditorType: "SmiServices.Microservices.CohortExtractor.Audit.NullAuditExtractions" RequestFulfillerType: "SmiServices.Microservices.CohortExtractor.RequestFulfillers.FromCataloguesExtractionRequestFulfiller" - ProjectPathResolverType: "SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.DefaultProjectPathResolver" + ProjectPathResolverType: "SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.StudySeriesSOPProjectPathResolver" ExtractAnonRoutingKey: anon ExtractIdentRoutingKey: ident # Writes (Producer) to this exchange From a398a389e98df59e1ce15fdbc1f6637f5dd864c4 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Sun, 6 Oct 2024 16:14:19 +0100 Subject: [PATCH 15/18] delete obsolete tests, as extraction must have all UIDs now --- ...taloguesExtractionRequestFulfillerTests.cs | 79 ------------------- 1 file changed, 79 deletions(-) diff --git a/tests/SmiServices.IntegrationTests/Microservices/CohortExtractor/FromCataloguesExtractionRequestFulfillerTests.cs b/tests/SmiServices.IntegrationTests/Microservices/CohortExtractor/FromCataloguesExtractionRequestFulfillerTests.cs index 5e52d9f4f..714ae948d 100644 --- a/tests/SmiServices.IntegrationTests/Microservices/CohortExtractor/FromCataloguesExtractionRequestFulfillerTests.cs +++ b/tests/SmiServices.IntegrationTests/Microservices/CohortExtractor/FromCataloguesExtractionRequestFulfillerTests.cs @@ -33,85 +33,6 @@ protected override void SetUp() TestLogger.Setup(); } - [TestCase(DatabaseType.MicrosoftSQLServer)] - [TestCase(DatabaseType.MySql)] - public void FromCataloguesExtractionRequestFulfiller_NormalMatching_SeriesInstanceUIDOnly(DatabaseType databaseType) - { - var db = GetCleanedServer(databaseType); - - var dt = new DataTable(); - dt.Columns.Add("SeriesInstanceUID"); - dt.Columns.Add("Extractable", typeof(bool)); - dt.Columns.Add(QueryToExecuteColumnSet.DefaultImagePathColumnName); - - dt.Rows.Add("123", true, "/images/1.dcm"); - dt.Rows.Add("123", false, "/images/2.dcm"); - dt.Rows.Add("1234", false, "/images/3.dcm"); - dt.Rows.Add("1234", true, "/images/4.dcm"); - - dt.SetDoNotReType(true); - - var tbl = db.CreateTable("FromCataloguesExtractionRequestFulfillerTests", dt); - var catalogue = Import(tbl); - - var fulfiller = new FromCataloguesExtractionRequestFulfiller([catalogue]); - - var matching = fulfiller.GetAllMatchingFiles(new ExtractionRequestMessage - { - KeyTag = "SeriesInstanceUID", - ExtractionIdentifiers = new List(["123"]), - }, new NullAuditExtractions()).ToArray(); - - Assert.That(matching, Has.Length.EqualTo(1)); - Assert.Multiple(() => - { - Assert.That(matching[0].Accepted, Has.Count.EqualTo(2)); - Assert.That(matching[0].Accepted.Count(f => f.FilePathValue.Equals("/images/1.dcm")), Is.EqualTo(1)); - Assert.That(matching[0].Accepted.Count(f => f.FilePathValue.Equals("/images/2.dcm")), Is.EqualTo(1)); - }); - } - - [TestCase(DatabaseType.MicrosoftSQLServer)] - [TestCase(DatabaseType.MySql)] - public void FromCataloguesExtractionRequestFulfiller_MandatoryFilter_SeriesInstanceUIDOnly(DatabaseType databaseType) - { - var db = GetCleanedServer(databaseType); - - var dt = new DataTable(); - dt.Columns.Add("SeriesInstanceUID"); - dt.Columns.Add("Extractable", typeof(bool)); - dt.Columns.Add(QueryToExecuteColumnSet.DefaultImagePathColumnName); - - dt.Rows.Add("123", true, "/images/1.dcm"); - dt.Rows.Add("123", false, "/images/2.dcm"); - dt.Rows.Add("1234", false, "/images/3.dcm"); - dt.Rows.Add("1234", true, "/images/4.dcm"); - - dt.SetDoNotReType(true); - - var tbl = db.CreateTable("FromCataloguesExtractionRequestFulfillerTests", dt); - var catalogue = Import(tbl); - - var ei = catalogue.GetAllExtractionInformation(ExtractionCategory.Any).First(); - var filter = new ExtractionFilter(CatalogueRepository, "Extractable only", ei) - { - IsMandatory = true, - WhereSQL = "Extractable = 1" - }; - filter.SaveToDatabase(); - var fulfiller = new FromCataloguesExtractionRequestFulfiller([catalogue]); - - var matching = fulfiller.GetAllMatchingFiles(new ExtractionRequestMessage - { - KeyTag = "SeriesInstanceUID", - ExtractionIdentifiers = new List(["123"]), - }, new NullAuditExtractions()).ToArray(); - - Assert.That(matching, Has.Length.EqualTo(1)); - Assert.That(matching[0].Accepted, Has.Count.EqualTo(1)); - Assert.That(matching[0].Accepted.Count(f => f.FilePathValue.Equals("/images/1.dcm")), Is.EqualTo(1)); - } - [TestCase(DatabaseType.MicrosoftSQLServer)] [TestCase(DatabaseType.MySql)] From 3bdb69c13241f1099cdafbea68d0daa45eb8c3ce Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Mon, 7 Oct 2024 11:42:16 +0100 Subject: [PATCH 16/18] set MessageHeader.CurrentProgramName at test fixture level --- .../Messaging/ExtractionRequestQueueConsumerTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs index 23084e10a..2c44c37a4 100644 --- a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/Messaging/ExtractionRequestQueueConsumerTest.cs @@ -28,7 +28,6 @@ public class ExtractionRequestQueueConsumerTest public void OneTimeSetUp() { TestLogger.Setup(); - MessageHeader.CurrentProgramName = nameof(ExtractionRequestQueueConsumerTest); } [OneTimeTearDown] From 18cc004a388a13ba2a21e98eb6afc39bae4f91d6 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Mon, 7 Oct 2024 14:25:21 +0100 Subject: [PATCH 17/18] remove opaque ObjectFactory nonsense in favour of ProjectPathResolverFactory --- data/microserviceConfigs/default.yaml | 2 +- docs/config.yaml | 2 +- .../CohortExtractor/CohortExtractorHost.cs | 3 +-- .../ProjectPathResolverFactory.cs | 17 +++++++++++++++++ .../NoSuffixProjectPathResolverTests.cs | 8 -------- tests/SmiServices.UnitTests/default.yaml | 2 +- 6 files changed, 21 insertions(+), 13 deletions(-) create mode 100644 src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs diff --git a/data/microserviceConfigs/default.yaml b/data/microserviceConfigs/default.yaml index 71b653c78..78e8417c1 100644 --- a/data/microserviceConfigs/default.yaml +++ b/data/microserviceConfigs/default.yaml @@ -77,7 +77,7 @@ CohortExtractorOptions: AuditorType: "SmiServices.Microservices.CohortExtractor.Audit.NullAuditExtractions" RequestFulfillerType: "SmiServices.Microservices.CohortExtractor.RequestFulfillers.FromCataloguesExtractionRequestFulfiller" - ProjectPathResolverType: "SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.StudySeriesSOPProjectPathResolver" + ProjectPathResolverType: "StudySeriesSOPProjectPathResolver" ExtractAnonRoutingKey: anon ExtractIdentRoutingKey: ident # Writes (Producer) to this exchange diff --git a/docs/config.yaml b/docs/config.yaml index 0a0c6aebd..d1712412c 100644 --- a/docs/config.yaml +++ b/docs/config.yaml @@ -77,7 +77,7 @@ CohortExtractorOptions: AuditorType: "Microservices.CohortExtractor.Audit.NullAuditExtractions" RequestFulfillerType: "Microservices.CohortExtractor.Execution.RequestFulfillers.FromCataloguesExtractionRequestFulfiller" - ProjectPathResolverType: "Microservices.CohortExtractor.Execution.ProjectPathResolvers.StudySeriesSOPProjectPathResolver" + ProjectPathResolverType: "StudySeriesSOPProjectPathResolver" ExtractAnonRoutingKey: anon ExtractIdentRoutingKey: ident # Writes (Producer) to this exchange diff --git a/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs b/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs index b4b1ae248..38b53cf39 100644 --- a/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs +++ b/src/SmiServices/Microservices/CohortExtractor/CohortExtractorHost.cs @@ -162,8 +162,7 @@ private void InitializeExtractionSources(IRDMPPlatformRepositoryServiceLocator r _pathResolver = string.IsNullOrWhiteSpace(_consumerOptions.ProjectPathResolverType) ? new StudySeriesSOPProjectPathResolver(_fileSystem) - : ObjectFactory.CreateInstance( - _consumerOptions.ProjectPathResolverType, typeof(IProjectPathResolver).Assembly, repositoryLocator); + : ProjectPathResolverFactory.Create(_consumerOptions.ProjectPathResolverType, _fileSystem); } } } diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs new file mode 100644 index 000000000..77510bd33 --- /dev/null +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs @@ -0,0 +1,17 @@ +using System; +using System.IO.Abstractions; + +namespace SmiServices.Microservices.CohortExtractor.ProjectPathResolvers; +internal class ProjectPathResolverFactory +{ + public static IProjectPathResolver Create(string projectPathResolverType, IFileSystem fileSystem) + { + return projectPathResolverType switch + { + "StudySeriesSOPProjectPathResolver" => new StudySeriesSOPProjectPathResolver(fileSystem), + "NoSuffixProjectPathResolver" => new NoSuffixProjectPathResolver(fileSystem), + "StudySeriesOriginalFilenameProjectPathResolver" => new StudySeriesOriginalFilenameProjectPathResolver(fileSystem), + _ => throw new NotImplementedException($"No case for IProjectPathResolver type '{projectPathResolverType}'"), + }; + } +} diff --git a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs index 3e2d57446..272f2f65f 100644 --- a/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs +++ b/tests/SmiServices.UnitTests/Microservices/CohortExtractor/NoSuffixProjectPathResolverTests.cs @@ -1,5 +1,4 @@ using NUnit.Framework; -using SmiServices.Common.Helpers; using SmiServices.Common.Messages.Extraction; using SmiServices.Microservices.CohortExtractor.ProjectPathResolvers; using SmiServices.Microservices.CohortExtractor.RequestFulfillers; @@ -73,12 +72,5 @@ public void GetOutputPath_Extensions(string expectedOutput, string inputFile) Assert.That(actualPath, Is.EqualTo(expectedPath)); } - - [Test] - public void CanBeConstructedByReflection() - { - var instance = new MicroserviceObjectFactory().CreateInstance("SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.NoSuffixProjectPathResolver", typeof(IProjectPathResolver).Assembly, _fileSystem); - Assert.That(instance, Is.InstanceOf()); - } } } diff --git a/tests/SmiServices.UnitTests/default.yaml b/tests/SmiServices.UnitTests/default.yaml index 71b653c78..78e8417c1 100644 --- a/tests/SmiServices.UnitTests/default.yaml +++ b/tests/SmiServices.UnitTests/default.yaml @@ -77,7 +77,7 @@ CohortExtractorOptions: AuditorType: "SmiServices.Microservices.CohortExtractor.Audit.NullAuditExtractions" RequestFulfillerType: "SmiServices.Microservices.CohortExtractor.RequestFulfillers.FromCataloguesExtractionRequestFulfiller" - ProjectPathResolverType: "SmiServices.Microservices.CohortExtractor.ProjectPathResolvers.StudySeriesSOPProjectPathResolver" + ProjectPathResolverType: "StudySeriesSOPProjectPathResolver" ExtractAnonRoutingKey: anon ExtractIdentRoutingKey: ident # Writes (Producer) to this exchange From 7f6d1ae863acf706200eb463a9324349b4497899 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Mon, 7 Oct 2024 14:46:13 +0100 Subject: [PATCH 18/18] use nameof instead of strings --- .../ProjectPathResolvers/ProjectPathResolverFactory.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs index 77510bd33..f8633cbda 100644 --- a/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs +++ b/src/SmiServices/Microservices/CohortExtractor/ProjectPathResolvers/ProjectPathResolverFactory.cs @@ -8,9 +8,9 @@ public static IProjectPathResolver Create(string projectPathResolverType, IFileS { return projectPathResolverType switch { - "StudySeriesSOPProjectPathResolver" => new StudySeriesSOPProjectPathResolver(fileSystem), - "NoSuffixProjectPathResolver" => new NoSuffixProjectPathResolver(fileSystem), - "StudySeriesOriginalFilenameProjectPathResolver" => new StudySeriesOriginalFilenameProjectPathResolver(fileSystem), + nameof(StudySeriesSOPProjectPathResolver) => new StudySeriesSOPProjectPathResolver(fileSystem), + nameof(NoSuffixProjectPathResolver) => new NoSuffixProjectPathResolver(fileSystem), + nameof(StudySeriesOriginalFilenameProjectPathResolver) => new StudySeriesOriginalFilenameProjectPathResolver(fileSystem), _ => throw new NotImplementedException($"No case for IProjectPathResolver type '{projectPathResolverType}'"), }; }