Skip to content

DatabaseTarget - Added support for DbCommand Properties #3839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
304NotModified merged 1 commit into from
Mar 15, 2020
Merged

DatabaseTarget - Added support for DbCommand Properties #3839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/NLog/Targets/DatabaseObjectPropertyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ public DatabaseObjectPropertyInfo()
[DefaultValue(null)]
public CultureInfo Culture { get; set; }

internal bool SetPropertyValue(IDbConnection dbConnection, object propertyValue)
internal bool SetPropertyValue(object dbObject, object propertyValue)
{
var dbConnectionType = dbConnection.GetType();
var dbConnectionType = dbObject.GetType();
var propertySetterCache = _propertySetter;
if (!propertySetterCache.Equals(Name, dbConnectionType))
{
Expand All @@ -102,7 +102,7 @@ internal bool SetPropertyValue(IDbConnection dbConnection, object propertyValue)
_propertySetter = propertySetterCache;
}

return propertySetterCache.PropertySetter?.SetPropertyValue(dbConnection, propertyValue) ?? false;
return propertySetterCache.PropertySetter?.SetPropertyValue(dbObject, propertyValue) ?? false;
}

private struct PropertySetterCacheItem
Expand Down
56 changes: 36 additions & 20 deletions src/NLog/Targets/DatabaseTarget.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,14 @@ public Layout DBPassword
[ArrayParameter(typeof(DatabaseObjectPropertyInfo), "connectionproperty")]
public IList<DatabaseObjectPropertyInfo> ConnectionProperties { get; } = new List<DatabaseObjectPropertyInfo>();

/// <summary>
/// Gets the collection of properties. Each item contains a mapping
/// between NLog layout and a property on the DbCommand instance
/// </summary>
/// <docgen category='Connection Options' order='10' />
[ArrayParameter(typeof(DatabaseObjectPropertyInfo), "commandproperty")]
public IList<DatabaseObjectPropertyInfo> CommandProperties { get; } = new List<DatabaseObjectPropertyInfo>();

/// <summary>
/// Configures isolated transaction batch writing. If supported by the database, then it will improve insert performance.
/// </summary>
Expand Down Expand Up @@ -358,29 +366,29 @@ internal IDbConnection OpenConnection(string connectionString, LogEventInfo logE
connection.ConnectionString = connectionString;
if (ConnectionProperties?.Count > 0)
{
ApplyConnectionProperties(connection, logEventInfo ?? LogEventInfo.CreateNullEvent());
ApplyDatabaseObjectProperties(connection, ConnectionProperties, logEventInfo ?? LogEventInfo.CreateNullEvent());
}

connection.Open();
return connection;
}

