Skip to content

Commit f333ce6

Browse files
committed
Initialize DataTableCreator and SqlTableCreator with options on contruction to reduce dependencies on SinkOptions and ColumnOption in the sink classes.
1 parent 9c59c51 commit f333ce6

File tree

11 files changed

+71
-65
lines changed

11 files changed

+71
-65
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: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ internal MSSqlServerAuditSink(
8888
throw new InvalidOperationException("Table name must be specified!");
8989
}
9090

91-
columnOptions = columnOptions ?? new ColumnOptions();
92-
columnOptions.FinalizeConfigurationForSinkConstructor();
9391
if (columnOptions.DisableTriggers)
9492
throw new NotSupportedException($"The {nameof(ColumnOptions.DisableTriggers)} option is not supported for auditing.");
9593

@@ -106,9 +104,9 @@ internal MSSqlServerAuditSink(
106104
throw new InvalidOperationException($"DataTableCreator is not initialized!");
107105
}
108106

109-
using (var eventTable = sinkDependencies.DataTableCreator.CreateDataTable(sinkOptions.TableName, columnOptions))
107+
using (var eventTable = sinkDependencies.DataTableCreator.CreateDataTable())
110108
{
111-
sinkDependencies.SqlTableCreator.CreateTable(sinkOptions.SchemaName, sinkOptions.TableName, eventTable, columnOptions);
109+
sinkDependencies.SqlTableCreator.CreateTable(eventTable);
112110
}
113111
}
114112
}

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,13 @@ 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
{
@@ -111,9 +109,6 @@ internal MSSqlServerSink(
111109
throw new InvalidOperationException("Table name must be specified!");
112110
}
113111

114-
columnOptions = columnOptions ?? new ColumnOptions();
115-
columnOptions.FinalizeConfigurationForSinkConstructor();
116-
117112
if (sinkDependencies == null)
118113
{
119114
throw new ArgumentNullException(nameof(sinkDependencies));
@@ -125,15 +120,15 @@ internal MSSqlServerSink(
125120
{
126121
throw new InvalidOperationException($"DataTableCreator is not initialized!");
127122
}
128-
_eventTable = sinkDependencies.DataTableCreator.CreateDataTable(sinkOptions.TableName, columnOptions);
123+
_eventTable = sinkDependencies.DataTableCreator.CreateDataTable();
129124

130125
if (sinkOptions.AutoCreateSqlTable)
131126
{
132127
if (sinkDependencies?.SqlBulkBatchWriter == null)
133128
{
134129
throw new InvalidOperationException($"SqlTableCreator is not initialized!");
135130
}
136-
sinkDependencies.SqlTableCreator.CreateTable(sinkOptions.SchemaName, sinkOptions.TableName, _eventTable, columnOptions);
131+
sinkDependencies.SqlTableCreator.CreateTable(_eventTable);
137132
}
138133
}
139134

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/SqlTableCreator.cs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,33 @@ namespace Serilog.Sinks.MSSqlServer.Platform
88
{
99
internal class SqlTableCreator : ISqlTableCreator
1010
{
11+
private readonly string _tableName;
12+
private readonly string _schemaName;
13+
private readonly ColumnOptions _columnOptions;
1114
private readonly ISqlCreateTableWriter _sqlCreateTableWriter;
1215
private readonly ISqlConnectionFactory _sqlConnectionFactory;
1316

14-
public SqlTableCreator(ISqlCreateTableWriter sqlCreateTableWriter, ISqlConnectionFactory sqlConnectionFactory)
17+
public SqlTableCreator(
18+
string tableName,
19+
string schemaName,
20+
ColumnOptions columnOptions,
21+
ISqlCreateTableWriter sqlCreateTableWriter,
22+
ISqlConnectionFactory sqlConnectionFactory)
1523
{
24+
_tableName = tableName ?? throw new ArgumentNullException(nameof(tableName));
25+
_schemaName = schemaName ?? throw new ArgumentNullException(nameof(schemaName));
26+
_columnOptions = columnOptions ?? throw new ArgumentNullException(nameof(columnOptions));
1627
_sqlCreateTableWriter = sqlCreateTableWriter ?? throw new ArgumentNullException(nameof(sqlCreateTableWriter));
1728
_sqlConnectionFactory = sqlConnectionFactory ?? throw new ArgumentNullException(nameof(sqlConnectionFactory));
1829
}
1930

20-
public void CreateTable(string schemaName, string tableName, DataTable dataTable, ColumnOptions columnOptions)
31+
public void CreateTable(DataTable dataTable)
2132
{
2233
try
2334
{
2435
using (var conn = _sqlConnectionFactory.Create())
2536
{
26-
var sql = _sqlCreateTableWriter.GetSqlFromDataTable(schemaName, tableName, dataTable, columnOptions);
37+
var sql = _sqlCreateTableWriter.GetSqlFromDataTable(_schemaName, _tableName, dataTable, _columnOptions);
2738
using (var cmd = new SqlCommand(sql, conn))
2839
{
2940
conn.Open();
@@ -33,7 +44,7 @@ public void CreateTable(string schemaName, string tableName, DataTable dataTable
3344
}
3445
catch (Exception ex)
3546
{
36-
SelfLog.WriteLine($"Exception creating table {tableName}:\n{ex}");
47+
SelfLog.WriteLine($"Exception creating table {_tableName}:\n{ex}");
3748
}
3849
}
3950
}

test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/MSSqlServerAuditSinkTests.cs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public MSSqlServerAuditSinkTests()
2727
{
2828
_dataTable = new DataTable(_tableName);
2929
_dataTableCreatorMock = new Mock<IDataTableCreator>();
30-
_dataTableCreatorMock.Setup(d => d.CreateDataTable(It.IsAny<string>(), It.IsAny<Serilog.Sinks.MSSqlServer.ColumnOptions>()))
30+
_dataTableCreatorMock.Setup(d => d.CreateDataTable())
3131
.Returns(_dataTable);
3232

3333
_sqlTableCreatorMock = new Mock<ISqlTableCreator>();
@@ -51,7 +51,7 @@ public void InitializeWithAutoCreateSqlTableCallsDataTableCreator()
5151
SetupSut(options, autoCreateSqlTable: true);
5252

5353
// Assert
54-
_dataTableCreatorMock.Verify(c => c.CreateDataTable(_tableName, options), Times.Once);
54+
_dataTableCreatorMock.Verify(c => c.CreateDataTable(), Times.Once);
5555
}
5656

5757
[Fact]
@@ -64,22 +64,19 @@ public void InitializeWithoutAutoCreateSqlTableDoesNotCallDataTableCreator()
6464
SetupSut(options, autoCreateSqlTable: false);
6565

6666
// Assert
67-
_dataTableCreatorMock.Verify(c => c.CreateDataTable(It.IsAny<string>(),
68-
It.IsAny<Serilog.Sinks.MSSqlServer.ColumnOptions>()), Times.Never);
67+
_dataTableCreatorMock.Verify(c => c.CreateDataTable(), Times.Never);
6968
}
7069

7170
[Fact]
7271
public void InitializeWithAutoCreateSqlTableCallsSqlTableCreator()
7372
{
74-
// Arrange
7573
var options = new Serilog.Sinks.MSSqlServer.ColumnOptions();
7674

7775
// Act
7876
SetupSut(options, autoCreateSqlTable: true);
7977

8078
// Assert
81-
_sqlTableCreatorMock.Verify(c => c.CreateTable(_schemaName, _tableName, _dataTable, options),
82-
Times.Once);
79+
_sqlTableCreatorMock.Verify(c => c.CreateTable(_dataTable), Times.Once);
8380
}
8481

8582
[Fact]
@@ -92,9 +89,7 @@ public void InitializeWithoutAutoCreateSqlTableDoesNotCallSqlTableCreator()
9289
SetupSut(options, autoCreateSqlTable: false);
9390

9491
// Assert
95-
_sqlTableCreatorMock.Verify(c => c.CreateTable(It.IsAny<string>(), It.IsAny<string>(),
96-
It.IsAny<DataTable>(), It.IsAny<Serilog.Sinks.MSSqlServer.ColumnOptions>()),
97-
Times.Never);
92+
_sqlTableCreatorMock.Verify(c => c.CreateTable(It.IsAny<DataTable>()), Times.Never);
9893
}
9994

10095
private void SetupSut(

test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/MSSqlServerSinkTests.cs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public MSSqlServerSinkTests()
2727
{
2828
_dataTable = new DataTable(_tableName);
2929
_dataTableCreatorMock = new Mock<IDataTableCreator>();
30-
_dataTableCreatorMock.Setup(d => d.CreateDataTable(It.IsAny<string>(), It.IsAny<Serilog.Sinks.MSSqlServer.ColumnOptions>()))
30+
_dataTableCreatorMock.Setup(d => d.CreateDataTable())
3131
.Returns(_dataTable);
3232

3333
_sqlTableCreatorMock = new Mock<ISqlTableCreator>();
@@ -48,10 +48,10 @@ public void InitializeCallsDataTableCreator()
4848
var options = new Serilog.Sinks.MSSqlServer.ColumnOptions();
4949

5050
// Act
51-
SetupSut(options, autoCreateSqlTable: false);
51+
SetupSut(autoCreateSqlTable: false);
5252

5353
// Assert
54-
_dataTableCreatorMock.Verify(c => c.CreateDataTable(_tableName, options), Times.Once);
54+
_dataTableCreatorMock.Verify(c => c.CreateDataTable(), Times.Once);
5555
}
5656

5757
[Fact]
@@ -61,11 +61,10 @@ public void InitializeWithAutoCreateSqlTableCallsSqlTableCreator()
6161
var options = new Serilog.Sinks.MSSqlServer.ColumnOptions();
6262

6363
// Act
64-
SetupSut(options, autoCreateSqlTable: true);
64+
SetupSut(autoCreateSqlTable: true);
6565

6666
// Assert
67-
_sqlTableCreatorMock.Verify(c => c.CreateTable(_schemaName, _tableName, _dataTable, options),
68-
Times.Once);
67+
_sqlTableCreatorMock.Verify(c => c.CreateTable(_dataTable), Times.Once);
6968
}
7069

7170
[Fact]
@@ -75,25 +74,21 @@ public void InitializeWithoutAutoCreateSqlTableDoesNotCallSqlTableCreator()
7574
var options = new Serilog.Sinks.MSSqlServer.ColumnOptions();
7675

7776
// Act
78-
SetupSut(options, autoCreateSqlTable: false);
77+
SetupSut(autoCreateSqlTable: false);
7978

8079
// Assert
81-
_sqlTableCreatorMock.Verify(c => c.CreateTable(It.IsAny<string>(), It.IsAny<string>(),
82-
It.IsAny<DataTable>(), It.IsAny<Serilog.Sinks.MSSqlServer.ColumnOptions>()),
83-
Times.Never);
80+
_sqlTableCreatorMock.Verify(c => c.CreateTable(It.IsAny<DataTable>()), Times.Never);
8481
}
8582

86-
private void SetupSut(
87-
Serilog.Sinks.MSSqlServer.ColumnOptions options,
88-
bool autoCreateSqlTable = false)
83+
private void SetupSut(bool autoCreateSqlTable = false)
8984
{
9085
var sinkOptions = new SinkOptions
9186
{
9287
TableName = _tableName,
9388
SchemaName = _schemaName,
9489
AutoCreateSqlTable = autoCreateSqlTable
9590
};
96-
_sut = new MSSqlServerSink(sinkOptions, options, _sinkDependencies);
91+
_sut = new MSSqlServerSink(sinkOptions, _sinkDependencies);
9792
}
9893

9994
protected virtual void Dispose(bool disposing)

0 commit comments

Comments
 (0)