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

Apply pre-convention configuration to nullable instances #25638

Merged
merged 1 commit into from
Aug 23, 2021
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
104 changes: 62 additions & 42 deletions src/EFCore/Metadata/Internal/ModelConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,56 @@ public ModelConfiguration()
public virtual bool IsEmpty()
=> _properties.Count == 0 && _ignoredTypes.Count == 0 && _typeMappings.Count == 0;

/// <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 ModelConfiguration Validate()
{
Type? configuredType = null;
var stringType = GetConfigurationType(typeof(string), null, ref configuredType);
if (stringType != null
&& stringType != TypeConfigurationType.Property)
{
throw new InvalidOperationException(
CoreStrings.UnconfigurableType(
typeof(string).DisplayName(fullName: false),
stringType,
TypeConfigurationType.Property,
configuredType!.DisplayName(fullName: false)));
}

configuredType = null;
var intType = GetConfigurationType(typeof(int?), null, ref configuredType);
if (intType != null
&& intType != TypeConfigurationType.Property)
{
throw new InvalidOperationException(
CoreStrings.UnconfigurableType(
typeof(int?).DisplayName(fullName: false),
intType,
TypeConfigurationType.Property,
configuredType!.DisplayName(fullName: false)));
}

configuredType = null;
var propertyBagType = GetConfigurationType(Model.DefaultPropertyBagType, null, ref configuredType);
if (propertyBagType != null
&& !propertyBagType.Value.IsEntityType())
{
throw new InvalidOperationException(
CoreStrings.UnconfigurableType(
Model.DefaultPropertyBagType.DisplayName(fullName: false),
propertyBagType,
TypeConfigurationType.SharedTypeEntityType,
configuredType!.DisplayName(fullName: false)));
}

return this;
}

/// <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 Expand Up @@ -69,6 +119,12 @@ public virtual bool IsEmpty()

Type? configuredType = null;

if (type.IsNullableValueType())
{
configurationType = GetConfigurationType(
Nullable.GetUnderlyingType(type)!, configurationType, ref configuredType, getBaseTypes: false);
}

