Merge pull request #188 from OctopusDeploy/core/change-default-select…

…-behaviour Change default TableColumnNameResolver back to Select All
OctopusDeploy · Mar 8, 2022 · 9119575 · 9119575
2 parents de7c60e + 87d60ef
commit 9119575
Show file tree

Hide file tree

Showing 8 changed files with 92 additions and 11 deletions.
diff --git a/source/Nevermore.IntegrationTests/Advanced/JsonLastTableColumnResolverFixture.cs b/source/Nevermore.IntegrationTests/Advanced/JsonLastTableColumnResolverFixture.cs
@@ -0,0 +1,28 @@
+using System;
+using FluentAssertions;
+using Nevermore.IntegrationTests.Model;
+using Nevermore.IntegrationTests.SetUp;
+using Nevermore.TableColumnNameResolvers;
+using NUnit.Framework;
+
+namespace Nevermore.IntegrationTests.Advanced
+{
+    public class JsonLastTableColumnResolverFixture : FixtureWithRelationalStore
+    {
+        public override void OneTimeSetUp()
+        {
+            base.OneTimeSetUp();
+            NoMonkeyBusiness();
+            Configuration.TableColumnNameResolver = executor => new JsonLastTableColumnNameResolver(executor);
+        }
+
+        [Test]
+        public void ShouldSelectAllColumnNamesWithJsonLast()
+        {
+            using var readTransaction = Store.BeginReadTransaction();
+            var selectQuery = readTransaction.Query<Customer>().DebugViewRawQuery();
+
+            selectQuery.Should().Be($"SELECT Id,FirstName,LastName,Nickname,Roles,Balance,IsVip,JSON{Environment.NewLine}FROM [TestSchema].[Customer]{Environment.NewLine}ORDER BY [Id]");
+        }
+    }
+}
diff --git a/source/Nevermore.IntegrationTests/Advanced/MissingTableFixture.cs b/source/Nevermore.IntegrationTests/Advanced/MissingTableFixture.cs
@@ -2,6 +2,7 @@
 using FluentAssertions;
 using Nevermore.IntegrationTests.SetUp;
 using Nevermore.Mapping;
+using Nevermore.TableColumnNameResolvers;
 using NUnit.Framework;
 
 namespace Nevermore.IntegrationTests.Advanced
@@ -26,6 +27,7 @@ public override void OneTimeSetUp()
             base.OneTimeSetUp();
             NoMonkeyBusiness();
             Configuration.DocumentMaps.Register(new MissingMap());
+            Configuration.TableColumnNameResolver = queryExecutor => new JsonLastTableColumnNameResolver(queryExecutor);
         }
 
         [Test]

