Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Type mapping fixes from API reviews #22032

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Type mapping fixes from API reviews #22032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
24 changes: 2 additions & 22 deletions src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
Expand All @@ -453,17 +445,5 @@ protected virtual string ParseStoreTypeName(

return storeTypeName;
}

/// <summary>
/// Returns whether the store type name base interprets
/// nameBase(n) as a precision rather than a length
/// </summary>
/// <param name="storeTypeNameBase"> The name base of the store type </param>
/// <returns>
/// <see langword="true" /> if the store type name base interprets nameBase(n)
/// as a precision rather than a length, <see langword="false" /> otherwise.
/// </returns>
protected virtual bool StoreTypeNameBaseUsesPrecision([NotNull] string storeTypeNameBase)
=> false;
}
}
47 changes: 0 additions & 47 deletions src/EFCore.Relational/Storage/RelationalTypeMappingSourceExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,52 +106,5 @@ public static RelationalTypeMapping GetMapping(

throw new InvalidOperationException(RelationalStrings.UnsupportedStoreType(typeName));
}

/// <summary>
/// <para>
/// Finds the type mapping for a given <see cref="Type" /> and additional facets, throwing if no mapping is found.
/// </para>
/// <para>
/// Note: Only call this method if there is no <see cref="IProperty" /> available, otherwise
/// call <see cref="GetMapping(IRelationalTypeMappingSource, IProperty)" />
/// </para>
/// </summary>
/// <param name="typeMappingSource"> The type mapping source. </param>
/// <param name="type"> The CLR type. </param>
/// <param name="storeTypeName"> The database type name. </param>
/// <param name="keyOrIndex"> If <see langword="true" />, then a special mapping for a key or index may be returned. </param>
/// <param name="unicode">
/// Specify <see langword="true" /> for Unicode mapping, <see langword="false" /> for Ansi mapping or <see langword="null" /> for the default.
/// </param>
/// <param name="size"> Specifies a size for the mapping, or <see langword="null" /> for default. </param>
/// <param name="rowVersion"> Specifies a row-version, or <see langword="null" /> for default. </param>
/// <param name="fixedLength"> Specifies a fixed length mapping, or <see langword="null" /> for default. </param>
/// <param name="precision"> Specifies a precision for the mapping, or <see langword="null" /> for default. </param>
/// <param name="scale"> Specifies a scale for the mapping, or <see langword="null" /> for default. </param>
/// <returns> The type mapping, or <see langword="null" /> if none was found. </returns>
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));
}
}
}
23 changes: 19 additions & 4 deletions src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
/// </summary>
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;
}
}
}
4 changes: 2 additions & 2 deletions test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
56 changes: 52 additions & 4 deletions test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqliteTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>(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<long>(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<byte[]>(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<double>(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<string>(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null);
}

Expand All @@ -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<long>(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null);
}
Expand All @@ -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<byte[]>(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null);
}
Expand All @@ -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<double>(mapping, inferred: true, maxLength: null, unicode: null, fixedLength: null);
}
Expand Down
21 changes: 19 additions & 2 deletions test/EFCore.Relational.Tests/TestUtilities/TestRelationalTypeMappingSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}