Fixup newline parsing in CohortPackager

CHANGELOG.md

@@ -6,10 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## [Unreleased]
 
--
+### Fixed
+
+-   [#581](https://github.com/SMI/SmiServices/pull/581) Fixed a bug where newlines would never be correctly parsed from the config option in CohortPackager
 
 ## [1.14.1] - 2021-02-04
 
+### Fixed
+
 -   [#576](https://github.com/SMI/SmiServices/pull/576) Fixup Windows package build
 
 ## [1.14.0] - 2021-02-04
@@ -34,7 +38,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Fixed
 
--   Fixed a bug where newlines would never be correctly parsed from the config option in CohortPackager
 -   CohortPackager: Don't try and create the jobId file when recreating an existing report
 -   CohortPackager.Tests: Fix a flaky test caused by NUnit setup/teardown code when running tests in parallel
 -   CohortPackager.Tests: Fix a flaky test caused by using the same MongoDB database name when running tests in parallel

src/common/Smi.Common/Options/GlobalOptions.cs

@@ -312,6 +312,11 @@ public class CohortPackagerOptions : IOptions
         public string ReporterType { get; set; }
         public string NotifierType { get; set; }
         public string ReportFormat { get; set; }
+
+        /// <summary>
+        /// The newline to use when writing extraction report files. Note that a "\r\n" string
+        /// in the YAML config will bee automatically escaped to "\\r\\n" in this string.
+        /// </summary>
         public string ReportNewLine { get; set; }
 
         public override string ToString()

src/microservices/Microservices.CohortPackager/Execution/JobProcessing/Reporting/JobReporterBase.cs

@@ -50,12 +50,12 @@ [CanBeNull] string reportNewLine
             else
             {
                 Logger.Warn($"Not passed a specific newline string for creating reports. Defaulting to Environment.NewLine ('{Regex.Escape(Environment.NewLine)}')");
-                ReportNewLine = Environment.NewLine;
+                ReportNewLine = Regex.Escape(Environment.NewLine);
             }
 
             _csvConfiguration = new CsvConfiguration(CultureInfo.InvariantCulture)
             {
-                NewLine = ParseToCsvNewLine(Regex.Escape(ReportNewLine)),
+                NewLine = ReportNewLine,
             };
         }
 
@@ -459,19 +459,6 @@ private void WriteVerificationValues(IEnumerable<KeyValuePair<string, List<strin
             sb.Append(ReportNewLine);
         }
 
-        // NOTE(rkm 2020-12-10) The NewLine class only exists in the CsvHelper lib, so can't really use throughout the sln. As far as I
-        // can tell, this is the most straightforward way to parse a "NewLine" from an input string. The input string must already be escaped.
-        // NOTE(jas 2021-01-18) CsvHelper has changed to a strange hack. Any single char = use that for newline; null = use \r\n instead.
-        // NOTE(jas 2021-01-19) CsvHelper has switched to using plain strings. Much more sensible. Above notes now historical.
-        private static string ParseToCsvNewLine(string newLine) =>
-            newLine switch
-            {
-                @"\r" => "\r",
-                @"\r\n" => "\r\n",
-                @"\n" => "\n",
-                _ => throw new ArgumentException($"No case for '{Regex.Escape(newLine)}'")
-            };
-
         protected abstract void ReleaseUnmanagedResources();
         public abstract void Dispose();
         ~JobReporterBase() => ReleaseUnmanagedResources();

tests/microservices/Microservices.CohortPackager.Tests/Execution/JobProcessing/Reporting/JobReporterBaseTest.cs

@@ -7,6 +7,8 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using System.Text.RegularExpressions;
+using Smi.Common.Options;
 
 
 namespace Microservices.CohortPackager.Tests.Execution.JobProcessing.Reporting
@@ -15,8 +17,8 @@ namespace Microservices.CohortPackager.Tests.Execution.JobProcessing.Reporting
     [TestFixture]
     public class JobReporterBaseTest
     {
-        private const string WindowsNewLine = "\r\n";
-        private const string LinuxNewLine = "\n";
+        private const string WindowsNewLine = @"\r\n";
+        private const string LinuxNewLine = @"\n";
 
         private static readonly TestDateTimeProvider _dateTimeProvider = new TestDateTimeProvider();
 
@@ -754,7 +756,22 @@ public void Constructor_NoNewLine_SetToEnvironment()
         {
             var mockJobStore = new Mock<IExtractJobStore>(MockBehavior.Strict);
             var reporter = new TestJobReporter(mockJobStore.Object, ReportFormat.Split, null);
-            Assert.AreEqual(Environment.NewLine, reporter.ReportNewLine);
+            Assert.AreEqual(Regex.Escape(Environment.NewLine), reporter.ReportNewLine);
+        }
+
+        [Test]
+        public void ReportNewLine_LoadFromYaml_EscapesNewlines()
+        {
+            string yaml = @"
+CurrentDirectory:
+CohortPackagerOptions:
+    ReportNewLine: '\r\n'
+";
+            string tmpConfig = Path.GetTempFileName() + ".yaml";
+            File.WriteAllText(tmpConfig, yaml);
+            GlobalOptions globals = new GlobalOptionsFactory().Load(tmpConfig);
+
+            Assert.AreEqual(WindowsNewLine, globals.CohortPackagerOptions.ReportNewLine);
         }
     }