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.

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

Don't identify dictionary indexer as NRT #22068

Merged
merged 2 commits into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,15 @@ public override ConventionSet CreateConventionSet()

conventionSet.KeyAddedConventions.Add(sqlServerInMemoryTablesConvention);

var sqlServerOnDeleteConvention = new SqlServerOnDeleteConvention(Dependencies, RelationalDependencies);
ReplaceConvention(conventionSet.ForeignKeyAddedConventions, valueGenerationConvention);
ReplaceConvention(conventionSet.ForeignKeyAddedConventions, (CascadeDeleteConvention)sqlServerOnDeleteConvention);

ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, valueGenerationConvention);

conventionSet.SkipNavigationForeignKeyChangedConventions.Add(new SqlServerOnDeleteConvention(Dependencies, RelationalDependencies));
ReplaceConvention(conventionSet.ForeignKeyRequirednessChangedConventions, (CascadeDeleteConvention)sqlServerOnDeleteConvention);

conventionSet.SkipNavigationForeignKeyChangedConventions.Add(sqlServerOnDeleteConvention);

conventionSet.IndexAddedConventions.Add(sqlServerInMemoryTablesConvention);
conventionSet.IndexAddedConventions.Add(sqlServerIndexConvention);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// A convention that configures the OnDelete behavior for foreign keys on the join entity type for
/// self-referencing skip navigations
/// </summary>
public class SqlServerOnDeleteConvention : ISkipNavigationForeignKeyChangedConvention
public class SqlServerOnDeleteConvention : CascadeDeleteConvention, ISkipNavigationForeignKeyChangedConvention
{
/// <summary>
/// Creates a new instance of <see cref="SqlServerOnDeleteConvention" />.
Expand All @@ -23,6 +23,7 @@ public class SqlServerOnDeleteConvention : ISkipNavigationForeignKeyChangedConve
public SqlServerOnDeleteConvention(
[NotNull] ProviderConventionSetBuilderDependencies dependencies,
[NotNull] RelationalConventionSetBuilderDependencies relationalDependencies)
: base(dependencies)
{
}

Expand All @@ -33,21 +34,34 @@ public virtual void ProcessSkipNavigationForeignKeyChanged(
IConventionForeignKey oldForeignKey,
IConventionContext<IConventionForeignKey> context)
{
var selfReferencingSkipNavigation = skipNavigationBuilder.Metadata;
if (foreignKey == null
|| foreignKey.DeleteBehavior != DeleteBehavior.Cascade
|| selfReferencingSkipNavigation.Inverse == null
|| selfReferencingSkipNavigation.TargetEntityType != selfReferencingSkipNavigation.DeclaringEntityType)
foreignKey?.Builder?.OnDelete(GetTargetDeleteBehavior(foreignKey));
}

/// <inheritdoc />
protected override DeleteBehavior GetTargetDeleteBehavior(IConventionForeignKey foreignKey)
{
var deleteBehavior = base.GetTargetDeleteBehavior(foreignKey);
if (deleteBehavior != DeleteBehavior.Cascade)
{
return;
return deleteBehavior;
}

var selfReferencingSkipNavigation = foreignKey.GetReferencingSkipNavigations()
.FirstOrDefault(s => s.Inverse != null && s.TargetEntityType == s.DeclaringEntityType);
if (selfReferencingSkipNavigation == null)
{
return deleteBehavior;
}

if (selfReferencingSkipNavigation == selfReferencingSkipNavigation.DeclaringEntityType.GetDeclaredSkipNavigations()
.First(s => s == selfReferencingSkipNavigation || s == selfReferencingSkipNavigation.Inverse))
{
foreignKey.Builder.OnDelete(DeleteBehavior.ClientCascade);
selfReferencingSkipNavigation.Inverse.ForeignKey?.Builder.OnDelete(null);
selfReferencingSkipNavigation.Inverse.ForeignKey?.Builder.OnDelete(
GetTargetDeleteBehavior(selfReferencingSkipNavigation.Inverse.ForeignKey));
return DeleteBehavior.ClientCascade;
}

return deleteBehavior;
}
}
}
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 @@ -136,9 +136,9 @@ where ov.Customer.Orders.Any()
}

[ConditionalTheory(Skip = "Issue #17246")]
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(@"");
}
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