Merge pull request #456 from dhermes/fix-451-part1

Address first part of 451: Remove Key.from_path

Author: dhermes

gcloud/datastore/__init__.py

@@ -145,7 +145,7 @@ def get_entity(key):
     :param key: The name of the item to retrieve.
 
     :rtype: :class:`gcloud.datastore.entity.Entity` or ``None``
-    :return: The requested entity, or ``None`` if there was no match found.
+    :returns: The requested entity, or ``None`` if there was no match found.
     """
     return _require_dataset().get_entity(key)
 
@@ -157,7 +157,7 @@ def get_entities(keys):
     :param keys: The name of the item to retrieve.
 
     :rtype: list of :class:`gcloud.datastore.entity.Entity`
-    :return: The requested entities.
+    :returns: The requested entities.
     """
     return _require_dataset().get_entities(keys)
 
@@ -172,6 +172,6 @@ def allocate_ids(incomplete_key, num_ids):
     :param num_ids: The number of IDs to allocate.
 
     :rtype: list of :class:`gcloud.datastore.key.Key`
-    :return: The (complete) keys allocated with `incomplete_key` as root.
+    :returns: The (complete) keys allocated with `incomplete_key` as root.
     """
     return _require_dataset().allocate_ids(incomplete_key, num_ids)

gcloud/datastore/dataset.py

@@ -18,7 +18,6 @@
 from gcloud.datastore.entity import Entity
 from gcloud.datastore.query import Query
 from gcloud.datastore.transaction import Transaction
-from gcloud.datastore.key import Key
 
 
 class Dataset(object):
@@ -118,26 +117,17 @@ def transaction(self, *args, **kwargs):
         kwargs['dataset'] = self
         return Transaction(*args, **kwargs)
 
-    def get_entity(self, key_or_path):
+    def get_entity(self, key):
         """Retrieves entity from the dataset, along with its attributes.
 
-        :type key_or_path: :class:`gcloud.datastore.key.Key` or path
-        :param key_or_path: The name of the item to retrieve or sequence
-                            of even length, where the first of each pair
-                            is a string representing the 'kind' of the
-                            path element, and the second of the pair is
-                            either a string (for the path element's name)
-                            or an integer (for its id).
+        :type key: :class:`gcloud.datastore.key.Key` or path
+        :param key: The key of the entity to be retrieved.
 
-        :rtype: :class:`gcloud.datastore.entity.Entity` or ``None``
-        :return: The requested entity, or ``None`` if there was no match found.
+        :rtype: :class:`gcloud.datastore.entity.Entity` or `NoneType`
+        :returns: The requested entity, or ``None`` if there was no
+                  match found.
         """
-
-        if isinstance(key_or_path, Key):
-            entities = self.get_entities([key_or_path])
-        else:
-            key = Key.from_path(*key_or_path)
-            entities = self.get_entities([key])
+        entities = self.get_entities([key])
 
         if entities:
             return entities[0]
@@ -159,7 +149,7 @@ def get_entities(self, keys, missing=None, deferred=None):
                         Use only as a keyword param.
 
         :rtype: list of :class:`gcloud.datastore.entity.Entity`
-        :return: The requested entities.
+        :returns: The requested entities.
         """
         entity_pbs = self.connection().lookup(
             dataset_id=self.id(),
@@ -193,7 +183,7 @@ def allocate_ids(self, incomplete_key, num_ids):
         :param num_ids: The number of IDs to allocate.
 
         :rtype: list of :class:`gcloud.datastore.key.Key`
-        :return: The (complete) keys allocated with `incomplete_key` as root.
+        :returns: The (complete) keys allocated with `incomplete_key` as root.
         :raises: `ValueError` if `incomplete_key` is not a partial key.
         """
         if not incomplete_key.is_partial():

gcloud/datastore/key.py

@@ -15,9 +15,6 @@
 """Create / interact with gcloud datastore keys."""
 
 import copy
-from itertools import izip
-
-import six
 
 from gcloud.datastore import datastore_v1_pb2 as datastore_pb
 
@@ -90,40 +87,6 @@ def to_protobuf(self):
 
         return key
 
-    @classmethod
-    def from_path(cls, *args, **kwargs):
-        """Factory method for creating a key based on a path.
-
-        :type args: :class:`tuple`
-        :param args: sequence of even length, where the first of each pair is a
-                     string representing the 'kind' of the path element, and
-                     the second of the pair is either a string (for the path
-                     element's name) or an integer (for its id).
-
-        :type kwargs: :class:`dict`
-        :param kwargs: Other named parameters which can be passed to
-                       :func:`Key.__init__`.
-
-        :rtype: :class:`gcloud.datastore.key.Key`
-        :returns: a new :class:`Key` instance
-        """
-        if len(args) % 2:
-            raise ValueError('Must pass an even number of args.')
-
-        path = []
-        items = iter(args)
-
-        for kind, id_or_name in izip(items, items):
-            entry = {'kind': kind}
-            if isinstance(id_or_name, six.string_types):
-                entry['name'] = id_or_name
-            else:
-                entry['id'] = id_or_name
-            path.append(entry)
-
-        kwargs['path'] = path
-        return cls(**kwargs)
-
     def is_partial(self):
         """Boolean test: is the key fully mapped onto a backend entity?
 

gcloud/datastore/query.py

@@ -172,22 +172,15 @@ def ancestor(self, ancestor):
         This will return a clone of the current :class:`Query` filtered
         by the ancestor provided.  For example::
 
-          >>> parent_key = Key.from_path('Person', '1')
+          >>> parent_key = Key(path=[{'kind': 'Person', 'name': '1'}])
           >>> query = dataset.query('Person')
           >>> filtered_query = query.ancestor(parent_key)
 
-        If you don't have a :class:`gcloud.datastore.key.Key` but just
-        know the path, you can provide that as well::
-
-          >>> query = dataset.query('Person')
-          >>> filtered_query = query.ancestor(['Person', '1'])
-
         Each call to ``.ancestor()`` returns a cloned :class:`Query`,
         however a query may only have one ancestor at a time.
 
-        :type ancestor: :class:`gcloud.datastore.key.Key` or list
-        :param ancestor: Either a Key or a path of the form
-                         ``['Kind', 'id or name', 'Kind', 'id or name', ...]``.
+        :type ancestor: :class:`gcloud.datastore.key.Key`
+        :param ancestor: A Key to an entity
 
         :rtype: :class:`Query`
         :returns: A Query filtered by the ancestor provided.
@@ -211,13 +204,9 @@ def ancestor(self, ancestor):
         if not ancestor:
             return clone
 
-        # If a list was provided, turn it into a Key.
-        if isinstance(ancestor, list):
-            ancestor = Key.from_path(*ancestor)
-
         # If we don't have a Key value by now, something is wrong.
         if not isinstance(ancestor, Key):
-            raise TypeError('Expected list or Key, got %s.' % type(ancestor))
+            raise TypeError('Expected Key, got %s.' % type(ancestor))
 
         # Get the composite filter and add a new property filter.
         composite_filter = clone._pb.filter.composite_filter

gcloud/datastore/test_dataset.py

@@ -109,39 +109,6 @@ def test_get_entity_hit(self):
         self.assertEqual(list(result), ['foo'])
         self.assertEqual(result['foo'], 'Foo')
 
-    def test_get_entity_path(self):
-        from gcloud.datastore.connection import datastore_pb
-        DATASET_ID = 'DATASET'
-        KIND = 'Kind'
-        ID = 1234
-        PATH = [{'kind': KIND, 'id': ID}]
-        entity_pb = datastore_pb.Entity()
-        entity_pb.key.partition_id.dataset_id = DATASET_ID
-        path_element = entity_pb.key.path_element.add()
-        path_element.kind = KIND
-        path_element.id = ID
-        prop = entity_pb.property.add()
-        prop.name = 'foo'
-        prop.value.string_value = 'Foo'
-        connection = _Connection(entity_pb)
-        dataset = self._makeOne(DATASET_ID, connection)
-        result = dataset.get_entity([KIND, ID])
-        key = result.key()
-        self.assertEqual(key._dataset_id, DATASET_ID)
-        self.assertEqual(key.path(), PATH)
-        self.assertEqual(list(result), ['foo'])
-        self.assertEqual(result['foo'], 'Foo')
-
-    def test_get_entity_odd_nonetype(self):
-        DATASET_ID = 'DATASET'
-        KIND = 'Kind'
-        connection = _Connection()
-        dataset = self._makeOne(DATASET_ID, connection)
-        with self.assertRaises(ValueError):
-            dataset.get_entity([KIND])
-        with self.assertRaises(TypeError):
-            dataset.get_entity(None)
-
     def test_get_entities_miss(self):
         from gcloud.datastore.key import Key
         DATASET_ID = 'DATASET'

gcloud/datastore/test_key.py

@@ -102,39 +102,6 @@ def test_to_protobuf_w_explicit_path(self):
         self.assertEqual(elems[2].name, '')
         self.assertEqual(elems[2].id, 0)
 
-    def test_from_path_empty(self):
-        key = self._getTargetClass().from_path()
-        self.assertEqual(key._dataset_id, None)
-        self.assertEqual(key.namespace(), None)
-        self.assertEqual(key.kind(), '')
-        self.assertEqual(key.path(), [{'kind': ''}])
-
-    def test_from_path_single_element(self):
-        self.assertRaises(ValueError, self._getTargetClass().from_path, 'abc')
-
-    def test_from_path_three_elements(self):
-        self.assertRaises(ValueError, self._getTargetClass().from_path,
-                          'abc', 'def', 'ghi')
-
-    def test_from_path_two_elements_second_string(self):
-        key = self._getTargetClass().from_path('abc', 'def')
-        self.assertEqual(key.kind(), 'abc')
-        self.assertEqual(key.path(), [{'kind': 'abc', 'name': 'def'}])
-
-    def test_from_path_two_elements_second_int(self):
-        key = self._getTargetClass().from_path('abc', 123)
-        self.assertEqual(key.kind(), 'abc')
-        self.assertEqual(key.path(), [{'kind': 'abc', 'id': 123}])
-
-    def test_from_path_nested(self):
-        key = self._getTargetClass().from_path('abc', 'def', 'ghi', 123)
-        self.assertEqual(key.kind(), 'ghi')
-        expected_path = [
-            {'kind': 'abc', 'name': 'def'},
-            {'kind': 'ghi', 'id': 123},
-        ]
-        self.assertEqual(key.path(), expected_path)
-
     def test_is_partial_no_name_or_id(self):
         key = self._makeOne()
         self.assertTrue(key.is_partial())
@@ -282,9 +249,13 @@ def test_parent_default(self):
         self.assertEqual(key.parent(), None)
 
     def test_parent_explicit_top_level(self):
-        key = self._getTargetClass().from_path('abc', 'def')
+        key = self._makeOne(path=[{'kind': 'abc', 'name': 'def'}])
         self.assertEqual(key.parent(), None)
 
     def test_parent_explicit_nested(self):
-        key = self._getTargetClass().from_path('abc', 'def', 'ghi', 123)
-        self.assertEqual(key.parent().path(), [{'kind': 'abc', 'name': 'def'}])
+        parent_part = {'kind': 'abc', 'name': 'def'}
+        key = self._makeOne(path=[
+            parent_part,
+            {'kind': 'ghi', 'id': 123},
+        ])
+        self.assertEqual(key.parent().path(), [parent_part])

gcloud/datastore/test_query.py

@@ -168,9 +168,10 @@ def test_filter_w_whitespace_property_name(self):
         self.assertEqual(p_pb.value.string_value, u'John')
         self.assertEqual(p_pb.operator, datastore_pb.PropertyFilter.EQUAL)
 
-    def test_ancestor_w_non_key_non_list(self):
+    def test_ancestor_w_non_key(self):
         query = self._makeOne()
         self.assertRaises(TypeError, query.ancestor, object())
+        self.assertRaises(TypeError, query.ancestor, ['KIND', 'NAME'])
 
     def test_ancestor_wo_existing_ancestor_query_w_key_and_propfilter(self):
         from gcloud.datastore.key import Key
@@ -208,39 +209,27 @@ def test_ancestor_wo_existing_ancestor_query_w_key(self):
         self.assertEqual(p_pb.property.name, '__key__')
         self.assertEqual(p_pb.value.key_value, key.to_protobuf())
 
-    def test_ancestor_wo_existing_ancestor_query_w_list(self):
+    def test_ancestor_clears_existing_ancestor_query_w_only(self):
         from gcloud.datastore.key import Key
         _KIND = 'KIND'
         _ID = 123
         key = Key(path=[{'kind': _KIND, 'id': _ID}])
         query = self._makeOne()
-        after = query.ancestor([_KIND, _ID])
-        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
-        f_pb, = list(q_pb.filter.composite_filter.filter)
-        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_ancestor_clears_existing_ancestor_query_w_only(self):
-        _KIND = 'KIND'
-        _ID = 123
-        query = self._makeOne()
-        between = query.ancestor([_KIND, _ID])
+        between = query.ancestor(key)
         after = between.ancestor(None)
         self.assertFalse(after is query)
         self.assertTrue(isinstance(after, self._getTargetClass()))
         q_pb = after.to_protobuf()
         self.assertEqual(list(q_pb.filter.composite_filter.filter), [])
 
     def test_ancestor_clears_existing_ancestor_query_w_others(self):
+        from gcloud.datastore.key import Key
         _KIND = 'KIND'
         _ID = 123
         _NAME = u'NAME'
+        key = Key(path=[{'kind': _KIND, 'id': _ID}])
         query = self._makeOne().filter('name', '=', _NAME)
-        between = query.ancestor([_KIND, _ID])
+        between = query.ancestor(key)
         after = between.ancestor(None)
         self.assertFalse(after is query)
         self.assertTrue(isinstance(after, self._getTargetClass()))

pylintrc_default

@@ -1,4 +1,4 @@
-# PyLint config for 'gcloud' *library* code. 
+# PyLint config for 'gcloud' *library* code.
 #
 # NOTES:
 #
@@ -65,14 +65,14 @@ ignore =
 # DEFAULT:  disable=
 # RATIONALE:
 # - maybe-no-member:  bi-modal functions confuse pylint type inference.
-# - no-member:  indirections in protobuf-generated code 
+# - no-member:  indirections in protobuf-generated code
 # - protected-access: helpers use '_foo' of classes from generated code.
 # - redefined-builtin:  use of 'id', 'type', 'filter' args in API-bound funcs;
 #                       use of 'NotImplemented' to map HTTP response code.
 # - similarities:  'Bucket' and 'Key' define 'metageneration' and 'owner' with
 #                  identical implementation but different docstrings.
 # - star-args:  standard Python idioms for varargs:
-#                  ancestor = Key.from_path(*ancestor)
+#                  ancestor = Query().filter(*order_props)
 disable =
     maybe-no-member,
     no-member,
@@ -201,7 +201,7 @@ max-module-lines=1500
 
 # Good variable names which should always be accepted, separated by a comma
 # DEFAULT:  good-names=i,j,k,ex,Run,_
-# RATIONALE:  'pb' and 'id' have well-understood meainings in the code. 
+# RATIONALE:  'pb' and 'id' have well-understood meainings in the code.
 good-names = i, j, k, ex, Run, _,
              pb,
              id,

regression/datastore.py

@@ -146,7 +146,7 @@ def test_empty_kind(self):
 class TestDatastoreSaveKeys(TestDatastore):
 
     def test_save_key_self_reference(self):
-        key = datastore.key.Key.from_path('Person', 'name')
+        key = datastore.key.Key(path=[{'kind': 'Person', 'name': 'name'}])
         entity = datastore.entity.Entity(kind=None).key(key)
         entity['fullName'] = u'Full name'
         entity['linkedTo'] = key  # Self reference.
@@ -337,7 +337,7 @@ def test_query_group_by(self):
 class TestDatastoreTransaction(TestDatastore):
 
     def test_transaction(self):
-        key = datastore.key.Key.from_path('Company', 'Google')
+        key = datastore.key.Key(path=[{'kind': 'Company', 'name': 'Google'}])
         entity = datastore.entity.Entity(kind=None).key(key)
         entity['url'] = u'www.google.com'