diff --git a/source/Nevermore.IntegrationTests/SetUp/SchemaGenerator.cs b/source/Nevermore.IntegrationTests/SetUp/SchemaGenerator.cs
@@ -17,14 +17,12 @@ public static void WriteTableSchema(DocumentMap mapping, string tableNameOverrid
             var identity = mapping.IsIdentityId ? " IDENTITY(1,1)" : null;
             result.Append($"  [Id] {GetDatabaseType(mapping.IdColumn)}{identity} NOT NULL CONSTRAINT [PK_{tableName}_Id] PRIMARY KEY CLUSTERED, ").AppendLine();
 
-            // purposely put the [JSON] column second as we want to ensure that tables don't have to be created
-            // with the type before the [JSON] column as was a previous restriction
-            result.AppendFormat("  [JSON] NVARCHAR(MAX) NOT NULL").AppendLine(); 
-
             foreach (var column in mapping.WritableIndexedColumns())
             {
-                result.AppendFormat("  ,[{0}] {1} {2} ", column.ColumnName, GetDatabaseType(column).ToUpperInvariant(), IsNullable(column) ? "NULL" : "NOT NULL").AppendLine();
+                result.AppendFormat("  [{0}] {1} {2}, ", column.ColumnName, GetDatabaseType(column).ToUpperInvariant(), IsNullable(column) ? "NULL" : "NOT NULL").AppendLine();
             }
+
+            result.AppendFormat("  [JSON] NVARCHAR(MAX) NOT NULL").AppendLine(); 
 
             if (mapping.IsRowVersioningEnabled)
                 result.Append("  ,[RowVersion] TIMESTAMP").AppendLine();

diff --git a/source/Nevermore.Tests/CachingTableColumnNamesFixture.cs b/source/Nevermore.Tests/CachingTableColumnNamesFixture.cs
@@ -0,0 +1,56 @@
+using System;
+using System.Collections.Generic;
+using FluentAssertions;
+using Nevermore.TableColumnNameResolvers;
+using NUnit.Framework;
+
+namespace Nevermore.Tests
+{
+    public class CachingTableColumnNamesFixture
+    {
+        class MockTableNameResolverForCaching : ITableColumnNameResolver
+        {
+            public static int TimesQueried;
+
+            readonly Dictionary<string, string[]> tableToColumnNames;
+
+            public MockTableNameResolverForCaching(Dictionary<string, string[]> tableToColumnNames)
+            {
+                this.tableToColumnNames = tableToColumnNames;
+            }
+
+            public string[] GetColumnNames(string schemaName, string tableName)
+            {
+                TimesQueried++;
+                if (tableToColumnNames.ContainsKey(tableName))
+                {
+                    return tableToColumnNames[tableName];    
+                }
+
+                throw new Exception($"Column names for table {tableName} were not specified in creation");
+            }
+        }
+
+        [Test]
+        public void ShouldCacheColumnNameForSchema()
+        {
+            const string tableName = "VideoGame";
+            var columnNames = new[] {"Title", "Genre", "ReleaseDate"};
+
+            var tableCache = new TableColumnsCache();
+            var map = new Dictionary<string, string[]>();
+            map.Add(tableName, columnNames);
+            var cachingColumnNameResolvers = new CachingTableColumnNameResolver(
+                new MockTableNameResolverForCaching(map), tableCache);
+
+            var columns = cachingColumnNameResolvers.GetColumnNames("", tableName);
+            columns.Should().BeEquivalentTo(columnNames);
+
+            // Query again to hit the caching
+            cachingColumnNameResolvers.GetColumnNames("", tableName);
+            columns.Should().BeEquivalentTo(columnNames);
+
+            MockTableNameResolverForCaching.TimesQueried.Should().Be(1);
+        }
+    }
+}
diff --git a/source/Nevermore/RelationalStore.cs b/source/Nevermore/RelationalStore.cs
@@ -17,14 +17,12 @@ public class RelationalStore : IRelationalStore
     {
         readonly Lazy<RelationalTransactionRegistry> registry;
         readonly Lazy<KeyAllocator> keyAllocator;
-        readonly ITableColumnsCache tableColumnsCache;
 
         public RelationalStore(IRelationalStoreConfiguration configuration)
         {
             Configuration = configuration;
             registry = new Lazy<RelationalTransactionRegistry>(() => new RelationalTransactionRegistry(new SqlConnectionStringBuilder(configuration.ConnectionString)));
             keyAllocator = new Lazy<KeyAllocator>(() => new KeyAllocator(this, configuration.KeyBlockSize));
-            tableColumnsCache = new TableColumnsCache();
         }
 
         public void WriteCurrentTransactions(StringBuilder output) => registry.Value.WriteCurrentTransactions(output);

diff --git a/source/Nevermore/RelationalStoreConfiguration.cs b/source/Nevermore/RelationalStoreConfiguration.cs
@@ -50,8 +50,7 @@ public RelationalStoreConfiguration(Func<string> connectionStringFunc)
 
             DocumentMaps = new DocumentMapRegistry(PrimaryKeyHandlers);
 
-            var tableColumnsCache = new TableColumnsCache();
-            TableColumnNameResolver = queryExecutor => new CachingTableColumnNameResolver(new JsonLastTableColumnNameResolver(queryExecutor), tableColumnsCache);
+            TableColumnNameResolver = _ => new SelectAllColumnsTableResolver();
 
             AllowSynchronousOperations = true;
 

diff --git a/source/Nevermore/TableColumnNameResolvers/CachingTableColumnNameResolver.cs b/source/Nevermore/TableColumnNameResolvers/CachingTableColumnNameResolver.cs
@@ -2,7 +2,7 @@
 
 namespace Nevermore.TableColumnNameResolvers
 {
-    internal class CachingTableColumnNameResolver : ITableColumnNameResolver
+    public class CachingTableColumnNameResolver : ITableColumnNameResolver
     {
         readonly ITableColumnNameResolver inner;
         readonly ITableColumnsCache tableColumnsCache;

diff --git a/source/Nevermore/TableColumnNameResolvers/JsonLastTableColumnNameResolver.cs b/source/Nevermore/TableColumnNameResolvers/JsonLastTableColumnNameResolver.cs
@@ -3,7 +3,7 @@
 
 namespace Nevermore.TableColumnNameResolvers
 {
-    internal class JsonLastTableColumnNameResolver : ITableColumnNameResolver
+    public class JsonLastTableColumnNameResolver : ITableColumnNameResolver
     {
         readonly IReadQueryExecutor queryExecutor;