OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
453 stars 160 forks source link

Missing property error when applying select manually #1231

Open KnapSac opened 4 months ago

KnapSac commented 4 months ago

Assemblies affected ASP.NET Core OData 8.0.0 and above

Describe the bug Because I want to apply $filter manually (I can't rely on the default EF integration, but still want to take advantage of my DB), I also have to apply $select manually. When I do this, selecting a subset of the properties gives an error about a missing property on the EDM instance. When selecting all properties, it works as expected.

Reproduce steps For the code, see this reproduction.

Data Model See repro code.

EDM (CSDL) Model See repro code.

Request/Response http://localhost:5247/Customers?$select=id

Expected behavior A response with only the ids of all instances.

Additional context

fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
      An unhandled exception has occurred while executing the request.
      System.InvalidOperationException: The EDM instance of type '[ODataSelectRepro.Customer Nullable=True]' is missing the property 'Name'.
         at Microsoft.AspNetCore.OData.Formatter.ResourceContext.GetPropertyValue(String propertyName)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSerializer.CreateStructuralProperty(IEdmStructuralProperty structuralProperty, ResourceContext resourceContext)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSerializer.CreateStructuralPropertyBag(SelectExpandNode selectExpandNode, ResourceContext resourceContext)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSerializer.CreateResource(SelectExpandNode selectExpandNode, ResourceContext resourceContext)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSerializer.WriteResourceAsync(Object graph, ODataWriter writer, ODataSerializerContext writeContext, IEdmTypeReference expectedType)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSerializer.WriteObjectInlineAsync(Object graph, IEdmTypeReference expectedType, ODataWriter writer, ODataSerializerContext writeContext)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSetSerializer.WriteResourceSetItemAsync(Object item, IEdmStructuredTypeReference elementType, Boolean isUntypedCollection, IEdmTypeReference resourceSetType, ODataWriter writer, IODataEdmTypeSerializer resourceSerializer, ODataSerializerContext writeContext)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSetSerializer.WriteResourceSetAsync(IEnumerable enumerable, IEdmTypeReference resourceSetType, ODataWriter writer, ODataSerializerContext writeContext)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSetSerializer.WriteObjectInlineAsync(Object graph, IEdmTypeReference expectedType, ODataWriter writer, ODataSerializerContext writeContext)
         at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSetSerializer.WriteObjectAsync(Object graph, Type type, ODataMessageWriter messageWriter, ODataSerializerContext writeContext)
         at Microsoft.AspNetCore.OData.Formatter.ODataOutputFormatterHelper.WriteToStreamAsync(Type type, Object value, IEdmModel model, ODataVersion version, Uri baseAddress, MediaTypeHeaderValue contentType, HttpRequest request, IHeaderDictionary requestHeaders, IODataSerializerProvider serializerProvider)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeResultAsync>g__Logged|22_0(ResourceInvoker invoker, IActionResult result)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeResultFilters>g__Awaited|28_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)         at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)
julealgon commented 4 months ago

Check if it makes any difference if you remove the Ok from your action implementation:

Switch from this:

    public ActionResult< IQueryable > Get(
        ODataQueryOptions< Customer > queryOptions )
    {
        IQueryable< Customer > values = s_Customers.AsQueryable( );

        return Ok(
            queryOptions.SelectExpand.ApplyTo(
                values,
                new ODataQuerySettings( ) ) );
    }

To this:

    public ActionResult< IQueryable > Get(
        ODataQueryOptions< Customer > queryOptions )
    {
        IQueryable< Customer > values = s_Customers.AsQueryable( );

        return queryOptions.SelectExpand.ApplyTo(
            values,
            new ODataQuerySettings( ) ) ;
    }
KnapSac commented 4 months ago

Removing the Ok doesn't compile, if I change the return type to IQueryable it does, but the return value is missing the @odata.context property.

Without wrapping return type of Get in ActionResult:

[{"Id":1},{"Id":2},{"Id":3}]

When wrapping return type of Get with ActionResult and selecting name too:

{"@odata.context":"http://localhost:5247/$metadata#Customers","value":[{"Id":1,"Name":"Customer 1"},{"Id":2,"Name":"Customer 2"},{"Id":3,"Name":"Customer 3"}]}
julealgon commented 4 months ago

@KnapSac I tried some different things with your sample and I believe this is the correct way to approach your problem if you only want to apply the select portion from ODataQueryOptions:

    public ActionResult<IQueryable> Get(
        ODataQueryOptions<Customer> queryOptions)
    {
        IQueryable values = s_Customers.AsQueryable();

        values = queryOptions.ApplyTo(values, ignoreQueryOptions: ~AllowedQueryOptions.Select);

        return Ok(values);
    }

Instead of attempting to explicitly apply only the SelectExpand property, you apply the whole ODataQueryOptions object and specify which operations you want to exclude. In this case, we "exclude everything but $select" by passing the negated select option.

However.... when I test this, for whatever reason, I noticed that even if I pass something else on the ignoreQueryOptions, the actual select operation still happens. The only difference I see is that when $select is "blocked", it doesn't show in the @odata.context path.

I don't understand what is going on here... it's as if the [EnableQuery] attribute is somehow active on the action, even though it is not specified. But then... if I remove the ApplyTo call completely and just return the original datasource, it does NOT perform the selection at all.

Goes without saying I'm also fairly confused at what is going on here. We might have to wait for someone on the team to clarify that behavior.

In the meantime, I hope this helps.

xuzhg commented 4 months ago

@KnapSac Thanks @julealgon. Changing the codes as below does work as expected:

    public ActionResult< IQueryable > Get(
        ODataQueryOptions< Customer > queryOptions )
    {
        IQueryable< Customer > values = s_Customers.AsQueryable( );

        return Ok(
            queryOptions.ApplyTo(
                values,
                new ODataQuerySettings( ) ) );

        //return Ok(
        //    queryOptions.SelectExpand.ApplyTo(
       //        values,
        //        new ODataQuerySettings()));
    }

image

The reason is that OData serializer needs the SelectExpandClause to decide which properties should be included. In your previous implementation, OData serializer can't get the 'SelectExpandClause' since you call it from queryOptions.SelectExpand.ApplyTo(...).

KnapSac commented 4 months ago

The reason is that OData serializer needs the SelectExpandClause to decide which properties should be included. In your previous implementation, OData serializer can't get the 'SelectExpandClause' since you call it from queryOptions.SelectExpand.ApplyTo(...).

Ah, that makes sense. And as @julealgon suggested, I can use the ignoreQueryOptions to skip applying e.g. $filter, which I can then apply manually.

Instead of attempting to explicitly apply only the SelectExpand property, you apply the whole ODataQueryOptions object and specify which operations you want to exclude. In this case, we "exclude everything but $select" by passing the negated select option.

However.... when I test this, for whatever reason, I noticed that even if I pass something else on the ignoreQueryOptions, the actual select operation still happens. The only difference I see is that when $select is "blocked", it doesn't show in the @odata.context path.

I can reproduce this behavior too, and to me that does seem like a bug in the library.

julealgon commented 4 months ago

@xuzhg can you take a look at this apparent problem with the partial ApplyTo call? It is incredibly weird to me what is going on, because when I inspect the results after the call, it does seem to respect the ignoreQueryOptions parameter, but when it comes to showing the results, the serializer somehow still applies the selection to it.

When I first saw that, I thought maybe the [EnableQuery] filter was being applied somehow, but I checked the codebase yesterday to be sure and I don't see any existing mechanism that would "automatically" include that filter, so it can't be that.