Merge pull request #27 from Stravaig-Projects/#16/command-timeout

colinangusmackay · web-flow · commit d7d8495ce151 · 2022-02-09T22:26:04.000Z
#16 Add ability to set the command timeout
diff --git a/README.md b/README.md
@@ -37,7 +37,8 @@ with the corresponding information in `appsettings.json`, e.g.:
             "SchemaName": "Stravaig",
             "TableName": "AppConfiguration",
             "RefreshSeconds": 90,
-            "ConnectionStringName": "ConfigDB"
+            "ConnectionStringName": "ConfigDB",
+            "CommandTimeout": 10
         }
     }
 }
diff --git a/Reset-WipReleaseNotes.ps1 b/Reset-WipReleaseNotes.ps1
@@ -13,12 +13,7 @@ $content = @(
 "",
 "### Dependent Packages",
 "",
-"- .NET 5.0",
-"  - No changes",
-"- .NET Core 3.1",
-"  - No changes",
-"- General",
-"  - No changes",
+"- No changes",
 "",
 "---",
 "",
diff --git a/release-notes/wip-release-notes.md b/release-notes/wip-release-notes.md
@@ -4,20 +4,15 @@
 
 Date: ???
 
-### Bugs
-
 ### Features
 
-### Miscellaneous
+- #16: Allow command timeout to be configured.
 
 ### Dependent Packages
 
-- .NET 5.0
-  - No changes
-- .NET Core 3.1
-  - No changes
-- General
-  - No changes
+- coverlet.collector from 3.1.1 to 3.1.2
+- Microsoft.Extensions.Configuration from 6.0.0 to 6.0.1
+- Microsoft.Extensions.Hosting from 6.0.0 to 6.0.1
 
 ---
 
diff --git a/src/Stravaig.Configuration.SqlServer.Tests/FakeDataLoader.cs b/src/Stravaig.Configuration.SqlServer.Tests/FakeDataLoader.cs
@@ -23,8 +23,8 @@ public IDictionary<string, string> RetrieveData(SqlServerConfigurationSource sou
 
 public class FakeSqlServerConfigurationWatcher : ISqlServerConfigurationWatcher
 {
-    ILogger _logger = NullLogger.Instance;
-    private SqlServerConfigurationProvider _provider;
+    ILogger? _logger = NullLogger.Instance;
+    private SqlServerConfigurationProvider? _provider;
     
     public void EnsureStarted()
     {
@@ -35,10 +35,8 @@ public void AttachLogger(ILogger logger)
         _logger = logger;
     }
 
-    public void AttacheProvider(SqlServerConfigurationProvider provider)
+    public void AttachProvider(SqlServerConfigurationProvider provider)
     {
         _provider = provider;
     }
-    
-    
 }
diff --git a/src/Stravaig.Configuration.SqlServer.Tests/SqlServerConfigurationProviderTests.cs b/src/Stravaig.Configuration.SqlServer.Tests/SqlServerConfigurationProviderTests.cs
@@ -103,7 +103,7 @@ public void ReplayLogExceptionOnLoadFailure()
         thirdLog.PropertyDictionary["exceptionMessage"].ShouldBe("Dummy Exception.");
         logs[3].OriginalMessage.ShouldBe("End of replay.");
     }
-    
+
     private SqlServerConfigurationProvider SetupProvider()
     {
         _fakeLoader = new FakeDataLoader()
diff --git a/src/Stravaig.Configuration.SqlServer.Tests/SqlServerConfigurationSourceExtensionTests.cs b/src/Stravaig.Configuration.SqlServer.Tests/SqlServerConfigurationSourceExtensionTests.cs
@@ -0,0 +1,83 @@
+using System;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
+using NUnit.Framework;
+using Shouldly;
+using Stravaig.Configuration.SqlServer.Glue;
+using Stravaig.Extensions.Logging.Diagnostics;
+using Stravaig.Extensions.Logging.Diagnostics.Render;
+
+namespace Stravaig.Configuration.SqlServer.Tests;
+
+[TestFixture]
+public class SqlServerConfigurationSourceExtensionTests
+{
+    private const string DummyConnectionString = "Server=localhost;Database=testing;Connection Timeout=5";
+
+    [Test]
+    public void CreateLoggerWhenNotExpectingALoggerIsNullLogger()
+    {
+        var source = new SqlServerConfigurationSource(DummyConnectionString, expectLogger: false);
+
+        source.CreateLogger().ShouldBe(NullLogger<SqlServerConfigurationProvider>.Instance);
+    }
+    
+    [Test]
+    public void CreateLoggerWhenExpectingALoggerIsReplayLogger()
+    {
+        var source = new SqlServerConfigurationSource(DummyConnectionString, expectLogger: true);
+
+        source.CreateLogger().ShouldBeOfType<ReplayLogger<SqlServerConfigurationProvider>>();
+    }
+    
+    [Test]
+    public void CreateLoggerIndicatesHowTheProviderIsSetup()
+    {
+        var source = new SqlServerConfigurationSource(DummyConnectionString, expectLogger: true, refreshInterval: TimeSpan.FromSeconds(120), commandTimeout: TimeSpan.FromSeconds(10));
+        var replayLogger = (ReplayLogger<SqlServerConfigurationProvider>)source.CreateLogger();
+        var captureLogger = new TestCaptureLogger<SqlServerConfigurationProvider>();
+        replayLogger.Replay(captureLogger);
+
+        var logs = captureLogger.GetLogs();
+        logs.RenderLogs(Formatter.SimpleBySequence, Sink.Console);
+
+        var descriptionLog = logs[1];
+        descriptionLog.OriginalMessage.ShouldBe("SQL Server Configuration Provider will retrieve from [{serverName}].[{databaseName}].[{schemaName}].[{tableName}] {frequency}. Will wait {connectionTimeout} seconds to connect, and {commandTimeout} seconds to retrieve data.");
+        descriptionLog.PropertyDictionary["serverName"].ShouldBe("localhost");
+        descriptionLog.PropertyDictionary["databaseName"].ShouldBe("testing");
+        descriptionLog.PropertyDictionary["schemaName"].ShouldBe("Stravaig");
+        descriptionLog.PropertyDictionary["tableName"].ShouldBe("AppConfiguration");
+        descriptionLog.PropertyDictionary["frequency"].ShouldBe("every 120 seconds");
+        descriptionLog.PropertyDictionary["connectionTimeout"].ShouldBe(5);
+        descriptionLog.PropertyDictionary["commandTimeout"].ShouldBe(10);
+    }
+    
+    [Test]
+    public void CreateLoggerWarnsOfInterleavePossibility()
+    {
+        var source = new SqlServerConfigurationSource(DummyConnectionString, expectLogger: true, refreshInterval: TimeSpan.FromSeconds(12), commandTimeout: TimeSpan.FromSeconds(30));
+        var replayLogger = (ReplayLogger<SqlServerConfigurationProvider>)source.CreateLogger();
+        var captureLogger = new TestCaptureLogger<SqlServerConfigurationProvider>();
+        replayLogger.Replay(captureLogger);
+
+        var logs = captureLogger.GetLogs();
+        logs.RenderLogs(Formatter.SimpleBySequence, Sink.Console);
+
+        var descriptionLog = logs[1];
+        descriptionLog.OriginalMessage.ShouldBe("SQL Server Configuration Provider will retrieve from [{serverName}].[{databaseName}].[{schemaName}].[{tableName}] {frequency}. Will wait {connectionTimeout} seconds to connect, and {commandTimeout} seconds to retrieve data.");
+        descriptionLog.PropertyDictionary["serverName"].ShouldBe("localhost");
+        descriptionLog.PropertyDictionary["databaseName"].ShouldBe("testing");
+        descriptionLog.PropertyDictionary["schemaName"].ShouldBe("Stravaig");
+        descriptionLog.PropertyDictionary["tableName"].ShouldBe("AppConfiguration");
+        descriptionLog.PropertyDictionary["frequency"].ShouldBe("every 12 seconds");
+        descriptionLog.PropertyDictionary["connectionTimeout"].ShouldBe(5);
+        descriptionLog.PropertyDictionary["commandTimeout"].ShouldBe(30);
+
+        var warningLog = logs[2];
+        warningLog.OriginalMessage.ShouldBe("The refresh interval, {refreshInterval} seconds, should be greater than combined connection, {connectionTimeout} seconds, and command, {commandTimeout} seconds, timeouts. A new refresh cycle may start before the previous cycle is complete.");
+        warningLog.LogLevel.ShouldBe(LogLevel.Warning);
+        warningLog.PropertyDictionary["refreshInterval"].ShouldBe(12);
+        warningLog.PropertyDictionary["connectionTimeout"].ShouldBe(5);
+        warningLog.PropertyDictionary["commandTimeout"].ShouldBe(30);
+    }
+}
diff --git a/src/Stravaig.Configuration.SqlServer/Glue/DataLoader.cs b/src/Stravaig.Configuration.SqlServer/Glue/DataLoader.cs
@@ -1,4 +1,5 @@
-﻿using System.Collections.Generic;
+﻿using System;
+using System.Collections.Generic;
 using System.Data.SqlClient;
 
 namespace Stravaig.Configuration.SqlServer.Glue;
@@ -15,6 +16,7 @@ public IDictionary<string, string> RetrieveData(SqlServerConfigurationSource sou
         using SqlConnection connection = new SqlConnection(source.ConnectionString);
         connection.Open();
         var cmd = new SqlCommand(sql, connection);
+        cmd.CommandTimeout = (int)source.CommandTimeout.TotalSeconds;
         using var reader = cmd.ExecuteReader();
         return MaterialiseData(reader);
     }
diff --git a/src/Stravaig.Configuration.SqlServer/Glue/DefaultValues.cs b/src/Stravaig.Configuration.SqlServer/Glue/DefaultValues.cs
@@ -3,6 +3,8 @@ namespace Stravaig.Configuration.SqlServer.Glue;
 internal static class DefaultValues
 {
     public const int NoRefresh = 0;
+    public const int CommandTimeout = 15;
+    public const int ConnectionTimeOut = 15;
     public const string SchemaName = "Stravaig";
     public const string TableName = "AppConfiguration";
     public const string ConfigurationSection = "Stravaig:AppConfiguration";
diff --git a/src/Stravaig.Configuration.SqlServer/Glue/Log.cs b/src/Stravaig.Configuration.SqlServer/Glue/Log.cs
@@ -58,4 +58,16 @@ public static partial class Log
         Level = LogLevel.Warning,
         Message = "The replay limit was reached. There are {excessCount} logs that cannot be replayed.")]
     public static partial void ReplayLimitExceeded(this ILogger logger, int excessCount);
+
+    [LoggerMessage(
+        EventId = 10,
+        Level = LogLevel.Information,
+        Message = "SQL Server Configuration Provider will retrieve from [{serverName}].[{databaseName}].[{schemaName}].[{tableName}] {frequency}. Will wait {connectionTimeout} seconds to connect, and {commandTimeout} seconds to retrieve data.")]
+    public static partial void InitialProviderDescription(this ILogger logger, string serverName, string databaseName, string schemaName, string tableName, string frequency, int connectionTimeout, int commandTimeout);
+    
+    [LoggerMessage(
+        EventId = 11,
+        Level = LogLevel.Warning,
+        Message = "The refresh interval, {refreshInterval} seconds, should be greater than combined connection, {connectionTimeout} seconds, and command, {commandTimeout} seconds, timeouts. A new refresh cycle may start before the previous cycle is complete.")]
+    public static partial void WarnOfCycleInterleave(this ILogger logger, int refreshInterval, int connectionTimeout, int commandTimeout);
 }
