Fix to #31100 - Switch to storing enums as ints in JSON instead of st…

…rings (#31317)

src/EFCore.Relational/Metadata/Conventions/RelationalMapToJsonConvention.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.EntityFrameworkCore.Storage.Json;
+
 namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
 
 /// <summary>
@@ -52,10 +54,17 @@ public virtual void ProcessModelFinalizing(
     {
         foreach (var jsonEntityType in modelBuilder.Metadata.GetEntityTypes().Where(e => e.IsMappedToJson()))
         {
-            foreach (var enumProperty in jsonEntityType.GetDeclaredProperties().Where(p => p.ClrType.UnwrapNullableType().IsEnum))
+            foreach (var enumProperty in jsonEntityType
+                         .GetDeclaredProperties()
+                         .Where(p => p.ClrType.UnwrapNullableType().IsEnum))
             {
-                // by default store enums as strings - values should be human-readable
-                enumProperty.Builder.HasConversion(typeof(string));
+                // If the enum is mapped with no conversion, then use the reader/writer that handles legacy string values and warns.
+                if (enumProperty.GetValueConverter() == null
+                    && enumProperty.GetProviderClrType() == null)
+                {
+                    enumProperty.SetJsonValueReaderWriterType(
+                        typeof(JsonWarningEnumReaderWriter<>).MakeGenericType(enumProperty.ClrType.UnwrapNullableType()));
+                }
             }
         }
     }

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs

@@ -877,7 +877,7 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
                     RelationalStrings.JsonRequiredEntityWithNullJson(typeof(TEntity).Name));
             }
 
-            var manager = new Utf8JsonReaderManager(jsonReaderData);
+            var manager = new Utf8JsonReaderManager(jsonReaderData, queryContext.QueryLogger);
             var tokenType = manager.CurrentReader.TokenType;
 
             if (tokenType == JsonTokenType.Null)
@@ -914,7 +914,7 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
                 return default;
             }
 
-            var manager = new Utf8JsonReaderManager(jsonReaderData);
+            var manager = new Utf8JsonReaderManager(jsonReaderData, queryContext.QueryLogger);
             var tokenType = manager.CurrentReader.TokenType;
 
             if (tokenType == JsonTokenType.Null)
@@ -946,7 +946,7 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
                     manager.CaptureState();
                     var entity = innerShaper(queryContext, newKeyPropertyValues, jsonReaderData);
                     result.Add(entity);
-                    manager = new Utf8JsonReaderManager(manager.Data);
+                    manager = new Utf8JsonReaderManager(manager.Data, queryContext.QueryLogger);
 
                     if (manager.CurrentReader.TokenType != JsonTokenType.EndObject)
                     {
@@ -1003,7 +1003,7 @@ private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedColle
                 return;
             }
 
-            var manager = new Utf8JsonReaderManager(jsonReaderData);
+            var manager = new Utf8JsonReaderManager(jsonReaderData, queryContext.QueryLogger);
             var tokenType = manager.CurrentReader.TokenType;
 
             if (tokenType != JsonTokenType.StartArray)
@@ -1032,7 +1032,7 @@ private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedColle
                         fixup(entity, resultElement);
                     }
 
