Skip to content

Commit

Permalink
wip: refactor only one SqlCommand
Browse files Browse the repository at this point in the history
  • Loading branch information
@ckadluba
ckadluba committed Nov 6, 2024
1 parent fecfd13 commit 1f942d7
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 48 deletions.
2 changes: 2 additions & 0 deletions src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ internal interface ISqlCommandWrapper : IDisposable
CommandType CommandType { get; set; }
string CommandText { get; set; }

void SetConnection(ISqlConnectionWrapper sqlConnectionWrapper);
void ClearParameters();
void AddParameter(string parameterName, object value);
int ExecuteNonQuery();
Task<int> ExecuteNonQueryAsync();
Expand Down
10 changes: 10 additions & 0 deletions src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ public string CommandText
set => _sqlCommand.CommandText = value;
}

public void SetConnection(ISqlConnectionWrapper sqlConnectionWrapper)
{
_sqlCommand.Connection = sqlConnectionWrapper.SqlConnection;
}

public void ClearParameters()
{
_sqlCommand.Parameters.Clear();
}

public void AddParameter(string parameterName, object value)
{
var parameter = new SqlParameter(parameterName, value ?? DBNull.Value);
Expand Down
120 changes: 86 additions & 34 deletions src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertStatementWriter.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Serilog.Debugging;
using Serilog.Events;
using Serilog.Sinks.MSSqlServer.Output;
using Serilog.Sinks.MSSqlServer.Platform.SqlClient;
using static System.FormattableString;

namespace Serilog.Sinks.MSSqlServer.Platform
Expand All @@ -18,6 +20,7 @@ internal class SqlInsertStatementWriter : ISqlLogEventWriter
private readonly ISqlCommandFactory _sqlCommandFactory;
private readonly ILogEventDataGenerator _logEventDataGenerator;

private ISqlCommandWrapper _sqlCommand;
private bool _disposedValue;

public SqlInsertStatementWriter(
Expand All @@ -29,9 +32,11 @@ public SqlInsertStatementWriter(
{
_tableName = tableName ?? throw new ArgumentNullException(nameof(tableName));
_schemaName = schemaName ?? throw new ArgumentNullException(nameof(schemaName));
_sqlConnectionFactory = sqlConnectionFactory ?? throw new ArgumentNullException(nameof(sqlConnectionFactory));
_sqlConnectionFactory =
sqlConnectionFactory ?? throw new ArgumentNullException(nameof(sqlConnectionFactory));
_sqlCommandFactory = sqlCommandFactory ?? throw new ArgumentNullException(nameof(sqlCommandFactory));
_logEventDataGenerator = logEventDataGenerator ?? throw new ArgumentNullException(nameof(logEventDataGenerator));
_logEventDataGenerator =
logEventDataGenerator ?? throw new ArgumentNullException(nameof(logEventDataGenerator));
}

public void WriteEvent(LogEvent logEvent) => WriteEvents(new[] { logEvent }).GetAwaiter().GetResult();
Expand All @@ -46,38 +51,51 @@ public async Task WriteEvents(IEnumerable<LogEvent> events)

foreach (var logEvent in events)
{
using (var command = _sqlCommandFactory.CreateCommand(cn))
// using (var command = _sqlCommandFactory.CreateCommand(cn))
// {
// command.CommandType = CommandType.Text;
//
// var fieldList = new StringBuilder(Invariant($"INSERT INTO [{_schemaName}].[{_tableName}] ("));
// var parameterList = new StringBuilder(") VALUES (");
//
// var index = 0;
// foreach (var field in _logEventDataGenerator.GetColumnsAndValues(logEvent))
// {
// if (index != 0)
// {
// fieldList.Append(',');
// parameterList.Append(',');
// }
//
// fieldList.Append(Invariant($"[{field.Key}]"));
// parameterList.Append("@P");
// parameterList.Append(index);
//
// command.AddParameter(Invariant($"@P{index}"), field.Value);
//
// index++;
// }
//
// parameterList.Append(')');
// fieldList.Append(parameterList);
//
// command.CommandText = fieldList.ToString();
//
// await command.ExecuteNonQueryAsync().ConfigureAwait(false);
// }

var fields = _logEventDataGenerator.GetColumnsAndValues(logEvent).ToList();
InitializeSqlCommand(cn, fields);

var index = 0;
_sqlCommand.ClearParameters();
foreach (var field in fields)
{
command.CommandType = CommandType.Text;

var fieldList = new StringBuilder(Invariant($"INSERT INTO [{_schemaName}].[{_tableName}] ("));
var parameterList = new StringBuilder(") VALUES (");

var index = 0;
foreach (var field in _logEventDataGenerator.GetColumnsAndValues(logEvent))
{
if (index != 0)
{
fieldList.Append(',');
parameterList.Append(',');
}

fieldList.Append(Invariant($"[{field.Key}]"));
parameterList.Append("@P");
parameterList.Append(index);

command.AddParameter(Invariant($"@P{index}"), field.Value);

index++;
}

parameterList.Append(')');
fieldList.Append(parameterList);

command.CommandText = fieldList.ToString();

await command.ExecuteNonQueryAsync().ConfigureAwait(false);
_sqlCommand.AddParameter(Invariant($"@P{index}"), field.Value);
index++;
}

await _sqlCommand.ExecuteNonQueryAsync().ConfigureAwait(false);
}
}
}
Expand Down Expand Up @@ -108,16 +126,50 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
// TODO dispose SqlCommand etc.
_sqlCommand?.Dispose();
}

_disposedValue = true;
}
}

private void InitializeSqlCommand()
private void InitializeSqlCommand(ISqlConnectionWrapper sqlConnection,
IEnumerable<KeyValuePair<string, object>> logEventFields)
{
// Optimization: generate INSERT statement and SqlCommand only once
// and reuse it with different values and SqlConnections because
// the structure does not change.
if (_sqlCommand == null)
{
_sqlCommand = _sqlCommandFactory.CreateCommand(sqlConnection);
_sqlCommand.CommandType = CommandType.Text;

var fieldList = new StringBuilder(Invariant($"INSERT INTO [{_schemaName}].[{_tableName}] ("));
var parameterList = new StringBuilder(") VALUES (");

var index = 0;
foreach (var field in logEventFields)
{
if (index != 0)
{
fieldList.Append(',');
parameterList.Append(',');
}

fieldList.Append(Invariant($"[{field.Key}]"));
parameterList.Append("@P");
parameterList.Append(index);

index++;
}

parameterList.Append(')');
fieldList.Append(parameterList);

_sqlCommand.CommandText = fieldList.ToString();
}

_sqlCommand.SetConnection(sqlConnection);
}
}
}
15 changes: 1 addition & 14 deletions ...rilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public async Task WriteEventsCallsSqlConnectionWrappeCreateCommand()

// Assert
_sqlCommandFactoryMock.Verify(c => c.CreateCommand(_sqlConnectionWrapperMock.Object),
Times.Exactly(logEvents.Count));
Times.Once);
}

[Fact]
Expand Down Expand Up @@ -189,19 +189,6 @@ public async Task WriteEventsCallsSqlCommandWrapperExecuteNonQueryAsync()
_sqlCommandWrapperMock.Verify(c => c.ExecuteNonQueryAsync(), Times.Exactly(logEvents.Count));
}

[Fact]
public async Task WriteEventsCallsSqlCommandWrapperDispose()
{
// Arrange
var logEvents = CreateLogEvents();

// Act
await _sut.WriteEvents(logEvents);

// Assert
_sqlCommandWrapperMock.Verify(c => c.Dispose(), Times.Exactly(logEvents.Count));
}

[Fact]
public async Task WriteEventsCallsLogEventDataGeneratorGetColumnsAndValuesForEachLogEvent()
{
Expand Down

0 comments on commit 1f942d7

Please sign in to comment.