Repository: digitalocean/pynetbox
Pull request #266 (2020-07-07 18:19:31)

Fixes 242: Copy local_context_data as-is in Record._add_cache()
In #242 the problem seems to be that `local_context_data` contents are not properly copied to `Record._init_cache`, so `update()` and `save()` operations fail to recognize the changed contents of `local_context_data`.

This PR adds a check that for `local_context_data` it uses `copy.deepcopy()` to copy the dict contents in the init cache (and all other fields are handled as earlier). `local_context_data` only contains user-entered data so no specific attribute checks should be performed for it.

With this change the test code in https://github.com/digitalocean/pynetbox/issues/242#issuecomment-654988849 works and saves the `local_context_data` change.

@zachmoody feel free to evaluate if some other way to resolve the issue is more correct. Btw, there is an unused `Record._compare()` method but I didn't touch it yet. I can remove it as well if needed. (Update: oops `_compare()` seems to be used in tests I see)


Patch:
diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py
index 5285685..29e124d 100644
--- a/pynetbox/core/response.py
+++ b/pynetbox/core/response.py
@@ -13,6 +13,8 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 """
+import copy
+
 import pynetbox.core.app
 from six.moves.urllib.parse import urlsplit
 from pynetbox.core.query import Request
@@ -235,7 +237,10 @@ class Record(object):
 
     def _add_cache(self, item):
         key, value = item
-        self._init_cache.append((key, get_return(value)))
+        if key == "local_context_data":
+            self._init_cache.append((key, copy.deepcopy(value)))
+        else:
+            self._init_cache.append((key, get_return(value)))
 
     def _parse_values(self, values):
         """ Parses values init arg.
@@ -252,7 +257,9 @@ class Record(object):
         for k, v in values.items():
             if isinstance(v, dict):
                 lookup = getattr(self.__class__, k, None)
-                if k == "custom_fields" or hasattr(lookup, "_json_field"):
+                if k in ["custom_fields", "local_context_data"] or hasattr(
+                    lookup, "_json_field"
+                ):
                     self._add_cache((k, v.copy()))
                     setattr(self, k, v)
                     continue
@@ -275,20 +282,6 @@ class Record(object):
         app, name = urlsplit(url).path.split("/")[2:4]
         return getattr(pynetbox.core.app.App(self.api, app), name)
 
-    def _compare(self):
-        """Compares current attributes to values at instantiation.
-
-        In order to be idempotent we run this method in `save()`.
-
-        Returns:
-            Boolean value, True indicates current instance has the same
-            attributes as the ones passed to `values`.
-        """
-
-        if self.serialize(init=True) == self.serialize():
-            return True
-        return False
-
     def full_details(self):
         """Queries the hyperlinked endpoint if 'url' is defined.
 


Test patch:
diff --git a/tests/test_dcim.py b/tests/test_dcim.py
index 47119ea..3403f93 100644
--- a/tests/test_dcim.py
+++ b/tests/test_dcim.py
@@ -111,7 +111,7 @@ class Generic(object):
                     headers=HEADERS,
                 )
 
-        def test_compare(self):
+        def test_diff(self):
             with patch(
                 "pynetbox.core.query.requests.sessions.Session.get",
                 return_value=Response(
@@ -120,7 +120,7 @@ class Generic(object):
             ):
                 ret = getattr(nb, self.name).get(1)
                 self.assertTrue(ret)
-                self.assertTrue(ret._compare())
+                self.assertEqual(ret._diff(), set())
 
         def test_serialize(self):
             with patch(
@@ -186,7 +186,7 @@ class DeviceTestCase(Generic.Tests):
         ret.serial = "123123123123"
         ret_serialized = ret.serialize()
         self.assertTrue(ret_serialized)
-        self.assertFalse(ret._compare())
+        self.assertEqual(ret._diff(), {"serial"})
         self.assertEqual(ret_serialized["serial"], "123123123123")
 
     @patch(
@@ -264,7 +264,7 @@ class SiteTestCase(Generic.Tests):
         """
         ret = getattr(nb, self.name).get(1)
         ret.custom_fields["test_custom"] = "Testing"
-        self.assertFalse(ret._compare())
+        self.assertEqual(ret._diff(), {"custom_fields"})
         self.assertTrue(ret.serialize())
         self.assertEqual(ret.custom_fields["test_custom"], "Testing")
 
diff --git a/tests/test_ipam.py b/tests/test_ipam.py
index 8cc1200..050c88e 100644
--- a/tests/test_ipam.py
+++ b/tests/test_ipam.py
@@ -107,7 +107,7 @@ class PrefixTestCase(Generic.Tests):
         ret.prefix = "10.1.2.0/24"
         ret_serialized = ret.serialize()
         self.assertTrue(ret_serialized)
-        self.assertFalse(ret._compare())
+        self.assertEqual(ret._diff(), {"prefix"})
         self.assertEqual(ret_serialized["prefix"], "10.1.2.0/24")
 
     @patch(
@@ -216,7 +216,7 @@ class IPAddressTestCase(Generic.Tests):
         ret.description = "testing"
         ret_serialized = ret.serialize()
         self.assertTrue(ret_serialized)
-        self.assertFalse(ret._compare())
+        self.assertEqual(ret._diff(), {"description"})
         self.assertEqual(ret_serialized["address"], "10.0.255.1/32")
         self.assertEqual(ret_serialized["description"], "testing")
 
diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py
index c93452c..f37d430 100644
--- a/tests/unit/test_response.py
+++ b/tests/unit/test_response.py
@@ -55,12 +55,16 @@ class RecordTestCase(unittest.TestCase):
             "nested_dict": {"id": 222, "name": "bar"},
             "tags": ["foo", "bar"],
             "int_list": [123, 321, 231],
+            "local_context_data": {"data": ["one"]},
         }
         test = Record(test_values, None, None)
         test.tags.append("baz")
         test.nested_dict = 1
         test.string_field = "foobaz"
-        self.assertEqual(test._diff(), {"tags", "nested_dict", "string_field"})
+        test.local_context_data["data"].append("two")
+        self.assertEqual(
+            test._diff(), {"tags", "nested_dict", "string_field", "local_context_data"}
+        )
 
     def test_diff_append_records_list(self):
         test_values = {