From 80e47a7f1ce15d0b6da5a1fef4f7cc15582c0811 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 13:38:14 -0400
Subject: [PATCH 01/12] 100% branch coverage for gcloud.datastore.helpers.

Surfaced bug w/ unknown type passed to get_protobuf_attribute_and_value().
---
 gcloud/datastore/helpers.py      | 2 ++
 gcloud/datastore/test_helpers.py | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/gcloud/datastore/helpers.py b/gcloud/datastore/helpers.py
index 960ba3410be1..2e8aa5f80bda 100644
--- a/gcloud/datastore/helpers.py
+++ b/gcloud/datastore/helpers.py
@@ -60,6 +60,8 @@ def get_protobuf_attribute_and_value(val):
         name, value = 'integer', long(val)  # Always cast to a long.
     elif isinstance(val, basestring):
         name, value = 'string', val
+    else:
+        raise ValueError("Unknown protobuf attr type %s" % type(val))
 
     return name + '_value', value
 
diff --git a/gcloud/datastore/test_helpers.py b/gcloud/datastore/test_helpers.py
index e976a475668c..69f19fe9817f 100644
--- a/gcloud/datastore/test_helpers.py
+++ b/gcloud/datastore/test_helpers.py
@@ -83,6 +83,11 @@ def test_unicode(self):
         self.assertEqual(name, 'string_value')
         self.assertEqual(value, u'str')
 
+    def test_object(self):
+        class _Foo(object):
+            pass
+        self.assertRaises(ValueError, self._callFUT, _Foo())
+
 
 class Test_get_value_from_protobuf(unittest2.TestCase):
 

From df450025c3651335523f351758d9ca0cde5aa4d2 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 13:42:35 -0400
Subject: [PATCH 02/12] Toward 100% branch coverage for gcloud.datastore.key.

Surfaced bug w/ path element holding both id and name in Key.from_protobuf.
---
 gcloud/datastore/key.py      | 2 +-
 gcloud/datastore/test_key.py | 9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gcloud/datastore/key.py b/gcloud/datastore/key.py
index 6eb9a6b4c929..fa46cad28c43 100644
--- a/gcloud/datastore/key.py
+++ b/gcloud/datastore/key.py
@@ -66,7 +66,7 @@ def from_protobuf(cls, pb, dataset=None):
             if element.HasField('id'):
                 element_dict['id'] = element.id
 
-            elif element.HasField('name'):
+            if element.HasField('name'):
                 element_dict['name'] = element.name
 
             path.append(element_dict)
diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py
index b722b326ac48..73a59d6bdd1f 100644
--- a/gcloud/datastore/test_key.py
+++ b/gcloud/datastore/test_key.py
@@ -101,9 +101,16 @@ def test_from_protobuf_w_path_in_pb(self):
         pb = self._makePB(_DATASET, _NAMESPACE)
         _PARENT = 'PARENT'
         _CHILD = 'CHILD'
+        _GRANDCHILD = 'GRANDCHILD'
         _ID = 1234
+        _ID2 = 5678
         _NAME = 'NAME'
-        _PATH = [{'kind': _PARENT, 'name': _NAME}, {'kind': _CHILD, 'id': _ID}]
+        _NAME2 = 'NAME2'
+        _PATH = [
+            {'kind': _PARENT, 'name': _NAME},
+            {'kind': _CHILD, 'id': _ID},
+            {'kind': _GRANDCHILD, 'id': _ID2, 'name': _NAME2},
+        ]
         pb = self._makePB(path=_PATH)
         key = self._getTargetClass().from_protobuf(pb)
         self.assertEqual(key.path(), _PATH)

From 18d864072c24ea47469b856154f0e40082c8a3d8 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 13:45:24 -0400
Subject: [PATCH 03/12] Toward 100% branch coverage for gcloud.datastore.key.

Cover edge condition in Key.to_protobuf where dataset has empty ID.
---
 gcloud/datastore/test_key.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py
index 73a59d6bdd1f..fdda1dbe121a 100644
--- a/gcloud/datastore/test_key.py
+++ b/gcloud/datastore/test_key.py
@@ -127,6 +127,13 @@ def test_to_protobuf_defaults(self):
         self.assertEqual(elem.name, '')
         self.assertEqual(elem.id, 0)
 
