From 02feb86df534f43486e80aa91aa9d745dc3bc114 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Fri, 1 Oct 2021 22:23:49 +0100 Subject: [PATCH] Mark internal the ValueConverter constructors that allow conversion of nulls. Resolves #26230. Also revert built-in converters that were changed to allow this in 6.0 back to their 5.0 behavior. --- .../Storage/ValueConversion/BytesToStringConverter.cs | 5 ++--- .../ValueConversion/IPAddressToBytesConverter.cs | 5 ++--- .../ValueConversion/IPAddressToStringConverter.cs | 5 ++--- .../ValueConversion/Internal/StringNumberConverter.cs | 3 +-- .../ValueConversion/Internal/StringUriConverter.cs | 5 ++--- .../ValueConversion/NumberToStringConverter.cs | 1 - .../PhysicalAddressToBytesConverter.cs | 5 ++--- .../PhysicalAddressToStringConverter.cs | 5 ++--- .../Storage/ValueConversion/StringToBytesConverter.cs | 5 ++--- .../ValueConversion/StringToNumberConverter.cs | 1 - src/EFCore/Storage/ValueConversion/ValueConverter.cs | 10 +++++++++- src/EFCore/Storage/ValueConversion/ValueConverter`.cs | 11 ++++++++++- .../Storage/BytesToStringConverterTest.cs | 4 +--- .../Storage/IPAddressToBytesConverterTest.cs | 6 ------ .../Storage/IPAddressToStringConverterTest.cs | 8 -------- .../Storage/PhysicalAddressToBytesConverterTest.cs | 2 -- .../Storage/PhysicalAddressToStringConverterTest.cs | 5 ----- .../Storage/StringToBytesConverterTest.cs | 2 -- test/EFCore.Tests/Storage/StringToUriConverterTest.cs | 2 -- 19 files changed, 35 insertions(+), 55 deletions(-) diff --git a/src/EFCore/Storage/ValueConversion/BytesToStringConverter.cs b/src/EFCore/Storage/ValueConversion/BytesToStringConverter.cs index df92c2db6df..69c9bd07b05 100644 --- a/src/EFCore/Storage/ValueConversion/BytesToStringConverter.cs +++ b/src/EFCore/Storage/ValueConversion/BytesToStringConverter.cs @@ -36,9 +36,8 @@ public BytesToStringConverter() /// public BytesToStringConverter(ConverterMappingHints? mappingHints) : base( - v => v == null ? null : Convert.ToBase64String(v), - v => v == null ? null : Convert.FromBase64String(v), - convertsNulls: true, + v => Convert.ToBase64String(v!), + v => Convert.FromBase64String(v!), mappingHints) { } diff --git a/src/EFCore/Storage/ValueConversion/IPAddressToBytesConverter.cs b/src/EFCore/Storage/ValueConversion/IPAddressToBytesConverter.cs index 338fcd21715..99873980920 100644 --- a/src/EFCore/Storage/ValueConversion/IPAddressToBytesConverter.cs +++ b/src/EFCore/Storage/ValueConversion/IPAddressToBytesConverter.cs @@ -38,9 +38,8 @@ public IPAddressToBytesConverter() /// public IPAddressToBytesConverter(ConverterMappingHints? mappingHints) : base( - v => v == null ? default : v.GetAddressBytes(), - v => v == null ? default : new IPAddress(v), - convertsNulls: true, + v => v!.GetAddressBytes(), + v => new IPAddress(v!), _defaultHints.With(mappingHints)) { } diff --git a/src/EFCore/Storage/ValueConversion/IPAddressToStringConverter.cs b/src/EFCore/Storage/ValueConversion/IPAddressToStringConverter.cs index bb32112ead2..2e20a4a6927 100644 --- a/src/EFCore/Storage/ValueConversion/IPAddressToStringConverter.cs +++ b/src/EFCore/Storage/ValueConversion/IPAddressToStringConverter.cs @@ -40,7 +40,6 @@ public IPAddressToStringConverter(ConverterMappingHints? mappingHints) : base( ToString(), ToIPAddress(), - convertsNulls: true, _defaultHints.With(mappingHints)) { } @@ -52,9 +51,9 @@ public IPAddressToStringConverter(ConverterMappingHints? mappingHints) = new(typeof(IPAddress), typeof(string), i => new IPAddressToStringConverter(i.MappingHints), _defaultHints); private new static Expression> ToString() - => v => v == null ? default : v.ToString(); + => v => v!.ToString(); private static Expression> ToIPAddress() - => v => v == null ? default : IPAddress.Parse(v); + => v => IPAddress.Parse(v!); } } diff --git a/src/EFCore/Storage/ValueConversion/Internal/StringNumberConverter.cs b/src/EFCore/Storage/ValueConversion/Internal/StringNumberConverter.cs index 0ddc099136c..1ede03dd3df 100644 --- a/src/EFCore/Storage/ValueConversion/Internal/StringNumberConverter.cs +++ b/src/EFCore/Storage/ValueConversion/Internal/StringNumberConverter.cs @@ -33,9 +33,8 @@ public class StringNumberConverter : ValueConverter< public StringNumberConverter( Expression> convertToProviderExpression, Expression> convertFromProviderExpression, - bool convertsNulls, ConverterMappingHints? mappingHints = null) - : base(convertToProviderExpression, convertFromProviderExpression, convertsNulls, mappingHints) + : base(convertToProviderExpression, convertFromProviderExpression, mappingHints) { } diff --git a/src/EFCore/Storage/ValueConversion/Internal/StringUriConverter.cs b/src/EFCore/Storage/ValueConversion/Internal/StringUriConverter.cs index b2ecaaebd6e..74a77f8fecb 100644 --- a/src/EFCore/Storage/ValueConversion/Internal/StringUriConverter.cs +++ b/src/EFCore/Storage/ValueConversion/Internal/StringUriConverter.cs @@ -27,7 +27,6 @@ public StringUriConverter( : base( convertToProviderExpression, convertFromProviderExpression, - convertsNulls: true, mappingHints) { } @@ -39,7 +38,7 @@ public StringUriConverter( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// protected new static Expression> ToString() - => v => v != null ? v.ToString() : null; + => v => v!.ToString(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -48,6 +47,6 @@ public StringUriConverter( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// protected static Expression> ToUri() - => v => v != null ? new Uri(v, UriKind.RelativeOrAbsolute) : null; + => v => new Uri(v!, UriKind.RelativeOrAbsolute); } } diff --git a/src/EFCore/Storage/ValueConversion/NumberToStringConverter.cs b/src/EFCore/Storage/ValueConversion/NumberToStringConverter.cs index b4720f695f6..06f1822d1a6 100644 --- a/src/EFCore/Storage/ValueConversion/NumberToStringConverter.cs +++ b/src/EFCore/Storage/ValueConversion/NumberToStringConverter.cs @@ -39,7 +39,6 @@ public NumberToStringConverter(ConverterMappingHints? mappingHints) : base( ToString(), ToNumber(), - typeof(TNumber).IsNullableType(), _defaultHints.With(mappingHints)) { } diff --git a/src/EFCore/Storage/ValueConversion/PhysicalAddressToBytesConverter.cs b/src/EFCore/Storage/ValueConversion/PhysicalAddressToBytesConverter.cs index 7e6ce3501b2..e118fc3a48d 100644 --- a/src/EFCore/Storage/ValueConversion/PhysicalAddressToBytesConverter.cs +++ b/src/EFCore/Storage/ValueConversion/PhysicalAddressToBytesConverter.cs @@ -38,9 +38,8 @@ public PhysicalAddressToBytesConverter() /// public PhysicalAddressToBytesConverter(ConverterMappingHints? mappingHints) : base( - v => v == null ? default : v.GetAddressBytes(), - v => v == null ? default : new PhysicalAddress(v), - convertsNulls: true, + v => v!.GetAddressBytes(), + v => new PhysicalAddress(v!), _defaultHints.With(mappingHints)) { } diff --git a/src/EFCore/Storage/ValueConversion/PhysicalAddressToStringConverter.cs b/src/EFCore/Storage/ValueConversion/PhysicalAddressToStringConverter.cs index 1ba1b624530..58df881d88f 100644 --- a/src/EFCore/Storage/ValueConversion/PhysicalAddressToStringConverter.cs +++ b/src/EFCore/Storage/ValueConversion/PhysicalAddressToStringConverter.cs @@ -42,7 +42,6 @@ public PhysicalAddressToStringConverter(ConverterMappingHints? mappingHints) : base( ToString(), ToPhysicalAddress(), - convertsNulls: true, _defaultHints.With(mappingHints)) { } @@ -54,9 +53,9 @@ public PhysicalAddressToStringConverter(ConverterMappingHints? mappingHints) = new(typeof(PhysicalAddress), typeof(string), i => new PhysicalAddressToStringConverter(i.MappingHints), _defaultHints); private new static Expression> ToString() - => v => v == null ? default! : v.ToString(); + => v => v!.ToString(); private static Expression> ToPhysicalAddress() - => v => v == null ? default! : PhysicalAddress.Parse(v); + => v => PhysicalAddress.Parse(v!); } } diff --git a/src/EFCore/Storage/ValueConversion/StringToBytesConverter.cs b/src/EFCore/Storage/ValueConversion/StringToBytesConverter.cs index 049e39777d2..94ba9465989 100644 --- a/src/EFCore/Storage/ValueConversion/StringToBytesConverter.cs +++ b/src/EFCore/Storage/ValueConversion/StringToBytesConverter.cs @@ -28,9 +28,8 @@ public StringToBytesConverter( Encoding encoding, ConverterMappingHints? mappingHints = null) : base( - v => v == null ? null : encoding.GetBytes(v), - v => v == null ? null : encoding.GetString(v), - convertsNulls: true, + v => encoding.GetBytes(v!), + v => encoding.GetString(v!), mappingHints) { } diff --git a/src/EFCore/Storage/ValueConversion/StringToNumberConverter.cs b/src/EFCore/Storage/ValueConversion/StringToNumberConverter.cs index a1aeb415da2..e9541bf14e1 100644 --- a/src/EFCore/Storage/ValueConversion/StringToNumberConverter.cs +++ b/src/EFCore/Storage/ValueConversion/StringToNumberConverter.cs @@ -39,7 +39,6 @@ public StringToNumberConverter(ConverterMappingHints? mappingHints) : base( ToNumber(), ToString(), - typeof(TNumber).IsNullableType(), _defaultHints.With(mappingHints)) { } diff --git a/src/EFCore/Storage/ValueConversion/ValueConverter.cs b/src/EFCore/Storage/ValueConversion/ValueConverter.cs index 4857c8be896..553b26f9dd1 100644 --- a/src/EFCore/Storage/ValueConversion/ValueConverter.cs +++ b/src/EFCore/Storage/ValueConversion/ValueConverter.cs @@ -49,7 +49,14 @@ protected ValueConverter( } /// - /// Initializes a new instance of the class. + /// + /// Initializes a new instance of the class, allowing conversion of + /// nulls. + /// + /// + /// Warning: this is currently an internal API since converting nulls to and from the database can lead to broken + /// queries and other issues. See https://github.com/dotnet/efcore/issues/26230 for more information. + /// /// /// /// See EF Core value converters for more information. @@ -72,6 +79,7 @@ protected ValueConverter( /// Hints that can be used by the to create data types with appropriate /// facets for the converted data. /// + [EntityFrameworkInternal] protected ValueConverter( LambdaExpression convertToProviderExpression, LambdaExpression convertFromProviderExpression, diff --git a/src/EFCore/Storage/ValueConversion/ValueConverter`.cs b/src/EFCore/Storage/ValueConversion/ValueConverter`.cs index dcaa37f8e9f..06743d16280 100644 --- a/src/EFCore/Storage/ValueConversion/ValueConverter`.cs +++ b/src/EFCore/Storage/ValueConversion/ValueConverter`.cs @@ -3,6 +3,7 @@ using System; using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; namespace Microsoft.EntityFrameworkCore.Storage.ValueConversion @@ -40,7 +41,14 @@ public ValueConverter( } /// - /// Initializes a new instance of the class. + /// + /// Initializes a new instance of the class, allowing conversion of + /// nulls. + /// + /// + /// Warning: this is currently an internal API since converting nulls to and from the database can lead to broken + /// queries and other issues. See https://github.com/dotnet/efcore/issues/26230 for more information. + /// /// /// /// See EF Core value converters for more information. @@ -55,6 +63,7 @@ public ValueConverter( /// Hints that can be used by the to create data types with appropriate /// facets for the converted data. /// + [EntityFrameworkInternal] public ValueConverter( Expression> convertToProviderExpression, Expression> convertFromProviderExpression, diff --git a/test/EFCore.Tests/Storage/BytesToStringConverterTest.cs b/test/EFCore.Tests/Storage/BytesToStringConverterTest.cs index eabb9f47d10..22130d7fa93 100644 --- a/test/EFCore.Tests/Storage/BytesToStringConverterTest.cs +++ b/test/EFCore.Tests/Storage/BytesToStringConverterTest.cs @@ -15,11 +15,10 @@ public class BytesToStringConverterTest public void Can_convert_strings_to_bytes() { var converter = _bytesToStringConverter.ConvertToProviderExpression.Compile(); - Assert.True(_bytesToStringConverter.ConvertsNulls); + Assert.False(_bytesToStringConverter.ConvertsNulls); Assert.Equal("U3DEsW7MiGFsIFRhcA==", converter(new byte[] { 83, 112, 196, 177, 110, 204, 136, 97, 108, 32, 84, 97, 112 })); Assert.Equal("", converter(Array.Empty())); - Assert.Null(converter(null)); } [ConditionalFact] @@ -29,7 +28,6 @@ public void Can_convert_bytes_to_strings() Assert.Equal(new byte[] { 83, 112, 196, 177, 110, 204, 136, 97, 108, 32, 84, 97, 112 }, converter("U3DEsW7MiGFsIFRhcA==")); Assert.Equal(Array.Empty(), converter("")); - Assert.Null(converter(null)); } [ConditionalFact] diff --git a/test/EFCore.Tests/Storage/IPAddressToBytesConverterTest.cs b/test/EFCore.Tests/Storage/IPAddressToBytesConverterTest.cs index dde1236372b..b0d34787762 100644 --- a/test/EFCore.Tests/Storage/IPAddressToBytesConverterTest.cs +++ b/test/EFCore.Tests/Storage/IPAddressToBytesConverterTest.cs @@ -25,8 +25,6 @@ public void Can_convert_ipaddress_ipv4_to_bytes(string ipv4) var bytes = ip.GetAddressBytes(); Assert.Equal(bytes, converter(ip)); - - Assert.Null(converter(null)); } [ConditionalTheory] @@ -57,8 +55,6 @@ public void Can_convert_bytes_to_ipaddress_ipv4(byte[] bytesIPV4Invalid) Assert.Throws( () => converter(bytesIPV4Invalid)); - - Assert.Null(converter(null)); } [ConditionalTheory] @@ -75,8 +71,6 @@ public void Can_convert_bytes_to_ipaddress_ipv6(string ipv6) var bytes = ip.GetAddressBytes(); Assert.Equal(ip, converter(bytes)); - - Assert.Null(converter(null)); } [ConditionalTheory] diff --git a/test/EFCore.Tests/Storage/IPAddressToStringConverterTest.cs b/test/EFCore.Tests/Storage/IPAddressToStringConverterTest.cs index 99cbd19a524..f11fbd5f0ea 100644 --- a/test/EFCore.Tests/Storage/IPAddressToStringConverterTest.cs +++ b/test/EFCore.Tests/Storage/IPAddressToStringConverterTest.cs @@ -23,8 +23,6 @@ public void Can_convert_ipaddress_ipv4_to_String(string ipv4) var converter = _ipAddressToString.ConvertToProviderExpression.Compile(); Assert.Equal(ipv4, converter(IPAddress.Parse(ipv4))); - - Assert.Null(converter(null)); } [ConditionalTheory] @@ -38,8 +36,6 @@ public void Can_convert_ipaddress_ipv6_to_String(string ipv6) var converter = _ipAddressToString.ConvertToProviderExpression.Compile(); Assert.Equal(ipv6, converter(IPAddress.Parse(ipv6))); - - Assert.Null(converter(null)); } [ConditionalTheory] @@ -55,8 +51,6 @@ public void Can_convert_String_to_ipaddress_ipv4(string ipv4) Assert.Equal( IPAddress.Parse(ipv4), converter(ipv4)); - - Assert.Null(converter(null)); } [ConditionalTheory] @@ -72,8 +66,6 @@ public void Can_convert_String_to_ipaddress_ipv6(string ipv6) Assert.Equal( IPAddress.Parse(ipv6), converter(ipv6)); - - Assert.Null(converter(null)); } } } diff --git a/test/EFCore.Tests/Storage/PhysicalAddressToBytesConverterTest.cs b/test/EFCore.Tests/Storage/PhysicalAddressToBytesConverterTest.cs index 7cea1ced725..a92c2d237ad 100644 --- a/test/EFCore.Tests/Storage/PhysicalAddressToBytesConverterTest.cs +++ b/test/EFCore.Tests/Storage/PhysicalAddressToBytesConverterTest.cs @@ -21,7 +21,6 @@ public void Can_convert_physical_address_to_bytes(string address) var bytes = physicalAddress.GetAddressBytes(); Assert.Equal(bytes, converter(physicalAddress)); - Assert.Null(converter(null)); } [ConditionalTheory] @@ -46,7 +45,6 @@ public void Can_convert_bytes_to_physical_address(string address) var bytes = physicalAddress.GetAddressBytes(); Assert.Equal(physicalAddress, converter(bytes)); - Assert.Null(converter(null)); } [ConditionalTheory] diff --git a/test/EFCore.Tests/Storage/PhysicalAddressToStringConverterTest.cs b/test/EFCore.Tests/Storage/PhysicalAddressToStringConverterTest.cs index 86e011656ee..f84112fabae 100644 --- a/test/EFCore.Tests/Storage/PhysicalAddressToStringConverterTest.cs +++ b/test/EFCore.Tests/Storage/PhysicalAddressToStringConverterTest.cs @@ -26,8 +26,6 @@ public void Can_convert_physical_address_to_String(string physicalAddress) Assert.Equal( alphaNumerics.Replace(physicalAddress, ""), converter(PhysicalAddress.Parse(physicalAddress))); - - Assert.Null(converter(null)); } [ConditionalTheory] @@ -39,8 +37,6 @@ public void Can_convert_String_to_physical_address(string physicalAddress) Assert.Equal( PhysicalAddress.Parse(physicalAddress), converter(physicalAddress)); - - Assert.Null(converter(null)); } [ConditionalTheory] @@ -59,7 +55,6 @@ public void Can_convert_bytes_to_physical_address(byte[] bytesPhysicalAddressInv converter(physicalAddress); }); - Assert.Null(converter(null)); Assert.Equal($"An invalid physical address was specified: '{physicalAddress}'.", exception.Message); } diff --git a/test/EFCore.Tests/Storage/StringToBytesConverterTest.cs b/test/EFCore.Tests/Storage/StringToBytesConverterTest.cs index ae137802392..2f256f7fb45 100644 --- a/test/EFCore.Tests/Storage/StringToBytesConverterTest.cs +++ b/test/EFCore.Tests/Storage/StringToBytesConverterTest.cs @@ -19,7 +19,6 @@ public void Can_convert_strings_to_UTF8() Assert.Equal(new byte[] { 83, 112, 196, 177, 110, 204, 136, 97, 108, 32, 84, 97, 112 }, converter("Spın̈al Tap")); Assert.Equal(Array.Empty(), converter("")); - Assert.Null(converter(null)); } [ConditionalFact] @@ -29,7 +28,6 @@ public void Can_convert_UTF8_to_strings() Assert.Equal("Spın̈al Tap", converter(new byte[] { 83, 112, 196, 177, 110, 204, 136, 97, 108, 32, 84, 97, 112 })); Assert.Equal("", converter(Array.Empty())); - Assert.Null(converter(null)); } [ConditionalFact] diff --git a/test/EFCore.Tests/Storage/StringToUriConverterTest.cs b/test/EFCore.Tests/Storage/StringToUriConverterTest.cs index 2c07ae65ff6..7e2ea994843 100644 --- a/test/EFCore.Tests/Storage/StringToUriConverterTest.cs +++ b/test/EFCore.Tests/Storage/StringToUriConverterTest.cs @@ -21,7 +21,6 @@ public void Can_convert_strings_to_uris() Assert.Equal(new Uri("ftp://www.github.com", UriKind.Absolute), converter("ftp://www.github.com/")); Assert.Equal(new Uri(".", UriKind.Relative), converter(".")); - Assert.Null(converter(null)); Assert.Throws(() => converter("http:///")); } @@ -48,7 +47,6 @@ public void Can_convert_uris_to_strings() Assert.Equal("/relative/path", converter(new Uri("/relative/path", UriKind.Relative))); Assert.Equal("ftp://www.github.com/", converter(new Uri("ftp://www.github.com/", UriKind.Absolute))); Assert.Equal(".", converter(new Uri(".", UriKind.Relative))); - Assert.Null(converter(null)); } [ConditionalFact]