Skip to content

Commit a74ed9e

Browse files
tseavertseaver
authored
Merge pull request googleapis#1861 from tseaver/1859-datastore-imperative_txn_miss
Add support for serialized/transactional read in 'Client.get{,_multi}'.
2 parents 835d2dc + 357951b commit a74ed9e

3 files changed

Lines changed: 86 additions & 6 deletions

File tree

‎gcloud/datastore/client.py‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ def current_transaction(self):
223223
if isinstance(transaction, Transaction):
224224
return transaction
225225

226-
def get(self, key, missing=None, deferred=None):
226+
def get(self, key, missing=None, deferred=None, transaction=None):
227227
"""Retrieve an entity from a single key (if it exists).
228228
229229
.. note::
@@ -244,15 +244,19 @@ def get(self, key, missing=None, deferred=None):
244244
:param deferred: (Optional) If a list is passed, the keys returned
245245
by the backend as "deferred" will be copied into it.
246246
247+
:type transaction: :class:`gcloud.datastore.transaction.Transaction`
248+
:param transaction: (Optional) Transaction to use for read consistency.
249+
If not passed, uses current transaction, if set.
250+
247251
:rtype: :class:`gcloud.datastore.entity.Entity` or ``NoneType``
248252
:returns: The requested entity if it exists.
249253
"""
250254
entities = self.get_multi(keys=[key], missing=missing,
251-
deferred=deferred)
255+
deferred=deferred, transaction=transaction)
252256
if entities:
253257
return entities[0]
254258

