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 8.2.1 does not materialize data service responses #3124

Open
dotnetero opened this issue Nov 18, 2024 · 7 comments
Open

OData.Client 8.2.1 does not materialize data service responses #3124

dotnetero opened this issue Nov 18, 2024 · 7 comments

Comments

@dotnetero
Copy link

Assemblies affected

  • Microsoft.OData.Client 8.2.1
  • Microsoft.OData.Core 8.2.1
  • Microsoft.OData.Edm 8.2.1
  • Microsoft.Spatial 8.2.1

Other libraries used in the project:

  • Microsoft.EntityFrameworkCore 9.0.0
  • Microsoft.EntityFrameworkCore.Design 9.0.0
  • Microsoft.EntityFrameworkCore.SqlServer 9.0.0
  • Microsoft.EntityFrameworkCore.Tools 9.0.0
  • Microsoft.AspNetCore.OData 9.1.0

Describe the bug

In all requests made by using a unit of work that encapsulates a DataServiceContext, it is not possible to materialize the results, obtaining the same error in all methods "Value cannot be null (Parameter 'key')".

Multiple client applications were programmed on Blazor Server, Wpf Core, among others. These must consume multiple services made with AspNetCore.OData, prior to updating to the assemblies mentioned above, everything worked fine, at this time the calls to the OData services do not materialize.
AspNetCore.OData services work well and return results correctly.

Reproduce steps

  • Create an API with AspNetCore.OData, which consumes data from a database in Sql Server with or without primary keys.
  • Use the OData Connected Service extension to read the metadata and create the class for consumption in client applications.
  • Make a request to the data service, and it should throw the error mentioned above.

Data Model

image

EDM (CSDL) Model

image

Request/Response

Request:
image

Response:
The following image shows that the data service adequately responds to all methods:
image
The response it´s empty on blazor server app or Wpf app.

Expected behavior

Whether the data service returns IQueryable, Enumerable, List, etc. It is expected to be able to materialize these results for consumption in client applications.

Screenshots
image
image
image
image

Additional context

If you use Microsoft.OData.Client 8.1.0, everything works as expected. Leaving client applications with Net 9 and data services with AspNetCore.Odata 9.1.0 and EntityFrameworkCore 9.0.0.

@WanjohiSammy
Copy link
Contributor

@dotnetero Thanks for reporting this.

In the look of the exception thrown, it seems like the issue is on Microsoft.OData.Client and not Microsoft.AspNetCore.OData 9.1.0.

Could you kindly share a CSDL that can help us repro this issue. The one you shared above is not sufficient to repro the issue. It seems like Expediente, Delito, and Actuacion are all Complex Types and have 2 properties which differ from the response you shared above.

@habbes
Copy link
Contributor

habbes commented Nov 19, 2024

Tried OData client 8.2.1 and 8.2.2 against the TrippinService and it returned expected resoults. So there could be something else related to this specific scenario that causes things to break.

@habbes
Copy link
Contributor

habbes commented Nov 19, 2024

The code that you have shared is supposed to be called when the object is an entity type:

// inside ObjectMaterializerLog
   internal void CreatedInstance(MaterializerEntry entry)
   {
       // omitted for brevity

       if (IsEntity(entry) && entry.IsTracking && !entry.Entry.IsTransient)
       {
           this.identityStack.Add(entry.Id, entry.Entry);
           if (this.mergeOption == MergeOption.AppendOnly)
           {
               this.appendOnlyEntries.Add(entry.Id, entry.Entry);
           }
       }
   }

But in your model, Expediente is a complex type, not an entity type. I think the client is incorrectly treating this complex type like an entity type because it has an Id field. In the past we used to detect ID fields as key properties, but I think we recently merged a fix that auto-detected Id as a key property. I think the client could be treating the Poco as entity if it has what is detected as a key property even if the server model. The client generates a ClientEdmModel based on the client-side CLR types, separately from the server model.

Here's an excerpt of ClientEdmModel.GetOrCreateEdmType, it decides whether a CLR type is an entity type based on whether or not it contains key properties (which may have been auto-detected) without consulting the server model:

PropertyInfo[] keyProperties;
bool hasProperties;
Type[] hierarchy = ClientEdmModel.GetTypeHierarchy(type, out keyProperties, out hasProperties);

Debug.Assert(keyProperties == null || keyProperties.Length == 0 || keyProperties.All(p => p.DeclaringType == keyProperties[0].DeclaringType), "All key properties must be declared on the same type.");

bool isEntity = keyProperties != null;
keyProperties = keyProperties ?? ClientTypeUtil.EmptyPropertyInfoArray;
foreach (Type t in hierarchy)
{
    // Pass in the full list of key properties for the most base type to be added there.  We only allow key properties to be
    // declared on the same type.
    IEdmStructuredType edmBaseType = cachedEdmType == null ? null : cachedEdmType.EdmType as IEdmStructuredType;
    cachedEdmType = this.GetOrCreateEdmTypeInternal(edmBaseType, t, keyProperties, isEntity, t == type ? hasProperties : (bool?)null);

    // Pass in an empty PropertyInfo array on subsequent derived types.
    keyProperties = ClientTypeUtil.EmptyPropertyInfoArray;
}

I found an old issue that predicted this problem: #177
I think it went so long undetected because ID fields are more rare compared to Id for complex types (none of which is recommended)

Another issue that asks whether the ClientEdmModel is a good or necessary idea: #2321

@dotnetero
Copy link
Author

Tried OData client 8.2.1 and 8.2.2 against the TrippinService and it returned expected resoults. So there could be something else related to this specific scenario that causes things to break.

https://1drv.ms/u/c/97eaa7be3d6c4d3e/EZtyIUzhfDpPvSn0oZJun_QBbb6cWVmLgPe4IAtyBQthqg?e=AaN4rd

Greetings @WanjohiSammy , sorry for the delay, there is some work, I hope the project I share, will allow you to get the necessary information to get to the problem of complex types with a property called Id.

@dotnetero
Copy link
Author

@dotnetero Thanks for reporting this.

In the look of the exception thrown, it seems like the issue is on Microsoft.OData.Client and not Microsoft.AspNetCore.OData 9.1.0.

Could you kindly share a CSDL that can help us repro this issue. The one you shared above is not sufficient to repro the issue. It seems like Expediente, Delito, and Actuacion are all Complex Types and have 2 properties which differ from the response you shared above.

They have plans to no longer allow Id(ID) or ClassName + Id(ID) name properties in complex types, by modifying projects and removing such properties, everything works fine.
If they could share with the community if there will no longer be support for those complex types with such properties.

I add sample code, so you can observe the problem:
https://1drv.ms/u/c/97eaa7be3d6c4d3e/EZtyIUzhfDpPvSn0oZJun_QBbb6cWVmLgPe4IAtyBQthqg?e=AaN4rd

@WanjohiSammy
Copy link
Contributor

@dotnetero
Copy link
Author

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

No branches or pull requests

3 participants