Skip to content

Commit

Permalink
Avoid interior use of 'key()' setter-getter.
Browse files Browse the repository at this point in the history
Instead, use '_key' directly, adding a '_must_key' property which
raises a NoKey exception if None (for use in cases which require a key).

Allows removal of the pylint 'maybe-no-member' comments.
  • Loading branch information
tseaver committed Oct 17, 2014
1 parent 906700b commit 7c55a04
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 33 deletions.
71 changes: 38 additions & 33 deletions gcloud/datastore/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
from gcloud.datastore.key import Key


class NoKey(RuntimeError):
pass


class Entity(dict): # pylint: disable=too-many-public-methods
""":type dataset: :class:`gcloud.datastore.dataset.Dataset`
:param dataset: The dataset in which this entity belongs.
Expand Down Expand Up @@ -82,8 +86,8 @@ def dataset(self):
be `None`. It also means that if you change the key on the entity,
this will refer to that key's dataset.
"""
if self.key():
return self.key().dataset()
if self._key:
return self._key.dataset()

def key(self, key=None):
"""Get or set the :class:`.datastore.key.Key` on the current entity.
Expand Down Expand Up @@ -117,8 +121,8 @@ def kind(self):
which knows its Kind.
"""

if self.key():
return self.key().kind()
if self._key:
return self._key.kind()

@classmethod
def from_key(cls, key):
Expand Down Expand Up @@ -162,6 +166,18 @@ def from_protobuf(cls, pb, dataset=None): # pylint: disable=invalid-name

return entity

@property
def _must_key(self):
"""Return our key.
:rtype: :class:`gcloud.datastore.key.Key`.
:returns: our key
:raises: NoKey if key is None
"""
if self._key is None:
raise NoKey('no key')
return self._key

def reload(self):
"""Reloads the contents of this entity from the datastore.
Expand All @@ -174,11 +190,8 @@ def reload(self):
exists remotely, however it will *not* override any properties that
exist only locally.
"""

# Note that you must have a valid key, otherwise this makes no sense.
# pylint: disable=maybe-no-member
entity = self.dataset().get_entity(self.key().to_protobuf())
# pylint: enable=maybe-no-member
key = self._must_key
entity = key.dataset().get_entity(key.to_protobuf())

if entity:
self.update(entity)
Expand All @@ -190,27 +203,24 @@ def save(self):
:rtype: :class:`gcloud.datastore.entity.Entity`
:returns: The entity with a possibly updated Key.
"""
# pylint: disable=maybe-no-member
key_pb = self.dataset().connection().save_entity(
dataset_id=self.dataset().id(),
key_pb=self.key().to_protobuf(), properties=dict(self))
# pylint: enable=maybe-no-member
key = self._must_key
dataset = key.dataset()
connection = dataset.connection()
key_pb = connection.save_entity(
dataset_id=dataset.id(),
key_pb=key.to_protobuf(),
properties=dict(self))

# If we are in a transaction and the current entity needs an
# automatically assigned ID, tell the transaction where to put that.
transaction = self.dataset().connection().transaction()
# pylint: disable=maybe-no-member
if transaction and self.key().is_partial():
transaction = connection.transaction()
if transaction and key.is_partial():
transaction.add_auto_id_entity(self)
# pylint: enable=maybe-no-member

if isinstance(key_pb, datastore_pb.Key):
updated_key = Key.from_protobuf(key_pb)
# Update the path (which may have been altered).
# pylint: disable=maybe-no-member
key = self.key().path(updated_key.path())
# pylint: enable=maybe-no-member
self.key(key)
self._key = key.path(updated_key.path())

return self

Expand All @@ -222,20 +232,15 @@ def delete(self):
set on the entity. Whatever is stored remotely using the key on the
entity will be deleted.
"""
# NOTE: pylint thinks key() is an Entity, hence key().to_protobuf()
# is not defined. This is because one branch of the return
# in the key() definition returns self.
# pylint: disable=maybe-no-member
self.dataset().connection().delete_entity(
dataset_id=self.dataset().id(), key_pb=self.key().to_protobuf())
# pylint: enable=maybe-no-member
key = self._must_key
dataset = key.dataset()
dataset.connection().delete_entity(
dataset_id=dataset.id(), key_pb=key.to_protobuf())

def __repr__(self):
# An entity should have a key all the time (even if it's partial).
if self.key():
# pylint: disable=maybe-no-member
return '<Entity%s %s>' % (self.key().path(),
if self._key:
return '<Entity%s %s>' % (self._key.path(),
super(Entity, self).__repr__())
# pylint: enable=maybe-no-member
else:
return '<Entity %s>' % (super(Entity, self).__repr__())
24 changes: 24 additions & 0 deletions gcloud/datastore/test_entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ def test_from_protobuf(self):
self.assertEqual(key.kind(), _KIND)
self.assertEqual(key.id(), _ID)

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

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

def test_reload_miss(self):
dataset = _Dataset()
key = _Key(dataset)
Expand All @@ -105,6 +113,14 @@ def test_reload_hit(self):
self.assertTrue(entity.reload() is entity)
self.assertEqual(entity['foo'], 'Bar')

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

dataset = _Dataset()
entity = self._makeOne(None, None)
entity['foo'] = 'Foo'
self.assertRaises(NoKey, entity.save)

def test_save_wo_transaction_wo_auto_id_wo_returned_key(self):
connection = _Connection()
dataset = _Dataset(connection)
Expand Down Expand Up @@ -167,6 +183,14 @@ def test_save_w_returned_key(self):
(_DATASET_ID, 'KEY', {'foo': 'Foo'}))
self.assertEqual(key._path, [{'kind': _KIND, 'id': _ID}])

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

dataset = _Dataset()
entity = self._makeOne(None, None)
entity['foo'] = 'Foo'
self.assertRaises(NoKey, entity.delete)

def test_delete(self):
connection = _Connection()
dataset = _Dataset(connection)
Expand Down

0 comments on commit 7c55a04

Please sign in to comment.