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

Missing property error when applying select manually #1231

Open
KnapSac opened this issue May 8, 2024 · 6 comments
Open

Missing property error when applying select manually #1231

KnapSac opened this issue May 8, 2024 · 6 comments
Assignees
Labels
bug Something isn't working investigated

Comments

@KnapSac
Copy link

KnapSac commented May 8, 2024

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)
@KnapSac KnapSac added the bug Something isn't working label May 8, 2024
@julealgon
Copy link
Contributor

julealgon commented May 8, 2024

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
Copy link
Author

KnapSac commented May 8, 2024

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
Copy link
Contributor

@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
Copy link
Member

xuzhg commented May 9, 2024

@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(...).

@xuzhg xuzhg self-assigned this May 9, 2024
@KnapSac
Copy link
Author

KnapSac commented May 10, 2024

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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working investigated
Projects
None yet
Development

No branches or pull requests

3 participants