Skip to content

Commit

Permalink
Don't discover base or derived types for owned types
Browse files Browse the repository at this point in the history
Validate owned type inheritance
Allow to disconnect mapped base type

Fixes #10200
Fixes #10715
  • Loading branch information
AndriySvyryd committed Feb 2, 2018
1 parent 76d3da5 commit 2507a80
Show file tree
Hide file tree
Showing 18 changed files with 268 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public class DiscriminatorConvention : IBaseTypeChangedConvention
public class DiscriminatorConvention : IBaseTypeChangedConvention, IEntityTypeRemovedConvention
{
/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand All @@ -22,10 +22,9 @@ public virtual bool Apply(InternalEntityTypeBuilder entityTypeBuilder, EntityTyp
{
if (oldBaseType != null
&& oldBaseType.BaseType == null
&& !oldBaseType.GetDerivedTypes().Any())
&& oldBaseType.GetDirectlyDerivedTypes().Count == 0)
{
var oldBaseTypeBuilder = oldBaseType.Builder;
oldBaseTypeBuilder?.Relational(ConfigurationSource.Convention).HasDiscriminator(propertyInfo: null);
oldBaseType.Builder?.Relational(ConfigurationSource.Convention).HasDiscriminator(propertyInfo: null);
}

var entityType = entityTypeBuilder.Metadata;
Expand Down Expand Up @@ -68,6 +67,23 @@ public virtual bool Apply(InternalEntityTypeBuilder entityTypeBuilder, EntityTyp
return true;
}

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual bool Apply(InternalModelBuilder modelBuilder, EntityType type)
{
var oldBaseType = type.BaseType;
if (oldBaseType != null
&& oldBaseType.BaseType == null
&& oldBaseType.GetDirectlyDerivedTypes().Count == 0)
{
oldBaseType.Builder?.Relational(ConfigurationSource.Convention).HasDiscriminator(propertyInfo: null);
}

return true;
}

private void SetDefaultDiscriminatorValues(IReadOnlyList<EntityType> entityTypes, DiscriminatorBuilder discriminator)
{
foreach (var entityType in entityTypes)
Expand Down
10 changes: 6 additions & 4 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ protected virtual void ValidateClrInheritance(
}

if (!entityType.HasDefiningNavigation()
&& entityType.FindDeclaredOwnership() == null)
&& entityType.FindDeclaredOwnership() == null
&& entityType.BaseType != null)
{
var baseClrType = entityType.ClrType?.GetTypeInfo().BaseType;
while (baseClrType != null)
Expand All @@ -216,7 +217,6 @@ protected virtual void ValidateClrInheritance(
throw new InvalidOperationException(
CoreStrings.InconsistentInheritance(entityType.DisplayName(), baseEntityType.DisplayName()));
}
ValidateClrInheritance(model, baseEntityType, validEntityTypes);
break;
}
baseClrType = baseClrType.GetTypeInfo().BaseType;
Expand Down Expand Up @@ -269,7 +269,9 @@ protected virtual void ValidateOwnership([NotNull] IModel model)

if (ownerships.Count == 1)
{
if (entityType.BaseType != null)
var ownership = ownerships[0];
if (entityType.BaseType != null
&& ownership.DeclaringEntityType == entityType)
{
throw new InvalidOperationException(CoreStrings.OwnedDerivedType(entityType.DisplayName()));
}
Expand All @@ -296,7 +298,7 @@ protected virtual void ValidateOwnership([NotNull] IModel model)
fk.PrincipalEntityType.DisplayName(),
fk.PrincipalToDependent.Name,
entityType.DisplayName(),
ownerships[0].PrincipalEntityType.DisplayName()));
ownership.PrincipalEntityType.DisplayName()));
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/EFCore/Metadata/Conventions/ConventionSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ public class ConventionSet
/// </summary>
public virtual IList<IEntityTypeIgnoredConvention> EntityTypeIgnoredConventions { get; } = new List<IEntityTypeIgnoredConvention>();

/// <summary>
/// Conventions to run when an entity type is removed.
/// </summary>
public virtual IList<IEntityTypeRemovedConvention> EntityTypeRemovedConventions { get; } = new List<IEntityTypeRemovedConvention>();

/// <summary>
/// Conventions to run when a property is ignored.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ public virtual InternalEntityTypeBuilder Apply(InternalEntityTypeBuilder entityT
}

var baseEntityType = FindClosestBaseType(entityType);
if (baseEntityType == null
|| baseEntityType.FindOwnership() != null)
{
return entityTypeBuilder;
}

