openstack/swift
Code Issues Proposed changes

Use object name from request in bulk Errors

Browse Source 
This will allow users to more easily match the failed objects in Errors
for bulk delete requests to the object names they provided in the request.
For extract Errors, it reports the failed path from the tar archive.

DocImpact

Change-Id: I084057810fc4fb7fdac05494cc6fec2cbf81bb9d
This commit is contained in:
Brian D. Burns 2013-06-22 13:39:53 -04:00
parent 13347af64c
commit 6768d5b4be
2 changed files with 116 additions and 58 deletions
swift/common/middleware
bulk.py
test/unit/common/middleware
test_bulk.py

52
swift/common/middleware/bulk.py

@ -23,8 +23,7 @@ from swift.common.swob import Request, HTTPBadGateway, \
HTTPLengthRequired, HTTPException, HTTPServerError, wsgify
from swift.common.utils import json, get_logger
from swift.common.constraints import check_utf8, MAX_FILE_SIZE
from swift.common.http import HTTP_BAD_REQUEST, HTTP_UNAUTHORIZED, \
HTTP_NOT_FOUND
from swift.common.http import HTTP_UNAUTHORIZED, HTTP_NOT_FOUND
from swift.common.constraints import MAX_OBJECT_NAME_LENGTH, \
MAX_CONTAINER_NAME_LENGTH
@ -112,7 +111,7 @@ class Bulk(object):
responses. This is because a short request body sent from the client could
result in many operations on the proxy server and precautions need to be
made to prevent the request from timing out due to lack of activity. To
this end, the client will always receive a 200 Ok response, regardless of
this end, the client will always receive a 200 OK response, regardless of
the actual success of the call. The body of the response must be parsed to
determine the actual success of the operation. In addition to this the
client may receive zero or more whitespace characters prepended to the
@ -123,12 +122,12 @@ class Bulk(object):
text/plain, application/json, application/xml, and text/xml. An example
body is as follows:
{"Response Code": "201 Created",
{"Response Status": "201 Created",
"Response Body": "",
"Errors": [],
"Number Files Created": 10}
If all valid files were uploaded successfully the Response Code will be a
If all valid files were uploaded successfully the Response Status will be
201 Created. If any files failed to be created the response code
corresponds to the subrequest's error. Possible codes are 400, 401, 502 (on
server errors), etc. In both cases the response body will specify the
@ -158,17 +157,17 @@ class Bulk(object):
/container_name
The response is similar to bulk deletes as in every response will be a 200
Ok and you must parse the response body for acutal results. An example
OK and you must parse the response body for actual results. An example
response is:
{"Number Not Found": 0,
"Response Code": "200 OK",
"Response Status": "200 OK",
"Response Body": "",
"Errors": [],
"Number Deleted": 6}
If all items were successfully deleted (or did not exist), the Response
Code will be a 200 Ok. If any failed to delete, the response code
Status will be 200 OK. If any failed to delete, the response code
corresponds to the subrequest's error. Possible codes are 400, 401, 502 (on
server errors), etc. In all cases the response body will specify the number
of items successfully deleted, not found, and a list of those that failed.
@ -294,12 +293,13 @@ class Bulk(object):
separator = '\r\n\r\n'
last_yield = time()
yield ' '
obj_to_delete = obj_to_delete.strip().lstrip('/')
obj_to_delete = obj_to_delete.strip()
if not obj_to_delete:
continue
delete_path = '/'.join(['', vrs, account, obj_to_delete])
delete_path = '/'.join(['', vrs, account,
obj_to_delete.lstrip('/')])
if not check_utf8(delete_path):
failed_files.append([quote(delete_path),
failed_files.append([quote(obj_to_delete),
HTTPPreconditionFailed().status])
continue
new_env = req.environ.copy()
@ -316,13 +316,13 @@ class Bulk(object):
elif resp.status_int == HTTP_NOT_FOUND:
resp_dict['Number Not Found'] += 1
elif resp.status_int == HTTP_UNAUTHORIZED:
failed_files.append([quote(delete_path),
HTTP_UNAUTHORIZED])
failed_files.append([quote(obj_to_delete),
HTTPUnauthorized().status])
raise HTTPUnauthorized(request=req)
else:
if resp.status_int // 100 == 5:
failed_file_response_type = HTTPBadGateway
failed_files.append([quote(delete_path), resp.status])
failed_files.append([quote(obj_to_delete), resp.status])
if failed_files:
resp_dict['Response Status'] = \
@ -354,7 +354,7 @@ class Bulk(object):
:params req: a swob Request
:params compress_type: specifying the compression type of the tar.
Accepts '', 'gz, or 'bz2'
Accepts '', 'gz', or 'bz2'
"""
resp_dict = {'Response Status': HTTPCreated().status,
'Response Body': '', 'Number Files Created': 0}
@ -406,12 +406,12 @@ class Bulk(object):
container = obj_path.split('/', 1)[0]
if not check_utf8(destination):
failed_files.append(
[quote(destination[:MAX_PATH_LENGTH]),
[quote(obj_path[:MAX_PATH_LENGTH]),
HTTPPreconditionFailed().status])
continue
if tar_info.size > MAX_FILE_SIZE:
failed_files.append([
quote(destination[:MAX_PATH_LENGTH]),
quote(obj_path[:MAX_PATH_LENGTH]),
HTTPRequestEntityTooLarge().status])
continue
if container not in existing_containers:
@ -420,16 +420,16 @@ class Bulk(object):
req, '/'.join(['', vrs, account, container]))
existing_containers.add(container)
except CreateContainerError, err:
failed_files.append([
quote(obj_path[:MAX_PATH_LENGTH]),
err.status])
if err.status_int == HTTP_UNAUTHORIZED:
raise HTTPUnauthorized(request=req)
failed_files.append([
quote(destination[:MAX_PATH_LENGTH]),
err.status])
continue
except ValueError:
failed_files.append([
quote(destination[:MAX_PATH_LENGTH]),
HTTP_BAD_REQUEST])
quote(obj_path[:MAX_PATH_LENGTH]),
HTTPBadRequest().status])
continue
if len(existing_containers) > self.max_containers:
raise HTTPBadRequest(
@ -451,13 +451,13 @@ class Bulk(object):
else:
if resp.status_int == HTTP_UNAUTHORIZED:
failed_files.append([
quote(destination[:MAX_PATH_LENGTH]),
HTTP_UNAUTHORIZED])
quote(obj_path[:MAX_PATH_LENGTH]),
HTTPUnauthorized().status])
raise HTTPUnauthorized(request=req)
if resp.status_int // 100 == 5:
failed_response_type = HTTPBadGateway
failed_files.append([
quote(destination[:MAX_PATH_LENGTH]), resp.status])
quote(obj_path[:MAX_PATH_LENGTH]), resp.status])
if failed_files:
resp_dict['Response Status'] = failed_response_type().status
@ -469,7 +469,7 @@ class Bulk(object):
resp_dict['Response Status'] = err.status
resp_dict['Response Body'] = err.body
except tarfile.TarError, tar_error:
resp_dict['Response Status'] = HTTPBadRequest().status,
resp_dict['Response Status'] = HTTPBadRequest().status
resp_dict['Response Body'] = 'Invalid Tar File: %s' % tar_error
except Exception:
self.logger.exception('Error in extract archive.')

122
test/unit/common/middleware/test_bulk.py

@ -282,23 +282,33 @@ class TestUntar(unittest.TestCase):
def test_extract_tar_fail_cont_401(self):
self.build_tar()
req = Request.blank('/unauth/acc/')
req = Request.blank('/unauth/acc/',
headers={'Accept': 'application/json'})
req.environ['wsgi.input'] = open(os.path.join(self.testdir,
'tar_fails.tar'))
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(
req, '', out_content_type='text/plain')
self.assertTrue('Response Status: 401 Unauthorized' in resp_body)
resp_body = self.handle_extract_and_iter(req, '')
self.assertEquals(self.app.calls, 1)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Response Status'], '401 Unauthorized')
self.assertEquals(
resp_data['Errors'],
[['base_fails1/sub_dir1/sub1_file1', '401 Unauthorized']])
def test_extract_tar_fail_obj_401(self):
self.build_tar()
req = Request.blank('/create_obj_unauth/acc/cont/')
req = Request.blank('/create_obj_unauth/acc/cont/',
headers={'Accept': 'application/json'})
req.environ['wsgi.input'] = open(os.path.join(self.testdir,
'tar_fails.tar'))
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(
req, '', out_content_type='text/plain')
self.assertTrue('Response Status: 401 Unauthorized' in resp_body)
resp_body = self.handle_extract_and_iter(req, '')
self.assertEquals(self.app.calls, 2)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Response Status'], '401 Unauthorized')
self.assertEquals(
resp_data['Errors'],
[['cont/base_fails1/sub_dir1/sub1_file1', '401 Unauthorized']])
def test_extract_tar_fail_obj_name_len(self):
self.build_tar()
@ -308,22 +318,28 @@ class TestUntar(unittest.TestCase):
'tar_fails.tar'))
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(req, '')
self.assertEquals(self.app.calls, 6)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Number Files Created'], 4)
self.assertEquals(resp_data['Errors'][0][0],
'/tar_works/acc/cont/base_fails1/' + ('f' * 101))
self.assertEquals(
resp_data['Errors'],
[['cont/base_fails1/' + ('f' * 101), '400 Bad Request']])
def test_extract_tar_fail_compress_type(self):
self.build_tar()
req = Request.blank('/tar_works/acc/cont/')
req = Request.blank('/tar_works/acc/cont/',
headers={'Accept': 'application/json'})
req.environ['wsgi.input'] = open(os.path.join(self.testdir,
'tar_fails.tar'))
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(req, 'gz')
self.assert_('400 Bad Request' in resp_body)
self.assertEquals(self.app.calls, 0)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Response Status'], '400 Bad Request')
self.assertEquals(
resp_data['Response Body'], 'Invalid Tar File: not a gzip file')
def test_extract_tar_fail_max_file_name_length(self):
def test_extract_tar_fail_max_failed_extractions(self):
self.build_tar()
with patch.object(self.bulk, 'max_failed_extractions', 1):
self.app.calls = 0
@ -333,10 +349,12 @@ class TestUntar(unittest.TestCase):
'tar_fails.tar'))
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(req, '')
resp_data = json.loads(resp_body)
self.assertEquals(self.app.calls, 5)
self.assertEquals(resp_data['Errors'][0][0],
'/tar_works/acc/cont/base_fails1/' + ('f' * 101))
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Number Files Created'], 3)
self.assertEquals(
resp_data['Errors'],
[['cont/base_fails1/' + ('f' * 101), '400 Bad Request']])
@patch.object(bulk, 'MAX_FILE_SIZE', 4)
def test_extract_tar_fail_max_file_size(self):
@ -356,7 +374,10 @@ class TestUntar(unittest.TestCase):
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(req, '')
resp_data = json.loads(resp_body)
self.assert_(resp_data['Errors'][0][1].startswith('413'))
self.assertEquals(
resp_data['Errors'],
[['cont' + self.testdir + '/test/sub_dir1/sub1_file1',
'413 Request Entity Too Large']])
def test_extract_tar_fail_max_cont(self):
dir_tree = [{'sub_dir1': ['sub1_file1']},
@ -367,17 +388,21 @@ class TestUntar(unittest.TestCase):
with patch.object(self.bulk, 'max_containers', 1):
self.app.calls = 0
body = open(os.path.join(self.testdir, 'tar_fails.tar')).read()
req = Request.blank('/tar_works/acc/', body=body)
req = Request.blank('/tar_works/acc/', body=body,
headers={'Accept': 'application/json'})
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(req, '')
self.assertEquals(self.app.calls, 3)
self.assert_('400 Bad Request' in resp_body)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Response Status'], '400 Bad Request')
self.assertEquals(
resp_data['Response Body'],
'More than 1 base level containers in tar.')
def test_extract_tar_fail_create_cont(self):
dir_tree = [{'base_fails1': [
{'sub_dir1': ['sub1_file1']},
{'sub_dir2': ['sub2_file1', 'sub2_file2']},
'f\xde',
{'./sub_dir3': [{'sub4_dir1': 'sub4_file1'}]}]}]
self.build_tar(dir_tree)
req = Request.blank('/create_cont_fail/acc/cont/',
@ -388,7 +413,7 @@ class TestUntar(unittest.TestCase):
resp_body = self.handle_extract_and_iter(req, '')
resp_data = json.loads(resp_body)
self.assertEquals(self.app.calls, 4)
self.assertEquals(len(resp_data['Errors']), 5)
self.assertEquals(len(resp_data['Errors']), 4)
def test_extract_tar_fail_create_cont_value_err(self):
self.build_tar()
@ -406,6 +431,29 @@ class TestUntar(unittest.TestCase):
resp_data = json.loads(resp_body)
self.assertEquals(self.app.calls, 0)
self.assertEquals(len(resp_data['Errors']), 5)
self.assertEquals(
resp_data['Errors'][0],
['cont/base_fails1/sub_dir1/sub1_file1', '400 Bad Request'])
def test_extract_tar_fail_unicode(self):
dir_tree = [{'sub_dir1': ['sub1_file1']},
{'sub_dir2': ['sub2\xdefile1', 'sub2_file2']},
{'sub_\xdedir3': [{'sub4_dir1': 'sub4_file1'}]}]
self.build_tar(dir_tree)
req = Request.blank('/tar_works/acc/',
headers={'Accept': 'application/json'})
req.environ['wsgi.input'] = open(os.path.join(self.testdir,
'tar_fails.tar'))
req.headers['transfer-encoding'] = 'chunked'
resp_body = self.handle_extract_and_iter(req, '')
resp_data = json.loads(resp_body)
self.assertEquals(self.app.calls, 4)
self.assertEquals(resp_data['Number Files Created'], 2)
self.assertEquals(resp_data['Response Status'], '400 Bad Request')
self.assertEquals(
resp_data['Errors'],
[['sub_dir2/sub2%DEfile1', '412 Precondition Failed'],
['sub_%DEdir3/sub4_dir1/sub4_file1', '412 Precondition Failed']])
def test_get_response_body(self):
txt_body = bulk.get_response_body(
@ -535,10 +583,8 @@ class TestDelete(unittest.TestCase):
self.assertEquals(len(resp_data['Errors']), 2)
self.assertEquals(
resp_data['Errors'],
[[urllib.quote('/delete_works/AUTH_Acc/c/ objbadutf8'),
'412 Precondition Failed'],
[urllib.quote('/delete_works/AUTH_Acc/c/f\xdebadutf8'),
'412 Precondition Failed']])
[[urllib.quote('c/ objbadutf8'), '412 Precondition Failed'],
[urllib.quote('/c/f\xdebadutf8'), '412 Precondition Failed']])
def test_bulk_delete_no_body(self):
req = Request.blank('/unauth/AUTH_acc/')
@ -551,16 +597,25 @@ class TestDelete(unittest.TestCase):
self.assertTrue('400 Bad Request' in resp_body)
def test_bulk_delete_unauth(self):
req = Request.blank('/unauth/AUTH_acc/', body='/c/f\n')
req = Request.blank('/unauth/AUTH_acc/', body='/c/f\n/c/f2\n',
headers={'Accept': 'application/json'})
req.method = 'DELETE'
resp_body = self.handle_delete_and_iter(req)
self.assertTrue('401 Unauthorized' in resp_body)
self.assertEquals(self.app.calls, 1)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Errors'], [['/c/f', '401 Unauthorized']])
self.assertEquals(resp_data['Response Status'], '401 Unauthorized')
def test_bulk_delete_500_resp(self):
req = Request.blank('/broke/AUTH_acc/', body='/c/f\n')
req = Request.blank('/broke/AUTH_acc/', body='/c/f\nc/f2\n',
headers={'Accept': 'application/json'})
req.method = 'DELETE'
resp_body = self.handle_delete_and_iter(req)
self.assertTrue('502 Bad Gateway' in resp_body)
resp_data = json.loads(resp_body)
self.assertEquals(
resp_data['Errors'],
[['/c/f', '500 Internal Error'], ['c/f2', '500 Internal Error']])
self.assertEquals(resp_data['Response Status'], '502 Bad Gateway')
def test_bulk_delete_bad_path(self):
req = Request.blank('/delete_cont_fail/')
@ -574,19 +629,22 @@ class TestDelete(unittest.TestCase):
resp_body = self.handle_delete_and_iter(req)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Number Deleted'], 0)
self.assertEquals(resp_data['Errors'][0][1], '409 Conflict')
self.assertEquals(resp_data['Errors'], [['c', '409 Conflict']])
self.assertEquals(resp_data['Response Status'], '400 Bad Request')
def test_bulk_delete_bad_file_too_long(self):
req = Request.blank('/delete_works/AUTH_Acc',
headers={'Accept': 'application/json'})
req.method = 'DELETE'
data = '/c/f\nc/' + ('1' * bulk.MAX_PATH_LENGTH) + '\n/c/f'
bad_file = 'c/' + ('1' * bulk.MAX_PATH_LENGTH)
data = '/c/f\n' + bad_file + '\n/c/f'
req.environ['wsgi.input'] = StringIO(data)
req.headers['Transfer-Encoding'] = 'chunked'
resp_body = self.handle_delete_and_iter(req)
resp_data = json.loads(resp_body)
self.assertEquals(resp_data['Number Deleted'], 2)
self.assertEquals(resp_data['Errors'][0][1], '400 Bad Request')
self.assertEquals(resp_data['Errors'], [[bad_file, '400 Bad Request']])
self.assertEquals(resp_data['Response Status'], '400 Bad Request')
def test_bulk_delete_bad_file_over_twice_max_length(self):
body = '/c/f\nc/' + ('123456' * bulk.MAX_PATH_LENGTH) + '\n'