From 91eeff8c42d745016676b6f4454d7986da163516 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Sun, 11 Mar 2018 23:42:38 -0700 Subject: [PATCH] Don't discover collection of string as navigation Handle OwnedAttribute correctly Convert types to owned in a more robust way Fixes #11074 Fixes #11152 --- .../DataAnnotationTestBase.cs | 10 ++- .../Internal/BaseTypeDiscoveryConvention.cs | 2 +- .../ForeignKeyPropertyDiscoveryConvention.cs | 9 ++- .../OwnedEntityTypeAttributeConvention.cs | 17 ++-- src/EFCore/Metadata/Internal/EntityType.cs | 11 ++- .../Metadata/Internal/EntityTypeExtensions.cs | 4 +- .../Internal/InternalEntityTypeBuilder.cs | 27 +++++++ .../Metadata/Internal/InternalModelBuilder.cs | 14 ++-- .../Internal/InternalRelationshipBuilder.cs | 77 +++++++++---------- src/Shared/PropertyInfoExtensions.cs | 2 +- .../Internal/MigrationsModelDifferTest.cs | 10 ++- .../ModelBuilding/InheritanceTestBase.cs | 10 ++- .../ModelBuilding/NonRelationshipTestBase.cs | 19 +++++ .../ModelBuilding/OwnedTypesTestBase.cs | 65 +++++++++++++--- test/EFCore.Tests/ModelBuilding/TestModel.cs | 8 ++ 15 files changed, 208 insertions(+), 77 deletions(-) diff --git a/src/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/src/EFCore.Specification.Tests/DataAnnotationTestBase.cs index c262923b771..aea80ef472f 100644 --- a/src/EFCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/src/EFCore.Specification.Tests/DataAnnotationTestBase.cs @@ -2089,10 +2089,16 @@ public virtual void OwnedEntityTypeAttribute_configures_all_references_as_owned( modelBuilder.Entity(); modelBuilder.Entity(); + Validate(modelBuilder); + Assert.True(model.FindEntityType(typeof(Book)).FindNavigation(nameof(Book.AdditionalDetails)).ForeignKey.IsOwnership); var one = model.FindEntityType(typeof(One)); - Assert.True(one.FindNavigation(nameof(One.Details)).ForeignKey.IsOwnership); - Assert.True(one.FindNavigation(nameof(One.AdditionalDetails)).ForeignKey.IsOwnership); + var ownership1 = one.FindNavigation(nameof(One.Details)).ForeignKey; + Assert.True(ownership1.IsOwnership); + Assert.NotNull(ownership1.DeclaringEntityType.FindProperty(nameof(Details.Name))); + var ownership2 = one.FindNavigation(nameof(One.AdditionalDetails)).ForeignKey; + Assert.True(ownership2.IsOwnership); + Assert.NotNull(ownership2.DeclaringEntityType.FindProperty(nameof(Details.Name))); } public abstract class DataAnnotationFixtureBase : SharedStoreFixtureBase diff --git a/src/EFCore/Metadata/Conventions/Internal/BaseTypeDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/Internal/BaseTypeDiscoveryConvention.cs index 7d2dd40cb06..c9304be4bc1 100644 --- a/src/EFCore/Metadata/Conventions/Internal/BaseTypeDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/Internal/BaseTypeDiscoveryConvention.cs @@ -28,7 +28,7 @@ public virtual InternalEntityTypeBuilder Apply(InternalEntityTypeBuilder entityT var baseEntityType = FindClosestBaseType(entityType); if (baseEntityType == null - || baseEntityType.FindOwnership() != null) + || baseEntityType.DefiningNavigationName != null) { return entityTypeBuilder; } diff --git a/src/EFCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs b/src/EFCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs index 1f827cd0f91..942d332c0ff 100644 --- a/src/EFCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs +++ b/src/EFCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs @@ -62,6 +62,8 @@ public virtual InternalRelationshipBuilder Apply(InternalRelationshipBuilder rel if (foreignKey.DeclaringEntityType.DefiningEntityType == foreignKey.PrincipalEntityType || foreignKey.IsOwnership || foreignKey.DeclaringEntityType.IsQueryType + || foreignKey.IsSelfReferencing() + || foreignKey.PrincipalToDependent?.IsCollection() == true || foreignKey.DeclaringEntityType.FindOwnership() != null) { relationshipBuilder = relationshipBuilder.RelatedEntityTypes( @@ -180,16 +182,17 @@ private static string GetPropertyBaseName(ForeignKey foreignKey) var foreignKey = relationshipBuilder.Metadata; if (ConfigurationSource.Convention.Overrides(foreignKey.GetPrincipalEndConfigurationSource()) && foreignKey.DeclaringEntityType.DefiningEntityType != foreignKey.PrincipalEntityType - && !foreignKey.DeclaringEntityType.IsQueryType && !foreignKey.IsOwnership + && !foreignKey.DeclaringEntityType.IsQueryType && !foreignKey.IsSelfReferencing() - && (foreignKey.PrincipalToDependent?.IsCollection() != true)) + && foreignKey.PrincipalToDependent?.IsCollection() != true + && foreignKey.DeclaringEntityType.FindOwnership() == null) { var candidatePropertiesOnPrincipal = FindCandidateForeignKeyProperties(foreignKey, onDependent: false); if (candidatePropertiesOnPrincipal != null && !foreignKey.PrincipalEntityType.FindForeignKeysInHierarchy(candidatePropertiesOnPrincipal).Any()) { - // Ambiguous principal end + // Principal end became ambiguous if (relationshipBuilder.Metadata.GetPrincipalEndConfigurationSource() == ConfigurationSource.Convention) { relationshipBuilder.Metadata.SetPrincipalEndConfigurationSource(null); diff --git a/src/EFCore/Metadata/Conventions/Internal/OwnedEntityTypeAttributeConvention.cs b/src/EFCore/Metadata/Conventions/Internal/OwnedEntityTypeAttributeConvention.cs index e530843b123..ea7223834d1 100644 --- a/src/EFCore/Metadata/Conventions/Internal/OwnedEntityTypeAttributeConvention.cs +++ b/src/EFCore/Metadata/Conventions/Internal/OwnedEntityTypeAttributeConvention.cs @@ -16,11 +16,16 @@ public class OwnedEntityTypeAttributeConvention : EntityTypeAttributeConvention< /// directly from your code. This API may change or be removed in future releases. /// public override InternalEntityTypeBuilder Apply(InternalEntityTypeBuilder entityTypeBuilder, OwnedAttribute attribute) - => (entityTypeBuilder.Metadata.HasClrType() - ? entityTypeBuilder.ModelBuilder.Owned(entityTypeBuilder.Metadata.ClrType, ConfigurationSource.DataAnnotation) - : entityTypeBuilder.ModelBuilder.Owned(entityTypeBuilder.Metadata.Name, ConfigurationSource.DataAnnotation)) - && !entityTypeBuilder.Metadata.HasDefiningNavigation() - ? null - : entityTypeBuilder; + { + if (entityTypeBuilder.Metadata.HasClrType()) + { + entityTypeBuilder.ModelBuilder.Owned(entityTypeBuilder.Metadata.ClrType, ConfigurationSource.DataAnnotation); + } + else + { + entityTypeBuilder.ModelBuilder.Owned(entityTypeBuilder.Metadata.Name, ConfigurationSource.DataAnnotation); + } + return entityTypeBuilder.Metadata.Builder; + } } } diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index 37192735258..3e59c1d4e2a 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -983,7 +983,7 @@ public virtual IEnumerable GetDerivedForeignKeys() /// directly from your code. This API may change or be removed in future releases. /// public virtual IEnumerable GetDerivedForeignKeysInclusive() - => GetDeclaredForeignKeys().Concat(GetDerivedTypes().SelectMany(et => et.GetDeclaredForeignKeys())); + => GetDeclaredForeignKeys().Concat(GetDerivedForeignKeys()); /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used @@ -1152,12 +1152,19 @@ public virtual IEnumerable GetReferencingForeignKeys() public virtual IEnumerable GetDeclaredReferencingForeignKeys() => DeclaredReferencingForeignKeys ?? Enumerable.Empty(); + /// + /// 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. + /// + public virtual IEnumerable GetDerivedReferencingForeignKeys() + => GetDerivedTypes().SelectMany(et => et.GetDeclaredReferencingForeignKeys()); + /// /// 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. /// public virtual IEnumerable GetDerivedReferencingForeignKeysInclusive() - => GetDeclaredReferencingForeignKeys().Concat(GetDerivedTypes().SelectMany(et => et.GetDeclaredReferencingForeignKeys())); + => GetDeclaredReferencingForeignKeys().Concat(GetDerivedReferencingForeignKeys()); private SortedSet DeclaredReferencingForeignKeys { get; set; } diff --git a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs index cb227ef6bbf..b958cc71207 100644 --- a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs +++ b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs @@ -255,7 +255,7 @@ public static bool IsInDefinitionPath([NotNull] this IEntityType entityType, [No /// 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. /// - public static bool IsInOwnershipPath([NotNull] this EntityType entityType, [NotNull] EntityType entityTypeToOwn) + public static bool IsInOwnershipPath([NotNull] this EntityType entityType, [NotNull] EntityType targetType) { var owner = entityType; while (true) @@ -267,7 +267,7 @@ public static bool IsInOwnershipPath([NotNull] this EntityType entityType, [NotN } owner = ownOwnership.PrincipalEntityType; - if (owner == entityTypeToOwn) + if (owner == targetType) { return true; } diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 4688fbcc31b..4b424a63191 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -782,6 +782,7 @@ public virtual InternalEntityTypeBuilder HasBaseType([CanBeNull] EntityType base List detachedIndexes = null; HashSet removedInheritedPropertiesToDuplicate = null; + List< (string, ConfigurationSource)> membersToIgnore = null; if (Metadata.BaseType != null) { var removedInheritedProperties = new HashSet( @@ -843,10 +844,36 @@ public virtual InternalEntityTypeBuilder HasBaseType([CanBeNull] EntityType base } } } + + foreach (var ignoredMember in Metadata.BaseType.GetIgnoredMembers()) + { + if (baseEntityType != null + && (baseEntityType.FindProperty(ignoredMember) != null + || baseEntityType.FindNavigation(ignoredMember) != null)) + { + continue; + } + + if (membersToIgnore == null) + { + membersToIgnore = new List<(string, ConfigurationSource)>(); + } + + membersToIgnore.Add( + (ignoredMember, Metadata.BaseType.FindDeclaredIgnoredMemberConfigurationSource(ignoredMember).Value)); + } } Metadata.HasBaseType(baseEntityType, configurationSource); + if (membersToIgnore != null) + { + foreach (var ignoreTuple in membersToIgnore) + { + Ignore(ignoreTuple.Item1, ignoreTuple.Item2); + } + } + if (removedInheritedPropertiesToDuplicate != null) { foreach (var property in removedInheritedPropertiesToDuplicate) diff --git a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs index 11537831a20..72f54ea522b 100644 --- a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs @@ -267,8 +267,7 @@ public virtual InternalEntityTypeBuilder Query([NotNull] Type clrType) [NotNull] Type type, ConfigurationSource configurationSource) => Owned(new TypeIdentity(type), configurationSource); - private bool Owned( - TypeIdentity type, ConfigurationSource configurationSource) + private bool Owned(TypeIdentity type, ConfigurationSource configurationSource) { if (IsIgnored(type, configurationSource)) { @@ -292,12 +291,11 @@ public virtual InternalEntityTypeBuilder Query([NotNull] Type clrType) throw new InvalidOperationException(CoreStrings.ClashingNonOwnedEntityType(entityType.DisplayName())); } - var potentialOwnerships = entityType.GetForeignKeys().Where(fk => fk.PrincipalToDependent != null).ToList(); - foreach (var foreignKey in potentialOwnerships) - { - foreignKey.PrincipalEntityType.FindNavigation(foreignKey.PrincipalToDependent.Name).ForeignKey.Builder - .IsOwnership(true, configurationSource); - } + var ownership = entityType.GetForeignKeys().FirstOrDefault(fk => + fk.PrincipalToDependent != null + && !fk.PrincipalEntityType.IsInOwnershipPath(entityType) + && !fk.PrincipalEntityType.IsInDefinitionPath(clrType)); + ownership?.Builder.IsOwnership(true, configurationSource); } if (clrType == null) diff --git a/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs b/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs index 0a0385032bb..d6cb0753db7 100644 --- a/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs @@ -828,8 +828,8 @@ public virtual bool CanSetRequired(bool isRequired, ConfigurationSource? configu ConfigurationSource? configurationSource, bool shouldThrow) { - if ((isRequired == null) - || (properties == null)) + if (isRequired == null + || properties == null) { return true; } @@ -883,6 +883,8 @@ public virtual InternalRelationshipBuilder IsOwnership(bool ownership, Configura { otherOwnership.Builder.IsOwnership(false, configurationSource); } + + Metadata.SetIsOwnership(true, configurationSource); } else if (otherOwnerships.Count > 0) { @@ -893,6 +895,7 @@ public virtual InternalRelationshipBuilder IsOwnership(bool ownership, Configura } var otherOwnership = otherOwnerships.Single(); + Metadata.SetIsOwnership(true, configurationSource); Metadata.DeclaringEntityType.Builder.RemoveForeignKey(Metadata, Metadata.GetConfigurationSource()); if (otherOwnership.Builder.IsWeakTypeDefinition(configurationSource) == null) @@ -919,6 +922,7 @@ public virtual InternalRelationshipBuilder IsOwnership(bool ownership, Configura } else { + Metadata.SetIsOwnership(true, configurationSource); newRelationshipBuilder.Metadata.DeclaringEntityType.Builder.HasBaseType((Type)null, configurationSource); } @@ -928,12 +932,18 @@ public virtual InternalRelationshipBuilder IsOwnership(bool ownership, Configura newRelationshipBuilder.Metadata.Properties.Select(p => p.Name).ToList(), ConfigurationSource.Convention); } } + else + { + newRelationshipBuilder.Metadata.SetIsOwnership(false, configurationSource); + } - newRelationshipBuilder.Metadata.SetIsOwnership(ownership, configurationSource); - - foreach (var directlyDerivedType in newRelationshipBuilder.Metadata.DeclaringEntityType.GetDirectlyDerivedTypes().ToList()) + foreach (var derivedForeignKey in newRelationshipBuilder.Metadata.DeclaringEntityType.GetDerivedForeignKeys() + .Where(fk => fk.PrincipalToDependent != null) + .Concat(newRelationshipBuilder.Metadata.DeclaringEntityType.GetDerivedReferencingForeignKeys() + .Where(fk => fk.DependentToPrincipal != null)).ToList()) { - directlyDerivedType.Builder.HasBaseType((string)null, ConfigurationSource.Convention); + derivedForeignKey.DeclaringEntityType.Builder + .RemoveForeignKey(derivedForeignKey, configurationSource); } return batch.Run(newRelationshipBuilder); @@ -951,45 +961,34 @@ public virtual InternalRelationshipBuilder IsWeakTypeDefinition(ConfigurationSou return this; } - if (Metadata.GetConfigurationSource().Overrides(ConfigurationSource.Explicit) - || !Metadata.PrincipalEntityType.IsInDefinitionPath(Metadata.DeclaringEntityType.ClrType)) + EntityType newEntityType; + if (Metadata.DeclaringEntityType.ClrType == null) { - EntityType newEntityType; - if (Metadata.DeclaringEntityType.ClrType == null) - { - newEntityType = ModelBuilder.Entity( - Metadata.DeclaringEntityType.Name, - Metadata.PrincipalToDependent.Name, - Metadata.PrincipalEntityType, - Metadata.DeclaringEntityType.GetConfigurationSource()).Metadata; - } - else - { - newEntityType = ModelBuilder.Entity( - Metadata.DeclaringEntityType.ClrType, - Metadata.PrincipalToDependent.Name, - Metadata.PrincipalEntityType, - Metadata.DeclaringEntityType.GetConfigurationSource()).Metadata; - } - - var newOwnership = newEntityType.GetForeignKeys().SingleOrDefault(fk => fk.IsOwnership); - if (newOwnership == null) - { - Debug.Assert(Metadata.Builder != null); - return Metadata.Builder; - } - - Debug.Assert(Metadata.Builder == null); - ModelBuilder.Metadata.ConventionDispatcher.Tracker.Update(Metadata, newOwnership); - return newOwnership.Builder; + newEntityType = ModelBuilder.Entity( + Metadata.DeclaringEntityType.Name, + Metadata.PrincipalToDependent.Name, + Metadata.PrincipalEntityType, + Metadata.DeclaringEntityType.GetConfigurationSource()).Metadata; + } + else + { + newEntityType = ModelBuilder.Entity( + Metadata.DeclaringEntityType.ClrType, + Metadata.PrincipalToDependent.Name, + Metadata.PrincipalEntityType, + Metadata.DeclaringEntityType.GetConfigurationSource()).Metadata; } - if (Metadata.Builder.IsOwnership(false, configurationSource) == null) + var newOwnership = newEntityType.GetForeignKeys().SingleOrDefault(fk => fk.IsOwnership); + if (newOwnership == null) { - return null; + Debug.Assert(Metadata.Builder != null); + return Metadata.Builder; } - return this; + Debug.Assert(Metadata.Builder == null); + ModelBuilder.Metadata.ConventionDispatcher.Tracker.Update(Metadata, newOwnership); + return newOwnership.Builder; } /// diff --git a/src/Shared/PropertyInfoExtensions.cs b/src/Shared/PropertyInfoExtensions.cs index 3298732e0d1..daa66cb8855 100644 --- a/src/Shared/PropertyInfoExtensions.cs +++ b/src/Shared/PropertyInfoExtensions.cs @@ -39,7 +39,7 @@ public static bool IsCandidateProperty(this PropertyInfo propertyInfo, bool need targetType = targetSequenceType ?? targetType; targetType = targetType.UnwrapNullableType(); - if (typeMappingSource.FindMapping(propertyInfo) != null + if (typeMappingSource.FindMapping(targetType) != null || targetType.GetTypeInfo().IsInterface || targetType.GetTypeInfo().IsValueType || targetType == typeof(object) diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs index bee476b18d9..8b19eedfaaa 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs @@ -6692,12 +6692,18 @@ public void Move_properties_to_owned_type_with_existing_ownership() public void Rename_property_on_owned_type_and_add_similar_to_owner() { Execute( - source => source.Entity().OwnsOne(o => o.Billing).Property("OldZip"), + source => source.Entity( + x => + { + x.OwnsOne(o => o.Billing).Property("OldZip"); + x.Ignore(o => o.Shipping); + }), target => target.Entity( x => { x.Property("NotZip"); x.OwnsOne(o => o.Billing).Property("NewZip"); + x.Ignore(o => o.Shipping); }), operations => { @@ -6723,12 +6729,14 @@ public void Rename_property_on_owning_type_and_add_similar_to_owned() { x.Property("OldDate"); x.OwnsOne(o => o.Billing); + x.Ignore(o => o.Shipping); }), target => target.Entity( x => { x.Property("NewDate"); x.OwnsOne(o => o.Billing).Property("AnotherDate"); + x.Ignore(o => o.Shipping); }), operations => { diff --git a/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs b/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs index 8bdd9a9a614..68212026e84 100644 --- a/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/InheritanceTestBase.cs @@ -151,6 +151,7 @@ public virtual void Setting_base_type_to_null_fixes_relationships() var dependentEntityBuilder = modelBuilder.Entity(); var derivedDependentEntityBuilder = modelBuilder.Entity(); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.SpecialCustomer)); + derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.ShippingAddress)); Assert.Same(principalEntityBuilder.Metadata, derivedPrincipalEntityBuilder.Metadata.BaseType); Assert.Same(dependentEntityBuilder.Metadata, derivedDependentEntityBuilder.Metadata.BaseType); @@ -196,6 +197,7 @@ public virtual void Pulling_relationship_to_a_derived_type_creates_relationships var derivedDependentEntityBuilder = modelBuilder.Entity(); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.BackOrder)); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.SpecialCustomer)); + derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.ShippingAddress)); var otherDerivedDependentEntityBuilder = modelBuilder.Entity(); otherDerivedDependentEntityBuilder.Ignore(nameof(BackOrder.SpecialOrder)); @@ -244,6 +246,7 @@ public virtual void Pulling_relationship_to_a_derived_type_reverted_creates_rela var derivedDependentEntityBuilder = modelBuilder.Entity(); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.BackOrder)); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.SpecialCustomer)); + derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.ShippingAddress)); var otherDerivedDependentEntityBuilder = modelBuilder.Entity(); otherDerivedDependentEntityBuilder.Ignore(nameof(BackOrder.SpecialOrder)); @@ -276,6 +279,7 @@ public virtual void Pulling_relationship_to_a_derived_type_many_to_one_creates_r dependentEntityBuilder.Ignore(nameof(Order.Customer)); var derivedDependentEntityBuilder = modelBuilder.Entity(); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.BackOrder)); + derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.ShippingAddress)); var otherDerivedPrincipalEntityBuilder = modelBuilder.Entity(); derivedPrincipalEntityBuilder @@ -305,6 +309,7 @@ public virtual void Pulling_relationship_to_a_derived_type_one_to_one_creates_re var derivedDependentEntityBuilder = modelBuilder.Entity(); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.BackOrder)); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.SpecialCustomer)); + derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.ShippingAddress)); principalEntityBuilder .HasOne(e => (SpecialOrder)e.Order) @@ -358,6 +363,7 @@ public virtual void Pulling_relationship_to_a_derived_type_with_fk_creates_relat var derivedDependentEntityBuilder = modelBuilder.Entity(); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.BackOrder)); derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.SpecialCustomer)); + derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.ShippingAddress)); var otherDerivedDependentEntityBuilder = modelBuilder.Entity(); otherDerivedDependentEntityBuilder.Ignore(nameof(BackOrder.SpecialOrder)); @@ -391,6 +397,7 @@ public virtual void Can_promote_shadow_fk_to_the_base_type() dependentEntityBuilder.Ignore(e => e.Customer); var derivedDependentEntityBuilder = modelBuilder.Entity(); derivedDependentEntityBuilder.Ignore(e => e.SpecialCustomerId); + derivedDependentEntityBuilder.Ignore(e => e.ShippingAddress); dependentEntityBuilder .HasOne() @@ -418,6 +425,7 @@ public virtual void Removing_a_key_triggers_fk_discovery_on_derived_types() var dependentEntityBuilder = modelBuilder.Entity(); dependentEntityBuilder.Ignore(nameof(Order.Customer)); var derivedDependentEntityBuilder = modelBuilder.Entity(); + derivedDependentEntityBuilder.Ignore(nameof(SpecialOrder.ShippingAddress)); dependentEntityBuilder.Property("SpecialCustomerId"); derivedPrincipalEntityBuilder @@ -446,7 +454,6 @@ public virtual void Index_removed_when_covered_by_an_inherited_foreign_key() modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); - modelBuilder.Ignore(); var principalEntityBuilder = modelBuilder.Entity(); var derivedPrincipalEntityBuilder = modelBuilder.Entity(); @@ -517,7 +524,6 @@ public virtual void Index_removed_when_covered_by_an_inherited_index() modelBuilder.Ignore(); modelBuilder.Ignore(); modelBuilder.Ignore(); - modelBuilder.Ignore(); modelBuilder.Entity(); var derivedPrincipalEntityBuilder = modelBuilder.Entity(); diff --git a/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs b/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs index 0d19e54771d..ffe0c8d1bd4 100644 --- a/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Linq; using System.Text; using Microsoft.EntityFrameworkCore.ChangeTracking; @@ -1006,6 +1007,24 @@ private abstract class BadCustomValueGenerator2 : CustomValueGenerator { } + [Fact] + public virtual void Throws_for_collection_of_string() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity(); + + Assert.Equal( + CoreStrings.PropertyNotAdded( + nameof(StringCollectionEntity), nameof(StringCollectionEntity.Property), "ICollection"), + Assert.Throws(() => modelBuilder.Validate()).Message); + } + + protected class StringCollectionEntity + { + public ICollection Property { get; set; } + } + [Fact] public virtual void Can_set_unicode_for_properties() { diff --git a/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs b/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs index a585a0668c6..5ec03a628c0 100644 --- a/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs +++ b/test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs @@ -99,6 +99,22 @@ public virtual void Can_configure_owned_type_properties() Assert.Equal(new[] { nameof(CustomerDetails.CustomerId), "foo" }, ownee.GetProperties().Select(p => p.Name).ToArray()); } + [Fact] + public virtual void Can_configure_single_owned_type_using_attribute() + { + var modelBuilder = CreateModelBuilder(); + var model = modelBuilder.Model; + + modelBuilder.Entity(); + + modelBuilder.Validate(); + + var owner = model.FindEntityType(typeof(SpecialOrder)); + var ownership = owner.FindNavigation(nameof(SpecialOrder.ShippingAddress)).ForeignKey; + Assert.True(ownership.IsOwnership); + Assert.NotNull(ownership.DeclaringEntityType.FindProperty(nameof(StreetAddress.Street))); + } + [Fact] public virtual void Can_configure_ownership_foreign_key() { @@ -240,38 +256,52 @@ public virtual void Can_map_base_of_owned_type_first() public virtual void Can_map_derived_of_owned_type() { var modelBuilder = CreateModelBuilder(); - var model = modelBuilder.Model; + var model = (Model)modelBuilder.Model; modelBuilder.Ignore(); modelBuilder.Entity().OwnsOne(c => c.Details); modelBuilder.Entity(); - modelBuilder.Validate(); - var owner = model.FindEntityType(typeof(OrderCombination)); var owned = owner.FindNavigation(nameof(OrderCombination.Details)).ForeignKey.DeclaringEntityType; - Assert.Empty(owned.GetDirectlyDerivedTypes()); + Assert.NotEmpty(owned.GetDirectlyDerivedTypes()); + Assert.Empty(model.GetEntityTypes().SelectMany(e => e.GetDeclaredNavigations()).Where(n => + { + var targetType = n.GetTargetType().ClrType; + return targetType != typeof(DetailsBase) && typeof(DetailsBase).IsAssignableFrom(targetType); + })); Assert.Equal(1, owned.GetForeignKeys().Count()); Assert.Equal(1, model.GetEntityTypes().Count(e => e.ClrType == typeof(DetailsBase))); - Assert.Null(model.FindEntityType(typeof(CustomerDetails)).BaseType); + Assert.Same(owned, model.FindEntityType(typeof(CustomerDetails)).BaseType); + + modelBuilder.Entity().Ignore(c => c.Details); + modelBuilder.Entity().Ignore(c => c.Details); + modelBuilder.Validate(); } [Fact] public virtual void Can_map_derived_of_owned_type_first() { var modelBuilder = CreateModelBuilder(); - var model = modelBuilder.Model; + var model = (Model)modelBuilder.Model; modelBuilder.Entity().OwnsOne(c => c.Details); - modelBuilder.Validate(); - var owner = model.FindEntityType(typeof(OrderCombination)); var owned = owner.FindNavigation(nameof(OrderCombination.Details)).ForeignKey.DeclaringEntityType; - Assert.Empty(owned.GetDirectlyDerivedTypes()); + Assert.NotEmpty(owned.GetDirectlyDerivedTypes()); + Assert.Empty(model.GetEntityTypes().SelectMany(e => e.GetDeclaredNavigations()).Where(n => + { + var targetType = n.GetTargetType().ClrType; + return targetType != typeof(DetailsBase) && typeof(DetailsBase).IsAssignableFrom(targetType); + })); Assert.Equal(1, owned.GetForeignKeys().Count()); Assert.Equal(1, model.GetEntityTypes().Count(e => e.ClrType == typeof(DetailsBase))); - Assert.Null(model.FindEntityType(typeof(CustomerDetails)).BaseType); + Assert.Same(owned, model.FindEntityType(typeof(CustomerDetails)).BaseType); + + modelBuilder.Entity().Ignore(c => c.Details); + modelBuilder.Entity().Ignore(c => c.Details); + modelBuilder.Validate(); } [Fact] @@ -364,6 +394,21 @@ public virtual void Can_configure_all_ownerships_with_one_call() VerifyOwnedBookLabelModel(modelBuilder.Model); } + [Fact] + public virtual void Can_configure_all_ownerships_with_one_call_afterwards() + { + var modelBuilder = CreateModelBuilder(); + + modelBuilder.Entity(); + modelBuilder.Owned(); + modelBuilder.Entity().OwnsOne(b => b.Label); + modelBuilder.Entity().OwnsOne(b => b.AlternateLabel); + + modelBuilder.Validate(); + + VerifyOwnedBookLabelModel(modelBuilder.Model); + } + [Fact] public virtual void Collection_navigation_to_owned_entity_type_is_ignored() { diff --git a/test/EFCore.Tests/ModelBuilding/TestModel.cs b/test/EFCore.Tests/ModelBuilding/TestModel.cs index bf8eea4b37b..3d9962eba6e 100644 --- a/test/EFCore.Tests/ModelBuilding/TestModel.cs +++ b/test/EFCore.Tests/ModelBuilding/TestModel.cs @@ -132,6 +132,13 @@ protected class Order public OrderDetails Details { get; set; } } + [Owned] + public class StreetAddress + { + public string Street { get; set; } + public string City { get; set; } + } + [NotMapped] protected class SpecialOrder : Order { @@ -139,6 +146,7 @@ protected class SpecialOrder : Order public SpecialCustomer SpecialCustomer { get; set; } public BackOrder BackOrder { get; set; } public OrderCombination SpecialOrderCombination { get; set; } + public StreetAddress ShippingAddress { get; set; } } protected class BackOrder : Order