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

…rings

Problem was that in 7.0 we stored enums inside JSON as strings by default (by applying EnumToStringConverter by convention), but in 8.0 we are changing this to int.
This is a breaking change and it's extra problematic for databases that used EF JSON functionality in 7.0. This can easily create a scenario where there is a mix of string and int representation for an enum value within the same document.
(some data was added in 7.0, and then some in 8.0 before customer realized that the breaking change has been made). To mitigate this we are adding a fallback mechanism when reading enum data that is part of JSON entity. We try to read as int and if that fails we try to read again as string. This way should minimize the disruption, moreover any data saved back to the database will be saved in the new format, so over time everything should normalize.
We will still throw when projecting individual enum properties of a JSON entity (as opposed to the entire entity), because materialization for this goes through different path, indistinguishable from normal enum value read from column in relational table.

Fixes #31100

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

@@ -50,13 +50,5 @@ public class RelationalMapToJsonConvention : IEntityTypeAnnotationChangedConvent
         IConventionModelBuilder modelBuilder,
         IConventionContext<IConventionModelBuilder> context)
     {
-        foreach (var jsonEntityType in modelBuilder.Metadata.GetEntityTypes().Where(e => e.IsMappedToJson()))
-        {
-            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));
-            }
-        }
     }
 }

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

@@ -64,6 +64,12 @@ private sealed partial class ShaperProcessingExpressionVisitor : ExpressionVisit
         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;
@@ -2339,7 +2345,6 @@ Expression valueExpression
             {
                 // 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);
@@ -2357,6 +2362,47 @@ Expression valueExpression
                     resultExpression);
             }
 
