Merge pull request #481 from dhermes/fix-477-part4

Address fourth part of 477: Moving Entity.delete to Key (for datastore).

Author: dhermes

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)

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 '<Entity%s %s>' % (self._key.path,

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).

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):
 

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), '<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 = ()

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')

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()