Skip to content

Commit 4dec431

Browse files
ckadlubackadluba
authored
Merge pull request #271 from ckadluba/refactor-SinkTraits
Restructured sinks, added unit tests.
2 parents f9708f3 + 2f7c2b9 commit 4dec431

File tree

13 files changed

+384
-167
lines changed

13 files changed

+384
-167
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
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 = "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 = "Serilog.Sinks.MSSqlServer")]
10-
[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.String,System.String,System.Data.DataTable,Serilog.Sinks.MSSqlServer.ColumnOptions)")]
10+
[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)")]
1111
[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.Sinks.MSSqlServer.Platform.SqlBulkBatchWriter.WriteBatch(System.Collections.Generic.IEnumerable{Serilog.Events.LogEvent},System.Data.DataTable)~System.Threading.Tasks.Task")]
1212
[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")]
1313
[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")]

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@ internal static SinkDependencies Create(
3131

3232
var sinkDependencies = new SinkDependencies
3333
{
34-
SqlTableCreator = new SqlTableCreator(new SqlCreateTableWriter(), sqlConnectionFactory),
35-
DataTableCreator = new DataTableCreator(),
34+
SqlTableCreator = new SqlTableCreator(
35+
sinkOptions.TableName, sinkOptions.SchemaName, columnOptions,
36+
new SqlCreateTableWriter(), sqlConnectionFactory),
37+
DataTableCreator = new DataTableCreator(sinkOptions.TableName, columnOptions),
3638
SqlBulkBatchWriter = new SqlBulkBatchWriter(
3739
sinkOptions.TableName, sinkOptions.SchemaName,
3840
columnOptions.DisableTriggers, sqlConnectionFactory, logEventDataGenerator),

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

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
namespace Serilog.Sinks.MSSqlServer
2424
{
2525
/// <summary>
26-
/// Writes log events as rows in a table of MSSqlServer database using Audit logic, meaning that each row is synchronously committed
27-
/// and any errors that occur are propagated to the caller.
26+
/// Writes log events as rows in a table of MSSqlServer database using Audit logic, meaning that each row is synchronously committed
27+
/// and any errors that occur are propagated to the caller.
2828
/// </summary>
2929
public class MSSqlServerAuditSink : ILogEventSink, IDisposable
3030
{
@@ -83,41 +83,28 @@ internal MSSqlServerAuditSink(
8383
ColumnOptions columnOptions,
8484
SinkDependencies sinkDependencies)
8585
{
86-
if (sinkOptions?.TableName == null)
87-
{
88-
throw new InvalidOperationException("Table name must be specified!");
89-
}
86+
ValidateParameters(sinkOptions, columnOptions);
87+
CheckSinkDependencies(sinkDependencies);
9088

91-
columnOptions = columnOptions ?? new ColumnOptions();
92-
columnOptions.FinalizeConfigurationForSinkConstructor();
93-
if (columnOptions.DisableTriggers)
94-
throw new NotSupportedException($"The {nameof(ColumnOptions.DisableTriggers)} option is not supported for auditing.");
89+
_sqlLogEventWriter = sinkDependencies.SqlLogEventWriter;
9590

96-
if (sinkDependencies == null)
97-
{
98-
throw new ArgumentNullException(nameof(sinkDependencies));
99-
}
100-
_sqlLogEventWriter = sinkDependencies?.SqlLogEventWriter ?? throw new InvalidOperationException($"SqlLogEventWriter is not initialized!");
101-
102-
if (sinkOptions.AutoCreateSqlTable)
103-
{
104-
if (sinkDependencies?.DataTableCreator == null)
105-
{
106-
throw new InvalidOperationException($"DataTableCreator is not initialized!");
107-
}
108-
109-
using (var eventTable = sinkDependencies.DataTableCreator.CreateDataTable(sinkOptions.TableName, columnOptions))
110-
{
111-
sinkDependencies.SqlTableCreator.CreateTable(sinkOptions.SchemaName, sinkOptions.TableName, eventTable, columnOptions);
112-
}
113-
}
91+
CreateTable(sinkOptions, sinkDependencies);
11492
}
11593

11694
/// <summary>Emit the provided log event to the sink.</summary>
11795
/// <param name="logEvent">The log event to write.</param>
11896
public void Emit(LogEvent logEvent) =>
11997
_sqlLogEventWriter.WriteEvent(logEvent);
12098

99+
/// <summary>
100+
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
101+
/// </summary>
102+
public void Dispose()
103+
{
104+
Dispose(true);
105+
GC.SuppressFinalize(this);
106+
}
107+
121108
/// <summary>
122109
/// Releases the unmanaged resources used by the Serilog.Sinks.MSSqlServer.MSSqlServerAuditSink and optionally
123110
/// releases the managed resources.
@@ -128,13 +115,49 @@ protected virtual void Dispose(bool disposing)
128115
// This class needn't to dispose anything. This is just here for sink interface compatibility.
129116
}
130117

131-
/// <summary>
132-
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
133-
/// </summary>
134-
public void Dispose()
118+
private static void ValidateParameters(SinkOptions sinkOptions, ColumnOptions columnOptions)
135119
{
136-
Dispose(true);
137-
GC.SuppressFinalize(this);
120+
if (sinkOptions?.TableName == null)
121+
{
122+
throw new InvalidOperationException("Table name must be specified!");
123+
}
124+
125+
if (columnOptions.DisableTriggers)
126+
throw new NotSupportedException($"The {nameof(ColumnOptions.DisableTriggers)} option is not supported for auditing.");
127+
}
128+
129+
private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
130+
{
131+
if (sinkDependencies == null)
132+
{
133+
throw new ArgumentNullException(nameof(sinkDependencies));
134+
}
135+
136+
if (sinkDependencies.DataTableCreator == null)
137+
{
138+
throw new InvalidOperationException($"DataTableCreator is not initialized!");
139+
}
140+
141+
if (sinkDependencies.SqlTableCreator == null)
142+
{
143+
throw new InvalidOperationException($"SqlTableCreator is not initialized!");
144+
}
145+
146+
if (sinkDependencies.SqlLogEventWriter == null)
147+
{
148+
throw new InvalidOperationException($"SqlLogEventWriter is not initialized!");
149+
}
150+
}
151+
152+
private static void CreateTable(SinkOptions sinkOptions, SinkDependencies sinkDependencies)
153+
{
154+
if (sinkOptions.AutoCreateSqlTable)
155+
{
156+
using (var eventTable = sinkDependencies.DataTableCreator.CreateDataTable())
157+
{
158+
sinkDependencies.SqlTableCreator.CreateTable(eventTable);
159+
}
160+
}
138161
}
139162
}
140163
}

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

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
namespace Serilog.Sinks.MSSqlServer
2727
{
2828
/// <summary>
29-
/// Writes log events as rows in a table of MSSqlServer database.
29+
/// Writes log events as rows in a table of MSSqlServer database.
3030
/// </summary>
3131
public class MSSqlServerSink : PeriodicBatchingSink
3232
{
@@ -94,63 +94,37 @@ public MSSqlServerSink(
9494
IFormatProvider formatProvider = null,
9595
ColumnOptions columnOptions = null,
9696
ITextFormatter logEventFormatter = null)
97-
: this(sinkOptions, columnOptions,
98-
SinkDependenciesFactory.Create(connectionString, sinkOptions, formatProvider, columnOptions, logEventFormatter))
97+
: this(sinkOptions, SinkDependenciesFactory.Create(connectionString, sinkOptions, formatProvider, columnOptions, logEventFormatter))
9998
{
10099
}
101100

102101
// Internal constructor with injectable dependencies for better testability
103102
internal MSSqlServerSink(
104103
SinkOptions sinkOptions,
105-
ColumnOptions columnOptions,
106104
SinkDependencies sinkDependencies)
107105
: base(sinkOptions?.BatchPostingLimit ?? DefaultBatchPostingLimit, sinkOptions?.BatchPeriod ?? DefaultPeriod)
108106
{
109-
if (sinkOptions?.TableName == null)
110-
{
111-
throw new InvalidOperationException("Table name must be specified!");
112-
}
113-
114-
columnOptions = columnOptions ?? new ColumnOptions();
115-
columnOptions.FinalizeConfigurationForSinkConstructor();
116-
117-
if (sinkDependencies == null)
118-
{
119-
throw new ArgumentNullException(nameof(sinkDependencies));
120-
}
121-
122-
_sqlBulkBatchWriter = sinkDependencies?.SqlBulkBatchWriter ?? throw new InvalidOperationException($"SqlBulkBatchWriter is not initialized!");
107+
ValidateParameters(sinkOptions);
108+
CheckSinkDependencies(sinkDependencies);
123109

124-
if (sinkDependencies?.DataTableCreator == null)
125-
{
126-
throw new InvalidOperationException($"DataTableCreator is not initialized!");
127-
}
128-
_eventTable = sinkDependencies.DataTableCreator.CreateDataTable(sinkOptions.TableName, columnOptions);
110+
_sqlBulkBatchWriter = sinkDependencies.SqlBulkBatchWriter;
111+
_eventTable = sinkDependencies.DataTableCreator.CreateDataTable();
129112

130-
if (sinkOptions.AutoCreateSqlTable)
131-
{
132-
if (sinkDependencies?.SqlBulkBatchWriter == null)
133-
{
134-
throw new InvalidOperationException($"SqlTableCreator is not initialized!");
135-
}
136-
sinkDependencies.SqlTableCreator.CreateTable(sinkOptions.SchemaName, sinkOptions.TableName, _eventTable, columnOptions);
137-
}
113+
CreateTable(sinkOptions, sinkDependencies);
138114
}
139115

140116
/// <summary>
141-
/// Emit a batch of log events, running asynchronously.
117+
/// Emit a batch of log events, running asynchronously.
142118
/// </summary>
143119
/// <param name="events">The events to emit.</param>
144120
/// <remarks>
145-
/// Override either <see cref="PeriodicBatchingSink.EmitBatch" /> or <see cref="PeriodicBatchingSink.EmitBatchAsync" />
146-
/// ,
147-
/// not both.
121+
/// Override either <see cref="PeriodicBatchingSink.EmitBatch" /> or <see cref="PeriodicBatchingSink.EmitBatchAsync" />, not both.
148122
/// </remarks>
149123
protected override Task EmitBatchAsync(IEnumerable<LogEvent> events) =>
150124
_sqlBulkBatchWriter.WriteBatch(events, _eventTable);
151125

152126
/// <summary>
153-
/// Disposes the connection
127+
/// Disposes the connection
154128
/// </summary>
155129
/// <param name="disposing"></param>
156130
protected override void Dispose(bool disposing)
@@ -161,5 +135,44 @@ protected override void Dispose(bool disposing)
161135
_eventTable.Dispose();
162136
}
163137
}
138+
139+
private static void ValidateParameters(SinkOptions sinkOptions)
140+
{
141+
if (sinkOptions?.TableName == null)
142+
{
143+
throw new InvalidOperationException("Table name must be specified!");
144+
}
145+
}
146+
147+
private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
148+
{
149+
if (sinkDependencies == null)
150+
{
151+
throw new ArgumentNullException(nameof(sinkDependencies));
152+
}
153+
154+
if (sinkDependencies.DataTableCreator == null)
155+
{
156+
throw new InvalidOperationException($"DataTableCreator is not initialized!");
157+
}
158+
159+
if (sinkDependencies.SqlTableCreator == null)
160+
{
161+
throw new InvalidOperationException($"SqlTableCreator is not initialized!");
162+
}
163+
164+
if (sinkDependencies.SqlBulkBatchWriter == null)
165+
{
166+
throw new InvalidOperationException($"SqlBulkBatchWriter is not initialized!");
167+
}
168+
}
169+
170+
private void CreateTable(SinkOptions sinkOptions, SinkDependencies sinkDependencies)
171+
{
172+
if (sinkOptions.AutoCreateSqlTable)
173+
{
174+
sinkDependencies.SqlTableCreator.CreateTable(_eventTable);
175+
}
176+
}
164177
}
165178
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/DataTableCreator.cs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,39 @@
1+
using System;
12
using System.Data;
23

34
namespace Serilog.Sinks.MSSqlServer.Platform
45
{
56
internal class DataTableCreator : IDataTableCreator
67
{
7-
public DataTable CreateDataTable(string tableName, ColumnOptions columnOptions)
8+
private readonly string _tableName;
9+
private readonly ColumnOptions _columnOptions;
10+
11+
public DataTableCreator(string tableName, ColumnOptions columnOptions)
12+
{
13+
_tableName = tableName ?? throw new ArgumentNullException(nameof(tableName));
14+
_columnOptions = columnOptions ?? throw new ArgumentNullException(nameof(columnOptions));
15+
}
16+
17+
public DataTable CreateDataTable()
818
{
9-
var eventsTable = new DataTable(tableName);
19+
var eventsTable = new DataTable(_tableName);
1020

11-
foreach (var standardColumn in columnOptions.Store)
21+
foreach (var standardColumn in _columnOptions.Store)
1222
{
13-
var standardOpts = columnOptions.GetStandardColumnOptions(standardColumn);
23+
var standardOpts = _columnOptions.GetStandardColumnOptions(standardColumn);
1424
var dataColumn = standardOpts.AsDataColumn();
1525
eventsTable.Columns.Add(dataColumn);
16-
if (standardOpts == columnOptions.PrimaryKey)
26+
if (standardOpts == _columnOptions.PrimaryKey)
1727
eventsTable.PrimaryKey = new DataColumn[] { dataColumn };
1828
}
1929

20-
if (columnOptions.AdditionalColumns != null)
30+
if (_columnOptions.AdditionalColumns != null)
2131
{
22-
foreach (var addCol in columnOptions.AdditionalColumns)
32+
foreach (var addCol in _columnOptions.AdditionalColumns)
2333
{
2434
var dataColumn = addCol.AsDataColumn();
2535
eventsTable.Columns.Add(dataColumn);
26-
if (addCol == columnOptions.PrimaryKey)
36+
if (addCol == _columnOptions.PrimaryKey)
2737
eventsTable.PrimaryKey = new DataColumn[] { dataColumn };
2838
}
2939
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/IDataTableCreator.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ namespace Serilog.Sinks.MSSqlServer.Platform
44
{
55
internal interface IDataTableCreator
66
{
7-
DataTable CreateDataTable(string tableName, ColumnOptions columnOptions);
7+
DataTable CreateDataTable();
88
}
9-
}
9+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlTableCreator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ namespace Serilog.Sinks.MSSqlServer.Platform
44
{
55
internal interface ISqlTableCreator
66
{
7-
void CreateTable(string schemaName, string tableName, DataTable dataTable, ColumnOptions columnOptions);
7+
void CreateTable(DataTable dataTable);
88
}
99
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlCreateTableWriter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ public string GetSqlFromDataTable(string schemaName, string tableName, DataTable
1010
{
1111
var sql = new StringBuilder();
1212
var ix = new StringBuilder();
13-
int indexCount = 1;
13+
var indexCount = 1;
1414

1515
// start schema check and DDL (wrap in EXEC to make a separate batch)
1616
sql.AppendLine($"IF(NOT EXISTS(SELECT * FROM sys.schemas WHERE name = '{schemaName}'))");
@@ -24,7 +24,7 @@ public string GetSqlFromDataTable(string schemaName, string tableName, DataTable
2424
sql.AppendLine($"CREATE TABLE [{schemaName}].[{tableName}] ( ");
2525

2626
// build column list
27-
int i = 1;
27+
var i = 1;
2828
foreach (DataColumn column in dataTable.Columns)
2929
{
3030
var common = (SqlColumn)column.ExtendedProperties["SqlColumn"];

0 commit comments

Comments
 (0)