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

OData API Explorer Should Support OData Query Options #400

Closed
4 tasks done
commonsensesoftware opened this issue Nov 12, 2018 · 6 comments
Closed
4 tasks done

OData API Explorer Should Support OData Query Options #400

commonsensesoftware opened this issue Nov 12, 2018 · 6 comments
Assignees
Milestone

Comments

@commonsensesoftware
Copy link
Collaborator

commonsensesoftware commented Nov 12, 2018

The API Explorer for OData does not currently support the Query Options, Query Settings, or Model Bound configurations. These are important pieces of API documentation that should be provided by the API Explorer so that they can be leveraged by other tools such as Swagger generators.

Features

  • Support exploring Query Options
  • Support exploring Query Settings
  • Support exploring Model Bindings
  • Support providing information by conventions, attributes, or imperatively
@LuukN2
Copy link

LuukN2 commented Nov 20, 2018

This feature would be awesome since there'd be no need for operation processors anymore :)

In the project I use we do not use the ODataQueryOptions attribute as a parameter on our controllers.
We use the EnableQuery attribute at a class or method level and a return type of Queryable or Singleresult.
If the return type is a single entity only the expand and select options are added to the Swagger document since these are the only ones supported for a single entity.
Then we filter the allowed operations using the AllowedQueriesProperty on the attribute.

Having a range attribute added to the $skip and $top options would be nice, you can get the maximum values for these from the enable query attribute with the MaxTop and MaxSkip properties.

@commonsensesoftware
Copy link
Collaborator Author

That's definitely one of the targeted scenarios. Personally, I've moved away from all the attribute stuff, in particular, [EnableQuery] because you can't take advantage of the asynchronous support of the underlying data provider; for example, EF's async enumerator. This has everything to do with the non-async nature of IQueryable as opposed to OData. It would make a lot more sense if that was a natively understood concept. Now that that C# 8.0 is finally introducing IAsyncEnumerable, maybe that will be a reality, if not at least in ASP.NET Core.

That said, attributes, imperatively in code, or configured for the entire application at startup are all places where this information can be defined in part or in whole. My initial thinking is that each option would be broken out into it's own respective parameter. Most parameters are straight forward, but a few like $select and $expand are more of a challenge to document with anything useful beyond the fact they can be used. Providing descriptions and examples will be something that should be enabled. Since the OData libs no longer require it, there should be a configuration option as to whether the built-in parameters should use the $ prefix. It's probably worth considering how custom hooks can be achieved as well for things like the OData aggregation extensions (ex: $groupby).

@SiberaIndustries
Copy link

SiberaIndustries commented Nov 21, 2018

Wow it is nice to see that our solution is very similar to yours @LuukN2. Well done.
We also tried to document the correct query options by the return types, but finally our current solution looks like this:

public class ODataQueryOptionsFilter: IOperationFilter
{
    public void Apply(Operation operation, OperationFilterContext context)
    {
        var queryAttribute = context.MethodInfo.GetCustomAttributes(true)
            .Union(context.MethodInfo.DeclaringType.GetCustomAttributes(true))
            .OfType<EnableQueryAttribute>().FirstOrDefault();

        if (queryAttribute != null)
        {
            if (queryAttribute.AllowedQueryOptions.HasFlag(AllowedQueryOptions.Select))
            {
                operation.Parameters.Add(new NonBodyParameter
                {
                    Name = "$select",
                    In = "query",
                    Description = "Selects which properties to include in the response.",
                    Type = "string"
                });
            }

            if (queryAttribute.AllowedQueryOptions.HasFlag(AllowedQueryOptions.Expand))
            {
                operation.Parameters.Add(new NonBodyParameter
                {
                    Name = "$expand",
                    In = "query",
                    Description = "Expands related entities inline.",
                    Type = "string"
                });
            }

            // Additional OData query options are available for collections of entities only
            if (context.MethodInfo.ReturnType.IsArray ||
                typeof(IQueryable).IsAssignableFrom(context.MethodInfo.ReturnType) ||
                typeof(IEnumerable).IsAssignableFrom(context.MethodInfo.ReturnType))
            {
                if (queryAttribute.AllowedQueryOptions.HasFlag(AllowedQueryOptions.Filter))
                {
                    operation.Parameters.Add(new NonBodyParameter
                    {
                        Name = "$filter",
                        In = "query",
                        Description = "Filters the results, based on a Boolean condition.",
                        Type = "string"
                    });
                }

                // Same for OrderBy, Top, Skip, Count
            }
        }
    }
}

