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

Unknown database enum values are read as the default enum value when using .HasConversion<string> #24084

Closed
Zero3 opened this issue Feb 8, 2021 · 4 comments · Fixed by #24527
Labels
area-change-tracking breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@Zero3
Copy link

Zero3 commented Feb 8, 2021

When querying an entity with an enum field that has been mapped with .HasConversion<string>() (or manually with an EnumToStringConverter<>), Entity Framework reads unknown values in the database as the default value of the enum. This fallback behaviour is undesirable, as it can cause silent data corruption in memory.

Example

Enum in code

public enum State
{
	MyFirstState,
	MySecondState,
	MyThirdState
}

Data in database

Id	State
1	MyThirdState
2	MyThirdState
3	MyFourthState

Expected behaviour

Entity Framework throws an exception, either at model time or query time, as MyFourthState is not a recognized State value.

Actual behaviour

Entity Framework silently reads MyFourthState as MyFirstState, leading to data corruption in memory.

Entity Framework seems to assume that it is valid to translate the unknown string value to the default value of enums (i.e. whatever string represents 0 internally, in this case the first state).

Offending code

Seems to be the default fallback in this method in StringEnumConverter.cs, but I did not test:

        private static TEnum ConvertToEnum(string value)
            => Enum.TryParse<TEnum>(value, out var result)
                ? result
                : Enum.TryParse(value, true, out result)
                    ? result
                    : ulong.TryParse(value, out var ulongValue)
                        ? (TEnum)(object)ulongValue
                        : long.TryParse(value, out var longValue)
                            ? (TEnum)(object)longValue
                            : default;

Notes

I am unaware of whether the current behaviour is by design for some reason unbeknownst to me. Maybe there are other use cases that depend on the current behaviour.

Provider and version information

EF Core version: 5.0.2
Database provider: Microsoft.EntityFrameworkCore.Sqlite (i did not test with other providers, but this issue should not be provider dependent)
Target framework: netcoreapp3.1

@ajcvickers ajcvickers self-assigned this Feb 8, 2021
@ajcvickers ajcvickers added this to the 6.0.0 milestone Feb 8, 2021
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 26, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview4 Mar 26, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-preview4, 6.0.0 Nov 8, 2021
@lbargery
Copy link

lbargery commented Mar 17, 2022

This seems like quite a breaking change...

We tended to have a None = 0 value for all our enums which dealt with this well.

Does this mean that if 3 components reference the same database and we add a new enum value (which is mapped to a string), that ALL 3 components need releasing before we could add any rows in the database containing this new value?

If so, that is quite a change in our release process and would much prefer previous behaviour (would be great to be able to pass in a "default value" to the converter)

@ajcvickers
Copy link
Member

@lbargery It should be very easy to implement your own value converters with the old behavior for your enums.

@timg83
Copy link

timg83 commented Oct 6, 2022

it seems like the new enum conversion also doesn't take into account the EnumMember attribute. EF Core 3.1 did use that attribute. This behavior seems to be changed in EF Core 6 also.

For example:
The enum member
[EnumMember(Value = "Natuurlijk Persoon")] NatuurlijkPersoon

did map correctly in EF Core 3.1. EF Core 6 throws the InvalidOperationException

@ajcvickers
Copy link
Member

@timg83 Please open a new issue and attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants