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

Attribute routing controllers/endpoints end up in Non-OData Endpoint Mappings #805

Closed
serrnovik opened this issue Jan 12, 2023 · 12 comments
Closed
Labels
bug Something isn't working

Comments

@serrnovik
Copy link

Using

  • latest ASP.NET Core OData 8.0.12 (same issue with at least several lower minor versions),
  • net6.0
  • Asp NetCore Version 6.0.9

I'm unable to make Attribute routes work properly. Having problem with my own controller I've tried following to the letter example in (newly updated) documentation:

    public class Customer
    {
        public int Id { get; set; }
        public string Name { get; set; }
    }

    [Route("odata")]
    public class CustomersController : ODataController
    {
        [EnableQuery]
        [HttpGet("Customers")]
        public ActionResult List()
        {
            return Ok(new List<Customer>
            {
                new Customer { Id = 1, Name = "Customer 1" },
                new Customer { Id = 2, Name = "Customer 2" }
            });
        }

        [HttpGet("Customers({key})")]
        public ActionResult Details([FromRoute] int key)
        {
            return Ok(new Customer { Id = key, Name = $"Customer {key}" });
        }
    }

Endpoints are working, but without odata (no "@odata.context": "..." and most importantly "@odata.count")

Requesting: http://localhost:5001/odata/customers/

image

Untitled-1

which is coherent with the fact that routes fall under Non-OData Endpoint Mappings

Tried:

  • disabling all convention model based endpoints
  • changing options.EnableAttributeRouting true/false
  • using full path for each method instead of controller Route attribute
  • simplified Startup configuration to the minimum
 services.AddControllers()
                .AddOData(
                    options =>
                    {
                        options.EnableAttributeRouting = true;
                    });
...

app.UseODataRouteDebug();
app.UseRouting();
app.UseEndpoints(endpoints =>
{
    endpoints.MapControllers();
});

Convention based routes work just fine.

Documentation clearly says that is it not expected behavior.

What could be the issue?

@serrnovik serrnovik added the bug Something isn't working label Jan 12, 2023
@julealgon
Copy link
Contributor

I'm not seeing where you are declaring the "odata" prefix in the OData setup. Also, are you trying to use the EDM-less flow?

@serrnovik
Copy link
Author

As in example it is set like this:

[Route("odata")]
public class CustomersController : ODataController```

however setting full path with HttpGet attribute gives the same result.

[HttpGet("odata/Customers")]

What I'm trying to do handle a bit more complex scenario, like optionally passing some extra params and/or filters.
Similar example was shown, here https://devblogs.microsoft.com/odata/routing-in-asp-net-core-8-0-preview/#attribute-route-template to get Address for a Customer:

[ODataRoute("Customers({id})/Address", "odata")]
public IHttpActionResult GetAddress(int id) {}

ODataRoute has been substituted since then, but example is still valid

@julealgon
Copy link
Contributor

As in example it is set like this:

[Route("odata")]
public class CustomersController : ODataController```

however setting full path with HttpGet attribute gives the same result.

[HttpGet("odata/Customers")]

I think you misunderstood my comment. I understand you are adding the "odata" via the attribute routing in the controller, but you are not registering this prefix when registering OData in the startup at all.

If you don't pass anything to OData, it will assume that your endpoints all start at the root of the server and not at ~/odata.

You should call AddRouteComponents in your OData setup and pass the "odata" prefix so that it matches your prefix in attribute routing.

What I'm trying to do handle a bit more complex scenario, like optionally passing some extra params and/or filters. Similar example was shown, here https://devblogs.microsoft.com/odata/routing-in-asp-net-core-8-0-preview/#attribute-route-template to get Address for a Customer:

[ODataRoute("Customers({id})/Address", "odata")]
public IHttpActionResult GetAddress(int id) {}

ODataRoute has been substituted since then, but example is still valid

I'm not seeing extra parameters or filters in that code though. Can you elaborate?

@xuzhg
Copy link
Member

xuzhg commented Jan 12, 2023

@serrnovik As @julealgon mentioned, you'd set up the route component using the prefix and model.

services.AddControllers()
.AddOData(
options =>
{
options.AddRouteComponents("odata", model)
});

@serrnovik
Copy link
Author

@xuzhg @julealgon thank you for your answers,

If you don't pass anything to OData, it will assume that your endpoints all start at the root of the server and not at ~/odata.

You should call AddRouteComponents in your OData setup and pass the "odata" prefix so that it matches your prefix in attribute routing.

When I used conventionional routes, I've in fact used

options.AddRouteComponents(routePrefix: "odata/MyEntity",   model: myEntityDataModel);

Here the usage is clear. I confirm, in fact in simple case if I add model routes are getting mapped.

So, in the example bellow:

public class Customer
    {
        public int Id { get; set; }
        public string Name { get; set; }
    }

    
    [Route("odata")]
    public class CustomersController : ODataController
    {
        [EnableQuery]
        [HttpGet("Customers")]
        public ActionResult List()
        {
            return Ok(new List<Customer>
            {
                new Customer { Id = 1, Name = "Customer 1" },
                new Customer { Id = 2, Name = "Customer 2" }
            });
        }

        [HttpGet("Customers({key})")]
        public ActionResult Details([FromRoute] int key)
        {
            return Ok(new Customer { Id = key, Name = $"Customer {key}" });
        }


        [HttpGet("Customers({key})/Address")]
        public ActionResult Address([FromRoute] int key)
        {
            return Ok("Sample street 1");
        }


        [HttpGet("BestCustomers")]
        public ActionResult ListBest()
        {
            return Ok(new List<Customer>
            {
                new Customer { Id = 2, Name = "Customer 2" }
            });
        }
    }

With simple model

    public static class CustomersModel
    {
        public static IEdmModel GetEdmModel()
        {
            var builder = new ODataConventionModelBuilder();
            builder.EntitySet<Customer>("Customers");
            var model = builder.GetEdmModel();
            return model;
        }
    }

and

services.AddControllers()
                .AddOData(
                    options =>
                    {
                        options.Count().Filter().Select().OrderBy().SetMaxTop(500);
                        options.AddRouteComponents(routePrefix: "odata",
                            model: custormesDataModel);
                        options.EnableAttributeRouting = true;
                    });

We have two routes build (as non conventional)
odata
and we in fact have
image
and
image

In my application I would have controller that requests data from server with it's own business logic and I would build a model like c#

    var builder = new ODataConventionModelBuilder();
    builder.EnableLowerCamelCase();
    return builder.GetEdmModel();

However the EntitySet name of the model must match the route prefix...

The question

So maybe in fact, the question was not exact enough, from the blog and documentation I had an idea that I can, using custom routes and maybe some extra steps have endpoints like

http://localhost:5001/odata/BestCustomers
http://localhost:5001/odata/Customers({key})/Address)

and be able to feed them to odata aware client.

Can this be done? Right now I've created separate controller for the case like BestCustomers in example above, but this doesn't look like elegant solution.

And for the example with Adress it is even less clean to create separate model/controller

@julealgon
Copy link
Contributor

julealgon commented Jan 12, 2023

So maybe in fact, the question was not exact enough, from the blog and documentation I had an idea that I can, using custom routes and maybe some extra steps have endpoints like

http://localhost:5001/odata/BestCustomers http://localhost:5001/odata/Customers({key})/Address)

and be able to feed them to odata aware client.

Can this be done? Right now I've created separate controller for the case like BestCustomers in example above, but this doesn't look like elegant solution.

And for the example with Adress it is even less clean to create separate model/controller

@serrnovik That /Address route should be fully OData-compatible as long as you add the Address field in the Customer type. The routes need to match something in the EDM definition, and since you don't have the Address property in Customer right now, it can't understand what the route means and ignores it completely. It could be either a primitive value (say a string), a complex type object, a contained entity, or even a full-blown entityset of its own.

Similarly, the BestCustomers route would cause OData to try and detect it as an EntitySet, which will not work for you as you don't have a BestCustomer entity (nor you should have, of course). For this use case, I'd probably suggest a collection-scoped custom function. You can declare the function in your EDM model and set it to return results from the Customers entityset. Once the function is defined, you should be able to call it like this:

  • /odata/Customers/BestCustomers

(or, if you are willing to rename the route):

  • /odata/Customers/Best

The code to declare such function would look similar to the following:

var customerType = builder.EntityType<Customer>();
customerType.Collection
    .Function("BestCustomers")
    .ReturnsCollectionFromEntitySet<Customer>("Customers");

EDIT:

Also, on an unrelated note, I'd strongly recommend you start leveraging the ActionResult<T> type and drop the usage of return Ok(x) in favor of just return x. The latter approach provides you with compile-time type safety of the return value and can actually influence how some of the OData operations work on the returned type.

@serrnovik
Copy link
Author

@julealgon Thank you for this detailed explanation. Actually it fully answers my question/problem, especially with the use of custom function.

Thanks for the ActionResult note as well.

@broomfn
Copy link

broomfn commented Feb 13, 2023

Also, on an unrelated note, I'd strongly recommend you start leveraging the ActionResult<T> type and drop the usage of return Ok(x) in favor of just return x. The latter approach provides you with compile-time type safety of the return value and can actually influence how some of the OData operations work on the returned type.

The MS examples of OData 8 seem to suggest using Ok(x), see https://learn.microsoft.com/en-us/odata/webapi-8/fundamentals/navigation-routing?tabs=net60%2Cvisual-studio for example?

@julealgon
Copy link
Contributor

The MS examples of OData 8 seem to suggest using Ok(x), see https://learn.microsoft.com/en-us/odata/webapi-8/fundamentals/navigation-routing?tabs=net60%2Cvisual-studio for example?

That's not good... the examples in the doc should definitely be updated IMHO.

@xuzhg FYI

@xuzhg
Copy link
Member

xuzhg commented Feb 13, 2023

@gathogojr

@gathogojr
Copy link
Contributor

@serrnovik @broomfn @julealgon For the benefit of other users, the return type of ActionResult is also valid syntax. That said, your comment about type-safety is noted and appreciated. We're happy to adopt that if the users relate with that better.

@julealgon
Copy link
Contributor

@serrnovik @broomfn @julealgon For the benefit of other users, the return type of ActionResult is also valid syntax. That said, your comment about type-safety is noted and appreciated. We're happy to adopt that if the users relate with that better.

Not all flows will work correctly with IActionResult/ActionResult @gathogojr .

Take this problem for example:

ActionResult<T> passes the type of T along into the underlying ObjectResult, which the EnableQuery attribute then leverages.

This is the key aspect that makes ActionResult<T> work:

Using just ActionResult with Ok in these scenarios would just not work: the payload would not be idendified as OData.

It is valid syntax for sure, but since it can lead to these hard to debug problems, I'd recommend discouraging it on new code in the documentation, or find a way to change how OData works to support both.

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

No branches or pull requests

5 participants