Merge pull request #362 from Particular/timeout-manager-index-check-f…

…ixes

Fixing TimeoutManager index validator for quoted schemas

src/NServiceBus.NHibernate.Tests/TimeoutPersister/When_checking_db_indexes.cs

@@ -0,0 +1,123 @@
+namespace NServiceBus.NHibernate.Tests.TimeoutPersister
+{
+    using System.Data.SqlClient;
+    using System.Threading.Tasks;
+    using global::NHibernate.Cfg;
+    using global::NHibernate.Dialect;
+    using global::NHibernate.Mapping.ByCode;
+    using global::NHibernate.Tool.hbm2ddl;
+    using NUnit.Framework;
+    using TimeoutPersisters.NHibernate.Config;
+    using TimeoutPersisters.NHibernate.Installer;
+
+    class When_checking_db_indexes
+    {
+        SchemaExport schemaExport;
+
+        string dbSchemaName;
+        bool dbSchemaNeedsQuoting;
+
+        [Test]
+        public async Task Should_detect_existing_TimeoutEntity_index_in_default_configuration()
+        {
+            var configuration = await CreateTimeoutManagerObjects();
+
+            var validationResult = new TimeoutsIndexValidator(configuration).Validate();
+
+            Assert.IsTrue(validationResult.IsValid, "Validation should pass for default db structure.");
+        }
+
+        [Test]
+        public async Task Should_detect_missing_TimeoutEntity_index_in_unquoted_schema()
+        {
+            dbSchemaName = "some_schema";
+
+            var configuration = await CreateTimeoutManagerObjects();
+            await DropIndex();
+
+            var validationResult = new TimeoutsIndexValidator(configuration).Validate();
+
+            Assert.IsFalse(validationResult.IsValid, "Validation should fail if the index is missing.");
+        }
+
+        [Test]
+        public async Task Should_detect_existing_TimeoutEntity_index_in_quoted_schema()
+        {
+            dbSchemaName = "quoted-schema";
+            dbSchemaNeedsQuoting = true;
+
+            var configuration = await CreateTimeoutManagerObjects();
+
+            var validationResult = new TimeoutsIndexValidator(configuration).Validate();
+
+            Assert.IsTrue(validationResult.IsValid, "Validation should pass for existing index in quoted schema.");
+        }
+
+        [Test]
+        public async Task Should_detect_missing_TimeoutEntity_index_in_quoted_schema()
+        {
+            dbSchemaName = "quoted-schema";
+            dbSchemaNeedsQuoting = true;
+
+            var configuration = await CreateTimeoutManagerObjects();
+            await DropIndex();
+
+            var validationResult = new TimeoutsIndexValidator(configuration).Validate();
+
+            Assert.IsFalse(validationResult.IsValid, "Validation should fail if index is missing in quoted schema.");
+        }
+
+        async Task<Configuration> CreateTimeoutManagerObjects()
+        {
+            var configuration = new Configuration()
+                .DataBaseIntegration(x =>
+                {
+                    x.Dialect<MsSql2012Dialect>();
+                    x.ConnectionString = Consts.SqlConnectionString;
+                });
+
+            if (dbSchemaName != null)
+            {
+                await CreateDbSchema();
+
+                configuration.SetProperty(Environment.DefaultSchema, dbSchemaNeedsQuoting ? $"[{dbSchemaName}]" : dbSchemaName);
+            }
+
+            var mapper = new ModelMapper();
+            mapper.AddMapping<TimeoutEntityMap>();
+
+            configuration.AddMapping(mapper.CompileMappingForAllExplicitlyAddedEntities());
+
+            schemaExport = new SchemaExport(configuration);
+            await schemaExport.CreateAsync(false, true);
+            return configuration;
+        }
+
+        Task CreateDbSchema() => ExecuteCommand($"IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = N'{dbSchemaName}') EXEC('CREATE SCHEMA [{dbSchemaName}] AUTHORIZATION [dbo]');");
+
+        Task DropDbSchema() => ExecuteCommand($"DROP SCHEMA [{dbSchemaName}]");
+
+        Task DropIndex() => ExecuteCommand($"DROP INDEX {TimeoutEntityMap.EndpointIndexName} ON [{dbSchemaName}].[TimeoutEntity]");
+
+        static async Task ExecuteCommand(string commandText)
+        {
+            using (var connection = new SqlConnection(Consts.SqlConnectionString))
+            {
+                await connection.OpenAsync();
+
+                await new SqlCommand(commandText, connection).ExecuteNonQueryAsync();
+            }
+        }
+
+        [TearDown]
+        public async Task TearDown()
+        {
+             await schemaExport.DropAsync(false, true);
+
+             if (dbSchemaName != null)
+             {
+                 await DropDbSchema();
+             }
+        }
+    }
+}

