AutoMapper / AutoMapper.Extensions.OData

Creates LINQ expressions from ODataQueryOptions and executes the query.
MIT License
140 stars 38 forks source link

Improve handling of Enumerations in subqueries #170

Closed wbuck closed 1 year ago

wbuck commented 1 year ago

Currently enum's in subqueries are handled by first converting the enum to its underlying integer value and then converting that integer value to a string. This implies that the user is storing the underlying integer enum value as string's in the backing store.

I believe this ignores the two main use cases of how people actually store enum's in the backing store. Those two main use cases are:

  1. Storing enum's as int's in the backing store
  2. Storing enum variants name as string's in the backing store.

The following query:

SomeEntities($filter=SomeEnumProp eq 'SomeEnumVariant')

Produces the following expression:

SomeEntitites.Where($it => Convert($It.SomeEnumProp).ToString() == "SomeEnumVariant")

This, I believe is incorrect. Entity Framework handles such a query by converting both the member access and the constant to the underlying enum integer value and then performing the comparison:

 SomeEntitites.Where($it => Convert($It.SomeEnumProp) == 1)

This I believe is the correct way to handle enum's. This would also allow the user to store the enum's as string's in the backing store as the user of the API could cast the value prior to performing the binary comparison:

 SomeEntities($filter=cast(SomeEnumProp, Edm.String) eq 'SomeEnumVariant')

Which produces:

SomeEntities.Where($it => $it.SomeEnumProp.ToString() eq "SomeEnumVariant")
BlaiseD commented 1 year ago

Changing this mapping configuration to opts => opts.MapFrom(x => x.Grade.ToString()) will cause this test to fail.

Unless I'm mistaken the OData repo uses similar logic to ours for handling enums.

I think the operations are still granular enough to handle a cast from the request where it's possible.

wbuck commented 1 year ago

Why would you expect opts => opts.MapFrom(x => x.Grade.ToString()) to succeed here if the backing store is storing the enumerations as integers? E.g., "A" == 1 will not succeed.

What I'd like to understand is how the following is correct:

Convert(SomeEnumValue).ToString() == "123" 

How often do folks store an underlying enum value (int) as string's in their backing store?

I'd also be remiss not to point out that ApplyTo does not do that when dealing with enum's.

I would expect the same expression whether it's a sub query or not. The ApplyTo function produces the following:

Convert(SomeEnumValue) == 123;

This is how I would expect it to work.

Or are you suggesting that the AutoMapper mapping take care of the conversions?

So if the user is storing their enum's as integers then they'd use the following mapping:

CreateMap<Entity, Model>()
    .ForMember(dest => dest.MyEnum, opts => opts.MapFrom(src => (MyEnum)src.MyIntEnum));

@BlaiseD If you have a test that you can show me where the DB is storing a collection of enum's as int's that would be really helpful because no matter how I setup the mapping the query is incorrect.

BlaiseD commented 1 year ago

I would have started with. RootQuery`ApplyTo` behaves differently from subqueries. e.g. RootQuery: SomeString, QueryWithSubQuery: OtherString.

Here are the different results. Ok to create a new simplified issue if you're interested.

I've updated the issue template.

wbuck commented 1 year ago

@BlaiseD OK I've created a new issue. I'm hoping it'll be a bit more clear the issue I was seeing.