Skip to content

Commit eb4f43b

Browse files
ckadlubackadluba
committed
Resolve hierarchical property expressions for additional columns
* Refactored processing of AdditionalProperties: do not iterate LogEvent properties but iterate columns (more efficient and allows binding one property to multiple columns). * Refactored PropertiesColumnDataGenerator into three new classes: AdditionalColumnDataGenerator, ColumnSimplePropertyDataGenerator and ColumnHierarchicalPropertyDataGenerator. * Restructured, repaired and added unit tests.
1 parent a131a45 commit eb4f43b

16 files changed

+599
-295
lines changed

‎src/Serilog.Sinks.MSSqlServer/GlobalSuppressions.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77

88
[assembly: SuppressMessage("Globalization", "CA1303:Do not pass literals as localized parameters", Justification = "Supplying string literals and not using resources is accepted within this project.", Scope = "namespaceanddescendants", Target = "~N:Serilog.Sinks.MSSqlServer")]
99
[assembly: SuppressMessage("Security", "CA2100:Review SQL queries for security vulnerabilities", Justification = "Too hard to change. Accepted for now.", Scope = "namespaceanddescendants", Target = "~N:Serilog.Sinks.MSSqlServer")]
10+
[assembly: SuppressMessage("Design", "CA1034:Nested types should not be visible", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "namespaceanddescendants", Target = "~N:Serilog.Sinks.MSSqlServer")]
11+
[assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "type", Target = "~T:Serilog.Sinks.MSSqlServer.ColumnOptions")]
12+
[assembly: SuppressMessage("Design", "CA1010:Collections should implement generic interface", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "namespaceanddescendants", Target = "~N:Serilog.Sinks.MSSqlServer")]
13+
[assembly: SuppressMessage("Performance", "CA1822: Mark members as static", Justification = "Using instance members over static for better testablilty.", Scope = "namespaceanddescendants", Target = "~N:Serilog.Sinks.MSSqlServer")]
1014
[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.SetProperty.IfNotNull``1(System.String,Serilog.Sinks.MSSqlServer.SetProperty.PropertySetter{``0})")]
1115
[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.Platform.SqlTableCreator.CreateTable(System.Data.DataTable)")]
1216
[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.Platform.SqlBulkBatchWriter.WriteBatch(System.Collections.Generic.IEnumerable{Serilog.Events.LogEvent},System.Data.DataTable)~System.Threading.Tasks.Task")]
1317
[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.Output.StandardColumnDataGenerator.ConvertPropertiesToXmlStructure(System.Collections.Generic.IEnumerable{System.Collections.Generic.KeyValuePair{System.String,Serilog.Events.LogEventPropertyValue}})~System.String")]
14-
[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.Output.PropertiesColumnDataGenerator.TryChangeType(System.Object,System.Type,System.Object@)~System.Boolean")]
15-
[assembly: SuppressMessage("Design", "CA1034:Nested types should not be visible", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "namespaceanddescendants", Target = "~N:Serilog.Sinks.MSSqlServer")]
16-
[assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "type", Target = "~T:Serilog.Sinks.MSSqlServer.ColumnOptions")]
17-
[assembly: SuppressMessage("Design", "CA1010:Collections should implement generic interface", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "namespaceanddescendants", Target = "~N:Serilog.Sinks.MSSqlServer")]
18-
[assembly: SuppressMessage("Performance", "CA1822: Member AllowNull does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "member", Target = "~P:Serilog.Sinks.MSSqlServer.ColumnOptions.IdColumnOptions.AllowNull")]
18+
[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.Output.AdditionalColumnDataGenerator.TryChangeType(System.Object,System.Type,System.Object@)~System.Boolean")]
1919
[assembly: SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "type", Target = "~T:Serilog.Sinks.MSSqlServer.StandardColumnConfigException")]
2020
[assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.MSSqlServerSink.EmitBatchAsync(System.Collections.Generic.IEnumerable{Serilog.Events.LogEvent})~System.Threading.Tasks.Task")]

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Dependencies/SinkDependenciesFactory.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using Serilog.Sinks.MSSqlServer.Output;
44
using Serilog.Sinks.MSSqlServer.Platform;
55
using Serilog.Sinks.MSSqlServer.Platform.SqlClient;
6+
using Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Output;
67

78
namespace Serilog.Sinks.MSSqlServer.Dependencies
89
{
@@ -27,7 +28,9 @@ internal static SinkDependencies Create(
2728
new StandardColumnDataGenerator(columnOptions, formatProvider,
2829
new XmlPropertyFormatter(),
2930
logEventFormatter),
30-
new PropertiesColumnDataGenerator(columnOptions));
31+
new AdditionalColumnDataGenerator(
32+
new ColumnSimplePropertyValueResolver(),
33+
new ColumnHierarchicalPropertyValueResolver()));
3134

3235
var sinkDependencies = new SinkDependencies
3336
{

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/AdditionalColumnDataGenerator.cs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.ComponentModel;
4+
using Serilog.Events;
5+
using Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Output;
6+
7+
namespace Serilog.Sinks.MSSqlServer.Output
8+
{
9+
internal class AdditionalColumnDataGenerator : IAdditionalColumnDataGenerator
10+
{
11+
private readonly IColumnSimplePropertyValueResolver _columnSimplePropertyValueResolver;
12+
private readonly IColumnHierarchicalPropertyValueResolver _columnHierarchicalPropertyValueResolver;
13+
14+
public AdditionalColumnDataGenerator(
15+
IColumnSimplePropertyValueResolver columnSimplePropertyValueResolver,
16+
IColumnHierarchicalPropertyValueResolver columnHierarchicalPropertyValueResolver)
17+
{
18+
_columnSimplePropertyValueResolver = columnSimplePropertyValueResolver
19+
?? throw new ArgumentNullException(nameof(columnSimplePropertyValueResolver));
20+
_columnHierarchicalPropertyValueResolver = columnHierarchicalPropertyValueResolver
21+
?? throw new ArgumentNullException(nameof(columnHierarchicalPropertyValueResolver));
22+
}
23+
24+
public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn additionalColumn, IReadOnlyDictionary<string, LogEventPropertyValue> properties)
25+
{
26+
var property = !additionalColumn.HasHierarchicalPropertyName
27+
? _columnSimplePropertyValueResolver.GetPropertyValueForColumn(additionalColumn, properties)
28+
: _columnHierarchicalPropertyValueResolver.GetPropertyValueForColumn(additionalColumn, properties);
29+
30+
var columnName = additionalColumn.ColumnName;
31+
if (property.Value == null)
32+
{
33+
return new KeyValuePair<string, object>(columnName, DBNull.Value);
34+
}
35+
36+
if (!(property.Value is ScalarValue scalarValue))
37+
{
38+
return new KeyValuePair<string, object>(columnName, property.Value.ToString());
39+
}
40+
41+
var columnType = additionalColumn.AsDataColumn().DataType;
42+
if (columnType.IsAssignableFrom(scalarValue.Value.GetType()))
43+
{
44+
return new KeyValuePair<string, object>(columnName, scalarValue.Value);
45+
}
46+
47+
if (TryChangeType(scalarValue.Value, columnType, out var conversion))
48+
{
49+
return new KeyValuePair<string, object>(columnName, conversion);
50+
}
51+
else
52+
{
53+
return new KeyValuePair<string, object>(columnName, property.Value.ToString());
54+
}
55+
}
56+
57+
private static bool TryChangeType(object obj, Type type, out object conversion)
58+
{
59+
conversion = null;
60+
try
61+
{
62+
conversion = TypeDescriptor.GetConverter(type).ConvertFrom(obj);
63+
return true;
64+
}
65+
catch
66+
{
67+
return false;
68+
}
69+
}
70+
}
71+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/ColumnHierarchicalPropertyValueResolver.cs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using Serilog.Events;
5+
6+
namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Output
7+
{
8+
internal class ColumnHierarchicalPropertyValueResolver : IColumnHierarchicalPropertyValueResolver
9+
{
10+
public KeyValuePair<string, LogEventPropertyValue> GetPropertyValueForColumn(SqlColumn additionalColumn, IReadOnlyDictionary<string, LogEventPropertyValue> properties)
11+
{
12+
var propertyNameHierarchy = additionalColumn.PropertyNameHierarchy;
13+
KeyValuePair<string, LogEventPropertyValue>? nullableProperty = properties.FirstOrDefault(p => p.Key == propertyNameHierarchy[0]);
14+
if (nullableProperty == null)
15+
{
16+
// Top level property not found, return default
17+
return default;
18+
}
19+
20+
var property = nullableProperty.Value;
21+
if (property.Value is StructureValue structureValue)
22+
{
23+
// Continue on sub property level
24+
var propertyNameHierarchyRemainder = new ArraySegment<string>(propertyNameHierarchy.ToArray(), 1, additionalColumn.PropertyNameHierarchy.Count - 1);
25+
return GetSubPropertyValueForColumnRecursive(propertyNameHierarchyRemainder, structureValue.Properties);
26+
}
27+
else
28+
{
29+
// Sub property not found, return default
30+
return default;
31+
}
32+
}
33+
34+
private KeyValuePair<string, LogEventPropertyValue> GetSubPropertyValueForColumnRecursive(IReadOnlyList<string> propertyNameHierarchy, IReadOnlyList<LogEventProperty> properties)
35+
{
36+
var property = properties.FirstOrDefault(p => p.Name == propertyNameHierarchy[0]);
37+
if (property == null)
38+
{
39+
// Current sub property not found, return default
40+
return default;
41+
}
42+
43+
if (propertyNameHierarchy.Count == 1)
44+
{
45+
// Current sub property found and no further levels in property name of column
46+
// Return final property value
47+
return new KeyValuePair<string, LogEventPropertyValue>(property.Name, property.Value);
48+
}
49+
else
50+
{
51+
if (property.Value is StructureValue structureValue)
52+
{
53+
// Continue on next sub property level
54+
var propertyNameHierarchyRemainder = new ArraySegment<string>(propertyNameHierarchy.ToArray(), 1, propertyNameHierarchy.Count - 1);
55+
return GetSubPropertyValueForColumnRecursive(propertyNameHierarchyRemainder, structureValue.Properties);
56+
}
57+
else
58+
{
59+
// Next sub property not found, return default
60+
return default;
61+
}
62+
}
63+
}
64+
}
65+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/ColumnSimplePropertyValueResolver.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using Serilog.Events;
5+
6+
namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Output
7+
{
8+
internal class ColumnSimplePropertyValueResolver : IColumnSimplePropertyValueResolver
9+
{
10+
public KeyValuePair<string, LogEventPropertyValue> GetPropertyValueForColumn(SqlColumn additionalColumn, IReadOnlyDictionary<string, LogEventPropertyValue> properties)
11+
{
12+
return properties.FirstOrDefault(p => p.Key == additionalColumn.PropertyName);
13+
}
14+
}
15+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/IAdditionalColumnDataGenerator.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
using System.Collections.Generic;
2+
using Serilog.Events;
3+
4+
namespace Serilog.Sinks.MSSqlServer.Output
5+
{
6+
internal interface IAdditionalColumnDataGenerator
7+
{
8+
KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn additionalColumn, IReadOnlyDictionary<string, LogEventPropertyValue> properties);
9+
}
10+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/IColumnHierarchicalPropertyValueResolver.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
using Serilog.Events;
2+
using System.Collections.Generic;
3+
4+
namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Output
5+
{
6+
internal interface IColumnHierarchicalPropertyValueResolver
7+
{
8+
KeyValuePair<string, LogEventPropertyValue> GetPropertyValueForColumn(SqlColumn additionalColumn, IReadOnlyDictionary<string, LogEventPropertyValue> properties);
9+
}
10+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/IColumnSimplePropertyValueResolver.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
using Serilog.Events;
2+
using System.Collections.Generic;
3+
4+
namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Output
5+
{
6+
internal interface IColumnSimplePropertyValueResolver
7+
{
8+
KeyValuePair<string, LogEventPropertyValue> GetPropertyValueForColumn(SqlColumn additionalColumn, IReadOnlyDictionary<string, LogEventPropertyValue> properties);
9+
}
10+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/IPropertiesColumnDataGenerator.cs

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/LogEventDataGenerator.cs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Linq;
34
using Serilog.Events;
45

56
namespace Serilog.Sinks.MSSqlServer.Output
@@ -8,31 +9,32 @@ internal class LogEventDataGenerator : ILogEventDataGenerator
89
{
910
private readonly ColumnOptions _columnOptions;
1011
private readonly IStandardColumnDataGenerator _standardColumnDataGenerator;
11-
private readonly IPropertiesColumnDataGenerator _propertiesColumnDataGenerator;
12+
private readonly IAdditionalColumnDataGenerator _additionalColumnDataGenerator;
1213

1314
public LogEventDataGenerator(
1415
ColumnOptions columnOptions,
1516
IStandardColumnDataGenerator standardColumnDataGenerator,
16-
IPropertiesColumnDataGenerator propertiesColumnDataGenerator)
17+
IAdditionalColumnDataGenerator additionalColumnDataGenerator)
1718
{
1819
_columnOptions = columnOptions ?? throw new ArgumentNullException(nameof(columnOptions));
1920
_standardColumnDataGenerator = standardColumnDataGenerator ?? throw new ArgumentNullException(nameof(standardColumnDataGenerator));
20-
_propertiesColumnDataGenerator = propertiesColumnDataGenerator ?? throw new ArgumentNullException(nameof(propertiesColumnDataGenerator));
21+
_additionalColumnDataGenerator = additionalColumnDataGenerator ?? throw new ArgumentNullException(nameof(additionalColumnDataGenerator));
2122
}
2223

2324
public IEnumerable<KeyValuePair<string, object>> GetColumnsAndValues(LogEvent logEvent)
2425
{
25-
foreach (var column in _columnOptions.Store)
26+
// skip Id (auto-incrementing identity)
27+
foreach (var column in _columnOptions.Store.Where(c => c != StandardColumn.Id))
2628
{
27-
// skip Id (auto-incrementing identity)
28-
if (column != StandardColumn.Id)
29-
yield return _standardColumnDataGenerator.GetStandardColumnNameAndValue(column, logEvent);
29+
yield return _standardColumnDataGenerator.GetStandardColumnNameAndValue(column, logEvent);
3030
}
3131

3232
if (_columnOptions.AdditionalColumns != null)
3333
{
34-
foreach (var columnValuePair in _propertiesColumnDataGenerator.ConvertPropertiesToColumn(logEvent.Properties))
35-
yield return columnValuePair;
34+
foreach (var additionalColumn in _columnOptions.AdditionalColumns)
35+
{
36+
yield return _additionalColumnDataGenerator.GetAdditionalColumnNameAndValue(additionalColumn, logEvent.Properties);
37+
}
3638
}
3739
}
3840
}

0 commit comments

Comments
 (0)