From ec54e98d87fd48afa895ff4d112ddca06dd4e713 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 13:41:51 -0500 Subject: [PATCH 1/8] Don't mangle FUT into testcase method names. --- gcloud/datastore/test_api.py | 50 ++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/gcloud/datastore/test_api.py b/gcloud/datastore/test_api.py index eba32a2ca5e2..6853d8bcda4a 100644 --- a/gcloud/datastore/test_api.py +++ b/gcloud/datastore/test_api.py @@ -30,30 +30,30 @@ def _monkey(self, dataset_id): from gcloud._testing import _Monkey return _Monkey(_implicit_environ, DATASET_ID=dataset_id) - def test__require_dataset_implicit_unset(self): + def test_implicit_unset(self): with self._monkey(None): with self.assertRaises(EnvironmentError): self._callFUT() - def test__require_dataset_implicit_unset_passed_explicitly(self): + def test_implicit_unset_passed_explicitly(self): ID = 'DATASET' with self._monkey(None): self.assertEqual(self._callFUT(ID), ID) - def test__require_dataset_id_implicit_set(self): + def test_id_implicit_set(self): IMPLICIT_ID = 'IMPLICIT' with self._monkey(IMPLICIT_ID): stored_id = self._callFUT() self.assertTrue(stored_id is IMPLICIT_ID) - def test__require_dataset_id_implicit_set_passed_explicitly(self): + def test_id_implicit_set_passed_explicitly(self): ID = 'DATASET' IMPLICIT_ID = 'IMPLICIT' with self._monkey(IMPLICIT_ID): self.assertEqual(self._callFUT(ID), ID) -class Test_require_connection(unittest2.TestCase): +class Test__require_connection(unittest2.TestCase): _MARKER = object() @@ -68,22 +68,22 @@ def _monkey(self, connection): from gcloud._testing import _Monkey return _Monkey(_implicit_environ, CONNECTION=connection) - def test__require_connection_implicit_unset(self): + def test_implicit_unset(self): with self._monkey(None): with self.assertRaises(EnvironmentError): self._callFUT() - def test__require_connection_implicit_unset_passed_explicitly(self): + def test_implicit_unset_passed_explicitly(self): CONNECTION = object() with self._monkey(None): self.assertTrue(self._callFUT(CONNECTION) is CONNECTION) - def test__require_connection_implicit_set(self): + def test_implicit_set(self): IMPLICIT_CONNECTION = object() with self._monkey(IMPLICIT_CONNECTION): self.assertTrue(self._callFUT() is IMPLICIT_CONNECTION) - def test__require_connection_implicit_set_passed_explicitly(self): + def test_implicit_set_passed_explicitly(self): IMPLICIT_CONNECTION = object() CONNECTION = object() with self._monkey(IMPLICIT_CONNECTION): @@ -97,11 +97,11 @@ def _callFUT(self, keys, missing=None, deferred=None, connection=None): return get(keys, missing=missing, deferred=deferred, connection=connection) - def test_get_no_keys(self): + def test_no_keys(self): results = self._callFUT([]) self.assertEqual(results, []) - def test_get_miss(self): + def test_miss(self): from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -111,7 +111,7 @@ def test_get_miss(self): results = self._callFUT([key], connection=connection) self.assertEqual(results, []) - def test_get_miss_w_missing(self): + def test_miss_w_missing(self): from gcloud.datastore import datastore_v1_pb2 as datastore_pb from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -138,7 +138,7 @@ def test_get_miss_w_missing(self): self.assertEqual([missed.key.to_protobuf() for missed in missing], [key.to_protobuf()]) - def test_get_miss_w_deferred(self): + def test_miss_w_deferred(self): from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -172,7 +172,7 @@ def _make_entity_pb(self, dataset_id, kind, integer_id, return entity_pb - def test_get_hit(self): + def test_hit(self): from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -199,7 +199,7 @@ def test_get_hit(self): self.assertEqual(list(result), ['foo']) self.assertEqual(result['foo'], 'Foo') - def test_get_hit_multiple_keys_same_dataset(self): + def test_hit_multiple_keys_same_dataset(self): from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -226,7 +226,7 @@ def test_get_hit_multiple_keys_same_dataset(self): self.assertEqual(retrieved2.key.path, key2.path) self.assertEqual(dict(retrieved2), {}) - def test_get_hit_multiple_keys_different_dataset(self): + def test_hit_multiple_keys_different_dataset(self): from gcloud.datastore.key import Key DATASET_ID1 = 'DATASET' @@ -240,7 +240,7 @@ def test_get_hit_multiple_keys_different_dataset(self): with self.assertRaises(ValueError): self._callFUT([key1, key2], connection=object()) - def test_get_implicit(self): + def test_implicit(self): from gcloud.datastore import _implicit_environ from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -284,7 +284,7 @@ def _callFUT(self, keys, connection=None): from gcloud.datastore.api import delete return delete(keys, connection=connection) - def test_delete_no_batch(self): + def test_no_batch(self): from gcloud.datastore.test_batch import _Connection from gcloud.datastore.test_batch import _Key @@ -296,7 +296,7 @@ def test_delete_no_batch(self): result = self._callFUT([key], connection=connection) self.assertEqual(result, None) - def test_delete_existing_batch(self): + def test_existing_batch(self): from gcloud._testing import _Monkey from gcloud.datastore import api from gcloud.datastore.batch import _Batches @@ -324,7 +324,7 @@ def test_delete_existing_batch(self): self.assertEqual(len(deletes), 1) self.assertEqual(deletes[0], key._key) - def test_delete_implicit_connection(self): + def test_implicit_connection(self): from gcloud._testing import _Monkey from gcloud.datastore import _implicit_environ from gcloud.datastore import api @@ -354,14 +354,14 @@ def test_delete_implicit_connection(self): self.assertEqual(len(deletes), 1) self.assertEqual(deletes[0], key._key) - def test_delete_no_keys(self): + def test_no_keys(self): from gcloud.datastore import _implicit_environ self.assertEqual(_implicit_environ.CONNECTION, None) result = self._callFUT([]) self.assertEqual(result, None) - def test_delete_no_connection(self): + def test_no_connection(self): from gcloud.datastore import _implicit_environ from gcloud.datastore.test_batch import _Key @@ -380,7 +380,7 @@ def _callFUT(self, incomplete_key, num_ids, connection=None): from gcloud.datastore.api import allocate_ids return allocate_ids(incomplete_key, num_ids, connection=connection) - def test_allocate_ids(self): + def test_w_explicit_connection(self): from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -397,7 +397,7 @@ def test_allocate_ids(self): self.assertEqual(CONNECTION._called_dataset_id, DATASET_ID) self.assertEqual(len(CONNECTION._called_key_pbs), NUM_IDS) - def test_allocate_ids_implicit(self): + def test_w_implicit_connection(self): from gcloud.datastore import _implicit_environ from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection @@ -413,7 +413,7 @@ def test_allocate_ids_implicit(self): # Check the IDs returned. self.assertEqual([key.id for key in result], range(NUM_IDS)) - def test_allocate_ids_with_complete(self): + def test_with_already_completed_key(self): from gcloud.datastore import _implicit_environ from gcloud.datastore.key import Key from gcloud.datastore.test_connection import _Connection From b2d33ff92e3f5eaaea23d6fb676ee826a4b39ab4 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 13:56:19 -0500 Subject: [PATCH 2/8] Add assertions about commits. --- gcloud/datastore/test_api.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gcloud/datastore/test_api.py b/gcloud/datastore/test_api.py index 6853d8bcda4a..6797fb75bb4c 100644 --- a/gcloud/datastore/test_api.py +++ b/gcloud/datastore/test_api.py @@ -295,6 +295,10 @@ def test_no_batch(self): result = self._callFUT([key], connection=connection) self.assertEqual(result, None) + self.assertEqual(len(connection._committed), 1) + dataset_id, mutation = connection._committed[0] + self.assertEqual(dataset_id, _DATASET) + self.assertEqual(list(mutation.delete), [key.to_protobuf()]) def test_existing_batch(self): from gcloud._testing import _Monkey @@ -323,6 +327,7 @@ def test_existing_batch(self): deletes = list(CURR_BATCH.mutation.delete) self.assertEqual(len(deletes), 1) self.assertEqual(deletes[0], key._key) + self.assertEqual(len(connection._committed), 0) def test_implicit_connection(self): from gcloud._testing import _Monkey @@ -353,6 +358,7 @@ def test_implicit_connection(self): deletes = list(CURR_BATCH.mutation.delete) self.assertEqual(len(deletes), 1) self.assertEqual(deletes[0], key._key) + self.assertEqual(len(connection._committed), 0) def test_no_keys(self): from gcloud.datastore import _implicit_environ From 61cb02d4ecbdac836cd91a45d46393c3ce5a0635 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 14:13:47 -0500 Subject: [PATCH 3/8] Add 'datastore.put' API. Toward #514. --- gcloud/datastore/api.py | 26 ++++++++ gcloud/datastore/test_api.py | 121 +++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+) diff --git a/gcloud/datastore/api.py b/gcloud/datastore/api.py index 91a90c49f7b7..4d5b9b616a1c 100644 --- a/gcloud/datastore/api.py +++ b/gcloud/datastore/api.py @@ -133,6 +133,32 @@ def get(keys, missing=None, deferred=None, connection=None): return entities +def put(entities, connection=None): + """Save the entities in the Cloud Datastore. + + :type entities: list of :class:`gcloud.datastore.entity.Entity` + :param entities: The entities to be saved to the datastore. + + :type connection: :class:`gcloud.datastore.connection.Connection` + :param connection: Optional connection used to connect to datastore. + """ + if not entities: + return + + connection = connection or _implicit_environ.CONNECTION + + current = _BATCHES.top + in_batch = current is not None + if not in_batch: + keys = [entity.key for entity in entities] + dataset_id = _get_dataset_id_from_keys(keys) + current = Batch(dataset_id=dataset_id, connection=connection) + for entity in entities: + current.put(entity) + if not in_batch: + current.commit() + + def delete(keys, connection=None): """Delete the keys in the Cloud Datastore. diff --git a/gcloud/datastore/test_api.py b/gcloud/datastore/test_api.py index 6797fb75bb4c..28a15014791b 100644 --- a/gcloud/datastore/test_api.py +++ b/gcloud/datastore/test_api.py @@ -278,6 +278,127 @@ def test_implicit(self): self.assertEqual(result['foo'], 'Foo') +class Test_put_function(unittest2.TestCase): + + def _callFUT(self, entities, connection=None): + from gcloud.datastore.api import put + return put(entities, connection=connection) + + def test_no_batch_w_partial_key(self): + from gcloud.datastore.test_batch import _Connection + from gcloud.datastore.test_batch import _Entity + from gcloud.datastore.test_batch import _Key + + # Build basic mocks needed to delete. + _DATASET = 'DATASET' + connection = _Connection() + entity = _Entity(foo=u'bar') + key = entity.key = _Key(_DATASET) + key._id = None + + result = self._callFUT([entity], connection=connection) + self.assertEqual(result, None) + self.assertEqual(len(connection._committed), 1) + dataset_id, mutation = connection._committed[0] + self.assertEqual(dataset_id, _DATASET) + inserts = list(mutation.insert_auto_id) + self.assertEqual(len(inserts), 1) + self.assertEqual(inserts[0].key, key.to_protobuf()) + properties = list(inserts[0].property) + self.assertEqual(properties[0].name, 'foo') + self.assertEqual(properties[0].value.string_value, u'bar') + + def test_existing_batch_w_completed_key(self): + from gcloud._testing import _Monkey + from gcloud.datastore import api + from gcloud.datastore.batch import _Batches + from gcloud.datastore.batch import Batch + from gcloud.datastore.test_batch import _Connection + from gcloud.datastore.test_batch import _Entity + from gcloud.datastore.test_batch import _Key + + # Build basic mocks needed to delete. + _DATASET = 'DATASET' + connection = _Connection() + entity = _Entity(foo=u'bar') + key = entity.key = _Key(_DATASET) + + # Set up mock Batch on stack so we can check it is used. + _BATCHES = _Batches() + CURR_BATCH = Batch(dataset_id=_DATASET, connection=connection) + _BATCHES.push(CURR_BATCH) + + with _Monkey(api, _BATCHES=_BATCHES): + result = self._callFUT([entity], connection=connection) + + self.assertEqual(result, None) + self.assertEqual(len(CURR_BATCH.mutation.insert_auto_id), 0) + upserts = list(CURR_BATCH.mutation.upsert) + self.assertEqual(len(upserts), 1) + self.assertEqual(upserts[0].key, key.to_protobuf()) + properties = list(upserts[0].property) + self.assertEqual(properties[0].name, 'foo') + self.assertEqual(properties[0].value.string_value, u'bar') + self.assertEqual(len(CURR_BATCH.mutation.delete), 0) + + def test_implicit_connection(self): + from gcloud._testing import _Monkey + from gcloud.datastore import _implicit_environ + from gcloud.datastore import api + from gcloud.datastore.batch import _Batches + from gcloud.datastore.batch import Batch + from gcloud.datastore.test_batch import _Connection + from gcloud.datastore.test_batch import _Entity + from gcloud.datastore.test_batch import _Key + + # Build basic mocks needed to delete. + _DATASET = 'DATASET' + connection = _Connection() + entity = _Entity(foo=u'bar') + key = entity.key = _Key(_DATASET) + + # Set up mock Batch on stack so we can check it is used. + _BATCHES = _Batches() + + with _Monkey(_implicit_environ, CONNECTION=connection): + CURR_BATCH = Batch(dataset_id=_DATASET) + _BATCHES.push(CURR_BATCH) + with _Monkey(api, _BATCHES=_BATCHES): + result = self._callFUT([entity]) + + self.assertEqual(result, None) + self.assertEqual(len(CURR_BATCH.mutation.insert_auto_id), 0) + self.assertEqual(len(CURR_BATCH.mutation.upsert), 1) + upserts = list(CURR_BATCH.mutation.upsert) + self.assertEqual(len(upserts), 1) + self.assertEqual(upserts[0].key, key.to_protobuf()) + properties = list(upserts[0].property) + self.assertEqual(properties[0].name, 'foo') + self.assertEqual(properties[0].value.string_value, u'bar') + self.assertEqual(len(CURR_BATCH.mutation.delete), 0) + + def test_no_entities(self): + from gcloud.datastore import _implicit_environ + + self.assertEqual(_implicit_environ.CONNECTION, None) + result = self._callFUT([]) + self.assertEqual(result, None) + + def test_no_connection(self): + from gcloud.datastore import _implicit_environ + from gcloud.datastore.test_batch import _Entity + from gcloud.datastore.test_batch import _Key + + # Build basic mocks needed to delete. + _DATASET = 'DATASET' + entity = _Entity(foo=u'bar') + entity.key = _Key(_DATASET) + + self.assertEqual(_implicit_environ.CONNECTION, None) + with self.assertRaises(ValueError): + self._callFUT([entity]) + + class Test_delete_function(unittest2.TestCase): def _callFUT(self, keys, connection=None): From 84d47f9e8015617df8961051ef48066d4ed02c09 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 14:41:11 -0500 Subject: [PATCH 4/8] Remove 'Entity.save'. --- .../_components/datastore-getting-started.rst | 2 +- docs/_components/datastore-quickstart.rst | 2 +- docs/index.rst | 2 +- gcloud/datastore/__init__.py | 1 + gcloud/datastore/demo/demo.py | 20 ++-- gcloud/datastore/entity.py | 55 +-------- gcloud/datastore/test_entity.py | 105 ------------------ gcloud/datastore/transaction.py | 39 ++----- regression/datastore.py | 14 +-- regression/populate_datastore.py | 4 +- 10 files changed, 34 insertions(+), 210 deletions(-) diff --git a/docs/_components/datastore-getting-started.rst b/docs/_components/datastore-getting-started.rst index 98ae01a152ee..457f9ce79b74 100644 --- a/docs/_components/datastore-getting-started.rst +++ b/docs/_components/datastore-getting-started.rst @@ -44,7 +44,7 @@ Open a Python console and... >>> entity = datastore.Entity(key=datastore.Key('Person')) >>> entity['name'] = 'Your name' >>> entity['age'] = 25 - >>> entity.save() + >>> datastore.put([entity]) >>> list(Query(kind='Person').fetch()) [] diff --git a/docs/_components/datastore-quickstart.rst b/docs/_components/datastore-quickstart.rst index 0112d9a8afb0..8a8a4b658eb8 100644 --- a/docs/_components/datastore-quickstart.rst +++ b/docs/_components/datastore-quickstart.rst @@ -58,7 +58,7 @@ you can create entities and save them:: >>> entity = datastore.Entity(key=datastore.Key('Person')) >>> entity['name'] = 'Your name' >>> entity['age'] = 25 - >>> entity.save() + >>> datastore.put([entity]) >>> list(datastore.Query(kind='Person').fetch()) [] diff --git a/docs/index.rst b/docs/index.rst index 4f1e24bbb89f..56d1d7e4e3b3 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -34,7 +34,7 @@ Cloud Datastore entity = datastore.Entity(key=datastore.Key('Person')) entity['name'] = 'Your name' entity['age'] = 25 - entity.save() + datastore.put([entity]) Cloud Storage ~~~~~~~~~~~~~ diff --git a/gcloud/datastore/__init__.py b/gcloud/datastore/__init__.py index 75a410fa20cc..0a9c515a5ed8 100644 --- a/gcloud/datastore/__init__.py +++ b/gcloud/datastore/__init__.py @@ -53,6 +53,7 @@ from gcloud.datastore.api import allocate_ids from gcloud.datastore.api import delete from gcloud.datastore.api import get +from gcloud.datastore.api import put from gcloud.datastore.batch import Batch from gcloud.datastore.connection import Connection from gcloud.datastore.entity import Entity diff --git a/gcloud/datastore/demo/demo.py b/gcloud/datastore/demo/demo.py index 7a4c40eb3b3b..908d05e9b1af 100644 --- a/gcloud/datastore/demo/demo.py +++ b/gcloud/datastore/demo/demo.py @@ -28,7 +28,7 @@ toy.update({'name': 'Toy'}) # Now let's save it to our datastore: -toy.save() +datastore.put([toy]) # If we look it up by its key, we should find it... print(datastore.get([toy.key])) @@ -55,7 +55,7 @@ entity = datastore.Entity(key) entity['name'] = name entity['age'] = age - entity.save() + datastore.put([entity]) # We'll start by look at all Thing entities: query = datastore.Query(kind='Thing') @@ -76,18 +76,18 @@ # You can also work inside a transaction. # (Check the official docs for explanations of what's happening here.) -with datastore.Transaction(): +with datastore.Transaction() as xact: print('Creating and saving an entity...') key = datastore.Key('Thing', 'foo') thing = datastore.Entity(key) thing['age'] = 10 - thing.save() + xact.put(thing) print('Creating and saving another entity...') key2 = datastore.Key('Thing', 'bar') thing2 = datastore.Entity(key2) thing2['age'] = 15 - thing2.save() + xact.put(thing2) print('Committing the transaction...') @@ -95,11 +95,11 @@ datastore.delete([key, key2]) # To rollback a transaction, just call .rollback() -with datastore.Transaction() as t: +with datastore.Transaction() as xact: key = datastore.Key('Thing', 'another') thing = datastore.Entity(key) - thing.save() - t.rollback() + xact.put(thing) + xact.rollback() # Let's check if the entity was actually created: created = datastore.get([key]) @@ -107,10 +107,10 @@ # Remember, a key won't be complete until the transaction is commited. # That is, while inside the transaction block, thing.key will be incomplete. -with datastore.Transaction(): +with datastore.Transaction() as xact: key = datastore.Key('Thing') # partial thing = datastore.Entity(key) - thing.save() + xact.put(thing) print(thing.key) # This will still be partial print(thing.key) # This will be complete diff --git a/gcloud/datastore/entity.py b/gcloud/datastore/entity.py index c1fc8c3e8f54..de635012314e 100644 --- a/gcloud/datastore/entity.py +++ b/gcloud/datastore/entity.py @@ -14,8 +14,6 @@ """Class for representing a single entity in the Cloud Datastore.""" -from gcloud.datastore import _implicit_environ - class NoKey(RuntimeError): """Exception raised by Entity methods which require a key.""" @@ -70,8 +68,8 @@ class Entity(dict): any decoding / encoding step. :type key: :class:`gcloud.datastore.key.Key` - :param key: Optional key to be set on entity. Required for :meth:`save()` - or :meth:`reload()`. + :param key: Optional key to be set on entity. Required for + :fund:`gcloud.datastore.put()` :type exclude_from_indexes: tuple of string :param exclude_from_indexes: Names of fields whose values are not to be @@ -104,55 +102,6 @@ def exclude_from_indexes(self): """ return frozenset(self._exclude_from_indexes) - @property - def _must_key(self): - """Return our key, or raise NoKey if not set. - - :rtype: :class:`gcloud.datastore.key.Key`. - :returns: The entity's key. - :raises: :class:`NoKey` if no key is set. - """ - if self.key is None: - raise NoKey() - return self.key - - def save(self, connection=None): - """Save the entity in the Cloud Datastore. - - .. note:: - Any existing properties for the entity will be replaced by those - currently set on this instance. Already-stored properties which do - not correspond to keys set on this instance will be removed from - the datastore. - - .. note:: - Property values which are "text" (``unicode`` in Python2, ``str`` in - Python3) map to 'string_value' in the datastore; values which are - "bytes" (``str`` in Python2, ``bytes`` in Python3) map to - 'blob_value'. - - :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 - assigned, new_id = connection.save_entity( - dataset_id=key.dataset_id, - key_pb=key.to_protobuf(), - properties=dict(self), - exclude_from_indexes=self.exclude_from_indexes) - - # If we are in a transaction and the current entity needs an - # automatically assigned ID, tell the transaction where to put that. - transaction = connection.transaction() - if transaction and key.is_partial: - transaction.add_auto_id_entity(self) - - if assigned: - # Update the key (which may have been altered). - self.key = self.key.completed_key(new_id) - def __repr__(self): if self.key: return '' % (self.key.path, diff --git a/gcloud/datastore/test_entity.py b/gcloud/datastore/test_entity.py index 87062bdcf869..2f753f79704b 100644 --- a/gcloud/datastore/test_entity.py +++ b/gcloud/datastore/test_entity.py @@ -53,79 +53,6 @@ def test_ctor_explicit(self): self.assertEqual(sorted(entity.exclude_from_indexes), sorted(_EXCLUDE_FROM_INDEXES)) - def test__must_key_no_key(self): - from gcloud.datastore.entity import NoKey - - entity = self._makeOne() - self.assertRaises(NoKey, getattr, entity, '_must_key') - - def test_save_no_key(self): - from gcloud.datastore.entity import NoKey - - entity = self._makeOne() - entity['foo'] = 'Foo' - self.assertRaises(NoKey, entity.save) - - def test_save_wo_transaction_wo_auto_id_wo_returned_key(self): - connection = _Connection() - key = _Key() - entity = self._makeOne(key=key) - entity['foo'] = 'Foo' - entity.save(connection=connection) - self.assertEqual(entity['foo'], 'Foo') - self.assertEqual(connection._saved, - (_DATASET_ID, 'KEY', {'foo': 'Foo'}, ())) - self.assertEqual(key._path, None) - - def test_save_w_transaction_wo_partial_key(self): - connection = _Connection() - transaction = connection._transaction = _Transaction() - key = _Key() - entity = self._makeOne(key=key) - entity['foo'] = 'Foo' - entity.save(connection=connection) - self.assertEqual(entity['foo'], 'Foo') - self.assertEqual(connection._saved, - (_DATASET_ID, 'KEY', {'foo': 'Foo'}, ())) - self.assertEqual(transaction._added, ()) - self.assertEqual(key._path, None) - - def test_save_w_transaction_w_partial_key(self): - connection = _Connection() - transaction = connection._transaction = _Transaction() - key = _Key() - key._partial = True - entity = self._makeOne(key=key) - entity['foo'] = 'Foo' - entity.save(connection=connection) - self.assertEqual(entity['foo'], 'Foo') - self.assertEqual(connection._saved, - (_DATASET_ID, 'KEY', {'foo': 'Foo'}, ())) - self.assertEqual(transaction._added, (entity,)) - self.assertEqual(key._path, None) - - def test_save_w_returned_key_exclude_from_indexes(self): - from gcloud.datastore import datastore_v1_pb2 as datastore_pb - from gcloud.datastore.key import Key - - key_pb = datastore_pb.Key() - key_pb.partition_id.dataset_id = _DATASET_ID - key_pb.path_element.add(kind=_KIND, id=_ID) - connection = _Connection() - connection._save_result = (True, _ID) - key = Key('KIND', dataset_id=_DATASET_ID) - entity = self._makeOne(key=key, exclude_from_indexes=['foo']) - entity['foo'] = 'Foo' - entity.save(connection=connection) - self.assertEqual(entity['foo'], 'Foo') - self.assertEqual(connection._saved[0], _DATASET_ID) - self.assertEqual(connection._saved[1], key.to_protobuf()) - self.assertEqual(connection._saved[2], {'foo': 'Foo'}) - self.assertEqual(connection._saved[3], ('foo',)) - self.assertEqual(len(connection._saved), 4) - - self.assertEqual(entity.key._path, [{'kind': _KIND, 'id': _ID}]) - def test___repr___no_key_empty(self): entity = self._makeOne() self.assertEqual(repr(entity), '') @@ -149,38 +76,6 @@ class _Key(object): def __init__(self, dataset_id=_DATASET_ID): self.dataset_id = dataset_id - def to_protobuf(self): - return self._key - - @property - def is_partial(self): - return self._partial - @property def path(self): return self._path - - -class _Connection(object): - _transaction = _saved = _deleted = None - _save_result = (False, None) - - def transaction(self): - return self._transaction - - def save_entity(self, dataset_id, key_pb, properties, - exclude_from_indexes=()): - self._saved = (dataset_id, key_pb, properties, - tuple(exclude_from_indexes)) - return self._save_result - - -class _Transaction(object): - _added = () - - def __nonzero__(self): - return True - __bool__ = __nonzero__ - - def add_auto_id_entity(self, entity): - self._added += (entity,) diff --git a/gcloud/datastore/transaction.py b/gcloud/datastore/transaction.py index 55eb82e39142..0a31b2a5fd29 100644 --- a/gcloud/datastore/transaction.py +++ b/gcloud/datastore/transaction.py @@ -32,16 +32,16 @@ class Transaction(Batch): >>> datastore.set_defaults() - >>> with Transaction() - ... entity1.save() - ... entity2.save() + >>> with Transaction() as xact: + ... datastore.put(entity1) + ... datastore.put(entity2) Because it derives from :class:`Batch`, :class`Transaction` also provides :meth:`put` and :meth:`delete` methods:: - >>> with Transaction() - ... transaction.put(entity1) - ... transaction.delete(entity2.key) + >>> with Transaction() as xact: + ... xact.put(entity1) + ... xact.delete(entity2.key) By default, the transaction is rolled back if the transaction block exits with an error:: @@ -59,7 +59,7 @@ class Transaction(Batch): >>> with Transaction(): ... entity = Entity(key=Key('Thing')) - ... entity.save() + ... datastore.put([entity]) ``entity`` won't have a complete Key until the transaction is committed. @@ -69,7 +69,7 @@ class Transaction(Batch): >>> with Transaction(): ... entity = Entity(key=Key('Thing')) - ... entity.save() + ... datastore.put([entity]) ... assert entity.key.is_partial # There is no ID on this key. >>> assert not entity.key.is_partial # There *is* an ID. @@ -89,34 +89,13 @@ class Transaction(Batch): >>> transaction.begin() >>> entity = Entity(key=Key('Thing')) - >>> entity.save() + >>> datastore.put([entity]) >>> if error: ... transaction.rollback() ... else: ... transaction.commit() - For now, this library will enforce a rule of one transaction per - connection. That is, If you want to work with two transactions at - the same time (for whatever reason), that must happen over two - separate :class:`gcloud.datastore.connection.Connection` s. - - For example, this is perfectly valid:: - - >>> with Transaction(): - ... entity = Entity(key=Key('Thing')) - ... entity.save() - - However, this **wouldn't** be acceptable:: - - >>> with Transaction(): - ... Entity(key=Key('Thing')).save() - ... with Transaction(): - ... Entity(key=Key('Thing')).save() - - Technically, it looks like the Protobuf API supports this type of - pattern, however it makes the code particularly messy. - :type dataset_id: string :param dataset_id: The ID of the dataset. diff --git a/regression/datastore.py b/regression/datastore.py index 7c73a2012683..0e73f459ef13 100644 --- a/regression/datastore.py +++ b/regression/datastore.py @@ -77,7 +77,7 @@ def _get_post(self, id_or_name=None, post_content=None): def _generic_test_post(self, name=None, key_id=None): entity = self._get_post(id_or_name=(name or key_id)) - entity.save() + datastore.put([entity]) # Register entity to be deleted. self.case_entities_to_delete.append(entity) @@ -108,9 +108,9 @@ def test_post_with_generated_id(self): self._generic_test_post() def test_save_multiple(self): - with datastore.Transaction(): + with datastore.Transaction() as xact: entity1 = self._get_post() - entity1.save() + xact.put(entity1) # Register entity to be deleted. self.case_entities_to_delete.append(entity1) @@ -124,7 +124,7 @@ def test_save_multiple(self): 'rating': 4.5, } entity2 = self._get_post(post_content=second_post_content) - entity2.save() + xact.put(entity2) # Register entity to be deleted. self.case_entities_to_delete.append(entity2) @@ -146,7 +146,7 @@ def test_save_key_self_reference(self): entity['fullName'] = u'Full name' entity['linkedTo'] = key # Self reference. - entity.save() + datastore.put([entity]) self.case_entities_to_delete.append(entity) query = datastore.Query(kind='Person') @@ -340,10 +340,10 @@ def test_transaction(self): entity = datastore.Entity(key=datastore.Key('Company', 'Google')) entity['url'] = u'www.google.com' - with datastore.Transaction(): + with datastore.Transaction() as xact: results = datastore.get([entity.key]) if len(results) == 0: - entity.save() + xact.put(entity) self.case_entities_to_delete.append(entity) # This will always return after the transaction. diff --git a/regression/populate_datastore.py b/regression/populate_datastore.py index 2e61b2ee53cc..d61ab4ade389 100644 --- a/regression/populate_datastore.py +++ b/regression/populate_datastore.py @@ -82,14 +82,14 @@ def add_characters(): - with datastore.Transaction(): + with datastore.Transaction() as xact: for key_path, character in zip(KEY_PATHS, CHARACTERS): if key_path[-1] != character['name']: raise ValueError(('Character and key don\'t agree', key_path, character)) entity = datastore.Entity(key=datastore.Key(*key_path)) entity.update(character) - entity.save() + xact.put(entity) print('Adding Character %s %s' % (character['name'], character['family'])) From 6873f1dd3d1618a3d7000d029c7ea71e4fcfb3d8 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 16:20:37 -0500 Subject: [PATCH 5/8] Remove never-raised exception. See: https://github.com/GoogleCloudPlatform/gcloud-python/pull/548#discussion_r22966597 --- gcloud/datastore/entity.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/gcloud/datastore/entity.py b/gcloud/datastore/entity.py index de635012314e..e2ab07628cef 100644 --- a/gcloud/datastore/entity.py +++ b/gcloud/datastore/entity.py @@ -15,10 +15,6 @@ """Class for representing a single entity in the Cloud Datastore.""" -class NoKey(RuntimeError): - """Exception raised by Entity methods which require a key.""" - - class Entity(dict): """Entities are akin to rows in a relational database From e27070eddd93ea634129c64ed2ddd7032cda642e Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 16:27:38 -0500 Subject: [PATCH 6/8] Raise if None is passed as a key to '_get_dataset_id_from_keys'. See: https://github.com/GoogleCloudPlatform/gcloud-python/pull/548#discussion_r22966660 --- gcloud/datastore/api.py | 3 +++ gcloud/datastore/test_api.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/gcloud/datastore/api.py b/gcloud/datastore/api.py index 4d5b9b616a1c..a63c5bddf088 100644 --- a/gcloud/datastore/api.py +++ b/gcloud/datastore/api.py @@ -70,6 +70,9 @@ def _get_dataset_id_from_keys(keys): :returns: The dataset ID of the keys. :raises: :class:`ValueError` if the key dataset IDs don't agree. """ + if any(key is None for key in keys): + raise ValueError('None not allowed') + dataset_id = keys[0].dataset_id # Rather than creating a list or set of all dataset IDs, we iterate # and check. We could allow the backend to check this for us if IDs diff --git a/gcloud/datastore/test_api.py b/gcloud/datastore/test_api.py index 28a15014791b..e254342af64a 100644 --- a/gcloud/datastore/test_api.py +++ b/gcloud/datastore/test_api.py @@ -90,6 +90,29 @@ def test_implicit_set_passed_explicitly(self): self.assertTrue(self._callFUT(CONNECTION) is CONNECTION) +class Test__get_dataset_id_from_keys(unittest2.TestCase): + + def _callFUT(self, keys): + from gcloud.datastore.api import _get_dataset_id_from_keys + return _get_dataset_id_from_keys(keys) + + def test_empty(self): + self.assertRaises(IndexError, self._callFUT, []) + + def test_w_None(self): + self.assertRaises(ValueError, self._callFUT, [None]) + + def test_w_mismatch(self): + foo = _Key('foo') + bar = _Key('bar') + self.assertRaises(ValueError, self._callFUT, [foo, bar]) + + def test_w_match(self): + foo1 = _Key('foo') + foo2 = _Key('foo') + self.assertEqual(self._callFUT([foo1, foo2]), 'foo') + + class Test_get_function(unittest2.TestCase): def _callFUT(self, keys, missing=None, deferred=None, connection=None): @@ -552,3 +575,8 @@ def test_with_already_completed_key(self): COMPLETE_KEY = Key('KIND', 1234) self.assertRaises(ValueError, self._callFUT, COMPLETE_KEY, 2) + + +class _Key(object): + def __init__(self, dataset_id): + self.dataset_id = dataset_id From 8a6127e8fa36f154b9e521fd9ec2e1a490dc05f3 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 16:29:16 -0500 Subject: [PATCH 7/8] Use transaction explicitly, as it is not on the stack. See: https://github.com/GoogleCloudPlatform/gcloud-python/pull/548#discussion_r22967715 --- gcloud/datastore/transaction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcloud/datastore/transaction.py b/gcloud/datastore/transaction.py index 0a31b2a5fd29..4b0b93f8d0ba 100644 --- a/gcloud/datastore/transaction.py +++ b/gcloud/datastore/transaction.py @@ -89,7 +89,7 @@ class Transaction(Batch): >>> transaction.begin() >>> entity = Entity(key=Key('Thing')) - >>> datastore.put([entity]) + >>> transaction.put([entity]) >>> if error: ... transaction.rollback() From d46ed28b346b591ca48929a2ca09ef01b2c16a2c Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 14 Jan 2015 16:35:23 -0500 Subject: [PATCH 8/8] Squash lint errors. --- gcloud/datastore/entity.py | 2 +- gcloud/datastore/test_api.py | 25 ++++++++++++++----------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/gcloud/datastore/entity.py b/gcloud/datastore/entity.py index e2ab07628cef..08fc3b5edd9c 100644 --- a/gcloud/datastore/entity.py +++ b/gcloud/datastore/entity.py @@ -65,7 +65,7 @@ class Entity(dict): :type key: :class:`gcloud.datastore.key.Key` :param key: Optional key to be set on entity. Required for - :fund:`gcloud.datastore.put()` + :func:`gcloud.datastore.put()` :type exclude_from_indexes: tuple of string :param exclude_from_indexes: Names of fields whose values are not to be diff --git a/gcloud/datastore/test_api.py b/gcloud/datastore/test_api.py index e254342af64a..c1ddc1527907 100644 --- a/gcloud/datastore/test_api.py +++ b/gcloud/datastore/test_api.py @@ -96,6 +96,14 @@ def _callFUT(self, keys): from gcloud.datastore.api import _get_dataset_id_from_keys return _get_dataset_id_from_keys(keys) + def _make_key(self, dataset_id): + + class _Key(object): + def __init__(self, dataset_id): + self.dataset_id = dataset_id + + return _Key(dataset_id) + def test_empty(self): self.assertRaises(IndexError, self._callFUT, []) @@ -103,14 +111,14 @@ def test_w_None(self): self.assertRaises(ValueError, self._callFUT, [None]) def test_w_mismatch(self): - foo = _Key('foo') - bar = _Key('bar') - self.assertRaises(ValueError, self._callFUT, [foo, bar]) + key1 = self._make_key('foo') + key2 = self._make_key('bar') + self.assertRaises(ValueError, self._callFUT, [key1, key2]) def test_w_match(self): - foo1 = _Key('foo') - foo2 = _Key('foo') - self.assertEqual(self._callFUT([foo1, foo2]), 'foo') + key1 = self._make_key('foo') + key2 = self._make_key('foo') + self.assertEqual(self._callFUT([key1, key2]), 'foo') class Test_get_function(unittest2.TestCase): @@ -575,8 +583,3 @@ def test_with_already_completed_key(self): COMPLETE_KEY = Key('KIND', 1234) self.assertRaises(ValueError, self._callFUT, COMPLETE_KEY, 2) - - -class _Key(object): - def __init__(self, dataset_id): - self.dataset_id = dataset_id