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

$apply is not showing meta data info when I use IActionResult #181

Open
komdil opened this issue May 24, 2021 · 21 comments
Open

$apply is not showing meta data info when I use IActionResult #181

komdil opened this issue May 24, 2021 · 21 comments

Comments

@komdil
Copy link

komdil commented May 24, 2021

I have Get action in controller:

        public IActionResult Get(ODataQueryOptions<TEntity> queryOptions, CancellationToken cancellationToken)
        {
            var query = MyDBContext.Set<Student>().ToList();
            return Ok(query);
        }

When I send this query: Student?$apply=aggregate($count as OrderCount)
It is returning value without meta data.

Actual:

[
    {
        "OrderCount": 3
    }
]

Expected:

{
    "@odata.context": "https://localhost:44383/$metadata#Student(OrderCount)",
    "value": [
        {
            "@odata.id": null,
            "OrderCount": 3
        }
    ]
}

It is working fine when Get looks like:

        public IEnumerable<TEntity> Get(ODataQueryOptions queryOptions, CancellationToken cancellationToken)
        {
            return MyDBContext.Set<TEntity>().ToList();
        }

It is causing problems when entity is Open Type. In open type it looks like:

{
    "$type": "System.Linq.EnumerableQuery`1[[Microsoft.AspNet.OData.Query.Expressions.NoGroupByAggregationWrapper, Microsoft.AspNetCore.OData]], System.Linq.Queryable",
    "$values": [
        {
            "$id": "1",
            "$type": "System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.Object, System.Private.CoreLib]], System.Private.CoreLib",
            "OrderCount": 3
        }
    ]
}

My project is in .NET 5

@ekomut
Copy link

ekomut commented Jun 10, 2021

And in this first case, " $count=true " has no effect.

@JanKotschenreuther
Copy link

JanKotschenreuther commented Jun 11, 2021

In use:

  • .NET 5
  • Microsoft.AspNetCore.OData 8.0.0-rc3
  • Microsoft.EntityFrameworkCore.SqlServer 5.0.4
  • Microsoft.EntityFrameworkCore.Tools 5.0.4
  • Microsoft.EntityFrameworkCore.Proxies 5.0.4 (currently required for Lazy Loading when using $expand)

I am pretty happy to be able to use §apply and groupby now, like for example:

  • {{baseUrl}}/odata/Article?$apply=groupby((Brand/Id))&$top=10&$skip=30&$orderby=Brand/Id&$count=true
  • {{baseUrl}}/odata/Article?$orderby=Brand/Name&$apply=filter(contains(Brand/Name,'de'))/groupby((Brand/Name))&$count=true

Filter, groupBy, orderBy and paging with top and skip actually work.

But the result of those queries do not conform to the usual OData-Results and therefor do not contain the count and nextLink for example.
One of those Properties is required to be able to page through the results without ending up on pages that return no results.
The count is also very important to indicate the number of datasets available for a certain query.

A usual OData-Result is returned by queries without $apply:

Request-Url: "{{baseUrl}}/odata/Article?$select=Id&$expand=Brand($select=Name)&$top=3&$count=true"
Response:
{
    "@odata.context": "https://{{baseUrl}}/odata/$metadata#Article(Id,Brand(Name))",
    "@odata.count": 13023,
    "value": [
        {
            "Id": 1,
            "Brand": {
                "Name": "Example1"
            }
        },
        {
            "Id": 2,
            "Brand": {
                "Name": "Example2"
            }
        },
        {
            "Id": 3,
            "Brand": {
                "Name": "Example3"
            }
        }
    ]
}

When using $apply the wrapping OData-Result-Object is missing which deliveres @odata.count:

Request-Url: "{{baseUrl}}/odata/Article?$apply=groupby((Brand/Id))&$top=3&$skip=30&$orderby=Brand/Id&$count=true"
Response:
[
    {
        "Brand": {
            "Id": 1302
        }
    },
    {
        "Brand": {
            "Id": 1303
        }
    },
    {
        "Brand": {
            "Id": 1304
        }
    }
]

I did not notice until now, but I am also missing @odata.nextLink which has already been returned on previous versions for usual OData-Queries without $apply.

Our actions look like this:

[EnableQuery]
public IActionResult Get()
{
	var dbContext = HttpContext.RequestServices.GetService<TDbContext>();
	var dbSet = dbContext.Set<TEntity>();

	return Ok(dbSet);
}

@vonckm-kadaster
Copy link

