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

Feature ask: Need the have ability to set custom query validator into SelectExpandQueryValidator #767

Open
SamGuoMsft opened this issue Dec 13, 2022 · 0 comments · Fixed by #795
Assignees
Labels
bug Something isn't working

Comments

@SamGuoMsft
Copy link

Assemblies affected
Which assemblies and versions are known to be affected e.g.: ASP.NET Core OData 8.x

Describe the bug
@xuzhg, based on our discussion, I am just listing three issues which I hope we can improve:

  1. Can't set custom query validator into SelectExpandQueryValidator, e.g.: The construct directly create the FilterQueryValidator, rather than expose as parameter, so customer can use DI or explicitly set it. Source code link.
  2. Even SelectExpandQueryValidator allow us pass custom FilterQueryValidator via constructor, it will still not work due to the current code does some kind of hack) to create an internal virtual method to allow SelectExpandQueryValidator to set local properties like defaultValidationSetting. So even I create my custom filter query validator, I still can't set these fields and it will cause null reference issue eventually.
  3. In the current FilterQueryValidator, if customer doesn't register EDM model, the validation will not honor the model bound filter attribute due to we don't set property and structured type if no ODataPath, source code is [here](https://github.com/OData/AspNetCoreOData/blob/3db5e5fb94df2968b1b354bc37a47907b89892be/src/Microsoft.AspNetCore.OData/Query/Validator/FilterQueryValidator.cs#:~:text=if%20(filterQueryOption.,%7D), but if people use filter under expand clause, then the model bound filter attribute start working due to it invoke the hacky internal method which I mentioned in 2nd issue, and the internal method will set the property and structured type based on navigation property. So, it is not consistent.

Reproduce steps
The simplest set of steps to reproduce the issue. If possible, reference a commit that demonstrates the issue.

Data Model
Please share your Data model, for example, your C# class.

EDM (CSDL) Model
Please share your Edm model, for example, CSDL file.
You can send $metadata to get a CSDL XML content.

Request/Response
Please share your request Uri, head or the request body
Please share your response head, body.

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Please share your call stack or any error message
Add any other context about the problem here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants