Merge pull request #289 from tseaver/260-break_datastore_cycles_move_…

…from_protobuf

Fix #260: break datastore cycles by moving 'from_protobuf' classmethods

docs/datastore-api.rst

@@ -56,3 +56,9 @@ Transactions
  :members:
  :undoc-members:
  :show-inheritance:
+
+Helper functions
+----------------
+
+.. automodule:: gcloud.datastore.helpers
+ :members:

gcloud/datastore/connection.py

@@ -2,7 +2,7 @@
 
 from gcloud import connection
 from gcloud.datastore import datastore_v1_pb2 as datastore_pb
-from gcloud.datastore import _helpers
+from gcloud.datastore import helpers
 from gcloud.datastore.dataset import Dataset
 
 
@@ -376,7 +376,7 @@ def save_entity(self, dataset_id, key_pb, properties):
  prop.name = name
 
  # Set the appropriate value.
- _helpers._set_protobuf_value(prop.value, value)
+ helpers._set_protobuf_value(prop.value, value)
 
  # If this is in a transaction, we should just return True. The
  # transaction will handle assigning any keys as necessary.

gcloud/datastore/dataset.py

@@ -1,5 +1,10 @@
 """Create / interact with gcloud datastore datasets."""
 
+from gcloud.datastore import helpers
+from gcloud.datastore.entity import Entity
+from gcloud.datastore.query import Query
+from gcloud.datastore.transaction import Transaction
+
 
 class Dataset(object):
  """A dataset in the Cloud Datastore.
@@ -70,8 +75,6 @@ def query(self, *args, **kwargs):
  :rtype: :class:`gcloud.datastore.query.Query`
  :returns: a new Query instance, bound to this dataset.
  """
- # This import is here to avoid circular references.
- from gcloud.datastore.query import Query
  kwargs['dataset'] = self
  return Query(*args, **kwargs)
 
@@ -84,8 +87,6 @@ def entity(self, kind):
  :rtype: :class:`gcloud.datastore.entity.Entity`
  :returns: a new Entity instance, bound to this dataset.
  """
- # This import is here to avoid circular references.
- from gcloud.datastore.entity import Entity
  return Entity(dataset=self, kind=kind)
 
  def transaction(self, *args, **kwargs):
@@ -98,8 +99,6 @@ def transaction(self, *args, **kwargs):
  :rtype: :class:`gcloud.datastore.transaction.Transaction`
  :returns: a new Transaction instance, bound to this dataset.
  """
- # This import is here to avoid circular references.
- from gcloud.datastore.transaction import Transaction
  kwargs['dataset'] = self
  return Transaction(*args, **kwargs)
 
@@ -125,15 +124,13 @@ def get_entities(self, keys):
  :rtype: list of :class:`gcloud.datastore.entity.Entity`
  :return: The requested entities.
  """
- # This import is here to avoid circular references.
- from gcloud.datastore.entity import Entity
-
  entity_pbs = self.connection().lookup(
  dataset_id=self.id(),
  key_pbs=[k.to_protobuf() for k in keys]
  )
 
  entities = []
  for entity_pb in entity_pbs:
- entities.append(Entity.from_protobuf(entity_pb, dataset=self))
+ entities.append(helpers.entity_from_protobuf(
+ entity_pb, dataset=self))
  return entities

gcloud/datastore/entity.py

@@ -146,32 +146,6 @@ def from_key(cls, key, dataset=None):
 
  return cls(dataset).key(key)
 
- @classmethod
- def from_protobuf(cls, pb, dataset=None):
- """Factory method for creating an entity based on a protobuf.
-
- The protobuf should be one returned from the Cloud Datastore
- Protobuf API.
-
- :type pb: :class:`gcloud.datastore.datastore_v1_pb2.Entity`
- :param pb: The Protobuf representing the entity.
-
- :returns: The :class:`Entity` derived from the
- :class:`gcloud.datastore.datastore_v1_pb2.Entity`.
- """
-
- # This is here to avoid circular imports.
- from gcloud.datastore import _helpers
-
- key = Key.from_protobuf(pb.key)
- entity = cls.from_key(key, dataset)
-
- for property_pb in pb.property:
- value = _helpers._get_value_from_property_pb(property_pb)
- entity[property_pb.name] = value
-
- return entity
-
  @property
  def _must_key(self):
  """Return our key, or raise NoKey if not set.
