From 161b71cbfdf264c1f07eed9099ac5bc53b396cd0 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 8 May 2023 13:08:43 +0100 Subject: [PATCH 1/3] Update handling for bools and enums with database defaults Fixes #15070 - Warn for enums like we do for bools - Don't warn if a sentinel value has been configured - Update warning message to mention sentinel value - Set the sentinel by convention for non-nullable bools with a default value --- .../Diagnostics/RelationalLoggerExtensions.cs | 19 ++- .../RelationalModelValidator.cs | 15 +- .../RelationalValueGenerationConvention.cs | 10 ++ .../Properties/RelationalStrings.Designer.cs | 10 +- .../Properties/RelationalStrings.resx | 58 ++++---- .../RelationalModelValidatorTest.cs | 130 ++++++++++++++---- .../StoreGeneratedTestBase.cs | 75 ++++++++++ .../StoreGeneratedSentinelSqlServerTest.cs | 10 ++ .../StoreGeneratedSqlServerTestBase.cs | 9 ++ .../StoreGeneratedSqliteTest.cs | 9 ++ .../Infrastructure/ModelValidatorTestBase.cs | 27 ++++ 11 files changed, 300 insertions(+), 72 deletions(-) diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index 0b6bf9c850f..a9a57a2504d 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -2505,7 +2505,14 @@ public static void BoolWithDefaultWarning( if (diagnostics.ShouldLog(definition)) { - definition.Log(diagnostics, property.Name, property.DeclaringEntityType.DisplayName()); + var defaultValue = property.ClrType.GetDefaultValue(); + definition.Log( + diagnostics, + property.ClrType.ShortDisplayName(), + property.Name, + property.DeclaringEntityType.DisplayName(), + defaultValue == null ? "null" : defaultValue.ToString()!, + property.ClrType.ShortDisplayName()); } if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) @@ -2521,9 +2528,15 @@ public static void BoolWithDefaultWarning( private static string BoolWithDefaultWarning(EventDefinitionBase definition, EventData payload) { - var d = (EventDefinition)definition; + var d = (EventDefinition)definition; var p = (PropertyEventData)payload; - return d.GenerateMessage(p.Property.Name, p.Property.DeclaringEntityType.DisplayName()); + var defaultValue = p.Property.ClrType.GetDefaultValue(); + return d.GenerateMessage( + p.Property.ClrType.ShortDisplayName(), + p.Property.Name, + p.Property.DeclaringEntityType.DisplayName(), + defaultValue == null ? "null" : defaultValue.ToString()!, + p.Property.ClrType.ShortDisplayName()); } /// diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index b86e6ea8aa4..d242f68a343 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -675,21 +675,24 @@ protected virtual void ValidateBoolsWithDefaults( { foreach (var property in entityType.GetDeclaredProperties()) { - if (property.ClrType == typeof(bool) + if (!property.ClrType.IsNullableType() + && (property.ClrType.IsEnum || property.ClrType == typeof(bool)) && property.ValueGenerated != ValueGenerated.Never - && property.FieldInfo?.FieldType != typeof(bool?) + && property.FieldInfo?.FieldType.IsNullableType() != true + && !((IConventionProperty)property).GetSentinelConfigurationSource().HasValue && (StoreObjectIdentifier.Create(property.DeclaringEntityType, StoreObjectType.Table) is { } table - && (IsNotNullAndFalse(property.GetDefaultValue(table)) + && (IsNotNullAndNotDefault(property.GetDefaultValue(table)) || property.GetDefaultValueSql(table) != null))) { logger.BoolWithDefaultWarning(property); } + + bool IsNotNullAndNotDefault(object? value) + => value != null + && !property.ClrType.IsDefaultValue(value); } } - static bool IsNotNullAndFalse(object? value) - => value != null - && (value is not bool asBool || asBool); } /// diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs index 98468aba168..fb89db8aadb 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs @@ -56,6 +56,16 @@ public virtual void ProcessPropertyAnnotationChanged( switch (name) { case RelationalAnnotationNames.DefaultValue: + if ((((IProperty)property).TryGetMemberInfo(forMaterialization: false, forSet: false, out var member, out _) + ? member!.GetMemberType() + : property.ClrType) + == typeof(bool) + && Equals(true, property.GetDefaultValue())) + { + propertyBuilder.HasSentinel(true); + } + + goto case RelationalAnnotationNames.DefaultValueSql; case RelationalAnnotationNames.DefaultValueSql: case RelationalAnnotationNames.ComputedColumnSql: propertyBuilder.ValueGenerated(GetValueGenerated(property)); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 0fe96122904..d0fb4fd1135 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -2337,9 +2337,9 @@ public static EventDefinition LogBeginningTransaction(IDiagnosticsLogger } /// - /// The 'bool' property '{property}' on entity type '{entityType}' is configured with a database-generated default. This default will always be used for inserts when the property has the value 'false', since this is the CLR default for the 'bool' type. Consider using the nullable 'bool?' type instead, so that the default will only be used for inserts when the property value is 'null'. + /// The '{type}' property '{property}' on entity type '{entityType}' is configured with a database-generated default, but has no configured sentinel value. The database-generated default will always be used for inserts when the property has the value '{defaultValue}', since this is the CLR default for the '{type2}' type. Consider using a nullable type, using a nullable backing field, or setting the sentinel value for the property to ensure the database default is used when, and only when, appropriate. See https://aka.ms/efcore-docs-default-values for more information. /// - public static EventDefinition LogBoolWithDefaultWarning(IDiagnosticsLogger logger) + public static EventDefinition LogBoolWithDefaultWarning(IDiagnosticsLogger logger) { var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogBoolWithDefaultWarning; if (definition == null) @@ -2347,18 +2347,18 @@ public static EventDefinition LogBoolWithDefaultWarning(IDiagnos definition = NonCapturingLazyInitializer.EnsureInitialized( ref ((RelationalLoggingDefinitions)logger.Definitions).LogBoolWithDefaultWarning, logger, - static logger => new EventDefinition( + static logger => new EventDefinition( logger.Options, RelationalEventId.BoolWithDefaultWarning, LogLevel.Warning, "RelationalEventId.BoolWithDefaultWarning", - level => LoggerMessage.Define( + level => LoggerMessage.Define( level, RelationalEventId.BoolWithDefaultWarning, _resourceManager.GetString("LogBoolWithDefaultWarning")!))); } - return (EventDefinition)definition; + return (EventDefinition)definition; } /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 938c9f6d455..d6969185c9a 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1,17 +1,17 @@  - @@ -593,8 +593,8 @@ Debug RelationalEventId.TransactionStarting string - The 'bool' property '{property}' on entity type '{entityType}' is configured with a database-generated default. This default will always be used for inserts when the property has the value 'false', since this is the CLR default for the 'bool' type. Consider using the nullable 'bool?' type instead, so that the default will only be used for inserts when the property value is 'null'. - Warning RelationalEventId.BoolWithDefaultWarning string string + The '{type}' property '{property}' on entity type '{entityType}' is configured with a database-generated default, but has no configured sentinel value. The database-generated default will always be used for inserts when the property has the value '{defaultValue}', since this is the CLR default for the '{type2}' type. Consider using a nullable type, using a nullable backing field, or setting the sentinel value for the property to ensure the database default is used when, and only when, appropriate. See https://aka.ms/efcore-docs-default-values for more information. + Warning RelationalEventId.BoolWithDefaultWarning string string string string string Closed connection to database '{database}' on server '{server}' ({elapsed}ms). diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index d4a06ed9143..bdb4a93fc85 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -112,15 +112,14 @@ public override void Detects_missing_discriminator_property() [ConditionalFact] public virtual void Ignores_bool_with_default_value_false() { - var modelBuilder = CreateConventionlessModelBuilder(); + var modelBuilder = CreateConventionModelBuilder(); var model = modelBuilder.Model; var entityType = model.AddEntityType(typeof(E)); - SetPrimaryKey(entityType); - entityType.AddProperty("ImNot", typeof(bool?)).SetDefaultValue(false); - entityType.AddProperty("ImNotUsed", typeof(bool)).SetDefaultValue(false); + entityType.FindProperty("ImNot")!.SetDefaultValue(false); + entityType.FindProperty("ImNotUsed")!.SetDefaultValue(false); - var property = entityType.AddProperty("ImBool", typeof(bool)); + var property = entityType.FindProperty("ImBool")!; property.SetDefaultValue(false); property.ValueGenerated = ValueGenerated.OnAdd; @@ -130,61 +129,134 @@ public virtual void Ignores_bool_with_default_value_false() } [ConditionalFact] - public virtual void Detects_bool_with_default_value_not_false() + public virtual void Bool_with_true_default_value_okay_because_sentinel_set_to_true() { - var modelBuilder = CreateConventionlessModelBuilder(); + var modelBuilder = CreateConventionModelBuilder(); var model = modelBuilder.Model; var entityType = model.AddEntityType(typeof(E)); - SetPrimaryKey(entityType); - entityType.AddProperty("ImNot", typeof(bool?)).SetDefaultValue(true); - entityType.AddProperty("ImNotUsed", typeof(bool)).SetDefaultValue(true); + entityType.FindProperty("ImNot")!.SetDefaultValue(true); + entityType.FindProperty("ImNotUsed")!.SetDefaultValue(true); - var property = entityType.AddProperty("ImBool", typeof(bool)); + var property = entityType.FindProperty("ImBool")!; property.SetDefaultValue(true); property.ValueGenerated = ValueGenerated.OnAdd; - VerifyWarning( - RelationalResources.LogBoolWithDefaultWarning(new TestLogger()) - .GenerateMessage("ImBool", "E"), modelBuilder); + Assert.True((bool)property.Sentinel!); + + Assert.DoesNotContain(LoggerFactory.Log, l => l.Level == LogLevel.Warning); } [ConditionalFact] // Issue #28509 public virtual void Bool_with_default_value_and_nullable_backing_field_is_fine() { - var modelBuilder = CreateConventionlessModelBuilder(); + var modelBuilder = CreateConventionModelBuilder(); var model = modelBuilder.Model; var entityType = model.AddEntityType(typeof(E2)); - SetPrimaryKey(entityType); - var property = entityType.AddProperty(typeof(E2).GetAnyProperty("ImBool")!); + var property = entityType.FindProperty("ImBool")!; property.SetField("_imBool"); property.SetDefaultValue(true); property.ValueGenerated = ValueGenerated.OnAdd; - VerifyLogDoesNotContain( - RelationalResources.LogBoolWithDefaultWarning(new TestLogger()) - .GenerateMessage("ImBool", "E2"), modelBuilder); + Assert.DoesNotContain(LoggerFactory.Log, l => l.Level == LogLevel.Warning); } [ConditionalFact] public virtual void Detects_bool_with_default_expression() { - var modelBuilder = CreateConventionlessModelBuilder(); + var modelBuilder = CreateConventionModelBuilder(); var model = modelBuilder.Model; var entityType = model.AddEntityType(typeof(E)); - SetPrimaryKey(entityType); - entityType.AddProperty("ImNot", typeof(bool?)).SetDefaultValueSql("TRUE"); - entityType.AddProperty("ImNotUsed", typeof(bool)).SetDefaultValueSql("TRUE"); - - var property = entityType.AddProperty("ImBool", typeof(bool)); + entityType.FindProperty("ImNot")!.SetDefaultValueSql("TRUE"); + var property = entityType.FindProperty("ImBool")!; property.SetDefaultValueSql("TRUE"); property.ValueGenerated = ValueGenerated.OnAddOrUpdate; VerifyWarning( RelationalResources.LogBoolWithDefaultWarning(new TestLogger()) - .GenerateMessage("ImBool", "E"), modelBuilder); + .GenerateMessage("bool", "ImBool", "E", "False", "bool"), modelBuilder); + } + + [ConditionalFact] + public virtual void Ignores_enum_with_default_value_matching_CLR_default() + { + var modelBuilder = CreateConventionModelBuilder(); + var model = modelBuilder.Model; + + var entityType = model.AddEntityType(typeof(WithEnum)); + entityType.FindProperty(nameof(WithEnum.EnumWithDefaultConstraint))!.SetDefaultValue(X.A); + entityType.FindProperty(nameof(WithEnum.NullableEnum))!.SetDefaultValue(X.B); + + Validate(modelBuilder); + + Assert.DoesNotContain(LoggerFactory.Log, l => l.Level == LogLevel.Warning); + } + + [ConditionalFact] + public virtual void Detects_enum_with_database_default_not_set_to_CLR_default() + { + var modelBuilder = CreateConventionModelBuilder(); + var model = modelBuilder.Model; + + var entityType = model.AddEntityType(typeof(WithEnum)); + entityType.FindProperty(nameof(WithEnum.EnumWithDefaultConstraint))!.SetDefaultValue(X.B); + entityType.FindProperty(nameof(WithEnum.NullableEnum))!.SetDefaultValue(X.B); + + Validate(modelBuilder); + + VerifyWarning( + RelationalResources.LogBoolWithDefaultWarning(new TestLogger()) + .GenerateMessage("X", "EnumWithDefaultConstraint", "WithEnum", "A", "X"), modelBuilder); + } + + [ConditionalFact] + public virtual void Enum_with_database_default_not_set_to_CLR_default_okay_if_sentinel_set() + { + var modelBuilder = CreateConventionModelBuilder(); + var model = modelBuilder.Model; + + var entityType = model.AddEntityType(typeof(WithEnum)); + var property = entityType.FindProperty(nameof(WithEnum.EnumWithDefaultConstraint))!; + property.SetDefaultValue(X.B); + property.Sentinel = X.B; + entityType.FindProperty(nameof(WithEnum.NullableEnum))!.SetDefaultValue(X.B); + + Validate(modelBuilder); + + Assert.DoesNotContain(LoggerFactory.Log, l => l.Level == LogLevel.Warning); + } + + [ConditionalFact] + public virtual void Enum_with_database_default_not_set_to_CLR_default_and_nullable_backing_field_is_fine() + { + var modelBuilder = CreateConventionModelBuilder(); + var model = modelBuilder.Model; + + var entityType = model.AddEntityType(typeof(WithEnum2)); + entityType.FindProperty(nameof(WithEnum2.EnumWithDefaultConstraint))!.SetDefaultValue(X.B); + + Validate(modelBuilder); + + Assert.DoesNotContain(LoggerFactory.Log, l => l.Level == LogLevel.Warning); + } + + [ConditionalFact] + public virtual void Detects_enum_with_default_expression() + { + var modelBuilder = CreateConventionModelBuilder(); + var model = modelBuilder.Model; + + var entityType = model.AddEntityType(typeof(WithEnum)); + entityType.FindProperty(nameof(WithEnum.EnumWithDefaultConstraint))!.SetDefaultValueSql("SQL"); + entityType.FindProperty(nameof(WithEnum.NullableEnum))!.SetDefaultValueSql("SQL"); + + Validate(modelBuilder); + + VerifyWarning( + RelationalResources.LogBoolWithDefaultWarning(new TestLogger()) + .GenerateMessage("X", "EnumWithDefaultConstraint", "WithEnum", "A", "X"), modelBuilder); } [ConditionalFact] @@ -195,10 +267,10 @@ public virtual void Detects_primary_key_with_default_value() var entityA = model.AddEntityType(typeof(A)); SetPrimaryKey(entityA); - entityA.FindProperty("Id").SetDefaultValue(1); + entityA.FindProperty("Id")!.SetDefaultValue(1); AddProperties(entityA); - entityA.FindProperty("Id").SetDefaultValue(1); + entityA.FindProperty("Id")!.SetDefaultValue(1); VerifyWarning( RelationalResources.LogKeyHasDefaultValue( diff --git a/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs b/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs index f784d9d387b..34c08b76fc8 100644 --- a/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs +++ b/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs @@ -1333,6 +1333,72 @@ public virtual void Nullable_fields_get_defaults_when_not_set() Assert.Equal(0, entity.NullableBackedIntZeroDefault); }); + [ConditionalFact] + public virtual void Properties_get_database_defaults_when_set_to_sentinel_values() + => ExecuteWithStrategyInTransaction( + context => + { + var entity = new WithNoBackingFields + { + Id = Fixture.IntSentinel, + TrueDefault = true, // Bool sentinel is always true by convention when default value is true + NonZeroDefault = Fixture.IntSentinel, + FalseDefault = Fixture.BoolSentinel, + ZeroDefault = Fixture.IntSentinel + }; + + context.Add(entity); + + context.SaveChanges(); + + Assert.NotEqual(0, entity.Id); + Assert.True(entity.TrueDefault); + Assert.Equal(-1, entity.NonZeroDefault); + Assert.False(entity.FalseDefault); + Assert.Equal(0, entity.ZeroDefault); + }, + context => + { + var entity = context.Set().Single(); + Assert.True(entity.TrueDefault); + Assert.Equal(-1, entity.NonZeroDefault); + Assert.False(entity.FalseDefault); + Assert.Equal(0, entity.ZeroDefault); + }); + + [ConditionalFact] + public virtual void Properties_get_set_values_when_not_set_to_sentinel_values() + => ExecuteWithStrategyInTransaction( + context => + { + var entity = new WithNoBackingFields + { + Id = Fixture.IntSentinel, + TrueDefault = false, // Bool sentinel is always true by convention when default value is true + NonZeroDefault = 3, + FalseDefault = !Fixture.BoolSentinel, + ZeroDefault = 5 + }; + + context.Add(entity); + + context.SaveChanges(); + + Assert.NotEqual(0, entity.Id); + Assert.False(entity.TrueDefault); + Assert.Equal(3, entity.NonZeroDefault); + Assert.Equal(!Fixture.BoolSentinel, entity.FalseDefault); + Assert.Equal(5, entity.ZeroDefault); + }, + context => + { + var entity = context.Set().Single(); + Assert.False(entity.TrueDefault); + Assert.Equal(3, entity.NonZeroDefault); + Assert.Equal(!Fixture.BoolSentinel, entity.FalseDefault); + Assert.Equal(5, entity.ZeroDefault); + }); + [ConditionalFact] public virtual void Nullable_fields_store_non_defaults_when_set() => ExecuteWithStrategyInTransaction( @@ -1690,6 +1756,15 @@ public int? NonNullableAsNullable } } + protected class WithNoBackingFields + { + public int Id { get; set; } + public bool TrueDefault { get; set; } + public int NonZeroDefault { get; set; } + public bool FalseDefault { get; set; } + public int ZeroDefault { get; set; } + } + protected class WithNullableBackingFields { public static WithNullableBackingFields Create(int? intSentinel, bool? boolSentinel) diff --git a/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSentinelSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSentinelSqlServerTest.cs index 6765ae7401f..3adde73a103 100644 --- a/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSentinelSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSentinelSqlServerTest.cs @@ -175,6 +175,16 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.Property(e => e.NonNullableAsNullable).HasSentinel(IntSentinel); }); + modelBuilder.Entity( + b => + { + b.Property(e => e.Id).HasSentinel(IntSentinel); + b.Property(e => e.TrueDefault).HasSentinel(BoolSentinel); + b.Property(e => e.NonZeroDefault).HasSentinel(IntSentinel); + b.Property(e => e.FalseDefault).HasSentinel(BoolSentinel); + b.Property(e => e.ZeroDefault).HasSentinel(IntSentinel); + }); + modelBuilder.Entity( b => { diff --git a/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTestBase.cs b/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTestBase.cs index e9d2663e068..1c9e1b19377 100644 --- a/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTestBase.cs +++ b/test/EFCore.SqlServer.FunctionalTests/StoreGeneratedSqlServerTestBase.cs @@ -888,6 +888,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.Property(e => e.NonNullableAsNullable).HasComputedColumnSql("1"); }); + modelBuilder.Entity( + b => + { + b.Property(e => e.TrueDefault).HasDefaultValue(true); + b.Property(e => e.NonZeroDefault).HasDefaultValue(-1); + b.Property(e => e.FalseDefault).HasDefaultValue(false); + b.Property(e => e.ZeroDefault).HasDefaultValue(0); + }); + modelBuilder.Entity( b => { diff --git a/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs index 73f9e891480..5efc717b1be 100644 --- a/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/StoreGeneratedSqliteTest.cs @@ -103,6 +103,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.Property(e => e.OnUpdateThrowBeforeThrowAfter).HasDefaultValue("Rabbit"); }); + modelBuilder.Entity( + b => + { + b.Property(e => e.TrueDefault).HasDefaultValue(true); + b.Property(e => e.NonZeroDefault).HasDefaultValue(-1); + b.Property(e => e.FalseDefault).HasDefaultValue(false); + b.Property(e => e.ZeroDefault).HasDefaultValue(0); + }); + modelBuilder.Entity( b => { diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs index 21d0475aa2c..fd4ff4ac6ba 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs @@ -180,6 +180,33 @@ public bool ImBool } } + protected enum X + { + A, + B + } + + protected class WithEnum + { + public int Id { get; set; } + public X EnumWithDefaultConstraint { get; set; } + public X EnumNoDefaultConstraint { get; set; } + public X? NullableEnum { get; set; } + } + + protected class WithEnum2 + { + private X? _enumWithDefaultConstraint; + + public int Id { get; set; } + + public X EnumWithDefaultConstraint + { + get => _enumWithDefaultConstraint ?? X.B; + set => _enumWithDefaultConstraint = value; + } + } + protected class EntityWithInvalidProperties { public int Id { get; set; } From 45e5903ec3742623b645c3283052f4a3550bf5e6 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 8 May 2023 16:02:04 +0100 Subject: [PATCH 2/3] Don't fail build on internal API usage --- .../Infrastructure/RelationalModelValidator.cs | 2 ++ .../Metadata/Conventions/RelationalValueGenerationConvention.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index d242f68a343..eeea8151f91 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -689,7 +689,9 @@ protected virtual void ValidateBoolsWithDefaults( bool IsNotNullAndNotDefault(object? value) => value != null +#pragma warning disable EF1001 // Internal EF Core API usage. && !property.ClrType.IsDefaultValue(value); +#pragma warning restore EF1001 // Internal EF Core API usage. } } diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs index fb89db8aadb..0b0aeaf704b 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs @@ -56,9 +56,11 @@ public virtual void ProcessPropertyAnnotationChanged( switch (name) { case RelationalAnnotationNames.DefaultValue: +#pragma warning disable EF1001 // Internal EF Core API usage. if ((((IProperty)property).TryGetMemberInfo(forMaterialization: false, forSet: false, out var member, out _) ? member!.GetMemberType() : property.ClrType) +#pragma warning restore EF1001 // Internal EF Core API usage. == typeof(bool) && Equals(true, property.GetDefaultValue())) { From f9bafb9d055adf590fe307bfd5152dc9580704a2 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Tue, 9 May 2023 09:13:39 +0100 Subject: [PATCH 3/3] Only set sentinel when default value annotation is set --- .../Metadata/Conventions/RelationalValueGenerationConvention.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs index 0b0aeaf704b..b480b83dab7 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalValueGenerationConvention.cs @@ -64,7 +64,7 @@ public virtual void ProcessPropertyAnnotationChanged( == typeof(bool) && Equals(true, property.GetDefaultValue())) { - propertyBuilder.HasSentinel(true); + propertyBuilder.HasSentinel(annotation != null ? true : null); } goto case RelationalAnnotationNames.DefaultValueSql;