Add Constructor to ODataQueryOptions and ODataQueryOptions<TEntity> for Dictionary-Based Initialization Without HttpRequest and optional IEdmModel #1537
Conversation
| /// <param name="model">The EDM model (can be null for non-model scenarios).</param> | ||
| /// <param name="queryParameters">The OData query parameters as a dictionary.</param> | ||
| /// <param name="path">The ODataPath (optional, can be null).</param> | ||
| public ODataQueryOptions(IEdmModel model, IDictionary<string, string> queryParameters, ODataPath path = null) |
There was a problem hiding this comment.
Can you add another constructor without the IEdmModel model parameter to support non-model scenarios better?
There was a problem hiding this comment.
@wertzui I have added it. Could you please take a look
There was a problem hiding this comment.
@wertzui Could you try to use it with your scenario and share a sample project that can be used as a testing
There was a problem hiding this comment.
Do you have a nightly NuGet feed or something similar where I can get this specific version? You can find my current implementation without these changes at https://github.com/wertzui/AutoMCP
There was a problem hiding this comment.
@wertzui I have generated a nightly that you can use. Here is the link:
Microsoft.AspNetCore.OData 9.4.1-Nightly202511050747
There was a problem hiding this comment.
I tried it, you can get the source here.
However I got this exception:
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
---> System.ArgumentNullException: Value cannot be null. (Parameter 'request')
at Microsoft.AspNetCore.OData.Extensions.HttpRequestExtensions.ODataFeature(HttpRequest request)
at Microsoft.AspNetCore.OData.Query.ODataQueryOptions.ApplyTo(IQueryable query, ODataQuerySettings querySettings)
at Microsoft.AspNetCore.OData.Query.ODataQueryOptions`1.ApplyTo(IQueryable query, ODataQuerySettings querySettings)
at Microsoft.AspNetCore.OData.Query.ODataQueryOptions.ApplyTo(IQueryable query)
at Microsoft.AspNetCore.OData.Query.ODataQueryOptions`1.ApplyTo(IQueryable query)
at AutoMcp.Example.Controllers.WeatherForecastController.GetMultiple(ODataQueryOptions`1 options)
I think, the problem is the kind of "late-binding" which is used.
Instead of having something like a factory which creates an ODataQueryOptions without any dependencies, the dependencies are only resolved when ApplyTo() is called and only then the actual expressions are created.
|
/AzurePipelines run |
|
No pipelines are associated with this pull request. |
|
/AzurePipelines run |
|
No pipelines are associated with this pull request. |
|
/AzurePipelines run |
|
No pipelines are associated with this pull request. |
|
/AzurePipelines run |
|
No pipelines are associated with this pull request. |
WanjohiSammy
changed the title
| context.ElementType, | ||
| context.NavigationSource, | ||
| queryParameters, | ||
| context.RequestContainer); |
There was a problem hiding this comment.
where do you get the RequestContainer? you use 'context.RequestContainer', is it valid?
| /// the given <paramref name="elementClrType"/>.</param> | ||
| /// <param name="elementClrType">The CLR type of the element of the collection being queried.</param> | ||
| /// <param name="path">The parsed <see cref="ODataPath"/>.</param> | ||
| public ODataQueryOptions(IDictionary<string, string> queryParameters, Type elementClrType, IEdmModel model = null, ODataPath path = null) |
There was a problem hiding this comment.
My suggestion is to accept
- IserviceProvider or
- other required services?
There was a problem hiding this comment.
Which services are required by ODataQueryOptions?
Description
fixes #1531
This PR introduces a new constructor to both
ODataQueryOptionsandODataQueryOptions<TEntity>that allows initialization directly from anIDictionary<string, string>and an optionalIEdmModel/ODataPath, removing the requirement for an HttpRequest or ASP.NET Core infrastructure. This enables easier instantiation of OData query options in scenarios such as deserialization from JSON, tool integration, and testing.Key changes: