From b96e297dbbbd47e69abbd1b5f468e1fc84b9ef6d Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Mon, 19 Aug 2013 18:38:35 +0100 Subject: [PATCH] Updating resources with PATCH This patch enables clients to update the resources using the PATCH HTTP method. PATCH allows partial updates on the documents, reducing network and processing overhead. Attributes may be removed, replaced, or added in a single request. Closes-Bug: #1220107 Change-Id: I8187f32b86f05fda58360cd3a51f07ae43742a3a --- doc/source/dev/api-spec-v1.rst | 100 ++++++++++++++++++++++++- ironic/api/controllers/v1/base.py | 7 -- ironic/api/controllers/v1/chassis.py | 37 +++++---- ironic/api/controllers/v1/node.py | 52 ++++++++----- ironic/api/controllers/v1/port.py | 34 ++++++--- ironic/api/controllers/v1/utils.py | 33 ++++++++ ironic/objects/base.py | 7 ++ ironic/tests/api/test_chassis.py | 106 +++++++++++++++++++++++--- ironic/tests/api/test_nodes.py | 54 ++++++++++++-- ironic/tests/api/test_ports.py | 108 ++++++++++++++++++++++++++- requirements.txt | 1 + 11 files changed, 470 insertions(+), 69 deletions(-) diff --git a/doc/source/dev/api-spec-v1.rst b/doc/source/dev/api-spec-v1.rst index d831cb4492..4047500839 100644 --- a/doc/source/dev/api-spec-v1.rst +++ b/doc/source/dev/api-spec-v1.rst @@ -21,6 +21,7 @@ General Concepts - SubResource_ - Security_ - Versioning_ +- `Updating Resources`_ Links and Relationships ------------------------ @@ -141,6 +142,99 @@ with a particular version will result the API will assume the use of the default version. When both URL version and MIME type are specified and conflicting the URL version takes precedence. +Updating Resources +------------------- + +The PATCH HTTP method is used to update a resource in the API. PATCH +allows clients to do partial updates to a resource, sending only the +attributes requiring modification. Operations supported are "remove", +"add" and "replace", multiple operations can be combined in a single +request. + +The request body must conform to the 'application/json-patch+json' +media type (RFC 6902) and response body will represent the updated +resource entity. + +Example:: + + PATCH /chassis/4505e16b-47d6-424c-ae78-e0ef1b600700 + + [ + {"path": "/description", "value": "new description", "op": "replace"}, + {"path": "/extra/foo", "value": "bar", "op": "add"}, + {"path": "/extra/noop", "op": "remove"} + ] + +Different types of attributes that exists in the resource will be either +removed, added or replaced according to the following rules: + +Singular attributes +^^^^^^^^^^^^^^^^^^^^ + +An "add" or "replace" operation replaces the value of an existing +attribute with a new value. Adding new attributes to the root document +of the resource is not allowed. + +The "remove" operation resets the target attribute to its default value. + +Example, replacing an attribute:: + + PATCH /chassis/4505e16b-47d6-424c-ae78-e0ef1b600700 + + [ + {"path": "/description", "value": "new description", "op": "replace"} + ] + + +Example, removing an attribute:: + + PATCH /chassis/4505e16b-47d6-424c-ae78-e0ef1b600700 + + [ + {"path": "/description", "op": "remove"} + ] + +*Note: This operation will not remove the description attribute from +the document but instead will reset it to its default value.* + +Multi-valued attributes +^^^^^^^^^^^^^^^^^^^^^^^^ + +In case of an "add" operation the attribute is added to the collection +if the it does not exist and merged if a matching attribute is present. + +The "remove" operation removes the target attribute from the collection. + +The "replace" operation replaces the value at the target attribute with +a new value. + +Example, adding an attribute to the collection:: + + PATCH /chassis/4505e16b-47d6-424c-ae78-e0ef1b600700 + + [ + {"path": "/extra/foo", "value": "bar", "op": "add"} + ] + + +Example, removing an attribute from the collection:: + + PATCH /chassis/4505e16b-47d6-424c-ae78-e0ef1b600700 + + [ + {"path": "/extra/foo", "op": "remove"} + ] + + +Example, removing **all** attributes from the collection:: + + PATCH /chassis/4505e16b-47d6-424c-ae78-e0ef1b600700 + + [ + {"path": "/extra", "op": "remove"} + ] + + Resource Definitions ##################### @@ -343,7 +437,7 @@ Verb Path Response GET /nodes List nodes. GET /nodes/ Retrieve a specific node. POST /nodes Create a new node -PUT /nodes/ Update a node +PATCH /nodes/ Update a node DELETE /nodes/ Delete node and all associated ports ======= ============= ========== @@ -478,7 +572,7 @@ Verb Path Response GET /chassis List chassis GET /chassis/ Retrieve a specific chassis POST /chassis Create a new chassis -PUT /chassis/ Update a chassis +PATCH /chassis/ Update a chassis DELETE /chassis/ Delete chassis and remove all associations between nodes ======= ============= ========== @@ -543,7 +637,7 @@ Verb Path Response GET /ports List ports GET /ports/ Retrieve a specific port POST /ports Create a new port -PUT /ports/ Update a port +PATCH /ports/ Update a port DELETE /ports/ Delete port and remove all associations between nodes ======= ============= ========== diff --git a/ironic/api/controllers/v1/base.py b/ironic/api/controllers/v1/base.py index 1d14953dc5..6e62c8c39f 100644 --- a/ironic/api/controllers/v1/base.py +++ b/ironic/api/controllers/v1/base.py @@ -27,13 +27,6 @@ class APIBase(wtypes.Base): if hasattr(self, k) and getattr(self, k) != wsme.Unset) - def as_terse_dict(self): - """Render this object as a dict of its non-None fields.""" - return dict((k, getattr(self, k)) - for k in self.fields - if hasattr(self, k) and - getattr(self, k) not in [wsme.Unset, None]) - @classmethod def from_rpc_object(cls, m): return cls(**m.as_dict()) diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index 81635a8f91..1394a1b7ce 100644 --- a/ironic/api/controllers/v1/chassis.py +++ b/ironic/api/controllers/v1/chassis.py @@ -16,6 +16,8 @@ # License for the specific language governing permissions and limitations # under the License. +import jsonpatch + import pecan from pecan import rest @@ -23,14 +25,13 @@ import wsme from wsme import types as wtypes import wsmeext.pecan as wsme_pecan -from ironic import objects - 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 utils from ironic.common import exception +from ironic import objects from ironic.openstack.common import log LOG = log.getLogger(__name__) @@ -144,21 +145,31 @@ class ChassisController(rest.RestController): raise wsme.exc.ClientSideError(_("Invalid data")) return Chassis.convert_with_links(new_chassis) - @wsme_pecan.wsexpose(Chassis, unicode, body=Chassis) - def patch(self, uuid, delta_chassis): + @wsme_pecan.wsexpose(Chassis, unicode, body=[unicode]) + def patch(self, uuid, patch): """Update an existing chassis.""" chassis = objects.Chassis.get_by_uuid(pecan.request.context, uuid) - nn_delta_ch = delta_chassis.as_terse_dict() - # Ensure immutable keys are not present - # TODO(lucasagomes): Not sure if 'id' will ever be present here - # the translation should occur before it reaches this point - if any(v for v in nn_delta_ch if v in ("id", "uuid")): - raise wsme.exc.ClientSideError(_("'uuid' is immutable")) + chassis_dict = chassis.as_dict() - for k in nn_delta_ch: - chassis[k] = nn_delta_ch[k] + # These are internal values that shouldn't be part of the patch + internal_attrs = ['id', 'updated_at', 'created_at'] + [chassis_dict.pop(attr, None) for attr in internal_attrs] + + utils.validate_patch(patch) + try: + final_patch = jsonpatch.apply_patch(chassis_dict, + jsonpatch.JsonPatch(patch)) + except jsonpatch.JsonPatchException as e: + LOG.exception(e) + raise wsme.exc.ClientSideError(_("Patching Error: %s") % e) + + # In case of a remove operation, add the missing fields back to + # the document with their default value + defaults = objects.Chassis.get_defaults() + defaults.update(final_patch) + + chassis.update(defaults) chassis.save() - return Chassis.convert_with_links(chassis) @wsme_pecan.wsexpose(None, unicode, status_code=204) diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index aebd31aa90..5b00ff49ee 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -15,6 +15,8 @@ # License for the specific language governing permissions and limitations # under the License. +import jsonpatch + import pecan from pecan import rest @@ -22,8 +24,6 @@ import wsme from wsme import types as wtypes import wsmeext.pecan as wsme_pecan -from ironic import objects - from ironic.api.controllers.v1 import base from ironic.api.controllers.v1 import collection from ironic.api.controllers.v1 import link @@ -31,6 +31,7 @@ from ironic.api.controllers.v1 import port from ironic.api.controllers.v1 import state from ironic.api.controllers.v1 import utils from ironic.common import exception +from ironic import objects from ironic.openstack.common import log LOG = log.getLogger(__name__) @@ -300,34 +301,46 @@ class NodesController(rest.RestController): raise wsme.exc.ClientSideError(_("Invalid data")) return Node.convert_with_links(new_node) - @wsme_pecan.wsexpose(Node, unicode, body=Node, status=200) - def patch(self, node_id, node_data): + @wsme_pecan.wsexpose(Node, unicode, body=[unicode]) + def patch(self, uuid, patch): """Update an existing node. TODO(deva): add exception handling """ - # NOTE: WSME is creating an api v1 Node object with all fields - # so we eliminate non-supplied fields by converting - # to a dict and stripping keys with value=None - delta = node_data.as_terse_dict() + node = objects.Node.get_by_uuid(pecan.request.context, uuid) + node_dict = node.as_dict() + + # These are internal values that shouldn't be part of the patch + internal_attrs = ['id', 'updated_at', 'created_at'] + [node_dict.pop(attr, None) for attr in internal_attrs] + + utils.validate_patch(patch) + patch_obj = jsonpatch.JsonPatch(patch) # Prevent states from being updated - state_rel_attr = ['power_state', 'target_power_state', - 'provision_state', 'target_provision_state'] - if any((getattr(node_data, attr) for attr in state_rel_attr)): + state_rel_path = ['/power_state', '/target_power_state', + '/provision_state', '/target_provision_state'] + if any(p['path'] in state_rel_path for p in patch_obj): raise wsme.exc.ClientSideError(_("Changing states is not allowed " "here; You must use the " "nodes/%s/state interface.") - % node_id) + % uuid) + try: + final_patch = jsonpatch.apply_patch(node_dict, patch_obj) + except jsonpatch.JsonPatchException as e: + LOG.exception(e) + raise wsme.exc.ClientSideError(_("Patching Error: %s") % e) response = wsme.api.Response(Node(), status_code=200) try: - node = objects.Node.get_by_uuid( - pecan.request.context, node_id) - for k in delta.keys(): - node[k] = delta[k] - node = pecan.request.rpcapi.update_node( - pecan.request.context, node) + # In case of a remove operation, add the missing fields back to + # the document with their default value + defaults = objects.Node.get_defaults() + defaults.update(final_patch) + + node.update(defaults) + node = pecan.request.rpcapi.update_node(pecan.request.context, + node) response.obj = node except exception.InvalidParameterValue: response.status_code = 400 @@ -341,7 +354,8 @@ class NodesController(rest.RestController): # after wsme 0.5b3 is released if response.status_code not in [200, 202]: raise wsme.exc.ClientSideError(_( - "Error updating node %s") % node_id) + "Error updating node %s") % uuid) + return Node.convert_with_links(response.obj) @wsme_pecan.wsexpose(None, unicode, status_code=204) diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 6a776cda20..6300de9064 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -15,6 +15,8 @@ # License for the specific language governing permissions and limitations # under the License. +import jsonpatch + import pecan from pecan import rest @@ -22,13 +24,12 @@ import wsme from wsme import types as wtypes import wsmeext.pecan as wsme_pecan -from ironic import objects - 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 utils from ironic.common import exception +from ironic import objects from ironic.openstack.common import log LOG = log.getLogger(__name__) @@ -123,15 +124,30 @@ class PortsController(rest.RestController): raise wsme.exc.ClientSideError(_("Invalid data")) return Port.convert_with_links(new_port) - @wsme_pecan.wsexpose(Port, unicode, body=Port) - def patch(self, uuid, port_data): + @wsme_pecan.wsexpose(Port, unicode, body=[unicode]) + def patch(self, uuid, patch): """Update an existing port.""" - # TODO(wentian): add rpc handle, - # eg. if update fails because node is already locked port = objects.Port.get_by_uuid(pecan.request.context, uuid) - nn_delta_p = port_data.as_terse_dict() - for k in nn_delta_p: - port[k] = nn_delta_p[k] + port_dict = port.as_dict() + + # These are internal values that shouldn't be part of the patch + internal_attrs = ['id', 'updated_at', 'created_at'] + [port_dict.pop(attr, None) for attr in internal_attrs] + + utils.validate_patch(patch) + try: + final_patch = jsonpatch.apply_patch(port_dict, + jsonpatch.JsonPatch(patch)) + except jsonpatch.JsonPatchException as e: + LOG.exception(e) + raise wsme.exc.ClientSideError(_("Patching Error: %s") % e) + + # In case of a remove operation, add the missing fields back to + # the document with their default value + defaults = objects.Port.get_defaults() + defaults.update(final_patch) + + port.update(defaults) port.save() return Port.convert_with_links(port) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 3cd410cab2..50851dd39f 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -16,6 +16,7 @@ # License for the specific language governing permissions and limitations # under the License. +import re import wsme from oslo.config import cfg @@ -36,3 +37,35 @@ def validate_sort_dir(sort_dir): "Acceptable values are " "'asc' or 'desc'") % sort_dir) return sort_dir + + +def validate_patch(patch): + """Performs a basic validation on patch.""" + + if not isinstance(patch, list): + patch = [patch] + + for p in patch: + path_pattern = re.compile("^/[a-zA-Z0-9-_]+(/[a-zA-Z0-9-_]+)*$") + + 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) diff --git a/ironic/objects/base.py b/ironic/objects/base.py index 1f5599ed7a..875d45982c 100644 --- a/ironic/objects/base.py +++ b/ironic/objects/base.py @@ -375,6 +375,13 @@ class IronicObject(object): for k in self.fields if hasattr(self, k)) + @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_chassis.py b/ironic/tests/api/test_chassis.py index a792290c09..f6a2f5f0e2 100644 --- a/ironic/tests/api/test_chassis.py +++ b/ironic/tests/api/test_chassis.py @@ -100,26 +100,114 @@ class TestListChassis(base.FunctionalTest): class TestPatch(base.FunctionalTest): - def test_update_chassis(self): + def setUp(self): + super(TestPatch, self).setUp() cdict = dbutils.get_test_chassis() self.post_json('/chassis', cdict) - description = 'chassis-new-description' - response = self.patch_json('/chassis/%s' % cdict['uuid'], - {'description': description}) - self.assertEqual(response.content_type, 'application/json') - self.assertEqual(response.status_code, 200) - result = self.get_json('/chassis/%s' % cdict['uuid']) - self.assertEqual(result['description'], description) def test_update_not_found(self): uuid = uuidutils.generate_uuid() - response = self.patch_json('/chassis/%s' % uuid, {'extra': {'a': 'b'}}, + response = self.patch_json('/chassis/%s' % uuid, + [{'path': '/extra/a', 'value': 'b', + 'op': 'add'}], expect_errors=True) # TODO(yuriyz): change to 404 (bug 1200517) self.assertEqual(response.status_int, 500) self.assertEqual(response.content_type, 'application/json') self.assertTrue(response.json['error_message']) + def test_replace_singular(self): + cdict = dbutils.get_test_chassis() + description = 'chassis-new-description' + response = self.patch_json('/chassis/%s' % cdict['uuid'], + [{'path': '/description', + 'value': description, 'op': 'replace'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/chassis/%s' % cdict['uuid']) + self.assertEqual(result['description'], description) + + def test_replace_multi(self): + extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"} + cdict = dbutils.get_test_chassis(extra=extra, + uuid=uuidutils.generate_uuid()) + self.post_json('/chassis', cdict) + new_value = 'new value' + response = self.patch_json('/chassis/%s' % cdict['uuid'], + [{'path': '/extra/foo2', + 'value': new_value, 'op': 'replace'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/chassis/%s' % cdict['uuid']) + + extra["foo2"] = new_value + self.assertEqual(result['extra'], extra) + + def test_remove_singular(self): + cdict = dbutils.get_test_chassis(extra={'a': 'b'}, + uuid=uuidutils.generate_uuid()) + self.post_json('/chassis', cdict) + response = self.patch_json('/chassis/%s' % cdict['uuid'], + [{'path': '/description', 'op': 'remove'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/chassis/%s' % cdict['uuid']) + self.assertEqual(result['description'], None) + + # Assert nothing else was changed + self.assertEqual(result['uuid'], cdict['uuid']) + self.assertEqual(result['extra'], cdict['extra']) + + def test_remove_multi(self): + extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"} + cdict = dbutils.get_test_chassis(extra=extra, description="foobar", + uuid=uuidutils.generate_uuid()) + self.post_json('/chassis', cdict) + + # Removing one item from the collection + response = self.patch_json('/chassis/%s' % cdict['uuid'], + [{'path': '/extra/foo2', 'op': 'remove'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/chassis/%s' % cdict['uuid']) + extra.pop("foo2") + self.assertEqual(result['extra'], extra) + + # Removing the collection + response = self.patch_json('/chassis/%s' % cdict['uuid'], + [{'path': '/extra', 'op': 'remove'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/chassis/%s' % cdict['uuid']) + self.assertEqual(result['extra'], {}) + + # Assert nothing else was changed + self.assertEqual(result['uuid'], cdict['uuid']) + self.assertEqual(result['description'], cdict['description']) + + def test_add_singular(self): + cdict = dbutils.get_test_chassis() + response = self.patch_json('/chassis/%s' % cdict['uuid'], + [{'path': '/foo', 'value': 'bar', + 'op': 'add'}], + expect_errors=True) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_int, 400) + self.assertTrue(response.json['error_message']) + + def test_add_multi(self): + cdict = dbutils.get_test_chassis() + response = self.patch_json('/chassis/%s' % cdict['uuid'], + [{'path': '/extra/foo1', 'value': 'bar1', + 'op': 'add'}, + {'path': '/extra/foo2', 'value': 'bar2', + 'op': 'add'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/chassis/%s' % cdict['uuid']) + expected = {"foo1": "bar1", "foo2": "bar2"} + self.assertEqual(result['extra'], expected) + class TestPost(base.FunctionalTest): diff --git a/ironic/tests/api/test_nodes.py b/ironic/tests/api/test_nodes.py index cd0bdd5757..13851c98db 100644 --- a/ironic/tests/api/test_nodes.py +++ b/ironic/tests/api/test_nodes.py @@ -152,7 +152,9 @@ class TestPatch(base.FunctionalTest): self.mox.ReplayAll() response = self.patch_json('/nodes/%s' % self.node['uuid'], - {'instance_uuid': 'fake instance uuid'}) + [{'path': '/instance_uuid', + 'value': 'fake instance uuid', + 'op': 'replace'}]) self.assertEqual(response.content_type, 'application/json') self.assertEqual(response.status_code, 200) self.mox.VerifyAll() @@ -169,8 +171,13 @@ class TestPatch(base.FunctionalTest): self.mox.ReplayAll() response = self.patch_json('/nodes/%s' % self.node['uuid'], - {'driver_info': {'this': 'foo', 'that': 'bar'}}, - expect_errors=True) + [{'path': '/driver_info/this', + 'value': 'foo', + 'op': 'add'}, + {'path': '/driver_info/that', + 'value': 'bar', + 'op': 'add'}], + expect_errors=True) self.assertEqual(response.content_type, 'application/json') self.assertEqual(response.status_code, 400) self.mox.VerifyAll() @@ -183,13 +190,50 @@ class TestPatch(base.FunctionalTest): self.mox.ReplayAll() response = self.patch_json('/nodes/%s' % self.node['uuid'], - {'instance_uuid': 'fake instance uuid'}, - expect_errors=True) + [{'path': '/instance_uuid', + 'value': 'fake instance uuid', + 'op': 'replace'}], + expect_errors=True) self.assertEqual(response.content_type, 'application/json') # TODO(deva): change to 409 when wsme 0.5b3 released self.assertEqual(response.status_code, 400) self.mox.VerifyAll() + def test_add_ok(self): + rpcapi.ConductorAPI.update_node(mox.IgnoreArg(), mox.IgnoreArg()).\ + AndReturn(self.node) + self.mox.ReplayAll() + + response = self.patch_json('/nodes/%s' % self.node['uuid'], + [{'path': '/extra/foo', + 'value': 'bar', + 'op': 'add'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + self.mox.VerifyAll() + + def test_add_fail(self): + self.assertRaises(webtest.app.AppError, self.patch_json, + '/nodes/%s' % self.node['uuid'], + [{'path': '/foo', 'value': 'bar', 'op': 'add'}]) + + def test_remove_ok(self): + rpcapi.ConductorAPI.update_node(mox.IgnoreArg(), mox.IgnoreArg()).\ + AndReturn(self.node) + self.mox.ReplayAll() + + response = self.patch_json('/nodes/%s' % self.node['uuid'], + [{'path': '/extra', + 'op': 'remove'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + self.mox.VerifyAll() + + def test_remove_fail(self): + self.assertRaises(webtest.app.AppError, self.patch_json, + '/nodes/%s' % self.node['uuid'], + [{'path': '/extra/non-existent', 'op': 'remove'}]) + class TestPost(base.FunctionalTest): diff --git a/ironic/tests/api/test_ports.py b/ironic/tests/api/test_ports.py index 6a1529f6d1..57635e4361 100644 --- a/ironic/tests/api/test_ports.py +++ b/ironic/tests/api/test_ports.py @@ -82,7 +82,9 @@ class TestPatch(base.FunctionalTest): pdict = dbutils.get_test_port() extra = {'foo': 'bar'} response = self.patch_json('/ports/%s' % pdict['uuid'], - {'extra': extra}) + [{'path': '/extra/foo', + 'value': 'bar', + 'op': 'add'}]) self.assertEqual(response.content_type, 'application/json') self.assertEqual(response.status_code, 200) result = self.get_json('/ports/%s' % pdict['uuid']) @@ -92,7 +94,9 @@ class TestPatch(base.FunctionalTest): pdict = dbutils.get_test_port() extra = {'foo': 'bar'} response = self.patch_json('/ports/%s' % pdict['address'], - {'extra': extra}) + [{'path': '/extra/foo', + 'value': 'bar', + 'op': 'add'}]) self.assertEqual(response.content_type, 'application/json') self.assertEqual(response.status_code, 200) result = self.get_json('/ports/%s' % pdict['uuid']) @@ -100,13 +104,109 @@ class TestPatch(base.FunctionalTest): def test_update_not_found(self): uuid = uuidutils.generate_uuid() - response = self.patch_json('/ports/%s' % uuid, {'extra': {'a': 'b'}}, - expect_errors=True) + response = self.patch_json('/ports/%s' % uuid, + [{'path': '/extra/a', + 'value': 'b', + 'op': 'add'}], + expect_errors=True) # TODO(yuriyz): change to 404 (bug 1200517) self.assertEqual(response.status_int, 500) self.assertEqual(response.content_type, 'application/json') self.assertTrue(response.json['error_message']) + def test_replace_singular(self): + pdict = dbutils.get_test_port() + address = 'AA:BB:CC:DD:EE:FF' + response = self.patch_json('/ports/%s' % pdict['uuid'], + [{'path': '/address', + 'value': address, 'op': 'replace'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/ports/%s' % pdict['uuid']) + self.assertEqual(result['address'], address) + + def test_replace_multi(self): + extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"} + pdict = dbutils.get_test_port(extra=extra, + uuid=uuidutils.generate_uuid()) + self.post_json('/ports', pdict) + new_value = 'new value' + response = self.patch_json('/ports/%s' % pdict['uuid'], + [{'path': '/extra/foo2', + 'value': new_value, 'op': 'replace'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/ports/%s' % pdict['uuid']) + + extra["foo2"] = new_value + self.assertEqual(result['extra'], extra) + + def test_remove_singular(self): + pdict = dbutils.get_test_port(extra={'a': 'b'}, + uuid=uuidutils.generate_uuid()) + self.post_json('/ports', pdict) + response = self.patch_json('/ports/%s' % pdict['uuid'], + [{'path': '/address', 'op': 'remove'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/ports/%s' % pdict['uuid']) + self.assertEqual(result['address'], None) + + # Assert nothing else was changed + self.assertEqual(result['uuid'], pdict['uuid']) + self.assertEqual(result['extra'], pdict['extra']) + + def test_remove_multi(self): + extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"} + pdict = dbutils.get_test_port(extra=extra, + address="AA:BB:CC:DD:EE:FF", + uuid=uuidutils.generate_uuid()) + self.post_json('/ports', pdict) + + # Removing one item from the collection + response = self.patch_json('/ports/%s' % pdict['uuid'], + [{'path': '/extra/foo2', 'op': 'remove'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/ports/%s' % pdict['uuid']) + extra.pop("foo2") + self.assertEqual(result['extra'], extra) + + # Removing the collection + response = self.patch_json('/ports/%s' % pdict['uuid'], + [{'path': '/extra', 'op': 'remove'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/ports/%s' % pdict['uuid']) + self.assertEqual(result['extra'], {}) + + # Assert nothing else was changed + self.assertEqual(result['uuid'], pdict['uuid']) + self.assertEqual(result['address'], pdict['address']) + + def test_add_singular(self): + pdict = dbutils.get_test_port() + response = self.patch_json('/ports/%s' % pdict['uuid'], + [{'path': '/foo', 'value': 'bar', + 'op': 'add'}], + expect_errors=True) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_int, 400) + self.assertTrue(response.json['error_message']) + + def test_add_multi(self): + pdict = dbutils.get_test_port() + response = self.patch_json('/ports/%s' % pdict['uuid'], + [{'path': '/extra/foo1', 'value': 'bar1', + 'op': 'add'}, + {'path': '/extra/foo2', 'value': 'bar2', + 'op': 'add'}]) + self.assertEqual(response.content_type, 'application/json') + self.assertEqual(response.status_code, 200) + result = self.get_json('/ports/%s' % pdict['uuid']) + expected = {"foo1": "bar1", "foo2": "bar2"} + self.assertEqual(result['extra'], expected) + class TestPost(base.FunctionalTest): diff --git a/requirements.txt b/requirements.txt index 0a7d90a6c3..0a889cdf5f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,5 +22,6 @@ websockify>=0.5.1,<0.6 oslo.config>=1.1.0 pecan>=0.2.0 six<1.4.0 +jsonpatch>=1.1 WSME>=0.5b2 Cheetah>=2.4.4