From 3fb7e753d7a3bbaa8af634ed04d8a850621b74fb Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Wed, 12 Aug 2020 09:04:59 -0700 Subject: [PATCH] Type mapping fixes from API reviews * Removed `StoreTypeNameBaseUsesPrecision` * Removed large GetMapping overload that was added in 5.0 but is never called * Fixed issue when reverse engineering where different casing for type name was being ignored Part of #20409 --- .../Internal/ScaffoldingTypeMapper.cs | 2 +- .../Storage/RelationalTypeMappingSource.cs | 24 +------- .../RelationalTypeMappingSourceExtensions.cs | 47 ---------------- .../Internal/SqlServerTypeMappingSource.cs | 23 ++++++-- .../RelationalScaffoldingModelFactoryTest.cs | 4 +- .../ScaffoldingTypeMapperSqliteTest.cs | 56 +++++++++++++++++-- .../TestRelationalTypeMappingSource.cs | 21 ++++++- 7 files changed, 95 insertions(+), 82 deletions(-) diff --git a/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs b/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs index 365b60b61b4..f6467f22ba7 100644 --- a/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs +++ b/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs @@ -73,7 +73,7 @@ public virtual TypeScaffoldingInfo FindMapping( scale: mapping.Scale); if (defaultTypeMapping != null - && string.Equals(defaultTypeMapping.StoreType, storeType, StringComparison.OrdinalIgnoreCase)) + && string.Equals(defaultTypeMapping.StoreType, storeType, StringComparison.Ordinal)) { canInfer = true; diff --git a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs index 8b8550936e4..41ad0e17c3f 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs @@ -414,7 +414,7 @@ protected virtual string ParseStoreTypeName( var openParen = storeTypeName.IndexOf("(", StringComparison.Ordinal); if (openParen > 0) { - string storeTypeNameBase = storeTypeName.Substring(0, openParen).Trim(); + var storeTypeNameBase = storeTypeName.Substring(0, openParen).Trim(); var closeParen = storeTypeName.IndexOf(")", openParen + 1, StringComparison.Ordinal); if (closeParen > openParen) { @@ -435,15 +435,7 @@ protected virtual string ParseStoreTypeName( else if (int.TryParse( storeTypeName.Substring(openParen + 1, closeParen - openParen - 1).Trim(), out var parsedSize)) { - if (StoreTypeNameBaseUsesPrecision(storeTypeNameBase)) - { - precision = parsedSize; - scale = 0; - } - else - { - size = parsedSize; - } + size = parsedSize; } return storeTypeNameBase; @@ -453,17 +445,5 @@ protected virtual string ParseStoreTypeName( return storeTypeName; } - - /// - /// Returns whether the store type name base interprets - /// nameBase(n) as a precision rather than a length - /// - /// The name base of the store type - /// - /// if the store type name base interprets nameBase(n) - /// as a precision rather than a length, otherwise. - /// - protected virtual bool StoreTypeNameBaseUsesPrecision([NotNull] string storeTypeNameBase) - => false; } } diff --git a/src/EFCore.Relational/Storage/RelationalTypeMappingSourceExtensions.cs b/src/EFCore.Relational/Storage/RelationalTypeMappingSourceExtensions.cs index 1a15093e428..84be3d31c19 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMappingSourceExtensions.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMappingSourceExtensions.cs @@ -106,52 +106,5 @@ public static RelationalTypeMapping GetMapping( throw new InvalidOperationException(RelationalStrings.UnsupportedStoreType(typeName)); } - - /// - /// - /// Finds the type mapping for a given and additional facets, throwing if no mapping is found. - /// - /// - /// Note: Only call this method if there is no available, otherwise - /// call - /// - /// - /// The type mapping source. - /// The CLR type. - /// The database type name. - /// If , then a special mapping for a key or index may be returned. - /// - /// Specify for Unicode mapping, for Ansi mapping or for the default. - /// - /// Specifies a size for the mapping, or for default. - /// Specifies a row-version, or for default. - /// Specifies a fixed length mapping, or for default. - /// Specifies a precision for the mapping, or for default. - /// Specifies a scale for the mapping, or for default. - /// The type mapping, or if none was found. - public static RelationalTypeMapping GetMapping( - [NotNull] this IRelationalTypeMappingSource typeMappingSource, - [NotNull] Type type, - [CanBeNull] string storeTypeName, - bool keyOrIndex = false, - bool? unicode = null, - int? size = null, - bool? rowVersion = null, - bool? fixedLength = null, - int? precision = null, - int? scale = null) - { - Check.NotNull(typeMappingSource, nameof(typeMappingSource)); - Check.NotNull(type, nameof(type)); - - var mapping = typeMappingSource.FindMapping( - type, storeTypeName, keyOrIndex, unicode, size, rowVersion, fixedLength, precision, scale); - if (mapping != null) - { - return mapping; - } - - throw new InvalidOperationException(RelationalStrings.UnsupportedType(type)); - } } } diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs index 6f8a33170b2..2ae11062f82 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs @@ -8,8 +8,6 @@ using System.Linq; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.ChangeTracking; -using Microsoft.EntityFrameworkCore.Metadata; -using Microsoft.EntityFrameworkCore.SqlServer.Internal; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.Extensions.DependencyInjection; @@ -323,7 +321,24 @@ private RelationalTypeMapping FindRawMapping(RelationalTypeMappingInfo mappingIn /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - protected override bool StoreTypeNameBaseUsesPrecision(string storeTypeNameBase) - => _nameBasesUsingPrecision.Contains(storeTypeNameBase); + protected override string ParseStoreTypeName( + string storeTypeName, + out bool? unicode, + out int? size, + out int? precision, + out int? scale) + { + var parsedName = base.ParseStoreTypeName(storeTypeName, out unicode, out size, out precision, out scale); + + if (size.HasValue + && storeTypeName != null + && _nameBasesUsingPrecision.Any(n => storeTypeName.StartsWith(n, StringComparison.OrdinalIgnoreCase))) + { + precision = size; + size = null; + } + + return parsedName; + } } } diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs index 55bb4d669a2..ef385e75b6d 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs @@ -303,9 +303,9 @@ public void Do_not_use_database_names_for_columns() [InlineData("nvarchar(450)", null)] [InlineData("datetime2(4)", null)] [InlineData("DateTime2(4)", "DateTime2(4)")] - public void Column_type_annotation(string StoreType, string expectedColumnType) + public void Column_type_annotation(string storeType, string expectedColumnType) { - var column = new DatabaseColumn { Table = Table, Name = "Col", StoreType = StoreType }; + var column = new DatabaseColumn { Table = Table, Name = "Col", StoreType = storeType }; var info = new DatabaseModel { diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqliteTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqliteTest.cs index 104c271928f..18a6f479db1 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqliteTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqliteTest.cs @@ -18,10 +18,58 @@ public class ScaffoldingTypeMapperSqliteTest [InlineData(false, true)] [InlineData(true, false)] [InlineData(true, true)] - public void Maps_text_column(bool keyOrIndex, bool rowVersion) + public void Maps_text_column_with_abnormal_casing(bool keyOrIndex, bool rowVersion) { var mapping = CreateMapper().FindMapping("text", keyOrIndex, rowVersion); + AssertMapping(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null); + } + + [ConditionalTheory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void Maps_integer_column_with_abnormal_casing(bool keyOrIndex, bool rowVersion) + { + var mapping = CreateMapper().FindMapping("integer", keyOrIndex, rowVersion); + + AssertMapping(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null); + } + + [ConditionalTheory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void Maps_blob_column_with_abnormal_casing(bool keyOrIndex, bool rowVersion) + { + var mapping = CreateMapper().FindMapping("blob", keyOrIndex, rowVersion); + + AssertMapping(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null); + } + + [ConditionalTheory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void Maps_real_column_with_abnormal_casing(bool keyOrIndex, bool rowVersion) + { + var mapping = CreateMapper().FindMapping("real", keyOrIndex, rowVersion); + + AssertMapping(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null); + } + + [ConditionalTheory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void Maps_text_column(bool keyOrIndex, bool rowVersion) + { + var mapping = CreateMapper().FindMapping("TEXT", keyOrIndex, rowVersion); + AssertMapping(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null); } @@ -32,7 +80,7 @@ public void Maps_text_column(bool keyOrIndex, bool rowVersion) [InlineData(true, true)] public void Maps_integer_column(bool keyOrIndex, bool rowVersion) { - var mapping = CreateMapper().FindMapping("integer", keyOrIndex, rowVersion); + var mapping = CreateMapper().FindMapping("INTEGER", keyOrIndex, rowVersion); AssertMapping(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null); } @@ -44,7 +92,7 @@ public void Maps_integer_column(bool keyOrIndex, bool rowVersion) [InlineData(true, true)] public void Maps_blob_column(bool keyOrIndex, bool rowVersion) { - var mapping = CreateMapper().FindMapping("blob", keyOrIndex, rowVersion); + var mapping = CreateMapper().FindMapping("BLOB", keyOrIndex, rowVersion); AssertMapping(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null); } @@ -56,7 +104,7 @@ public void Maps_blob_column(bool keyOrIndex, bool rowVersion) [InlineData(true, true)] public void Maps_real_column(bool keyOrIndex, bool rowVersion) { - var mapping = CreateMapper().FindMapping("real", keyOrIndex, rowVersion); + var mapping = CreateMapper().FindMapping("REAL", keyOrIndex, rowVersion); AssertMapping(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null); } diff --git a/test/EFCore.Relational.Tests/TestUtilities/TestRelationalTypeMappingSource.cs b/test/EFCore.Relational.Tests/TestUtilities/TestRelationalTypeMappingSource.cs index db44bfe338c..de3ba9c2600 100644 --- a/test/EFCore.Relational.Tests/TestUtilities/TestRelationalTypeMappingSource.cs +++ b/test/EFCore.Relational.Tests/TestUtilities/TestRelationalTypeMappingSource.cs @@ -236,7 +236,24 @@ protected override RelationalTypeMapping FindMapping(in RelationalTypeMappingInf : null; } - protected override bool StoreTypeNameBaseUsesPrecision(string storeTypeNameBase) - => "default_decimal_mapping" == storeTypeNameBase; + protected override string ParseStoreTypeName( + string storeTypeName, + out bool? unicode, + out int? size, + out int? precision, + out int? scale) + { + var parsedName = base.ParseStoreTypeName(storeTypeName, out unicode, out size, out precision, out scale); + + if (size.HasValue + && storeTypeName?.StartsWith("default_decimal_mapping", StringComparison.OrdinalIgnoreCase) == true) + { + precision = size; + size = null; + scale = 0; + } + + return parsedName; + } } }