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 4141299
Show file tree
Hide file tree
Showing 38 changed files with 799 additions and 423 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
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
2 changes: 1 addition & 1 deletion src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ private void SetServiceProperties(EntityState oldState, EntityState newState)
{
this[serviceProperty]
= serviceProperty
.GetParameterBinding()
.ParameterBinding
.ServiceDelegate(
new MaterializationContext(
ValueBuffer.Empty,
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
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private void Process(IConventionEntityTypeBuilder entityTypeBuilder)
}

entityTypeBuilder.ServiceProperty(propertyInfo)?.HasParameterBinding(
(ServiceParameterBinding)factory.Bind(entityType, propertyInfo.PropertyType, propertyInfo.GetSimpleMemberName()));
(ServiceParameterBinding)factory.Bind(entityType, propertyInfo.PropertyType, name));
}
}

Expand Down

0 comments on commit 4141299

Please sign in to comment.