vonckm-kadaster commented Jul 14, 2021

i'm going to follow this, i have the same problem (missing meta data info) when using IQueryable as controller action result.

public IQueryable<Student> Get(ODataQueryOptions<TEntity> queryOptions, CancellationToken cancellationToken)
      {
          var query = MyDBContext.Set<Student>().AsQueryable()
          return Ok(query);
      }

used packages:
NET Core 5
Microsoft.AspNetCore.OData: 8.0.1
Microsoft.AspNetCore.Mvc.NewtonsoftJson: 5.0.7

@komdil
Copy link
Author

komdil commented Jan 10, 2022

Is there any updates here?

@komdil
Copy link
Author

komdil commented Feb 4, 2022

Any progress?

@julealgon
Copy link
Contributor

@komdil and @vonckm-kadaster , a "payload without metadata" is a normal AspNetCore MVC payload, which means your endpoints are not being detected as actual OData endpoints.

Can you try changing the signatures and/or using attribute routing?

For example, I think the extra CancellationToken parameter might cause issues with the default conventions and you may have to go with an explicit route using [HttpGet] with a route template.

Regardless, please use the route debug middleware to make sure your routes are actually OData routes.

@cympatic
Copy link

I had the same issue and after debuging I found that the ODataOutputFormatter is returning false in the method CanWriteResult because it seems to be unable to determine the type on line 127-131

The ODataOutputFormatter.CanWriteResult returns false because the context.ObjectType is not set and the context.Object.GetType() returns an IEnumerable<GroupByWrapper> or descending class of GroupByWrapper for which no proper PayLoadKind can be determined. This occurred when an ObjectResult is used as controller action result. When IQueryable<> is used as controller action result the context.ObjectType is set in the ODataOutputFormatter.CanWriteResult and a proper PayLoadKind is determined.

As solution I created a custom EnableQueryAttribute to use instead and override the method OnActionExecuted(ActionExecutedContext actionExecutedContext). This sets the context.ObjectType when the method ODataOutputFormatter.CanWriteResult is entered and a proper PayLoadKind can be determined and result in OData-result

        public override void OnActionExecuted(ActionExecutedContext actionExecutedContext)
        {
            if (actionExecutedContext is null)
            {
                throw new ArgumentNullException(nameof(actionExecutedContext));
            }

            if (actionExecutedContext.HttpContext.Response != null &&
                IsSuccessStatusCode(actionExecutedContext.HttpContext.Response.StatusCode) &&
                actionExecutedContext.Result is ObjectResult content &&
                content.Value != null &&
                content.DeclaredType == null)
            {
                // To help the `ODataOutputFormatter` to determine the correct output class
                // the `content.DeclaredType` needs to be set before appling the `$apply`-option
                // so that a valid `OData`-result is produced
                // https://github.com/OData/AspNetCoreOData/blob/4de92f52a346606a447ec4df96c5f3cd05642f50/src/Microsoft.AspNetCore.OData/Formatter/ODataOutputFormatter.cs#L127-L131
                content.DeclaredType = content.Value.GetType();
            }

            base.OnActionExecuted(actionExecutedContext);
        }

        private static bool IsSuccessStatusCode(int statusCode)
        {
            return statusCode >= 200 && statusCode < 300;
        }

@julealgon
Copy link
Contributor

@cympatic could you share the controller code where you had to use this custom attribute on?

@cympatic
Copy link

cympatic commented Dec 31, 2022

Not directly. However, it's easily reproducible by using the E2E EntitySetAggregation tests in this repository.

The EntitySetAggregationTests.AggregationOnEntitySetWorks(method: "average", expected: 100) succeed normal where the Get -method in the EntitySetAggregationController is

        [EnableQuery]
        public IQueryable<Customer> Get()
        {
            return _context.Customers;
        }

and the result will be:
{"value":[{"Orders":[{"TotalPrice":100.0}]}]}

But changing the Get-method in the EntitySetAggregationController to

        [EnableQuery]
        public IActionResult Get()
        {
            return Ok(_context.Customers);
        }

Then the result will be
[{"Orders":[{"TotalPrice":100}]}]

Creating a custom EnableQueryAttribute like

    public class MyEnableQueryAttribute : EnableQueryAttribute
    {
        public override void OnActionExecuted(ActionExecutedContext actionExecutedContext)
        {
            if (actionExecutedContext is null)
            {
                throw new ArgumentNullException(nameof(actionExecutedContext));
            }

            if (actionExecutedContext.HttpContext.Response != null &&
                IsSuccessStatusCode(actionExecutedContext.HttpContext.Response.StatusCode) &&
                actionExecutedContext.Result is ObjectResult content &&
                content.Value != null &&
                content.DeclaredType == null)
            {
                // To help the `ODataOutputFormatter` to determine the correct output class
                // the `content.DeclaredType` needs to be set before appling the `$apply`-option
                // so that a valid `OData`-result is produced
                // https://github.com/OData/AspNetCoreOData/blob/4de92f52a346606a447ec4df96c5f3cd05642f50/src/Microsoft.AspNetCore.OData/Formatter/ODataOutputFormatter.cs#L127-L131
                content.DeclaredType = content.Value.GetType();
            }

            base.OnActionExecuted(actionExecutedContext);
        }

        private static bool IsSuccessStatusCode(int statusCode)
        {
            return statusCode >= 200 && statusCode < 300;
        }
    }

and updating the Get-method to

        [MyEnableQuery]
        public IActionResult Get()
        {
            return Ok(_context.Customers);
        }

will result in
{"value":[{"Orders":[{"TotalPrice":100.0}]}]}

And the specific test will succeed again

A commit pushed to the fork I've: example issue 181.

@julealgon
Copy link
Contributor

Not directly. However, it's easily reproducible by using the E2E EntitySetAggregation tests in this repository.

The EntitySetAggregationTests.AggregationOnEntitySetWorks(method: "average", expected: 100) succeed normal ...

Maybe I'm missing something obvious, but that test doesn't work at all for me:

Message: 
System.InvalidOperationException : The LINQ expression '(GroupByShaperExpression:
KeySelector: new NoGroupByWrapper(),
ElementSelector:(EntityShaperExpression:
EntityType: Customer
ValueBufferExpression:
(ProjectionBindingExpression: EmptyProjectionMember)
IsNullable: False
)
)
.SelectMany($it => $it.Orders)' could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync(). See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.

