Skip to content

Commit 078c98f

Browse files
maumarmaumar
authored
Fix to #31100 - Switch to storing enums as ints in JSON instead of strings (#31317)
1 parent 5b556d3 commit 078c98f

23 files changed

+617
-168
lines changed

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using Microsoft.EntityFrameworkCore.Storage.Json;
5+
46
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
57

68
/// <summary>
@@ -52,10 +54,17 @@ public virtual void ProcessModelFinalizing(
5254
{
5355
foreach (var jsonEntityType in modelBuilder.Metadata.GetEntityTypes().Where(e => e.IsMappedToJson()))
5456
{
55-
foreach (var enumProperty in jsonEntityType.GetDeclaredProperties().Where(p => p.ClrType.UnwrapNullableType().IsEnum))
57+
foreach (var enumProperty in jsonEntityType
58+
.GetDeclaredProperties()
59+
.Where(p => p.ClrType.UnwrapNullableType().IsEnum))
5660
{
57-
// by default store enums as strings - values should be human-readable
58-
enumProperty.Builder.HasConversion(typeof(string));
61+
// If the enum is mapped with no conversion, then use the reader/writer that handles legacy string values and warns.
62+
if (enumProperty.GetValueConverter() == null
63+
&& enumProperty.GetProviderClrType() == null)
64+
{
65+
enumProperty.SetJsonValueReaderWriterType(
66+
typeof(JsonWarningEnumReaderWriter<>).MakeGenericType(enumProperty.ClrType.UnwrapNullableType()));
67+
}
5968
}
6069
}
6170
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
877877
RelationalStrings.JsonRequiredEntityWithNullJson(typeof(TEntity).Name));
878878
}
879879

880-
var manager = new Utf8JsonReaderManager(jsonReaderData);
880+
var manager = new Utf8JsonReaderManager(jsonReaderData, queryContext.QueryLogger);
881881
var tokenType = manager.CurrentReader.TokenType;
882882

883883
if (tokenType == JsonTokenType.Null)
@@ -914,7 +914,7 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
914914
return default;
915915
}
916916

917-
var manager = new Utf8JsonReaderManager(jsonReaderData);
917+
var manager = new Utf8JsonReaderManager(jsonReaderData, queryContext.QueryLogger);
918918
var tokenType = manager.CurrentReader.TokenType;
919919

920920
if (tokenType == JsonTokenType.Null)
@@ -946,7 +946,7 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
946946
manager.CaptureState();
947947
var entity = innerShaper(queryContext, newKeyPropertyValues, jsonReaderData);
948948
result.Add(entity);
949-
manager = new Utf8JsonReaderManager(manager.Data);
949+
manager = new Utf8JsonReaderManager(manager.Data, queryContext.QueryLogger);
950950

951951
if (manager.CurrentReader.TokenType != JsonTokenType.EndObject)
952952
{
@@ -1003,7 +1003,7 @@ private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedColle
10031003
return;
10041004
}
10051005

1006-
var manager = new Utf8JsonReaderManager(jsonReaderData);
1006+
var manager = new Utf8JsonReaderManager(jsonReaderData, queryContext.QueryLogger);
10071007
var tokenType = manager.CurrentReader.TokenType;
10081008

10091009
if (tokenType != JsonTokenType.StartArray)
@@ -1032,7 +1032,7 @@ private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedColle
10321032
fixup(entity, resultElement);
10331033
}
10341034

