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

Make sure navigations on property bag entity types don't use CLR properties. #25960

Merged
merged 1 commit into from
Sep 10, 2021
Merged

Make sure navigations on property bag entity types don't use CLR properties. #25960

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
2 changes: 1 addition & 1 deletion src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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