Added a ModelBuilder.FinalizeModel method to support building outside…

… the context Fixes #11738 Naming to be discussed. This runs the model-based conventions and model validation. Also moved setting or product version annotation to any use of the ModelBuilder and then removed from places in the tests that this would cause problems when the version changes.
dotnet · Oct 3, 2018 · de17e6c · de17e6c
1 parent c4b8eaf
commit de17e6c
Show file tree

Hide file tree

Showing 22 changed files with 210 additions and 72 deletions.
diff --git a/samples/OracleProvider/test/OracleProvider.FunctionalTests/F1OracleFixture.cs b/samples/OracleProvider/test/OracleProvider.FunctionalTests/F1OracleFixture.cs
@@ -1,6 +1,7 @@
 // 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 Microsoft.EntityFrameworkCore.Metadata.Conventions;
 using Microsoft.EntityFrameworkCore.TestModels.ConcurrencyModel;
 using Microsoft.EntityFrameworkCore.TestUtilities;
 
@@ -10,9 +11,12 @@ public class F1OracleFixture : F1RelationalFixture
     {
         protected override ITestStoreFactory TestStoreFactory => OracleTestStoreFactory.Instance;
 
-        protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+        public override ModelBuilder CreateModelBuilder()
+            => new ModelBuilder(OracleConventionSetBuilder.Build());
+
+        protected override void BuildModelExternal(ModelBuilder modelBuilder)
         {
-            base.OnModelCreating(modelBuilder, context);
+            base.BuildModelExternal(modelBuilder);
 
             modelBuilder.Entity<Chassis>().Property<byte[]>("Version").IsRowVersion();
             modelBuilder.Entity<Driver>().Property<byte[]>("Version").IsRowVersion();

diff --git a/src/EFCore.InMemory/Metadata/Conventions/Internal/InMemoryConventionSetBuilder.cs b/src/EFCore.InMemory/Metadata/Conventions/Internal/InMemoryConventionSetBuilder.cs
@@ -1,8 +1,10 @@
 // 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 Microsoft.EntityFrameworkCore.Metadata.Conventions;
 using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
+using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.EntityFrameworkCore.InMemory.Metadata.Conventions.Internal
 {
@@ -17,5 +19,25 @@ public class InMemoryConventionSetBuilder : IConventionSetBuilder
         ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public virtual ConventionSet AddConventions(ConventionSet conventionSet) => conventionSet;
+
+        /// <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 static ConventionSet Build()
+        {
+            var serviceProvider = new ServiceCollection()
+                .AddEntityFrameworkInMemoryDatabase()
+                .AddDbContext<DbContext>(o => o.UseInMemoryDatabase(Guid.NewGuid().ToString()))
+                .BuildServiceProvider();
+
+            using (var serviceScope = serviceProvider.GetRequiredService<IServiceScopeFactory>().CreateScope())
+            {
+                using (var context = serviceScope.ServiceProvider.GetService<DbContext>())
+                {
+                    return ConventionSet.CreateConventionSet(context);
+                }
+            }
+        }
     }
 }
diff --git a/src/EFCore.Relational.Specification.Tests/F1RelationalFixture.cs b/src/EFCore.Relational.Specification.Tests/F1RelationalFixture.cs
@@ -15,9 +15,9 @@ public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder build
             => base.AddOptions(builder).ConfigureWarnings(w =>
                 w.Ignore(RelationalEventId.BatchSmallerThanMinBatchSize));
 
-        protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+        protected override void BuildModelExternal(ModelBuilder modelBuilder)
         {
-            base.OnModelCreating(modelBuilder, context);
+            base.BuildModelExternal(modelBuilder);
 
             modelBuilder.Entity<Chassis>().ToTable("Chassis");
             modelBuilder.Entity<Team>().ToTable("Teams").Property(e => e.Id).ValueGeneratedNever();

diff --git a/src/EFCore.Relational.Specification.Tests/MigrationSqlGeneratorTestBase.cs b/src/EFCore.Relational.Specification.Tests/MigrationSqlGeneratorTestBase.cs
@@ -725,6 +725,7 @@ protected virtual void Generate(params MigrationOperation[] operation)
         protected virtual void Generate(Action<ModelBuilder> buildAction, params MigrationOperation[] operation)
         {
             var modelBuilder = TestHelpers.CreateConventionBuilder();
+            modelBuilder.Model.RemoveAnnotation(CoreAnnotationNames.ProductVersionAnnotation);
             buildAction(modelBuilder);
 
             var batch = TestHelpers.CreateContextServices().GetRequiredService<IMigrationsSqlGenerator>()

diff --git a/src/EFCore.Specification.Tests/DataAnnotationTestBase.cs b/src/EFCore.Specification.Tests/DataAnnotationTestBase.cs
@@ -48,21 +48,22 @@ public virtual ModelBuilder CreateModelBuilder()
             var conventionSetBuilder = CreateConventionSetBuilder(context);
             var conventionSet = new CoreConventionSetBuilder(context.GetService<CoreConventionSetBuilderDependencies>())
                 .CreateConventionSet();
+
             conventionSet = conventionSetBuilder == null
                 ? conventionSet
                 : conventionSetBuilder.AddConventions(conventionSet);
+
+            conventionSet.ModelBuiltConventions.Add(
+                new ValidatingConvention(context.GetService<IModelValidator>()));
+
             return new ModelBuilder(conventionSet);
         }
 
         protected virtual IConventionSetBuilder CreateConventionSetBuilder(DbContext context)
             => new CompositeConventionSetBuilder(context.GetService<IEnumerable<IConventionSetBuilder>>().ToList());
 
         protected virtual void Validate(ModelBuilder modelBuilder)
-        {
-            modelBuilder.GetInfrastructure().Metadata.Validate();
-            var context = CreateContext();
-            context.GetService<IModelValidator>().Validate(modelBuilder.Model);
-        }
+            => modelBuilder.FinalizeModel();
 
         protected class Person
         {

diff --git a/src/EFCore.Specification.Tests/F1FixtureBase.cs b/src/EFCore.Specification.Tests/F1FixtureBase.cs
@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using Microsoft.EntityFrameworkCore.Diagnostics;
+using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.TestModels.ConcurrencyModel;
 
 namespace Microsoft.EntityFrameworkCore
@@ -13,13 +14,30 @@ public abstract class F1FixtureBase : SharedStoreFixtureBase<F1Context>
         protected override bool UsePooling => true;
 
         public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
-            => base.AddOptions(builder).ConfigureWarnings(w =>
-                w.Ignore(CoreEventId.SaveChangesStarting, CoreEventId.SaveChangesCompleted));
+            => base.AddOptions(builder)
+                .UseModel(CreateModelExternal())
+                .ConfigureWarnings(
+                    w => w.Ignore(CoreEventId.SaveChangesStarting, CoreEventId.SaveChangesCompleted));
 
         protected override bool ShouldLogCategory(string logCategory)
             => logCategory == DbLoggerCategory.Update.Name;
 
-        protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+        private IModel CreateModelExternal()
+        {
+            // Doing this differently here from other tests to have regression coverage for
+            // building models externally from the context instance.
+            var builder = CreateModelBuilder();
+
+            BuildModelExternal(builder);
+
+            builder.FinalizeModel();
+
+            return builder.Model;
+        }
+
+        public abstract ModelBuilder CreateModelBuilder();
+
+        protected virtual void BuildModelExternal(ModelBuilder modelBuilder)
         {
             modelBuilder.Entity<Chassis>(b => { b.HasKey(c => c.TeamId); });
 

diff --git a/src/EFCore.Specification.Tests/OptimisticConcurrencyTestBase.cs b/src/EFCore.Specification.Tests/OptimisticConcurrencyTestBase.cs
@@ -7,6 +7,7 @@
 using Microsoft.EntityFrameworkCore.ChangeTracking;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.TestModels.ConcurrencyModel;
 using Microsoft.EntityFrameworkCore.TestUtilities.Xunit;
@@ -28,6 +29,18 @@ protected OptimisticConcurrencyTestBase(TFixture fixture)
 
         protected TFixture Fixture { get; }
 
+        [Fact]
+        public virtual void External_model_builder_uses_validation()
+        {
+            var modelBuilder = Fixture.CreateModelBuilder();
+            modelBuilder.Entity("Dummy");
+
+            Assert.Equal(
+                CoreStrings.ShadowEntity("Dummy"),
+                Assert.Throws<InvalidOperationException>
+                    (() => modelBuilder.FinalizeModel()).Message);
+        }
+
         [Fact]
         public virtual void Nullable_client_side_concurrency_token_can_be_used()
         {

diff --git a/src/EFCore.Specification.Tests/TestUtilities/TestModelSource.cs b/src/EFCore.Specification.Tests/TestUtilities/TestModelSource.cs
@@ -3,7 +3,6 @@
 
 using System;
 using Microsoft.EntityFrameworkCore.Infrastructure;
-using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
@@ -24,20 +23,17 @@ private TestModelSource(Action<ModelBuilder, DbContext> onModelCreating, ModelSo
         protected override IModel CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
         {
             var conventionSet = CreateConventionSet(conventionSetBuilder);
+            conventionSet.ModelBuiltConventions.Add(new ValidatingConvention(validator));
 
             var modelBuilder = new ModelBuilder(conventionSet);
-            var model = modelBuilder.GetInfrastructure().Metadata;
-            model.SetProductVersion(ProductInfo.GetVersion());
 
             Dependencies.ModelCustomizer.Customize(modelBuilder, context);
 
             _onModelCreating(modelBuilder, context);
 
-            model.Validate();
+            modelBuilder.FinalizeModel();
 
-            validator.Validate(model);
-
-            return model;
+            return modelBuilder.GetInfrastructure().Metadata;
         }
 
         public static Func<IServiceProvider, IModelSource> GetFactory(Action<ModelBuilder> onModelCreating)

diff --git a/src/EFCore/Infrastructure/ModelSource.cs b/src/EFCore/Infrastructure/ModelSource.cs
@@ -5,7 +5,6 @@
 using System.Collections.Concurrent;
 using System.Threading;
 using JetBrains.Annotations;
-using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Conventions;
 using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
@@ -76,18 +75,15 @@ protected virtual IModel CreateModel(
             Check.NotNull(validator, nameof(validator));
 
             var conventionSet = CreateConventionSet(conventionSetBuilder);
+            conventionSet.ModelBuiltConventions.Add(new ValidatingConvention(validator));
 
             var modelBuilder = new ModelBuilder(conventionSet);
-            var model = modelBuilder.GetInfrastructure().Metadata;
-            model.SetProductVersion(ProductInfo.GetVersion());
 
             Dependencies.ModelCustomizer.Customize(modelBuilder, context);
 
-            model.Validate();
+            modelBuilder.FinalizeModel();
 
-            validator.Validate(model);
-
-            return model;
+            return modelBuilder.GetInfrastructure().Metadata;
         }
 
         /// <summary>

diff --git a/src/EFCore/Metadata/Conventions/ConventionSet.cs b/src/EFCore/Metadata/Conventions/ConventionSet.cs
@@ -162,7 +162,15 @@ public class ConventionSet
         ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         public static ConventionSet CreateConventionSet([NotNull] DbContext context)
-            => new CompositeConventionSetBuilder(context.GetService<IEnumerable<IConventionSetBuilder>>().ToList())
-                .AddConventions(context.GetService<ICoreConventionSetBuilder>().CreateConventionSet());
+        {
+            var conventionSet = new CompositeConventionSetBuilder(
+                    context.GetService<IEnumerable<IConventionSetBuilder>>().ToList())
+                .AddConventions(
+                    context.GetService<ICoreConventionSetBuilder>().CreateConventionSet());
+
+            conventionSet.ModelBuiltConventions.Add(new ValidatingConvention(context.GetService<IModelValidator>()));
+
+            return conventionSet;
+        }
     }
 }
diff --git a/src/EFCore/Metadata/Conventions/Internal/ValidatingConvention.cs b/src/EFCore/Metadata/Conventions/Internal/ValidatingConvention.cs
@@ -0,0 +1,38 @@
+// 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.Infrastructure;
+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 class ValidatingConvention : IModelBuiltConvention
+    {
+        private readonly IModelValidator _validator;
+
+        /// <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 ValidatingConvention([NotNull] IModelValidator validator)
+        {
+            _validator = validator;
+        }
+
+        /// <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 InternalModelBuilder Apply(InternalModelBuilder modelBuilder)
+        {
+            _validator.Validate(modelBuilder.Metadata);
+
+            return modelBuilder;
+        }
+    }
+}
diff --git a/src/EFCore/ModelBuilder.cs b/src/EFCore/ModelBuilder.cs
@@ -5,6 +5,7 @@
 using System.ComponentModel;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Builders;
 using Microsoft.EntityFrameworkCore.Metadata.Conventions;
@@ -38,6 +39,8 @@ public ModelBuilder([NotNull] ConventionSet conventions)
             Check.NotNull(conventions, nameof(conventions));
 
             _builder = new InternalModelBuilder(new Model(conventions));
+
+            _builder.Metadata.SetProductVersion(ProductInfo.GetVersion());
         }
 
         /// <summary>
@@ -393,6 +396,21 @@ public virtual ModelBuilder UsePropertyAccessMode(PropertyAccessMode propertyAcc
             return this;
         }
 
+        /// <summary>
+        ///     Forces post-processing on the model such that it is ready for use by the runtime. This post
+        ///     processing happens automatically when using OnModelCreating; this method allows it to be run
+        ///     explicitly in cases where the automatic execution is not possible.
+        /// </summary>
+        /// <returns>
+        ///     The same <see cref="ModelBuilder" /> instance so that additional configuration calls can be chained.
+        /// </returns>
+        public virtual ModelBuilder FinalizeModel()
+        {
+            Builder.Metadata.Validate();
+
+            return this;
+        }
+
         private InternalModelBuilder Builder => this.GetInfrastructure();
 
         #region Hidden System.Object members

diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs
@@ -273,6 +273,7 @@ public void Snapshot_with_enum_discriminator_uses_converted_values()
                             codeHelper))));
 
             var modelBuilder = RelationalTestHelpers.Instance.CreateConventionBuilder();
