From 029249c803ecdec66cf854a9de998a021f423398 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 3 Nov 2014 14:05:16 -0800 Subject: [PATCH 1/4] Adding extra parameters for querying Cloud Storage keys. --- gcloud/storage/iterator.py | 10 ++++++++-- gcloud/storage/key.py | 11 ++++++----- gcloud/storage/test_bucket.py | 6 ++++-- gcloud/storage/test_iterator.py | 7 +++++++ 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/gcloud/storage/iterator.py b/gcloud/storage/iterator.py index 9f3d098df4cf..b6e2e3512641 100644 --- a/gcloud/storage/iterator.py +++ b/gcloud/storage/iterator.py @@ -38,11 +38,12 @@ class Iterator(object): :type path: string :param path: The path to query for the list of items. """ - def __init__(self, connection, path): + def __init__(self, connection, path, extra_params=None): self.connection = connection self.path = path self.page_number = 0 self.next_page_token = None + self.extra_params = extra_params def __iter__(self): """Iterate through the list of items.""" @@ -69,8 +70,13 @@ def get_query_params(self): :rtype: dict or None :returns: A dictionary of query parameters or None if there are none. """ + result = None if self.next_page_token: - return {'pageToken': self.next_page_token} + result = {'pageToken': self.next_page_token} + if self.extra_params is not None: + result = result or {} + result.update(self.extra_params) + return result def get_next_page_response(self): """Requests the next page from the path provided. diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py index e7e5559b9f65..cac96b2471f9 100644 --- a/gcloud/storage/key.py +++ b/gcloud/storage/key.py @@ -342,9 +342,9 @@ def upload_from_string(self, data, content_type='text/plain'): """ string_buffer = StringIO() string_buffer.write(data) - self.set_contents_from_file(file_obj=string_buffer, rewind=True, - size=string_buffer.len, - content_type=content_type) + self.upload_from_file(file_obj=string_buffer, rewind=True, + size=string_buffer.len, + content_type=content_type) return self # NOTE: Alias for boto-like API. @@ -369,10 +369,11 @@ class _KeyIterator(Iterator): :type bucket: :class:`gcloud.storage.bucket.Bucket` :param bucket: The bucket from which to list keys. """ - def __init__(self, bucket): + def __init__(self, bucket, extra_params=None): self.bucket = bucket super(_KeyIterator, self).__init__( - connection=bucket.connection, path=bucket.path + '/o') + connection=bucket.connection, path=bucket.path + '/o', + extra_params=extra_params) def get_items_from_response(self, response): """Factory method, yields :class:`.storage.key.Key` items from response. diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py index 7ca687acea76..153c9858d532 100644 --- a/gcloud/storage/test_bucket.py +++ b/gcloud/storage/test_bucket.py @@ -334,8 +334,9 @@ def __init__(self, bucket, name): self._bucket = bucket self._name = name - def set_contents_from_filename(self, filename): + def upload_from_filename(self, filename): _uploaded.append((self._bucket, self._name, filename)) + bucket = self._makeOne() with _Monkey(MUT, Key=_Key): bucket.upload_file(FILENAME) @@ -354,8 +355,9 @@ def __init__(self, bucket, name): self._bucket = bucket self._name = name - def set_contents_from_filename(self, filename): + def upload_from_filename(self, filename): _uploaded.append((self._bucket, self._name, filename)) + bucket = self._makeOne() with _Monkey(MUT, Key=_Key): bucket.upload_file(FILENAME, KEY) diff --git a/gcloud/storage/test_iterator.py b/gcloud/storage/test_iterator.py index 954bc46f1473..7ebfd7dc5a90 100644 --- a/gcloud/storage/test_iterator.py +++ b/gcloud/storage/test_iterator.py @@ -75,6 +75,13 @@ def test_get_query_params_w_token(self): self.assertEqual(iterator.get_query_params(), {'pageToken': TOKEN}) + def test_get_query_params_extra_params(self): + connection = _Connection() + PATH = '/foo' + extra_params = {'key': 'val'} + iterator = self._makeOne(connection, PATH, extra_params=extra_params) + self.assertEqual(iterator.get_query_params(), extra_params) + def test_get_next_page_response_new_no_token_in_response(self): PATH = '/foo' TOKEN = 'token' From 88e5234e94d07537ab787edf286d683a51632eb2 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 3 Nov 2014 14:17:20 -0800 Subject: [PATCH 2/4] Testing interaction with page token and extra params in iterator. Also making sure tests pass by using the `upload_` prefix instead of the `set_contents_` prefix. --- gcloud/storage/bucket.py | 4 ++-- gcloud/storage/iterator.py | 12 +++++++++++- gcloud/storage/test_bucket.py | 6 ++++-- gcloud/storage/test_iterator.py | 19 +++++++++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py index 6efa3a0d7afb..40c035e9c0f2 100644 --- a/gcloud/storage/bucket.py +++ b/gcloud/storage/bucket.py @@ -300,7 +300,7 @@ def upload_file(self, filename, key=None): if key is None: key = os.path.basename(filename) key = self.new_key(key) - return key.set_contents_from_filename(filename) + return key.upload_from_filename(filename) def upload_file_object(self, file_obj, key=None): """Shortcut method to upload a file object into this bucket. @@ -340,7 +340,7 @@ def upload_file_object(self, file_obj, key=None): key = self.new_key(key) else: key = self.new_key(os.path.basename(file_obj.name)) - return key.set_contents_from_file(file_obj) + return key.upload_from_file(file_obj) def configure_website(self, main_page_suffix=None, not_found_page=None): """Configure website-related metadata. diff --git a/gcloud/storage/iterator.py b/gcloud/storage/iterator.py index b6e2e3512641..b7efd4a6da43 100644 --- a/gcloud/storage/iterator.py +++ b/gcloud/storage/iterator.py @@ -38,12 +38,22 @@ class Iterator(object): :type path: string :param path: The path to query for the list of items. """ + + PAGE_TOKEN = 'pageToken' + RESERVED_PARAMS = frozenset([PAGE_TOKEN]) + def __init__(self, connection, path, extra_params=None): self.connection = connection self.path = path self.page_number = 0 self.next_page_token = None self.extra_params = extra_params + if self.extra_params is not None: + reserved_in_use = self.RESERVED_PARAMS.intersection( + extra_params.keys()) + if reserved_in_use: + raise ValueError(('Using a reserved parameter', + reserved_in_use)) def __iter__(self): """Iterate through the list of items.""" @@ -72,7 +82,7 @@ def get_query_params(self): """ result = None if self.next_page_token: - result = {'pageToken': self.next_page_token} + result = {self.PAGE_TOKEN: self.next_page_token} if self.extra_params is not None: result = result or {} result.update(self.extra_params) diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py index 153c9858d532..600bce898060 100644 --- a/gcloud/storage/test_bucket.py +++ b/gcloud/storage/test_bucket.py @@ -376,8 +376,9 @@ def __init__(self, bucket, name): self._bucket = bucket self._name = name - def set_contents_from_file(self, fh): + def upload_from_file(self, fh): _uploaded.append((self._bucket, self._name, fh)) + bucket = self._makeOne() with _Monkey(MUT, Key=_Key): bucket.upload_file_object(FILEOBJECT) @@ -397,8 +398,9 @@ def __init__(self, bucket, name): self._bucket = bucket self._name = name - def set_contents_from_file(self, fh): + def upload_from_file(self, fh): _uploaded.append((self._bucket, self._name, fh)) + bucket = self._makeOne() with _Monkey(MUT, Key=_Key): bucket.upload_file_object(FILEOBJECT, KEY) diff --git a/gcloud/storage/test_iterator.py b/gcloud/storage/test_iterator.py index 7ebfd7dc5a90..ab6f3516f1b7 100644 --- a/gcloud/storage/test_iterator.py +++ b/gcloud/storage/test_iterator.py @@ -82,6 +82,25 @@ def test_get_query_params_extra_params(self): iterator = self._makeOne(connection, PATH, extra_params=extra_params) self.assertEqual(iterator.get_query_params(), extra_params) + def test_get_query_params_w_token_and_extra_params(self): + connection = _Connection() + PATH = '/foo' + TOKEN = 'token' + extra_params = {'key': 'val'} + iterator = self._makeOne(connection, PATH, extra_params=extra_params) + iterator.next_page_token = TOKEN + + expected_query = extra_params.copy() + expected_query.update({'pageToken': TOKEN}) + self.assertEqual(iterator.get_query_params(), expected_query) + + def test_get_query_params_w_token_collision(self): + connection = _Connection() + PATH = '/foo' + extra_params = {'pageToken': 'val'} + self.assertRaises(ValueError, self._makeOne, connection, PATH, + extra_params=extra_params) + def test_get_next_page_response_new_no_token_in_response(self): PATH = '/foo' TOKEN = 'token' From 018795fd57dbf5d66803531b62f308dca4d7087b Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 4 Nov 2014 13:38:22 -0800 Subject: [PATCH 3/4] Using {} as default in iterator for extra_params and get_query_params. --- gcloud/storage/iterator.py | 23 +++++++++-------------- gcloud/storage/test_bucket.py | 10 +++++----- gcloud/storage/test_iterator.py | 6 +++--- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/gcloud/storage/iterator.py b/gcloud/storage/iterator.py index b7efd4a6da43..ae5edc1e7b0c 100644 --- a/gcloud/storage/iterator.py +++ b/gcloud/storage/iterator.py @@ -47,17 +47,15 @@ def __init__(self, connection, path, extra_params=None): self.path = path self.page_number = 0 self.next_page_token = None - self.extra_params = extra_params - if self.extra_params is not None: - reserved_in_use = self.RESERVED_PARAMS.intersection( - extra_params.keys()) - if reserved_in_use: - raise ValueError(('Using a reserved parameter', - reserved_in_use)) + self.extra_params = extra_params or {} + reserved_in_use = self.RESERVED_PARAMS.intersection( + self.extra_params.keys()) + if reserved_in_use: + raise ValueError(('Using a reserved parameter', + reserved_in_use)) def __iter__(self): """Iterate through the list of items.""" - while self.has_next_page(): response = self.get_next_page_response() for item in self.get_items_from_response(response): @@ -80,12 +78,9 @@ def get_query_params(self): :rtype: dict or None :returns: A dictionary of query parameters or None if there are none. """ - result = None - if self.next_page_token: - result = {self.PAGE_TOKEN: self.next_page_token} - if self.extra_params is not None: - result = result or {} - result.update(self.extra_params) + result = ({self.PAGE_TOKEN: self.next_page_token} + if self.next_page_token else {}) + result.update(self.extra_params) return result def get_next_page_response(self): diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py index 600bce898060..50841d16323c 100644 --- a/gcloud/storage/test_bucket.py +++ b/gcloud/storage/test_bucket.py @@ -77,7 +77,7 @@ def test___iter___empty(self): kw, = connection._requested self.assertEqual(kw['method'], 'GET') self.assertEqual(kw['path'], '/b/%s/o' % NAME) - self.assertEqual(kw['query_params'], None) + self.assertEqual(kw['query_params'], {}) def test___iter___non_empty(self): NAME = 'name' @@ -91,7 +91,7 @@ def test___iter___non_empty(self): kw, = connection._requested self.assertEqual(kw['method'], 'GET') self.assertEqual(kw['path'], '/b/%s/o' % NAME) - self.assertEqual(kw['query_params'], None) + self.assertEqual(kw['query_params'], {}) def test___contains___miss(self): NAME = 'name' @@ -154,7 +154,7 @@ def test_get_all_keys_empty(self): kw, = connection._requested self.assertEqual(kw['method'], 'GET') self.assertEqual(kw['path'], '/b/%s/o' % NAME) - self.assertEqual(kw['query_params'], None) + self.assertEqual(kw['query_params'], {}) def test_get_all_keys_non_empty(self): NAME = 'name' @@ -168,7 +168,7 @@ def test_get_all_keys_non_empty(self): kw, = connection._requested self.assertEqual(kw['method'], 'GET') self.assertEqual(kw['path'], '/b/%s/o' % NAME) - self.assertEqual(kw['query_params'], None) + self.assertEqual(kw['query_params'], {}) def test_new_key_existing(self): from gcloud.storage.key import Key @@ -715,7 +715,7 @@ def get_items_from_response(self, response): self.assertEqual(kw[0]['query_params'], {'projection': 'full'}) self.assertEqual(kw[1]['method'], 'GET') self.assertEqual(kw[1]['path'], '/b/%s/o' % NAME) - self.assertEqual(kw[1]['query_params'], None) + self.assertEqual(kw[1]['query_params'], {}) class TestBucketIterator(unittest2.TestCase): diff --git a/gcloud/storage/test_iterator.py b/gcloud/storage/test_iterator.py index ab6f3516f1b7..b7567602218d 100644 --- a/gcloud/storage/test_iterator.py +++ b/gcloud/storage/test_iterator.py @@ -36,7 +36,7 @@ def _get_items(response): kw, = connection._requested self.assertEqual(kw['method'], 'GET') self.assertEqual(kw['path'], PATH) - self.assertEqual(kw['query_params'], None) + self.assertEqual(kw['query_params'], {}) def test_has_next_page_new(self): connection = _Connection() @@ -64,7 +64,7 @@ def test_get_query_params_no_token(self): connection = _Connection() PATH = '/foo' iterator = self._makeOne(connection, PATH) - self.assertEqual(iterator.get_query_params(), None) + self.assertEqual(iterator.get_query_params(), {}) def test_get_query_params_w_token(self): connection = _Connection() @@ -116,7 +116,7 @@ def test_get_next_page_response_new_no_token_in_response(self): kw, = connection._requested self.assertEqual(kw['method'], 'GET') self.assertEqual(kw['path'], PATH) - self.assertEqual(kw['query_params'], None) + self.assertEqual(kw['query_params'], {}) def test_get_next_page_response_no_token(self): connection = _Connection() From 2f28e9f9b1522f09f13d09931040c6a472739c30 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 4 Nov 2014 14:13:19 -0800 Subject: [PATCH 4/4] Updating docstring in Iterator.get_query_params. --- gcloud/storage/iterator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcloud/storage/iterator.py b/gcloud/storage/iterator.py index ae5edc1e7b0c..8986560175be 100644 --- a/gcloud/storage/iterator.py +++ b/gcloud/storage/iterator.py @@ -75,8 +75,8 @@ def has_next_page(self): def get_query_params(self): """Getter for query parameters for the next request. - :rtype: dict or None - :returns: A dictionary of query parameters or None if there are none. + :rtype: dict + :returns: A dictionary of query parameters. """ result = ({self.PAGE_TOKEN: self.next_page_token} if self.next_page_token else {})