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

Jump to bottom

Updates to shared-type entity type handling in proxies #22408

Merged
merged 3 commits into from
Sep 5, 2020
Merged

Updates to shared-type entity type handling in proxies #22408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
debd89b
Updates to shared-type entity type handling in proxies
ajcvickers Sep 4, 2020
995919d
Updated strings
ajcvickers Sep 4, 2020
168e93b
Make sure non-shared-type entity types aren't discovered while a shar…
AndriySvyryd Sep 5, 2020
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
42 changes: 37 additions & 5 deletions src/EFCore.Proxies/Properties/ProxiesStrings.Designer.cs
View file
Open in desktop

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

20 changes: 16 additions & 4 deletions src/EFCore.Proxies/Properties/ProxiesStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,19 +117,31 @@
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="DictionaryCannotBeProxied" xml:space="preserve">
<value>The type '{dictionaryType}' used for shared entity type '{entityType}' is not suitable for use as a change-tracking proxy because its indexer property is not virtual. Consider using an implementation of '{interfaceType}' that allows overriding of the indexer.</value>
</data>
<data name="EntityTypeNotFoundShared" xml:space="preserve">
<value>The type '{clrType}' is configured as a shared-type entity type, but the entity type name is not known. Ensure that CreateProxy is called on a DbSet created specifically for the shared-type entity type through use of a `DbContext.Set` overload that accepts an entity type name.</value>
</data>
<data name="EntityTypeNotFoundWeak" xml:space="preserve">
<value>Cannot create a proxy for '{typeName}' because it is mapped to multiple owned entity types. Proxy creation is not supported for owned types used more than once in the model.</value>
</data>
<data name="FieldProperty" xml:space="preserve">
<value>Property '{property}' on entity type '{entityType}' is mapped without a CLR property. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
<value>Property '{property}' on entity type '{entityType}' is mapped without a CLR property. 'UseChangeTrackingProxies' requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. 'UseLazyLoadingProxies' requires only the navigation properties be virtual.</value>
</data>
<data name="ItsASeal" xml:space="preserve">
<value>Entity type '{entityType}' is sealed. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
<value>Entity type '{entityType}' is sealed. 'UseChangeTrackingProxies' requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. 'UseLazyLoadingProxies' requires only the navigation properties be virtual.</value>
</data>
<data name="NonVirtualIndexerProperty" xml:space="preserve">
<value>The mapped indexer property on entity type '{entityType}' is not virtual. 'UseChangeTrackingProxies' requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. 'UseLazyLoadingProxies' requires only the navigation properties be virtual.</value>
</data>
<data name="NonVirtualProperty" xml:space="preserve">
<value>Property '{property}' on entity type '{entityType}' is not virtual. UseChangeTrackingProxies requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. UseLazyLoadingProxies requires only the navigation properties be virtual.</value>
<value>Property '{1_entityType}.{0_property}' is not virtual. 'UseChangeTrackingProxies' requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. 'UseLazyLoadingProxies' requires only the navigation properties be virtual.</value>
</data>
<data name="ProxiesNotEnabled" xml:space="preserve">
<value>Unable to create proxy for '{entityType}' because proxies are not enabled. Call 'DbContextOptionsBuilder.UseChangeTrackingProxies' or 'DbContextOptionsBuilder.UseLazyLoadingProxies' to enable proxies.</value>
</data>
<data name="ProxyServicesMissing" xml:space="preserve">
<value>UseChangeTrackingProxies and UseLazyLoadingProxies each require AddEntityFrameworkProxies to be called on the internal service provider used.</value>
<value>'UseChangeTrackingProxies' and 'UseLazyLoadingProxies' each require AddEntityFrameworkProxies to be called on the internal service provider used.</value>
</data>
</root>
77 changes: 58 additions & 19 deletions src/EFCore.Proxies/Proxies/Internal/ProxyBindingRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@ public virtual void ProcessModelFinalizing(
{
foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
{
if (entityType.ClrType?.IsAbstract == false)
var clrType = entityType.ClrType;
if (clrType?.IsAbstract == false)
{
if (entityType.ClrType.IsSealed)
if (clrType.IsSealed)
{
throw new InvalidOperationException(ProxiesStrings.ItsASeal(entityType.DisplayName()));
}
Expand Down Expand Up @@ -102,15 +103,15 @@ public virtual void ProcessModelFinalizing(
}

if (_options.UseChangeTrackingProxies
&& navigationBase.PropertyInfo.SetMethod?.IsVirtual == false)
&& navigationBase.PropertyInfo.SetMethod?.IsReallyVirtual() == false)
{
throw new InvalidOperationException(
ProxiesStrings.NonVirtualProperty(navigationBase.Name, entityType.DisplayName()));
}

if (_options.UseLazyLoadingProxies)
{
if (!navigationBase.PropertyInfo.GetMethod.IsVirtual
if (!navigationBase.PropertyInfo.GetMethod.IsReallyVirtual()
&& (!(navigationBase is INavigation navigation
&& navigation.ForeignKey.IsOwnership)))
{
Expand All @@ -122,6 +123,59 @@ public virtual void ProcessModelFinalizing(
}
}

if (_options.UseChangeTrackingProxies)
{
var indexerChecked = false;
foreach (var property in entityType.GetDeclaredProperties()
.Where(p => !p.IsShadowProperty()))
{
if (property.IsIndexerProperty())
{
if (!indexerChecked)
{
indexerChecked = true;

if (!property.PropertyInfo.SetMethod.IsReallyVirtual())
{
if (clrType.IsGenericType
&& clrType.GetGenericTypeDefinition() == typeof(Dictionary<,>)
&& clrType.GenericTypeArguments[0] == typeof(string))
{
if (entityType.GetProperties().Any(p => !p.IsPrimaryKey()))
{
throw new InvalidOperationException(
ProxiesStrings.DictionaryCannotBeProxied(
clrType.ShortDisplayName(),
entityType.DisplayName(),
typeof(IDictionary<,>).MakeGenericType(clrType.GenericTypeArguments)
.ShortDisplayName()));
}
}
else
{
throw new InvalidOperationException(
ProxiesStrings.NonVirtualIndexerProperty(entityType.DisplayName()));
}
}
}
}
else
{
if (property.PropertyInfo == null)
{
throw new InvalidOperationException(
ProxiesStrings.FieldProperty(property.Name, entityType.DisplayName()));
}

if (property.PropertyInfo.SetMethod?.IsReallyVirtual() == false)
{
throw new InvalidOperationException(
ProxiesStrings.NonVirtualProperty(property.Name, entityType.DisplayName()));
}
}
}
}

void UpdateConstructorBindings(string bindingAnnotationName, InstantiationBinding binding)
{
if (_options.UseLazyLoadingProxies)
Expand Down Expand Up @@ -176,21 +230,6 @@ void UpdateConstructorBindings(string bindingAnnotationName, InstantiationBindin
new ObjectArrayParameterBinding(binding.ParameterBindings)
},
proxyType));

foreach (var prop in entityType.GetDeclaredProperties().Where(p => !p.IsShadowProperty()))
{
if (prop.PropertyInfo == null)
{
throw new InvalidOperationException(
ProxiesStrings.FieldProperty(prop.Name, entityType.DisplayName()));
}

if (prop.PropertyInfo.SetMethod?.IsVirtual == false)
{
throw new InvalidOperationException(
ProxiesStrings.NonVirtualProperty(prop.Name, entityType.DisplayName()));
}
}
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/EFCore.Proxies/Proxies/Internal/ProxyFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ public virtual object Create(
var entityType = context.Model.FindRuntimeEntityType(type);
if (entityType == null)
{
if (context.Model.IsShared(type))
{
throw new InvalidOperationException(ProxiesStrings.EntityTypeNotFoundShared(type.ShortDisplayName()));
}

if (context.Model.HasEntityTypeWithDefiningNavigation(type))
{
throw new InvalidOperationException(ProxiesStrings.EntityTypeNotFoundWeak(type.ShortDisplayName()));
}

throw new InvalidOperationException(CoreStrings.EntityTypeNotFound(type.ShortDisplayName()));
}

Expand Down
1 change: 1 addition & 0 deletions src/EFCore/Metadata/Internal/InternalModelBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ private InternalEntityTypeBuilder Entity(
return null;
}

using var batch = Metadata.ConventionDispatcher.DelayConventions();
var clrType = type.Type;
EntityType entityType;
if (type.IsNamed)
Expand Down
3 changes: 3 additions & 0 deletions src/Shared/MemberInfoExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,8 @@ public static string GetSimpleMemberName(this MemberInfo member)
var index = name.LastIndexOf('.');
return index >= 0 ? name.Substring(index + 1) : name;
}

public static bool IsReallyVirtual(this MethodInfo method)
=> method.IsVirtual && !method.IsFinal;
}
}
Loading