+    def test_to_protobuf_w_explicit_dataset_empty_id(self):
+        from gcloud.datastore.dataset import Dataset
+        dataset = Dataset('')
+        key = self._makeOne(dataset)
+        pb = key.to_protobuf()
+        self.assertEqual(pb.partition_id.dataset_id, '')
+
     def test_to_protobuf_w_explicit_dataset_no_prefix(self):
         from gcloud.datastore.dataset import Dataset
         _DATASET = 'DATASET'

From 0b17af93b5820c60dd345dd38733b8ef83e44eba Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 13:51:34 -0400
Subject: [PATCH 04/12] Toward 100% branch coverage for gcloud.datastore.key.

Cover edge conditions in Key.to_protobuf where path elements are missing keys.
---
 gcloud/datastore/test_key.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py
index fdda1dbe121a..e2144964ca7e 100644
--- a/gcloud/datastore/test_key.py
+++ b/gcloud/datastore/test_key.py
@@ -169,14 +169,22 @@ def test_to_protobuf_w_explicit_path(self):
         _CHILD = 'CHILD'
         _ID = 1234
         _NAME = 'NAME'
-        _PATH = [{'kind': _PARENT, 'name': _NAME}, {'kind': _CHILD, 'id': _ID}]
+        _PATH = [
+            {'kind': _PARENT, 'name': _NAME},
+            {'kind': _CHILD, 'id': _ID},
+            {},
+        ]
         key = self._makeOne(path=_PATH)
         pb = key.to_protobuf()
         elems = list(pb.path_element)
+        self.assertEqual(len(elems), len(_PATH))
         self.assertEqual(elems[0].kind, _PARENT)
         self.assertEqual(elems[0].name, _NAME)
         self.assertEqual(elems[1].kind, _CHILD)
         self.assertEqual(elems[1].id, _ID)
+        self.assertEqual(elems[2].kind, '')
+        self.assertEqual(elems[2].name, '')
+        self.assertEqual(elems[2].id, 0)
 
     def test_from_path_empty(self):
         key = self._getTargetClass().from_path()

From 70570db90165f0f5b19e94a0158f59f8c79d5875 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 13:57:42 -0400
Subject: [PATCH 05/12] 100% branch coverage for gcloud.datastore.key.

Cover edge conditions in Key.{kind,id,name} where path is empty.
---
 gcloud/datastore/test_key.py | 51 +++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py
index e2144964ca7e..31d41d56fa4e 100644
--- a/gcloud/datastore/test_key.py
+++ b/gcloud/datastore/test_key.py
@@ -284,6 +284,15 @@ def test_path_setter(self):
         self.assertEqual(after.namespace(), _NAMESPACE)
         self.assertEqual(after.path(), _PATH)
 
+    def test_kind_getter_empty_path(self):
+        from gcloud.datastore.dataset import Dataset
+        _DATASET = 'DATASET'
+        _NAMESPACE = 'NAMESPACE'
+        dataset = Dataset(_DATASET)
+        key = self._makeOne(dataset, _NAMESPACE)
+        key._path = () # edge case
+        self.assertEqual(key.kind(), None)
+
     def test_kind_setter(self):
         from gcloud.datastore.dataset import Dataset
         _DATASET = 'DATASET'
@@ -301,22 +310,14 @@ def test_kind_setter(self):
         self.assertEqual(after.namespace(), _NAMESPACE)
         self.assertEqual(after.path(), [{'kind': _KIND_AFTER, 'name': _NAME}])
 
-    def test_name_setter(self):
+    def test_id_getter_empty_path(self):
         from gcloud.datastore.dataset import Dataset
         _DATASET = 'DATASET'
         _NAMESPACE = 'NAMESPACE'
-        _KIND = 'KIND'
-        _NAME_BEFORE = 'NAME_BEFORE'
-        _NAME_AFTER = 'NAME_AFTER'
-        _PATH = [{'kind': _KIND, 'name': _NAME_BEFORE}]
         dataset = Dataset(_DATASET)
-        key = self._makeOne(dataset, _NAMESPACE, _PATH)
-        after = key.name(_NAME_AFTER)
-        self.assertFalse(after is key)
-        self.assertTrue(isinstance(after, self._getTargetClass()))
-        self.assertTrue(after.dataset() is dataset)
-        self.assertEqual(after.namespace(), _NAMESPACE)
-        self.assertEqual(after.path(), [{'kind': _KIND, 'name': _NAME_AFTER}])
+        key = self._makeOne(dataset, _NAMESPACE)
+        key._path = () # edge case
+        self.assertEqual(key.id(), None)
 
     def test_id_setter(self):
         from gcloud.datastore.dataset import Dataset