Stack Trace: 
RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
MethodCallExpression.Accept(ExpressionVisitor visitor)
ExpressionVisitor.Visit(Expression node)
RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
MethodCallExpression.Accept(ExpressionVisitor visitor)
ExpressionVisitor.Visit(Expression node)
RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
MethodCallExpression.Accept(ExpressionVisitor visitor)
ExpressionVisitor.Visit(Expression node)
RelationalSqlTranslatingExpressionVisitor.Translate(Expression expression)
RelationalProjectionBindingExpressionVisitor.Visit(Expression expression)
RelationalProjectionBindingExpressionVisitor.VisitMemberAssignment(MemberAssignment memberAssignment)
ExpressionVisitor.VisitMemberBinding(MemberBinding node)
RelationalProjectionBindingExpressionVisitor.VisitMemberInit(MemberInitExpression memberInitExpression)
MemberInitExpression.Accept(ExpressionVisitor visitor)
ExpressionVisitor.Visit(Expression node)
RelationalProjectionBindingExpressionVisitor.Visit(Expression expression)
RelationalProjectionBindingExpressionVisitor.VisitMemberAssignment(MemberAssignment memberAssignment)
ExpressionVisitor.VisitMemberBinding(MemberBinding node)
RelationalProjectionBindingExpressionVisitor.VisitMemberInit(MemberInitExpression memberInitExpression)
MemberInitExpression.Accept(ExpressionVisitor visitor)
ExpressionVisitor.Visit(Expression node)
RelationalProjectionBindingExpressionVisitor.Visit(Expression expression)
RelationalProjectionBindingExpressionVisitor.Translate(SelectExpression selectExpression, Expression expression)
RelationalQueryableMethodTranslatingExpressionVisitor.TranslateSelect(ShapedQueryExpression source, LambdaExpression selector)
QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
MethodCallExpression.Accept(ExpressionVisitor visitor)
ExpressionVisitor.Visit(Expression node)
QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
Database.CompileQuery[TResult](Expression query, Boolean async)
QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
<>c__DisplayClass12_01.<ExecuteAsync>b__0() CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func1 compiler)
CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func1 compiler) QueryCompiler.ExecuteAsync[TResult](Expression query, CancellationToken cancellationToken) EntityQueryProvider.ExecuteAsync[TResult](Expression expression, CancellationToken cancellationToken) EntityQueryable1.GetAsyncEnumerator(CancellationToken cancellationToken)
AsyncEnumerableReader.ReadInternal[T](Object value)
ObjectResultExecutor.ExecuteAsyncEnumerable(ActionContext context, ObjectResult result, Object asyncEnumerable, Func2 reader) ResourceInvoker.<InvokeResultAsync>g__Logged|21_0(ResourceInvoker invoker, IActionResult result) ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|29_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted) ResourceInvoker.Rethrow(ResultExecutedContextSealed context) ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted) ResourceInvoker.InvokeResultFilters() --- End of stack trace from previous location where exception was thrown --- ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|19_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted) ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker) EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger) <<SendAsync>g__RunRequestAsync|0>d.MoveNext() --- End of stack trace from previous location where exception was thrown --- ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) CookieContainerHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) HttpClient.FinishSendAsyncBuffered(Task1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
EntitySetAggregationTests.AggregationOnEntitySetWorks(String method, Int32 expected) line 65
--- End of stack trace from previous location where exception was thrown ---

@cympatic
Copy link

That's strange. I just forked this repository, cloned it to my machine, removed the Skip in the Theory-attribute on that test, and ran the test as-is. The database EntitySetAggregationContext is created based on the connectionstring Server=(localdb)\mssqllocaldb;Database=EntitySetAggregationContext;Trusted_Connection=True; as expected and the test is executed successfully.

@cympatic
Copy link

cympatic commented Jan 1, 2023

OData.Issue.181.zip
I've created and attached a small sample project based on the EntitySetAggregationTests. Hopefully this helps

@julealgon
Copy link
Contributor

Hopefully this helps

It does.

Took me a while to realize how to make this work (for some weird reason...), but I can see now how it's supposed to be handled.

Change the method as per the following:

    [EnableQuery]
    [HttpGet("AsActionResult")]
    public ActionResult<IQueryable<Customer>> AsActionResult()
    {
        return _context.Customers;
    }

The conversion from T to ActionResult<T> actually generates an ObjectResult with populated DeclaredType, which makes everything work as expected.

Using IActionResult is mostly considered a bad practice these days when returning typed data from controllers (should be reserved to void-returning methods).

In this scenario, I'd not recommend any changes be made to OData/EnableQueryAttribute at all.

@cympatic
Copy link

cympatic commented Jan 2, 2023

The change

    [EnableQuery]
    [HttpGet("AsActionResult")]
    public ActionResult<IQueryable<Customer>> AsActionResult()
    {
        return _context.Customers;
    }

result still in a JSON result and not in an OData result when $apply is used. When this method results in a proper OData result the tests for this method will fail.

The conversion from T to ActionResult actually generates an ObjectResult with populated DeclaredType, which makes everything work as expected.
This is not true when using $apply. This only work when IQueryable<Customer> is used.

Using IActionResult works for queries without $apply and isn't the issue. It's when $apply is used as mentioned in my earlier post

The ODataOutputFormatter.CanWriteResult returns false because the context.ObjectType is not set and the context.Object.GetType() returns an IEnumerable<GroupByWrapper> or descending class of GroupByWrapper for which no proper PayLoadKind can be determined. This occurred when an ObjectResult is used as controller action result. When IQueryable<> is used as controller action result the context.ObjectType is set in the ODataOutputFormatter.CanWriteResult and a proper PayLoadKind is determined.

@julealgon
Copy link
Contributor

julealgon commented Jan 2, 2023

The change

    [EnableQuery]
    [HttpGet("AsActionResult")]
    public ActionResult<IQueryable<Customer>> AsActionResult()
    {
        return _context.Customers;
    }

result still in a JSON result and not in an OData result when $apply is used.

It definitely works for me:

image

When this method results in a proper OData result the tests for this method will fail.

The test is failing for me with that change.

EDIT: @cympatic did you perhaps forget to remove the Ok(...)? That is a very subtle detail but it actually makes a difference.

@cympatic
Copy link

cympatic commented Jan 2, 2023

@cympatic did you perhaps forget to remove the Ok(...)? That is a very subtle detail but it actually makes a difference.

Thanks! That was indeed the problem, I missed that

@cympatic
Copy link

cympatic commented Jan 3, 2023

@julealgon is it fair to say that this issue is solved? Or at least explained well enough?

@julealgon
Copy link
Contributor

@julealgon is it fair to say that this issue is solved? Or at least explained well enough?

As I mentioned above, from my perspective this appears to be working properly considering how using ActionResult<T> for modern projects is the standard way of providing the return type at compile time.

I'm not part of the OData team however, so I can't close this myself.

Either @komdil can validate and close, or we can wait for someone on the team to take a look.

@jimbromley-avanade
Copy link

Prior to closing I would like to encourage updating samples / documentation at minimum to reflect the requirement of using ActionResult<T> over IActionResult.

@audacity76
Copy link

@cympatic Thanks for bringing this up and providing a workaround!

In my case I had to apply the ODataQueryOptions on my own. The documentation shows this example:

public IQueryable<Customer> Get(ODataQueryOptions<Customer> options)
{
    IQueryable results = options.ApplyTo(Customers.AsQueryable());

    return results as IQueryable<Customer>;
}

But this code doesn't work if select or apply is used because after applying the query isn't of type IQueryable<Customer>. So the code needs to be changed to something like:

public IQueryable Get(ODataQueryOptions<Customer> options)
{
    IQueryable results = options.ApplyTo(Customers.AsQueryable());

    return results;
}

This in turn brings up the problem from this reported issue. I ended up with a simple ActionFilterAttribute that I can attach to these special cases:

public class ODataTypeAttribute : ActionFilterAttribute
{
    public override void OnActionExecuted(ActionExecutedContext actionExecutedContext)
    {
        if (actionExecutedContext is null)
        {
            throw new ArgumentNullException(nameof(actionExecutedContext));
        }

        if (actionExecutedContext.HttpContext.Response != null &&
            IsSuccessStatusCode(actionExecutedContext.HttpContext.Response.StatusCode) &&
            actionExecutedContext.Result is ObjectResult content &&
            content.Value != null &&
            (content.DeclaredType == typeof(ActionResult) || content.DeclaredType == typeof(IQueryable) 
            || content.DeclaredType == typeof(Task<ActionResult>) || content.DeclaredType == typeof(Task<IQueryable>))
            )
        {
            // To help the `ODataOutputFormatter` to determine the correct output class
            // the `content.DeclaredType` needs to be set so that a valid `OData`-result is produced
            // https://github.com/OData/AspNetCoreOData/blob/4de92f52a346606a447ec4df96c5f3cd05642f50/src/Microsoft.AspNetCore.OData/Formatter/ODataOutputFormatter.cs#L127-L131

            var returnType = content.Value.GetType();
            if (returnType.IsConstructedGenericType)
            {
                returnType = returnType.GenericTypeArguments[0];
            }

            content.DeclaredType = returnType;
        }

        base.OnActionExecuted(actionExecutedContext);
    }

    private static bool IsSuccessStatusCode(int statusCode)
    {
        return statusCode >= 200 && statusCode < 300;
    }
}

@xuzhg Documentation needs to be adjusted and is there a real solution to that problem?

@keatkeat87
Copy link

summary to any newcomer

Date: 2024-04-12
.NET: 8.0.4
OData: 8.2.5

Official document example:
image

My Controller Action -- return OkObjectResult

public ActionResult<IQueryable<Person>> GetPeople()
{
  return Ok(db.People); 
}

Simple Query: http://localhost:5237/api/people

{
  "@odata.context": "http://localhost:5237/api/$metadata#people",
  "value": [
    {
      "id": 1,
      "name": "Derrick",
      "startDate": "2023-01-01",
      "timeOfDay": "16:50:00.0000000"
    },
    {
      "id": 2,
      "name": "Derrick",
      "startDate": "2023-01-01",
      "timeOfDay": "16:50:00.0000000"
    }
  ]
}

perfect!
Query with $apply: http://localhost:5237/api/people?$apply=groupby((name),aggregate($count as count))&$count=true

[
  {
    "name": "Derrick",
    "count": 2
  }
]

missing @odata.context and @odata.count

My Controller Action -- return IQueryable

public ActionResult<IQueryable<Person>> GetPeople()
{
  return db.People;
}

Query with $apply: http://localhost:5237/api/people?$apply=groupby((name),aggregate($count as count))&$count=true

{
  "@odata.context": "http://localhost:5237/api/$metadata#people(name,count)",
  "@odata.count": 1,
  "value": [
    {
      "@odata.id": null,
      "name": "Derrick",
      "count": 2
    }
  ]
}

perfect!

The conclusion is:
The example on the official website only applies to cases without $apply.
In cases where $apply is used, OkObjectResult cannot be returned; IQueryable must be returned directly.
It is recommended that everyone always return IQueryable.

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

No branches or pull requests

9 participants