Use PATCH instead of PUT for v2 image modification
With the previous partial put syntax in the v2 api, it was not possible to delete image properties. This patch provides a more expressive syntax in the form of the HTTP PATCH verb. This implementation depends on the `application/openstack-images-v2.0-json-patch` content-type which is intended to be a compatible subset of application/json-patch. Addresses bug 1039821. Change-Id: I510ab397f9b8b5bd1deec0fb25855e305e7ec86e
This commit is contained in:
Mark Washenberger
Brian Waldon
parent
3be563029a
commit
9c94a72c14
@ -16,6 +16,7 @@
|
||||
import copy
|
||||
import datetime
|
||||
import json
|
||||
import re
|
||||
import urllib
|
||||
|
||||
import webob.exc
|
||||
@ -148,21 +149,29 @@ class ImagesController(object):
|
||||
return self._append_tags(req.context, image)
|
||||
|
||||
@utils.mutating
|
||||
def update(self, req, image_id, image):
|
||||
def update(self, req, image_id, changes):
|
||||
self._enforce(req, 'modify_image')
|
||||
is_public = image.get('is_public')
|
||||
if is_public:
|
||||
self._enforce(req, 'publicize_image')
|
||||
tags = self._extract_tags(image)
|
||||
|
||||
context = req.context
|
||||
try:
|
||||
image = self.db_api.image_update(req.context, image_id, image)
|
||||
image = self.db_api.image_get(context, image_id)
|
||||
except (exception.NotFound, exception.Forbidden):
|
||||
msg = ("Failed to find image %(image_id)s to update" % locals())
|
||||
LOG.info(msg)
|
||||
raise webob.exc.HTTPNotFound(explanation=msg)
|
||||
|
||||
image = self._normalize_properties(dict(image))
|
||||
updates = self._extract_updates(req, image, changes)
|
||||
|
||||
tags = None
|
||||
if len(updates) > 0:
|
||||
tags = self._extract_tags(updates)
|
||||
purge_props = 'properties' in updates
|
||||
try:
|
||||
image = self.db_api.image_update(context, image_id, updates,
|
||||
purge_props)
|
||||
except (exception.NotFound, exception.Forbidden):
|
||||
raise webob.exc.HTTPNotFound()
|
||||
image = self._normalize_properties(dict(image))
|
||||
|
||||
v2.update_image_read_acl(req, self.db_api, image)
|
||||
|
||||
@ -175,6 +184,76 @@ class ImagesController(object):
|
||||
self.notifier.info('image.update', image)
|
||||
return image
|
||||
|
||||
def _extract_updates(self, req, image, changes):
|
||||
""" Determine the updates to pass to the database api.
|
||||
|
||||
Given the current image, convert a list of changes to be made
|
||||
into the corresponding update dictionary that should be passed to
|
||||
db_api.image_update.
|
||||
|
||||
Changes have the following parts
|
||||
op - 'add' a new attribute, 'replace' an existing attribute, or
|
||||
'remove' an existing attribute.
|
||||
path - A list of path parts for determining which attribute the
|
||||
the operation applies to.
|
||||
value - For 'add' and 'replace', the new value the attribute should
|
||||
assume.
|
||||
|
||||
For the current use case, there are two types of valid paths. For base
|
||||
attributes (fields stored directly on the Image object) the path
|
||||
must take the form ['<attribute name>']. These attributes are always
|
||||
present so the only valid operation on them is 'replace'. For image
|
||||
properties, the path takes the form ['properties', '<property name>']
|
||||
and all operations are valid.
|
||||
|
||||
Future refactoring should simplify this code by hardening the image
|
||||
abstraction such that database details such as how image properties
|
||||
are stored do not have any influence here.
|
||||
"""
|
||||
updates = {}
|
||||
property_updates = image['properties']
|
||||
for change in changes:
|
||||
path = change['path']
|
||||
if len(path) == 1:
|
||||
assert change['op'] == 'replace'
|
||||
key = change['path'][0]
|
||||
if key == 'is_public' and change['value']:
|
||||
self._enforce(req, 'publicize_image')
|
||||
updates[key] = change['value']
|
||||
else:
|
||||
assert len(path) == 2
|
||||
assert path[0] == 'properties'
|
||||
update_method_name = '_do_%s_property' % change['op']
|
||||
assert hasattr(self, update_method_name)
|
||||
update_method = getattr(self, update_method_name)
|
||||
update_method(property_updates, change)
|
||||
updates['properties'] = property_updates
|
||||
return updates
|
||||
|
||||
def _do_replace_property(self, updates, change):
|
||||
""" Replace a single image property, ensuring it's present. """
|
||||
key = change['path'][1]
|
||||
if key not in updates:
|
||||
msg = _("Property %s does not exist.")
|
||||
raise webob.exc.HTTPConflict(msg % key)
|
||||
updates[key] = change['value']
|
||||
|
||||
def _do_add_property(self, updates, change):
|
||||
""" Add a new image property, ensuring it does not already exist. """
|
||||
key = change['path'][1]
|
||||
if key in updates:
|
||||
msg = _("Property %s already present.")
|
||||
raise webob.exc.HTTPConflict(msg % key)
|
||||
updates[key] = change['value']
|
||||
|
||||
def _do_remove_property(self, updates, change):
|
||||
""" Remove an image property, ensuring it's present. """
|
||||
key = change['path'][1]
|
||||
if key not in updates:
|
||||
msg = _("Property %s does not exist.")
|
||||
raise webob.exc.HTTPConflict(msg % key)
|
||||
del updates[key]
|
||||
|
||||
@utils.mutating
|
||||
def delete(self, req, image_id):
|
||||
self._enforce(req, 'delete_image')
|
||||
@ -196,16 +275,21 @@ class ImagesController(object):
|
||||
|
||||
|
||||
class RequestDeserializer(wsgi.JSONRequestDeserializer):
|
||||
|
||||
_readonly_properties = ['created_at', 'updated_at', 'status', 'checksum',
|
||||
'size', 'direct_url', 'self', 'file', 'schema']
|
||||
_reserved_properties = ['owner', 'is_public', 'location',
|
||||
'deleted', 'deleted_at']
|
||||
_base_properties = ['checksum', 'created_at', 'container_format',
|
||||
'disk_format', 'id', 'min_disk', 'min_ram', 'name', 'size',
|
||||
'status', 'tags', 'updated_at', 'visibility', 'protected']
|
||||
|
||||
def __init__(self, schema=None):
|
||||
super(RequestDeserializer, self).__init__()
|
||||
self.schema = schema or get_schema()
|
||||
|
||||
def _parse_image(self, request):
|
||||
output = super(RequestDeserializer, self).default(request)
|
||||
if not 'body' in output:
|
||||
msg = _('Body expected in request.')
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
body = output.pop('body')
|
||||
body = self._get_request_body(request)
|
||||
try:
|
||||
self.schema.validate(body)
|
||||
except exception.InvalidObject as e:
|
||||
@ -218,9 +302,7 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer):
|
||||
# Create a dict of base image properties, with user- and deployer-
|
||||
# defined properties contained in a 'properties' dictionary
|
||||
image = {'properties': body}
|
||||
for key in ['checksum', 'created_at', 'container_format',
|
||||
'disk_format', 'id', 'min_disk', 'min_ram', 'name', 'size',
|
||||
'status', 'tags', 'updated_at', 'visibility', 'protected']:
|
||||
for key in self._base_properties:
|
||||
try:
|
||||
image[key] = image['properties'].pop(key)
|
||||
except KeyError:
|
||||
@ -231,17 +313,23 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer):
|
||||
|
||||
return {'image': image}
|
||||
|
||||
@staticmethod
|
||||
def _check_readonly(image):
|
||||
for key in ['created_at', 'updated_at', 'status', 'checksum', 'size',
|
||||
'direct_url', 'self', 'file', 'schema']:
|
||||
def _get_request_body(self, request):
|
||||
output = super(RequestDeserializer, self).default(request)
|
||||
if not 'body' in output:
|
||||
msg = _('Body expected in request.')
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
return output['body']
|
||||
|
||||
@classmethod
|
||||
def _check_readonly(cls, image):
|
||||
for key in cls._readonly_properties:
|
||||
if key in image:
|
||||
msg = "Attribute \'%s\' is read-only." % key
|
||||
raise webob.exc.HTTPForbidden(explanation=unicode(msg))
|
||||
|
||||
@staticmethod
|
||||
def _check_reserved(image):
|
||||
for key in ['owner', 'is_public', 'location', 'deleted', 'deleted_at']:
|
||||
@classmethod
|
||||
def _check_reserved(cls, image):
|
||||
for key in cls._reserved_properties:
|
||||
if key in image:
|
||||
msg = "Attribute \'%s\' is reserved." % key
|
||||
raise webob.exc.HTTPForbidden(explanation=unicode(msg))
|
||||
@ -249,8 +337,108 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer):
|
||||
def create(self, request):
|
||||
return self._parse_image(request)
|
||||
|
||||
def _get_change_operation(self, raw_change):
|
||||
op = None
|
||||
for key in ['replace', 'add', 'remove']:
|
||||
if key in raw_change:
|
||||
if op is not None:
|
||||
msg = _('Operation objects must contain only one member'
|
||||
' named "add", "remove", or "replace".')
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
op = key
|
||||
if op is None:
|
||||
msg = _('Operation objects must contain exactly one member'
|
||||
' named "add", "remove", or "replace".')
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
return op
|
||||
|
||||
def _get_change_path(self, raw_change, op):
|
||||
key = self._decode_json_pointer(raw_change[op])
|
||||
if key in self._readonly_properties:
|
||||
msg = "Attribute \'%s\' is read-only." % key
|
||||
raise webob.exc.HTTPForbidden(explanation=unicode(msg))
|
||||
if key in self._reserved_properties:
|
||||
msg = "Attribute \'%s\' is reserved." % key
|
||||
raise webob.exc.HTTPForbidden(explanation=unicode(msg))
|
||||
|
||||
# For image properties, we need to put "properties" at the beginning
|
||||
if key not in self._base_properties:
|
||||
return ['properties', key]
|
||||
return [key]
|
||||
|
||||
def _decode_json_pointer(self, pointer):
|
||||
""" Parse a json pointer.
|
||||
|
||||
Json Pointers are defined in
|
||||
http://tools.ietf.org/html/draft-pbryan-zyp-json-pointer .
|
||||
The pointers use '/' for separation between object attributes, such
|
||||
that '/A/B' would evaluate to C in {"A": {"B": "C"}}. A '/' character
|
||||
in an attribute name is encoded as "~1" and a '~' character is encoded
|
||||
as "~0".
|
||||
"""
|
||||
self._validate_json_pointer(pointer)
|
||||
return pointer.lstrip('/').replace('~1', '/').replace('~0', '~')
|
||||
|
||||
def _validate_json_pointer(self, pointer):
|
||||
""" Validate a json pointer.
|
||||
|
||||
We only accept a limited form of json pointers. Specifically, we do
|
||||
not allow multiple levels of indirection, so there can only be one '/'
|
||||
in the pointer, located at the start of the string.
|
||||
"""
|
||||
if not pointer.startswith('/'):
|
||||
msg = _('Pointer `%s` does not start with "/".' % pointer)
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
if '/' in pointer[1:]:
|
||||
msg = _('Pointer `%s` contains more than one "/".' % pointer)
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
if re.match('~[^01]', pointer):
|
||||
msg = _('Pointer `%s` contains "~" not part of'
|
||||
' a recognized escape sequence.' % pointer)
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
def _get_change_value(self, raw_change, op):
|
||||
if 'value' not in raw_change:
|
||||
msg = _('Operation "%s" requires a member named "value".')
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg % op)
|
||||
return raw_change['value']
|
||||
|
||||
def _validate_change(self, change):
|
||||
if change['op'] == 'delete':
|
||||
return
|
||||
partial_image = {change['path'][-1]: change['value']}
|
||||
try:
|
||||
self.schema.validate(partial_image)
|
||||
except exception.InvalidObject as e:
|
||||
raise webob.exc.HTTPBadRequest(explanation=unicode(e))
|
||||
|
||||
def update(self, request):
|
||||
return self._parse_image(request)
|
||||
changes = []
|
||||
valid_content_types = [
|
||||
'application/openstack-images-v2.0-json-patch'
|
||||
]
|
||||
if request.content_type not in valid_content_types:
|
||||
headers = {'Accept-Patch': ','.join(valid_content_types)}
|
||||
raise webob.exc.HTTPUnsupportedMediaType(headers=headers)
|
||||
body = self._get_request_body(request)
|
||||
if not isinstance(body, list):
|
||||
msg = _('Request body must be a JSON array of operation objects.')
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
for raw_change in body:
|
||||
if not isinstance(raw_change, dict):
|
||||
msg = _('Operations must be JSON objects.')
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
op = self._get_change_operation(raw_change)
|
||||
path = self._get_change_path(raw_change, op)
|
||||
change = {'op': op, 'path': path}
|
||||
if not op == 'remove':
|
||||
change['value'] = self._get_change_value(raw_change, op)
|
||||
self._validate_change(change)
|
||||
if change['path'] == ['visibility']:
|
||||
change['path'] = ['is_public']
|
||||
change['value'] = change['value'] == 'public'
|
||||
changes.append(change)
|
||||
return {'changes': changes}
|
||||
|
||||
def _validate_limit(self, limit):
|
||||
try:
|
||||
|
||||
@ -51,7 +51,7 @@ class API(wsgi.Router):
|
||||
mapper.connect('/images/{image_id}',
|
||||
controller=images_resource,
|
||||
action='update',
|
||||
conditions={'method': ['PUT']})
|
||||
conditions={'method': ['PATCH']})
|
||||
mapper.connect('/images/{image_id}',
|
||||
controller=images_resource,
|
||||
action='show',
|
||||
|
||||
@ -89,24 +89,34 @@ class TestImages(functional.FunctionalTest):
|
||||
self.assertFalse('size' in image)
|
||||
self.assertEqual('bar', image['foo'])
|
||||
self.assertEqual(False, image['protected'])
|
||||
self.assertEqual('kernel', image['type'])
|
||||
self.assertTrue(image['created_at'])
|
||||
self.assertTrue(image['updated_at'])
|
||||
self.assertEqual(image['updated_at'], image['created_at'])
|
||||
|
||||
# The image should be mutable, including adding new properties
|
||||
# The image should be mutable, including adding and removing properties
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
data = json.dumps({'name': 'image-2', 'format': 'vhd',
|
||||
'foo': 'baz', 'ping': 'pong', 'protected': True})
|
||||
response = requests.put(path, headers=self._headers(), data=data)
|
||||
self.assertEqual(200, response.status_code)
|
||||
media_type = 'application/openstack-images-v2.0-json-patch'
|
||||
headers = self._headers({'content-type': media_type})
|
||||
data = json.dumps([
|
||||
{'replace': '/name', 'value': 'image-2'},
|
||||
{'replace': '/disk_format', 'value': 'vhd'},
|
||||
{'replace': '/foo', 'value': 'baz'},
|
||||
{'add': '/ping', 'value': 'pong'},
|
||||
{'replace': '/protected', 'value': True},
|
||||
{'remove': '/type'},
|
||||
])
|
||||
response = requests.patch(path, headers=headers, data=data)
|
||||
self.assertEqual(200, response.status_code, response.text)
|
||||
|
||||
# Returned image entity should reflect the changes
|
||||
image = json.loads(response.text)
|
||||
self.assertEqual('image-2', image['name'])
|
||||
self.assertEqual('vhd', image['format'])
|
||||
self.assertEqual('vhd', image['disk_format'])
|
||||
self.assertEqual('baz', image['foo'])
|
||||
self.assertEqual('pong', image['ping'])
|
||||
self.assertEqual(True, image['protected'])
|
||||
self.assertFalse('type' in image, response.text)
|
||||
|
||||
# Updates should persist across requests
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
@ -118,6 +128,7 @@ class TestImages(functional.FunctionalTest):
|
||||
self.assertEqual('baz', image['foo'])
|
||||
self.assertEqual('pong', image['ping'])
|
||||
self.assertEqual(True, image['protected'])
|
||||
self.assertFalse('type' in image, response.text)
|
||||
|
||||
# Try to download data before its uploaded
|
||||
path = self._url('/v2/images/%s/file' % image_id)
|
||||
@ -167,9 +178,11 @@ class TestImages(functional.FunctionalTest):
|
||||
|
||||
# Unprotect image for deletion
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
data = json.dumps({'protected': False})
|
||||
response = requests.put(path, headers=self._headers(), data=data)
|
||||
self.assertEqual(200, response.status_code)
|
||||
media_type = 'application/openstack-images-v2.0-json-patch'
|
||||
headers = self._headers({'content-type': media_type})
|
||||
data = json.dumps([{'replace': '/protected', 'value': False}])
|
||||
response = requests.patch(path, headers=headers, data=data)
|
||||
self.assertEqual(200, response.status_code, response.text)
|
||||
|
||||
# Deletion should work
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
@ -234,11 +247,11 @@ class TestImages(functional.FunctionalTest):
|
||||
# TENANT2 should not be able to modify the image, either
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
headers = self._headers({
|
||||
'Content-Type': 'application/json',
|
||||
'Content-Type': 'application/openstack-images-v2.0-json-patch',
|
||||
'X-Tenant-Id': TENANT2,
|
||||
})
|
||||
data = json.dumps({'name': 'image-2'})
|
||||
response = requests.put(path, headers=headers, data=data)
|
||||
data = json.dumps([{'replace': '/name', 'value': 'image-2'}])
|
||||
response = requests.patch(path, headers=headers, data=data)
|
||||
self.assertEqual(404, response.status_code)
|
||||
|
||||
# TENANT2 should not be able to delete the image, either
|
||||
@ -250,11 +263,11 @@ class TestImages(functional.FunctionalTest):
|
||||
# Publicize the image as an admin of TENANT1
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
headers = self._headers({
|
||||
'Content-Type': 'application/json',
|
||||
'Content-Type': 'application/openstack-images-v2.0-json-patch',
|
||||
'X-Roles': 'admin',
|
||||
})
|
||||
data = json.dumps({'visibility': 'public'})
|
||||
response = requests.put(path, headers=headers, data=data)
|
||||
data = json.dumps([{'replace': '/visibility', 'value': 'public'}])
|
||||
response = requests.patch(path, headers=headers, data=data)
|
||||
self.assertEqual(200, response.status_code)
|
||||
|
||||
# TENANT3 should now see the image in their list
|
||||
@ -274,11 +287,11 @@ class TestImages(functional.FunctionalTest):
|
||||
# TENANT3 still should not be able to modify the image
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
headers = self._headers({
|
||||
'Content-Type': 'application/json',
|
||||
'Content-Type': 'application/openstack-images-v2.0-json-patch',
|
||||
'X-Tenant-Id': TENANT3,
|
||||
})
|
||||
data = json.dumps({'name': 'image-2'})
|
||||
response = requests.put(path, headers=headers, data=data)
|
||||
data = json.dumps([{'replace': '/name', 'value': 'image-2'}])
|
||||
response = requests.patch(path, headers=headers, data=data)
|
||||
self.assertEqual(404, response.status_code)
|
||||
|
||||
# TENANT3 should not be able to delete the image, either
|
||||
@ -307,9 +320,11 @@ class TestImages(functional.FunctionalTest):
|
||||
|
||||
# Update image with duplicate tag - it should be ignored
|
||||
path = self._url('/v2/images/%s' % image_id)
|
||||
headers = self._headers({'Content-Type': 'application/json'})
|
||||
data = json.dumps({'tags': ['sniff', 'snozz', 'snozz']})
|
||||
response = requests.put(path, headers=headers, data=data)
|
||||
media_type = 'application/openstack-images-v2.0-json-patch'
|
||||
headers = self._headers({'content-type': media_type})
|
||||
data = json.dumps([{'replace': '/tags',
|
||||
'value': ['sniff', 'snozz', 'snozz']}])
|
||||
response = requests.patch(path, headers=headers, data=data)
|
||||
self.assertEqual(200, response.status_code)
|
||||
tags = json.loads(response.text)['tags']
|
||||
self.assertEqual(['snozz', 'sniff'], tags)
|
||||
|
||||
@ -359,13 +359,12 @@ class TestImagesController(test_utils.BaseTestCase):
|
||||
}
|
||||
self.assertEqual(output_log, expected_log)
|
||||
|
||||
def test_update(self):
|
||||
def test_update_no_changes(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = {'name': 'image-2'}
|
||||
output = self.controller.update(request, UUID1, image)
|
||||
self.assertEqual(UUID1, output['id'])
|
||||
self.assertEqual('image-2', output['name'])
|
||||
self.assertNotEqual(output['created_at'], output['updated_at'])
|
||||
output = self.controller.update(request, UUID1, changes=[])
|
||||
self.assertEqual(output['id'], UUID1)
|
||||
self.assertEqual(output['created_at'], output['updated_at'])
|
||||
self.assertTrue('tags' in output)
|
||||
output_log = self.notifier.get_log()
|
||||
expected_log = {'notification_type': "INFO",
|
||||
'event_type': "image.update",
|
||||
@ -373,16 +372,158 @@ class TestImagesController(test_utils.BaseTestCase):
|
||||
}
|
||||
self.assertEqual(output_log, expected_log)
|
||||
|
||||
def test_update_non_existent(self):
|
||||
def test_update_image_doesnt_exist(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = {'name': 'image-2'}
|
||||
self.assertRaises(webob.exc.HTTPNotFound, self.controller.update,
|
||||
request, utils.generate_uuid(), image)
|
||||
request, utils.generate_uuid(), changes=[])
|
||||
|
||||
def test_update_replace_base_attribute(self):
|
||||
self.db.image_update(None, UUID1, {'properties': {'foo': 'bar'}})
|
||||
request = unit_test_utils.get_fake_request()
|
||||
changes = [{'op': 'replace', 'path': ['name'], 'value': 'fedora'}]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertEqual(output['id'], UUID1)
|
||||
self.assertEqual(output['name'], 'fedora')
|
||||
self.assertEqual(output['properties'], {'foo': 'bar'})
|
||||
self.assertNotEqual(output['created_at'], output['updated_at'])
|
||||
|
||||
def test_update_replace_tags(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
changes = [
|
||||
{'op': 'replace', 'path': ['tags'], 'value': ['king', 'kong']},
|
||||
]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertEqual(output['id'], UUID1)
|
||||
self.assertEqual(output['tags'], ['king', 'kong'])
|
||||
self.assertNotEqual(output['created_at'], output['updated_at'])
|
||||
|
||||
def test_update_replace_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
properties = {'foo': 'bar', 'snitch': 'golden'}
|
||||
self.db.image_update(None, UUID1, {'properties': properties})
|
||||
|
||||
output = self.controller.show(request, UUID1)
|
||||
self.assertEqual(output['properties']['foo'], 'bar')
|
||||
self.assertEqual(output['properties']['snitch'], 'golden')
|
||||
|
||||
changes = [
|
||||
{'op': 'replace', 'path': ['properties', 'foo'], 'value': 'baz'},
|
||||
]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertEqual(output['id'], UUID1)
|
||||
self.assertEqual(output['properties']['foo'], 'baz')
|
||||
self.assertEqual(output['properties']['snitch'], 'golden')
|
||||
self.assertNotEqual(output['created_at'], output['updated_at'])
|
||||
|
||||
def test_update_add_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
output = self.controller.show(request, UUID1)
|
||||
self.assertEqual(len(output['properties']), 0)
|
||||
|
||||
changes = [
|
||||
{'op': 'add', 'path': ['properties', 'murphy'], 'value': 'brown'},
|
||||
]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertEqual(output['id'], UUID1)
|
||||
self.assertEqual(output['properties'], {'murphy': 'brown'})
|
||||
self.assertNotEqual(output['created_at'], output['updated_at'])
|
||||
|
||||
def test_update_remove_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
properties = {'foo': 'bar', 'snitch': 'golden'}
|
||||
self.db.image_update(None, UUID1, {'properties': properties})
|
||||
|
||||
output = self.controller.show(request, UUID1)
|
||||
self.assertEqual(output['properties']['foo'], 'bar')
|
||||
self.assertEqual(output['properties']['snitch'], 'golden')
|
||||
|
||||
changes = [
|
||||
{'op': 'remove', 'path': ['properties', 'snitch']},
|
||||
]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertEqual(output['id'], UUID1)
|
||||
self.assertEqual(output['properties'], {'foo': 'bar'})
|
||||
self.assertNotEqual(output['created_at'], output['updated_at'])
|
||||
|
||||
def test_update_multiple_changes(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
properties = {'foo': 'bar', 'snitch': 'golden'}
|
||||
self.db.image_update(None, UUID1, {'properties': properties})
|
||||
|
||||
changes = [
|
||||
{'op': 'replace', 'path': ['min_ram'], 'value': 128},
|
||||
{'op': 'replace', 'path': ['properties', 'foo'], 'value': 'baz'},
|
||||
{'op': 'remove', 'path': ['properties', 'snitch']},
|
||||
{'op': 'add', 'path': ['properties', 'kb'], 'value': 'dvorak'},
|
||||
]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertEqual(output['id'], UUID1)
|
||||
self.assertEqual(output['min_ram'], 128)
|
||||
self.assertEqual(len(output['properties']), 2)
|
||||
self.assertEqual(output['properties']['foo'], 'baz')
|
||||
self.assertEqual(output['properties']['kb'], 'dvorak')
|
||||
self.assertNotEqual(output['created_at'], output['updated_at'])
|
||||
|
||||
def test_update_replace_missing_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
output = self.controller.show(request, UUID1)
|
||||
self.assertEqual(len(output['properties']), 0)
|
||||
|
||||
changes = [
|
||||
{'op': 'replace', 'path': ['properties', 'foo'], 'value': 'baz'},
|
||||
]
|
||||
self.assertRaises(webob.exc.HTTPConflict,
|
||||
self.controller.update, request, UUID1, changes)
|
||||
|
||||
def test_update_add_property_already_present(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
properties = {'foo': 'bar'}
|
||||
self.db.image_update(None, UUID1, {'properties': properties})
|
||||
|
||||
output = self.controller.show(request, UUID1)
|
||||
self.assertEqual(output['properties'], {'foo': 'bar'})
|
||||
|
||||
changes = [
|
||||
{'op': 'add', 'path': ['properties', 'foo'], 'value': 'baz'},
|
||||
]
|
||||
self.assertRaises(webob.exc.HTTPConflict,
|
||||
self.controller.update, request, UUID1, changes)
|
||||
|
||||
def test_update_remove_missing_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
output = self.controller.show(request, UUID1)
|
||||
self.assertEqual(len(output['properties']), 0)
|
||||
|
||||
changes = [
|
||||
{'op': 'remove', 'path': ['properties', 'foo']},
|
||||
]
|
||||
self.assertRaises(webob.exc.HTTPConflict,
|
||||
self.controller.update, request, UUID1, changes)
|
||||
|
||||
def test_update_assertion_errors(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
changes = [
|
||||
{'op': 'add', 'path': ['properties', 'a', 'b'], 'value': 'c'},
|
||||
{'op': 'add', 'path': ['prolilies', 'foo'], 'value': 'bar'},
|
||||
{'op': 'replace', 'path': [], 'value': 'stuff'},
|
||||
{'op': 'add', 'path': ['name'], 'value': 'Beulah'},
|
||||
{'op': 'remove', 'path': ['name']},
|
||||
{'op': 'test', 'path': ['properties', 'options'], 'value': 'puts'},
|
||||
]
|
||||
for change in changes:
|
||||
try:
|
||||
self.controller.update(request, UUID1, [change])
|
||||
except AssertionError:
|
||||
pass # AssertionError is the desired behavior
|
||||
else:
|
||||
self.fail('Failed to raise AssertionError on %s' % change)
|
||||
|
||||
def test_update_duplicate_tags(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = {'tags': ['ping', 'ping']}
|
||||
output = self.controller.update(request, UUID1, image)
|
||||
changes = [
|
||||
{'op': 'replace', 'path': ['tags'], 'value': ['ping', 'ping']},
|
||||
]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertEqual(['ping'], output['tags'])
|
||||
output_log = self.notifier.get_log()
|
||||
expected_log = {'notification_type': "INFO",
|
||||
@ -448,34 +589,25 @@ class TestImagesControllerPolicies(base.IsolatedUnitTest):
|
||||
rules = {"modify_image": False}
|
||||
self.policy.set_rules(rules)
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = {'name': 'image-2'}
|
||||
changes = [{'op': 'replace', 'path': ['name'], 'value': 'image-2'}]
|
||||
self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
|
||||
request, UUID1, image)
|
||||
request, UUID1, changes)
|
||||
|
||||
def test_update_publicize_image_unauthorized(self):
|
||||
rules = {"publicize_image": False}
|
||||
self.policy.set_rules(rules)
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = {'name': 'image-1', 'is_public': True}
|
||||
changes = [{'op': 'replace', 'path': ['is_public'], 'value': True}]
|
||||
self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
|
||||
request, UUID1, image)
|
||||
request, UUID1, changes)
|
||||
|
||||
def test_update_public_image_unauthorized(self):
|
||||
rules = {"modify_image": False}
|
||||
self.policy.set_rules(rules)
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = {'name': 'image-1', 'is_public': True}
|
||||
self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
|
||||
request, UUID1, image)
|
||||
|
||||
def test_update_public_image_unauthorized_but_not_publicizing(self):
|
||||
def test_update_depublicize_image_unauthorized(self):
|
||||
rules = {"publicize_image": False}
|
||||
self.policy.set_rules(rules)
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = {'name': 'image-2', 'is_public': False}
|
||||
output = self.controller.update(request, UUID1, image)
|
||||
self.assertEqual(UUID1, output['id'])
|
||||
self.assertEqual('image-2', output['name'])
|
||||
changes = [{'op': 'replace', 'path': ['is_public'], 'value': False}]
|
||||
output = self.controller.update(request, UUID1, changes)
|
||||
self.assertFalse(output['is_public'])
|
||||
|
||||
def test_delete_unauthorized(self):
|
||||
rules = {"delete_image": False}
|
||||
@ -557,58 +689,210 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
|
||||
self.assertRaises(webob.exc.HTTPForbidden,
|
||||
self.deserializer.create, request)
|
||||
|
||||
def test_update(self):
|
||||
def _get_fake_patch_request(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.body = json.dumps({
|
||||
'id': UUID3,
|
||||
'name': 'image-1',
|
||||
'visibility': 'public',
|
||||
'tags': ['one', 'two'],
|
||||
'container_format': 'ami',
|
||||
'disk_format': 'ami',
|
||||
'min_ram': 128,
|
||||
'min_disk': 10,
|
||||
'foo': 'bar',
|
||||
'protected': True,
|
||||
})
|
||||
request.content_type = 'application/openstack-images-v2.0-json-patch'
|
||||
return request
|
||||
|
||||
def test_update_empty_body(self):
|
||||
request = self._get_fake_patch_request()
|
||||
request.body = json.dumps([])
|
||||
output = self.deserializer.update(request)
|
||||
expected = {'image': {
|
||||
'id': UUID3,
|
||||
'name': 'image-1',
|
||||
'is_public': True,
|
||||
'tags': ['one', 'two'],
|
||||
'container_format': 'ami',
|
||||
'disk_format': 'ami',
|
||||
'min_ram': 128,
|
||||
'min_disk': 10,
|
||||
'properties': {'foo': 'bar'},
|
||||
'protected': True,
|
||||
}}
|
||||
self.assertEqual(expected, output)
|
||||
expected = {'changes': []}
|
||||
self.assertEquals(output, expected)
|
||||
|
||||
def test_update_no_body(self):
|
||||
def test_update_invalid_content_type(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
self.assertRaises(webob.exc.HTTPBadRequest, self.deserializer.update,
|
||||
request)
|
||||
request.content_type = 'application/json-patch'
|
||||
request.body = json.dumps([])
|
||||
try:
|
||||
self.deserializer.update(request)
|
||||
except webob.exc.HTTPUnsupportedMediaType as e:
|
||||
# desired result, but must have correct Accept-Patch header
|
||||
self.assertEqual(e.headers['Accept-Patch'],
|
||||
'application/openstack-images-v2.0-json-patch')
|
||||
else:
|
||||
self.fail('Did not raise HTTPUnsupportedMediaType')
|
||||
|
||||
def test_update_readonly_attributes_forbidden(self):
|
||||
def test_update_body_not_a_list(self):
|
||||
bodies = [
|
||||
{'created_at': ISOTIME},
|
||||
{'updated_at': ISOTIME},
|
||||
{'status': 'saving'},
|
||||
{'direct_url': 'http://example.com'},
|
||||
{'size': 10},
|
||||
{'checksum': 'asdf'},
|
||||
{'self': 'http://example.com'},
|
||||
{'file': 'http://example.com'},
|
||||
{'schema': 'http://example.com'},
|
||||
{'add': '/someprop', 'value': 'somevalue'},
|
||||
'just some string',
|
||||
123,
|
||||
True,
|
||||
False,
|
||||
None,
|
||||
]
|
||||
for body in bodies:
|
||||
request = self._get_fake_patch_request()
|
||||
request.body = json.dumps(body)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
def test_update_invalid_changes(self):
|
||||
changes = [
|
||||
['a', 'list', 'of', 'stuff'],
|
||||
'just some string',
|
||||
123,
|
||||
True,
|
||||
False,
|
||||
None,
|
||||
]
|
||||
for change in changes:
|
||||
request = self._get_fake_patch_request()
|
||||
request.body = json.dumps([change])
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
def test_update(self):
|
||||
request = self._get_fake_patch_request()
|
||||
body = [
|
||||
{'replace': '/name', 'value': 'fedora'},
|
||||
{'replace': '/tags', 'value': ['king', 'kong']},
|
||||
{'replace': '/foo', 'value': 'bar'},
|
||||
{'add': '/bebim', 'value': 'bap'},
|
||||
{'remove': '/sparks'},
|
||||
]
|
||||
request.body = json.dumps(body)
|
||||
output = self.deserializer.update(request)
|
||||
expected = {'changes': [
|
||||
{'op': 'replace', 'path': ['name'], 'value': 'fedora'},
|
||||
{'op': 'replace', 'path': ['tags'], 'value': ['king', 'kong']},
|
||||
{'op': 'replace', 'path': ['properties', 'foo'], 'value': 'bar'},
|
||||
{'op': 'add', 'path': ['properties', 'bebim'], 'value': 'bap'},
|
||||
{'op': 'remove', 'path': ['properties', 'sparks']},
|
||||
]}
|
||||
self.assertEquals(output, expected)
|
||||
|
||||
def test_update_base_attributes(self):
|
||||
request = self._get_fake_patch_request()
|
||||
body = [
|
||||
{'replace': '/id', 'value': UUID1},
|
||||
{'replace': '/name', 'value': 'fedora'},
|
||||
{'replace': '/visibility', 'value': 'public'},
|
||||
{'replace': '/visibility', 'value': 'private'},
|
||||
{'replace': '/tags', 'value': ['king', 'kong']},
|
||||
{'replace': '/protected', 'value': True},
|
||||
{'replace': '/container_format', 'value': 'bare'},
|
||||
{'replace': '/disk_format', 'value': 'raw'},
|
||||
{'replace': '/min_ram', 'value': 128},
|
||||
{'replace': '/min_disk', 'value': 10},
|
||||
]
|
||||
request.body = json.dumps(body)
|
||||
output = self.deserializer.update(request)
|
||||
expected = {'changes': [
|
||||
{'op': 'replace', 'path': ['id'], 'value': UUID1},
|
||||
{'op': 'replace', 'path': ['name'], 'value': 'fedora'},
|
||||
{'op': 'replace', 'path': ['is_public'], 'value': True},
|
||||
{'op': 'replace', 'path': ['is_public'], 'value': False},
|
||||
{'op': 'replace', 'path': ['tags'], 'value': ['king', 'kong']},
|
||||
{'op': 'replace', 'path': ['protected'], 'value': True},
|
||||
{'op': 'replace', 'path': ['container_format'], 'value': 'bare'},
|
||||
{'op': 'replace', 'path': ['disk_format'], 'value': 'raw'},
|
||||
{'op': 'replace', 'path': ['min_ram'], 'value': 128},
|
||||
{'op': 'replace', 'path': ['min_disk'], 'value': 10},
|
||||
]}
|
||||
self.assertEquals(output, expected)
|
||||
|
||||
def test_update_readonly_attributes(self):
|
||||
samples = {
|
||||
'status': 'active',
|
||||
'checksum': 'abcdefghijklmnopqrstuvwxyz012345',
|
||||
'size': 9001,
|
||||
'created_at': ISOTIME,
|
||||
'updated_at': ISOTIME,
|
||||
'direct_url': '/a/b/c/d',
|
||||
'self': '/e/f/g/h',
|
||||
'file': '/e/f/g/h/file',
|
||||
'schema': '/i/j/k',
|
||||
}
|
||||
|
||||
for key, value in samples.items():
|
||||
request = self._get_fake_patch_request()
|
||||
body = [{'replace': '/%s' % key, 'value': value}]
|
||||
request.body = json.dumps(body)
|
||||
try:
|
||||
self.deserializer.update(request)
|
||||
except webob.exc.HTTPForbidden:
|
||||
pass # desired behavior
|
||||
else:
|
||||
self.fail("Updating %s did not result in HTTPForbidden" % key)
|
||||
|
||||
def test_update_reserved_attributes(self):
|
||||
samples = {
|
||||
'owner': TENANT1,
|
||||
'is_public': True,
|
||||
'location': '/a/b/c/d',
|
||||
'deleted': False,
|
||||
'deleted_at': ISOTIME,
|
||||
}
|
||||
|
||||
for key, value in samples.items():
|
||||
request = self._get_fake_patch_request()
|
||||
body = [{'replace': '/%s' % key, 'value': value}]
|
||||
request.body = json.dumps(body)
|
||||
try:
|
||||
self.deserializer.update(request)
|
||||
except webob.exc.HTTPForbidden:
|
||||
pass # desired behavior
|
||||
else:
|
||||
self.fail("Updating %s did not result in HTTPForbidden" % key)
|
||||
|
||||
def test_update_invalid_attributes(self):
|
||||
keys = [
|
||||
'noslash',
|
||||
'//twoslash',
|
||||
'/other/twoslash'
|
||||
'/trailingslash/',
|
||||
'/lone~tilde'
|
||||
'/trailingtilde~'
|
||||
]
|
||||
|
||||
for body in bodies:
|
||||
request = unit_test_utils.get_fake_request()
|
||||
for key in keys:
|
||||
request = self._get_fake_patch_request()
|
||||
body = [{'replace': '%s' % key, 'value': 'dummy'}]
|
||||
request.body = json.dumps(body)
|
||||
self.assertRaises(webob.exc.HTTPForbidden,
|
||||
self.deserializer.update, request)
|
||||
try:
|
||||
self.deserializer.update(request)
|
||||
except webob.exc.HTTPBadRequest:
|
||||
pass # desired behavior
|
||||
else:
|
||||
self.fail("Updating %s did not result in HTTPBadRequest" % key)
|
||||
|
||||
def test_update_pointer_encoding(self):
|
||||
samples = {
|
||||
'/keywith~1slash': 'keywith/slash',
|
||||
'/keywith~0tilde': 'keywith~tilde',
|
||||
'/tricky~01': 'tricky~1',
|
||||
}
|
||||
|
||||
for encoded, decoded in samples.items():
|
||||
request = self._get_fake_patch_request()
|
||||
body = [{'replace': '%s' % encoded, 'value': 'dummy'}]
|
||||
request.body = json.dumps(body)
|
||||
output = self.deserializer.update(request)
|
||||
self.assertEqual(output['changes'][0]['path'][1], decoded)
|
||||
|
||||
def test_update_multiple_operations(self):
|
||||
request = self._get_fake_patch_request()
|
||||
body = [{'replace': '/foo', 'add': '/bar', 'value': 'snore'}]
|
||||
request.body = json.dumps(body)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
def test_update_missing_operations(self):
|
||||
request = self._get_fake_patch_request()
|
||||
body = [{'value': 'arcata'}]
|
||||
request.body = json.dumps(body)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
def test_update_missing_value(self):
|
||||
request = self._get_fake_patch_request()
|
||||
body = [{'replace': '/colburn'}]
|
||||
request.body = json.dumps(body)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
def test_index(self):
|
||||
marker = utils.generate_uuid()
|
||||
@ -755,19 +1039,18 @@ class TestImagesDeserializerWithExtendedSchema(test_utils.BaseTestCase):
|
||||
|
||||
def test_update(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.body = json.dumps({'name': 'image-1', 'pants': 'off'})
|
||||
request.content_type = 'application/openstack-images-v2.0-json-patch'
|
||||
request.body = json.dumps([{'add': '/pants', 'value': 'off'}])
|
||||
output = self.deserializer.update(request)
|
||||
expected = {
|
||||
'image': {
|
||||
'name': 'image-1',
|
||||
'properties': {'pants': 'off'},
|
||||
},
|
||||
}
|
||||
expected = {'changes': [
|
||||
{'op': 'add', 'path': ['properties', 'pants'], 'value': 'off'},
|
||||
]}
|
||||
self.assertEqual(expected, output)
|
||||
|
||||
def test_update_bad_data(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.body = json.dumps({'name': 'image-1', 'pants': 'borked'})
|
||||
request.content_type = 'application/openstack-images-v2.0-json-patch'
|
||||
request.body = json.dumps([{'add': '/pants', 'value': 'cutoffs'}])
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
@ -792,18 +1075,33 @@ class TestImagesDeserializerWithAdditionalProperties(test_utils.BaseTestCase):
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.create, request)
|
||||
|
||||
def test_update_with_numeric_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.content_type = 'application/openstack-images-v2.0-json-patch'
|
||||
request.body = json.dumps([{'add': '/foo', 'value': 123}])
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
def test_create_with_list_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.body = json.dumps({'foo': ['bar']})
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.create, request)
|
||||
|
||||
def test_update_with_list_property(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.content_type = 'application/openstack-images-v2.0-json-patch'
|
||||
request.body = json.dumps([{'add': '/foo', 'value': ['bar', 'baz']}])
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
def test_update(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.body = json.dumps({'foo': 'bar'})
|
||||
request.content_type = 'application/openstack-images-v2.0-json-patch'
|
||||
request.body = json.dumps([{'add': '/foo', 'value': 'bar'}])
|
||||
output = self.deserializer.update(request)
|
||||
expected = {'image': {'properties': {'foo': 'bar'}}}
|
||||
self.assertEqual(expected, output)
|
||||
change = {'op': 'add', 'path': ['properties', 'foo'], 'value': 'bar'}
|
||||
self.assertEqual(output, {'changes': [change]})
|
||||
|
||||
|
||||
class TestImagesDeserializerNoAdditionalProperties(test_utils.BaseTestCase):
|
||||
@ -822,7 +1120,8 @@ class TestImagesDeserializerNoAdditionalProperties(test_utils.BaseTestCase):
|
||||
|
||||
def test_update(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
request.body = json.dumps({'foo': 'bar'})
|
||||
request.content_type = 'application/openstack-images-v2.0-json-patch'
|
||||
request.body = json.dumps([{'add': '/foo', 'value': 'bar'}])
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.update, request)
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user