Skip to content

Commit

Permalink
Move ConstructionBinding from an annotation to an interface
Browse files Browse the repository at this point in the history
Move OnModelFinalized invocation to ModelRuntimeInitializer

Part of #19213
  • Loading branch information
AndriySvyryd committed Feb 3, 2021
1 parent b848377 commit 64293d7
Show file tree
Hide file tree
Showing 35 changed files with 790 additions and 410 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ public virtual CosmosOptionsExtension WithConnectionString([CanBeNull] string? c
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual string? DatabaseName
=> _databaseName;
public virtual string DatabaseName
=> _databaseName!;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down Expand Up @@ -550,6 +550,7 @@ public override long GetServiceProviderHashCode()
hashCode = (hashCode * 397) ^ (Extension._accountKey?.GetHashCode() ?? 0);
}

hashCode = (hashCode * 397) ^ (Extension._databaseName?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (Extension._region?.GetHashCode() ?? 0);
hashCode = (hashCode * 3) ^ (Extension._connectionMode?.GetHashCode() ?? 0);
hashCode = (hashCode * 3) ^ (Extension._limitToEndpoint?.GetHashCode() ?? 0);
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Proxies/Proxies/Internal/IProxyFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public interface IProxyFactory
/// </summary>
Type CreateProxyType(
[NotNull] ProxiesOptionsExtension options,
[NotNull] IEntityType entityType);
[NotNull] IReadOnlyEntityType entityType);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
259 changes: 131 additions & 128 deletions src/EFCore.Proxies/Proxies/Internal/ProxyBindingRewriter.cs

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions src/EFCore.Proxies/Proxies/Internal/ProxyFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ public class ProxyFactory : IProxyFactory
/// </summary>
public virtual Type CreateProxyType(
ProxiesOptionsExtension options,
IEntityType entityType)
IReadOnlyEntityType entityType)
=> _generator.ProxyBuilder.CreateClassProxyType(
entityType.ClrType,
GetInterfacesToProxy(options, entityType),
GetInterfacesToProxy(options, entityType.ClrType),
ProxyGenerationOptions.Default);

/// <summary>
Expand Down Expand Up @@ -100,7 +100,7 @@ public class ProxyFactory : IProxyFactory
object[] constructorArguments)
=> _generator.CreateClassProxy(
entityType.ClrType,
GetInterfacesToProxy(options, entityType),
GetInterfacesToProxy(options, entityType.ClrType),
ProxyGenerationOptions.Default,
constructorArguments,
GetNotifyChangeInterceptors(options, entityType, new LazyLoadingInterceptor(entityType, loader)));
Expand Down Expand Up @@ -143,14 +143,14 @@ public class ProxyFactory : IProxyFactory
object[] constructorArguments)
=> _generator.CreateClassProxy(
entityType.ClrType,
GetInterfacesToProxy(options, entityType),
GetInterfacesToProxy(options, entityType.ClrType),
ProxyGenerationOptions.Default,
constructorArguments,
GetNotifyChangeInterceptors(options, entityType));

