OData / odata.net

ODataLib: Open Data Protocol - .NET Libraries and Frameworks
https://docs.microsoft.com/odata
Other
675 stars 348 forks source link

OData.Client 8 preview fails to translate navigation query #2984

Open uffelauesen opened 1 month ago

uffelauesen commented 1 month ago

Linq navigation fails to translate.

Assemblies affected

OData .Net lib 8 preview 1 and 2

Reproduce steps

Using the trippin reference service the following linq query fails to translate to a request Uri...

        var ctx = new Container(new Uri("https://services.odata.org/TripPinRESTierService/"));
        var q = from p in ctx.People
                where p.UserName == "russellwhyte"
                select p.BestFriend;

        Console.WriteLine(q);

Expected result

The query would translate to https://services.odata.org/TripPinRESTierService/People('russellwhyte')/BestFriend

Actual result

You get an exception:

Error translating Linq expression to URI: Can only specify query options (orderby, where, take, skip) after last navigation. Unhandled exception. System.NotSupportedException: Can only specify query options (orderby, where, take, skip) after last navigation. at Microsoft.OData.Client.ResourceBinder.ValidationRules.RequireCanNavigate(Expression e) at Microsoft.OData.Client.ResourceBinder.IsValidNavigationSource(Expression input, ResourceExpression& sourceExpression) at Microsoft.OData.Client.ResourceBinder.AnalyzeNavigation(MethodCallExpression mce, DataServiceContext context) at Microsoft.OData.Client.ResourceBinder.VisitMethodCall(MethodCallExpression mce) at Microsoft.OData.Client.ALinqExpressionVisitor.Visit(Expression exp) at Microsoft.OData.Client.DataServiceALinqExpressionVisitor.Visit(Expression exp) at Microsoft.OData.Client.ALinqExpressionVisitor.VisitExpressionList(ReadOnlyCollection1 original) at Microsoft.OData.Client.ALinqExpressionVisitor.VisitMethodCall(MethodCallExpression m) at Microsoft.OData.Client.ResourceBinder.VisitMethodCall(MethodCallExpression mce) at Microsoft.OData.Client.ALinqExpressionVisitor.Visit(Expression exp) at Microsoft.OData.Client.DataServiceALinqExpressionVisitor.Visit(Expression exp) at Microsoft.OData.Client.ResourceBinder.Bind(Expression e, DataServiceContext context) at Microsoft.OData.Client.DataServiceQueryProvider.Translate(Expression e) at Microsoft.OData.Client.DataServiceQuery1.Translate() at Microsoft.OData.Client.DataServiceQuery1.Execute() at Microsoft.OData.Client.DataServiceQuery1.GetEnumerator() at System.Linq.Enumerable.TryGetSingle[TSource](IEnumerable1 source, Boolean& found) at System.Linq.Enumerable.Single[TSource](IEnumerable1 source) at Microsoft.OData.Client.DataServiceQueryProvider.ReturnSingleton[TElement](Expression expression) at Microsoft.OData.Client.DataServiceQueryProvider.Execute[TResult](Expression expression) at OData4ClientTest.Program.Main(String[] args) in C:\Users\Z6UFE\Source\Repos\OData4ClientTest\Program.cs:line 17

Additional detail

Rolling back to OData.Client 7.21.2 and everything works again.

habbes commented 1 month ago

@uffelauesen thanks for trying out the perview and thanks for reporting this.

Could you try this instead:

ctx.People.ByKey("russelwhyte").Select(p => p.BestFriend).GetValue // or GetValueAsync()

The problem here is that when you use the where clause against the People entity set, it will always return a collection of Person entities. When you add select and expand query options, those properties will be nested inside the person entities. The OData spec currently provides no way to return only the expanded objects (without having them nested in the parent) through query options. That's why the LINQ expression fails, it doesn't map to any OData URI pattern.

Why did it work in 7.x? Well in 7.x, when we had a where clause whose predicate is a match against the key property, we would convert that in key lookup (equivalent to ByKey()), so it would be translated to /People('russelwhyte'). This would return a single value if the match exists or throw a 404 error otherwise. This special case was inconsistent with how we handle a where clause and caused some issues. Now we translate the same where clause to /People?$filter=UserName eq 'russelwhyte', which is consistent to how other where clause scenarios are handled. This returns an empty collection instead of an error when there are no matches, and returns a collection with a single item if only one item matches.

You can enable the 7.x behaviour by setting the DataServiceContext.KeyComparisonGeneratesFilterQuery property to false. Note that this setting is currently marked as deprecated.

I would recommend using the ByKey() pattern at the top, which would translate the query to People('russellwhyte')/BestFriend nested url, which returns the expected result.

If you want to return a collection of nested objects using a where clause, I'd suggest the following workaround:

// fetch people with expanded navigation property using OData URI People?$expand=BestFriend
var people = ctx.People.Expand('BestFriend').ExecuteAsync();
// perform a local LINQ transform on the result data
var bestFriends = people.Select(p => p.BestFriend);
uffelauesen commented 3 weeks ago

@habbes thanks for the detailed explanation.

I have tried both options - ByKey() as well as setting KeyComparisonGeneratesFilterQuery to false and can confirm that both works as expected.

Maybe the deprecated property should be kept for easy backwards compatible behavior without the need to rewrite each and every query.

I do however find the thrown exception to be confusing or unclear of what the actual problem is.

Btw are there any reason why DataServiceQuerySingle does not have an overload for ToString returning the coresponding url of the query, like you have for DataServiceQuery? I find the ToString overload very handy during debugging.

Thanks again Uffe

habbes commented 3 weeks ago

@uffelauesen thanks for the feedback about retaining the KeyComparisonGeneratesFilterQuery. The property will remain in ODL 8. We should also take this into consideration when deciding whether to keep it in 9.x.

We could look into improving the error message as well.

Lastly, DataServiceQuerySingle.ToString(), I'm not sure if there's any reason it doesn't return the corresponding URL like DataServiceQuery. It could be a feature gap. You could create an issue to request that.