Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Properly detect nullability in nested types #24713

Merged
merged 1 commit into from
Apr 21, 2021
Merged

Properly detect nullability in nested types #24713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 40 additions & 31 deletions src/EFCore/Metadata/Conventions/NonNullableConventionBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,46 +96,55 @@ PropertyInfo p

// No attribute on the member, try to find a NullableContextAttribute on the declaring type
var type = memberInfo.DeclaringType;
if (type != null)
if (type is not null)
{
if (state.TypeCache.TryGetValue(type, out var cachedTypeNonNullable))
// We currently don't calculate support nullability for generic properties, since calculating that is complex
// (depends on the nullability of generic type argument).
// However, we special case Dictionary as it's used for property bags, and specifically don't identify its indexer
// as non-nullable.
if (memberInfo is PropertyInfo property
&& property.IsIndexerProperty()
&& type.IsGenericType
&& type.GetGenericTypeDefinition() == typeof(Dictionary<,>))
{
return cachedTypeNonNullable;
return false;
}

if (Attribute.GetCustomAttributes(type)
.FirstOrDefault(a => a.GetType().FullName == NullableContextAttributeFullName) is Attribute contextAttr)
return DoesTypeHaveNonNullableContext(type, state);
}

return false;
}

private bool DoesTypeHaveNonNullableContext(Type type, NonNullabilityConventionState state)
{
if (state.TypeCache.TryGetValue(type, out var cachedTypeNonNullable))
{
return cachedTypeNonNullable;
}

if (Attribute.GetCustomAttributes(type)
.FirstOrDefault(a => a.GetType().FullName == NullableContextAttributeFullName) is Attribute contextAttr)
{
var attributeType = contextAttr.GetType();

if (attributeType != state.NullableContextAttrType)
{
var attributeType = contextAttr.GetType();

if (attributeType != state.NullableContextAttrType)
{
state.NullableContextFlagFieldInfo = attributeType.GetField("Flag");
state.NullableContextAttrType = attributeType;
}

if (state.NullableContextFlagFieldInfo?.GetValue(contextAttr) is byte flag)
{
// We currently don't calculate support nullability for generic properties, since calculating that is complex
// (depends on the nullability of generic type argument).
// However, we special case Dictionary as it's used for property bags, and specifically don't identify its indexer
// as non-nullable.
if (memberInfo is PropertyInfo property
&& property.IsIndexerProperty()
&& type.IsGenericType
&& type.GetGenericTypeDefinition() == typeof(Dictionary<,>))
{
return false;
}

return state.TypeCache[type] = flag == 1;
}
state.NullableContextFlagFieldInfo = attributeType.GetField("Flag");
state.NullableContextAttrType = attributeType;
}

return state.TypeCache[type] = false;
if (state.NullableContextFlagFieldInfo?.GetValue(contextAttr) is byte flag)
{
return state.TypeCache[type] = flag == 1;
}
}
else if (type.IsNested)
{
return state.TypeCache[type] = DoesTypeHaveNonNullableContext(type.DeclaringType!, state);
}

return false;
return state.TypeCache[type] = false;
}

private NonNullabilityConventionState GetOrInitializeState(IConventionModelBuilder modelBuilder)
Expand Down
70 changes: 32 additions & 38 deletions test/EFCore.Specification.Tests/ValueConvertersEndToEndTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,9 +631,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.Property(e => e.NullableIntAsNullableLong).HasConversion(new CastingConverter<int?, long?>());