-                    manager = new Utf8JsonReaderManager(manager.Data);
+                    manager = new Utf8JsonReaderManager(manager.Data, queryContext.QueryLogger);
                     if (manager.CurrentReader.TokenType != JsonTokenType.EndObject)
                     {
                         throw new InvalidOperationException(

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs

@@ -44,7 +44,8 @@ private static readonly ConstructorInfo JsonReaderDataConstructor
             = typeof(JsonReaderData).GetConstructor(new[] { typeof(Stream) })!;
 
         private static readonly ConstructorInfo JsonReaderManagerConstructor
-            = typeof(Utf8JsonReaderManager).GetConstructor(new[] { typeof(JsonReaderData) })!;
+            = typeof(Utf8JsonReaderManager).GetConstructor(
+                new[] { typeof(JsonReaderData), typeof(IDiagnosticsLogger<DbLoggerCategory.Query>) })!;
 
         private static readonly MethodInfo Utf8JsonReaderManagerMoveNextMethod
             = typeof(Utf8JsonReaderManager).GetMethod(nameof(Utf8JsonReaderManager.MoveNext), Array.Empty<Type>())!;
@@ -64,6 +65,12 @@ private static readonly MethodInfo Utf8JsonReaderTrySkipMethod
         private static readonly PropertyInfo Utf8JsonReaderTokenTypeProperty
             = typeof(Utf8JsonReader).GetProperty(nameof(Utf8JsonReader.TokenType))!;
 
+        private static readonly MethodInfo Utf8JsonReaderGetStringMethod
+            = typeof(Utf8JsonReader).GetMethod(nameof(Utf8JsonReader.GetString), Array.Empty<Type>())!;
+
+        private static readonly MethodInfo EnumParseMethodInfo
+            = typeof(Enum).GetMethod(nameof(Enum.Parse), new[] { typeof(Type), typeof(string) })!;
+
         private readonly RelationalShapedQueryCompilingExpressionVisitor _parentVisitor;
         private readonly ISet<string>? _tags;
         private readonly bool _isTracking;
@@ -73,6 +80,7 @@ private static readonly PropertyInfo Utf8JsonReaderTokenTypeProperty
         private readonly bool _generateCommandCache;
         private readonly ParameterExpression _resultCoordinatorParameter;
         private readonly ParameterExpression? _executionStrategyParameter;
+        private readonly IDiagnosticsLogger<DbLoggerCategory.Query> _queryLogger;
 
         /// <summary>
         ///     States scoped to SelectExpression
@@ -154,6 +162,7 @@ public ShaperProcessingExpressionVisitor(
             bool indexMap)
         {
             _parentVisitor = parentVisitor;
+            _queryLogger = parentVisitor.QueryCompilationContext.Logger;
             _resultCoordinatorParameter = Parameter(
                 splitQuery ? typeof(SplitQueryResultCoordinator) : typeof(SingleQueryResultCoordinator), "resultCoordinator");
             _executionStrategyParameter = splitQuery ? Parameter(typeof(IExecutionStrategy), "executionStrategy") : null;
@@ -187,6 +196,7 @@ private ShaperProcessingExpressionVisitor(
             ReaderColumn?[]? readerColumns)
         {
             _parentVisitor = parentVisitor;
+            _queryLogger = parentVisitor.QueryCompilationContext.Logger;
             _resultCoordinatorParameter = resultCoordinatorParameter;
 
             _selectExpression = selectExpression;
@@ -209,6 +219,7 @@ private ShaperProcessingExpressionVisitor(
             ISet<string> tags)
         {
             _parentVisitor = parentVisitor;
+            _queryLogger = parentVisitor.QueryCompilationContext.Logger;
             _resultCoordinatorParameter = resultCoordinatorParameter;
             _executionStrategyParameter = executionStrategyParameter;
 
@@ -1295,7 +1306,9 @@ private Expression CreateJsonShapers(
                 entityShaperExpression.EntityType,
                 _isTracking,
                 jsonReaderDataShaperLambdaParameter,
-                innerShapersMap, innerFixupMap).Rewrite(entityShaperMaterializer);
+                innerShapersMap,
+                innerFixupMap,
+                _queryLogger).Rewrite(entityShaperMaterializer);
 
             var entityShaperMaterializerVariable = Variable(
                 entityShaperMaterializer.Type,
@@ -1413,6 +1426,7 @@ private sealed class JsonEntityMaterializerRewriter : ExpressionVisitor
             private readonly ParameterExpression _jsonReaderDataParameter;
             private readonly IDictionary<string, Expression> _innerShapersMap;
             private readonly IDictionary<string, LambdaExpression> _innerFixupMap;
+            private readonly IDiagnosticsLogger<DbLoggerCategory.Query> _queryLogger;
 
             private static readonly PropertyInfo JsonEncodedTextEncodedUtf8BytesProperty
                 = typeof(JsonEncodedText).GetProperty(nameof(JsonEncodedText.EncodedUtf8Bytes))!;
@@ -1426,13 +1440,15 @@ public JsonEntityMaterializerRewriter(
                 bool isTracking,
                 ParameterExpression jsonReaderDataParameter,
                 IDictionary<string, Expression> innerShapersMap,
-                IDictionary<string, LambdaExpression> innerFixupMap)
+                IDictionary<string, LambdaExpression> innerFixupMap,
+                IDiagnosticsLogger<DbLoggerCategory.Query> queryLogger)
             {
                 _entityType = entityType;
                 _isTracking = isTracking;
                 _jsonReaderDataParameter = jsonReaderDataParameter;
                 _innerShapersMap = innerShapersMap;
                 _innerFixupMap = innerFixupMap;
+                _queryLogger = queryLogger;
             }
 
             public BlockExpression Rewrite(BlockExpression jsonEntityShaperMaterializer)
@@ -1501,7 +1517,8 @@ protected override Expression VisitSwitch(SwitchExpression switchExpression)
                             managerVariable,
                             New(
                                 JsonReaderManagerConstructor,
-                                _jsonReaderDataParameter)),
+                                _jsonReaderDataParameter,
+                                Constant(_queryLogger))),
                         // tokenType = jsonReaderManager.CurrentReader.TokenType
                         Assign(
                             tokenTypeVariable,
@@ -1657,7 +1674,8 @@ protected override Expression VisitSwitch(SwitchExpression switchExpression)
                         var moveNext = Call(managerVariable, Utf8JsonReaderManagerMoveNextMethod);
                         var captureState = Call(managerVariable, Utf8JsonReaderManagerCaptureStateMethod);
                         var assignment = Assign(propertyVariable, innerShaperMapElement.Value);
-                        var managerRecreation = Assign(managerVariable, New(JsonReaderManagerConstructor, _jsonReaderDataParameter));
+                        var managerRecreation = Assign(
+                            managerVariable, New(JsonReaderManagerConstructor, _jsonReaderDataParameter, Constant(_queryLogger)));
 
                         readExpressions.Add(
                             Block(
@@ -2011,7 +2029,8 @@ private static IList<T> PopulateList<T>(IList<T> buffer, IList<T> target)
                         jsonReaderDataVariable,
                         Default(typeof(JsonReaderData))),
                     Block(
-                        Assign(jsonReaderManagerVariable, New(JsonReaderManagerConstructor, jsonReaderDataVariable)),
+                        Assign(
+                            jsonReaderManagerVariable, New(JsonReaderManagerConstructor, jsonReaderDataVariable, Constant(_queryLogger))),
                         Call(jsonReaderManagerVariable, Utf8JsonReaderManagerMoveNextMethod),
                         Call(jsonReaderManagerVariable, Utf8JsonReaderManagerCaptureStateMethod)));
 
@@ -2373,7 +2392,6 @@ private Expression CreateReadJsonPropertyValueExpression(
             {
                 // in case of null value we can't just use the JsonReader method, but rather check the current token type
                 // if it's JsonTokenType.Null means value is null, only if it's not we are safe to read the value
-
                 if (resultExpression.Type != property.ClrType)
                 {
                     resultExpression = Convert(resultExpression, property.ClrType);

src/EFCore/Diagnostics/CoreEventId.cs

@@ -75,6 +75,7 @@ private enum Id
         NavigationBaseIncludeIgnored,
         DistinctAfterOrderByWithoutRowLimitingOperatorWarning,
         QueryCanceled,
+        StringEnumValueInJson,
 
         // Infrastructure events
         SensitiveDataLoggingEnabledWarning = CoreBaseId + 400,
@@ -326,6 +327,16 @@ public static readonly EventId DistinctAfterOrderByWithoutRowLimitingOperatorWar
     public static readonly EventId QueryCanceled
         = MakeQueryId(Id.QueryCanceled);
 
+    /// <summary>
+    ///     A string value for an enum was read from JSON. Starting with EF Core 8, a breaking change was made to store enum
+    ///     values in JSON as numbers by default. See https://aka.ms/efcore-docs-jsonenums for details.
+    /// </summary>
+    /// <remarks>
+    ///     This event is in the <see cref="DbLoggerCategory.Query" /> category.
+    /// </remarks>
+    public static readonly EventId StringEnumValueInJson
+        = MakeQueryId(Id.StringEnumValueInJson);
+
     private static readonly string _infraPrefix = DbLoggerCategory.Infrastructure.Name + ".";
 
     private static EventId MakeInfraId(Id id)

src/EFCore/Diagnostics/CoreLoggerExtensions.cs

@@ -2583,6 +2583,40 @@ private static string ReferenceChangeDetectedSensitive(EventDefinitionBase defin
             p.EntityEntry.GetInfrastructure().BuildCurrentValuesString(p.Navigation.DeclaringEntityType.FindPrimaryKey()!.Properties));
     }
 
+    /// <summary>
+    ///     Logs for the <see cref="CoreEventId.StringEnumValueInJson" /> event.
+    /// </summary>
+    /// <param name="diagnostics">The diagnostics logger to use.</param>
+    /// <param name="enumType">The type.</param>
+    public static void StringEnumValueInJson(
+        this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics,
+        Type enumType)
+    {
+        var definition = CoreResources.LogStringEnumValueInJson(diagnostics);
+
+        if (diagnostics.ShouldLog(definition))
+        {
+            definition.Log(diagnostics, enumType.ShortDisplayName());
+        }
+
+        if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+        {
+            var eventData = new TypeEventData(
+                definition,
+                StringEnumValueInJson,
+                enumType);
+
+            diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+        }
+    }
+
+    private static string StringEnumValueInJson(EventDefinitionBase definition, EventData payload)
+    {
+        var d = (EventDefinition<string>)definition;
+        var p = (TypeEventData)payload;
+        return d.GenerateMessage(p.ClrType.ShortDisplayName());
+    }
+
     /// <summary>
     ///     Logs for the <see cref="CoreEventId.StartedTracking" /> event.
     /// </summary>

src/EFCore/Diagnostics/ILoggingOptions.cs

@@ -39,4 +39,11 @@ public interface ILoggingOptions : ISingletonOptions
     ///     Reflects the option set by <see cref="DbContextOptionsBuilder.ConfigureWarnings" />.
     /// </summary>
     WarningsConfiguration WarningsConfiguration { get; }
+
+    /// <summary>
+    ///    Returns <see langword="true"/> if a warning about string values for the given enum type has not yet been performed.
+    /// </summary>
+    /// <param name="type">The type to check.</param>
+    /// <returns>Whether or not a warning has been issued.</returns>
+    bool ShouldWarnForEnumType(Type type);
 }

src/EFCore/Diagnostics/Internal/LoggingOptions.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Concurrent;
+
 namespace Microsoft.EntityFrameworkCore.Diagnostics.Internal;
 
 /// <summary>
@@ -11,6 +13,8 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics.Internal;
 /// </summary>
 public class LoggingOptions : ILoggingOptions
 {
+    private readonly ConcurrentDictionary<Type, bool> _warnedForStringEnums = new();
+
     /// <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
@@ -98,4 +102,16 @@ public virtual void Validate(IDbContextOptions options)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual WarningsConfiguration WarningsConfiguration { get; private set; } = null!;
+
+    /// <inheritdoc />
+    public virtual bool ShouldWarnForEnumType(Type type)
+    {
+        if (_warnedForStringEnums.ContainsKey(type))
+        {
+            return false;
+        }
+
+        _warnedForStringEnums[type] = true;
+        return true;
+    }
 }

src/EFCore/Diagnostics/LoggingDefinitions.cs

@@ -223,6 +223,15 @@ public abstract class LoggingDefinitions
     [EntityFrameworkInternal]
     public EventDefinitionBase? LogSkipCollectionChangeDetectedSensitive;
 
+    /// <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>
+    [EntityFrameworkInternal]
+    public EventDefinitionBase? LogStringEnumValueInJson;
+
     /// <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

src/EFCore/Diagnostics/TypeEventData.cs

@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Diagnostics;
+
+/// <summary>
+///     A <see cref="DiagnosticSource" /> event payload class for events that reference a <see cref="Type" />.
+/// </summary>
+/// <remarks>
+///     See <see href="https://aka.ms/efcore-docs-diagnostics">Logging, events, and diagnostics</see> for more information and examples.
+/// </remarks>
+public class TypeEventData : EventData
+{
+    /// <summary>
+    ///     Constructs the event payload.
+    /// </summary>
+    /// <param name="eventDefinition">The event definition.</param>
+    /// <param name="messageGenerator">A delegate that generates a log message for this event.</param>
+    /// <param name="clrType">The <see cref="Type"/> associated with this event.</param>
+    public TypeEventData(
+        EventDefinitionBase eventDefinition,
+        Func<EventDefinitionBase, EventData, string> messageGenerator,
+        Type clrType)
+        : base(eventDefinition, messageGenerator)
+    {
+        ClrType = clrType;
+    }
+
+    /// <summary>
+    ///     The <see cref="Type"/> associated with this event.
+    /// </summary>
+    public virtual Type ClrType { get; }
+}