From d9c4b51c76ee946e5ba9a8eb43021ff3ce1a785f Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Thu, 1 Aug 2019 16:38:19 -0700 Subject: [PATCH] Fix type mappings for "text", "ntext", and "image" Fixes #15953 --- .../Internal/SqlServerByteArrayTypeMapping.cs | 21 +++++++++--- .../Internal/SqlServerStringTypeMapping.cs | 17 ++++++++-- .../Internal/SqlServerTypeMappingSource.cs | 15 +++++++-- .../Internal/CSharpDbContextGeneratorTest.cs | 4 +-- .../RelationalScaffoldingModelFactoryTest.cs | 2 +- .../Storage/RelationalTypeMappingTest.cs | 31 +++++++++++++----- .../CustomConvertersTestBase.cs | 2 +- .../BuiltInDataTypesSqlServerTest.cs | 32 +++++++++++++++---- .../Storage/SqlServerTypeMappingTest.cs | 18 +++++++---- 9 files changed, 107 insertions(+), 35 deletions(-) diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerByteArrayTypeMapping.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerByteArrayTypeMapping.cs index d2cbdc22a0a..71a47fe840c 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerByteArrayTypeMapping.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerByteArrayTypeMapping.cs @@ -2,10 +2,12 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Data; using System.Data.Common; using System.Globalization; using System.Text; using JetBrains.Annotations; +using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Storage; @@ -21,6 +23,8 @@ public class SqlServerByteArrayTypeMapping : ByteArrayTypeMapping { private const int MaxSize = 8000; + private readonly SqlDbType? _sqlDbType; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -32,15 +36,17 @@ public SqlServerByteArrayTypeMapping( int? size = null, bool fixedLength = false, ValueComparer comparer = null, + SqlDbType? sqlDbType = null, StoreTypePostfix? storeTypePostfix = null) - : base( + : this( new RelationalTypeMappingParameters( new CoreTypeMappingParameters(typeof(byte[]), null, comparer), storeType ?? (fixedLength ? "binary" : "varbinary"), storeTypePostfix ?? StoreTypePostfix.Size, System.Data.DbType.Binary, size: size, - fixedLength: fixedLength)) + fixedLength: fixedLength), + sqlDbType) { } @@ -50,9 +56,10 @@ public SqlServerByteArrayTypeMapping( /// 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 SqlServerByteArrayTypeMapping(RelationalTypeMappingParameters parameters) + protected SqlServerByteArrayTypeMapping(RelationalTypeMappingParameters parameters, SqlDbType? sqlDbType) : base(parameters) { + _sqlDbType = sqlDbType; } private static int CalculateSize(int? size) @@ -64,7 +71,7 @@ private static int CalculateSize(int? size) /// The parameters for this mapping. /// The newly created mapping. protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) - => new SqlServerByteArrayTypeMapping(parameters); + => new SqlServerByteArrayTypeMapping(parameters, _sqlDbType); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -83,6 +90,12 @@ protected override void ConfigureParameter(DbParameter parameter) var length = (value as byte[])?.Length; var maxSpecificSize = CalculateSize(Size); + if (_sqlDbType.HasValue + && parameter is SqlParameter sqlParameter) // To avoid crashing wrapping providers + { + sqlParameter.SqlDbType = _sqlDbType.Value; + } + parameter.Size = value == null || value == DBNull.Value || length != null && length <= maxSpecificSize ? maxSpecificSize : -1; diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs index af5a5c1e5b2..aa5a33b1901 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs @@ -5,6 +5,7 @@ using System.Data; using System.Data.Common; using JetBrains.Annotations; +using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore.Storage; namespace Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal @@ -20,6 +21,7 @@ public class SqlServerStringTypeMapping : StringTypeMapping private const int UnicodeMax = 4000; private const int AnsiMax = 8000; + private readonly SqlDbType? _sqlDbType; private readonly int _maxSpecificSize; /// @@ -33,6 +35,7 @@ public SqlServerStringTypeMapping( bool unicode = false, int? size = null, bool fixedLength = false, + SqlDbType? sqlDbType = null, StoreTypePostfix? storeTypePostfix = null) : this( new RelationalTypeMappingParameters( @@ -42,7 +45,8 @@ public SqlServerStringTypeMapping( GetDbType(unicode, fixedLength), unicode, size, - fixedLength)) + fixedLength), + sqlDbType) { } @@ -62,10 +66,11 @@ private static string GetStoreName(bool unicode, bool fixedLength) => unicode /// 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 SqlServerStringTypeMapping(RelationalTypeMappingParameters parameters) + protected SqlServerStringTypeMapping(RelationalTypeMappingParameters parameters, SqlDbType? sqlDbType) : base(parameters) { _maxSpecificSize = CalculateSize(parameters.Unicode, parameters.Size); + _sqlDbType = sqlDbType; } private static int CalculateSize(bool unicode, int? size) @@ -83,7 +88,7 @@ private static int CalculateSize(bool unicode, int? size) /// The parameters for this mapping. /// The newly created mapping. protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) - => new SqlServerStringTypeMapping(parameters); + => new SqlServerStringTypeMapping(parameters, _sqlDbType); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -101,6 +106,12 @@ protected override void ConfigureParameter(DbParameter parameter) var value = parameter.Value; var length = (value as string)?.Length; + if (_sqlDbType.HasValue + && parameter is SqlParameter sqlParameter) // To avoid crashing wrapping providers + { + sqlParameter.SqlDbType = _sqlDbType.Value; + } + parameter.Size = value == null || value == DBNull.Value || length != null && length <= _maxSpecificSize ? _maxSpecificSize : -1; diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs index 053d0e68a34..4d3502a653e 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs @@ -64,6 +64,9 @@ private readonly BoolTypeMapping _bool private readonly SqlServerStringTypeMapping _fixedLengthUnicodeString = new SqlServerStringTypeMapping(unicode: true, fixedLength: true); + private readonly SqlServerStringTypeMapping _textUnicodeString + = new SqlServerStringTypeMapping("ntext", unicode: true, sqlDbType: SqlDbType.NText); + private readonly SqlServerStringTypeMapping _variableLengthUnicodeString = new SqlServerStringTypeMapping(unicode: true); @@ -73,6 +76,9 @@ private readonly SqlServerStringTypeMapping _variableLengthMaxUnicodeString private readonly SqlServerStringTypeMapping _fixedLengthAnsiString = new SqlServerStringTypeMapping(fixedLength: true); + private readonly SqlServerStringTypeMapping _textAnsiString + = new SqlServerStringTypeMapping("text", sqlDbType: SqlDbType.Text); + private readonly SqlServerStringTypeMapping _variableLengthAnsiString = new SqlServerStringTypeMapping(); @@ -82,6 +88,9 @@ private readonly SqlServerStringTypeMapping _variableLengthMaxAnsiString private readonly SqlServerByteArrayTypeMapping _variableLengthBinary = new SqlServerByteArrayTypeMapping(); + private readonly SqlServerByteArrayTypeMapping _imageBinary + = new SqlServerByteArrayTypeMapping("image", sqlDbType: SqlDbType.Image); + private readonly SqlServerByteArrayTypeMapping _variableLengthMaxBinary = new SqlServerByteArrayTypeMapping("varbinary(max)", storeTypePostfix: StoreTypePostfix.None); @@ -197,14 +206,14 @@ public SqlServerTypeMappingSource( { "decimal", _decimal }, { "double precision", _double }, { "float", _double }, - { "image", _variableLengthBinary }, + { "image", _imageBinary }, { "int", _int }, { "money", _money }, { "national char varying", _variableLengthUnicodeString }, { "national character varying", _variableLengthUnicodeString }, { "national character", _fixedLengthUnicodeString }, { "nchar", _fixedLengthUnicodeString }, - { "ntext", _variableLengthUnicodeString }, + { "ntext", _textUnicodeString }, { "numeric", _decimal }, { "nvarchar", _variableLengthUnicodeString }, { "nvarchar(max)", _variableLengthMaxUnicodeString }, @@ -214,7 +223,7 @@ public SqlServerTypeMappingSource( { "smallint", _short }, { "smallmoney", _money }, { "sql_variant", _sqlVariant }, - { "text", _variableLengthAnsiString }, + { "text", _textAnsiString }, { "time", _time }, { "timestamp", _rowversion }, { "tinyint", _byte }, diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs index 73094fe8d83..029365f38c2 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs @@ -145,7 +145,7 @@ public void Plugins_work() scaffoldedModel.ContextFile.Code); } - [Fact] + [ConditionalFact] public void Comments_use_fluent_api() { Test( @@ -166,7 +166,7 @@ public void Comments_use_fluent_api() model.FindEntityType("TestNamespace.Entity").GetProperty("Property").GetComment())); } - [Fact] + [ConditionalFact] public void Entity_comments_use_fluent_api() { Test( diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs index 13f7473ca63..001ef996584 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/RelationalScaffoldingModelFactoryTest.cs @@ -1801,7 +1801,7 @@ public void Unmapped_column_is_ignored() Assert.Equal(1, columns.Count); } - [Fact] + [ConditionalFact] public void Column_and_table_comments() { var database = new DatabaseModel diff --git a/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs b/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs index 05fdd25d07d..0da28fce8a3 100644 --- a/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs +++ b/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Data; using System.Data.Common; +using System.Linq; using System.Reflection; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Storage.ValueConversion; @@ -93,9 +94,16 @@ public virtual void Create_and_clone_with_converter(Type mappingType, Type clrTy Assert.Equal(StoreTypePostfix.PrecisionAndScale, clone.StoreTypePostfix); } - [ConditionalTheory] - [InlineData(typeof(ByteArrayTypeMapping), typeof(byte[]))] - public virtual void Create_and_clone_sized_mappings_with_converter(Type mappingType, Type clrType) + [ConditionalFact] + public virtual void Create_and_clone_sized_mappings_with_converter() + { + ConversionCloneTest(typeof(ByteArrayTypeMapping), typeof(byte[])); + } + + protected virtual void ConversionCloneTest( + Type mappingType, + Type clrType, + params object[] additionalArgs) { var mapping = (RelationalTypeMapping)Activator.CreateInstance( mappingType, @@ -108,7 +116,7 @@ public virtual void Create_and_clone_sized_mappings_with_converter(Type mappingT size: 33, fixedLength: true, storeTypePostfix: StoreTypePostfix.Size) - }, + }.Concat(additionalArgs).ToArray(), null, null); @@ -149,9 +157,16 @@ public virtual void Create_and_clone_sized_mappings_with_converter(Type mappingT Assert.Equal(StoreTypePostfix.Size, clone.StoreTypePostfix); } - [ConditionalTheory] - [InlineData(typeof(StringTypeMapping), typeof(string))] - public virtual void Create_and_clone_unicode_sized_mappings_with_converter(Type mappingType, Type clrType) + [ConditionalFact] + public virtual void Create_and_clone_unicode_sized_mappings_with_converter() + { + UnicodeConversionCloneTest(typeof(StringTypeMapping), typeof(string)); + } + + protected virtual void UnicodeConversionCloneTest( + Type mappingType, + Type clrType, + params object[] additionalArgs) { var mapping = (RelationalTypeMapping)Activator.CreateInstance( mappingType, @@ -165,7 +180,7 @@ public virtual void Create_and_clone_unicode_sized_mappings_with_converter(Type unicode: false, fixedLength: true, storeTypePostfix: StoreTypePostfix.Size) - }, + }.Concat(additionalArgs).ToArray(), null, null); diff --git a/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs b/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs index ad7a08f475d..1d03928082a 100644 --- a/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs +++ b/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs @@ -234,7 +234,7 @@ protected class Email public static implicit operator string(Email email) => email._value; } - [ConditionalFact(Skip = "Issue #14935. Cannot eval 'where [e].Fuel.Equals(value(Microsoft.EntityFrameworkCore.CustomConvertersTestBase`1+Fuel[Microsoft.EntityFrameworkCore.CustomConvertersSqliteTest+CustomConvertersSqliteFixture]))'")] + [ConditionalFact] public virtual void Can_query_and_update_with_conversion_for_custom_struct() { using (var context = CreateContext()) diff --git a/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs index f459f300d04..373acaf35b6 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs @@ -6,6 +6,7 @@ using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using System.Text; +using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; @@ -569,12 +570,23 @@ public virtual void Can_query_using_any_mapped_data_types_with_nulls() entity, context.Set().Single(e => e.Int == 911 && e.StringAsNationalCharacterVaryingMax == param29)); - // TODO: See issue#15953 - //string param30 = null; - //Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.StringAsText == param30)); + string param30 = null; + var message30 = Assert.Throws( + () => context.Set().Single(e => e.Int == 911 && e.StringAsText == param30)).Message; - //string param31 = null; - //Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.StringAsNtext == param31)); + // Expect: "The data types text and text are incompatible in the equal to operator." + Assert.NotEqual( + message30.IndexOf("text", StringComparison.Ordinal), + message30.LastIndexOf("text", StringComparison.Ordinal)); + + string param31 = null; + var message31 = Assert.Throws( + () => context.Set().Single(e => e.Int == 911 && e.StringAsNtext == param31)).Message; + + // Expect: "The data types ntext and ntext are incompatible in the equal to operator." + Assert.NotEqual( + message31.IndexOf("ntext", StringComparison.Ordinal), + message31.LastIndexOf("ntext", StringComparison.Ordinal)); byte[] param35 = null; Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.BytesAsVarbinaryMax == param35)); @@ -583,8 +595,14 @@ public virtual void Can_query_using_any_mapped_data_types_with_nulls() Assert.Same( entity, context.Set().Single(e => e.Int == 911 && e.BytesAsBinaryVaryingMax == param36)); - //byte[] param37 = null; - //Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.BytesAsImage == param37)); + byte[] param37 = null; + var message37 = Assert.Throws( + () => context.Set().Single(e => e.Int == 911 && e.BytesAsImage == param37)).Message; + + // Expect: "The data types image and image are incompatible in the equal to operator." + Assert.NotEqual( + message37.IndexOf("image", StringComparison.Ordinal), + message37.LastIndexOf("image", StringComparison.Ordinal)); decimal? param38 = null; Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.Decimal == param38)); diff --git a/test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs b/test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs index 368a55cee9d..6a51eeedb1b 100644 --- a/test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs +++ b/test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs @@ -105,16 +105,22 @@ public override void Create_and_clone_with_converter(Type mappingType, Type clrT base.Create_and_clone_with_converter(mappingType, clrType); } - [InlineData(typeof(SqlServerByteArrayTypeMapping), typeof(byte[]))] - public override void Create_and_clone_sized_mappings_with_converter(Type mappingType, Type clrType) + [ConditionalFact] + public virtual void Create_and_clone_SQL_Server_sized_mappings_with_converter() { - base.Create_and_clone_sized_mappings_with_converter(mappingType, clrType); + ConversionCloneTest( + typeof(SqlServerByteArrayTypeMapping), + typeof(byte[]), + SqlDbType.Image); } - [InlineData(typeof(SqlServerStringTypeMapping), typeof(string))] - public override void Create_and_clone_unicode_sized_mappings_with_converter(Type mappingType, Type clrType) + [ConditionalFact] + public virtual void Create_and_clone_SQL_Server_unicode_sized_mappings_with_converter() { - base.Create_and_clone_unicode_sized_mappings_with_converter(mappingType, clrType); + UnicodeConversionCloneTest( + typeof(SqlServerStringTypeMapping), + typeof(string), + SqlDbType.Text); } [ConditionalFact]