diff --git a/src/Stravaig.Configuration.SqlServer/Glue/SourceBuilder.cs b/src/Stravaig.Configuration.SqlServer/Glue/SourceBuilder.cs
@@ -21,11 +21,12 @@ public static SqlServerConfigurationSource BuildSource(IConfigurationBuilder bui
             ApplyFromConnectionStringsSection(options, configRoot.Value);
 
         return new SqlServerConfigurationSource(
-            options.ConnectionString ?? throw new SqlServerConfigurationProviderException("Cannot build a SQL Server Configuration Provider without a connection string."),
-            options.IsLoggerExpected,
-            TimeSpan.FromSeconds(options.RefreshSeconds),
-            options.SchemaName ?? throw new SqlServerConfigurationProviderException("The schema name is required to use SQL Server Configuration."),
-            options.TableName ?? throw new SqlServerConfigurationProviderException("The table name is required to use SQL Server Configuration."));
+            connectionString: options.ConnectionString ?? throw new SqlServerConfigurationProviderException("Cannot build a SQL Server Configuration Provider without a connection string."),
+            expectLogger: options.IsLoggerExpected,
+            commandTimeout: TimeSpan.FromSeconds(options.CommandTimeout),
+            refreshInterval: TimeSpan.FromSeconds(options.RefreshSeconds),
+            schemaName: options.SchemaName ?? throw new SqlServerConfigurationProviderException("The schema name is required to use SQL Server Configuration."),
+            tableName: options.TableName ?? throw new SqlServerConfigurationProviderException("The table name is required to use SQL Server Configuration."));
     }
 
     private static void ApplyFromConnectionStringsSection(SqlServerConfigurationOptions options, IConfigurationRoot configRoot)
diff --git a/src/Stravaig.Configuration.SqlServer/Glue/SqlServerConfigurationSourceExtensions.cs b/src/Stravaig.Configuration.SqlServer/Glue/SqlServerConfigurationSourceExtensions.cs
@@ -0,0 +1,32 @@
+using System;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
+
+namespace Stravaig.Configuration.SqlServer.Glue;
+
+internal static class SqlServerConfigurationSourceExtensions
+{
+    public static ILogger<SqlServerConfigurationProvider> CreateLogger(this SqlServerConfigurationSource source)
+    {
+        if (!source.ExpectLogger) 
+            return NullLogger<SqlServerConfigurationProvider>.Instance;
+
+        var logger = new ReplayLogger<SqlServerConfigurationProvider>();
+        logger.InitialProviderDescription(
+            source.ServerName,
+            source.DatabaseName,
+            source.SchemaName,
+            source.TableName,
+            frequency: source.RefreshInterval == TimeSpan.Zero ? "once only" : $"every {source.RefreshInterval.TotalSeconds} seconds",
+            (int)source.ConnectionTimeout.TotalSeconds,
+            (int)source.CommandTimeout.TotalSeconds);
+        
+        if (source.ConnectionTimeout + source.CommandTimeout > source.RefreshInterval)
+            logger.WarnOfCycleInterleave(
+                (int)source.RefreshInterval.TotalSeconds,
+                (int)source.ConnectionTimeout.TotalSeconds,
+                (int)source.CommandTimeout.TotalSeconds);
+        return logger;
+
+    }
+}
diff --git a/src/Stravaig.Configuration.SqlServer/SqlServerConfigurationOptions.cs b/src/Stravaig.Configuration.SqlServer/SqlServerConfigurationOptions.cs
@@ -15,6 +15,8 @@ public class SqlServerConfigurationOptions
     public string TableName { get; set; } = DefaultValues.TableName;
 
     public int RefreshSeconds { get; set; } = DefaultValues.NoRefresh;
