Skip to content

Commit

Permalink
Simplify TypeMappingInfo by removing explicit value conversion info
Browse files Browse the repository at this point in the history 
Because providers do not need this info, and based in decisions made for the model snapshot/seed data, should not behave differently based on it.

Instead, the part of the code that knows about converters--in TypeMappingSource--now handles the converters separately, creating appropriate clones of TypeMappingInfo as needed with information from the converter.
  • Loading branch information
@ajcvickers
ajcvickers committed Mar 9, 2018
1 parent cb0b919 commit 9172c66
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 171 deletions.
2 changes: 1 addition & 1 deletion samples/OracleProvider/src/OracleProvider/Storage/Internal/OracleTypeMappingSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ protected override RelationalTypeMapping FindMapping(RelationalTypeMappingInfo m

private RelationalTypeMapping FindRawMapping(RelationalTypeMappingInfo mappingInfo)
{
var clrType = mappingInfo.ProviderClrType;
var clrType = mappingInfo.ClrType;
var storeTypeName = mappingInfo.StoreTypeName;
var storeTypeNameBase = mappingInfo.StoreTypeNameBase;

Expand Down
12 changes: 6 additions & 6 deletions src/EFCore.Relational/Storage/Internal/FallbackRelationalTypeMappingSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ private RelationalTypeMapping FindMappingForProperty(RelationalTypeMappingInfo m

private RelationalTypeMapping FindMappingForClrType(RelationalTypeMappingInfo mappingInfo)
{
if (mappingInfo.ProviderClrType == null
if (mappingInfo.ClrType == null
|| (mappingInfo.StoreTypeName != null
&& _relationalTypeMapper.FindMapping(mappingInfo.StoreTypeName) != null))
{
return null;
}

if (mappingInfo.ProviderClrType == typeof(string)
if (mappingInfo.ClrType == typeof(string)
&& _relationalTypeMapper.StringMapper != null)
{
return _relationalTypeMapper.StringMapper.FindMapping(
Expand All @@ -89,7 +89,7 @@ private RelationalTypeMapping FindMappingForClrType(RelationalTypeMappingInfo ma
mappingInfo.Size);
}

if (mappingInfo.ProviderClrType == typeof(byte[])
if (mappingInfo.ClrType == typeof(byte[])
&& _relationalTypeMapper.ByteArrayMapper != null)
{
return _relationalTypeMapper.ByteArrayMapper.FindMapping(
Expand All @@ -98,7 +98,7 @@ private RelationalTypeMapping FindMappingForClrType(RelationalTypeMappingInfo ma
mappingInfo.Size);
}

return _relationalTypeMapper.FindMapping(mappingInfo.ProviderClrType);
return _relationalTypeMapper.FindMapping(mappingInfo.ClrType);
}

private RelationalTypeMapping FindMappingForStoreTypeName(RelationalTypeMappingInfo mappingInfo)
Expand All @@ -115,8 +115,8 @@ private RelationalTypeMapping FindMappingForStoreTypeName(RelationalTypeMappingI

private RelationalTypeMapping FilterByClrType(RelationalTypeMapping mapping, RelationalTypeMappingInfo mappingInfo)
=> mapping != null
&& (mappingInfo.ProviderClrType == null
|| mappingInfo.ProviderClrType == mapping.ClrType)
&& (mappingInfo.ClrType == null
|| mappingInfo.ClrType == mapping.ClrType)
? mapping
: null;
}
Expand Down
5 changes: 1 addition & 4 deletions src/EFCore.Relational/Storage/RelationalTypeMappingExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,11 @@ public static class RelationalTypeMappingExtensions
storeTypeName = mapping.StoreType;
}

var hints = mappingInfo.ValueConverterInfo?.MappingHints;

var size = mapping.Size == -1 ? -1 : (int?)null;
if (size != -1)
{
size = mappingInfo.Size
?? mapping.Size
?? hints?.Size;
?? mapping.Size;

if (size != mapping.Size)
{
Expand Down
28 changes: 9 additions & 19 deletions src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,9 @@ protected RelationalTypeMappingInfo([NotNull] IProperty property)
.Select(p => (string)p[RelationalAnnotationNames.ColumnType])
.FirstOrDefault(t => t != null);

var fixedLength = principals.Select(p => p.Relational().IsFixedLength).FirstOrDefault(t => t);

if (!fixedLength)
{
var customConverter = principals
.Select(p => p.GetValueConverter())
.FirstOrDefault(c => c != null);

var mappingHints = customConverter?.MappingHints;

fixedLength = (mappingHints as RelationalConverterMappingHints)?.IsFixedLength == true;
}
var fixedLength = principals
.Select(p => p.Relational().IsFixedLength)
.FirstOrDefault(t => t);

IsFixedLength = fixedLength;
StoreTypeName = storeTypeName;
Expand Down Expand Up @@ -97,14 +88,13 @@ protected RelationalTypeMappingInfo([NotNull] MemberInfo member)
/// Creates a new instance of <see cref="RelationalTypeMappingInfo" /> with the given <see cref="ValueConverterInfo" />.
/// </summary>
/// <param name="source"> The source info. </param>
/// <param name="builtInConverter"> The converter to apply. </param>
/// <param name="converter"> The converter to apply. </param>
protected RelationalTypeMappingInfo(
[NotNull] RelationalTypeMappingInfo source,
ValueConverterInfo builtInConverter)
: base(source, builtInConverter)
ValueConverterInfo converter)
: base(source, converter)
{
// ReSharper disable once VirtualMemberCallInConstructor
var mappingHints = ValueConverterInfo?.MappingHints ?? default;
var mappingHints = converter.MappingHints;

StoreTypeName = source.StoreTypeName;
StoreTypeNameBase = ParseStoreTypeName(source.StoreTypeName, out _parsedSize, out _parsedPrecision, out _parsedScale, out _isMax);
Expand All @@ -116,7 +106,7 @@ protected RelationalTypeMappingInfo([NotNull] MemberInfo member)
/// </summary>
/// <param name="type"> The CLR type in the model for which mapping is needed. </param>
/// <param name="keyOrIndex"> If <c>true</c>, then a special mapping for a key or index may be returned. </param>
/// <param name="unicode"> Specifies Unicode or Ansi mapping, or <c>null</c> for default. </param>
/// <param name="unicode"> Specifies Unicode or ANSI mapping, or <c>null</c> for default. </param>
/// <param name="size"> Specifies a size for the mapping, or <c>null</c> for default. </param>
/// <param name="rowVersion"> Specifies a row-version, or <c>null</c> for default. </param>
/// <param name="fixedLength"> Specifies a fixed length mapping, or <c>null</c> for default. </param>
Expand Down Expand Up @@ -253,7 +243,7 @@ public override bool Equals(object obj)
/// <returns> The hash code. </returns>
public override int GetHashCode()
{
var hashCode = (StoreTypeName != null ? StoreTypeName.GetHashCode() : 0);
var hashCode = StoreTypeName?.GetHashCode() ?? 0;
hashCode = (hashCode * 397) ^ (IsFixedLength?.GetHashCode() ?? 0);
return hashCode;
}
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public virtual RelationalTypeMapping FindMapping(string storeTypeName)
/// </summary>
/// <param name="type"> The CLR type. </param>
/// <param name="keyOrIndex"> If <c>true</c>, then a special mapping for a key or index may be returned. </param>
/// <param name="unicode"> Specifies Unicode or Ansi mapping, or <c>null</c> for default. </param>
/// <param name="unicode"> Specifies Unicode or ANSI mapping, or <c>null</c> for default. </param>
/// <param name="size"> Specifies a size for the mapping, or <c>null</c> for default. </param>
/// <param name="rowVersion"> Specifies a row-version, or <c>null</c> for default. </param>
/// <param name="fixedLength"> Specifies a fixed length mapping, or <c>null</c> for default. </param>
Expand Down Expand Up @@ -215,7 +215,7 @@ private ConcreteRelationalTypeMappingInfo(ConcreteRelationalTypeMappingInfo sour
{
}

public override TypeMappingInfo WithBuiltInConverter(ValueConverterInfo converterInfo)
public override TypeMappingInfo WithConverter(ValueConverterInfo converterInfo)
=> new ConcreteRelationalTypeMappingInfo(this, converterInfo);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ protected override RelationalTypeMapping FindMapping(RelationalTypeMappingInfo m

private RelationalTypeMapping FindRawMapping(RelationalTypeMappingInfo mappingInfo)
{
var clrType = mappingInfo.ProviderClrType;
var clrType = mappingInfo.ClrType;
var storeTypeName = mappingInfo.StoreTypeName;
var storeTypeNameBase = mappingInfo.StoreTypeNameBase;

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class SqliteTypeMappingSource : RelationalTypeMappingSource
/// </summary>
protected override RelationalTypeMapping FindMapping(RelationalTypeMappingInfo mappingInfo)
{
var clrType = mappingInfo.ProviderClrType;
var clrType = mappingInfo.ClrType;
if (clrType != null
&& _clrTypeMappings.TryGetValue(clrType, out var mapping))
{
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Storage/Internal/FallbackTypeMappingSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ protected override CoreTypeMapping FindMapping(TypeMappingInfo mappingInfo)
{
Check.NotNull(mappingInfo, nameof(mappingInfo));

return _typeMapper.IsTypeMapped(mappingInfo.ProviderClrType)
? new CoreTypeMapping(mappingInfo.ProviderClrType)
return _typeMapper.IsTypeMapped(mappingInfo.ClrType)
? new CoreTypeMapping(mappingInfo.ClrType)
: null;
}
}
Expand Down
113 changes: 18 additions & 95 deletions src/EFCore/Storage/TypeMappingInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ namespace Microsoft.EntityFrameworkCore.Storage
/// </summary>
public abstract class TypeMappingInfo
{
private readonly ValueConverter _customConverter;

/// <summary>
/// Creates a new instance of <see cref="TypeMappingInfo" />.
/// </summary>
Expand All @@ -34,53 +32,17 @@ protected TypeMappingInfo([NotNull] IProperty property)
{
Check.NotNull(property, nameof(property));

Property = property;

MemberInfo = property.GetIdentifyingMemberInfo();

var principals = property.FindPrincipals().ToList();

ConfiguredProviderClrType = principals
.Select(p => p.GetProviderClrType())
.FirstOrDefault(t => t != null)
?.UnwrapNullableType();

_customConverter = principals
.Select(p => p.GetValueConverter())
.FirstOrDefault(c => c != null);

var mappingHints = _customConverter?.MappingHints;

if (_customConverter != null)
{
ValueConverterInfo = new ValueConverterInfo(
_customConverter.ModelClrType,
_customConverter.ProviderClrType,
i => _customConverter,
mappingHints);
}

Property = property;
MemberInfo = property.GetIdentifyingMemberInfo();
IsKeyOrIndex = property.IsKeyOrForeignKey() || property.IsIndex();

Size = principals.Select(p => p.GetMaxLength()).FirstOrDefault(t => t != null) ?? mappingHints?.Size;

IsUnicode = principals.Select(p => p.IsUnicode()).FirstOrDefault(t => t != null)
?? mappingHints?.IsUnicode;

Size = principals.Select(p => p.GetMaxLength()).FirstOrDefault(t => t != null);
IsUnicode = principals.Select(p => p.IsUnicode()).FirstOrDefault(t => t != null);
IsRowVersion = property.IsConcurrencyToken && property.ValueGenerated == ValueGenerated.OnAddOrUpdate;

Precision = mappingHints?.Precision;

Scale = mappingHints?.Scale;

ModelClrType = property.ClrType.UnwrapNullableType();

ProviderClrType = CreateProviderClrType();
ClrType = property.ClrType.UnwrapNullableType();
}

private Type CreateProviderClrType()
=> ValueConverterInfo?.ProviderClrType.UnwrapNullableType() ?? ConfiguredProviderClrType ?? ModelClrType;

/// <summary>
/// Creates a new instance of <see cref="TypeMappingInfo" />.
/// </summary>
Expand All @@ -89,8 +51,7 @@ protected TypeMappingInfo([NotNull] Type type)
{
Check.NotNull(type, nameof(type));

ModelClrType = type.UnwrapNullableType();
ProviderClrType = CreateProviderClrType();
ClrType = type.UnwrapNullableType();
}

/// <summary>
Expand All @@ -101,9 +62,8 @@ protected TypeMappingInfo([NotNull] MemberInfo member)
{
Check.NotNull(member, nameof(member));

ModelClrType = member.GetMemberType().UnwrapNullableType();
ClrType = member.GetMemberType().UnwrapNullableType();
MemberInfo = member;
ProviderClrType = CreateProviderClrType();
}

/// <summary>
Expand Down Expand Up @@ -138,52 +98,34 @@ protected TypeMappingInfo([NotNull] MemberInfo member)
/// Creates a new instance of <see cref="TypeMappingInfo" /> with the given <see cref="ValueConverterInfo" />.
/// </summary>
/// <param name="source"> The source info. </param>
/// <param name="builtInConverter"> The converter to apply. </param>
/// <param name="converter"> The converter to apply. </param>
protected TypeMappingInfo(
[NotNull] TypeMappingInfo source,
ValueConverterInfo builtInConverter)
ValueConverterInfo converter)
{
Check.NotNull(source, nameof(source));

Property = source.Property;
ModelClrType = source.ModelClrType;
ConfiguredProviderClrType = source.ConfiguredProviderClrType;
IsRowVersion = source.IsRowVersion;
IsKeyOrIndex = source.IsKeyOrIndex;
MemberInfo = source.MemberInfo;

if (source._customConverter != null)
{
_customConverter = source._customConverter;

ValueConverterInfo = new ValueConverterInfo(
_customConverter.ModelClrType,
builtInConverter.ProviderClrType,
i => _customConverter.ComposeWith(builtInConverter.Create()),
builtInConverter.MappingHints == null
? _customConverter.MappingHints
: builtInConverter.MappingHints.With(_customConverter.MappingHints));
}
else
{
ValueConverterInfo = builtInConverter;
}

// ReSharper disable once VirtualMemberCallInConstructor
var mappingHints = ValueConverterInfo?.MappingHints;
var mappingHints = converter.MappingHints;

Size = source.Size ?? mappingHints?.Size;
IsUnicode = source.IsUnicode ?? mappingHints?.IsUnicode;
Scale = source.Scale ?? mappingHints?.Scale;
Precision = source.Precision ?? mappingHints?.Precision;
ProviderClrType = CreateProviderClrType();

ClrType = converter.ProviderClrType.UnwrapNullableType();
}

/// <summary>
/// Returns a new <see cref="TypeMappingInfo" /> with the given converter applied.
/// </summary>
/// <param name="converterInfo"> The converter to apply. </param>
/// <returns> The new mapping info. </returns>
public abstract TypeMappingInfo WithBuiltInConverter(ValueConverterInfo converterInfo);
public abstract TypeMappingInfo WithConverter(ValueConverterInfo converterInfo);

/// <summary>
/// The property for which mapping is needed.
Expand Down Expand Up @@ -220,47 +162,30 @@ protected TypeMappingInfo([NotNull] MemberInfo member)
/// </summary>
public virtual int? Scale { get; }

/// <summary>
/// The CLR type set to use when reading/writing to/from the store.
/// </summary>
public virtual Type ConfiguredProviderClrType { get; }

/// <summary>
/// The field or property info for the property.
/// </summary>
public virtual MemberInfo MemberInfo { get; }

/// <summary>
/// The <see cref="ValueConverter" /> to use when reading/writing to/from the database provider.
/// </summary>
public virtual ValueConverterInfo? ValueConverterInfo { get; }

/// <summary>
/// The CLR type in the model.
/// </summary>
public virtual Type ModelClrType { get; }

/// <summary>
/// The CLR type targeted by the type mapping when reading/writing to/from the databaseProvider.
/// </summary>
public virtual Type ProviderClrType { get; }
public virtual Type ClrType { get; }

/// <summary>
/// Compares this <see cref="TypeMappingInfo" /> to another to check if they represent the same mapping.
/// </summary>
/// <param name="other"> The other object. </param>
/// <returns> <c>True</c> if they represent the same mapping; <c>false</c> otherwise. </returns>
protected virtual bool Equals([NotNull] TypeMappingInfo other)
=> ModelClrType == other.ModelClrType
=> ClrType == other.ClrType
&& MemberInfo == other.MemberInfo
&& ConfiguredProviderClrType == other.ConfiguredProviderClrType
&& IsKeyOrIndex == other.IsKeyOrIndex
&& Size == other.Size
&& IsUnicode == other.IsUnicode
&& IsRowVersion == other.IsRowVersion
&& Precision == other.Precision
&& Scale == other.Scale
&& Equals(_customConverter, other._customConverter);
&& Scale == other.Scale;

/// <summary>
/// Compares this <see cref="TypeMappingInfo" /> to another to check if they represent the same mapping.
Expand All @@ -279,16 +204,14 @@ public override bool Equals(object obj)
/// <returns> The hash code. </returns>
public override int GetHashCode()
{
var hashCode = (ConfiguredProviderClrType != null ? ConfiguredProviderClrType.GetHashCode() : 0);
var hashCode = ClrType?.GetHashCode() ?? 0;
hashCode = (hashCode * 397) ^ IsKeyOrIndex.GetHashCode();
hashCode = (hashCode * 397) ^ (Size?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (MemberInfo?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (IsUnicode?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (IsRowVersion?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (Scale?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (Precision?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (_customConverter?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (ModelClrType?.GetHashCode() ?? 0);
return hashCode;
}
}
Expand Down

0 comments on commit 9172c66

Please sign in to comment.