From 26650420d651f967bded7aa89d09426d2e555fac Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Tue, 16 Apr 2024 22:33:41 +0100 Subject: [PATCH 1/6] wip: add test for CohortExtractorHost constructor --- .../Execution/CohortExtractorHostTests.cs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs diff --git a/tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs b/tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs new file mode 100644 index 000000000..fc0078926 --- /dev/null +++ b/tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs @@ -0,0 +1,60 @@ +using NUnit.Framework; +using Smi.Common.Tests; +using Smi.Common.Options; +using Microservices.CohortExtractor.Execution; +using FAnsi; + +namespace Microservices.CohortExtractor.Tests.Execution; + +[RequiresRabbit, RequiresRelationalDb(DatabaseType.MySql)] +internal class CohortExtractorHostTests +{ + #region Fixture Methods + + [OneTimeSetUp] + public void OneTimeSetUp() + { + TestLogger.Setup(); + } + + [OneTimeTearDown] + public void OneTimeTearDown() { } + + #endregion + + #region Test Methods + + [SetUp] + public void SetUp() { } + + [TearDown] + public void TearDown() { } + + #endregion + + #region Tests + + [Test] + public void Constructor_HappyPath() + { + // Arrange + + var globals = new GlobalOptionsFactory().Load(nameof(Constructor_HappyPath)); + + using var tester = new MicroserviceTester( + globals.RabbitOptions!, + globals.CohortPackagerOptions!.FileCollectionInfoOptions! + ); + tester.CreateExchange("TEST.ExtractFileExchange"); + + var host = new CohortExtractorHost(globals, null, null); + + // Act + + host.Start(); + + // TODO + } + + #endregion +} From 7dc7af4e433e2d71613a27b24c42c934e9853cd7 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 19 Apr 2024 14:44:13 +0100 Subject: [PATCH 2/6] Revert "wip: add test for CohortExtractorHost constructor" This reverts commit 26650420d651f967bded7aa89d09426d2e555fac. --- .../Execution/CohortExtractorHostTests.cs | 60 ------------------- 1 file changed, 60 deletions(-) delete mode 100644 tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs diff --git a/tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs b/tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs deleted file mode 100644 index fc0078926..000000000 --- a/tests/microservices/Microservices.CohortExtractor.Tests/Execution/CohortExtractorHostTests.cs +++ /dev/null @@ -1,60 +0,0 @@ -using NUnit.Framework; -using Smi.Common.Tests; -using Smi.Common.Options; -using Microservices.CohortExtractor.Execution; -using FAnsi; - -namespace Microservices.CohortExtractor.Tests.Execution; - -[RequiresRabbit, RequiresRelationalDb(DatabaseType.MySql)] -internal class CohortExtractorHostTests -{ - #region Fixture Methods - - [OneTimeSetUp] - public void OneTimeSetUp() - { - TestLogger.Setup(); - } - - [OneTimeTearDown] - public void OneTimeTearDown() { } - - #endregion - - #region Test Methods - - [SetUp] - public void SetUp() { } - - [TearDown] - public void TearDown() { } - - #endregion - - #region Tests - - [Test] - public void Constructor_HappyPath() - { - // Arrange - - var globals = new GlobalOptionsFactory().Load(nameof(Constructor_HappyPath)); - - using var tester = new MicroserviceTester( - globals.RabbitOptions!, - globals.CohortPackagerOptions!.FileCollectionInfoOptions! - ); - tester.CreateExchange("TEST.ExtractFileExchange"); - - var host = new CohortExtractorHost(globals, null, null); - - // Act - - host.Start(); - - // TODO - } - - #endregion -} From b7ad22d4bc34bd2543b84f43108b1aaadaf0da93 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 19 Apr 2024 14:46:17 +0100 Subject: [PATCH 3/6] fix DynamicRejector constructor to work with reflection --- .../Dynamic/DynamicRejector.cs | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs b/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs index faa7e2f10..0ce18ae7c 100644 --- a/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs +++ b/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs @@ -12,13 +12,25 @@ public class DynamicRejector : IRejector private readonly Script _script; private const string DefaultDynamicRulesPath = "./DynamicRules.txt"; - public DynamicRejector(string? dynamicRulesPath, IFileSystem? fileSystem = null) + public DynamicRejector() { - if (dynamicRulesPath == null) - dynamicRulesPath = DefaultDynamicRulesPath; + // TODO(rkm 2024-04-19) Fix/rework MicroserviceObjectFactory and CohortExtractor to allow overriding these + var dynamicRulesPath = DefaultDynamicRulesPath; + var fileSystem = new FileSystem(); + + _script = LoadScript(dynamicRulesPath, fileSystem); + } + public DynamicRejector(string? dynamicRulesPath, IFileSystem? fileSystem = null) + { + dynamicRulesPath ??= DefaultDynamicRulesPath; fileSystem ??= new FileSystem(); + _script = LoadScript(dynamicRulesPath, fileSystem); + } + + private static Script LoadScript(string dynamicRulesPath, IFileSystem fileSystem) + { if (!fileSystem.File.Exists(dynamicRulesPath)) throw new System.IO.FileNotFoundException($"Could not find rules file '{dynamicRulesPath}'"); @@ -27,7 +39,7 @@ public DynamicRejector(string? dynamicRulesPath, IFileSystem? fileSystem = null) if (string.IsNullOrWhiteSpace(dynamicRules)) throw new ArgumentOutOfRangeException("Rules file is empty"); - _script = CSharpScript.Create( + return CSharpScript.Create( dynamicRules, ScriptOptions.Default.WithReferences(typeof(Convert).Assembly).WithWarningLevel(0), typeof(Payload) From cd34974f94f96e98ab63bbd068c7dee021ed39fb Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 19 Apr 2024 14:51:27 +0100 Subject: [PATCH 4/6] add test to ensure no-args constructor exists --- .../RequestFulfillers/Dynamic/DynamicRejectorTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/microservices/Microservices.CohortExtractor.Tests/Execution/RequestFulfillers/Dynamic/DynamicRejectorTests.cs b/tests/microservices/Microservices.CohortExtractor.Tests/Execution/RequestFulfillers/Dynamic/DynamicRejectorTests.cs index 4e5491887..26a2c1bd5 100644 --- a/tests/microservices/Microservices.CohortExtractor.Tests/Execution/RequestFulfillers/Dynamic/DynamicRejectorTests.cs +++ b/tests/microservices/Microservices.CohortExtractor.Tests/Execution/RequestFulfillers/Dynamic/DynamicRejectorTests.cs @@ -40,5 +40,11 @@ public void Constructor_MissingRulesFile_Throws() Assert.Throws(() => new DynamicRejector("foo.txt", new MockFileSystem())); } + [Test] + public void Constructor_ExistsWithNoArgs() + { + _ = new DynamicRejector(); + } + #endregion } From 54c19a3059dcb414f2202b15298990bcd1a2d9f0 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 19 Apr 2024 14:54:16 +0100 Subject: [PATCH 5/6] add news file --- news/1799-bugfix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1799-bugfix.md diff --git a/news/1799-bugfix.md b/news/1799-bugfix.md new file mode 100644 index 000000000..c773b31a9 --- /dev/null +++ b/news/1799-bugfix.md @@ -0,0 +1 @@ +Re-add a parameterless constructor to DynamicRejector so it can be loaded via reflection in CohortExtractorHost. From bfe2452d9ff4bcf65e90d806615be79de4bc718a Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Fri, 19 Apr 2024 15:25:00 +0100 Subject: [PATCH 6/6] simplify constructor call --- .../RequestFulfillers/Dynamic/DynamicRejector.cs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs b/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs index 0ce18ae7c..0cde6d169 100644 --- a/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs +++ b/src/microservices/Microservices.CohortExtractor/Execution/RequestFulfillers/Dynamic/DynamicRejector.cs @@ -13,24 +13,13 @@ public class DynamicRejector : IRejector private const string DefaultDynamicRulesPath = "./DynamicRules.txt"; public DynamicRejector() - { - // TODO(rkm 2024-04-19) Fix/rework MicroserviceObjectFactory and CohortExtractor to allow overriding these - var dynamicRulesPath = DefaultDynamicRulesPath; - var fileSystem = new FileSystem(); - - _script = LoadScript(dynamicRulesPath, fileSystem); - } + : this(null) {} public DynamicRejector(string? dynamicRulesPath, IFileSystem? fileSystem = null) { dynamicRulesPath ??= DefaultDynamicRulesPath; fileSystem ??= new FileSystem(); - _script = LoadScript(dynamicRulesPath, fileSystem); - } - - private static Script LoadScript(string dynamicRulesPath, IFileSystem fileSystem) - { if (!fileSystem.File.Exists(dynamicRulesPath)) throw new System.IO.FileNotFoundException($"Could not find rules file '{dynamicRulesPath}'"); @@ -39,7 +28,7 @@ private static Script LoadScript(string dynamicRulesPath, IFileSystem fi if (string.IsNullOrWhiteSpace(dynamicRules)) throw new ArgumentOutOfRangeException("Rules file is empty"); - return CSharpScript.Create( + _script = CSharpScript.Create( dynamicRules, ScriptOptions.Default.WithReferences(typeof(Convert).Assembly).WithWarningLevel(0), typeof(Payload)