diff --git a/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationFixtureBase.cs b/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationFixtureBase.cs index 428382e8958..de4fe849413 100644 --- a/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationFixtureBase.cs +++ b/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationFixtureBase.cs @@ -4,6 +4,7 @@ using System; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; +using Microsoft.EntityFrameworkCore.Internal; namespace Microsoft.EntityFrameworkCore.Specification.Tests { @@ -14,6 +15,8 @@ public abstract class DataAnnotationFixtureBase public abstract DataAnnotationContext CreateContext(TTestStore testStore); + public abstract ModelValidator ThrowingValidator { get; } + protected virtual void OnModelCreating(ModelBuilder modelBuilder) { } diff --git a/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationTestBase.cs b/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationTestBase.cs index 2cd4ef87184..d3a24baaca6 100644 --- a/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationTestBase.cs +++ b/src/Microsoft.EntityFrameworkCore.Specification.Tests/DataAnnotationTestBase.cs @@ -2,7 +2,13 @@ // 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.Linq; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal; +using Microsoft.EntityFrameworkCore.Storage; using Xunit; namespace Microsoft.EntityFrameworkCore.Specification.Tests @@ -25,6 +31,53 @@ protected DataAnnotationTestBase(TFixture fixture) protected TTestStore TestStore { get; } public void Dispose() => TestStore.Dispose(); + + public virtual ModelBuilder CreateModelBuilder() + { + var context = CreateContext(); + var conventionSetBuilder = context.GetService().ConventionSetBuilder; + var conventionSet = context.GetService().CreateConventionSet(); + conventionSet = conventionSetBuilder == null + ? conventionSet + : conventionSetBuilder.AddConventions(conventionSet); + return new ModelBuilder(conventionSet); + } + + protected virtual void Validate(IModel model) => Fixture.ThrowingValidator.Validate(model); + + [Fact] + public virtual void Key_from_base_type_is_recognized_if_discovered_through_relationship() + { + var builder = CreateModelBuilder(); + + builder.Entity(); + + Validate(builder.Model); + Assert.True(GetProperty(builder, nameof(OKeyBase.OrderLineNo)).IsPrimaryKey()); + Assert.True(GetProperty(builder, nameof(DODerived.OrderLineNo)).IsPrimaryKey()); + } + + protected class SRelated + { + public int SRelatedId { get; set; } + + public ICollection OKeyBases { get; set; } + public ICollection DADeriveds { get; set; } + } + + protected class OKeyBase + { + [Key] + public int OrderLineNo { get; set; } + + public int Quantity { get; set; } + } + + protected class DODerived : OKeyBase + { + public SRelated DARelated { get; set; } + public string Special { get; set; } + } [Fact] public virtual void ConcurrencyCheckAttribute_throws_if_value_in_database_changed() @@ -193,5 +246,8 @@ public virtual void TimestampAttribute_throws_if_value_in_database_changed() Assert.Throws(() => context.SaveChanges()); } } + + protected static IMutableProperty GetProperty(ModelBuilder modelBuilder, string name) + => modelBuilder.Model.FindEntityType(typeof(TEntity)).FindProperty(name); } } diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/KeyAttributeConvention.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/KeyAttributeConvention.cs index 6eca63473b2..1b7ccfa60dd 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/KeyAttributeConvention.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/KeyAttributeConvention.cs @@ -30,7 +30,7 @@ public override InternalPropertyBuilder Apply(InternalPropertyBuilder propertyBu var entityType = propertyBuilder.Metadata.DeclaringEntityType; if (entityType.BaseType != null) { - throw new InvalidOperationException(CoreStrings.KeyAttributeOnDerivedEntity(entityType.DisplayName(), propertyBuilder.Metadata.Name)); + return propertyBuilder; } var entityTypeBuilder = entityType.Builder; @@ -64,20 +64,33 @@ public override InternalPropertyBuilder Apply(InternalPropertyBuilder propertyBu public virtual InternalModelBuilder Apply(InternalModelBuilder modelBuilder) { var entityTypes = modelBuilder.Metadata.GetEntityTypes(); - foreach (var entityType in entityTypes.Where(et => et.BaseType == null)) + foreach (var entityType in entityTypes) { - var currentPrimaryKey = entityType.FindPrimaryKey(); - if ((currentPrimaryKey != null) - && (currentPrimaryKey.Properties.Count > 1)) + if (entityType.BaseType == null) { - var newKey = entityType.Builder.PrimaryKey( - new List { currentPrimaryKey.Properties.First().Name }, ConfigurationSource.DataAnnotation); - if (newKey != null) + var currentPrimaryKey = entityType.FindPrimaryKey(); + if (currentPrimaryKey != null + && currentPrimaryKey.Properties.Count > 1 + && entityType.GetPrimaryKeyConfigurationSource() == ConfigurationSource.DataAnnotation) { throw new InvalidOperationException(CoreStrings.CompositePKWithDataAnnotation(entityType.DisplayName())); } } + else + { + foreach (var declaredProperty in entityType.GetDeclaredProperties()) + { + var propertyInfo = declaredProperty.PropertyInfo; + var attributes = propertyInfo?.GetCustomAttributes(true); + if (attributes?.Any() == true) + { + throw new InvalidOperationException( + CoreStrings.KeyAttributeOnDerivedEntity(entityType.DisplayName(), declaredProperty.Name)); + } + } + } } + return modelBuilder; } } diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/PropertyAttributeConvention.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/PropertyAttributeConvention.cs index 019c2b58380..b3d099ffbb8 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/PropertyAttributeConvention.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/PropertyAttributeConvention.cs @@ -25,10 +25,7 @@ public virtual InternalPropertyBuilder Apply(InternalPropertyBuilder propertyBui { Check.NotNull(propertyBuilder, nameof(propertyBuilder)); - var clrType = propertyBuilder.Metadata.DeclaringEntityType.ClrType; - var propertyName = propertyBuilder.Metadata.Name; - - var propertyInfo = clrType?.GetRuntimeProperties().FirstOrDefault(pi => pi.Name == propertyName); + var propertyInfo = propertyBuilder.Metadata.PropertyInfo; var attributes = propertyInfo?.GetCustomAttributes(true); if (attributes != null) { diff --git a/test/Microsoft.EntityFrameworkCore.InMemory.FunctionalTests/DataAnnotationInMemoryFixture.cs b/test/Microsoft.EntityFrameworkCore.InMemory.FunctionalTests/DataAnnotationInMemoryFixture.cs index cf6601b7bac..b8df9321579 100644 --- a/test/Microsoft.EntityFrameworkCore.InMemory.FunctionalTests/DataAnnotationInMemoryFixture.cs +++ b/test/Microsoft.EntityFrameworkCore.InMemory.FunctionalTests/DataAnnotationInMemoryFixture.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 Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Specification.Tests; using Microsoft.Extensions.DependencyInjection; @@ -18,9 +19,22 @@ public DataAnnotationInMemoryFixture() _serviceProvider = new ServiceCollection() .AddEntityFrameworkInMemoryDatabase() .AddSingleton(TestInMemoryModelSource.GetFactory(OnModelCreating)) + .AddSingleton() .BuildServiceProvider(); } + public override ModelValidator ThrowingValidator + => _serviceProvider.GetService(); + + // ReSharper disable once ClassNeverInstantiated.Local + private class ThrowingModelValidator : ModelValidator + { + protected override void ShowWarning(string message) + { + throw new InvalidOperationException(message); + } + } + public override InMemoryTestStore CreateTestStore() { return InMemoryTestStore.GetOrCreateShared(DatabaseName, () => diff --git a/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/DataAnnotationSqlServerFixture.cs b/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/DataAnnotationSqlServerFixture.cs index 14fe051f82a..788d3518374 100644 --- a/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/DataAnnotationSqlServerFixture.cs +++ b/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/DataAnnotationSqlServerFixture.cs @@ -2,8 +2,12 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.EntityFrameworkCore.Internal; +using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Specification.Tests; using Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests.Utilities; +using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.EntityFrameworkCore.Storage.Internal; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -26,6 +30,28 @@ public DataAnnotationSqlServerFixture() .BuildServiceProvider(); } + public override ModelValidator ThrowingValidator + => new ThrowingModelValidator( + _serviceProvider.GetService>(), + new SqlServerAnnotationProvider(), + new SqlServerTypeMapper()); + + private class ThrowingModelValidator : RelationalModelValidator + { + public ThrowingModelValidator( + ILogger loggerFactory, + IRelationalAnnotationProvider relationalExtensions, + IRelationalTypeMapper typeMapper) + : base(loggerFactory, relationalExtensions, typeMapper) + { + } + + protected override void ShowWarning(string message) + { + throw new InvalidOperationException(message); + } + } + public override SqlServerTestStore CreateTestStore() { return SqlServerTestStore.GetOrCreateShared(DatabaseName, () => diff --git a/test/Microsoft.EntityFrameworkCore.Sqlite.FunctionalTests/DataAnnotationSqliteFixture.cs b/test/Microsoft.EntityFrameworkCore.Sqlite.FunctionalTests/DataAnnotationSqliteFixture.cs index 24291509e63..720fac2979b 100644 --- a/test/Microsoft.EntityFrameworkCore.Sqlite.FunctionalTests/DataAnnotationSqliteFixture.cs +++ b/test/Microsoft.EntityFrameworkCore.Sqlite.FunctionalTests/DataAnnotationSqliteFixture.cs @@ -2,7 +2,12 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.EntityFrameworkCore.Internal; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Specification.Tests; +using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.EntityFrameworkCore.Storage.Internal; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -25,6 +30,28 @@ public DataAnnotationSqliteFixture() .BuildServiceProvider(); } + public override ModelValidator ThrowingValidator + => new ThrowingModelValidator( + _serviceProvider.GetService>(), + new SqliteAnnotationProvider(), + new SqliteTypeMapper()); + + private class ThrowingModelValidator : RelationalModelValidator + { + public ThrowingModelValidator( + ILogger loggerFactory, + IRelationalAnnotationProvider relationalExtensions, + IRelationalTypeMapper typeMapper) + : base(loggerFactory, relationalExtensions, typeMapper) + { + } + + protected override void ShowWarning(string message) + { + throw new InvalidOperationException(message); + } + } + public override SqliteTestStore CreateTestStore() => SqliteTestStore.GetOrCreateShared(DatabaseName, () => { diff --git a/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Conventions/Internal/PropertyAttributeConventionTest.cs b/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Conventions/Internal/PropertyAttributeConventionTest.cs index 36bdcffd070..09c179c74e5 100644 --- a/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Conventions/Internal/PropertyAttributeConventionTest.cs +++ b/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Conventions/Internal/PropertyAttributeConventionTest.cs @@ -192,7 +192,7 @@ public void KeyAttribute_throws_when_setting_key_in_derived_type() Assert.Equal( CoreStrings.KeyAttributeOnDerivedEntity(derivedEntityTypeBuilder.Metadata.DisplayName(), propertyBuilder.Metadata.Name), - Assert.Throws(() => new KeyAttributeConvention().Apply(propertyBuilder)).Message); + Assert.Throws(() => new KeyAttributeConvention().Apply(derivedEntityTypeBuilder.ModelBuilder)).Message); } [Fact]