Skip to content

Enable MinimalApi for OData design #1431

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Enable MinimalApi for OData design #1431

wants to merge 7 commits into from

Conversation

xuzhg
Copy link
Member

@xuzhg xuzhg commented Mar 4, 2025

This is the design draft to support minimal API.

Seeking for reviews and feedbacks.

Don't hesitate to leave your comments in the PRs.

If you want to try the changes, just fetch it from the branch
https://github.com/OData/AspNetCoreOData/tree/miniApi_9x.

Try the 'ODataMiniApi' project from the AspNetCoreOData.sln.

It's not finished yet and I will update this branch frequently.

This is the design draft to support minimal API

## Serialization

The result of OData query will be serialized as normal JSON payload. Be noted, it doesn’t contain the OData control metadata, for example @odata.context.

Choose a reason for hiding this comment

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

Would that also include @odata.count, since that's essential in paging situations

Copy link
Member Author

@xuzhg xuzhg Mar 4, 2025

Choose a reason for hiding this comment

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

Would that also include @odata.count, since that's essential in paging situations

If you return PageResult<T> from the route handler, it should contain the 'count' and 'nextlink' properties. I will update the sample to refresh that.

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the better way to specify the PageSize (Server side) value for the route handler?
I think maybe to use the metadata. for example

app.MapGet("/customers", () => ....).WithPageSize(5);

@mguinness

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, Ok. I remembered that I have the overload for the 'AddODataQueryEndpointFilter' to config the query settings.
I updated the docs to refresh this part.

Choose a reason for hiding this comment

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

Wasn't aware of PageResult so that works. Regarding page size, that would suffice if EnableQuery attribute isn't possible.

1. First, we need the “prefix” twice to map the route handler and Edm Model.
2. Second, it could be confusing with the existing controller/action mode.

- Build the Edm model on the fly (Edm model-less)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we build an edm model on the fly, so we can support model-less scenarios, I think this makes a case of support model-less in the query options parsers, expression trees and other parts of our stack. That way we can achieve model-less without paying the cost of building and caching a model on the fly. This is separate issue from this design, we can investigate it separately. It's something to consider is we're currently investigating perf improvements to the parser.


I think that’s true because:

- It’s minimal API, developers only need the data with query functionalities.
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that there are customers for whom query options support is sufficient and they don't need an OData compliant service. But this statement rules out customers building OData services (including OData-compliant responses) that would prefer or benefit from using minimal APIs over controllers. Are the insights that there's not demand from this group? Or is that we should prioritize this use case later? For example, the comment from @mguinness already suggests that some OData conventions and semantics are desirable in the response.

- It’s minimal API, developers only need the data with query functionalities.
- More important, Minimal API removes the formatters because and just because it’s minimal.

In this case, we must add the JsonConverter to all OData query related classes, for example: SelectSome<T>.
Copy link
Contributor

Choose a reason for hiding this comment

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

There was an exploration of porting the OData Json writer to System.Text.Json JsonConverters, that investigation got stalled at some point, but this is a good opportunity to pick it up again. I will share a proposal soon.

@Lonli-Lokli
Copy link

How will it work with versioning? Using this https://github.com/dotnet/aspnet-api-versioning

@Kebechet
Copy link

Kebechet commented Mar 29, 2025

@xuzhg Is there an ETA when this can be released ? at least as a prerelase version or temporarely as completely different nuget ?

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.

5 participants