Changing Transaction constructor to take dataset ID and connection.

Also - Fixed some unfound bugs due to disagreeing code and mocks. - Fixed test pollution of _implicit_environ namespace.

Author: dhermes

gcloud/datastore/connection.py

@@ -149,7 +149,7 @@ def mutation(self):
  (if one exists) or or a new mutation instance.
  """
  if self.transaction():
- return self.transaction().mutation()
+ return self.transaction().mutation
  else:
  return datastore_pb.Mutation()
 

gcloud/datastore/test___init__.py

@@ -40,11 +40,14 @@ class Test_set_default_dataset(unittest2.TestCase):
  def setUp(self):
  from gcloud.datastore import _implicit_environ
  self._replaced_dataset = _implicit_environ.DATASET
+ self._replaced_dataset_id = _implicit_environ.DATASET_ID
  _implicit_environ.DATASET = None
+ _implicit_environ.DATASET_ID = None
 
  def tearDown(self):
  from gcloud.datastore import _implicit_environ
  _implicit_environ.DATASET = self._replaced_dataset
+ _implicit_environ.DATASET_ID = self._replaced_dataset_id
 
  def _callFUT(self, dataset_id=None):
  from gcloud.datastore import set_default_dataset

gcloud/datastore/test_connection.py

@@ -203,8 +203,8 @@ class Mutation(object):
  pass
 
  class Xact(object):
- def mutation(self):
- return Mutation()
+ mutation = Mutation()
+
  conn = self._makeOne()
  conn.transaction(Xact())
  found = conn.mutation()
@@ -1036,11 +1036,9 @@ def test_save_entity_wo_transaction_w_auto_id(self):
  def test_save_entity_w_transaction(self):
  from gcloud.datastore import datastore_v1_pb2 as datastore_pb
 
- mutation = datastore_pb.Mutation()
-
  class Xact(object):
- def mutation(self):
- return mutation
+ mutation = datastore_pb.Mutation()
+
  DATASET_ID = 'DATASET'
  key_pb = self._make_key_pb(DATASET_ID)
  rsp_pb = datastore_pb.CommitResponse()
@@ -1057,11 +1055,9 @@ def test_save_entity_w_transaction_nested_entity(self):
  from gcloud.datastore import datastore_v1_pb2 as datastore_pb
  from gcloud.datastore.entity import Entity
 
- mutation = datastore_pb.Mutation()
-
  class Xact(object):
- def mutation(self):
- return mutation
+ mutation = datastore_pb.Mutation()
+
  DATASET_ID = 'DATASET'
  nested = Entity()
  nested['bar'] = u'Bar'
@@ -1112,11 +1108,9 @@ def test_delete_entities_wo_transaction(self):
  def test_delete_entities_w_transaction(self):
  from gcloud.datastore import datastore_v1_pb2 as datastore_pb
 
- mutation = datastore_pb.Mutation()
-
  class Xact(object):
- def mutation(self):
- return mutation
+ mutation = datastore_pb.Mutation()
+
  DATASET_ID = 'DATASET'
  key_pb = self._make_key_pb(DATASET_ID)
  rsp_pb = datastore_pb.CommitResponse()

gcloud/datastore/test_entity.py

@@ -21,11 +21,20 @@
 
 class TestEntity(unittest2.TestCase):
 
- def _getTargetClass(self):
+ def setUp(self):
  from gcloud.datastore import _implicit_environ
- from gcloud.datastore.entity import Entity
-
+ self._replaced_dataset = _implicit_environ.DATASET
+ self._replaced_dataset_id = _implicit_environ.DATASET_ID
  _implicit_environ.DATASET = None
+ _implicit_environ.DATASET_ID = None
+
+ def tearDown(self):
+ from gcloud.datastore import _implicit_environ
+ _implicit_environ.DATASET = self._replaced_dataset
+ _implicit_environ.DATASET_ID = self._replaced_dataset_id
+
+ def _getTargetClass(self):
+ from gcloud.datastore.entity import Entity
  return Entity
 
  def _makeOne(self, key=None, exclude_from_indexes=()):

gcloud/datastore/test_helpers.py

@@ -20,11 +20,14 @@ class Test_entity_from_protobuf(unittest2.TestCase):
  def setUp(self):
  from gcloud.datastore import _implicit_environ
  self._replaced_dataset = _implicit_environ.DATASET
+ self._replaced_dataset_id = _implicit_environ.DATASET_ID
  _implicit_environ.DATASET = None
+ _implicit_environ.DATASET_ID = None
 
  def tearDown(self):
  from gcloud.datastore import _implicit_environ
  _implicit_environ.DATASET = self._replaced_dataset
+ _implicit_environ.DATASET_ID = self._replaced_dataset_id
 
  def _callFUT(self, val):
  from gcloud.datastore.helpers import entity_from_protobuf

gcloud/datastore/test_key.py

@@ -20,6 +20,17 @@ class TestKey(unittest2.TestCase):
  def setUp(self):
  self._DEFAULT_DATASET = 'DATASET'
 
+ from gcloud.datastore import _implicit_environ
+ self._replaced_dataset = _implicit_environ.DATASET
+ self._replaced_dataset_id = _implicit_environ.DATASET_ID
+ _implicit_environ.DATASET = None
+ _implicit_environ.DATASET_ID = None
+
+ def tearDown(self):
+ from gcloud.datastore import _implicit_environ
+ _implicit_environ.DATASET = self._replaced_dataset
+ _implicit_environ.DATASET_ID = self._replaced_dataset_id
+
  def _getTargetClass(self):
  from gcloud.datastore import _implicit_environ
  from gcloud.datastore.dataset import Dataset

gcloud/datastore/test_query.py

@@ -20,11 +20,14 @@ class TestQuery(unittest2.TestCase):
  def setUp(self):
  from gcloud.datastore import _implicit_environ
  self._replaced_dataset = _implicit_environ.DATASET
+ self._replaced_dataset_id = _implicit_environ.DATASET_ID
  _implicit_environ.DATASET = None
+ _implicit_environ.DATASET_ID = None
 
  def tearDown(self):
  from gcloud.datastore import _implicit_environ
  _implicit_environ.DATASET = self._replaced_dataset
+ _implicit_environ.DATASET_ID = self._replaced_dataset_id
 
  def _getTargetClass(self):
  from gcloud.datastore.query import Query
@@ -429,11 +432,14 @@ class TestIterator(unittest2.TestCase):
  def setUp(self):
  from gcloud.datastore import _implicit_environ
  self._replaced_dataset = _implicit_environ.DATASET
+ self._replaced_dataset_id = _implicit_environ.DATASET_ID
  _implicit_environ.DATASET = None
+ _implicit_environ.DATASET_ID = None
 
  def tearDown(self):
  from gcloud.datastore import _implicit_environ
  _implicit_environ.DATASET = self._replaced_dataset
+ _implicit_environ.DATASET_ID = self._replaced_dataset_id
 
  def _getTargetClass(self):
  from gcloud.datastore.query import Iterator

gcloud/datastore/test_transaction.py

@@ -22,49 +22,60 @@ def _getTargetClass(self):
 
  return Transaction
 
- def _makeOne(self, dataset=None):
- return self._getTargetClass()(dataset)
+ def _makeOne(self, dataset_id=None, connection=None):
+ return self._getTargetClass()(dataset_id=dataset_id,
+ connection=connection)
+
+ def test_ctor_missing_required(self):
+ from gcloud.datastore import _implicit_environ
+
+ self.assertEqual(_implicit_environ.DATASET, None)
+
+ with self.assertRaises(ValueError):
+ self._makeOne()
+ with self.assertRaises(ValueError):
+ self._makeOne(dataset_id=object())
+ with self.assertRaises(ValueError):
+ self._makeOne(connection=object())
 
  def test_ctor(self):
  from gcloud.datastore.datastore_v1_pb2 import Mutation
 
  _DATASET = 'DATASET'
  connection = _Connection()
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  self.assertEqual(xact._dataset_id, _DATASET)
  self.assertEqual(xact._connection, connection)
  self.assertEqual(xact.id, None)
  self.assertTrue(isinstance(xact.mutation, Mutation))
  self.assertEqual(len(xact._auto_id_entities), 0)
 
  def test_ctor_with_env(self):
+ from gcloud._testing import _Monkey
  from gcloud.datastore import _implicit_environ
 
  DATASET_ID = 'DATASET'
  CONNECTION = _Connection()
- DATASET = _Dataset(DATASET_ID, CONNECTION)
 
- _implicit_environ.DATASET = DATASET
+ with _Monkey(_implicit_environ, DATASET_ID=DATASET_ID,
+ CONNECTION=CONNECTION):
+ transaction = self._makeOne()
 
- transaction = self._makeOne(dataset=None)
  self.assertEqual(transaction._dataset_id, DATASET_ID)
  self.assertEqual(transaction._connection, CONNECTION)
 
  def test_add_auto_id_entity(self):
  entity = _Entity()
  _DATASET = 'DATASET'
  connection = _Connection()
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  xact.add_auto_id_entity(entity)
  self.assertEqual(xact._auto_id_entities, [entity])
 
  def test_begin(self):
  _DATASET = 'DATASET'
  connection = _Connection(234)
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  xact.begin()
  self.assertEqual(xact.id, 234)
  self.assertEqual(connection._begun, _DATASET)
@@ -73,8 +84,7 @@ def test_begin(self):
  def test_rollback(self):
  _DATASET = 'DATASET'
  connection = _Connection(234)
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  xact.begin()
  xact.rollback()
  self.assertEqual(xact.id, None)
@@ -84,8 +94,7 @@ def test_rollback(self):
  def test_commit_no_auto_ids(self):
  _DATASET = 'DATASET'
  connection = _Connection(234)
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  xact._mutation = mutation = object()
  xact.begin()
  xact.commit()
@@ -100,8 +109,7 @@ def test_commit_w_auto_ids(self):
  connection = _Connection(234)
  connection._commit_result = _CommitResult(
  _make_key(_KIND, _ID, _DATASET))
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  entity = _Entity()
  xact.add_auto_id_entity(entity)
  xact._mutation = mutation = object()
@@ -110,13 +118,12 @@ def test_commit_w_auto_ids(self):
  self.assertEqual(connection._committed, (_DATASET, mutation))
  self.assertTrue(connection._xact is None)
  self.assertEqual(xact.id, None)
- self.assertEqual(entity._key._path, [{'kind': _KIND, 'id': _ID}])
+ self.assertEqual(entity.key.path, [{'kind': _KIND, 'id': _ID}])
 
  def test_commit_w_already(self):
  _DATASET = 'DATASET'
  connection = _Connection(234)
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  xact._mutation = object()
  xact.begin()
  connection.transaction(()) # Simulate previous commit via false-ish.
@@ -128,8 +135,7 @@ def test_commit_w_already(self):
  def test_context_manager_no_raise(self):
  _DATASET = 'DATASET'
  connection = _Connection(234)
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  xact._mutation = mutation = object()
  with xact:
  self.assertEqual(xact.id, 234)
@@ -144,8 +150,7 @@ class Foo(Exception):
  pass
  _DATASET = 'DATASET'
  connection = _Connection(234)
- dataset = _Dataset(_DATASET, connection)
- xact = self._makeOne(dataset)
+ xact = self._makeOne(dataset_id=_DATASET, connection=connection)
  xact._mutation = object()
  try:
  with xact:
@@ -173,19 +178,6 @@ def _make_key(kind, id, dataset_id):
  return key
 
 
-class _Dataset(object):
-
- def __init__(self, id, connection=None):
- self._id = id
- self._connection = connection
-
- def id(self):
- return self._id
-
- def connection(self):
- return self._connection
-
-
 class _Connection(object):
  _marker = object()
  _begun = _rolled_back = _committed = _xact = None
@@ -221,9 +213,4 @@ class _Entity(object):
 
  def __init__(self):
  from gcloud.datastore.key import Key
- self._key = Key('KIND', dataset_id='DATASET')
-
- def key(self, key=None):
- if key is not None:
- self._key = key
- return self._key
+ self.key = Key('KIND', dataset_id='DATASET')

gcloud/datastore/transaction.py

@@ -121,14 +121,24 @@ class Transaction(object):
  ... with dataset2.transaction():
  ... dataset2.entity('Thing').save()
 
- :type dataset: :class:`gcloud.datastore.dataset.Dataset`
- :param dataset: The dataset to which this :class:`Transaction` belongs.
+ :type dataset_id: :class:`str`.
+ :param dataset_id: The ID of the dataset.
+
+ :type connection: :class:`gcloud.datastore.connection.Connection`
+ :param connection: The connection used to connect to datastore.
+
+ :raises: :class:`ValueError` if either a connection or dataset ID
+ are not set.
  """
 
- def __init__(self, dataset=None):
- dataset = dataset or _implicit_environ.DATASET
- self._connection = dataset.connection()
- self._dataset_id = dataset.id()
+ def __init__(self, dataset_id=None, connection=None):
+ self._connection = connection or _implicit_environ.CONNECTION
+ self._dataset_id = dataset_id or _implicit_environ.DATASET_ID
+
+ if self._connection is None or self._dataset_id is None:
+ raise ValueError('A transaction must have a connection and '
+ 'a dataset ID set.')
+
  self._id = None
  self._mutation = datastore_pb.Mutation()
  self._auto_id_entities = []
@@ -227,7 +237,7 @@ def commit(self):
  for i, entity in enumerate(self._auto_id_entities):
  key_pb = result.insert_auto_id_key[i]
  new_id = key_pb.path_element[-1].id
- entity.key(entity.key().completed_key(new_id))
+ entity.key = entity.key.completed_key(new_id)
 
  # Tell the connection that the transaction is over.
  self.connection.transaction(None)