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

[feature/netcore] ODataActionParameters is not binding parameters #1249

Closed
gordon-matt opened this issue Feb 25, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@gordon-matt
Copy link

commented Feb 25, 2018

I am migrating some code from MVC5 to .NET Core and I can see that ODataActionParameters is not binding parameters.

public virtual async Task<IEnumerable<ContentBlock>> GetByPageId(ODataQueryOptions<ContentBlock> options, ODataActionParameters parameters)
{
    // 'parameters' is empty when I debug here...
}

I am definitely registering the OData Action in the same way as before:

var getByPageIdAction = builder.EntityType<ContentBlock>().Collection.Action("GetByPageId");
getByPageIdAction.Parameter<Guid>("pageId");
getByPageIdAction.ReturnsCollectionFromEntitySet<ContentBlock>("ContentBlocks");

And I can see that the pageId param is definitely being sent in the request.

Anything else you need from me in order to investigate further? Thanks in advance.

@gordon-matt

This comment has been minimized.

Copy link
Author

commented Mar 23, 2018

Any progress?

@Domitnator

This comment has been minimized.

Copy link

commented Apr 3, 2018

I am running into the same issue!

@robward-ms robward-ms added question and removed bug labels Apr 3, 2018

@robward-ms

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2018

[FromBody] is no longer implicit in AspNetCore. We could add it by default but then you lose some control.

From the docs:

There can be at most one parameter per action decorated with [FromBody]. The ASP.NET Core MVC run-time delegates the responsibility of reading the request stream to the formatter. Once the request stream is read for a parameter, it's generally not possible to read the request stream again for binding other [FromBody] parameters.

See this test for an example.

@gordon-matt

This comment has been minimized.

Copy link
Author

commented Apr 4, 2018

@robward-ms Thanks, but I see a problem with that. Although adding [FromBody] fixes the issue with ODataActionParameters, that leaves me with a small problem in 2 of my actions... 1 of them being this one. If you look at my method, you will see it takes both an ODataQueryOptions and an ODataActionParameters. Now ODataQueryOptions is not being bound because [FromBody] is only applied to the ODataActionParameters. This was not a problem in MVC5, but obviously in .NET Core we need [FromBody] and it can only be applied to one parameter... so what do you suggest here? Is there something you can do to automatically bind the ODataQueryOptions?

UPDATE: I just realized: I may be able to convert this into an OData function instead of an OData action... and solve it that way.. because I only have 1 param I am getting from the ODataActionParameters and I can just make it a Guid and use the [FromODataUri] attribute on that. I will try that and let you know what happens

@gordon-matt

This comment has been minimized.

Copy link
Author

commented Apr 4, 2018

@robward-ms OK yes, I solved this by doing the following:

  • Removed the [HttpPost] attribute from my method and

  • Changed the method signature to the following:
    public virtual async Task<IEnumerable<ContentBlock>> GetByPageId([FromODataUri] Guid pageId, ODataQueryOptions<ContentBlock> options)

  • Finally: changed the registration from action to function as follows:

var getByPageIdFunction = builder.EntityType<ContentBlock>().Collection.Function("GetByPageId");
getByPageIdFunction.Parameter<Guid>("pageId");
getByPageIdFunction.ReturnsCollectionFromEntitySet<ContentBlock>("ContentBlockApi");

Seems to be working fine now. So for OData actions, we can only have 1 param and decorate it with [FromBody]. Else, use an OData function instead, which in my case actually makes more sense anyway.

@Domitnator Hope this helps you too...

@gordon-matt gordon-matt closed this Apr 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.