diff --git a/google/cloud/datastore/helpers.py b/google/cloud/datastore/helpers.py index 5627d8a3..7222fbdf 100644 --- a/google/cloud/datastore/helpers.py +++ b/google/cloud/datastore/helpers.py @@ -22,6 +22,7 @@ from google.protobuf import struct_pb2 from google.type import latlng_pb2 +from proto.datetime_helpers import DatetimeWithNanoseconds from google.cloud._helpers import _datetime_to_pb_timestamp from google.cloud.datastore_v1.types import datastore as datastore_pb2 @@ -33,8 +34,8 @@ def _get_meaning(value_pb, is_list=False): """Get the meaning from a protobuf value. - :type value_pb: :class:`.entity_pb2.Value` - :param value_pb: The protobuf value to be checked for an + :type value_pb: :class:`.entity_pb2.Value._pb` + :param value_pb: The *raw* protobuf value to be checked for an associated meaning. :type is_list: bool @@ -47,14 +48,9 @@ def _get_meaning(value_pb, is_list=False): means it just returns a list of meanings. If all the list meanings agree, it just condenses them. """ - meaning = None if is_list: - values = ( - value_pb._pb.array_value.values - if hasattr(value_pb, "_pb") - else value_pb.array_value.values - ) + values = value_pb.array_value.values # An empty list will have no values, hence no shared meaning # set among them. @@ -65,16 +61,18 @@ def _get_meaning(value_pb, is_list=False): # the rest which may be enum/int values. all_meanings = [_get_meaning(sub_value_pb) for sub_value_pb in values] unique_meanings = set(all_meanings) + if len(unique_meanings) == 1: # If there is a unique meaning, we preserve it. - meaning = unique_meanings.pop() + return unique_meanings.pop() else: # We know len(value_pb.array_value.values) > 0. # If the meaning is not unique, just return all of them. - meaning = all_meanings + return all_meanings + elif value_pb.meaning: # Simple field (int32). - meaning = value_pb.meaning + return value_pb.meaning - return meaning + return None def _new_value_pb(entity_pb, name): @@ -89,29 +87,12 @@ def _new_value_pb(entity_pb, name): :rtype: :class:`.entity_pb2.Value` :returns: The new ``Value`` protobuf that was added to the entity. """ - properties = entity_pb.properties - try: - properties = properties._pb - except AttributeError: - # TODO(microgenerator): shouldn't need this. the issue is that - # we have wrapped and non-wrapped protos coming here. - pass + # TODO(microgenerator): shouldn't need this. the issue is that + # we have wrapped and non-wrapped protos coming here. + properties = getattr(entity_pb.properties, "_pb", entity_pb.properties) return properties.get_or_create(name) -def _property_tuples(entity_pb): - """Iterator of name, ``Value`` tuples from entity properties. - - :type entity_pb: :class:`.entity_pb2.Entity` - :param entity_pb: An entity protobuf to add a new property to. - - :rtype: :class:`generator` - :returns: An iterator that yields tuples of a name and ``Value`` - corresponding to properties on the entity. - """ - return iter(entity_pb.properties.items()) - - def entity_from_protobuf(pb): """Factory method for creating an entity based on a protobuf. @@ -124,21 +105,18 @@ def entity_from_protobuf(pb): :rtype: :class:`google.cloud.datastore.entity.Entity` :returns: The entity derived from the protobuf. """ - if not isinstance(pb, entity_pb2.Entity): - proto_pb = entity_pb2.Entity.wrap(pb) - else: - proto_pb = pb + if isinstance(pb, entity_pb2.Entity): pb = pb._pb key = None - if "key" in proto_pb: # Message field (Key) - key = key_from_protobuf(proto_pb.key) + if pb.HasField("key"): # Message field (Key) + key = key_from_protobuf(pb.key) entity_props = {} entity_meanings = {} exclude_from_indexes = [] - for prop_name, value_pb in _property_tuples(proto_pb._pb): + for prop_name, value_pb in pb.properties.items(): value = _get_value_from_value_pb(value_pb) entity_props[prop_name] = value @@ -384,7 +362,7 @@ def _pb_attr_value(val): return name + "_value", value -def _get_value_from_value_pb(value): +def _get_value_from_value_pb(pb): """Given a protobuf for a Value, get the correct value. The Cloud Datastore Protobuf API returns a Property Protobuf which @@ -394,56 +372,47 @@ def _get_value_from_value_pb(value): 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 value_pb: :class:`.entity_pb2.Value` - :param value_pb: The Value Protobuf. + :type pb: :class:`.entity_pb2.Value._pb` + :param pb: The *raw* Value Protobuf. :rtype: object :returns: The value provided by the Protobuf. :raises: :class:`ValueError ` if no value type has been set. """ - if not getattr(value, "_pb", False): - # Coerce raw pb type into proto-plus pythonic type. - value = entity_pb2.Value.wrap(value) - - value_type = value._pb.WhichOneof("value_type") + value_type = pb.WhichOneof("value_type") if value_type == "timestamp_value": - # Do not access `._pb` here, as that returns a Timestamp proto, - # but this should return a Pythonic `DatetimeWithNanoseconds` value, - # which is found at `value.timestamp_value` - result = value.timestamp_value + result = DatetimeWithNanoseconds.from_timestamp_pb(pb.timestamp_value) elif value_type == "key_value": - result = key_from_protobuf(value._pb.key_value) + result = key_from_protobuf(pb.key_value) elif value_type == "boolean_value": - result = value._pb.boolean_value + result = pb.boolean_value elif value_type == "double_value": - result = value._pb.double_value + result = pb.double_value elif value_type == "integer_value": - result = value._pb.integer_value + result = pb.integer_value elif value_type == "string_value": - result = value._pb.string_value + result = pb.string_value elif value_type == "blob_value": - result = value._pb.blob_value + result = pb.blob_value elif value_type == "entity_value": - result = entity_from_protobuf(value._pb.entity_value) + result = entity_from_protobuf(pb.entity_value) elif value_type == "array_value": result = [ - _get_value_from_value_pb(value) for value in value._pb.array_value.values + _get_value_from_value_pb(item_value) for item_value in pb.array_value.values ] elif value_type == "geo_point_value": - result = GeoPoint( - value._pb.geo_point_value.latitude, value._pb.geo_point_value.longitude, - ) + result = GeoPoint(pb.geo_point_value.latitude, pb.geo_point_value.longitude,) elif value_type == "null_value": result = None diff --git a/tests/unit/test_batch.py b/tests/unit/test_batch.py index 78c1db20..ead00623 100644 --- a/tests/unit/test_batch.py +++ b/tests/unit/test_batch.py @@ -118,8 +118,6 @@ def test_put_entity_w_partial_key(self): self.assertEqual(batch._partial_key_entities, [entity]) def test_put_entity_w_completed_key(self): - from google.cloud.datastore.helpers import _property_tuples - project = "PROJECT" properties = {"foo": "bar", "baz": "qux", "spam": [1, 2, 3], "frotz": []} client = _Client(project) @@ -134,7 +132,7 @@ def test_put_entity_w_completed_key(self): mutated_entity = _mutated_pb(self, batch.mutations, "upsert") self.assertEqual(mutated_entity.key, key._key) - prop_dict = dict(_property_tuples(mutated_entity)) + prop_dict = dict(mutated_entity.properties.items()) self.assertEqual(len(prop_dict), 4) self.assertFalse(prop_dict["foo"].exclude_from_indexes) self.assertTrue(prop_dict["baz"].exclude_from_indexes) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 3c75a5fb..5127fd60 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -802,7 +802,6 @@ def test_put_multi_w_single_empty_entity(self): def test_put_multi_no_batch_w_partial_key_w_retry_w_timeout(self): from google.cloud.datastore_v1.types import datastore as datastore_pb2 - from google.cloud.datastore.helpers import _property_tuples entity = _Entity(foo=u"bar") key = entity.key = _Key(_Key.kind, None) @@ -838,15 +837,13 @@ def test_put_multi_no_batch_w_partial_key_w_retry_w_timeout(self): mutated_entity = _mutated_pb(self, mutations, "insert") self.assertEqual(mutated_entity.key, key.to_protobuf()) - prop_list = list(_property_tuples(mutated_entity)) + prop_list = list(mutated_entity.properties.items()) self.assertTrue(len(prop_list), 1) name, value_pb = prop_list[0] self.assertEqual(name, "foo") self.assertEqual(value_pb.string_value, u"bar") def test_put_multi_existing_batch_w_completed_key(self): - from google.cloud.datastore.helpers import _property_tuples - creds = _make_credentials() client = self._make_one(credentials=creds) entity = _Entity(foo=u"bar") @@ -859,7 +856,7 @@ def test_put_multi_existing_batch_w_completed_key(self): mutated_entity = _mutated_pb(self, CURR_BATCH.mutations, "upsert") self.assertEqual(mutated_entity.key, key.to_protobuf()) - prop_list = list(_property_tuples(mutated_entity)) + prop_list = list(mutated_entity.properties.items()) self.assertTrue(len(prop_list), 1) name, value_pb = prop_list[0] self.assertEqual(name, "foo") diff --git a/tests/unit/test_helpers.py b/tests/unit/test_helpers.py index 5b602cff..c37499ca 100644 --- a/tests/unit/test_helpers.py +++ b/tests/unit/test_helpers.py @@ -33,28 +33,6 @@ def test_it(self): self.assertEqual(entity_pb._pb.properties[name], result) -class Test__property_tuples(unittest.TestCase): - def _call_fut(self, entity_pb): - from google.cloud.datastore.helpers import _property_tuples - - return _property_tuples(entity_pb) - - def test_it(self): - import types - from google.cloud.datastore_v1.types import entity as entity_pb2 - from google.cloud.datastore.helpers import _new_value_pb - - entity_pb = entity_pb2.Entity() - name1 = "foo" - name2 = "bar" - val_pb1 = _new_value_pb(entity_pb, name1) - val_pb2 = _new_value_pb(entity_pb, name2) - - result = self._call_fut(entity_pb) - self.assertIsInstance(result, types.GeneratorType) - self.assertEqual(sorted(result), sorted([(name1, val_pb1), (name2, val_pb2)])) - - class Test_entity_from_protobuf(unittest.TestCase): def _call_fut(self, val): from google.cloud.datastore.helpers import entity_from_protobuf @@ -221,11 +199,9 @@ def _call_fut(self, entity): return entity_to_protobuf(entity) def _compare_entity_proto(self, entity_pb1, entity_pb2): - from google.cloud.datastore.helpers import _property_tuples - self.assertEqual(entity_pb1.key, entity_pb2.key) - value_list1 = sorted(_property_tuples(entity_pb1)) - value_list2 = sorted(_property_tuples(entity_pb2)) + value_list1 = sorted(entity_pb1.properties.items()) + value_list2 = sorted(entity_pb2.properties.items()) self.assertEqual(len(value_list1), len(value_list2)) for pair1, pair2 in zip(value_list1, value_list2): name1, val1 = pair1 @@ -668,12 +644,12 @@ def _call_fut(self, pb): return _get_value_from_value_pb(pb) - def _makePB(self, attr_name, value): + def _makePB(self, attr_name, attr_value): from google.cloud.datastore_v1.types import entity as entity_pb2 - pb = entity_pb2.Value() - setattr(pb, attr_name, value) - return pb + value = entity_pb2.Value() + setattr(value._pb, attr_name, attr_value) + return value def test_datetime(self): import calendar @@ -683,67 +659,67 @@ def test_datetime(self): micros = 4375 utc = datetime.datetime(2014, 9, 16, 10, 19, 32, micros, UTC) - pb = entity_pb2.Value() - pb._pb.timestamp_value.seconds = calendar.timegm(utc.timetuple()) - pb._pb.timestamp_value.nanos = 1000 * micros - self.assertEqual(self._call_fut(pb), utc) + value = entity_pb2.Value() + value._pb.timestamp_value.seconds = calendar.timegm(utc.timetuple()) + value._pb.timestamp_value.nanos = 1000 * micros + self.assertEqual(self._call_fut(value._pb), utc) def test_key(self): from google.cloud.datastore_v1.types import entity as entity_pb2 from google.cloud.datastore.key import Key - pb = entity_pb2.Value() + value = entity_pb2.Value() expected = Key("KIND", 1234, project="PROJECT").to_protobuf() - pb.key_value._pb.CopyFrom(expected._pb) - found = self._call_fut(pb) + value.key_value._pb.CopyFrom(expected._pb) + found = self._call_fut(value._pb) self.assertEqual(found.to_protobuf(), expected) def test_bool(self): - pb = self._makePB("boolean_value", False) - self.assertEqual(self._call_fut(pb), False) + value = self._makePB("boolean_value", False) + self.assertEqual(self._call_fut(value._pb), False) def test_float(self): - pb = self._makePB("double_value", 3.1415926) - self.assertEqual(self._call_fut(pb), 3.1415926) + value = self._makePB("double_value", 3.1415926) + self.assertEqual(self._call_fut(value._pb), 3.1415926) def test_int(self): - pb = self._makePB("integer_value", 42) - self.assertEqual(self._call_fut(pb), 42) + value = self._makePB("integer_value", 42) + self.assertEqual(self._call_fut(value._pb), 42) def test_bytes(self): - pb = self._makePB("blob_value", b"str") - self.assertEqual(self._call_fut(pb), b"str") + value = self._makePB("blob_value", b"str") + self.assertEqual(self._call_fut(value._pb), b"str") def test_unicode(self): - pb = self._makePB("string_value", u"str") - self.assertEqual(self._call_fut(pb), u"str") + value = self._makePB("string_value", u"str") + self.assertEqual(self._call_fut(value._pb), u"str") def test_entity(self): from google.cloud.datastore_v1.types import entity as entity_pb2 from google.cloud.datastore.entity import Entity from google.cloud.datastore.helpers import _new_value_pb - pb = entity_pb2.Value() - entity_pb = pb.entity_value + value = entity_pb2.Value() + entity_pb = value.entity_value entity_pb._pb.key.path.add(kind="KIND") entity_pb.key.partition_id.project_id = "PROJECT" value_pb = _new_value_pb(entity_pb, "foo") value_pb.string_value = "Foo" - entity = self._call_fut(pb) + entity = self._call_fut(value._pb) self.assertIsInstance(entity, Entity) self.assertEqual(entity["foo"], "Foo") def test_array(self): from google.cloud.datastore_v1.types import entity as entity_pb2 - pb = entity_pb2.Value() - array_pb = pb.array_value.values + value = entity_pb2.Value() + array_pb = value.array_value.values item_pb = array_pb._pb.add() item_pb.string_value = "Foo" item_pb = array_pb._pb.add() item_pb.string_value = "Bar" - items = self._call_fut(pb) + items = self._call_fut(value._pb) self.assertEqual(items, ["Foo", "Bar"]) def test_geo_point(self): @@ -754,8 +730,8 @@ def test_geo_point(self): lat = -3.14 lng = 13.37 geo_pt_pb = latlng_pb2.LatLng(latitude=lat, longitude=lng) - pb = entity_pb2.Value(geo_point_value=geo_pt_pb) - result = self._call_fut(pb) + value = entity_pb2.Value(geo_point_value=geo_pt_pb) + result = self._call_fut(value._pb) self.assertIsInstance(result, GeoPoint) self.assertEqual(result.latitude, lat) self.assertEqual(result.longitude, lng) @@ -764,16 +740,16 @@ def test_null(self): from google.protobuf import struct_pb2 from google.cloud.datastore_v1.types import entity as entity_pb2 - pb = entity_pb2.Value(null_value=struct_pb2.NULL_VALUE) - result = self._call_fut(pb) + value = entity_pb2.Value(null_value=struct_pb2.NULL_VALUE) + result = self._call_fut(value._pb) self.assertIsNone(result) def test_unknown(self): from google.cloud.datastore_v1.types import entity as entity_pb2 - pb = entity_pb2.Value() + value = entity_pb2.Value() with self.assertRaises(ValueError): - self._call_fut(pb) + self._call_fut(value._pb) class Test_set_protobuf_value(unittest.TestCase): @@ -860,18 +836,16 @@ def test_unicode(self): def test_entity_empty_wo_key(self): from google.cloud.datastore.entity import Entity - from google.cloud.datastore.helpers import _property_tuples pb = self._makePB() entity = Entity() self._call_fut(pb, entity) value = pb.entity_value self.assertEqual(value.key.SerializeToString(), b"") - self.assertEqual(len(list(_property_tuples(value))), 0) + self.assertEqual(len(list(value.properties.items())), 0) def test_entity_w_key(self): from google.cloud.datastore.entity import Entity - from google.cloud.datastore.helpers import _property_tuples from google.cloud.datastore.key import Key name = "foo" @@ -884,7 +858,7 @@ def test_entity_w_key(self): entity_pb = pb.entity_value self.assertEqual(entity_pb.key, key.to_protobuf()._pb) - prop_dict = dict(_property_tuples(entity_pb)) + prop_dict = dict(entity_pb.properties.items()) self.assertEqual(len(prop_dict), 1) self.assertEqual(list(prop_dict.keys()), [name]) self.assertEqual(prop_dict[name].string_value, value) diff --git a/tests/unit/test_query.py b/tests/unit/test_query.py index 89bc7e2c..dcb4e9f5 100644 --- a/tests/unit/test_query.py +++ b/tests/unit/test_query.py @@ -15,6 +15,7 @@ import unittest import mock +import pytest class TestQuery(unittest.TestCase): @@ -527,6 +528,7 @@ def test__process_query_results_done(self): self.assertIsNone(iterator.next_page_token) self.assertFalse(iterator._more_results) + @pytest.mark.filterwarnings("ignore") def test__process_query_results_bad_enum(self): iterator = self._make_one(None, None) more_results_enum = 999