@@ -248,9 +222,11 @@ def save(self):
  transaction.add_auto_id_entity(self)
 
  if isinstance(key_pb, datastore_pb.Key):
- updated_key = Key.from_protobuf(key_pb)
+ path = [
+ {'kind': element.kind, 'id': element.id, 'name': element.name}
+ for element in key_pb.path_element]
  # Update the path (which may have been altered).
- self._key = key.path(updated_key.path())
+ self._key = key.path(path)
 
  return self
 

gcloud/datastore/_helpers.py → gcloud/datastore/helpers.py

@@ -1,7 +1,9 @@
 """Helper functions for dealing with Cloud Datastore's Protobuf API.
 
-These functions are *not* part of the API.
+The non-private functions are part of the API.
 """
+__all__ = ('entity_from_protobuf', 'key_from_protobuf')
+
 import calendar
 import datetime
 
@@ -14,6 +16,60 @@
 INT_VALUE_CHECKER = Int64ValueChecker()
 
 
+def entity_from_protobuf(pb, dataset=None):
+ """Factory method for creating an entity based on a protobuf.
+
+ The protobuf should be one returned from the Cloud Datastore
+ Protobuf API.
+
+ :type pb: :class:`gcloud.datastore.datastore_v1_pb2.Entity`
+ :param pb: The Protobuf representing the entity.
+
+ :rtype: :class:`gcloud.datastore.entity.Entity`
+ :returns: The entity derived from the protobuf.
+ """
+ key = key_from_protobuf(pb.key)
+ entity = Entity.from_key(key, dataset)
+
+ for property_pb in pb.property:
+ value = _get_value_from_property_pb(property_pb)
+ entity[property_pb.name] = value
+
+ return entity
+
+
+def key_from_protobuf(pb):
+ """Factory method for creating a key based on a protobuf.
+
+ The protobuf should be one returned from the Cloud Datastore
+ Protobuf API.
+
+ :type pb: :class:`gcloud.datastore.datastore_v1_pb2.Key`
+ :param pb: The Protobuf representing the key.
+
+ :rtype: :class:`gcloud.datastore.key.Key`
+ :returns: a new `Key` instance
+ """
+ path = []
+ for element in pb.path_element:
+ element_dict = {'kind': element.kind}
+
+ if element.HasField('id'):
+ element_dict['id'] = element.id
+
+ # This is safe: we expect proto objects returned will only have
+ # one of `name` or `id` set.
+ if element.HasField('name'):
+ element_dict['name'] = element.name
+
+ path.append(element_dict)
+
+ dataset_id = pb.partition_id.dataset_id or None
+ namespace = pb.partition_id.namespace
+
+ return Key(path, namespace, dataset_id)
+
+
 def _get_protobuf_attribute_and_value(val):
  """Given a value, return the protobuf attribute name and proper value.
 
@@ -105,7 +161,7 @@ def _get_value_from_value_pb(value_pb):
  result = naive.replace(tzinfo=pytz.utc)
 
  elif value_pb.HasField('key_value'):
- result = Key.from_protobuf(value_pb.key_value)
+ result = key_from_protobuf(value_pb.key_value)
 
  elif value_pb.HasField('boolean_value'):
  result = value_pb.boolean_value
@@ -123,7 +179,7 @@ def _get_value_from_value_pb(value_pb):
  result = value_pb.blob_value
 
  elif value_pb.HasField('entity_value'):
- result = Entity.from_protobuf(value_pb.entity_value)
+ result = entity_from_protobuf(value_pb.entity_value)
 
  elif value_pb.list_value:
  result = [_get_value_from_value_pb(x) for x in value_pb.list_value]

gcloud/datastore/key.py

@@ -42,38 +42,6 @@ def _clone(self):
  """
  return copy.deepcopy(self)
 