private void ApplyConnectionProperties(IDbConnection connection, LogEventInfo logEventInfo)
private void ApplyDatabaseObjectProperties(object databaseObject, IList<DatabaseObjectPropertyInfo> objectProperties, LogEventInfo logEventInfo)
{
for (int i = 0; i < ConnectionProperties.Count; ++i)
for (int i = 0; i < objectProperties.Count; ++i)
{
var propertyInfo = ConnectionProperties[i];
var propertyInfo = objectProperties[i];
try
{
var propertyValue = GetDatabaseConnectionValue(logEventInfo, propertyInfo);
if (!propertyInfo.SetPropertyValue(connection, propertyValue))
var propertyValue = GetDatabaseObjectPropertyValue(logEventInfo, propertyInfo);
if (!propertyInfo.SetPropertyValue(databaseObject, propertyValue))
{
InternalLogger.Warn("DatabaseTarget(Name={0}): Failed to lookup property {1} on {2}", Name, propertyInfo.Name, connection.GetType());
InternalLogger.Warn("DatabaseTarget(Name={0}): Failed to lookup property {1} on {2}", Name, propertyInfo.Name, databaseObject.GetType());
}
}
catch (Exception ex)
{
InternalLogger.Error(ex, "DatabaseTarget(Name={0}): Failed to assign value for property {1} on {2}", Name, propertyInfo.Name, connection.GetType());
InternalLogger.Error(ex, "DatabaseTarget(Name={0}): Failed to assign value for property {1} on {2}", Name, propertyInfo.Name, databaseObject.GetType());
if (ex.MustBeRethrown())
throw;
}
Expand Down Expand Up @@ -699,7 +707,7 @@ private void WriteLogEventBatchToDatabase(IList<AsyncLogEventInfo> logEvents, st
{
for (int i = 0; i < logEvents.Count; ++i)
{
ExecuteDbCommandWithParameters(logEvents[i].LogEvent, dbTransaction);
ExecuteDbCommandWithParameters(logEvents[i].LogEvent, _activeConnection, dbTransaction);
}

dbTransaction?.Commit();
Expand Down Expand Up @@ -767,7 +775,7 @@ private void WriteLogEventToDatabase(LogEventInfo logEvent, string connectionStr
{
EnsureConnectionOpen(connectionString, logEvent);

ExecuteDbCommandWithParameters(logEvent, null);
ExecuteDbCommandWithParameters(logEvent, _activeConnection, null);

transactionScope.Complete(); //not really needed as there is no transaction at all.
}
Expand All @@ -790,12 +798,9 @@ private void WriteLogEventToDatabase(LogEventInfo logEvent, string connectionStr
/// <summary>
/// Write logEvent to database
/// </summary>
private void ExecuteDbCommandWithParameters(LogEventInfo logEvent, IDbTransaction dbTransaction)
private void ExecuteDbCommandWithParameters(LogEventInfo logEvent, IDbConnection dbConnection, IDbTransaction dbTransaction)
{
var commandText = RenderLogEvent(CommandText, logEvent);
InternalLogger.Trace("DatabaseTarget(Name={0}): Executing {1}: {2}", Name, CommandType, commandText);

using (IDbCommand command = CreateDbCommandWithParameters(logEvent, CommandType, commandText, Parameters))
using (IDbCommand command = CreateDbCommand(logEvent, dbConnection))
{
if (dbTransaction != null)
command.Transaction = dbTransaction;
Expand All @@ -805,11 +810,22 @@ private void ExecuteDbCommandWithParameters(LogEventInfo logEvent, IDbTransactio
}
}

internal IDbCommand CreateDbCommand(LogEventInfo logEvent, IDbConnection dbConnection)
{
var commandText = RenderLogEvent(CommandText, logEvent);
InternalLogger.Trace("DatabaseTarget(Name={0}): Executing {1}: {2}", Name, CommandType, commandText);
return CreateDbCommandWithParameters(logEvent, dbConnection, CommandType, commandText, Parameters);
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA2100:Review SQL queries for security vulnerabilities", Justification = "It's up to the user to ensure proper quoting.")]
private IDbCommand CreateDbCommandWithParameters(LogEventInfo logEvent, CommandType commandType, string dbCommandText, IList<DatabaseParameterInfo> databaseParameterInfos)
private IDbCommand CreateDbCommandWithParameters(LogEventInfo logEvent, IDbConnection dbConnection, CommandType commandType, string dbCommandText, IList<DatabaseParameterInfo> databaseParameterInfos)
{
var dbCommand = _activeConnection.CreateCommand();
var dbCommand = dbConnection.CreateCommand();
dbCommand.CommandType = commandType;
if (CommandProperties?.Count > 0)
{
ApplyDatabaseObjectProperties(dbCommand, CommandProperties, logEvent);
}
dbCommand.CommandText = dbCommandText;

for (int i = 0; i < databaseParameterInfos.Count; ++i)
Expand Down Expand Up @@ -965,7 +981,7 @@ private void RunInstallCommands(InstallationContext installationContext, IEnumer

installationContext.Trace("DatabaseTarget(Name={0}) - Executing {1} '{2}'", Name, commandInfo.CommandType, commandText);

using (IDbCommand command = CreateDbCommandWithParameters(logEvent, commandInfo.CommandType, commandText, commandInfo.Parameters))
using (IDbCommand command = CreateDbCommandWithParameters(logEvent, _activeConnection, commandInfo.CommandType, commandText, commandInfo.Parameters))
{
try
{
Expand Down Expand Up @@ -1081,7 +1097,7 @@ protected internal virtual object GetDatabaseParameterValue(LogEventInfo logEven
return RenderObjectValue(logEvent, parameterInfo.Name, parameterInfo.Layout, parameterInfo.ParameterType, parameterInfo.Format, parameterInfo.Culture);
}

private object GetDatabaseConnectionValue(LogEventInfo logEvent, DatabaseObjectPropertyInfo connectionInfo)
private object GetDatabaseObjectPropertyValue(LogEventInfo logEvent, DatabaseObjectPropertyInfo connectionInfo)
{
return RenderObjectValue(logEvent, connectionInfo.Name, connectionInfo.Layout, connectionInfo.PropertyType, connectionInfo.Format, connectionInfo.Culture);
}
Expand Down Expand Up @@ -1163,7 +1179,7 @@ private static object CreateDefaultValue(Type dbParameterType)
///
/// Transactions aren't in .NET Core: https://github.com/dotnet/corefx/issues/2949
/// </summary>
private class TransactionScope : IDisposable
private sealed class TransactionScope : IDisposable
{
private readonly TransactionScopeOption suppress;

Expand Down
26 changes: 26 additions & 0 deletions tests/NLog.UnitTests/Targets/DatabaseTargetTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,32 @@ public void AccessTokenWithInvalidTypeCannotBeSet()
Assert.Throws<FormatException>(() => databaseTarget.OpenConnection(".", null));
}

[Fact]
public void CommandTimeoutShouldBeSet()
{
// Arrange
var commandTimeout = "123";
MockDbConnection.ClearLog();
var databaseTarget = new DatabaseTarget
{
DBProvider = typeof(MockDbConnection).AssemblyQualifiedName,
CommandText = "command1",
};
databaseTarget.CommandProperties.Add(new DatabaseObjectPropertyInfo() { Name = "CommandTimeout", Layout = commandTimeout, PropertyType = typeof(int) });
databaseTarget.Initialize(new LoggingConfiguration());

// Act
var connection = databaseTarget.OpenConnection(".", null);
var command1 = databaseTarget.CreateDbCommand(LogEventInfo.CreateNullEvent(), connection);
var command2 = databaseTarget.CreateDbCommand(LogEventInfo.CreateNullEvent(), connection); // Twice because we use compiled method on 2nd attempt

// Assert
var sqlCommand1 = Assert.IsType<MockDbCommand>(command1);
Assert.Equal(commandTimeout, sqlCommand1.CommandTimeout.ToString()); // Verify dynamic setter method invoke assigns correctly
var sqlCommand2 = Assert.IsType<MockDbCommand>(command2);
Assert.Equal(commandTimeout, sqlCommand2.CommandTimeout.ToString()); // Verify compiled method also assigns correctly
}

[Fact]
public void SqlServerShorthandNotationTest()
{
Expand Down