1035-
manager = new Utf8JsonReaderManager(manager.Data);
1035+
manager = new Utf8JsonReaderManager(manager.Data, queryContext.QueryLogger);
10361036
if (manager.CurrentReader.TokenType != JsonTokenType.EndObject)
10371037
{
10381038
throw new InvalidOperationException(

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ private static readonly ConstructorInfo JsonReaderDataConstructor
4444
= typeof(JsonReaderData).GetConstructor(new[] { typeof(Stream) })!;
4545

4646
private static readonly ConstructorInfo JsonReaderManagerConstructor
47-
= typeof(Utf8JsonReaderManager).GetConstructor(new[] { typeof(JsonReaderData) })!;
47+
= typeof(Utf8JsonReaderManager).GetConstructor(
48+
new[] { typeof(JsonReaderData), typeof(IDiagnosticsLogger<DbLoggerCategory.Query>) })!;
4849

4950
private static readonly MethodInfo Utf8JsonReaderManagerMoveNextMethod
5051
= typeof(Utf8JsonReaderManager).GetMethod(nameof(Utf8JsonReaderManager.MoveNext), Array.Empty<Type>())!;
@@ -64,6 +65,12 @@ private static readonly MethodInfo Utf8JsonReaderTrySkipMethod
6465
private static readonly PropertyInfo Utf8JsonReaderTokenTypeProperty
6566
= typeof(Utf8JsonReader).GetProperty(nameof(Utf8JsonReader.TokenType))!;
6667

68+
private static readonly MethodInfo Utf8JsonReaderGetStringMethod
69+
= typeof(Utf8JsonReader).GetMethod(nameof(Utf8JsonReader.GetString), Array.Empty<Type>())!;
70+
71+
private static readonly MethodInfo EnumParseMethodInfo
72+
= typeof(Enum).GetMethod(nameof(Enum.Parse), new[] { typeof(Type), typeof(string) })!;
73+
6774
private readonly RelationalShapedQueryCompilingExpressionVisitor _parentVisitor;
6875
private readonly ISet<string>? _tags;
6976
private readonly bool _isTracking;
@@ -73,6 +80,7 @@ private static readonly PropertyInfo Utf8JsonReaderTokenTypeProperty
7380
private readonly bool _generateCommandCache;
7481
private readonly ParameterExpression _resultCoordinatorParameter;
7582
private readonly ParameterExpression? _executionStrategyParameter;
83+
private readonly IDiagnosticsLogger<DbLoggerCategory.Query> _queryLogger;
7684

7785
/// <summary>
7886
/// States scoped to SelectExpression
@@ -154,6 +162,7 @@ public ShaperProcessingExpressionVisitor(
154162
bool indexMap)
155163
{
156164
_parentVisitor = parentVisitor;
165+
_queryLogger = parentVisitor.QueryCompilationContext.Logger;
157166
_resultCoordinatorParameter = Parameter(
158167
splitQuery ? typeof(SplitQueryResultCoordinator) : typeof(SingleQueryResultCoordinator), "resultCoordinator");
159168
_executionStrategyParameter = splitQuery ? Parameter(typeof(IExecutionStrategy), "executionStrategy") : null;
@@ -187,6 +196,7 @@ private ShaperProcessingExpressionVisitor(
187196
ReaderColumn?[]? readerColumns)
188197
{
189198
_parentVisitor = parentVisitor;
199+
_queryLogger = parentVisitor.QueryCompilationContext.Logger;
190200
_resultCoordinatorParameter = resultCoordinatorParameter;
191201

192202
_selectExpression = selectExpression;
@@ -209,6 +219,7 @@ private ShaperProcessingExpressionVisitor(
209219
ISet<string> tags)
210220
{
211221
_parentVisitor = parentVisitor;
222+
_queryLogger = parentVisitor.QueryCompilationContext.Logger;
212223
_resultCoordinatorParameter = resultCoordinatorParameter;
213224
_executionStrategyParameter = executionStrategyParameter;
214225

@@ -1295,7 +1306,9 @@ private Expression CreateJsonShapers(
12951306
entityShaperExpression.EntityType,
12961307
_isTracking,
12971308
jsonReaderDataShaperLambdaParameter,
1298-
innerShapersMap, innerFixupMap).Rewrite(entityShaperMaterializer);
1309+
innerShapersMap,
1310+
innerFixupMap,
1311+
_queryLogger).Rewrite(entityShaperMaterializer);
12991312

13001313
var entityShaperMaterializerVariable = Variable(
13011314
entityShaperMaterializer.Type,
@@ -1413,6 +1426,7 @@ private sealed class JsonEntityMaterializerRewriter : ExpressionVisitor
14131426
private readonly ParameterExpression _jsonReaderDataParameter;
14141427
private readonly IDictionary<string, Expression> _innerShapersMap;
14151428
private readonly IDictionary<string, LambdaExpression> _innerFixupMap;
1429+
private readonly IDiagnosticsLogger<DbLoggerCategory.Query> _queryLogger;
14161430

14171431
private static readonly PropertyInfo JsonEncodedTextEncodedUtf8BytesProperty
14181432
= typeof(JsonEncodedText).GetProperty(nameof(JsonEncodedText.EncodedUtf8Bytes))!;
@@ -1426,13 +1440,15 @@ public JsonEntityMaterializerRewriter(
14261440
bool isTracking,
14271441
ParameterExpression jsonReaderDataParameter,
14281442
IDictionary<string, Expression> innerShapersMap,
1429-
IDictionary<string, LambdaExpression> innerFixupMap)
1443+
IDictionary<string, LambdaExpression> innerFixupMap,
1444+
IDiagnosticsLogger<DbLoggerCategory.Query> queryLogger)
14301445
{
14311446
_entityType = entityType;
14321447
_isTracking = isTracking;
14331448
_jsonReaderDataParameter = jsonReaderDataParameter;
14341449
_innerShapersMap = innerShapersMap;
14351450
_innerFixupMap = innerFixupMap;
1451+
_queryLogger = queryLogger;
14361452
}
14371453

14381454
public BlockExpression Rewrite(BlockExpression jsonEntityShaperMaterializer)
@@ -1501,7 +1517,8 @@ protected override Expression VisitSwitch(SwitchExpression switchExpression)
15011517
managerVariable,
15021518
New(
15031519
JsonReaderManagerConstructor,
1504-
_jsonReaderDataParameter)),
1520+
_jsonReaderDataParameter,
1521+
Constant(_queryLogger))),
15051522
// tokenType = jsonReaderManager.CurrentReader.TokenType
15061523
Assign(
15071524
tokenTypeVariable,
@@ -1657,7 +1674,8 @@ protected override Expression VisitSwitch(SwitchExpression switchExpression)
16571674
var moveNext = Call(managerVariable, Utf8JsonReaderManagerMoveNextMethod);
16581675
var captureState = Call(managerVariable, Utf8JsonReaderManagerCaptureStateMethod);
16591676
var assignment = Assign(propertyVariable, innerShaperMapElement.Value);
1660-
var managerRecreation = Assign(managerVariable, New(JsonReaderManagerConstructor, _jsonReaderDataParameter));
1677+
var managerRecreation = Assign(
1678+
managerVariable, New(JsonReaderManagerConstructor, _jsonReaderDataParameter, Constant(_queryLogger)));
16611679

16621680
readExpressions.Add(
16631681
Block(
@@ -2011,7 +2029,8 @@ private static IList<T> PopulateList<T>(IList<T> buffer, IList<T> target)
20112029
jsonReaderDataVariable,
20122030
Default(typeof(JsonReaderData))),
20132031
Block(
2014-
Assign(jsonReaderManagerVariable, New(JsonReaderManagerConstructor, jsonReaderDataVariable)),
2032+
Assign(
2033+
jsonReaderManagerVariable, New(JsonReaderManagerConstructor, jsonReaderDataVariable, Constant(_queryLogger))),
20152034
Call(jsonReaderManagerVariable, Utf8JsonReaderManagerMoveNextMethod),
20162035
Call(jsonReaderManagerVariable, Utf8JsonReaderManagerCaptureStateMethod)));
20172036

@@ -2373,7 +2392,6 @@ private Expression CreateReadJsonPropertyValueExpression(
23732392
{
23742393
// in case of null value we can't just use the JsonReader method, but rather check the current token type
23752394
// if it's JsonTokenType.Null means value is null, only if it's not we are safe to read the value
2376-
23772395
if (resultExpression.Type != property.ClrType)
23782396
{
23792397
resultExpression = Convert(resultExpression, property.ClrType);

‎src/EFCore/Diagnostics/CoreEventId.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ private enum Id
7575
NavigationBaseIncludeIgnored,
7676
DistinctAfterOrderByWithoutRowLimitingOperatorWarning,
7777
QueryCanceled,
78+
StringEnumValueInJson,
7879

7980
// Infrastructure events
8081
SensitiveDataLoggingEnabledWarning = CoreBaseId + 400,
@@ -326,6 +327,16 @@ public static readonly EventId DistinctAfterOrderByWithoutRowLimitingOperatorWar
326327
public static readonly EventId QueryCanceled
327328
= MakeQueryId(Id.QueryCanceled);
328329

330+
/// <summary>
331+
/// A string value for an enum was read from JSON. Starting with EF Core 8, a breaking change was made to store enum
332+
/// values in JSON as numbers by default. See https://aka.ms/efcore-docs-jsonenums for details.
333+
/// </summary>
334+
/// <remarks>
335+
/// This event is in the <see cref="DbLoggerCategory.Query" /> category.
336+
/// </remarks>
337+
public static readonly EventId StringEnumValueInJson
338+
= MakeQueryId(Id.StringEnumValueInJson);
339+
329340
private static readonly string _infraPrefix = DbLoggerCategory.Infrastructure.Name + ".";
330341

331342
private static EventId MakeInfraId(Id id)

‎src/EFCore/Diagnostics/CoreLoggerExtensions.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2583,6 +2583,40 @@ private static string ReferenceChangeDetectedSensitive(EventDefinitionBase defin
25832583
p.EntityEntry.GetInfrastructure().BuildCurrentValuesString(p.Navigation.DeclaringEntityType.FindPrimaryKey()!.Properties));
25842584
}
25852585

2586+
/// <summary>
2587+
/// Logs for the <see cref="CoreEventId.StringEnumValueInJson" /> event.
2588+
/// </summary>
2589+
/// <param name="diagnostics">The diagnostics logger to use.</param>
2590+
/// <param name="enumType">The type.</param>
2591+
public static void StringEnumValueInJson(
2592+
this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics,
2593+
Type enumType)
2594+
{
2595+
var definition = CoreResources.LogStringEnumValueInJson(diagnostics);
2596+
2597+
if (diagnostics.ShouldLog(definition))
2598+
{
2599+
definition.Log(diagnostics, enumType.ShortDisplayName());
2600+
}
2601+
2602+
if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
2603+
{
2604+
var eventData = new TypeEventData(
2605+
definition,
2606+
StringEnumValueInJson,
2607+
enumType);
2608+
2609+
diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
2610+
}
2611+
}
2612+
2613+
private static string StringEnumValueInJson(EventDefinitionBase definition, EventData payload)
2614+
{
2615+
var d = (EventDefinition<string>)definition;
2616+
var p = (TypeEventData)payload;
2617+
return d.GenerateMessage(p.ClrType.ShortDisplayName());
2618+
}
2619+
25862620
/// <summary>
25872621
/// Logs for the <see cref="CoreEventId.StartedTracking" /> event.
25882622
/// </summary>

‎src/EFCore/Diagnostics/ILoggingOptions.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,11 @@ public interface ILoggingOptions : ISingletonOptions
3939
/// Reflects the option set by <see cref="DbContextOptionsBuilder.ConfigureWarnings" />.
4040
/// </summary>
4141
WarningsConfiguration WarningsConfiguration { get; }
42+
43+
/// <summary>
44+
/// Returns <see langword="true"/> if a warning about string values for the given enum type has not yet been performed.
45+
/// </summary>
46+
/// <param name="type">The type to check.</param>
47+
/// <returns>Whether or not a warning has been issued.</returns>
48+
bool ShouldWarnForEnumType(Type type);
4249
}

‎src/EFCore/Diagnostics/Internal/LoggingOptions.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Collections.Concurrent;
5+
46
namespace Microsoft.EntityFrameworkCore.Diagnostics.Internal;
57

68
/// <summary>
@@ -11,6 +13,8 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics.Internal;
1113
/// </summary>
1214
public class LoggingOptions : ILoggingOptions
1315
{
16+
private readonly ConcurrentDictionary<Type, bool> _warnedForStringEnums = new();
17+
1418
/// <summary>
1519
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
1620
/// 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)
98102
/// doing so can result in application failures when updating to a new Entity Framework Core release.
99103
/// </summary>
100104
public virtual WarningsConfiguration WarningsConfiguration { get; private set; } = null!;
105+
106+
/// <inheritdoc />
107+
public virtual bool ShouldWarnForEnumType(Type type)
108+
{
109+
if (_warnedForStringEnums.ContainsKey(type))
110+
{
111+
return false;
112+
}
113+
114+
_warnedForStringEnums[type] = true;
115+
return true;
116+
}
101117
}

‎src/EFCore/Diagnostics/LoggingDefinitions.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,15 @@ public abstract class LoggingDefinitions
223223
[EntityFrameworkInternal]
224224
public EventDefinitionBase? LogSkipCollectionChangeDetectedSensitive;
225225

226+
/// <summary>
227+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
228+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
229+
/// any release. You should only use it directly in your code with extreme caution and knowing that
230+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
231+
/// </summary>
232+
[EntityFrameworkInternal]
233+
public EventDefinitionBase? LogStringEnumValueInJson;
234+
226235
/// <summary>
227236
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
228237
/// the same compatibility standards as public APIs. It may be changed or removed without notice in

‎src/EFCore/Diagnostics/TypeEventData.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.EntityFrameworkCore.Diagnostics;
5+
6+
/// <summary>
7+
/// A <see cref="DiagnosticSource" /> event payload class for events that reference a <see cref="Type" />.
8+
/// </summary>
9+
/// <remarks>
10+
/// See <see href="https://aka.ms/efcore-docs-diagnostics">Logging, events, and diagnostics</see> for more information and examples.
11+
/// </remarks>
12+
public class TypeEventData : EventData
13+
{
14+
/// <summary>
15+
/// Constructs the event payload.
16+
/// </summary>
17+
/// <param name="eventDefinition">The event definition.</param>
18+
/// <param name="messageGenerator">A delegate that generates a log message for this event.</param>
19+
/// <param name="clrType">The <see cref="Type"/> associated with this event.</param>
20+
public TypeEventData(
21+
EventDefinitionBase eventDefinition,
22+
Func<EventDefinitionBase, EventData, string> messageGenerator,
23+
Type clrType)
24+
: base(eventDefinition, messageGenerator)
25+
{
26+
ClrType = clrType;
27+
}
28+
29+
/// <summary>
30+
/// The <see cref="Type"/> associated with this event.
31+
/// </summary>
32+
public virtual Type ClrType { get; }
33+
}

0 commit comments

Comments
 (0)