Skip to content

Commit a257a9c

Browse files
dhermesdhermes
committed
Removing Key.compare_to_proto and using just the saved ID.
Making Connection.save_entity return the auto allocated ID instead of the entire PB.
1 parent cc59310 commit a257a9c

6 files changed

Lines changed: 24 additions & 183 deletions

File tree

‎gcloud/datastore/connection.py‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,11 @@ def save_entity(self, dataset_id, key_pb, properties,
444444
445445
:type exclude_from_indexes: sequence of str
446446
:param exclude_from_indexes: Names of properties *not* to be indexed.
447+
448+
:rtype: :class:`tuple`
449+
:returns: The pair (`assigned`, `new_id`) where `assigned` is a boolean
450+
indicating if a new ID has been assigned and `new_id` is
451+
either `None` or an integer that has been assigned.
447452
"""
448453
mutation = self.mutation()
449454

@@ -477,14 +482,18 @@ def save_entity(self, dataset_id, key_pb, properties,
477482
# If this is in a transaction, we should just return True. The
478483
# transaction will handle assigning any keys as necessary.
479484
if self.transaction():
480-
return True
485+
return False, None
481486

482487
result = self.commit(dataset_id, mutation)
483-
# If this was an auto-assigned ID, return the new Key.
488+
# If this was an auto-assigned ID, return the new Key. We don't
489+
# verify that this matches the original `key_pb` but trust the
490+
# backend to uphold the values sent (e.g. dataset ID).
484491
if auto_id:
485-
return result.insert_auto_id_key[0]
492+
inserted_key_pb = result.insert_auto_id_key[0]
493+
# Assumes the backend has set `id` without checking HasField('id').
494+
return True, inserted_key_pb.path_element[-1].id
486495

487-
return True
496+
return False, None
488497

489498
def delete_entities(self, dataset_id, key_pbs):
490499
"""Delete keys from a dataset in the Cloud Datastore.

‎gcloud/datastore/entity.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
"""Class for representing a single entity in the Cloud Datastore."""
1616

1717
from gcloud.datastore import _implicit_environ
18-
from gcloud.datastore import datastore_v1_pb2 as datastore_pb
1918
from gcloud.datastore.key import Key
2019

2120

@@ -241,7 +240,7 @@ def save(self):
241240
key = self._must_key
242241
dataset = self._must_dataset
243242
connection = dataset.connection()
244-
key_pb = connection.save_entity(
243+
assigned, new_id = connection.save_entity(
245244
dataset_id=dataset.id(),
246245
key_pb=key.to_protobuf(),
247246
properties=dict(self),
@@ -253,9 +252,9 @@ def save(self):
253252
if transaction and key.is_partial:
254253
transaction.add_auto_id_entity(self)
255254

256-
if isinstance(key_pb, datastore_pb.Key):
255+
if assigned:
257256
# Update the key (which may have been altered).
258-
self.key(self.key().compare_to_proto(key_pb))
257+
self.key(self.key().completed_key(new_id))
259258

260259
return self
261260

‎gcloud/datastore/key.py‎

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -153,92 +153,6 @@ def completed_key(self, id_or_name):
153153
new_key._flat_path += (id_or_name,)
154154
return new_key
155155

156-
def _validate_protobuf_dataset_id(self, protobuf):
157-
"""Checks that dataset ID on protobuf matches current one.
158-
159-
The value of the dataset ID may have changed from unprefixed
160-
(e.g. 'foo') to prefixed (e.g. 's~foo' or 'e~foo').
161-
162-
:type protobuf: :class:`gcloud.datastore.datastore_v1_pb2.Key`
163-
:param protobuf: A protobuf representation of the key. Expected to be
164-
returned after a datastore operation.
165-
166-
:rtype: :class:`str`
167-
"""
168-
proto_dataset_id = protobuf.partition_id.dataset_id
169-
if proto_dataset_id == self.dataset_id:
170-
return
171-
172-
# Since they don't match, we check to see if `proto_dataset_id` has a
173-
# prefix.
174-
unprefixed = None
175-
prefix = proto_dataset_id[:2]
176-
if prefix in ('s~', 'e~'):
177-
unprefixed = proto_dataset_id[2:]
178-
179-
if unprefixed != self.dataset_id:
180-
raise ValueError('Dataset ID on protobuf does not match.',
181-
proto_dataset_id, self.dataset_id)
182-
183-
def compare_to_proto(self, protobuf):
184-
"""Checks current key against a protobuf; updates if partial.
185-
186-
If the current key is partial, returns a new key that has been
187-
completed otherwise returns the current key.
188-
189-
The value of the dataset ID may have changed from implicit (i.e. None,
190-
with the ID implied from the dataset.Dataset object associated with the
191-
Entity/Key), but if it was implicit before, we leave it as implicit.
192-
193-
:type protobuf: :class:`gcloud.datastore.datastore_v1_pb2.Key`
194-
:param protobuf: A protobuf representation of the key. Expected to be
195-
returned after a datastore operation.
196-
197-
:rtype: :class:`gcloud.datastore.key.Key`
198-
:returns: The current key if not partial.
199-
:raises: `ValueError` if the namespace or dataset ID of `protobuf`
200-
don't match the current values or if the path from `protobuf`
201-
doesn't match.
202-
"""
203-
if self.namespace is None:
204-
if protobuf.partition_id.HasField('namespace'):
205-
raise ValueError('Namespace unset on key but set on protobuf.')
206-
elif protobuf.partition_id.namespace != self.namespace:
207-
raise ValueError('Namespace on protobuf does not match.',
208-
protobuf.partition_id.namespace, self.namespace)
209-
210-
# Check that dataset IDs match if not implicit.
211-
if self.dataset_id is not None:
212-
self._validate_protobuf_dataset_id(protobuf)
213-
214-
path = []
215-
for element in protobuf.path_element:
216-
key_part = {}
217-
for descriptor, value in element._fields.items():
218-
key_part[descriptor.name] = value
219-
path.append(key_part)
220-
221-
if path == self.path:
222-
return self
223-
224-
if not self.is_partial:
225-
raise ValueError('Proto path does not match completed key.',
226-
path, self.path)
227-
228-
last_part = path[-1]
229-
id_or_name = None
230-
if 'id' in last_part:
231-
id_or_name = last_part.pop('id')
232-
elif 'name' in last_part:
233-
id_or_name = last_part.pop('name')
234-
235-
# We have edited path by popping from the last part, so check again.
236-
if path != self.path:
237-
raise ValueError('Proto path does not match partial key.',
238-
path, self.path)
239-
240-
return self.complete_key(id_or_name)
241-
242156
def to_protobuf(self):
243157
"""Return a protobuf corresponding to the key.
244158

‎gcloud/datastore/test_connection.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,7 @@ def test_save_entity_wo_transaction_w_upsert(self):
926926
])
927927
http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
928928
result = conn.save_entity(DATASET_ID, key_pb, {'foo': u'Foo'})
929-
self.assertEqual(result, True)
929+
self.assertEqual(result, (False, None))
930930
cw = http._called_with
931931
self._verifyProtobufCall(cw, URI, conn)
932932
rq_class = datastore_pb.CommitRequest
@@ -967,7 +967,7 @@ def test_save_entity_w_exclude_from_indexes(self):
967967
result = conn.save_entity(DATASET_ID, key_pb,
968968
{'foo': u'Foo', 'bar': [u'bar1', u'bar2']},
969969
exclude_from_indexes=['foo', 'bar'])
970-
self.assertEqual(result, True)
970+
self.assertEqual(result, (False, None))
971971
cw = http._called_with
972972
self._verifyProtobufCall(cw, URI, conn)
973973
rq_class = datastore_pb.CommitRequest
@@ -1018,7 +1018,7 @@ def test_save_entity_wo_transaction_w_auto_id(self):
10181018
])
10191019
http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
10201020
result = conn.save_entity(DATASET_ID, key_pb, {'foo': u'Foo'})
1021-
self.assertEqual(result, updated_key_pb)
1021+
self.assertEqual(result, (True, 1234))
10221022
cw = http._called_with
10231023
self._verifyProtobufCall(cw, URI, conn)
10241024
rq_class = datastore_pb.CommitRequest
@@ -1054,7 +1054,7 @@ def mutation(self):
10541054
conn.transaction(Xact())
10551055
http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
10561056
result = conn.save_entity(DATASET_ID, key_pb, {'foo': u'Foo'})
1057-
self.assertEqual(result, True)
1057+
self.assertEqual(result, (False, None))
10581058
self.assertEqual(http._called_with, None)
10591059
mutation = conn.mutation()
10601060
self.assertEqual(len(mutation.upsert), 1)
@@ -1077,7 +1077,7 @@ def mutation(self):
10771077
conn.transaction(Xact())
10781078
http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
10791079
result = conn.save_entity(DATASET_ID, key_pb, {'foo': nested})
1080-
self.assertEqual(result, True)
1080+
self.assertEqual(result, (False, None))
10811081
self.assertEqual(http._called_with, None)
10821082
mutation = conn.mutation()
10831083
self.assertEqual(len(mutation.upsert), 1)

‎gcloud/datastore/test_entity.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ def test_save_w_returned_key_exclude_from_indexes(self):
194194
key_pb.partition_id.dataset_id = _DATASET_ID
195195
key_pb.path_element.add(kind=_KIND, id=_ID)
196196
connection = _Connection()
197-
connection._save_result = key_pb
197+
connection._save_result = (True, _ID)
198198
dataset = _Dataset(connection)
199199
key = Key('KIND', dataset_id='DATASET')
200200
entity = self._makeOne(dataset, exclude_from_indexes=['foo'])
@@ -287,12 +287,12 @@ def get_entities(self, keys):
287287
return [self.get(key) for key in keys]
288288

289289
def allocate_ids(self, incomplete_key, num_ids):
290-
return [incomplete_key.complete_key(i + 1) for i in range(num_ids)]
290+
return [incomplete_key.completed_key(i + 1) for i in range(num_ids)]
291291

292292

293293
class _Connection(object):
294294
_transaction = _saved = _deleted = None
295-
_save_result = True
295+
_save_result = (False, None)
296296

297297
def transaction(self):
298298
return self._transaction

‎gcloud/datastore/test_key.py‎

Lines changed: 0 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -86,87 +86,6 @@ def test_completed_key_on_complete(self):
8686
key = self._makeOne('KIND', 1234)
8787
self.assertRaises(ValueError, key.completed_key, 5678)
8888

89-
def test_compare_to_proto_incomplete_w_id(self):
90-
_ID = 1234
91-
key = self._makeOne('KIND')
92-
pb = key.to_protobuf()
93-
pb.path_element[0].id = _ID
94-
new_key = key.compare_to_proto(pb)
95-
self.assertFalse(new_key is key)
96-
self.assertEqual(new_key.id, _ID)
97-
self.assertEqual(new_key.name, None)
98-
99-
def test_compare_to_proto_incomplete_w_name(self):
100-
_NAME = 'NAME'
101-
key = self._makeOne('KIND')
102-
pb = key.to_protobuf()
103-
pb.path_element[0].name = _NAME
104-
new_key = key.compare_to_proto(pb)
105-
self.assertFalse(new_key is key)
106-
self.assertEqual(new_key.id, None)
107-
self.assertEqual(new_key.name, _NAME)
108-
109-
def test_compare_to_proto_incomplete_w_incomplete(self):
110-
key = self._makeOne('KIND')
111-
pb = key.to_protobuf()
112-
new_key = key.compare_to_proto(pb)
113-
self.assertTrue(new_key is key)
114-
115-
def test_compare_to_proto_incomplete_w_bad_path(self):
116-
key = self._makeOne('KIND1', 1234, 'KIND2')
117-
pb = key.to_protobuf()
118-
pb.path_element[0].kind = 'NO_KIND'
119-
self.assertRaises(ValueError, key.compare_to_proto, pb)
120-
121-
def test_compare_to_proto_complete_w_id(self):
122-
key = self._makeOne('KIND', 1234)
123-
pb = key.to_protobuf()
124-
pb.path_element[0].id = 5678
125-
self.assertRaises(ValueError, key.compare_to_proto, pb)
126-
127-
def test_compare_to_proto_complete_w_name(self):
128-
key = self._makeOne('KIND', 1234)
129-
pb = key.to_protobuf()
130-
pb.path_element[0].name = 'NAME'
131-
self.assertRaises(ValueError, key.compare_to_proto, pb)
132-
133-
def test_compare_to_proto_complete_w_incomplete(self):
134-
key = self._makeOne('KIND', 1234)
135-
pb = key.to_protobuf()
136-
pb.path_element[0].ClearField('id')
137-
self.assertRaises(ValueError, key.compare_to_proto, pb)
138-
139-
def test_compare_to_proto_complete_diff_dataset(self):
140-
key = self._makeOne('KIND', 1234, dataset_id='DATASET')
141-
pb = key.to_protobuf()
142-
pb.partition_id.dataset_id = 's~' + key.dataset_id
143-
new_key = key.compare_to_proto(pb)
144-
self.assertTrue(new_key is key)
145-
146-
def test_compare_to_proto_complete_bad_dataset(self):
147-
key = self._makeOne('KIND', 1234, dataset_id='DATASET')
148-
pb = key.to_protobuf()
149-
pb.partition_id.dataset_id = 'BAD_PRE~' + key.dataset_id
150-
self.assertRaises(ValueError, key.compare_to_proto, pb)
151-
152-
def test_compare_to_proto_complete_valid_namespace(self):
153-
key = self._makeOne('KIND', 1234, namespace='NAMESPACE')
154-
pb = key.to_protobuf()
155-
new_key = key.compare_to_proto(pb)
156-
self.assertTrue(new_key is key)
157-
158-
def test_compare_to_proto_complete_namespace_unset_on_pb(self):
159-
key = self._makeOne('KIND', 1234, namespace='NAMESPACE')
160-
pb = key.to_protobuf()
161-
pb.partition_id.ClearField('namespace')
162-
self.assertRaises(ValueError, key.compare_to_proto, pb)
163-
164-
def test_compare_to_proto_complete_namespace_unset_on_key(self):
165-
key = self._makeOne('KIND', 1234)
166-
pb = key.to_protobuf()
167-
pb.partition_id.namespace = 'NAMESPACE'
168-
self.assertRaises(ValueError, key.compare_to_proto, pb)
169-
17089
def test_to_protobuf_defaults(self):
17190
from gcloud.datastore.datastore_v1_pb2 import Key as KeyPB
17291
_KIND = 'KIND'

0 commit comments

Comments
 (0)