Address peer review and add more unit tests

Author: jhonabreul

src/embed_tests/TestPropertyAccess.cs

@@ -1035,6 +1035,34 @@ def GetValue(self, fixture):
             }
         }
 
+        [Test]
+        public void TestGetNullPublicDynamicObjectPropertyWorks()
+        {
+            dynamic model = PyModule.FromString("module", @"
+from clr import AddReference
+AddReference(""Python.EmbeddingTest"")
+AddReference(""System"")
+
+from Python.EmbeddingTest import *
+
+class TestGetNullPublicDynamicObjectPropertyWorks:
+    def GetValue(self, fixture):
+        return fixture.DynamicProperty
+
+    def IsNone(self, fixture):
+        return fixture.DynamicProperty is None
+").GetAttr("TestGetNullPublicDynamicObjectPropertyWorks").Invoke();
+
+            dynamic fixture = new DynamicFixture();
+            fixture.DynamicProperty = null;
+
+            using (Py.GIL())
+            {
+                Assert.IsNull(model.GetValue(fixture));
+                Assert.IsTrue(model.IsNone(fixture).As<bool>());
+            }
+        }
+
         [Test]
         public void TestGetNonExistingPublicDynamicObjectPropertyThrows()
         {
@@ -1115,6 +1143,33 @@ def GetPythonValue(self):
             }
         }
 
+        [Test]
+        public void TestSetNullPublicDynamicObjectPropertyWorks()
+        {
+            dynamic model = PyModule.FromString("module", $@"
+from clr import AddReference
+AddReference(""Python.EmbeddingTest"")
+AddReference(""System"")
+
+from datetime import datetime
+import System
+from Python.EmbeddingTest import *
+
+class TestSetNullPublicDynamicObjectPropertyWorks:
+    def SetValue(self, fixture):
+        fixture.DynamicProperty = None
+").GetAttr("TestSetNullPublicDynamicObjectPropertyWorks").Invoke();
+
+            dynamic fixture = new DynamicFixture();
+
+            using (Py.GIL())
+            {
+                model.SetValue(fixture);
+
+                Assert.IsTrue(fixture.DynamicProperty.IsNone());
+            }
+        }
+
         [Test]
         public void TestSetPublicNonDynamicObjectPropertyToActualPropertyWorks()
         {
@@ -1128,10 +1183,10 @@ from datetime import datetime
 import System
 from Python.EmbeddingTest import *
 
-class TestGetPublicDynamicObjectPropertyWorks:
+class TestSetPublicNonDynamicObjectPropertyToActualPropertyWorks:
     def SetValue(self, fixture):
         fixture.NonDynamicProperty = ""{expected}""
-").GetAttr("TestGetPublicDynamicObjectPropertyWorks").Invoke();
+").GetAttr("TestSetPublicNonDynamicObjectPropertyToActualPropertyWorks").Invoke();
 
             var fixture = new DynamicFixture();
 

src/runtime/Types/DynamicClassObject.cs

@@ -1,11 +1,8 @@
 using System;
 using System.Collections.Generic;
 using System.Dynamic;
-using System.Linq;
 using System.Runtime.CompilerServices;
 
-using Fasterflect;
-
 using RuntimeBinder = Microsoft.CSharp.RuntimeBinder;
 
 namespace Python.Runtime
@@ -23,12 +20,12 @@ internal DynamicClassObject(Type tp) : base(tp)
         {
         }
 
-        private static Dictionary<Tuple<Type, string>, CallSite<Func<CallSite, object, object>>> _getAttrCallSites = new();
-        private static Dictionary<Tuple<Type, string>, CallSite<Func<CallSite, object, object, object>>> _setAttrCallSites = new();
+        private static Dictionary<ValueTuple<Type, string>, CallSite<Func<CallSite, object, object>>> _getAttrCallSites = new();
+        private static Dictionary<ValueTuple<Type, string>, CallSite<Func<CallSite, object, object, object>>> _setAttrCallSites = new();
 
         private static CallSite<Func<CallSite, object, object>> GetAttrCallSite(string name, Type objectType)
         {
-            var key = Tuple.Create(objectType, name);
+            var key = ValueTuple.Create(objectType, name);
             if (!_getAttrCallSites.TryGetValue(key, out var callSite))
             {
                 var binder = RuntimeBinder.Binder.GetMember(
@@ -45,7 +42,7 @@ private static CallSite<Func<CallSite, object, object>> GetAttrCallSite(string n
 
         private static CallSite<Func<CallSite, object, object, object>> SetAttrCallSite(string name, Type objectType)
         {
-            var key = Tuple.Create(objectType, name);
+            var key = ValueTuple.Create(objectType, name);
             if (!_setAttrCallSites.TryGetValue(key, out var callSite))
             {
                 var binder = RuntimeBinder.Binder.SetMember(
@@ -111,15 +108,16 @@ public static int tp_setattro(BorrowedReference ob, BorrowedReference key, Borro
             var name = Runtime.GetManagedString(key);
 
             // If the key corresponds to a member of the class, we let the default implementation handle it.
-            if (clrObj.inst.GetType().GetMember(name).Length != 0)
+            var clrObjectType = clrObj.inst.GetType();
+            if (clrObjectType.GetMember(name).Length != 0)
             {
                 return Runtime.PyObject_GenericSetAttr(ob, key, val);
             }
 
             // If the value is a managed object, we get it from the reference. If it is a Python object, we assign it as is.
             var value = ((CLRObject)GetManagedObject(val))?.inst ?? PyObject.FromNullableReference(val);
 
-            var callsite = SetAttrCallSite(name, clrObj.inst.GetType());
+            var callsite = SetAttrCallSite(name, clrObjectType);
             callsite.Target(callsite, clrObj.inst, value);
 
             return 0;