Merge "Skip non-base properties in patch method"

3 files changed, 67 insertions, 4 deletions

diff --git a/glanceclient/v2/schemas.py b/glanceclient/v2/schemas.py
index 7cd169d..0dd2869 100644
--- a/glanceclient/v2/schemas.py
+++ b/glanceclient/v2/schemas.py
@@ -15,6 +15,7 @@
 
 import copy
 import jsonpatch
+import six
 import warlock.model as warlock
 
 
@@ -34,9 +35,10 @@ class SchemaBasedModel(warlock.Model):
         original = copy.deepcopy(self.__dict__['__original__'])
         new = dict(self)
         if self.__dict__['schema']:
-            for prop in self.schema['properties']:
-                if prop not in original and prop in new:
-                    original[prop] = None
+            for (name, prop) in six.iteritems(self.schema['properties']):
+                if (name not in original and name in new and
+                        prop.get('is_base', True)):
+                    original[name] = None
 
         return jsonpatch.make_patch(original, dict(self)).to_string()
 
diff --git a/tests/v2/test_images.py b/tests/v2/test_images.py
index 8e23a21..8e30074 100644
--- a/tests/v2/test_images.py
+++ b/tests/v2/test_images.py
@@ -58,6 +58,7 @@ data_fixtures = {
                             'required': ['url', 'metadata'],
                         },
                     },
+                    'color': {'type': 'string', 'is_base': False},
                 },
                 'additionalProperties': {'type': 'string'}
             },
@@ -125,6 +126,7 @@ data_fixtures = {
                 'name': 'image-3',
                 'barney': 'rubble',
                 'george': 'jetson',
+                'color': 'red',
             },
         ),
         'PATCH': (
@@ -367,7 +369,8 @@ schema_fixtures = {
                             },
                             'required': ['url', 'metadata'],
                         }
-                    }
+                    },
+                    'color': {'type': 'string', 'is_base': False},
                 },
                 'additionalProperties': {'type': 'string'}
             }
@@ -692,6 +695,38 @@ class TestController(testtools.TestCase):
         with testtools.ExpectedException(TypeError):
             self.controller.update(image_id, **params)
 
+    def test_update_add_custom_property(self):
+        image_id = '3a4560a1-e585-443e-9b39-553b46ec92d1'
+        params = {'color': 'red'}
+        image = self.controller.update(image_id, **params)
+        expect_hdrs = {
+            'Content-Type': 'application/openstack-images-v2.1-json-patch',
+        }
+        expect_body = '[{"path": "/color", "value": "red", "op": "add"}]'
+        expect = [
+            ('GET', '/v2/images/%s' % image_id, {}, None),
+            ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body),
+            ('GET', '/v2/images/%s' % image_id, {}, None),
+        ]
+        self.assertEqual(expect, self.api.calls)
+        self.assertEqual(image_id, image.id)
+
+    def test_update_replace_custom_property(self):
+        image_id = 'e7e59ff6-fa2e-4075-87d3-1a1398a07dc3'
+        params = {'color': 'blue'}
+        image = self.controller.update(image_id, **params)
+        expect_hdrs = {
+            'Content-Type': 'application/openstack-images-v2.1-json-patch',
+        }
+        expect_body = '[{"path": "/color", "value": "blue", "op": "replace"}]'
+        expect = [
+            ('GET', '/v2/images/%s' % image_id, {}, None),
+            ('PATCH', '/v2/images/%s' % image_id, expect_hdrs, expect_body),
+            ('GET', '/v2/images/%s' % image_id, {}, None),
+        ]
+        self.assertEqual(expect, self.api.calls)
+        self.assertEqual(image_id, image.id)
+
     def test_location_ops_when_server_disabled_location_ops(self):
         # Location operations should not be allowed if server has not
         # enabled location related operations
diff --git a/tests/v2/test_schemas.py b/tests/v2/test_schemas.py
index bc17cc4..ff7ddc2 100644
--- a/tests/v2/test_schemas.py
+++ b/tests/v2/test_schemas.py
@@ -50,6 +50,7 @@ _SCHEMA = schemas.Schema({
     'properties': {
         'name': {'type': 'string'},
         'color': {'type': 'string'},
+        'shape': {'type': 'string', 'is_base': False},
     },
 })
 
@@ -170,3 +171,28 @@ class TestSchemaBasedModel(testtools.TestCase):
         expected = '[{"path": "/color", "op": "remove"}]'
         self.assertTrue(compare_json_patches(patch, expected))
         self.assertEqual(expected, patch)
+
+    def test_patch_should_add_missing_custom_properties(self):
+        obj = {
+            'name': 'fred'
+        }
+
+        original = self.model(obj)
+        original['shape'] = 'circle'
+
+        patch = original.patch
+        expected = '[{"path": "/shape", "value": "circle", "op": "add"}]'
+        self.assertTrue(compare_json_patches(patch, expected))
+
+    def test_patch_should_replace_custom_properties(self):
+        obj = {
+            'name': 'fred',
+            'shape': 'circle'
+        }
+
+        original = self.model(obj)
+        original['shape'] = 'square'
+
+        patch = original.patch
+        expected = '[{"path": "/shape", "value": "square", "op": "replace"}]'
+        self.assertTrue(compare_json_patches(patch, expected))