Fix #169: remove Connection.delete_entity API

gcloud/datastore/connection.py

@@ -411,25 +411,3 @@ def delete_entities(self, dataset_id, key_pbs):
             return True
         else:
             return self.commit(dataset_id, mutation)
-
-    def delete_entity(self, dataset_id, key_pb):
-        """Delete a single key from a dataset in the Cloud Datastore.
-
-        This method deals only with
-        :class:`gcloud.datastore.datastore_v1_pb2.Key` protobufs
-        and not with any of the other abstractions.
-        For example, it's used under the hood in the
-        :func:`gcloud.datastore.entity.Entity.delete` method.
-
-        :type dataset_id: string
-        :param dataset_id: The dataset from which to delete the key.
-
-        :type key_pb: :class:`gcloud.datastore.datastore_v1_pb2.Key`
-        :param key_pb: The key to delete from the datastore.
-
-        :rtype: boolean (if in a transaction) or else
-                :class:`gcloud.datastore.datastore_v1_pb2.MutationResult`.
-        :returns: True (if in a transaction) or else a mutation result
-                  protobuf.
-        """
-        return self.delete_entities(dataset_id, [key_pb])

gcloud/datastore/entity.py

@@ -19,6 +19,10 @@
 from gcloud.datastore.key import Key
 
 
+class NoKey(RuntimeError):
+    """Exception raised by Entity methods which require a key."""
+
+
 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.
@@ -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.
@@ -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):
@@ -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.
 
@@ -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)
@@ -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
 
@@ -222,20 +232,16 @@ 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_entities(
+            dataset_id=dataset.id(),
+            key_pbs=[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__())

gcloud/datastore/test_connection.py

@@ -790,71 +790,6 @@ def mutation(self):
         mutation = conn.mutation()
         self.assertEqual(len(mutation.delete), 1)
 
-    def test_delete_entity_wo_transaction(self):
-        from gcloud.datastore.connection import datastore_pb
-        from gcloud.datastore.dataset import Dataset
-        from gcloud.datastore.key import Key
-
-        DATASET_ID = 'DATASET'
-        key_pb = Key(dataset=Dataset(DATASET_ID),
-                     path=[{'kind': 'Kind', 'id': 1234}]).to_protobuf()
-        rsp_pb = datastore_pb.CommitResponse()
-        conn = self._makeOne()
-        URI = '/'.join([conn.API_BASE_URL,
-                        'datastore',
-                        conn.API_VERSION,
-                        'datasets',
-                        DATASET_ID,
-                        'commit',
-                        ])
-        http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
-        result = conn.delete_entity(DATASET_ID, key_pb)
-        self.assertEqual(result.index_updates, 0)
-        self.assertEqual(list(result.insert_auto_id_key), [])
-        cw = http._called_with
-        self.assertEqual(cw['uri'], URI)
-        self.assertEqual(cw['method'], 'POST')
-        self.assertEqual(cw['headers'],
-                         {'Content-Type': 'application/x-protobuf',
-                          'Content-Length': '30',
-                          'User-Agent': conn.USER_AGENT,
-                          })
-        rq_class = datastore_pb.CommitRequest
-        request = rq_class()
-        request.ParseFromString(cw['body'])
-        self.assertEqual(request.transaction, '')
-        mutation = request.mutation
-        self.assertEqual(len(mutation.insert_auto_id), 0)
-        self.assertEqual(len(mutation.upsert), 0)
-        deletes = list(mutation.delete)
-        self.assertEqual(len(deletes), 1)
-        delete = deletes[0]
-        self.assertEqual(delete, key_pb)
-        self.assertEqual(request.mode, rq_class.NON_TRANSACTIONAL)
-
-    def test_delete_entity_w_transaction(self):
-        from gcloud.datastore.connection import datastore_pb
-        from gcloud.datastore.dataset import Dataset
-        from gcloud.datastore.key import Key
-
-        mutation = datastore_pb.Mutation()
-
-        class Xact(object):
-            def mutation(self):
-                return mutation
-        DATASET_ID = 'DATASET'
-        key_pb = Key(dataset=Dataset(DATASET_ID),
-                     path=[{'kind': 'Kind', 'id': 1234}]).to_protobuf()
-        rsp_pb = datastore_pb.CommitResponse()
-        conn = self._makeOne()
-        conn.transaction(Xact())
-        http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
-        result = conn.delete_entity(DATASET_ID, key_pb)
-        self.assertEqual(result, True)
-        self.assertEqual(http._called_with, None)
-        mutation = conn.mutation()
-        self.assertEqual(len(mutation.delete), 1)
-
 
 class Http(object):
 

gcloud/datastore/test_entity.py

@@ -85,6 +85,13 @@ 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
+
+        entity = self._makeOne(None, None)
+        entity['foo'] = 'Foo'
+        self.assertRaises(NoKey, entity.reload)
+
     def test_reload_miss(self):
         dataset = _Dataset()
         key = _Key(dataset)
@@ -105,6 +112,13 @@ 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
+
+        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)
@@ -167,6 +181,13 @@ 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
+
+        entity = self._makeOne(None, None)
+        entity['foo'] = 'Foo'
+        self.assertRaises(NoKey, entity.delete)
+
     def test_delete(self):
         connection = _Connection()
         dataset = _Dataset(connection)
@@ -175,10 +196,25 @@ def test_delete(self):
         entity.key(key)
         entity['foo'] = 'Foo'
         self.assertTrue(entity.delete() is None)
-        self.assertEqual(connection._deleted, (_DATASET_ID, 'KEY'))
+        self.assertEqual(connection._deleted, (_DATASET_ID, ['KEY']))
+
+    def test___repr___no_key_empty(self):
+        entity = self._makeOne(None, None)
+        self.assertEqual(repr(entity), '<Entity {}>')
+
+    def test___repr___w_key_non_empty(self):
+        connection = _Connection()
+        dataset = _Dataset(connection)
+        key = _Key(dataset)
+        key.path('/bar/baz')
+        entity = self._makeOne()
+        entity.key(key)
+        entity['foo'] = 'Foo'
+        self.assertEqual(repr(entity), "<Entity/bar/baz {'foo': 'Foo'}>")
 
 
 class _Key(object):
+    _MARKER = object()
     _key = 'KEY'
     _partial = False
     _path = None
@@ -195,7 +231,9 @@ def to_protobuf(self):
     def is_partial(self):
         return self._partial
 
-    def path(self, path):
+    def path(self, path=_MARKER):
+        if path is self._MARKER:
+            return self._path
         self._path = path
 
 
@@ -224,8 +262,8 @@ def save_entity(self, dataset_id, key_pb, properties):
         self._saved = (dataset_id, key_pb, properties)
         return self._save_result
 
-    def delete_entity(self, dataset_id, key_pb):
-        self._deleted = (dataset_id, key_pb)
+    def delete_entities(self, dataset_id, key_pbs):
+        self._deleted = (dataset_id, key_pbs)
 
 
 class _Transaction(object):