b.Property(e => e.BytesAsString).HasConversion(
(ValueConverter?)new BytesToStringConverter(), new ArrayStructuralComparer<byte>()).IsRequired(); // Issue #24686
(ValueConverter?)new BytesToStringConverter(), new ArrayStructuralComparer<byte>());
b.Property(e => e.BytesAsNullableString).HasConversion(
(ValueConverter?)new BytesToStringConverter(), new ArrayStructuralComparer<byte>()).IsRequired(); // Issue #24686
(ValueConverter?)new BytesToStringConverter(), new ArrayStructuralComparer<byte>());
b.Property(e => e.NullableBytesAsString).HasConversion(
new BytesToStringConverter(), new ArrayStructuralComparer<byte>());
b.Property(e => e.NullableBytesAsNullableString).HasConversion(
Expand Down Expand Up @@ -684,29 +684,25 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.Property(e => e.NullableGuidToBytes).HasConversion(new GuidToBytesConverter());
b.Property(e => e.NullableGuidToNullableBytes).HasConversion(new GuidToBytesConverter());

b.Property(e => e.IPAddressToString).HasConversion((ValueConverter?)new IPAddressToStringConverter()).IsRequired(); // Issue #24686
b.Property(e => e.IPAddressToNullableString).HasConversion((ValueConverter?)new IPAddressToStringConverter())
.IsRequired(); // Issue #24686
b.Property(e => e.IPAddressToString).HasConversion((ValueConverter?)new IPAddressToStringConverter());
b.Property(e => e.IPAddressToNullableString).HasConversion((ValueConverter?)new IPAddressToStringConverter());
b.Property(e => e.NullableIPAddressToString).HasConversion(new IPAddressToStringConverter());
b.Property(e => e.NullableIPAddressToNullableString).HasConversion(new IPAddressToStringConverter());

b.Property(e => e.IPAddressToBytes).HasConversion((ValueConverter?)new IPAddressToBytesConverter()).IsRequired(); // Issue #24686
b.Property(e => e.IPAddressToNullableBytes).HasConversion((ValueConverter?)new IPAddressToBytesConverter())
.IsRequired(); // Issue #24686
b.Property(e => e.IPAddressToBytes).HasConversion((ValueConverter?)new IPAddressToBytesConverter());
b.Property(e => e.IPAddressToNullableBytes).HasConversion((ValueConverter?)new IPAddressToBytesConverter());
b.Property(e => e.NullableIPAddressToBytes).HasConversion(new IPAddressToBytesConverter());
b.Property(e => e.NullableIPAddressToNullableBytes).HasConversion(new IPAddressToBytesConverter());

b.Property(e => e.PhysicalAddressToString).HasConversion((ValueConverter?)new PhysicalAddressToStringConverter())
.IsRequired(); // Issue #24686
b.Property(e => e.PhysicalAddressToString).HasConversion((ValueConverter?)new PhysicalAddressToStringConverter());
b.Property(e => e.PhysicalAddressToNullableString)
.HasConversion((ValueConverter?)new PhysicalAddressToStringConverter()).IsRequired(); // Issue #24686
.HasConversion((ValueConverter?)new PhysicalAddressToStringConverter());
b.Property(e => e.NullablePhysicalAddressToString).HasConversion(new PhysicalAddressToStringConverter());
b.Property(e => e.NullablePhysicalAddressToNullableString).HasConversion(new PhysicalAddressToStringConverter());

b.Property(e => e.PhysicalAddressToBytes).HasConversion((ValueConverter?)new PhysicalAddressToBytesConverter())
.IsRequired(); // Issue #24686
b.Property(e => e.PhysicalAddressToBytes).HasConversion((ValueConverter?)new PhysicalAddressToBytesConverter());
b.Property(e => e.PhysicalAddressToNullableBytes)
.HasConversion((ValueConverter?)new PhysicalAddressToBytesConverter()).IsRequired(); // Issue #24686
.HasConversion((ValueConverter?)new PhysicalAddressToBytesConverter());
b.Property(e => e.NullablePhysicalAddressToBytes).HasConversion(new PhysicalAddressToBytesConverter());
b.Property(e => e.NullablePhysicalAddressToNullableBytes).HasConversion(new PhysicalAddressToBytesConverter());

Expand All @@ -720,54 +716,52 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.Property(e => e.NullableNumberToBytes).HasConversion(new NumberToBytesConverter<sbyte>());
b.Property(e => e.NullableNumberToNullableBytes).HasConversion(new NumberToBytesConverter<sbyte>());

b.Property(e => e.StringToBool).HasConversion(new StringToBoolConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableBool).HasConversion(new StringToBoolConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToBool).HasConversion(new StringToBoolConverter());
b.Property(e => e.StringToNullableBool).HasConversion(new StringToBoolConverter());
b.Property(e => e.NullableStringToBool).HasConversion((ValueConverter?)new StringToBoolConverter());
b.Property(e => e.NullableStringToNullableBool).HasConversion((ValueConverter?)new StringToBoolConverter());

b.Property(e => e.StringToBytes).HasConversion((ValueConverter?)new StringToBytesConverter(Encoding.UTF32))
.IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableBytes).HasConversion((ValueConverter?)new StringToBytesConverter(Encoding.UTF32))
.IsRequired(); // Issue #24686
b.Property(e => e.StringToBytes).HasConversion((ValueConverter?)new StringToBytesConverter(Encoding.UTF32));
b.Property(e => e.StringToNullableBytes).HasConversion((ValueConverter?)new StringToBytesConverter(Encoding.UTF32));
b.Property(e => e.NullableStringToBytes).HasConversion(new StringToBytesConverter(Encoding.UTF32));
b.Property(e => e.NullableStringToNullableBytes).HasConversion(new StringToBytesConverter(Encoding.UTF32));

b.Property(e => e.StringToChar).HasConversion(new StringToCharConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableChar).HasConversion(new StringToCharConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToChar).HasConversion(new StringToCharConverter());
b.Property(e => e.StringToNullableChar).HasConversion(new StringToCharConverter());
b.Property(e => e.NullableStringToChar).HasConversion((ValueConverter?)new StringToCharConverter());
b.Property(e => e.NullableStringToNullableChar).HasConversion((ValueConverter?)new StringToCharConverter());

b.Property(e => e.StringToDateTime).HasConversion(new StringToDateTimeConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableDateTime).HasConversion(new StringToDateTimeConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToDateTime).HasConversion(new StringToDateTimeConverter());
b.Property(e => e.StringToNullableDateTime).HasConversion(new StringToDateTimeConverter());
b.Property(e => e.NullableStringToDateTime).HasConversion((ValueConverter?)new StringToDateTimeConverter());
b.Property(e => e.NullableStringToNullableDateTime).HasConversion((ValueConverter?)new StringToDateTimeConverter());

