Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapping an enum to an integer breaks JSON data #30598

Closed
Mavox opened this issue Mar 30, 2023 · 10 comments
Closed

Mapping an enum to an integer breaks JSON data #30598

Mavox opened this issue Mar 30, 2023 · 10 comments

Comments

@Mavox
Copy link

Mavox commented Mar 30, 2023

Noticed an issue when attempting to map an enum in a json object to an integer.
When saving an enum as an integer to a json object the value is saved not as an integer but as a string with an integer in it. This breaks the mapping when reading data from the context as it expects the string to be an integer.

An attempt at an example follows:

Take the following entity:

public class MyEntity
{
    public MyJson MyJsonProperty { get; set; }
}

public class MyJson
{
    public MyEnum MyEnumProperty { get; set; }
}

public enum MyEnum
{
    MyFirstEnum = 0,
    MySecondEnum = 1
}

With the following mapping in the dbcontext

...
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilderEntity<MyEntity>().OwnsOne(x => x.MyJsonProperty, jsonBuilder =>
    {
        jsonBuilder.ToJson();
        jsonBuilder.Property(x => x.MyEnumProperty).HasConversion<int>();
    }
}
...

When saving an enum in MyEntity.MyJsonProperty.MyEnumProperty the actual Json in the database looks like this:
{ "MyEnumProperty": "1" }

And when reading the value from the database, an exception is thrown since it cannot map the string "1" to the enum MyEnum.MySecondEnum.

Exception:

System.Text.Json.JsonException: The JSON value could not be converted to System.Int32. Path: $ | LineNumber: 0 | BytePositionInLine: 3.
       ---> System.InvalidOperationException: Cannot get the value of a token type 'String' as a number.
@maumar
Copy link
Contributor

maumar commented Mar 30, 2023

I tried to reproduce this on 7.0 bits and everything seems to be working fine for me. I'm using this code:

    [ConditionalFact]
    public void Json_enum_test()
    {
        using (var ctx = new MyContext())
        {
            ctx.Database.EnsureDeleted();
            ctx.Database.EnsureCreated();

            var entity = new MyEntity { MyJsonProperty = new MyJson { MyEnumProperty = MyEnum.MySecondEnum } };
            ctx.Set<MyEntity>().Add(entity);
            ctx.SaveChanges();
        }


        using (var ctx = new MyContext())
        {
            var query = ctx.Set<MyEntity>().ToList();
        }
    }

    public class MyContext : DbContext
    {
        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<MyEntity>().OwnsOne(x => x.MyJsonProperty, jsonBuilder =>
            {
                jsonBuilder.ToJson();
                jsonBuilder.Property(x => x.MyEnumProperty).HasConversion<int>();
            });
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Repro;Trusted_Connection=True;MultipleActiveResultSets=true");
        }
    }

    public class MyEntity
    {
        public int Id { get; set; }
        public MyJson MyJsonProperty { get; set; }
    }

    public class MyJson
    {
        public MyEnum MyEnumProperty { get; set; }
    }

    public enum MyEnum
    {
        MyFirstEnum = 0,
        MySecondEnum = 1
    }

@Mavox can you modify the code above, so that it matches your scenario and reproduces the problem?
In the code above, enum is correctly saved as a number: {"MyEnumProperty":1} rather than string like it's in your case.

@Mavox
Copy link
Author

Mavox commented Mar 31, 2023

@maumar I will try to reproduce the error with the code you provided. I'll get back to you with the results.

@Mavox
Copy link
Author

Mavox commented Mar 31, 2023

I have managed to reproduce the error with the example code provided by @maumar as a baseline. Seems to be an issue not when inserting new values, but when updating values. See example code below:

[ConditionalFact]
public void Json_enum_test()
{
    using (var ctx = new MyContext())
    {
        ctx.Database.EnsureDeleted();
        ctx.Database.EnsureCreated();

        var entity = new MyEntity { MyJsonProperty = new MyJson { MyEnumProperty = MyEnum.MySecondEnum } };
        ctx.Set<MyEntity>().Add(entity);
        ctx.SaveChanges();
    }


    using (var ctx = new MyContext())
    {
        var entity = ctx.Set<MyEntity>().First();
        entity.MyJsonProperty.MyEnumProperty = MyEnum.MyFirstEnum;
        ctx.SaveChanges();
    }

    using (var ctx = new MyContext())
    {
        var entity = ctx.Set<MyEntity>().First();
    }
}

public class MyContext : DbContext
{
    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<MyEntity>().OwnsOne(x => x.MyJsonProperty, jsonBuilder =>
        {
            jsonBuilder.ToJson();
            jsonBuilder.Property(x => x.MyEnumProperty).HasConversion<int>();
        });
    }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Repro;Trusted_Connection=True;MultipleActiveResultSets=true");
    }
}

public class MyEntity
{
    public int Id { get; set; }
    public MyJson MyJsonProperty { get; set; }
}

public class MyJson
{
    public MyEnum MyEnumProperty { get; set; }
}

public enum MyEnum
{
    MyFirstEnum = 0,
    MySecondEnum = 1
}

@maumar
Copy link
Contributor

maumar commented Mar 31, 2023

@Mavox thank you for the quick response. I'm able to reproduce this on 7.0 bits and was able to confirm that it's actually a dupe of #30330.

@ajcvickers this seems like a relatively common scenario (people unhappy with by-default enum-to-string conversion will hit this when trying to update the property value), and fix is small/safe. We should maybe consider patching, given the new information.

@Mavox
Copy link
Author

Mavox commented Mar 31, 2023

@maumar If I understand #30330 correctly then this issue is not limited to enums only, but all non string values (like booleans and numbers)?

@maumar
Copy link
Contributor

maumar commented Mar 31, 2023

correct, it affects properties that have a converter, where without the converter the property would have been stored as string and with the converter it's supposed to be stored as numeric or boolean value.

@Mavox
Copy link
Author

Mavox commented Mar 31, 2023

Alright, good to know 😅. Then it may be more of an issue in my specific case than I initially thought. I'll continue to monitor #30330 and hope for a patch. Thank you for your quick and effective assistance 🥇 .

@ajcvickers
Copy link
Contributor

@maumar Yeah, let's take it to Tactics for servicing.

@maumar
Copy link
Contributor

maumar commented Apr 5, 2023

@Mavox in case you missed it, #30330 has been approved for patch and the fix will be available in the 7.0.6 version. Thanks for bringing our attention to this case.

@Mavox
Copy link
Author

Mavox commented Apr 5, 2023

@maumar I've noticed, but thank you for notifying me, very kind of you 😀.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants