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 identify them
Replaced some usages of GetIdentifyingMemberInfo() with more appropriate code.

Fixes #25501
Fixes #25459
  • Loading branch information
@AndriySvyryd
AndriySvyryd authored Sep 10, 2021
1 parent e1de574 commit 65bc528
Show file tree
Hide file tree
Showing 22 changed files with 190 additions and 189 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
4 changes: 4 additions & 0 deletions src/EFCore/Metadata/IReadOnlyNavigationBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,9 @@ public interface IReadOnlyNavigationBase : IReadOnlyPropertyBase
/// </summary>
bool IsEagerLoaded
=> (bool?)this[CoreAnnotationNames.EagerLoaded] ?? false;

/// <inheritdoc />
// TODO: Remove when #3864 is implemented
bool IReadOnlyPropertyBase.IsShadowProperty() => this.GetIdentifyingMemberInfo() == null;
}
}
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
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private static readonly MethodInfo _createObservableHashSet
}

var memberInfo = GetMostDerivedMemberInfo();
var propertyType = memberInfo.GetMemberType();
var propertyType = navigation.IsIndexerProperty() ? navigation.ClrType : memberInfo.GetMemberType();
var elementType = propertyType.TryGetElementType(typeof(IEnumerable<>));

if (elementType == null)
Expand Down Expand Up @@ -110,7 +110,7 @@ MemberInfo GetMostDerivedMemberInfo()
: propertyInfo == null
? fieldInfo
: fieldInfo.FieldType.IsAssignableFrom(propertyInfo.PropertyType)
? (MemberInfo)propertyInfo
? propertyInfo
: fieldInfo;
}
}
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
Loading

0 comments on commit 65bc528

Please sign in to comment.