Skip to content

Commit

Permalink
Remove 2.1 provider fallback behavior
Browse files Browse the repository at this point in the history 
* Removed precision/scale 2.1 fallback behavior
* Made RelationalTypeMapping.Clone() abstract

Part of #12405
  • Loading branch information
@roji
roji committed Feb 28, 2019
1 parent 6364dea commit c5167be
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 106 deletions.
119 changes: 24 additions & 95 deletions src/EFCore.Relational/Storage/RelationalTypeMapping.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,6 @@ public RelationalTypeMappingParameters(
Precision = precision ?? converterHints?.Precision;
Scale = scale ?? converterHints?.Scale;
FixedLength = fixedLength;
PrecisionAndScaleOverriden = false;
}

// #12405
private RelationalTypeMappingParameters(
CoreTypeMappingParameters coreParameters,
[NotNull] string storeType,
StoreTypePostfix storeTypePostfix = StoreTypePostfix.None,
DbType? dbType = null,
bool unicode = false,
int? size = null,
bool fixedLength = false,
int? precision = null,
int? scale = null,
bool precisionAndScaleOverriden = false)
: this(coreParameters, storeType, storeTypePostfix, dbType, unicode, size, fixedLength, precision, scale)
{
PrecisionAndScaleOverriden = precisionAndScaleOverriden;
}

/// <summary>
Expand Down Expand Up @@ -124,14 +106,6 @@ private RelationalTypeMappingParameters(
/// </summary>
public int? Scale { get; }

/// <summary>
/// This is provided for compatibility with 2.1 providers and shouldn't be used
/// </summary>
// If not set fallback to 2.1 behavior by using Precision and Scale from the converter
// #12405
[Obsolete("This is provided for compatibility with 2.1 providers and shouldn't be used")]
public bool PrecisionAndScaleOverriden { get; }

/// <summary>
/// The mapping fixed-length flag.
/// </summary>
Expand All @@ -158,8 +132,7 @@ public RelationalTypeMappingParameters WithTypeMappingInfo(in RelationalTypeMapp
mappingInfo.Size ?? Size,
mappingInfo.IsFixedLength ?? FixedLength,
mappingInfo.Precision ?? Precision,
mappingInfo.Scale ?? Scale,
PrecisionAndScaleOverriden);
mappingInfo.Scale ?? Scale);

/// <summary>
/// Creates a new <see cref="RelationalTypeMappingParameters" /> parameter object with the given
Expand All @@ -182,8 +155,7 @@ public RelationalTypeMappingParameters WithStoreTypeAndSize(
size,
FixedLength,
Precision,
Scale,
PrecisionAndScaleOverriden);
Scale);

/// <summary>
/// Creates a new <see cref="RelationalTypeMappingParameters" /> parameter object with the given
Expand All @@ -204,8 +176,7 @@ public RelationalTypeMappingParameters WithPrecisionAndScale(
Size,
FixedLength,
precision,
scale,
precisionAndScaleOverriden: true);
scale);

/// <summary>
/// Creates a new <see cref="RelationalTypeMappingParameters" /> parameter object with the given
Expand All @@ -223,8 +194,7 @@ public RelationalTypeMappingParameters WithComposedConverter([CanBeNull] ValueCo
Size,
FixedLength,
Precision,
Scale,
PrecisionAndScaleOverriden);
Scale);
}

private static readonly MethodInfo _getFieldValueMethod
Expand Down Expand Up @@ -259,8 +229,6 @@ private static MethodInfo GetDataReaderMethod(string name) =>
/// </summary>
public static readonly RelationalTypeMapping NullMapping = new NullTypeMapping("NULL");

private readonly bool _precisionAndScaleOverriden;