Of course this is not a good solution at all, but it is currently working for the most of our cases. OData is complex and leads to many challanges in combination of an IOperationFilter.

Allowed Query Options

In my opinion the most difficult challange is the extraction of AllowedQueryOptions by the response types of action methods, because it does not work for different types such as IActionResult or even the fact that the response of a POST requests must have the possibility to be expandable and selectable too as described in the OData specs in 5.1 OData Spec:

For GET, PATCH, and PUT requests the following rules apply:

  • Resource paths identifying a single entity, a complex type instance, a collection of entities, or a collection of complex type instances allow $compute, $expand and $select.
  • Resource paths identifying a collection allow $filter, $search, $count, $orderby, $skip, and $top.
  • Resource paths ending in /$count allow $filter and $search.
  • Resource paths not ending in /$count or /$batch allow $format.

POST requests to an entity set follow the same rules as GET requests that return a single entity.

System query options SHOULD NOT be applied to a DELETE request.

Navigational Properties

Nested navigational properties presents another challange. Check out the following OData request:

http://localhost:5000/api/Persons(1337)?$expand=Details($expand=Address))

So you want the information of the Person with the identifier 1337 including the Address which is hold by a Details entity, used in the Person entity. Weird, but possible and valid. In the future and of the usability point of view, it would be nice to see an expandable mechanism, but this requires a collection of reflected navigation types which then can be used in a generator.

The same applies to the provided information of navigational properties in general, because OData provides the information of navigational properties in the $metadata docs only. An issue is also tracked in the Swashbuckle repo e.g.

So to put all in a nutshell I just tried to name some challanges of what kind of information must be provide to a Swagger generator and as @commonsensesoftware said in #397 I also want to share my current implementation with you as a temporary suggestion and maybe I bet the community have some improvements :)

@commonsensesoftware commonsensesoftware changed the title OData API Explorer Should Support ODataQueryOptions and ODataQuerySettings OData API Explorer Should Support OData Query Options Dec 5, 2018
@commonsensesoftware
Copy link
Collaborator Author

I've managed to start some work on this. The high-level thinking is to use an approach which is similar to the existing conventions applied to API versions. This will allow supporting settings applied by decorating code with attributes as well as scenarios where attributes are not or cannot be used.

I don't have anything ready to push and review just yet, but this is what it's shaping up to look like:

using static Microsoft.AspNet.OData.Query.AllowedLogicalOperators;
using static Microsoft.AspNet.OData.Query.AllowedQueryOptions;

AddODataApiExplorer( options =>
{
    options.QueryOptions.NoDollarPrefix = true;
    options.QueryOptions
           .Controller<ValuesController>()
           .Action( c => c.Get() ).AllowTop( max: 100 ).Allow( Or | And )
           .Action( c => c.Get(default) ).Allow( Select | Expand );
})

The current implementation behind the scenes is based on documenting the query options using a constructed ODataValidationSettings instance; however, I've tried carefully to not expose it or make it the required mechanism for which documenting query options is based off of. This feels like it makes sense since it's already the reciprocal of defining and validating query options.

Configuring query options can be verbose so I tried to make it as succinct as possible. Here's the full list of current convention methods:

  • Allow( AllowedArithmeticOperators arithmeticOperators )
  • Allow( AllowedFunctions functions )
  • Allow( AllowedLogicalOperators logicalOperators )
  • Allow( AllowedQueryOptions queryOptions )
  • AllowSkip( int max )
  • AllowTop( int max )
  • AllowExpand( int maxDepth )
  • AllowAnyAll( int maxExpressionDepth )
  • AllowFilter( int maxNodeCount )
  • AllowOrderBy( int maxNodeCount, IEnumerable<string> properties )
  • Use( ODataValidationSettings validationSettings )

The AllowXXX methods allow a query option with additional, option-specific, settings.

NOTE: The Use method is an implementation detail and not a requirement of the conventions API.

At this point, I don't really see a need to have controller-level conventions. Although OData allows this, I can't think of a single time where this ever made sense in practice. In other words, no two controller actions have the exact same set of query options or even overlapping query options. The only scenario I could think of whether that might be true is on controller with a bunch of unbounded functions. Arguably, if you're doing that, you're doing it all wrong. ;) Do you guys have any thoughts? Obviously attributes will still need to be supported, but supporting by these conventions adds quite a bit of complexity that I don't think has a high value proposition or may not even be used.

