odata.net icon indicating copy to clipboard operation
odata.net copied to clipboard

OData.Client - Projection on expanded paths produces long query with redundant parts

Open uffelauesen opened this issue 1 year ago • 6 comments

OData.Client produces unnecessary long URIs with a lot of repeated redundancy for projected expand paths.

Assemblies affected

OData.Client .Net lib 7.x and 8.x

Reproduce steps

Using the TripPin service consider the following query on People that expands and projects BestFriend (and BestFriend's Friends) and Friends...

var query = from p in ctx.People
            select new Person
            {
                UserName = p.UserName,
                BestFriend = new Person
                {
                    UserName = p.BestFriend.UserName,
                    FirstName = p.BestFriend.FirstName,
                    LastName = p.BestFriend.LastName,
                    Age = p.BestFriend.Age,
                    Friends = new DataServiceCollection<Person>(
                        from f in p.BestFriend.Friends
                        select new Person
                        {
                            UserName = f.UserName,
                            FirstName = f.FirstName,
                            LastName = f.LastName,
                            Age = f.Age
                        })
                },
                Friends = new DataServiceCollection<Person>(
                    from f in p.Friends
                    select new Person
                    {
                        UserName = f.UserName,
                        FirstName = f.FirstName,
                        LastName = f.LastName,
                        Age = f.Age
                    })
            };

Console.WriteLine(query.ToString());

Expected result

The query/request produced would have projected properties ($select) inside a single expand path making the query/request as short as possible, like...

/People?$expand=BestFriend($select=UserName,FirstName,LastName,Age;$expand=Friends($select=UserName,FirstName,LastName,Age)),Friends($select=UserName,FirstName,LastName,Age)&$select=UserName

Actual result

The expand paths are repeated for each projected property...

/People?$expand=BestFriend($select=UserName),BestFriend($select=FirstName),BestFriend($select=LastName),BestFriend($select=Age),BestFriend($expand=Friends($select=UserName)),BestFriend($expand=Friends($select=FirstName)),BestFriend($expand=Friends($select=LastName)),BestFriend($expand=Friends($select=Age)),Friends($select=UserName),Friends($select=FirstName),Friends($select=LastName),Friends($select=Age)&$select=UserName

Additional detail

The order of properties and expand/select could be different from the expected result, but the structure should be intact.

uffelauesen avatar Aug 26 '24 07:08 uffelauesen

This is a bug according to the standard:

A property MUST NOT appear in more than one expand item.

corranrogue9 avatar Aug 27 '24 16:08 corranrogue9

@uffelauesen What is the end goal The expected URL is not returning any data. Getting:

{
    "error": {
        "code": "",
        "message": "An error has occurred."
    }
}

WanjohiSammy avatar Aug 27 '24 16:08 WanjohiSammy

@WanjohiSammy Yes - it looks like the reference trippin service has an unrelated bug and fails to execute either query. Please don’t get too caught up in this - I was trying to describe the bug in a model context that is public available (like the trippin service). Too bad the trippin service fails, never the less the expected URI result is correct as I described - to the best of my knowledge. I can supply a decoupled unit test tomorrow that shows the issue if you need. Thanks for looking into it. Uffe

uffelauesen avatar Aug 27 '24 16:08 uffelauesen

@WanjohiSammy The TripPin reference service failed on both queries what seems to be due to HomeAddress being null for some people returned. This is probably an unrelated bug in the reference service. I have rewritten the example in to not select HomeAddress but I have added another expand level to make it even more clear what is going wrong. Both actual query and expected query now passes and produces the same results against the TripPin service. The actual (wrong) query probably passes dues to the fact the UriParser does a normalization of expand and select trees in SelectExpandSemanticBinder.Bind. Do you need an unit test example or is everything clear? Thanks for looking into this. Uffe

uffelauesen avatar Aug 28 '24 07:08 uffelauesen

@WanjohiSammy Please consider rewriting the ProjectionAnalyzer, SelectExpandPathBuilder, SelectExpandPathToStringVisitor to use something similar to QueryToken (and its derrived types ExpandToken, SelectToken etc...) that can handle tree structures. Please also consider getting the visitors (currently in ProjectionAnalyser) to also handle missing ODataV4 features like filtering and ordering of expanded entity collections as described here: #2735

uffelauesen avatar Aug 28 '24 07:08 uffelauesen

Thanks @uffelauesen

@gathogojr, @xuzhg, @ElizabethOkerio, @corranrogue9, @paulodero, @habbes: I have just confirmed that the below URL is returning data. You can confirm: /People?$expand=BestFriend($select=UserName),BestFriend($select=FirstName),BestFriend($select=LastName),BestFriend($select=Age),BestFriend($expand=Friends($select=UserName)),BestFriend($expand=Friends($select=FirstName)),BestFriend($expand=Friends($select=LastName)),BestFriend($expand=Friends($select=Age)),Friends($select=UserName),Friends($select=FirstName),Friends($select=LastName),Friends($select=Age)&$select=UserName

The endpoint that was initially there had an issue with HomeAddress being null for some people and hence the error.

  1. When you select/filter on a ComplexType or Navigation Property that is null, you will get an error. Example:
  2. This is a bug according to the standard:

A property MUST NOT appear in more than one expand item.

WanjohiSammy avatar Aug 28 '24 08:08 WanjohiSammy