From 53c06dc4ec9ab386c7aebf97865a4f9c5d144b75 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 23 Jan 2018 14:24:52 -0800 Subject: [PATCH 1/2] Datastore: Allow empty lists in protobuf array value --- datastore/google/cloud/datastore/helpers.py | 20 +++++++++++--------- datastore/tests/system/test_system.py | 15 +++++++++++++++ datastore/tests/unit/test_helpers.py | 20 ++++++++++++++++++++ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/datastore/google/cloud/datastore/helpers.py b/datastore/google/cloud/datastore/helpers.py index f3838668fe3d..9a67d9fdb31c 100644 --- a/datastore/google/cloud/datastore/helpers.py +++ b/datastore/google/cloud/datastore/helpers.py @@ -135,15 +135,17 @@ def entity_from_protobuf(pb): # special-cased and we require all ``exclude_from_indexes`` values # in a list agree. if is_list: - exclude_values = set(value_pb.exclude_from_indexes - for value_pb in value_pb.array_value.values) - if len(exclude_values) != 1: - raise ValueError('For an array_value, subvalues must either ' - 'all be indexed or all excluded from ' - 'indexes.') - - if exclude_values.pop(): - exclude_from_indexes.append(prop_name) + if len(value) > 0: + exclude_values = set(value_pb.exclude_from_indexes + for value_pb + in value_pb.array_value.values) + if len(exclude_values) != 1: + raise ValueError('For an array_value, subvalues must ' + 'either all be indexed or all excluded ' + 'from indexes.') + + if exclude_values.pop(): + exclude_from_indexes.append(prop_name) else: if value_pb.exclude_from_indexes: exclude_from_indexes.append(prop_name) diff --git a/datastore/tests/system/test_system.py b/datastore/tests/system/test_system.py index 3ab7295f50c4..693b92b3d16c 100644 --- a/datastore/tests/system/test_system.py +++ b/datastore/tests/system/test_system.py @@ -425,6 +425,21 @@ def test_query_distinct_on(self): self.assertEqual(entities[0]['name'], 'Catelyn') self.assertEqual(entities[1]['name'], 'Arya') + def test_empty_array_value(self): + from google.cloud import datastore + from google.cloud.datastore_v1.proto import datastore_pb2 + from google.cloud.datastore_v1.proto import entity_pb2 + client = self.CLIENT + key = client.key('Foo', 'Bar') + entity = datastore.Entity(key=key) + with client.batch() as batch: + batch.put(entity) + mutations = batch.mutations + mutation = mutations[0] + value_pb = mutation.upsert.properties.get_or_create('name-name') + value_pb.array_value.CopyFrom(entity_pb2.ArrayValue(values=[])) + client.get(key) + class TestDatastoreTransaction(TestDatastore): diff --git a/datastore/tests/unit/test_helpers.py b/datastore/tests/unit/test_helpers.py index 3624665a2a05..daeaec1cfcdf 100644 --- a/datastore/tests/unit/test_helpers.py +++ b/datastore/tests/unit/test_helpers.py @@ -135,6 +135,26 @@ def test_mismatched_value_indexed(self): with self.assertRaises(ValueError): self._call_fut(entity_pb) + def test_index_mismatch_ignores_empty_list(self): + from google.cloud.datastore_v1.proto import entity_pb2 + from google.cloud.datastore.helpers import _new_value_pb + + _PROJECT = 'PROJECT' + _KIND = 'KIND' + _ID = 1234 + entity_pb = entity_pb2.Entity() + entity_pb.key.partition_id.project_id = _PROJECT + entity_pb.key.path.add(kind=_KIND, id=_ID) + + array_val_pb = _new_value_pb(entity_pb, 'baz') + array_pb = array_val_pb.array_value.values + array_val_pb.array_value.CopyFrom(entity_pb2.ArrayValue(values=[])) + + unindexed_value_pb1 = array_pb.add() + entity = self._call_fut(entity_pb) + entity_dict = dict(entity) + self.assertEqual(entity_dict['baz'], []) + def test_entity_no_key(self): from google.cloud.datastore_v1.proto import entity_pb2 From d6c6b9be73b72f9d7cddf46d1d2e71fd9f236b58 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 23 Jan 2018 16:02:23 -0800 Subject: [PATCH 2/2] Review Changes --- datastore/google/cloud/datastore/helpers.py | 22 ++++++++++----------- datastore/tests/unit/test_helpers.py | 4 +--- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/datastore/google/cloud/datastore/helpers.py b/datastore/google/cloud/datastore/helpers.py index 9a67d9fdb31c..964eb2a37204 100644 --- a/datastore/google/cloud/datastore/helpers.py +++ b/datastore/google/cloud/datastore/helpers.py @@ -134,18 +134,16 @@ def entity_from_protobuf(pb): # Check if ``value_pb`` was excluded from index. Lists need to be # special-cased and we require all ``exclude_from_indexes`` values # in a list agree. - if is_list: - if len(value) > 0: - exclude_values = set(value_pb.exclude_from_indexes - for value_pb - in value_pb.array_value.values) - if len(exclude_values) != 1: - raise ValueError('For an array_value, subvalues must ' - 'either all be indexed or all excluded ' - 'from indexes.') - - if exclude_values.pop(): - exclude_from_indexes.append(prop_name) + if is_list and len(value) > 0: + exclude_values = set(value_pb.exclude_from_indexes + for value_pb in value_pb.array_value.values) + if len(exclude_values) != 1: + raise ValueError('For an array_value, subvalues must either ' + 'all be indexed or all excluded from ' + 'indexes.') + + if exclude_values.pop(): + exclude_from_indexes.append(prop_name) else: if value_pb.exclude_from_indexes: exclude_from_indexes.append(prop_name) diff --git a/datastore/tests/unit/test_helpers.py b/datastore/tests/unit/test_helpers.py index daeaec1cfcdf..4fc4c179dbcc 100644 --- a/datastore/tests/unit/test_helpers.py +++ b/datastore/tests/unit/test_helpers.py @@ -147,10 +147,8 @@ def test_index_mismatch_ignores_empty_list(self): entity_pb.key.path.add(kind=_KIND, id=_ID) array_val_pb = _new_value_pb(entity_pb, 'baz') - array_pb = array_val_pb.array_value.values array_val_pb.array_value.CopyFrom(entity_pb2.ArrayValue(values=[])) - - unindexed_value_pb1 = array_pb.add() + entity = self._call_fut(entity_pb) entity_dict = dict(entity) self.assertEqual(entity_dict['baz'], [])