Skip to content

Commit

Permalink
Fix type mappings for "text", "ntext", and "image"
Browse files Browse the repository at this point in the history
Fixes #15953
  • Loading branch information
ajcvickers committed Aug 1, 2019
1 parent 53f64ac commit 2dece0c
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -21,6 +23,8 @@ public class SqlServerByteArrayTypeMapping : ByteArrayTypeMapping
{
private const int MaxSize = 8000;

private readonly SqlDbType? _sqlDbType;

/// <summary>
/// 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
Expand All @@ -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)
{
}

Expand All @@ -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.
/// </summary>
protected SqlServerByteArrayTypeMapping(RelationalTypeMappingParameters parameters)
protected SqlServerByteArrayTypeMapping(RelationalTypeMappingParameters parameters, SqlDbType? sqlDbType)
: base(parameters)
{
_sqlDbType = sqlDbType;
}

private static int CalculateSize(int? size)
Expand All @@ -64,7 +71,7 @@ private static int CalculateSize(int? size)
/// <param name="parameters"> The parameters for this mapping. </param>
/// <returns> The newly created mapping. </returns>
protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
=> new SqlServerByteArrayTypeMapping(parameters);
=> new SqlServerByteArrayTypeMapping(parameters, _sqlDbType);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

/// <summary>
Expand All @@ -33,6 +35,7 @@ public SqlServerStringTypeMapping(
bool unicode = false,
int? size = null,
bool fixedLength = false,
SqlDbType? sqlDbType = null,
StoreTypePostfix? storeTypePostfix = null)
: this(
new RelationalTypeMappingParameters(
Expand All @@ -42,7 +45,8 @@ public SqlServerStringTypeMapping(
GetDbType(unicode, fixedLength),
unicode,
size,
fixedLength))
fixedLength),
sqlDbType)
{
}

Expand All @@ -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.
/// </summary>
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)
Expand All @@ -83,7 +88,7 @@ private static int CalculateSize(bool unicode, int? size)
/// <param name="parameters"> The parameters for this mapping. </param>
/// <returns> The newly created mapping. </returns>
protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
=> new SqlServerStringTypeMapping(parameters);
=> new SqlServerStringTypeMapping(parameters, _sqlDbType);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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();

Expand All @@ -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);

Expand Down Expand Up @@ -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 },
Expand All @@ -214,7 +223,7 @@ public SqlServerTypeMappingSource(
{ "smallint", _short },
{ "smallmoney", _money },
{ "sql_variant", _sqlVariant },
{ "text", _variableLengthAnsiString },
{ "text", _textAnsiString },
{ "time", _time },
{ "timestamp", _rowversion },
{ "tinyint", _byte },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void Plugins_work()
scaffoldedModel.ContextFile.Code);
}

[Fact]
[ConditionalFact]
public void Comments_use_fluent_api()
{
Test(
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 23 additions & 8 deletions test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
UnicodeConversionCloneTest(typeof(ByteArrayTypeMapping), typeof(byte[]));
}

protected virtual void ConversionCloneTest(
Type mappingType,
Type clrType,
params object[] additionalArgs)
{
var mapping = (RelationalTypeMapping)Activator.CreateInstance(
mappingType,
Expand All @@ -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);

Expand Down Expand Up @@ -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,
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -569,12 +570,23 @@ public virtual void Can_query_using_any_mapped_data_types_with_nulls()
entity,
context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsNationalCharacterVaryingMax == param29));

// TODO: See issue#15953
//string param30 = null;
//Assert.Same(entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsText == param30));
string param30 = null;
var message30 = Assert.Throws<SqlException>(
() => context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.StringAsText == param30)).Message;

//string param31 = null;
//Assert.Same(entity, context.Set<MappedNullableDataTypes>().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<SqlException>(
() => context.Set<MappedNullableDataTypes>().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<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.BytesAsVarbinaryMax == param35));
Expand All @@ -583,8 +595,14 @@ public virtual void Can_query_using_any_mapped_data_types_with_nulls()
Assert.Same(
entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.BytesAsBinaryVaryingMax == param36));

//byte[] param37 = null;
//Assert.Same(entity, context.Set<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.BytesAsImage == param37));
byte[] param37 = null;
var message37 = Assert.Throws<SqlException>(
() => context.Set<MappedNullableDataTypes>().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<MappedNullableDataTypes>().Single(e => e.Int == 911 && e.Decimal == param38));
Expand Down
18 changes: 12 additions & 6 deletions test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
UnicodeConversionCloneTest(
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]
Expand Down

0 comments on commit 2dece0c

Please sign in to comment.