OData / lab

This repository is for exploring new ideas and developing early prototypes of various OData stacks.
Other
48 stars 59 forks source link

ByKey takes Dictionary<string, object> rather than IDictionary<string, object> #96

Closed EricAtMS closed 4 years ago

EricAtMS commented 5 years ago

Hello, I've been working on a memory-efficient representation of a compound key and after discussing my design with my infra team, I was shown some of the work they are doing to consume generated types using this project. One initial issue I've noticed is that my type supports all of the interfaces that a ReadOnlyDictionary<string, object> supports, but the ByKey method in ODataConnectedService/src/Templates/ODataT4CodeGenerator.ttinclude takes a concrete Dictionary rather than IDictionary. I do not have much familiarity with this project to know if it is the only place where I may have an issue inter-operating with my key type, but could this logic be changed to use the IDictionary interface instead of the concrete type? Are there other such places in the code that would need to be updated to support a custom key?

KenitoInc commented 4 years ago

@odero @KanishManuja-MS @mikepizzo I have been looking into this issue of passing an IDictionary instead of a Dictionary. The main bottleneck is that the fact that the Client's Serializer.GetKeyString (Microsoft.OData.Client.Serializer.GetKeyString) receives a DataServiceContext Object and a Concrete Dictionary GetKeyString(DataServiceContext context, Dictionary<string, object> keys); We will need to update the Client Library by overloading the GetKeyString method before allowing IDictionary

mikepizzo commented 4 years ago

Since Dictionary implements IDictionary, it should be a non-breaking change to take an IDictionary<string,object> instead of a Dictionary<string,object> in the client library. Unless there is something specific in the implementation class that we rely upon, we typically try to take interfaces instead of concrete classes, so this seems like a reasonable change.

KenitoInc commented 4 years ago

Issue Fixed