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

The property '{propertyname}' cannot be used in the $expand query option #2815

Open
mattkleiny opened this issue Nov 6, 2023 · 6 comments
Open
Assignees

Comments

@mattkleiny
Copy link

Periodically when querying the OData endpoints in one of our products, clients receive an unexpected 400 error stating: The property '{propertyname}' cannot be used in the $expand query option. This occurs on different entity sets.

Normally, these properties are 'expandable' and work correctly. However, when the server is under load they start failing periodically with a 400 error.

Assemblies affected

Microsoft.AspNet.OData 7.7.2

Reproduce steps

  1. Set-up a project in IIS with the OData library.
  2. Have it serve 2 entity sets with 2 different models that each include a complex property.
  3. Add an .Expand() option of allowed to those entity sets and those particular complex properties.
  4. Run a stress test against the endpoint that simultaneously executes those endpoints. Notice that it periodically fails with a 400 error.

Expected result

The response should be successful, and $expand as appropriate.

Actual result

The response will periodically fail with a 400 error stating that "The property '{propertyname}' cannot be used in the $expand query option".

Additional detail

Here's a repository reproducing the issue https://github.com/mattkleiny/odata-bug-repro

@xuzhg
Copy link
Member

xuzhg commented Nov 6, 2023

@mattkleiny Your service has the following configuration:

config.Expand(QueryOptionSetting.Disabled);

This line disables all $expand. Try to use:

config.Expand();

@mattkleiny
Copy link
Author

Thanks for getting back to me @xuzhg.

We need to selectively enable .Expand() on specific entity sets. In the repro I gave you, it's disabled by default with config.Expand(QueryOptionSetting.Disabled); in top-level config, and then specifically enabled on the two entity sets.

It will fail intermittently. Some requests do succeed and produce the correct response. Here's an example after just a few seconds of running, and I've updated the repro to more clearly show:

There were 195 successful responses and 25 failed responses for /v3/Organisation?$expand=Contacts
There were 341 successful responses and 18 failed responses for /v3/Contact?$expand=BusinessCards

Debugging a little bit, it looks like isExpandable is false in this block of code when we have many simultaneous requests, and if the default query option was enabled with config.Expand();, it would side-step the issue by not throwing an exception here.

if (edmModel != null)
{
ValidateOtherQueryOptionInExpand(property, edmModel, expandItem, validationSettings);
bool isExpandable;
ExpandConfiguration expandConfiguration;
isExpandable = EdmLibHelpers.IsExpandable(property.Name,
pathProperty,
pathStructuredType,
edmModel,
out expandConfiguration);
if (isExpandable)
{
int maxDepth = expandConfiguration.MaxDepth;
if (maxDepth > 0 && (remainDepth == null || maxDepth < remainDepth))
{
remainDepth = maxDepth;
}
}
else if (!isExpandable)
{
if (!_defaultQuerySettings.EnableExpand ||
(expandConfiguration != null && expandConfiguration.ExpandType == SelectExpandType.Disabled))
{
throw new ODataException(Error.Format(SRResources.NotExpandablePropertyUsedInExpand,
property.Name));
}
}
}

I was tracing the source of this information back into EdmLibHelpers.IsExpandable and unfortunately it gets quite complicated quite quickly, it seems to be sourced through an annotation of some kind and stored in a versioned dictionary... it does look like there is shared data across multiple requests underneath and I suspect it's a race condition of some sort, but I'm just speculating 🤷‍♂️.

@xuzhg
Copy link
Member

xuzhg commented Nov 7, 2023

Ok, It seems thread-safe problem and I already refactored it in our 8 version at OData/AspNetCoreOData#800.

Do you mind to try our 8.x version? It's easy to update. If you need me to try, please share the 'write' permission to your GitHub repository.

We have a plan to port back the changes from 8 to 7, but it needs time to finish it.

@mattkleiny
Copy link
Author

Thanks @xuzhg! I've given you write access, if you'd like to try to update.

I was under the impression that the ASP .NET Core version that you linked to would be incompatible with our old .NET Framework based app. It would be a much larger piece of work to migrate off of .NET Framework, of course. But if there's a way we can simply update to the 8.x package then that's great.

@xuzhg
Copy link
Member

xuzhg commented Nov 10, 2023

@mattkleiny
Copy link
Author

Thanks @xuzhg for opening that PR. I see that the change involved is primarily about using an ASP.NET Core project? That would be great, but sadly we can't update our stack at this point.

Hoping that the fix will get backported into the Web API library at some point. Should I leave this issue open until then? Others might have a similar problem.

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

2 participants