$select query option inside $expand is ignored #1412
Comments
|
Would you mind adding the C# class for your data model to the description? Would make it a lot easier for the contributors to reproduce the problem. |
|
Our model is dynamic and therefore is untyped. We use EDM objects instead of C# classes.
Cheers,
Alex
On Feb 7, 2025, at 08:07, Anas Ismail Khan ***@***.***> wrote:
Would you mind adding the C# class for your data model to the description?
—
Reply to this email directly, view it on GitHub<#1412 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADLM2QQ65YANQNM7KBKRMYD2OSV2LAVCNFSM6AAAAABWT7OTSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNBSHA3DEMJYGA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
We have noticed another potentially related issue. If a navigation property is populated on a parent EDM object, OData serializer will output it in the response even if it's not explicitly selected / expanded. For example, the following query /Sites('3C3C802D-2D04-49D9-8E33-C09A64530A9C')?$select=Id,Name returns this response: {
"@odata.context":"https://localhost:7068/api/v1/$metadata#Sites(Id,Name)/$entity",
"Id":"3C3C802D-2D04-49D9-8E33-C09A64530A9C",
"Name":"Site A",
"Plants@odata.context":"https://localhost:7068/api/v1/$metadata#Sites('3C3C802D-2D04-49D9-8E33-C09A64530A9C')/Plants",
"Plants":[]
}Same as with the original issue, this behavior did not exist until ASP.NET Core OData 9.1.3. If we roll back to 9.1.1 everything works as expected and we get: {
"@odata.context":"https://localhost:7068/api/v1/$metadata#Sites(Id,Name)/$entity",
"Id":"3C3C802D-2D04-49D9-8E33-C09A64530A9C",
"Name":"Site A"
} |
My bad, you did mention that. However, it appears that you're using a custom serializer so it would help if you added that. Could you share your serializer? You seem to be using a custom one. |
|
We do have custom serializer but it's irrelevant because the first call it makes is base.CreateResource(selectExpandNode, resourceContext) and this is where it fails with MissingMethodExeption. So our custom code does not even have a chance. As I mentioned, one difference we observe between 9.1.1 and 9.1.3 is that in the former package SelectExpandNode.SelectedStructuralProperties collection only has selected properties and in the latter package this collection has ALL properties, ignoring $select query option. public class SmartODataResourceSerializer : ODataResourceSerializer
{
/// <inheritdoc/>
public override ODataResource CreateResource(SelectExpandNode selectExpandNode,
ResourceContext resourceContext)
{
// This call fails with MissingMethodException trying to populate default values for properties
var resource = base.CreateResource(selectExpandNode, resourceContext);
if (null != resource)
{
// Our custom code
}
return resource;
}
} |
|
I think I see the problem. Looking into it. |
@xuzhg, is this default behavior now? Because I can see what's causing it, I just don't understand why. |
|
@korygin Can you share a simple repro that can help us understand the issue better and also add the necessary to avoid similar regression in the future? |
|
@gathogojr , If you look at PR #1413, you can find instructions for reproducing the bug. I think I have fixed it and I was hoping you could test my PR for me. |
|
@anasik Even better is to add a test (most preferably an E2E) to verify your change. Otherwise nothing will prevent people from making changes that cause a similar regression. This regression happened because no test covered the scenario. If we don't add one, it can easily happen again |
Yeah I was thinking of adding one but since the sample already existed, I wasn't sure of what you guys would prefer in this case. I guess I'll go ahead and add one? |
|
@gathogojr, I've added the tests to the PR. |
|
@gathogojr, this is a little unprofessional. I respect that you're a major contributor to this project and that you were assigned to work on this bug. But that was after I had already submitted a PR with a viable fix.
C.C. @habbes |
|
@gathogojr watch out not to discourage external contributions man, @anasik has a good point here. OData is already niche as it is, so we don't have a ton of people interested in contributing in the first place. If the team then ignores contributions and makes the changes on their own it will only make that worse. |
|
@anasik I will provide feedback on your pull request. There are a few problems that we identified that go beyond the reported issue, some of which I haven't even covered in my pull request. We'll work on those in due time. For example, you'd agree with me that the currently implementation of |
|
@gathogojr, I appreciate that. But yeah, if you'd asked, I would have been more than willing to investigate and analyse further. Before this bug was reported, I had no idea what a Delta was, but I caught up and even fixed it. I obviously don't know the codebase as well as you guys do but I perform my due diligence and am always up for the challenge. |
Assemblies affected
ASP.NET Core OData > 9.1.1
Describe the bug
When using $select query option inside $expand, OData query builder ignores it and includes ALL structural properties in SelectExpandNode.SelectedStructuralProperties collection. This causes OData resource serializer to fail while trying to create default values for properties that are not populated on expanded EDM entity object(s) because, for example, string type doesn't have the default constructor.
This behavior first appeared in ASP.NET Core OData 9.1.3 and didn't exist in ASP.NET Core OData <= 9.1.1 (in prior versions only selected properties were populated in SelectExpandNode.SelectedStructuralProperties).
Reproduce steps
We use untyped EDM implementation. Any model that has a simple parent -> child relationship will work. For the model below, the following query will fail: /Sites('3C3C802D-2D04-49D9-8E33-C09A64530A9C')/Plants('E63F816D-B7E7-4FC9-AE62-7E709AFF290E')?$select=Name&$expand=Areas($select=Name) if expanded Area EdmEntityObject only has Name property set as requested by the query.
Data Model
Please share your Data model, for example, your C# class.
EDM (CSDL) Model
Request/Response
Please share your request Uri, head or the request body
/Sites('3C3C802D-2D04-49D9-8E33-C09A64530A9C')/Plants('E63F816D-B7E7-4FC9-AE62-7E709AFF290E')?$select=Name&$expand=Areas($select=Name)
Please share your response head, body.
HTTP 200 (OK) response with no body. The following error is thrown from OData resource serializer (notice that SelectedStructuralProperties lists ALL properties even though only Name was selected):
Expected behavior
A clear and concise description of what you expected to happen.
The request succeeds with expanded entity only having selected Name property as shown in the screenshot below taken with ASP.NET Core OData 9.1.1
Screenshots
If applicable, add screenshots to help explain your problem.
Additional context
Please share your call stack or any error message
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: