Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[automated] Merge branch 'release/8.0' => 'main' #31708

Merged
1 commit merged into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,9 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
{
// Attempt to translate access into a primitive collection property (i.e. array column)

// TODO: We should be detecting primitive collections by looking at GetElementType() of the property and not at its type
// mapping; but #31469 is blocking that for shadow properties.
if (_sqlTranslator.TryTranslatePropertyAccess(methodCallExpression, out var translatedExpression, out var property)
&& property is IProperty regularProperty
&& translatedExpression is SqlExpression
{
TypeMapping.ElementTypeMapping: RelationalTypeMapping
} sqlExpression)
&& property is IProperty { IsPrimitiveCollection: true } regularProperty
&& translatedExpression is SqlExpression sqlExpression)
{
var tableAlias = sqlExpression switch
{
Expand Down Expand Up @@ -2309,10 +2304,8 @@ static TableExpressionBase FindRootTableExpressionForColumn(ColumnExpression col
}
}

// TODO: Check that the property is a primitive collection property directly once we have that in metadata, rather than
// looking at the type mapping.
var property = type.FindProperty(memberName);
if (property?.GetRelationalTypeMapping().ElementTypeMapping is null)
if (property?.IsPrimitiveCollection != true)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2003,7 +2003,7 @@ protected override Expression VisitBinary(BinaryExpression node)
if (node.Right is MethodCallExpression methodCallExpression
&& IsPropertyAssignment(methodCallExpression, out var property, out var parameter))
{
if (property!.GetTypeMapping().ElementTypeMapping != null
if (property!.IsPrimitiveCollection
&& !property.ClrType.IsArray)
{
var currentVariable = Variable(parameter!.Type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
// which case we only have the CLR type (note that we cannot produce different SQLs based on the nullability of an *element* in
// a parameter collection - our caching mechanism only supports varying by the nullability of the parameter itself (i.e. the
// collection).
// TODO: if property is non-null, GetElementType() should never be null, but we have #31469 for shadow properties
var isElementNullable = property?.GetElementType() is null
? elementClrType.IsNullableType()
: property.GetElementType()!.IsNullable;
var isElementNullable = property?.GetElementType()!.IsNullable;

#pragma warning disable EF1001 // Internal EF Core API usage.
var selectExpression = new SelectExpression(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ protected override void AppendUpdateColumnValue(
stringBuilder.Append(columnModification.JsonPath);
stringBuilder.Append("', ");

if (columnModification.Property != null
&& columnModification.Property.GetTypeMapping().ElementTypeMapping == null)
if (columnModification.Property is { IsPrimitiveCollection: false })
{
base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,7 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis
// which case we only have the CLR type (note that we cannot produce different SQLs based on the nullability of an *element* in
// a parameter collection - our caching mechanism only supports varying by the nullability of the parameter itself (i.e. the
// collection).
// TODO: if property is non-null, GetElementType() should never be null, but we have #31469 for shadow properties
var isElementNullable = property?.GetElementType() is null
? elementClrType.IsNullableType()
: property.GetElementType()!.IsNullable;
var isElementNullable = property?.GetElementType()!.IsNullable;

#pragma warning disable EF1001 // Internal EF Core API usage.
var selectExpression = new SelectExpression(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ protected override void AppendUpdateColumnValue(
stringBuilder.Append(columnModification.JsonPath);
stringBuilder.Append("', ");

if (columnModification.Property != null
&& columnModification.Property.GetTypeMapping().ElementTypeMapping == null)
if (columnModification.Property is { IsPrimitiveCollection: false })
{
var providerClrType = (columnModification.Property.GetTypeMapping().Converter?.ProviderClrType
?? columnModification.Property.ClrType).UnwrapNullableType();
Expand Down
2 changes: 0 additions & 2 deletions src/EFCore/ChangeTracking/ListComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ public static bool Compare(IEnumerable<TElement>? a, IEnumerable<TElement>? b, V
throw new InvalidOperationException(
CoreStrings.BadListType(
(a is IList<TElement?> ? b : a).GetType().ShortDisplayName(),
typeof(ListComparer<TElement?>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
}

Expand Down Expand Up @@ -129,7 +128,6 @@ public static IList<TElement> Snapshot(IEnumerable<TElement> source, ValueCompar
throw new InvalidOperationException(
CoreStrings.BadListType(
source.GetType().ShortDisplayName(),
typeof(ListComparer<TElement?>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
}

Expand Down
2 changes: 0 additions & 2 deletions src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ public static bool Compare(IEnumerable<TElement?>? a, IEnumerable<TElement?>? b,
throw new InvalidOperationException(
CoreStrings.BadListType(
(a is IList<TElement?> ? b : a).GetType().ShortDisplayName(),
typeof(NullableValueTypeListComparer<TElement>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName()));
}

Expand Down Expand Up @@ -130,7 +129,6 @@ public static int GetHashCode(IEnumerable<TElement?> source, ValueComparer<TElem
throw new InvalidOperationException(
CoreStrings.BadListType(
source.GetType().ShortDisplayName(),
typeof(NullableValueTypeListComparer<TElement>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName()));
}

Expand Down
2 changes: 0 additions & 2 deletions src/EFCore/ChangeTracking/ObjectListComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ public static bool Compare(IEnumerable<TElement>? a, IEnumerable<TElement>? b, V
throw new InvalidOperationException(
CoreStrings.BadListType(
(a is IList<TElement?> ? b : a).GetType().ShortDisplayName(),
typeof(ListComparer<TElement?>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
}

Expand Down Expand Up @@ -128,7 +127,6 @@ public static IList<TElement> Snapshot(IEnumerable<TElement> source, ValueCompar
throw new InvalidOperationException(
CoreStrings.BadListType(
source.GetType().ShortDisplayName(),
typeof(ListComparer<TElement?>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
}

Expand Down
38 changes: 38 additions & 0 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public virtual void Validate(IModel model, IDiagnosticsLogger<DbLoggerCategory.M
ValidateQueryFilters(model, logger);
ValidateData(model, logger);
ValidateTypeMappings(model, logger);
ValidatePrimitiveCollections(model, logger);
ValidateTriggers(model, logger);
LogShadowProperties(model, logger);
}
Expand Down Expand Up @@ -1003,6 +1004,43 @@ static void Validate(ITypeBase typeBase, IDiagnosticsLogger<DbLoggerCategory.Mod
}
}

/// <summary>
/// Validates the mapping of primitive collection properties the model.
/// </summary>
/// <param name="model">The model to validate.</param>
/// <param name="logger">The logger to use.</param>
protected virtual void ValidatePrimitiveCollections(
IModel model,
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
{
foreach (var entityType in model.GetEntityTypes())
{
Validate(entityType, logger);
}

static void Validate(ITypeBase typeBase, IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
{
foreach (var property in typeBase.GetDeclaredProperties())
{
var elementClrType = property.GetElementType()?.ClrType;
if (property is { IsPrimitiveCollection: true, ClrType.IsArray: false, ClrType.IsSealed: true }
&& elementClrType is { IsSealed: true }
&& elementClrType.TryGetElementType(typeof(IList<>)) == null)
{
throw new InvalidOperationException(
CoreStrings.BadListType(
property.ClrType.ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementClrType).ShortDisplayName()));
}
}

foreach (var complexProperty in typeBase.GetDeclaredComplexProperties())
{
Validate(complexProperty.ComplexType, logger);
}
}
}

/// <summary>
/// Validates the mapping/configuration of query filters in the model.
/// </summary>
Expand Down
55 changes: 55 additions & 0 deletions src/EFCore/Metadata/Conventions/ElementMappingConvention.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;

/// <summary>
/// A convention that ensures property mappings have any ElementMapping discovered by the type mapper.
/// </summary>
/// <remarks>
/// <para>
/// See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.
/// </para>
/// </remarks>
public class ElementMappingConvention : IModelFinalizingConvention
{
/// <summary>
/// Creates a new instance of <see cref="ElementMappingConvention" />.
/// </summary>
/// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
public ElementMappingConvention(ProviderConventionSetBuilderDependencies dependencies)
{
Dependencies = dependencies;
}

/// <summary>
/// Dependencies for this service.
/// </summary>
protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; }

/// <inheritdoc />
public void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext<IConventionModelBuilder> context)
{
foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
{
Validate(entityType);
}

void Validate(IConventionTypeBase typeBase)
{
foreach (var property in typeBase.GetDeclaredProperties())
{
var typeMapping = Dependencies.TypeMappingSource.FindMapping((IProperty)property);
if (typeMapping is { ElementTypeMapping: not null })
{
property.SetElementType(property.ClrType.TryGetElementType(typeof(IEnumerable<>)));
}
}

foreach (var complexProperty in typeBase.GetDeclaredComplexProperties())
{
Validate(complexProperty.ComplexType);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public virtual ConventionSet CreateConventionSet()
conventionSet.Add(new BackingFieldConvention(Dependencies));
conventionSet.Add(new QueryFilterRewritingConvention(Dependencies));
conventionSet.Add(new RuntimeModelConvention(Dependencies));
conventionSet.Add(new ElementMappingConvention(Dependencies));

return conventionSet;
}
Expand Down
11 changes: 6 additions & 5 deletions src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ protected virtual RuntimeModel Create(IModel model)
var elementType = property.GetElementType();
if (elementType != null)
{
var runtimeElementType = Create(runtimeProperty, elementType);
var runtimeElementType = Create(runtimeProperty, elementType, property.IsPrimitiveCollection);
CreateAnnotations(
elementType, runtimeElementType, static (convention, annotations, source, target, runtime) =>
convention.ProcessElementTypeAnnotations(annotations, source, target, runtime));
Expand Down Expand Up @@ -398,7 +398,7 @@ private static RuntimeProperty Create(IProperty property, RuntimeTypeBase runtim
property.GetJsonValueReaderWriter(),
property.GetTypeMapping());

private static RuntimeElementType Create(RuntimeProperty runtimeProperty, IElementType element)
private static RuntimeElementType Create(RuntimeProperty runtimeProperty, IElementType element, bool primitiveCollection)
=> runtimeProperty.SetElementType(
element.ClrType,
element.IsNullable,
Expand All @@ -410,7 +410,8 @@ private static RuntimeElementType Create(RuntimeProperty runtimeProperty, IEleme
element.GetValueConverter(),
element.GetValueComparer(),
element.GetJsonValueReaderWriter(),
element.GetTypeMapping());
element.GetTypeMapping(),
primitiveCollection);

/// <summary>
/// Updates the property annotations that will be set on the read-only object.
Expand Down Expand Up @@ -524,7 +525,7 @@ private RuntimeComplexProperty Create(IComplexProperty complexProperty, RuntimeE
var elementType = property.GetElementType();
if (elementType != null)
{
var runtimeElementType = Create(runtimeProperty, elementType);
var runtimeElementType = Create(runtimeProperty, elementType, property.IsPrimitiveCollection);
CreateAnnotations(
elementType, runtimeElementType, static (convention, annotations, source, target, runtime) =>
convention.ProcessElementTypeAnnotations(annotations, source, target, runtime));
Expand Down Expand Up @@ -571,7 +572,7 @@ private RuntimeComplexProperty Create(IComplexProperty complexProperty, RuntimeC
var elementType = property.GetElementType();
if (elementType != null)
{
var runtimeElementType = Create(runtimeProperty, elementType);
var runtimeElementType = Create(runtimeProperty, elementType, property.IsPrimitiveCollection);
CreateAnnotations(
elementType, runtimeElementType, static (convention, annotations, source, target, runtime) =>
convention.ProcessElementTypeAnnotations(annotations, source, target, runtime));
Expand Down
2 changes: 2 additions & 0 deletions src/EFCore/Metadata/Internal/InternalPropertyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ public virtual bool CanSetValueGeneratorFactory(
{
if (CanSetConversion(converter, configurationSource))
{
Metadata.SetElementType(null, configurationSource);
Metadata.SetProviderClrType(null, configurationSource);
Metadata.SetValueConverter(converter, configurationSource);

Expand Down Expand Up @@ -531,6 +532,7 @@ public virtual bool CanSetConversion(
{
if (CanSetConversion(providerClrType, configurationSource))
{
Metadata.SetElementType(null, configurationSource);
Metadata.SetValueConverter((ValueConverter?)null, configurationSource);
Metadata.SetProviderClrType(providerClrType, configurationSource);

Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/Metadata/Internal/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,8 @@ public virtual bool IsPrimitiveCollection
{
var elementType = GetElementType();
return elementType != null
&& ClrType.TryGetElementType(typeof(IEnumerable<>))?.IsAssignableFrom(elementType!.ClrType) == true;
&& ClrType.TryGetElementType(typeof(IEnumerable<>))?.UnwrapNullableType()
.IsAssignableFrom(elementType.ClrType.UnwrapNullableType()) == true;
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/EFCore/Metadata/RuntimeProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public RuntimeProperty(
/// <param name="valueComparer">The <see cref="ValueComparer" /> for this property.</param>
/// <param name="jsonValueReaderWriter">The <see cref="JsonValueReaderWriter" /> for this property.</param>
/// <param name="typeMapping">The <see cref="CoreTypeMapping" /> for this property.</param>
/// <param name="primitiveCollection">A value indicating whether this property represents a primitive collection.</param>
/// <returns>The newly created property.</returns>
public virtual RuntimeElementType SetElementType(
Type clrType,
Expand All @@ -137,7 +138,8 @@ public virtual RuntimeElementType SetElementType(
ValueConverter? valueConverter = null,
ValueComparer? valueComparer = null,
JsonValueReaderWriter? jsonValueReaderWriter = null,
CoreTypeMapping? typeMapping = null)
CoreTypeMapping? typeMapping = null,
bool primitiveCollection = false)
{
var elementType = new RuntimeElementType(
clrType,
Expand All @@ -155,7 +157,7 @@ public virtual RuntimeElementType SetElementType(

SetAnnotation(CoreAnnotationNames.ElementType, elementType);

IsPrimitiveCollection = ClrType.TryGetElementType(typeof(IEnumerable<>))?.IsAssignableFrom(clrType) == true;
IsPrimitiveCollection = primitiveCollection;

return elementType;
}
Expand Down
8 changes: 4 additions & 4 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@
<value>The type '{givenType}' cannot be used as a 'JsonValueReaderWriter' because it does not inherit from the generic 'JsonValueReaderWriter&lt;TValue&gt;'. Make sure to inherit json reader/writers from 'JsonValueReaderWriter&lt;TValue&gt;'.</value>
</data>
<data name="BadListType" xml:space="preserve">
<value>The type '{givenType}' cannot be used with '{comparerType}' because it does not implement '{listType}'. Collections of primitive types must be ordered lists.</value>
<value>The type '{givenType}' cannot be used as a primitive collection because it is not an array and does not implement '{listType}'. Collections of primitive types must be arrays or ordered lists.</value>
</data>
<data name="BadValueComparerType" xml:space="preserve">
<value>The type '{givenType}' cannot be used as a value comparer because it does not inherit from '{expectedType}'. Make sure to inherit value comparers from '{expectedType}'.</value>
Expand Down
Loading
Loading