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

@@ -95,14 +95,16 @@ public bool IsPropNameValid(string name, out string reason)
     /// </summary>
     /// <param name="key"></param>
     /// <returns></returns>
+    [IgnoreTheItemAttribute]
     public object this[string key]
     {
       get
       {
         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<Type, List<PropertyInfo>> propInfoCache = new Dictionary<Type, List<PropertyInfo>>();
+
+    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();
+      }
+    }
+
     /// <summary>
     /// Gets all of the property names on this class, dynamic or not.
     /// </summary>
@@ -146,10 +159,10 @@ public override IEnumerable<string> GetDynamicMemberNames()
       var names = new List<string>();
       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<string> GetDynamicMemberNames()
     public IEnumerable<string> GetInstanceMembersNames()
     {
       var names = new List<string>();
-      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<string> GetInstanceMembersNames()
     public IEnumerable<PropertyInfo> GetInstanceMembers()
     {
       var names = new List<PropertyInfo>();
-      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<string> GetMemberNames()
       var names = new List<string>();
       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<string, object> GetMembers()
     {
       //typed members
       var dic = new Dictionary<string, object>();
-      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<string> GetDynamicMembers()
 
   }
 
+  /// <summary>
+  /// 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
+  /// </summary>
+  internal class IgnoreTheItemAttribute : Attribute { }
+
 }

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";
+    }
   }
 }