From 63c3ffc38bc715f424e05363675fcab124a1d1a3 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 12 Sep 2022 16:53:30 +0100 Subject: [PATCH] Make original value parameter check for sproc into a warning Fixes #28602 --- .../Diagnostics/RelationalEventId.cs | 15 +++++++ .../Diagnostics/RelationalLoggerExtensions.cs | 40 +++++++++++++++++++ .../RelationalLoggingDefinitions.cs | 9 +++++ .../RelationalModelValidator.cs | 17 ++++---- .../Properties/RelationalStrings.Designer.cs | 33 +++++++++++---- .../Properties/RelationalStrings.resx | 7 ++-- .../RelationalModelValidatorTest.cs | 6 +-- .../SqliteModelValidatorTest.cs | 8 ++++ 8 files changed, 112 insertions(+), 23 deletions(-) diff --git a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs index 2f97c3bb0aa..7e6b49d07c6 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs @@ -102,6 +102,7 @@ private enum Id ForeignKeyTpcPrincipalWarning, TpcStoreGeneratedIdentityWarning, KeyPropertiesNotMappedToTable, + StoredProcedureConcurrencyTokenNotMapped, // Update events BatchReadyForExecution = CoreEventId.RelationalBaseId + 700, @@ -872,6 +873,20 @@ private static EventId MakeValidationId(Id id) public static readonly EventId KeyPropertiesNotMappedToTable = MakeValidationId(Id.KeyPropertiesNotMappedToTable); + /// + /// An entity type is mapped to the stored procedure with a concurrency token not mapped to any original value parameter. + /// + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId StoredProcedureConcurrencyTokenNotMapped = + MakeValidationId(Id.StoredProcedureConcurrencyTokenNotMapped); + /// /// A foreign key specifies properties which don't map to the related tables. /// diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index 52b4eba0d40..dea7e0c1fc9 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -3122,6 +3122,46 @@ private static string OptionalDependentWithoutIdentifyingPropertyWarning(EventDe return d.GenerateMessage(p.EntityType.DisplayName()); } + /// + /// Logs the event. + /// + /// The diagnostics logger to use. + /// The property which represents the concurrency token. + public static void StoredProcedureConcurrencyTokenNotMapped( + this IDiagnosticsLogger diagnostics, + IProperty concurrencyProperty) + { + var definition = RelationalResources.LogStoredProcedureConcurrencyTokenNotMapped(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + var entityType = concurrencyProperty.DeclaringEntityType; + definition.Log(diagnostics, + entityType.DisplayName(), + entityType.GetUpdateStoredProcedure()?.Name ?? "unnamed", + concurrencyProperty.Name); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new PropertyEventData( + definition, + StoredProcedureConcurrencyTokenNotMapped, + concurrencyProperty); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string StoredProcedureConcurrencyTokenNotMapped(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (PropertyEventData)payload; + var property = p.Property; + var entityType = property.DeclaringEntityType; + return d.GenerateMessage(entityType.DisplayName(), entityType.GetUpdateStoredProcedure()?.Name ?? "unnamed", property.Name); + } + /// /// Logs for the event. /// diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs index 86a8eedeaf1..61d56e82c53 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs @@ -286,6 +286,15 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions [EntityFrameworkInternal] public EventDefinitionBase? LogPossibleUnintendedUseOfEquals; + /// + /// 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. + /// + [EntityFrameworkInternal] + public EventDefinitionBase? LogStoredProcedureConcurrencyTokenNotMapped; + /// /// 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.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index 6fca7d36af0..6db7475e4bd 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -221,7 +221,7 @@ protected virtual void ValidateStoredProcedures( if (deleteStoredProcedure != null) { AddSproc(StoreObjectType.DeleteStoredProcedure, entityType, storedProcedures); - ValidateSproc(deleteStoredProcedure, mappingStrategy); + ValidateSproc(deleteStoredProcedure, mappingStrategy, logger); sprocCount++; } @@ -229,7 +229,7 @@ protected virtual void ValidateStoredProcedures( if (insertStoredProcedure != null) { AddSproc(StoreObjectType.InsertStoredProcedure, entityType, storedProcedures); - ValidateSproc(insertStoredProcedure, mappingStrategy); + ValidateSproc(insertStoredProcedure, mappingStrategy, logger); sprocCount++; } @@ -237,7 +237,7 @@ protected virtual void ValidateStoredProcedures( if (updateStoredProcedure != null) { AddSproc(StoreObjectType.UpdateStoredProcedure, entityType, storedProcedures); - ValidateSproc(updateStoredProcedure, mappingStrategy); + ValidateSproc(updateStoredProcedure, mappingStrategy, logger); sprocCount++; } @@ -288,7 +288,10 @@ static void AddSproc( } } - private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy) + private static void ValidateSproc( + IStoredProcedure sproc, + string mappingStrategy, + IDiagnosticsLogger logger) { var entityType = sproc.EntityType; var storeObjectIdentifier = sproc.GetStoreIdentifier(); @@ -646,11 +649,7 @@ private static void ValidateSproc(IStoredProcedure sproc, string mappingStrategy if (originalValueProperties.Values.FirstOrDefault(p => p.IsConcurrencyToken) is { } missedConcurrencyToken) { - throw new InvalidOperationException( - RelationalStrings.StoredProcedureConcurrencyTokenNotMapped( - entityType.DisplayName(), - storeObjectIdentifier.DisplayName(), - missedConcurrencyToken.Name)); + logger.StoredProcedureConcurrencyTokenNotMapped(missedConcurrencyToken); } if (sproc.ResultColumns.Any(c => c != sproc.FindRowsAffectedResultColumn())) diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 609cb1461dc..ab2b57b9605 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1475,14 +1475,6 @@ public static string SqlQueryUnmappedType(object? elementType) GetString("SqlQueryUnmappedType", nameof(elementType)), elementType); - /// - /// The entity type '{entityType}' is mapped to the stored procedure '{sproc}', but the concurrency token '{token}' is not mapped to any original value parameter. - /// - public static string StoredProcedureConcurrencyTokenNotMapped(object? entityType, object? sproc, object? token) - => string.Format( - GetString("StoredProcedureConcurrencyTokenNotMapped", nameof(entityType), nameof(sproc), nameof(token)), - entityType, sproc, token); - /// /// Current value parameter '{parameter}' is not allowed on delete stored procedure '{sproc}'. Use HasOriginalValueParameter() instead. /// @@ -3475,6 +3467,31 @@ public static EventDefinition LogOptionalDependentWithoutIdentifyingProp return (EventDefinition)definition; } + /// + /// The entity type '{entityType}' is mapped to the stored procedure '{sproc}', but the concurrency token '{token}' is not mapped to any original value parameter. + /// + public static EventDefinition LogStoredProcedureConcurrencyTokenNotMapped(IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogStoredProcedureConcurrencyTokenNotMapped; + if (definition == null) + { + definition = NonCapturingLazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogStoredProcedureConcurrencyTokenNotMapped, + logger, + static logger => new EventDefinition( + logger.Options, + RelationalEventId.StoredProcedureConcurrencyTokenNotMapped, + LogLevel.Error, + "RelationalEventId.StoredProcedureConcurrencyTokenNotMapped", + level => LoggerMessage.Define( + level, + RelationalEventId.StoredProcedureConcurrencyTokenNotMapped, + _resourceManager.GetString("LogStoredProcedureConcurrencyTokenNotMapped")!))); + } + + return (EventDefinition)definition; + } + /// /// Possible unintended use of method 'Equals' for arguments '{left}' and '{right}' of different types in a query. This comparison will always return false. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 69c299c8160..0344ed321e6 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -769,6 +769,10 @@ The entity type '{entityType}' is an optional dependent using table sharing without any required non shared property that could be used to identify whether the entity exists. If all nullable properties contain a null value in database then an object instance won't be created in the query. Add a required property to create instances with null values for other properties or mark the incoming navigation as required to always create an instance. Warning RelationalEventId.OptionalDependentWithoutIdentifyingPropertyWarning string + + The entity type '{entityType}' is mapped to the stored procedure '{sproc}', but the concurrency token '{token}' is not mapped to any original value parameter. + Error RelationalEventId.StoredProcedureConcurrencyTokenNotMapped string string string + Possible unintended use of method 'Equals' for arguments '{left}' and '{right}' of different types in a query. This comparison will always return false. Warning RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning string string @@ -976,9 +980,6 @@ The element type '{elementType}' used in 'SqlQuery' method is not natively supported by your database provider. Either use a supported element type, or use ModelConfigurationBuilder.DefaultTypeMapping to define a mapping for your type. - - The entity type '{entityType}' is mapped to the stored procedure '{sproc}', but the concurrency token '{token}' is not mapped to any original value parameter. - Current value parameter '{parameter}' is not allowed on delete stored procedure '{sproc}'. Use HasOriginalValueParameter() instead. diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 81360d1df89..da40d195c77 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -2960,9 +2960,9 @@ public virtual void Detects_unmapped_concurrency_token() .HasRowsAffectedReturnValue()) .Property(a => a.Name).IsRowVersion(); - VerifyError( - RelationalStrings.StoredProcedureConcurrencyTokenNotMapped(nameof(Animal), "Animal_Update", nameof(Animal.Name)), - modelBuilder); + VerifyWarning( + RelationalResources.LogStoredProcedureConcurrencyTokenNotMapped(new TestLogger()) + .GenerateMessage(nameof(Animal), "Animal_Update", nameof(Animal.Name)), modelBuilder, LogLevel.Error); } [ConditionalFact] diff --git a/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs b/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs index 56ea021eb5f..60e8853a4fe 100644 --- a/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs +++ b/test/EFCore.Sqlite.Tests/Infrastructure/SqliteModelValidatorTest.cs @@ -127,6 +127,14 @@ public override void Passes_on_derived_entity_type_not_mapped_to_a_stored_proced Assert.Equal(SqliteStrings.StoredProceduresNotSupported(nameof(Animal)), exception.Message); } + public override void Detects_unmapped_concurrency_token() + { + var exception = + Assert.Throws(() => base.Detects_unmapped_concurrency_token()); + + Assert.Equal(SqliteStrings.StoredProceduresNotSupported(nameof(Animal)), exception.Message); + } + public override void Store_generated_in_composite_key() { var modelBuilder = CreateConventionModelBuilder();