Skip to content

Commit

Permalink
Don't identify dictionary indexer as NRT
Browse files Browse the repository at this point in the history
Fixes #20718
  • Loading branch information
roji committed Aug 14, 2020
1 parent 654e171 commit 8979c3d
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 8 deletions.
18 changes: 15 additions & 3 deletions src/EFCore/Metadata/Conventions/NonNullableConventionBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ protected virtual bool IsNonNullableReferenceType(
return false;
}

// For C# 8.0 nullable types, the C# currently synthesizes a NullableAttribute that expresses nullability into assemblies
// it produces. If the model is spread across more than one assembly, there will be multiple versions of this attribute,
// so look for it by name, caching to avoid reflection on every check.
// For C# 8.0 nullable types, the C# compiler currently synthesizes a NullableAttribute that expresses nullability into
// assemblies it produces. If the model is spread across more than one assembly, there will be multiple versions of this
// attribute, so look for it by name, caching to avoid reflection on every check.
// Note that this may change - if https://github.com/dotnet/corefx/issues/36222 is done we can remove all of this.

// First look for NullableAttribute on the member itself
Expand Down Expand Up @@ -114,6 +114,18 @@ protected virtual bool IsNonNullableReferenceType(

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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Properties/CoreStrings.Designer.cs

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

2 changes: 1 addition & 1 deletion src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,7 @@
<value>Cannot set backing field '{field}' for the indexer property '{entityType}.{property}'. Indexer properties are not allowed to use a backing field.</value>
</data>
<data name="ClashingSharedType" xml:space="preserve">
<value>The entity type '{entityType}' cannot be added to the model because a shared entity type with the same clr type already exists.</value>
<value>The entity type '{entityType}' cannot be added to the model because a shared entity type with the same CLR type already exists.</value>
</data>
<data name="SkipNavigationInUseBySkipNavigation" xml:space="preserve">
<value>The skip navigation '{skipNavigation}' cannot be removed because it is set as the inverse of the skip navigation '{inverseSkipNavigation}' on '{referencingEntityType}'. All referencing skip navigations must be removed before this skip navigation can be removed.</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ where ov.Customer.Orders.Any()

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task KeylesEntity_groupby(bool async)
public virtual Task KeylessEntity_groupby(bool async)
{
return AssertQuery(
async,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ public override void Auto_initialized_view_set()
@"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] FROM [Customers] AS [c]");
}

public override async Task KeylesEntity_groupby(bool async)
public override async Task KeylessEntity_groupby(bool async)
{
await base.KeylesEntity_groupby(async);
await base.KeylessEntity_groupby(async);

AssertSql(
@"SELECT [c].[City] AS [Key], COUNT(*) AS [Count], COALESCE(SUM(CAST(LEN([c].[Address]) AS int)), 0) AS [Sum]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Diagnostics.CodeAnalysis;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
Expand Down Expand Up @@ -82,6 +83,15 @@ public void Reference_nullability_sets_is_nullable_correctly(Type type, string p
Assert.Equal(expectedNullable, entityTypeBuilder.Property(propertyName).Metadata.IsNullable);
}

[ConditionalFact]
public void Non_nullability_ignores_context_on_generic_types()
{
var modelBuilder = CreateModelBuilder();
var entityTypeBuilder = modelBuilder.SharedTypeEntity<Dictionary<string, object>>("SomeBag");
entityTypeBuilder.IndexerProperty(typeof(string), "b");
Assert.True(entityTypeBuilder.Property("b").Metadata.IsNullable);
}

private void RunConvention(InternalPropertyBuilder propertyBuilder)
{
var context = new ConventionContext<IConventionPropertyBuilder>(
Expand Down

0 comments on commit 8979c3d

Please sign in to comment.