@@ -335,6 +336,32 @@ def test_id_setter(self):
         self.assertEqual(after.namespace(), _NAMESPACE)
         self.assertEqual(after.path(), [{'kind': _KIND, 'id': _ID_AFTER}])
 
+    def test_name_getter_empty_path(self):
+        from gcloud.datastore.dataset import Dataset
+        _DATASET = 'DATASET'
+        _NAMESPACE = 'NAMESPACE'
+        dataset = Dataset(_DATASET)
+        key = self._makeOne(dataset, _NAMESPACE)
+        key._path = () # edge case
+        self.assertEqual(key.name(), None)
+
+    def test_name_setter(self):
+        from gcloud.datastore.dataset import Dataset
+        _DATASET = 'DATASET'
+        _NAMESPACE = 'NAMESPACE'
+        _KIND = 'KIND'
+        _NAME_BEFORE = 'NAME_BEFORE'
+        _NAME_AFTER = 'NAME_AFTER'
+        _PATH = [{'kind': _KIND, 'name': _NAME_BEFORE}]
+        dataset = Dataset(_DATASET)
+        key = self._makeOne(dataset, _NAMESPACE, _PATH)
+        after = key.name(_NAME_AFTER)
+        self.assertFalse(after is key)
+        self.assertTrue(isinstance(after, self._getTargetClass()))
+        self.assertTrue(after.dataset() is dataset)
+        self.assertEqual(after.namespace(), _NAMESPACE)
+        self.assertEqual(after.path(), [{'kind': _KIND, 'name': _NAME_AFTER}])
+
     def test_id_or_name_no_name_or_id(self):
         key = self._makeOne()
         self.assertEqual(key.id_or_name(), None)

From 21a3d022bafeb0105971cc9776e764ddd7004e1c Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 14:09:55 -0400
Subject: [PATCH 06/12] 100% branch coverage for gcloud.datastore.query.

Edge cases in Query.ancestor w/ existing non-ancestor filters.
---
 gcloud/datastore/test_query.py | 37 +++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/gcloud/datastore/test_query.py b/gcloud/datastore/test_query.py
index 9219bf58d6de..4c2cb132266c 100644
--- a/gcloud/datastore/test_query.py
+++ b/gcloud/datastore/test_query.py
@@ -77,6 +77,26 @@ def test_ancestor_w_non_key_non_list(self):
         # XXX s.b. ValueError
         self.assertRaises(TypeError, query.ancestor, object())
 
+    def test_ancester_wo_existing_ancestor_query_w_key_and_propfilter(self):
+        from gcloud.datastore.key import Key
+        _KIND = 'KIND'
+        _ID = 123
+        _NAME = 'NAME'
+        key = Key(path=[{'kind': _KIND, 'id': _ID}])
+        query = self._makeOne().filter('name =', _NAME)
+        after = query.ancestor(key)
+        self.assertFalse(after is query)
+        self.assertTrue(isinstance(after, self._getTargetClass()))
+        q_pb = after.to_protobuf()
+        self.assertEqual(q_pb.filter.composite_filter.operator, 1)  # AND
+        n_pb, f_pb, = list(q_pb.filter.composite_filter.filter)
+        p_pb = n_pb.property_filter
+        self.assertEqual(p_pb.property.name, 'name')
+        self.assertEqual(p_pb.value.string_value, _NAME)
+        p_pb = f_pb.property_filter
+        self.assertEqual(p_pb.property.name, '__key__')
+        self.assertEqual(p_pb.value.key_value, key.to_protobuf())
+
     def test_ancester_wo_existing_ancestor_query_w_key(self):
         from gcloud.datastore.key import Key
         _KIND = 'KIND'
@@ -109,7 +129,7 @@ def test_ancester_wo_existing_ancestor_query_w_list(self):
         self.assertEqual(p_pb.property.name, '__key__')
         self.assertEqual(p_pb.value.key_value, key.to_protobuf())
 
-    def test_ancester_clears_existing_ancestor_query(self):
+    def test_ancester_clears_existing_ancestor_query_w_only(self):
         _KIND = 'KIND'
         _ID = 123
         query = self._makeOne()
