Fixes #1412: $select query nested inside $expand was being ignored.

[anasik]

Fixes #1412

How I reproduced the issue.

I was able to recreate the issue using the `ODataDynamicModel` sample in the repository. I went to the following endpoint:

http://localhost:5000/odata/mydatasource/Products?$expand=ContainedDetailInfo($select=ID)

And saw this:

{
  "@odata.context": "http://localhost:5001/odata/mydatasource/$metadata#Products(ContainedDetailInfo(ID))",
  "value": [
    {
      "Name": "abc",
      "ID": 1,
      "ContainedDetailInfo": {
        "ID": 88,
        "Title": "abc_containeddetailinfo"
      }
    },
    {
      "Name": "def",
      "ID": 2,
      "ContainedDetailInfo": {
        "ID": 99,
        "Title": "def_containeddetailinfo"
      }
    }
  ]
}

When instead I should have been seeing this:

{
  "@odata.context": "http://localhost:5001/odata/mydatasource/$metadata#Products(ContainedDetailInfo(ID))",
  "value": [
    {
      "Name": "abc",
      "ID": 1,
      "ContainedDetailInfo": {
        "ID": 88
      }
    },
    {
      "Name": "def",
      "ID": 2,
      "ContainedDetailInfo": {
        "ID": 99
      }
    }
  ]
}

I was able to isolate the problem to commit a852d91 with parent d0b2f76.

One key difference was that the parent commit was using WriteComplexAndExpandedNavigationPropertyAsync for the expanded property whereas the child commit was using WriteDeltaComplexAndExpandedNavigationPropertyAsync, which I assume was intentional since that commit was literally titled "Fix deltas".

The WriteDeltaNavigationPropertiesAsync function, when calling WriteDeltaComplexAndExpandedNavigationPropertyAsync, was not passing in a SelectItem which is needed to create the nested ODataSerializerContext.

Initially, I modified the WriteDeltaNavigationPropertiesAsync to extract the ExpandedNavigationSelectItem from selectExpandNode.ExpandedProperties and pass it down to WriteDeltaComplexAndExpandedNavigationPropertyAsync.

While this fixed the main issue, it didn't solve the problem where navigations were auto-expanding.

Then I tried to see why the delta route was being taken altogether and see if I could fix that instead. Since this was a dynamic model and EdmEntityObject indirectly implements both IDelta and IEdmChangedObject, I decided to compare the generic type instead.

I ran every possible test that ran on my machine both before and after. An equal number of tests passed so I presume this doesn't break anything.

[anasik]

@gathogojr, I just added the tests for reproducing the bugs from the issue.

[anasik]

@WanjohiSammy, could you please take a look?

[gathogojr]

@anasik I'll illustrate my concern with this change by presenting a typeless scenario where the current logic does not correctly express the intended state.

Consider the following contrived OData sample service:

Edm Model

var model = new EdmModel();

var orderEntityType = model.AddEntityType("ns", "Order");
var orderIdProperty = orderEntityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32);
orderEntityType.AddKeys(orderIdProperty);
orderEntityType.AddStructuralProperty("Amount", EdmPrimitiveTypeKind.Decimal);

var entityContainer = model.AddEntityContainer("Default", "Container");
entityContainer.AddEntitySet("Orders", orderEntityType);

Service Configuration

builder.Services.AddControllers().AddOData(
    options => options.EnableQueryFeatures().AddRouteComponents("delta", model));

Controller

public class DeltaOrdersController : ODataController
{
    [HttpGet("delta/Orders")]
    public EdmChangedObjectCollection Get()
    {
        var model = Request.ODataFeature().Model;
        var orderEntityType = model.SchemaElements.SingleOrDefault(d => d.Name.Equals("Order")) as IEdmEntityType;

        var changedOrders = new EdmChangedObjectCollection(orderEntityType);

        var orderDeltaObject = new EdmDeltaResourceObject(orderEntityType);
        orderDeltaObject.TrySetPropertyValue("Id", 1);
        orderDeltaObject.TrySetPropertyValue("Amount", 130);

        changedOrders.Add(orderDeltaObject);

        return changedOrders;
    }
}

Observing the Behavior

If you send a GET request to the delta/Orders endpoint and place a breakpoint where your change was made, you'll see the following behavior:

The intention behind this check is to confirm whether we're writing a delta resource. While it correctly identifies Delta<T> (typed) scenarios, it does not work as expected for typeless scenarios.

The intended behavior was for the check to return true for both cases—hence the inclusion of graph is IEdmChangedObject. However, the challenge is that almost all types used in typeless scenarios implement this interface.

Background on the Issue

Originally, EdmStructuredObject (the base class for EdmEntityObject and EdmComplexObject) did not implement IEdmChangedObject. However, four years ago, a change was introduced through [this commit](ee93798) to make EdmStructuredObject implement IEdmChangedObject. This allowed both EdmEntityObject and EdmComplexObject to represent delta payloads.

As a result, EdmDeltaResourceObject and EdmDeltaComplexObject became redundant and should have been deprecated, as noted in this TODO:
[EdmDeltaResourceObject.cs#L24](https://github.com/OData/AspNetCoreOData/blame/c60d9e0e9f68d23ee8ccd84e854ef7e430fe6edb/src/Microsoft.AspNetCore.OData/Formatter/Value/EdmDeltaResourceObject.cs#L24)

Why This Is Tricky

Determining whether a typeless object represents a standard or delta resource purely by inspecting the object itself, its type, or its implemented interfaces is unreliable. Without referencing the context, the check can easily return incorrect results.

When considering the use of context to distinguish delta resources, we must also ensure that anyone overriding public methods can use it correctly. This is why we're approaching this more comprehensively—to ensure the logic is correct for all scenarios.

Let me know if this makes sense or if you'd like to discuss further.
cc. @julealgon @mikepizzo @xuzhg @ElizabethOkerio