Merge pull request #888 from dhermes/fix-887

Only allowing tuple or list for ctor inputs which assume a sequence

gcloud/_helpers.py

@@ -72,6 +72,29 @@ def top(self):
             return self._stack[-1]
 
 
+def _ensure_tuple_or_list(arg_name, tuple_or_list):
+    """Ensures an input is a tuple or list.
+
+    This effectively reduces the iterable types allowed to a very short
+    whitelist: list and tuple.
+
+    :type arg_name: string
+    :param arg_name: Name of argument to use in error message.
+
+    :type tuple_or_list: sequence of string
+    :param tuple_or_list: Sequence to be verified.
+
+    :rtype: list of string
+    :returns: The ``tuple_or_list`` passed in cast to a ``list``.
+    :raises: class:`TypeError` if the ``tuple_or_list`` is not a tuple or
+             list.
+    """
+    if not isinstance(tuple_or_list, (tuple, list)):
+        raise TypeError('Expected %s to be a tuple or list. '
+                        'Received %r' % (arg_name, tuple_or_list))
+    return list(tuple_or_list)
+
+
 class _LazyProperty(object):
     """Descriptor for lazy loaded property.
 

gcloud/datastore/entity.py

@@ -15,6 +15,9 @@
 """Class for representing a single entity in the Cloud Datastore."""
 
 
+from gcloud._helpers import _ensure_tuple_or_list
+
+
 class Entity(dict):
     """Entities are akin to rows in a relational database
 
@@ -76,7 +79,8 @@ class Entity(dict):
     def __init__(self, key=None, exclude_from_indexes=()):
         super(Entity, self).__init__()
         self.key = key
-        self._exclude_from_indexes = set(exclude_from_indexes)
+        self._exclude_from_indexes = set(_ensure_tuple_or_list(
+            'exclude_from_indexes', exclude_from_indexes))
 
     def __eq__(self, other):
         """Compare two entities for equality.

gcloud/datastore/query.py

@@ -16,6 +16,7 @@
 
 import base64
 
+from gcloud._helpers import _ensure_tuple_or_list
 from gcloud.datastore import _implicit_environ
 from gcloud.datastore import _datastore_v1_pb2 as datastore_pb
 from gcloud.datastore import helpers
@@ -90,9 +91,9 @@ def __init__(self,
         self._namespace = namespace
         self._ancestor = ancestor
         self._filters = list(filters)
-        self._projection = list(projection)
-        self._order = list(order)
-        self._group_by = list(group_by)
+        self._projection = _ensure_tuple_or_list('projection', projection)
+        self._order = _ensure_tuple_or_list('order', order)
+        self._group_by = _ensure_tuple_or_list('group_by', group_by)
 
     @property
     def dataset_id(self):

gcloud/datastore/test_entity.py

@@ -52,6 +52,12 @@ def test_ctor_explicit(self):
         self.assertEqual(sorted(entity.exclude_from_indexes),
                          sorted(_EXCLUDE_FROM_INDEXES))
 
+    def test_ctor_bad_exclude_from_indexes(self):
+        BAD_EXCLUDE_FROM_INDEXES = object()
+        key = _Key()
+        self.assertRaises(TypeError, self._makeOne, key=key,
+                          exclude_from_indexes=BAD_EXCLUDE_FROM_INDEXES)
+
     def test___eq_____ne___w_non_entity(self):
         from gcloud.datastore.key import Key
         key = Key(_KIND, _ID, dataset_id=_DATASET_ID)

gcloud/datastore/test_query.py

@@ -79,6 +79,27 @@ def test_ctor_explicit(self):
         self.assertEqual(query.order, ORDER)
         self.assertEqual(query.group_by, GROUP_BY)
 
+    def test_ctor_bad_projection(self):
+        _DATASET = 'DATASET'
+        _KIND = 'KIND'
+        BAD_PROJECTION = object()
+        self.assertRaises(TypeError, self._makeOne, _KIND, _DATASET,
+                          projection=BAD_PROJECTION)
+
+    def test_ctor_bad_order(self):
+        _DATASET = 'DATASET'
+        _KIND = 'KIND'
+        BAD_ORDER = object()
+        self.assertRaises(TypeError, self._makeOne, _KIND, _DATASET,
+                          order=BAD_ORDER)
+
+    def test_ctor_bad_group_by(self):
+        _DATASET = 'DATASET'
+        _KIND = 'KIND'
+        BAD_GROUP_BY = object()
+        self.assertRaises(TypeError, self._makeOne, _KIND, _DATASET,
+                          group_by=BAD_GROUP_BY)
+
     def test_namespace_setter_w_non_string(self):
         _DATASET = 'DATASET'
         query = self._makeOne(dataset_id=_DATASET)

gcloud/test__helpers.py

@@ -43,6 +43,33 @@ def test_it(self):
         self.assertEqual(list(batches), [])
 
 
+class Test__ensure_tuple_or_list(unittest2.TestCase):
+
+    def _callFUT(self, arg_name, tuple_or_list):
+        from gcloud._helpers import _ensure_tuple_or_list
+        return _ensure_tuple_or_list(arg_name, tuple_or_list)
+
+    def test_valid_tuple(self):
+        valid_tuple_or_list = ('a', 'b', 'c', 'd')
+        result = self._callFUT('ARGNAME', valid_tuple_or_list)
+        self.assertEqual(result, ['a', 'b', 'c', 'd'])
+
+    def test_valid_list(self):
+        valid_tuple_or_list = ['a', 'b', 'c', 'd']
+        result = self._callFUT('ARGNAME', valid_tuple_or_list)
+        self.assertEqual(result, valid_tuple_or_list)
+
+    def test_invalid(self):
+        invalid_tuple_or_list = object()
+        with self.assertRaises(TypeError):
+            self._callFUT('ARGNAME', invalid_tuple_or_list)
+
+    def test_invalid_iterable(self):
+        invalid_tuple_or_list = 'FOO'
+        with self.assertRaises(TypeError):
+            self._callFUT('ARGNAME', invalid_tuple_or_list)
+
+
 class Test__LazyProperty(unittest2.TestCase):
 
     def _getTargetClass(self):