From cd072ac607ca43f606247385088a3dde60165338 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Thu, 21 Nov 2013 14:10:26 +0000 Subject: [PATCH] Reworks Chassis validations This makes use of complex types validations of WSME that now works. Change-Id: I5cd46d5c2caeb3f3c4cd1ef90448e7fd3c81404d Closes-Bug: #1223847 --- ironic/api/controllers/v1/base.py | 13 ------ ironic/api/controllers/v1/chassis.py | 64 ++++++++++++---------------- ironic/api/controllers/v1/utils.py | 37 ---------------- ironic/objects/base.py | 8 ---- ironic/tests/api/test_utils.py | 59 ------------------------- 5 files changed, 28 insertions(+), 153 deletions(-) diff --git a/ironic/api/controllers/v1/base.py b/ironic/api/controllers/v1/base.py index 2d253e277f..7e7a49a8d2 100644 --- a/ironic/api/controllers/v1/base.py +++ b/ironic/api/controllers/v1/base.py @@ -35,19 +35,6 @@ class APIBase(wtypes.Base): if hasattr(self, k) and getattr(self, k) != wsme.Unset) - # TODO(lucasagomes): Deprecated. Remove it after updating the chassis - # and nodes elements - @classmethod - def from_rpc_object(cls, m, fields=None): - """Convert a RPC object to an API object.""" - obj_dict = m.as_dict() - # Unset non-required fields so they do not appear - # in the message body - obj_dict.update(dict((k, wsme.Unset) - for k in obj_dict.keys() - if fields and k not in fields)) - return cls(**obj_dict) - def unset_fields_except(self, except_list=None): """Unset fields so they don't appear in the message body. diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index 052e5f9f5b..f7710abea6 100644 --- a/ironic/api/controllers/v1/chassis.py +++ b/ironic/api/controllers/v1/chassis.py @@ -30,6 +30,7 @@ from ironic.api.controllers.v1 import base from ironic.api.controllers.v1 import collection from ironic.api.controllers.v1 import link from ironic.api.controllers.v1 import node +from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils from ironic.common import exception from ironic import objects @@ -39,6 +40,10 @@ from ironic.openstack.common import log LOG = log.getLogger(__name__) +class ChassisPatchType(types.JsonPatchType): + pass + + class Chassis(base.APIBase): """API representation of a chassis. @@ -47,8 +52,7 @@ class Chassis(base.APIBase): a chassis. """ - # NOTE: translate 'id' publicly to 'uuid' internally - uuid = wtypes.text + uuid = types.uuid "The UUID of the chassis" description = wtypes.text @@ -70,17 +74,10 @@ class Chassis(base.APIBase): @classmethod def convert_with_links(cls, rpc_chassis, expand=True): - fields = ['uuid', 'description'] if not expand else None - chassis = Chassis.from_rpc_object(rpc_chassis, fields) - chassis.links = [link.Link.make_link('self', - pecan.request.host_url, - 'chassis', chassis.uuid), - link.Link.make_link('bookmark', - pecan.request.host_url, - 'chassis', chassis.uuid) - ] - - if expand: + chassis = Chassis(**rpc_chassis.as_dict()) + if not expand: + chassis.unset_fields_except(['uuid', 'description']) + else: chassis.nodes = [link.Link.make_link('self', pecan.request.host_url, 'chassis', @@ -91,6 +88,13 @@ class Chassis(base.APIBase): chassis.uuid + "/nodes", bookmark=True) ] + chassis.links = [link.Link.make_link('self', + pecan.request.host_url, + 'chassis', chassis.uuid), + link.Link.make_link('bookmark', + pecan.request.host_url, + 'chassis', chassis.uuid) + ] return chassis @@ -196,41 +200,29 @@ class ChassisController(rest.RestController): LOG.exception(e) return Chassis.convert_with_links(new_chassis) - @wsme_pecan.wsexpose(Chassis, wtypes.text, body=[wtypes.text]) + @wsme.validate(wtypes.text, [ChassisPatchType]) + @wsme_pecan.wsexpose(Chassis, wtypes.text, body=[ChassisPatchType]) def patch(self, uuid, patch): """Update an existing chassis. :param uuid: UUID of a chassis. :param patch: a json PATCH document to apply to this chassis. """ - chassis = objects.Chassis.get_by_uuid(pecan.request.context, uuid) - chassis_dict = chassis.as_dict() - - utils.validate_patch(patch) + rpc_chassis = objects.Chassis.get_by_uuid(pecan.request.context, uuid) try: - patched_chassis = jsonpatch.apply_patch(chassis_dict, - jsonpatch.JsonPatch(patch)) + chassis = Chassis(**jsonpatch.apply_patch(rpc_chassis.as_dict(), + jsonpatch.JsonPatch(patch))) except jsonpatch.JsonPatchException as e: LOG.exception(e) raise wsme.exc.ClientSideError(_("Patching Error: %s") % e) - defaults = objects.Chassis.get_defaults() - for key in defaults: - # Internal values that shouldn't be part of the patch - if key in ['id', 'updated_at', 'created_at']: - continue + # Update only the fields that have changed + for field in objects.Chassis.fields: + if rpc_chassis[field] != getattr(chassis, field): + rpc_chassis[field] = getattr(chassis, field) - # In case of a remove operation, add the missing fields back - # to the document with their default value - if key in chassis_dict and key not in patched_chassis: - patched_chassis[key] = defaults[key] - - # Update only the fields that have changed - if chassis[key] != patched_chassis[key]: - chassis[key] = patched_chassis[key] - - chassis.save() - return Chassis.convert_with_links(chassis) + rpc_chassis.save() + return Chassis.convert_with_links(rpc_chassis) @wsme_pecan.wsexpose(None, wtypes.text, status_code=204) def delete(self, uuid): diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index a1cfb78c31..26ce41566f 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -16,7 +16,6 @@ # License for the specific language governing permissions and limitations # under the License. -import re import wsme from oslo.config import cfg @@ -39,42 +38,6 @@ def validate_sort_dir(sort_dir): return sort_dir -# TODO(lucasagomes): Deprecated. Remove it after updating chassis and nodes -def validate_patch(patch): - """Performs a basic validation on patch.""" - - if not isinstance(patch, list): - patch = [patch] - - path_pattern = re.compile("(/[\w-]+)+$") - for p in patch: - if not isinstance(p, dict) or \ - any(key for key in ["path", "op"] if key not in p): - raise wsme.exc.ClientSideError(_("Invalid patch format: %s") - % str(p)) - - path = p["path"] - op = p["op"] - - if op not in ["add", "replace", "remove"]: - raise wsme.exc.ClientSideError(_("Operation not supported: %s") - % op) - - if not path_pattern.match(path): - raise wsme.exc.ClientSideError(_("Invalid path: %s") % path) - - if op == "add": - if path.count('/') == 1: - raise wsme.exc.ClientSideError(_("Adding an additional " - "attribute (%s) to the " - "resource is not allowed") - % path) - if op in ["remove", "replace", "add"]: - if path.startswith('/uuid'): - raise wsme.exc.ClientSideError(_("UUIDs can not be removed " - "or replaced")) - - class ValidTypes(wsme.types.UserType): """User type for validate that value has one of a few types.""" diff --git a/ironic/objects/base.py b/ironic/objects/base.py index 9a4862ce1b..5f2783f777 100644 --- a/ironic/objects/base.py +++ b/ironic/objects/base.py @@ -384,14 +384,6 @@ class IronicObject(object): for k in self.fields if hasattr(self, k)) - # TODO(lucasagomes): Deprecated. Remove it after updating chassis and nodes - @classmethod - def get_defaults(cls): - """Return a dict of its fields with their default value.""" - return dict((k, v(None)) - for k, v in cls.fields.iteritems() - if k != "id" and callable(v)) - class ObjectListBase(object): """Mixin class for lists of objects. diff --git a/ironic/tests/api/test_utils.py b/ironic/tests/api/test_utils.py index 6d266a3423..e472054887 100644 --- a/ironic/tests/api/test_utils.py +++ b/ironic/tests/api/test_utils.py @@ -49,65 +49,6 @@ class TestApiUtils(base.FunctionalTest): utils.validate_sort_dir, 'fake-sort') - def test_validate_patch(self): - patch = [{'op': 'remove', 'value': 'bar', 'path': '/foo'}] - utils.validate_patch(patch) - - patch = [{'op': 'add', 'value': 'bar', 'path': '/extra/foo'}] - utils.validate_patch(patch) - - patch = [{'op': 'replace', 'value': 'bar', 'path': '/foo'}] - utils.validate_patch(patch) - - def test_validate_patch_wrong_format(self): - # missing path - patch = [{'op': 'remove'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - # wrong op - patch = [{'op': 'foo', 'value': 'bar', 'path': '/foo'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - - def test_validate_patch_wrong_path(self): - # non-alphanumeric characters - patch = [{'path': '/fo^o', 'op': 'remove'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - # empty path - patch = [{'path': '', 'op': 'remove'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - - patch = [{'path': '/', 'op': 'remove'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - - def test_validate_patch_uuid(self): - patch = [{'op': 'remove', 'path': '/uuid'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - - patch = [{'op': 'replace', - 'value': '105f5cd9-ae67-480a-8c10-62040213b8fd', - 'path': '/uuid'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - - patch = [{'op': 'add', - 'value': '105f5cd9-ae67-480a-8c10-62040213b8fd', - 'path': '/uuid'}] - self.assertRaises(wsme.exc.ClientSideError, - utils.validate_patch, - patch) - def test_valid_types(self): vt = utils.ValidTypes(wsme.types.text, six.integer_types)