Odata returns generic type error when enum is a key

[biboller]

Our project uses enums as keys. For example we use weekday as a key here.

This wouldn’t be a problem expect that when getting the data from odata the mapping fails with the following error:

       This operation is only valid on generic types.

Assemblies affected

Microsoft.OData.Core 7.x

Reproduce steps

Add an enum as a key

Expected result

We expect enum's to be useable as keys

Additional detail

In the file OdatatypeInfo.cs in the method GetKeyProperties() we found that the program assumes that the property should be generic type and afterwards checks if it is an enum.

We think the order should be changed in the if statement and the IsEnum() should be inverted.

Are we over looking something? We tried getting, updating and posting data with this change. Which all worked without problem

[mikepizzo]

Yes, we should support enums as keys.

@gathogojr -- Note: that the code shown above as the current logic in the first block is slightly different than what I see in the repo.

In the repo, the conditional check is:

                if (!PrimitiveType.IsKnownType(key.PropertyType) && !(key.PropertyType.GetGenericTypeDefinition() == typeof(System.Nullable<>) && key.PropertyType.GetGenericArguments().First().IsEnum()))

which is different than above reported code in that:

1. the IsEnum check is on keyProperty.PropertyTypeGetGenericArguments().First().IsEnum(), rather than keyProperty.PropertyType.IsEnum()
2. the IsEnum check is grouped with the GetGenericTypeDefinition() check rather than a separate check

I suspect the reason that the fixed code doesn't throw is not because of ordering, or even grouping, but because it doesn't call GetGenericArguments.

I'm not sure the rationale behind the existing logic that would make us think that an unknown type must be generic (or why we specifically check for an enum if it is), but if we don't know for sure that key.PropertyType is generic we should check IsGenericType before calling GetGenericTypeDefinition or GetGenericArguments.

[kyleweishaupt]

I'm also encountering the same issue when attempting to access the Microsoft Business Central API through OData V4. We're using the latest Microsoft.OData NuGet packages, 7.21.1. Any attempt to enumerate a collection with an enum key returns the error about generic types, even when trying to project to a new class.

[nmushovic]

To expand on the above, it looks like the Business Central API converted quite a few string to Enums. The ItemReference class uses an Enum as a key field and we're getting the error in GetKeyProperties().

[biboller]

@mikepizzo Yes the problem seems to be that GetGenericTypeDefinition is called too early. The code should accept simple types and enums as valid keys and reject generic types. So changing the ordering of the if statement ensures that GetGenericTypeDefinition isn't called if the key is an enum. Can we contribute on this issue by creating a pr to fix this?

[MikkelGlerup]

Any news on this?
We're having the same problem when using Odata together with Business Central

[nmushovic]

We used a work around by selecting odatakeys and not selecting the enum key fields. https://www.cloudfronts.com/blog/d365-business-central/using-odatakeyfields-for-fetching-records-in-business-central-web-services/

[MikkelGlerup]

Sadly that isn't going to work for me.
I'm getting the error when running this code
var linesQuery = new DataServiceCollection<MtSalesInvoiceLine>(_mtApiContext.MtSalesInvoiceLines.AddQueryOption("$expand", "dimensionSetLines").Where(s => s.DocumentId == salesOrder.Id));

There's an enum key field somewhere in the hierarchy.

Will update when I find a solution though

[MikkelGlerup]

Found out my error was due to breaking changes being introduced on Business Centrals side with version "24.1.18927.19282". Running the same code towards an Business Central Cloud environment running "24.0.16410.17628" succeeds.

[nmushovic]

@MikkelGlerup , do you have a link to the breaking change?

[MikkelGlerup]

@nmushovic I made my own issue and gave it more details here

EDIT:
I was wrong, it had nothing to do with version

[martinfbluesoftcz]

Hello, can someone to aprove this fix: #3013
It is critical because our app which communicates with MS Bussiness Central does not work after new API definition with enums was updated.
@gathogojr @habbes @xuzhg @ElizabethOkerio @paulodero
Thank you very much.

[mikepizzo]

See my comment in the issue. I'm not sure why we allow nullable<enum>, since keys have to be non-nullable, but changing that now would be a breaking change. However, the proposed change allows *any* nullable<T>, which makes the issue much worse. I've suggested a change to support enums and nullable<enum>, but not other nullables. Please take a look. Thanks,

…

-Mike From: martinfbluesoftcz ***@***.***> Sent: Thursday, July 11, 2024 8:20 AM To: OData/odata.net ***@***.***> Cc: Comment ***@***.***> Subject: Re: [OData/odata.net] Odata returns generic type error when enum is a key (Issue #2964) Hello, can someone to aprove this fix: #3013<#3013> It is critical because our app which communicates with MS Bussiness Central does not work after new API definition with enums was updated. @gathogojr<https://github.com/gathogojr> @habbes<https://github.com/habbes> @xuzhg<https://github.com/xuzhg> @ElizabethOkerio<https://github.com/ElizabethOkerio> @paulodero<https://github.com/paulodero> Thank you very much. - Reply to this email directly, view it on GitHub<#2964 (comment)> or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACCCOXSACC6225EQGE7N26TZL2PATBFKMF2HI4TJMJ2XIZLTSWBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVEYTENBTGUYTKNZXURXGC3LFVFUGC427NRQWEZLMQKSXMYLMOVS2SNRRGI2DCNRXGUZ2I3TBNVS2S2DBONPWYYLCMVWKY43VMJVGKY3UL52HS4DFVREXG43VMVBW63LNMVXHJJTUN5YGSY3TSSBKI5DZOBS2U4TFOBXXG2LUN5ZHTJLWMFWHKZNIGIZTGMBYGMZTRAVEOR4XAZNFNFZXG5LFUV3GC3DVMWVDEMRZG43DANRRGI3YFJDUPFYGLJLMMFRGK3FFOZQWY5LFVEYTENBTGUYTKNZXQKSHI6LQMWSWYYLCMVWKK5TBNR2WLKJWGEZDIMJWG42THJ3UOJUWOZ3FOKTGG4TFMF2GK>. You are receiving this email because you commented on the thread. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[martinfbluesoftcz]

@mikepizzo It looks fine now.

[martinBluesoft]

Do you know when a new version with a fix for this bug will be released?