From 1f942d7c2a20cdbf16d6040de44ec1ad3a57620d Mon Sep 17 00:00:00 2001 From: Christian Kadluba <10721825+ckadluba@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:30:44 +0100 Subject: [PATCH] wip: refactor only one SqlCommand --- .../Platform/SqlClient/ISqlCommandWrapper.cs | 2 + .../Platform/SqlClient/SqlCommandWrapper.cs | 10 ++ .../Platform/SqlInsertStatementWriter.cs | 120 +++++++++++++----- .../Platform/SqlInsertStatementWriterTests.cs | 15 +-- 4 files changed, 99 insertions(+), 48 deletions(-) diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs index e88449f..76f0896 100644 --- a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs +++ b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs @@ -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 ExecuteNonQueryAsync(); diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapper.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapper.cs index 92df9cf..fd844d5 100644 --- a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapper.cs +++ b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapper.cs @@ -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); diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertStatementWriter.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertStatementWriter.cs index 9661554..0420a45 100644 --- a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertStatementWriter.cs +++ b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertStatementWriter.cs @@ -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 @@ -18,6 +20,7 @@ internal class SqlInsertStatementWriter : ISqlLogEventWriter private readonly ISqlCommandFactory _sqlCommandFactory; private readonly ILogEventDataGenerator _logEventDataGenerator; + private ISqlCommandWrapper _sqlCommand; private bool _disposedValue; public SqlInsertStatementWriter( @@ -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(); @@ -46,38 +51,51 @@ public async Task WriteEvents(IEnumerable 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); } } } @@ -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> 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); } } } diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterTests.cs index de83025..07f6a04 100644 --- a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterTests.cs +++ b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterTests.cs @@ -112,7 +112,7 @@ public async Task WriteEventsCallsSqlConnectionWrappeCreateCommand() // Assert _sqlCommandFactoryMock.Verify(c => c.CreateCommand(_sqlConnectionWrapperMock.Object), - Times.Exactly(logEvents.Count)); + Times.Once); } [Fact] @@ -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() {