Skip to content

Commit

Permalink
fix empty list issue #403 by add a _set_protobuf_property helper fun…
Browse files Browse the repository at this point in the history
…ction.
  • Loading branch information
lucemia committed Jan 12, 2015
1 parent 6bde37d commit a250cb2
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 13 deletions.
19 changes: 6 additions & 13 deletions gcloud/datastore/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,10 @@ def save_entity(self, dataset_id, key_pb, properties,
will be replaced by those passed in ``properties``; properties
not passed in ``properties`` no longer be set for the entity.
.. note::
When saving an entity to the backend, a property value set as
an empty list cannot be saved and will be ignored.
:type dataset_id: string
:param dataset_id: The ID of the dataset in which to save the entity.
Expand Down Expand Up @@ -463,19 +467,8 @@ def save_entity(self, dataset_id, key_pb, properties,
insert.key.CopyFrom(key_pb)

for name, value in properties.items():
prop = insert.property.add()
# Set the name of the property.
prop.name = name

# Set the appropriate value.
helpers._set_protobuf_value(prop.value, value)

if name in exclude_from_indexes:
if not isinstance(value, list):
prop.value.indexed = False

for sub_value in prop.value.list_value:
sub_value.indexed = False
helpers._set_protobuf_property(insert.property, name, value,
name not in exclude_from_indexes)

# If this is in a transaction, we should just return True. The
# transaction will handle assigning any keys as necessary.
Expand Down
38 changes: 38 additions & 0 deletions gcloud/datastore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,44 @@ def _get_value_from_property_pb(property_pb):
return _get_value_from_value_pb(property_pb.value)


def _set_protobuf_property(property_pb, name, value, indexed):
"""Assign 'name', 'value', 'indexed' to the correct 'property_pb'.
Some value (empty list) cannot be directly assigned; this function handles
them correctly.
:type property_pb: :class:`gcloud.datastore.datastore_v1_pb2.Property`
:param property_pb: The value protobuf to which the value is being
assigned.
:type name: string
:param name: The name to be assigned.
:type value: `datetime.datetime`, boolean, float, integer, string,
:class:`gcloud.datastore.key.Key`,
:class:`gcloud.datastore.entity.Entity`,
:param value: The value to be assigned.
:type indexed: boolean
:param indexed: The flag indicates the property should to be indexed or
not.
"""
if isinstance(value, list) and len(value) == 0:
return

prop = property_pb.add()
prop.name = name

_set_protobuf_value(prop.value, value)

if not indexed:
if not isinstance(value, list):
prop.value.indexed = False

for sub_value in prop.value.list_value:
sub_value.indexed = False


def _set_protobuf_value(value_pb, val):
"""Assign 'val' to the correct subfield of 'value_pb'.
Expand Down
39 changes: 39 additions & 0 deletions gcloud/datastore/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,45 @@ def test_allocate_ids_non_empty(self):
for key_before, key_after in zip(before_key_pbs, request.key):
_compare_key_pb_after_request(self, key_before, key_after)

def test_save_entity_w_empty_list_value(self):
from gcloud.datastore import datastore_v1_pb2 as datastore_pb

DATASET_ID = 'DATASET'
key_pb = self._make_key_pb(DATASET_ID)
rsp_pb = datastore_pb.CommitResponse()
conn = self._makeOne()
URI = '/'.join([
conn.API_BASE_URL,
'datastore',
conn.API_VERSION,
'datasets',
DATASET_ID,
'commit',
])
http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
result = conn.save_entity(DATASET_ID, key_pb,
{'foo': u'Foo', 'bar': []})
self.assertEqual(result, (False, None))
cw = http._called_with
self._verifyProtobufCall(cw, URI, conn)
rq_class = datastore_pb.CommitRequest
request = rq_class()
request.ParseFromString(cw['body'])
self.assertEqual(request.transaction, '')
mutation = request.mutation
self.assertEqual(len(mutation.insert_auto_id), 0)
upserts = list(mutation.upsert)
self.assertEqual(len(upserts), 1)
upsert = upserts[0]
_compare_key_pb_after_request(self, key_pb, upsert.key)
props = list(upsert.property)
self.assertEqual(len(props), 1)
self.assertEqual(props[0].name, 'foo')
self.assertEqual(props[0].value.string_value, u'Foo')
self.assertEqual(props[0].value.indexed, True)
self.assertEqual(len(mutation.delete), 0)
self.assertEqual(request.mode, rq_class.NON_TRANSACTIONAL)

def test_save_entity_wo_transaction_w_upsert(self):
from gcloud.datastore import datastore_v1_pb2 as datastore_pb

Expand Down

0 comments on commit a250cb2

Please sign in to comment.