From 2cfc19023e9f442603194ee89cee44ab0a9fcc6b Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 20 Feb 2020 17:06:33 -0800 Subject: [PATCH] Introduce KeylessAttribute Resolves #19246 Also related #19964 - Add KeylessAttribute in Abstractions packge - Add convention to detect KeylessAttribute and configure the entityType to be keyless - Scaffolding when using data annotations - Generate KeylessAttribute over entity class - Skip HasNoKey from fluent API configuration Tests: - Add tests for OwnedAttribute - Add tests for KeylessAttribute - Add test for scaffolding to verify behavior with data annotations --- src/EFCore.Abstractions/KeylessAttribute.cs | 15 +++ .../Internal/CSharpDbContextGenerator.cs | 7 +- .../Internal/CSharpEntityTypeGenerator.cs | 20 +++- .../ProviderConventionSetBuilder.cs | 1 + .../KeylessEntityTypeAttributeConvention.cs | 39 ++++++ .../Internal/CSharpEntityTypeGeneratorTest.cs | 111 ++++++++++++++++-- .../Internal/ModelCodeGeneratorTestBase.cs | 1 + .../EntityTypeAttributeConventionTest.cs | 110 +++++++++++++++++ .../PropertyAttributeConventionTest.cs | 12 +- 9 files changed, 297 insertions(+), 19 deletions(-) create mode 100644 src/EFCore.Abstractions/KeylessAttribute.cs create mode 100644 src/EFCore/Metadata/Conventions/KeylessEntityTypeAttributeConvention.cs diff --git a/src/EFCore.Abstractions/KeylessAttribute.cs b/src/EFCore.Abstractions/KeylessAttribute.cs new file mode 100644 index 00000000000..23675b983c7 --- /dev/null +++ b/src/EFCore.Abstractions/KeylessAttribute.cs @@ -0,0 +1,15 @@ +// 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; + +namespace Microsoft.EntityFrameworkCore +{ + /// + /// Marks a type as keyless entity. + /// + [AttributeUsage(AttributeTargets.Class)] + public sealed class KeylessAttribute : Attribute + { + } +} diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs index f071ed31970..75c5d8fdba8 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs @@ -457,9 +457,12 @@ private void GenerateKey(IKey key, IEntityType entityType, bool useDataAnnotatio { if (key == null) { - var line = new List { $".{nameof(EntityTypeBuilder.HasNoKey)}()" }; + if (!useDataAnnotations) + { + var line = new List { $".{nameof(EntityTypeBuilder.HasNoKey)}()" }; - AppendMultiLineFluentApi(entityType, line); + AppendMultiLineFluentApi(entityType, line); + } return; } diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs index 60f71bec2bf..58aeff299ee 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs @@ -66,6 +66,7 @@ public virtual string WriteCode(IEntityType entityType, string @namespace, bool { _sb.AppendLine("using System.ComponentModel.DataAnnotations;"); _sb.AppendLine("using System.ComponentModel.DataAnnotations.Schema;"); + _sb.AppendLine("using Microsoft.EntityFrameworkCore;"); // For attributes coming out of Abstractions } foreach (var ns in entityType.GetProperties() @@ -132,9 +133,18 @@ protected virtual void GenerateEntityTypeDataAnnotations( { Check.NotNull(entityType, nameof(entityType)); + GenerateKeylessAttribute(entityType); GenerateTableAttribute(entityType); } + private void GenerateKeylessAttribute(IEntityType entityType) + { + if (entityType.FindPrimaryKey() == null) + { + _sb.AppendLine(new AttributeWriter(nameof(KeylessAttribute))); + } + } + private void GenerateTableAttribute(IEntityType entityType) { var tableName = entityType.GetTableName(); @@ -156,7 +166,7 @@ private void GenerateTableAttribute(IEntityType entityType) tableAttribute.AddParameter($"{nameof(TableAttribute.Schema)} = {_code.Literal(schema)}"); } - _sb.AppendLine(tableAttribute.ToString()); + _sb.AppendLine(tableAttribute); } } @@ -278,7 +288,7 @@ private void GenerateMaxLengthAttribute(IProperty property) lengthAttribute.AddParameter(_code.Literal(maxLength.Value)); - _sb.AppendLine(lengthAttribute.ToString()); + _sb.AppendLine(lengthAttribute); } } @@ -288,7 +298,7 @@ private void GenerateRequiredAttribute(IProperty property) && property.ClrType.IsNullableType() && !property.IsPrimaryKey()) { - _sb.AppendLine(new AttributeWriter(nameof(RequiredAttribute)).ToString()); + _sb.AppendLine(new AttributeWriter(nameof(RequiredAttribute))); } } @@ -351,7 +361,7 @@ private void GenerateForeignKeyAttribute(INavigation navigation) foreignKeyAttribute.AddParameter($"nameof({navigation.ForeignKey.Properties.First().Name})"); } - _sb.AppendLine(foreignKeyAttribute.ToString()); + _sb.AppendLine(foreignKeyAttribute); } } } @@ -372,7 +382,7 @@ private void GenerateInversePropertyAttribute(INavigation navigation) ? $"nameof({inverseNavigation.DeclaringEntityType.Name}.{inverseNavigation.Name})" : _code.Literal(inverseNavigation.Name)); - _sb.AppendLine(inversePropertyAttribute.ToString()); + _sb.AppendLine(inversePropertyAttribute); } } } diff --git a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs index ea3ffde0637..592beb830e7 100644 --- a/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs +++ b/src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs @@ -64,6 +64,7 @@ public virtual ConventionSet CreateConventionSet() conventionSet.EntityTypeAddedConventions.Add(new NotMappedEntityTypeAttributeConvention(Dependencies)); conventionSet.EntityTypeAddedConventions.Add(new OwnedEntityTypeAttributeConvention(Dependencies)); + conventionSet.EntityTypeAddedConventions.Add(new KeylessEntityTypeAttributeConvention(Dependencies)); conventionSet.EntityTypeAddedConventions.Add(new NotMappedMemberAttributeConvention(Dependencies)); conventionSet.EntityTypeAddedConventions.Add(new BaseTypeDiscoveryConvention(Dependencies)); conventionSet.EntityTypeAddedConventions.Add(propertyDiscoveryConvention); diff --git a/src/EFCore/Metadata/Conventions/KeylessEntityTypeAttributeConvention.cs b/src/EFCore/Metadata/Conventions/KeylessEntityTypeAttributeConvention.cs new file mode 100644 index 00000000000..979dd4215ad --- /dev/null +++ b/src/EFCore/Metadata/Conventions/KeylessEntityTypeAttributeConvention.cs @@ -0,0 +1,39 @@ +// 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.ComponentModel.DataAnnotations.Schema; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; + +namespace Microsoft.EntityFrameworkCore.Metadata.Conventions +{ + /// + /// A convention that ignores entity types that have the . + /// + public class KeylessEntityTypeAttributeConvention : EntityTypeAttributeConventionBase + { + /// + /// Creates a new instance of . + /// + /// Parameter object containing dependencies for this convention. + public KeylessEntityTypeAttributeConvention([NotNull] ProviderConventionSetBuilderDependencies dependencies) + : base(dependencies) + { + } + + /// + /// Called after an entity type is added to the model if it has an attribute. + /// + /// The builder for the entity type. + /// The attribute. + /// Additional information associated with convention execution. + protected override void ProcessEntityTypeAdded( + IConventionEntityTypeBuilder entityTypeBuilder, + KeylessAttribute attribute, + IConventionContext context) + { + entityTypeBuilder.HasNoKey(fromDataAnnotation: true); + } + } +} diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs index b2d8426aa8a..23723b110b0 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs @@ -37,6 +37,7 @@ public void Navigation_properties() using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; namespace TestNamespace { @@ -58,7 +59,7 @@ public Post() } } ", - postFile.Code); + postFile.Code, ignoreLineEndingDifferences: true); }, model => { @@ -97,6 +98,7 @@ public void Navigation_property_with_same_type_and_navigation_name() using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; namespace TestNamespace { @@ -112,7 +114,7 @@ public partial class Post } } ", - postFile.Code); + postFile.Code, ignoreLineEndingDifferences: true); }, model => { @@ -152,6 +154,7 @@ public void Navigation_property_with_same_type_and_property_name() using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; namespace TestNamespace { @@ -167,7 +170,7 @@ public partial class Post } } ", - postFile.Code); + postFile.Code, ignoreLineEndingDifferences: true); }, model => { @@ -208,6 +211,7 @@ public void Navigation_property_with_same_type_and_other_navigation_name() using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; namespace TestNamespace { @@ -227,7 +231,7 @@ public partial class Post } } ", - postFile.Code); + postFile.Code, ignoreLineEndingDifferences: true); }, model => { @@ -275,6 +279,7 @@ public void Composite_key() using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; namespace TestNamespace { @@ -287,7 +292,7 @@ public partial class Post } } ", - postFile.Code); + postFile.Code, ignoreLineEndingDifferences: true); }, model => { @@ -302,9 +307,26 @@ public void Views_dont_generate_TableAttribute() Test( modelBuilder => modelBuilder.Entity("Vista").ToView("Vistas", "dbo"), new ModelCodeGenerationOptions { UseDataAnnotations = true }, - code => Assert.DoesNotContain( - "[Table(", - code.AdditionalFiles.First(f => f.Path == "Vista.cs").Code), + code => + { + var vistaFile = code.AdditionalFiles.First(f => f.Path == "Vista.cs"); + Assert.Equal( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace +{ + [Keyless] + public partial class Vista + { + } +} +", + vistaFile.Code, ignoreLineEndingDifferences: true); + }, model => { var entityType = model.FindEntityType("TestNamespace.Vista"); @@ -312,5 +334,78 @@ public void Views_dont_generate_TableAttribute() Assert.Equal("dbo", entityType.GetSchema()); }); } + + [ConditionalFact] + public void Keyless_entity_generates_KeylesssAttribute() + { + Test( + modelBuilder => modelBuilder.Entity("Vista").HasNoKey(), + new ModelCodeGenerationOptions { UseDataAnnotations = true }, + code => + { + var vistaFile = code.AdditionalFiles.First(f => f.Path == "Vista.cs"); + Assert.Equal( + @"using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore; + +namespace TestNamespace +{ + [Keyless] + public partial class Vista + { + } +} +", + vistaFile.Code, ignoreLineEndingDifferences: true); + + Assert.Equal( + @"using System; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace TestNamespace +{ + public partial class TestDbContext : DbContext + { + public TestDbContext() + { + } + + public TestDbContext(DbContextOptions options) + : base(options) + { + } + + public virtual DbSet Vista { get; set; } + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + if (!optionsBuilder.IsConfigured) + { +#warning To protect potentially sensitive information in your connection string, you should move it out of source code. See http://go.microsoft.com/fwlink/?LinkId=723263 for guidance on storing connection strings. + optionsBuilder.UseSqlServer(""Initial Catalog=TestDatabase""); + } + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + OnModelCreatingPartial(modelBuilder); + } + + partial void OnModelCreatingPartial(ModelBuilder modelBuilder); + } +} +", + code.ContextFile.Code, ignoreLineEndingDifferences: true); + }, + model => + { + var entityType = model.FindEntityType("TestNamespace.Vista"); + Assert.Null(entityType.FindPrimaryKey()); + }); + } } } diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/ModelCodeGeneratorTestBase.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/ModelCodeGeneratorTestBase.cs index b0a87de75c3..e0bc1500fe8 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/ModelCodeGeneratorTestBase.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/ModelCodeGeneratorTestBase.cs @@ -49,6 +49,7 @@ protected void Test( { References = { + BuildReference.ByName("Microsoft.EntityFrameworkCore.Abstractions"), BuildReference.ByName("Microsoft.EntityFrameworkCore"), BuildReference.ByName("Microsoft.EntityFrameworkCore.Relational"), BuildReference.ByName("Microsoft.EntityFrameworkCore.SqlServer") diff --git a/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs index ef453a8df7b..377f35125e7 100644 --- a/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/EntityTypeAttributeConventionTest.cs @@ -1,7 +1,12 @@ // 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; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; +using System.Linq; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal; @@ -54,12 +59,91 @@ public void NotMappedAttribute_ignores_entityTypes_with_conventional_builder() #endregion + #region OwnedAttribute + + [ConditionalFact] + public void OwnedAttribute_configures_entity_as_owned() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + modelBuilder.Entity(); + + Assert.Equal(2, modelBuilder.Model.GetEntityTypes().Count()); + Assert.True(modelBuilder.Model.FindEntityType(typeof(Customer)).FindNavigation(nameof(Customer.Address)).ForeignKey.IsOwnership); + } + + [ConditionalFact] + public void Entity_marked_with_OwnedAttribute_cannot_be_configured_as_regular_entity() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + Assert.Equal( + CoreStrings.ClashingOwnedEntityType(nameof(Address)), + Assert.Throws( + () => modelBuilder.Entity().HasOne(e => e.Address).WithOne(e => e.Customer)).Message); + } + + #endregion + + #region KeylessAttribute + + [ConditionalFact] + public void KeylessAttribute_overrides_configuration_from_convention() + { + var modelBuilder = new InternalModelBuilder(new Model()); + + var entityBuilder = modelBuilder.Entity(typeof(KeylessEntity), ConfigurationSource.Convention); + entityBuilder.Property("Id", ConfigurationSource.Convention); + entityBuilder.PrimaryKey(new List { "Id" }, ConfigurationSource.Convention); + + Assert.NotNull(entityBuilder.Metadata.FindPrimaryKey()); + + RunConvention(entityBuilder); + + Assert.Null(entityBuilder.Metadata.FindPrimaryKey()); + Assert.True(entityBuilder.Metadata.IsKeyless); + } + + [ConditionalFact] + public void KeylessAttribute_can_be_overriden_using_explicit_configuration() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + var entityBuilder = modelBuilder.Entity(); + + Assert.True(entityBuilder.Metadata.IsKeyless); + + entityBuilder.HasKey(e => e.Id); + + Assert.False(entityBuilder.Metadata.IsKeyless); + Assert.NotNull(entityBuilder.Metadata.FindPrimaryKey()); + } + + [ConditionalFact] + public void KeyAttribute_overrides_keyless_attribute() + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + + var entityBuilder = modelBuilder.Entity(); + + Assert.False(entityBuilder.Metadata.IsKeyless); + Assert.NotNull(entityBuilder.Metadata.FindPrimaryKey()); + } + + #endregion + private void RunConvention(InternalEntityTypeBuilder entityTypeBuilder) { var context = new ConventionContext(entityTypeBuilder.Metadata.Model.ConventionDispatcher); new NotMappedEntityTypeAttributeConvention(CreateDependencies()) .ProcessEntityTypeAdded(entityTypeBuilder, context); + + new OwnedEntityTypeAttributeConvention(CreateDependencies()) + .ProcessEntityTypeAdded(entityTypeBuilder, context); + + new KeylessEntityTypeAttributeConvention(CreateDependencies()) + .ProcessEntityTypeAdded(entityTypeBuilder, context); } private ProviderConventionSetBuilderDependencies CreateDependencies() @@ -79,5 +163,31 @@ private class B public virtual A NavToA { get; set; } } + + private class Customer + { + public int Id { get; set; } + public Address Address { get; set; } + } + + [Owned] + private class Address + { + public int Id { get; set; } + public Customer Customer { get; set; } + } + + [Keyless] + private class KeylessEntity + { + public int Id { get; set; } + } + + [Keyless] + private class KeyClash + { + [Key] + public int MyId { get; set; } + } } } diff --git a/test/EFCore.Tests/Metadata/Conventions/PropertyAttributeConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/PropertyAttributeConventionTest.cs index 537aec09ff6..fefead75d40 100644 --- a/test/EFCore.Tests/Metadata/Conventions/PropertyAttributeConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/PropertyAttributeConventionTest.cs @@ -509,6 +509,10 @@ public void TimestampAttribute_on_field_sets_concurrency_token_with_conventional Assert.True(entityTypeBuilder.Property(nameof(F.Timestamp)).Metadata.IsConcurrencyToken); } + #endregion + + #region BackingFieldAttribute + [ConditionalFact] public void BackingFieldAttribute_overrides_configuration_from_convention_source() { @@ -574,15 +578,12 @@ private static void RunConvention(InternalPropertyBuilder propertyBuilder) new DatabaseGeneratedAttributeConvention(dependencies) .ProcessPropertyAdded(propertyBuilder, context); - new KeyAttributeConvention(dependencies) + new RequiredPropertyAttributeConvention(dependencies) .ProcessPropertyAdded(propertyBuilder, context); new MaxLengthAttributeConvention(dependencies) .ProcessPropertyAdded(propertyBuilder, context); - new RequiredPropertyAttributeConvention(dependencies) - .ProcessPropertyAdded(propertyBuilder, context); - new StringLengthAttributeConvention(dependencies) .ProcessPropertyAdded(propertyBuilder, context); @@ -591,6 +592,9 @@ private static void RunConvention(InternalPropertyBuilder propertyBuilder) new BackingFieldAttributeConvention(dependencies) .ProcessPropertyAdded(propertyBuilder, context); + + new KeyAttributeConvention(dependencies) + .ProcessPropertyAdded(propertyBuilder, context); } private void RunConvention(InternalEntityTypeBuilder entityTypeBuilder)