private class NullTypeMapping : RelationalTypeMapping
{
public NullTypeMapping(string storeType)
Expand All @@ -280,40 +248,24 @@ protected RelationalTypeMapping(RelationalTypeMappingParameters parameters)
: base(parameters.CoreParameters)
{
Parameters = parameters;
_precisionAndScaleOverriden = parameters.PrecisionAndScaleOverriden;

var size = parameters.Size;
var storeType = parameters.StoreType;

if (storeType != null)
{
StoreTypeNameBase = GetBaseName(storeType);
if (size != null
&& parameters.StoreTypePostfix == StoreTypePostfix.Size)
if (size != null && parameters.StoreTypePostfix == StoreTypePostfix.Size)
{
storeType = StoreTypeNameBase + "(" + size + ")";
}
else if (parameters.StoreTypePostfix == StoreTypePostfix.PrecisionAndScale
|| parameters.StoreTypePostfix == StoreTypePostfix.Precision)
{
var precision = parameters.Precision;
var converter = parameters.CoreParameters.Converter;
// Fallback to 2.1 behavior
// #12405
var oldBehavior = !_precisionAndScaleOverriden;
if (oldBehavior)
{
precision = converter?.MappingHints?.Precision;
}

if (precision != null)
{
var scale = parameters.Scale;
if (oldBehavior)
{
scale = converter.MappingHints?.Scale;
}

storeType = StoreTypeNameBase
+ "("
+ (scale == null || parameters.StoreTypePostfix == StoreTypePostfix.Precision
Expand Down Expand Up @@ -368,8 +320,7 @@ protected RelationalTypeMapping(
/// </summary>
/// <param name="parameters"> The parameters for this mapping. </param>
/// <returns> The newly created mapping. </returns>
protected virtual RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
=> throw new NotImplementedException(CoreStrings.ConverterCloneNotImplemented(GetType().ShortDisplayName()));
protected abstract RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters);

/// <summary>
/// Creates a copy of this mapping.
Expand Down Expand Up @@ -405,8 +356,6 @@ public override CoreTypeMapping Clone(ValueConverter converter)
/// <returns> The cloned mapping, or the original mapping if no clone was needed. </returns>
public virtual RelationalTypeMapping Clone(in RelationalTypeMappingInfo mappingInfo)
{
var checkStoreTypeAndSize = true;
RelationalTypeMapping clone = null;
if ((mappingInfo.Scale != null
&& mappingInfo.Scale != Parameters.Scale
&& StoreTypePostfix == StoreTypePostfix.PrecisionAndScale)
Expand All @@ -415,47 +364,27 @@ public virtual RelationalTypeMapping Clone(in RelationalTypeMappingInfo mappingI
&& (StoreTypePostfix == StoreTypePostfix.PrecisionAndScale
|| StoreTypePostfix == StoreTypePostfix.Precision)))
{
var oldBehavior = !_precisionAndScaleOverriden;
if (!oldBehavior)
{
var storeTypeChanged = mappingInfo.StoreTypeNameBase != null
&& !string.Equals(mappingInfo.StoreTypeNameBase, StoreTypeNameBase, StringComparison.OrdinalIgnoreCase);

clone = storeTypeChanged
? Clone(Parameters.WithTypeMappingInfo(mappingInfo))
: Clone(
mappingInfo.Precision ?? Parameters.Precision,
mappingInfo.Scale ?? Parameters.Scale);

// Fallback to 2.1 behavior if Clone is not overriden
// #12405
oldBehavior = clone.GetType() != GetType();
}

checkStoreTypeAndSize = oldBehavior;
}

if (checkStoreTypeAndSize)
{
var storeTypeOrSizeChanged = (mappingInfo.Size != null
&& mappingInfo.Size != Size
&& StoreTypePostfix == StoreTypePostfix.Size)
|| (mappingInfo.StoreTypeName != null
&& !string.Equals(mappingInfo.StoreTypeName, StoreType, StringComparison.OrdinalIgnoreCase));

clone = storeTypeOrSizeChanged
? Clone(
mappingInfo.StoreTypeName ?? StoreType,
mappingInfo.Size ?? Size)
: this;
var storeTypeChanged = mappingInfo.StoreTypeNameBase != null
&& !string.Equals(mappingInfo.StoreTypeNameBase, StoreTypeNameBase, StringComparison.OrdinalIgnoreCase);

return storeTypeChanged
? Clone(Parameters.WithTypeMappingInfo(mappingInfo))
: Clone(
mappingInfo.Precision ?? Parameters.Precision,
mappingInfo.Scale ?? Parameters.Scale);
}

if (clone.GetType() != GetType())
{
throw new NotImplementedException(CoreStrings.ConverterCloneNotImplemented(GetType().ShortDisplayName()));
}
var storeTypeOrSizeChanged = (mappingInfo.Size != null
&& mappingInfo.Size != Size
&& StoreTypePostfix == StoreTypePostfix.Size)
|| (mappingInfo.StoreTypeName != null
&& !string.Equals(mappingInfo.StoreTypeName, StoreType, StringComparison.OrdinalIgnoreCase));

return clone;
return storeTypeOrSizeChanged
? Clone(
mappingInfo.StoreTypeName ?? StoreType,
mappingInfo.Size ?? Size)
: this;
}

/// <summary>
Expand Down
8 changes: 0 additions & 8 deletions src/EFCore/Properties/CoreStrings.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions src/EFCore/Properties/CoreStrings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -920,9 +920,6 @@
<data name="ConvertersCannotBeComposed" xml:space="preserve">
<value>Cannot compose converter from '{typeOneIn}' to '{typeOneOut}' with converter from '{typeTwoIn}' to '{typeTwoOut}' because the output type of the first converter is different from the input type of the second converter.</value>
</data>
<data name="ConverterCloneNotImplemented" xml:space="preserve">
<value>The '{mapping}' does not support value conversions. Support for value conversions typically requires changes in the database provider.</value>
</data>
<data name="ConverterBadType" xml:space="preserve">
<value>The value converter '{converter}' cannot be used with type '{type}'. This converter can only be used with {allowed}.</value>
</data>
Expand Down
6 changes: 6 additions & 0 deletions test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,9 @@ public SimpleTestTypeMapping(

public override Expression GenerateCodeLiteral(object value)
=> _literalExpressionFunc((T)value);

protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
=> throw new NotSupportedException();
}

private class SimpleTestNonImplementedTypeMapping : RelationalTypeMapping
Expand All @@ -642,6 +645,9 @@ public SimpleTestNonImplementedTypeMapping()
: base("storeType", typeof(SimpleTestType))
{
}

protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
=> throw new NotSupportedException();
}
}

Expand Down
3 changes: 3 additions & 0 deletions test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,9 @@ public static object CreateParameters(
unicode: unicide,
fixedLength: fixedLength);
}

protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
=> new FakeTypeMapping(parameters);
}

[Fact]
Expand Down

0 comments on commit c5167be

Please sign in to comment.