Skip to content

Commit

Permalink
Removing Key.get() and Entity.reload().
Browse files Browse the repository at this point in the history
Removed Entity.reload() because it relied on the existence
of Key.get(). Replacing it would have required import
gcloud.datastore.get in entity.py, which would have again
been a cyclic import.

With an eye on removing the Entity class (see googleapis#490),
removing Entity.reload() is not such a big deal.

Fixes googleapis#517.
  • Loading branch information
dhermes committed Jan 8, 2015
1 parent 1d6cfb4 commit 81397ea
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 146 deletions.
4 changes: 2 additions & 2 deletions gcloud/datastore/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ def get_connection():
>>> connection = datastore.get_connection()
>>> key1 = Key('Kind', 1234, dataset_id='dataset1')
>>> key2 = Key('Kind', 1234, dataset_id='dataset2')
>>> entity1 = key1.get(connection=connection)
>>> entity2 = key2.get(connection=connection)
>>> entity1 = datastore.get(key1, connection=connection)
>>> entity2 = datastore.get(key2, connection=connection)
:rtype: :class:`gcloud.datastore.connection.Connection`
:returns: A connection defined with the proper credentials.
Expand Down
5 changes: 2 additions & 3 deletions gcloud/datastore/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,13 @@ def lookup(self, dataset_id, key_pbs,
This method deals only with protobufs
(:class:`gcloud.datastore.datastore_v1_pb2.Key` and
:class:`gcloud.datastore.datastore_v1_pb2.Entity`) and is used
under the hood for methods like
:meth:`gcloud.datastore.key.Key.get`:
under the hood in :func:`gcloud.datastore.get`:
>>> from gcloud import datastore
>>> from gcloud.datastore.key import Key
>>> datastore.set_default_connection()
>>> key = Key('MyKind', 1234, dataset_id='dataset-id')
>>> key.get()
>>> datastore.get(key)
<Entity object>
Using the ``connection`` class directly:
Expand Down
27 changes: 2 additions & 25 deletions gcloud/datastore/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ class Entity(dict):
This means you could take an existing entity and change the key
to duplicate the object.
Use :meth:`gcloud.datastore.key.Key.get` to retrieve an existing entity.
Use :func:`gcloud.datastore.get` to retrieve an existing entity.
>>> key.get()
>>> datastore.get(key)
<Entity[{'kind': 'EntityKind', id: 1234}] {'property': 'value'}>
You can the set values on the entity just like you would on any
Expand Down Expand Up @@ -116,29 +116,6 @@ def _must_key(self):
raise NoKey()
return self.key

def reload(self, connection=None):
"""Reloads the contents of this entity from the datastore.
This method takes the :class:`gcloud.datastore.key.Key`, loads all
properties from the Cloud Datastore, and sets the updated properties on
the current object.
.. warning::
This will override any existing properties if a different value
exists remotely, however it will *not* override any properties that
exist only locally.
:type connection: :class:`gcloud.datastore.connection.Connection`
:param connection: Optional connection used to connect to datastore.
"""
connection = connection or _implicit_environ.CONNECTION

key = self._must_key
entity = key.get(connection=connection)

if entity:
self.update(entity)

def save(self, connection=None):
"""Save the entity in the Cloud Datastore.
Expand Down
23 changes: 0 additions & 23 deletions gcloud/datastore/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,29 +217,6 @@ def to_protobuf(self):

return key

def get(self, connection=None):
"""Retrieve entity corresponding to the current key.
:type connection: :class:`gcloud.datastore.connection.Connection`
:param connection: Optional connection used to connect to datastore.
:rtype: :class:`gcloud.datastore.entity.Entity` or :class:`NoneType`
:returns: The requested entity, or ``None`` if there was no
match found.
"""
# Temporary cylic import, needs to be removed.
from gcloud.datastore import api

# We allow partial keys to attempt a get, the backend will fail.
connection = connection or _implicit_environ.CONNECTION
entities = api.get([self], connection=connection)

if entities:
result = entities[0]
# We assume that the backend has not changed the key.
result.key = self
return result

def delete(self, connection=None):
"""Delete the key in the Cloud Datastore.
Expand Down
30 changes: 0 additions & 30 deletions gcloud/datastore/test_entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,32 +59,6 @@ def test__must_key_no_key(self):
entity = self._makeOne()
self.assertRaises(NoKey, getattr, entity, '_must_key')

def test_reload_no_key(self):
from gcloud.datastore.entity import NoKey

entity = self._makeOne()
entity['foo'] = 'Foo'
self.assertRaises(NoKey, entity.reload)

def test_reload_miss(self):
key = _Key()
key._stored = None # Explicit miss.
entity = self._makeOne(key=key)
entity['foo'] = 'Foo'
# Does not raise, does not update on miss.
entity.reload()
self.assertEqual(entity['foo'], 'Foo')

def test_reload_hit(self):
key = _Key()
NEW_VAL = 'Baz'
key._stored = {'foo': NEW_VAL}
entity = self._makeOne(key=key)
entity['foo'] = 'Foo'
entity.reload()
self.assertEqual(entity['foo'], NEW_VAL)
self.assertEqual(entity.keys(), ['foo'])

def test_save_no_key(self):
from gcloud.datastore.entity import NoKey

Expand Down Expand Up @@ -186,10 +160,6 @@ def is_partial(self):
def path(self):
return self._path

def get(self, connection=None):
self._connection_used = connection
return self._stored


class _Connection(object):
_transaction = _saved = _deleted = None
Expand Down
56 changes: 0 additions & 56 deletions gcloud/datastore/test_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,62 +232,6 @@ def test_to_protobuf_w_no_kind(self):
pb = key.to_protobuf()
self.assertFalse(pb.path_element[0].HasField('kind'))

def test_get_explicit_connection_miss(self):
from gcloud.datastore.test_connection import _Connection

cnxn_lookup_result = []
cnxn = _Connection(*cnxn_lookup_result)
key = self._makeOne('KIND', 1234, dataset_id=self._DEFAULT_DATASET)
entity = key.get(connection=cnxn)
self.assertEqual(entity, None)

def test_get_implicit_connection_miss(self):
from gcloud._testing import _Monkey
from gcloud.datastore import _implicit_environ
from gcloud.datastore.test_connection import _Connection

cnxn_lookup_result = []
cnxn = _Connection(*cnxn_lookup_result)
key = self._makeOne('KIND', 1234, dataset_id=self._DEFAULT_DATASET)
with _Monkey(_implicit_environ, CONNECTION=cnxn):
entity = key.get()
self.assertEqual(entity, None)

def test_get_explicit_connection_hit(self):
from gcloud.datastore import datastore_v1_pb2
from gcloud.datastore.test_connection import _Connection

KIND = 'KIND'
ID = 1234

# Make a bogus entity PB to be returned from fake Connection.
entity_pb = datastore_v1_pb2.Entity()
entity_pb.key.partition_id.dataset_id = self._DEFAULT_DATASET
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'

# Make fake connection.
cnxn_lookup_result = [entity_pb]
cnxn = _Connection(*cnxn_lookup_result)

# Create key and look-up.
key = self._makeOne(KIND, ID, dataset_id=self._DEFAULT_DATASET)
entity = key.get(connection=cnxn)
self.assertEqual(entity.items(), [('foo', 'Foo')])
self.assertTrue(entity.key is key)

def test_get_no_connection(self):
from gcloud.datastore import _implicit_environ

self.assertEqual(_implicit_environ.CONNECTION, None)
key = self._makeOne('KIND', 1234, dataset_id=self._DEFAULT_DATASET)
with self.assertRaises(EnvironmentError):
key.get()

def test_delete_explicit_connection(self):
from gcloud.datastore.test_connection import _Connection

Expand Down
3 changes: 0 additions & 3 deletions pylintrc_default
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,13 @@ ignore =
# identical implementation but different docstrings.
# - star-args: standard Python idioms for varargs:
# ancestor = Query().filter(*order_props)
# - cyclic-import: temporary workaround to support Key.get until Dataset
# is removed in #477.
disable =
maybe-no-member,
no-member,
protected-access,
redefined-builtin,
similarities,
star-args,
cyclic-import,


[REPORTS]
Expand Down
23 changes: 19 additions & 4 deletions regression/datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,12 @@ def _generic_test_post(self, name=None, key_id=None):
self.assertEqual(entity.key.name, name)
if key_id is not None:
self.assertEqual(entity.key.id, key_id)
retrieved_entity = entity.key.get()
retrieved_entity = datastore.get(entity.key)
# Check the keys are the same.
self.assertTrue(retrieved_entity.key is entity.key)
self.assertEqual(retrieved_entity.key.path, entity.key.path)
self.assertEqual(retrieved_entity.key.namespace, entity.key.namespace)
self.assertTrue(_compare_dataset_ids(
retrieved_entity.key.dataset_id, entity.key.dataset_id))

# Check the data is the same.
retrieved_dict = dict(retrieved_entity.items())
Expand Down Expand Up @@ -343,14 +346,26 @@ def test_transaction(self):
entity['url'] = u'www.google.com'

with Transaction():
retrieved_entity = entity.key.get()
retrieved_entity = datastore.get(entity.key)
if retrieved_entity is None:
entity.save()
self.case_entities_to_delete.append(entity)

# This will always return after the transaction.
retrieved_entity = entity.key.get()
retrieved_entity = datastore.get(entity.key)
self.case_entities_to_delete.append(retrieved_entity)
retrieved_dict = dict(retrieved_entity.items())
entity_dict = dict(entity.items())
self.assertEqual(retrieved_dict, entity_dict)


def _compare_dataset_ids(dataset_id1, dataset_id2):
if dataset_id1 == dataset_id2:
return True

if dataset_id1.startswith('s~') or dataset_id1.startswith('e~'):
dataset_id1 = dataset_id1[2:]
if dataset_id2.startswith('s~') or dataset_id2.startswith('e~'):
dataset_id2 = dataset_id2[2:]

return dataset_id1 == dataset_id2

0 comments on commit 81397ea

Please sign in to comment.