+            // see issue #31100
+            // in 7.0 we stored enums as strings by default (by applying EnumToStringConverter by convention on enums inside JSON entities)
+            // in 8.0 we are reverting this decision and store enums as int (their natural representation in JSON as well as in database using EF)
+            // however it is a breaking change and it makes it very hard for people who used 7.0 version to work with legacy representation
+            // users can easily land in a situation where they have a mix of int and string representations
+            // update to 8.0, start adding more entities to the database, new entities are stored as ints, whereas old ones remain string
+            // to mitigate this we provide fallback mechanism here - if we can't read enum value using new default (EnumToNumberConverter)
+            // we assume it's the legacy scenario and attempt to parse from string
+            if (property.ClrType.UnwrapNullableType().IsEnum
+                && property.GetRelationalTypeMapping().Converter is ValueConverter converter
+                && converter.GetType() is Type { IsGenericType: true } converterType
+                && converterType.GetGenericTypeDefinition() == typeof(EnumToNumberConverter<,>)
+                && converterType.GetGenericArguments() is Type[] genericArgs
+                && genericArgs[0] == converter.ModelClrType
+                && genericArgs[1] == converter.ProviderClrType)
+            {
+                // try
+                // {
+                //     JsonConvertedValueReaderWriter<MyEnumType, underlying_type>.FromJsonTyped(...));
+                // }
+                // catch(InvalidOperationException e)
+                // {
+                //      (MyEnumType)Enum.Parse(typeof(MyEnumType), jsonReaderManager.CurrentReader.GetString());
+                // }
+                var exceptionParameter = Parameter(typeof(InvalidOperationException), name: "e");
+                var catchBlock = Catch(
+                    exceptionParameter,
+                    Convert(
+                        Call(
+                            EnumParseMethodInfo,
+                            Constant(converter.ModelClrType),
+                            Call(
+                                Field(
+                                    jsonReaderManagerParameter,
+                                    Utf8JsonReaderManagerCurrentReaderField),
+                                Utf8JsonReaderGetStringMethod)),
+                        property.ClrType));
+
+                resultExpression = TryCatch(resultExpression, catchBlock);
+            }
+
             if (_detailedErrorsEnabled)
             {
                 var exceptionParameter = Parameter(typeof(Exception), name: "e");

test/EFCore.Relational.Specification.Tests/Query/JsonQueryAdHocTestBase.cs

@@ -707,6 +707,123 @@ public MyJsonEntityShadowPropertiesWithCtor(string name)
 
     #endregion
 
+    //#region EnumLegacyValues
+
+    //[ConditionalTheory]
+    //[MemberData(nameof(IsAsyncData))]
+    //public virtual async Task Read_enum_property_with_legacy_values(bool async)
+    //{
+    //    var contextFactory = await InitializeAsync<MyContextEnumLegacyValues>(
+    //        seed: SeedEnumLegacyValues);
+
+    //    using (var context = contextFactory.CreateContext())
+    //    {
+    //        var query = context.Entities.Select(x => new
+    //        {
+    //            x.Reference.IntEnum,
+    //            x.Reference.ByteEnum,
+    //            x.Reference.LongEnum,
+    //            x.Reference.NullableEnum
+    //        });
+
+    //        var result = async
+    //            ? await query.ToListAsync()
+    //            : query.ToList();
+
+    //        Assert.Equal(1, result.Count);
+    //        //Assert.Equal("Foo", result[0].ShadowString);
+    //        //Assert.Equal(143, result[0].ShadowInt);
+    //    }
+    //}
+
+    //[ConditionalTheory]
+    //[MemberData(nameof(IsAsyncData))]
+    //public virtual async Task Read_json_entity_with_enum_properties_with_legacy_values(bool async)
+    //{
+    //    var contextFactory = await InitializeAsync<MyContextEnumLegacyValues>(
+    //        seed: SeedEnumLegacyValues);
+
+    //    using (var context = contextFactory.CreateContext())
+    //    {
+    //        var query = context.Entities.Select(x => x.Reference).AsNoTracking();
+
+    //        var result = async
+    //            ? await query.ToListAsync()
+    //            : query.ToList();
+
+    //        Assert.Equal(1, result.Count);
+    //        //Assert.Equal("Foo", result[0].ShadowString);
+    //        //Assert.Equal(143, result[0].ShadowInt);
+    //    }
+    //}
+
+    //protected abstract void SeedEnumLegacyValues(MyContextEnumLegacyValues ctx);
+
+    //protected class MyContextEnumLegacyValues : DbContext
+    //{
+    //    public MyContextEnumLegacyValues(DbContextOptions options)
+    //        : base(options)
+    //    {
+    //    }
+
+    //    public DbSet<MyEntityEnumLegacyValues> Entities { get; set; }
+
+    //    protected override void OnModelCreating(ModelBuilder modelBuilder)
+    //    {
+    //        modelBuilder.Entity<MyEntityEnumLegacyValues>().Property(x => x.Id).ValueGeneratedNever();
+    //        modelBuilder.Entity<MyEntityEnumLegacyValues>().OwnsOne(x => x.Reference, b =>
+    //        {
+    //            b.ToJson();
+    //        });
+    //        modelBuilder.Entity<MyEntityEnumLegacyValues>().OwnsMany(x => x.Collection, b =>
+    //        {
+    //            b.ToJson();
+    //        });
+    //    }
+    //}
+
+    //public class MyEntityEnumLegacyValues
+    //{
+    //    public int Id { get; set; }
+    //    public string Name { get; set; }
+
+    //    public MyJsonEntityEnumLegacyValues Reference { get; set; }
+    //    public List<MyJsonEntityEnumLegacyValues> Collection { get; set; }
+    //}
+
+    //public class MyJsonEntityEnumLegacyValues
+    //{
+    //    public string Name { get; set; }
+
+    //    public IntEnumLegacyValues IntEnum { get; set; }
+    //    public ByteEnumLegacyValues ByteEnum { get; set; }
+    //    public LongEnumLegacyValues LongEnum { get; set; }
+    //    public IntEnumLegacyValues? NullableEnum { get; set; }
+    //}
+
+    //public enum IntEnumLegacyValues
+    //{
+    //    Foo,
+    //    Bar,
+    //    Baz,
+    //}
+
+    //public enum ByteEnumLegacyValues : byte
+    //{
+    //    Seattle,
+    //    Redmond,
+    //    Bellevue,
+    //}
+
+    //public enum LongEnumLegacyValues : long
+    //{
+    //    One,
+    //    Two,
+    //    Three,
+    //}
+
+    //#endregion
+
     protected TestSqlLoggerFactory TestSqlLoggerFactory
         => (TestSqlLoggerFactory)ListLoggerFactory;
 }

test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryAdHocSqlServerTest.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.Data.SqlClient;
+
 namespace Microsoft.EntityFrameworkCore.Query;
 
 public class JsonQueryAdHocSqlServerTest : JsonQueryAdHocTestBase
@@ -140,4 +142,158 @@ protected override void SeedShadowProperties(MyContextShadowProperties ctx)
 1,
 N'e1')");
     }
