OData / odata.net

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

Client - inefficient query generated from projection with only expansion #2938

Open uffelauesen opened 5 months ago

uffelauesen commented 5 months ago

Queries with projection of some part that only includes navigation properties will produce a request with only $expand forcing the server to return all primitive and complex properties even though they will not get materialized by the projection.

Assemblies affected

Microsoft.OData.Client 7.21.0

Reproduce steps

Addressing the TripPin service consider the following OData Client code...

            var ctx = new Container(new Uri("https://services.odata.org/TripPinRESTierService/"));

            var q = from p in ctx.People
                    where p.UserName == "russellwhyte"
                    select new Person
                    {
                        Friends = new Microsoft.OData.Client.DataServiceCollection<Person>(
                            from f in p.Friends
                            select new Person
                            {
                                FirstName = f.FirstName
                            })

                    };

            foreach (var p in q)
            {
                Console.WriteLine("p.UserName: " + p.UserName); // Not part of projection - will be null
                foreach (var f in p.Friends)
                {
                    Console.WriteLine("\tf.UserName: " + f.UserName); // Not part of projection - will be null
                    Console.WriteLine("\tf.FirstName: " + f.FirstName); // This is the only primitive type property in the projection and is the only primitive property that will have value given this query and its projection
                }
            }

The only primitive/complex type property getting materialized is the Friends FirstName. All other primitive/complex properties will be null.

Expected result

The generated request will include a $select=Friends letting the server know that it shouldn't return any other properties from the root Person object.

Generated GET request should looks similar this:

/People('russellwhyte')?$expand=Friends($select=FirstName)&$select=Friends Producing this response from the server:

{
  "@odata.context": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/$metadata#People(Friends,Friends(FirstName))/$entity",
  "@odata.type": "#Trippin.Person",
  "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')",
  "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')",
  "Friends@odata.associationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/Friends/$ref",
  "Friends@odata.navigationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/Friends",
  "Friends@odata.type": "#Collection(Trippin.Person)",
  "Friends": [
    {
      "@odata.type": "#Trippin.Person",
      "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('scottketchum')",
      "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('scottketchum')",
      "FirstName": "Scott"
    },
    {
      "@odata.type": "#Trippin.Person",
      "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('ronaldmundy')",
      "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('ronaldmundy')",
      "FirstName": "Ronald"
    },
    {
      "@odata.type": "#Trippin.Person",
      "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('javieralfred')",
      "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('javieralfred')",
      "FirstName": "Javier"
    }
  ]
}

Actual result

The generated request does not include a $selectfor the root path. The generated GET request will be:

