Allow generic dictionaries for dynamic entities #1767
Conversation
| @@ -50,7 +50,7 @@ public object Instantiate() | |||
|
|
|||
| protected virtual IDictionary GenerateMap() | |||
| { | |||
| return new Hashtable(); | |||
| return new Dictionary<string, object>(); | |||
There was a problem hiding this comment.
I think it's better to create a new type DynamicEntityInstantiator in case anyone has subclassed from that. And make this class obsolete.
And obsolete the old one To be squashed.
| public object Instantiate() | ||
| { | ||
| var map = GenerateMap(); | ||
| if (_entityName != null) |
There was a problem hiding this comment.
All NHibernate paths always assign a non-null EntityName to PersistentClass, but PersistentClass does not guarantee it will never be null. Now for the purpose here, we do not need a special handling of null. If EntityName is null, the old code would have added it to _isInstanceEntityNames nonetheless.
| { | ||
| if (!(obj is IDictionary<string, object> that)) | ||
| return false; | ||
| if (_entityName == null) |
| if (_entityName == null) | ||
| return true; | ||
|
|
||
| var type = (string) that[Key]; |
There was a problem hiding this comment.
That could fail. There is a difference between IDictionary and IDictionary<,>:
var dictionary = new Dictionary<string, object>();
IDictionary a = dictionary;
var x = a["SomeThing"]; // x is null
IDictionary<string, object> b = dictionary;
var y = b["SomeThing"]; // KeyNotFoundExceptionThere was a problem hiding this comment.
Yes my bad, forgot about it.
| return true; | ||
|
|
||
| var type = (string) that[Key]; | ||
| return type == null || _isInstanceEntityNames.Contains(type); |
There was a problem hiding this comment.
This condition is likely to be incorrect. In my mind: if type == null then this dictionary is not an instance of the entity type.
Coded too fast... To be squashed.
|
I just realised that dynamic entities are not queryable by Linq. |
|
This change was done for criteria and HQL, as showcased by the added tests. Does the lack of Linq support have any impact on what should have been done? |
|
No, it’s all fine. |
Follow-up to #755
Generic dictionaries could already be used for creating dynamic entities, but not for working with dynamic entities loaded from the database.
Possible breaking change: querying a dynamic entity as a
Hashtableinstead of anIDictionaryis no more supported.