return entityTypeBuilder.HasBaseType(baseEntityType, ConfigurationSource.Convention);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ public virtual InternalEntityTypeBuilder OnEntityTypeAdded([NotNull] InternalEnt
public virtual bool OnEntityTypeIgnored([NotNull] InternalModelBuilder modelBuilder, [NotNull] string name, [CanBeNull] Type type)
=> _scope.OnEntityTypeIgnored(Check.NotNull(modelBuilder, nameof(modelBuilder)), Check.NotNull(name, nameof(name)), type);

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual bool OnEntityTypeRemoved([NotNull] InternalModelBuilder modelBuilder, [NotNull] EntityType type)
=> _scope.OnEntityTypeRemoved(Check.NotNull(modelBuilder, nameof(modelBuilder)), Check.NotNull(type, nameof(type)));

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
Expand Down
33 changes: 33 additions & 0 deletions src/EFCore/Metadata/Conventions/Internal/ConventionNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ public virtual bool OnEntityTypeIgnored([NotNull] InternalModelBuilder modelBuil
return true;
}

public virtual bool OnEntityTypeRemoved([NotNull] InternalModelBuilder modelBuilder, [NotNull] EntityType type)
{
Add(new OnEntityTypeRemovedNode(modelBuilder, type));
return true;
}

public virtual InternalEntityTypeBuilder OnEntityTypeMemberIgnored(
[NotNull] InternalEntityTypeBuilder entityTypeBuilder,
[NotNull] string ignoredMemberName)
Expand Down Expand Up @@ -295,6 +301,19 @@ public override bool OnEntityTypeIgnored(InternalModelBuilder modelBuilder, stri
return true;
}

public override bool OnEntityTypeRemoved(InternalModelBuilder modelBuilder, EntityType type)
{
foreach (var entityTypeRemovedConvention in _conventionSet.EntityTypeRemovedConventions)
{
if (!entityTypeRemovedConvention.Apply(modelBuilder, type))
{
return false;
}
}

return true;
}

public override InternalEntityTypeBuilder OnEntityTypeMemberIgnored(
InternalEntityTypeBuilder entityTypeBuilder, string ignoredMemberName)
{
Expand Down Expand Up @@ -772,6 +791,20 @@ public OnEntityTypeIgnoredNode(InternalModelBuilder modelBuilder, string name, T
public override ConventionNode Accept(ConventionVisitor visitor) => visitor.VisitOnEntityTypeIgnored(this);
}

private class OnEntityTypeRemovedNode : ConventionNode
{
public OnEntityTypeRemovedNode(InternalModelBuilder modelBuilder, EntityType type)
{
ModelBuilder = modelBuilder;
Type = type;
}

public InternalModelBuilder ModelBuilder { get; }
public EntityType Type { get; }

public override ConventionNode Accept(ConventionVisitor visitor) => visitor.VisitOnEntityTypeRemoved(this);
}

private class OnEntityTypeMemberIgnoredNode : ConventionNode
{
public OnEntityTypeMemberIgnoredNode(InternalEntityTypeBuilder entityTypeBuilder, string ignoredMemberName)
Expand Down
7 changes: 7 additions & 0 deletions src/EFCore/Metadata/Conventions/Internal/ConventionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public virtual ConventionScope VisitConventionScope(ConventionScope node)

public virtual OnEntityTypeAddedNode VisitOnEntityTypeAdded(OnEntityTypeAddedNode node) => node;
public virtual OnEntityTypeIgnoredNode VisitOnEntityTypeIgnored(OnEntityTypeIgnoredNode node) => node;
public virtual OnEntityTypeRemovedNode VisitOnEntityTypeRemoved(OnEntityTypeRemovedNode node) => node;
public virtual OnEntityTypeMemberIgnoredNode VisitOnEntityTypeMemberIgnored(OnEntityTypeMemberIgnoredNode node) => node;
public virtual OnBaseEntityTypeChangedNode VisitOnBaseEntityTypeChanged(OnBaseEntityTypeChangedNode node) => node;
public virtual OnEntityTypeAnnotationChangedNode VisitOnEntityTypeAnnotationChanged(OnEntityTypeAnnotationChangedNode node) => node;
Expand Down Expand Up @@ -80,6 +81,12 @@ public override OnEntityTypeIgnoredNode VisitOnEntityTypeIgnored(OnEntityTypeIgn
return null;
}

public override OnEntityTypeRemovedNode VisitOnEntityTypeRemoved(OnEntityTypeRemovedNode node)
{
Dispatcher._immediateConventionScope.OnEntityTypeRemoved(node.ModelBuilder, node.Type);
return null;
}

public override OnEntityTypeMemberIgnoredNode VisitOnEntityTypeMemberIgnored(OnEntityTypeMemberIgnoredNode node)
{
Dispatcher._immediateConventionScope.OnEntityTypeMemberIgnored(node.EntityTypeBuilder, node.IgnoredMemberName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ public virtual InternalEntityTypeBuilder Apply(InternalEntityTypeBuilder entityT
var entityType = entityTypeBuilder.Metadata;
var clrType = entityType.ClrType;
if (clrType == null
|| entityType.HasDefiningNavigation())
|| entityType.HasDefiningNavigation()
|| entityType.FindOwnership() != null)
{
return entityTypeBuilder;
}
Expand All @@ -31,7 +32,7 @@ public virtual InternalEntityTypeBuilder Apply(InternalEntityTypeBuilder entityT
t => t != entityType
&& t.HasClrType()
&& !t.HasDefiningNavigation()
&& t.FindDeclaredOwnership() == null
&& t.FindOwnership() == null
&& ((t.BaseType == null && clrType.GetTypeInfo().IsAssignableFrom(t.ClrType.GetTypeInfo()))
|| (t.BaseType == entityType.BaseType && FindClosestBaseType(t) == entityType)))
.ToList();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata.Internal;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
{
/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public interface IEntityTypeRemovedConvention
{
/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
bool Apply([NotNull] InternalModelBuilder modelBuilder, [NotNull] EntityType type);
}
}
8 changes: 3 additions & 5 deletions src/EFCore/Metadata/Internal/EntityTypeExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -484,8 +484,7 @@ public static IEnumerable<IForeignKey> GetDeclaredReferencingForeignKeys([NotNul
public static IEnumerable<INavigation> FindDerivedNavigations(
[NotNull] this IEntityType entityType, [NotNull] string navigationName)
=> entityType.GetDerivedTypes().SelectMany(
et =>
et.GetDeclaredNavigations().Where(navigation => navigationName == navigation.Name));
et => et.GetDeclaredNavigations().Where(navigation => navigationName == navigation.Name));

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand All @@ -508,8 +507,7 @@ public static IEnumerable<IServiceProperty> GetDeclaredServiceProperties([NotNul
public static IEnumerable<IProperty> FindDerivedProperties(
[NotNull] this IEntityType entityType, [NotNull] string propertyName)
=> entityType.GetDerivedTypes().SelectMany(
et =>
et.GetDeclaredProperties().Where(property => propertyName.Equals(property.Name)));
et => et.GetDeclaredProperties().Where(property => propertyName.Equals(property.Name)));

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand Down
6 changes: 5 additions & 1 deletion src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -923,13 +923,17 @@ public virtual InternalRelationshipBuilder IsOwnership(bool ownership, Configura
newRelationshipBuilder.Metadata.DeclaringEntityType.Builder.HasBaseType((Type)null, configurationSource);
}

// TODO: Move to a convention
newRelationshipBuilder.Metadata.DeclaringEntityType.Builder.PrimaryKey(
newRelationshipBuilder.Metadata.Properties.Select(p => p.Name).ToList(), ConfigurationSource.Convention);
}

newRelationshipBuilder.Metadata.SetIsOwnership(ownership, configurationSource);

foreach (var directlyDerivedType in newRelationshipBuilder.Metadata.DeclaringEntityType.GetDirectlyDerivedTypes().ToList())
{
directlyDerivedType.Builder.HasBaseType((string)null, ConfigurationSource.Convention);
}

return batch.Run(newRelationshipBuilder);
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/EFCore/Metadata/Internal/Model.cs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ public virtual EntityType RemoveEntityType([CanBeNull] EntityType entityType)
}

entityType.Builder = null;

ConventionDispatcher.OnEntityTypeRemoved(Builder, entityType);

return entityType;
}

Expand Down
8 changes: 5 additions & 3 deletions test/EFCore.Tests/Infrastructure/CoreModelValidatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,15 +260,17 @@ public virtual void Pases_on_correct_inheritance()
}

[Fact]
public virtual void Detects_base_type_not_set()
public virtual void Detects_skipped_base_type()
{
var model = new Model();
var entityA = model.AddEntityType(typeof(A));
SetPrimaryKey(entityA);
var entityD = model.AddEntityType(typeof(D));
SetPrimaryKey(entityD);
SetBaseType(entityD, entityA);
var entityF = model.AddEntityType(typeof(F));
SetBaseType(entityF, entityA);

VerifyError(CoreStrings.InconsistentInheritance(entityD.DisplayName(), entityA.DisplayName()), model);
VerifyError(CoreStrings.InconsistentInheritance(nameof(F), nameof(D)), model);
}

[Fact]
Expand Down
4 changes: 4 additions & 0 deletions test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ protected class D : A
{
}

protected class F : D
{
}

protected abstract class Abstract : A
{
}
Expand Down
Loading

0 comments on commit 2507a80

Please sign in to comment.