From c5167be1993fa7d81a53407d2f23e95091e549a5 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 26 Feb 2019 14:04:55 +0100 Subject: [PATCH] Remove 2.1 provider fallback behavior * Removed precision/scale 2.1 fallback behavior * Made RelationalTypeMapping.Clone() abstract Part of #12405 --- .../Storage/RelationalTypeMapping.cs | 119 ++++-------------- src/EFCore/Properties/CoreStrings.Designer.cs | 8 -- src/EFCore/Properties/CoreStrings.resx | 3 - .../Design/Internal/CSharpHelperTest.cs | 6 + .../Storage/RelationalTypeMappingTest.cs | 3 + 5 files changed, 33 insertions(+), 106 deletions(-) diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs index d649daa2c02..add859a39bc 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs @@ -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; } /// @@ -124,14 +106,6 @@ private RelationalTypeMappingParameters( /// public int? Scale { get; } - /// - /// This is provided for compatibility with 2.1 providers and shouldn't be used - /// - // 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; } - /// /// The mapping fixed-length flag. /// @@ -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); /// /// Creates a new parameter object with the given @@ -182,8 +155,7 @@ public RelationalTypeMappingParameters WithStoreTypeAndSize( size, FixedLength, Precision, - Scale, - PrecisionAndScaleOverriden); + Scale); /// /// Creates a new parameter object with the given @@ -204,8 +176,7 @@ public RelationalTypeMappingParameters WithPrecisionAndScale( Size, FixedLength, precision, - scale, - precisionAndScaleOverriden: true); + scale); /// /// Creates a new parameter object with the given @@ -223,8 +194,7 @@ public RelationalTypeMappingParameters WithComposedConverter([CanBeNull] ValueCo Size, FixedLength, Precision, - Scale, - PrecisionAndScaleOverriden); + Scale); } private static readonly MethodInfo _getFieldValueMethod @@ -259,8 +229,6 @@ private static MethodInfo GetDataReaderMethod(string name) => /// public static readonly RelationalTypeMapping NullMapping = new NullTypeMapping("NULL"); - private readonly bool _precisionAndScaleOverriden; - private class NullTypeMapping : RelationalTypeMapping { public NullTypeMapping(string storeType) @@ -280,7 +248,6 @@ protected RelationalTypeMapping(RelationalTypeMappingParameters parameters) : base(parameters.CoreParameters) { Parameters = parameters; - _precisionAndScaleOverriden = parameters.PrecisionAndScaleOverriden; var size = parameters.Size; var storeType = parameters.StoreType; @@ -288,8 +255,7 @@ protected RelationalTypeMapping(RelationalTypeMappingParameters parameters) if (storeType != null) { StoreTypeNameBase = GetBaseName(storeType); - if (size != null - && parameters.StoreTypePostfix == StoreTypePostfix.Size) + if (size != null && parameters.StoreTypePostfix == StoreTypePostfix.Size) { storeType = StoreTypeNameBase + "(" + size + ")"; } @@ -297,23 +263,9 @@ protected RelationalTypeMapping(RelationalTypeMappingParameters parameters) || 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 @@ -368,8 +320,7 @@ protected RelationalTypeMapping( /// /// The parameters for this mapping. /// The newly created mapping. - protected virtual RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) - => throw new NotImplementedException(CoreStrings.ConverterCloneNotImplemented(GetType().ShortDisplayName())); + protected abstract RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters); /// /// Creates a copy of this mapping. @@ -405,8 +356,6 @@ public override CoreTypeMapping Clone(ValueConverter converter) /// The cloned mapping, or the original mapping if no clone was needed. public virtual RelationalTypeMapping Clone(in RelationalTypeMappingInfo mappingInfo) { - var checkStoreTypeAndSize = true; - RelationalTypeMapping clone = null; if ((mappingInfo.Scale != null && mappingInfo.Scale != Parameters.Scale && StoreTypePostfix == StoreTypePostfix.PrecisionAndScale) @@ -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; } /// diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index fe38e412ec3..8f6114ef90e 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2206,14 +2206,6 @@ public static string ConvertersCannotBeComposed([CanBeNull] object typeOneIn, [C GetString("ConvertersCannotBeComposed", nameof(typeOneIn), nameof(typeOneOut), nameof(typeTwoIn), nameof(typeTwoOut)), typeOneIn, typeOneOut, typeTwoIn, typeTwoOut); - /// - /// The '{mapping}' does not support value conversions. Support for value conversions typically requires changes in the database provider. - /// - public static string ConverterCloneNotImplemented([CanBeNull] object mapping) - => string.Format( - GetString("ConverterCloneNotImplemented", nameof(mapping)), - mapping); - /// /// The value converter '{converter}' cannot be used with type '{type}'. This converter can only be used with {allowed}. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 2eda132ee10..aa826dd832a 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -920,9 +920,6 @@ 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. - - The '{mapping}' does not support value conversions. Support for value conversions typically requires changes in the database provider. - The value converter '{converter}' cannot be used with type '{type}'. This converter can only be used with {allowed}. diff --git a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs index 5684f87591c..5f48d64b009 100644 --- a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs +++ b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs @@ -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 @@ -642,6 +645,9 @@ public SimpleTestNonImplementedTypeMapping() : base("storeType", typeof(SimpleTestType)) { } + + protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) + => throw new NotSupportedException(); } } diff --git a/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs b/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs index 5ff0d98648f..bfc8cae3bdc 100644 --- a/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs +++ b/test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs @@ -243,6 +243,9 @@ public static object CreateParameters( unicode: unicide, fixedLength: fixedLength); } + + protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) + => new FakeTypeMapping(parameters); } [Fact]