OData / WebApi

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

Bug, ODataUriParser.ParseUri fail when using $expand($orderby) together #1744

Open keatkeat87 opened 5 years ago

keatkeat87 commented 5 years ago

Short summary (3-5 sentences) describing the issue. i m trying to use ODataUriParser to parser url in controller. actually my main purpose is want to use this function in CustomEnableQuery to modify the request query. my idea is generate a limit url and match to the request url. so i need to use ODataUriParser to parse url. it work fine at all condition, but get error in $expand=colors($orderby=Id desc) case. error is "Value cannot be null.Parameter name: type". i m not sure this is odata/webapi bug or odata/lib. i make a simple environment to reproduce the error. please clone it from github. thank a lot. if you need more information, just let me know.

Assemblies affected

*Which assemblies and versions are known to be affected e.g. OData WebApi lib 6.1.0 Microsoft.AspNetCore.OData 7.1.0 Microsoft.OData.Core 7.5.3 asp.net core 2.2.1

Reproduce steps

The simplest set of steps to reproduce the issue. If possible, reference a commit that demonstrates the issue. git clone https://github.com/keatkeat87/aspnet-core-odata-parser-expand-issue F5 run visit: http://localhost:49295/odata/Products/Rpc.GetProducts?$expand=colors($orderby = Id desc) note : the port is dynamic

Expected result

parser without error

Additional detail

Please look at the ProductsController.cs code.

EdmModel : `public static IEdmModel GetEdmModel() { ODataConventionModelBuilder builder = new ODataConventionModelBuilder(); builder.EntitySet("Products"); var GetProducts = builder.EntityType().Collection.Function("GetProducts"); GetProducts.ReturnsCollectionFromEntitySet("Products"); GetProducts.Namespace = "Rpc";

builder.EntitySet<Color>("Colors");
return builder.GetEdmModel();

}`

Model Class: public class Product { public int Id { get; set; } public string code { get; set; } public List<Color> colors { get; set; } } public class Color { public int Id { get; set; } public string name { get; set; } public Product product { get; set; } }

Controller : `public class ProductsController : ODataController { [HttpGet] [EnableQuery] public IActionResult GetProducts() { var domain = $"{ Request.Scheme }://{ Request.Host.Value }"; Uri domainAndPrefixApi = new Uri(domain + "/odata"); //var requestUri = new Uri($"{domain}/odata/Products/Rpc.GetProducts?$expand=colors"); // ok var requestUri = new Uri($"{domain}/odata/Products/Rpc.GetProducts?$expand=colors($orderby=Id desc)"); // fail //var requestUri = new Uri($"{domain}/odata/Products/Rpc.GetProducts?$expand=colors($filter=Id eq 5)"); // fail var model = HttpContext.Request.GetModel(); try { var parser = new ODataUriParser(Request.GetModel(), domainAndPrefixApi, requestUri) { Resolver = new ODataUriResolver { EnableCaseInsensitive = true } }; var request = parser.ParseUri(); } catch (Exception ex) { // Value cannot be null.Parameter name: type throw ex; }

    return Ok(new List<Product> {
        new Product { Id = 1, code = "mk100", colors = new List<Color> { new Color { Id = 1, name = "red" }, new Color { Id = 2, name = "yellow" } } },
        new Product { Id = 2, code = "mk200", colors = new List<Color> { new Color { Id = 1, name = "red" }, new Color { Id = 2, name = "yellow" } } },
        new Product { Id = 3, code = "mk200", colors = new List<Color> { new Color { Id = 1, name = "red" }, new Color { Id = 2, name = "yellow" } } },
        new Product { Id = 4, code = "mk400", colors = new List<Color> { new Color { Id = 1, name = "red" }, new Color { Id = 2, name = "yellow" } } }
    }.AsQueryable());
}

}`

Optional, details of the root cause if known. Delete this section if you have no additional details to add.

KanishManuja-MS commented 5 years ago

@keatkeat87 I am unable to run your project for some reason. Anyway, have you considered relying on conventional routing instead of creating your own function for GetProducts? Why do you need to parse the URI? Can you share the metadata that gets shown when you run the project?

Seems like the parser at some point is not able to figure out the type parameter perhaps because of the model is passed in. Have you tried passing in the static model directly from the config?

keatkeat87 commented 5 years ago

Hi KanishManuja-MS, Thank for reply.

have you considered relying on conventional routing instead of creating your own function for GetProducts?

in my application, have member and admin role need to use /api/products to get data. but different role have limited data to see. for example, member can only get published product and admin can get all products. so i create two function in controller instead of /api/products api/products/Rpc.getAllProducts api/products/Rpc.getPublishedProducts

Why do you need to parse the URI?

in some case, i need to make sure the request query is under my control. for example, member can access /api/products($expand=models) but not /api/products($expand=models, supplier) so i intercept EnableQuery, and parse request url, if i found some expand property is not allow, then i will modify the query. /api/products($expand=models, supplier) become /api/products($expand=models)

Can you share the metadata that gets shown when you run the project?

https://github.com/keatkeat87/aspnet-core-odata-parser-expand-issue/blob/master/metadata please visit this link.

Have you tried passing in the static model directly from the config?

I m sorry, i not familiar with OData. do you mean regenerate model instead of using HttpContext.Request.GetModel() ?

keatkeat87 commented 5 years ago

for your information if use conventional routing instead of creating Function GetProducts, it work fine.