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

Support Enums as Keys in OData Client #3013

Merged
merged 8 commits into from
Jul 22, 2024
Merged

Conversation

WanjohiSammy
Copy link
Contributor

@WanjohiSammy WanjohiSammy commented Jul 4, 2024

Issues

This pull request fixes #2964.

Description

This change involves adding functionality to support the use of enumeration types as keys.

Currently, attempting to query entities with enums as keys through OData Client, results in an error stating that the operation is not valid for non-generic types.

Exception:
This operation is only valid on generic types.

Change:
This modification will address this issue, allowing enums as keys without causing exceptions.
For example, the below Employee object has EmpType which is a key of Type Enum and NullableEmpType which is nullable generic Type with Enum as generic argument.

public class Employee
{
    [System.ComponentModel.DataAnnotations.Key]
    public int EmpNumber { get; set; }

    // Enum - Employee Type Key
    [System.ComponentModel.DataAnnotations.Key]
    public EmployeeType EmpType { get; set; }

    [System.ComponentModel.DataAnnotations.Key]
    public EmployeeType? NullableEmpType { get; set; }

    public string Name { get; set; }

    public decimal Salary { get; set; }
}

public enum EmployeeType
{
    Hybrid = 1,
    FullTime = 2,
    PartTime = 3
}

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

@WanjohiSammy WanjohiSammy changed the title Support Enums as Keys Support Enums as Keys in OData Client Jul 4, 2024
Copy link
Contributor

@habbes habbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to add a test that shows that querying data by keys with enum executes corectly without throwing an exception, just to make sure that the big-picture scenario that is describe in the original issue is resolved.

mikepizzo
mikepizzo previously approved these changes Jul 12, 2024
@WanjohiSammy
Copy link
Contributor Author

I think it would be good to add a test that shows that querying data by keys with enum executes corectly without throwing an exception, just to make sure that the big-picture scenario that is describe in the original issue is resolved.

@habbes Yes, this can be done. However, I have added tests on the specific methods that are called when getting KeyProperties. GetKeyProperties() method is the one that was throwing exception when enum was used as keys.

test/FunctionalTests/Microsoft.OData.Client.Tests/Metadata/ClientTypeUtilTests.cs

@gathogojr
Copy link
Contributor

#3013 (comment)

@WanjohiSammy Probably add a test(s) to verify that nullable properties are not permitted (other than nullable enum property). Also create an issue to fix it in next major release such that no nullable property area allowed. We published ODL 8 RC so I don't think we can sneak in the change in ODL 8

@WanjohiSammy
Copy link
Contributor Author

#3013 (comment)

@WanjohiSammy Probably add a test(s) to verify that nullable properties are not permitted (other than nullable enum property). Also create an issue to fix it in next major release such that no nullable property area allowed. We published ODL 8 RC so I don't think we can sneak in the change in ODL 8

@gathogojr I already added a test for this case where I am using a Key of Type struct. This will throw an exception.
IFTypeProperty_HasNullableGenericTypeKey_OfTypeStruct_GetKeyPropertiesOnType_ThrowsException()

@WanjohiSammy
Copy link
Contributor Author

I think it would be good to add a test that shows that querying data by keys with enum executes corectly without throwing an exception, just to make sure that the big-picture scenario that is describe in the original issue is resolved.

@habbes Thanks for this. I have added these tests in the file test/FunctionalTests/Microsoft.OData.Client.Tests/Tracking/DataServiceContextQueryTests.cs

mikepizzo
mikepizzo previously approved these changes Jul 15, 2024
habbes
habbes previously approved these changes Jul 17, 2024
mikepizzo
mikepizzo previously approved these changes Jul 18, 2024
Copy link
Member

@mikepizzo mikepizzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution and follow-up on feedback.

@WanjohiSammy WanjohiSammy dismissed stale reviews from mikepizzo and habbes via 6a5667f July 19, 2024 15:24
@WanjohiSammy WanjohiSammy merged commit c313eae into main Jul 22, 2024
5 checks passed
WanjohiSammy added a commit that referenced this pull request Jul 22, 2024
* Checkout from 'main'

* Add and modify test cases

* Only allow nullable Generic Type Enum

* Adding a test that throws Exception if key is nullable generic type with struct as generic argument

* Adding a test to mock querying data with enum as keys

* Add a test to select single entity

* Add test to filter by enum member name and a test to use ByKey to filter by composite keys

* Add a test to filter by enum key only using ByKey()
WanjohiSammy added a commit that referenced this pull request Jul 23, 2024
…nteger value in squote and without squote (#3018)

* Support Enums as Keys in OData Client (#3013)

* Checkout from 'main'

* Add and modify test cases

* Only allow nullable Generic Type Enum

* Adding a test that throws Exception if key is nullable generic type with struct as generic argument

* Adding a test to mock querying data with enum as keys

* Add a test to select single entity

* Add test to filter by enum member name and a test to use ByKey to filter by composite keys

* Add a test to filter by enum key only using ByKey()

* Allow $filtering with enum integer value in squote and without squote (#3014)

* Allow comparison with underlying integral for Enum member and add capability to convert integral value to enum member name

* Bump with tests

* Move helper methods to EnumHelper, use them to check for enum values and adds more tests to handle float, long and int.

* Use explicit/exact type instead of 'var'

* Add more tests to handle huge, long values

* Add assertion for comparing enum member name with the result

* Move extension methods from Edm EnumHelper.cs to Core in EdmExtensionMethods.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Odata returns generic type error when enum is a key
6 participants