Skip to content

Commit

Permalink
Separate element type from primitive collection (#31677)
Browse files Browse the repository at this point in the history
  • Loading branch information
ajcvickers authored Sep 11, 2023
1 parent ecfc1ce commit 88edb55
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 35 deletions.
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Builders/IConventionPropertyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -545,13 +545,13 @@ bool CanSetProviderValueComparer(
/// 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>
IConventionElementTypeBuilder? SetElementType(bool elementType, bool fromDataAnnotation = false);
IConventionElementTypeBuilder? SetElementType(Type? elementType, bool fromDataAnnotation = false);

/// <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>
bool CanSetElementType(bool elementType, bool fromDataAnnotation = false);
bool CanSetElementType(Type? elementType, bool fromDataAnnotation = false);
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ private void Process(IConventionEntityTypeBuilder entityTypeBuilder)
var propertyBuilder = entityTypeBuilder.Property(propertyInfo);
if (mapping?.ElementTypeMapping != null)
{
propertyBuilder?.SetElementType(true);
var elementType = propertyInfo.PropertyType.TryGetElementType(typeof(IEnumerable<>));
if (elementType != null)
{
propertyBuilder?.SetElementType(elementType);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/IConventionProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ bool IsImplicitlyCreated()
/// <param name="elementType">If <see langword="true"/>, then the type mapping has an element type, otherwise it is removed.</param>
/// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
/// <returns>The configuration for the elements.</returns>
IConventionElementType? SetElementType(bool elementType, bool fromDataAnnotation = false);
IConventionElementType? SetElementType(Type? elementType, bool fromDataAnnotation = false);

/// <summary>
/// Returns the configuration source for <see cref="IReadOnlyProperty.GetElementType" />.
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/IMutableProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ void SetProviderValueComparer(
/// Sets the configuration for elements of the primitive collection represented by this property.
/// </summary>
/// <param name="elementType">If <see langword="true"/>, then this is a collection of primitive elements.</param>
void SetElementType(bool elementType);
void SetElementType(Type? elementType);

/// <inheritdoc />
bool IReadOnlyProperty.IsNullable
Expand Down
6 changes: 6 additions & 0 deletions src/EFCore/Metadata/IReadOnlyProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,12 @@ CoreTypeMapping GetTypeMapping()
/// <returns>The configuration for the elements.</returns>
IReadOnlyElementType? GetElementType();

/// <summary>
/// A property is a primitive collection if it has an element type that matches the element type of the CLR type.
/// </summary>
/// <returns><see langword="true"/> if the property represents a primitive collection.</returns>
bool IsPrimitiveCollection { get; }

/// <summary>
/// Finds the first principal property that the given property is constrained by
/// if the given property is part of a foreign key.
Expand Down
14 changes: 7 additions & 7 deletions src/EFCore/Metadata/Internal/InternalPropertyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ public virtual bool CanSetConversion(Type? providerClrType, ConfigurationSource?
{
if (CanSetConverter(converterType, configurationSource))
{
Metadata.SetElementType(false, configurationSource);
Metadata.SetElementType(null, configurationSource);
Metadata.SetProviderClrType(null, configurationSource);
Metadata.SetValueConverter(converterType, configurationSource);

Expand Down Expand Up @@ -776,13 +776,13 @@ public virtual bool CanSetProviderValueComparer(
/// 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 InternalElementTypeBuilder? SetElementType(bool elementType, ConfigurationSource configurationSource)
public virtual InternalElementTypeBuilder? SetElementType(Type? elementType, ConfigurationSource configurationSource)
{
if (CanSetElementType(elementType, configurationSource))
{
Metadata.SetElementType(elementType, configurationSource);
Metadata.SetValueConverter((Type?)null, configurationSource);
return new InternalElementTypeBuilder((ElementType)Metadata.GetElementType()!, ModelBuilder);
return new InternalElementTypeBuilder(Metadata.GetElementType()!, ModelBuilder);
}

return null;
Expand All @@ -794,9 +794,9 @@ public virtual bool CanSetProviderValueComparer(
/// 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 bool CanSetElementType(bool elementType, ConfigurationSource? configurationSource)
public virtual bool CanSetElementType(Type? elementType, ConfigurationSource? configurationSource)
=> configurationSource.Overrides(Metadata.GetElementTypeConfigurationSource())
&& (elementType != (Metadata.GetElementType() != null));
&& (elementType != Metadata.GetElementType()?.ClrType);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down Expand Up @@ -1523,7 +1523,7 @@ bool IConventionPropertyBuilder.CanSetProviderValueComparer(
/// 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>
IConventionElementTypeBuilder? IConventionPropertyBuilder.SetElementType(bool elementType, bool fromDataAnnotation)
IConventionElementTypeBuilder? IConventionPropertyBuilder.SetElementType(Type? elementType, bool fromDataAnnotation)
=> SetElementType(elementType, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <summary>
Expand All @@ -1532,6 +1532,6 @@ bool IConventionPropertyBuilder.CanSetProviderValueComparer(
/// 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>
bool IConventionPropertyBuilder.CanSetElementType(bool elementType, bool fromDataAnnotation)
bool IConventionPropertyBuilder.CanSetElementType(Type? elementType, bool fromDataAnnotation)
=> CanSetElementType(elementType, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
}
11 changes: 10 additions & 1 deletion src/EFCore/Metadata/Internal/InternalTypeBaseBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,16 @@ public static bool IsCompatible(MemberInfo? newMemberInfo, PropertyBase existing
{
var builder = Property(propertyType, propertyName, memberInfo, typeConfigurationSource, configurationSource);

builder?.SetElementType(true, configurationSource!.Value);
if (builder != null)
{
var elementClrType = builder.Metadata.ClrType.TryGetElementType(typeof(IEnumerable<>));
if (elementClrType == null)
{
throw new InvalidOperationException(CoreStrings.NotCollection(builder.Metadata.ClrType.ShortDisplayName(), propertyName));
}

builder.SetElementType(elementClrType, configurationSource!.Value);
}

return builder;
}
Expand Down
36 changes: 24 additions & 12 deletions src/EFCore/Metadata/Internal/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1208,32 +1208,44 @@ public virtual CoreTypeMapping? TypeMapping
public virtual ElementType? GetElementType()
=> (ElementType?)this[CoreAnnotationNames.ElementType];

/// <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 bool IsPrimitiveCollection
{
get
{
var elementType = GetElementType();
return elementType != null
&& ClrType.TryGetElementType(typeof(IEnumerable<>))?.IsAssignableFrom(elementType!.ClrType) == true;
}
}

/// <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 ElementType? SetElementType(
bool elementType,
Type? elementType,
ConfigurationSource configurationSource)
{
var existingElementType = GetElementType();
if (existingElementType == null
&& elementType)
if (elementType != null
&& elementType != existingElementType?.ClrType)
{
var elementClrType = ClrType.TryGetElementType(typeof(IEnumerable<>));
if (elementClrType == null)
{
throw new InvalidOperationException(CoreStrings.NotCollection(ClrType.ShortDisplayName(), Name));
}
var newElementType = new ElementType(elementClrType, this, configurationSource);
var newElementType = new ElementType(elementType, this, configurationSource);
SetAnnotation(CoreAnnotationNames.ElementType, newElementType, configurationSource);
OnElementTypeSet(newElementType, null);
return newElementType;
}

if (existingElementType != null && !elementType)
if (elementType == null
&& existingElementType != null)
{
existingElementType.SetRemovedFromModel();
RemoveAnnotation(CoreAnnotationNames.ElementType);
Expand Down Expand Up @@ -2017,7 +2029,7 @@ void IMutableProperty.SetJsonValueReaderWriterType(Type? readerWriterType)
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[DebuggerStepThrough]
IConventionElementType? IConventionProperty.SetElementType(bool elementType, bool fromDataAnnotation)
IConventionElementType? IConventionProperty.SetElementType(Type? elementType, bool fromDataAnnotation)
=> SetElementType(
elementType,
fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
Expand All @@ -2029,7 +2041,7 @@ void IMutableProperty.SetJsonValueReaderWriterType(Type? readerWriterType)
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[DebuggerStepThrough]
void IMutableProperty.SetElementType(bool elementType)
void IMutableProperty.SetElementType(Type? elementType)
=> SetElementType(elementType, ConfigurationSource.Explicit);

/// <summary>
Expand Down
10 changes: 10 additions & 0 deletions src/EFCore/Metadata/RuntimeProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ public virtual RuntimeElementType SetElementType(

SetAnnotation(CoreAnnotationNames.ElementType, elementType);

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

return elementType;
}

Expand Down Expand Up @@ -305,6 +307,14 @@ public override object? Sentinel
public virtual IElementType? GetElementType()
=> (IElementType?)this[CoreAnnotationNames.ElementType];

/// <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 bool IsPrimitiveCollection { get; private set; }

/// <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
20 changes: 10 additions & 10 deletions test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3689,12 +3689,12 @@ public void OnPropertyElementTypeChanged_calls_conventions_in_order(bool useBuil

if (useBuilder)
{
Assert.NotNull(propertyBuilder.SetElementType(true, ConfigurationSource.Convention));
elementType = (ElementType)propertyBuilder.Metadata.GetElementType()!;
Assert.NotNull(propertyBuilder.SetElementType(typeof(int), ConfigurationSource.Convention));
elementType = propertyBuilder.Metadata.GetElementType()!;
}
else
{
elementType = (ElementType)propertyBuilder.Metadata.SetElementType(true, ConfigurationSource.Convention);
elementType = propertyBuilder.Metadata.SetElementType(typeof(int), ConfigurationSource.Convention);
}

if (useScope)
Expand All @@ -3710,12 +3710,12 @@ public void OnPropertyElementTypeChanged_calls_conventions_in_order(bool useBuil

if (useBuilder)
{
Assert.Null(propertyBuilder.SetElementType(true, ConfigurationSource.Convention));
elementType = (ElementType)propertyBuilder.Metadata.GetElementType()!;
Assert.Null(propertyBuilder.SetElementType(typeof(int), ConfigurationSource.Convention));
elementType = propertyBuilder.Metadata.GetElementType()!;
}
else
{
elementType = (ElementType)propertyBuilder.Metadata.SetElementType(true, ConfigurationSource.Convention);
elementType = propertyBuilder.Metadata.SetElementType(typeof(int), ConfigurationSource.Convention);
}

Assert.Equal(new (object, object)[] { (null, elementType) }, convention1.Calls);
Expand All @@ -3724,11 +3724,11 @@ public void OnPropertyElementTypeChanged_calls_conventions_in_order(bool useBuil

if (useBuilder)
{
Assert.NotNull(propertyBuilder.SetElementType(false, ConfigurationSource.Convention));
Assert.NotNull(propertyBuilder.SetElementType(null, ConfigurationSource.Convention));
}
else
{
Assert.Null(propertyBuilder.Metadata.SetElementType(false, ConfigurationSource.Convention));
Assert.Null(propertyBuilder.Metadata.SetElementType(null, ConfigurationSource.Convention));
}

Assert.Equal(new (object, object)[] { (null, elementType), (elementType, null) }, convention1.Calls);
Expand Down Expand Up @@ -5069,7 +5069,7 @@ public void OnElementTypeAnnotationChanged_calls_conventions_in_order(bool useBu
var builder = new InternalModelBuilder(new Model(conventions));
var elementTypeBuilder = builder.Entity(typeof(SpecialOrder), ConfigurationSource.Convention)!
.Property(nameof(SpecialOrder.OrderIds), ConfigurationSource.Convention)!
.SetElementType(true, ConfigurationSource.Convention)!;
.SetElementType(typeof(int), ConfigurationSource.Convention)!;

var scope = useScope ? builder.Metadata.ConventionDispatcher.DelayConventions() : null;

Expand Down Expand Up @@ -5174,7 +5174,7 @@ public void OnElementTypeNullabilityChanged_calls_conventions_in_order(bool useB
var builder = new InternalModelBuilder(model);
var elementTypeBuilder = builder.Entity(typeof(SpecialOrder), ConfigurationSource.Convention)!
.Property(nameof(SpecialOrder.Notes), ConfigurationSource.Convention)!
.SetElementType(true, ConfigurationSource.Convention)!;
.SetElementType(typeof(string), ConfigurationSource.Convention)!;

if (useBuilder)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ public JsonValueReaderWriter GetJsonValueReaderWriter()
IReadOnlyElementType IReadOnlyProperty.GetElementType()
=> GetElementType();

public bool IsPrimitiveCollection { get; }

public IElementType GetElementType()
=> throw new NotImplementedException();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ public JsonValueReaderWriter GetJsonValueReaderWriter()
IReadOnlyElementType IReadOnlyProperty.GetElementType()
=> GetElementType();

public bool IsPrimitiveCollection { get; }

public IElementType GetElementType()
=> throw new NotImplementedException();

Expand Down
36 changes: 36 additions & 0 deletions test/EFCore.Tests/Metadata/Internal/PropertyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,42 @@ public void Throws_when_JsonValueReaderWriter_instance_cannot_be_created(Type ty
() => property.GetJsonValueReaderWriter()).Message);
}

[ConditionalFact]
public void Can_set_element_type_for_primitive_collection()
{
var model = CreateModel();
var entityType = model.AddEntityType(typeof(object));
var property = entityType.AddProperty("Random", typeof(IList<int>));
property.SetElementType(typeof(int));

Assert.Equal(typeof(int), property.GetElementType()!.ClrType);
Assert.True(property.IsPrimitiveCollection);
}

[ConditionalFact]
public void Can_set_derived_element_type_for_primitive_collection()
{
var model = CreateModel();
var entityType = model.AddEntityType(typeof(object));
var property = entityType.AddProperty("Random", typeof(IList<object>));
property.SetElementType(typeof(int));

Assert.Equal(typeof(int), property.GetElementType()!.ClrType);
Assert.True(property.IsPrimitiveCollection);
}

[ConditionalFact]
public void Can_set_element_type_for_non_primitive_collection()
{
var model = CreateModel();
var entityType = model.AddEntityType(typeof(object));
var property = entityType.AddProperty("Random", typeof(Random));
property.SetElementType(typeof(int));

Assert.Equal(typeof(int), property.GetElementType()!.ClrType);
Assert.False(property.IsPrimitiveCollection);
}

private class SimpleJasonValueReaderWriter : JsonValueReaderWriter<string>
{
public override string FromJsonTyped(ref Utf8JsonReaderManager manager, object existingObject = null)
Expand Down

0 comments on commit 88edb55

Please sign in to comment.