From 027b3de8c61b2b8f967a0b823dfe00585f451fd2 Mon Sep 17 00:00:00 2001 From: Brian Curtin Date: Sun, 29 Mar 2015 12:04:28 -0500 Subject: [PATCH] Rework update_attrs so dirty list always updated By bypassing Resource's MutableMapping methods and setting values directly on the underlying _attrs dictionary, we were missing out on properly keeping the dirty list up to date. Without this change, calling update often results in no update request being made because the attributes were set but didn't trip the dirty list handling that comes from Resource.__setitem__. This change unifies the handling for both *args and **kwargs that get passed in, and handles both cases when response body values are passed in (during creation, through self[key]) and when actual property names are used (e.g., during update calls in the proxies). Additionally this includes some adjustments to tests that were taking advantage of the previous incorrect way. Change-Id: Ifc6fa1498d052f8b03cac92cc976b86c68c8fa3b --- openstack/resource.py | 8 ++--- openstack/tests/unit/test_resource.py | 35 ++++++++++++------- openstack/tests/unit/volume/v2/test_volume.py | 2 +- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/openstack/resource.py b/openstack/resource.py index cc17eb89..3d911159 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -31,6 +31,7 @@ There are plenty of examples of use of this class in the SDK code. import abc import collections +import itertools import six from six.moves.urllib import parse as url_parse @@ -434,13 +435,10 @@ class Resource(collections.MutableMapping): :rtype: None """ # ensure setters are called for type coercion - for key, value in dict(*args).items(): + for key, value in itertools.chain(dict(*args).items(), kwargs.items()): if key != "id": # id property is read only - self._attrs[key] = value setattr(self, key, value) - - for key, value in kwargs.items(): - setattr(self, key, value) + self[key] = value def get_headers(self): if HEADERS in self._attrs: diff --git a/openstack/tests/unit/test_resource.py b/openstack/tests/unit/test_resource.py index 698ead5c..19918539 100644 --- a/openstack/tests/unit/test_resource.py +++ b/openstack/tests/unit/test_resource.py @@ -178,11 +178,10 @@ class PropTests(base.TestCase): attr = resource.prop("something", alias="attr") val = "hey" - args = {"attr": val} + args = {"something": val} sot = Test(args) self.assertEqual(val, sot._attrs["something"]) - self.assertIsNone(sot._attrs.get("attr")) self.assertEqual(val, sot.attr) @@ -523,13 +522,13 @@ class ResourceTests(base.TestTransportBase): FakeResource.resource_key] self.assertEqual(4, len(last_req)) - self.assertEqual('True', last_req['enabled']) + self.assertTrue(last_req['enabled']) self.assertEqual(fake_name, last_req['name']) self.assertEqual(fake_attr1, last_req['attr1']) self.assertEqual(fake_attr2, last_req['attr2']) self.assertEqual(fake_id, obj.id) - self.assertEqual('True', obj['enabled']) + self.assertTrue(obj['enabled']) self.assertEqual(fake_name, obj['name']) self.assertEqual(fake_attr1, obj['attr1']) self.assertEqual(fake_attr2, obj['attr2']) @@ -879,23 +878,35 @@ class ResourceTests(base.TestTransportBase): moe = resource.prop("the-attr") larry = resource.prop("the-attr2") curly = resource.prop("the-attr3", type=int) + shemp = resource.prop("the-attr4") value1 = "one" value2 = "two" value3 = "3" value4 = "fore" + value5 = "fiver" sot = Test({"the-attr": value1}) sot.update_attrs({"the-attr2": value2, "notprop": value4}) - sot.update_attrs(curly=value3) - + self.assertTrue(sot.is_dirty) self.assertEqual(value1, sot.moe) self.assertEqual(value1, sot["the-attr"]) self.assertEqual(value2, sot.larry) + self.assertEqual(value4, sot.notprop) + + sot._reset_dirty() + + sot.update_attrs(curly=value3) + self.assertTrue(sot.is_dirty) self.assertEqual(int, type(sot.curly)) self.assertEqual(int(value3), sot.curly) - self.assertEqual(value4, sot["notprop"]) + + sot._reset_dirty() + + sot.update_attrs(**{"the-attr4": value5}) + self.assertTrue(sot.is_dirty) + self.assertEqual(value5, sot.shemp) def test_get_id(self): class Test(resource.Resource): @@ -951,12 +962,12 @@ class ResourceTests(base.TestTransportBase): def test_boolstr_prop(self): faker = FakeResource(fake_data) - self.assertEqual(True, faker.enabled) - self.assertEqual('True', faker['enabled']) + self.assertTrue(faker.enabled) + self.assertTrue(faker['enabled']) - faker.enabled = False - self.assertEqual(False, faker.enabled) - self.assertEqual('False', faker['enabled']) + faker._attrs['enabled'] = False + self.assertFalse(faker.enabled) + self.assertFalse(faker['enabled']) # should fail fast def set_invalid(): diff --git a/openstack/tests/unit/volume/v2/test_volume.py b/openstack/tests/unit/volume/v2/test_volume.py index 2407726b..8db03307 100644 --- a/openstack/tests/unit/volume/v2/test_volume.py +++ b/openstack/tests/unit/volume/v2/test_volume.py @@ -74,7 +74,7 @@ class TestVolume(testtools.TestCase): self.assertEqual(VOLUME["bootable"], sot.bootable) self.assertEqual(VOLUME["created_at"], sot.created) self.assertEqual(VOLUME["description"], sot.description) - self.assertEqual(VOLUME["volume_type"], sot.volume_type) + self.assertEqual(VOLUME["volume_type"], sot.type) self.assertEqual(VOLUME["snapshot_id"], sot.snapshot) self.assertEqual(VOLUME["source_volid"], sot.source_volume) self.assertEqual(VOLUME["metadata"], sot.metadata)