/People('russellwhyte')?$expand=Friends($select=FirstName

Forcing the server to return all properties from the root Person (russllwhyte)...

{ "@odata.context": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/$metadata#People(Friends(FirstName))/$entity", "@odata.type": "#Trippin.Person", "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')", "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')", "UserName": "russellwhyte", "FirstName": "Russell", "LastName": "Whyte", "MiddleName": null, "Gender@odata.type": "#Trippin.PersonGender", "Gender": "Male", "Age": null, "Emails@odata.type": "#Collection(String)", "Emails": [ "Russell@example.com", "Russell@contoso.com" ], "FavoriteFeature@odata.type": "#Trippin.Feature", "FavoriteFeature": "Feature1", "Features@odata.type": "#Collection(Trippin.Feature)", "Features": [ "Feature1", "Feature2" ], "AddressInfo@odata.type": "#Collection(Trippin.Location)", "AddressInfo": [ { "@odata.type": "#Trippin.Location", "Address": "187 Suffolk Ln.", "City": { "@odata.type": "#Trippin.City", "Name": "Boise", "CountryRegion": "United States", "Region": "ID" } } ], "HomeAddress": null, "BestFriend@odata.associationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/BestFriend/$ref", "BestFriend@odata.navigationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/BestFriend", "Trips@odata.associationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/Trips/$ref", "Trips@odata.navigationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/Trips", "Friends@odata.associationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/Friends/$ref", "Friends@odata.navigationLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('russellwhyte')/Friends", "Friends@odata.type": "#Collection(Trippin.Person)", "Friends": [ { "@odata.type": "#Trippin.Person", "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('scottketchum')", "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('scottketchum')", "FirstName": "Scott" }, { "@odata.type": "#Trippin.Person", "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('ronaldmundy')", "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('ronaldmundy')", "FirstName": "Ronald" }, { "@odata.type": "#Trippin.Person", "@odata.id": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('javieralfred')", "@odata.editLink": "https://services.odata.org/TripPinRESTierService/(S(gpx2xgjh4kwnmdzkydoag3qr))/People('javieralfred')", "FirstName": "Javier" } ] }

Additional detail

Optional, details of the root cause if known. Delete this section if you have no additional details to add.

corranrogue9 commented 5 months ago

Unfortunately, /People('russellwhyte')?$expand=Friends($select=FirstName)&$select=Friends is not a valid request because Friends, being a navigation property, cannot be used in a $select statement. A workaround could be to either $select a known value like UserName, or just directly calling /People('russellwhyte')/Friends.

uffelauesen commented 5 months ago

Sorry - I don’t think that’s correct. Your reference TripPin service on data.org accepts it’s just fine and returns a payload respecting the intended projection. Could you please check again. OData V3 client behaves correctly in this regard and issues a select part letting the server know what to return - and that is not everything.

uffelauesen commented 5 months ago

Hi @corranrogue9 According to the protocol documentation at odata.org navigation properties are perfectly valid in $select. Please check https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#_Toc31361040 where it is stated that:

_If the select item is a navigation property, then the corresponding navigation link is represented in the response. If the navigation property also appears in an $expand query option, then it is additionally represented as inline content. This inline content can itself be restricted with a nested $select query option, see section 5.1.2._

The ABNF also states that navigation properties are allowed as select property, please check https://[docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt](https://docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt). Where the $select is described as:

select = ( "$select" / "select" ) EQ selectItem ( COMMA selectItem ) selectItem = STAR / allOperationsInSchema / [ ( qualifiedEntityTypeName / qualifiedComplexTypeName ) "/" ] ( selectProperty / qualifiedActionName
/ qualifiedFunctionName
) selectProperty = primitiveProperty
/ primitiveColProperty [ OPEN selectOptionPC
( SEMI selectOptionPC ) CLOSE ] / navigationProperty / selectPath [ OPEN selectOption *( SEMI selectOption ) CLOSE / "/" selectProperty
]

Also please note that the OData Client for OData V3 did include the navigation property in the $select - correctly letting a V3 service know that it shouldn't return all primitive and complex properties for that specific path when they were not part of the projection. OData V3 client (Microsoft.Data.Services.Client 5.8.5) produces this request for the same query (on V3 the $select is not wrapped inside the $expand):

/People('russellwhyte')?$expand=Friends&$select=Friends/FirstName On OData V3 client - if we have the expanded Firends navigation property selected projectionless the request will include the navigation property in the $select letting the service know that it should return all propeties from the Friends navigation path, but none of the properties from the root path People('russelwhyte'). So a V3 query like...

            var q = from p in ctx.People
                    where p.UserName == "russellwhyte"
                    select new Person
                    {
                        Friends = p.Friends
                    };

Will produce a request like:

/People('russellwhyte')?$expand=Friends&$select=Friends/*

As the V3 protocol had all selects sitting on the root level there would be a select on the root path in such situations. The V4 protocol has the select properties nested within the $expand path. This leads to this "bug" where the root path will become projectionless in theses scenarios where the materializer clearly have it projected and all other properties than Friends on the root path is ignored.

If there are other ways to resolve this issue than including the navigation property Friends in the root $select, then that is also fine - as long as we can agree on that we have a performance issue here with queries like this. Our model have rather large entity types (having hundreds of properties) - this issue is really bad for developers doing queries like this - they have all the projections in place minimizing the response payload to only include what they actually need - but the actual payload is wrong as the projection is missing on the root path.

Thanks for looking into this issue Uffe