From 06ff865d19b522c48e23637d1234fc8f6b9401ab Mon Sep 17 00:00:00 2001 From: Janie Richling Date: Wed, 17 Aug 2016 00:54:09 -0500 Subject: [PATCH] Allow DLO manifest to be updated when using post-as-copy Currently when using fast-post, the manifest is updated with the given 'x-object-manifest' header on a POST. If no such header is supplied, then the manifest will change to a regular object. This is not currently true when using post-as-copy. This patch changes the DLO POST using post-as-copy behavior to match that of using fast-post. It was also documented that 'x-object-manifest' must be provided on a POST to a manifest file. Change-Id: Ie1143ab1a2c8f8c21e258a36badbff5d947769d4 Closes-bug: 1612991 --- doc/source/api/large_objects.rst | 6 ++ doc/source/overview_large_objects.rst | 4 + swift/common/middleware/copy.py | 5 +- swift/common/middleware/dlo.py | 14 ++-- test/functional/tests.py | 93 ++++++++++++++++++++++-- test/unit/common/middleware/test_copy.py | 32 ++++++++ 6 files changed, 141 insertions(+), 13 deletions(-) diff --git a/doc/source/api/large_objects.rst b/doc/source/api/large_objects.rst index 3e167f7530..a4cfb6bc7b 100644 --- a/doc/source/api/large_objects.rst +++ b/doc/source/api/large_objects.rst @@ -168,6 +168,12 @@ of segments to a second location and update the manifest to point to this new location. During the upload of the new segments, the original manifest is still available to download the first set of segments. +.. note:: + + When updating a manifest object using a POST request, a + ``X-Object-Manifest`` header must be included for the + object to continue to behave as a manifest object. + **Example Upload segment of large object request: HTTP** .. code:: diff --git a/doc/source/overview_large_objects.rst b/doc/source/overview_large_objects.rst index 425bb1e8c7..e96c87015f 100644 --- a/doc/source/overview_large_objects.rst +++ b/doc/source/overview_large_objects.rst @@ -57,6 +57,10 @@ Additional Notes /`` header will be returned with the concatenated object so you can tell where it's getting its segments from. +* When updating a manifest object using a POST request, a + ``X-Object-Manifest`` header must be included for the object to + continue to behave as a manifest object. + * The response's ``Content-Length`` for a ``GET`` or ``HEAD`` on the manifest file will be the sum of all the segments in the ``/`` listing, dynamically. So, uploading additional segments after the manifest is diff --git a/swift/common/middleware/copy.py b/swift/common/middleware/copy.py index 4d6c52d77b..5265fe9307 100644 --- a/swift/common/middleware/copy.py +++ b/swift/common/middleware/copy.py @@ -489,8 +489,9 @@ class ServerSideCopyMiddleware(object): params['multipart-manifest'] = 'put' if 'X-Object-Manifest' in source_resp.headers: del params['multipart-manifest'] - sink_req.headers['X-Object-Manifest'] = \ - source_resp.headers['X-Object-Manifest'] + if 'swift.post_as_copy' not in sink_req.environ: + sink_req.headers['X-Object-Manifest'] = \ + source_resp.headers['X-Object-Manifest'] sink_req.params = params # Set data source, content length and etag for the PUT request diff --git a/swift/common/middleware/dlo.py b/swift/common/middleware/dlo.py index 1c27800eb2..aa6057c042 100644 --- a/swift/common/middleware/dlo.py +++ b/swift/common/middleware/dlo.py @@ -85,13 +85,17 @@ available to download the first set of segments. .. note:: + When updating a manifest object using a POST request, a + ``X-Object-Manifest`` header must be included for the object to + continue to behave as a manifest object. + The manifest file should have no content. However, this is not enforced. If the manifest path itself conforms to container/prefix specified in - X-Object-Manifest, and if manifest has some content/data in it, it would - also be considered as segment and manifest's content will be part of the - concatenated GET response. The order of concatenation follows the usual DLO - logic which is - the order of concatenation adheres to order returned when - segment names are sorted. + ``X-Object-Manifest``, and if manifest has some content/data in it, it + would also be considered as segment and manifest's content will be part of + the concatenated GET response. The order of concatenation follows the usual + DLO logic which is - the order of concatenation adheres to order returned + when segment names are sorted. Here's an example using ``curl`` with tiny 1-byte segments:: diff --git a/test/functional/tests.py b/test/functional/tests.py index ff75272e54..6c5dd7f856 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -2671,6 +2671,7 @@ class TestDlo(Base): def test_dlo_post_with_manifest_header(self): # verify that performing a POST to a DLO manifest # preserves the fact that it is a manifest file. + # verify that the x-object-manifest header may be updated. # create a new manifest for this test to avoid test coupling. x_o_m = self.env.container.file('man1').info()['x_object_manifest'] @@ -2684,25 +2685,105 @@ class TestDlo(Base): contents = file_item.read(parms={}) self.assertEqual(expected_contents, contents) - # POST to the manifest file - # include the x-object-manifest in case running with fast-post + # POST a modified x-object-manifest value + new_x_o_m = x_o_m.rstrip('lower') + 'upper' file_item.post({'x-object-meta-foo': 'bar', - 'x-object-manifest': x_o_m}) + 'x-object-manifest': new_x_o_m}) - # Verify x-object-manifest still intact + # verify that x-object-manifest was updated file_item.info() resp_headers = file_item.conn.response.getheaders() - self.assertIn(('x-object-manifest', x_o_m), resp_headers) + self.assertIn(('x-object-manifest', new_x_o_m), resp_headers) self.assertIn(('x-object-meta-foo', 'bar'), resp_headers) # verify that manifest content was not changed manifest_contents = file_item.read(parms={'multipart-manifest': 'get'}) self.assertEqual('manifest-contents', manifest_contents) - # verify that manifest still points to original content + # verify that updated manifest points to new content + expected_contents = ''.join([(c * 10) for c in 'ABCDE']) contents = file_item.read(parms={}) self.assertEqual(expected_contents, contents) + # Now revert the manifest to point to original segments, including a + # multipart-manifest=get param just to check that has no effect + file_item.post({'x-object-manifest': x_o_m}, + parms={'multipart-manifest': 'get'}) + + # verify that x-object-manifest was reverted + info = file_item.info() + self.assertIn('x_object_manifest', info) + self.assertEqual(x_o_m, info['x_object_manifest']) + + # verify that manifest content was not changed + manifest_contents = file_item.read(parms={'multipart-manifest': 'get'}) + self.assertEqual('manifest-contents', manifest_contents) + + # verify that updated manifest points new content + expected_contents = ''.join([(c * 10) for c in 'abcde']) + contents = file_item.read(parms={}) + self.assertEqual(expected_contents, contents) + + def test_dlo_post_without_manifest_header(self): + # verify that a POST to a DLO manifest object with no + # x-object-manifest header will cause the existing x-object-manifest + # header to be lost + + # create a new manifest for this test to avoid test coupling. + x_o_m = self.env.container.file('man1').info()['x_object_manifest'] + file_item = self.env.container.file(Utils.create_name()) + file_item.write('manifest-contents', hdrs={"X-Object-Manifest": x_o_m}) + + # sanity checks + manifest_contents = file_item.read(parms={'multipart-manifest': 'get'}) + self.assertEqual('manifest-contents', manifest_contents) + expected_contents = ''.join([(c * 10) for c in 'abcde']) + contents = file_item.read(parms={}) + self.assertEqual(expected_contents, contents) + + # POST with no x-object-manifest header + file_item.post({}) + + # verify that existing x-object-manifest was removed + info = file_item.info() + self.assertNotIn('x_object_manifest', info) + + # verify that object content was not changed + manifest_contents = file_item.read(parms={'multipart-manifest': 'get'}) + self.assertEqual('manifest-contents', manifest_contents) + + # verify that object is no longer a manifest + contents = file_item.read(parms={}) + self.assertEqual('manifest-contents', contents) + + def test_dlo_post_with_manifest_regular_object(self): + # verify that performing a POST to a regular object + # with a manifest header will create a DLO. + + # Put a regular object + file_item = self.env.container.file(Utils.create_name()) + file_item.write('file contents', hdrs={}) + + # sanity checks + file_contents = file_item.read(parms={}) + self.assertEqual('file contents', file_contents) + + # get the path associated with man1 + x_o_m = self.env.container.file('man1').info()['x_object_manifest'] + + # POST a x-object-manifest value to the regular object + file_item.post({'x-object-manifest': x_o_m}) + + # verify that the file is now a manifest + manifest_contents = file_item.read(parms={'multipart-manifest': 'get'}) + self.assertEqual('file contents', manifest_contents) + expected_contents = ''.join([(c * 10) for c in 'abcde']) + contents = file_item.read(parms={}) + self.assertEqual(expected_contents, contents) + file_item.info() + resp_headers = file_item.conn.response.getheaders() + self.assertIn(('x-object-manifest', x_o_m), resp_headers) + class TestDloUTF8(Base2, TestDlo): set_up = False diff --git a/test/unit/common/middleware/test_copy.py b/test/unit/common/middleware/test_copy.py index 4c2643dd92..ff0c642573 100644 --- a/test/unit/common/middleware/test_copy.py +++ b/test/unit/common/middleware/test_copy.py @@ -210,6 +210,38 @@ class TestServerSideCopyMiddleware(unittest.TestCase): self.assertEqual(len(self.authorized), 1) self.assertRequestEqual(req, self.authorized[0]) + def test_POST_as_COPY_dynamic_large_object_manifest(self): + self.app.register('GET', '/v1/a/c/o', swob.HTTPOk, + {'X-Object-Manifest': 'orig_manifest'}, 'passed') + self.app.register('PUT', '/v1/a/c/o', swob.HTTPCreated, {}) + req = Request.blank('/v1/a/c/o', method='POST', + headers={'X-Object-Manifest': 'new_manifest'}) + status, headers, body = self.call_ssc(req) + self.assertEqual(status, '202 Accepted') + + calls = self.app.calls_with_headers + method, path, req_headers = calls[1] + self.assertEqual('PUT', method) + self.assertEqual('new_manifest', req_headers['x-object-manifest']) + self.assertEqual(len(self.authorized), 1) + self.assertRequestEqual(req, self.authorized[0]) + + def test_POST_as_COPY_dynamic_large_object_no_manifest(self): + self.app.register('GET', '/v1/a/c/o', swob.HTTPOk, + {'X-Object-Manifest': 'orig_manifest'}, 'passed') + self.app.register('PUT', '/v1/a/c/o', swob.HTTPCreated, {}) + req = Request.blank('/v1/a/c/o', method='POST', + headers={}) + status, headers, body = self.call_ssc(req) + self.assertEqual(status, '202 Accepted') + + calls = self.app.calls_with_headers + method, path, req_headers = calls[1] + self.assertEqual('PUT', method) + self.assertNotIn('X-Object-Manifest', req_headers) + self.assertEqual(len(self.authorized), 1) + self.assertRequestEqual(req, self.authorized[0]) + def test_basic_put_with_x_copy_from(self): self.app.register('GET', '/v1/a/c/o', swob.HTTPOk, {}, 'passed') self.app.register('PUT', '/v1/a/c/o2', swob.HTTPCreated, {})