Skip to content

Commit

Permalink
Apply pre-convention configuration to nullable instances
Browse files Browse the repository at this point in the history
Fixes #25416
Fixes #25521
  • Loading branch information
AndriySvyryd authored Aug 23, 2021
1 parent 366e0c9 commit a42a71f
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 58 deletions.
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 @@ -318,7 +318,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

0 comments on commit a42a71f

Please sign in to comment.