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.

Sign up for GitHub

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

Jump to bottom

OriginalNameAttribute not being captured when DataServiceQuery<T> is casted to DataServiceQuery<InterfaceT> #2856

Open
joecarl opened this issue Feb 14, 2024 · 4 comments · Fixed by #2862 · May be fixed by #3067
Open

OriginalNameAttribute not being captured when DataServiceQuery<T> is casted to DataServiceQuery<InterfaceT> #2856

joecarl opened this issue Feb 14, 2024 · 4 comments · Fixed by #2862 · May be fixed by #3067
Assignees
@habbes
Labels
investigating

Comments

@joecarl
Copy link
Contributor

joecarl commented Feb 14, 2024 •
edited
Loading

I'm getting the error described in the title when trying to resolve an OData query.

Assemblies affected

Microsoft.OData.Client 7.20.0
NetFramework 4.6.1

Reproduce steps

// Declared model and interface:

[EntitySet("TestEntities")]
public class TestModel : ITestModel
{
	[OriginalName("userId")]
	public int Id { get; set; }

	[OriginalName("user")]
	public string UserName { get; set; }	
}

public interface ITestModel
{
	int Id { get; set; }
	string UserName { get; set; }
}


// Declared context class:

public class MyContext : DataServiceContext
{
	public DataServiceQuery<TestModel> TestEntities;

	public MyContext(Uri serviceRoot) : base(serviceRoot)
	{      
		Format.UseJson();
		TestEntities = CreateQuery<TestModel>("TestEntities");
	}
}


// Main program:

var ctx = new MyContext(uri);

ctx.TestEntities.Where(c => c.Id == 1).FirstOrDefault(); // Works fine

IQueryable<ITestModel> q = ctx.TestEntities;
q.Where(c => c.Id == 1).FirstOrDefault(); // Exception is thrown here because the query is
                                          // being built using `Id` instead of `userId`

Expected result

No exception should be thrown and the query should be built using the object's original type attributtes.

Actual result

An exception is being thrown showing the following OData service error response:

{"error":{"code":"BadRequest","message":"Could not find a property named 'Id' on type 'Microsoft.NAV.testModel'...
@joecarl joecarl mentioned this issue Feb 19, 2024
Merged
2 tasks
@habbes habbes self-assigned this Feb 20, 2024
@habbes
Copy link
Contributor

habbes commented Feb 21, 2024

Hello @joecarl, this sounds like a feature request. Curious, does adding the [OriginalName] attributes to the interface properties fix the issue?

@joecarl
Copy link
Contributor Author

joecarl commented Feb 21, 2024

Yes, adding the attribute to the interface properties "fixes" the issue. Unfortunately I cannot rely on this approach because I have multiple implementations for the same model, one for Sql db, another for OData endpoints and I will eventually have even more implementations for other OData endpoints where properties names may also be different.

I already fixed this myself though and uploaded a PR #2862, did you have time to review it? (It's just one line of code)

@gathogojr
Copy link
Contributor

@joecarl I'm also curious whether adding the OriginalName attribute to the interface properties only (without adding to the classes that implement the interface) could work...

With respect to your pull request, the one line change is not enough. Tests are necessary taking into consideration the following scenarios:

  • 1 level of inheritance, e.g., interface A {} + class B : {} and class A {} +class B : A {} , with the OriginalName attribute applied at the class level - per your approach.
  • Greater-than-1 levels of inheritance, e.g., interface A {} + abstract class B : A {} + class C : B {} and interface A {} + interface B : A {} + class C : B {}, with the OriginalName attribute applied at the class level - per your approach.
  • Any other possible scenarios that can help confirm that the fix works as expected.

@joecarl
Copy link
Contributor Author

joecarl commented Feb 23, 2024

@gathogojr Yes, adding the OriginalName attribute to the interface properties only also works. I cannot rely on that approach either though.

I added tests regarding all the commented scenarios.

Also, I found another related issue. It happens only when OrderBy is used:

Exception has occurred: CLR/System.MissingMethodException
An unhandled exception of type 'System.MissingMethodException' occurred in System.Private.CoreLib.dll: 'Cannot dynamically create an instance of type 'Microsoft.OData.Client.Tests.ALinq.ICustomer'. Reason: Cannot create an instance of an interface.'
   at System.RuntimeType.ActivatorCache..ctor(RuntimeType rt)
   at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean wrapExceptions)
   at Microsoft.OData.Client.Util.ActivatorCreateInstance(Type type, Object[] arguments) in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Util.cs:line 405
   at Microsoft.OData.Client.Materialization.MaterializationPolicy.CreateNewInstance(IEdmTypeReference edmTypeReference, Type type) in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Materialization/MaterializationPolicy.cs:line 27
   at Microsoft.OData.Client.Materialization.EntryValueMaterializationPolicy.ResolveByCreatingWithType(MaterializerEntry entry, Type type) in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Materialization/EntryValueMaterializationPolicy.cs:line 339
   at Microsoft.OData.Client.Materialization.EntryValueMaterializationPolicy.Materialize(MaterializerEntry entry, Type expectedEntryType, Boolean includeLinks) in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Materialization/EntryValueMaterializationPolicy.cs:line 204
   at Microsoft.OData.Client.Materialization.ODataEntityMaterializer.DirectMaterializePlan(ODataEntityMaterializer materializer, MaterializerEntry entry, Type expectedEntryType) in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Materialization/ODataEntityMaterializer.cs:line 573
   at Microsoft.OData.Client.Materialization.ODataEntityMaterializerInvoker.DirectMaterializePlan(Object materializer, Object entry, Type expectedEntryType) in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Materialization/ODataEntityMaterializerInvoker.cs:line 175
   at Microsoft.OData.Client.ProjectionPlan.Run(ODataEntityMaterializer materializer, ODataResource entry, Type expectedType) in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/ProjectionPlan.cs:line 78
   at Microsoft.OData.Client.Materialization.ODataEntityMaterializer.ReadImplementation() in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Materialization/ODataEntityMaterializer.cs:line 927
   at Microsoft.OData.Client.Materialization.ODataMaterializer.Read() in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/Materialization/ODataMaterializer.cs:line 336
   at Microsoft.OData.Client.MaterializeAtom.MoveNextInternal() in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/MaterializeFromAtom.cs:line 349
   at Microsoft.OData.Client.MaterializeAtom.MoveNext() in /workspaces/testnet/odata.net/src/Microsoft.OData.Client/MaterializeFromAtom.cs:line 281
   at System.Linq.Enumerable.<CastIterator>d__68`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Microsoft.OData.Client.Tests.ALinq.PreserveTypesAndAttributesTests.OneLevelCase1() in /workspaces/testnet/odata.net/test/FunctionalTests/Microsoft.OData.Client.Tests/ALinq/PreserveTypesAndAttributesTests.cs:line 32
   at Program.<Main>$(String[] args) in /workspaces/testnet/tests/Program.cs:line 100

I made another one line code change that fixes the issue, but this time I'm not sure wether It's 100% appropiate or not.
It simply consist in ignoring the second parameter at:

protected override QueryableResourceExpression CreateCloneWithNewTypes(Type newType, Type newResourceType)

Please have a look at the PR. Thanks!

This was referenced Sep 19, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@habbes habbes

Labels
investigating
Projects
None yet
Milestone
No milestone
3 participants
@habbes @joecarl @gathogojr