From 2782c47d1e131d1c425a1888560aa5eeadfdb533 Mon Sep 17 00:00:00 2001 From: lajones Date: Wed, 18 May 2016 15:02:37 -0700 Subject: [PATCH] Storing NavProp names directly on the IModel generated by RelationalScaffoldingModelFactory and then drawing the names directly from there which allows us to stop using the ScaffoldingForeignKeyAnnotations altogether. --- .../ScaffoldingForeignKeyAnnotations.cs | 37 ------ .../Metadata/ScaffoldingMetadataExtensions.cs | 3 - ...tityFrameworkCore.Relational.Design.csproj | 1 - .../RelationalScaffoldingModelFactory.cs | 95 ++++++------- .../Internal/ModelConfiguration.cs | 125 +++++++++--------- .../Metadata/Internal/ForeignKey.cs | 4 +- .../ScaffoldingMetadataExtenstionsTest.cs | 27 ---- 7 files changed, 112 insertions(+), 180 deletions(-) delete mode 100644 src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingForeignKeyAnnotations.cs diff --git a/src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingForeignKeyAnnotations.cs b/src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingForeignKeyAnnotations.cs deleted file mode 100644 index 4a903943f78..00000000000 --- a/src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingForeignKeyAnnotations.cs +++ /dev/null @@ -1,37 +0,0 @@ -// 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; -using Microsoft.EntityFrameworkCore.Scaffolding.Metadata.Internal; -using Microsoft.EntityFrameworkCore.Utilities; - -namespace Microsoft.EntityFrameworkCore.Scaffolding.Metadata -{ - public class ScaffoldingForeignKeyAnnotations : RelationalForeignKeyAnnotations - { - public ScaffoldingForeignKeyAnnotations([NotNull] IForeignKey foreignKey) - : base(foreignKey, ScaffoldingFullAnnotationNames.Instance) - { - } - - public virtual string DependentEndNavigation - { - get { return (string)Annotations.GetAnnotation(ScaffoldingFullAnnotationNames.Instance.DependentEndNavigation, null); } - [param: CanBeNull] set { Annotations.SetAnnotation(ScaffoldingFullAnnotationNames.Instance.DependentEndNavigation, null, Check.NullButNotEmpty(value, nameof(value))); } - } - - public virtual string PrincipalEndNavigation - { - get { return (string)Annotations.GetAnnotation(ScaffoldingFullAnnotationNames.Instance.PrincipalEndNavigation, null); } - [param: CanBeNull] - set - { - Annotations.SetAnnotation( - ScaffoldingFullAnnotationNames.Instance.PrincipalEndNavigation, - null, - Check.NullButNotEmpty(value, nameof(value))); - } - } - } -} diff --git a/src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingMetadataExtensions.cs b/src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingMetadataExtensions.cs index e953f9c60d1..3a39735c410 100644 --- a/src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingMetadataExtensions.cs +++ b/src/Microsoft.EntityFrameworkCore.Relational.Design/Metadata/ScaffoldingMetadataExtensions.cs @@ -15,8 +15,5 @@ public static ScaffoldingModelAnnotations Scaffolding([NotNull] this IModel mode public static ScaffoldingPropertyAnnotations Scaffolding([NotNull] this IProperty property) => new ScaffoldingPropertyAnnotations(Check.NotNull(property, nameof(property))); - - public static ScaffoldingForeignKeyAnnotations Scaffolding([NotNull] this IForeignKey foreignKey) - => new ScaffoldingForeignKeyAnnotations(Check.NotNull(foreignKey, nameof(foreignKey))); } } diff --git a/src/Microsoft.EntityFrameworkCore.Relational.Design/Microsoft.EntityFrameworkCore.Relational.Design.csproj b/src/Microsoft.EntityFrameworkCore.Relational.Design/Microsoft.EntityFrameworkCore.Relational.Design.csproj index c3c4f902f19..9f9c7b9759f 100644 --- a/src/Microsoft.EntityFrameworkCore.Relational.Design/Microsoft.EntityFrameworkCore.Relational.Design.csproj +++ b/src/Microsoft.EntityFrameworkCore.Relational.Design/Microsoft.EntityFrameworkCore.Relational.Design.csproj @@ -76,7 +76,6 @@ - diff --git a/src/Microsoft.EntityFrameworkCore.Relational.Design/RelationalScaffoldingModelFactory.cs b/src/Microsoft.EntityFrameworkCore.Relational.Design/RelationalScaffoldingModelFactory.cs index db63379259a..79e755e7739 100644 --- a/src/Microsoft.EntityFrameworkCore.Relational.Design/RelationalScaffoldingModelFactory.cs +++ b/src/Microsoft.EntityFrameworkCore.Relational.Design/RelationalScaffoldingModelFactory.cs @@ -131,8 +131,6 @@ protected virtual ModelBuilder VisitDatabaseModel([NotNull] ModelBuilder modelBu VisitSequences(modelBuilder, databaseModel.Sequences); VisitTables(modelBuilder, databaseModel.Tables); VisitForeignKeys(modelBuilder, databaseModel.Tables.SelectMany(table => table.ForeignKeys).ToList()); - // TODO can we add navigation properties inline with adding foreign keys? - VisitNavigationProperties(modelBuilder); return modelBuilder; } @@ -451,6 +449,14 @@ protected virtual ModelBuilder VisitForeignKeys([NotNull] ModelBuilder modelBuil VisitForeignKey(modelBuilder, fk); } + // Note: must completely assign all foreign keys before assigning + // navigation properties otherwise naming of navigation properties + // when there are multiple foreign keys does not work. + foreach (var foreignKey in modelBuilder.Model.GetEntityTypes().SelectMany(et => et.GetForeignKeys())) + { + AddNavigationProperties(foreignKey); + } + return modelBuilder; } @@ -559,59 +565,56 @@ protected virtual IMutableForeignKey VisitForeignKey([NotNull] ModelBuilder mode return key; } - protected virtual void VisitNavigationProperties([NotNull] ModelBuilder modelBuilder) + protected virtual void AddNavigationProperties([NotNull] IMutableForeignKey foreignKey) { - Check.NotNull(modelBuilder, nameof(modelBuilder)); + Check.NotNull(foreignKey, nameof(foreignKey)); - // TODO perf cleanup can we do this in 1 loop instead of 2? - var model = modelBuilder.Model; var modelUtilities = new ModelUtilities(); + var dependentEndExistingIdentifiers = ExistingIdentifiers(foreignKey.DeclaringEntityType); + var dependentEndNavigationPropertyCandidateName = + modelUtilities.GetDependentEndCandidateNavigationPropertyName(foreignKey); + var dependentEndNavigationPropertyName = + CSharpUtilities.Instance.GenerateCSharpIdentifier( + dependentEndNavigationPropertyCandidateName, + dependentEndExistingIdentifiers, + NavigationUniquifier); + + foreignKey.HasDependentToPrincipal(dependentEndNavigationPropertyName); + + var principalEndExistingIdentifiers = ExistingIdentifiers(foreignKey.PrincipalEntityType); + var principalEndNavigationPropertyCandidateName = foreignKey.IsSelfReferencing() + ? string.Format( + CultureInfo.CurrentCulture, + SelfReferencingPrincipalEndNavigationNamePattern, + dependentEndNavigationPropertyName) + : modelUtilities.GetPrincipalEndCandidateNavigationPropertyName( + foreignKey, dependentEndNavigationPropertyName); + var principalEndNavigationPropertyName = + CSharpUtilities.Instance.GenerateCSharpIdentifier( + principalEndNavigationPropertyCandidateName, + principalEndExistingIdentifiers, + NavigationUniquifier); + + foreignKey.HasPrincipalToDependent(principalEndNavigationPropertyName); + } - var entityTypeToExistingIdentifiers = new Dictionary>(); - foreach (var entityType in model.GetEntityTypes()) + // Stores the names of the EntityType itself and its Properties, but does not include any Navigation Properties + private Dictionary> _entityTypeAndPropertyIdentifiers = new Dictionary>(); + protected virtual List ExistingIdentifiers([NotNull] IEntityType entityType) + { + Check.NotNull(entityType, nameof(entityType)); + + List existingIdentifiers; + if (!_entityTypeAndPropertyIdentifiers.TryGetValue(entityType, out existingIdentifiers)) { - var existingIdentifiers = new List(); - entityTypeToExistingIdentifiers.Add(entityType, existingIdentifiers); + existingIdentifiers = new List(); existingIdentifiers.Add(entityType.Name); existingIdentifiers.AddRange(entityType.GetProperties().Select(p => p.Name)); + _entityTypeAndPropertyIdentifiers[entityType] = existingIdentifiers; } - foreach (var entityType in model.GetEntityTypes()) - { - var dependentEndExistingIdentifiers = entityTypeToExistingIdentifiers[entityType]; - foreach (var foreignKey in entityType.GetForeignKeys()) - { - // set up the name of the navigation property on the dependent end of the foreign key - var dependentEndNavigationPropertyCandidateName = - modelUtilities.GetDependentEndCandidateNavigationPropertyName(foreignKey); - var dependentEndNavigationPropertyName = - CSharpUtilities.Instance.GenerateCSharpIdentifier( - dependentEndNavigationPropertyCandidateName, - dependentEndExistingIdentifiers, - NavigationUniquifier); - foreignKey.Scaffolding().DependentEndNavigation = dependentEndNavigationPropertyName; - dependentEndExistingIdentifiers.Add(dependentEndNavigationPropertyName); - - // set up the name of the navigation property on the principal end of the foreign key - var principalEndExistingIdentifiers = - entityTypeToExistingIdentifiers[foreignKey.PrincipalEntityType]; - var principalEndNavigationPropertyCandidateName = - foreignKey.IsSelfReferencing() - ? string.Format( - CultureInfo.CurrentCulture, - SelfReferencingPrincipalEndNavigationNamePattern, - dependentEndNavigationPropertyName) - : modelUtilities.GetPrincipalEndCandidateNavigationPropertyName( - foreignKey, dependentEndNavigationPropertyName); - var principalEndNavigationPropertyName = - CSharpUtilities.Instance.GenerateCSharpIdentifier( - principalEndNavigationPropertyCandidateName, - principalEndExistingIdentifiers, - NavigationUniquifier); - foreignKey.Scaffolding().PrincipalEndNavigation = principalEndNavigationPropertyName; - principalEndExistingIdentifiers.Add(principalEndNavigationPropertyName); - } - } + existingIdentifiers.AddRange(entityType.GetNavigations().Select(p => p.Name)); + return existingIdentifiers; } private static void AssignOnDeleteAction( diff --git a/src/Microsoft.EntityFrameworkCore.Tools.Core/Scaffolding/Configuration/Internal/ModelConfiguration.cs b/src/Microsoft.EntityFrameworkCore.Tools.Core/Scaffolding/Configuration/Internal/ModelConfiguration.cs index 240771cca45..2088b258068 100644 --- a/src/Microsoft.EntityFrameworkCore.Tools.Core/Scaffolding/Configuration/Internal/ModelConfiguration.cs +++ b/src/Microsoft.EntityFrameworkCore.Tools.Core/Scaffolding/Configuration/Internal/ModelConfiguration.cs @@ -573,24 +573,28 @@ public virtual void AddNavigationProperties([NotNull] EntityConfiguration entity foreach (var foreignKey in otherEntityType .GetForeignKeys().Where(fk => fk.PrincipalEntityType == entityConfiguration.EntityType)) { - var referencedType = foreignKey.IsUnique - ? otherEntityType.Name - : "ICollection<" + otherEntityType.Name + ">"; - var navPropConfiguration = - _configurationFactory.CreateNavigationPropertyConfiguration( - referencedType, - foreignKey.Scaffolding().PrincipalEndNavigation); - - if (foreignKey.PrincipalKey.IsPrimaryKey()) + var principalNavProp = foreignKey.PrincipalToDependent; + if (principalNavProp != null) { - navPropConfiguration.AttributeConfigurations.Add( - _configurationFactory.CreateAttributeConfiguration( - nameof(InversePropertyAttribute), - CSharpUtilities.DelimitString( - foreignKey.Scaffolding().DependentEndNavigation))); + var referencedType = foreignKey.IsUnique + ? otherEntityType.Name + : "ICollection<" + otherEntityType.Name + ">"; + var navPropConfiguration = + _configurationFactory.CreateNavigationPropertyConfiguration( + referencedType, principalNavProp.Name); + + var dependentNavProp = foreignKey.DependentToPrincipal; + if (foreignKey.PrincipalKey.IsPrimaryKey() && + dependentNavProp != null) + { + navPropConfiguration.AttributeConfigurations.Add( + _configurationFactory.CreateAttributeConfiguration( + nameof(InversePropertyAttribute), + CSharpUtilities.DelimitString(dependentNavProp.Name))); + } + + entityConfiguration.NavigationPropertyConfigurations.Add(navPropConfiguration); } - - entityConfiguration.NavigationPropertyConfigurations.Add(navPropConfiguration); } } @@ -598,54 +602,50 @@ public virtual void AddNavigationProperties([NotNull] EntityConfiguration entity { // set up the navigation property on this end of foreign keys owned by this EntityType // (i.e. this EntityType is the dependent) - var dependentEndNavPropConfiguration = - _configurationFactory.CreateNavigationPropertyConfiguration( - foreignKey.PrincipalEntityType.Name, - foreignKey.Scaffolding().DependentEndNavigation); - - if (foreignKey.PrincipalKey.IsPrimaryKey()) + var dependentNavProp = foreignKey.DependentToPrincipal; + if (dependentNavProp != null) { - dependentEndNavPropConfiguration.AttributeConfigurations.Add( - _configurationFactory.CreateAttributeConfiguration( - nameof(ForeignKeyAttribute), - CSharpUtilities.DelimitString( - string.Join(",", foreignKey.Properties.Select(p => p.Name))))); - dependentEndNavPropConfiguration.AttributeConfigurations.Add( - _configurationFactory.CreateAttributeConfiguration( - nameof(InversePropertyAttribute), - CSharpUtilities.DelimitString( - foreignKey.Scaffolding().PrincipalEndNavigation))); - } + var dependentEndNavPropConfiguration = + _configurationFactory.CreateNavigationPropertyConfiguration( + foreignKey.PrincipalEntityType.Name, dependentNavProp.Name); - entityConfiguration.NavigationPropertyConfigurations.Add( - dependentEndNavPropConfiguration); + var principalNavProp = foreignKey.PrincipalToDependent; + if (foreignKey.PrincipalKey.IsPrimaryKey() && principalNavProp != null) + { + dependentEndNavPropConfiguration.AttributeConfigurations.Add( + _configurationFactory.CreateAttributeConfiguration( + nameof(ForeignKeyAttribute), + CSharpUtilities.DelimitString( + string.Join(",", foreignKey.Properties.Select(p => p.Name))))); + dependentEndNavPropConfiguration.AttributeConfigurations.Add( + _configurationFactory.CreateAttributeConfiguration( + nameof(InversePropertyAttribute), + CSharpUtilities.DelimitString(principalNavProp.Name))); + } - // set up the other navigation property for self-referencing foreign keys owned by this EntityType - if (((ForeignKey)foreignKey).IsSelfReferencing()) - { - var referencedType = foreignKey.IsUnique - ? foreignKey.DeclaringEntityType.Name - : "ICollection<" + foreignKey.DeclaringEntityType.Name + ">"; - var principalEndNavPropConfiguration = - _configurationFactory.CreateNavigationPropertyConfiguration( - referencedType, - foreignKey.Scaffolding().PrincipalEndNavigation); - principalEndNavPropConfiguration.AttributeConfigurations.Add( - _configurationFactory.CreateAttributeConfiguration( - nameof(InversePropertyAttribute), - CSharpUtilities.DelimitString( - foreignKey.Scaffolding().DependentEndNavigation))); entityConfiguration.NavigationPropertyConfigurations.Add( - principalEndNavPropConfiguration); + dependentEndNavPropConfiguration); + + // set up the other navigation property for self-referencing foreign keys owned by this EntityType + if (((ForeignKey)foreignKey).IsSelfReferencing() && principalNavProp != null) + { + var referencedType = foreignKey.IsUnique + ? foreignKey.DeclaringEntityType.Name + : "ICollection<" + foreignKey.DeclaringEntityType.Name + ">"; + var principalEndNavPropConfiguration = + _configurationFactory.CreateNavigationPropertyConfiguration( + referencedType, principalNavProp.Name); + principalEndNavPropConfiguration.AttributeConfigurations.Add( + _configurationFactory.CreateAttributeConfiguration( + nameof(InversePropertyAttribute), + CSharpUtilities.DelimitString(dependentNavProp.Name))); + entityConfiguration.NavigationPropertyConfigurations.Add( + principalEndNavPropConfiguration); + } } } } - public virtual void AddNavigationPropertyConfiguration( - [NotNull] NavigationPropertyConfiguration navigationPropertyConfiguration) - { - } - public virtual void AddNavigationPropertyInitializers([NotNull] EntityConfiguration entityConfiguration) { Check.NotNull(entityConfiguration, nameof(entityConfiguration)); @@ -656,13 +656,12 @@ public virtual void AddNavigationPropertyInitializers([NotNull] EntityConfigurat foreach (var foreignKey in otherEntityType .GetForeignKeys().Where(fk => fk.PrincipalEntityType == entityConfiguration.EntityType)) { - var navigationPropertyName = - foreignKey.Scaffolding().PrincipalEndNavigation; - if (!foreignKey.IsUnique) + var navigationProperty = foreignKey.PrincipalToDependent; + if (!foreignKey.IsUnique && navigationProperty != null) { entityConfiguration.NavigationPropertyInitializerConfigurations.Add( _configurationFactory.CreateNavigationPropertyInitializerConfiguration( - navigationPropertyName, otherEntityType.Name)); + navigationProperty.Name, otherEntityType.Name)); } } } @@ -674,10 +673,8 @@ public virtual void AddRelationshipConfiguration([NotNull] EntityConfiguration e foreach (var foreignKey in entityConfiguration.EntityType.GetForeignKeys()) { - var dependentEndNavigationPropertyName = - foreignKey.Scaffolding().DependentEndNavigation; - var principalEndNavigationPropertyName = - foreignKey.Scaffolding().PrincipalEndNavigation; + var dependentEndNavigationPropertyName = foreignKey.DependentToPrincipal.Name; + var principalEndNavigationPropertyName = foreignKey.PrincipalToDependent.Name; var relationshipConfiguration = _configurationFactory .CreateRelationshipConfiguration( diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/ForeignKey.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/ForeignKey.cs index 2b938a133ac..bf700153b85 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/ForeignKey.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/ForeignKey.cs @@ -214,7 +214,7 @@ private Navigation Navigation( DeclaringEntityType.Builder, PrincipalEntityType.Builder, oldNavigation.Name, - oldNavigation.PropertyInfo); + oldNavigation.PropertyInfo); } else { @@ -222,7 +222,7 @@ private Navigation Navigation( PrincipalEntityType.Builder, DeclaringEntityType.Builder, oldNavigation.Name, - oldNavigation.PropertyInfo); + oldNavigation.PropertyInfo); } } diff --git a/test/Microsoft.EntityFrameworkCore.Relational.Design.Tests/ScaffoldingMetadataExtenstionsTest.cs b/test/Microsoft.EntityFrameworkCore.Relational.Design.Tests/ScaffoldingMetadataExtenstionsTest.cs index 64e3dc5f69a..42a560099fc 100644 --- a/test/Microsoft.EntityFrameworkCore.Relational.Design.Tests/ScaffoldingMetadataExtenstionsTest.cs +++ b/test/Microsoft.EntityFrameworkCore.Relational.Design.Tests/ScaffoldingMetadataExtenstionsTest.cs @@ -25,33 +25,6 @@ public void It_adds_provider_method_names() Assert.Null(model.Scaffolding().UseProviderMethodName); } - [Fact] - public void It_adds_reads_nav_prop_names() - { - var modelBuilder = new ModelBuilder(new ConventionSet()); - IMutableForeignKey fk = null; - modelBuilder.Entity("A", b => - { - b.Property("Id"); - var key = b.HasKey("Id"); - var fkProp = b.Property("ParentId"); - fk = b.Metadata.AddForeignKey(fkProp.Metadata, key.Metadata, b.Metadata); - }); - - Assert.Null(fk.Scaffolding().DependentEndNavigation); - Assert.Null(fk.Scaffolding().PrincipalEndNavigation); - - fk.Scaffolding().PrincipalEndNavigation = "PrincipalEnd"; - fk.Scaffolding().DependentEndNavigation = "DependentEnd"; - Assert.Equal("PrincipalEnd", fk.Scaffolding().PrincipalEndNavigation); - Assert.Equal("DependentEnd", fk.Scaffolding().DependentEndNavigation); - - fk.Scaffolding().PrincipalEndNavigation = null; - fk.Scaffolding().DependentEndNavigation = null; - Assert.Null(fk.Scaffolding().DependentEndNavigation); - Assert.Null(fk.Scaffolding().PrincipalEndNavigation); - } - [Fact] public void It_sets_gets_entity_type_errors() {