From 8bf8bad82fd52a695f3bf334d7e13429cb302bdb Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 16 Oct 2014 13:28:00 -0400 Subject: [PATCH 1/4] Rename helper memor to indicate usage. Expected argument is a Property protobuf. --- gcloud/datastore/_helpers.py | 2 +- gcloud/datastore/entity.py | 2 +- gcloud/datastore/test__helpers.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gcloud/datastore/_helpers.py b/gcloud/datastore/_helpers.py index 792b3d494c42..4cfe43f079da 100644 --- a/gcloud/datastore/_helpers.py +++ b/gcloud/datastore/_helpers.py @@ -72,7 +72,7 @@ def _get_protobuf_attribute_and_value(val): return name + '_value', value -def _get_value_from_protobuf(pb): +def _get_value_from_property_pb(pb): """Given a protobuf for a Property, get the correct value. The Cloud Datastore Protobuf API returns a Property Protobuf diff --git a/gcloud/datastore/entity.py b/gcloud/datastore/entity.py index 034e11f10212..5101a637222a 100644 --- a/gcloud/datastore/entity.py +++ b/gcloud/datastore/entity.py @@ -157,7 +157,7 @@ def from_protobuf(cls, pb, dataset=None): # pylint: disable=invalid-name entity = cls.from_key(key) for property_pb in pb.property: - value = _helpers._get_value_from_protobuf(property_pb) + value = _helpers._get_value_from_property_pb(property_pb) entity[property_pb.name] = value return entity diff --git a/gcloud/datastore/test__helpers.py b/gcloud/datastore/test__helpers.py index a31711fd9661..eb9592a2c664 100644 --- a/gcloud/datastore/test__helpers.py +++ b/gcloud/datastore/test__helpers.py @@ -94,12 +94,12 @@ def test_object(self): self.assertRaises(ValueError, self._callFUT, object()) -class Test__get_value_from_protobuf(unittest2.TestCase): +class Test__get_value_from_property_pb(unittest2.TestCase): def _callFUT(self, pb): - from gcloud.datastore._helpers import _get_value_from_protobuf + from gcloud.datastore._helpers import _get_value_from_property_pb - return _get_value_from_protobuf(pb) + return _get_value_from_property_pb(pb) def _makePB(self, attr_name, value): from gcloud.datastore.datastore_v1_pb2 import Property From 0a2cd1363cb56967af6de8e1512036ae03d375cb Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 16 Oct 2014 13:37:24 -0400 Subject: [PATCH 2/4] Factor out value_pb bits from '_get_value_from_property_pb'. --- gcloud/datastore/_helpers.py | 54 ++++++++++++++++++++----------- gcloud/datastore/test__helpers.py | 43 ++++++++++++++++-------- 2 files changed, 65 insertions(+), 32 deletions(-) diff --git a/gcloud/datastore/_helpers.py b/gcloud/datastore/_helpers.py index 4cfe43f079da..600ca534b4b4 100644 --- a/gcloud/datastore/_helpers.py +++ b/gcloud/datastore/_helpers.py @@ -72,8 +72,8 @@ def _get_protobuf_attribute_and_value(val): return name + '_value', value -def _get_value_from_property_pb(pb): - """Given a protobuf for a Property, get the correct value. +def _get_value_from_value_pb(value_pb): + """Given a protobuf for a Value, get the correct value. The Cloud Datastore Protobuf API returns a Property Protobuf which has one value set and the rest blank. @@ -82,40 +82,58 @@ def _get_value_from_property_pb(pb): Some work is done to coerce the return value into a more useful type (particularly in the case of a timestamp value, or a key value). - :type pb: :class:`gcloud.datastore.datastore_v1_pb2.Property` - :param pb: The Property Protobuf. + :type value_pb: :class:`gcloud.datastore.datastore_v1_pb2.Value` + :param value_pb: The Value Protobuf. :returns: The value provided by the Protobuf. """ - if pb.value.HasField('timestamp_microseconds_value'): - microseconds = pb.value.timestamp_microseconds_value + if value_pb.HasField('timestamp_microseconds_value'): + microseconds = value_pb.timestamp_microseconds_value naive = (datetime.utcfromtimestamp(0) + timedelta(microseconds=microseconds)) return naive.replace(tzinfo=pytz.utc) - elif pb.value.HasField('key_value'): - return Key.from_protobuf(pb.value.key_value) + elif value_pb.HasField('key_value'): + return Key.from_protobuf(value_pb.key_value) - elif pb.value.HasField('boolean_value'): - return pb.value.boolean_value + elif value_pb.HasField('boolean_value'): + return value_pb.boolean_value - elif pb.value.HasField('double_value'): - return pb.value.double_value + elif value_pb.HasField('double_value'): + return value_pb.double_value - elif pb.value.HasField('integer_value'): - return pb.value.integer_value + elif value_pb.HasField('integer_value'): + return value_pb.integer_value - elif pb.value.HasField('string_value'): - return pb.value.string_value + elif value_pb.HasField('string_value'): + return value_pb.string_value - elif pb.value.HasField('entity_value'): - return Entity.from_protobuf(pb.value.entity_value) + elif value_pb.HasField('entity_value'): + return Entity.from_protobuf(value_pb.entity_value) else: return None +def _get_value_from_property_pb(property_pb): + """Given a protobuf for a Property, get the correct value. + + The Cloud Datastore Protobuf API returns a Property Protobuf + which has one value set and the rest blank. + This function retrieves the the one value provided. + + Some work is done to coerce the return value into a more useful type + (particularly in the case of a timestamp value, or a key value). + + :type property_pb: :class:`gcloud.datastore.datastore_v1_pb2.Property` + :param property_pb: The Property Protobuf. + + :returns: The value provided by the Protobuf. + """ + return _get_value_from_value_pb(property_pb.value) + + def _set_protobuf_value(value_pb, val): """Assign 'val' to the correct subfield of 'value_pb'. diff --git a/gcloud/datastore/test__helpers.py b/gcloud/datastore/test__helpers.py index eb9592a2c664..4a7e58c8627b 100644 --- a/gcloud/datastore/test__helpers.py +++ b/gcloud/datastore/test__helpers.py @@ -94,19 +94,19 @@ def test_object(self): self.assertRaises(ValueError, self._callFUT, object()) -class Test__get_value_from_property_pb(unittest2.TestCase): +class Test__get_value_from_value_pb(unittest2.TestCase): def _callFUT(self, pb): - from gcloud.datastore._helpers import _get_value_from_property_pb + from gcloud.datastore._helpers import _get_value_from_value_pb - return _get_value_from_property_pb(pb) + return _get_value_from_value_pb(pb) def _makePB(self, attr_name, value): - from gcloud.datastore.datastore_v1_pb2 import Property + from gcloud.datastore.datastore_v1_pb2 import Value - prop = Property() - setattr(prop.value, attr_name, value) - return prop + pb = Value() + setattr(pb, attr_name, value) + return pb def test_datetime(self): import calendar @@ -119,7 +119,7 @@ def test_datetime(self): self.assertEqual(self._callFUT(pb), utc) def test_key(self): - from gcloud.datastore.datastore_v1_pb2 import Property + from gcloud.datastore.datastore_v1_pb2 import Value from gcloud.datastore.dataset import Dataset from gcloud.datastore.key import Key @@ -127,9 +127,9 @@ def test_key(self): _KIND = 'KIND' _ID = 1234 _PATH = [{'kind': _KIND, 'id': _ID}] - pb = Property() + pb = Value() expected = Key(dataset=Dataset(_DATASET), path=_PATH).to_protobuf() - pb.value.key_value.CopyFrom(expected) + pb.key_value.CopyFrom(expected) found = self._callFUT(pb) self.assertEqual(found.to_protobuf(), expected) @@ -154,11 +154,11 @@ def test_unicode(self): self.assertEqual(self._callFUT(pb), u'str') def test_entity(self): - from gcloud.datastore.datastore_v1_pb2 import Property + from gcloud.datastore.datastore_v1_pb2 import Value from gcloud.datastore.entity import Entity - pb = Property() - entity_pb = pb.value.entity_value + pb = Value() + entity_pb = pb.entity_value prop_pb = entity_pb.property.add() prop_pb.name = 'foo' prop_pb.value.string_value = 'Foo' @@ -167,10 +167,25 @@ def test_entity(self): self.assertEqual(entity['foo'], 'Foo') def test_unknown(self): + from gcloud.datastore.datastore_v1_pb2 import Value + + pb = Value() + self.assertEqual(self._callFUT(pb), None) # XXX desirable? + + +class Test__get_value_from_property_pb(unittest2.TestCase): + + def _callFUT(self, pb): + from gcloud.datastore._helpers import _get_value_from_property_pb + + return _get_value_from_property_pb(pb) + + def test_it(self): from gcloud.datastore.datastore_v1_pb2 import Property pb = Property() - self.assertEqual(self._callFUT(pb), None) # XXX desirable? + pb.value.string_value = 'value' + self.assertEqual(self._callFUT(pb), 'value') class Test_set_protobuf_value(unittest2.TestCase): From 61e3c2c2856ee2601d218947b13547eeddeff61f Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 16 Oct 2014 13:44:17 -0400 Subject: [PATCH 3/4] Add support for 'list_value' field of a Value protobuf. Fixes #223. Manually lands Tagtoo@0475d5e, included by mistake in PR #126. --- gcloud/datastore/_helpers.py | 3 +++ gcloud/datastore/test__helpers.py | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/gcloud/datastore/_helpers.py b/gcloud/datastore/_helpers.py index 600ca534b4b4..706d24c3758c 100644 --- a/gcloud/datastore/_helpers.py +++ b/gcloud/datastore/_helpers.py @@ -112,6 +112,9 @@ def _get_value_from_value_pb(value_pb): elif value_pb.HasField('entity_value'): return Entity.from_protobuf(value_pb.entity_value) + elif value_pb.list_value: + return [_get_value_from_value_pb(x) for x in value_pb.list_value] + else: return None diff --git a/gcloud/datastore/test__helpers.py b/gcloud/datastore/test__helpers.py index 4a7e58c8627b..52538301ee18 100644 --- a/gcloud/datastore/test__helpers.py +++ b/gcloud/datastore/test__helpers.py @@ -166,6 +166,18 @@ def test_entity(self): self.assertTrue(isinstance(entity, Entity)) self.assertEqual(entity['foo'], 'Foo') + def test_list(self): + from gcloud.datastore.datastore_v1_pb2 import Value + + pb = Value() + list_pb = pb.list_value + item_pb = list_pb.add() + item_pb.string_value = 'Foo' + item_pb = list_pb.add() + item_pb.string_value = 'Bar' + items = self._callFUT(pb) + self.assertEqual(items, ['Foo', 'Bar']) + def test_unknown(self): from gcloud.datastore.datastore_v1_pb2 import Value From fc4353fb3a4bb64050365df02ddf03ff7f0a684d Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 16 Oct 2014 16:56:01 -0400 Subject: [PATCH 4/4] Kowtow to pylint's over-fussy 'too-many-return-statements'. Incorporates feedback from @dhermes. --- gcloud/datastore/_helpers.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/gcloud/datastore/_helpers.py b/gcloud/datastore/_helpers.py index 706d24c3758c..2b23d62484c3 100644 --- a/gcloud/datastore/_helpers.py +++ b/gcloud/datastore/_helpers.py @@ -88,35 +88,35 @@ def _get_value_from_value_pb(value_pb): :returns: The value provided by the Protobuf. """ + result = None if value_pb.HasField('timestamp_microseconds_value'): microseconds = value_pb.timestamp_microseconds_value naive = (datetime.utcfromtimestamp(0) + timedelta(microseconds=microseconds)) - return naive.replace(tzinfo=pytz.utc) + result = naive.replace(tzinfo=pytz.utc) elif value_pb.HasField('key_value'): - return Key.from_protobuf(value_pb.key_value) + result = Key.from_protobuf(value_pb.key_value) elif value_pb.HasField('boolean_value'): - return value_pb.boolean_value + result = value_pb.boolean_value elif value_pb.HasField('double_value'): - return value_pb.double_value + result = value_pb.double_value elif value_pb.HasField('integer_value'): - return value_pb.integer_value + result = value_pb.integer_value elif value_pb.HasField('string_value'): - return value_pb.string_value + result = value_pb.string_value elif value_pb.HasField('entity_value'): - return Entity.from_protobuf(value_pb.entity_value) + result = Entity.from_protobuf(value_pb.entity_value) elif value_pb.list_value: - return [_get_value_from_value_pb(x) for x in value_pb.list_value] + result = [_get_value_from_value_pb(x) for x in value_pb.list_value] - else: - return None + return result def _get_value_from_property_pb(property_pb):