Merge "Updating resources with PATCH"

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/<id>    Retrieve a specific node.
 POST     /nodes         Create a new node
-PUT      /nodes/<id>    Update a node
+PATCH    /nodes/<id>    Update a node
 DELETE   /nodes/<id>    Delete node and all associated ports
 =======  =============  ==========
 
@@ -478,7 +572,7 @@ Verb     Path           Response
 GET      /chassis       List chassis
 GET      /chassis/<id>  Retrieve a specific chassis
 POST     /chassis       Create a new chassis
-PUT      /chassis/<id>  Update a chassis
+PATCH    /chassis/<id>  Update a chassis
 DELETE   /chassis/<id>  Delete chassis and remove all associations between
                         nodes
 =======  =============  ==========
@@ -543,7 +637,7 @@ Verb     Path           Response
 GET      /ports         List ports
 GET      /ports/<id>    Retrieve a specific port
 POST     /ports         Create a new port
-PUT      /ports/<id>    Update a port
+PATCH    /ports/<id>    Update a port
 DELETE   /ports/<id>    Delete port and remove all associations between nodes
 =======  =============  ==========
 

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())

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)
+    @wsme_pecan.wsexpose(Chassis, unicode, body=[unicode])
-    def patch(self, uuid, delta_chassis):
+    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()
+        chassis_dict = chassis.as_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"))
 
-        for k in nn_delta_ch:
+        # These are internal values that shouldn't be part of the patch
-            chassis[k] = nn_delta_ch[k]
+        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)

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)
+    @wsme_pecan.wsexpose(Node, unicode, body=[unicode])
-    def patch(self, node_id, node_data):
+    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
+        node = objects.Node.get_by_uuid(pecan.request.context, uuid)
-        #       so we eliminate non-supplied fields by converting
+        node_dict = node.as_dict()
-        #       to a dict and stripping keys with value=None
+
-        delta = node_data.as_terse_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',
+        state_rel_path = ['/power_state', '/target_power_state',
-                          'provision_state', 'target_provision_state']
+                          '/provision_state', '/target_provision_state']
-        if any((getattr(node_data, attr) for attr in state_rel_attr)):
+        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(
+            # In case of a remove operation, add the missing fields back to
-                        pecan.request.context, node_id)
+            # the document with their default value
-            for k in delta.keys():
+            defaults = objects.Node.get_defaults()
-                node[k] = delta[k]
+            defaults.update(final_patch)
-            node = pecan.request.rpcapi.update_node(
+
-                    pecan.request.context, node)
+            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)

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)
+    @wsme_pecan.wsexpose(Port, unicode, body=[unicode])
-    def patch(self, uuid, port_data):
+    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()
+        port_dict = port.as_dict()
-        for k in nn_delta_p:
+
-            port[k] = nn_delta_p[k]
+        # 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)
 

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)

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.

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):
 

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'}},
+                                   [{'path': '/driver_info/this',
-                expect_errors=True)
+                                     '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'},
+                                   [{'path': '/instance_uuid',
-                expect_errors=True)
+                                     '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):
 

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'}},
+        response = self.patch_json('/ports/%s' % uuid,
-                                   expect_errors=True)
+                                   [{'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):
 

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