openstack/deb-python-openstacksdk
Code Issues Proposed changes

Allow alternate_id to be accessed directly

Browse Source 
When a Resource has an alternate_id but not a primary id, its usage is a
bit inconsistent. While we make things work internally, such as when you
do a Resource.get call where we find the alternate_id, you still can't
just call foo.id and get a valid ID back.

The Keypair resource in compute is one of these examples, and its
functional tests are currently failing because they depend on
passing in "sot.id". While they could be engineered around that,
it's certainly a valid usecase to support, and this change
enables it. It does so by implementing __getattribute__ on the Resource
class and then special-casing a request for the "id" name. First we try
to get the primary "id" directly, and if nothing happens there then we
try to get an alternate_id.

Additionally, this changes the special case in the Resource.create
method that was handling any create call with an existing ID as
requiring that it go through the PUT branch when id exists. Relying on
the id value itself being set for determining that branching is
problematic in that these alternate_id resources would then trigger that
path, sending creates which must happen by POST into the PUT branch just
because their alternate_id becomes additionally known as the canonical
id. Instead, we now have put_create as the special case, which is a
fairly rare case as it stands. At least object_store will need to have
its resources set with that flag once it is converted over, though there
may be others as well.

Change-Id: Ia1fd1782f4ad8fa856c16d75bba4d8931307a6e7
This commit is contained in:
Brian Curtin
2016-06-29 14:19:54 -04:00
parent 2b63c43a3c
commit 586d9afdab
2 changed files with 54 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
openstack/resource2.py
View File

@@ -243,6 +243,8 @@ class Resource(object):
allow_head = False
#: Use PATCH for update operations on this resource.
patch_update = False
#: Use PUT for create operations on this resource.
put_create = False
def __init__(self, synchronized=False, **attrs):
# NOTE: _collect_attrs modifies **attrs in place, removing
@@ -276,6 +278,21 @@ class Resource(object):
self._header.attributes == comparand._header.attributes,
self._uri.attributes == comparand._uri.attributes])
def __getattribute__(self, name):
"""Return an attribute on this instance
This is mostly a pass-through except for a specialization on
the 'id' name, as this can exist under a different name via the
`alternate_id` argument to resource.Body.
"""
if name == "id":
if name in self._body:
return self._body[name]
else:
return self._body[self._alternate_id()]
else:
return object.__getattribute__(self, name)
def _update(self, **attrs):
"""Given attributes, update them on this instance
@@ -495,16 +512,16 @@ class Resource(object):
if not self.allow_create:
raise exceptions.MethodNotSupported(self, "create")
if self.id is None:
request = self._prepare_request(requires_id=False,
prepend_key=True)
response = session.post(request.uri, endpoint_filter=self.service,
json=request.body, headers=request.headers)
else:
if self.put_create:
request = self._prepare_request(requires_id=True,
prepend_key=True)
response = session.put(request.uri, endpoint_filter=self.service,
json=request.body, headers=request.headers)
else:
request = self._prepare_request(requires_id=False,
prepend_key=True)
response = session.post(request.uri, endpoint_filter=self.service,
json=request.body, headers=request.headers)
self._translate_response(response)
return self

42
openstack/tests/unit/test_resource2.py
View File

@@ -409,6 +409,7 @@ class TestResource(base.TestCase):
self.assertFalse(sot.allow_list)
self.assertFalse(sot.allow_head)
self.assertFalse(sot.patch_update)
self.assertFalse(sot.put_create)
def test_repr(self):
a = {"a": 1}
@@ -576,6 +577,11 @@ class TestResource(base.TestCase):
self.assertTrue("alt", Test._alternate_id())
value = "lol"
sot = Test(alt=value)
self.assertEqual(sot.alt, value)
self.assertEqual(sot.id, value)
def test__get_id_instance(self):
class Test(resource2.Resource):
id = resource2.Body("id")
@@ -800,13 +806,15 @@ class TestResourceActions(base.TestCase):
self.session.delete = mock.Mock(return_value=self.response)
self.session.head = mock.Mock(return_value=self.response)
def _test_create(self, requires_id=False, prepend_key=False):
if not requires_id:
self.sot.id = None
def _test_create(self, cls, requires_id=False, prepend_key=False):
id = "id" if requires_id else None
sot = cls(id=id)
sot._prepare_request = mock.Mock(return_value=self.request)
sot._translate_response = mock.Mock()
result = self.sot.create(self.session)
result = sot.create(self.session)
self.sot._prepare_request.assert_called_once_with(
sot._prepare_request.assert_called_once_with(
requires_id=requires_id, prepend_key=prepend_key)
if requires_id:
self.session.put.assert_called_once_with(
@@ -819,14 +827,26 @@ class TestResourceActions(base.TestCase):
endpoint_filter=self.service_name,
json=self.request.body, headers=self.request.headers)
self.sot._translate_response.assert_called_once_with(self.response)
self.assertEqual(result, self.sot)
sot._translate_response.assert_called_once_with(self.response)
self.assertEqual(result, sot)
def test_create_with_id(self):
self._test_create(requires_id=True, prepend_key=True)
def test_put_create(self):
class Test(resource2.Resource):
service = self.service_name
base_path = self.base_path
allow_create = True
put_create = True
def test_create_without_id(self):
self._test_create(requires_id=False, prepend_key=True)
self._test_create(Test, requires_id=True, prepend_key=True)
def test_post_create(self):
class Test(resource2.Resource):
service = self.service_name
base_path = self.base_path
allow_create = True
put_create = False
self._test_create(Test, requires_id=False, prepend_key=True)
def test_get(self):
result = self.sot.get(self.session)