Merge pull request #522 from dhermes/remove-key-get

Remove Key.get() and Entity.reload()

Author: dhermes

gcloud/datastore/__init__.py

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

gcloud/datastore/connection.py

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

gcloud/datastore/entity.py

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

gcloud/datastore/key.py

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

gcloud/datastore/test_entity.py

@@ -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
 
@@ -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

gcloud/datastore/test_key.py

@@ -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
 

pylintrc_default

@@ -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]

regression/datastore.py

@@ -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())
@@ -343,14 +346,30 @@ 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~'):
+        # If `dataset_id1` is prefixed and not matching, then the only way
+        # they can match is if `dataset_id2` is unprefixed.
+        return dataset_id1[2:] == dataset_id2
+    elif dataset_id2.startswith('s~') or dataset_id2.startswith('e~'):
+        # Here we know `dataset_id1` is unprefixed and `dataset_id2`
+        # is prefixed.
+        return dataset_id1 == dataset_id2[2:]
+
+    return False