Merge "Fix cleanup segment container"

swift3/acl_handlers.py

@@ -160,12 +160,23 @@ class BucketAclHandler(BaseAclHandler):
     def DELETE(self, app):
         if self.container.endswith(MULTIUPLOAD_SUFFIX):
             # anyways, delete multiupload container doesn't need acls
+            # because it depends on GET segment container result for
+            # cleanup
             pass
         else:
             return self._handle_acl(app, 'DELETE')
 
+    def HEAD(self, app):
+        if self.method == 'DELETE':
+            return self._handle_acl(app, 'DELETE')
+        else:
+            return self._handle_acl(app, 'HEAD')
+
     def GET(self, app):
-        if self.method != 'DELETE':
+        if self.method == 'DELETE' and \
+                self.container.endswith(MULTIUPLOAD_SUFFIX):
+            pass
+        else:
             return self._handle_acl(app, 'GET')
 
     def PUT(self, app):

swift3/controllers/bucket.py

@@ -41,6 +41,19 @@ class BucketController(Controller):
         container = req.container_name + MULTIUPLOAD_SUFFIX
         marker = ''
         seg = ''
+
+        try:
+            resp = req.get_response(self.app, 'HEAD')
+            if int(resp.sw_headers['X-Container-Object-Count']) > 0:
+                raise BucketNotEmpty()
+            # FIXME: This extra HEAD saves unexpected segment deletion
+            # but if a complete multipart upload happen while cleanup
+            # segment container below, completed object may be missing its
+            # segments unfortunately. To be safer, it might be good
+            # to handle if the segments can be deleted for each object.
+        except NoSuchBucket:
+            pass
+
         try:
             while True:
                 # delete all segments
@@ -187,9 +200,9 @@ class BucketController(Controller):
         """
         Handle DELETE Bucket request
         """
-        resp = req.get_response(self.app)
         if CONF.allow_multipart_uploads:
             self._delete_segments_bucket(req)
+        resp = req.get_response(self.app)
         return resp
 
     def POST(self, req):

swift3/test/functional/test_multi_upload.py

@@ -573,6 +573,60 @@ class TestSwift3MultiUpload(Swift3FunctionalTestCase):
                                    query=query)
         self.assertEquals(status, 200)
 
+    def test_delete_bucket_multi_upload_object_exisiting(self):
+        bucket = 'bucket'
+        keys = ['obj1']
+        uploads = []
+
+        results_generator = self._initiate_multi_uploads_result_generator(
+            bucket, keys)
+
+        # Initiate Multipart Upload
+        for expected_key, (status, _, body) in \
+                izip(keys, results_generator):
+            self.assertEquals(status, 200)  # sanity
+            elem = fromstring(body, 'InitiateMultipartUploadResult')
+            key = elem.find('Key').text
+            self.assertEquals(expected_key, key)  # sanity
+            upload_id = elem.find('UploadId').text
+            self.assertTrue(upload_id is not None)  # sanity
+            self.assertTrue((key, upload_id) not in uploads)
+            uploads.append((key, upload_id))
+
+        self.assertEquals(len(uploads), len(keys))  # sanity
+
+        # Upload Part
+        key, upload_id = uploads[0]
+        content = 'a' * MIN_SEGMENT_SIZE
+        status, headers, body = \
+            self._upload_part(bucket, key, upload_id, content)
+        self.assertEquals(status, 200)
+
+        # Complete Multipart Upload
+        key, upload_id = uploads[0]
+        etags = [md5(content).hexdigest()]
+        xml = self._gen_comp_xml(etags)
+        status, headers, body = \
+            self._complete_multi_upload(bucket, key, upload_id, xml)
+        self.assertEquals(status, 200)  # sanity
+
+        # GET multipart object
+        status, headers, body = \
+            self.conn.make_request('GET', bucket, key)
+        self.assertEquals(status, 200)  # sanity
+        self.assertEquals(content, body)  # sanity
+
+        # DELETE bucket while the object existing
+        status, headers, body = \
+            self.conn.make_request('DELETE', bucket)
+        self.assertEquals(status, 409)  # sanity
+
+        # The object must still be there.
+        status, headers, body = \
+            self.conn.make_request('GET', bucket, key)
+        self.assertEquals(status, 200)  # sanity
+        self.assertEquals(content, body)  # sanity
+
 
 if __name__ == '__main__':
     unittest.main()

swift3/test/unit/test_bucket.py

@@ -74,7 +74,6 @@ class TestSwift3Bucket(Swift3TestCase):
 
     def setUp(self):
         super(TestSwift3Bucket, self).setUp()
-
         self.setup_objects()
 
     def test_bucket_HEAD(self):
@@ -434,23 +433,34 @@ class TestSwift3Bucket(Swift3TestCase):
         status, headers, body = self.call_swift3(req)
         self.assertEquals(self._get_error_code(body), 'MalformedXML')
 
+    def _test_method_error_delete(self, path, sw_resp):
+        self.swift.register('HEAD', '/v1/AUTH_test' + path, sw_resp, {}, None)
+        return self._test_method_error('DELETE', path, sw_resp)
+
     @s3acl
     def test_bucket_DELETE_error(self):
-        code = self._test_method_error('DELETE', '/bucket',
+        code = self._test_method_error_delete('/bucket', swob.HTTPUnauthorized)
-                                       swob.HTTPUnauthorized)
         self.assertEquals(code, 'SignatureDoesNotMatch')
-        code = self._test_method_error('DELETE', '/bucket', swob.HTTPForbidden)
+        code = self._test_method_error_delete('/bucket', swob.HTTPForbidden)
         self.assertEquals(code, 'AccessDenied')
-        code = self._test_method_error('DELETE', '/bucket', swob.HTTPNotFound)
+        code = self._test_method_error_delete('/bucket', swob.HTTPNotFound)
         self.assertEquals(code, 'NoSuchBucket')
+        code = self._test_method_error_delete('/bucket', swob.HTTPServerError)
+        self.assertEquals(code, 'InternalError')
+
+        # bucket not empty is now validated at swift3
+        self.swift.register('HEAD', '/v1/AUTH_test/bucket', swob.HTTPNoContent,
+                            {'X-Container-Object-Count': '1'}, None)
         code = self._test_method_error('DELETE', '/bucket', swob.HTTPConflict)
         self.assertEquals(code, 'BucketNotEmpty')
-        code = self._test_method_error('DELETE', '/bucket',
-                                       swob.HTTPServerError)
-        self.assertEquals(code, 'InternalError')
 
     @s3acl
     def test_bucket_DELETE(self):
+        # overwrite default HEAD to return x-container-object-count
+        self.swift.register(
+            'HEAD', '/v1/AUTH_test/bucket', swob.HTTPNoContent,
+            {'X-Container-Object-Count': 0}, None)
+
         req = Request.blank('/bucket',
                             environ={'REQUEST_METHOD': 'DELETE'},
                             headers={'Authorization': 'AWS test:tester:hmac',
@@ -458,6 +468,27 @@ class TestSwift3Bucket(Swift3TestCase):
         status, headers, body = self.call_swift3(req)
         self.assertEquals(status.split()[0], '204')
 
+    @s3acl
+    def test_bucket_DELETE_error_while_segment_bucket_delete(self):
+        # An error occured while deleting segment objects
+        self.swift.register('DELETE', '/v1/AUTH_test/bucket+segments/lily',
+                            swob.HTTPServiceUnavailable, {}, json.dumps([]))
+        # overwrite default HEAD to return x-container-object-count
+        self.swift.register(
+            'HEAD', '/v1/AUTH_test/bucket', swob.HTTPNoContent,
+            {'X-Container-Object-Count': 0}, None)
+
+        req = Request.blank('/bucket',
+                            environ={'REQUEST_METHOD': 'DELETE'},
+                            headers={'Authorization': 'AWS test:tester:hmac',
+                                     'Date': self.get_date_header()})
+        status, headers, body = self.call_swift3(req)
+        self.assertEquals(status.split()[0], '503')
+        called = [(method, path) for method, path, _ in
+                  self.swift.calls_with_headers]
+        # Don't delete original bucket when error occured in segment container
+        self.assertNotIn(('DELETE', '/v1/AUTH_test/bucket'), called)
+
     def _test_bucket_for_s3acl(self, method, account):
         req = Request.blank('/bucket',
                             environ={'REQUEST_METHOD': method},
@@ -514,18 +545,27 @@ class TestSwift3Bucket(Swift3TestCase):
         status, headers, body = self._test_bucket_for_s3acl('DELETE',
                                                             'test:other')
         self.assertEquals(self._get_error_code(body), 'AccessDenied')
+        # Don't delete anything in backend Swift
+        called = [method for method, _, _ in self.swift.calls_with_headers]
+        self.assertNotIn('DELETE', called)
 
     @s3acl(s3acl_only=True)
     def test_bucket_DELETE_with_write_permission(self):
         status, headers, body = self._test_bucket_for_s3acl('DELETE',
                                                             'test:write')
         self.assertEquals(self._get_error_code(body), 'AccessDenied')
+        # Don't delete anything in backend Swift
+        called = [method for method, _, _ in self.swift.calls_with_headers]
+        self.assertNotIn('DELETE', called)
 
     @s3acl(s3acl_only=True)
     def test_bucket_DELETE_with_fullcontrol_permission(self):
         status, headers, body = \
             self._test_bucket_for_s3acl('DELETE', 'test:full_control')
         self.assertEquals(self._get_error_code(body), 'AccessDenied')
+        # Don't delete anything in backend Swift
+        called = [method for method, _, _ in self.swift.calls_with_headers]
+        self.assertNotIn('DELETE', called)
 
 if __name__ == '__main__':
     unittest.main()