openstack
/
swift
Code Issues Proposed changes

Make DLO manifest copying work with ?multipart-manifest=get 

If a client issues a COPY request for a DLO with the query param
multipart-manifest=get, then the expectation is a new
manifest. However, this wasn't the case; X-Object-Manifest wasn't
being set on the new object, so the result was a normal object with
the same contents as the old manifest (typically 0 bytes).

There was already a mechanism by which middlewares could modify COPY
requests; this commit extends that so they can set headers on the new
object.

Note that this has nothing to do with a "normal" DLO copy, i.e. one
without multipart-manifest=get. That one makes a new object that's the
concatenation of the segments, and it was working just fine.

Change-Id: I1073af9fee6e34ebdfad7b1a89aeb05e4523a151
This commit is contained in:
Samuel Merritt 2014-02-27 22:38:53 -08:00
parent 076634e23c
commit 4f24ef87d6
6 changed files with 134 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
swift/common/middleware/dlo.py
View File

@ -281,8 +281,9 @@ class DynamicLargeObject(object):
return self.app(env, start_response) return self.app(env, start_response)
# install our COPY-callback hook # install our COPY-callback hook
env['swift.copy_response_hook'] = self.copy_response_hook( env['swift.copy_hook'] = self.copy_hook(
env.get('swift.copy_response_hook', lambda req, resp: resp)) env.get('swift.copy_hook',
lambda src_req, src_resp, sink_req: src_resp))
if ((req.method == 'GET' or req.method == 'HEAD') and if ((req.method == 'GET' or req.method == 'HEAD') and
req.params.get('multipart-manifest') != 'get'): req.params.get('multipart-manifest') != 'get'):
@ -313,14 +314,21 @@ class DynamicLargeObject(object):
body=('X-Object-Manifest must be in the ' body=('X-Object-Manifest must be in the '
'format container/prefix')) 'format container/prefix'))
def copy_response_hook(self, inner_hook): def copy_hook(self, inner_hook):
def dlo_copy_hook(req, resp): def dlo_copy_hook(source_req, source_resp, sink_req):
x_o_m = resp.headers.get('X-Object-Manifest') x_o_m = source_resp.headers.get('X-Object-Manifest')
if (x_o_m and req.params.get('multipart-manifest') != 'get'): if x_o_m:
resp = GetContext(self, self.logger).get_or_head_response( if source_req.params.get('multipart-manifest') == 'get':
req, x_o_m, resp.headers.items()) # To copy the manifest, we let the copy proceed as normal,
return inner_hook(req, resp) # but ensure that X-Object-Manifest is set on the new
# object.
sink_req.headers['X-Object-Manifest'] = x_o_m
else:
ctx = GetContext(self, self.logger)
source_resp = ctx.get_or_head_response(
source_req, x_o_m, source_resp.headers.items())
return inner_hook(source_req, source_resp, sink_req)
return dlo_copy_hook return dlo_copy_hook

21
swift/common/middleware/slo.py
View File

@ -521,14 +521,16 @@ class StaticLargeObject(object):
""" """
return SloGetContext(self).handle_slo_get_or_head(req, start_response) return SloGetContext(self).handle_slo_get_or_head(req, start_response)
def copy_response_hook(self, inner_hook): def copy_hook(self, inner_hook):
def slo_hook(req, resp): def slo_hook(source_req, source_resp, sink_req):
if (config_true_value(resp.headers.get('X-Static-Large-Object')) x_slo = source_resp.headers.get('X-Static-Large-Object')
and req.params.get('multipart-manifest') != 'get'): if (config_true_value(x_slo)
resp = SloGetContext(self).get_or_head_response( and source_req.params.get('multipart-manifest') != 'get'):
req, resp.headers.items(), resp.app_iter) source_resp = SloGetContext(self).get_or_head_response(
return inner_hook(req, resp) source_req, source_resp.headers.items(),
source_resp.app_iter)
return inner_hook(source_req, source_resp, sink_req)
return slo_hook return slo_hook
@ -749,8 +751,9 @@ class StaticLargeObject(object):
return self.app(env, start_response) return self.app(env, start_response)
# install our COPY-callback hook # install our COPY-callback hook
env['swift.copy_response_hook'] = self.copy_response_hook( env['swift.copy_hook'] = self.copy_hook(
env.get('swift.copy_response_hook', lambda req, resp: resp)) env.get('swift.copy_hook',
lambda src_req, src_resp, sink_req: src_resp))
try: try:
if req.method == 'PUT' and \ if req.method == 'PUT' and \