255-
def get_multi(self, keys, missing=None, deferred=None):
259+
def get_multi(self, keys, missing=None, deferred=None, transaction=None):
256260
"""Retrieve entities, along with their attributes.
257261
258262
:type keys: list of :class:`gcloud.datastore.key.Key`
@@ -268,6 +272,10 @@ def get_multi(self, keys, missing=None, deferred=None):
268272
by the backend as "deferred" will be copied into it.
269273
If the list is not empty, an error will occur.
270274
275+
:type transaction: :class:`gcloud.datastore.transaction.Transaction`
276+
:param transaction: (Optional) Transaction to use for read consistency.
277+
If not passed, uses current transaction, if set.
278+
271279
:rtype: list of :class:`gcloud.datastore.entity.Entity`
272280
:returns: The requested entities.
273281
:raises: :class:`ValueError` if one or more of ``keys`` has a project
@@ -281,7 +289,8 @@ def get_multi(self, keys, missing=None, deferred=None):
281289
if current_id != self.project:
282290
raise ValueError('Keys do not match project')
283291

284-
transaction = self.current_transaction
292+
if transaction is None:
293+
transaction = self.current_transaction
285294

286295
entity_pbs = _extended_lookup(
287296
connection=self.connection,

‎gcloud/datastore/test_client.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,10 @@ def _get_multi(*args, **kw):
231231
self.assertEqual(_called_with[0][1]['keys'], [key])
232232
self.assertTrue(_called_with[0][1]['missing'] is None)
233233
self.assertTrue(_called_with[0][1]['deferred'] is None)
234+
self.assertTrue(_called_with[0][1]['transaction'] is None)
234235

235236
def test_get_hit(self):
237+
TXN_ID = '123'
236238
_called_with = []
237239
_entity = object()
238240

@@ -246,12 +248,13 @@ def _get_multi(*args, **kw):
246248

247249
key, missing, deferred = object(), [], []
248250

249-
self.assertTrue(client.get(key, missing, deferred) is _entity)
251+
self.assertTrue(client.get(key, missing, deferred, TXN_ID) is _entity)
250252

251253
self.assertEqual(_called_with[0][0], ())
252254
self.assertEqual(_called_with[0][1]['keys'], [key])
253255
self.assertTrue(_called_with[0][1]['missing'] is missing)
254256
self.assertTrue(_called_with[0][1]['deferred'] is deferred)
257+
self.assertEqual(_called_with[0][1]['transaction'], TXN_ID)
255258

256259
def test_get_multi_no_keys(self):
257260
creds = object()
@@ -412,6 +415,40 @@ def test_get_multi_hit(self):
412415
self.assertEqual(list(result), ['foo'])
413416
self.assertEqual(result['foo'], 'Foo')
414417

418+
def test_get_multi_hit_w_transaction(self):
419+
from gcloud.datastore.key import Key
420+
421+
TXN_ID = '123'
422+
KIND = 'Kind'
423+
ID = 1234
424+
PATH = [{'kind': KIND, 'id': ID}]
425+
426+
# Make a found entity pb to be returned from mock backend.
427+
entity_pb = _make_entity_pb(self.PROJECT, KIND, ID, 'foo', 'Foo')
428+
429+
# Make a connection to return the entity pb.
430+
creds = object()
431+
client = self._makeOne(credentials=creds)
432+
client.connection._add_lookup_result([entity_pb])
433+
434+
key = Key(KIND, ID, project=self.PROJECT)
435+
txn = client.transaction()
436+
txn._id = TXN_ID
437+
result, = client.get_multi([key], transaction=txn)
438+
new_key = result.key
439+
440+
# Check the returned value is as expected.
441+
self.assertFalse(new_key is key)
442+
self.assertEqual(new_key.project, self.PROJECT)
443+
self.assertEqual(new_key.path, PATH)
444+
self.assertEqual(list(result), ['foo'])
445+
self.assertEqual(result['foo'], 'Foo')
446+
447+
cw = client.connection._lookup_cw
448+
self.assertEqual(len(cw), 1)
449+
_, _, _, transaction_id = cw[0]
450+
self.assertEqual(transaction_id, TXN_ID)
451+
415452
def test_get_multi_hit_multiple_keys_same_project(self):
416453
from gcloud.datastore.key import Key
417454

‎system_tests/datastore.py‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ def test_query_distinct_on(self):
417417

418418
class TestDatastoreTransaction(TestDatastore):
419419

420-
def test_transaction(self):
420+
def test_transaction_via_with_statement(self):
421421
entity = datastore.Entity(key=Config.CLIENT.key('Company', 'Google'))
422422
entity['url'] = u'www.google.com'
423423

@@ -432,6 +432,40 @@ def test_transaction(self):
432432
self.case_entities_to_delete.append(retrieved_entity)
433433
self.assertEqual(retrieved_entity, entity)
434434

435+
def test_transaction_via_explicit_begin_get_commit(self):
436+
# See https://github.com/GoogleCloudPlatform/gcloud-python/issues/1859
437+
# Note that this example lacks the threading which provokes the race
438+
# condition in that issue: we are basically just exercising the
439+
# "explict" path for using transactions.
440+
BEFORE_1 = 100
441+
BEFORE_2 = 0
442+
TRANSFER_AMOUNT = 40
443+
key1 = Config.CLIENT.key('account', '123')
444+
account1 = datastore.Entity(key=key1)
445+
account1['balance'] = BEFORE_1
446+
key2 = Config.CLIENT.key('account', '234')
447+
account2 = datastore.Entity(key=key2)
448+
account2['balance'] = BEFORE_2
449+
Config.CLIENT.put_multi([account1, account2])
450+
self.case_entities_to_delete.append(account1)
451+
self.case_entities_to_delete.append(account2)
452+
453+
xact = Config.CLIENT.transaction()
454+
xact.begin()
455+
from_account = Config.CLIENT.get(key1, transaction=xact)
456+
to_account = Config.CLIENT.get(key2, transaction=xact)
457+
from_account['balance'] -= TRANSFER_AMOUNT
458+
to_account['balance'] += TRANSFER_AMOUNT
459+
460+
xact.put(from_account)
461+
xact.put(to_account)
462+
xact.commit()
463+
464+
after1 = Config.CLIENT.get(key1)
465+
after2 = Config.CLIENT.get(key2)
466+
self.assertEqual(after1['balance'], BEFORE_1 - TRANSFER_AMOUNT)
467+
self.assertEqual(after2['balance'], BEFORE_2 + TRANSFER_AMOUNT)
468+
435469
def test_failure_with_contention(self):
436470
contention_prop_name = 'baz'
437471
local_client = clone_client(Config.CLIENT)

0 commit comments

Comments
 (0)