-
Notifications
You must be signed in to change notification settings - Fork 171
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
base: main
Are you sure you want to change the base?
Conversation
This is the design draft to support minimal API
docs/designs/Enable.MinimalApi.md
Outdated
|
||
## 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. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
docs/designs/Enable.MinimalApi.md
Outdated
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) |
There was a problem hiding this comment.
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.
docs/designs/Enable.MinimalApi.md
Outdated
|
||
I think that’s true because: | ||
|
||
- It’s minimal API, developers only need the data with query functionalities. |
There was a problem hiding this comment.
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.
docs/designs/Enable.MinimalApi.md
Outdated
- 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>. |
There was a problem hiding this comment.
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.
How will it work with versioning? Using this https://github.com/dotnet/aspnet-api-versioning |
@xuzhg Is there an ETA when this can be released ? at least as a prerelase version or temporarely as completely different nuget ? |
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.