@@ -120,6 +140,21 @@ def test_ancester_clears_existing_ancestor_query(self):
         q_pb = after.to_protobuf()
         self.assertEqual(list(q_pb.filter.composite_filter.filter), [])
 
+    def test_ancester_clears_existing_ancestor_query_w_others(self):
+        _KIND = 'KIND'
+        _ID = 123
+        _NAME = 'NAME'
+        query = self._makeOne().filter('name =', _NAME)
+        between = query.ancestor([_KIND, _ID])
+        after = between.ancestor(None)
+        self.assertFalse(after is query)
+        self.assertTrue(isinstance(after, self._getTargetClass()))
+        q_pb = after.to_protobuf()
+        n_pb, = list(q_pb.filter.composite_filter.filter)
+        p_pb = n_pb.property_filter
+        self.assertEqual(p_pb.property.name, 'name')
+        self.assertEqual(p_pb.value.string_value, _NAME)
+
     def test_kind_setter_wo_existing(self):
         from gcloud.datastore.dataset import Dataset
         _DATASET = 'DATASET'

From 4f35659de31fff0452a5f295df9e117fd16a54a1 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 14:11:20 -0400
Subject: [PATCH 07/12] 100% branch coverage for gcloud.datastore.test_key.

Don't worry about path elem w/o kind.
---
 gcloud/datastore/test_key.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py
index 31d41d56fa4e..2dd5f23e307f 100644
--- a/gcloud/datastore/test_key.py
+++ b/gcloud/datastore/test_key.py
@@ -19,8 +19,7 @@ def _makePB(self, dataset_id=None, namespace=None, path=()):
             pb.partition_id.namespace = namespace
         for elem in path:
             added = pb.path_element.add()
-            if 'kind' in elem:
-                added.kind = elem['kind']
+            added.kind = elem['kind']
             if 'id' in elem:
                 added.id = elem['id']
             if 'name' in elem:

From f4cf251aa880324e296ccc1b57d6b0bf545f3890 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 14:14:26 -0400
Subject: [PATCH 08/12] 100% branch coverage for gcloud.storage.acl.

Edge case in ACL.__iter__ for empty role.
---
 gcloud/storage/test_acl.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py
index e268eb712f1c..663923663151 100644
--- a/gcloud/storage/test_acl.py
+++ b/gcloud/storage/test_acl.py
@@ -153,6 +153,14 @@ def test___iter___non_empty_w_roles(self):
         self.assertEqual(list(acl),
                          [{'entity': '%s-%s' % (TYPE, ID), 'role': ROLE}])
 
+    def test___iter___non_empty_w_empty_role(self):
+        TYPE = 'type'
+        ID = 'id'
+        acl = self._makeOne()
+        entity = acl.entity(TYPE, ID)
+        entity.grant('')
+        self.assertEqual(list(acl), [])
+
     def test_entity_from_dict_allUsers(self):
         ROLE = 'role'
         acl = self._makeOne()

From a3c0405e020b6366f0f1a5fe9acf3dc35f02adaa Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 14:20:15 -0400
Subject: [PATCH 09/12] 100% branch coverage for gcloud.storage.connection.

Exercise Connection.delete_bucket w/ 'force' passed as False.
---
 gcloud/storage/test_connection.py | 37 +++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/gcloud/storage/test_connection.py b/gcloud/storage/test_connection.py
index 213016af25c2..fcac3276c30e 100644
--- a/gcloud/storage/test_connection.py
+++ b/gcloud/storage/test_connection.py
@@ -491,6 +491,41 @@ def test_create_bucket_ok(self):
     def test_delete_bucket_defaults_miss(self):
         _deleted_keys = []
 
+        class _Key(object):
+            pass
+
+        class _Bucket(object):
+
+            def __init__(self, name):
+                self._name = name
+                self.path = '/b/' + name
+
+        PROJECT = 'project'
+        KEY = 'key'
+        conn = self._makeOne(PROJECT)
+        URI = '/'.join([conn.API_BASE_URL,
+                        'storage',
+                        conn.API_VERSION,
+                        'b',
+                        'key?project=%s' % PROJECT,
+                        ])
+        http = conn._http = Http({'status': '200',
+                                  'content-type': 'application/json',
+                                  },
+                                 '{}')
+
+        def _new_bucket(name):
+            return _Bucket(name)
+
+        conn.new_bucket = _new_bucket
+        self.assertEqual(conn.delete_bucket(KEY), True)
+        self.assertEqual(_deleted_keys, [])
+        self.assertEqual(http._called_with['method'], 'DELETE')
+        self.assertEqual(http._called_with['uri'], URI)
+
+    def test_delete_bucket_force_True(self):
+        _deleted_keys = []
+
         class _Key(object):
 
             def __init__(self, name):