src/NServiceBus.NHibernate/TimeoutPersisters/NHibernateTimeoutStorage.cs

@@ -3,6 +3,7 @@ namespace NServiceBus.Features
     using System;
     using System.Threading.Tasks;
     using global::NHibernate.Cfg;
+    using Logging;
     using Persistence.NHibernate;
     using TimeoutPersisters.NHibernate;
     using TimeoutPersisters.NHibernate.Config;
@@ -67,14 +68,31 @@ static bool RunInstaller(FeatureConfigurationContext context)
 
         class DetectIncorrectIndexesStartupTask : FeatureStartupTask
         {
+            static readonly ILog Logger = LogManager.GetLogger(typeof(DetectIncorrectIndexesStartupTask));
+
             public DetectIncorrectIndexesStartupTask(Configuration configuration)
             {
                 this.configuration = configuration;
             }
 
             protected override Task OnStart(IMessageSession session)
             {
-                new IncorrectIndexDetector(configuration).LogWarningIfTimeoutEntityIndexIsIncorrect();
+                var result = new TimeoutsIndexValidator(configuration).Validate();
+
+                if (result.IsValid)
+                {
+                    return Task.FromResult(0);
+                }
+
+                if (result.Exception != null)
+                {
+                    Logger.Warn(result.ErrorDescription, result.Exception);
+                }
+                else
+                {
+                    Logger.Warn(result.ErrorDescription);
+                }
+
                 return Task.FromResult(0);
             }
 

src/NServiceBus.NHibernate/TimeoutPersisters/IncorrectIndexDetector.cs → src/NServiceBus.NHibernate/TimeoutPersisters/TimeoutsIndexValidator.cs

@@ -12,11 +12,11 @@
     using System.Data.Common;
     using System.Linq;
 
-    class IncorrectIndexDetector
+    class TimeoutsIndexValidator
     {
-        static readonly ILog Logger = LogManager.GetLogger(typeof(IncorrectIndexDetector));
+        static readonly ILog Logger = LogManager.GetLogger(typeof(TimeoutsIndexValidator));
 
-        public IncorrectIndexDetector(Configuration configuration)
+        public TimeoutsIndexValidator(Configuration configuration)
         {
             dialect = Dialect.GetDialect(configuration.Properties);
             connectionHelper = new ManagedProviderConnectionHelper(MergeProperties(configuration.Properties));
@@ -33,7 +33,7 @@ Dictionary<string, string> MergeProperties(IDictionary<string, string> propertie
             return props;
         }
 
-        public void LogWarningIfTimeoutEntityIndexIsIncorrect()
+        public IndexValidationResult Validate()
         {
             try
             {
@@ -43,24 +43,38 @@ public void LogWarningIfTimeoutEntityIndexIsIncorrect()
                 var index = GetIndex(connection, timeoutEntityMapping, TimeoutEntityMap.EndpointIndexName);
                 if (index == null)
                 {
-                    Logger.Warn($"Could not find {TimeoutEntityMap.EndpointIndexName} index. This may cause significant performance degradation of message deferral. Consult NServiceBus NHibernate persistence documentation for details on how to create this index.");
-                    return;
+                    return new IndexValidationResult
+                    {
+                        IsValid = false,
+                        ErrorDescription = $"Could not find {TimeoutEntityMap.EndpointIndexName} index. This may cause significant performance degradation of message deferral. Consult NServiceBus NHibernate persistence documentation for details on how to create this index."
+                    };
                 }
 
-                var indexCorrect = index.ColumnsCount == 2
+                Logger.Debug($"Detected {TimeoutEntityMap.EndpointIndexName} ({index.ColumnNameAt(1)}, {index.ColumnNameAt(2)})");
+
+                var validColumns = index.ColumnsCount == 2
                     && index.ColumnNameAt(1).Equals(nameof(TimeoutEntity.Endpoint), StringComparison.InvariantCultureIgnoreCase)
                     && index.ColumnNameAt(2).Equals(nameof(TimeoutEntity.Time), StringComparison.InvariantCultureIgnoreCase);
 
-                Logger.Debug($"Detected {TimeoutEntityMap.EndpointIndexName} ({index.ColumnNameAt(1)}, {index.ColumnNameAt(2)})");
-
-                if (!indexCorrect)
+                if (!validColumns)
                 {
-                    Logger.Warn($"The {TimeoutEntityMap.EndpointIndexName} index has incorrect column order. This may cause significant performance degradation of message deferral. Consult NServiceBus NHibernate persistence documentation for details on how to create this index.");
+                    return new IndexValidationResult
+                    {
+                        IsValid = false,
+                        ErrorDescription = $"The {TimeoutEntityMap.EndpointIndexName} index has incorrect column order. This may cause significant performance degradation of message deferral. Consult NServiceBus NHibernate persistence documentation for details on how to create this index."
+                    };
                 }
+
+                return new IndexValidationResult {IsValid = true};
             }
             catch (Exception e)
             {
-                Logger.Warn($"Could not inspect {TimeoutEntityMap.EndpointIndexName} index definition.", e);
+                return new IndexValidationResult
+                {
+                    IsValid = false,
+                    ErrorDescription = $"Could not inspect {TimeoutEntityMap.EndpointIndexName} index definition.",
+                    Exception = e
+                };
             }
             finally
             {
@@ -75,19 +89,33 @@ public void LogWarningIfTimeoutEntityIndexIsIncorrect()
             }
         }
 
-        private IIndex GetIndex(DbConnection connection, PersistentClass entity, string indexName)
+        IIndex GetIndex(DbConnection connection, PersistentClass entity, string indexName)
         {
-
-
             // Check if running on SQL Server.
             if (typeof(MsSql2005Dialect).IsAssignableFrom(dialect.GetType()))
             {
-                var restrictions = new string[5]
+                string EnsureUnquoted(string name)
                 {
-                    entity.Table.Catalog,
-                    entity.Table.Schema,
-                    entity.Table.Name,
-                    indexName,
+                    if (null == name || name.Length < 2)
+                    {
+                        return name;
+                    }
+
+                    if ('[' == name[0] && ']' == name[name.Length - 1]) // quoted?
+                    {
+                        name = name.Substring(1, name.Length - 2); // remove outer brackets
+                        name = name.Replace("]]", "]"); // un-escape right-bracket
+                    }
+
+                    return name;
+                }
+
+                var restrictions = new []
+                {
+                    EnsureUnquoted(entity.Table.Catalog),
+                    EnsureUnquoted(entity.Table.Schema),
+                    EnsureUnquoted(entity.Table.Name),
+                    EnsureUnquoted(indexName),
                     null
                 };
                 return new SqlServerIndex(connection.GetSchema("IndexColumns", restrictions));
@@ -109,6 +137,15 @@ private IIndex GetIndex(DbConnection connection, PersistentClass entity, string
             return null;
         }
 
+        public class IndexValidationResult
+        {
+            public bool IsValid { get; set; }
+
+            public string ErrorDescription { get; set; }
+
+            public Exception Exception { get; set; }
+        }
+
         interface IIndex
         {
             int ColumnsCount { get; }