Permalink
Browse files

Various fixes for delete_keys() as follows.

- added many unit tests and fixed assertions
- missing version_id so it was always passing %s.
- fixed tuple of 2 in length check to check key not tuple.
- don't try to delete multiple keys with 0 objects.
- Added Prefixes and unknown objects as errors
- Error results were mistakenly added as Deleted objects.
- Key and DeleteMarker names should be left as unicode
- fixed missing version_id for DeleteMarker
- The length check doesn't work well on resultsets which
  use a generator() so instead, lets try be nice and delete
  all objects in sets of 1000. See unit tests for examples
  but now the following is possible.

     res = bucket.delete_keys(bucket.list_versions())
  • Loading branch information...
1 parent 05cb4a8 commit a811b41829bc2079ba9f784896a5971a30114920 Thomas O'Dowd committed Jan 5, 2012
Showing with 200 additions and 75 deletions.
  1. +70 −49 boto/s3/bucket.py
  2. +2 −1 boto/s3/deletemarker.py
  3. +1 −1 boto/s3/key.py
  4. +4 −6 boto/s3/multidelete.py
  5. +123 −18 tests/s3/test_multidelete.py
View
119 boto/s3/bucket.py
@@ -32,6 +32,7 @@
from boto.s3.multipart import MultiPartUpload
from boto.s3.multipart import CompleteMultiPartUpload
from boto.s3.multidelete import MultiDeleteResult
+from boto.s3.multidelete import Error
from boto.s3.bucketlistresultset import BucketListResultSet
from boto.s3.bucketlistresultset import VersionedBucketListResultSet
from boto.s3.bucketlistresultset import MultiPartUploadListResultSet
@@ -462,8 +463,8 @@ def delete_keys(self, keys, quiet=False, mfa_token=None, headers=None):
"""
Deletes a set of keys using S3's Multi-object delete API. If a
VersionID is specified for that key then that version is removed.
- Returns the XML response from S3, which contains Deleted and Error
- elements for each key you ask to delete.
+ Returns a MultiDeleteResult Object, which contains Deleted
+ and Error elements for each key you ask to delete.
:type keys: list
:param keys: A list of either key_names or (key_name, versionid) pairs
@@ -482,56 +483,76 @@ def delete_keys(self, keys, quiet=False, mfa_token=None, headers=None):
This value is required anytime you are
deleting versioned objects from a bucket
that has the MFADelete option on the bucket.
+
+ :returns: An instance of MultiDeleteResult
"""
- if len(keys) > 1000:
- raise BotoClientError('Max of 1000 keys can be deleted')
- headers = headers or {}
- query_args = 'delete'
+ ikeys = iter(keys)
+ result = MultiDeleteResult(self)
provider = self.connection.provider
- data = """<?xml version="1.0" encoding="UTF-8"?>"""
- data += "<Delete>"
- if quiet:
- data += "<Quiet>true</Quiet>"
- skipped = []
- for key in keys:
- if isinstance(key, basestring):
- key_name = key
- version_id = None
- elif isinstance(key, tuple) and len(tuple) == 2:
- key_name, version_id = key
- elif isinstance(key, Key) or isinstance(key, DeleteMarker):
- key_name = key.name
- version_id = key.version_id
- else:
- skipped.append(key)
- continue
- data += "<Object><Key>%s</Key>" % xml.sax.saxutils.escape(key_name)
- if version_id:
- data += "<VersionId>%s</VersionId>"
- data += "</Object>"
- data += "</Delete>"
- if isinstance(data, unicode):
+ query_args = 'delete'
+ def delete_keys2(hdrs):
+ hdrs = hdrs or {}
+ data = u"""<?xml version="1.0" encoding="UTF-8"?>"""
+ data += u"<Delete>"
+ if quiet:
+ data += u"<Quiet>true</Quiet>"
+ count = 0
+ while count < 1000:
+ try:
+ key = ikeys.next()
+ except StopIteration:
+ break
+ if isinstance(key, basestring):
+ key_name = key
+ version_id = None
+ elif isinstance(key, tuple) and len(key) == 2:
+ key_name, version_id = key
+ elif (isinstance(key, Key) or isinstance(key, DeleteMarker)) and key.name:
+ key_name = key.name
+ version_id = key.version_id
+ else:
+ if isinstance(key, Prefix):
+ key_name = key.name
+ code = 'PrefixSkipped' # Don't delete Prefix
+ else:
+ key_name = repr(key) # try get a string
+ code = 'InvalidArgument' # other unknown type
+ message = 'Invalid. No delete action taken for this object.'
+ error = Error(key_name, code=code, message=message)
+ result.errors.append(error)
+ continue
+ count += 1
+ #key_name = key_name.decode('utf-8')
+ data += u"<Object><Key>%s</Key>" % xml.sax.saxutils.escape(key_name)
+ if version_id:
+ data += u"<VersionId>%s</VersionId>" % version_id
+ data += u"</Object>"
+ data += u"</Delete>"
+ if count <= 0:
+ return False # no more
data = data.encode('utf-8')
- fp = StringIO.StringIO(data)
- md5 = boto.utils.compute_md5(fp)
- headers['Content-MD5'] = md5[1]
- headers['Content-Type'] = 'text/xml'
- if mfa_token:
- headers[provider.mfa_header] = ' '.join(mfa_token)
- response = self.connection.make_request('POST', self.name,
- headers=headers,
- query_args=query_args,
- data=data)
- body = response.read()
- if response.status == 200:
- result = MultiDeleteResult(self)
- h = handler.XmlHandler(result, self)
- xml.sax.parseString(body, h)
- return result
- else:
- raise provider.storage_response_error(response.status,
- response.reason,
- body)
+ fp = StringIO.StringIO(data)
+ md5 = boto.utils.compute_md5(fp)
+ hdrs['Content-MD5'] = md5[1]
+ hdrs['Content-Type'] = 'text/xml'
+ if mfa_token:
+ hdrs[provider.mfa_header] = ' '.join(mfa_token)
+ response = self.connection.make_request('POST', self.name,
+ headers=hdrs,
+ query_args=query_args,
+ data=data)
+ body = response.read()
+ if response.status == 200:
+ h = handler.XmlHandler(result, self)
+ xml.sax.parseString(body, h)
+ return count >= 1000 # more?
+ else:
+ raise provider.storage_response_error(response.status,
+ response.reason,
+ body)
+ while delete_keys2(headers):
+ pass
+ return result
def delete_key(self, key_name, headers=None,
version_id=None, mfa_token=None):
View
3 boto/s3/deletemarker.py
@@ -25,6 +25,7 @@ class DeleteMarker:
def __init__(self, bucket=None, name=None):
self.bucket = bucket
self.name = name
+ self.version_id = None
self.is_latest = False
self.last_modified = None
self.owner = None
@@ -38,7 +39,7 @@ def startElement(self, name, attrs, connection):
def endElement(self, name, value, connection):
if name == 'Key':
- self.name = value.encode('utf-8')
+ self.name = value
elif name == 'IsLatest':
if value == 'true':
self.is_latest = True
View
2 boto/s3/key.py
@@ -355,7 +355,7 @@ def startElement(self, name, attrs, connection):
def endElement(self, name, value, connection):
if name == 'Key':
- self.name = value.encode('utf-8')
+ self.name = value
elif name == 'ETag':
self.etag = value
elif name == 'LastModified':
View
10 boto/s3/multidelete.py
@@ -128,13 +128,11 @@ def startElement(self, name, attrs, connection):
self.deleted.append(d)
return d
elif name == 'Error':
- d = Error()
- self.deleted.append(d)
- return d
+ e = Error()
+ self.errors.append(e)
+ return e
return None
def endElement(self, name, value, connection):
setattr(self, name, value)
-
-
-
+
View
141 tests/s3/test_multidelete.py
@@ -1,3 +1,5 @@
+# -*- coding: utf-8 -*-
+
# Copyright (c) 2011 Mitch Garnaat http://garnaat.org/
# All rights reserved.
#
@@ -26,50 +28,153 @@
import unittest
import time
+from boto.s3.key import Key
+from boto.s3.deletemarker import DeleteMarker
+from boto.s3.prefix import Prefix
from boto.s3.connection import S3Connection
from boto.exception import S3ResponseError
class S3MultiDeleteTest (unittest.TestCase):
- def test_1(self):
- print '--- running S3MultiDelete tests ---'
- c = S3Connection()
- # create a new, empty bucket
- bucket_name = 'multidelete-%d' % int(time.time())
- bucket = c.create_bucket(bucket_name)
+ def setUp(self):
+ self.conn = S3Connection()
+ self.bucket_name = 'multidelete-%d' % int(time.time())
+ self.bucket = self.conn.create_bucket(self.bucket_name)
+
+ def tearDown(self):
+ for key in self.bucket:
+ key.delete()
+ self.bucket.delete()
+
+ def test_delete_nothing(self):
+ result = self.bucket.delete_keys([])
+ self.assertEqual(len(result.deleted), 0)
+ self.assertEqual(len(result.errors), 0)
+
+ def test_delete_illegal(self):
+ result = self.bucket.delete_keys([{"dict":"notallowed"}])
+ self.assertEqual(len(result.deleted), 0)
+ self.assertEqual(len(result.errors), 1)
+
+ def test_delete_mix(self):
+ result = self.bucket.delete_keys(["king",
+ ("mice", None),
+ Key(name="regular"),
+ Key(),
+ Prefix(name="folder/"),
+ DeleteMarker(name="deleted"),
+ {"bad":"type"}])
+ self.assertEqual(len(result.deleted), 4)
+ self.assertEqual(len(result.errors), 3)
+
+ def test_delete_quietly(self):
+ result = self.bucket.delete_keys(["king"], quiet=True)
+ self.assertEqual(len(result.deleted), 0)
+ self.assertEqual(len(result.errors), 0)
+
+ def test_delete_must_escape(self):
+ result = self.bucket.delete_keys([Key(name=">_<;")])
+ self.assertEqual(len(result.deleted), 1)
+ self.assertEqual(len(result.errors), 0)
+
+ def test_delete_unknown_version(self):
+ no_ver = Key(name="no")
+ no_ver.version_id = "version"
+ result = self.bucket.delete_keys([no_ver])
+ self.assertEqual(len(result.deleted), 0)
+ self.assertEqual(len(result.errors), 1)
+
+ def test_delete_kanji(self):
+ result = self.bucket.delete_keys([u"漢字", Key(name=u"日本語")])
+ self.assertEqual(len(result.deleted), 2)
+ self.assertEqual(len(result.errors), 0)
+
+ def test_delete_empty_by_list(self):
+ result = self.bucket.delete_keys(self.bucket.list())
+ self.assertEqual(len(result.deleted), 0)
+ self.assertEqual(len(result.errors), 0)
+
+ def test_delete_kanji_by_list(self):
+ for key_name in [u"漢字", u"日本語", u"テスト"]:
+ key = self.bucket.new_key(key_name)
+ key.set_contents_from_string('this is a test')
+ result = self.bucket.delete_keys(self.bucket.list())
+ self.assertEqual(len(result.deleted), 3)
+ self.assertEqual(len(result.errors), 0)
+ def test_delete_with_prefixes(self):
+ for key_name in ["a", "a/b", "b"]:
+ key = self.bucket.new_key(key_name)
+ key.set_contents_from_string('this is a test')
+
+ # First delete all "files": "a" and "b"
+ result = self.bucket.delete_keys(self.bucket.list(delimiter="/"))
+ self.assertEqual(len(result.deleted), 2)
+ # Using delimiter will cause 1 common prefix to be listed
+ # which will be skipped as an error.
+ self.assertEqual(len(result.errors), 1)
+ self.assertEqual(result.errors[0].key, "a/")
+
+ # Next delete any remaining objects: "a/b"
+ result = self.bucket.delete_keys(self.bucket.list())
+ self.assertEqual(len(result.deleted), 1)
+ self.assertEqual(len(result.errors), 0)
+ self.assertEqual(result.deleted[0].key, "a/b")
+
+ def test_delete_too_many_versions(self):
+ # configure versioning first
+ self.bucket.configure_versioning(True)
+
+ # Add 1000 initial versions as DMs by deleting them :-)
+ # Adding 1000 objects is painful otherwise...
+ key_names = ['key-%03d' % i for i in range(0, 1000)]
+ result = self.bucket.delete_keys(key_names)
+ self.assertEqual(len(result.deleted), 1000)
+ self.assertEqual(len(result.errors), 0)
+
+ # delete them again to create 1000 more delete markers
+ result = self.bucket.delete_keys(key_names)
+ self.assertEqual(len(result.deleted), 1000)
+ self.assertEqual(len(result.errors), 0)
+
+ # Sometimes takes AWS sometime to settle
+ time.sleep(5)
+
+ # delete all versions to delete 2000 objects.
+ # this tests the 1000 limit.
+ result = self.bucket.delete_keys(self.bucket.list_versions())
+ self.assertEqual(len(result.deleted), 2000)
+ self.assertEqual(len(result.errors), 0)
+
+ def test_1(self):
nkeys = 100
# create a bunch of keynames
key_names = ['key-%03d' % i for i in range(0,nkeys)]
# create the corresponding keys
for key_name in key_names:
- key = bucket.new_key(key_name)
+ key = self.bucket.new_key(key_name)
key.set_contents_from_string('this is a test')
# now count keys in bucket
n = 0
- for key in bucket:
+ for key in self.bucket:
n += 1
- assert n == nkeys
+ self.assertEqual(n, nkeys)
# now delete them all
- result = bucket.delete_keys(key_names)
+ result = self.bucket.delete_keys(key_names)
- assert len(result.deleted) == nkeys
- assert len(result.errors) == 0
+ self.assertEqual(len(result.deleted), nkeys)
+ self.assertEqual(len(result.errors), 0)
time.sleep(5)
# now count keys in bucket
n = 0
- for key in bucket:
+ for key in self.bucket:
n += 1
- assert n == 0
-
- # now delete bucket
- bucket.delete()
- print '--- tests completed ---'
+ self.assertEqual(n, 0)

0 comments on commit a811b41

Please sign in to comment.