From b87846548e67f5e152677aff859237aebb2411a1 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 7 Oct 2014 13:19:50 -0400 Subject: [PATCH 1/3] Add Bucket.copy_key impl from PR #99, with tests. Hand apply / fix up patch from gsalgado's un-mergeable commit: https://github.com/gsalgado/gcloud-python/commit/1c80d618f265c01ac01a794b5af12f03a6438fb1. --- gcloud/storage/bucket.py | 24 +++++++++++++++++-- gcloud/storage/test_bucket.py | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py index ea64670a05d4..6ac9addb9f87 100644 --- a/gcloud/storage/bucket.py +++ b/gcloud/storage/bucket.py @@ -191,8 +191,28 @@ def delete_keys(self, keys): for key in keys: self.delete_key(key) - def copy_key(self): # pragma NO COVER - raise NotImplementedError + def copy_key(self, key, destination_bucket, new_name=None): + """Copy the given key to the given bucket, optionally with a new name. + + :type key: string or :class:`gcloud.storage.key.Key` + :param key: The key to be copied. + + :type destination_bucket: :class:`gcloud.storage.bucket.Bucket` + :param destination_bucket: The bucket into which the key should be + copied. + + :type new_name: string + :param new_name: (optional) the new name for the copied file. + + :rtype: :class:`gcloud.storage.key.Key` + :returns: The new Key. + """ + if new_name is None: + new_name = key.name + new_key = destination_bucket.new_key(new_name) + api_path = key.path + '/copyTo' + new_key.path + self.connection.api_request(method='POST', path=api_path) + return new_key def upload_file(self, filename, key=None): """Shortcut method to upload a file into this bucket. diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py index d32b227ed038..ff70e75cf4c7 100644 --- a/gcloud/storage/test_bucket.py +++ b/gcloud/storage/test_bucket.py @@ -248,6 +248,50 @@ def test_delete_keys_miss(self): self.assertEqual(kw[1]['method'], 'DELETE') self.assertEqual(kw[1]['path'], '/b/%s/o/%s' % (NAME, NONESUCH)) + def test_copy_keys_wo_name(self): + SOURCE = 'source' + DEST = 'dest' + KEY = 'key' + + class _Key(object): + name = KEY + path = '/b/%s/o/%s' % (SOURCE, KEY) + + connection = _Connection({}) + source = self._makeOne(connection, SOURCE) + dest = self._makeOne(connection, DEST) + key = _Key() + new_key = source.copy_key(key, dest) + self.assertTrue(new_key.bucket is dest) + self.assertEqual(new_key.name, KEY) + kw, = connection._requested + COPY_PATH = '/b/%s/o/%s/copyTo/b/%s/o/%s' % (SOURCE, KEY, DEST, KEY) + self.assertEqual(kw['method'], 'POST') + self.assertEqual(kw['path'], COPY_PATH) + + def test_copy_keys_w_name(self): + SOURCE = 'source' + DEST = 'dest' + KEY = 'key' + NEW_NAME = 'new_name' + + class _Key(object): + name = KEY + path = '/b/%s/o/%s' % (SOURCE, KEY) + + connection = _Connection({}) + source = self._makeOne(connection, SOURCE) + dest = self._makeOne(connection, DEST) + key = _Key() + new_key = source.copy_key(key, dest, NEW_NAME) + self.assertTrue(new_key.bucket is dest) + self.assertEqual(new_key.name, NEW_NAME) + kw, = connection._requested + COPY_PATH = ( + '/b/%s/o/%s/copyTo/b/%s/o/%s' % (SOURCE, KEY, DEST, NEW_NAME)) + self.assertEqual(kw['method'], 'POST') + self.assertEqual(kw['path'], COPY_PATH) + def test_upload_file_default_key(self): from gcloud.test_credentials import _Monkey from gcloud.storage import bucket as MUT From d4fe9bf84727b3ebbc3b2d1773168a900ce37bd5 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 7 Oct 2014 13:28:08 -0400 Subject: [PATCH 2/3] Add Key.rename from PR #99, with test. Manually applies patch from gsalgado: https://github.com/gsalgado/gcloud-python/commit/1c80d618f265c01ac01a794b5af12f03a6438fb1 But changes semantics to return the new key instance, leaving the original key unchanged. --- gcloud/storage/key.py | 14 ++++++++++++++ gcloud/storage/test_key.py | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py index 4562d81166ec..bac3f3b44737 100644 --- a/gcloud/storage/key.py +++ b/gcloud/storage/key.py @@ -134,6 +134,20 @@ def exists(self): return self.bucket.get_key(self.name) is not None + def rename(self, new_name): + """Renames this key. + + Effectively, copies key to the same bucket with a new name, then + deletes the key. + + :type new_name: string + :param new_name: The new name for this key. + + :rtype: :class:`Key` + :returns: The newly-copied key. + """ + return self.bucket.copy_key(self, self.bucket, new_name) + def delete(self): """Deletes a key from Cloud Storage. diff --git a/gcloud/storage/test_key.py b/gcloud/storage/test_key.py index 5926fea7ff19..48f43dd36bea 100644 --- a/gcloud/storage/test_key.py +++ b/gcloud/storage/test_key.py @@ -95,6 +95,23 @@ def test_exists_hit(self): bucket._keys[KEY] = 1 self.assertTrue(key.exists()) + def test_rename(self): + KEY = 'key' + NEW_NAME = 'new-name' + connection = _Connection() + bucket = _Bucket(connection) + key = self._makeOne(bucket, KEY) + bucket._keys[KEY] = 1 + orig_key_path = key.path + new_key = key.rename(NEW_NAME) + expected = [ + ['POST', orig_key_path + '/copyTo/b/bucket/o/%s' % NEW_NAME, None], + ['DELETE', orig_key_path, None]] + self.assertEqual(key.name, KEY) + self.assertEqual(new_key.name, NEW_NAME) + self.assertFalse(KEY in bucket._keys) + self.assertTrue(NEW_NAME in bucket._keys) + def test_delete(self): KEY = 'key' connection = _Connection() @@ -594,5 +611,9 @@ def __init__(self, connection): def get_key(self, key): return self._keys.get(key) # XXX s.b. 'key.name'? + def copy_key(self, key, destination_bucket, new_name): + destination_bucket._keys[new_name] = self._keys.pop(key.name) + return key.from_dict({'name': new_name}, bucket=destination_bucket) + def delete_key(self, key): del self._keys[key.name] # XXX s.b. 'key'? From dcaae7574720de412eb16202b4770afa59b31adc Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 7 Oct 2014 14:50:30 -0400 Subject: [PATCH 3/3] Ensure that Key.rename actually deletes old key as documents. Incorporate feedback from @dhermes. --- gcloud/storage/key.py | 4 +++- gcloud/storage/test_key.py | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py index bac3f3b44737..165c6f59dc41 100644 --- a/gcloud/storage/key.py +++ b/gcloud/storage/key.py @@ -146,7 +146,9 @@ def rename(self, new_name): :rtype: :class:`Key` :returns: The newly-copied key. """ - return self.bucket.copy_key(self, self.bucket, new_name) + new_key = self.bucket.copy_key(self, self.bucket, new_name) + self.bucket.delete_key(self) + return new_key def delete(self): """Deletes a key from Cloud Storage. diff --git a/gcloud/storage/test_key.py b/gcloud/storage/test_key.py index 48f43dd36bea..67fce59df204 100644 --- a/gcloud/storage/test_key.py +++ b/gcloud/storage/test_key.py @@ -104,12 +104,10 @@ def test_rename(self): bucket._keys[KEY] = 1 orig_key_path = key.path new_key = key.rename(NEW_NAME) - expected = [ - ['POST', orig_key_path + '/copyTo/b/bucket/o/%s' % NEW_NAME, None], - ['DELETE', orig_key_path, None]] self.assertEqual(key.name, KEY) self.assertEqual(new_key.name, NEW_NAME) self.assertFalse(KEY in bucket._keys) + self.assertTrue(KEY in bucket._deleted) self.assertTrue(NEW_NAME in bucket._keys) def test_delete(self): @@ -607,13 +605,15 @@ class _Bucket(object): def __init__(self, connection): self.connection = connection self._keys = {} + self._deleted = [] def get_key(self, key): return self._keys.get(key) # XXX s.b. 'key.name'? def copy_key(self, key, destination_bucket, new_name): - destination_bucket._keys[new_name] = self._keys.pop(key.name) + destination_bucket._keys[new_name] = self._keys[key.name] return key.from_dict({'name': new_name}, bucket=destination_bucket) def delete_key(self, key): del self._keys[key.name] # XXX s.b. 'key'? + self._deleted.append(key.name)