Change abstraction points for reverse engineering so SQL Server and S…

…Qlite don't both implement the same IMetadataReader => IModel call
dotnet · Oct 15, 2015 · 1e857bc · 1e857bc
1 parent 929f542
commit 1e857bc
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 75 deletions.
diff --git a/...yFramework.MicrosoftSqlServer.Design/ReverseEngineering/SqlServerMetadataModelProvider.cs b/...yFramework.MicrosoftSqlServer.Design/ReverseEngineering/SqlServerMetadataModelProvider.cs
@@ -18,7 +18,6 @@ namespace Microsoft.Data.Entity.SqlServer.Design.ReverseEngineering
 {
     public class SqlServerMetadataModelProvider : RelationalMetadataModelProvider
     {
-        private readonly IMetadataReader _metadataReader;
         private readonly SqlServerLiteralUtilities _sqlServerLiteralUtilities;
 
         private const int DefaultDateTimePrecision = 7;
@@ -29,24 +28,13 @@ public SqlServerMetadataModelProvider(
             [NotNull] IRelationalTypeMapper typeMapper,
             [NotNull] IMetadataReader metadataReader,
             [NotNull] SqlServerLiteralUtilities sqlServerLiteralUtilities)
-            : base(loggerFactory, typeMapper)
+            : base(loggerFactory, typeMapper, metadataReader)
         {
-            Check.NotNull(metadataReader, nameof(metadataReader));
             Check.NotNull(sqlServerLiteralUtilities, nameof(sqlServerLiteralUtilities));
 
-            _metadataReader = metadataReader;
             _sqlServerLiteralUtilities = sqlServerLiteralUtilities;
         }
 
-        public override IModel GetModel([NotNull] string connectionString, [CanBeNull] TableSelectionSet tableSelectionSet)
-        {
-            Check.NotEmpty(connectionString, nameof(connectionString));
-
-            var schemaInfo = _metadataReader.GetSchema(connectionString, tableSelectionSet ?? TableSelectionSet.InclusiveAll);
-
-            return GetModel(schemaInfo);
-        }
-
         protected override PropertyBuilder AddColumn([NotNull] EntityTypeBuilder builder, [NotNull] Column column)
         {
             var propertyBuilder = base.AddColumn(builder, column);
@@ -55,12 +43,6 @@ protected override PropertyBuilder AddColumn([NotNull] EntityTypeBuilder builder
 
             AddSqlServerDefaultValue(column, propertyBuilder);
 
-            if (column.DataType == "timestamp")
-            {
-                // synonym for rowversion
-                propertyBuilder.ValueGeneratedOnAddOrUpdate();
-            }
-
             return propertyBuilder;
         }
 

diff --git a/src/EntityFramework.MicrosoftSqlServer.Design/ReverseEngineering/SqlServerMetadataReader.cs b/src/EntityFramework.MicrosoftSqlServer.Design/ReverseEngineering/SqlServerMetadataReader.cs
@@ -155,7 +155,7 @@ RIGHT JOIN (SELECT * FROM sys.indexes WHERE is_primary_key = 1) AS i ON i.object
                         Scale = reader.IsDBNull(9) ? default(int?) : reader.GetInt32(9),
                         MaxLength = maxLength <= 0 ? default(int?) : maxLength,
                         IsIdentity = !reader.IsDBNull(11) && reader.GetBoolean(11),
-                        IsComputed = reader.GetBoolean(12)
+                        IsComputed = reader.GetBoolean(12) || dataTypeName == "timestamp"
                     };
 
                     table.Columns.Add(column);

diff --git a/src/EntityFramework.Relational.Design/ReverseEngineering/RelationalMetadataModelProvider.cs b/src/EntityFramework.Relational.Design/ReverseEngineering/RelationalMetadataModelProvider.cs
@@ -10,6 +10,7 @@
 using Microsoft.Data.Entity.Metadata.Builders;
 using Microsoft.Data.Entity.Metadata.Conventions;
 using Microsoft.Data.Entity.Relational.Design.Model;
+using Microsoft.Data.Entity.Relational.Design.ReverseEngineering.Internal;
 using Microsoft.Data.Entity.Storage;
 using Microsoft.Data.Entity.Utilities;
 using Microsoft.Extensions.Logging;
@@ -18,7 +19,7 @@
 
 namespace Microsoft.Data.Entity.Relational.Design.ReverseEngineering
 {
-    public abstract class RelationalMetadataModelProvider : MetadataModelProvider
+    public class RelationalMetadataModelProvider : MetadataModelProvider
     {
         internal static IReadOnlyList<string> IgnoredAnnotations { get; } = new List<string>
         {
@@ -30,18 +31,31 @@ public abstract class RelationalMetadataModelProvider : MetadataModelProvider
         private readonly Table _nullTable = new Table { };
         private CSharpUniqueNamer<Table> _tableNamer;
         private readonly IRelationalTypeMapper _typeMapper;
+        private readonly IMetadataReader _metadataReader;
 
-        protected RelationalMetadataModelProvider(
+        public RelationalMetadataModelProvider(
             [NotNull] ILoggerFactory loggerFactory,
-            [NotNull] IRelationalTypeMapper typeMapper)
+            [NotNull] IRelationalTypeMapper typeMapper,
+            [NotNull] IMetadataReader metadataReader)
             : base(loggerFactory)
         {
             Check.NotNull(typeMapper, nameof(typeMapper));
+            Check.NotNull(metadataReader, nameof(metadataReader));
 
             _typeMapper = typeMapper;
+            _metadataReader = metadataReader;
         }
 
-        protected virtual IModel GetModel([NotNull] SchemaInfo schemaInfo)
+        public override IModel GetModel([NotNull] string connectionString, [CanBeNull] TableSelectionSet tableSelectionSet)
+        {
+            Check.NotEmpty(connectionString, nameof(connectionString));
+
+            var schemaInfo = _metadataReader.GetSchema(connectionString, tableSelectionSet ?? TableSelectionSet.InclusiveAll);
+
+            return GetModelFromSchema(schemaInfo);
+        }
+
+        protected virtual IModel GetModelFromSchema([NotNull] SchemaInfo schemaInfo)
         {
             var modelBuilder = new ModelBuilder(new ConventionSet());
 

diff --git a/src/EntityFramework.Sqlite.Design/EntityFramework.Sqlite.Design.csproj b/src/EntityFramework.Sqlite.Design/EntityFramework.Sqlite.Design.csproj
@@ -54,7 +54,6 @@
     <Compile Include="ReverseEngineering\TableSelectionSetSqliteExtensions.cs" />
     <Compile Include="SqliteDesignTimeServices.cs" />
     <Compile Include="ReverseEngineering\SqliteDmlParser.cs" />
-    <Compile Include="ReverseEngineering\SqliteMetadataModelProvider.cs" />
     <Compile Include="ReverseEngineering\SqliteMetadataReader.cs" />
     <None Include="project.json" />
   </ItemGroup>

diff --git a/src/EntityFramework.Sqlite.Design/ReverseEngineering/SqliteMetadataModelProvider.cs b/src/EntityFramework.Sqlite.Design/ReverseEngineering/SqliteMetadataModelProvider.cs
diff --git a/src/EntityFramework.Sqlite.Design/SqliteDesignTimeServices.cs b/src/EntityFramework.Sqlite.Design/SqliteDesignTimeServices.cs
@@ -19,7 +19,7 @@ public class SqliteDesignTimeServices
         public virtual void ConfigureDesignTimeServices([NotNull] IServiceCollection serviceCollection)
         {
             serviceCollection
-                .AddSingleton<MetadataModelProvider, SqliteMetadataModelProvider>()
+                .AddSingleton<MetadataModelProvider, RelationalMetadataModelProvider>()
                 .AddSingleton<IRelationalTypeMapper, SqliteTypeMapper>()
                 .AddSingleton<IRelationalAnnotationProvider, SqliteAnnotationProvider>()
                 .AddSingleton<ConfigurationFactory, SqliteConfigurationFactory>()

diff --git a/.../EntityFramework.MicrosoftSqlServer.Design.FunctionalTests/SqlServerMetadataReaderTest.cs b/.../EntityFramework.MicrosoftSqlServer.Design.FunctionalTests/SqlServerMetadataReaderTest.cs
@@ -110,6 +110,7 @@ Name nvarchar(100) NOT NULL,
     Latitude decimal( 5, 2 ) DEFAULT 0.0,
     Created datetime2(6),
     Sum AS Latitude + 1.0,
+    Modified rowversion,
     Primary Key (Name, Id)
 );";
             var dbInfo = GetDatabaseInfo(sql);
@@ -163,6 +164,12 @@ Primary Key (Name, Id)
                     {
                         Assert.Equal("Sum", sum.Name);
                         Assert.True(sum.IsComputed);
+                    },
+                modified =>
+                    {
+                        Assert.Equal("Modified", modified.Name);
+                        Assert.True(modified.IsComputed);
+                        Assert.Equal("timestamp", modified.DataType); // intentional - testing the alias
                     });
         }
 

diff --git a/test/EntityFramework.Relational.Design.Tests/RelationalMetadataModelProviderTest.cs b/test/EntityFramework.Relational.Design.Tests/RelationalMetadataModelProviderTest.cs
@@ -558,18 +558,22 @@ public void Unique_names()
 
     public class FakeMetadataModelProvider : RelationalMetadataModelProvider
     {
-        public new IModel GetModel(SchemaInfo schemaInfo) => base.GetModel(schemaInfo);
-
-        public override IModel GetModel([NotNull] string connectionString, [CanBeNull] TableSelectionSet tableSelectionSet)
-        {
-            throw new NotImplementedException();
-        }
+        public IModel GetModel(SchemaInfo schemaInfo) => base.GetModelFromSchema(schemaInfo);
 
         public FakeMetadataModelProvider(
             [NotNull] ILoggerFactory loggerFactory)
             : base(loggerFactory,
-                new TestTypeMapper())
+                new TestTypeMapper(),
+                new FakeMetadataReader())
+        {
+        }
+    }
+
+    public class FakeMetadataReader : IMetadataReader
+    {
+        public virtual SchemaInfo GetSchema([NotNull] string connectionString, [NotNull] TableSelectionSet tableSelectionSet)
         {
+            throw new NotImplementedException();
         }
     }
 

diff --git a/test/EntityFramework.Sqlite.Design.FunctionalTests/SqliteMetadataModelProviderTest.cs b/test/EntityFramework.Sqlite.Design.FunctionalTests/SqliteMetadataModelProviderTest.cs
@@ -13,7 +13,6 @@
 using Microsoft.Data.Entity.Relational.Design.ReverseEngineering.Internal;
 using Microsoft.Data.Entity.Relational.Design.Utilities;
 using Microsoft.Data.Entity.Sqlite.Design;
-using Microsoft.Data.Entity.Sqlite.Design.ReverseEngineering;
 using Microsoft.Data.Entity.Sqlite.FunctionalTests;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
@@ -23,7 +22,7 @@ namespace EntityFramework.Sqlite.Design.FunctionalTests
 {
     public class SqliteMetadataModelProviderTest
     {
-        private readonly SqliteMetadataModelProvider _metadataModelProvider;
+        private readonly RelationalMetadataModelProvider _metadataModelProvider;
         private readonly SqliteTestStore _testStore;
         private readonly TestLogger _logger;
 
@@ -44,7 +43,7 @@ public SqliteMetadataModelProviderTest()
             serviceProvider.GetService<ILoggerFactory>().AddProvider(new TestLoggerProvider(_logger));
 
             _metadataModelProvider = serviceProvider
-                .GetService<MetadataModelProvider>() as SqliteMetadataModelProvider;
+                .GetService<MetadataModelProvider>() as RelationalMetadataModelProvider;
         }
 
         [Fact]

diff --git a/test/EntityFramework.Sqlite.Design.Tests/ApiConsistencyTest.cs b/test/EntityFramework.Sqlite.Design.Tests/ApiConsistencyTest.cs
@@ -8,6 +8,6 @@ namespace Microsoft.Data.Entity.Sqlite.Design
 {
     public class ApiConsistencyTest : ApiConsistencyTestBase
     {
-        protected override Assembly TargetAssembly => typeof(SqliteMetadataModelProvider).Assembly;
+        protected override Assembly TargetAssembly => typeof(SqliteMetadataReader).Assembly;
     }
 }