Skip to content

Commit

Permalink
Remove Key.from_path and derivative uses.
Browse files Browse the repository at this point in the history
Addresses first part of googleapis#451. Also removes uses of path in
place of key since Key constructor will enable such easy
use going forward.
  • Loading branch information
dhermes committed Dec 23, 2014
1 parent 7cd77f2 commit 3c394bb
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 167 deletions.
6 changes: 3 additions & 3 deletions gcloud/datastore/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)
28 changes: 9 additions & 19 deletions gcloud/datastore/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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]
Expand All @@ -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(),
Expand Down Expand Up @@ -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():
Expand Down
37 changes: 0 additions & 37 deletions gcloud/datastore/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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?
Expand Down
19 changes: 4 additions & 15 deletions gcloud/datastore/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
33 changes: 0 additions & 33 deletions gcloud/datastore/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
43 changes: 7 additions & 36 deletions gcloud/datastore/test_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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])
25 changes: 7 additions & 18 deletions gcloud/datastore/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()))
Expand Down
8 changes: 4 additions & 4 deletions pylintrc_default
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# PyLint config for 'gcloud' *library* code.
# PyLint config for 'gcloud' *library* code.
#
# NOTES:
#
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions regression/datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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'

Expand Down

0 comments on commit 3c394bb

Please sign in to comment.