The last part of the design that I'm struggling to resolve is how to provide flexibility in allowing developers to customize what the parameter description will be. Providing a default description is pretty straight forward, but customization may be needed because:

  • A developer doesn't like the description
  • A developer needs to expand or modify the description
  • The description needs to be localized

I thought about having some type of callback or other configuration method that would provide this hook, but thus far, I have not been able to come up with a way to do that without directly exposing ODataValidationSettings as part of the formal API. This information must be provided somehow in order to generate an appropriate message. For example, describing $orderby also requires the list of sortable properties. The only other approach I've thought of is having a per query option formatter API that defines parameters relevant to each query option. The values can be supplied from ODataValidationSettings without depending on it. Something like string DescribeOrderBy(IEnumerable<string> properties). A simpler approach might be something like:

interface IODataQueryOptionDescriptionProvider
{
    string Describe(AllowedQueryOptions option, DescriptionContext context)
}

Where the DescriptionContext is a subset of ODataValidationSettings information, which is likely even copied from an ODataValidationSettings instance.

Other alternatives include defining an interface to be used with an adapter over ODataValidationSettings, but I'm not sure how much value that really brings. I'm open to any suggestions you guys might have.

@commonsensesoftware
Copy link
Collaborator Author

Overview

The preview of the feature will have two models: attribute-based and convention-based. The order of precedence will be:

  • By Convention
  • Global Configuration
  • Enable Query Attribute
    • By Type
    • By Method
  • Model Bound Configuration

The following query options are not supported:

  • $format
  • $skiptoken
  • $deltatoken
  • $apply

These are infrequently used or do not have corresponding information that can be queried (that I can find). In addition, only the top-level structured type will be processed. While not impossible, processing the query options on related child structured types is not only complex, but could create potentially very verbose messages. Furthermore, child structured types cannot express independent query options as query parameters.

Query options will get their description from an IODataQueryOptionDescriptionProvider, which will provide the description for a given query option and a context which contains information about the allowed properties, functions, operators, and so on.

Feedback

This is iteration one, so I'm definitely looking for some feedback on how this is shaping up. The DefaultODataQueryOptionDescriptionProvider, in particular, will determine what the default query option descriptions will be. Currently, the text is largely based on the text defined in the OData specification. To keep the descriptions from being verbose, especially $filter, collections such as structured type properties are not listed if all or none are allowed. The same is true for functions, operators, and so on. I'm open to opinions as to whether the text should instead use something like "No functions are allowed" or "All functions are allowed", but consider this can make some query options like $filter, $select, $expand, and $orderby quite long.

Attribute Model

This relies on Model Bound attributes and the EnableQueryAttribute. The EnableQueryAttribute indicates options that cannot otherwise be set such as MaxTop, MaxSkip, allowed functions, and so on.

[Select]
[Select( "effectiveDate", SelectType = SelectExpandType.Disabled )]
public class Order
{
    public int Id { get; set; }
    public DateTime CreatedDate { get; set; } = DateTime.Now;
    public DateTime EffectiveDate { get; set; } = DateTime.Now;
    public string Customer { get; set; }
    public string Description { get; set; }
}

[ApiVersion( "3.0" )]
[ODataRoutePrefix( "Orders" )]
public class OrdersController : ODataController
{
    [ODataRoute]
    [Produces( "application/json" )]
    [ProducesResponseType( typeof( ODataValue<IEnumerable<Order>> ), Status200OK )]
    [EnableQuery( MaxTop = 100, AllowedQueryOptions = Select | Top | Skip | Count )]
    public IQueryable<Order> Get()
    {
        var orders = new[]
        {
          new Order(){ Id = 1, Customer = "John Doe" },
          new Order(){ Id = 2, Customer = "John Doe" },
          new Order(){ Id = 3, Customer = "Jane Doe", EffectiveDate = DateTime.UtcNow.AddDays(7d) }
        };

        return orders.AsQueryable();
    }

    [ODataRoute( "({key})" )]
    [Produces( "application/json" )]
    [ProducesResponseType( typeof( Order ), Status200OK )]
    [ProducesResponseType( Status404NotFound )]
    [EnableQuery( AllowedQueryOptions = Select )]
    public SingleResult<Order> Get( int key ) =>
      SingleResult.Create( new[] { new Order(){ Id = key, Customer = "John Doe" }}.AsQueryable() );
}

attribute-model

Figure 1: Attribute-based example

