Refactoring and removing duplicate code of "base.Manager" heirs

This commit does refactoring regarding the base.Manager and the
specific managers that inherit from it (Chassis, Node, Port and
Driver).

The specific managers had a lot of duplicate code in methods like:
get, create and vendor_passthru, they were literally equal, what does
not make sense once we already had the base.Manager inheritance to
avoid this type of problem.

This change moves the duplicate code to the parent (base.Manager) and
make the specific managers simpler by making the base.Manager more
useful regarding code reuse for these methods.

Change-Id: I9acef9ecc815df61d7ddabe3fae9eccdafe2eeed
Closes-Bug: #1517631
This commit is contained in:
Sinval Vieira 2015-11-17 21:20:21 -03:00
parent 5d0de8f392
commit 46b279bb36
11 changed files with 288 additions and 208 deletions

@ -17,11 +17,15 @@
Base utilities to build API operation managers and objects on top of.
"""
import abc
import copy
import six
import six.moves.urllib.parse as urlparse
from ironicclient.common.apiclient import base
from ironicclient.common.i18n import _
from ironicclient import exc
def getid(obj):
@ -36,17 +40,74 @@ def getid(obj):
return obj
@six.add_metaclass(abc.ABCMeta)
class Manager(object):
"""Provides CRUD operations with a particular API."""
resource_class = None
def __init__(self, api):
self.api = api
def _create(self, url, body):
resp, body = self.api.json_request('POST', url, body=body)
if body:
return self.resource_class(self, body)
def _path(self, id=None):
"""Returns a request path for a given resource identifier.
:param id: Identifier of the resource to generate the request path.
"""
return ('/v1/%s/%s' % (self._resource_name, id) if id
else '/v1/%s' % self._resource_name)
@abc.abstractproperty
def resource_class(self):
"""The resource class
"""
@abc.abstractproperty
def _resource_name(self):
"""The resource name.
"""
def get(self, resource_id, fields=None):
"""Retrieve a resource.
:param resource_id: Identifier of the resource.
:param fields: List of specific fields to be returned.
"""
if fields is not None:
resource_id = '%s?fields=' % resource_id
resource_id += ','.join(fields)
try:
return self._list(self._path(resource_id))[0]
except IndexError:
return None
def vendor_passthru(self, resource_identifier, method, args=None,
http_method='POST'):
"""Issue requests for vendor-specific actions on a given driver.
:param resource_identifier: Identifier of the resource.
:param method: Name of the vendor method.
:param args: Optional. The arguments to be passed to the method.
:param http_method: The HTTP method to use on the request.
Defaults to POST.
"""
if args is None:
args = {}
http_method = http_method.upper()
path = "%s/vendor_passthru/%s" % (resource_identifier, method)
if http_method in ('POST', 'PUT', 'PATCH'):
return self.update(path, args, http_method=http_method)
elif http_method == 'DELETE':
return self.delete(path)
elif http_method == 'GET':
return self.get(path)
else:
raise exc.InvalidAttribute(
_('Unknown HTTP method: %s') % http_method)
def _format_body_data(self, body, response_key):
if response_key:
@ -124,14 +185,57 @@ class Manager(object):
data = self._format_body_data(body, response_key)
return [obj_class(self, res, loaded=True) for res in data if res]
def _update(self, url, body, method='PATCH', response_key=None):
resp, body = self.api.json_request(method, url, body=body)
def update(self, resource_id, patch, method='PATCH'):
"""Update a resource.
:param resource_id: Resource identifier.
:param patch: New version of a given resource.
:param method: Name of the method for the request.
"""
url = self._path(resource_id)
resp, body = self.api.json_request(method, url, body=patch)
# PATCH/PUT requests may not return a body
if body:
return self.resource_class(self, body)
def _delete(self, url):
self.api.raw_request('DELETE', url)
def delete(self, resource_id):
"""Delete a resource.
:param resource_id: Resource identifier.
"""
self.api.raw_request('DELETE', self._path(resource_id))
@six.add_metaclass(abc.ABCMeta)
class CreateManager(Manager):
"""Provides creation operations with a particular API."""
@abc.abstractproperty
def _creation_attributes(self):
"""A list of required creation attributes for a resource type.
"""
def create(self, **kwargs):
"""Create a resource based on a kwargs dictionary of attributes.
:param kwargs: A dictionary containing the attributes of the resource
that will be created.
:raises exc.InvalidAttribute: For invalid attributes that are not
needed to create the resource.
"""
new = {}
for (key, value) in kwargs.items():
if key in self._creation_attributes:
new[key] = value
else:
raise exc.InvalidAttribute()
url = self._path()
resp, body = self.api.json_request('POST', url, body=new)
if body:
return self.resource_class(self, body)
class Resource(base.Resource):

@ -0,0 +1,142 @@
# Copyright 2013 OpenStack Foundation
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, 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 testtools
from ironicclient.common import base
from ironicclient import exc
from ironicclient.tests.unit import utils
TESTABLE_RESOURCE = {
'id': 987,
'uuid': '11111111-2222-3333-4444-555555555555',
'attribute1': '1',
'attribute2': '2',
}
CREATE_TESTABLE_RESOURCE = copy.deepcopy(TESTABLE_RESOURCE)
del CREATE_TESTABLE_RESOURCE['id']
del CREATE_TESTABLE_RESOURCE['uuid']
INVALID_ATTRIBUTE_TESTABLE_RESOURCE = {
'non-existent-attribute': 'blablabla',
'attribute1': '1',
'attribute2': '2',
}
UPDATED_TESTABLE_RESOURCE = copy.deepcopy(TESTABLE_RESOURCE)
NEW_ATTRIBUTE_VALUE = 'brand-new-attribute-value'
UPDATED_TESTABLE_RESOURCE['attribute1'] = NEW_ATTRIBUTE_VALUE
fake_responses = {
'/v1/testableresources':
{
'GET': (
{},
{"testableresources": [TESTABLE_RESOURCE]},
),
'POST': (
{},
CREATE_TESTABLE_RESOURCE,
),
},
'/v1/testableresources/%s' % TESTABLE_RESOURCE['uuid']:
{
'GET': (
{},
TESTABLE_RESOURCE,
),
'DELETE': (
{},
None,
),
'PATCH': (
{},
UPDATED_TESTABLE_RESOURCE,
),
},
}
class TestableResource(base.Resource):
def __repr__(self):
return "<TestableResource %s>" % self._info
class TestableManager(base.CreateManager):
resource_class = TestableResource
_creation_attributes = ['attribute1', 'attribute2']
_resource_name = 'testableresources'
def _path(self, id=None):
return ('/v1/testableresources/%s' % id if id
else '/v1/testableresources')
class ManagerTestCase(testtools.TestCase):
def setUp(self):
super(ManagerTestCase, self).setUp()
self.api = utils.FakeAPI(fake_responses)
self.manager = TestableManager(self.api)
def test_create(self):
resource = self.manager.create(**CREATE_TESTABLE_RESOURCE)
expect = [
('POST', '/v1/testableresources', {}, CREATE_TESTABLE_RESOURCE),
]
self.assertEqual(expect, self.api.calls)
self.assertTrue(resource)
self.assertIsInstance(resource, TestableResource)
def test_create_with_invalid_attribute(self):
self.assertRaises(exc.InvalidAttribute, self.manager.create,
**INVALID_ATTRIBUTE_TESTABLE_RESOURCE)
def test_get(self):
resource = self.manager.get(TESTABLE_RESOURCE['uuid'])
expect = [
('GET', '/v1/testableresources/%s' % TESTABLE_RESOURCE['uuid'],
{}, None),
]
self.assertEqual(expect, self.api.calls)
self.assertEqual(TESTABLE_RESOURCE['uuid'], resource.uuid)
self.assertEqual(TESTABLE_RESOURCE['attribute1'], resource.attribute1)
def test_update(self):
patch = {'op': 'replace',
'value': NEW_ATTRIBUTE_VALUE,
'path': '/attribute1'}
resource = self.manager.update(resource_id=TESTABLE_RESOURCE['uuid'],
patch=patch)
expect = [
('PATCH', '/v1/testableresources/%s' % TESTABLE_RESOURCE['uuid'],
{}, patch),
]
self.assertEqual(expect, self.api.calls)
self.assertEqual(NEW_ATTRIBUTE_VALUE, resource.attribute1)
def test_delete(self):
resource = self.manager.delete(resource_id=TESTABLE_RESOURCE['uuid'])
expect = [
('DELETE', '/v1/testableresources/%s' % TESTABLE_RESOURCE['uuid'],
{}, None),
]
self.assertEqual(expect, self.api.calls)
self.assertIsNone(resource)

@ -327,7 +327,7 @@ class ChassisManagerTest(testtools.TestCase):
self.assertTrue(chassis)
def test_delete(self):
chassis = self.mgr.delete(chassis_id=CHASSIS['uuid'])
chassis = self.mgr.delete(resource_id=CHASSIS['uuid'])
expect = [
('DELETE', '/v1/chassis/%s' % CHASSIS['uuid'], {}, None),
]
@ -338,7 +338,7 @@ class ChassisManagerTest(testtools.TestCase):
patch = {'op': 'replace',
'value': NEW_DESCR,
'path': '/description'}
chassis = self.mgr.update(chassis_id=CHASSIS['uuid'], patch=patch)
chassis = self.mgr.update(resource_id=CHASSIS['uuid'], patch=patch)
expect = [
('PATCH', '/v1/chassis/%s' % CHASSIS['uuid'], {}, patch),
]

@ -106,7 +106,7 @@ class DriverManagerTest(testtools.TestCase):
# anything to verify.
vendor_passthru_args = {'arg1': 'val1'}
kwargs = {
'driver_name': 'driver_name',
'resource_identifier': 'driver_name',
'method': 'method',
'args': vendor_passthru_args
}
@ -123,7 +123,7 @@ class DriverManagerTest(testtools.TestCase):
@mock.patch.object(driver.DriverManager, 'get')
def test_vendor_passthru_get(self, get_mock):
kwargs = {
'driver_name': 'driver_name',
'resource_identifier': 'driver_name',
'method': 'method',
'http_method': 'GET',
}
@ -135,7 +135,7 @@ class DriverManagerTest(testtools.TestCase):
@mock.patch.object(driver.DriverManager, 'delete')
def test_vendor_passthru_delete(self, delete_mock):
kwargs = {
'driver_name': 'driver_name',
'resource_identifier': 'driver_name',
'method': 'method',
'http_method': 'DELETE',
}
@ -147,7 +147,7 @@ class DriverManagerTest(testtools.TestCase):
@mock.patch.object(driver.DriverManager, 'delete')
def test_vendor_passthru_unknown_http_method(self, delete_mock):
kwargs = {
'driver_name': 'driver_name',
'resource_identifier': 'driver_name',
'method': 'method',
'http_method': 'UNKNOWN',
}

@ -597,7 +597,7 @@ class NodeManagerTest(testtools.TestCase):
self.assertTrue(node)
def test_delete(self):
node = self.mgr.delete(node_id=NODE1['uuid'])
node = self.mgr.delete(resource_id=NODE1['uuid'])
expect = [
('DELETE', '/v1/nodes/%s' % NODE1['uuid'], {}, None),
]
@ -608,7 +608,7 @@ class NodeManagerTest(testtools.TestCase):
patch = {'op': 'replace',
'value': NEW_DRIVER,
'path': '/driver'}
node = self.mgr.update(node_id=NODE1['uuid'], patch=patch)
node = self.mgr.update(resource_id=NODE1['uuid'], patch=patch)
expect = [
('PATCH', '/v1/nodes/%s' % NODE1['uuid'], {}, patch),
]
@ -863,7 +863,7 @@ class NodeManagerTest(testtools.TestCase):
# anything to verify.
vendor_passthru_args = {'arg1': 'val1'}
kwargs = {
'node_id': 'node_uuid',
'resource_identifier': 'node_uuid',
'method': 'method',
'args': vendor_passthru_args
}
@ -880,7 +880,7 @@ class NodeManagerTest(testtools.TestCase):
@mock.patch.object(node.NodeManager, 'get')
def test_vendor_passthru_get(self, get_mock):
kwargs = {
'node_id': 'node_uuid',
'resource_identifier': 'node_uuid',
'method': 'method',
'http_method': 'GET',
}
@ -892,7 +892,7 @@ class NodeManagerTest(testtools.TestCase):
@mock.patch.object(node.NodeManager, 'delete')
def test_vendor_passthru_delete(self, delete_mock):
kwargs = {
'node_id': 'node_uuid',
'resource_identifier': 'node_uuid',
'method': 'method',
'http_method': 'DELETE',
}
@ -904,7 +904,7 @@ class NodeManagerTest(testtools.TestCase):
@mock.patch.object(node.NodeManager, 'delete')
def test_vendor_passthru_unknown_http_method(self, delete_mock):
kwargs = {
'node_id': 'node_uuid',
'resource_identifier': 'node_uuid',
'method': 'method',
'http_method': 'UNKNOWN',
}

@ -291,7 +291,7 @@ class PortManagerTest(testtools.TestCase):
self.assertTrue(port)
def test_delete(self):
port = self.mgr.delete(port_id=PORT['uuid'])
port = self.mgr.delete(resource_id=PORT['uuid'])
expect = [
('DELETE', '/v1/ports/%s' % PORT['uuid'], {}, None),
]
@ -302,7 +302,7 @@ class PortManagerTest(testtools.TestCase):
patch = {'op': 'replace',
'value': NEW_ADDR,
'path': '/address'}
port = self.mgr.update(port_id=PORT['uuid'], patch=patch)
port = self.mgr.update(resource_id=PORT['uuid'], patch=patch)
expect = [
('PATCH', '/v1/ports/%s' % PORT['uuid'], {}, patch),
]

@ -20,20 +20,15 @@ from ironicclient.common import utils
from ironicclient import exc
CREATION_ATTRIBUTES = ['description', 'extra']
class Chassis(base.Resource):
def __repr__(self):
return "<Chassis %s>" % self._info
class ChassisManager(base.Manager):
class ChassisManager(base.CreateManager):
resource_class = Chassis
@staticmethod
def _path(id=None):
return '/v1/chassis/%s' % id if id else '/v1/chassis'
_resource_name = 'chassis'
_creation_attributes = ['description', 'extra']
def list(self, marker=None, limit=None, sort_key=None,
sort_dir=None, detail=False, fields=None):
@ -164,28 +159,3 @@ class ChassisManager(base.Manager):
else:
return self._list_pagination(self._path(path), "nodes",
limit=limit)
def get(self, chassis_id, fields=None):
if fields is not None:
chassis_id = '%s?fields=' % chassis_id
chassis_id += ','.join(fields)
try:
return self._list(self._path(chassis_id))[0]
except IndexError:
return None
def create(self, **kwargs):
new = {}
for (key, value) in kwargs.items():
if key in CREATION_ATTRIBUTES:
new[key] = value
else:
raise exc.InvalidAttribute()
return self._create(self._path(), new)
def delete(self, chassis_id):
return self._delete(self._path(chassis_id))
def update(self, chassis_id, patch):
return self._update(self._path(chassis_id), patch)

@ -14,8 +14,6 @@
# under the License.
from ironicclient.common import base
from ironicclient.common.i18n import _
from ironicclient import exc
class Driver(base.Resource):
@ -25,60 +23,13 @@ class Driver(base.Resource):
class DriverManager(base.Manager):
resource_class = Driver
_resource_name = 'drivers'
def list(self):
return self._list('/v1/drivers', "drivers")
def get(self, driver_name):
try:
return self._list('/v1/drivers/%s' % driver_name)[0]
except IndexError:
return None
def update(self, driver_name, patch, http_method='PATCH'):
path = '/v1/drivers/%s' % driver_name
return self._update(path, patch, method=http_method)
def delete(self, driver_name):
return self._delete('/v1/drivers/%s' % driver_name)
return self._list('/v1/drivers', self._resource_name)
def properties(self, driver_name):
try:
info = self._list('/v1/drivers/%s/properties' % driver_name)[0]
if info:
return info.to_dict()
return {}
except IndexError:
return {}
def vendor_passthru(self, driver_name, method, args=None,
http_method=None):
"""Issue requests for vendor-specific actions on a given driver.
:param driver_name: Name of the driver.
:param method: Name of the vendor method.
:param args: Optional. The arguments to be passed to the method.
:param http_method: The HTTP method to use on the request.
Defaults to POST.
"""
if args is None:
args = {}
if http_method is None:
http_method = 'POST'
http_method = http_method.upper()
path = "%s/vendor_passthru/%s" % (driver_name, method)
if http_method in ('POST', 'PUT', 'PATCH'):
return self.update(path, args, http_method=http_method)
elif http_method == 'DELETE':
return self.delete(path)
elif http_method == 'GET':
return self.get(path)
else:
raise exc.InvalidAttribute(
_('Unknown HTTP method: %s') % http_method)
return self.get('%s/properties' % driver_name).to_dict()
def get_vendor_passthru_methods(self, driver_name):
path = "%s/vendor_passthru/methods" % driver_name

@ -21,9 +21,6 @@ from ironicclient.common.i18n import _
from ironicclient.common import utils
from ironicclient import exc
CREATION_ATTRIBUTES = ['chassis_uuid', 'driver', 'driver_info', 'extra',
'uuid', 'properties', 'name']
_power_states = {
'on': 'power on',
@ -37,12 +34,11 @@ class Node(base.Resource):
return "<Node %s>" % self._info
class NodeManager(base.Manager):
class NodeManager(base.CreateManager):
resource_class = Node
@staticmethod
def _path(id=None):
return '/v1/nodes/%s' % id if id else '/v1/nodes'
_creation_attributes = ['chassis_uuid', 'driver', 'driver_info',
'extra', 'uuid', 'properties', 'name']
_resource_name = 'nodes'
def list(self, associated=None, maintenance=None, marker=None, limit=None,
detail=False, sort_key=None, sort_dir=None, fields=None,
@ -170,16 +166,6 @@ class NodeManager(base.Manager):
return self._list_pagination(self._path(path), "ports",
limit=limit)
def get(self, node_id, fields=None):
if fields is not None:
node_id = '%s?fields=' % node_id
node_id += ','.join(fields)
try:
return self._list(self._path(node_id))[0]
except IndexError:
return None
def get_by_instance_uuid(self, instance_uuid, fields=None):
path = '?instance_uuid=%s' % instance_uuid
if fields is not None:
@ -196,50 +182,6 @@ class NodeManager(base.Manager):
else:
raise exc.NotFound()
def create(self, **kwargs):
new = {}
for (key, value) in kwargs.items():
if key in CREATION_ATTRIBUTES:
new[key] = value
else:
raise exc.InvalidAttribute()
return self._create(self._path(), new)
def delete(self, node_id):
return self._delete(self._path(node_id))
def update(self, node_id, patch, http_method='PATCH'):
return self._update(self._path(node_id), patch, method=http_method)
def vendor_passthru(self, node_id, method, args=None, http_method=None):
"""Issue requests for vendor-specific actions on a given node.
:param node_id: The UUID of the node.
:param method: Name of the vendor method.
:param args: Optional. The arguments to be passed to the method.
:param http_method: The HTTP method to use on the request.
Defaults to POST.
"""
if args is None:
args = {}
if http_method is None:
http_method = 'POST'
http_method = http_method.upper()
path = "%s/vendor_passthru/%s" % (node_id, method)
if http_method in ('POST', 'PUT', 'PATCH'):
return self.update(path, args, http_method=http_method)
elif http_method == 'DELETE':
return self.delete(path)
elif http_method == 'GET':
return self.get(path)
else:
raise exc.InvalidAttribute(
_('Unknown HTTP method: %s') % http_method)
def set_maintenance(self, node_id, state, maint_reason=None):
"""Set the maintenance mode for the node.
@ -265,14 +207,14 @@ class NodeManager(base.Manager):
path = "%s/maintenance" % node_id
if maintenance_mode:
reason = {'reason': maint_reason}
return self._update(self._path(path), reason, method='PUT')
return self.update(path, reason, method='PUT')
else:
return self._delete(self._path(path))
return self.delete(path)
def set_power_state(self, node_id, state):
path = "%s/states/power" % node_id
target = {'target': _power_states.get(state, state)}
return self._update(self._path(path), target, method='PUT')
return self.update(path, target, method='PUT')
def validate(self, node_uuid):
path = "%s/validate" % node_uuid
@ -289,7 +231,7 @@ class NodeManager(base.Manager):
configdrive = utils.make_configdrive(configdrive)
body['configdrive'] = configdrive
return self._update(self._path(path), body, method='PUT')
return self.update(path, body, method='PUT')
def states(self, node_uuid):
path = "%s/states" % node_uuid
@ -312,12 +254,12 @@ class NodeManager(base.Manager):
"""
path = "%s/states/console" % node_uuid
target = {'enabled': enabled}
return self._update(self._path(path), target, method='PUT')
return self.update(path, target, method='PUT')
def set_boot_device(self, node_uuid, boot_device, persistent=False):
path = "%s/management/boot_device" % node_uuid
target = {'boot_device': boot_device, 'persistent': persistent}
return self._update(self._path(path), target, method='PUT')
return self.update(path, target, method='PUT')
def get_boot_device(self, node_uuid):
path = "%s/management/boot_device" % node_uuid

@ -19,20 +19,16 @@ from ironicclient.common.i18n import _
from ironicclient.common import utils
from ironicclient import exc
CREATION_ATTRIBUTES = ['address', 'extra', 'node_uuid']
class Port(base.Resource):
def __repr__(self):
return "<Port %s>" % self._info
class PortManager(base.Manager):
class PortManager(base.CreateManager):
resource_class = Port
@staticmethod
def _path(id=None):
return '/v1/ports/%s' % id if id else '/v1/ports'
_creation_attributes = ['address', 'extra', 'node_uuid']
_resource_name = 'ports'
def list(self, address=None, limit=None, marker=None, sort_key=None,
sort_dir=None, detail=False, fields=None):
@ -91,16 +87,6 @@ class PortManager(base.Manager):
return self._list_pagination(self._path(path), "ports",
limit=limit)
def get(self, port_id, fields=None):
if fields is not None:
port_id = '%s?fields=' % port_id
port_id += ','.join(fields)
try:
return self._list(self._path(port_id))[0]
except IndexError:
return None
def get_by_address(self, address, fields=None):
path = '?address=%s' % address
if fields is not None:
@ -115,18 +101,3 @@ class PortManager(base.Manager):
return ports[0]
else:
raise exc.NotFound()
def create(self, **kwargs):
new = {}
for (key, value) in kwargs.items():
if key in CREATION_ATTRIBUTES:
new[key] = value
else:
raise exc.InvalidAttribute()
return self._create(self._path(), new)
def delete(self, port_id):
return self._delete(self._path(port_id))
def update(self, port_id, patch):
return self._update(self._path(port_id), patch)