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 IAsyncenumerable #988

Merged

Conversation

ElizabethOkerio
Copy link
Contributor

@ElizabethOkerio ElizabethOkerio commented Jul 13, 2023

This PR fixes #390

Currently, If you have such a controller action method in OData AspNetCore:

 [EnableQuery]
 public IAsyncEnumerable<Customer> Get()
 {
      return _context.Customers.AsAsyncEnumerable();
 }

The request is processed asynchronously and a non-OData response is returned.

This change ensures that an OData response is returned.

The issue was, AspNetCore OData could not map an IAsyncEnumerable<> to an EdmCollectionType. To the DefaultODataTypeMapper I've added a check for an IAsyncEnumerable<>. If the collection is of type IAsyncEnumerable<> then it is mapped to an EdmCollectionType.

If we map the IAsyncEnumerable<> to an EdmCollectionType then the ODataResourceSetSerializer is used in serializing the response.

To the ODataResourceSetSerializer I've added a check for the collection type. If the type is of IAsyncEnumerable<> then we use the await foreach to loop through the resources in the collection. Otherwise, we use the normal foreach loop.

@ElizabethOkerio ElizabethOkerio force-pushed the feature/support_IAsyncEnumerable branch 2 times, most recently from e55bd00 to facbbc1 Compare July 17, 2023 08:33
@ElizabethOkerio ElizabethOkerio marked this pull request as ready for review July 17, 2023 08:59
@ElizabethOkerio ElizabethOkerio force-pushed the feature/support_IAsyncEnumerable branch from aec82d6 to 8b70194 Compare July 17, 2023 09:25
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.

It doesn't appear that we are processing the IAsyncEnumerable asynchronously (i.e. using await foreach). I think that's the whole point of this PR.

@habbes
Copy link
Contributor

habbes commented Jul 17, 2023

I think you should also test with IAsyncEnumerable objects that are not IEnumerable to make sure we support that scenario as well.

@ElizabethOkerio ElizabethOkerio force-pushed the feature/support_IAsyncEnumerable branch 2 times, most recently from 163a3ef to 2c9bc0f Compare July 18, 2023 09:38
@ElizabethOkerio ElizabethOkerio force-pushed the feature/support_IAsyncEnumerable branch from 32bfb13 to cdc77e0 Compare July 20, 2023 18:17
habbes
habbes previously approved these changes Aug 7, 2023
xuzhg
xuzhg previously approved these changes Aug 7, 2023
Copy link
Member

@xuzhg xuzhg left a comment

Choose a reason for hiding this comment

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

:shipit:

@ElizabethOkerio ElizabethOkerio force-pushed the feature/support_IAsyncEnumerable branch from 99139b7 to ad74c81 Compare August 8, 2023 07:39
@ElizabethOkerio ElizabethOkerio merged commit fa81478 into OData:main Aug 8, 2023
2 checks passed
ElizabethOkerio added a commit to ElizabethOkerio/AspNetCoreOData that referenced this pull request Aug 8, 2023
@aldrashan
Copy link
Contributor

Doesn't appear to work in all cases?
See OData/WebApi#2598 (comment)

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.

Async odata endpoint not returning edm model
5 participants