OData / WebApi

OData Web API: A server library built upon ODataLib and WebApi
https://docs.microsoft.com/odata
Other
853 stars 476 forks source link

Missing documentation for FilterQueryValidator #1651

Open mloffer opened 5 years ago

mloffer commented 5 years ago

WebAPI 2.2 documentation lists very little: https://docs.microsoft.com/en-us/aspnet/core/web-api/?view=aspnetcore-2.2

OData documentation is out of date: https://docs.microsoft.com/en-us/aspnet/web-api/overview/odata-support-in-aspnet-web-api/odata-security-guidance A) FilterQueryValidator requires DefaultQuerySettings to be passed to the base. Where should this come from? B) This article doesn't show how to use MyFilterQueryValidtor.

I have looked through the samples, but there are no relevant projects: https://github.com/OData/ODataSamples/tree/master/WebApiCore

Questions: How do I create one? How should DefaultQuerySettings be passed in? How do I use it to validate against a controller action?

Sorry to be so basic, but the documentation for this stack is nearly non-existent. It's hard to even know what to ask.

KanishManuja-MS commented 5 years ago

@mloffer Thanks for the feedback. We are working on making the documentation better. For the time being, for aspnetcore, you can use this in your startup.cs to set the DefaultQuerySetting with the parameters you want - app.UseMvc(b => { b.SetDefaultQuerySettings(new Microsoft.AspNet.OData.Query.DefaultQuerySettings()); });

If you are using aspnet, then you can use HttpConfiguration extension methods to do the same.

mloffer commented 5 years ago

I don't understand what explicitly setting DefaultQuerySettings does here? Isn't this done automatically behind the scenes? What does this have to do with writing a FilterQueryValidator?

KanishManuja-MS commented 5 years ago

@mloffer DefaultQuerySetting in the context of FilterQueryValidator simply checks if filter is enabled as a query option or not. It was not clear to me how are you planning on using the FilterQueryValidator; if you want to use your own then simply new a DefaultQuerySetting object with Enable filter property set to true.

I believe that the framework should take care of this automatically but it seems like you are trying to achieve something else here. Can you please give me more context about what you are trying to do?

mloffer commented 5 years ago

This is what I was looking to write. It's fairly similar to the way the AspNet guide says to do it. I've provided a new instance of DefaultQuerySetting like you suggested, but I am unclear on what the implications of this are. Am I at risk of having this attribute ignore global query settings I'm configuring elsewhere in the stack?

Also, is EnableQueryAttribute the right thing to use here? It's just QueryableAttribute in the AspNet guide, but this class doesn't exist in the AspNetCore implementation.

You can see that I would like to throw an error if the ODataQueryOptions.Filter is null, but the ValidateQuery method never gets called if some part of the query isn't provided by the consumer. For instance, I can provide $count=true and I get the error. But simply accessing the route does not enforce this behavior.

namespace AspNetCoreODataFilterQueryValidator
{
    public class FilterTitleAttribute: EnableQueryAttribute
    {
        public override void ValidateQuery(HttpRequest request, ODataQueryOptions queryOptions)
        {
            if(queryOptions.Filter == null)
            {
                throw new ODataException("You must filter on Title to use this route.");
            }

            var defaultQuerySettings = new DefaultQuerySettings { EnableFilter = true };
            queryOptions.Filter.Validator = new MyFilterQueryValidator(defaultQuerySettings);

            base.ValidateQuery(request, queryOptions);
        }
    }

    public class MyFilterQueryValidator : FilterQueryValidator
    {
        public MyFilterQueryValidator(DefaultQuerySettings defaultQuerySettings) : base(defaultQuerySettings) { }

        public override void Validate(FilterQueryOption filterQueryOption, ODataValidationSettings settings)
        {
            var count = filterQueryOption.CountMatches("Title");
            if(count != 1)
            {
                throw new ODataException("You must filter exactly one Title to use this route.");
            }
            base.Validate(filterQueryOption, settings);
        }
    }
}
KanishManuja-MS commented 5 years ago

@mloffer Thanks for the details. I believe I now understand what you are trying to do. ODataQueryOption.Filter.Validate only gets fired when there is a $filter query option in the request. By design, FilterQueryValidator does not support checking for absence of the filter rather it checks if the format is correct. With that said, there are a number of ways in which you can accomplish what you want -

  1. Use dependency injection - override ODataQueryValidator.Validate and specify your implementation in configure method of startup.cs.
  2. Throw an exception if queryOptions.Filter is null in the controller method itself. If it is not null, then your validation code will fire.

Let me know if you need more help, I can pull in other experts as well.

Thanks!

mloffer commented 5 years ago

That makes sense that ODataQueryOption.Filter.Validate wouldn't fire unless a filter were provided.

My question is why EnableQueryAttribute.ValidateQuery doesn't fire unless some query option is passed, such as $count? After all, it's not called EnableQueryOptionsAttribute. Is there a better attribute to inherit from for this task?

I mean, sure, I could put the null check in every controller action. But that violates DRY principles and makes the code very hard to refactor if that requirement ever changes.

ashygk commented 3 years ago

Hi,

I am having issues with invoking the ValidateSingleValuePropertyAccessNode in my FilterQueryValidator. I would like to set specified properties to be used in the filter query and throw validation error if any other properties are specified in the filter query. Anyone please assist.