Merge pull request #288 from ckadluba/refactor-SinkTraits

Refactoring, tests, fixes

Author: ckadluba

src/Serilog.Sinks.MSSqlServer/GlobalSuppressions.cs

@@ -7,6 +7,7 @@
 
 [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")]
 [assembly: SuppressMessage("Security", "CA2100:Review SQL queries for security vulnerabilities", Justification = "Too hard to change. Accepted for now.", Scope = "namespaceanddescendants", Target = "Serilog.Sinks.MSSqlServer")]
+[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.SetProperty.IfNotNull``1(System.String,Serilog.Sinks.MSSqlServer.SetProperty.PropertySetter{``0})")]
 [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)")]
 [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")]
 [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")]

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

@@ -21,6 +21,7 @@ internal static SinkDependencies Create(
 
             var sqlConnectionFactory =
                 new SqlConnectionFactory(connectionString,
+                    sinkOptions?.UseAzureManagedIdentity ?? default,
                     new AzureManagedServiceAuthenticator(
                         sinkOptions?.UseAzureManagedIdentity ?? default,
                         sinkOptions.AzureServiceTokenProviderResource));

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/AzureManagedServiceAuthenticator.cs

@@ -1,9 +1,5 @@
 using System;
-#if NET452
-using System.Data.SqlClient;
-#else
-using Microsoft.Data.SqlClient;
-#endif
+using System.Threading.Tasks;
 using Microsoft.Azure.Services.AppAuthentication;
 
 namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Platform
@@ -26,15 +22,14 @@ public AzureManagedServiceAuthenticator(bool useAzureManagedIdentity, string azu
             _azureServiceTokenProvider = new AzureServiceTokenProvider();
         }
 
-        public void SetAuthenticationToken(SqlConnection sqlConnection)
+        public Task<string> GetAuthenticationToken()
         {
             if (!_useAzureManagedIdentity)
             {
-                return;
+                return Task.FromResult((string)null);
             }
 
-            sqlConnection.AccessToken = _azureServiceTokenProvider.GetAccessTokenAsync(
-                _azureServiceTokenProviderResource).GetAwaiter().GetResult();
+            return _azureServiceTokenProvider.GetAccessTokenAsync(_azureServiceTokenProviderResource);
         }
     }
 }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/AzureManagedServiceAuthenticatorStub.cs

@@ -1,11 +1,6 @@
 using System;
-#if NET452
-using System.Data.SqlClient;
-#else
-using Microsoft.Data.SqlClient;
-#endif
-
 using System.Diagnostics.CodeAnalysis;
+using System.Threading.Tasks;
 
 // This is an empty stub implementaion of IAzureManagedServiceAuthenticator for the target frameworks
 // that don't support Azure Managed Identities (net452, net461, netstandard2.0, netcoreapp2.0).
@@ -29,8 +24,6 @@ public AzureManagedServiceAuthenticator(bool useAzureManagedIdentity, string azu
             _azureServiceTokenProviderResource = azureServiceTokenProviderResource;
         }
 
-        public void SetAuthenticationToken(SqlConnection sqlConnection)
-        {
-        }
+        public Task<string> GetAuthenticationToken() => Task.FromResult((string)null);
     }
 }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/IAzureManagedServiceAuthenticator.cs

@@ -1,13 +1,9 @@
-#if NET452
-using System.Data.SqlClient;
-#else
-using Microsoft.Data.SqlClient;
-#endif
+using System.Threading.Tasks;
 
 namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Platform
 {
     internal interface IAzureManagedServiceAuthenticator
     {
-        void SetAuthenticationToken(SqlConnection sqlConnection);
+        Task<string> GetAuthenticationToken();
     }
 }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlConnectionFactory.cs

@@ -1,13 +1,9 @@
-#if NET452
-using System.Data.SqlClient;
-#else
-using Microsoft.Data.SqlClient;
-#endif
+using Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Platform.SqlClient;
 
 namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Platform
 {
     internal interface ISqlConnectionFactory
     {
-        SqlConnection Create();
+        ISqlConnectionWrapper Create();
     }
 }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlBulkBatchWriter.cs

@@ -1,11 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Data;
-#if NET452
-using System.Data.SqlClient;
-#else
-using Microsoft.Data.SqlClient;
-#endif
 using System.Globalization;
 using System.Linq;
 using System.Threading.Tasks;
@@ -39,25 +34,20 @@ public SqlBulkBatchWriter(
 
         public async Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable)
         {
-            // Copy the events to the data table
-            FillDataTable(events, dataTable);
-
             try
             {
+                FillDataTable(events, dataTable);
+
                 using (var cn = _sqlConnectionFactory.Create())
                 {
                     await cn.OpenAsync().ConfigureAwait(false);
-                    using (var copy = _disableTriggers
-                            ? new SqlBulkCopy(cn)
-                            : new SqlBulkCopy(cn, SqlBulkCopyOptions.CheckConstraints | SqlBulkCopyOptions.FireTriggers, null)
-                    )
+                    using (var copy = cn.CreateSqlBulkCopy(_disableTriggers,
+                        string.Format(CultureInfo.InvariantCulture, "[{0}].[{1}]", _schemaName, _tableName)))
                     {
-                        copy.DestinationTableName = string.Format(CultureInfo.InvariantCulture, "[{0}].[{1}]", _schemaName, _tableName);
                         foreach (var column in dataTable.Columns)
                         {
                             var columnName = ((DataColumn)column).ColumnName;
-                            var mapping = new SqlBulkCopyColumnMapping(columnName, columnName);
-                            copy.ColumnMappings.Add(mapping);
+                            copy.AddSqlBulkCopyColumnMapping(columnName, columnName);
                         }
 
                         await copy.WriteToServerAsync(dataTable).ConfigureAwait(false);
@@ -70,7 +60,6 @@ public async Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable)
             }
             finally
             {
-                // Processed the items, clear for the next run
                 dataTable.Clear();
             }
         }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlBulkCopyWrapper.cs

@@ -0,0 +1,12 @@
+using System;
+using System.Data;
+using System.Threading.Tasks;
+
+namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Platform.SqlClient
+{
+    internal interface ISqlBulkCopyWrapper : IDisposable
+    {
+        void AddSqlBulkCopyColumnMapping(string sourceColumn, string destinationColumn);
+        Task WriteToServerAsync(DataTable table);
+    }
+}

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs

@@ -0,0 +1,14 @@
+using System;
+using System.Data;
+
+namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Platform.SqlClient
+{
+    internal interface ISqlCommandWrapper : IDisposable
+    {
+        CommandType CommandType { get; set; }
+        string CommandText { get; set; }
+
+        void AddParameter(string parameterName, object value);
+        int ExecuteNonQuery();
+    }
+}

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlConnectionWrapper.cs

@@ -0,0 +1,16 @@
+using System;
+using System.Threading.Tasks;
+
+namespace Serilog.Sinks.MSSqlServer.Sinks.MSSqlServer.Platform.SqlClient
+{
+    internal interface ISqlConnectionWrapper : IDisposable
+    {
+        string ConnectionString { get; }
+
+        void Open();
+        Task OpenAsync();
+        ISqlCommandWrapper CreateCommand();
+        ISqlCommandWrapper CreateCommand(string cmdText);
+        ISqlBulkCopyWrapper CreateSqlBulkCopy(bool disableTriggers, string destinationTableName);
+    }
+}