Convention Model

This model relies on Model Bound conventions and the new Query Option conventions. The Query Option configures options that cannot otherwise be set such as MaxTop, MaxSkip, allowed functions, and so on.

public class Person
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Email { get; set; }
    public string Phone { get; set; }
}

public class PeopleController : ODataController
{
    [Produces( "application/json" )]
    [ProducesResponseType( typeof( ODataValue<IEnumerable<Person>> ), Status200OK )]
    public IActionResult Get( ODataQueryOptions<Person> options )
    {
        var validationSettings = new ODataValidationSettings()
        {
            AllowedQueryOptions = Select | OrderBy | Top | Skip | Count,
            AllowedOrderByProperties = { "firstName", "lastName" },
            AllowedArithmeticOperators = AllowedArithmeticOperators.None,
            AllowedFunctions = AllowedFunctions.None,
            AllowedLogicalOperators = AllowedLogicalOperators.None,
            MaxOrderByNodeCount = 2,
            MaxTop = 100,
        };

        try
        {
            options.Validate( validationSettings );
        }
        catch ( ODataException )
        {
            return BadRequest();
        }

        var people = new[]
        {
            new Person()
            {
                Id = 1,
                FirstName = "John",
                LastName = "Doe",
                Email = "john.doe@somewhere.com",
                Phone = "555-987-1234",
            },
            new Person()
            {
                Id = 2,
                FirstName = "Bob",
                LastName = "Smith",
                Email = "bob.smith@somewhere.com",
                Phone = "555-654-4321",
            },
            new Person()
            {
                Id = 3,
                FirstName = "Jane",
                LastName = "Doe",
                Email = "jane.doe@somewhere.com",
                Phone = "555-789-3456",
            }
        };

        return Ok( options.ApplyTo( people.AsQueryable() ) );
    }

    [Produces( "application/json" )]
    [ProducesResponseType( typeof( Person ), Status200OK )]
    [ProducesResponseType( Status404NotFound )]
    public IActionResult Get( int key, ODataQueryOptions<Person> options )
    {
        var people = new[]
        {
            new Person()
            {
                Id = key,
                FirstName = "John",
                LastName = "Doe",
                Email = "john.doe@somewhere.com",
                Phone = "555-987-1234",
            }
        };

        var person = options.ApplyTo( people.AsQueryable() ).SingleOrDefault();

        if ( person == null )
        {
            return NotFound();
        }

        return Ok( person );
    }
}

public class PersonModelConfiguration : IModelConfiguration
{
    public void Apply( ODataModelBuilder builder, ApiVersion apiVersion )
    {
        var person = builder.EntitySet<Person>( "People" ).EntityType;

        person.HasKey( p => p.Id );
        person.Select().OrderBy( "firstName", "lastName" );

        if ( apiVersion < ApiVersions.V3 )
        {
            person.Ignore( p => p.Phone );
        }

        if ( apiVersion <= ApiVersions.V1 )
        {
            person.Ignore( p => p.Email );
        }

        if ( apiVersion > ApiVersions.V1 )
        {
            var function = person.Collection.Function( "NewHires" );

            function.Parameter<DateTime>( "Since" );
            function.ReturnsFromEntitySet<Person>( "People" );
        }

        if ( apiVersion > ApiVersions.V2 )
        {
            person.Action( "Promote" ).Parameter<string>( "title" );
        }
    }
}

public void ConfigureServices( IServiceCollection services )
{
    services.AddODataApiExplorer(
        options =>
        {
            // configure query options (which cannot otherwise be configured by OData conventions)
            options.QueryOptions.Controller<V2.PeopleController>()
                                .Action( c => c.Get( default( ODataQueryOptions<Person> ) ) )
                                    .Allow( Skip | Count ).AllowTop( 100 );

            options.QueryOptions.Controller<V3.PeopleController>()
                                .Action( c => c.Get( default( ODataQueryOptions<Person> ) ) )
                                    .Allow( Skip | Count ).AllowTop( 100 );
        } );
}

convention-model

Figure 2: Convention-based example

@commonsensesoftware commonsensesoftware added this to the 3.0 milestone Dec 13, 2018
@commonsensesoftware
Copy link
Collaborator Author

As it seems there isn't any additional feedback, I'm imminently about to complete the associated PR which will close this issue out. This is the final feature for 3.0 and the bug count has burned down. I plan releasing 3.0 ASAP; possibly later today. Any additional changes will either come in a patch or 3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants