From a949362021f24d3a7accabd4fd845f6d268392cb Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Thu, 21 Jul 2022 19:38:20 +0300 Subject: [PATCH] CORE: Allows for `Item` to be set as a dynamic or typed prop in `Base`-derived objects (#1422) fix(core): allows for `Item` to be set as a dynamic or typed prop in `Base`-derived objects + other minor cleanups: caches reflection calls & DRYs the GetProperties() calls. Regarding the `Item` prop discussions, please see this discourse thread: https://speckle.community/t/why-do-i-keep-forgetting-base-objects-cant-use-item-as-a-dynamic-member/3246/4 --- Core/Core/Models/DynamicBase.cs | 54 ++++++++++++++++++++++++--------- Core/Tests/BaseTests.cs | 24 +++++++++++++++ 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/Core/Core/Models/DynamicBase.cs b/Core/Core/Models/DynamicBase.cs index a75b2a9994..61f3e4d234 100644 --- a/Core/Core/Models/DynamicBase.cs +++ b/Core/Core/Models/DynamicBase.cs @@ -95,6 +95,7 @@ public bool IsPropNameValid(string name, out string reason) /// /// /// + [IgnoreTheItemAttribute] public object this[string key] { get @@ -102,7 +103,8 @@ public object this[string key] if (properties.ContainsKey(key)) return properties[key]; - var prop = GetType().GetProperty(key); + PopulatePropInfoCache(GetType()); + var prop = propInfoCache[GetType()].FirstOrDefault(p => p.Name == key); if (prop == null) return null; @@ -119,7 +121,8 @@ public object this[string key] return; } - var prop = this.GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public).FirstOrDefault(p => p.Name == key); + PopulatePropInfoCache(GetType()); + var prop = propInfoCache[GetType()].FirstOrDefault(p => p.Name == key); if (prop == null) { @@ -137,6 +140,16 @@ public object this[string key] } } + private static Dictionary> propInfoCache = new Dictionary>(); + + private static void PopulatePropInfoCache(Type type) + { + if(!propInfoCache.ContainsKey(type)) + { + propInfoCache[type] = type.GetProperties(BindingFlags.Instance | BindingFlags.Public).Where(p => p.GetCustomAttribute(typeof(IgnoreTheItemAttribute)) == null).ToList(); + } + } + /// /// Gets all of the property names on this class, dynamic or not. /// @@ -146,10 +159,10 @@ public override IEnumerable GetDynamicMemberNames() var names = new List(); foreach (var kvp in properties) names.Add(kvp.Key); - var pinfos = GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public); + //var pinfos = GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public); + PopulatePropInfoCache(GetType()); + var pinfos = propInfoCache[GetType()]; foreach (var pinfo in pinfos) names.Add(pinfo.Name); - - names.Remove("Item"); // TODO: investigate why we get Item out? return names; } @@ -160,10 +173,10 @@ public override IEnumerable GetDynamicMemberNames() public IEnumerable GetInstanceMembersNames() { var names = new List(); - var pinfos = GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public); + PopulatePropInfoCache(GetType()); + var pinfos = propInfoCache[GetType()]; foreach (var pinfo in pinfos) names.Add(pinfo.Name); - names.Remove("Item"); // TODO: investigate why we get Item out? return names; } @@ -174,7 +187,8 @@ public IEnumerable GetInstanceMembersNames() public IEnumerable GetInstanceMembers() { var names = new List(); - var pinfos = GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public); + PopulatePropInfoCache(GetType()); + var pinfos = propInfoCache[GetType()]; foreach (var pinfo in pinfos) if (pinfo.Name != "Item") names.Add(pinfo); @@ -191,11 +205,10 @@ public IEnumerable GetMemberNames() var names = new List(); foreach (var kvp in properties) names.Add(kvp.Key); - var pinfos = GetType() - .GetProperties(BindingFlags.Instance | BindingFlags.Public) - .Where(x => x.GetCustomAttribute(typeof(SchemaIgnore)) == null - && x.GetCustomAttribute(typeof(ObsoleteAttribute)) == null - && x.Name != "Item"); + PopulatePropInfoCache(GetType()); + var pinfos = propInfoCache[GetType()].Where(x => x.GetCustomAttribute(typeof(SchemaIgnore)) == null + && x.GetCustomAttribute(typeof(ObsoleteAttribute)) == null); + foreach (var pinfo in pinfos) names.Add(pinfo.Name); return names; @@ -209,10 +222,15 @@ public Dictionary GetMembers() { //typed members var dic = new Dictionary(); - var pinfos = GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public) - .Where(x => x.GetCustomAttribute(typeof(SchemaIgnore)) == null && x.GetCustomAttribute(typeof(ObsoleteAttribute)) == null && x.Name != "Item"); + + PopulatePropInfoCache(GetType()); + var pinfos = propInfoCache[GetType()].Where(x => x.GetCustomAttribute(typeof(SchemaIgnore)) == null + && x.GetCustomAttribute(typeof(ObsoleteAttribute)) == null); + + foreach (var pi in pinfos) dic.Add(pi.Name, pi.GetValue(this)); + //dynamic members foreach (var kvp in properties) dic.Add(kvp.Key, kvp.Value); @@ -236,4 +254,10 @@ public IEnumerable GetDynamicMembers() } + /// + /// This attribute is used internally to hide the this[key]{get; set;} property from inner reflection on members. + /// For more info see this discussion: https://speckle.community/t/why-do-i-keep-forgetting-base-objects-cant-use-item-as-a-dynamic-member/3246/5 + /// + internal class IgnoreTheItemAttribute : Attribute { } + } diff --git a/Core/Tests/BaseTests.cs b/Core/Tests/BaseTests.cs index ec3749a4eb..ff716ffa20 100755 --- a/Core/Tests/BaseTests.cs +++ b/Core/Tests/BaseTests.cs @@ -12,6 +12,25 @@ namespace Tests [TestFixture] public class BaseTests { + [Test] + public void CanGetSetDynamicItemProp() + { + var @base = new Base(); + @base["Item"] = "Item"; + + Assert.AreEqual(@base["Item"], "Item"); + } + + [Test] + public void CanGetSetTypedItemProp() + { + var @base = new ObjectWithItemProp(); + @base.Item = "baz"; + + Assert.AreEqual(@base["Item"], "baz"); + Assert.AreEqual(@base.Item, "baz"); + } + [Test(Description = "Checks if validation is performed in property names")] public void CanValidatePropNames() { @@ -160,5 +179,10 @@ public class SampleProp { public string name { get; set; } } + + public class ObjectWithItemProp : Base + { + public string Item { get; set; } = "Item"; + } } }