Initialize Delimiter from Attributes (#635)

* Initialize Delimiter from Attributes

If a DelimitedListTraceListener has a Delimiter attribute, use it
to initialize the Delimiter unless (and until) a delimiter is set
explicitly.

Also add unit tests for these scenarios.

Fix dotnet/corefx#39363

* Remove var usage

Co-Authored-By: Stephen Toub <stoub@microsoft.com>
* Remove unnecessary initialization

Co-authored-by: Stephen Toub <stoub@microsoft.com>

diff --git a/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System.Diagnostics.TextWriterTraceListener.csproj b/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System.Diagnostics.TextWriterTraceListener.csproj
index 17bc24b..5747595 100644 (file)
--- a/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System.Diagnostics.TextWriterTraceListener.csproj
+++ b/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System.Diagnostics.TextWriterTraceListener.csproj
@@ -42,6 +42,7 @@
     <Reference Include="System.ComponentModel.Primitives" />
     <Reference Include="System.Console" />
     <Reference Include="System.Collections.NonGeneric" />
+    <Reference Include="System.Collections.Specialized" />
     <Reference Include="System.Diagnostics.Process" />
     <Reference Include="System.Diagnostics.TraceSource" />
     <Reference Include="System.Runtime" />

diff --git a/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System/Diagnostics/DelimitedListTraceListener.cs b/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System/Diagnostics/DelimitedListTraceListener.cs
index 5161904..f62a962 100644 (file)
--- a/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System/Diagnostics/DelimitedListTraceListener.cs
+++ b/src/libraries/System.Diagnostics.TextWriterTraceListener/src/System/Diagnostics/DelimitedListTraceListener.cs
@@ -14,6 +14,7 @@ namespace System.Diagnostics
     {
         private string _delimiter = ";";
         private string _secondaryDelim = ",";
+        private bool _initializedDelim;
 
         public DelimitedListTraceListener(Stream stream) : base(stream)
         {
@@ -43,6 +44,17 @@ namespace System.Diagnostics
         {
             get
             {
+                lock (this)
+                {
+                    if (!_initializedDelim)
+                    {
+                        if (Attributes.ContainsKey("delimiter"))
+                        {
+                            _delimiter = Attributes["delimiter"];
+                        }
+                        _initializedDelim = true;
+                    }
+                }
                 return _delimiter;
             }
             set
@@ -56,6 +68,7 @@ namespace System.Diagnostics
                 lock (this)
                 {
                     _delimiter = value;
+                    _initializedDelim = true;
                 }
 
                 if (_delimiter == ",")

diff --git a/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/CommonUtilities.cs b/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/CommonUtilities.cs
index e220f73..5ca6f5e 100644 (file)
--- a/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/CommonUtilities.cs
+++ b/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/CommonUtilities.cs
@@ -11,7 +11,7 @@ namespace System.Diagnostics.TextWriterTraceListenerTests
 {
     internal static class CommonUtilities
     {
-        private const string DefaultDelimiter = ";";
+        internal const string DefaultDelimiter = ";";
 
         internal static void DeleteFile(string fileName)
         {

diff --git a/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/DelimiterWriteMethodTests.cs b/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/DelimiterWriteMethodTests.cs
index 77151a0..7e34c76 100644 (file)
--- a/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/DelimiterWriteMethodTests.cs
+++ b/src/libraries/System.Diagnostics.TextWriterTraceListener/tests/DelimiterWriteMethodTests.cs
@@ -182,6 +182,76 @@ namespace System.Diagnostics.TextWriterTraceListenerTests
             Assert.Equal(expected, File.Exists(_fileName) ? File.ReadAllText(_fileName) : "");
         }
 
+        [Theory]
+        [MemberData(nameof(TraceDataObjectArrayInvariants))]
+        public void AttributeDelimiter_ChangesDelimiter_Test(string delimiter, TraceFilter filter, TraceEventCache eventCache, string source, TraceEventType eventType, int id, object[] data)
+        {
+            string newDelimiter = "||";
+            using (var target = GetListener())
+            {
+                target.Filter = filter;
+                target.TraceOutputOptions = TraceOptions.ProcessId | TraceOptions.ThreadId | TraceOptions.DateTime | TraceOptions.Timestamp | TraceOptions.LogicalOperationStack | TraceOptions.Callstack;
+                target.Attributes.Add("delimiter", newDelimiter);
+                target.TraceData(eventCache, source, eventType, id, data);
+                Assert.Equal(newDelimiter, target.Delimiter);
+
+                target.Delimiter = delimiter;
+                Assert.Equal(delimiter, target.Delimiter);
+            }
+
+            string expected = CommonUtilities.ExpectedTraceDataOutput(newDelimiter, filter, eventCache, source, eventType, id, data);
+            Assert.Equal(expected, File.Exists(_fileName) ? File.ReadAllText(_fileName) : "");
+        }
+
+        [Theory]
+        [MemberData(nameof(TraceDataObjectArrayInvariants))]
+        public void AttributeDelimiter_IgnoredAfterDelimiterSet_Test(string delimiter, TraceFilter filter, TraceEventCache eventCache, string source, TraceEventType eventType, int id, object[] data)
+        {
+            string newDelimiter = "||";
+            using (var target = GetListener())
+            {
+                target.Filter = filter;
+                target.TraceOutputOptions = TraceOptions.ProcessId | TraceOptions.ThreadId | TraceOptions.DateTime | TraceOptions.Timestamp | TraceOptions.LogicalOperationStack | TraceOptions.Callstack;
+
+                // Setting target.Delimiter here should initialize the delimiter
+                // so that future delimiter attributes aren't used.
+                target.Delimiter = delimiter;
+
+                target.Attributes.Add("delimiter", newDelimiter);
+                target.TraceData(eventCache, source, eventType, id, data);
+                Assert.Equal(delimiter, target.Delimiter);
+            }
+
+            string expected = CommonUtilities.ExpectedTraceDataOutput(delimiter, filter, eventCache, source, eventType, id, data);
+            Assert.Equal(expected, File.Exists(_fileName) ? File.ReadAllText(_fileName) : "");
+        }
+
+        [Theory]
+        [MemberData(nameof(TraceDataObjectArrayInvariants))]
+        public void AttributeDelimiter_IgnoredAfterDelimiterRead_Test(string delimiter, TraceFilter filter, TraceEventCache eventCache, string source, TraceEventType eventType, int id, object[] data)
+        {
+            string newDelimiter = "||";
+            using (var target = GetListener())
+            {
+                target.Filter = filter;
+                target.TraceOutputOptions = TraceOptions.ProcessId | TraceOptions.ThreadId | TraceOptions.DateTime | TraceOptions.Timestamp | TraceOptions.LogicalOperationStack | TraceOptions.Callstack;
+
+                // Getting target.Delimiter here should initialize the delimiter
+                // so that future delimiter attributes aren't used.
+                Assert.Equal(CommonUtilities.DefaultDelimiter, target.Delimiter);
+
+                target.Attributes.Add("delimiter", newDelimiter);
+                target.TraceData(eventCache, source, eventType, id, data);
+
+                // Expclitly setting the delimiter property will update it, though.
+                target.Delimiter = delimiter;
+                Assert.Equal(delimiter, target.Delimiter);
+            }
+
+            string expected = CommonUtilities.ExpectedTraceDataOutput(CommonUtilities.DefaultDelimiter, filter, eventCache, source, eventType, id, data);
+            Assert.Equal(expected, File.Exists(_fileName) ? File.ReadAllText(_fileName) : "");
+        }
+
         protected override void Dispose(bool disposing)
         {
             base.Dispose(disposing);