private Type[] GetInterfacesToProxy(
ProxiesOptionsExtension options,
IEntityType entityType)
Type type)
{
var interfacesToProxy = new List<Type>();

Expand All @@ -161,12 +161,12 @@ public class ProxyFactory : IProxyFactory

if (options.UseChangeTrackingProxies)
{
if (!_notifyPropertyChangedInterface.IsAssignableFrom(entityType.ClrType))
if (!_notifyPropertyChangedInterface.IsAssignableFrom(type))
{
interfacesToProxy.Add(_notifyPropertyChangedInterface);
}

if (!_notifyPropertyChangingInterface.IsAssignableFrom(entityType.ClrType))
if (!_notifyPropertyChangingInterface.IsAssignableFrom(type))
{
interfacesToProxy.Add(_notifyPropertyChangingInterface);
}
Expand Down
10 changes: 9 additions & 1 deletion src/EFCore/Infrastructure/ModelRuntimeInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Utilities;
using Microsoft.Extensions.DependencyInjection;

Expand Down Expand Up @@ -56,12 +57,19 @@ public ModelRuntimeInitializer([NotNull] ModelRuntimeInitializerDependencies dep
if (model.SetModelDependencies(Dependencies.ModelDependencies))
{
InitializeModel(model, preValidation: true);

if (validationLogger != null
&& model is IMutableModel)
&& model is IConventionModel)
{
Dependencies.ModelValidator.Validate(model, validationLogger);
}

InitializeModel(model, preValidation: false);

if (model is Model mutableModel)
{
model = mutableModel.OnModelFinalized();
}
}

return model;
Expand Down
3 changes: 1 addition & 2 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -843,8 +843,7 @@ static bool ContainedInForeignKeyForAllConcreteTypes(IEntityType entityType, IPr
.Concat(entityType.GetDeclaredNavigations())
.Where(p => !p.IsShadowProperty() && !p.IsIndexerProperty()));

var constructorBinding = (InstantiationBinding?)entityType[CoreAnnotationNames.ConstructorBinding];

var constructorBinding = entityType.ConstructorBinding;
if (constructorBinding != null)
{
foreach (var consumedProperty in constructorBinding.ParameterBindings.SelectMany(p => p.ConsumedProperties))
Expand Down
18 changes: 17 additions & 1 deletion src/EFCore/Infrastructure/SingletonModelDependencies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,32 @@ public sealed record SingletonModelDependencies
/// </summary>
[EntityFrameworkInternal]
public SingletonModelDependencies(
[NotNull] ITypeMappingSource typeMappingSource)
[NotNull] ITypeMappingSource typeMappingSource,
[NotNull] IConstructorBindingFactory constructorBindingFactory,
[NotNull] IParameterBindingFactories parameterBindingFactories)
{
Check.NotNull(typeMappingSource, nameof(typeMappingSource));
Check.NotNull(constructorBindingFactory, nameof(constructorBindingFactory));
Check.NotNull(parameterBindingFactories, nameof(parameterBindingFactories));

TypeMappingSource = typeMappingSource;
ConstructorBindingFactory = constructorBindingFactory;
ParameterBindingFactories = parameterBindingFactories;
}

/// <summary>
/// The type mapper.
/// </summary>
public ITypeMappingSource TypeMappingSource { get; [param: NotNull] init; }

/// <summary>
/// The constructor binding factory.
/// </summary>
public IConstructorBindingFactory ConstructorBindingFactory { get; [param: NotNull] init; }

/// <summary>
/// The parameter binding factories.
/// </summary>
public IParameterBindingFactories ParameterBindingFactories { get; [param: NotNull] init; }
}
}
124 changes: 7 additions & 117 deletions src/EFCore/Metadata/Conventions/ConstructorBindingConvention.cs
Original file line number Diff line number Diff line change
@@ -1,13 +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 System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
Expand Down Expand Up @@ -46,122 +40,18 @@ public ConstructorBindingConvention([NotNull] ProviderConventionSetBuilderDepend
IConventionModelBuilder modelBuilder,
IConventionContext<IConventionModelBuilder> context)
{
foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
foreach (EntityType entityType in modelBuilder.Metadata.GetEntityTypes())
{
if (entityType.ClrType?.IsAbstract == false
&& entityType.Builder.CanSetAnnotation(CoreAnnotationNames.ConstructorBinding, null))
if (!entityType.ClrType.IsAbstract
&& ConfigurationSource.Convention.Overrides(entityType.GetConstructorBindingConfigurationSource()))
{
var maxServiceParams = 0;
var maxServiceOnlyParams = 0;
var minPropertyParams = int.MaxValue;
var foundBindings = new List<InstantiationBinding>();
var foundServiceOnlyBindings = new List<InstantiationBinding>();
var bindingFailures = new List<IEnumerable<ParameterInfo>>();
Dependencies.ConstructorBindingFactory.GetBindings(
(IMutableEntityType)entityType, out var constructorBinding, out var serviceOnlyBinding);

foreach (var constructor in entityType.ClrType.GetTypeInfo()
.DeclaredConstructors
.Where(c => !c.IsStatic))
{
// Trying to find the constructor with the most service properties
// followed by the least scalar property parameters
if (Dependencies.ConstructorBindingFactory.TryBindConstructor(
entityType, constructor, out var binding, out var failures))
{
var serviceParamCount = binding.ParameterBindings.OfType<ServiceParameterBinding>().Count();
var propertyParamCount = binding.ParameterBindings.Count - serviceParamCount;

if (propertyParamCount == 0)
{
if (serviceParamCount == maxServiceOnlyParams)
{
foundServiceOnlyBindings.Add(binding);
}
else if (serviceParamCount > maxServiceOnlyParams)
{
foundServiceOnlyBindings.Clear();
foundServiceOnlyBindings.Add(binding);

maxServiceOnlyParams = serviceParamCount;
}
}

if (serviceParamCount == maxServiceParams
&& propertyParamCount == minPropertyParams)
{
foundBindings.Add(binding);
}
else if (serviceParamCount > maxServiceParams)
{
foundBindings.Clear();
foundBindings.Add(binding);

maxServiceParams = serviceParamCount;
minPropertyParams = propertyParamCount;
}
else if (propertyParamCount < minPropertyParams)
{
foundBindings.Clear();
foundBindings.Add(binding);

maxServiceParams = serviceParamCount;
minPropertyParams = propertyParamCount;
}
}
else
{
bindingFailures.Add(failures);
}
}

if (foundBindings.Count == 0)
{
var constructorErrors = bindingFailures.SelectMany(f => f)
.GroupBy(f => (ConstructorInfo)f.Member)
.Select(
x => CoreStrings.ConstructorBindingFailed(
string.Join("', '", x.Select(f => f.Name)),
entityType.DisplayName()
+ "("
+ string.Join(
", ", x.Key.GetParameters().Select(
y => y.ParameterType.ShortDisplayName() + " " + y.Name)
)
+ ")"
)
);

throw new InvalidOperationException(
CoreStrings.ConstructorNotFound(
entityType.DisplayName(),
string.Join("; ", constructorErrors)));
}

if (foundBindings.Count > 1)
{
throw new InvalidOperationException(
CoreStrings.ConstructorConflict(
FormatConstructorString(entityType, foundBindings[0]),
FormatConstructorString(entityType, foundBindings[1])));
}

entityType.Builder.HasAnnotation(
CoreAnnotationNames.ConstructorBinding,
foundBindings[0]);

if (foundServiceOnlyBindings.Count == 1)
{
entityType.Builder.HasAnnotation(
CoreAnnotationNames.ServiceOnlyConstructorBinding,
foundServiceOnlyBindings[0]);
}
entityType.Builder.HasConstructorBinding(constructorBinding, ConfigurationSource.Convention);
entityType.Builder.HasServiceOnlyConstructorBinding(serviceOnlyBinding, ConfigurationSource.Convention);
}
}
}

private static string FormatConstructorString(IReadOnlyEntityType entityType, InstantiationBinding binding)
=> entityType.ClrType.ShortDisplayName()
+ "("
+ string.Join(", ", binding.ParameterBindings.Select(b => b.ParameterType.ShortDisplayName()))
+ ")";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ public sealed record ProviderConventionSetBuilderDependencies
Check.NotNull(validator, nameof(validator));

TypeMappingSource = typeMappingSource;
ConstructorBindingFactory = constructorBindingFactory;
ParameterBindingFactories = parameterBindingFactories;
MemberClassifier = memberClassifier;
ConstructorBindingFactory = constructorBindingFactory;
Logger = logger;
ValidationLogger = validationLogger;
SetFinder = setFinder;
_currentContext = currentContext;
ValidationLogger = validationLogger;
#pragma warning disable CS0618 // Type or member is obsolete
ModelValidator = validator;
#pragma warning restore CS0618 // Type or member is obsolete
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,6 @@ public IConventionModelBuilder OnModelFinalizing([NotNull] IConventionModelBuild
return modelBuilder;
}

public IModel OnModelFinalized([NotNull] IModel model)
{
foreach (var modelConvention in _conventionSet.ModelFinalizedConventions)
{
model = modelConvention.ProcessModelFinalized(model);
}

return model;
}

public IConventionModelBuilder OnModelInitialized([NotNull] IConventionModelBuilder modelBuilder)
{
using (_dispatcher.DelayConventions())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,6 @@ public virtual IConventionModelBuilder OnModelInitialized([NotNull] IConventionM
public virtual IConventionModelBuilder OnModelFinalizing([NotNull] IConventionModelBuilder modelBuilder)
=> _immediateConventionScope.OnModelFinalizing(modelBuilder);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual IModel OnModelFinalized([NotNull] IModel model)
=> _immediateConventionScope.OnModelFinalized(model);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
36 changes: 36 additions & 0 deletions src/EFCore/Metadata/IConstructorBindingFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,42 @@ namespace Microsoft.EntityFrameworkCore.Metadata
/// </summary>
public interface IConstructorBindingFactory
{
/// <summary>
/// Create a <see cref="InstantiationBinding" /> for the constructor with most parameters and
/// the constructor with only service property parameters.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="constructorBinding"> The binding for the constructor with most parameters. </param>
/// <param name="serviceOnlyBinding"> The binding for the constructor with only service property parameters. </param>
void GetBindings(
[NotNull] IConventionEntityType entityType,
[NotNull] out InstantiationBinding constructorBinding,
[NotNull] out InstantiationBinding? serviceOnlyBinding);

/// <summary>
/// Create a <see cref="InstantiationBinding" /> for the constructor with most parameters and
/// the constructor with only service property parameters.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="constructorBinding"> The binding for the constructor with most parameters. </param>
/// <param name="serviceOnlyBinding"> The binding for the constructor with only service property parameters. </param>
void GetBindings(
[NotNull] IMutableEntityType entityType,
[NotNull] out InstantiationBinding constructorBinding,
[NotNull] out InstantiationBinding? serviceOnlyBinding);

/// <summary>
/// Create a <see cref="InstantiationBinding" /> for the constructor with most parameters and
/// the constructor with only service property parameters.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="constructorBinding"> The binding for the constructor with most parameters. </param>
/// <param name="serviceOnlyBinding"> The binding for the constructor with only service property parameters. </param>
void GetBindings(
[NotNull] IReadOnlyEntityType entityType,
[NotNull] out InstantiationBinding constructorBinding,
[NotNull] out InstantiationBinding? serviceOnlyBinding);

/// <summary>
/// Attempts to create a <see cref="InstantiationBinding" /> for the given entity type and
/// <see cref="ConstructorInfo" />
Expand Down

0 comments on commit 64293d7

Please sign in to comment.