Skip to content

Commit

Permalink
Make sure navigations on property bag entity types don't use CLR prop…
Browse files Browse the repository at this point in the history 
…erties.

Make GetIdentifyingMemberInfo() return null for indexer properties since it doesn't uniquely identifies them
Replaced some usages of GetIdentifyingMemberInfo() with more appropriate code.

Fixes #25501
Fixes #25459
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Sep 10, 2021
1 parent 644aee4 commit e3ba491
Show file tree
Hide file tree
Showing 19 changed files with 179 additions and 120 deletions.
2 changes: 1 addition & 1 deletion src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ protected virtual Expression CreateReadShadowValueExpression(
IPropertyBase property)
=> Expression.Call(
parameter,
InternalEntityEntry.ReadShadowValueMethod.MakeGenericMethod(property.ClrType),
InternalEntityEntry.ReadShadowValueMethod.MakeGenericMethod((property as IProperty)?.ClrType ?? typeof(object)),
Expression.Constant(property.GetShadowIndex()));

/// <summary>
Expand Down
19 changes: 1 addition & 18 deletions src/EFCore/Metadata/IReadOnlyNavigation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,24 +144,7 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt
builder.Append(" (");
}

if (this.GetIdentifyingMemberInfo() == null)
{
// Shadow navigation
if (IsCollection)
{
builder.Append("IEnumerable<");
}
builder.Append(TargetEntityType.ClrType.ShortDisplayName());
if (IsCollection)
{
builder.Append(">");
}
builder.Append(")");
}
else
{
builder.Append(ClrType.ShortDisplayName()).Append(")");
}
builder.Append(ClrType.ShortDisplayName()).Append(")");