35
swift/proxy/controllers/obj.py
View File

@ -566,46 +566,49 @@ class ObjectController(Controller):
orig_container_name = self.container_name orig_container_name = self.container_name
self.object_name = src_obj_name self.object_name = src_obj_name
self.container_name = src_container_name self.container_name = src_container_name
sink_req = Request.blank(req.path_info,
environ=req.environ, headers=req.headers)
source_resp = self.GET(source_req)
# This gives middlewares a way to change the source; for example, # This gives middlewares a way to change the source; for example,
# this lets you COPY a SLO manifest and have the new object be the # this lets you COPY a SLO manifest and have the new object be the
# concatenation of the segments (like what a GET request gives # concatenation of the segments (like what a GET request gives
# the client), not a copy of the manifest file. # the client), not a copy of the manifest file.
source_resp = req.environ.get( hook = req.environ.get(
'swift.copy_response_hook', 'swift.copy_hook',
lambda req, resp: resp)(source_req, self.GET(source_req)) (lambda source_req, source_resp, sink_req: source_resp))
source_resp = hook(source_req, source_resp, sink_req)
if source_resp.status_int >= HTTP_MULTIPLE_CHOICES: if source_resp.status_int >= HTTP_MULTIPLE_CHOICES:
return source_resp return source_resp
self.object_name = orig_obj_name self.object_name = orig_obj_name
self.container_name = orig_container_name self.container_name = orig_container_name
new_req = Request.blank(req.path_info,
environ=req.environ, headers=req.headers)
data_source = iter(source_resp.app_iter) data_source = iter(source_resp.app_iter)
new_req.content_length = source_resp.content_length sink_req.content_length = source_resp.content_length
if new_req.content_length is None: if sink_req.content_length is None:
# This indicates a transfer-encoding: chunked source object, # This indicates a transfer-encoding: chunked source object,
# which currently only happens because there are more than # which currently only happens because there are more than
# CONTAINER_LISTING_LIMIT segments in a segmented object. In # CONTAINER_LISTING_LIMIT segments in a segmented object. In
# this case, we're going to refuse to do the server-side copy. # this case, we're going to refuse to do the server-side copy.
return HTTPRequestEntityTooLarge(request=req) return HTTPRequestEntityTooLarge(request=req)
if new_req.content_length > MAX_FILE_SIZE: if sink_req.content_length > MAX_FILE_SIZE:
return HTTPRequestEntityTooLarge(request=req) return HTTPRequestEntityTooLarge(request=req)
new_req.etag = source_resp.etag sink_req.etag = source_resp.etag
# we no longer need the X-Copy-From header # we no longer need the X-Copy-From header
del new_req.headers['X-Copy-From'] del sink_req.headers['X-Copy-From']
if not content_type_manually_set: if not content_type_manually_set:
new_req.headers['Content-Type'] = \ sink_req.headers['Content-Type'] = \
source_resp.headers['Content-Type'] source_resp.headers['Content-Type']
if not config_true_value( if not config_true_value(
new_req.headers.get('x-fresh-metadata', 'false')): sink_req.headers.get('x-fresh-metadata', 'false')):
copy_headers_into(source_resp, new_req) copy_headers_into(source_resp, sink_req)
copy_headers_into(req, new_req) copy_headers_into(req, sink_req)
# copy over x-static-large-object for POSTs and manifest copies # copy over x-static-large-object for POSTs and manifest copies
if 'X-Static-Large-Object' in source_resp.headers and \ if 'X-Static-Large-Object' in source_resp.headers and \
req.params.get('multipart-manifest') == 'get': req.params.get('multipart-manifest') == 'get':
new_req.headers['X-Static-Large-Object'] = \ sink_req.headers['X-Static-Large-Object'] = \
source_resp.headers['X-Static-Large-Object'] source_resp.headers['X-Static-Large-Object']
req = new_req req = sink_req
if 'x-delete-at' in req.headers: if 'x-delete-at' in req.headers:
try: try:

19
test/functional/tests.py
View File

@ -1645,6 +1645,25 @@ class TestDlo(Base):
file_contents, file_contents,
"aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeeeffffffffff") "aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeeeffffffffff")
def test_copy_manifest(self):
# Copying the manifest should result in another manifest
try:
man1_item = self.env.container.file('man1')
man1_item.copy(self.env.container.name, "copied-man1",
parms={'multipart-manifest': 'get'})
copied = self.env.container.file("copied-man1")
copied_contents = copied.read(parms={'multipart-manifest': 'get'})
self.assertEqual(copied_contents, "man1-contents")
copied_contents = copied.read()
self.assertEqual(
copied_contents,
"aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeee")
finally:
# try not to leave this around for other tests to stumble over
self.env.container.file("copied-man1").delete()
def test_dlo_if_match_get(self): def test_dlo_if_match_get(self):
manifest = self.env.container.file("man1") manifest = self.env.container.file("man1")
etag = manifest.info()['etag'] etag = manifest.info()['etag']

56
test/unit/common/middleware/test_dlo.py
View File

@ -796,7 +796,7 @@ class TestDloCopyHook(DloTestCase):
# slip this guy in there to pull out the hook # slip this guy in there to pull out the hook
def extract_copy_hook(env, sr): def extract_copy_hook(env, sr):
copy_hook[0] = env.get('swift.copy_response_hook') copy_hook[0] = env.get('swift.copy_hook')
return self.app(env, sr) return self.app(env, sr)
self.dlo = dlo.filter_factory({})(extract_copy_hook) self.dlo = dlo.filter_factory({})(extract_copy_hook)
@ -809,23 +809,55 @@ class TestDloCopyHook(DloTestCase):
self.assertTrue(self.copy_hook is not None) # sanity check self.assertTrue(self.copy_hook is not None) # sanity check
def test_copy_hook_passthrough(self): def test_copy_hook_passthrough(self):
req = swob.Request.blank('/v1/AUTH_test/c/man') source_req = swob.Request.blank(
# no X-Object-Manifest header, so do nothing '/v1/AUTH_test/c/man',
resp = swob.Response(request=req, status=200) environ={'REQUEST_METHOD': 'GET'})
sink_req = swob.Request.blank(
'/v1/AUTH_test/c/man',
environ={'REQUEST_METHOD': 'PUT'})
source_resp = swob.Response(request=source_req, status=200)
modified_resp = self.copy_hook(req, resp) # no X-Object-Manifest header, so do nothing
self.assertTrue(modified_resp is resp) modified_resp = self.copy_hook(source_req, source_resp, sink_req)
self.assertTrue(modified_resp is source_resp)
def test_copy_hook_manifest(self): def test_copy_hook_manifest(self):
req = swob.Request.blank('/v1/AUTH_test/c/man') source_req = swob.Request.blank(
resp = swob.Response(request=req, status=200, '/v1/AUTH_test/c/man',
headers={"X-Object-Manifest": "c/o"}, environ={'REQUEST_METHOD': 'GET'})
app_iter=["manifest"]) sink_req = swob.Request.blank(
'/v1/AUTH_test/c/man',
environ={'REQUEST_METHOD': 'PUT'})
source_resp = swob.Response(
request=source_req, status=200,
headers={"X-Object-Manifest": "c/o"},
app_iter=["manifest"])
modified_resp = self.copy_hook(req, resp) # it's a manifest, so copy the segments to make a normal object
self.assertTrue(modified_resp is not resp) modified_resp = self.copy_hook(source_req, source_resp, sink_req)
self.assertTrue(modified_resp is not source_resp)
self.assertEqual(modified_resp.etag, self.assertEqual(modified_resp.etag,
hashlib.md5("o1-etago2-etag").hexdigest()) hashlib.md5("o1-etago2-etag").hexdigest())
self.assertEqual(sink_req.headers.get('X-Object-Manifest'), None)
def test_copy_hook_manifest_with_multipart_manifest_get(self):
source_req = swob.Request.blank(
'/v1/AUTH_test/c/man',
environ={'REQUEST_METHOD': 'GET',
'QUERY_STRING': 'multipart-manifest=get'})
sink_req = swob.Request.blank(
'/v1/AUTH_test/c/man',
environ={'REQUEST_METHOD': 'PUT'})
source_resp = swob.Response(
request=source_req, status=200,
headers={"X-Object-Manifest": "c/o"},
app_iter=["manifest"])
# make sure the sink request (the backend PUT) gets X-Object-Manifest
# on it, but that's all
modified_resp = self.copy_hook(source_req, source_resp, sink_req)
self.assertTrue(modified_resp is source_resp)
self.assertEqual(sink_req.headers.get('X-Object-Manifest'), 'c/o')
class TestDloConfiguration(unittest.TestCase): class TestDloConfiguration(unittest.TestCase):

36
test/unit/common/middleware/test_slo.py
View File

@ -1459,7 +1459,7 @@ class TestSloCopyHook(SloTestCase):
# slip this guy in there to pull out the hook # slip this guy in there to pull out the hook
def extract_copy_hook(env, sr): def extract_copy_hook(env, sr):
copy_hook[0] = env['swift.copy_response_hook'] copy_hook[0] = env['swift.copy_hook']
return self.app(env, sr) return self.app(env, sr)
self.slo = slo.filter_factory({})(extract_copy_hook) self.slo = slo.filter_factory({})(extract_copy_hook)
@ -1472,23 +1472,33 @@ class TestSloCopyHook(SloTestCase):
self.assertTrue(self.copy_hook is not None) # sanity check self.assertTrue(self.copy_hook is not None) # sanity check
def test_copy_hook_passthrough(self): def test_copy_hook_passthrough(self):
req = Request.blank('/v1/AUTH_test/c/o') source_req = Request.blank(
'/v1/AUTH_test/c/o',
environ={'REQUEST_METHOD': 'GET'})
sink_req = Request.blank(
'/v1/AUTH_test/c/o',
environ={'REQUEST_METHOD': 'PUT'})
# no X-Static-Large-Object header, so do nothing # no X-Static-Large-Object header, so do nothing
resp = Response(request=req, status=200) source_resp = Response(request=source_req, status=200)
modified_resp = self.copy_hook(req, resp) modified_resp = self.copy_hook(source_req, source_resp, sink_req)
self.assertTrue(modified_resp is resp) self.assertTrue(modified_resp is source_resp)
def test_copy_hook_manifest(self): def test_copy_hook_manifest(self):
req = Request.blank('/v1/AUTH_test/c/o') source_req = Request.blank(
resp = Response(request=req, status=200, '/v1/AUTH_test/c/o',
headers={"X-Static-Large-Object": "true"}, environ={'REQUEST_METHOD': 'GET'})
app_iter=[json.dumps([{'name': '/c/o', sink_req = Request.blank(
'hash': 'obj-etag', '/v1/AUTH_test/c/o',
'bytes': '3'}])]) environ={'REQUEST_METHOD': 'PUT'})
source_resp = Response(request=source_req, status=200,
headers={"X-Static-Large-Object": "true"},
app_iter=[json.dumps([{'name': '/c/o',
'hash': 'obj-etag',
'bytes': '3'}])])
modified_resp = self.copy_hook(req, resp) modified_resp = self.copy_hook(source_req, source_resp, sink_req)
self.assertTrue(modified_resp is not resp) self.assertTrue(modified_resp is not source_resp)
self.assertEqual(modified_resp.etag, md5("obj-etag").hexdigest()) self.assertEqual(modified_resp.etag, md5("obj-etag").hexdigest())