From a7415162d9d3bbdd60dec63df5b584d390947257 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Fri, 2 Jan 2015 13:48:16 -0800 Subject: [PATCH] Moving Entity.delete to Key (for datastore). Addresses fourth part of #477. --- gcloud/datastore/connection.py | 5 ++-- gcloud/datastore/entity.py | 15 ----------- gcloud/datastore/key.py | 20 +++++++++++---- gcloud/datastore/test_dataset.py | 5 ++++ gcloud/datastore/test_entity.py | 20 --------------- gcloud/datastore/test_key.py | 43 +++++++++++++++++++++++++++----- regression/datastore.py | 4 +-- 7 files changed, 61 insertions(+), 51 deletions(-) diff --git a/gcloud/datastore/connection.py b/gcloud/datastore/connection.py index c71b23a33592..13e676d7481f 100644 --- a/gcloud/datastore/connection.py +++ b/gcloud/datastore/connection.py @@ -509,9 +509,8 @@ def delete_entities(self, dataset_id, key_pbs): :type key_pbs: list of :class:`gcloud.datastore.datastore_v1_pb2.Key` :param key_pbs: The keys to delete from the datastore. - :rtype: boolean (if in a transaction) or else - :class:`gcloud.datastore.datastore_v1_pb2.MutationResult`. - :returns: True + :rtype: boolean + :returns: `True` """ mutation = self.mutation() helpers._add_keys_to_request(mutation.delete, key_pbs) diff --git a/gcloud/datastore/entity.py b/gcloud/datastore/entity.py index 562efb87a6f3..fce6f3fe99cb 100644 --- a/gcloud/datastore/entity.py +++ b/gcloud/datastore/entity.py @@ -258,21 +258,6 @@ def save(self): return self - def delete(self): - """Delete the entity in the Cloud Datastore. - - .. note:: - This is based entirely off of the :class:`gcloud.datastore.key.Key` - set on the entity. Whatever is stored remotely using the key on the - entity will be deleted. - """ - key = self._must_key - dataset = self._must_dataset - dataset.connection().delete_entities( - dataset_id=dataset.id(), - key_pbs=[key.to_protobuf()], - ) - def __repr__(self): if self._key: return '' % (self._key.path, diff --git a/gcloud/datastore/key.py b/gcloud/datastore/key.py index b52a1f23e548..b573a64ff6fa 100644 --- a/gcloud/datastore/key.py +++ b/gcloud/datastore/key.py @@ -231,19 +231,16 @@ def get(self, connection=None): """Retrieve entity corresponding to the curretn key. :type connection: :class:`gcloud.datastore.connection.Connection` - :param connection: Optional connection used to connection to datastore. + :param connection: Optional connection used to connect to datastore. :rtype: :class:`gcloud.datastore.entity.Entity` or `NoneType` :returns: The requested entity, or ``None`` if there was no match found. - :raises: `ValueError` if the current key is partial. """ # Temporary import hack until Dataset is removed in #477. from gcloud.datastore.dataset import Dataset - if self.is_partial: - raise ValueError('Can only retrieve complete keys.') - + # We allow partial keys to attempt a get, the backend will fail. connection = connection or _implicit_environ.CONNECTION dataset = Dataset(self.dataset_id, connection=connection) entities = dataset.get_entities([self]) @@ -254,6 +251,19 @@ def get(self, connection=None): result.key(self) return result + def delete(self, connection=None): + """Delete the key in the Cloud Datastore. + + :type connection: :class:`gcloud.datastore.connection.Connection` + :param connection: Optional connection used to connect to datastore. + """ + # We allow partial keys to attempt a delete, the backend will fail. + connection = connection or _implicit_environ.CONNECTION + connection.delete_entities( + dataset_id=self.dataset_id, + key_pbs=[self.to_protobuf()], + ) + @property def is_partial(self): """Boolean indicating if the key has an ID (or name). diff --git a/gcloud/datastore/test_dataset.py b/gcloud/datastore/test_dataset.py index f21a1315b9ee..5924ef444515 100644 --- a/gcloud/datastore/test_dataset.py +++ b/gcloud/datastore/test_dataset.py @@ -157,6 +157,11 @@ def allocate_ids(self, dataset_id, key_pbs): num_pbs = len(key_pbs) return [_KeyProto(i) for i in range(num_pbs)] + def delete_entities(self, dataset_id, key_pbs): + self._called_dataset_id = dataset_id + self._called_key_pbs = key_pbs + return True + class _PathElementProto(object): diff --git a/gcloud/datastore/test_entity.py b/gcloud/datastore/test_entity.py index dcb3a38900ef..98bcba287083 100644 --- a/gcloud/datastore/test_entity.py +++ b/gcloud/datastore/test_entity.py @@ -213,23 +213,6 @@ def test_save_w_returned_key_exclude_from_indexes(self): self.assertEqual(entity.key()._path, [{'kind': _KIND, 'id': _ID}]) - def test_delete_no_key(self): - from gcloud.datastore.entity import NoKey - - entity = self._makeOne(None, None) - entity['foo'] = 'Foo' - self.assertRaises(NoKey, entity.delete) - - def test_delete(self): - connection = _Connection() - dataset = _Dataset(connection) - key = _Key() - entity = self._makeOne(dataset) - entity.key(key) - entity['foo'] = 'Foo' - self.assertTrue(entity.delete() is None) - self.assertEqual(connection._deleted, (_DATASET_ID, ['KEY'])) - def test___repr___no_key_empty(self): entity = self._makeOne(None, None) self.assertEqual(repr(entity), '') @@ -308,9 +291,6 @@ def save_entity(self, dataset_id, key_pb, properties, tuple(exclude_from_indexes)) return self._save_result - def delete_entities(self, dataset_id, key_pbs): - self._deleted = (dataset_id, key_pbs) - class _Transaction(object): _added = () diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py index 807d959c887b..22603def4e01 100644 --- a/gcloud/datastore/test_key.py +++ b/gcloud/datastore/test_key.py @@ -243,14 +243,45 @@ def test_get_explicit_connection_hit(self): self.assertEqual(entity.items(), [('foo', 'Foo')]) self.assertTrue(entity.key() is key) - def test_get_explicit_connection_partial_key(self): + def test_get_no_connection(self): + from gcloud.datastore import _implicit_environ + + self.assertEqual(_implicit_environ.CONNECTION, None) + key = self._makeOne('KIND', 1234) + with self.assertRaises(AttributeError): + key.get() + + def test_delete_explicit_connection(self): from gcloud.datastore.test_dataset import _Connection - cnxn_lookup_result = [] - cnxn = _Connection(*cnxn_lookup_result) - key = self._makeOne('KIND') - with self.assertRaises(ValueError): - key.get(connection=cnxn) + cnxn = _Connection() + key = self._makeOne('KIND', 1234) + result = key.delete(connection=cnxn) + self.assertEqual(result, None) + self.assertEqual(cnxn._called_dataset_id, self._DEFAULT_DATASET) + self.assertEqual(cnxn._called_key_pbs, [key.to_protobuf()]) + + def test_delete_implicit_connection(self): + from gcloud._testing import _Monkey + from gcloud.datastore import _implicit_environ + from gcloud.datastore.test_dataset import _Connection + + cnxn = _Connection() + key = self._makeOne('KIND', 1234) + with _Monkey(_implicit_environ, CONNECTION=cnxn): + result = key.delete() + + self.assertEqual(result, None) + self.assertEqual(cnxn._called_dataset_id, self._DEFAULT_DATASET) + self.assertEqual(cnxn._called_key_pbs, [key.to_protobuf()]) + + def test_delete_no_connection(self): + from gcloud.datastore import _implicit_environ + + self.assertEqual(_implicit_environ.CONNECTION, None) + key = self._makeOne('KIND', 1234) + with self.assertRaises(AttributeError): + key.delete() def test_is_partial_no_name_or_id(self): key = self._makeOne('KIND') diff --git a/regression/datastore.py b/regression/datastore.py index 4776bae279ae..eadf4224643a 100644 --- a/regression/datastore.py +++ b/regression/datastore.py @@ -40,7 +40,7 @@ def setUp(self): def tearDown(self): with Transaction(): for entity in self.case_entities_to_delete: - entity.delete() + entity.key().delete() class TestDatastoreAllocateIDs(TestDatastore): @@ -358,7 +358,7 @@ def test_transaction(self): # This will always return after the transaction. retrieved_entity = key.get() + 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) - retrieved_entity.delete()