Skip to content

Require that keys for 'put' / 'delete' match the 'dataset_id' of the batch #552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tseaver merged 11 commits into from
Jan 28, 2015
Merged

Require that keys for 'put' / 'delete' match the 'dataset_id' of the batch #552

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5aa16a5
Require that keys for 'put' / 'delete' match the 'dataset_id' of the …
tseaver Jan 15, 2015
24b9b8e
Remove '_get_dataset_id_from_keys'.
tseaver Jan 15, 2015
9bb4a3f
Work around the fact that keys updated from the server have prefixes.
tseaver Jan 15, 2015
25aae75
Infer connection / dataset ID from current batch...
tseaver Jan 15, 2015
bccf8fc
Add explicit 'dataset_id' to 'get'/'put'/'delete'.
tseaver Jan 15, 2015
258a2cd
Re-allow use of dataset ID from keys as a fallback.
tseaver Jan 15, 2015
d5d6400
Re-include keys when searching for a dataset ID.
tseaver Jan 27, 2015
d198d69
Fix typos.
tseaver Jan 27, 2015
db4c728
Rework '_require_dataset_id' to avoid making caller pass all keys.
tseaver Jan 27, 2015
a6cea31
Land '_dataset_ids_equal' in helpers.
tseaver Jan 28, 2015
49977da
Use 'helpers._dataset_ids_equal'.
tseaver Jan 28, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Work around the fact that keys updated from the server have prefixes. 
See #528.
  • Loading branch information
@tseaver
tseaver committed Jan 27, 2015
commit 9bb4a3f45a39022a97cae7809c46fc63cf0b8c2a
22 changes: 18 additions & 4 deletions gcloud/datastore/batch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,22 @@ def add_auto_id_entity(self, entity):

self._auto_id_entities.append(entity)

def _match_dataset_id(self, other_id):

This comment was marked as spam.

Sign in to view

"""Ensure that `other_id` matches our `dataset_id`.

Helper for :meth:`put` and :meth:`delete`.

:type other_id: string
:param other_id: the dataset ID to compare

:raises: ValueError if `other_id` does not match (even after stripping
any prefix).
"""
other_id = other_id.rsplit('~', 1)[-1]
our_id = self._dataset_id.rsplit('~', 1)[-1]
if other_id != our_id:
raise ValueError("Key must be from same dataset as batch")

def put(self, entity):
"""Remember an entity's state to be saved during ``commit``.

Expand All @@ -216,8 +232,7 @@ def put(self, entity):
if entity.key is None:
raise ValueError("Entity must have a key")

if entity.key.dataset_id != self._dataset_id:
raise ValueError("Key must be from same dataset as batch")
self._match_dataset_id(entity.key.dataset_id)

_assign_entity_to_mutation(
self.mutation, entity, self._auto_id_entities)
Expand All @@ -234,8 +249,7 @@ def delete(self, key):
if key.is_partial:
raise ValueError("Key must be complete")

if key.dataset_id != self._dataset_id:
raise ValueError("Key must be from same dataset as batch")
self._match_dataset_id(key.dataset_id)

key_pb = key.to_protobuf()
helpers._add_keys_to_request(self.mutation.delete, [key_pb])
Expand Down
51 changes: 51 additions & 0 deletions gcloud/datastore/test_batch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,41 @@ def test_put_entity_w_completed_key(self):
deletes = list(batch.mutation.delete)
self.assertEqual(len(deletes), 0)

def test_put_entity_w_completed_key_prefixed_dataset_id(self):
_DATASET = 'DATASET'
_PROPERTIES = {
'foo': 'bar',
'baz': 'qux',
'spam': [1, 2, 3],
'frotz': [], # will be ignored
}
connection = _Connection()
batch = self._makeOne(dataset_id=_DATASET, connection=connection)
entity = _Entity(_PROPERTIES)
entity.exclude_from_indexes = ('baz', 'spam')
key = entity.key = _Key('s~' + _DATASET)

batch.put(entity)

insert_auto_ids = list(batch.mutation.insert_auto_id)
self.assertEqual(len(insert_auto_ids), 0)
upserts = list(batch.mutation.upsert)
self.assertEqual(len(upserts), 1)

upsert = upserts[0]
self.assertEqual(upsert.key, key._key)
props = dict([(prop.name, prop.value) for prop in upsert.property])
self.assertTrue(props['foo'].indexed)
self.assertFalse(props['baz'].indexed)
self.assertTrue(props['spam'].indexed)
self.assertFalse(props['spam'].list_value[0].indexed)
self.assertFalse(props['spam'].list_value[1].indexed)
self.assertFalse(props['spam'].list_value[2].indexed)
self.assertFalse('frotz' in props)

deletes = list(batch.mutation.delete)
self.assertEqual(len(deletes), 0)

def test_delete_w_partial_key(self):
_DATASET = 'DATASET'
connection = _Connection()
Expand Down Expand Up @@ -236,6 +271,22 @@ def test_delete_w_completed_key(self):
self.assertEqual(len(deletes), 1)
self.assertEqual(deletes[0], key._key)

def test_delete_w_completed_key_w_prefixed_dataset_id(self):
_DATASET = 'DATASET'
connection = _Connection()
batch = self._makeOne(dataset_id=_DATASET, connection=connection)
key = _Key('s~' + _DATASET)

batch.delete(key)

insert_auto_ids = list(batch.mutation.insert_auto_id)
self.assertEqual(len(insert_auto_ids), 0)
upserts = list(batch.mutation.upsert)
self.assertEqual(len(upserts), 0)
deletes = list(batch.mutation.delete)
self.assertEqual(len(deletes), 1)
self.assertEqual(deletes[0], key._key)

def test_commit(self):
_DATASET = 'DATASET'
connection = _Connection()
Expand Down