fix: results returned by attributes property to show updates

JohnVillalovos · JohnVillalovos · commit f229f7f3f0b8 · 2022-02-02T14:27:46.000-08:00
Previously the `attributes` method would show the original values in a
Gitlab Object even if they had been updated. Correct this so that the
updated value will be returned.

Also use copy.deepcopy() to ensure that modifying the dictionary returned can
not also modify the object.
diff --git a/docs/api-usage.rst b/docs/api-usage.rst
@@ -197,10 +197,9 @@ the dictionary will have no impact on the GitLab Object. This can also be used
 to create a JSON representation of the object. There are two ways to retrieve a
 dictionary representation of the Gitlab Object.
 
- * `asdict()` method. Returns a dictionary with updated attributes having precedence.
- * `attributes` property. Returns a dictionary with original attributes having
-   precedence and then updated attributes. Also returns any relevant parent object
-   attributes.
+ * `asdict()` method. Returns a dictionary representation of the Gitlab object.
+ * `attributes` property. Returns a dictionary representation of the Gitlab object.
+   Also returns any relevant parent object attributes.
 
 .. note::
 
diff --git a/gitlab/base.py b/gitlab/base.py
@@ -146,7 +146,8 @@ def __setattr__(self, name: str, value: Any) -> None:
         self.__dict__["_updated_attrs"][name] = value
 
     def asdict(self) -> Dict[str, Any]:
-        data = copy.deepcopy(self._attrs)
+        data = {}
+        data.update(copy.deepcopy(self._attrs))
         data.update(copy.deepcopy(self._updated_attrs))
         return data
 
@@ -231,10 +232,10 @@ def encoded_id(self) -> Optional[Union[int, str]]:
 
     @property
     def attributes(self) -> Dict[str, Any]:
-        d = self.__dict__["_updated_attrs"].copy()
-        d.update(self.__dict__["_attrs"])
-        d.update(self.__dict__["_parent_attrs"])
-        return d
+        data = {}
+        data.update(copy.deepcopy(self._parent_attrs))
+        data.update(copy.deepcopy(self.asdict()))
+        return data
 
 
 class RESTObjectList:
diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py
@@ -284,21 +284,20 @@ def test_attributes(self, fake_manager):
         result = fake_object.attributes
         assert result == {"attr1": [1, 2, 3]}
 
-        # Updated attribute value is not reflected in `attributes`
+        # Updated attribute value is reflected in `attributes`
         fake_object.attr1 = "hello"
-        assert fake_object.attributes == {"attr1": [1, 2, 3]}
+        assert fake_object.attributes == {"attr1": "hello"}
         assert fake_object.attr1 == "hello"
         # New attribute is in `attributes`
         fake_object.new_attrib = "spam"
-        assert fake_object.attributes == {"attr1": [1, 2, 3], "new_attrib": "spam"}
+        assert fake_object.attributes == {"attr1": "hello", "new_attrib": "spam"}
 
-        # Modifying the dictionary can cause modification to the object :(
+        # Modifying the dictionary does not cause modifications to the object
+        fake_object = FakeObject(fake_manager, {"attr1": [1, 2, 3]})
         result = fake_object.attributes
         result["attr1"].append(10)
-        assert result == {"attr1": [1, 2, 3, 10], "new_attrib": "spam"}
-        assert fake_object.attributes == {"attr1": [1, 2, 3, 10], "new_attrib": "spam"}
-        assert fake_object.attr1 == "hello"
-
+        assert result == {"attr1": [1, 2, 3, 10]}
+        assert fake_object.attributes == {"attr1": [1, 2, 3]}
 
     def test_asdict_vs_attributes(self, fake_manager):
         fake_object = FakeObject(fake_manager, {"attr1": "foo"})
@@ -310,10 +309,6 @@ def test_asdict_vs_attributes(self, fake_manager):
         assert fake_object.attributes == fake_object.asdict()
         fake_object.attr2 = "eggs"
         assert fake_object.attributes == fake_object.asdict()
-        # Update attribute, return different result
+        # Update attribute, return same result
         fake_object.attr1 = "hello"
-        assert fake_object.attributes != fake_object.asdict()
-        # asdict() returns the updated value
-        assert fake_object.asdict() == {"attr1": "hello", "attr2": "eggs"}
-        # `attributes` returns original value
-        assert fake_object.attributes == {"attr1": "foo", "attr2": "eggs"}
+        assert fake_object.attributes == fake_object.asdict()
