From 9a546fcf90d80d5c2404ecea2c7307fb5fc6c462 Mon Sep 17 00:00:00 2001 From: lajones Date: Tue, 11 Feb 2020 16:05:32 -0800 Subject: [PATCH 1/3] SqlServerValueGenerationStrategy which conflicts now causes warning instead of error --- ...ctingValueGenerationStrategiesEventData.cs | 53 +++++++++++++++++++ .../Internal/SqlServerLoggingDefinitions.cs | 8 +++ .../Diagnostics/SqlServerEventId.cs | 15 ++++++ ...ServerDbContextOptionsBuilderExtensions.cs | 8 ++- .../Internal/SqlServerLoggerExtensions.cs | 44 +++++++++++++++ .../SqlServerStoreGenerationConvention.cs | 18 +++---- .../Properties/SqlServerStrings.Designer.cs | 24 +++++++++ .../Properties/SqlServerStrings.resx | 4 ++ .../SqlServerModelValidatorTest.cs | 47 ++++++++++++++-- .../Infrastructure/ModelValidatorTestBase.cs | 13 +++++ 10 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs diff --git a/src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs b/src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs new file mode 100644 index 00000000000..1b5d84a1b95 --- /dev/null +++ b/src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs @@ -0,0 +1,53 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Diagnostics; +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata; + +namespace Microsoft.EntityFrameworkCore.Diagnostics +{ + /// + /// A event payload class for events that have + /// a property. + /// + public class ConflictingValueGenerationStrategiesEventData : EventData + { + /// + /// Constructs the event payload. + /// + /// The event definition. + /// A delegate that generates a log message for this event. + /// The SQL Server value generation strategy. + /// The other value generation strategy. + /// The property. + public ConflictingValueGenerationStrategiesEventData( + [NotNull] EventDefinitionBase eventDefinition, + [NotNull] Func messageGenerator, + [NotNull] string sqlServerValueGenerationStrategy, + [NotNull] string otherValueGenerationStrategy, + [NotNull] IProperty property) + : base(eventDefinition, messageGenerator) + { + SqlServerValueGenerationStrategy = sqlServerValueGenerationStrategy; + OtherValueGenerationStrategy = otherValueGenerationStrategy; + Property = property; + } + + /// + /// The SQL Server value generation strategy. + /// + public virtual string SqlServerValueGenerationStrategy { get; } + + /// + /// The other value generation strategy. + /// + public virtual string OtherValueGenerationStrategy { get; } + + /// + /// The property. + /// + public virtual IProperty Property { get; } + } +} diff --git a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs index cf12db3d627..512a28960b6 100644 --- a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs +++ b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs @@ -140,5 +140,13 @@ public class SqlServerLoggingDefinitions : RelationalLoggingDefinitions /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public EventDefinitionBase LogReflexiveConstraintIgnored; + + /// + /// 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. + /// + public EventDefinitionBase LogConflictingValueGenerationStrategies; } } diff --git a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs index b5df7311019..a1a816e8b6b 100644 --- a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs +++ b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs @@ -26,6 +26,7 @@ private enum Id // Model validation events DecimalTypeDefaultWarning = CoreEventId.ProviderBaseId, ByteIdentityColumnWarning, + ConflictingValueGenerationStrategiesWarning, // Scaffolding events ColumnFound = CoreEventId.ProviderDesignBaseId, @@ -89,6 +90,20 @@ private enum Id /// public static readonly EventId ByteIdentityColumnWarning = MakeValidationId(Id.ByteIdentityColumnWarning); + /// + /// + /// There are conflicting value generation methods for a property. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the + /// payload when used with a . + /// + /// + public static readonly EventId ConflictingValueGenerationStrategiesWarning = MakeValidationId(Id.ConflictingValueGenerationStrategiesWarning); + private static readonly string _scaffoldingPrefix = DbLoggerCategory.Scaffolding.Name + "."; private static EventId MakeScaffoldingId(Id id) => new EventId((int)id, _scaffoldingPrefix + id); diff --git a/src/EFCore.SqlServer/Extensions/SqlServerDbContextOptionsBuilderExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerDbContextOptionsBuilderExtensions.cs index 946dc05f8bc..cc3e36c0a51 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerDbContextOptionsBuilderExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerDbContextOptionsBuilderExtensions.cs @@ -169,9 +169,13 @@ var coreOptionsExtension = optionsBuilder.Options.FindExtension() ?? new CoreOptionsExtension(); - coreOptionsExtension = coreOptionsExtension.WithWarningsConfiguration( + coreOptionsExtension = coreOptionsExtension. + WithWarningsConfiguration( coreOptionsExtension.WarningsConfiguration.TryWithExplicit( - RelationalEventId.AmbientTransactionWarning, WarningBehavior.Throw)); + RelationalEventId.AmbientTransactionWarning, WarningBehavior.Throw)). + WithWarningsConfiguration( + coreOptionsExtension.WarningsConfiguration.TryWithExplicit( + SqlServerEventId.ConflictingValueGenerationStrategiesWarning, WarningBehavior.Throw)); ((IDbContextOptionsBuilderInfrastructure)optionsBuilder).AddOrUpdateExtension(coreOptionsExtension); } diff --git a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs index 1343fdc7ddf..a566edc4047 100644 --- a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs +++ b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs @@ -90,6 +90,50 @@ private static string ByteIdentityColumnWarning(EventDefinitionBase definition, p.Property.DeclaringEntityType.DisplayName()); } + /// + /// 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. + /// + public static void ConflictingValueGenerationStrategiesWarning( + [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] string sqlServerValueGenerationStrategy, + [NotNull] string otherValueGenerationStrategy, + [NotNull] IProperty property) + { + var definition = SqlServerResources.LogConflictingValueGenerationStrategies(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, sqlServerValueGenerationStrategy, otherValueGenerationStrategy, + property.Name, property.DeclaringEntityType.DisplayName()); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new ConflictingValueGenerationStrategiesEventData( + definition, + ConflictingValueGenerationStrategiesWarning, + sqlServerValueGenerationStrategy, + otherValueGenerationStrategy, + property); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string ConflictingValueGenerationStrategiesWarning(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (ConflictingValueGenerationStrategiesEventData)payload; + return d.GenerateMessage( + p.SqlServerValueGenerationStrategy, + p.OtherValueGenerationStrategy, + p.Property.Name, + p.Property.DeclaringEntityType.DisplayName()); + } + /// /// 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 diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs index a750e93fb61..7666541f6ec 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs @@ -6,6 +6,7 @@ using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; +using Microsoft.EntityFrameworkCore.SqlServer.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal; // ReSharper disable once CheckNamespace @@ -102,25 +103,24 @@ protected override void Validate(IConventionProperty property) if (property.GetValueGenerationStrategyConfigurationSource() != null && property.GetValueGenerationStrategy() != SqlServerValueGenerationStrategy.None) { + var generationStrategy = property.GetValueGenerationStrategy().ToString(); + if (property.GetDefaultValue() != null) { - throw new InvalidOperationException( - RelationalStrings.ConflictingColumnServerGeneration( - "SqlServerValueGenerationStrategy", property.Name, "DefaultValue")); + Dependencies.ValidationLogger.ConflictingValueGenerationStrategiesWarning( + generationStrategy, "DefaultValue", property); } if (property.GetDefaultValueSql() != null) { - throw new InvalidOperationException( - RelationalStrings.ConflictingColumnServerGeneration( - "SqlServerValueGenerationStrategy", property.Name, "DefaultValueSql")); + Dependencies.ValidationLogger.ConflictingValueGenerationStrategiesWarning( + generationStrategy, "DefaultValueSql", property); } if (property.GetComputedColumnSql() != null) { - throw new InvalidOperationException( - RelationalStrings.ConflictingColumnServerGeneration( - "SqlServerValueGenerationStrategy", property.Name, "ComputedColumnSql")); + Dependencies.ValidationLogger.ConflictingValueGenerationStrategiesWarning( + generationStrategy, "ComputedColumnSql", property); } } diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 569b4699171..15031041ad6 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -572,5 +572,29 @@ public static EventDefinition LogReflexiveConstraintIgnored([Not return (EventDefinition)definition; } + + /// + /// Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences. + /// + public static EventDefinition LogConflictingValueGenerationStrategies([NotNull] IDiagnosticsLogger logger) + { + var definition = ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogConflictingValueGenerationStrategies; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogConflictingValueGenerationStrategies, + () => new EventDefinition( + logger.Options, + SqlServerEventId.ConflictingValueGenerationStrategiesWarning, + LogLevel.Warning, + "SqlServerEventId.ConflictingValueGenerationStrategiesWarning", + level => LoggerMessage.Define( + level, + SqlServerEventId.ConflictingValueGenerationStrategiesWarning, + _resourceManager.GetString("LogConflictingValueGenerationStrategies")))); + } + + return (EventDefinition)definition; + } } } diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index 07b4bdd0d67..a5cd31271e5 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -241,4 +241,8 @@ The '{methodName}' method is not supported because the query has switched to client-evaluation. Inspect the log to determine which query expressions are triggering client-evaluation. + + Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences. + Warning SqlServerEventId.ConflictingValueGenerationStrategiesWarning string string string string + \ No newline at end of file diff --git a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs index e68da90d3b6..7d31414ca98 100644 --- a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs +++ b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs @@ -3,6 +3,7 @@ using System; using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Diagnostics.Internal; @@ -281,9 +282,6 @@ public void Passes_for_non_key_SequenceHiLo_on_model() [InlineData("DefaultValue", "DefaultValueSql")] [InlineData("DefaultValue", "ComputedColumnSql")] [InlineData("DefaultValueSql", "ComputedColumnSql")] - [InlineData("SqlServerValueGenerationStrategy", "DefaultValue")] - [InlineData("SqlServerValueGenerationStrategy", "DefaultValueSql")] - [InlineData("SqlServerValueGenerationStrategy", "ComputedColumnSql")] public void Metadata_throws_when_setting_conflicting_serverGenerated_values(string firstConfiguration, string secondConfiguration) { var modelBuilder = CreateConventionalModelBuilder(); @@ -298,6 +296,49 @@ public void Metadata_throws_when_setting_conflicting_serverGenerated_values(stri modelBuilder.Model); } + [ConditionalTheory] + [InlineData(SqlServerValueGenerationStrategy.IdentityColumn, "DefaultValueSql")] + [InlineData(SqlServerValueGenerationStrategy.IdentityColumn, "ComputedColumnSql")] + [InlineData(SqlServerValueGenerationStrategy.SequenceHiLo, "DefaultValueSql")] + [InlineData(SqlServerValueGenerationStrategy.SequenceHiLo, "ComputedColumnSql")] + public void SqlServerValueGenerationStrategy_warns_when_setting_conflicting_value_generation_strategies( + SqlServerValueGenerationStrategy sqlServerValueGenerationStrategy, string conflictingValueGenerationStrategy) + { + var modelBuilder = CreateConventionalModelBuilder(); + + var propertyBuilder = modelBuilder.Entity().Property("Id"); + + propertyBuilder.Metadata.SetValueGenerationStrategy(sqlServerValueGenerationStrategy); + ConfigureProperty(propertyBuilder.Metadata, conflictingValueGenerationStrategy, "NEXT VALUE FOR [Id]"); + + VerifyWarning( + SqlServerResources.LogConflictingValueGenerationStrategies(new TestLogger()) + .GenerateMessage(sqlServerValueGenerationStrategy.ToString(), conflictingValueGenerationStrategy, "Id", nameof(Dog)), + modelBuilder.Model); + } + + [ConditionalTheory] + [InlineData(SqlServerValueGenerationStrategy.IdentityColumn)] + [InlineData(SqlServerValueGenerationStrategy.SequenceHiLo)] + public void SqlServerValueGenerationStrategy_warns_when_setting_conflicting_DefaultValue( + SqlServerValueGenerationStrategy sqlServerValueGenerationStrategy) + { + var modelBuilder = CreateConventionalModelBuilder(); + + var propertyBuilder = modelBuilder.Entity().Property("Id"); + + propertyBuilder.Metadata.SetValueGenerationStrategy(sqlServerValueGenerationStrategy); + ConfigureProperty(propertyBuilder.Metadata, "DefaultValue", "2"); + + VerifyWarnings( new[] { + SqlServerResources.LogConflictingValueGenerationStrategies(new TestLogger()) + .GenerateMessage(sqlServerValueGenerationStrategy.ToString(), "DefaultValue", "Id", nameof(Dog)), + RelationalResources.LogKeyHasDefaultValue(new TestLogger()) + .GenerateMessage("Id", nameof(Dog)) + }, + modelBuilder.Model); + } + protected virtual void ConfigureProperty(IMutableProperty property, string configuration, string value) { switch (configuration) diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs index 245ce62e18f..36066a474ae 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs @@ -255,6 +255,19 @@ protected virtual void VerifyWarning(string expectedMessage, IMutableModel model Assert.Equal(expectedMessage, logEntry.Message); } + protected virtual void VerifyWarnings(string[] expectedMessages, IMutableModel model, LogLevel level = LogLevel.Warning) + { + Validate(model); + var logEntries = LoggerFactory.Log.Where(l => l.Level == level); + Assert.Equal(expectedMessages.Length, logEntries.Count()); + + int count = 0; + foreach (var logEntry in logEntries) + { + Assert.Equal(expectedMessages[count++], logEntry.Message); + } + } + protected virtual void VerifyError(string expectedMessage, IMutableModel model) { var message = Assert.Throws(() => Validate(model)).Message; From 56632a5aa119251010d02d5ec178346542c54b9a Mon Sep 17 00:00:00 2001 From: lajones Date: Wed, 12 Feb 2020 14:30:16 -0800 Subject: [PATCH 2/3] Updating based on review comments. --- .../ConflictingValueGenerationStrategiesEventData.cs | 4 ++-- src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs | 6 +++--- .../Conventions/SqlServerStoreGenerationConvention.cs | 2 +- .../Properties/SqlServerStrings.Designer.cs | 2 +- src/EFCore.SqlServer/Properties/SqlServerStrings.resx | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs b/src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs index 1b5d84a1b95..bdfab7b547e 100644 --- a/src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs +++ b/src/EFCore.SqlServer/Diagnostics/ConflictingValueGenerationStrategiesEventData.cs @@ -25,7 +25,7 @@ public class ConflictingValueGenerationStrategiesEventData : EventData public ConflictingValueGenerationStrategiesEventData( [NotNull] EventDefinitionBase eventDefinition, [NotNull] Func messageGenerator, - [NotNull] string sqlServerValueGenerationStrategy, + SqlServerValueGenerationStrategy sqlServerValueGenerationStrategy, [NotNull] string otherValueGenerationStrategy, [NotNull] IProperty property) : base(eventDefinition, messageGenerator) @@ -38,7 +38,7 @@ public ConflictingValueGenerationStrategiesEventData( /// /// The SQL Server value generation strategy. /// - public virtual string SqlServerValueGenerationStrategy { get; } + public virtual SqlServerValueGenerationStrategy SqlServerValueGenerationStrategy { get; } /// /// The other value generation strategy. diff --git a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs index a566edc4047..8fc5b9c63e3 100644 --- a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs +++ b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs @@ -98,7 +98,7 @@ private static string ByteIdentityColumnWarning(EventDefinitionBase definition, /// public static void ConflictingValueGenerationStrategiesWarning( [NotNull] this IDiagnosticsLogger diagnostics, - [NotNull] string sqlServerValueGenerationStrategy, + SqlServerValueGenerationStrategy sqlServerValueGenerationStrategy, [NotNull] string otherValueGenerationStrategy, [NotNull] IProperty property) { @@ -106,7 +106,7 @@ public static void ConflictingValueGenerationStrategiesWarning( if (diagnostics.ShouldLog(definition)) { - definition.Log(diagnostics, sqlServerValueGenerationStrategy, otherValueGenerationStrategy, + definition.Log(diagnostics, sqlServerValueGenerationStrategy.ToString(), otherValueGenerationStrategy, property.Name, property.DeclaringEntityType.DisplayName()); } @@ -128,7 +128,7 @@ private static string ConflictingValueGenerationStrategiesWarning(EventDefinitio var d = (EventDefinition)definition; var p = (ConflictingValueGenerationStrategiesEventData)payload; return d.GenerateMessage( - p.SqlServerValueGenerationStrategy, + p.SqlServerValueGenerationStrategy.ToString(), p.OtherValueGenerationStrategy, p.Property.Name, p.Property.DeclaringEntityType.DisplayName()); diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs index 7666541f6ec..0e2850435b4 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerStoreGenerationConvention.cs @@ -103,7 +103,7 @@ protected override void Validate(IConventionProperty property) if (property.GetValueGenerationStrategyConfigurationSource() != null && property.GetValueGenerationStrategy() != SqlServerValueGenerationStrategy.None) { - var generationStrategy = property.GetValueGenerationStrategy().ToString(); + var generationStrategy = property.GetValueGenerationStrategy(); if (property.GetDefaultValue() != null) { diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 15031041ad6..6a1899af990 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -574,7 +574,7 @@ public static EventDefinition LogReflexiveConstraintIgnored([Not } /// - /// Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences. + /// Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity type '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences. /// public static EventDefinition LogConflictingValueGenerationStrategies([NotNull] IDiagnosticsLogger logger) { diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index a5cd31271e5..b786a62d3ab 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -242,7 +242,7 @@ The '{methodName}' method is not supported because the query has switched to client-evaluation. Inspect the log to determine which query expressions are triggering client-evaluation. - Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences. + Both the SqlServerValueGenerationStrategy {generationStrategy} and {otherGenerationStrategy} have been set on property '{propertyName}' on entity type '{entityName}'. Usually this is a mistake. Only use these at the same time if you are sure you understand the consequences. Warning SqlServerEventId.ConflictingValueGenerationStrategiesWarning string string string string \ No newline at end of file From 243e71437c28db221d2a188ebb19c4746b356e74 Mon Sep 17 00:00:00 2001 From: lajones Date: Wed, 12 Feb 2020 19:32:13 -0800 Subject: [PATCH 3/3] Updated to include functional tests. --- .../SqlServerValueGenerationConflictTest.cs | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 test/EFCore.SqlServer.FunctionalTests/SqlServerValueGenerationConflictTest.cs diff --git a/test/EFCore.SqlServer.FunctionalTests/SqlServerValueGenerationConflictTest.cs b/test/EFCore.SqlServer.FunctionalTests/SqlServerValueGenerationConflictTest.cs new file mode 100644 index 00000000000..68afb3bf05f --- /dev/null +++ b/test/EFCore.SqlServer.FunctionalTests/SqlServerValueGenerationConflictTest.cs @@ -0,0 +1,150 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; +using Microsoft.EntityFrameworkCore.SqlServer.Diagnostics.Internal; +using Microsoft.EntityFrameworkCore.SqlServer.Internal; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Xunit; + +// ReSharper disable InconsistentNaming +namespace Microsoft.EntityFrameworkCore +{ + public class SqlServerValueGenerationStrategyThrowTest : + SqlServerValueGenerationConflictTest + { + public SqlServerValueGenerationStrategyThrowTest( + SqlServerValueGenerationStrategyFixture fixture) + : base(fixture) + { + } + + [ConditionalFact] + public virtual void SqlServerValueGeneration_conflicting_with_existing_ValueGeneration_strategy_throws() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity() + .Property(e => e.Id) + .HasDefaultValueSql("2") + .UseHiLo(); + + Assert.Equal( + CoreStrings.WarningAsErrorTemplate( + SqlServerEventId.ConflictingValueGenerationStrategiesWarning, + SqlServerResources.LogConflictingValueGenerationStrategies( + new TestLogger()) + .GenerateMessage(SqlServerValueGenerationStrategy.SequenceHiLo.ToString(), "DefaultValueSql", "Id", nameof(Fred)), + "SqlServerEventId.ConflictingValueGenerationStrategiesWarning"), + Assert.Throws(() => Validate(modelBuilder)).Message); + } + + public class ThrowContext : DbContext + { + public ThrowContext(DbContextOptions options) + : base(options) + { + } + + public virtual DbSet Freds { get; set; } + + // use the normal behavior of ConflictingValueGenerationStrategiesWarning + // defined in UseSqlServer() + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseSqlServer(); + } + } + + public class SqlServerValueGenerationStrategyNoThrowTest : + SqlServerValueGenerationConflictTest + { + public SqlServerValueGenerationStrategyNoThrowTest( + SqlServerValueGenerationStrategyFixture fixture) + : base(fixture) + { + } + + [ConditionalFact] + public virtual void SqlServerValueGeneration_conflicting_with_existing_ValueGeneration_strategy_warns() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Entity() + .Property(e => e.Id) + .HasDefaultValueSql("2") + .UseHiLo(); + + // Assert - this does not throw + Validate(modelBuilder); + + var logEntry = Fixture.ListLoggerFactory.Log.Single( + l => l.Level == LogLevel.Warning && l.Id == SqlServerEventId.ConflictingValueGenerationStrategiesWarning); + Assert.Equal(SqlServerResources.LogConflictingValueGenerationStrategies( + new TestLogger()) + .GenerateMessage(SqlServerValueGenerationStrategy.SequenceHiLo.ToString(), "DefaultValueSql", "Id", nameof(Fred)), + logEntry.Message); + } + + public class NoThrowContext : DbContext + { + public NoThrowContext(DbContextOptions options) : base(options) + { + } + + public virtual DbSet Freds { get; set; } + + // override the normal behavior of ConflictingValueGenerationStrategiesWarning + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder + .UseSqlServer() + .ConfigureWarnings( + b => b.Log(SqlServerEventId.ConflictingValueGenerationStrategiesWarning)); + } + } + + public class SqlServerValueGenerationConflictTest + : IClassFixture> + where TContext : DbContext + { + public SqlServerValueGenerationConflictTest(SqlServerValueGenerationStrategyFixture fixture) + => Fixture = fixture; + + protected SqlServerValueGenerationStrategyFixture Fixture { get; } + + public TContext CreateContext() => (TContext)Fixture.CreateContext(); + + protected virtual ModelBuilder CreateModelBuilder() + { + var conventionSet = CreateConventionSetBuilder(CreateContext()).CreateConventionSet(); + + return new ModelBuilder(conventionSet); + } + + protected virtual IConventionSetBuilder CreateConventionSetBuilder(DbContext context) + => context.GetService(); + + protected virtual IModel Validate(ModelBuilder modelBuilder) + => modelBuilder.FinalizeModel(); + + public class Fred + { + public int Id { get; set; } + } + } + + public class SqlServerValueGenerationStrategyFixture : SharedStoreFixtureBase + where TContext : DbContext + { + protected override string StoreName { get; } = "SqlServerValueGenerationStrategy"; + protected override Type ContextType { get; } = typeof(TContext); + protected override ITestStoreFactory TestStoreFactory => SqlServerTestStoreFactory.Instance; + protected override bool ShouldLogCategory(string logCategory) + => logCategory == DbLoggerCategory.Model.Validation.Name; + protected override bool UsePooling => false; + } +}