@@ -507,6 +542,7 @@ def __init__(self, name):
 
             def __iter__(self):
                 return iter([_Key(x) for x in ('foo', 'bar')])
+
         PROJECT = 'project'
         KEY = 'key'
         conn = self._makeOne(PROJECT)
@@ -523,6 +559,7 @@ def __iter__(self):
 
         def _new_bucket(name):
             return _Bucket(name)
+
         conn.new_bucket = _new_bucket
         self.assertEqual(conn.delete_bucket(KEY, True), True)
         self.assertEqual(_deleted_keys, ['foo', 'bar'])

From b9bec6e2941349fc270809a3cde90d1793a4a178 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 14:38:50 -0400
Subject: [PATCH 10/12] Don't derive from object w/o need.

Incorporate feedback from @dhermes.
---
 gcloud/datastore/test_helpers.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/gcloud/datastore/test_helpers.py b/gcloud/datastore/test_helpers.py
index 69f19fe9817f..0161e7798cb2 100644
--- a/gcloud/datastore/test_helpers.py
+++ b/gcloud/datastore/test_helpers.py
@@ -84,9 +84,7 @@ def test_unicode(self):
         self.assertEqual(value, u'str')
 
     def test_object(self):
-        class _Foo(object):
-            pass
-        self.assertRaises(ValueError, self._callFUT, _Foo())
+        self.assertRaises(ValueError, self._callFUT, object())
 
 
 class Test_get_value_from_protobuf(unittest2.TestCase):

From 4ee2e17ab30781c072741561da48a6f73db16f5e Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 14:39:55 -0400
Subject: [PATCH 11/12] Appease pylint E261.

---
 gcloud/datastore/test_key.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py
index 2dd5f23e307f..5fca22f24358 100644
--- a/gcloud/datastore/test_key.py
+++ b/gcloud/datastore/test_key.py
@@ -289,7 +289,7 @@ def test_kind_getter_empty_path(self):
         _NAMESPACE = 'NAMESPACE'
         dataset = Dataset(_DATASET)
         key = self._makeOne(dataset, _NAMESPACE)
-        key._path = () # edge case
+        key._path = ()  # edge case
         self.assertEqual(key.kind(), None)
 
     def test_kind_setter(self):
@@ -315,7 +315,7 @@ def test_id_getter_empty_path(self):
         _NAMESPACE = 'NAMESPACE'
         dataset = Dataset(_DATASET)
         key = self._makeOne(dataset, _NAMESPACE)
-        key._path = () # edge case
+        key._path = ()  # edge case
         self.assertEqual(key.id(), None)
 
     def test_id_setter(self):
@@ -341,7 +341,7 @@ def test_name_getter_empty_path(self):
         _NAMESPACE = 'NAMESPACE'
         dataset = Dataset(_DATASET)
         key = self._makeOne(dataset, _NAMESPACE)
-        key._path = () # edge case
+        key._path = ()  # edge case
         self.assertEqual(key.name(), None)
 
     def test_name_setter(self):

From 03c740dba5a36e48b810a399f5d0e3f0b2d05027 Mon Sep 17 00:00:00 2001
From: Tres Seaver <tseaver@palladion.com>
Date: Tue, 7 Oct 2014 16:34:11 -0400
Subject: [PATCH 12/12] Add note to 'name' branch in Key.from_protobuf.

Document that we expect only one of 'id' or 'name' to be found, but
DTRT anyway if both happen to be present.
---
 gcloud/datastore/key.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcloud/datastore/key.py b/gcloud/datastore/key.py
index fa46cad28c43..16e0c4ea1f66 100644
--- a/gcloud/datastore/key.py
+++ b/gcloud/datastore/key.py
@@ -66,6 +66,8 @@ def from_protobuf(cls, pb, dataset=None):
             if element.HasField('id'):
                 element_dict['id'] = element.id
 
+            # This is safe: we expect proto objects returned will only have
+            # one of `name` or `id` set.
             if element.HasField('name'):
                 element_dict['name'] = element.name