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

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

Open
uffelauesen opened this issue Aug 26, 2024 · 6 comments
Assignees
Labels

Comments

@uffelauesen
Copy link
Contributor

uffelauesen commented Aug 26, 2024

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.

@corranrogue9
Copy link
Contributor

This is a bug according to the standard:

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

@WanjohiSammy
Copy link
Contributor

WanjohiSammy commented Aug 27, 2024

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

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

@WanjohiSammy WanjohiSammy self-assigned this Aug 27, 2024
@uffelauesen
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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

@WanjohiSammy
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants