Fixes for PR review

serilog-mssql · Nov 18, 2023 · 8b5424a · 8b5424a
1 parent 62ca911
commit 8b5424a
Show file tree

Hide file tree

Showing 10 changed files with 22 additions and 24 deletions.
diff --git a/src/Serilog.Sinks.MSSqlServer/Extensions/StringExtensions.cs b/src/Serilog.Sinks.MSSqlServer/Extensions/StringExtensions.cs
@@ -4,6 +4,11 @@ namespace Serilog.Sinks.MSSqlServer.Extensions
 {
     internal static class StringExtensions
     {
+        public static string TruncateOutput(this string value, int dataLength) =>
+            dataLength < 0
+                ? value     // No need to truncate if length set to maximum
+                : value.Truncate(dataLength, "...");
+
         public static string Truncate(this string value, int maxLength, string suffix)
         {
             if (value == null) return null;

diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/ExceptionColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/ExceptionColumnOptions.cs
@@ -27,7 +27,7 @@ public ExceptionColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (!SqlDataTypes.VariableCharactorColumnTypes.Contains(value))
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
                         throw new ArgumentException("The Standard Column \"Exception\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }

diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LevelColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LevelColumnOptions.cs
@@ -28,7 +28,7 @@ public LevelColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (!SqlDataTypes.VariableCharactorColumnTypes.Contains(value) && value != SqlDbType.TinyInt)
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value) && value != SqlDbType.TinyInt)
                         throw new ArgumentException("The Standard Column \"Level\" must be of data type NVarChar, VarChar or TinyInt.");
                     base.DataType = value;
                 }

diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LogEventColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LogEventColumnOptions.cs
@@ -27,7 +27,7 @@ public LogEventColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (!SqlDataTypes.VariableCharactorColumnTypes.Contains(value))
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
                         throw new ArgumentException("The Standard Column \"LogEvent\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }

diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageColumnOptions.cs
@@ -27,7 +27,7 @@ public MessageColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (!SqlDataTypes.VariableCharactorColumnTypes.Contains(value))
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
                         throw new ArgumentException("The Standard Column \"Message\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }

diff --git a/...Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageTemplateColumnOptions.cs b/...Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageTemplateColumnOptions.cs
@@ -27,7 +27,7 @@ public MessageTemplateColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (!SqlDataTypes.VariableCharactorColumnTypes.Contains(value))
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
                         throw new ArgumentException("The Standard Column \"MessageTemplate\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }

diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/AdditionalColumnDataGenerator.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/AdditionalColumnDataGenerator.cs
@@ -48,7 +48,7 @@ public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn ad
             {
                 if (SqlDataTypes.DataLengthRequired.Contains(additionalColumn.DataType))
                 {
-                    return new KeyValuePair<string, object>(columnName, TruncateOutput((string)scalarValue.Value, additionalColumn.DataLength));
+                    return new KeyValuePair<string, object>(columnName, scalarValue.Value.ToString().TruncateOutput(additionalColumn.DataLength));
 
                 }
                 return new KeyValuePair<string, object>(columnName, scalarValue.Value);
@@ -60,15 +60,13 @@ public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn ad
             }
             else
             {
-                return new KeyValuePair<string, object>(columnName, DBNull.Value);
+                if (additionalColumn.AllowNull) {
+                    return new KeyValuePair<string, object>(columnName, DBNull.Value);
+                }
+                return new KeyValuePair<string, object>(columnName, property.Value.ToString());
             }
         }
 
-        private static string TruncateOutput(string value, int dataLength) =>
-            dataLength < 0
-                ? value     // No need to truncate if length set to maximum
-                : value.Truncate(dataLength, string.Empty);
-
         private static bool TryChangeType(object obj, Type type, out object conversion)
         {
             conversion = null;

diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/StandardColumnDataGenerator.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/StandardColumnDataGenerator.cs
@@ -45,15 +45,15 @@ public KeyValuePair<string, object> GetStandardColumnNameAndValue(StandardColumn
             switch (column)
             {
                 case StandardColumn.Message:
-                    return new KeyValuePair<string, object>(_columnOptions.Message.ColumnName, TruncateOutput(logEvent.RenderMessage(_formatProvider), _columnOptions.Message.DataLength));
+                    return new KeyValuePair<string, object>(_columnOptions.Message.ColumnName, logEvent.RenderMessage(_formatProvider).TruncateOutput(_columnOptions.Message.DataLength));
                 case StandardColumn.MessageTemplate:
-                    return new KeyValuePair<string, object>(_columnOptions.MessageTemplate.ColumnName, TruncateOutput(logEvent.MessageTemplate.Text, _columnOptions.MessageTemplate.DataLength));
+                    return new KeyValuePair<string, object>(_columnOptions.MessageTemplate.ColumnName, logEvent.MessageTemplate.Text.TruncateOutput(_columnOptions.MessageTemplate.DataLength));
                 case StandardColumn.Level:
                     return new KeyValuePair<string, object>(_columnOptions.Level.ColumnName, _columnOptions.Level.StoreAsEnum ? (object)logEvent.Level : logEvent.Level.ToString());
                 case StandardColumn.TimeStamp:
                     return GetTimeStampStandardColumnNameAndValue(logEvent);
                 case StandardColumn.Exception:
-                    return new KeyValuePair<string, object>(_columnOptions.Exception.ColumnName, TruncateOutput(logEvent.Exception?.ToString(), _columnOptions.Exception.DataLength));
+                    return new KeyValuePair<string, object>(_columnOptions.Exception.ColumnName, logEvent.Exception?.ToString().TruncateOutput(_columnOptions.Exception.DataLength));
                 case StandardColumn.Properties:
                     return new KeyValuePair<string, object>(_columnOptions.Properties.ColumnName, ConvertPropertiesToXmlStructure(logEvent.Properties));
                 case StandardColumn.LogEvent:
@@ -63,11 +63,6 @@ public KeyValuePair<string, object> GetStandardColumnNameAndValue(StandardColumn
             }
         }
 
-        private static string TruncateOutput(string value, int dataLength) =>
-            dataLength < 0
-                ? value     // No need to truncate if length set to maximum
-                : value.Truncate(dataLength, "...");
-
         private KeyValuePair<string, object> GetTimeStampStandardColumnNameAndValue(LogEvent logEvent)
         {
             var dateTimeOffset = _columnOptions.TimeStamp.ConvertToUtc ? logEvent.Timestamp.ToUniversalTime() : logEvent.Timestamp;

diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlDataTypes.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlDataTypes.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Data;
 
 namespace Serilog.Sinks.MSSqlServer
@@ -54,11 +55,10 @@ public static class SqlDataTypes
         /// <summary>
         /// SQL column types for supported strings
         /// </summary>
-        public static readonly HashSet<SqlDbType> VariableCharactorColumnTypes = new HashSet<SqlDbType>
-        {
+        public static readonly ReadOnlyCollection<SqlDbType> VariableCharacterColumnTypes = new(new[] {
             SqlDbType.NVarChar,
             SqlDbType.VarChar
-        };
+        });
 
         /// <summary>
         /// The SQL column types which require a non-zero DataLength property.

diff --git a/...og.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Output/AdditionalColumnDataGeneratorTests.cs b/...og.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Output/AdditionalColumnDataGeneratorTests.cs
@@ -200,7 +200,7 @@ public void GetAdditionalColumnNameAndValueReturnsTruncatedForCharacterTypesWith
             // Assert
             _columnSimplePropertyValueResolver.Verify(r => r.GetPropertyValueForColumn(additionalColumn, properties), Times.Once);
             Assert.Equal(columnName, result.Key);
-            Assert.Equal("Additional", result.Value);
+            Assert.Equal("Additio...", result.Value);
         }
     }
 }