openstack/zaqar
Code Issues Proposed changes

Fix PATCH queue's metadata

Browse Source 
Currently, the way queue's PATCH HTTP method is implemented in API v2 is
not correct.

This patch makes PATCH HTTP method to work as RFC6902 recommends:
http://tools.ietf.org/html/rfc6902
This patch adds tests.
This patch makes Zaqar return 404 response in the case when the user
tries to perform PATCH on non-existing queue.

APIImpact
DocImpact

Closes-Bug: #1553002
Co-Authored-By: Fei Long Wang <flwang@catalyst.net.nz>
Co-Authored-By: Eva Balycheva <ubershy@gmail.com>
Change-Id: Id1f02c63e8f078ddb657d3e9bc83ac0668672748
This commit is contained in:
Fei Long Wang 2016-03-15 14:04:52 +13:00 committed by Eva Balycheva
parent 6e2b9e444d
commit 3ba349e11d
5 changed files with 309 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
zaqar/tests/unit/transport/wsgi/v2_0/test_queue_lifecycle.py
View File

@ -218,31 +218,86 @@ class TestQueueLifecycleMongoDB(base.V2Base):
def test_update_metadata(self):
xyz_queue_path = self.url_prefix + '/queues/xyz'
xyz_queue_path_metadata = xyz_queue_path
headers = {
'Client-ID': str(uuid.uuid4()),
'X-Project-ID': str(uuid.uuid4())
}
# Create
self.simulate_put(xyz_queue_path, headers=self.headers)
self.simulate_put(xyz_queue_path, headers=headers)
self.assertEqual(falcon.HTTP_201, self.srmock.status)
# Set meta
doc1 = '{"messages": {"ttl": 600}}'
headers.update({'Content-Type':
"application/openstack-messaging-v2.0-json-patch"})
# add metadata
doc1 = ('[{"op":"add", "path": "/metadata/key1", "value": 1},'
'{"op":"add", "path": "/metadata/key2", "value": 1}]')
self.simulate_patch(xyz_queue_path_metadata,
headers=self.headers,
headers=headers,
body=doc1)
self.assertEqual(falcon.HTTP_200, self.srmock.status)
# Update
doc2 = '{"messages": {"ttl": 100}}'
# replace metadata
doc2 = '[{"op":"replace", "path": "/metadata/key1", "value": 2}]'
self.simulate_patch(xyz_queue_path_metadata,
headers=self.headers,
headers=headers,
body=doc2)
self.assertEqual(falcon.HTTP_200, self.srmock.status)
# Get
result = self.simulate_get(xyz_queue_path_metadata,
headers=self.headers)
headers=headers)
result_doc = jsonutils.loads(result[0])
self.assertEqual({'key1': 2, 'key2': 1}, result_doc)
self.assertEqual(jsonutils.loads(doc2), result_doc)
# remove metadata
doc3 = '[{"op":"remove", "path": "/metadata/key1"}]'
self.simulate_patch(xyz_queue_path_metadata,
headers=headers,
body=doc3)
self.assertEqual(falcon.HTTP_200, self.srmock.status)
# Get
result = self.simulate_get(xyz_queue_path_metadata,
headers=headers)
result_doc = jsonutils.loads(result[0])
self.assertEqual({'key2': 1}, result_doc)
# replace non-existent metadata
doc4 = '[{"op":"replace", "path": "/metadata/key3", "value":2}]'
self.simulate_patch(xyz_queue_path_metadata,
headers=headers,
body=doc4)
self.assertEqual(falcon.HTTP_409, self.srmock.status)
# remove non-existent metadata
doc5 = '[{"op":"remove", "path": "/metadata/key3"}]'
self.simulate_patch(xyz_queue_path_metadata,
headers=headers,
body=doc5)
self.assertEqual(falcon.HTTP_409, self.srmock.status)
self.simulate_delete(xyz_queue_path, headers=headers)
# add metadata to non-existent queue
doc1 = ('[{"op":"add", "path": "/metadata/key1", "value": 1},'
'{"op":"add", "path": "/metadata/key2", "value": 1}]')
self.simulate_patch(xyz_queue_path_metadata,
headers=headers,
body=doc1)
self.assertEqual(falcon.HTTP_404, self.srmock.status)
# replace metadata in non-existent queue
doc4 = '[{"op":"replace", "path": "/metadata/key3", "value":2}]'
self.simulate_patch(xyz_queue_path_metadata,
headers=headers,
body=doc4)
self.assertEqual(falcon.HTTP_404, self.srmock.status)
# remove metadata from non-existent queue
doc5 = '[{"op":"remove", "path": "/metadata/key3"}]'
self.simulate_patch(xyz_queue_path_metadata,
headers=headers,
body=doc5)
self.assertEqual(falcon.HTTP_404, self.srmock.status)
def test_list(self):
arbitrary_number = 644079696574693

27
zaqar/tests/unit/transport/wsgi/v2_0/test_validation.py
View File

@ -150,3 +150,30 @@ class TestValidation(base.V2Base):
self.project_id,
body='{"_max_messages_post_size": 257}')
self.assertEqual(falcon.HTTP_400, self.srmock.status)
def test_queue_patching(self):
headers = {
'Client-ID': str(uuid.uuid4()),
'Content-Type': "application/openstack-messaging-v2.0-json-patch"
}
# Wrong JSON pointer
self.simulate_patch(self.queue_path,
self.project_id,
headers=headers,
body='[{"op":"add","path":"/a","value":2}]')
self.assertEqual(falcon.HTTP_400, self.srmock.status)
# Wrong op
self.simulate_patch(self.queue_path,
self.project_id,
headers=headers,
body='[{"op":"a","path":"/metadata/a","value":2}]')
self.assertEqual(falcon.HTTP_400, self.srmock.status)
self.simulate_patch(self.queue_path,
self.project_id,
headers=headers,
body='[{"op":"add","path":"/metadata/a",'
'"value":2}]')
self.assertEqual(falcon.HTTP_200, self.srmock.status)

127
zaqar/transport/validation.py
View File

@ -106,6 +106,7 @@ class Validator(object):
self._conf.register_opts(_TRANSPORT_LIMITS_OPTIONS,
group=_TRANSPORT_LIMITS_GROUP)
self._limits_conf = self._conf[_TRANSPORT_LIMITS_GROUP]
self._supported_operations = ('add', 'remove', 'replace')
def queue_identification(self, queue, project):
"""Restrictions on a project id & queue name pair.
@ -131,6 +132,132 @@ class Validator(object):
_(u'Queue names may only contain ASCII letters, digits, '
'underscores, and dashes.'))
def _get_change_operation_d10(self, raw_change):
op = raw_change.get('op')
if op is None:
msg = (_('Unable to find `op` in JSON Schema change. '
'It must be one of the following: %(available)s.') %
{'available': ', '.join(self._supported_operations)})
raise ValidationFailed(msg)
if op not in self._supported_operations:
msg = (_('Invalid operation: `%(op)s`. '
'It must be one of the following: %(available)s.') %
{'op': op,
'available': ', '.join(self._supported_operations)})
raise ValidationFailed(msg)
return op
def _get_change_path_d10(self, raw_change):
try:
return raw_change['path']
except KeyError:
msg = _("Unable to find '%s' in JSON Schema change") % 'path'
raise ValidationFailed(msg)
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)
ret = []
for part in pointer.lstrip('/').split('/'):
ret.append(part.replace('~1', '/').replace('~0', '~').strip())
return ret
def _validate_json_pointer(self, pointer):
"""Validate a json pointer.
We only accept a limited form of json pointers.
"""
if not pointer.startswith('/'):
msg = _('Pointer `%s` does not start with "/".') % pointer
raise ValidationFailed(msg)
if re.search('/\s*?/', pointer[1:]):
msg = _('Pointer `%s` contains adjacent "/".') % pointer
raise ValidationFailed(msg)
if len(pointer) > 1 and pointer.endswith('/'):
msg = _('Pointer `%s` end with "/".') % pointer
raise ValidationFailed(msg)
if pointer[1:].strip() == '/':
msg = _('Pointer `%s` does not contains valid token.') % pointer
raise ValidationFailed(msg)
if re.search('~[^01]', pointer) or pointer.endswith('~'):
msg = _('Pointer `%s` contains "~" not part of'
' a recognized escape sequence.') % pointer
raise ValidationFailed(msg)
def _get_change_value(self, raw_change, op):
if 'value' not in raw_change:
msg = _('Operation "{0}" requires a member named "value".')
raise ValidationFailed(msg, op)
return raw_change['value']
def _validate_change(self, change):
if change['op'] == 'remove':
return
path_root = change['path'][0]
if len(change['path']) >= 1 and path_root.lower() != 'metadata':
msg = _("The root of path must be metadata, e.g /metadata/key.")
raise ValidationFailed(msg)
def _validate_path(self, op, path):
limits = {'add': 2, 'remove': 2, 'replace': 2}
if len(path) != limits.get(op, 2):
msg = _("Invalid JSON pointer for this resource: "
"'/%s, e.g /metadata/key'") % '/'.join(path)
raise ValidationFailed(msg)
def _parse_json_schema_change(self, raw_change, draft_version):
if draft_version == 10:
op = self._get_change_operation_d10(raw_change)
path = self._get_change_path_d10(raw_change)
else:
msg = _('Unrecognized JSON Schema draft version')
raise ValidationFailed(msg)
path_list = self._decode_json_pointer(path)
return op, path_list
def queue_patching(self, request, changes):
washed_changes = []
content_types = {
'application/openstack-messaging-v2.0-json-patch': 10,
}
json_schema_version = content_types[request.content_type]
if not isinstance(changes, list):
msg = _('Request body must be a JSON array of operation objects.')
raise ValidationFailed(msg)
for raw_change in changes:
if not isinstance(raw_change, dict):
msg = _('Operations must be JSON objects.')
raise ValidationFailed(msg)
(op, path) = self._parse_json_schema_change(raw_change,
json_schema_version)
# NOTE(flwang): Now the 'path' is a list.
self._validate_path(op, path)
change = {'op': op, 'path': path,
'json_schema_version': json_schema_version}
if not op == 'remove':
change['value'] = self._get_change_value(raw_change, op)
self._validate_change(change)
washed_changes.append(change)
return washed_changes
def queue_listing(self, limit=None, **kwargs):
"""Restrictions involving a list of queues.