if (IsCollection)
{
Expand Down
5 changes: 2 additions & 3 deletions src/EFCore/Metadata/IReadOnlyPropertyBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Reflection;
using Microsoft.EntityFrameworkCore.ChangeTracking;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Internal;

namespace Microsoft.EntityFrameworkCore.Metadata
{
Expand Down Expand Up @@ -60,7 +59,7 @@ public interface IReadOnlyPropertyBase : IReadOnlyAnnotatable
/// <returns>
/// <see langword="true" /> if the property is a shadow property, otherwise <see langword="false" />.
/// </returns>
bool IsShadowProperty() => this.GetIdentifyingMemberInfo() == null;
bool IsShadowProperty() => PropertyInfo == null && FieldInfo == null;

/// <summary>
/// Gets a value indicating whether this is an indexer property. An indexer property is one that is accessed through
Expand All @@ -70,7 +69,7 @@ public interface IReadOnlyPropertyBase : IReadOnlyAnnotatable
/// <see langword="true" /> if the property is an indexer property, otherwise <see langword="false" />.
/// </returns>
bool IsIndexerProperty()
=> this.GetIdentifyingMemberInfo() is PropertyInfo propertyInfo
=> PropertyInfo is PropertyInfo propertyInfo
&& propertyInfo == DeclaringType.FindIndexerPropertyInfo();

/// <summary>
Expand Down
19 changes: 1 addition & 18 deletions src/EFCore/Metadata/IReadOnlySkipNavigation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,24 +89,7 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt
builder.Append(" (");
}

if (this.GetIdentifyingMemberInfo() == null)
{
// Shadow navigation
if (IsCollection)
{
builder.Append("IEnumerable<");
}
builder.Append(TargetEntityType.ClrType.ShortDisplayName());
if (IsCollection)
{
builder.Append(">");
}
builder.Append(")");
}
else
{
builder.Append(ClrType.ShortDisplayName()).Append(")");
}
builder.Append(ClrType.ShortDisplayName()).Append(")");

if (IsCollection)
{
Expand Down
61 changes: 43 additions & 18 deletions src/EFCore/Metadata/Internal/EntityType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.EntityFrameworkCore.Utilities;
Expand Down Expand Up @@ -1460,7 +1461,13 @@ public virtual Navigation AddNavigation(
return AddNavigation(new MemberIdentity(navigationMember), foreignKey, pointsToPrincipal);
}

private Navigation AddNavigation(MemberIdentity navigationMember, ForeignKey foreignKey, bool pointsToPrincipal)
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual Navigation AddNavigation(MemberIdentity navigationMember, ForeignKey foreignKey, bool pointsToPrincipal)
{
EnsureMutable();

Expand Down Expand Up @@ -1504,7 +1511,7 @@ private Navigation AddNavigation(MemberIdentity navigationMember, ForeignKey for
{
ValidateClrMember(name, memberInfo);
}
else
else if (!IsPropertyBag)
{
memberInfo = ClrType.GetMembersInHierarchy(name).FirstOrDefault();
}
Expand All @@ -1519,6 +1526,10 @@ private Navigation AddNavigation(MemberIdentity navigationMember, ForeignKey for
!pointsToPrincipal && !foreignKey.IsUnique,
shouldThrow: true);
}
else if (IsPropertyBag)
{
memberInfo = FindIndexerPropertyInfo()!;
}

var navigation = new Navigation(name, memberInfo as PropertyInfo, memberInfo as FieldInfo, foreignKey);

Expand Down Expand Up @@ -1665,7 +1676,7 @@ public virtual IEnumerable<Navigation> GetNavigations()
{
ValidateClrMember(name, memberInfo);
}
else
else if (!IsPropertyBag)
{
memberInfo = ClrType.GetMembersInHierarchy(name).FirstOrDefault();
}
Expand All @@ -1680,6 +1691,10 @@ public virtual IEnumerable<Navigation> GetNavigations()
collection,
shouldThrow: true);
}
else if (IsPropertyBag)
{
memberInfo = FindIndexerPropertyInfo()!;
}

var skipNavigation = new SkipNavigation(
name,
Expand Down Expand Up @@ -1725,7 +1740,9 @@ public virtual IEnumerable<Navigation> GetNavigations()
return memberInfo.GetMemberType();
}

var clashingMemberInfo = ClrType.GetMembersInHierarchy(name).FirstOrDefault();
var clashingMemberInfo = IsPropertyBag
? null
: ClrType.GetMembersInHierarchy(name).FirstOrDefault();
if (clashingMemberInfo != null)
{
throw new InvalidOperationException(
Expand Down Expand Up @@ -2301,7 +2318,7 @@ public virtual IEnumerable<Index> GetIndexes()
return AddProperty(
name,
propertyType,
ClrType.GetMembersInHierarchy(name).FirstOrDefault(),
null,
typeConfigurationSource,
configurationSource);
}
Expand Down Expand Up @@ -2332,10 +2349,18 @@ public virtual IEnumerable<Index> GetIndexes()
string name,
ConfigurationSource configurationSource)
{
var clrMember = ClrType.GetMembersInHierarchy(name).FirstOrDefault();
if (clrMember == null)
MemberInfo? clrMember;
if (IsPropertyBag)
{
clrMember = FindIndexerPropertyInfo()!;
}
else
{
throw new InvalidOperationException(CoreStrings.NoPropertyType(name, DisplayName()));
clrMember = ClrType.GetMembersInHierarchy(name).FirstOrDefault();
if (clrMember == null)
{
throw new InvalidOperationException(CoreStrings.NoPropertyType(name, DisplayName()));
}
}

return AddProperty(clrMember, configurationSource);
Expand Down Expand Up @@ -2386,9 +2411,7 @@ public virtual IEnumerable<Index> GetIndexes()
}
else
{
Check.DebugAssert(
ClrType.GetMembersInHierarchy(name).FirstOrDefault() == null,
"MemberInfo not supplied for non-shadow property");
memberInfo = ClrType.GetMembersInHierarchy(name).FirstOrDefault();
}

if (memberInfo != null
Expand Down Expand Up @@ -3013,7 +3036,7 @@ public virtual IEnumerable<ServiceProperty> GetDerivedServiceProperties()
var data = new List<Dictionary<string, object?>>();
var valueConverters = new Dictionary<string, ValueConverter?>(StringComparer.Ordinal);
var properties = GetProperties()
.Concat<IReadOnlyPropertyBase>(GetNavigations())
.Concat<IPropertyBase>(GetNavigations())
.Concat(GetSkipNavigations())
.ToDictionary(p => p.Name);
foreach (var rawSeed in _data)
Expand All @@ -3029,15 +3052,17 @@ public virtual IEnumerable<ServiceProperty> GetDerivedServiceProperties()
{
ValueConverter? valueConverter = null;
if (providerValues
&& propertyBase is IReadOnlyProperty property
&& propertyBase is IProperty property
&& !valueConverters.TryGetValue(propertyBase.Name, out valueConverter))
{
valueConverter = property.GetTypeMapping().Converter;
valueConverters[propertyBase.Name] = valueConverter;
}

propertyBase.TryGetMemberInfo(forConstruction: false, forSet: false, out var memberInfo, out _);

object? value = null;
switch (propertyBase.GetIdentifyingMemberInfo())
switch (memberInfo)
{
case PropertyInfo propertyInfo:
if (propertyBase.IsIndexerProperty())
Expand Down Expand Up @@ -4828,7 +4853,7 @@ IMutableProperty IMutableEntityType.AddProperty(string name, Type propertyType)
propertyType,
setTypeConfigurationSource
? fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention
: (ConfigurationSource?)null,
: null,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
Expand All @@ -4840,7 +4865,7 @@ IMutableProperty IMutableEntityType.AddProperty(string name, Type propertyType)
[DebuggerStepThrough]
IMutableProperty IMutableEntityType.AddProperty(string name, Type propertyType, MemberInfo? memberInfo)
=> AddProperty(
name, propertyType, memberInfo ?? ClrType.GetMembersInHierarchy(name).FirstOrDefault(),
name, propertyType, memberInfo,
ConfigurationSource.Explicit, ConfigurationSource.Explicit)!;

/// <summary>
Expand All @@ -4859,10 +4884,10 @@ IMutableProperty IMutableEntityType.AddProperty(string name, Type propertyType,
=> AddProperty(
name,
propertyType,
memberInfo ?? ClrType.GetMembersInHierarchy(name).FirstOrDefault(),
memberInfo,
setTypeConfigurationSource
? fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention
: (ConfigurationSource?)null,
: null,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
Expand Down
18 changes: 13 additions & 5 deletions src/EFCore/Metadata/Internal/EntityTypeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,20 @@ public static MemberInfo GetNavigationMemberInfo(
this IReadOnlyEntityType entityType,
string navigationName)
{
var memberInfo = entityType.ClrType.GetMembersInHierarchy(navigationName).FirstOrDefault();

if (memberInfo == null)
MemberInfo? memberInfo;
if (entityType.IsPropertyBag)
{
throw new InvalidOperationException(
CoreStrings.NoClrNavigation(navigationName, entityType.DisplayName()));
memberInfo = entityType.FindIndexerPropertyInfo()!;
}
else
{
memberInfo = entityType.ClrType.GetMembersInHierarchy(navigationName).FirstOrDefault();

if (memberInfo == null)
{
throw new InvalidOperationException(
CoreStrings.NoClrNavigation(navigationName, entityType.DisplayName()));
}
}

return memberInfo;
Expand Down
44 changes: 28 additions & 16 deletions src/EFCore/Metadata/Internal/ForeignKey.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,17 @@ public virtual void UpdatePrincipalEndConfigurationSource(ConfigurationSource co
ConfigurationSource configurationSource)
=> Navigation(MemberIdentity.Create(property), configurationSource, pointsToPrincipal: true);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual Navigation? SetDependentToPrincipal(
MemberIdentity? property,
ConfigurationSource configurationSource)
=> Navigation(property, configurationSource, pointsToPrincipal: true);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down Expand Up @@ -402,6 +413,15 @@ public virtual void UpdateDependentToPrincipalConfigurationSource(ConfigurationS
public virtual Navigation? SetPrincipalToDependent(MemberInfo? property, ConfigurationSource configurationSource)
=> Navigation(MemberIdentity.Create(property), configurationSource, pointsToPrincipal: false);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual Navigation? SetPrincipalToDependent(MemberIdentity? property, ConfigurationSource configurationSource)
=> Navigation(property, configurationSource, pointsToPrincipal: false);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down Expand Up @@ -497,18 +517,11 @@ public virtual void UpdatePrincipalToDependentConfigurationSource(ConfigurationS
}

Navigation? navigation = null;
var property = propertyIdentity?.MemberInfo;
if (property != null)
{
navigation = pointsToPrincipal
? DeclaringEntityType.AddNavigation(property, this, pointsToPrincipal: true)
: PrincipalEntityType.AddNavigation(property, this, pointsToPrincipal: false);
}
else if (name != null)
if (propertyIdentity?.Name != null)
{
navigation = pointsToPrincipal
? DeclaringEntityType.AddNavigation(name, this, pointsToPrincipal: true)
: PrincipalEntityType.AddNavigation(name, this, pointsToPrincipal: false);
? DeclaringEntityType.AddNavigation(propertyIdentity.Value, this, pointsToPrincipal: true)
: PrincipalEntityType.AddNavigation(propertyIdentity.Value, this, pointsToPrincipal: false);
}

if (pointsToPrincipal)
Expand Down Expand Up @@ -595,14 +608,13 @@ public virtual bool IsUnique
CoreStrings.NonUniqueRequiredDependentForeignKey(Properties.Format(), DeclaringEntityType.DisplayName()));
}

var navigationMember = PrincipalToDependent?.GetIdentifyingMemberInfo();
if (unique.HasValue
&& PrincipalEntityType.ClrType != Model.DefaultPropertyBagType
&& DeclaringEntityType.ClrType != Model.DefaultPropertyBagType
&& PrincipalToDependent != null)
&& navigationMember != null)
{
if (!Internal.Navigation.IsCompatible(
PrincipalToDependent.Name,
PrincipalToDependent.GetIdentifyingMemberInfo()!,
PrincipalToDependent!.Name,
navigationMember,
PrincipalEntityType,
DeclaringEntityType,
!unique,
Expand All @@ -618,7 +630,7 @@ public virtual bool IsUnique

_isUniqueConfigurationSource = unique == null
? null
: (ConfigurationSource?)configurationSource.Max(_isUniqueConfigurationSource);
: configurationSource.Max(_isUniqueConfigurationSource);

return IsUnique != oldUnique
? DeclaringEntityType.Model.ConventionDispatcher.OnForeignKeyUniquenessChanged(Builder)
Expand Down
Loading

0 comments on commit e3ba491

Please sign in to comment.