+            modelBuilder.Model.RemoveAnnotation(CoreAnnotationNames.ProductVersionAnnotation);
             modelBuilder.Entity<WithAnnotations>(
                 eb =>
                 {
@@ -282,7 +283,7 @@ public void Snapshot_with_enum_discriminator_uses_converted_values()
                     eb.Property<RawEnum>("EnumDiscriminator").HasConversion<int>();
                 });
 
-            modelBuilder.GetInfrastructure().Metadata.Validate();
+            modelBuilder.FinalizeModel();
 
             var modelSnapshotCode = generator.GenerateSnapshot(
                 "MyNamespace",
@@ -336,7 +337,7 @@ private static void AssertConverter(ValueConverter valueConverter, string expect
             var property = modelBuilder.Entity<WithAnnotations>().Property(e => e.Id).Metadata;
             property.SetMaxLength(1000);
 
-            modelBuilder.GetInfrastructure().Metadata.Validate();
+            modelBuilder.FinalizeModel();
 
             var codeHelper = new CSharpHelper(new SqlServerTypeMappingSource(
                 TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
@@ -520,6 +521,7 @@ public void Snapshots_compile()
             var generator = CreateMigrationsCodeGenerator();
 
             var modelBuilder = RelationalTestHelpers.Instance.CreateConventionBuilder();
+            modelBuilder.Model.RemoveAnnotation(CoreAnnotationNames.ProductVersionAnnotation);
             modelBuilder.Entity<EntityWithConstructorBinding>(
                 x =>
                 {
@@ -539,7 +541,7 @@ public void Snapshots_compile()
             var property2 = entityType.AddProperty("Ham", typeof(RawEnum));
             property2.SetValueConverter(new ValueConverter<RawEnum, string>(v => v.ToString(), v => (RawEnum)Enum.Parse(typeof(RawEnum), v), new ConverterMappingHints(size: 10)));
 
-            modelBuilder.GetInfrastructure().Metadata.Validate();
+            modelBuilder.FinalizeModel();
 
             var modelSnapshotCode = generator.GenerateSnapshot(
                 "MyNamespace",