7
zaqar/transport/wsgi/errors.py
View File

@ -84,3 +84,10 @@ class HTTPNotFound(falcon.HTTPNotFound):
def __init__(self, description):
super(HTTPNotFound, self).__init__(title=self.TITLE,
description=description)
class HTTPUnsupportedMediaType(falcon.HTTPUnsupportedMediaType):
"""Wraps falcon.HTTPUnsupportedMediaType with contextual title."""
def __init__(self, description):
super(HTTPUnsupportedMediaType, self).__init__(description)

98
zaqar/transport/wsgi/v2_0/queues.py
View File

@ -118,43 +118,111 @@ class ItemResource(object):
jsonschema. Appropriate errors are returned in each case for
badly formatted input.
:returns: HTTP | 200,400,503
:returns: HTTP | 200,400,409,503
"""
LOG.debug(u'PATCH queue - name: %s', queue_name)
try:
# Place JSON size restriction before parsing
self._validate.queue_metadata_length(req.content_length)
except validation.ValidationFailed as ex:
LOG.debug(ex)
raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex))
raise wsgi_errors.HTTPBadRequestBody(six.text_type(ex))
# NOTE(flwang): See below link to get more details about draft 10,
# tools.ietf.org/html/draft-ietf-appsawg-json-patch-10
content_types = {
'application/openstack-messaging-v2.0-json-patch': 10,
}
if req.content_type not in content_types:
headers = {'Accept-Patch':
', '.join(sorted(content_types.keys()))}
msg = _("Accepted media type for PATCH: %s.")
LOG.debug(msg % headers)
raise wsgi_errors.HTTPUnsupportedMediaType(msg % headers)
# Deserialize queue metadata
metadata = None
if req.content_length:
document = wsgi_utils.deserialize(req.stream, req.content_length)
metadata = wsgi_utils.sanitize(document, spec=None)
try:
changes = utils.read_json(req.stream, req.content_length)
changes = wsgi_utils.sanitize(changes,
spec=None, doctype=list)
except utils.MalformedJSON as ex:
LOG.debug(ex)
description = _(u'Request body could not be parsed.')
raise wsgi_errors.HTTPBadRequestBody(description)
except utils.OverflowedJSONInteger as ex:
LOG.debug(ex)
description = _(u'JSON contains integer that is too large.')
raise wsgi_errors.HTTPBadRequestBody(description)
except Exception as ex:
# Error while reading from the network/server
LOG.exception(ex)
description = _(u'Request body could not be read.')
raise wsgi_errors.HTTPServiceUnavailable(description)
else:
msg = _("PATCH body could not be empty for update.")
LOG.debug(msg)
raise wsgi_errors.HTTPBadRequestBody(msg)
try:
changes = self._validate.queue_patching(req, changes)
# NOTE(Eva-i): using 'get_metadata' instead of 'get', so
# QueueDoesNotExist error will be thrown in case of non-existent
# queue.
metadata = self._queue_controller.get_metadata(queue_name,
project=project_id)
for change in changes:
change_method_name = '_do_%s' % change['op']
change_method = getattr(self, change_method_name)
change_method(req, metadata, change)
self._validate.queue_metadata_putting(metadata)
old_metadata = self._queue_controller.get(queue_name,
project=project_id)
old_metadata.update(metadata)
self._queue_controller.set_metadata(queue_name,
old_metadata,
metadata,
project_id)
resp_dict = self._queue_controller.get(queue_name,
project=project_id)
except storage_errors.DoesNotExist as ex:
LOG.debug(ex)
raise wsgi_errors.HTTPNotFound(ex)
raise wsgi_errors.HTTPNotFound(six.text_type(ex))
except validation.ValidationFailed as ex:
LOG.debug(ex)
raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex))
raise wsgi_errors.HTTPBadRequestBody(six.text_type(ex))
except wsgi_errors.HTTPConflict as ex:
raise ex
except Exception as ex:
LOG.exception(ex)
description = _(u'Queue could not be updated.')
raise wsgi_errors.HTTPServiceUnavailable(description)
resp.body = utils.to_json(resp_dict)
resp.body = utils.to_json(metadata)
def _do_replace(self, req, metadata, change):
path = change['path']
path_child = path[1]
value = change['value']
if path_child in metadata:
metadata[path_child] = value
else:
msg = _("Can't replace non-existent object %s.")
raise wsgi_errors.HTTPConflict(msg % path_child)
def _do_add(self, req, metadata, change):
path = change['path']
path_child = path[1]
value = change['value']
metadata[path_child] = value
def _do_remove(self, req, metadata, change):
path = change['path']
path_child = path[1]
if path_child in metadata:
metadata.pop(path_child)
else:
msg = _("Can't remove non-existent object %s.")
raise wsgi_errors.HTTPConflict(msg % path_child)
class CollectionResource(object):