+
+    public int CommandTimeout { get; set; } = DefaultValues.CommandTimeout;
     
     public SqlServerConfigurationOptions FromExistingConfiguration(string configurationSection = DefaultValues.ConfigurationSection)
     {
diff --git a/src/Stravaig.Configuration.SqlServer/SqlServerConfigurationProvider.cs b/src/Stravaig.Configuration.SqlServer/SqlServerConfigurationProvider.cs
@@ -16,7 +16,7 @@ public class SqlServerConfigurationProvider : ConfigurationProvider
     private readonly ISqlServerConfigurationWatcher _watcher;
 
     public SqlServerConfigurationProvider(SqlServerConfigurationSource source)
-        : this (source, new DataLoader(), NullSqlServerConfigurationWatcher.Instance, CreateLogger(source))
+        : this (source, new DataLoader(), NullSqlServerConfigurationWatcher.Instance, source.CreateLogger())
     {
         _watcher = CreateWatcher(source, this);
     }
@@ -34,14 +34,7 @@ internal SqlServerConfigurationProvider(
         _watcher = watcher;
         _watcher.AttachLogger(_logger);
     }
-
-    private static ILogger<SqlServerConfigurationProvider> CreateLogger(SqlServerConfigurationSource source)
-    {
-        return source.ExpectLogger
-            ? new ReplayLogger<SqlServerConfigurationProvider>()
-            : NullLogger<SqlServerConfigurationProvider>.Instance;
-    }
-
+    
     private static ISqlServerConfigurationWatcher CreateWatcher(SqlServerConfigurationSource source, SqlServerConfigurationProvider provider)
     {
         return source.RefreshInterval == TimeSpan.Zero
diff --git a/src/Stravaig.Configuration.SqlServer/SqlServerConfigurationSource.cs b/src/Stravaig.Configuration.SqlServer/SqlServerConfigurationSource.cs
@@ -2,6 +2,7 @@
 using System.Data.SqlClient;
 using System.Text.RegularExpressions;
 using Microsoft.Extensions.Configuration;
+using Stravaig.Configuration.SqlServer.Glue;
 
 namespace Stravaig.Configuration.SqlServer;
 
@@ -13,14 +14,25 @@ public class SqlServerConfigurationSource : IConfigurationSource
     public string TableName { get; private set; }
     public TimeSpan RefreshInterval { get; private set; }
     public bool ExpectLogger { get; private set; }
+    
+    public TimeSpan CommandTimeout { get; private set; }
 
     private readonly Lazy<string> _serverName;
     public string ServerName => _serverName.Value;
 
     private readonly Lazy<string> _databaseName;
     public string DatabaseName => _databaseName.Value;
 
-    public SqlServerConfigurationSource(string connectionString, bool expectLogger = false, TimeSpan? refreshInterval = null, string schemaName = "Stravaig", string tableName = "AppConfiguration")
+    private readonly Lazy<TimeSpan> _connectionTimeout;
+    public TimeSpan ConnectionTimeout => _connectionTimeout.Value;
+
+    public SqlServerConfigurationSource(
+        string connectionString,
+        bool expectLogger = false,
+        TimeSpan? commandTimeout = null,
+        TimeSpan? refreshInterval = null,
+        string schemaName = "Stravaig",
+        string tableName = "AppConfiguration")
     {
         ValidateConnectionString(connectionString);
         ValidateSchemaName(schemaName);
@@ -30,9 +42,11 @@ public SqlServerConfigurationSource(string connectionString, bool expectLogger =
         SchemaName = schemaName;
         TableName = tableName;
         RefreshInterval = refreshInterval ?? TimeSpan.Zero;
+        CommandTimeout = commandTimeout ?? TimeSpan.FromSeconds(DefaultValues.CommandTimeout);
 
         _serverName = new Lazy<string>(() => new SqlConnectionStringBuilder(connectionString).DataSource);
         _databaseName = new Lazy<string>(() => new SqlConnectionStringBuilder(connectionString).InitialCatalog);
+        _connectionTimeout = new Lazy<TimeSpan>(() => TimeSpan.FromSeconds(new SqlConnectionStringBuilder(connectionString).ConnectTimeout));
     }
 
     private void ValidateTableName(string tableName)
diff --git a/version.txt b/version.txt
@@ -1 +1 @@
-0.3.1
+0.4.0

Original file line number	Diff line number	Diff line change
@@ -37,7 +37,8 @@ with the corresponding information in `appsettings.json`, e.g.:
`37`	`37`	`"SchemaName": "Stravaig",`
`38`	`38`	`"TableName": "AppConfiguration",`
`39`	`39`	`"RefreshSeconds": 90,`
`40`		`- "ConnectionStringName": "ConfigDB"`
	`40`	`+ "ConnectionStringName": "ConfigDB",`
	`41`	`+ "CommandTimeout": 10`
`41`	`42`	`}`
`42`	`43`	`}`
`43`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,8 @@ public IDictionary<string, string> RetrieveData(SqlServerConfigurationSource sou`
`23`	`23`
`24`	`24`	`public class FakeSqlServerConfigurationWatcher : ISqlServerConfigurationWatcher`
`25`	`25`	`{`
`26`		`- ILogger _logger = NullLogger.Instance;`
`27`		`- private SqlServerConfigurationProvider _provider;`
	`26`	`+ ILogger? _logger = NullLogger.Instance;`
	`27`	`+ private SqlServerConfigurationProvider? _provider;`
`28`	`28`
`29`	`29`	`public void EnsureStarted()`
`30`	`30`	`{`
`@@ -35,10 +35,8 @@ public void AttachLogger(ILogger logger)`
`35`	`35`	`_logger = logger;`
`36`	`36`	`}`
`37`	`37`
`38`		`- public void AttacheProvider(SqlServerConfigurationProvider provider)`
	`38`	`+ public void AttachProvider(SqlServerConfigurationProvider provider)`
`39`	`39`	`{`
`40`	`40`	`_provider = provider;`
`41`	`41`	`}`
`42`		`-`
`43`		`-`
`44`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ public void ReplayLogExceptionOnLoadFailure()`
`103`	`103`	`thirdLog.PropertyDictionary["exceptionMessage"].ShouldBe("Dummy Exception.");`
`104`	`104`	`logs[3].OriginalMessage.ShouldBe("End of replay.");`
`105`	`105`	`}`
`106`		`-`
	`106`	`+`
`107`	`107`	`private SqlServerConfigurationProvider SetupProvider()`
`108`	`108`	`{`
`109`	`109`	`_fakeLoader = new FakeDataLoader()`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,8 @@ namespace Stravaig.Configuration.SqlServer.Glue;`
`3`	`3`	`internal static class DefaultValues`
`4`	`4`	`{`
`5`	`5`	`public const int NoRefresh = 0;`
	`6`	`+ public const int CommandTimeout = 15;`
	`7`	`+ public const int ConnectionTimeOut = 15;`
`6`	`8`	`public const string SchemaName = "Stravaig";`
`7`	`9`	`public const string TableName = "AppConfiguration";`
`8`	`10`	`public const string ConfigurationSection = "Stravaig:AppConfiguration";`