if (type.IsConstructedGenericType)
{
configurationType = GetConfigurationType(
Expand Down Expand Up @@ -178,23 +234,6 @@ public virtual PropertyConfiguration GetOrAddProperty(Type type)
var property = FindProperty(type);
if (property == null)
{
if (type == typeof(object)
|| type == typeof(ExpandoObject)
|| type == typeof(SortedDictionary<string, object>)
|| type == typeof(Dictionary<string, object>)
|| type == typeof(IDictionary<string, object>)
|| type == typeof(IReadOnlyDictionary<string, object>)
|| type == typeof(IDictionary)
|| type == typeof(ICollection<KeyValuePair<string, object>>)
|| type == typeof(IReadOnlyCollection<KeyValuePair<string, object>>)
|| type == typeof(ICollection)
|| type == typeof(IEnumerable<KeyValuePair<string, object>>)
|| type == typeof(IEnumerable))
{
throw new InvalidOperationException(
CoreStrings.UnconfigurableType(type.DisplayName(fullName: false), TypeConfigurationType.Property));
}

RemoveIgnored(type);

property = new PropertyConfiguration(type);
Expand Down Expand Up @@ -232,8 +271,8 @@ public virtual bool RemoveProperty(Type type)
/// </summary>
public virtual PropertyConfiguration GetOrAddTypeMapping(Type type)
{
var scalar = FindTypeMapping(type);
if (scalar == null)
var typeMappingConfiguration = FindTypeMapping(type);
if (typeMappingConfiguration == null)
{
if (type == typeof(object)
|| type == typeof(ExpandoObject)
Expand All @@ -243,14 +282,14 @@ public virtual PropertyConfiguration GetOrAddTypeMapping(Type type)
|| !type.IsInstantiable())
{
throw new InvalidOperationException(
CoreStrings.UnconfigurableType(type.DisplayName(fullName: false), "DefaultTypeMapping"));
CoreStrings.UnconfigurableTypeMapping(type.DisplayName(fullName: false)));
}

scalar = new PropertyConfiguration(type);
_typeMappings.Add(type, scalar);
typeMappingConfiguration = new PropertyConfiguration(type);
_typeMappings.Add(type, typeMappingConfiguration);
}

return scalar;
return typeMappingConfiguration;
}

/// <summary>
Expand All @@ -272,25 +311,6 @@ public virtual PropertyConfiguration GetOrAddTypeMapping(Type type)
/// </summary>
public virtual void AddIgnored(Type type)
{
if (type.UnwrapNullableType() == typeof(int)
|| type == typeof(string)
|| type == typeof(object)
|| type == typeof(ExpandoObject)
|| type == typeof(SortedDictionary<string, object>)
|| type == typeof(Dictionary<string, object>)
|| type == typeof(IDictionary<string, object>)
|| type == typeof(IReadOnlyDictionary<string, object>)
|| type == typeof(IDictionary)
|| type == typeof(ICollection<KeyValuePair<string, object>>)
|| type == typeof(IReadOnlyCollection<KeyValuePair<string, object>>)
|| type == typeof(ICollection)
|| type == typeof(IEnumerable<KeyValuePair<string, object>>)
|| type == typeof(IEnumerable))
{
throw new InvalidOperationException(
CoreStrings.UnconfigurableType(type.DisplayName(fullName: false), TypeConfigurationType.Ignored));
}

RemoveProperty(type);
_ignoredTypes.Add(type);
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/ModelConfigurationBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ public virtual ModelConfigurationBuilder DefaultTypeMapping(
/// <param name="modelDependencies"> The dependencies object used during model building. </param>
/// <returns> The configured <see cref="ModelBuilder" />. </returns>
public virtual ModelBuilder CreateModelBuilder(ModelDependencies? modelDependencies)
=> new(_conventions, modelDependencies, _modelConfiguration.IsEmpty() ? null : _modelConfiguration);
=> new(_conventions, modelDependencies, _modelConfiguration.IsEmpty() ? null : _modelConfiguration.Validate());

#region Hidden System.Object members

Expand Down
18 changes: 14 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.

5 changes: 4 additions & 1 deletion src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1501,7 +1501,10 @@
<value>Unable to set 'IsUnique' to '{isUnique}' on the relationship associated with the navigation '{2_entityType}.{1_navigationName}' because the navigation has the opposite multiplicity.</value>
</data>
<data name="UnconfigurableType" xml:space="preserve">
<value>The type '{type}' cannot be configured as '{configuration}'. The current model building logic is unable to honor this configuration.</value>
<value>The type '{type}' cannot be configured as '{configuration}' since model building assumes that it is configured as '{expectedConfiguration}'. Remove the unsupported configuration for '{configurationType}'.</value>
</data>
<data name="UnconfigurableTypeMapping" xml:space="preserve">
<value>Default type mapping cannot be configured for the type '{type}' since it's not a valid scalar type. Remove the unsupported configuration.</value>
</data>
<data name="UnhandledExpressionNode" xml:space="preserve">
<value>Unhandled expression node type '{nodeType}'.</value>
Expand Down
7 changes: 6 additions & 1 deletion src/Shared/SharedTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public static Type UnwrapNullableType(this Type type)
=> Nullable.GetUnderlyingType(type) ?? type;

public static bool IsNullableValueType(this Type type)
=> type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>);
=> type.IsConstructedGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>);

public static bool IsNullableType(this Type type)
=> !type.IsValueType || type.IsNullableValueType();
Expand Down Expand Up @@ -312,6 +312,11 @@ public static List<Type> GetBaseTypesAndInterfacesInclusive(this Type type)
type = typesToProcess.Dequeue();
baseTypes.Add(type);

if (type.IsNullableValueType())
{
typesToProcess.Enqueue(Nullable.GetUnderlyingType(type)!);
}

if (type.IsConstructedGenericType)
{
typesToProcess.Enqueue(type.GetGenericTypeDefinition());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public TestModelBuilder CreateModelBuilder(
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> validationLogger)
=> new(Conventions,
modelDependencies,
ModelConfiguration.IsEmpty() ? null : ModelConfiguration,
ModelConfiguration.IsEmpty() ? null : ModelConfiguration.Validate(),
modelRuntimeInitializer,
validationLogger);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,15 @@ public virtual void Can_set_store_type_for_property_type()
{
c.Properties<int>().HaveColumnType("smallint");
c.Properties<string>().HaveColumnType("nchar(max)");
c.Properties(typeof(Nullable<>)).HavePrecision(2);
});

modelBuilder.Entity<Quarks>(
b =>
{
b.Property<int>("Charm");
b.Property<string>("Strange");
b.Property<int>("Top");
b.Property<int?>("Top");
b.Property<string>("Bottom");
});

Expand All @@ -101,9 +102,13 @@ public virtual void Can_set_store_type_for_property_type()
Assert.Equal("smallint", entityType.FindProperty(Customer.IdProperty.Name).GetColumnType());
Assert.Equal("smallint", entityType.FindProperty("Up").GetColumnType());
Assert.Equal("nchar(max)", entityType.FindProperty("Down").GetColumnType());
Assert.Equal("smallint", entityType.FindProperty("Charm").GetColumnType());
var charm = entityType.FindProperty("Charm");
Assert.Equal("smallint", charm.GetColumnType());
Assert.Null(charm.GetPrecision());
Assert.Equal("nchar(max)", entityType.FindProperty("Strange").GetColumnType());
Assert.Equal("smallint", entityType.FindProperty("Top").GetColumnType());
var top = entityType.FindProperty("Top");
Assert.Equal("smallint", top.GetColumnType());
Assert.Equal(2, top.GetPrecision());
Assert.Equal("nchar(max)", entityType.FindProperty("Bottom").GetColumnType());
}

Expand Down
10 changes: 5 additions & 5 deletions test/EFCore.Tests/ModelBuilding/NonRelationshipTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -419,14 +419,14 @@ public virtual void Properties_can_be_ignored_by_type()
[ConditionalFact]
public virtual void Int32_cannot_be_ignored()
{
Assert.Equal(CoreStrings.UnconfigurableType("int", "Ignored"),
Assert.Equal(CoreStrings.UnconfigurableType("int?", "Ignored", "Property", "int"),
Assert.Throws<InvalidOperationException>(() => CreateModelBuilder(c => c.IgnoreAny<int>())).Message);
}

[ConditionalFact]
public virtual void Object_cannot_be_ignored()
{
Assert.Equal(CoreStrings.UnconfigurableType("object", "Ignored"),
Assert.Equal(CoreStrings.UnconfigurableType("string", "Ignored", "Property", "object"),
Assert.Throws<InvalidOperationException>(() => CreateModelBuilder(c => c.IgnoreAny<object>())).Message);
}

Expand Down Expand Up @@ -1410,17 +1410,17 @@ protected class StringCollectionEntity
[ConditionalFact]
public virtual void Object_cannot_be_configured_as_property()
{
Assert.Equal(CoreStrings.UnconfigurableType("object", "Property"),
Assert.Equal(CoreStrings.UnconfigurableType("Dictionary<string, object>", "Property", "SharedTypeEntityType", "object"),
Assert.Throws<InvalidOperationException>(() => CreateModelBuilder(c => c.Properties<object>())).Message);
}

[ConditionalFact]
public virtual void Property_bag_cannot_be_configured_as_property()
{
Assert.Equal(CoreStrings.UnconfigurableType("Dictionary<string, object>", "Property"),
Assert.Equal(CoreStrings.UnconfigurableType("Dictionary<string, object>", "Property", "SharedTypeEntityType", "Dictionary<string, object>"),
Assert.Throws<InvalidOperationException>(() => CreateModelBuilder(c => c.Properties<Dictionary<string, object>>())).Message);

Assert.Equal(CoreStrings.UnconfigurableType("IDictionary<string, object>", "Property"),
Assert.Equal(CoreStrings.UnconfigurableType("Dictionary<string, object>", "Property", "SharedTypeEntityType", "IDictionary<string, object>"),
Assert.Throws<InvalidOperationException>(() => CreateModelBuilder(c => c.Properties<IDictionary<string, object>>())).Message);
}

Expand Down
3 changes: 3 additions & 0 deletions tools/Resources.tt
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
#>
// <auto-generated />

using System;
using System.Reflection;
using System.Resources;
<#
if (!model.NoDiagnostics)
{
#>
using System.Threading;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.Extensions.Logging;
Expand Down
2 changes: 2 additions & 0 deletions tools/SqliteResources.tt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#>
// <auto-generated />

using System;
using System.Reflection;
using System.Resources;

#nullable enable
Expand Down