- @classmethod
- def from_protobuf(cls, pb):
- """Factory method for creating a key based on a protobuf.
-
- The protobuf should be one returned from the Cloud Datastore
- Protobuf API.
-
- :type pb: :class:`gcloud.datastore.datastore_v1_pb2.Key`
- :param pb: The Protobuf representing the key.
-
- :rtype: :class:`gcloud.datastore.key.Key`
- :returns: a new `Key` instance
- """
- path = []
- for element in pb.path_element:
- element_dict = {'kind': element.kind}
-
- if element.HasField('id'):
- element_dict['id'] = element.id
-
- # This is safe: we expect proto objects returned will only have
- # one of `name` or `id` set.
- if element.HasField('name'):
- element_dict['name'] = element.name
-
- path.append(element_dict)
-
- dataset_id = pb.partition_id.dataset_id or None
- namespace = pb.partition_id.namespace
-
- return cls(path, namespace, dataset_id)
-
  def to_protobuf(self):
  """Return a protobuf corresponding to the key.
 

gcloud/datastore/query.py

@@ -3,8 +3,7 @@
 import base64
 
 from gcloud.datastore import datastore_v1_pb2 as datastore_pb
-from gcloud.datastore import _helpers
-from gcloud.datastore.entity import Entity
+from gcloud.datastore import helpers
 from gcloud.datastore.key import Key
 
 
@@ -151,7 +150,7 @@ def filter(self, expression, value):
  property_filter.operator = operator
 
  # Set the value to filter on based on the type.
- _helpers._set_protobuf_value(property_filter.value, value)
+ helpers._set_protobuf_value(property_filter.value, value)
  return clone
 
  def ancestor(self, ancestor):
@@ -343,7 +342,7 @@ def fetch(self, limit=None):
  entity_pbs, end_cursor = query_results[:2]
 
  self._cursor = end_cursor
- return [Entity.from_protobuf(entity, dataset=self.dataset())
+ return [helpers.entity_from_protobuf(entity, dataset=self.dataset())
  for entity in entity_pbs]
 
  def cursor(self):

gcloud/datastore/test_entity.py

@@ -76,48 +76,6 @@ def test_from_key_w_dataset(self):
  self.assertEqual(key.kind(), _KIND)
  self.assertEqual(key.id(), _ID)
 
- def test_from_protobuf_wo_dataset(self):
- from gcloud.datastore import datastore_v1_pb2 as datastore_pb
-
- entity_pb = datastore_pb.Entity()
- entity_pb.key.partition_id.dataset_id = _DATASET_ID
- entity_pb.key.path_element.add(kind=_KIND, id=_ID)
- entity_pb.key.partition_id.dataset_id = _DATASET_ID
- prop_pb = entity_pb.property.add()
- prop_pb.name = 'foo'
- prop_pb.value.string_value = 'Foo'
- klass = self._getTargetClass()
- entity = klass.from_protobuf(entity_pb)
- self.assertTrue(entity.dataset() is None)
- self.assertEqual(entity.kind(), _KIND)
- self.assertEqual(entity['foo'], 'Foo')
- key = entity.key()
- self.assertEqual(key._dataset_id, _DATASET_ID)
- self.assertEqual(key.kind(), _KIND)
- self.assertEqual(key.id(), _ID)
-
- def test_from_protobuf_w_dataset(self):
- from gcloud.datastore import datastore_v1_pb2 as datastore_pb
- from gcloud.datastore.dataset import Dataset
-
- entity_pb = datastore_pb.Entity()
- entity_pb.key.partition_id.dataset_id = _DATASET_ID
- entity_pb.key.path_element.add(kind=_KIND, id=_ID)
- entity_pb.key.partition_id.dataset_id = _DATASET_ID
- prop_pb = entity_pb.property.add()
- prop_pb.name = 'foo'
- prop_pb.value.string_value = 'Foo'
- dataset = Dataset(_DATASET_ID)
- klass = self._getTargetClass()
- entity = klass.from_protobuf(entity_pb, dataset)
- self.assertTrue(entity.dataset() is dataset)
- self.assertEqual(entity.kind(), _KIND)
- self.assertEqual(entity['foo'], 'Foo')
- key = entity.key()
- self.assertEqual(key._dataset_id, _DATASET_ID)
- self.assertEqual(key.kind(), _KIND)
- self.assertEqual(key.id(), _ID)
-
  def test__must_key_no_key(self):
  from gcloud.datastore.entity import NoKey
 
@@ -224,7 +182,7 @@ def test_save_w_returned_key(self):
  self.assertEqual(entity['foo'], 'Foo')
  self.assertEqual(connection._saved,
  (_DATASET_ID, 'KEY', {'foo': 'Foo'}))
- self.assertEqual(key._path, [{'kind': _KIND, 'id': _ID}])
+ self.assertEqual(key._path, [{'kind': _KIND, 'id': _ID, 'name': ''}])
 
  def test_delete_no_key(self):
  from gcloud.datastore.entity import NoKey