+
+    #region EnumLegacyValues
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Read_enum_property_with_legacy_values(bool async)
+    {
+        var contextFactory = await InitializeAsync<MyContextEnumLegacyValues>(
+            seed: SeedEnumLegacyValues);
+
+        using (var context = contextFactory.CreateContext())
+        {
+            var query = context.Entities.Select(x => new
+            {
+                x.Reference.IntEnum,
+                x.Reference.ByteEnum,
+                x.Reference.LongEnum,
+                x.Reference.NullableEnum
+            });
+
+            var exception = async
+                ? await (Assert.ThrowsAsync<SqlException>(() => query.ToListAsync()))
+                : Assert.Throws<SqlException>(() => query.ToList());
+
+            // Conversion failed when converting the nvarchar value '...' to data type int
+            Assert.Equal(245, exception.Number);
+        }
+    }
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Read_json_entity_with_enum_properties_with_legacy_values(bool async)
+    {
+        var contextFactory = await InitializeAsync<MyContextEnumLegacyValues>(
+            seed: SeedEnumLegacyValues);
+
+        using (var context = contextFactory.CreateContext())
+        {
+            var query = context.Entities.Select(x => x.Reference).AsNoTracking();
+
+            var result = async
+                ? await query.ToListAsync()
+                : query.ToList();
+
+            Assert.Equal(1, result.Count);
+            Assert.Equal(ByteEnumLegacyValues.Redmond, result[0].ByteEnum);
+            Assert.Equal(IntEnumLegacyValues.Foo, result[0].IntEnum);
+            Assert.Equal(LongEnumLegacyValues.Three, result[0].LongEnum);
+            Assert.Equal(IntEnumLegacyValues.Bar, result[0].NullableEnum);
+        }
+    }
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Read_json_entity_collection_with_enum_properties_with_legacy_values(bool async)
+    {
+        var contextFactory = await InitializeAsync<MyContextEnumLegacyValues>(
+            seed: SeedEnumLegacyValues);
+
+        using (var context = contextFactory.CreateContext())
+        {
+            var query = context.Entities.Select(x => x.Collection).AsNoTracking();
+
+            var result = async
+                ? await query.ToListAsync()
+                : query.ToList();
+
+            Assert.Equal(1, result.Count);
+            Assert.Equal(2, result[0].Count);
+            Assert.Equal(ByteEnumLegacyValues.Bellevue, result[0][0].ByteEnum);
+            Assert.Equal(IntEnumLegacyValues.Foo, result[0][0].IntEnum);
+            Assert.Equal(LongEnumLegacyValues.One, result[0][0].LongEnum);
+            Assert.Equal(IntEnumLegacyValues.Bar, result[0][0].NullableEnum);
+            Assert.Equal(ByteEnumLegacyValues.Seattle, result[0][1].ByteEnum);
+            Assert.Equal(IntEnumLegacyValues.Baz, result[0][1].IntEnum);
+            Assert.Equal(LongEnumLegacyValues.Two, result[0][1].LongEnum);
+            Assert.Null(result[0][1].NullableEnum);
+        }
+    }
+
+    private void SeedEnumLegacyValues(MyContextEnumLegacyValues ctx)
+    {
+        ctx.Database.ExecuteSqlRaw(@"INSERT INTO [Entities] ([Collection], [Reference], [Id], [Name])
+VALUES(
+N'[{{""ByteEnum"":""Bellevue"",""IntEnum"":""Foo"",""LongEnum"":""One"",""Name"":""e1_c1"",""NullableEnum"":""Bar""}},{{""ByteEnum"":""Seattle"",""IntEnum"":""Baz"",""LongEnum"":""Two"",""Name"":""e1_c2"",""NullableEnum"":null}}]',
+N'{{""ByteEnum"":""Redmond"",""IntEnum"":""Foo"",""LongEnum"":""Three"",""Name"":""e1_r"",""NullableEnum"":""Bar""}}',
+1,
+N'e1')");
+    }
+
+    private class MyContextEnumLegacyValues : DbContext
+    {
+        public MyContextEnumLegacyValues(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        public DbSet<MyEntityEnumLegacyValues> Entities { get; set; }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            modelBuilder.Entity<MyEntityEnumLegacyValues>().Property(x => x.Id).ValueGeneratedNever();
+            modelBuilder.Entity<MyEntityEnumLegacyValues>().OwnsOne(x => x.Reference, b =>
+            {
+                b.ToJson();
+            });
+            modelBuilder.Entity<MyEntityEnumLegacyValues>().OwnsMany(x => x.Collection, b =>
+            {
+                b.ToJson();
+            });
+        }
+    }
+
+    private class MyEntityEnumLegacyValues
+    {
+        public int Id { get; set; }
+        public string Name { get; set; }
+
+        public MyJsonEntityEnumLegacyValues Reference { get; set; }
+        public List<MyJsonEntityEnumLegacyValues> Collection { get; set; }
+    }
+
+    private class MyJsonEntityEnumLegacyValues
+    {
+        public string Name { get; set; }
+
+        public IntEnumLegacyValues IntEnum { get; set; }
+        public ByteEnumLegacyValues ByteEnum { get; set; }
+        public LongEnumLegacyValues LongEnum { get; set; }
+        public IntEnumLegacyValues? NullableEnum { get; set; }
+    }
+
+    private enum IntEnumLegacyValues
+    {
+        Foo,
+        Bar,
+        Baz,
+    }
+
+    private enum ByteEnumLegacyValues : byte
+    {
+        Seattle,
+        Redmond,
+        Bellevue,
+    }
+
+    private enum LongEnumLegacyValues : long
+    {
+        One,
+        Two,
+        Three,
+    }
+
+    #endregion
 }