b.Property(e => e.StringToDateTimeOffset).HasConversion(new StringToDateTimeOffsetConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableDateTimeOffset).HasConversion(new StringToDateTimeOffsetConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToDateTimeOffset).HasConversion(new StringToDateTimeOffsetConverter());
b.Property(e => e.StringToNullableDateTimeOffset).HasConversion(new StringToDateTimeOffsetConverter());
b.Property(e => e.NullableStringToDateTimeOffset)
.HasConversion((ValueConverter?)new StringToDateTimeOffsetConverter());
b.Property(e => e.NullableStringToNullableDateTimeOffset)
.HasConversion((ValueConverter?)new StringToDateTimeOffsetConverter());

b.Property(e => e.StringToEnum).HasConversion(new StringToEnumConverter<TheExperience>()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableEnum).HasConversion(new StringToEnumConverter<TheExperience>()).IsRequired(); // Issue #24686
b.Property(e => e.StringToEnum).HasConversion(new StringToEnumConverter<TheExperience>());
b.Property(e => e.StringToNullableEnum).HasConversion(new StringToEnumConverter<TheExperience>());
b.Property(e => e.NullableStringToEnum).HasConversion((ValueConverter?)new StringToEnumConverter<TheExperience>());
b.Property(e => e.NullableStringToNullableEnum)
.HasConversion((ValueConverter?)new StringToEnumConverter<TheExperience>());

b.Property(e => e.StringToGuid).HasConversion(new StringToGuidConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableGuid).HasConversion(new StringToGuidConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToGuid).HasConversion(new StringToGuidConverter());
b.Property(e => e.StringToNullableGuid).HasConversion(new StringToGuidConverter());
b.Property(e => e.NullableStringToGuid).HasConversion((ValueConverter?)new StringToGuidConverter());
b.Property(e => e.NullableStringToNullableGuid).HasConversion((ValueConverter?)new StringToGuidConverter());

b.Property(e => e.StringToNumber).HasConversion(new StringToNumberConverter<byte>()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableNumber).HasConversion(new StringToNumberConverter<byte>()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNumber).HasConversion(new StringToNumberConverter<byte>());
b.Property(e => e.StringToNullableNumber).HasConversion(new StringToNumberConverter<byte>());
b.Property(e => e.NullableStringToNumber).HasConversion((ValueConverter?)new StringToNumberConverter<byte>());
b.Property(e => e.NullableStringToNullableNumber)
.HasConversion((ValueConverter?)new StringToNumberConverter<byte>());

b.Property(e => e.StringToTimeSpan).HasConversion(new StringToTimeSpanConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToNullableTimeSpan).HasConversion(new StringToTimeSpanConverter()).IsRequired(); // Issue #24686
b.Property(e => e.StringToTimeSpan).HasConversion(new StringToTimeSpanConverter());
b.Property(e => e.StringToNullableTimeSpan).HasConversion(new StringToTimeSpanConverter());
b.Property(e => e.NullableStringToTimeSpan).HasConversion((ValueConverter?)new StringToTimeSpanConverter());
b.Property(e => e.NullableStringToNullableTimeSpan).HasConversion((ValueConverter?)new StringToTimeSpanConverter());

Expand All @@ -781,13 +775,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.Property(e => e.NullableTimeSpanToString).HasConversion(new TimeSpanToStringConverter());
b.Property(e => e.NullableTimeSpanToNullableString).HasConversion(new TimeSpanToStringConverter());

b.Property(e => e.UriToString).HasConversion((ValueConverter?)new UriToStringConverter()).IsRequired(); // Issue #24686
b.Property(e => e.UriToNullableString).HasConversion((ValueConverter?)new UriToStringConverter()).IsRequired(); // Issue #24686
b.Property(e => e.UriToString).HasConversion((ValueConverter?)new UriToStringConverter());
b.Property(e => e.UriToNullableString).HasConversion((ValueConverter?)new UriToStringConverter());
b.Property(e => e.NullableUriToString).HasConversion(new UriToStringConverter());
b.Property(e => e.NullableUriToNullableString).HasConversion(new UriToStringConverter());

b.Property(e => e.NonNullIntToNullString).HasConversion(new NonNullIntToNullStringConverter()).IsRequired(); // Issue #24686
b.Property(e => e.NonNullIntToNonNullString).HasConversion(new NonNullIntToNonNullStringConverter()).IsRequired(); // Issue #24686
b.Property(e => e.NonNullIntToNullString).HasConversion(new NonNullIntToNullStringConverter());
b.Property(e => e.NonNullIntToNonNullString).HasConversion(new NonNullIntToNonNullStringConverter());
b.Property(e => e.NullIntToNullString).HasConversion(new NullIntToNullStringConverter()).IsRequired(false);
b.Property(e => e.NullIntToNonNullString).HasConversion(new NullIntToNonNullStringConverter()).IsRequired(false);

Expand Down