From aa473fd84f518d68a3ff9a58a377c44f41a9781c Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 7 Jan 2016 10:33:30 -0800 Subject: [PATCH 1/2] Updating CommitRequest, Mutation and helpers for v1beta3. --- gcloud/datastore/batch.py | 18 ++++++++++------- gcloud/datastore/connection.py | 8 +++++--- gcloud/datastore/test_batch.py | 30 ++++++++++++++-------------- gcloud/datastore/test_client.py | 15 +++++++++----- gcloud/datastore/test_connection.py | 20 +++++++++---------- gcloud/datastore/test_transaction.py | 4 +++- gcloud/datastore/transaction.py | 2 +- 7 files changed, 55 insertions(+), 42 deletions(-) diff --git a/gcloud/datastore/batch.py b/gcloud/datastore/batch.py index de2cb021b39d..2e3a3d8b328f 100644 --- a/gcloud/datastore/batch.py +++ b/gcloud/datastore/batch.py @@ -119,7 +119,8 @@ def _add_partial_key_entity_pb(self): :returns: The newly created entity protobuf that will be updated and sent with a commit. """ - return self.mutations.insert_auto_id.add() + new_mutation = self.mutations.add() + return new_mutation.insert def _add_complete_key_entity_pb(self): """Adds a new mutation for an entity with a completed key. @@ -131,7 +132,8 @@ def _add_complete_key_entity_pb(self): # We use ``upsert`` for entities with completed keys, rather than # ``insert`` or ``update``, in order not to create race conditions # based on prior existence / removal of the entity. - return self.mutations.upsert.add() + new_mutation = self.mutations.add() + return new_mutation.upsert def _add_delete_key_pb(self): """Adds a new mutation for a key to be deleted. @@ -140,7 +142,8 @@ def _add_delete_key_pb(self): :returns: The newly created key protobuf that will be deleted when sent with a commit. """ - return self.mutations.delete.add() + new_mutation = self.mutations.add() + return new_mutation.delete @property def mutations(self): @@ -152,10 +155,11 @@ def mutations(self): adding a new mutation. This getter returns the protobuf that has been built-up so far. - :rtype: :class:`gcloud.datastore._generated.datastore_pb2.Mutation` - :returns: The Mutation protobuf to be sent in the commit request. + :rtype: iterable + :returns: The list of :class:`._generated.datastore_pb2.Mutation` + protobufs to be sent in the commit request. """ - return self._commit_request.mutation + return self._commit_request.mutations def put(self, entity): """Remember an entity's state to be saved during :meth:`commit`. @@ -172,7 +176,7 @@ def put(self, entity): "bytes" ('str' in Python2, 'bytes' in Python3) map to 'blob_value'. When an entity has a partial key, calling :meth:`commit` sends it as - an ``insert_auto_id`` mutation and the key is completed. On return, + an ``insert`` mutation and the key is completed. On return, the key for the ``entity`` passed in is updated to match the key ID assigned by the server. diff --git a/gcloud/datastore/connection.py b/gcloud/datastore/connection.py index deae2796d516..d9352f86d8d4 100644 --- a/gcloud/datastore/connection.py +++ b/gcloud/datastore/connection.py @@ -16,6 +16,7 @@ import os +from gcloud._helpers import _has_field from gcloud import connection from gcloud.environment_vars import GCD_HOST from gcloud.exceptions import make_exception @@ -431,7 +432,8 @@ def _parse_commit_response(commit_response_pb): :class:`._generated.entity_pb2.Key` for each incomplete key that was completed in the commit. """ - mut_result = commit_response_pb.mutation_result - index_updates = mut_result.index_updates - completed_keys = list(mut_result.insert_auto_id_key) + mut_results = commit_response_pb.mutation_results + index_updates = commit_response_pb.index_updates + completed_keys = [mut_result.key for mut_result in mut_results + if _has_field(mut_result, 'key')] return index_updates, completed_keys diff --git a/gcloud/datastore/test_batch.py b/gcloud/datastore/test_batch.py index a8d96370cce9..72869784fffa 100644 --- a/gcloud/datastore/test_batch.py +++ b/gcloud/datastore/test_batch.py @@ -38,7 +38,9 @@ def test_ctor(self): self.assertEqual(batch.namespace, _NAMESPACE) self.assertTrue(batch._id is None) self.assertEqual(batch._status, batch._INITIAL) - self.assertTrue(isinstance(batch.mutations, datastore_pb2.Mutation)) + self.assertTrue(isinstance(batch._commit_request, + datastore_pb2.CommitRequest)) + self.assertTrue(batch.mutations is batch._commit_request.mutations) self.assertEqual(batch._partial_key_entities, []) def test_current(self): @@ -90,7 +92,7 @@ def test_put_entity_w_partial_key(self): batch.put(entity) - mutated_entity = _mutated_pb(self, batch.mutations, 'insert_auto_id') + mutated_entity = _mutated_pb(self, batch.mutations, 'insert') self.assertEqual(mutated_entity.key, key._key) self.assertEqual(batch._partial_key_entities, [entity]) @@ -424,20 +426,18 @@ def current_batch(self): return self._batches[0] -def _assert_num_mutations(test_case, mutation_pb, num_mutations): - total_mutations = (len(mutation_pb.upsert) + - len(mutation_pb.update) + - len(mutation_pb.insert) + - len(mutation_pb.insert_auto_id) + - len(mutation_pb.delete)) - test_case.assertEqual(total_mutations, num_mutations) +def _assert_num_mutations(test_case, mutation_pb_list, num_mutations): + test_case.assertEqual(len(mutation_pb_list), num_mutations) -def _mutated_pb(test_case, mutation_pb, mutation_type): +def _mutated_pb(test_case, mutation_pb_list, mutation_type): # Make sure there is only one mutation. - _assert_num_mutations(test_case, mutation_pb, 1) + _assert_num_mutations(test_case, mutation_pb_list, 1) - mutated_pbs = getattr(mutation_pb, mutation_type, []) - # Make sure we have exactly one protobuf. - test_case.assertEqual(len(mutated_pbs), 1) - return mutated_pbs[0] + # We grab the only mutation. + mutated_pb = mutation_pb_list[0] + # Then check if it is the correct type. + test_case.assertEqual(mutated_pb.WhichOneof('operation'), + mutation_type) + + return getattr(mutated_pb, mutation_type) diff --git a/gcloud/datastore/test_client.py b/gcloud/datastore/test_client.py index 9908816e3644..0b698d84f7dd 100644 --- a/gcloud/datastore/test_client.py +++ b/gcloud/datastore/test_client.py @@ -612,6 +612,7 @@ def test_put_multi_no_batch_w_partial_key(self): from gcloud.datastore.test_batch import _Entity from gcloud.datastore.test_batch import _Key from gcloud.datastore.test_batch import _KeyPB + from gcloud.datastore.test_batch import _mutated_pb entity = _Entity(foo=u'bar') key = entity.key = _Key(self.PROJECT) @@ -628,15 +629,16 @@ def test_put_multi_no_batch_w_partial_key(self): (project, commit_req, transaction_id) = client.connection._commit_cw[0] self.assertEqual(project, self.PROJECT) - inserts = list(commit_req.mutation.insert_auto_id) - self.assertEqual(len(inserts), 1) - self.assertEqual(inserts[0].key, key.to_protobuf()) - prop_list = list(_property_tuples(inserts[0])) + mutated_entity = _mutated_pb(self, commit_req.mutations, 'insert') + self.assertEqual(mutated_entity.key, key.to_protobuf()) + + prop_list = list(_property_tuples(mutated_entity)) self.assertTrue(len(prop_list), 1) name, value_pb = prop_list[0] self.assertEqual(name, 'foo') self.assertEqual(value_pb.string_value, u'bar') + self.assertTrue(transaction_id is None) def test_put_multi_existing_batch_w_completed_key(self): @@ -688,6 +690,7 @@ def test_delete_multi_no_keys(self): def test_delete_multi_no_batch(self): from gcloud.datastore.test_batch import _Key + from gcloud.datastore.test_batch import _mutated_pb key = _Key(self.PROJECT) @@ -701,7 +704,9 @@ def test_delete_multi_no_batch(self): (project, commit_req, transaction_id) = client.connection._commit_cw[0] self.assertEqual(project, self.PROJECT) - self.assertEqual(list(commit_req.mutation.delete), [key.to_protobuf()]) + + mutated_key = _mutated_pb(self, commit_req_pb.mutations, 'delete') + self.assertEqual(mutated_key, key.to_protobuf()) self.assertTrue(transaction_id is None) def test_delete_multi_w_existing_batch(self): diff --git a/gcloud/datastore/test_connection.py b/gcloud/datastore/test_connection.py index f50682b87301..a209b4a8b22c 100644 --- a/gcloud/datastore/test_connection.py +++ b/gcloud/datastore/test_connection.py @@ -643,8 +643,8 @@ def test_commit_wo_transaction(self): key_pb = self._make_key_pb(PROJECT) rsp_pb = datastore_pb2.CommitResponse() req_pb = datastore_pb2.CommitRequest() - mutation = req_pb.mutation - insert = mutation.upsert.add() + mutation = req_pb.mutations.add() + insert = mutation.upsert insert.key.CopyFrom(key_pb) value_pb = _new_value_pb(insert, 'foo') value_pb.string_value = u'Foo' @@ -675,7 +675,7 @@ def mock_parse(response): request = rq_class() request.ParseFromString(cw['body']) self.assertEqual(request.transaction, b'') - self.assertEqual(request.mutation, mutation) + self.assertEqual(list(request.mutations), [mutation]) self.assertEqual(request.mode, rq_class.NON_TRANSACTIONAL) self.assertEqual(_parsed, [rsp_pb]) @@ -689,8 +689,8 @@ def test_commit_w_transaction(self): key_pb = self._make_key_pb(PROJECT) rsp_pb = datastore_pb2.CommitResponse() req_pb = datastore_pb2.CommitRequest() - mutation = req_pb.mutation - insert = mutation.upsert.add() + mutation = req_pb.mutations.add() + insert = mutation.upsert insert.key.CopyFrom(key_pb) value_pb = _new_value_pb(insert, 'foo') value_pb.string_value = u'Foo' @@ -721,7 +721,7 @@ def mock_parse(response): request = rq_class() request.ParseFromString(cw['body']) self.assertEqual(request.transaction, b'xact') - self.assertEqual(request.mutation, mutation) + self.assertEqual(list(request.mutations), [mutation]) self.assertEqual(request.mode, rq_class.TRANSACTIONAL) self.assertEqual(_parsed, [rsp_pb]) @@ -833,10 +833,10 @@ def test_it(self): ), ] response = datastore_pb2.CommitResponse( - mutation_result=datastore_pb2.MutationResult( - index_updates=index_updates, - insert_auto_id_key=keys, - ), + mutation_results=[ + datastore_pb2.MutationResult(key=key) for key in keys + ], + index_updates=index_updates, ) result = self._callFUT(response) self.assertEqual(result, (index_updates, keys)) diff --git a/gcloud/datastore/test_transaction.py b/gcloud/datastore/test_transaction.py index d7fb9bc9096b..5f780f63388a 100644 --- a/gcloud/datastore/test_transaction.py +++ b/gcloud/datastore/test_transaction.py @@ -35,7 +35,9 @@ def test_ctor_defaults(self): self.assertEqual(xact.connection, connection) self.assertEqual(xact.id, None) self.assertEqual(xact._status, self._getTargetClass()._INITIAL) - self.assertTrue(isinstance(xact.mutations, datastore_pb2.Mutation)) + self.assertTrue(isinstance(xact._commit_request, + datastore_pb2.CommitRequest)) + self.assertTrue(xact.mutations is xact._commit_request.mutations) self.assertEqual(len(xact._partial_key_entities), 0) def test_current(self): diff --git a/gcloud/datastore/transaction.py b/gcloud/datastore/transaction.py index 25c92729e9ca..41397f354f11 100644 --- a/gcloud/datastore/transaction.py +++ b/gcloud/datastore/transaction.py @@ -24,7 +24,7 @@ class Transaction(Batch): or none succeed (transactionally). For example, the following snippet of code will put the two ``save`` - operations (either ``insert_auto_id`` or ``upsert``) into the same + operations (either ``insert`` or ``upsert``) into the same mutation, and execute those within a transaction:: >>> from gcloud import datastore From 97d6e87bb325f6fcb01713c50f1c5d64a765cf81 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Fri, 12 Feb 2016 16:38:20 -0800 Subject: [PATCH 2/2] Follow-on commit to clean-up out-of-sync code. The sync-failure was caused by breaking up the v1beta3 changes into distinct and small changes. --- gcloud/datastore/connection.py | 3 +-- gcloud/datastore/helpers.py | 1 - gcloud/datastore/test_batch.py | 14 ++++++++------ gcloud/datastore/test_client.py | 2 +- gcloud/datastore/test_helpers.py | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/gcloud/datastore/connection.py b/gcloud/datastore/connection.py index d9352f86d8d4..b3921f8b992f 100644 --- a/gcloud/datastore/connection.py +++ b/gcloud/datastore/connection.py @@ -16,7 +16,6 @@ import os -from gcloud._helpers import _has_field from gcloud import connection from gcloud.environment_vars import GCD_HOST from gcloud.exceptions import make_exception @@ -435,5 +434,5 @@ def _parse_commit_response(commit_response_pb): mut_results = commit_response_pb.mutation_results index_updates = commit_response_pb.index_updates completed_keys = [mut_result.key for mut_result in mut_results - if _has_field(mut_result, 'key')] + if mut_result.HasField('key')] # Message field (Key) return index_updates, completed_keys diff --git a/gcloud/datastore/helpers.py b/gcloud/datastore/helpers.py index 269e3e97645c..6ce3e08870e6 100644 --- a/gcloud/datastore/helpers.py +++ b/gcloud/datastore/helpers.py @@ -23,7 +23,6 @@ import six from gcloud._helpers import _datetime_to_pb_timestamp -from gcloud._helpers import _has_field from gcloud._helpers import _pb_timestamp_to_datetime from gcloud.datastore._generated import entity_pb2 as _entity_pb2 from gcloud.datastore.entity import Entity diff --git a/gcloud/datastore/test_batch.py b/gcloud/datastore/test_batch.py index 72869784fffa..18d79aef94d5 100644 --- a/gcloud/datastore/test_batch.py +++ b/gcloud/datastore/test_batch.py @@ -123,9 +123,10 @@ def test_put_entity_w_completed_key(self): self.assertFalse(prop_dict['foo'].exclude_from_indexes) self.assertTrue(prop_dict['baz'].exclude_from_indexes) self.assertFalse(prop_dict['spam'].exclude_from_indexes) - self.assertTrue(prop_dict['spam'].list_value[0].exclude_from_indexes) - self.assertTrue(prop_dict['spam'].list_value[1].exclude_from_indexes) - self.assertTrue(prop_dict['spam'].list_value[2].exclude_from_indexes) + spam_values = prop_dict['spam'].array_value.values + self.assertTrue(spam_values[0].exclude_from_indexes) + self.assertTrue(spam_values[1].exclude_from_indexes) + self.assertTrue(spam_values[2].exclude_from_indexes) self.assertFalse('frotz' in prop_dict) def test_put_entity_w_completed_key_prefixed_project(self): @@ -155,9 +156,10 @@ def test_put_entity_w_completed_key_prefixed_project(self): self.assertFalse(prop_dict['foo'].exclude_from_indexes) self.assertTrue(prop_dict['baz'].exclude_from_indexes) self.assertFalse(prop_dict['spam'].exclude_from_indexes) - self.assertTrue(prop_dict['spam'].list_value[0].exclude_from_indexes) - self.assertTrue(prop_dict['spam'].list_value[1].exclude_from_indexes) - self.assertTrue(prop_dict['spam'].list_value[2].exclude_from_indexes) + spam_values = prop_dict['spam'].array_value.values + self.assertTrue(spam_values[0].exclude_from_indexes) + self.assertTrue(spam_values[1].exclude_from_indexes) + self.assertTrue(spam_values[2].exclude_from_indexes) self.assertFalse('frotz' in prop_dict) def test_delete_w_partial_key(self): diff --git a/gcloud/datastore/test_client.py b/gcloud/datastore/test_client.py index 0b698d84f7dd..35547574c996 100644 --- a/gcloud/datastore/test_client.py +++ b/gcloud/datastore/test_client.py @@ -705,7 +705,7 @@ def test_delete_multi_no_batch(self): commit_req, transaction_id) = client.connection._commit_cw[0] self.assertEqual(project, self.PROJECT) - mutated_key = _mutated_pb(self, commit_req_pb.mutations, 'delete') + mutated_key = _mutated_pb(self, commit_req.mutations, 'delete') self.assertEqual(mutated_key, key.to_protobuf()) self.assertTrue(transaction_id is None) diff --git a/gcloud/datastore/test_helpers.py b/gcloud/datastore/test_helpers.py index 4fd61a036b18..d59f605d278f 100644 --- a/gcloud/datastore/test_helpers.py +++ b/gcloud/datastore/test_helpers.py @@ -309,7 +309,7 @@ def test_inverts_to_protobuf(self): array_val1.exclude_from_indexes = False array_val1.meaning = meaning = 22 array_val1.blob_value = b'\xe2\x98\x83' - array_val2 = val_pb4.array_value.add() + array_val2 = val_pb4.array_value.values.add() array_val2.exclude_from_indexes = False array_val2.meaning = meaning array_val2.blob_value = b'\xe2\x98\x85'